[v2,05/10] spapr, ppc: Remove VPM0/RMLS hacks for POWER9
diff mbox series

Message ID 20200107044827.471355-6-david@gibson.dropbear.id.au
State New
Headers show
Series
  • target/ppc: Correct some errors with real mode handling
Related show

Commit Message

David Gibson Jan. 7, 2020, 4:48 a.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 within the 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>
---
 hw/ppc/spapr_cpu_core.c | 6 +-----
 target/ppc/mmu-hash64.c | 8 --------
 2 files changed, 1 insertion(+), 13 deletions(-)

Comments

Cédric Le Goater Jan. 7, 2020, 2:35 p.m. UTC | #1
On 1/7/20 5:48 AM, David Gibson 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 within the 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(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 8339c4c0f8..daebcd9f38 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 d878180df5..d7f9933e6d 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -1124,14 +1124,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();
>
Alexey Kardashevskiy Jan. 9, 2020, 7:33 a.m. UTC | #2
On 07/01/2020 15:48, David Gibson 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 within the cpu to be treated
> as absolute physical addresses.


s/within the modelled within the cpu/within the modelled cpu/ ? Thanks,


> 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>
> ---
>  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 8339c4c0f8..daebcd9f38 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 d878180df5..d7f9933e6d 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -1124,14 +1124,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 Jan. 13, 2020, 3:38 a.m. UTC | #3
On Thu, Jan 09, 2020 at 06:33:36PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 07/01/2020 15:48, David Gibson 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 within the cpu to be treated
> > as absolute physical addresses.
> 
> 
> s/within the modelled within the cpu/within the modelled cpu/ ?
> Thanks,

Thanks.

Patch
diff mbox series

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 8339c4c0f8..daebcd9f38 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 d878180df5..d7f9933e6d 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -1124,14 +1124,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();