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

[Qemu-devel] [QEMU-PPC] [PATCH V2 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 V2 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 V2 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>
---
 hw/ppc/spapr.c      | 11 ++++++++++-
 hw/ppc/spapr_caps.c | 10 ++++++++++
 include/hw/compat.h |  2 ++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 969db6cde2..e2ebb76242 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3941,13 +3941,20 @@ static const TypeInfo spapr_machine_info = {
 /*
  * pseries-2.12
  */
+#define SPAPR_COMPAT_2_12                                              \
+    HW_COMPAT_2_12
+
 static void spapr_machine_2_12_instance_options(MachineState *machine)
 {
 }
 
 static void spapr_machine_2_12_class_options(MachineClass *mc)
 {
-    /* Defaults for the latest behaviour inherited from the base class */
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
+    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
+    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12);
 }
 
 DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
@@ -3969,6 +3976,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;
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 7f31850dfa..13238239da 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,8 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_2_12
+
 #define HW_COMPAT_2_11 \
     {\
         .driver   = "hpet",\
-- 
2.13.6

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

[Greg Kurz]

On Wed, 14 Feb 2018 17:51:33 +1100
Suraj Jitindar Singh <sjitindarsingh@gmail.com> 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>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

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

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

[Greg Kurz]

On Wed, 14 Feb 2018 17:51:34 +1100
Suraj Jitindar Singh <sjitindarsingh@gmail.com> wrote:

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

Reviewed-by: Greg Kurz <groug@kaod.org>

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

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

[Greg Kurz]

On Wed, 14 Feb 2018 17:51:35 +1100
Suraj Jitindar Singh <sjitindarsingh@gmail.com> wrote:

> 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>
> ---
>  hw/ppc/spapr.c      | 11 ++++++++++-
>  hw/ppc/spapr_caps.c | 10 ++++++++++
>  include/hw/compat.h |  2 ++
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 969db6cde2..e2ebb76242 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3941,13 +3941,20 @@ static const TypeInfo spapr_machine_info = {
>  /*
>   * pseries-2.12
>   */
> +#define SPAPR_COMPAT_2_12                                              \
> +    HW_COMPAT_2_12
> +
>  static void spapr_machine_2_12_instance_options(MachineState *machine)
>  {
>  }
>  
>  static void spapr_machine_2_12_class_options(MachineClass *mc)
>  {
> -    /* Defaults for the latest behaviour inherited from the base class */
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
> +    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
> +    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;

As written in the comment, the default for the latest machine type should
be set at the base class level, in spapr_machine_class_init()... not sure
to understand why you set it here...

> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12);

And so, you shouldn't need to setup 2.12 compat mode before 2.12 was even
released :)

>  }
>  
>  DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> @@ -3969,6 +3976,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;
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 7f31850dfa..13238239da 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -1,6 +1,8 @@
>  #ifndef HW_COMPAT_H
>  #define HW_COMPAT_H
>  
> +#define HW_COMPAT_2_12
> +
>  #define HW_COMPAT_2_11 \
>      {\
>          .driver   = "hpet",\

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

[Suraj Jitindar Singh]

On Wed, 2018-02-14 at 15:20 +0100, Greg Kurz wrote:
> On Wed, 14 Feb 2018 17:51:35 +1100
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> wrote:
> 
> > 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>
> > ---
> >  hw/ppc/spapr.c      | 11 ++++++++++-
> >  hw/ppc/spapr_caps.c | 10 ++++++++++
> >  include/hw/compat.h |  2 ++
> >  3 files changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 969db6cde2..e2ebb76242 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3941,13 +3941,20 @@ static const TypeInfo spapr_machine_info =
> > {
> >  /*
> >   * pseries-2.12
> >   */
> > +#define
> > SPAPR_COMPAT_2_12                                              \
> > +    HW_COMPAT_2_12
> > +
> >  static void spapr_machine_2_12_instance_options(MachineState
> > *machine)
> >  {
> >  }
> >  
> >  static void spapr_machine_2_12_class_options(MachineClass *mc)
> >  {
> > -    /* Defaults for the latest behaviour inherited from the base
> > class */
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> > +    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
> > +    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;
> 
> As written in the comment, the default for the latest machine type
> should
> be set at the base class level, in spapr_machine_class_init()... not
> sure
> to understand why you set it here...

Ah yes, my bad.

> 
> > +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12);
> 
> And so, you shouldn't need to setup 2.12 compat mode before 2.12 was
> even
> released :)

Fair enough :)

> 
> >  }
> >  
> >  DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> > @@ -3969,6 +3976,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;
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 7f31850dfa..13238239da 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -1,6 +1,8 @@
> >  #ifndef HW_COMPAT_H
> >  #define HW_COMPAT_H
> >  
> > +#define HW_COMPAT_2_12
> > +
> >  #define HW_COMPAT_2_11 \
> >      {\
> >          .driver   = "hpet",\
> 
>