[Qemu-devel] [QEMU-PPC] [PATCH V3 1/3] ppc/spapr-caps: Change migration macro to take full spapr-cap name

[Qemu-devel] [QEMU-PPC] [PATCH V3 1/3] ppc/spapr-caps: Change migration macro to take full spapr-cap name

[Suraj Jitindar Singh]

Change the macro that generates the vmstate migration field and the needed
function for the spapr-caps to take the full spapr-cap name. This has
the benefit of meaning this instance will be picked up when greping
for the spapr-caps and making it more obvious what this macro is doing.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 hw/ppc/spapr_caps.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 62efdaee38..e69d308560 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -350,34 +350,34 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr)
 }
 
 /* Used to generate the migration field and needed function for a spapr cap */
-#define SPAPR_CAP_MIG_STATE(cap, ccap)                  \
-static bool spapr_cap_##cap##_needed(void *opaque)      \
+#define SPAPR_CAP_MIG_STATE(sname, cap)                 \
+static bool spapr_cap_##sname##_needed(void *opaque)    \
 {                                                       \
     sPAPRMachineState *spapr = opaque;                  \
                                                         \
-    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
-           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
-            spapr->def.caps[SPAPR_CAP_##ccap]);         \
+    return spapr->cmd_line_caps[cap] &&                 \
+           (spapr->eff.caps[cap] !=                     \
+            spapr->def.caps[cap]);                      \
 }                                                       \
                                                         \
-const VMStateDescription vmstate_spapr_cap_##cap = {    \
-    .name = "spapr/cap/" #cap,                          \
+const VMStateDescription vmstate_spapr_cap_##sname = {  \
+    .name = "spapr/cap/" #sname,                        \
     .version_id = 1,                                    \
     .minimum_version_id = 1,                            \
-    .needed = spapr_cap_##cap##_needed,                 \
+    .needed = spapr_cap_##sname##_needed,               \
     .fields = (VMStateField[]) {                        \
-        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
+        VMSTATE_UINT8(mig.caps[cap],                    \
                       sPAPRMachineState),               \
         VMSTATE_END_OF_LIST()                           \
     },                                                  \
 }
 
-SPAPR_CAP_MIG_STATE(htm, HTM);
-SPAPR_CAP_MIG_STATE(vsx, VSX);
-SPAPR_CAP_MIG_STATE(dfp, DFP);
-SPAPR_CAP_MIG_STATE(cfpc, CFPC);
-SPAPR_CAP_MIG_STATE(sbbc, SBBC);
-SPAPR_CAP_MIG_STATE(ibs, IBS);
+SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM);
+SPAPR_CAP_MIG_STATE(vsx, SPAPR_CAP_VSX);
+SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP);
+SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC);
+SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC);
+SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS);
 
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V3 2/3] ppc/spapr-caps: Convert spapr-cap-ibs to be a boolean

[Suraj Jitindar Singh]

The spapr-cap cap-ibs can only have values broken or fixed as there is
no workaround. Currently setting the value workaround will hit an assert
if the guest makes the hcall h_get_cpu_characteristics.

Thus this capability is better suited to being represented as a boolean.
Setting this to OFF corresponds to the old BROKEN, that is no indirect
branch serialisation. Setting this to ON corresponds to the old FIXED,
that is indirect branches are serialised.

Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 hw/ppc/spapr.c      |  2 +-
 hw/ppc/spapr_caps.c | 12 ++++++------
 target/ppc/kvm.c    |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 32a876be56..969db6cde2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3886,7 +3886,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     smc->default_caps.caps[SPAPR_CAP_DFP] = SPAPR_CAP_ON;
     smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
     smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
-    smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
+    smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_OFF;
     spapr_caps_add_properties(smc, &error_abort);
 }
 
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index e69d308560..05997b0842 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -207,9 +207,9 @@ static void cap_safe_indirect_branch_apply(sPAPRMachineState *spapr,
 {
     if (tcg_enabled() && val) {
         /* TODO - for now only allow broken for TCG */
-        error_setg(errp, "Requested safe indirect branch capability level not supported by tcg, try a different value for cap-ibs");
+        error_setg(errp, "Indirect Branch Serialisation support not available, try cap-ibs=off");
     } else if (kvm_enabled() && (val > kvmppc_get_cap_safe_indirect_branch())) {
-        error_setg(errp, "Requested safe indirect branch capability level not supported by kvm, try a different value for cap-ibs");
+        error_setg(errp, "Indirect Branch Serialisation support not available, try cap-ibs=off");
     }
 }
 
@@ -263,11 +263,11 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
     },
     [SPAPR_CAP_IBS] = {
         .name = "ibs",
-        .description = "Indirect Branch Serialisation" VALUE_DESC_TRISTATE,
+        .description = "Indirect Branch Serialisation",
         .index = SPAPR_CAP_IBS,
-        .get = spapr_cap_get_tristate,
-        .set = spapr_cap_set_tristate,
-        .type = "string",
+        .get = spapr_cap_get_bool,
+        .set = spapr_cap_set_bool,
+        .type = "bool",
         .apply = cap_safe_indirect_branch_apply,
     },
 };
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 9842b3bb12..3e3e5f9c1f 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2495,7 +2495,7 @@ static void kvmppc_get_cpu_characteristics(KVMState *s)
     }
     /* Parse and set cap_ppc_safe_indirect_branch */
     if (c.character & H_CPU_CHAR_BCCTRL_SERIALISED) {
-        cap_ppc_safe_indirect_branch = 2;
+        cap_ppc_safe_indirect_branch = 1;
     }
 }
 
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V3 3/3] ppc/spapr-caps: For pseries-2.12 change spapr-cap defaults

