[Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps

[Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps

[Suraj Jitindar Singh]

The following patch series adds 3 new tristate capabilities and their
associated handling.

A new H-Call is implemented which a guest will use to query the 
requirement for and availability of workarounds for certain cpu 
behaviours.

Applies on top of David's tree: ppc-for-2.12

The main changes to V5 are:
- Add patch 2/7 to series to introduce macros to generate migration fields
- Fix bisect breakage in 3/7 due to unused function
- Use assert in 7/7 for invalid cap values

Suraj Jitindar Singh (7):
  target/ppc/kvm: Add cap_ppc_safe_[cache/bounds_check/indirect_branch]
  target/ppc/spapr_caps: Add macro to generate spapr_caps migration
    vmstate
  target/ppc/spapr_caps: Add support for tristate spapr_capabilities
  target/ppc/spapr_caps: Add new tristate cap safe_cache
  target/ppc/spapr_caps: Add new tristate cap safe_bounds_check
  target/ppc/spapr_caps: Add new tristate cap safe_indirect_branch
  target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

 hw/ppc/spapr.c                  |   6 ++
 hw/ppc/spapr_caps.c             | 202 +++++++++++++++++++++++++++++-----------
 hw/ppc/spapr_hcall.c            |  58 ++++++++++++
 include/hw/ppc/spapr.h          |  28 +++++-
 linux-headers/asm-powerpc/kvm.h |   8 ++
 linux-headers/linux/kvm.h       |   3 +
 target/ppc/kvm.c                |  58 ++++++++++++
 target/ppc/kvm_ppc.h            |  18 ++++
 8 files changed, 326 insertions(+), 55 deletions(-)

-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 1/7] target/ppc/kvm: Add cap_ppc_safe_[cache/bounds_check/indirect_branch]

[Suraj Jitindar Singh]

Add three new kvm capabilities used to represent the level of host support
for three corresponding workarounds.

Host support for each of the capabilities is queried through the
new ioctl KVM_PPC_GET_CPU_CHAR which returns four uint64 quantities. The
first two, character and behaviour, represent the available
characteristics of the cpu and the behaviour of the cpu respectively.
The second two, c_mask and b_mask, represent the mask of known bits for
the character and beheviour dwords respectively.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/hw/ppc/spapr.h          | 12 +++++++++
 linux-headers/asm-powerpc/kvm.h |  8 ++++++
 linux-headers/linux/kvm.h       |  3 +++
 target/ppc/kvm.c                | 58 +++++++++++++++++++++++++++++++++++++++++
 target/ppc/kvm_ppc.h            | 18 +++++++++++++
 5 files changed, 99 insertions(+)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 0f5628f22e..eded0ea57d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -295,6 +295,18 @@ struct sPAPRMachineState {
 #define H_DABRX_KERNEL     (1ULL<<(63-62))
 #define H_DABRX_USER       (1ULL<<(63-63))
 
+/* Values for KVM_PPC_GET_CPU_CHAR & H_GET_CPU_CHARACTERISTICS */
+#define H_CPU_CHAR_SPEC_BAR_ORI31               PPC_BIT(0)
+#define H_CPU_CHAR_BCCTRL_SERIALISED            PPC_BIT(1)
+#define H_CPU_CHAR_L1D_FLUSH_ORI30              PPC_BIT(2)
+#define H_CPU_CHAR_L1D_FLUSH_TRIG2              PPC_BIT(3)
+#define H_CPU_CHAR_L1D_THREAD_PRIV              PPC_BIT(4)
+#define H_CPU_CHAR_HON_BRANCH_HINTS             PPC_BIT(5)
+#define H_CPU_CHAR_THR_RECONF_TRIG              PPC_BIT(6)
+#define H_CPU_BEHAV_FAVOUR_SECURITY             PPC_BIT(0)
+#define H_CPU_BEHAV_L1D_FLUSH_PR                PPC_BIT(1)
+#define H_CPU_BEHAV_BNDS_CHK_SPEC_BAR           PPC_BIT(2)
+
 /* Each control block has to be on a 4K boundary */
 #define H_CB_ALIGNMENT     4096
 
diff --git a/linux-headers/asm-powerpc/kvm.h b/linux-headers/asm-powerpc/kvm.h
index 61d6049f4c..5d2cb26848 100644
--- a/linux-headers/asm-powerpc/kvm.h
+++ b/linux-headers/asm-powerpc/kvm.h
@@ -443,6 +443,14 @@ struct kvm_ppc_rmmu_info {
 	__u32	ap_encodings[8];
 };
 
+/* For KVM_PPC_GET_CPU_CHAR */
+struct kvm_ppc_cpu_char {
+        __u64   character;      /* characteristics of the CPU */
+        __u64   behaviour;      /* recommended software behaviour */
+        __u64   c_mask;         /* valid bits in character */
+        __u64   b_mask;         /* valid bits in behaviour */
+};
+
 /* Per-vcpu XICS interrupt controller state */
 #define KVM_REG_PPC_ICP_STATE	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8c)
 
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index ce6c2f11f4..c35f1bd363 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -932,6 +932,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_HYPERV_SYNIC2 148
 #define KVM_CAP_HYPERV_VP_INDEX 149
 #define KVM_CAP_S390_AIS_MIGRATION 150
+#define KVM_CAP_PPC_GET_CPU_CHAR 151
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1261,6 +1262,8 @@ struct kvm_s390_ucas_mapping {
 #define KVM_PPC_CONFIGURE_V3_MMU  _IOW(KVMIO,  0xaf, struct kvm_ppc_mmuv3_cfg)
 /* Available with KVM_CAP_PPC_RADIX_MMU */
 #define KVM_PPC_GET_RMMU_INFO	  _IOW(KVMIO,  0xb0, struct kvm_ppc_rmmu_info)
+/* Available with KVM_CAP_PPC_GET_CPU_CHAR */
+#define KVM_PPC_GET_CPU_CHAR      _IOR(KVMIO,  0xb1, struct kvm_ppc_cpu_char)
 
 /* ioctl for vm fd */
 #define KVM_CREATE_DEVICE	  _IOWR(KVMIO,  0xe0, struct kvm_create_device)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 914be687e7..b16f731522 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -89,6 +89,9 @@ static int cap_mmu_radix;
 static int cap_mmu_hash_v3;
 static int cap_resize_hpt;
 static int cap_ppc_pvr_compat;
+static int cap_ppc_safe_cache;
+static int cap_ppc_safe_bounds_check;
+static int cap_ppc_safe_indirect_branch;
 
 static uint32_t debug_inst_opcode;
 
@@ -121,6 +124,7 @@ static bool kvmppc_is_pr(KVMState *ks)
 }
 
 static int kvm_ppc_register_host_cpu_type(MachineState *ms);
+static void kvmppc_get_cpu_characteristics(KVMState *s);
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
@@ -147,6 +151,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);
     cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);
     cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);
