[Qemu-devel] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Qemu-devel] [QEMU-PPC] [PATCH V3 0/6] 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 first patch from the previous revision has already been merged:
hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation

The main changes to V3 are:
- Split up the addition of the tristate caps into 5 patches
  - 1/6 query the caps from the hypervisor and parse the new return format
  - 2/6 add support for the new caps
  - 3-5/6 add each of the three new caps
- Patch 6/6 Unchanged

Suraj Jitindar Singh (6):
  target/ppc/kvm: Add cap_ppc_safe_[cache/bounds_check/indirect_branch]
  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             | 178 ++++++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_hcall.c            |  66 +++++++++++++++
 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, 364 insertions(+), 1 deletion(-)

-- 
2.13.6

[Qemu-devel] [QEMU-PPC] [PATCH V3 1/6] 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>
---
 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..8cce855cab 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.
@@ -372,6 +377,44 @@ struct ppc_radix_page_info *kvm_get_radix_page_info(void)
     return radix_page_info;
 }
 
+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;
+    }
+}
+
 target_ulong kvmppc_configure_v3_mmu(PowerPCCPU *cpu,
                                      bool radix, bool gtse,
                                      uint64_t proc_tbl)
@@ -2456,6 +2499,21 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
     return cap_mmu_hash_v3;
 }
 
+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 V4 1/6] 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>
---

V3 -> V4:
- Move kvmppc_get_cpu_characteristics() function implementation to fix
  compilation on some targets.

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

Re: [Qemu-devel] [QEMU-PPC] [PATCH V4 1/6] target/ppc/kvm: Add cap_ppc_safe_[cache/bounds_check/indirect_branch]

[David Gibson]

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

On Mon, Jan 15, 2018 at 05:58:37PM +1100, Suraj Jitindar Singh wrote:
> 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>

Waiting for the kernel part to go in before merging, though.

> ---
> 
> V3 -> V4:
> - Move kvmppc_get_cpu_characteristics() function implementation to fix
>   compilation on some targets.
> 
> ---
>  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;

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

[Qemu-devel] [QEMU-PPC] [PATCH V3 2/6] 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>
---
 hw/ppc/spapr_caps.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  4 ++++
 2 files changed, 64 insertions(+)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index d5c9ce774a..436250d77b 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -73,6 +73,66 @@ 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 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 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

Re: [Qemu-devel] [QEMU-PPC] [PATCH V3 2/6] target/ppc/spapr_caps: Add support for tristate spapr_capabilities

[David Gibson]

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

On Mon, Jan 15, 2018 at 05:32:31PM +1100, Suraj Jitindar Singh wrote:
> 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>

Applied to ppc-for-2.12.

> ---
>  hw/ppc/spapr_caps.c    | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  4 ++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index d5c9ce774a..436250d77b 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -73,6 +73,66 @@ 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 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 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 {

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

[Qemu-devel] [QEMU-PPC] [PATCH V3 3/6] 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>
---
 hw/ppc/spapr.c         |  2 ++
 hw/ppc/spapr_caps.c    | 40 ++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  5 ++++-
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3e528fe91e..5d62dc9968 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
     }
 };
@@ -3863,6 +3864,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 436250d77b..bc2b2c3590 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -180,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] = {
@@ -209,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,
@@ -345,6 +366,25 @@ const VMStateDescription vmstate_spapr_cap_dfp = {
     },
 };
 
+static bool spapr_cap_cfpc_needed(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+
+    return spapr->cmd_line_caps[SPAPR_CAP_CFPC] &&
+           (spapr->eff.caps[SPAPR_CAP_CFPC] != spapr->def.caps[SPAPR_CAP_CFPC]);
+}
+
+const VMStateDescription vmstate_spapr_cap_cfpc = {
+    .name = "spapr/cap/cfpc",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = spapr_cap_cfpc_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(mig.caps[SPAPR_CAP_CFPC], sPAPRMachineState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
     sPAPRCapabilities default_caps;
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

Re: [Qemu-devel] [QEMU-PPC] [PATCH V3 3/6] target/ppc/spapr_caps: Add new tristate cap safe_cache

[David Gibson]

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

On Mon, Jan 15, 2018 at 05:32:32PM +1100, Suraj Jitindar Singh wrote:
> 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>

> ---
>  hw/ppc/spapr.c         |  2 ++
>  hw/ppc/spapr_caps.c    | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  5 ++++-
>  3 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3e528fe91e..5d62dc9968 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
>      }
>  };
> @@ -3863,6 +3864,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 436250d77b..bc2b2c3590 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -180,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] = {
> @@ -209,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,
> @@ -345,6 +366,25 @@ const VMStateDescription vmstate_spapr_cap_dfp = {
>      },
>  };
>  
> +static bool spapr_cap_cfpc_needed(void *opaque)
> +{
> +    sPAPRMachineState *spapr = opaque;
> +
> +    return spapr->cmd_line_caps[SPAPR_CAP_CFPC] &&
> +           (spapr->eff.caps[SPAPR_CAP_CFPC] != spapr->def.caps[SPAPR_CAP_CFPC]);
> +}
> +
> +const VMStateDescription vmstate_spapr_cap_cfpc = {
> +    .name = "spapr/cap/cfpc",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = spapr_cap_cfpc_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_CFPC], sPAPRMachineState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};

Hrm, some macros to build these would probably be a good idea..

>  void spapr_caps_reset(sPAPRMachineState *spapr)
>  {
>      sPAPRCapabilities default_caps;
> 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)
>  {

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

[Qemu-devel] [QEMU-PPC] [PATCH V3 4/6] 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>
---
 hw/ppc/spapr.c         |  2 ++
 hw/ppc/spapr_caps.c    | 39 +++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  5 ++++-
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5d62dc9968..02a0cb656b 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
     }
 };
@@ -3865,6 +3866,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 bc2b2c3590..518e019cf7 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,
@@ -385,6 +405,25 @@ const VMStateDescription vmstate_spapr_cap_cfpc = {
     },
 };
 
