[v6,06/18] spapr, ppc: Remove VPM0/RMLS hacks for POWER9
diff mbox series

Message ID 20200224233724.46415-7-david@gibson.dropbear.id.au
State New
Headers show
Series
  • [v6,01/18] pseries: Update SLOF firmware image
Related show

Commit Message

David Gibson Feb. 24, 2020, 11:37 p.m. UTC
For the "pseries" machine, we use "virtual hypervisor" mode where we
only model the CPU in non-hypervisor privileged mode.  This means that
we need guest physical addresses within the modelled cpu to be treated
as absolute physical addresses.

We used to do that by clearing LPCR[VPM0] and setting LPCR[RMLS] to a high
limit so that the old offset based translation for guest mode applied,
which does what we need.  However, POWER9 has removed support for that
translation mode, which meant we had some ugly hacks to keep it working.

We now explicitly handle this sort of translation for virtual hypervisor
mode, so the hacks aren't necessary.  We don't need to set VPM0 and RMLS
from the machine type code - they're now ignored in vhyp mode.  On the cpu
side we don't need to allow LPCR[RMLS] to be set on POWER9 in vhyp mode -
that was only there to allow the hack on the machine side.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_cpu_core.c | 6 +-----
 target/ppc/mmu-hash64.c | 8 --------
 2 files changed, 1 insertion(+), 13 deletions(-)

Comments

Greg Kurz Feb. 25, 2020, 11:29 a.m. UTC | #1
On Tue, 25 Feb 2020 10:37:12 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> For the "pseries" machine, we use "virtual hypervisor" mode where we
> only model the CPU in non-hypervisor privileged mode.  This means that
> we need guest physical addresses within the modelled cpu to be treated
> as absolute physical addresses.
> 
> We used to do that by clearing LPCR[VPM0] and setting LPCR[RMLS] to a high
> limit so that the old offset based translation for guest mode applied,
> which does what we need.  However, POWER9 has removed support for that
> translation mode, which meant we had some ugly hacks to keep it working.
> 
> We now explicitly handle this sort of translation for virtual hypervisor
> mode, so the hacks aren't necessary.  We don't need to set VPM0 and RMLS
> from the machine type code - they're now ignored in vhyp mode.  On the cpu
> side we don't need to allow LPCR[RMLS] to be set on POWER9 in vhyp mode -
> that was only there to allow the hack on the machine side.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_cpu_core.c | 6 +-----
>  target/ppc/mmu-hash64.c | 8 --------
>  2 files changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index d09125d9af..ea5e11f1d9 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -58,14 +58,10 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>       * we don't get spurious wakups before an RTAS start-cpu call.
>       * For the same reason, set PSSCR_EC.
>       */
> -    lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
> +    lpcr &= ~(LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
>      lpcr |= LPCR_LPES0 | LPCR_LPES1;
>      env->spr[SPR_PSSCR] |= PSSCR_EC;
>  
> -    /* Set RMLS to the max (ie, 16G) */
> -    lpcr &= ~LPCR_RMLS;
> -    lpcr |= 1ull << LPCR_RMLS_SHIFT;
> -
>      ppc_store_lpcr(cpu, lpcr);
>  
>      /* Set a full AMOR so guest can use the AMR as it sees fit */
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index e372c42add..caf47ad6fc 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -1126,14 +1126,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
>                        (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
>                        LPCR_DEE | LPCR_OEE)) | LPCR_MER | LPCR_GTSE | LPCR_TC |
>                        LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE);
> -        /*
> -         * If we have a virtual hypervisor, we need to bring back RMLS. It
> -         * doesn't exist on an actual P9 but that's all we know how to
> -         * configure with softmmu at the moment
> -         */
> -        if (cpu->vhyp) {
> -            lpcr |= (val & LPCR_RMLS);
> -        }
>          break;
>      default:
>          g_assert_not_reached();
Greg Kurz Feb. 25, 2020, 3:58 p.m. UTC | #2
On Tue, 25 Feb 2020 12:29:00 +0100
Greg Kurz <groug@kaod.org> wrote:

