| Message ID | 20161020065912.16132-3-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <clg@kaod.org> wrote: > On 10/20/2016 08:59 AM, Nicholas Piggin wrote: > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > target-ppc/excp_helper.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > index 53c4075..477af10 100644 > > --- a/target-ppc/excp_helper.c > > +++ b/target-ppc/excp_helper.c > > @@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > > /* indicate that we resumed from power save mode */ > > msr |= 0x10000; > > new_msr |= ((target_ulong)1 << MSR_ME); > > + new_msr |= (target_ulong)MSR_HVB; > > + } else { > > + /* The ISA specifies the HV bit is set when the hardware interrupt > > + * is raised, however when hypervisors deliver the exception to > > + * guests, it should not be set. > > + */ > > } > > - > > - new_msr |= (target_ulong)MSR_HVB; > > ail = 0; > > break; > > case POWERPC_EXCP_DSEG: /* Data segment exception */ > > > > should not that be cleared later on in powerpc_excp() by : > > env->msr = new_msr & env->msr_mask; > > ? but the routine is rather long so I might be missing a branch. No you're right, so it can't leak into the guest, phew! The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry. Perhaps a cleaner way is for the system reset case to set new_msr according to the ISA, and then apply the msr_mask (or at least mask out HV) before calculating the exception model? Any preference? Thanks, Nick
On Fri, Oct 21, 2016 at 12:40:58AM +1100, Nicholas Piggin wrote: > On Thu, 20 Oct 2016 15:08:07 +0200 > Cédric Le Goater <clg@kaod.org> wrote: > > > On 10/20/2016 08:59 AM, Nicholas Piggin wrote: > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > > --- > > > target-ppc/excp_helper.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > > index 53c4075..477af10 100644 > > > --- a/target-ppc/excp_helper.c > > > +++ b/target-ppc/excp_helper.c > > > @@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > > > /* indicate that we resumed from power save mode */ > > > msr |= 0x10000; > > > new_msr |= ((target_ulong)1 << MSR_ME); > > > + new_msr |= (target_ulong)MSR_HVB; > > > + } else { > > > + /* The ISA specifies the HV bit is set when the hardware interrupt > > > + * is raised, however when hypervisors deliver the exception to > > > + * guests, it should not be set. > > > + */ > > > } > > > - > > > - new_msr |= (target_ulong)MSR_HVB; > > > ail = 0; > > > break; > > > case POWERPC_EXCP_DSEG: /* Data segment exception */ > > > > > > > should not that be cleared later on in powerpc_excp() by : > > > > env->msr = new_msr & env->msr_mask; > > > > ? but the routine is rather long so I might be missing a branch. > > No you're right, so it can't leak into the guest, phew! > > The problem I get is the interrupt code doing some things differently > depending on on the HV bit. For example what I noticed is the guest > losing its LE bit upon entry. > > Perhaps a cleaner way is for the system reset case to set new_msr > according to the ISA, and then apply the msr_mask (or at least mask > out HV) before calculating the exception model? Any preference? I think the proposed revision makes sense.
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c index 53c4075..477af10 100644 --- a/target-ppc/excp_helper.c +++ b/target-ppc/excp_helper.c @@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) /* indicate that we resumed from power save mode */ msr |= 0x10000; new_msr |= ((target_ulong)1 << MSR_ME); + new_msr |= (target_ulong)MSR_HVB; + } else { + /* The ISA specifies the HV bit is set when the hardware interrupt + * is raised, however when hypervisors deliver the exception to + * guests, it should not be set. + */ } - - new_msr |= (target_ulong)MSR_HVB; ail = 0; break; case POWERPC_EXCP_DSEG: /* Data segment exception */
Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)