[Suraj Jitindar Singh]

For the pseries-2.12 machine type, make the spapr-caps SPAPR_CAP_CFPC
and SPAPR_CAP_SBBC default to workaround. Thus if the host is capable
the guest will be able to take advantage of these workarounds by default.
Otherwise if the host doesn't have these capabilities qemu will fail to
start and they will have to be explicitly disabled on the command line
with:
-machine pseries,cap-cfpc=broken,cap-sbbc=broken

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

---

V2 -> V3:
 - Set caps to workaround in the class default rather than the
   pseries-2.12 initialiser.
---
 hw/ppc/spapr.c      |  6 ++++--
 hw/ppc/spapr_caps.c | 10 ++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 969db6cde2..4fe2c3b90c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3884,8 +3884,8 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
     smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
     smc->default_caps.caps[SPAPR_CAP_DFP] = SPAPR_CAP_ON;
-    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
-    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
+    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
+    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;
     smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_OFF;
     spapr_caps_add_properties(smc, &error_abort);
 }
@@ -3969,6 +3969,8 @@ static void spapr_machine_2_11_class_options(MachineClass *mc)
 
     spapr_machine_2_12_class_options(mc);
     smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_ON;
+    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
+    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
 }
 
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 05997b0842..c25c2bca52 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -281,11 +281,21 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
 
     caps = smc->default_caps;
 
+    if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00,
+                          0, spapr->max_compat_pvr)) {
+        caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
+    }
+
     if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07,
                           0, spapr->max_compat_pvr)) {
         caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
     }
 
+    if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06_PLUS,
+                          0, spapr->max_compat_pvr)) {
+        caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
+    }
+
     if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06,
                           0, spapr->max_compat_pvr)) {
         caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_OFF;
-- 
2.13.6

Re: [Qemu-devel] [QEMU-PPC] [PATCH V3 1/3] ppc/spapr-caps: Change migration macro to take full spapr-cap name

[David Gibson]

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

On Thu, Feb 15, 2018 at 11:44:41AM +1100, Suraj Jitindar Singh wrote:
> Change the macro that generates the vmstate migration field and the needed
> function for the spapr-caps to take the full spapr-cap name. This has
> the benefit of meaning this instance will be picked up when greping
> for the spapr-caps and making it more obvious what this macro is doing.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Applied, thanks.

> ---
>  hw/ppc/spapr_caps.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 62efdaee38..e69d308560 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -350,34 +350,34 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr)
>  }
>  
>  /* Used to generate the migration field and needed function for a spapr cap */
> -#define SPAPR_CAP_MIG_STATE(cap, ccap)                  \
> -static bool spapr_cap_##cap##_needed(void *opaque)      \
> +#define SPAPR_CAP_MIG_STATE(sname, cap)                 \
> +static bool spapr_cap_##sname##_needed(void *opaque)    \
>  {                                                       \
>      sPAPRMachineState *spapr = opaque;                  \
>                                                          \
> -    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
> -           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
> -            spapr->def.caps[SPAPR_CAP_##ccap]);         \
> +    return spapr->cmd_line_caps[cap] &&                 \
> +           (spapr->eff.caps[cap] !=                     \
> +            spapr->def.caps[cap]);                      \
>  }                                                       \
>                                                          \
> -const VMStateDescription vmstate_spapr_cap_##cap = {    \
> -    .name = "spapr/cap/" #cap,                          \
> +const VMStateDescription vmstate_spapr_cap_##sname = {  \
> +    .name = "spapr/cap/" #sname,                        \
>      .version_id = 1,                                    \
>      .minimum_version_id = 1,                            \
> -    .needed = spapr_cap_##cap##_needed,                 \
> +    .needed = spapr_cap_##sname##_needed,               \
>      .fields = (VMStateField[]) {                        \
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
> +        VMSTATE_UINT8(mig.caps[cap],                    \
>                        sPAPRMachineState),               \
>          VMSTATE_END_OF_LIST()                           \
>      },                                                  \
>  }
>  
> -SPAPR_CAP_MIG_STATE(htm, HTM);
> -SPAPR_CAP_MIG_STATE(vsx, VSX);
> -SPAPR_CAP_MIG_STATE(dfp, DFP);
> -SPAPR_CAP_MIG_STATE(cfpc, CFPC);
> -SPAPR_CAP_MIG_STATE(sbbc, SBBC);
> -SPAPR_CAP_MIG_STATE(ibs, IBS);
> +SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM);
> +SPAPR_CAP_MIG_STATE(vsx, SPAPR_CAP_VSX);
> +SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP);
> +SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC);
> +SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC);
> +SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS);
>  
>  void spapr_caps_reset(sPAPRMachineState *spapr)
>  {

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