+static bool spapr_cap_sbbc_needed(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+
+    return spapr->cmd_line_caps[SPAPR_CAP_SBBC] &&
+           (spapr->eff.caps[SPAPR_CAP_SBBC] != spapr->def.caps[SPAPR_CAP_SBBC]);
+}
+
+const VMStateDescription vmstate_spapr_cap_sbbc = {
+    .name = "spapr/cap/sbbc",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = spapr_cap_sbbc_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(mig.caps[SPAPR_CAP_SBBC], sPAPRMachineState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
     sPAPRCapabilities default_caps;
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

Re: [Qemu-devel] [QEMU-PPC] [PATCH V3 4/6] target/ppc/spapr_caps: Add new tristate cap safe_bounds_check

[David Gibson]

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

On Mon, Jan 15, 2018 at 05:32:33PM +1100, Suraj Jitindar Singh wrote:
> 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>

> ---
>  hw/ppc/spapr.c         |  2 ++
>  hw/ppc/spapr_caps.c    | 39 +++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  5 ++++-
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5d62dc9968..02a0cb656b 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
>      }
>  };
> @@ -3865,6 +3866,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 bc2b2c3590..518e019cf7 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,
> @@ -385,6 +405,25 @@ const VMStateDescription vmstate_spapr_cap_cfpc = {
>      },
>  };
>  
> +static bool spapr_cap_sbbc_needed(void *opaque)
> +{
> +    sPAPRMachineState *spapr = opaque;
> +
> +    return spapr->cmd_line_caps[SPAPR_CAP_SBBC] &&
> +           (spapr->eff.caps[SPAPR_CAP_SBBC] != spapr->def.caps[SPAPR_CAP_SBBC]);
> +}
> +
> +const VMStateDescription vmstate_spapr_cap_sbbc = {
> +    .name = "spapr/cap/sbbc",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = spapr_cap_sbbc_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_SBBC], sPAPRMachineState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
>  void spapr_caps_reset(sPAPRMachineState *spapr)
>  {
>      sPAPRCapabilities default_caps;
> 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)
>  {

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

[Qemu-devel] [QEMU-PPC] [PATCH V3 5/6] 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>
---
 hw/ppc/spapr.c         |  2 ++
 hw/ppc/spapr_caps.c    | 39 +++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  5 ++++-
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 02a0cb656b..269c1c7857 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
     }
 };
@@ -3867,6 +3868,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 518e019cf7..0b36333a85 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,
@@ -424,6 +444,25 @@ const VMStateDescription vmstate_spapr_cap_sbbc = {
     },
 };
 