+    kvmppc_get_cpu_characteristics(s);
     /*
      * Note: setting it to false because there is not such capability
      * in KVM at this moment.
@@ -2456,6 +2461,59 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
     return cap_mmu_hash_v3;
 }
 
+static void kvmppc_get_cpu_characteristics(KVMState *s)
+{
+    struct kvm_ppc_cpu_char c;
+    int ret;
+
+    /* Assume broken */
+    cap_ppc_safe_cache = 0;
+    cap_ppc_safe_bounds_check = 0;
+    cap_ppc_safe_indirect_branch = 0;
+
+    ret = kvm_vm_check_extension(s, KVM_CAP_PPC_GET_CPU_CHAR);
+    if (!ret) {
+        return;
+    }
+    ret = kvm_vm_ioctl(s, KVM_PPC_GET_CPU_CHAR, &c);
+    if (ret < 0) {
+        return;
+    }
+    /* Parse and set cap_ppc_safe_cache */
+    if (~c.behaviour & c.b_mask & H_CPU_BEHAV_L1D_FLUSH_PR) {
+        cap_ppc_safe_cache = 2;
+    } else if ((c.character & c.c_mask & H_CPU_CHAR_L1D_THREAD_PRIV) &&
+               (c.character & c.c_mask & (H_CPU_CHAR_L1D_FLUSH_ORI30 |
+                                          H_CPU_CHAR_L1D_FLUSH_TRIG2))) {
+        cap_ppc_safe_cache = 1;
+    }
+    /* Parse and set cap_ppc_safe_bounds_check */
+    if (~c.behaviour & c.b_mask & H_CPU_BEHAV_BNDS_CHK_SPEC_BAR) {
+        cap_ppc_safe_bounds_check = 2;
+    } else if (c.character & c.c_mask & H_CPU_CHAR_SPEC_BAR_ORI31) {
+        cap_ppc_safe_bounds_check = 1;
+    }
+    /* Parse and set cap_ppc_safe_indirect_branch */
+    if (c.character & H_CPU_CHAR_BCCTRL_SERIALISED) {
+        cap_ppc_safe_indirect_branch = 2;
+    }
+}
+
+int kvmppc_get_cap_safe_cache(void)
+{
+    return cap_ppc_safe_cache;
+}
+
+int kvmppc_get_cap_safe_bounds_check(void)
+{
+    return cap_ppc_safe_bounds_check;
+}
+
+int kvmppc_get_cap_safe_indirect_branch(void)
+{
+    return cap_ppc_safe_indirect_branch;
+}
+
 PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
 {
     uint32_t host_pvr = mfpvr();
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index ecb55493cc..39830baa77 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -59,6 +59,9 @@ bool kvmppc_has_cap_fixup_hcalls(void);
 bool kvmppc_has_cap_htm(void);
 bool kvmppc_has_cap_mmu_radix(void);
 bool kvmppc_has_cap_mmu_hash_v3(void);
+int kvmppc_get_cap_safe_cache(void);
+int kvmppc_get_cap_safe_bounds_check(void);
+int kvmppc_get_cap_safe_indirect_branch(void);
 int kvmppc_enable_hwrng(void);
 int kvmppc_put_books_sregs(PowerPCCPU *cpu);
 PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
@@ -290,6 +293,21 @@ static inline bool kvmppc_has_cap_mmu_hash_v3(void)
     return false;
 }
 
+static inline int kvmppc_get_cap_safe_cache(void)
+{
+    return 0;
+}
+
+static inline int kvmppc_get_cap_safe_bounds_check(void)
+{
+    return 0;
+}
+
+static inline int kvmppc_get_cap_safe_indirect_branch(void)
+{
+    return 0;
+}
+
 static inline int kvmppc_enable_hwrng(void)
 {
     return -1;
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate

[Suraj Jitindar Singh]

The vmstate description and the contained needed function for migration
of spapr_caps is the same for each cap, with the name of the cap
substituted. As such introduce a macro to allow for easier generation of
these.

Convert the three existing spapr_caps (htm, vsx, and dfp) to use this
macro.

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

---

V5:
 - Patch added to series
---
 hw/ppc/spapr_caps.c | 78 +++++++++++++++++------------------------------------
 1 file changed, 24 insertions(+), 54 deletions(-)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index d5c9ce774a..5d52969bd5 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -228,62 +228,32 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr)
     return ok ? 0 : -EINVAL;
 }
 
-static bool spapr_cap_htm_needed(void *opaque)
-{
-    sPAPRMachineState *spapr = opaque;
-
-    return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
-           (spapr->eff.caps[SPAPR_CAP_HTM] != spapr->def.caps[SPAPR_CAP_HTM]);
-}
-
-const VMStateDescription vmstate_spapr_cap_htm = {
-    .name = "spapr/cap/htm",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .needed = spapr_cap_htm_needed,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
-        VMSTATE_END_OF_LIST()
-    },
-};
-
-static bool spapr_cap_vsx_needed(void *opaque)
-{
-    sPAPRMachineState *spapr = opaque;
-
-    return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
-           (spapr->eff.caps[SPAPR_CAP_VSX] != spapr->def.caps[SPAPR_CAP_VSX]);
+/* 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)      \
+{                                                       \
+    sPAPRMachineState *spapr = opaque;                  \
+                                                        \
+    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
+           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
+            spapr->def.caps[SPAPR_CAP_##ccap]);         \
+}                                                       \
+                                                        \
+const VMStateDescription vmstate_spapr_cap_##cap = {    \
+    .name = "spapr/cap/" #cap,                          \
+    .version_id = 1,                                    \
+    .minimum_version_id = 1,                            \
+    .needed = spapr_cap_##cap##_needed,                 \
+    .fields = (VMStateField[]) {                        \
+        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
+                      sPAPRMachineState),               \
+        VMSTATE_END_OF_LIST()                           \
+    },                                                  \
 }
 
-const VMStateDescription vmstate_spapr_cap_vsx = {
-    .name = "spapr/cap/vsx",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .needed = spapr_cap_vsx_needed,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
-        VMSTATE_END_OF_LIST()
-    },
-};
-
-static bool spapr_cap_dfp_needed(void *opaque)
-{
-    sPAPRMachineState *spapr = opaque;
-
-    return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
-           (spapr->eff.caps[SPAPR_CAP_DFP] != spapr->def.caps[SPAPR_CAP_DFP]);
-}
-
-const VMStateDescription vmstate_spapr_cap_dfp = {
-    .name = "spapr/cap/dfp",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .needed = spapr_cap_dfp_needed,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
-        VMSTATE_END_OF_LIST()
-    },
-};
+SPAPR_CAP_MIG_STATE(htm, HTM);
+SPAPR_CAP_MIG_STATE(vsx, VSX);
+SPAPR_CAP_MIG_STATE(dfp, DFP);
 
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 3/7] target/ppc/spapr_caps: Add support for tristate spapr_capabilities

[Suraj Jitindar Singh]

spapr_caps are used to represent the level of support for various
capabilities related to the spapr machine type. Currently there is
only support for boolean capabilities.

Add support for tristate capabilities by implementing their get/set
functions. These capabilities can have the values 0, 1 or 2
corresponding to broken, workaround and fixed.

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

---

V4 -> V5:
 - Add unused directive to fix bisection breakage
---
 hw/ppc/spapr_caps.c    | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  4 +++
 2 files changed, 70 insertions(+)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 5d52969bd5..d6f82b18fc 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -73,6 +73,72 @@ static void spapr_cap_set_bool(Object *obj, Visitor *v, const char *name,
     spapr->eff.caps[cap->index] = value ? SPAPR_CAP_ON : SPAPR_CAP_OFF;
 }
 
+static void __attribute__ ((unused)) spapr_cap_get_tristate(Object *obj,
+                                                            Visitor *v,
+                                                            const char *name,
+                                                            void *opaque,
+                                                            Error **errp)
+{
+    sPAPRCapabilityInfo *cap = opaque;
+    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+    char *val = NULL;
+    uint8_t value = spapr_get_cap(spapr, cap->index);
+
+    switch (value) {
+    case SPAPR_CAP_BROKEN:
+        val = g_strdup("broken");
+        break;
+    case SPAPR_CAP_WORKAROUND:
+        val = g_strdup("workaround");
+        break;
+    case SPAPR_CAP_FIXED:
+        val = g_strdup("fixed");
+        break;
+    default:
+        error_setg(errp, "Invalid value (%d) for cap-%s", value, cap->name);
+        return;
+    }
+
+    visit_type_str(v, name, &val, errp);
+    g_free(val);
+}
+
+static void __attribute__ ((unused)) spapr_cap_set_tristate(Object *obj,
+                                                            Visitor *v,
+                                                            const char *name,
+                                                            void *opaque,
+                                                            Error **errp)
+{
+    sPAPRCapabilityInfo *cap = opaque;
+    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+    char *val;
+    Error *local_err = NULL;
+    uint8_t value;
+
+    visit_type_str(v, name, &val, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (!strcasecmp(val, "broken")) {
+        value = SPAPR_CAP_BROKEN;
+    } else if (!strcasecmp(val, "workaround")) {
+        value = SPAPR_CAP_WORKAROUND;
+    } else if (!strcasecmp(val, "fixed")) {
+        value = SPAPR_CAP_FIXED;
+    } else {
+        error_setg(errp, "Invalid capability mode \"%s\" for cap-%s", val,
+                   cap->name);
+        goto out;
+    }
+
+    spapr->cmd_line_caps[cap->index] = true;
+    spapr->eff.caps[cap->index] = value;
+out:
+    g_free(val);
+}
+
 static void cap_htm_apply(sPAPRMachineState *spapr, uint8_t val, Error **errp)
 {
     if (!val) {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index eded0ea57d..61bb3632c4 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -69,6 +69,10 @@ typedef enum {
 /* Bool Caps */
 #define SPAPR_CAP_OFF                   0x00
 #define SPAPR_CAP_ON                    0x01
+/* Broken | Workaround | Fixed Caps */
+#define SPAPR_CAP_BROKEN                0x00
+#define SPAPR_CAP_WORKAROUND            0x01
+#define SPAPR_CAP_FIXED                 0x02
 
 typedef struct sPAPRCapabilities sPAPRCapabilities;
 struct sPAPRCapabilities {
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 4/7] target/ppc/spapr_caps: Add new tristate cap safe_cache

[Suraj Jitindar Singh]

Add new tristate cap cap-cfpc to represent the cache flush on privilege
change capability.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

---

V4 -> V5:
 - Use macro to generate migration state
---
 hw/ppc/spapr.c         |  2 ++
 hw/ppc/spapr_caps.c    | 36 ++++++++++++++++++++++++++----------
 include/hw/ppc/spapr.h |  5 ++++-
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 02e45dd010..ff8f1aeb59 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1791,6 +1791,7 @@ static const VMStateDescription vmstate_spapr = {
         &vmstate_spapr_cap_htm,
         &vmstate_spapr_cap_vsx,
         &vmstate_spapr_cap_dfp,
+        &vmstate_spapr_cap_cfpc,
         NULL
     }
 };
