[Qemu-devel] [PATCH 0/3] hw/ppc/spapr: Improvements related to the "ibm, pa-features" property

[Qemu-devel] [PATCH 0/3] hw/ppc/spapr: Improvements related to the "ibm, pa-features" property

[Thomas Huth]

First patch cleans up the code a little bit by moving all lines
related to setting the "ibm,pa-features" property into a separate
function. The second patch fixes a problem with detecting the
PowerISA version there. And the third patch takes care of setting
the bit for Transactional Memory only if it's really available.

Thomas Huth (3):
  hw/ppc/spapr: Move code related to "ibm,pa-features" to a separate
    function
  hw/ppc/spapr: Fix the selection of the processor features
  ppc: Check the availability of transactional memory

 hw/ppc/spapr.c       | 76 +++++++++++++++++++++++++++++++---------------------
 target-ppc/kvm.c     |  7 +++++
 target-ppc/kvm_ppc.h |  6 +++++
 3 files changed, 59 insertions(+), 30 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] hw/ppc/spapr: Move code related to "ibm, pa-features" to a separate function

[Thomas Huth]

The function spapr_populate_cpu_dt() has become quite big
already, and since we likely have to extend the pa-features
property for every new processor generation, it is nicer
if we put the related code into a separate function.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr.c | 66 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 648576e..7ac775d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -546,6 +546,41 @@ static int spapr_populate_memory(sPAPRMachineState *spapr, void *fdt)
     return 0;
 }
 
+/* Populate the "ibm,pa-features" property */
+static void spapr_populate_pa_features(CPUPPCState *env, void *fdt, int offset)
+{
+    uint8_t pa_features_206[] = { 6, 0,
+        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
+    uint8_t pa_features_207[] = { 24, 0,
+        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
+        0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
+    uint8_t *pa_features;
+    size_t pa_size;
+
+    if (env->mmu_model == POWERPC_MMU_2_06) {
+        pa_features = pa_features_206;
+        pa_size = sizeof(pa_features_206);
+    } else { /* env->mmu_model == POWERPC_MMU_2_07 */
+        pa_features = pa_features_207;
+        pa_size = sizeof(pa_features_207);
+    }
+
+    if (env->ci_large_pages) {
+        /*
+         * Note: we keep CI large pages off by default because a 64K capable
+         * guest provisioned with large pages might otherwise try to map a qemu
+         * framebuffer (or other kind of memory mapped PCI BAR) using 64K pages
+         * even if that qemu runs on a 4k host.
+         * We dd this bit back here if we are confident this is not an issue
+         */
+        pa_features[3] |= 0x20;
+    }
+
+    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
+}
+
 static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
                                   sPAPRMachineState *spapr)
 {
@@ -573,24 +608,6 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
     }
 
-    /* Note: we keep CI large pages off for now because a 64K capable guest
-     * provisioned with large pages might otherwise try to map a qemu
-     * framebuffer (or other kind of memory mapped PCI BAR) using 64K pages
-     * even if that qemu runs on a 4k host.
-     *
-     * We can later add this bit back when we are confident this is not
-     * an issue (!HV KVM or 64K host)
-     */
-    uint8_t pa_features_206[] = { 6, 0,
-        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
-    uint8_t pa_features_207[] = { 24, 0,
-        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
-        0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
-        0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
-    uint8_t *pa_features;
-    size_t pa_size;
-
     _FDT((fdt_setprop_cell(fdt, offset, "reg", index)));
     _FDT((fdt_setprop_string(fdt, offset, "device_type", "cpu")));
 
@@ -657,18 +674,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
                           page_sizes_prop, page_sizes_prop_size)));
     }
 