+static bool spapr_cap_ibs_needed(void *opaque)
+{
+    sPAPRMachineState *spapr = opaque;
+
+    return spapr->cmd_line_caps[SPAPR_CAP_IBS] &&
+           (spapr->eff.caps[SPAPR_CAP_IBS] != spapr->def.caps[SPAPR_CAP_IBS]);
+}
+
+const VMStateDescription vmstate_spapr_cap_ibs = {
+    .name = "spapr/cap/ibs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = spapr_cap_ibs_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(mig.caps[SPAPR_CAP_IBS], sPAPRMachineState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 void spapr_caps_reset(sPAPRMachineState *spapr)
 {
     sPAPRCapabilities default_caps;
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

Re: [Qemu-devel] [QEMU-PPC] [PATCH V3 5/6] target/ppc/spapr_caps: Add new tristate cap safe_indirect_branch

[David Gibson]

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

On Mon, Jan 15, 2018 at 05:32:34PM +1100, Suraj Jitindar Singh wrote:
> 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>

> ---
>  hw/ppc/spapr.c         |  2 ++
>  hw/ppc/spapr_caps.c    | 39 +++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  5 ++++-
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 02a0cb656b..269c1c7857 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
>      }
>  };
> @@ -3867,6 +3868,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 518e019cf7..0b36333a85 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,
> @@ -424,6 +444,25 @@ const VMStateDescription vmstate_spapr_cap_sbbc = {
>      },
>  };
>  
> +static bool spapr_cap_ibs_needed(void *opaque)
> +{
> +    sPAPRMachineState *spapr = opaque;
> +
> +    return spapr->cmd_line_caps[SPAPR_CAP_IBS] &&
> +           (spapr->eff.caps[SPAPR_CAP_IBS] != spapr->def.caps[SPAPR_CAP_IBS]);
> +}
> +
> +const VMStateDescription vmstate_spapr_cap_ibs = {
> +    .name = "spapr/cap/ibs",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = spapr_cap_ibs_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(mig.caps[SPAPR_CAP_IBS], sPAPRMachineState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
>  void spapr_caps_reset(sPAPRMachineState *spapr)
>  {
>      sPAPRCapabilities default_caps;
> 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)
>  {

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

[Qemu-devel] [QEMU-PPC] [PATCH V3 6/6] 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>
---
 hw/ppc/spapr_hcall.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 67 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 51eba52e86..a693d3b852 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1654,6 +1654,69 @@ 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 */
+        if (safe_cache != SPAPR_CAP_BROKEN) {
+            error_report("Invalid value for cap-cfpc (%d), assuming broken",
+                         safe_cache);
+        }
+        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 */
+        if (safe_bounds_check != SPAPR_CAP_BROKEN) {
+            error_report("Invalid value for cap-sbbc (%d), assuming broken",
+                         safe_bounds_check);
+        }
+        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 */
+        if (safe_indirect_branch != SPAPR_CAP_BROKEN) {
+            error_report("Invalid value for cap-ibs (%d), assuming broken",
+                         safe_indirect_branch);
+        }
+        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 +1796,9 @@ 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 V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[David Gibson]

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

On Mon, Jan 15, 2018 at 05:32:35PM +1100, Suraj Jitindar Singh wrote:
> 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>
> ---
>  hw/ppc/spapr_hcall.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  1 +
>  2 files changed, 67 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 51eba52e86..a693d3b852 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1654,6 +1654,69 @@ 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 */
> +        if (safe_cache != SPAPR_CAP_BROKEN) {

I think you just assert() for this.  The only way these could get a
different value is if there's a bug elsewhere.

> +            error_report("Invalid value for cap-cfpc (%d), assuming broken",
> +                         safe_cache);
> +        }
> +        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 */
> +        if (safe_bounds_check != SPAPR_CAP_BROKEN) {
> +            error_report("Invalid value for cap-sbbc (%d), assuming broken",
> +                         safe_bounds_check);
> +        }
> +        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 */
> +        if (safe_indirect_branch != SPAPR_CAP_BROKEN) {
> +            error_report("Invalid value for cap-ibs (%d), assuming broken",
> +                         safe_indirect_branch);
> +        }
> +        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 +1796,9 @@ 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

-- 
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] [QEMU-PPC] [PATCH V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[Alexey Kardashevskiy]

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

On 18/01/18 16:20, David Gibson wrote:
> On Mon, Jan 15, 2018 at 05:32:35PM +1100, Suraj Jitindar Singh wrote:
>> 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>
>> ---
>>  hw/ppc/spapr_hcall.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/ppc/spapr.h |  1 +
>>  2 files changed, 67 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 51eba52e86..a693d3b852 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1654,6 +1654,69 @@ 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 */
>> +        if (safe_cache != SPAPR_CAP_BROKEN) {
> 
> I think you just assert() for this.  The only way these could get a
> different value is if there's a bug elsewhere.


Why not return H_HARDWARE or other error?



-- 
Alexey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [PATCH V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[David Gibson]

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

On Thu, Jan 18, 2018 at 04:44:28PM +1100, Alexey Kardashevskiy wrote:
> On 18/01/18 16:20, David Gibson wrote:
> > On Mon, Jan 15, 2018 at 05:32:35PM +1100, Suraj Jitindar Singh wrote:
> >> 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>
> >> ---
> >>  hw/ppc/spapr_hcall.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  include/hw/ppc/spapr.h |  1 +
> >>  2 files changed, 67 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 51eba52e86..a693d3b852 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1654,6 +1654,69 @@ 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 */
> >> +        if (safe_cache != SPAPR_CAP_BROKEN) {
> > 
> > I think you just assert() for this.  The only way these could get a
> > different value is if there's a bug elsewhere.
> 
> 
> Why not return H_HARDWARE or other error?

Because what's the guest supposed to do with it.  This is an internal
qemu problem, so it should be dealt with via an internal qemu
mechanism.

-- 
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] [QEMU-PPC] [PATCH V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[Alexey Kardashevskiy]

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

On 18/01/18 16:53, David Gibson wrote:
> On Thu, Jan 18, 2018 at 04:44:28PM +1100, Alexey Kardashevskiy wrote:
>> On 18/01/18 16:20, David Gibson wrote:
>>> On Mon, Jan 15, 2018 at 05:32:35PM +1100, Suraj Jitindar Singh wrote:
>>>> 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>
>>>> ---
>>>>  hw/ppc/spapr_hcall.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  include/hw/ppc/spapr.h |  1 +
>>>>  2 files changed, 67 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>>> index 51eba52e86..a693d3b852 100644
>>>> --- a/hw/ppc/spapr_hcall.c
>>>> +++ b/hw/ppc/spapr_hcall.c
>>>> @@ -1654,6 +1654,69 @@ 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 */
>>>> +        if (safe_cache != SPAPR_CAP_BROKEN) {
>>>
>>> I think you just assert() for this.  The only way these could get a
>>> different value is if there's a bug elsewhere.
>>
>>
>> Why not return H_HARDWARE or other error?
> 
> Because what's the guest supposed to do with it. 

"oops"

> This is an internal
> qemu problem, so it should be dealt with via an internal qemu
> mechanism.

Do we have assert() enabled in production? If not, then assert == noop,
error_report is just a noise.



-- 
Alexey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [PATCH V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[Eric Blake]

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

On 01/18/2018 02:11 AM, Alexey Kardashevskiy wrote:

>>>> I think you just assert() for this.  The only way these could get a
>>>> different value is if there's a bug elsewhere.
>>>
>>>
>>> Why not return H_HARDWARE or other error?
>>
>> Because what's the guest supposed to do with it. 
> 
> "oops"
> 
>> This is an internal
>> qemu problem, so it should be dealt with via an internal qemu
>> mechanism.
> 
> Do we have assert() enabled in production? If not, then assert == noop,
> error_report is just a noise.

See commit 262a69f4.  Yes, assert() is enabled in production, for
security reasons (aka it's easier to do that than to audit that
migration is still safe even with no-op asserts).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [PATCH V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[David Gibson]

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

On Thu, Jan 18, 2018 at 02:30:27PM -0600, Eric Blake wrote:
> On 01/18/2018 02:11 AM, Alexey Kardashevskiy wrote:
> 
> >>>> I think you just assert() for this.  The only way these could get a
> >>>> different value is if there's a bug elsewhere.
> >>>
> >>>
> >>> Why not return H_HARDWARE or other error?
> >>
> >> Because what's the guest supposed to do with it. 
> > 
> > "oops"
> > 
> >> This is an internal
> >> qemu problem, so it should be dealt with via an internal qemu
> >> mechanism.
> > 
> > Do we have assert() enabled in production? If not, then assert == noop,
> > error_report is just a noise.
> 
> See commit 262a69f4.  Yes, assert() is enabled in production, for
> security reasons (aka it's easier to do that than to audit that
> migration is still safe even with no-op asserts).

TBH, assert()s usually aren't disabled in production.  That's the
theory behind them, but in practice AFAICT, the debugging utility
almost always outweighs the performance cost which is usually pretty
small.

-- 
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] [QEMU-PPC] [PATCH V3 6/6] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS

[David Gibson]

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

On Thu, Jan 18, 2018 at 07:11:41PM +1100, Alexey Kardashevskiy wrote:
> On 18/01/18 16:53, David Gibson wrote:
> > On Thu, Jan 18, 2018 at 04:44:28PM +1100, Alexey Kardashevskiy wrote:
> >> On 18/01/18 16:20, David Gibson wrote:
> >>> On Mon, Jan 15, 2018 at 05:32:35PM +1100, Suraj Jitindar Singh wrote:
> >>>> 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>
> >>>> ---
> >>>>  hw/ppc/spapr_hcall.c   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  include/hw/ppc/spapr.h |  1 +
> >>>>  2 files changed, 67 insertions(+)
> >>>>
> >>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >>>> index 51eba52e86..a693d3b852 100644
> >>>> --- a/hw/ppc/spapr_hcall.c
> >>>> +++ b/hw/ppc/spapr_hcall.c
> >>>> @@ -1654,6 +1654,69 @@ 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 */
> >>>> +        if (safe_cache != SPAPR_CAP_BROKEN) {
> >>>
> >>> I think you just assert() for this.  The only way these could get a
> >>> different value is if there's a bug elsewhere.
> >>
> >>
> >> Why not return H_HARDWARE or other error?
> > 
> > Because what's the guest supposed to do with it. 
> 
> "oops"

Thereby making what is definitely a qemu bug appear to be a guest
problem.

-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Andrea Bolognani]

On Mon, 2018-01-15 at 17:32 +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
> 
> The first patch from the previous revision has already been merged:
> hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation
> 
> The main changes to V3 are:
> - Split up the addition of the tristate caps into 5 patches
>   - 1/6 query the caps from the hypervisor and parse the new return format
>   - 2/6 add support for the new caps
>   - 3-5/6 add each of the three new caps
> - Patch 6/6 Unchanged

Correct me if I'm wrong, but it seems to me like there's no way
to figure out through QMP whether these new machine options can be
used for a given QEMU binary.

If so, that's very unfortunate because it means that libvirt has
only two options: 1) just use them if the user requests the
corresponding feature, which will lead to older QEMU binaries
simply refusing to start; or 2) perform a version number check,
which will not be accurate if downstream backports are involved.

Would this information be added to the MachineInfo struct, so that
query-machines reports it? Or would a new QMP command be more
appropriate for the task?

Alternatively, if there's any witness we can use instead of an
explicit capability, let me know. But I still think we should
think about a better long-term solution, especially because this
seems to be happening quite frequently lately: see the hpt-resizing
and max-cpu-compat machine properties, which are just as opaque
from an introspection point of view.

Sorry for not bringing this up earlier.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[David Gibson]

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

On Tue, Jan 16, 2018 at 02:47:13PM +0100, Andrea Bolognani wrote:
> On Mon, 2018-01-15 at 17:32 +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
> > 
> > The first patch from the previous revision has already been merged:
> > hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation
> > 
> > The main changes to V3 are:
> > - Split up the addition of the tristate caps into 5 patches
> >   - 1/6 query the caps from the hypervisor and parse the new return format
> >   - 2/6 add support for the new caps
> >   - 3-5/6 add each of the three new caps
> > - Patch 6/6 Unchanged
> 
> Correct me if I'm wrong, but it seems to me like there's no way
> to figure out through QMP whether these new machine options can be
> used for a given QEMU binary.

Uh, I don't think so.  These are machine options like any other (just
constructed a bit differently).  So they'll appear in qemu -machine
pseries,? and I believe that info can also be retrieved with QMP.

> If so, that's very unfortunate because it means that libvirt has
> only two options: 1) just use them if the user requests the
> corresponding feature, which will lead to older QEMU binaries
> simply refusing to start; or 2) perform a version number check,
> which will not be accurate if downstream backports are involved.
> 
> Would this information be added to the MachineInfo struct, so that
> query-machines reports it? Or would a new QMP command be more
> appropriate for the task?
> 
> Alternatively, if there's any witness we can use instead of an
> explicit capability, let me know. But I still think we should
> think about a better long-term solution, especially because this
> seems to be happening quite frequently lately: see the hpt-resizing
> and max-cpu-compat machine properties, which are just as opaque
> from an introspection point of view.
> 
> Sorry for not bringing this up earlier.
> 

-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Andrea Bolognani]

On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
> > Correct me if I'm wrong, but it seems to me like there's no way
> > to figure out through QMP whether these new machine options can be
> > used for a given QEMU binary.
> 
> Uh, I don't think so.  These are machine options like any other (just
> constructed a bit differently).  So they'll appear in qemu -machine
> pseries,? and I believe that info can also be retrieved with QMP.

Yes, they will indeed show up in the output of -machine pseries,?
but there's AFAICT no way to retrieve them via QMP. And libvirt
can't afford to spawn a QEMU process for each machine type
implemented by each QEMU binary installed on the system just to
figure out what properties they support; in fact, we've been
pushing away from that approach - which was used initially - for
years and we're now at the point where we only fall back to it
for positively ancient QEMU versions. So the information needs
to be available through QMP for libvirt to consume it.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[David Gibson]

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

On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
> On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
> > > Correct me if I'm wrong, but it seems to me like there's no way
> > > to figure out through QMP whether these new machine options can be
> > > used for a given QEMU binary.
> > 
> > Uh, I don't think so.  These are machine options like any other (just
> > constructed a bit differently).  So they'll appear in qemu -machine
> > pseries,? and I believe that info can also be retrieved with QMP.
> 
> Yes, they will indeed show up in the output of -machine pseries,?
> but there's AFAICT no way to retrieve them via QMP.