@@ -3878,6 +3879,7 @@ 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;
     spapr_caps_add_properties(smc, &error_abort);
 }
 
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index d6f82b18fc..d53da63b1c 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -73,11 +73,8 @@ static void spapr_cap_set_bool(Object *obj, Visitor *v, const char *name,
     spapr->eff.caps[cap->index] = value ? SPAPR_CAP_ON : SPAPR_CAP_OFF;
 }
 
-static void __attribute__ ((unused)) spapr_cap_get_tristate(Object *obj,
-                                                            Visitor *v,
-                                                            const char *name,
-                                                            void *opaque,
-                                                            Error **errp)
+static void spapr_cap_get_tristate(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
 {
     sPAPRCapabilityInfo *cap = opaque;
     sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
@@ -103,11 +100,8 @@ static void __attribute__ ((unused)) spapr_cap_get_tristate(Object *obj,
     g_free(val);
 }
 
-static void __attribute__ ((unused)) spapr_cap_set_tristate(Object *obj,
-                                                            Visitor *v,
-                                                            const char *name,
-                                                            void *opaque,
-                                                            Error **errp)
+static void spapr_cap_set_tristate(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
 {
     sPAPRCapabilityInfo *cap = opaque;
     sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
@@ -186,6 +180,18 @@ static void cap_dfp_apply(sPAPRMachineState *spapr, uint8_t val, Error **errp)
     }
 }
 
+static void cap_safe_cache_apply(sPAPRMachineState *spapr, uint8_t val,
+                                 Error **errp)
+{
+    if (tcg_enabled() && val) {
+        /* TODO - for now only allow broken for TCG */
+        error_setg(errp, "Requested safe cache capability level not supported by tcg, try a different value for cap-cfpc");
+    } else if (kvm_enabled() && (val > kvmppc_get_cap_safe_cache())) {
+        error_setg(errp, "Requested safe cache capability level not supported by kvm, try a different value for cap-cfpc");
+    }
+}
+
+#define VALUE_DESC_TRISTATE     " (broken, workaround, fixed)"
 
 sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
     [SPAPR_CAP_HTM] = {
@@ -215,6 +221,15 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
         .type = "bool",
         .apply = cap_dfp_apply,
     },
+    [SPAPR_CAP_CFPC] = {
+        .name = "cfpc",
+        .description = "Cache Flush on Privilege Change" VALUE_DESC_TRISTATE,
+        .index = SPAPR_CAP_CFPC,
+        .get = spapr_cap_get_tristate,
+        .set = spapr_cap_set_tristate,
+        .type = "string",
+        .apply = cap_safe_cache_apply,
+    },
 };
 
 static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
@@ -320,6 +335,7 @@ const VMStateDescription vmstate_spapr_cap_##cap = {    \
 SPAPR_CAP_MIG_STATE(htm, HTM);
 SPAPR_CAP_MIG_STATE(vsx, VSX);
 SPAPR_CAP_MIG_STATE(dfp, DFP);
+SPAPR_CAP_MIG_STATE(cfpc, CFPC);
 
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 61bb3632c4..ff476693d1 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -60,8 +60,10 @@ typedef enum {
 #define SPAPR_CAP_VSX                   0x01
 /* Decimal Floating Point */
 #define SPAPR_CAP_DFP                   0x02
+/* Cache Flush on Privilege Change */
+#define SPAPR_CAP_CFPC                  0x03
 /* Num Caps */
-#define SPAPR_CAP_NUM                   (SPAPR_CAP_DFP + 1)
+#define SPAPR_CAP_NUM                   (SPAPR_CAP_CFPC + 1)
 
 /*
  * Capability Values
@@ -779,6 +781,7 @@ int spapr_caps_pre_save(void *opaque);
 extern const VMStateDescription vmstate_spapr_cap_htm;
 extern const VMStateDescription vmstate_spapr_cap_vsx;
 extern const VMStateDescription vmstate_spapr_cap_dfp;
+extern const VMStateDescription vmstate_spapr_cap_cfpc;
 
 static inline uint8_t spapr_get_cap(sPAPRMachineState *spapr, int cap)
 {
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 5/7] target/ppc/spapr_caps: Add new tristate cap safe_bounds_check

[Suraj Jitindar Singh]

Add new tristate cap cap-sbbc to represent the speculation barrier
bounds checking capability.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

---

V4 -> V5:
 - Use macro to generate migration state
---
 hw/ppc/spapr.c         |  2 ++
 hw/ppc/spapr_caps.c    | 21 +++++++++++++++++++++
 include/hw/ppc/spapr.h |  5 ++++-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ff8f1aeb59..4cbed31b16 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1792,6 +1792,7 @@ static const VMStateDescription vmstate_spapr = {
         &vmstate_spapr_cap_vsx,
         &vmstate_spapr_cap_dfp,
         &vmstate_spapr_cap_cfpc,
+        &vmstate_spapr_cap_sbbc,
         NULL
     }
 };
@@ -3880,6 +3881,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     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;
     spapr_caps_add_properties(smc, &error_abort);
 }
 
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index d53da63b1c..ce1f74f26d 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -191,6 +191,17 @@ static void cap_safe_cache_apply(sPAPRMachineState *spapr, uint8_t val,
     }
 }
 
+static void cap_safe_bounds_check_apply(sPAPRMachineState *spapr, uint8_t val,
+                                        Error **errp)
+{
+    if (tcg_enabled() && val) {
+        /* TODO - for now only allow broken for TCG */
+        error_setg(errp, "Requested safe bounds check capability level not supported by tcg, try a different value for cap-sbbc");
+    } else if (kvm_enabled() && (val > kvmppc_get_cap_safe_bounds_check())) {
+        error_setg(errp, "Requested safe bounds check capability level not supported by kvm, try a different value for cap-sbbc");
+    }
+}
+
 #define VALUE_DESC_TRISTATE     " (broken, workaround, fixed)"
 
 sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