-    /* Do the ibm,pa-features property, adjust it for ci-large-pages */
-    if (env->mmu_model == POWERPC_MMU_2_06) {
-        pa_features = pa_features_206;
-        pa_size = sizeof(pa_features_206);
-    } else /* env->mmu_model == POWERPC_MMU_2_07 */ {
-        pa_features = pa_features_207;
-        pa_size = sizeof(pa_features_207);
-    }
-    if (env->ci_large_pages) {
-        pa_features[3] |= 0x20;
-    }
-    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
+    spapr_populate_pa_features(env, fdt, offset);
 
     _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
                            cs->cpu_index / vcpus_per_socket)));
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/3] hw/ppc/spapr: Move code related to "ibm, pa-features" to a separate function

[Cédric Le Goater]

On 09/28/2016 01:16 PM, Thomas Huth wrote:
> The function spapr_populate_cpu_dt() has become quite big
> already, and since we likely have to extend the pa-features
> property for every new processor generation, it is nicer
> if we put the related code into a separate function.

Looks good to me,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C. 

(It would be really nice to have a routine to build pa_features 
in a less cryptic way. )


> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/ppc/spapr.c | 66 ++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 648576e..7ac775d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -546,6 +546,41 @@ static int spapr_populate_memory(sPAPRMachineState *spapr, void *fdt)
>      return 0;
>  }
>  
> +/* Populate the "ibm,pa-features" property */
> +static void spapr_populate_pa_features(CPUPPCState *env, void *fdt, int offset)
> +{
> +    uint8_t pa_features_206[] = { 6, 0,
> +        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> +    uint8_t pa_features_207[] = { 24, 0,
> +        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
> +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> +    uint8_t *pa_features;
> +    size_t pa_size;
> +
> +    if (env->mmu_model == POWERPC_MMU_2_06) {
> +        pa_features = pa_features_206;
> +        pa_size = sizeof(pa_features_206);
> +    } else { /* env->mmu_model == POWERPC_MMU_2_07 */
> +        pa_features = pa_features_207;
> +        pa_size = sizeof(pa_features_207);
> +    }
> +
> +    if (env->ci_large_pages) {
> +        /*
> +         * Note: we keep CI large pages off by default because a 64K capable
> +         * guest provisioned with large pages might otherwise try to map a qemu
> +         * framebuffer (or other kind of memory mapped PCI BAR) using 64K pages
> +         * even if that qemu runs on a 4k host.
> +         * We dd this bit back here if we are confident this is not an issue
> +         */
> +        pa_features[3] |= 0x20;
> +    }
> +
> +    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
> +}
> +
>  static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>                                    sPAPRMachineState *spapr)
>  {
> @@ -573,24 +608,6 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>          _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
>      }
>  
> -    /* Note: we keep CI large pages off for now because a 64K capable guest
> -     * provisioned with large pages might otherwise try to map a qemu
> -     * framebuffer (or other kind of memory mapped PCI BAR) using 64K pages
> -     * even if that qemu runs on a 4k host.
> -     *
> -     * We can later add this bit back when we are confident this is not
> -     * an issue (!HV KVM or 64K host)
> -     */
> -    uint8_t pa_features_206[] = { 6, 0,
> -        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> -    uint8_t pa_features_207[] = { 24, 0,
> -        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
> -        0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> -        0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> -        0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
> -    uint8_t *pa_features;
> -    size_t pa_size;
> -
>      _FDT((fdt_setprop_cell(fdt, offset, "reg", index)));
>      _FDT((fdt_setprop_string(fdt, offset, "device_type", "cpu")));
>  
> @@ -657,18 +674,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>                            page_sizes_prop, page_sizes_prop_size)));
>      }
>  
> -    /* Do the ibm,pa-features property, adjust it for ci-large-pages */
> -    if (env->mmu_model == POWERPC_MMU_2_06) {
> -        pa_features = pa_features_206;
> -        pa_size = sizeof(pa_features_206);
> -    } else /* env->mmu_model == POWERPC_MMU_2_07 */ {
> -        pa_features = pa_features_207;
> -        pa_size = sizeof(pa_features_207);
> -    }
> -    if (env->ci_large_pages) {
> -        pa_features[3] |= 0x20;
> -    }
> -    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
> +    spapr_populate_pa_features(env, fdt, offset);
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
>                             cs->cpu_index / vcpus_per_socket)));
> 