> On Tue, 25 Feb 2020 10:37:12 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > For the "pseries" machine, we use "virtual hypervisor" mode where we
> > only model the CPU in non-hypervisor privileged mode.  This means that
> > we need guest physical addresses within the modelled cpu to be treated
> > as absolute physical addresses.
> > 
> > We used to do that by clearing LPCR[VPM0] and setting LPCR[RMLS] to a high
> > limit so that the old offset based translation for guest mode applied,
> > which does what we need.  However, POWER9 has removed support for that
> > translation mode, which meant we had some ugly hacks to keep it working.
> > 
> > We now explicitly handle this sort of translation for virtual hypervisor
> > mode, so the hacks aren't necessary.  We don't need to set VPM0 and RMLS
> > from the machine type code - they're now ignored in vhyp mode.  On the cpu
> > side we don't need to allow LPCR[RMLS] to be set on POWER9 in vhyp mode -
> > that was only there to allow the hack on the machine side.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 

Ah wait...

> >  hw/ppc/spapr_cpu_core.c | 6 +-----
> >  target/ppc/mmu-hash64.c | 8 --------
> >  2 files changed, 1 insertion(+), 13 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index d09125d9af..ea5e11f1d9 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -58,14 +58,10 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
> >       * we don't get spurious wakups before an RTAS start-cpu call.
> >       * For the same reason, set PSSCR_EC.
> >       */
> > -    lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
> > +    lpcr &= ~(LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);

... a few lines above, we have a comment that should be dropped as well.

     * Clearing VPM0 will also cause us to use RMOR in mmu-hash64.c for
     * real mode accesses, which thankfully defaults to 0 and isn't
     * accessible in guest mode.

My R-b tag stands anyway.