@@ -230,6 +241,15 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
         .type = "string",
         .apply = cap_safe_cache_apply,
     },
+    [SPAPR_CAP_SBBC] = {
+        .name = "sbbc",
+        .description = "Speculation Barrier Bounds Checking" VALUE_DESC_TRISTATE,
+        .index = SPAPR_CAP_SBBC,
+        .get = spapr_cap_get_tristate,
+        .set = spapr_cap_set_tristate,
+        .type = "string",
+        .apply = cap_safe_bounds_check_apply,
+    },
 };
 
 static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
@@ -336,6 +356,7 @@ 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);
 
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ff476693d1..c65be62e92 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -62,8 +62,10 @@ typedef enum {
 #define SPAPR_CAP_DFP                   0x02
 /* Cache Flush on Privilege Change */
 #define SPAPR_CAP_CFPC                  0x03
+/* Speculation Barrier Bounds Checking */
+#define SPAPR_CAP_SBBC                  0x04
 /* Num Caps */
-#define SPAPR_CAP_NUM                   (SPAPR_CAP_CFPC + 1)
+#define SPAPR_CAP_NUM                   (SPAPR_CAP_SBBC + 1)
 
 /*
  * Capability Values
@@ -782,6 +784,7 @@ extern const VMStateDescription vmstate_spapr_cap_htm;
 extern const VMStateDescription vmstate_spapr_cap_vsx;
 extern const VMStateDescription vmstate_spapr_cap_dfp;
 extern const VMStateDescription vmstate_spapr_cap_cfpc;
+extern const VMStateDescription vmstate_spapr_cap_sbbc;
 
 static inline uint8_t spapr_get_cap(sPAPRMachineState *spapr, int cap)
 {
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 6/7] target/ppc/spapr_caps: Add new tristate cap safe_indirect_branch

[Suraj Jitindar Singh]

Add new tristate cap cap-ibs to represent the indirect branch
serialisation capability.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

---

V4 -> V5:
 - Use macro to generate migration state
---
 hw/ppc/spapr.c         |  2 ++
 hw/ppc/spapr_caps.c    | 21 +++++++++++++++++++++
 include/hw/ppc/spapr.h |  5 ++++-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4cbed31b16..5544eeb853 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1793,6 +1793,7 @@ static const VMStateDescription vmstate_spapr = {
         &vmstate_spapr_cap_dfp,
         &vmstate_spapr_cap_cfpc,
         &vmstate_spapr_cap_sbbc,
+        &vmstate_spapr_cap_ibs,
         NULL
     }
 };
@@ -3882,6 +3883,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;
     spapr_caps_add_properties(smc, &error_abort);
 }
 
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index ce1f74f26d..62efdaee38 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -202,6 +202,17 @@ static void cap_safe_bounds_check_apply(sPAPRMachineState *spapr, uint8_t val,
     }
 }
 
+static void cap_safe_indirect_branch_apply(sPAPRMachineState *spapr,
+                                           uint8_t val, Error **errp)
+{
+    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");
+    } 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");
+    }
+}
+
 #define VALUE_DESC_TRISTATE     " (broken, workaround, fixed)"
 
 sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
@@ -250,6 +261,15 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
         .type = "string",
         .apply = cap_safe_bounds_check_apply,
     },
+    [SPAPR_CAP_IBS] = {
+        .name = "ibs",
+        .description = "Indirect Branch Serialisation" VALUE_DESC_TRISTATE,
+        .index = SPAPR_CAP_IBS,
+        .get = spapr_cap_get_tristate,
+        .set = spapr_cap_set_tristate,
+        .type = "string",
+        .apply = cap_safe_indirect_branch_apply,
+    },
 };
 
 static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
@@ -357,6 +377,7 @@ 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);
 
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index c65be62e92..549d7a4134 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -64,8 +64,10 @@ typedef enum {
 #define SPAPR_CAP_CFPC                  0x03
 /* Speculation Barrier Bounds Checking */
 #define SPAPR_CAP_SBBC                  0x04
+/* Indirect Branch Serialisation */
+#define SPAPR_CAP_IBS                   0x05
 /* Num Caps */
-#define SPAPR_CAP_NUM                   (SPAPR_CAP_SBBC + 1)
+#define SPAPR_CAP_NUM                   (SPAPR_CAP_IBS + 1)
 
 /*
  * Capability Values
@@ -785,6 +787,7 @@ extern const VMStateDescription vmstate_spapr_cap_vsx;
 extern const VMStateDescription vmstate_spapr_cap_dfp;
 extern const VMStateDescription vmstate_spapr_cap_cfpc;
 extern const VMStateDescription vmstate_spapr_cap_sbbc;
+extern const VMStateDescription vmstate_spapr_cap_ibs;
 
 static inline uint8_t spapr_get_cap(sPAPRMachineState *spapr, int cap)
 {
-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V5 7/7] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[Suraj Jitindar Singh]

The new H-Call H_GET_CPU_CHARACTERISTICS is used by the guest to query
behaviours and available characteristics of the cpu.

Implement the handler for this new H-Call which formulates its response
based on the setting of the spapr_caps cap-cfpc, cap-sbbc and cap-ibs.

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

---

V4 -> V5:
 - Assert for invalid cap value
---
 hw/ppc/spapr_hcall.c   | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 59 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 51eba52e86..4d0e6eb0cf 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1654,6 +1654,60 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     return H_SUCCESS;
 }
 
+static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
+                                              sPAPRMachineState *spapr,
+                                              target_ulong opcode,
+                                              target_ulong *args)
+{
+    uint64_t characteristics = H_CPU_CHAR_HON_BRANCH_HINTS &
+                               ~H_CPU_CHAR_THR_RECONF_TRIG;
+    uint64_t behaviour = H_CPU_BEHAV_FAVOUR_SECURITY;
+    uint8_t safe_cache = spapr_get_cap(spapr, SPAPR_CAP_CFPC);
+    uint8_t safe_bounds_check = spapr_get_cap(spapr, SPAPR_CAP_SBBC);
+    uint8_t safe_indirect_branch = spapr_get_cap(spapr, SPAPR_CAP_IBS);
+
+    switch (safe_cache) {
+    case SPAPR_CAP_WORKAROUND:
+        characteristics |= H_CPU_CHAR_L1D_FLUSH_ORI30;
+        characteristics |= H_CPU_CHAR_L1D_FLUSH_TRIG2;
+        characteristics |= H_CPU_CHAR_L1D_THREAD_PRIV;
+        behaviour |= H_CPU_BEHAV_L1D_FLUSH_PR;
+        break;
+    case SPAPR_CAP_FIXED:
+        break;
+    default: /* broken */
+        assert(safe_cache == SPAPR_CAP_BROKEN);
+        behaviour |= H_CPU_BEHAV_L1D_FLUSH_PR;
+        break;
+    }
+
+    switch (safe_bounds_check) {
+    case SPAPR_CAP_WORKAROUND:
+        characteristics |= H_CPU_CHAR_SPEC_BAR_ORI31;
+        behaviour |= H_CPU_BEHAV_BNDS_CHK_SPEC_BAR;
+        break;
+    case SPAPR_CAP_FIXED:
+        break;
+    default: /* broken */
+        assert(safe_bounds_check == SPAPR_CAP_BROKEN);
+        behaviour |= H_CPU_BEHAV_BNDS_CHK_SPEC_BAR;
+        break;
+    }
+
+    switch (safe_indirect_branch) {
+    case SPAPR_CAP_FIXED:
+        characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
+    default: /* broken */
+        assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
+        break;
+    }
+
+    args[0] = characteristics;
+    args[1] = behaviour;
+
+    return H_SUCCESS;
+}
+
 static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
 static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1];
 