Really!?  I thought introspecting object properties was QMP's bread
and butter.

> And libvirt
> can't afford to spawn a QEMU process for each machine type
> implemented by each QEMU binary installed on the system just to
> figure out what properties they support; in fact, we've been
> pushing away from that approach - which was used initially - for
> years and we're now at the point where we only fall back to it
> for positively ancient QEMU versions. So the information needs
> to be available through QMP for libvirt to consume it.

Right, I'm not arguing with that.  It's just that I thought that
standard QOM properties on QOM objects (the machine in this case) met
the criteria.

-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Alexey Kardashevskiy]

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

On 17/01/18 09:34, David Gibson wrote:
> On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
>> On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
>>>> Correct me if I'm wrong, but it seems to me like there's no way
>>>> to figure out through QMP whether these new machine options can be
>>>> used for a given QEMU binary.
>>>
>>> Uh, I don't think so.  These are machine options like any other (just
>>> constructed a bit differently).  So they'll appear in qemu -machine
>>> pseries,? and I believe that info can also be retrieved with QMP.
>>
>> Yes, they will indeed show up in the output of -machine pseries,?
>> but there's AFAICT no way to retrieve them via QMP.
> 
> Really!?  I thought introspecting object properties was QMP's bread
> and butter.


On a guest started with '-S':
{"execute": "qom-list", "arguments": {"path": "/machine"}}

returns:
{   'return': [   {'name': 'graphics', 'type': 'bool'},
                  {'name': 'phandle-start', 'type': 'int'},
                  {'name': 'dump-guest-core', 'type': 'bool'},
                  {'name': 'kernel-irqchip', 'type': 'OnOffSplit'},
                  {'name': 'accel', 'type': 'string'},
                  {'name': 'append', 'type': 'string'},
                  {'name': 'dumpdtb', 'type': 'string'},
                  {'name': 'igd-passthru', 'type': 'bool'},
                  {'name': 'dt-compatible', 'type': 'string'},
                  {'name': 'kernel', 'type': 'string'},
                  {'name': 'usb', 'type': 'bool'},
                  {'name': 'suppress-vmdesc', 'type': 'bool'},
                  {'name': 'dtb', 'type': 'string'},
                  {'name': 'firmware', 'type': 'string'},
                  {'name': 'mem-merge', 'type': 'bool'},
                  {'name': 'initrd', 'type': 'string'},
                  {'name': 'enforce-config-section', 'type': 'bool'},
                  {'name': 'kvm-shadow-mem', 'type': 'int'},
                  {'name': 'cap-dfp', 'type': 'bool'},
                  {'name': 'cap-htm', 'type': 'bool'},
                  {'name': 'cap-vsx', 'type': 'bool'},
                  {'name': 'vfio-no-msix-emulation', 'type': 'bool'},
                  {'name': 'kvm-type', 'type': 'string'},
                  {'name': 'max-cpu-compat', 'type': 'string'},
                  {   'name': 'dr-connector[268435480]',
                      'type': 'child<spapr-drc-cpu>'},
                  {'name': 'peripheral', 'type': 'child<container>'},
                  {   'name': 'dr-connector[268435472]',
                      'type': 'child<spapr-drc-cpu>'},
                  {'name': 'modern-hotplug-events', 'type': 'bool'},
                  {   'name': 'dr-connector[268435464]',
                      'type': 'child<spapr-drc-cpu>'},
                  {   'name': 'dr-connector[268435456]',
                      'type': 'child<spapr-drc-cpu>'},
                  {'name': 'peripheral-anon', 'type': 'child<container>'},
                  {'name': 'ics', 'type': 'child<icskvm>'},
                  {'name': 'vsmt', 'type': 'uint32'},
                  {'name': 'type', 'type': 'string'},
                  {'name': 'rtc-time', 'type': 'struct tm'},
                  {'name': 'unattached', 'type': 'child<container>'},
                  {'name': 'rtc', 'type': 'child<spapr-rtc>'},
                  {'name': 'resize-hpt', 'type': 'string'}]}


but still requires a running qemu, yes.

> 
>> And libvirt
>> can't afford to spawn a QEMU process for each machine type
>> implemented by each QEMU binary installed on the system just to
>> figure out what properties they support; in fact, we've been
>> pushing away from that approach - which was used initially - for
>> years and we're now at the point where we only fall back to it
>> for positively ancient QEMU versions. So the information needs
>> to be available through QMP for libvirt to consume it.
> 
> Right, I'm not arguing with that.  It's just that I thought that
> standard QOM properties on QOM objects (the machine in this case) met
> the criteria.
> 


-- 
Alexey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]

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

[David Gibson]

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