Re: [Qemu-devel] [PATCH 1/3] hw/ppc/spapr: Move code related to "ibm, pa-features" to a separate function

[David Gibson]

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

On Wed, Sep 28, 2016 at 03:07:31PM +0200, Cédric Le Goater wrote:
> On 09/28/2016 01:16 PM, Thomas Huth wrote:
> > The function spapr_populate_cpu_dt() has become quite big
> > already, and since we likely have to extend the pa-features
> > property for every new processor generation, it is nicer
> > if we put the related code into a separate function.
> 
> Looks good to me,
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thanks,
> 
> C. 
> 
> (It would be really nice to have a routine to build pa_features 
> in a less cryptic way. )

Applied, thanks.

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

[Qemu-devel] [PATCH 2/3] hw/ppc/spapr: Fix the selection of the processor features

[Thomas Huth]

The current code uses pa_features_206 for POWERPC_MMU_2_06, and
for everything else, it uses pa_features_207. This is bad in some
cases because there is also a "degraded" MMU version of ISA 2.06,
called POWERPC_MMU_2_06a, which should of course use the flags for
2.06 instead. And there is also the possibility that the user runs
the pseries machine with a POWER5+ or even 970 processor. In that
case we certainly do not want to set the flags for 2.07, and rather
simply skip the setting of the pa-features property instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7ac775d..46d6b90 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -559,12 +559,19 @@ static void spapr_populate_pa_features(CPUPPCState *env, void *fdt, int offset)
     uint8_t *pa_features;
     size_t pa_size;
 
-    if (env->mmu_model == POWERPC_MMU_2_06) {
+    switch (env->mmu_model) {
+    case POWERPC_MMU_2_06:
+    case POWERPC_MMU_2_06a:
         pa_features = pa_features_206;
         pa_size = sizeof(pa_features_206);
-    } else { /* env->mmu_model == POWERPC_MMU_2_07 */
+        break;
+    case POWERPC_MMU_2_07:
+    case POWERPC_MMU_2_07a:
         pa_features = pa_features_207;
         pa_size = sizeof(pa_features_207);
+        break;
+    default:
+        return;
     }
 
     if (env->ci_large_pages) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/3] hw/ppc/spapr: Fix the selection of the processor features

[Cédric Le Goater]

On 09/28/2016 01:16 PM, Thomas Huth wrote:
> The current code uses pa_features_206 for POWERPC_MMU_2_06, and
> for everything else, it uses pa_features_207. This is bad in some
> cases because there is also a "degraded" MMU version of ISA 2.06,
> called POWERPC_MMU_2_06a, which should of course use the flags for
> 2.06 instead. And there is also the possibility that the user runs
> the pseries machine with a POWER5+ or even 970 processor. In that
> case we certainly do not want to set the flags for 2.07, and rather
> simply skip the setting of the pa-features property instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ppc/spapr.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7ac775d..46d6b90 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -559,12 +559,19 @@ static void spapr_populate_pa_features(CPUPPCState *env, void *fdt, int offset)
>      uint8_t *pa_features;
>      size_t pa_size;
>  
> -    if (env->mmu_model == POWERPC_MMU_2_06) {
> +    switch (env->mmu_model) {
> +    case POWERPC_MMU_2_06:
> +    case POWERPC_MMU_2_06a:
>          pa_features = pa_features_206;
>          pa_size = sizeof(pa_features_206);
> -    } else { /* env->mmu_model == POWERPC_MMU_2_07 */
> +        break;
> +    case POWERPC_MMU_2_07:
> +    case POWERPC_MMU_2_07a:
>          pa_features = pa_features_207;
>          pa_size = sizeof(pa_features_207);
> +        break;
> +    default:
> +        return;
>      }
>  
>      if (env->ci_large_pages) {
> 

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

[Thomas Huth]

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

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

[Cédric Le Goater]

On 09/28/2016 01:16 PM, 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>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

and I gave them a try with a xenial guest and a f24 guest.

Thanks,

C.

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

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

[David Gibson]

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