@@ -1733,6 +1787,10 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(H_INVALIDATE_PID, h_invalidate_pid);
     spapr_register_hypercall(H_REGISTER_PROC_TBL, h_register_process_table);
 
+    /* hcall-get-cpu-characteristics */
+    spapr_register_hypercall(H_GET_CPU_CHARACTERISTICS,
+                             h_get_cpu_characteristics);
+
     /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
      * here between the "CI" and the "CACHE" variants, they will use whatever
      * mapping attributes qemu is using. When using KVM, the kernel will
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 549d7a4134..62c077ac20 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -404,6 +404,7 @@ struct sPAPRMachineState {
 #define H_GET_HCA_INFO          0x1B8
 #define H_GET_PERF_COUNT        0x1BC
 #define H_MANAGE_TRACE          0x1C0
+#define H_GET_CPU_CHARACTERISTICS 0x1C8
 #define H_FREE_LOGICAL_LAN_BUFFER 0x1D4
 #define H_QUERY_INT_STATE       0x1E4
 #define H_POLL_PENDING          0x1D8
-- 
2.13.6

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate

[David Gibson]

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

On Fri, Jan 19, 2018 at 04:00:00PM +1100, Suraj Jitindar Singh wrote:
> The vmstate description and the contained needed function for migration
> of spapr_caps is the same for each cap, with the name of the cap
> substituted. As such introduce a macro to allow for easier generation of
> these.
> 
> Convert the three existing spapr_caps (htm, vsx, and dfp) to use this
> macro.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Applied to ppc-for-2.12, thanks.

> 
> ---
> 
> V5:
>  - Patch added to series
> ---
>  hw/ppc/spapr_caps.c | 78 +++++++++++++++++------------------------------------
>  1 file changed, 24 insertions(+), 54 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index d5c9ce774a..5d52969bd5 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -228,62 +228,32 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr)
>      return ok ? 0 : -EINVAL;
>  }
>  
> -static bool spapr_cap_htm_needed(void *opaque)
> -{
> -    sPAPRMachineState *spapr = opaque;
> -
> -    return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
> -           (spapr->eff.caps[SPAPR_CAP_HTM] != spapr->def.caps[SPAPR_CAP_HTM]);
> -}
> -
> -const VMStateDescription vmstate_spapr_cap_htm = {
> -    .name = "spapr/cap/htm",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .needed = spapr_cap_htm_needed,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
> -        VMSTATE_END_OF_LIST()
> -    },
> -};
> -
> -static bool spapr_cap_vsx_needed(void *opaque)
> -{
> -    sPAPRMachineState *spapr = opaque;
> -
> -    return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
> -           (spapr->eff.caps[SPAPR_CAP_VSX] != spapr->def.caps[SPAPR_CAP_VSX]);
> +/* 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)      \
> +{                                                       \
> +    sPAPRMachineState *spapr = opaque;                  \
> +                                                        \
> +    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
> +           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
> +            spapr->def.caps[SPAPR_CAP_##ccap]);         \
> +}                                                       \
> +                                                        \
> +const VMStateDescription vmstate_spapr_cap_##cap = {    \
> +    .name = "spapr/cap/" #cap,                          \
> +    .version_id = 1,                                    \
> +    .minimum_version_id = 1,                            \
> +    .needed = spapr_cap_##cap##_needed,                 \
> +    .fields = (VMStateField[]) {                        \
> +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
> +                      sPAPRMachineState),               \
> +        VMSTATE_END_OF_LIST()                           \
> +    },                                                  \
>  }
>  
> -const VMStateDescription vmstate_spapr_cap_vsx = {
> -    .name = "spapr/cap/vsx",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .needed = spapr_cap_vsx_needed,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
> -        VMSTATE_END_OF_LIST()
> -    },
> -};
> -
> -static bool spapr_cap_dfp_needed(void *opaque)
> -{
> -    sPAPRMachineState *spapr = opaque;
> -
> -    return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
> -           (spapr->eff.caps[SPAPR_CAP_DFP] != spapr->def.caps[SPAPR_CAP_DFP]);
> -}
> -
> -const VMStateDescription vmstate_spapr_cap_dfp = {
> -    .name = "spapr/cap/dfp",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .needed = spapr_cap_dfp_needed,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
> -        VMSTATE_END_OF_LIST()
> -    },
> -};
> +SPAPR_CAP_MIG_STATE(htm, HTM);
> +SPAPR_CAP_MIG_STATE(vsx, VSX);
> +SPAPR_CAP_MIG_STATE(dfp, DFP);
>  
>  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 --]

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate

[Alexey Kardashevskiy]

On 19/01/18 16:00, Suraj Jitindar Singh wrote:
> The vmstate description and the contained needed function for migration
> of spapr_caps is the same for each cap, with the name of the cap
> substituted. As such introduce a macro to allow for easier generation of
> these.
> 
> Convert the three existing spapr_caps (htm, vsx, and dfp) to use this
> macro.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> 
> ---
> 
> V5:
>  - Patch added to series
> ---
>  hw/ppc/spapr_caps.c | 78 +++++++++++++++++------------------------------------
>  1 file changed, 24 insertions(+), 54 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index d5c9ce774a..5d52969bd5 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -228,62 +228,32 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr)
>      return ok ? 0 : -EINVAL;
>  }
>  
> -static bool spapr_cap_htm_needed(void *opaque)
> -{
> -    sPAPRMachineState *spapr = opaque;
> -
> -    return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
> -           (spapr->eff.caps[SPAPR_CAP_HTM] != spapr->def.caps[SPAPR_CAP_HTM]);
> -}
> -
> -const VMStateDescription vmstate_spapr_cap_htm = {
> -    .name = "spapr/cap/htm",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .needed = spapr_cap_htm_needed,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
> -        VMSTATE_END_OF_LIST()
> -    },
> -};
> -
> -static bool spapr_cap_vsx_needed(void *opaque)
> -{
> -    sPAPRMachineState *spapr = opaque;
> -
> -    return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
> -           (spapr->eff.caps[SPAPR_CAP_VSX] != spapr->def.caps[SPAPR_CAP_VSX]);
> +/* 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)      \
> +{                                                       \
> +    sPAPRMachineState *spapr = opaque;                  \
> +                                                        \
> +    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
> +           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
> +            spapr->def.caps[SPAPR_CAP_##ccap]);         \
> +}                                                       \
> +                                                        \
> +const VMStateDescription vmstate_spapr_cap_##cap = {    \
> +    .name = "spapr/cap/" #cap,                          \
> +    .version_id = 1,                                    \
> +    .minimum_version_id = 1,                            \
> +    .needed = spapr_cap_##cap##_needed,                 \
> +    .fields = (VMStateField[]) {                        \
> +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
> +                      sPAPRMachineState),               \
> +        VMSTATE_END_OF_LIST()                           \
> +    },                                                  \
>  }
>  
> -const VMStateDescription vmstate_spapr_cap_vsx = {
> -    .name = "spapr/cap/vsx",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .needed = spapr_cap_vsx_needed,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
> -        VMSTATE_END_OF_LIST()
> -    },
> -};
> -
> -static bool spapr_cap_dfp_needed(void *opaque)
> -{
> -    sPAPRMachineState *spapr = opaque;
> -
> -    return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
> -           (spapr->eff.caps[SPAPR_CAP_DFP] != spapr->def.caps[SPAPR_CAP_DFP]);
> -}
> -
> -const VMStateDescription vmstate_spapr_cap_dfp = {
> -    .name = "spapr/cap/dfp",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .needed = spapr_cap_dfp_needed,
> -    .fields = (VMStateField[]) {
> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
> -        VMSTATE_END_OF_LIST()
> -    },
> -};
> +SPAPR_CAP_MIG_STATE(htm, HTM);
> +SPAPR_CAP_MIG_STATE(vsx, VSX);
> +SPAPR_CAP_MIG_STATE(dfp, DFP);


A nit: I really dislike this kind of parameters which you cannot really
cscope/ctags for because they are joined to some other things.

"no matches found for cscope query g HTM of HTM" says vim's cscope when I
search for "HTM".


>  
>  void spapr_caps_reset(sPAPRMachineState *spapr)
>  {
> 


-- 
Alexey

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180119050005.29392-1-sjitindarsingh@gmail.com
Subject: [Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
dc317bcbc5 target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS
09c0dbcab4 target/ppc/spapr_caps: Add new tristate cap safe_indirect_branch
1145f7696a target/ppc/spapr_caps: Add new tristate cap safe_bounds_check
37c3445619 target/ppc/spapr_caps: Add new tristate cap safe_cache
0a8ff160dc target/ppc/spapr_caps: Add support for tristate spapr_capabilities
8aef783b9e target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate
09ad7f2093 target/ppc/kvm: Add cap_ppc_safe_[cache/bounds_check/indirect_branch]

=== OUTPUT BEGIN ===
Checking PATCH 1/7: target/ppc/kvm: Add cap_ppc_safe_[cache/bounds_check/indirect_branch]...
Checking PATCH 2/7: target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate...
Checking PATCH 3/7: target/ppc/spapr_caps: Add support for tristate spapr_capabilities...
Checking PATCH 4/7: target/ppc/spapr_caps: Add new tristate cap safe_cache...
ERROR: line over 90 characters
#75: FILE: hw/ppc/spapr_caps.c:188:
+        error_setg(errp, "Requested safe cache capability level not supported by tcg, try a different value for cap-cfpc");

ERROR: line over 90 characters
#77: FILE: hw/ppc/spapr_caps.c:190:
+        error_setg(errp, "Requested safe cache capability level not supported by kvm, try a different value for cap-cfpc");

total: 2 errors, 0 warnings, 98 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/7: target/ppc/spapr_caps: Add new tristate cap safe_bounds_check...
ERROR: line over 90 characters
#47: FILE: hw/ppc/spapr_caps.c:199:
+        error_setg(errp, "Requested safe bounds check capability level not supported by tcg, try a different value for cap-sbbc");

ERROR: line over 90 characters
#49: FILE: hw/ppc/spapr_caps.c:201:
+        error_setg(errp, "Requested safe bounds check capability level not supported by kvm, try a different value for cap-sbbc");

WARNING: line over 80 characters
#62: FILE: hw/ppc/spapr_caps.c:246:
+        .description = "Speculation Barrier Bounds Checking" VALUE_DESC_TRISTATE,

total: 2 errors, 1 warnings, 71 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/7: target/ppc/spapr_caps: Add new tristate cap safe_indirect_branch...
ERROR: line over 90 characters
#48: FILE: hw/ppc/spapr_caps.c:210:
+        error_setg(errp, "Requested safe indirect branch capability level not supported by tcg, try a different value for cap-ibs");

ERROR: line over 90 characters
#50: FILE: hw/ppc/spapr_caps.c:212:
+        error_setg(errp, "Requested safe indirect branch capability level not supported by kvm, try a different value for cap-ibs");

total: 2 errors, 0 warnings, 71 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 7/7: target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate

[Suraj Jitindar Singh]

On Fri, 2018-01-19 at 16:18 +1100, Alexey Kardashevskiy wrote:
> On 19/01/18 16:00, Suraj Jitindar Singh wrote:
> > The vmstate description and the contained needed function for
> > migration
> > of spapr_caps is the same for each cap, with the name of the cap
> > substituted. As such introduce a macro to allow for easier
> > generation of
> > these.
> > 
> > Convert the three existing spapr_caps (htm, vsx, and dfp) to use
> > this
> > macro.
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > 
> > ---
> > 
> > V5:
> >  - Patch added to series
> > ---
> >  hw/ppc/spapr_caps.c | 78 +++++++++++++++++----------------------
> > --------------
> >  1 file changed, 24 insertions(+), 54 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > index d5c9ce774a..5d52969bd5 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -228,62 +228,32 @@ int
> > spapr_caps_post_migration(sPAPRMachineState *spapr)
> >      return ok ? 0 : -EINVAL;
> >  }
> >  
> > -static bool spapr_cap_htm_needed(void *opaque)
> > -{
> > -    sPAPRMachineState *spapr = opaque;
> > -
> > -    return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
> > -           (spapr->eff.caps[SPAPR_CAP_HTM] != spapr-
> > >def.caps[SPAPR_CAP_HTM]);
> > -}
> > -
> > -const VMStateDescription vmstate_spapr_cap_htm = {
> > -    .name = "spapr/cap/htm",
> > -    .version_id = 1,
> > -    .minimum_version_id = 1,
> > -    .needed = spapr_cap_htm_needed,
> > -    .fields = (VMStateField[]) {
> > -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
> > -        VMSTATE_END_OF_LIST()
> > -    },
> > -};
> > -
> > -static bool spapr_cap_vsx_needed(void *opaque)
> > -{
> > -    sPAPRMachineState *spapr = opaque;
> > -
> > -    return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
> > -           (spapr->eff.caps[SPAPR_CAP_VSX] != spapr-
> > >def.caps[SPAPR_CAP_VSX]);
> > +/* 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)      \
> > +{                                                       \
> > +    sPAPRMachineState *spapr = opaque;                  \
> > +                                                        \
> > +    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
> > +           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
> > +            spapr->def.caps[SPAPR_CAP_##ccap]);         \
> > +}                                                       \
> > +                                                        \
> > +const VMStateDescription vmstate_spapr_cap_##cap = {    \
> > +    .name = "spapr/cap/" #cap,                          \
> > +    .version_id = 1,                                    \
> > +    .minimum_version_id = 1,                            \
> > +    .needed = spapr_cap_##cap##_needed,                 \
> > +    .fields = (VMStateField[]) {                        \
> > +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
> > +                      sPAPRMachineState),               \
> > +        VMSTATE_END_OF_LIST()                           \
> > +    },                                                  \
> >  }
> >  
> > -const VMStateDescription vmstate_spapr_cap_vsx = {
> > -    .name = "spapr/cap/vsx",
> > -    .version_id = 1,
> > -    .minimum_version_id = 1,
> > -    .needed = spapr_cap_vsx_needed,
> > -    .fields = (VMStateField[]) {
> > -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
> > -        VMSTATE_END_OF_LIST()
> > -    },
> > -};
> > -
> > -static bool spapr_cap_dfp_needed(void *opaque)
> > -{
> > -    sPAPRMachineState *spapr = opaque;
> > -
> > -    return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
> > -           (spapr->eff.caps[SPAPR_CAP_DFP] != spapr-
> > >def.caps[SPAPR_CAP_DFP]);
> > -}
> > -
> > -const VMStateDescription vmstate_spapr_cap_dfp = {
> > -    .name = "spapr/cap/dfp",
> > -    .version_id = 1,
> > -    .minimum_version_id = 1,
> > -    .needed = spapr_cap_dfp_needed,
> > -    .fields = (VMStateField[]) {
> > -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
> > -        VMSTATE_END_OF_LIST()
> > -    },
> > -};
> > +SPAPR_CAP_MIG_STATE(htm, HTM);
> > +SPAPR_CAP_MIG_STATE(vsx, VSX);
> > +SPAPR_CAP_MIG_STATE(dfp, DFP);
> 
> 
> A nit: I really dislike this kind of parameters which you cannot
> really
> cscope/ctags for because they are joined to some other things.
> 
> "no matches found for cscope query g HTM of HTM" says vim's cscope
> when I
> search for "HTM".

Would something like:
SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM);
SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_VSX);
SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_DFP);

be better?
I could send a follow up

> 
> 
> >  
> >  void spapr_caps_reset(sPAPRMachineState *spapr)
> >  {
> > 
> 
> 

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps

[David Gibson]

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

On Fri, Jan 19, 2018 at 03:59:58PM +1100, Suraj Jitindar Singh wrote:
> The following patch series adds 3 new tristate capabilities and their
> associated handling.
> 
> A new H-Call is implemented which a guest will use to query the 
> requirement for and availability of workarounds for certain cpu 
> behaviours.
> 
> Applies on top of David's tree: ppc-for-2.12

Applied to ppc-for-2.12 (except that the kernel header updates from
1/7 were dropped, since they were already included by Cornelia's
general update in 9cbb6362.

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

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate

[Alexey Kardashevskiy]

On 29/01/18 10:43, Suraj Jitindar Singh wrote:
> On Fri, 2018-01-19 at 16:18 +1100, Alexey Kardashevskiy wrote:
>> On 19/01/18 16:00, Suraj Jitindar Singh wrote:
>>> The vmstate description and the contained needed function for
>>> migration
>>> of spapr_caps is the same for each cap, with the name of the cap
>>> substituted. As such introduce a macro to allow for easier
>>> generation of
>>> these.
>>>
>>> Convert the three existing spapr_caps (htm, vsx, and dfp) to use
>>> this
>>> macro.
>>>
>>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>>
>>> ---
>>>
>>> V5:
>>>  - Patch added to series
>>> ---
>>>  hw/ppc/spapr_caps.c | 78 +++++++++++++++++----------------------
>>> --------------
>>>  1 file changed, 24 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>>> index d5c9ce774a..5d52969bd5 100644
>>> --- a/hw/ppc/spapr_caps.c
>>> +++ b/hw/ppc/spapr_caps.c
>>> @@ -228,62 +228,32 @@ int
>>> spapr_caps_post_migration(sPAPRMachineState *spapr)
>>>      return ok ? 0 : -EINVAL;
>>>  }
>>>  
>>> -static bool spapr_cap_htm_needed(void *opaque)
>>> -{
>>> -    sPAPRMachineState *spapr = opaque;
>>> -
>>> -    return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
>>> -           (spapr->eff.caps[SPAPR_CAP_HTM] != spapr-
>>>> def.caps[SPAPR_CAP_HTM]);
>>> -}
>>> -
>>> -const VMStateDescription vmstate_spapr_cap_htm = {
>>> -    .name = "spapr/cap/htm",
>>> -    .version_id = 1,
>>> -    .minimum_version_id = 1,
>>> -    .needed = spapr_cap_htm_needed,
>>> -    .fields = (VMStateField[]) {
>>> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
>>> -        VMSTATE_END_OF_LIST()
>>> -    },
>>> -};
>>> -
>>> -static bool spapr_cap_vsx_needed(void *opaque)
>>> -{
>>> -    sPAPRMachineState *spapr = opaque;
>>> -
>>> -    return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
>>> -           (spapr->eff.caps[SPAPR_CAP_VSX] != spapr-
>>>> def.caps[SPAPR_CAP_VSX]);
>>> +/* 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)      \
>>> +{                                                       \
>>> +    sPAPRMachineState *spapr = opaque;                  \
>>> +                                                        \
>>> +    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
>>> +           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
>>> +            spapr->def.caps[SPAPR_CAP_##ccap]);         \
>>> +}                                                       \
>>> +                                                        \
>>> +const VMStateDescription vmstate_spapr_cap_##cap = {    \
>>> +    .name = "spapr/cap/" #cap,                          \
>>> +    .version_id = 1,                                    \
>>> +    .minimum_version_id = 1,                            \
>>> +    .needed = spapr_cap_##cap##_needed,                 \
>>> +    .fields = (VMStateField[]) {                        \
>>> +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
>>> +                      sPAPRMachineState),               \
>>> +        VMSTATE_END_OF_LIST()                           \
>>> +    },                                                  \
>>>  }
>>>  
>>> -const VMStateDescription vmstate_spapr_cap_vsx = {
>>> -    .name = "spapr/cap/vsx",
>>> -    .version_id = 1,
>>> -    .minimum_version_id = 1,
>>> -    .needed = spapr_cap_vsx_needed,
>>> -    .fields = (VMStateField[]) {
>>> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
>>> -        VMSTATE_END_OF_LIST()
>>> -    },
>>> -};
>>> -
>>> -static bool spapr_cap_dfp_needed(void *opaque)
>>> -{
>>> -    sPAPRMachineState *spapr = opaque;
>>> -
>>> -    return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
>>> -           (spapr->eff.caps[SPAPR_CAP_DFP] != spapr-
>>>> def.caps[SPAPR_CAP_DFP]);
>>> -}
>>> -
>>> -const VMStateDescription vmstate_spapr_cap_dfp = {
>>> -    .name = "spapr/cap/dfp",
>>> -    .version_id = 1,
>>> -    .minimum_version_id = 1,
>>> -    .needed = spapr_cap_dfp_needed,
>>> -    .fields = (VMStateField[]) {
>>> -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
>>> -        VMSTATE_END_OF_LIST()
>>> -    },
>>> -};
>>> +SPAPR_CAP_MIG_STATE(htm, HTM);
>>> +SPAPR_CAP_MIG_STATE(vsx, VSX);
>>> +SPAPR_CAP_MIG_STATE(dfp, DFP);
>>
>>
>> A nit: I really dislike this kind of parameters which you cannot
>> really
>> cscope/ctags for because they are joined to some other things.
>>
>> "no matches found for cscope query g HTM of HTM" says vim's cscope
>> when I
>> search for "HTM".
> 
> Would something like:
> SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM);
> SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_VSX);
> SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_DFP);
> 
> be better?

It would indeed.

> I could send a follow up

Too late, David already merged :( And this seems to bother only me anyway.


> 
>>
>>
>>>  
>>>  void spapr_caps_reset(sPAPRMachineState *spapr)
>>>  {
>>>
>>
>>


-- 
Alexey

Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate

[Suraj Jitindar Singh]

On Mon, 2018-01-29 at 14:03 +1100, Alexey Kardashevskiy wrote:
> On 29/01/18 10:43, Suraj Jitindar Singh wrote:
> > On Fri, 2018-01-19 at 16:18 +1100, Alexey Kardashevskiy wrote:
> > > On 19/01/18 16:00, Suraj Jitindar Singh wrote:
> > > > The vmstate description and the contained needed function for
> > > > migration
> > > > of spapr_caps is the same for each cap, with the name of the
> > > > cap
> > > > substituted. As such introduce a macro to allow for easier
> > > > generation of
> > > > these.
> > > > 
> > > > Convert the three existing spapr_caps (htm, vsx, and dfp) to
> > > > use
> > > > this
> > > > macro.
> > > > 
> > > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > > > 
> > > > ---
> > > > 
> > > > V5:
> > > >  - Patch added to series
> > > > ---
> > > >  hw/ppc/spapr_caps.c | 78 +++++++++++++++++------------------
> > > > ----
> > > > --------------
> > > >  1 file changed, 24 insertions(+), 54 deletions(-)
> > > > 
> > > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > > > index d5c9ce774a..5d52969bd5 100644
> > > > --- a/hw/ppc/spapr_caps.c
> > > > +++ b/hw/ppc/spapr_caps.c
> > > > @@ -228,62 +228,32 @@ int
> > > > spapr_caps_post_migration(sPAPRMachineState *spapr)
> > > >      return ok ? 0 : -EINVAL;
> > > >  }
> > > >  
> > > > -static bool spapr_cap_htm_needed(void *opaque)
> > > > -{
> > > > -    sPAPRMachineState *spapr = opaque;
> > > > -
> > > > -    return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
> > > > -           (spapr->eff.caps[SPAPR_CAP_HTM] != spapr-
> > > > > def.caps[SPAPR_CAP_HTM]);
> > > > 
> > > > -}
> > > > -
> > > > -const VMStateDescription vmstate_spapr_cap_htm = {
> > > > -    .name = "spapr/cap/htm",
> > > > -    .version_id = 1,
> > > > -    .minimum_version_id = 1,
> > > > -    .needed = spapr_cap_htm_needed,
> > > > -    .fields = (VMStateField[]) {
> > > > -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM],
> > > > sPAPRMachineState),
> > > > -        VMSTATE_END_OF_LIST()
> > > > -    },
> > > > -};
> > > > -
> > > > -static bool spapr_cap_vsx_needed(void *opaque)
> > > > -{
> > > > -    sPAPRMachineState *spapr = opaque;
> > > > -
> > > > -    return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
> > > > -           (spapr->eff.caps[SPAPR_CAP_VSX] != spapr-
> > > > > def.caps[SPAPR_CAP_VSX]);
> > > > 
> > > > +/* 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)      \
> > > > +{                                                       \
> > > > +    sPAPRMachineState *spapr = opaque;                  \
> > > > +                                                        \
> > > > +    return spapr->cmd_line_caps[SPAPR_CAP_##ccap] &&    \
> > > > +           (spapr->eff.caps[SPAPR_CAP_##ccap] !=        \
> > > > +            spapr->def.caps[SPAPR_CAP_##ccap]);         \
> > > > +}                                                       \
> > > > +                                                        \
> > > > +const VMStateDescription vmstate_spapr_cap_##cap = {    \
> > > > +    .name = "spapr/cap/" #cap,                          \
> > > > +    .version_id = 1,                                    \
> > > > +    .minimum_version_id = 1,                            \
> > > > +    .needed = spapr_cap_##cap##_needed,                 \
> > > > +    .fields = (VMStateField[]) {                        \
> > > > +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap],       \
> > > > +                      sPAPRMachineState),               \
> > > > +        VMSTATE_END_OF_LIST()                           \
> > > > +    },                                                  \
> > > >  }
> > > >  
> > > > -const VMStateDescription vmstate_spapr_cap_vsx = {
> > > > -    .name = "spapr/cap/vsx",
> > > > -    .version_id = 1,
> > > > -    .minimum_version_id = 1,
> > > > -    .needed = spapr_cap_vsx_needed,
> > > > -    .fields = (VMStateField[]) {
> > > > -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX],
> > > > sPAPRMachineState),
> > > > -        VMSTATE_END_OF_LIST()
> > > > -    },
> > > > -};
> > > > -
> > > > -static bool spapr_cap_dfp_needed(void *opaque)
> > > > -{
> > > > -    sPAPRMachineState *spapr = opaque;
> > > > -
> > > > -    return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
> > > > -           (spapr->eff.caps[SPAPR_CAP_DFP] != spapr-
> > > > > def.caps[SPAPR_CAP_DFP]);
> > > > 
> > > > -}
> > > > -
> > > > -const VMStateDescription vmstate_spapr_cap_dfp = {
> > > > -    .name = "spapr/cap/dfp",
> > > > -    .version_id = 1,
> > > > -    .minimum_version_id = 1,
> > > > -    .needed = spapr_cap_dfp_needed,
> > > > -    .fields = (VMStateField[]) {
> > > > -        VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP],
> > > > sPAPRMachineState),
> > > > -        VMSTATE_END_OF_LIST()
> > > > -    },
> > > > -};
> > > > +SPAPR_CAP_MIG_STATE(htm, HTM);
> > > > +SPAPR_CAP_MIG_STATE(vsx, VSX);
> > > > +SPAPR_CAP_MIG_STATE(dfp, DFP);
> > > 
> > > 
> > > A nit: I really dislike this kind of parameters which you cannot
> > > really
> > > cscope/ctags for because they are joined to some other things.
> > > 
> > > "no matches found for cscope query g HTM of HTM" says vim's
> > > cscope
> > > when I
> > > search for "HTM".
> > 
> > Would something like:
> > SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM);
> > SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_VSX);
> > SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_DFP);
> > 
> > be better?
> 
> It would indeed.
> 
> > I could send a follow up
> 
> Too late, David already merged :( And this seems to bother only me
> anyway.

I mean it's annoying anyway, I just didn't really think about it when
writing it. So might fix it up with another patch...

> 
> 
> > 
> > > 
> > > 
> > > >  
> > > >  void spapr_caps_reset(sPAPRMachineState *spapr)
> > > >  {
> > > > 
> > > 
> > > 
> 
>