On Wed, Jan 17, 2018 at 10:26:28AM +1100, Alexey Kardashevskiy wrote:
> On 17/01/18 09:34, David Gibson wrote:
> > On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
> >> On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
> >>>> Correct me if I'm wrong, but it seems to me like there's no way
> >>>> to figure out through QMP whether these new machine options can be
> >>>> used for a given QEMU binary.
> >>>
> >>> Uh, I don't think so.  These are machine options like any other (just
> >>> constructed a bit differently).  So they'll appear in qemu -machine
> >>> pseries,? and I believe that info can also be retrieved with QMP.
> >>
> >> Yes, they will indeed show up in the output of -machine pseries,?
> >> but there's AFAICT no way to retrieve them via QMP.
> > 
> > Really!?  I thought introspecting object properties was QMP's bread
> > and butter.
> 
> 
> On a guest started with '-S':
> {"execute": "qom-list", "arguments": {"path": "/machine"}}
> 
> returns:
> {   'return': [   {'name': 'graphics', 'type': 'bool'},
>                   {'name': 'phandle-start', 'type': 'int'},
>                   {'name': 'dump-guest-core', 'type': 'bool'},
>                   {'name': 'kernel-irqchip', 'type': 'OnOffSplit'},
>                   {'name': 'accel', 'type': 'string'},
>                   {'name': 'append', 'type': 'string'},
>                   {'name': 'dumpdtb', 'type': 'string'},
>                   {'name': 'igd-passthru', 'type': 'bool'},
>                   {'name': 'dt-compatible', 'type': 'string'},
>                   {'name': 'kernel', 'type': 'string'},
>                   {'name': 'usb', 'type': 'bool'},
>                   {'name': 'suppress-vmdesc', 'type': 'bool'},
>                   {'name': 'dtb', 'type': 'string'},
>                   {'name': 'firmware', 'type': 'string'},
>                   {'name': 'mem-merge', 'type': 'bool'},
>                   {'name': 'initrd', 'type': 'string'},
>                   {'name': 'enforce-config-section', 'type': 'bool'},
>                   {'name': 'kvm-shadow-mem', 'type': 'int'},
>                   {'name': 'cap-dfp', 'type': 'bool'},
>                   {'name': 'cap-htm', 'type': 'bool'},
>                   {'name': 'cap-vsx', 'type': 'bool'},
                              ^^^^^^^
Here are the cap properties.  Is it just Suraj's new tristate ones
that aren't showing up?  If so that's weird...  are you sure you built
with those patches included?

>                   {'name': 'vfio-no-msix-emulation', 'type': 'bool'},
>                   {'name': 'kvm-type', 'type': 'string'},
>                   {'name': 'max-cpu-compat', 'type': 'string'},
>                   {   'name': 'dr-connector[268435480]',
>                       'type': 'child<spapr-drc-cpu>'},
>                   {'name': 'peripheral', 'type': 'child<container>'},
>                   {   'name': 'dr-connector[268435472]',
>                       'type': 'child<spapr-drc-cpu>'},
>                   {'name': 'modern-hotplug-events', 'type': 'bool'},
>                   {   'name': 'dr-connector[268435464]',
>                       'type': 'child<spapr-drc-cpu>'},
>                   {   'name': 'dr-connector[268435456]',
>                       'type': 'child<spapr-drc-cpu>'},
>                   {'name': 'peripheral-anon', 'type': 'child<container>'},
>                   {'name': 'ics', 'type': 'child<icskvm>'},
>                   {'name': 'vsmt', 'type': 'uint32'},
>                   {'name': 'type', 'type': 'string'},
>                   {'name': 'rtc-time', 'type': 'struct tm'},
>                   {'name': 'unattached', 'type': 'child<container>'},
>                   {'name': 'rtc', 'type': 'child<spapr-rtc>'},
>                   {'name': 'resize-hpt', 'type': 'string'}]}
> 
> 
> but still requires a running qemu, yes.
> 
> > 
> >> And libvirt
> >> can't afford to spawn a QEMU process for each machine type
> >> implemented by each QEMU binary installed on the system just to
> >> figure out what properties they support; in fact, we've been
> >> pushing away from that approach - which was used initially - for
> >> years and we're now at the point where we only fall back to it
> >> for positively ancient QEMU versions. So the information needs
> >> to be available through QMP for libvirt to consume it.
> > 
> > Right, I'm not arguing with that.  It's just that I thought that
> > standard QOM properties on QOM objects (the machine in this case) met
> > the criteria.
> > 
> 
> 




-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Alexey Kardashevskiy]

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

On 17/01/18 10:30, David Gibson wrote:
> On Wed, Jan 17, 2018 at 10:26:28AM +1100, Alexey Kardashevskiy wrote:
>> On 17/01/18 09:34, David Gibson wrote:
>>> On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
>>>> On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
>>>>>> Correct me if I'm wrong, but it seems to me like there's no way
>>>>>> to figure out through QMP whether these new machine options can be
>>>>>> used for a given QEMU binary.
>>>>>
>>>>> Uh, I don't think so.  These are machine options like any other (just
>>>>> constructed a bit differently).  So they'll appear in qemu -machine
>>>>> pseries,? and I believe that info can also be retrieved with QMP.
>>>>
>>>> Yes, they will indeed show up in the output of -machine pseries,?
>>>> but there's AFAICT no way to retrieve them via QMP.
>>>
>>> Really!?  I thought introspecting object properties was QMP's bread
>>> and butter.
>>
>>
>> On a guest started with '-S':
>> {"execute": "qom-list", "arguments": {"path": "/machine"}}
>>
>> returns:
>> {   'return': [   {'name': 'graphics', 'type': 'bool'},
>>                   {'name': 'phandle-start', 'type': 'int'},
>>                   {'name': 'dump-guest-core', 'type': 'bool'},
>>                   {'name': 'kernel-irqchip', 'type': 'OnOffSplit'},
>>                   {'name': 'accel', 'type': 'string'},
>>                   {'name': 'append', 'type': 'string'},
>>                   {'name': 'dumpdtb', 'type': 'string'},
>>                   {'name': 'igd-passthru', 'type': 'bool'},
>>                   {'name': 'dt-compatible', 'type': 'string'},
>>                   {'name': 'kernel', 'type': 'string'},
>>                   {'name': 'usb', 'type': 'bool'},
>>                   {'name': 'suppress-vmdesc', 'type': 'bool'},
>>                   {'name': 'dtb', 'type': 'string'},
>>                   {'name': 'firmware', 'type': 'string'},
>>                   {'name': 'mem-merge', 'type': 'bool'},
>>                   {'name': 'initrd', 'type': 'string'},
>>                   {'name': 'enforce-config-section', 'type': 'bool'},
>>                   {'name': 'kvm-shadow-mem', 'type': 'int'},
>>                   {'name': 'cap-dfp', 'type': 'bool'},
>>                   {'name': 'cap-htm', 'type': 'bool'},
>>                   {'name': 'cap-vsx', 'type': 'bool'},
>                               ^^^^^^^
> Here are the cap properties.  Is it just Suraj's new tristate ones
> that aren't showing up?  If so that's weird...  are you sure you built
> with those patches included?

I am sure I do not have them, my point was that QMP has it all if they are
present in qemu :)


Here is rerun:


 {"execute": "qom-list", "arguments": {"path": "/machine"}}

{   'return': [   {'name': 'graphics', 'type': 'bool'},
                  {'name': 'phandle-start', 'type': 'int'},
                  {'name': 'dump-guest-core', 'type': 'bool'},
                  {'name': 'kernel-irqchip', 'type': 'OnOffSplit'},
                  {'name': 'accel', 'type': 'string'},
                  {'name': 'append', 'type': 'string'},
                  {'name': 'dumpdtb', 'type': 'string'},
                  {'name': 'igd-passthru', 'type': 'bool'},
                  {'name': 'dt-compatible', 'type': 'string'},
                  {'name': 'kernel', 'type': 'string'},
                  {'name': 'usb', 'type': 'bool'},
                  {'name': 'suppress-vmdesc', 'type': 'bool'},
                  {'name': 'dtb', 'type': 'string'},
                  {'name': 'firmware', 'type': 'string'},
                  {'name': 'mem-merge', 'type': 'bool'},
                  {'name': 'initrd', 'type': 'string'},
                  {'name': 'enforce-config-section', 'type': 'bool'},
                  {'name': 'kvm-shadow-mem', 'type': 'int'},
                  {'name': 'cap-ibs', 'type': 'string'},
                  {'name': 'cap-cfpc', 'type': 'string'},
                  {'name': 'cap-sbbc', 'type': 'string'},
                  {'name': 'cap-dfp', 'type': 'bool'},
                  {'name': 'cap-htm', 'type': 'bool'},
                  {'name': 'cap-vsx', 'type': 'bool'},
                  {'name': 'kvm-type', 'type': 'string'},
                  {'name': 'max-cpu-compat', 'type': 'string'},
                  {   'name': 'dr-connector[268435480]',
                      'type': 'child<spapr-drc-cpu>'},
                  {'name': 'peripheral', 'type': 'child<container>'},
                  {   'name': 'dr-connector[268435472]',
                      'type': 'child<spapr-drc-cpu>'},
                  {'name': 'modern-hotplug-events', 'type': 'bool'},
                  {   'name': 'dr-connector[268435464]',
                      'type': 'child<spapr-drc-cpu>'},
                  {   'name': 'dr-connector[268435456]',
                      'type': 'child<spapr-drc-cpu>'},
                  {'name': 'peripheral-anon', 'type': 'child<container>'},
                  {'name': 'ics', 'type': 'child<icskvm>'},
                  {'name': 'vsmt', 'type': 'uint32'},
                  {'name': 'type', 'type': 'string'},
                  {'name': 'rtc-time', 'type': 'struct tm'},
                  {'name': 'unattached', 'type': 'child<container>'},
                  {'name': 'rtc', 'type': 'child<spapr-rtc>'},
                  {'name': 'resize-hpt', 'type': 'string'}]}


and (on unmodified host kernel):

 {"execute": "qom-get", "arguments": {"path": "/machine", "property":
"cap-ibs"}}
{'return': 'broken'}

etc.


-- 
Alexey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]

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

