Re: [Qemu-devel] [PATCH v3 1/4] target/ppc: export external HPT via virtual hypervisor

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3464 bytes --]

On Thu, Mar 15, 2018 at 01:33:59PM +0000, Cédric Le Goater wrote:
> commit e57ca75ce3b2 ("target/ppc: Manage external HPT via virtual
> hypervisor") exported a set of methods to manipulate the HPT from the
> core hash MMU but the base address of the HPT was not part of them and
> SPR_SDR1 is still used under some circumstances, which is incorrect
> for the sPAPR machines.
> 
> This is not a major change as only the logging should be impacted but
> nevertheless, it will help to introduce support for the hash MMU on
> POWER9 PowerNV machines.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

This doesn't make sense.  The whole point of the "virtual hypervisor"
is that the hash table doesn't live within the guest address space,
and therefore it *has* no meaningful base address.  Basically
ppc_hash64_hpt_base() should never be called if vhyp is set.  If it
is, that's a bug.

> ---
>  hw/ppc/spapr.c          | 8 ++++++++
>  target/ppc/cpu.h        | 1 +
>  target/ppc/mmu-hash64.h | 5 +++++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f1798457bc4d..2329664e0c2c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1327,6 +1327,13 @@ static hwaddr spapr_hpt_mask(PPCVirtualHypervisor *vhyp)
>      return HTAB_SIZE(spapr) / HASH_PTEG_SIZE_64 - 1;
>  }
>  
> +static hwaddr spapr_hpt_base(PPCVirtualHypervisor *vhyp)
> +{
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
> +
> +    return (hwaddr) spapr->htab;
> +}
> +
>  static target_ulong spapr_encode_hpt_for_kvm_pr(PPCVirtualHypervisor *vhyp)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
> @@ -4073,6 +4080,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      smc->phb_placement = spapr_phb_placement;
>      vhc->hypercall = emulate_spapr_hypercall;
>      vhc->hpt_mask = spapr_hpt_mask;
> +    vhc->hpt_base = spapr_hpt_base;
>      vhc->map_hptes = spapr_map_hptes;
>      vhc->unmap_hptes = spapr_unmap_hptes;
>      vhc->store_hpte = spapr_store_hpte;
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 7bde1884a142..4de0653a3984 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1258,6 +1258,7 @@ struct PPCVirtualHypervisorClass {
>      InterfaceClass parent;
>      void (*hypercall)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
>      hwaddr (*hpt_mask)(PPCVirtualHypervisor *vhyp);
> +    hwaddr (*hpt_base)(PPCVirtualHypervisor *vhyp);
>      const ppc_hash_pte64_t *(*map_hptes)(PPCVirtualHypervisor *vhyp,
>                                           hwaddr ptex, int n);
>      void (*unmap_hptes)(PPCVirtualHypervisor *vhyp,
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index d297b97d3773..0ade8d15d9e4 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -100,6 +100,11 @@ void ppc_hash64_update_rmls(CPUPPCState *env);
>  
>  static inline hwaddr ppc_hash64_hpt_base(PowerPCCPU *cpu)
>  {
> +    if (cpu->vhyp) {
> +        PPCVirtualHypervisorClass *vhc =
> +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> +        return vhc->hpt_base(cpu->vhyp);
> +    }
>      return cpu->env.spr[SPR_SDR1] & SDR_64_HTABORG;
>  }
>  

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]