Re: [Qemu-devel] [PATCH v2 3/3] target/ppc: generalize check on radix when in HV mode

[David Gibson]

On Fri, Feb 16, 2018 at 09:45:04AM +0100, Cédric Le Goater wrote:
> On a POWER9 processor, the first doubleword of the partition table
> entry (as pointed to by the PTCR) indicates whether the host uses HPT
> or Radix Tree translation for that partition. Use that bit to check
> for radix mode on pseries and powernv QEMU machines.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  Changes since v1:
> 
>  - fixed commit log
>  - introduced ppc64_v3_get_patbe0()
>  - renamed ppc64_radix() in ppc64_v3_radix()
>  
>  target/ppc/mmu-book3s-v3.c  | 16 +++++++++++++++-
>  target/ppc/mmu-book3s-v3.h  | 11 +++--------
>  target/ppc/mmu_helper.c     |  4 ++--
>  target/ppc/translate_init.c |  2 +-
>  4 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/target/ppc/mmu-book3s-v3.c b/target/ppc/mmu-book3s-v3.c
> index b60df4408f3b..9d05e07ef6bd 100644
> --- a/target/ppc/mmu-book3s-v3.c
> +++ b/target/ppc/mmu-book3s-v3.c
> @@ -23,10 +23,24 @@
>  #include "mmu-book3s-v3.h"
>  #include "mmu-radix64.h"
>  
> +bool ppc64_v3_radix(PowerPCCPU *cpu)
> +{
> +    CPUPPCState *env = &cpu->env;
> +
> +    if (msr_hv) {
> +        return ppc64_v3_get_patbe0(cpu) & PATBE0_HR;
> +    } else  {
> +        PPCVirtualHypervisorClass *vhc =
> +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> +
> +        return !!(vhc->get_patbe(cpu->vhyp) & PATBE1_GR);
> +    }

I think this is backwards.  If cpu->vhyp is set, you should always the
get_patbe() hook, before you go looking at anything else.

This is also wrong if you have a powernv platform but msr_hv is not
set - which is what you'll have once you get to the point of trying to
run guests within an emulated powernv machine.

> +}
> +
>  int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>                                int mmu_idx)
>  {
> -    if (ppc64_radix_guest(cpu)) { /* Guest uses radix */
> +    if (ppc64_v3_radix(cpu)) { /* radix mode */
>          return ppc_radix64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
>      } else { /* Guest uses hash */
>          return ppc_hash64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
> diff --git a/target/ppc/mmu-book3s-v3.h b/target/ppc/mmu-book3s-v3.h
> index a7ab580c3140..a12bb1e28b45 100644
> --- a/target/ppc/mmu-book3s-v3.h
> +++ b/target/ppc/mmu-book3s-v3.h
> @@ -29,7 +29,8 @@
>  #define PTCR_PATS               0x000000000000001FULL /* Partition Table 
> Size */
>  
>  /* Partition Table Entry Fields */
> -#define PATBE1_GR 0x8000000000000000
> +#define PATBE0_HR               PPC_BIT(0)            /* 1:Host Radix 0:HPT  
>  */
> +#define PATBE1_GR               PPC_BIT(0)            /* 1:Guest Radix 0:HPT 
>  */
>  
>  /* Process Table Entry */
>  struct prtb_entry {
> @@ -43,13 +44,7 @@ static inline bool ppc64_use_proc_tbl(PowerPCCPU *cpu)
>      return !!(cpu->env.spr[SPR_LPCR] & LPCR_UPRT);
>  }
>  
> -static inline bool ppc64_radix_guest(PowerPCCPU *cpu)
> -{
> -    PPCVirtualHypervisorClass *vhc =
> -        PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> -
> -    return !!(vhc->get_patbe(cpu->vhyp) & PATBE1_GR);
> -}
> +bool ppc64_v3_radix(PowerPCCPU *cpu);
>  
>  int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>                                int mmu_idx);
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 82e63552f617..81a43982e421 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -1285,7 +1285,7 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf, 
> CPUPPCState *env)
>          dump_slb(f, cpu_fprintf, ppc_env_get_cpu(env));
>          break;
>      case POWERPC_MMU_VER_3_00:
> -        if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
> +        if (ppc64_v3_radix(ppc_env_get_cpu(env))) {
>              /* TODO - Unsupported */
>          } else {
>              dump_slb(f, cpu_fprintf, ppc_env_get_cpu(env));
> @@ -1431,7 +1431,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr 
> addr)
>      case POWERPC_MMU_VER_2_07:
>          return ppc_hash64_get_phys_page_debug(cpu, addr);
>      case POWERPC_MMU_VER_3_00:
> -        if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
> +        if (ppc64_v3_radix(ppc_env_get_cpu(env))) {
>              return ppc_radix64_get_phys_page_debug(cpu, addr);
>          } else {
>              return ppc_hash64_get_phys_page_debug(cpu, addr);
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index c998ac2ee405..21d5dcd15386 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8967,7 +8967,7 @@ void cpu_ppc_set_papr(PowerPCCPU *cpu, 
> PPCVirtualHypervisor *vhyp)
>           * KVM but not under TCG. Update the default LPCR to keep new
>           * CPUs in sync when radix is enabled.
>           */
> -        if (ppc64_radix_guest(cpu)) {
> +        if (ppc64_v3_radix(cpu)) {
>              lpcr->default_value |= LPCR_UPRT | LPCR_GTSE;
>          } else {
>              lpcr->default_value &= ~(LPCR_UPRT | LPCR_GTSE);

-- 
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

signature.asc
Description: PGP signature