[David Gibson]

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

On Wed, Jan 17, 2018 at 10:46:45AM +1100, Alexey Kardashevskiy wrote:
> On 17/01/18 10:30, David Gibson wrote:
> > On Wed, Jan 17, 2018 at 10:26:28AM +1100, Alexey Kardashevskiy wrote:
> >> On 17/01/18 09:34, David Gibson wrote:
> >>> On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
> >>>> On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
> >>>>>> Correct me if I'm wrong, but it seems to me like there's no way
> >>>>>> to figure out through QMP whether these new machine options can be
> >>>>>> used for a given QEMU binary.
> >>>>>
> >>>>> Uh, I don't think so.  These are machine options like any other (just
> >>>>> constructed a bit differently).  So they'll appear in qemu -machine
> >>>>> pseries,? and I believe that info can also be retrieved with QMP.
> >>>>
> >>>> Yes, they will indeed show up in the output of -machine pseries,?
> >>>> but there's AFAICT no way to retrieve them via QMP.
> >>>
> >>> Really!?  I thought introspecting object properties was QMP's bread
> >>> and butter.
> >>
> >>
> >> On a guest started with '-S':
> >> {"execute": "qom-list", "arguments": {"path": "/machine"}}
> >>
> >> returns:
> >> {   'return': [   {'name': 'graphics', 'type': 'bool'},
> >>                   {'name': 'phandle-start', 'type': 'int'},
> >>                   {'name': 'dump-guest-core', 'type': 'bool'},
> >>                   {'name': 'kernel-irqchip', 'type': 'OnOffSplit'},
> >>                   {'name': 'accel', 'type': 'string'},
> >>                   {'name': 'append', 'type': 'string'},
> >>                   {'name': 'dumpdtb', 'type': 'string'},
> >>                   {'name': 'igd-passthru', 'type': 'bool'},
> >>                   {'name': 'dt-compatible', 'type': 'string'},
> >>                   {'name': 'kernel', 'type': 'string'},
> >>                   {'name': 'usb', 'type': 'bool'},
> >>                   {'name': 'suppress-vmdesc', 'type': 'bool'},
> >>                   {'name': 'dtb', 'type': 'string'},
> >>                   {'name': 'firmware', 'type': 'string'},
> >>                   {'name': 'mem-merge', 'type': 'bool'},
> >>                   {'name': 'initrd', 'type': 'string'},
> >>                   {'name': 'enforce-config-section', 'type': 'bool'},
> >>                   {'name': 'kvm-shadow-mem', 'type': 'int'},
> >>                   {'name': 'cap-dfp', 'type': 'bool'},
> >>                   {'name': 'cap-htm', 'type': 'bool'},
> >>                   {'name': 'cap-vsx', 'type': 'bool'},
> >                               ^^^^^^^
> > Here are the cap properties.  Is it just Suraj's new tristate ones
> > that aren't showing up?  If so that's weird...  are you sure you built
> > with those patches included?
> 
> I am sure I do not have them, my point was that QMP has it all if they are
> present in qemu :)

Oh sorry, I wasn't paying attention.  I thought that mail was from
Andrea, not you.

-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Andrea Bolognani]

On Wed, 2018-01-17 at 10:26 +1100, Alexey Kardashevskiy wrote:
> On 17/01/18 09:34, David Gibson wrote:
> > On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
> > > On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
> > > > > Correct me if I'm wrong, but it seems to me like there's no way
> > > > > to figure out through QMP whether these new machine options can be
> > > > > used for a given QEMU binary.
> > > > 
> > > > Uh, I don't think so.  These are machine options like any other (just
> > > > constructed a bit differently).  So they'll appear in qemu -machine
> > > > pseries,? and I believe that info can also be retrieved with QMP.
> > > 
> > > Yes, they will indeed show up in the output of -machine pseries,?
> > > but there's AFAICT no way to retrieve them via QMP.
> > 
> > Really!?  I thought introspecting object properties was QMP's bread
> > and butter.
> 
> On a guest started with '-S':
> {"execute": "qom-list", "arguments": {"path": "/machine"}}
> 
> returns:
> {   'return': [   {'name': 'graphics', 'type': 'bool'},
[...]
>                   {'name': 'cap-dfp', 'type': 'bool'},
>                   {'name': 'cap-htm', 'type': 'bool'},
>                   {'name': 'cap-vsx', 'type': 'bool'},
>                   {'name': 'vfio-no-msix-emulation', 'type': 'bool'},
>                   {'name': 'kvm-type', 'type': 'string'},
>                   {'name': 'max-cpu-compat', 'type': 'string'},
[...]
>                   {'name': 'resize-hpt', 'type': 'string'}]}
> 
> but still requires a running qemu, yes.

That's not a problem in itself; however, AFAICT the guest in
question also needs to be started with -machine pseries in order
for the above to work, which means it's not usable due to the
scalability issues mentioned earlier in the thread. We run QEMU
with -machine none, a single time, to probe for capabilities.

I looked further and device-list-properties looks like it would
do the trick; however it doesn't seem to work for machines:

  {"execute": "device-list-properties",
   "arguments": {"typename": "spapr-2.11-machine"}}
  {"error": {"class": "GenericError",
             "desc": "Parameter 'typename' expects device"}}

It works fine for the likes of virtio-scsi-pci and even
power9_v2.0-powerpc64-cpu, though. Any ideas? :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[David Gibson]

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

On Wed, Jan 17, 2018 at 09:54:57AM +0100, Andrea Bolognani wrote:
> On Wed, 2018-01-17 at 10:26 +1100, Alexey Kardashevskiy wrote:
> > On 17/01/18 09:34, David Gibson wrote:
> > > On Tue, Jan 16, 2018 at 03:46:20PM +0100, Andrea Bolognani wrote:
> > > > On Wed, 2018-01-17 at 00:54 +1100, David Gibson wrote:
> > > > > > Correct me if I'm wrong, but it seems to me like there's no way
> > > > > > to figure out through QMP whether these new machine options can be
> > > > > > used for a given QEMU binary.
> > > > > 
> > > > > Uh, I don't think so.  These are machine options like any other (just
> > > > > constructed a bit differently).  So they'll appear in qemu -machine
> > > > > pseries,? and I believe that info can also be retrieved with QMP.
> > > > 
> > > > Yes, they will indeed show up in the output of -machine pseries,?
> > > > but there's AFAICT no way to retrieve them via QMP.
> > > 
> > > Really!?  I thought introspecting object properties was QMP's bread
> > > and butter.
> > 
> > On a guest started with '-S':
> > {"execute": "qom-list", "arguments": {"path": "/machine"}}
> > 
> > returns:
> > {   'return': [   {'name': 'graphics', 'type': 'bool'},
> [...]
> >                   {'name': 'cap-dfp', 'type': 'bool'},
> >                   {'name': 'cap-htm', 'type': 'bool'},
> >                   {'name': 'cap-vsx', 'type': 'bool'},
> >                   {'name': 'vfio-no-msix-emulation', 'type': 'bool'},
> >                   {'name': 'kvm-type', 'type': 'string'},
> >                   {'name': 'max-cpu-compat', 'type': 'string'},
> [...]
> >                   {'name': 'resize-hpt', 'type': 'string'}]}
> > 
> > but still requires a running qemu, yes.
> 
> That's not a problem in itself; however, AFAICT the guest in
> question also needs to be started with -machine pseries in order
> for the above to work, which means it's not usable due to the
> scalability issues mentioned earlier in the thread. We run QEMU
> with -machine none, a single time, to probe for capabilities.
> 
> I looked further and device-list-properties looks like it would
> do the trick; however it doesn't seem to work for machines:
> 
>   {"execute": "device-list-properties",
>    "arguments": {"typename": "spapr-2.11-machine"}}
>   {"error": {"class": "GenericError",
>              "desc": "Parameter 'typename' expects device"}}
> 
> It works fine for the likes of virtio-scsi-pci and even
> power9_v2.0-powerpc64-cpu, though. Any ideas? :)

I'm guessing it's because machines aren't descended from TYPE_DEVICE.

Dammit.  I really can't see a reasonable way of addressing this other
than improving qapi in general to have a way of reporting machine
class properties.  Adding something ad-hoc for just these properties
of this machine seems like madness.

Nor can I think of a place to put these that would be both sensible
and more discoverable with existing mechanisms.

-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Andrea Bolognani]

On Thu, 2018-01-18 at 15:27 +1100, David Gibson wrote:
> > I looked further and device-list-properties looks like it would
> > do the trick; however it doesn't seem to work for machines:
> > 
> >   {"execute": "device-list-properties",
> >    "arguments": {"typename": "spapr-2.11-machine"}}
> >   {"error": {"class": "GenericError",
> >              "desc": "Parameter 'typename' expects device"}}
> > 
> > It works fine for the likes of virtio-scsi-pci and even
> > power9_v2.0-powerpc64-cpu, though. Any ideas? :)
> 
> I'm guessing it's because machines aren't descended from TYPE_DEVICE.
> 
> Dammit.  I really can't see a reasonable way of addressing this other
> than improving qapi in general to have a way of reporting machine
> class properties.  Adding something ad-hoc for just these properties
> of this machine seems like madness.
> 
> Nor can I think of a place to put these that would be both sensible
> and more discoverable with existing mechanisms.

The relationship between QOM/QAPI/QMP is not very clear in my mind,
as you might have guessed from my messages, so I don't think I can
offer much useful input. But if the properties are registered using
the same mechanism both for devices and machines, then maybe there
should be a QMP command that can list them regardless of the parent
type? object-list-properties or something like that.

I also noticed that the (AFAIK s390-specific) "loadparm" property
is detected by libvirt through the query-command-line-options QMP
command. Not sure what kind of trickery they employ to obtain that
result, though, and I'm not sure it's exactly an example of best
practices since it shows up on x86 and aarch64 too ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[David Gibson]

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

On Thu, Jan 18, 2018 at 04:55:39PM +0100, Andrea Bolognani wrote:
> On Thu, 2018-01-18 at 15:27 +1100, David Gibson wrote:
> > > I looked further and device-list-properties looks like it would
> > > do the trick; however it doesn't seem to work for machines:
> > > 
> > >   {"execute": "device-list-properties",
> > >    "arguments": {"typename": "spapr-2.11-machine"}}
> > >   {"error": {"class": "GenericError",
> > >              "desc": "Parameter 'typename' expects device"}}
> > > 
> > > It works fine for the likes of virtio-scsi-pci and even
> > > power9_v2.0-powerpc64-cpu, though. Any ideas? :)
> > 
> > I'm guessing it's because machines aren't descended from TYPE_DEVICE.
> > 
> > Dammit.  I really can't see a reasonable way of addressing this other
> > than improving qapi in general to have a way of reporting machine
> > class properties.  Adding something ad-hoc for just these properties
> > of this machine seems like madness.
> > 
> > Nor can I think of a place to put these that would be both sensible
> > and more discoverable with existing mechanisms.
> 
> The relationship between QOM/QAPI/QMP is not very clear in my mind,

You and me both :/

> as you might have guessed from my messages, so I don't think I can
> offer much useful input. But if the properties are registered using
> the same mechanism both for devices and machines, then maybe there
> should be a QMP command that can list them regardless of the parent
> type? object-list-properties or something like that.

So, it's a bit complicated.  The underlying mechanism is QOM
properties in all cases.  QOM properties can be constructed either on
a QOM class, or on a QOM instance.  I'm pretty sure
device-list-properties will only list those constructed on the class,
since an instance hasn't been created.

