Re: [Qemu-devel] [PATCH 3/3] ppc: Check the availability of transactional memory

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

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

On Wed, Sep 28, 2016 at 01:16:30PM +0200, Thomas Huth wrote:
> KVM-PR currently does not support transactional memory, and the
> implementation in TCG is just a fake. We should not announce TM
> support in the ibm,pa-features property when running on such a
> system, so disable it by default and only enable it if the KVM
> implementation supports it (i.e. recent versions of KVM-HV).
> These changes are based on some earlier work from Anton Blanchard
> (thanks!).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

So, I've applied this, but I do have one concern.  KVM HV has, IIUC,
supported TM in guests since basically forever, but the
KVM_CAP_PPC_HTM tag is pretty new.  So this change will break TM for
guests on KVM HV older than the capability flag.

So, I think we want a fallback which will set cap_htm if we detect KVM
HV using the usual KVM_CAP_PPC_GET_PVINFO hack.

I think we can do that as a follow up patch, but we definitely should
implement it before 2.8 is released.

> ---
>  hw/ppc/spapr.c       | 5 ++++-
>  target-ppc/kvm.c     | 7 +++++++
>  target-ppc/kvm_ppc.h | 6 ++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 46d6b90..0bdea5b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -555,7 +555,7 @@ static void spapr_populate_pa_features(CPUPPCState *env, void *fdt, int offset)
>          0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
>          0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
>          0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> -        0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> +        0x80, 0x00, 0x80, 0x00, 0x00, 0x00 };
>      uint8_t *pa_features;
>      size_t pa_size;
>  
> @@ -584,6 +584,9 @@ static void spapr_populate_pa_features(CPUPPCState *env, void *fdt, int offset)
>           */
>          pa_features[3] |= 0x20;
>      }
> +    if (kvmppc_has_cap_htm()) {
> +        pa_features[24] |= 0x80;    /* Transactional memory support */
> +    }
>  
>      _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
>  }
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index a18d4d5..e9a9faf 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -80,6 +80,7 @@ static int cap_ppc_watchdog;
>  static int cap_papr;
>  static int cap_htab_fd;
>  static int cap_fixup_hcalls;
> +static int cap_htm;             /* Hardware transactional memory support */
>  
>  static uint32_t debug_inst_opcode;
>  
> @@ -122,6 +123,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>       * only activated after this by kvmppc_set_papr() */
>      cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD);
>      cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL);
> +    cap_htm = kvm_vm_check_extension(s, KVM_CAP_PPC_HTM);
>  
>      if (!cap_interrupt_level) {
>          fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
> @@ -2353,6 +2355,11 @@ bool kvmppc_has_cap_fixup_hcalls(void)
>      return cap_fixup_hcalls;
>  }
>  
> +bool kvmppc_has_cap_htm(void)
> +{
> +    return cap_htm;
> +}
> +
>  static PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
>  {
>      ObjectClass *oc = OBJECT_CLASS(pcc);
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index a778184..bd1d78b 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -55,6 +55,7 @@ void kvmppc_hash64_free_pteg(uint64_t token);
>  void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
>                               target_ulong pte0, target_ulong pte1);
>  bool kvmppc_has_cap_fixup_hcalls(void);
> +bool kvmppc_has_cap_htm(void);
>  int kvmppc_enable_hwrng(void);
>  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
>  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> @@ -249,6 +250,11 @@ static inline bool kvmppc_has_cap_fixup_hcalls(void)
>      abort();
>  }
>  
> +static inline bool kvmppc_has_cap_htm(void)
> +{
> +    return false;
> +}
> +
>  static inline int kvmppc_enable_hwrng(void)
>  {
>      return -1;

-- 
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: 819 bytes --]