> >      lpcr |= LPCR_LPES0 | LPCR_LPES1;
> >      env->spr[SPR_PSSCR] |= PSSCR_EC;
> >  
> > -    /* Set RMLS to the max (ie, 16G) */
> > -    lpcr &= ~LPCR_RMLS;
> > -    lpcr |= 1ull << LPCR_RMLS_SHIFT;
> > -
> >      ppc_store_lpcr(cpu, lpcr);
> >  
> >      /* Set a full AMOR so guest can use the AMR as it sees fit */
> > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > index e372c42add..caf47ad6fc 100644
> > --- a/target/ppc/mmu-hash64.c
> > +++ b/target/ppc/mmu-hash64.c
> > @@ -1126,14 +1126,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> >                        (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> >                        LPCR_DEE | LPCR_OEE)) | LPCR_MER | LPCR_GTSE | LPCR_TC |
> >                        LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE);
> > -        /*
> > -         * If we have a virtual hypervisor, we need to bring back RMLS. It
> > -         * doesn't exist on an actual P9 but that's all we know how to
> > -         * configure with softmmu at the moment
> > -         */
> > -        if (cpu->vhyp) {
> > -            lpcr |= (val & LPCR_RMLS);
> > -        }
> >          break;
> >      default:
> >          g_assert_not_reached();
> 
>
David Gibson Feb. 26, 2020, 1 a.m. UTC | #3
On Tue, Feb 25, 2020 at 04:58:01PM +0100, Greg Kurz wrote:
> On Tue, 25 Feb 2020 12:29:00 +0100
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Tue, 25 Feb 2020 10:37:12 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > For the "pseries" machine, we use "virtual hypervisor" mode where we
> > > only model the CPU in non-hypervisor privileged mode.  This means that
> > > we need guest physical addresses within the modelled cpu to be treated
> > > as absolute physical addresses.
> > > 
> > > We used to do that by clearing LPCR[VPM0] and setting LPCR[RMLS] to a high
> > > limit so that the old offset based translation for guest mode applied,
> > > which does what we need.  However, POWER9 has removed support for that
> > > translation mode, which meant we had some ugly hacks to keep it working.
> > > 
> > > We now explicitly handle this sort of translation for virtual hypervisor
> > > mode, so the hacks aren't necessary.  We don't need to set VPM0 and RMLS
> > > from the machine type code - they're now ignored in vhyp mode.  On the cpu
> > > side we don't need to allow LPCR[RMLS] to be set on POWER9 in vhyp mode -
> > > that was only there to allow the hack on the machine side.
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > > ---
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> > 
> 
> Ah wait...
> 
> > >  hw/ppc/spapr_cpu_core.c | 6 +-----
> > >  target/ppc/mmu-hash64.c | 8 --------
> > >  2 files changed, 1 insertion(+), 13 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index d09125d9af..ea5e11f1d9 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -58,14 +58,10 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
> > >       * we don't get spurious wakups before an RTAS start-cpu call.
> > >       * For the same reason, set PSSCR_EC.
> > >       */
> > > -    lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
> > > +    lpcr &= ~(LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
> 
> ... a few lines above, we have a comment that should be dropped as well.
> 
>      * Clearing VPM0 will also cause us to use RMOR in mmu-hash64.c for
>      * real mode accesses, which thankfully defaults to 0 and isn't
>      * accessible in guest mode.

Removed, thanks.

> 
> My R-b tag stands anyway.
> 
> > >      lpcr |= LPCR_LPES0 | LPCR_LPES1;
> > >      env->spr[SPR_PSSCR] |= PSSCR_EC;
> > >  
> > > -    /* Set RMLS to the max (ie, 16G) */
> > > -    lpcr &= ~LPCR_RMLS;
> > > -    lpcr |= 1ull << LPCR_RMLS_SHIFT;
> > > -
> > >      ppc_store_lpcr(cpu, lpcr);
> > >  
> > >      /* Set a full AMOR so guest can use the AMR as it sees fit */
> > > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> > > index e372c42add..caf47ad6fc 100644
> > > --- a/target/ppc/mmu-hash64.c
> > > +++ b/target/ppc/mmu-hash64.c
> > > @@ -1126,14 +1126,6 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> > >                        (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
> > >                        LPCR_DEE | LPCR_OEE)) | LPCR_MER | LPCR_GTSE | LPCR_TC |
> > >                        LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE);
> > > -        /*
> > > -         * If we have a virtual hypervisor, we need to bring back RMLS. It
> > > -         * doesn't exist on an actual P9 but that's all we know how to
> > > -         * configure with softmmu at the moment
> > > -         */
> > > -        if (cpu->vhyp) {
> > > -            lpcr |= (val & LPCR_RMLS);
> > > -        }
> > >          break;
> > >      default:
> > >          g_assert_not_reached();
> > 
> > 
>

Patch
diff mbox series

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index d09125d9af..ea5e11f1d9 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -58,14 +58,10 @@  static void spapr_reset_vcpu(PowerPCCPU *cpu)
      * we don't get spurious wakups before an RTAS start-cpu call.
      * For the same reason, set PSSCR_EC.
      */
-    lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
+    lpcr &= ~(LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
     lpcr |= LPCR_LPES0 | LPCR_LPES1;
     env->spr[SPR_PSSCR] |= PSSCR_EC;
 
-    /* Set RMLS to the max (ie, 16G) */
-    lpcr &= ~LPCR_RMLS;
-    lpcr |= 1ull << LPCR_RMLS_SHIFT;
-
     ppc_store_lpcr(cpu, lpcr);
 
     /* Set a full AMOR so guest can use the AMR as it sees fit */
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index e372c42add..caf47ad6fc 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1126,14 +1126,6 @@  void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
                       (LPCR_PECE_L_MASK & (LPCR_PDEE | LPCR_HDEE | LPCR_EEE |
                       LPCR_DEE | LPCR_OEE)) | LPCR_MER | LPCR_GTSE | LPCR_TC |
                       LPCR_HEIC | LPCR_LPES0 | LPCR_HVICE | LPCR_HDICE);
-        /*
-         * If we have a virtual hypervisor, we need to bring back RMLS. It
-         * doesn't exist on an actual P9 but that's all we know how to
-         * configure with softmmu at the moment
-         */
-        if (cpu->vhyp) {
-            lpcr |= (val & LPCR_RMLS);
-        }
         break;
     default:
         g_assert_not_reached();