Nearly all properties should be created on the class, not the
instance... but in most cases they're not.  The cap properties *are*
created at the class level, so in principle should be listable in the
same way.

Things derived from TYPE_DEVICE can use that same mechanism directly
but more commonly use a table of properties that's brought over from
the older qdev stuff.  It's translated during class initialization, I
believe, into the QOM backend, but is still used because it's usually
less verbose than constructing QOM properties directly.

> I also noticed that the (AFAIK s390-specific) "loadparm" property
> is detected by libvirt through the query-command-line-options QMP
> command. Not sure what kind of trickery they employ to obtain that
> result, though, and I'm not sure it's exactly an example of best
> practices since it shows up on x86 and aarch64 too ;)

-- 
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] [QEMU-PPC] [PATCH V3 0/6] target/ppc: Rework spapr_caps

[Alexey Kardashevskiy]

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

On 19/01/18 13:22, David Gibson wrote:
> On Thu, Jan 18, 2018 at 04:55:39PM +0100, Andrea Bolognani wrote:
>> On Thu, 2018-01-18 at 15:27 +1100, David Gibson wrote:
>>>> I looked further and device-list-properties looks like it would
>>>> do the trick; however it doesn't seem to work for machines:
>>>>
>>>>   {"execute": "device-list-properties",
>>>>    "arguments": {"typename": "spapr-2.11-machine"}}
>>>>   {"error": {"class": "GenericError",
>>>>              "desc": "Parameter 'typename' expects device"}}
>>>>
>>>> It works fine for the likes of virtio-scsi-pci and even
>>>> power9_v2.0-powerpc64-cpu, though. Any ideas? :)
>>>
>>> I'm guessing it's because machines aren't descended from TYPE_DEVICE.
>>>
>>> Dammit.  I really can't see a reasonable way of addressing this other
>>> than improving qapi in general to have a way of reporting machine
>>> class properties.  Adding something ad-hoc for just these properties
>>> of this machine seems like madness.
>>>
>>> Nor can I think of a place to put these that would be both sensible
>>> and more discoverable with existing mechanisms.
>>
>> The relationship between QOM/QAPI/QMP is not very clear in my mind,
> 
> You and me both :/
> 
>> as you might have guessed from my messages, so I don't think I can
>> offer much useful input. But if the properties are registered using
>> the same mechanism both for devices and machines, then maybe there
>> should be a QMP command that can list them regardless of the parent
>> type? object-list-properties or something like that.
> 
> So, it's a bit complicated.  The underlying mechanism is QOM
> properties in all cases.

There are 2 types of properties really.

typedef struct DeviceClass {
[...]
    Property *props;

These are printed by QMP's device-list-properties. Machine is not a device
so we do not see them.


struct ObjectClass
{
[...]
    GHashTable *properties;

These are printed by qom-list/get but these QMP commands take path as an
argument and therefore require instances, not classes.


DeviceClass::props migrate to ObjectClass::properties in device_initfn.


So I guess we will either need to:
- merge all the properties to Object/ObjectClass or
- rename&teach device-list-properties to work with objects rather than
devices or
- add qom-list-properties which will only print QOM properties.


The latter is fairly simple, I'll post an rfc patch in few minutes, it will
do like this:


{"execute": "qom-list-properties", "arguments": {"typename":
"pseries-2.11-machine"}}


{   'return': [   {   'description': 'Set on/off to enable/disable graphics '
                                     'emulation',
                      'name': 'graphics',
                      'type': 'bool'},
                  {   'description': 'The first phandle ID we may generate '
                                     'dynamically',
                      'name': 'phandle-start',
                      'type': 'int'},
                  {   'description': 'Include guest memory in  a core dump',
                      'name': 'dump-guest-core',
                      'type': 'bool'},
                  {   'description': 'Configure KVM in-kernel irqchip',
                      'name': 'kernel-irqchip',
                      'type': 'OnOffSplit'},
                  {   'description': 'Accelerator list',
                      'name': 'accel',
                      'type': 'string'},
                  {   'description': 'Linux kernel command line',
                      'name': 'append',
                      'type': 'string'},
                  {   'description': 'Dump current dtb to a file and quit',
                      'name': 'dumpdtb',
                      'type': 'string'},
                  {   'description': 'Set on/off to enable/disable igd '
                                     'passthrou',
                      'name': 'igd-passthru',
                      'type': 'bool'},
                  {   'description': 'Overrides the "compatible" property of '
                                     'the dt root node',
                      'name': 'dt-compatible',
                      'type': 'string'},
                  {   'description': 'Linux kernel image file',
                      'name': 'kernel',
                      'type': 'string'},
                  {   'description': 'Set on/off to enable/disable usb',
                      'name': 'usb',
                      'type': 'bool'},
                  {   'description': 'Set on to disable self-describing '
                                     'migration',
                      'name': 'suppress-vmdesc',
                      'type': 'bool'},
                  {   'description': 'Linux kernel device tree file',
                      'name': 'dtb',
                      'type': 'string'},
                  {   'description': 'Firmware image',
                      'name': 'firmware',
                      'type': 'string'},
                  {   'description': 'Enable/disable memory merge support',
                      'name': 'mem-merge',
                      'type': 'bool'},
                  {   'description': 'Linux initial ramdisk file',
                      'name': 'initrd',
                      'type': 'string'},
                  {   'description': 'Set on to enforce configuration section '
                                     'migration',
                      'name': 'enforce-config-section',
                      'type': 'bool'},
                  {   'description': 'KVM shadow MMU size',
                      'name': 'kvm-shadow-mem',
                      'type': 'int'},
                  {   'description': 'Allow Decimal Floating Point (DFP)',
                      'name': 'cap-dfp',
                      'type': 'bool'},
                  {   'description': 'Allow Hardware Transactional Memory '
                                     '(HTM)',
                      'name': 'cap-htm',
                      'type': 'bool'},
                  {   'description': 'Allow Vector Scalar Extensions (VSX)',
                      'name': 'cap-vsx',
                      'type': 'bool'},
                  {'name': 'type', 'type': 'string'},
                  {   'description': 'Resizing of the Hash Page Table '
                                     '(enabled, disabled, required)',
                      'name': 'resize-hpt',
                      'type': 'string'},
                  {   'description': 'Maximum permitted CPU compatibility '
                                     'mode. Valid values are power6, power7, '
                                     'power7+, power8, power9.',
                      'name': 'max-cpu-compat',
                      'type': 'string'},
                  {   'description': 'Use dedicated hotplug event mechanism
in '
                                     'place of standard EPOW events when '
                                     'possible (required for memory
hot-unplug '
                                     'support)',
                      'name': 'modern-hotplug-events',
                      'type': 'bool'},
                  {   'description': 'Virtual SMT: KVM behaves as if this
were '
                                     "the host's SMT mode",
                      'name': 'vsmt',
                      'type': 'uint32'},
                  {   'description': 'Specifies the KVM virtualization mode '
                                     '(HV, PR)',
                      'name': 'kvm-type',
                      'type': 'string'},
                  {'name': 'vfio-no-msix-emulation', 'type': 'bool'}]}


-- 
Alexey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]