spapr: Cleanups to PAPR mode setup

[Qemu-devel] [PATCH for-2.13 09/10] target/ppc: Don't bother with MSR_EP in cpu_ppc_set_papr()

Posted by David Gibson 7 years, 9 months ago

cpu_ppc_set_papr() removes the EP and HV bits from the MSR mask.  While
removing the HV bit makes sense (a cpu in PAPR mode should never be
emulated in hypervisor mode), the EP bit is just bizarre.  Although it's
true that a papr mode guest shouldn't be able to change the exception
prefix, the MSR[EP] bit doesn't even exist on the cpus supported for PAPR
mode, so it's pointless to do anything with it here.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target/ppc/translate_init.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 5e89901149..bb5559d799 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8870,12 +8870,11 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
 
     cpu->vhyp = vhyp;
 
-    /* PAPR always has exception vectors in RAM not ROM. To ensure this,
-     * MSR[IP] should never be set.
-     *
-     * We also disallow setting of MSR_HV
+    /*
+     * With a virtual hypervisor mode we never allow the CPU to go
+     * hypervisor mode itself
      */
-    env->msr_mask &= ~((1ull << MSR_EP) | MSR_HVB);
+    env->msr_mask &= ~MSR_HVB;
 
     /* Tell KVM that we're in PAPR mode */
     if (kvm_enabled()) {
-- 
2.14.3

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-2.13 09/10] target/ppc: Don't bother with MSR_EP in cpu_ppc_set_papr()

Posted by Thomas Huth 7 years, 9 months ago

On 17.04.2018 09:17, David Gibson wrote:
> cpu_ppc_set_papr() removes the EP and HV bits from the MSR mask.  While
> removing the HV bit makes sense (a cpu in PAPR mode should never be
> emulated in hypervisor mode), the EP bit is just bizarre.  Although it's
> true that a papr mode guest shouldn't be able to change the exception
> prefix, the MSR[EP] bit doesn't even exist on the cpus supported for PAPR
> mode, so it's pointless to do anything with it here.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  target/ppc/translate_init.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 5e89901149..bb5559d799 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8870,12 +8870,11 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
>  
>      cpu->vhyp = vhyp;
>  
> -    /* PAPR always has exception vectors in RAM not ROM. To ensure this,
> -     * MSR[IP] should never be set.
> -     *
> -     * We also disallow setting of MSR_HV
> +    /*
> +     * With a virtual hypervisor mode we never allow the CPU to go
> +     * hypervisor mode itself
>       */
> -    env->msr_mask &= ~((1ull << MSR_EP) | MSR_HVB);
> +    env->msr_mask &= ~MSR_HVB;
>  
>      /* Tell KVM that we're in PAPR mode */
>      if (kvm_enabled()) {

Looks right.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [Qemu-ppc] [PATCH for-2.13 09/10] target/ppc: Don't bother with MSR_EP in cpu_ppc_set_papr()

Posted by David Gibson 7 years, 9 months ago

On Fri, Apr 20, 2018 at 08:08:59AM +0200, Thomas Huth wrote:
> On 17.04.2018 09:17, David Gibson wrote:
> > cpu_ppc_set_papr() removes the EP and HV bits from the MSR mask.  While
> > removing the HV bit makes sense (a cpu in PAPR mode should never be
> > emulated in hypervisor mode), the EP bit is just bizarre.  Although it's
> > true that a papr mode guest shouldn't be able to change the exception
> > prefix, the MSR[EP] bit doesn't even exist on the cpus supported for PAPR
> > mode, so it's pointless to do anything with it here.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  target/ppc/translate_init.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index 5e89901149..bb5559d799 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -8870,12 +8870,11 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
> >  
> >      cpu->vhyp = vhyp;
> >  
> > -    /* PAPR always has exception vectors in RAM not ROM. To ensure this,
> > -     * MSR[IP] should never be set.
> > -     *
> > -     * We also disallow setting of MSR_HV
> > +    /*
> > +     * With a virtual hypervisor mode we never allow the CPU to go
> > +     * hypervisor mode itself
> >       */
> > -    env->msr_mask &= ~((1ull << MSR_EP) | MSR_HVB);
> > +    env->msr_mask &= ~MSR_HVB;
> >  
> >      /* Tell KVM that we're in PAPR mode */
> >      if (kvm_enabled()) {
> 
> Looks right.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Turns out this one is pretty much independent of the rest of the
series, so I've merged it to ppc-for-2.13 already.


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson