QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5 0/3] ppc: Enable 2nd DAWR support on Power10
@ 2021-04-12 11:44 Ravi Bangoria
  2021-04-12 11:44 ` [PATCH v5 1/3] Linux headers: update from 5.12-rc3 Ravi Bangoria
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Ravi Bangoria @ 2021-04-12 11:44 UTC (permalink / raw)
  To: paulus, david
  Cc: ravi.bangoria, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	qemu-ppc, clg, pbonzini

This series enables 2nd DAWR support on p10 qemu guest. 2nd
DAWR is new watchpoint added in Power10 processor. Kernel/kvm
patches are already in[1]. Watchpoint on powerpc TCG guest is
not supported and thus 2nd DAWR is not enabled for TCG mode.

Patches apply fine on qemu/master branch (f2afdc2ad94b).

v4: https://lore.kernel.org/r/20210406053833.282907-1-ravi.bangoria@linux.ibm.com
v3->v4:
  - Make error message more proper.

v3: https://lore.kernel.org/r/20210330095350.36309-1-ravi.bangoria@linux.ibm.com
v3->v4:
  - spapr_dt_pa_features(): POWER10 processor is compatible with 3.0
    (PCR_COMPAT_3_00). No need to ppc_check_compat(3_10) for now as
    ppc_check_compati(3_00) will also be true. ppc_check_compat(3_10)
    can be added while introducing pa_features_310 in future.
  - Use error_append_hint() for hints. Also add ERRP_GUARD().
  - Add kvmppc_set_cap_dawr1() stub function for CONFIG_KVM=n.

v2: https://lore.kernel.org/r/20210329041906.213991-1-ravi.bangoria@linux.ibm.com
v2->v3:
  - Don't introduce pa_features_310[], instead, reuse pa_features_300[]
    for 3.1 guests, as there is no difference between initial values of
    them atm.
  - Call gen_spr_book3s_310_dbg() from init_proc_POWER10() instead of
    init_proc_POWER8(). Also, Don't call gen_spr_book3s_207_dbg() from
    gen_spr_book3s_310_dbg() as init_proc_POWER10() already calls it.

v1: https://lore.kernel.org/r/20200723104220.314671-1-ravi.bangoria@linux.ibm.com
[Apologies for long gap]
v1->v2:
  - Introduce machine capability cap-dawr1 to enable/disable
    the feature. By default, 2nd DAWR is OFF for guests even
    when host kvm supports it. User has to manually enable it
    with -machine cap-dawr1=on if he wishes to use it.
  - Split the header file changes into separate patch. (Sync
    headers from v5.12-rc3)

[1] https://git.kernel.org/torvalds/c/bd1de1a0e6eff

Ravi Bangoria (3):
  Linux headers: update from 5.12-rc3
  ppc: Rename current DAWR macros and variables
  ppc: Enable 2nd DAWR support on p10

 hw/ppc/spapr.c                                |  7 +-
 hw/ppc/spapr_caps.c                           | 32 +++++++
 include/hw/ppc/spapr.h                        |  8 +-
 include/standard-headers/drm/drm_fourcc.h     | 23 ++++-
 include/standard-headers/linux/input.h        |  2 +-
 .../standard-headers/rdma/vmw_pvrdma-abi.h    |  7 ++
 linux-headers/asm-generic/unistd.h            |  4 +-
 linux-headers/asm-mips/unistd_n32.h           |  1 +
 linux-headers/asm-mips/unistd_n64.h           |  1 +
 linux-headers/asm-mips/unistd_o32.h           |  1 +
 linux-headers/asm-powerpc/kvm.h               |  2 +
 linux-headers/asm-powerpc/unistd_32.h         |  1 +
 linux-headers/asm-powerpc/unistd_64.h         |  1 +
 linux-headers/asm-s390/unistd_32.h            |  1 +
 linux-headers/asm-s390/unistd_64.h            |  1 +
 linux-headers/asm-x86/kvm.h                   |  1 +
 linux-headers/asm-x86/unistd_32.h             |  1 +
 linux-headers/asm-x86/unistd_64.h             |  1 +
 linux-headers/asm-x86/unistd_x32.h            |  1 +
 linux-headers/linux/kvm.h                     | 89 +++++++++++++++++++
 linux-headers/linux/vfio.h                    | 27 ++++++
 target/ppc/cpu.h                              |  6 +-
 target/ppc/kvm.c                              | 12 +++
 target/ppc/kvm_ppc.h                          | 12 +++
 target/ppc/translate_init.c.inc               | 19 +++-
 25 files changed, 250 insertions(+), 11 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 1/3] Linux headers: update from 5.12-rc3
  2021-04-12 11:44 [PATCH v5 0/3] ppc: Enable 2nd DAWR support on Power10 Ravi Bangoria
@ 2021-04-12 11:44 ` Ravi Bangoria
  2021-04-12 11:44 ` [PATCH v5 2/3] ppc: Rename current DAWR macros and variables Ravi Bangoria
  2021-04-12 11:44 ` [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10 Ravi Bangoria
  2 siblings, 0 replies; 13+ messages in thread
From: Ravi Bangoria @ 2021-04-12 11:44 UTC (permalink / raw)
  To: paulus, david
  Cc: ravi.bangoria, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	qemu-ppc, clg, pbonzini

Update against Linux 5.12-rc3

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 include/standard-headers/drm/drm_fourcc.h     | 23 ++++-
 include/standard-headers/linux/input.h        |  2 +-
 .../standard-headers/rdma/vmw_pvrdma-abi.h    |  7 ++
 linux-headers/asm-generic/unistd.h            |  4 +-
 linux-headers/asm-mips/unistd_n32.h           |  1 +
 linux-headers/asm-mips/unistd_n64.h           |  1 +
 linux-headers/asm-mips/unistd_o32.h           |  1 +
 linux-headers/asm-powerpc/kvm.h               |  2 +
 linux-headers/asm-powerpc/unistd_32.h         |  1 +
 linux-headers/asm-powerpc/unistd_64.h         |  1 +
 linux-headers/asm-s390/unistd_32.h            |  1 +
 linux-headers/asm-s390/unistd_64.h            |  1 +
 linux-headers/asm-x86/kvm.h                   |  1 +
 linux-headers/asm-x86/unistd_32.h             |  1 +
 linux-headers/asm-x86/unistd_64.h             |  1 +
 linux-headers/asm-x86/unistd_x32.h            |  1 +
 linux-headers/linux/kvm.h                     | 89 +++++++++++++++++++
 linux-headers/linux/vfio.h                    | 27 ++++++
 18 files changed, 161 insertions(+), 4 deletions(-)

diff --git a/include/standard-headers/drm/drm_fourcc.h b/include/standard-headers/drm/drm_fourcc.h
index c47e19810c..a61ae520c2 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -526,6 +526,25 @@ extern "C" {
  */
 #define I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS fourcc_mod_code(INTEL, 7)
 
+/*
+ * Intel Color Control Surface with Clear Color (CCS) for Gen-12 render
+ * compression.
+ *
+ * The main surface is Y-tiled and is at plane index 0 whereas CCS is linear
+ * and at index 1. The clear color is stored at index 2, and the pitch should
+ * be ignored. The clear color structure is 256 bits. The first 128 bits
+ * represents Raw Clear Color Red, Green, Blue and Alpha color each represented
+ * by 32 bits. The raw clear color is consumed by the 3d engine and generates
+ * the converted clear color of size 64 bits. The first 32 bits store the Lower
+ * Converted Clear Color value and the next 32 bits store the Higher Converted
+ * Clear Color value when applicable. The Converted Clear Color values are
+ * consumed by the DE. The last 64 bits are used to store Color Discard Enable
+ * and Depth Clear Value Valid which are ignored by the DE. A CCS cache line
+ * corresponds to an area of 4x1 tiles in the main surface. The main surface
+ * pitch is required to be a multiple of 4 tile widths.
+ */
+#define I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC fourcc_mod_code(INTEL, 8)
+
 /*
  * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
  *
@@ -1035,9 +1054,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(uint64_t modifier)
  * Not all combinations are valid, and different SoCs may support different
  * combinations of layout and options.
  */
-#define __fourcc_mod_amlogic_layout_mask 0xf
+#define __fourcc_mod_amlogic_layout_mask 0xff
 #define __fourcc_mod_amlogic_options_shift 8
-#define __fourcc_mod_amlogic_options_mask 0xf
+#define __fourcc_mod_amlogic_options_mask 0xff
 
 #define DRM_FORMAT_MOD_AMLOGIC_FBC(__layout, __options) \
 	fourcc_mod_code(AMLOGIC, \
diff --git a/include/standard-headers/linux/input.h b/include/standard-headers/linux/input.h
index f89c986190..7822c24178 100644
--- a/include/standard-headers/linux/input.h
+++ b/include/standard-headers/linux/input.h
@@ -81,7 +81,7 @@ struct input_id {
  * in units per radian.
  * When INPUT_PROP_ACCELEROMETER is set the resolution changes.
  * The main axes (ABS_X, ABS_Y, ABS_Z) are then reported in
- * in units per g (units/g) and in units per degree per second
+ * units per g (units/g) and in units per degree per second
  * (units/deg/s) for rotational axes (ABS_RX, ABS_RY, ABS_RZ).
  */
 struct input_absinfo {
diff --git a/include/standard-headers/rdma/vmw_pvrdma-abi.h b/include/standard-headers/rdma/vmw_pvrdma-abi.h
index 0989426a3f..c30182a7ae 100644
--- a/include/standard-headers/rdma/vmw_pvrdma-abi.h
+++ b/include/standard-headers/rdma/vmw_pvrdma-abi.h
@@ -133,6 +133,13 @@ enum pvrdma_wc_flags {
 	PVRDMA_WC_FLAGS_MAX		= PVRDMA_WC_WITH_NETWORK_HDR_TYPE,
 };
 
+enum pvrdma_network_type {
+	PVRDMA_NETWORK_IB,
+	PVRDMA_NETWORK_ROCE_V1 = PVRDMA_NETWORK_IB,
+	PVRDMA_NETWORK_IPV4,
+	PVRDMA_NETWORK_IPV6
+};
+
 struct pvrdma_alloc_ucontext_resp {
 	uint32_t qp_tab_size;
 	uint32_t reserved;
diff --git a/linux-headers/asm-generic/unistd.h b/linux-headers/asm-generic/unistd.h
index 7287529177..ce58cff99b 100644
--- a/linux-headers/asm-generic/unistd.h
+++ b/linux-headers/asm-generic/unistd.h
@@ -861,9 +861,11 @@ __SYSCALL(__NR_faccessat2, sys_faccessat2)
 __SYSCALL(__NR_process_madvise, sys_process_madvise)
 #define __NR_epoll_pwait2 441
 __SC_COMP(__NR_epoll_pwait2, sys_epoll_pwait2, compat_sys_epoll_pwait2)
+#define __NR_mount_setattr 442
+__SYSCALL(__NR_mount_setattr, sys_mount_setattr)
 
 #undef __NR_syscalls
-#define __NR_syscalls 442
+#define __NR_syscalls 443
 
 /*
  * 32 bit systems traditionally used different
diff --git a/linux-headers/asm-mips/unistd_n32.h b/linux-headers/asm-mips/unistd_n32.h
index 59e53b6e07..2ca45a0122 100644
--- a/linux-headers/asm-mips/unistd_n32.h
+++ b/linux-headers/asm-mips/unistd_n32.h
@@ -371,6 +371,7 @@
 #define __NR_faccessat2	(__NR_Linux + 439)
 #define __NR_process_madvise	(__NR_Linux + 440)
 #define __NR_epoll_pwait2	(__NR_Linux + 441)
+#define __NR_mount_setattr	(__NR_Linux + 442)
 
 
 #endif /* _ASM_MIPS_UNISTD_N32_H */
diff --git a/linux-headers/asm-mips/unistd_n64.h b/linux-headers/asm-mips/unistd_n64.h
index 683558a7f8..c8df45e69c 100644
--- a/linux-headers/asm-mips/unistd_n64.h
+++ b/linux-headers/asm-mips/unistd_n64.h
@@ -347,6 +347,7 @@
 #define __NR_faccessat2	(__NR_Linux + 439)
 #define __NR_process_madvise	(__NR_Linux + 440)
 #define __NR_epoll_pwait2	(__NR_Linux + 441)
+#define __NR_mount_setattr	(__NR_Linux + 442)
 
 
 #endif /* _ASM_MIPS_UNISTD_N64_H */
diff --git a/linux-headers/asm-mips/unistd_o32.h b/linux-headers/asm-mips/unistd_o32.h
index ca6a7e5c0b..10ba4cf9f5 100644
--- a/linux-headers/asm-mips/unistd_o32.h
+++ b/linux-headers/asm-mips/unistd_o32.h
@@ -417,6 +417,7 @@
 #define __NR_faccessat2	(__NR_Linux + 439)
 #define __NR_process_madvise	(__NR_Linux + 440)
 #define __NR_epoll_pwait2	(__NR_Linux + 441)
+#define __NR_mount_setattr	(__NR_Linux + 442)
 
 
 #endif /* _ASM_MIPS_UNISTD_O32_H */
diff --git a/linux-headers/asm-powerpc/kvm.h b/linux-headers/asm-powerpc/kvm.h
index c3af3f324c..9f18fa090f 100644
--- a/linux-headers/asm-powerpc/kvm.h
+++ b/linux-headers/asm-powerpc/kvm.h
@@ -644,6 +644,8 @@ struct kvm_ppc_cpu_char {
 #define KVM_REG_PPC_MMCR3	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc1)
 #define KVM_REG_PPC_SIER2	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc2)
 #define KVM_REG_PPC_SIER3	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc3)
+#define KVM_REG_PPC_DAWR1	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc4)
+#define KVM_REG_PPC_DAWRX1	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc5)
 
 /* Transactional Memory checkpointed state:
  * This is all GPRs, all VSX regs and a subset of SPRs
diff --git a/linux-headers/asm-powerpc/unistd_32.h b/linux-headers/asm-powerpc/unistd_32.h
index 4624c90043..1d63e42fc4 100644
--- a/linux-headers/asm-powerpc/unistd_32.h
+++ b/linux-headers/asm-powerpc/unistd_32.h
@@ -424,6 +424,7 @@
 #define __NR_faccessat2	439
 #define __NR_process_madvise	440
 #define __NR_epoll_pwait2	441
+#define __NR_mount_setattr	442
 
 
 #endif /* _ASM_POWERPC_UNISTD_32_H */
diff --git a/linux-headers/asm-powerpc/unistd_64.h b/linux-headers/asm-powerpc/unistd_64.h
index 7e851b30bb..6a8708c0c5 100644
--- a/linux-headers/asm-powerpc/unistd_64.h
+++ b/linux-headers/asm-powerpc/unistd_64.h
@@ -396,6 +396,7 @@
 #define __NR_faccessat2	439
 #define __NR_process_madvise	440
 #define __NR_epoll_pwait2	441
+#define __NR_mount_setattr	442
 
 
 #endif /* _ASM_POWERPC_UNISTD_64_H */
diff --git a/linux-headers/asm-s390/unistd_32.h b/linux-headers/asm-s390/unistd_32.h
index c94d2c3a22..e5efe406e3 100644
--- a/linux-headers/asm-s390/unistd_32.h
+++ b/linux-headers/asm-s390/unistd_32.h
@@ -414,5 +414,6 @@
 #define __NR_faccessat2 439
 #define __NR_process_madvise 440
 #define __NR_epoll_pwait2 441
+#define __NR_mount_setattr 442
 
 #endif /* _ASM_S390_UNISTD_32_H */
diff --git a/linux-headers/asm-s390/unistd_64.h b/linux-headers/asm-s390/unistd_64.h
index 984a06b7eb..f0392fc6c7 100644
--- a/linux-headers/asm-s390/unistd_64.h
+++ b/linux-headers/asm-s390/unistd_64.h
@@ -362,5 +362,6 @@
 #define __NR_faccessat2 439
 #define __NR_process_madvise 440
 #define __NR_epoll_pwait2 441
+#define __NR_mount_setattr 442
 
 #endif /* _ASM_S390_UNISTD_64_H */
diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 8e76d3701d..5a3022c8af 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -112,6 +112,7 @@ struct kvm_ioapic_state {
 #define KVM_NR_IRQCHIPS          3
 
 #define KVM_RUN_X86_SMM		 (1 << 0)
+#define KVM_RUN_X86_BUS_LOCK     (1 << 1)
 
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
diff --git a/linux-headers/asm-x86/unistd_32.h b/linux-headers/asm-x86/unistd_32.h
index 18fb99dfa2..1374427c66 100644
--- a/linux-headers/asm-x86/unistd_32.h
+++ b/linux-headers/asm-x86/unistd_32.h
@@ -432,6 +432,7 @@
 #define __NR_faccessat2 439
 #define __NR_process_madvise 440
 #define __NR_epoll_pwait2 441
+#define __NR_mount_setattr 442
 
 
 #endif /* _ASM_X86_UNISTD_32_H */
diff --git a/linux-headers/asm-x86/unistd_64.h b/linux-headers/asm-x86/unistd_64.h
index bde959328d..e9d0707bc3 100644
--- a/linux-headers/asm-x86/unistd_64.h
+++ b/linux-headers/asm-x86/unistd_64.h
@@ -354,6 +354,7 @@
 #define __NR_faccessat2 439
 #define __NR_process_madvise 440
 #define __NR_epoll_pwait2 441
+#define __NR_mount_setattr 442
 
 
 #endif /* _ASM_X86_UNISTD_64_H */
diff --git a/linux-headers/asm-x86/unistd_x32.h b/linux-headers/asm-x86/unistd_x32.h
index 4ff6b17d3b..107aee76f2 100644
--- a/linux-headers/asm-x86/unistd_x32.h
+++ b/linux-headers/asm-x86/unistd_x32.h
@@ -307,6 +307,7 @@
 #define __NR_faccessat2 (__X32_SYSCALL_BIT + 439)
 #define __NR_process_madvise (__X32_SYSCALL_BIT + 440)
 #define __NR_epoll_pwait2 (__X32_SYSCALL_BIT + 441)
+#define __NR_mount_setattr (__X32_SYSCALL_BIT + 442)
 #define __NR_rt_sigaction (__X32_SYSCALL_BIT + 512)
 #define __NR_rt_sigreturn (__X32_SYSCALL_BIT + 513)
 #define __NR_ioctl (__X32_SYSCALL_BIT + 514)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 020b62a619..238c6c5847 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -216,6 +216,20 @@ struct kvm_hyperv_exit {
 	} u;
 };
 
+struct kvm_xen_exit {
+#define KVM_EXIT_XEN_HCALL          1
+	__u32 type;
+	union {
+		struct {
+			__u32 longmode;
+			__u32 cpl;
+			__u64 input;
+			__u64 result;
+			__u64 params[6];
+		} hcall;
+	} u;
+};
+
 #define KVM_S390_GET_SKEYS_NONE   1
 #define KVM_S390_SKEYS_MAX        1048576
 
@@ -251,6 +265,9 @@ struct kvm_hyperv_exit {
 #define KVM_EXIT_X86_RDMSR        29
 #define KVM_EXIT_X86_WRMSR        30
 #define KVM_EXIT_DIRTY_RING_FULL  31
+#define KVM_EXIT_AP_RESET_HOLD    32
+#define KVM_EXIT_X86_BUS_LOCK     33
+#define KVM_EXIT_XEN              34
 
 /* For KVM_EXIT_INTERNAL_ERROR */
 /* Emulate instruction failed. */
@@ -427,6 +444,8 @@ struct kvm_run {
 			__u32 index; /* kernel -> user */
 			__u64 data; /* kernel <-> user */
 		} msr;
+		/* KVM_EXIT_XEN */
+		struct kvm_xen_exit xen;
 		/* Fix the size of the union. */
 		char padding[256];
 	};
@@ -573,6 +592,7 @@ struct kvm_vapic_addr {
 #define KVM_MP_STATE_CHECK_STOP        6
 #define KVM_MP_STATE_OPERATING         7
 #define KVM_MP_STATE_LOAD              8
+#define KVM_MP_STATE_AP_RESET_HOLD     9
 
 struct kvm_mp_state {
 	__u32 mp_state;
@@ -1056,6 +1076,8 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
 #define KVM_CAP_SYS_HYPERV_CPUID 191
 #define KVM_CAP_DIRTY_LOG_RING 192
+#define KVM_CAP_X86_BUS_LOCK_EXIT 193
+#define KVM_CAP_PPC_DAWR1 194
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1129,6 +1151,11 @@ struct kvm_x86_mce {
 #endif
 
 #ifdef KVM_CAP_XEN_HVM
+#define KVM_XEN_HVM_CONFIG_HYPERCALL_MSR	(1 << 0)
+#define KVM_XEN_HVM_CONFIG_INTERCEPT_HCALL	(1 << 1)
+#define KVM_XEN_HVM_CONFIG_SHARED_INFO		(1 << 2)
+#define KVM_XEN_HVM_CONFIG_RUNSTATE		(1 << 3)
+
 struct kvm_xen_hvm_config {
 	__u32 flags;
 	__u32 msr;
@@ -1563,6 +1590,57 @@ struct kvm_pv_cmd {
 /* Available with KVM_CAP_DIRTY_LOG_RING */
 #define KVM_RESET_DIRTY_RINGS		_IO(KVMIO, 0xc7)
 
+/* Per-VM Xen attributes */
+#define KVM_XEN_HVM_GET_ATTR	_IOWR(KVMIO, 0xc8, struct kvm_xen_hvm_attr)
+#define KVM_XEN_HVM_SET_ATTR	_IOW(KVMIO,  0xc9, struct kvm_xen_hvm_attr)
+
+struct kvm_xen_hvm_attr {
+	__u16 type;
+	__u16 pad[3];
+	union {
+		__u8 long_mode;
+		__u8 vector;
+		struct {
+			__u64 gfn;
+		} shared_info;
+		__u64 pad[8];
+	} u;
+};
+
+/* Available with KVM_CAP_XEN_HVM / KVM_XEN_HVM_CONFIG_SHARED_INFO */
+#define KVM_XEN_ATTR_TYPE_LONG_MODE		0x0
+#define KVM_XEN_ATTR_TYPE_SHARED_INFO		0x1
+#define KVM_XEN_ATTR_TYPE_UPCALL_VECTOR		0x2
+
+/* Per-vCPU Xen attributes */
+#define KVM_XEN_VCPU_GET_ATTR	_IOWR(KVMIO, 0xca, struct kvm_xen_vcpu_attr)
+#define KVM_XEN_VCPU_SET_ATTR	_IOW(KVMIO,  0xcb, struct kvm_xen_vcpu_attr)
+
+struct kvm_xen_vcpu_attr {
+	__u16 type;
+	__u16 pad[3];
+	union {
+		__u64 gpa;
+		__u64 pad[8];
+		struct {
+			__u64 state;
+			__u64 state_entry_time;
+			__u64 time_running;
+			__u64 time_runnable;
+			__u64 time_blocked;
+			__u64 time_offline;
+		} runstate;
+	} u;
+};
+
+/* Available with KVM_CAP_XEN_HVM / KVM_XEN_HVM_CONFIG_SHARED_INFO */
+#define KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO	0x0
+#define KVM_XEN_VCPU_ATTR_TYPE_VCPU_TIME_INFO	0x1
+#define KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_ADDR	0x2
+#define KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_CURRENT	0x3
+#define KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_DATA	0x4
+#define KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_ADJUST	0x5
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */
@@ -1591,6 +1669,8 @@ enum sev_cmd_id {
 	KVM_SEV_DBG_ENCRYPT,
 	/* Guest certificates commands */
 	KVM_SEV_CERT_EXPORT,
+	/* Attestation report */
+	KVM_SEV_GET_ATTESTATION_REPORT,
 
 	KVM_SEV_NR_MAX,
 };
@@ -1643,6 +1723,12 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_attestation_report {
+	__u8 mnonce[16];
+	__u64 uaddr;
+	__u32 len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
@@ -1764,4 +1850,7 @@ struct kvm_dirty_gfn {
 	__u64 offset;
 };
 
+#define KVM_BUS_LOCK_DETECTION_OFF             (1 << 0)
+#define KVM_BUS_LOCK_DETECTION_EXIT            (1 << 1)
+
 #endif /* __LINUX_KVM_H */
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 609099e455..e38a488403 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -46,6 +46,12 @@
  */
 #define VFIO_NOIOMMU_IOMMU		8
 
+/* Supports VFIO_DMA_UNMAP_FLAG_ALL */
+#define VFIO_UNMAP_ALL			9
+
+/* Supports the vaddr flag for DMA map and unmap */
+#define VFIO_UPDATE_VADDR		10
+
 /*
  * The IOCTL interface is designed for extensibility by embedding the
  * structure length (argsz) and flags into structures passed between
@@ -1074,12 +1080,22 @@ struct vfio_iommu_type1_info_dma_avail {
  *
  * Map process virtual addresses to IO virtual addresses using the
  * provided struct vfio_dma_map. Caller sets argsz. READ &/ WRITE required.
+ *
+ * If flags & VFIO_DMA_MAP_FLAG_VADDR, update the base vaddr for iova, and
+ * unblock translation of host virtual addresses in the iova range.  The vaddr
+ * must have previously been invalidated with VFIO_DMA_UNMAP_FLAG_VADDR.  To
+ * maintain memory consistency within the user application, the updated vaddr
+ * must address the same memory object as originally mapped.  Failure to do so
+ * will result in user memory corruption and/or device misbehavior.  iova and
+ * size must match those in the original MAP_DMA call.  Protection is not
+ * changed, and the READ & WRITE flags must be 0.
  */
 struct vfio_iommu_type1_dma_map {
 	__u32	argsz;
 	__u32	flags;
 #define VFIO_DMA_MAP_FLAG_READ (1 << 0)		/* readable from device */
 #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1)	/* writable from device */
+#define VFIO_DMA_MAP_FLAG_VADDR (1 << 2)
 	__u64	vaddr;				/* Process virtual address */
 	__u64	iova;				/* IO virtual address */
 	__u64	size;				/* Size of mapping (bytes) */
@@ -1102,6 +1118,7 @@ struct vfio_bitmap {
  * field.  No guarantee is made to the user that arbitrary unmaps of iova
  * or size different from those used in the original mapping call will
  * succeed.
+ *
  * VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP should be set to get the dirty bitmap
  * before unmapping IO virtual addresses. When this flag is set, the user must
  * provide a struct vfio_bitmap in data[]. User must provide zero-allocated
@@ -1111,11 +1128,21 @@ struct vfio_bitmap {
  * indicates that the page at that offset from iova is dirty. A Bitmap of the
  * pages in the range of unmapped size is returned in the user-provided
  * vfio_bitmap.data.
+ *
+ * If flags & VFIO_DMA_UNMAP_FLAG_ALL, unmap all addresses.  iova and size
+ * must be 0.  This cannot be combined with the get-dirty-bitmap flag.
+ *
+ * If flags & VFIO_DMA_UNMAP_FLAG_VADDR, do not unmap, but invalidate host
+ * virtual addresses in the iova range.  Tasks that attempt to translate an
+ * iova's vaddr will block.  DMA to already-mapped pages continues.  This
+ * cannot be combined with the get-dirty-bitmap flag.
  */
 struct vfio_iommu_type1_dma_unmap {
 	__u32	argsz;
 	__u32	flags;
 #define VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP (1 << 0)
+#define VFIO_DMA_UNMAP_FLAG_ALL		     (1 << 1)
+#define VFIO_DMA_UNMAP_FLAG_VADDR	     (1 << 2)
 	__u64	iova;				/* IO virtual address */
 	__u64	size;				/* Size of mapping (bytes) */
 	__u8    data[];
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 2/3] ppc: Rename current DAWR macros and variables
  2021-04-12 11:44 [PATCH v5 0/3] ppc: Enable 2nd DAWR support on Power10 Ravi Bangoria
  2021-04-12 11:44 ` [PATCH v5 1/3] Linux headers: update from 5.12-rc3 Ravi Bangoria
@ 2021-04-12 11:44 ` Ravi Bangoria
  2021-04-19  4:47   ` David Gibson
  2021-04-12 11:44 ` [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10 Ravi Bangoria
  2 siblings, 1 reply; 13+ messages in thread
From: Ravi Bangoria @ 2021-04-12 11:44 UTC (permalink / raw)
  To: paulus, david
  Cc: ravi.bangoria, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	qemu-ppc, clg, pbonzini

Power10 is introducing second DAWR. Use real register names (with
suffix 0) from ISA for current macros and variables used by Qemu.

One exception to this is KVM_REG_PPC_DAWR[X]. This is from kernel
uapi header and thus not changed in kernel as well as Qemu.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/hw/ppc/spapr.h          | 2 +-
 target/ppc/cpu.h                | 4 ++--
 target/ppc/translate_init.c.inc | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index bf7cab7a2c..5f90bb26d5 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -363,7 +363,7 @@ struct SpaprMachineState {
 
 /* Values for 2nd argument to H_SET_MODE */
 #define H_SET_MODE_RESOURCE_SET_CIABR           1
-#define H_SET_MODE_RESOURCE_SET_DAWR            2
+#define H_SET_MODE_RESOURCE_SET_DAWR0           2
 #define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE     3
 #define H_SET_MODE_RESOURCE_LE                  4
 
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..cd02d65303 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1459,10 +1459,10 @@ typedef PowerPCCPU ArchCPU;
 #define SPR_MPC_BAR           (0x09F)
 #define SPR_PSPB              (0x09F)
 #define SPR_DPDES             (0x0B0)
-#define SPR_DAWR              (0x0B4)
+#define SPR_DAWR0             (0x0B4)
 #define SPR_RPR               (0x0BA)
 #define SPR_CIABR             (0x0BB)
-#define SPR_DAWRX             (0x0BC)
+#define SPR_DAWRX0            (0x0BC)
 #define SPR_HFSCR             (0x0BE)
 #define SPR_VRSAVE            (0x100)
 #define SPR_USPRG0            (0x100)
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index c03a7c4f52..879e6df217 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -7748,12 +7748,12 @@ static void gen_spr_book3s_dbg(CPUPPCState *env)
 
 static void gen_spr_book3s_207_dbg(CPUPPCState *env)
 {
-    spr_register_kvm_hv(env, SPR_DAWR, "DAWR",
+    spr_register_kvm_hv(env, SPR_DAWR0, "DAWR0",
                         SPR_NOACCESS, SPR_NOACCESS,
                         SPR_NOACCESS, SPR_NOACCESS,
                         &spr_read_generic, &spr_write_generic,
                         KVM_REG_PPC_DAWR, 0x00000000);
-    spr_register_kvm_hv(env, SPR_DAWRX, "DAWRX",
+    spr_register_kvm_hv(env, SPR_DAWRX0, "DAWRX0",
                         SPR_NOACCESS, SPR_NOACCESS,
                         SPR_NOACCESS, SPR_NOACCESS,
                         &spr_read_generic, &spr_write_generic,
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-12 11:44 [PATCH v5 0/3] ppc: Enable 2nd DAWR support on Power10 Ravi Bangoria
  2021-04-12 11:44 ` [PATCH v5 1/3] Linux headers: update from 5.12-rc3 Ravi Bangoria
  2021-04-12 11:44 ` [PATCH v5 2/3] ppc: Rename current DAWR macros and variables Ravi Bangoria
@ 2021-04-12 11:44 ` Ravi Bangoria
  2021-04-12 13:10   ` Cédric Le Goater
  2021-04-19  4:53   ` David Gibson
  2 siblings, 2 replies; 13+ messages in thread
From: Ravi Bangoria @ 2021-04-12 11:44 UTC (permalink / raw)
  To: paulus, david
  Cc: ravi.bangoria, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	qemu-ppc, clg, pbonzini

As per the PAPR, bit 0 of byte 64 in pa-features property indicates
availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
find whether kvm supports 2nd DAWR or not. If it's supported, allow
user to set the pa-feature bit in guest DT using cap-dawr1 machine
capability. Though, watchpoint on powerpc TCG guest is not supported
and thus 2nd DAWR is not enabled for TCG mode.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c                  |  7 ++++++-
 hw/ppc/spapr_caps.c             | 32 ++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h          |  6 +++++-
 target/ppc/cpu.h                |  2 ++
 target/ppc/kvm.c                | 12 ++++++++++++
 target/ppc/kvm_ppc.h            | 12 ++++++++++++
 target/ppc/translate_init.c.inc | 15 +++++++++++++++
 7 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 73a06df3b1..6317fad973 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -238,7 +238,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
         /* 54: DecFP, 56: DecI, 58: SHA */
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
-        /* 60: NM atomic, 62: RNG */
+        /* 60: NM atomic, 62: RNG, 64: DAWR1 (ISA 3.1) */
         0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
     };
     uint8_t *pa_features = NULL;
@@ -279,6 +279,9 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
          * in pa-features. So hide it from them. */
         pa_features[40 + 2] &= ~0x80; /* Radix MMU */
     }
+    if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
+        pa_features[66] |= 0x80;
+    }
 
     _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
 }
@@ -2003,6 +2006,7 @@ static const VMStateDescription vmstate_spapr = {
         &vmstate_spapr_cap_ccf_assist,
         &vmstate_spapr_cap_fwnmi,
         &vmstate_spapr_fwnmi,
+        &vmstate_spapr_cap_dawr1,
         NULL
     }
 };
@@ -4542,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
     smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
     smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
+    smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
     spapr_caps_add_properties(smc);
     smc->irq = &spapr_irq_dual;
     smc->dr_phb_enabled = true;
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 9ea7ddd1e9..98a6b15f29 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -523,6 +523,28 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
     }
 }
 
+static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
+                               Error **errp)
+{
+    ERRP_GUARD();
+    if (!val) {
+        return; /* Disable by default */
+    }
+
+    if (tcg_enabled()) {
+        error_setg(errp, "DAWR1 not supported in TCG.");
+        error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
+    } else if (kvm_enabled()) {
+        if (!kvmppc_has_cap_dawr1()) {
+            error_setg(errp, "DAWR1 not supported by KVM.");
+            error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
+        } else if (kvmppc_set_cap_dawr1(val) < 0) {
+            error_setg(errp, "Error enabling cap-dawr1 with KVM.");
+            error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
+        }
+    }
+}
+
 SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
     [SPAPR_CAP_HTM] = {
         .name = "htm",
@@ -631,6 +653,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
         .type = "bool",
         .apply = cap_fwnmi_apply,
     },
+    [SPAPR_CAP_DAWR1] = {
+        .name = "dawr1",
+        .description = "Allow 2nd Data Address Watchpoint Register (DAWR1)",
+        .index = SPAPR_CAP_DAWR1,
+        .get = spapr_cap_get_bool,
+        .set = spapr_cap_set_bool,
+        .type = "bool",
+        .apply = cap_dawr1_apply,
+    },
 };
 
 static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -771,6 +802,7 @@ SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
 SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
 SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
 SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
+SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
 
 void spapr_caps_init(SpaprMachineState *spapr)
 {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5f90bb26d5..51202b7c90 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -74,8 +74,10 @@ typedef enum {
 #define SPAPR_CAP_CCF_ASSIST            0x09
 /* Implements PAPR FWNMI option */
 #define SPAPR_CAP_FWNMI                 0x0A
+/* DAWR1 */
+#define SPAPR_CAP_DAWR1                 0x0B
 /* Num Caps */
-#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
+#define SPAPR_CAP_NUM                   (SPAPR_CAP_DAWR1 + 1)
 
 /*
  * Capability Values
@@ -366,6 +368,7 @@ struct SpaprMachineState {
 #define H_SET_MODE_RESOURCE_SET_DAWR0           2
 #define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE     3
 #define H_SET_MODE_RESOURCE_LE                  4
+#define H_SET_MODE_RESOURCE_SET_DAWR1           5
 
 /* Flags for H_SET_MODE_RESOURCE_LE */
 #define H_SET_MODE_ENDIAN_BIG    0
@@ -921,6 +924,7 @@ extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
 extern const VMStateDescription vmstate_spapr_cap_large_decr;
 extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
 extern const VMStateDescription vmstate_spapr_cap_fwnmi;
+extern const VMStateDescription vmstate_spapr_cap_dawr1;
 
 static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
 {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index cd02d65303..6a60416559 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1460,9 +1460,11 @@ typedef PowerPCCPU ArchCPU;
 #define SPR_PSPB              (0x09F)
 #define SPR_DPDES             (0x0B0)
 #define SPR_DAWR0             (0x0B4)
+#define SPR_DAWR1             (0x0B5)
 #define SPR_RPR               (0x0BA)
 #define SPR_CIABR             (0x0BB)
 #define SPR_DAWRX0            (0x0BC)
+#define SPR_DAWRX1            (0x0BD)
 #define SPR_HFSCR             (0x0BE)
 #define SPR_VRSAVE            (0x100)
 #define SPR_USPRG0            (0x100)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 104a308abb..fe3e8a13bb 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -89,6 +89,7 @@ static int cap_ppc_count_cache_flush_assist;
 static int cap_ppc_nested_kvm_hv;
 static int cap_large_decr;
 static int cap_fwnmi;
+static int cap_dawr1;
 
 static uint32_t debug_inst_opcode;
 
@@ -138,6 +139,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
     cap_large_decr = kvmppc_get_dec_bits();
     cap_fwnmi = kvm_vm_check_extension(s, KVM_CAP_PPC_FWNMI);
+    cap_dawr1 = kvm_vm_check_extension(s, KVM_CAP_PPC_DAWR1);
     /*
      * Note: setting it to false because there is not such capability
      * in KVM at this moment.
@@ -2091,6 +2093,16 @@ int kvmppc_set_fwnmi(PowerPCCPU *cpu)
     return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
 }
 
+bool kvmppc_has_cap_dawr1(void)
+{
+    return !!cap_dawr1;
+}
+
+int kvmppc_set_cap_dawr1(int enable)
+{
+    return kvm_vm_enable_cap(kvm_state, KVM_CAP_PPC_DAWR1, 0, enable);
+}
+
 int kvmppc_smt_threads(void)
 {
     return cap_ppc_smt ? cap_ppc_smt : 1;
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 989f61ace0..47248fbbfd 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -63,6 +63,8 @@ bool kvmppc_has_cap_htm(void);
 bool kvmppc_has_cap_mmu_radix(void);
 bool kvmppc_has_cap_mmu_hash_v3(void);
 bool kvmppc_has_cap_xive(void);
+bool kvmppc_has_cap_dawr1(void);
+int kvmppc_set_cap_dawr1(int enable);
 int kvmppc_get_cap_safe_cache(void);
 int kvmppc_get_cap_safe_bounds_check(void);
 int kvmppc_get_cap_safe_indirect_branch(void);
@@ -341,6 +343,16 @@ static inline bool kvmppc_has_cap_xive(void)
     return false;
 }
 
+static inline bool kvmppc_has_cap_dawr1(void)
+{
+    return false;
+}
+
+static inline int kvmppc_set_cap_dawr1(int enable)
+{
+    abort();
+}
+
 static inline int kvmppc_get_cap_safe_cache(void)
 {
     return 0;
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 879e6df217..8b76e191f1 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -7765,6 +7765,20 @@ static void gen_spr_book3s_207_dbg(CPUPPCState *env)
                         KVM_REG_PPC_CIABR, 0x00000000);
 }
 
+static void gen_spr_book3s_310_dbg(CPUPPCState *env)
+{
+    spr_register_kvm_hv(env, SPR_DAWR1, "DAWR1",
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        &spr_read_generic, &spr_write_generic,
+                        KVM_REG_PPC_DAWR1, 0x00000000);
+    spr_register_kvm_hv(env, SPR_DAWRX1, "DAWRX1",
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        SPR_NOACCESS, SPR_NOACCESS,
+                        &spr_read_generic, &spr_write_generic,
+                        KVM_REG_PPC_DAWRX1, 0x00000000);
+}
+
 static void gen_spr_970_dbg(CPUPPCState *env)
 {
     /* Breakpoints */
@@ -9142,6 +9156,7 @@ static void init_proc_POWER10(CPUPPCState *env)
     /* Common Registers */
     init_proc_book3s_common(env);
     gen_spr_book3s_207_dbg(env);
+    gen_spr_book3s_310_dbg(env);
 
     /* POWER8 Specific Registers */
     gen_spr_book3s_ids(env);
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-12 11:44 ` [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10 Ravi Bangoria
@ 2021-04-12 13:10   ` Cédric Le Goater
  2021-04-19  4:53   ` David Gibson
  1 sibling, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2021-04-12 13:10 UTC (permalink / raw)
  To: Ravi Bangoria, paulus, david
  Cc: mikey, kvm, mst, mpe, cohuck, qemu-devel, groug, qemu-ppc, pbonzini

On 4/12/21 1:44 PM, Ravi Bangoria wrote:
> As per the PAPR, bit 0 of byte 64 in pa-features property indicates
> availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
> find whether kvm supports 2nd DAWR or not. If it's supported, allow
> user to set the pa-feature bit in guest DT using cap-dawr1 machine
> capability. Though, watchpoint on powerpc TCG guest is not supported
> and thus 2nd DAWR is not enabled for TCG mode.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>

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

Thanks,

C.

> ---
>  hw/ppc/spapr.c                  |  7 ++++++-
>  hw/ppc/spapr_caps.c             | 32 ++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h          |  6 +++++-
>  target/ppc/cpu.h                |  2 ++
>  target/ppc/kvm.c                | 12 ++++++++++++
>  target/ppc/kvm_ppc.h            | 12 ++++++++++++
>  target/ppc/translate_init.c.inc | 15 +++++++++++++++
>  7 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 73a06df3b1..6317fad973 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -238,7 +238,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>          0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
>          /* 54: DecFP, 56: DecI, 58: SHA */
>          0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> -        /* 60: NM atomic, 62: RNG */
> +        /* 60: NM atomic, 62: RNG, 64: DAWR1 (ISA 3.1) */
>          0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>      };
>      uint8_t *pa_features = NULL;
> @@ -279,6 +279,9 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           * in pa-features. So hide it from them. */
>          pa_features[40 + 2] &= ~0x80; /* Radix MMU */
>      }
> +    if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
> +        pa_features[66] |= 0x80;
> +    }
>  
>      _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
>  }
> @@ -2003,6 +2006,7 @@ static const VMStateDescription vmstate_spapr = {
>          &vmstate_spapr_cap_ccf_assist,
>          &vmstate_spapr_cap_fwnmi,
>          &vmstate_spapr_fwnmi,
> +        &vmstate_spapr_cap_dawr1,
>          NULL
>      }
>  };
> @@ -4542,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> +    smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
>      spapr_caps_add_properties(smc);
>      smc->irq = &spapr_irq_dual;
>      smc->dr_phb_enabled = true;
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9ea7ddd1e9..98a6b15f29 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -523,6 +523,28 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>      }
>  }
>  
> +static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
> +                               Error **errp)
> +{
> +    ERRP_GUARD();
> +    if (!val) {
> +        return; /* Disable by default */
> +    }
> +
> +    if (tcg_enabled()) {
> +        error_setg(errp, "DAWR1 not supported in TCG.");
> +        error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
> +    } else if (kvm_enabled()) {
> +        if (!kvmppc_has_cap_dawr1()) {
> +            error_setg(errp, "DAWR1 not supported by KVM.");
> +            error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
> +        } else if (kvmppc_set_cap_dawr1(val) < 0) {
> +            error_setg(errp, "Error enabling cap-dawr1 with KVM.");
> +            error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
> +        }
> +    }
> +}
> +
>  SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>      [SPAPR_CAP_HTM] = {
>          .name = "htm",
> @@ -631,6 +653,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>          .type = "bool",
>          .apply = cap_fwnmi_apply,
>      },
> +    [SPAPR_CAP_DAWR1] = {
> +        .name = "dawr1",
> +        .description = "Allow 2nd Data Address Watchpoint Register (DAWR1)",
> +        .index = SPAPR_CAP_DAWR1,
> +        .get = spapr_cap_get_bool,
> +        .set = spapr_cap_set_bool,
> +        .type = "bool",
> +        .apply = cap_dawr1_apply,
> +    },
>  };
>  
>  static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> @@ -771,6 +802,7 @@ SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
>  SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
> +SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
>  
>  void spapr_caps_init(SpaprMachineState *spapr)
>  {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5f90bb26d5..51202b7c90 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -74,8 +74,10 @@ typedef enum {
>  #define SPAPR_CAP_CCF_ASSIST            0x09
>  /* Implements PAPR FWNMI option */
>  #define SPAPR_CAP_FWNMI                 0x0A
> +/* DAWR1 */
> +#define SPAPR_CAP_DAWR1                 0x0B
>  /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_DAWR1 + 1)
>  
>  /*
>   * Capability Values
> @@ -366,6 +368,7 @@ struct SpaprMachineState {
>  #define H_SET_MODE_RESOURCE_SET_DAWR0           2
>  #define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE     3
>  #define H_SET_MODE_RESOURCE_LE                  4
> +#define H_SET_MODE_RESOURCE_SET_DAWR1           5
>  
>  /* Flags for H_SET_MODE_RESOURCE_LE */
>  #define H_SET_MODE_ENDIAN_BIG    0
> @@ -921,6 +924,7 @@ extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
>  extern const VMStateDescription vmstate_spapr_cap_large_decr;
>  extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
>  extern const VMStateDescription vmstate_spapr_cap_fwnmi;
> +extern const VMStateDescription vmstate_spapr_cap_dawr1;
>  
>  static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
>  {
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index cd02d65303..6a60416559 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1460,9 +1460,11 @@ typedef PowerPCCPU ArchCPU;
>  #define SPR_PSPB              (0x09F)
>  #define SPR_DPDES             (0x0B0)
>  #define SPR_DAWR0             (0x0B4)
> +#define SPR_DAWR1             (0x0B5)
>  #define SPR_RPR               (0x0BA)
>  #define SPR_CIABR             (0x0BB)
>  #define SPR_DAWRX0            (0x0BC)
> +#define SPR_DAWRX1            (0x0BD)
>  #define SPR_HFSCR             (0x0BE)
>  #define SPR_VRSAVE            (0x100)
>  #define SPR_USPRG0            (0x100)
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb..fe3e8a13bb 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -89,6 +89,7 @@ static int cap_ppc_count_cache_flush_assist;
>  static int cap_ppc_nested_kvm_hv;
>  static int cap_large_decr;
>  static int cap_fwnmi;
> +static int cap_dawr1;
>  
>  static uint32_t debug_inst_opcode;
>  
> @@ -138,6 +139,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
>      cap_large_decr = kvmppc_get_dec_bits();
>      cap_fwnmi = kvm_vm_check_extension(s, KVM_CAP_PPC_FWNMI);
> +    cap_dawr1 = kvm_vm_check_extension(s, KVM_CAP_PPC_DAWR1);
>      /*
>       * Note: setting it to false because there is not such capability
>       * in KVM at this moment.
> @@ -2091,6 +2093,16 @@ int kvmppc_set_fwnmi(PowerPCCPU *cpu)
>      return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>  }
>  
> +bool kvmppc_has_cap_dawr1(void)
> +{
> +    return !!cap_dawr1;
> +}
> +
> +int kvmppc_set_cap_dawr1(int enable)
> +{
> +    return kvm_vm_enable_cap(kvm_state, KVM_CAP_PPC_DAWR1, 0, enable);
> +}
> +
>  int kvmppc_smt_threads(void)
>  {
>      return cap_ppc_smt ? cap_ppc_smt : 1;
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 989f61ace0..47248fbbfd 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -63,6 +63,8 @@ bool kvmppc_has_cap_htm(void);
>  bool kvmppc_has_cap_mmu_radix(void);
>  bool kvmppc_has_cap_mmu_hash_v3(void);
>  bool kvmppc_has_cap_xive(void);
> +bool kvmppc_has_cap_dawr1(void);
> +int kvmppc_set_cap_dawr1(int enable);
>  int kvmppc_get_cap_safe_cache(void);
>  int kvmppc_get_cap_safe_bounds_check(void);
>  int kvmppc_get_cap_safe_indirect_branch(void);
> @@ -341,6 +343,16 @@ static inline bool kvmppc_has_cap_xive(void)
>      return false;
>  }
>  
> +static inline bool kvmppc_has_cap_dawr1(void)
> +{
> +    return false;
> +}
> +
> +static inline int kvmppc_set_cap_dawr1(int enable)
> +{
> +    abort();
> +}
> +
>  static inline int kvmppc_get_cap_safe_cache(void)
>  {
>      return 0;
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 879e6df217..8b76e191f1 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -7765,6 +7765,20 @@ static void gen_spr_book3s_207_dbg(CPUPPCState *env)
>                          KVM_REG_PPC_CIABR, 0x00000000);
>  }
>  
> +static void gen_spr_book3s_310_dbg(CPUPPCState *env)
> +{
> +    spr_register_kvm_hv(env, SPR_DAWR1, "DAWR1",
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        &spr_read_generic, &spr_write_generic,
> +                        KVM_REG_PPC_DAWR1, 0x00000000);
> +    spr_register_kvm_hv(env, SPR_DAWRX1, "DAWRX1",
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        &spr_read_generic, &spr_write_generic,
> +                        KVM_REG_PPC_DAWRX1, 0x00000000);
> +}
> +
>  static void gen_spr_970_dbg(CPUPPCState *env)
>  {
>      /* Breakpoints */
> @@ -9142,6 +9156,7 @@ static void init_proc_POWER10(CPUPPCState *env)
>      /* Common Registers */
>      init_proc_book3s_common(env);
>      gen_spr_book3s_207_dbg(env);
> +    gen_spr_book3s_310_dbg(env);
>  
>      /* POWER8 Specific Registers */
>      gen_spr_book3s_ids(env);
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 2/3] ppc: Rename current DAWR macros and variables
  2021-04-12 11:44 ` [PATCH v5 2/3] ppc: Rename current DAWR macros and variables Ravi Bangoria
@ 2021-04-19  4:47   ` David Gibson
  0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2021-04-19  4:47 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: qemu-ppc, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	paulus, clg, pbonzini


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

On Mon, Apr 12, 2021 at 05:14:32PM +0530, Ravi Bangoria wrote:
> Power10 is introducing second DAWR. Use real register names (with
> suffix 0) from ISA for current macros and variables used by Qemu.
> 
> One exception to this is KVM_REG_PPC_DAWR[X]. This is from kernel
> uapi header and thus not changed in kernel as well as Qemu.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

This stands independently of the other patches, so I've applied it to ppc-for-6.1.

> ---
>  include/hw/ppc/spapr.h          | 2 +-
>  target/ppc/cpu.h                | 4 ++--
>  target/ppc/translate_init.c.inc | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index bf7cab7a2c..5f90bb26d5 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -363,7 +363,7 @@ struct SpaprMachineState {
>  
>  /* Values for 2nd argument to H_SET_MODE */
>  #define H_SET_MODE_RESOURCE_SET_CIABR           1
> -#define H_SET_MODE_RESOURCE_SET_DAWR            2
> +#define H_SET_MODE_RESOURCE_SET_DAWR0           2
>  #define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE     3
>  #define H_SET_MODE_RESOURCE_LE                  4
>  
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..cd02d65303 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1459,10 +1459,10 @@ typedef PowerPCCPU ArchCPU;
>  #define SPR_MPC_BAR           (0x09F)
>  #define SPR_PSPB              (0x09F)
>  #define SPR_DPDES             (0x0B0)
> -#define SPR_DAWR              (0x0B4)
> +#define SPR_DAWR0             (0x0B4)
>  #define SPR_RPR               (0x0BA)
>  #define SPR_CIABR             (0x0BB)
> -#define SPR_DAWRX             (0x0BC)
> +#define SPR_DAWRX0            (0x0BC)
>  #define SPR_HFSCR             (0x0BE)
>  #define SPR_VRSAVE            (0x100)
>  #define SPR_USPRG0            (0x100)
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index c03a7c4f52..879e6df217 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -7748,12 +7748,12 @@ static void gen_spr_book3s_dbg(CPUPPCState *env)
>  
>  static void gen_spr_book3s_207_dbg(CPUPPCState *env)
>  {
> -    spr_register_kvm_hv(env, SPR_DAWR, "DAWR",
> +    spr_register_kvm_hv(env, SPR_DAWR0, "DAWR0",
>                          SPR_NOACCESS, SPR_NOACCESS,
>                          SPR_NOACCESS, SPR_NOACCESS,
>                          &spr_read_generic, &spr_write_generic,
>                          KVM_REG_PPC_DAWR, 0x00000000);
> -    spr_register_kvm_hv(env, SPR_DAWRX, "DAWRX",
> +    spr_register_kvm_hv(env, SPR_DAWRX0, "DAWRX0",
>                          SPR_NOACCESS, SPR_NOACCESS,
>                          SPR_NOACCESS, SPR_NOACCESS,
>                          &spr_read_generic, &spr_write_generic,

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-12 11:44 ` [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10 Ravi Bangoria
  2021-04-12 13:10   ` Cédric Le Goater
@ 2021-04-19  4:53   ` David Gibson
  2021-04-21  6:20     ` Ravi Bangoria
  1 sibling, 1 reply; 13+ messages in thread
From: David Gibson @ 2021-04-19  4:53 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: qemu-ppc, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	paulus, clg, pbonzini


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

On Mon, Apr 12, 2021 at 05:14:33PM +0530, Ravi Bangoria wrote:
> As per the PAPR, bit 0 of byte 64 in pa-features property indicates
> availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
> find whether kvm supports 2nd DAWR or not. If it's supported, allow
> user to set the pa-feature bit in guest DT using cap-dawr1 machine
> capability. Though, watchpoint on powerpc TCG guest is not supported
> and thus 2nd DAWR is not enabled for TCG mode.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>

So, I'm actually not sure if using an spapr capability is what we want
to do here.  The problem is that presumably the idea is to at some
point make the DAWR1 capability default to on (on POWER10, at least).
But at that point you'll no longer to be able to start TCG guests
without explicitly disabling it.  That's technically correct, since we
don't implement DAWR1 in TCG, but then we also don't implement DAWR0
and we let that slide... which I think is probably going to cause less
irritation on balance.

I'm wondering if we're actually just better off setting the pa feature
just based on the guest CPU model.  TCG will be broken if you try to
use it, but then, it already is.  AFAIK there's no inherent reason we
couldn't implement DAWR support in TCG, it's just never been worth the
trouble.

> ---
>  hw/ppc/spapr.c                  |  7 ++++++-
>  hw/ppc/spapr_caps.c             | 32 ++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h          |  6 +++++-
>  target/ppc/cpu.h                |  2 ++
>  target/ppc/kvm.c                | 12 ++++++++++++
>  target/ppc/kvm_ppc.h            | 12 ++++++++++++
>  target/ppc/translate_init.c.inc | 15 +++++++++++++++
>  7 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 73a06df3b1..6317fad973 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -238,7 +238,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>          0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
>          /* 54: DecFP, 56: DecI, 58: SHA */
>          0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
> -        /* 60: NM atomic, 62: RNG */
> +        /* 60: NM atomic, 62: RNG, 64: DAWR1 (ISA 3.1) */
>          0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
>      };
>      uint8_t *pa_features = NULL;
> @@ -279,6 +279,9 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>           * in pa-features. So hide it from them. */
>          pa_features[40 + 2] &= ~0x80; /* Radix MMU */
>      }
> +    if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
> +        pa_features[66] |= 0x80;
> +    }
>  
>      _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
>  }
> @@ -2003,6 +2006,7 @@ static const VMStateDescription vmstate_spapr = {
>          &vmstate_spapr_cap_ccf_assist,
>          &vmstate_spapr_cap_fwnmi,
>          &vmstate_spapr_fwnmi,
> +        &vmstate_spapr_cap_dawr1,
>          NULL
>      }
>  };
> @@ -4542,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> +    smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
>      spapr_caps_add_properties(smc);
>      smc->irq = &spapr_irq_dual;
>      smc->dr_phb_enabled = true;
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9ea7ddd1e9..98a6b15f29 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -523,6 +523,28 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>      }
>  }
>  
> +static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
> +                               Error **errp)
> +{
> +    ERRP_GUARD();
> +    if (!val) {
> +        return; /* Disable by default */
> +    }
> +
> +    if (tcg_enabled()) {
> +        error_setg(errp, "DAWR1 not supported in TCG.");
> +        error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
> +    } else if (kvm_enabled()) {
> +        if (!kvmppc_has_cap_dawr1()) {
> +            error_setg(errp, "DAWR1 not supported by KVM.");
> +            error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
> +        } else if (kvmppc_set_cap_dawr1(val) < 0) {
> +            error_setg(errp, "Error enabling cap-dawr1 with KVM.");
> +            error_append_hint(errp, "Try appending -machine cap-dawr1=off\n");
> +        }
> +    }
> +}
> +
>  SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>      [SPAPR_CAP_HTM] = {
>          .name = "htm",
> @@ -631,6 +653,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>          .type = "bool",
>          .apply = cap_fwnmi_apply,
>      },
> +    [SPAPR_CAP_DAWR1] = {
> +        .name = "dawr1",
> +        .description = "Allow 2nd Data Address Watchpoint Register (DAWR1)",
> +        .index = SPAPR_CAP_DAWR1,
> +        .get = spapr_cap_get_bool,
> +        .set = spapr_cap_set_bool,
> +        .type = "bool",
> +        .apply = cap_dawr1_apply,
> +    },
>  };
>  
>  static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> @@ -771,6 +802,7 @@ SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
>  SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
> +SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
>  
>  void spapr_caps_init(SpaprMachineState *spapr)
>  {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5f90bb26d5..51202b7c90 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -74,8 +74,10 @@ typedef enum {
>  #define SPAPR_CAP_CCF_ASSIST            0x09
>  /* Implements PAPR FWNMI option */
>  #define SPAPR_CAP_FWNMI                 0x0A
> +/* DAWR1 */
> +#define SPAPR_CAP_DAWR1                 0x0B
>  /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_DAWR1 + 1)
>  
>  /*
>   * Capability Values
> @@ -366,6 +368,7 @@ struct SpaprMachineState {
>  #define H_SET_MODE_RESOURCE_SET_DAWR0           2
>  #define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE     3
>  #define H_SET_MODE_RESOURCE_LE                  4
> +#define H_SET_MODE_RESOURCE_SET_DAWR1           5
>  
>  /* Flags for H_SET_MODE_RESOURCE_LE */
>  #define H_SET_MODE_ENDIAN_BIG    0
> @@ -921,6 +924,7 @@ extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
>  extern const VMStateDescription vmstate_spapr_cap_large_decr;
>  extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
>  extern const VMStateDescription vmstate_spapr_cap_fwnmi;
> +extern const VMStateDescription vmstate_spapr_cap_dawr1;
>  
>  static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
>  {
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index cd02d65303..6a60416559 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1460,9 +1460,11 @@ typedef PowerPCCPU ArchCPU;
>  #define SPR_PSPB              (0x09F)
>  #define SPR_DPDES             (0x0B0)
>  #define SPR_DAWR0             (0x0B4)
> +#define SPR_DAWR1             (0x0B5)
>  #define SPR_RPR               (0x0BA)
>  #define SPR_CIABR             (0x0BB)
>  #define SPR_DAWRX0            (0x0BC)
> +#define SPR_DAWRX1            (0x0BD)
>  #define SPR_HFSCR             (0x0BE)
>  #define SPR_VRSAVE            (0x100)
>  #define SPR_USPRG0            (0x100)
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb..fe3e8a13bb 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -89,6 +89,7 @@ static int cap_ppc_count_cache_flush_assist;
>  static int cap_ppc_nested_kvm_hv;
>  static int cap_large_decr;
>  static int cap_fwnmi;
> +static int cap_dawr1;
>  
>  static uint32_t debug_inst_opcode;
>  
> @@ -138,6 +139,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
>      cap_large_decr = kvmppc_get_dec_bits();
>      cap_fwnmi = kvm_vm_check_extension(s, KVM_CAP_PPC_FWNMI);
> +    cap_dawr1 = kvm_vm_check_extension(s, KVM_CAP_PPC_DAWR1);
>      /*
>       * Note: setting it to false because there is not such capability
>       * in KVM at this moment.
> @@ -2091,6 +2093,16 @@ int kvmppc_set_fwnmi(PowerPCCPU *cpu)
>      return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>  }
>  
> +bool kvmppc_has_cap_dawr1(void)
> +{
> +    return !!cap_dawr1;
> +}
> +
> +int kvmppc_set_cap_dawr1(int enable)
> +{
> +    return kvm_vm_enable_cap(kvm_state, KVM_CAP_PPC_DAWR1, 0, enable);
> +}
> +
>  int kvmppc_smt_threads(void)
>  {
>      return cap_ppc_smt ? cap_ppc_smt : 1;
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 989f61ace0..47248fbbfd 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -63,6 +63,8 @@ bool kvmppc_has_cap_htm(void);
>  bool kvmppc_has_cap_mmu_radix(void);
>  bool kvmppc_has_cap_mmu_hash_v3(void);
>  bool kvmppc_has_cap_xive(void);
> +bool kvmppc_has_cap_dawr1(void);
> +int kvmppc_set_cap_dawr1(int enable);
>  int kvmppc_get_cap_safe_cache(void);
>  int kvmppc_get_cap_safe_bounds_check(void);
>  int kvmppc_get_cap_safe_indirect_branch(void);
> @@ -341,6 +343,16 @@ static inline bool kvmppc_has_cap_xive(void)
>      return false;
>  }
>  
> +static inline bool kvmppc_has_cap_dawr1(void)
> +{
> +    return false;
> +}
> +
> +static inline int kvmppc_set_cap_dawr1(int enable)
> +{
> +    abort();
> +}
> +
>  static inline int kvmppc_get_cap_safe_cache(void)
>  {
>      return 0;
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 879e6df217..8b76e191f1 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -7765,6 +7765,20 @@ static void gen_spr_book3s_207_dbg(CPUPPCState *env)
>                          KVM_REG_PPC_CIABR, 0x00000000);
>  }
>  
> +static void gen_spr_book3s_310_dbg(CPUPPCState *env)
> +{
> +    spr_register_kvm_hv(env, SPR_DAWR1, "DAWR1",
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        &spr_read_generic, &spr_write_generic,
> +                        KVM_REG_PPC_DAWR1, 0x00000000);
> +    spr_register_kvm_hv(env, SPR_DAWRX1, "DAWRX1",
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        SPR_NOACCESS, SPR_NOACCESS,
> +                        &spr_read_generic, &spr_write_generic,
> +                        KVM_REG_PPC_DAWRX1, 0x00000000);
> +}
> +
>  static void gen_spr_970_dbg(CPUPPCState *env)
>  {
>      /* Breakpoints */
> @@ -9142,6 +9156,7 @@ static void init_proc_POWER10(CPUPPCState *env)
>      /* Common Registers */
>      init_proc_book3s_common(env);
>      gen_spr_book3s_207_dbg(env);
> +    gen_spr_book3s_310_dbg(env);
>  
>      /* POWER8 Specific Registers */
>      gen_spr_book3s_ids(env);

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-19  4:53   ` David Gibson
@ 2021-04-21  6:20     ` Ravi Bangoria
  2021-04-21  6:31       ` Cédric Le Goater
  2021-05-05  5:50       ` David Gibson
  0 siblings, 2 replies; 13+ messages in thread
From: Ravi Bangoria @ 2021-04-21  6:20 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-ppc, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	paulus, clg, pbonzini, Ravi Bangoria

Hi David,

On 4/19/21 10:23 AM, David Gibson wrote:
> On Mon, Apr 12, 2021 at 05:14:33PM +0530, Ravi Bangoria wrote:
>> As per the PAPR, bit 0 of byte 64 in pa-features property indicates
>> availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
>> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
>> find whether kvm supports 2nd DAWR or not. If it's supported, allow
>> user to set the pa-feature bit in guest DT using cap-dawr1 machine
>> capability. Though, watchpoint on powerpc TCG guest is not supported
>> and thus 2nd DAWR is not enabled for TCG mode.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> So, I'm actually not sure if using an spapr capability is what we want
> to do here.  The problem is that presumably the idea is to at some
> point make the DAWR1 capability default to on (on POWER10, at least).
> But at that point you'll no longer to be able to start TCG guests
> without explicitly disabling it.  That's technically correct, since we
> don't implement DAWR1 in TCG, but then we also don't implement DAWR0
> and we let that slide... which I think is probably going to cause less
> irritation on balance.

Ok. Probably something like this is what you want?

Power10 behavior:
   - KVM does not support DAWR1: Boot the guest without DAWR1
     support (No warnings). Error out only if user tries with
     cap-dawr1=on.
   - KVM supports DAWR1: Boot the guest with DAWR1 support, unless
     user specifies cap-dawr1=off.
   - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
     DAWR0 (Should be fixed in future while adding PowerPC watch-
     point support in TCG mode)

Power10 predecessor behavior:
   - KVM guest: Boot the guest without DAWR1 support. Error out
     if user tries with cap-dawr1=on.
   - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
     DAWR0 (Should be fixed in future while adding PowerPC watch-
     point support in TCG mode)

> I'm wondering if we're actually just better off setting the pa feature
> just based on the guest CPU model.  TCG will be broken if you try to
> use it, but then, it already is.  AFAIK there's no inherent reason we
> couldn't implement DAWR support in TCG, it's just never been worth the
> trouble.

Correct. Probably there is no practical usecase for DAWR in TCG mode.

Thanks,
Ravi


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-21  6:20     ` Ravi Bangoria
@ 2021-04-21  6:31       ` Cédric Le Goater
  2021-04-21  6:54         ` Ravi Bangoria
  2021-05-05  5:50       ` David Gibson
  1 sibling, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2021-04-21  6:31 UTC (permalink / raw)
  To: Ravi Bangoria, David Gibson
  Cc: mikey, kvm, mst, mpe, cohuck, qemu-devel, groug, qemu-ppc,
	pbonzini, paulus

On 4/21/21 8:20 AM, Ravi Bangoria wrote:
> Hi David,
> 
> On 4/19/21 10:23 AM, David Gibson wrote:
>> On Mon, Apr 12, 2021 at 05:14:33PM +0530, Ravi Bangoria wrote:
>>> As per the PAPR, bit 0 of byte 64 in pa-features property indicates
>>> availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
>>> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
>>> find whether kvm supports 2nd DAWR or not. If it's supported, allow
>>> user to set the pa-feature bit in guest DT using cap-dawr1 machine
>>> capability. Though, watchpoint on powerpc TCG guest is not supported
>>> and thus 2nd DAWR is not enabled for TCG mode.
>>>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>> Reviewed-by: Greg Kurz <groug@kaod.org>
>>
>> So, I'm actually not sure if using an spapr capability is what we want
>> to do here.  The problem is that presumably the idea is to at some
>> point make the DAWR1 capability default to on (on POWER10, at least).
>> But at that point you'll no longer to be able to start TCG guests
>> without explicitly disabling it.  That's technically correct, since we
>> don't implement DAWR1 in TCG, but then we also don't implement DAWR0
>> and we let that slide... which I think is probably going to cause less
>> irritation on balance.
> 
> Ok. Probably something like this is what you want?
> 
> Power10 behavior:
>   - KVM does not support DAWR1: Boot the guest without DAWR1
>     support (No warnings). Error out only if user tries with
>     cap-dawr1=on.
>   - KVM supports DAWR1: Boot the guest with DAWR1 support, unless
>     user specifies cap-dawr1=off.
>   - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
>     DAWR0 (Should be fixed in future while adding PowerPC watch-
>     point support in TCG mode)
> 
> Power10 predecessor behavior:
>   - KVM guest: Boot the guest without DAWR1 support. Error out
>     if user tries with cap-dawr1=on.
>   - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
>     DAWR0 (Should be fixed in future while adding PowerPC watch-
>     point support in TCG mode)
> 
>> I'm wondering if we're actually just better off setting the pa feature
>> just based on the guest CPU model.  TCG will be broken if you try to
>> use it, but then, it already is.  AFAIK there's no inherent reason we
>> couldn't implement DAWR support in TCG, it's just never been worth the
>> trouble.
> 
> Correct. Probably there is no practical usecase for DAWR in TCG mode.

What's the expected behavior ? Is it to generate a DSI if we have a DAWR
match ? 

C. 
 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-21  6:31       ` Cédric Le Goater
@ 2021-04-21  6:54         ` Ravi Bangoria
  2021-04-22  1:56           ` David Gibson
  0 siblings, 1 reply; 13+ messages in thread
From: Ravi Bangoria @ 2021-04-21  6:54 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Ravi Bangoria, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	qemu-ppc, pbonzini, paulus, David Gibson

Hi Cedric,

On 4/21/21 12:01 PM, Cédric Le Goater wrote:
> On 4/21/21 8:20 AM, Ravi Bangoria wrote:
>> Hi David,
>>
>> On 4/19/21 10:23 AM, David Gibson wrote:
>>> On Mon, Apr 12, 2021 at 05:14:33PM +0530, Ravi Bangoria wrote:
>>>> As per the PAPR, bit 0 of byte 64 in pa-features property indicates
>>>> availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
>>>> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
>>>> find whether kvm supports 2nd DAWR or not. If it's supported, allow
>>>> user to set the pa-feature bit in guest DT using cap-dawr1 machine
>>>> capability. Though, watchpoint on powerpc TCG guest is not supported
>>>> and thus 2nd DAWR is not enabled for TCG mode.
>>>>
>>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>>> Reviewed-by: Greg Kurz <groug@kaod.org>
>>>
>>> So, I'm actually not sure if using an spapr capability is what we want
>>> to do here.  The problem is that presumably the idea is to at some
>>> point make the DAWR1 capability default to on (on POWER10, at least).
>>> But at that point you'll no longer to be able to start TCG guests
>>> without explicitly disabling it.  That's technically correct, since we
>>> don't implement DAWR1 in TCG, but then we also don't implement DAWR0
>>> and we let that slide... which I think is probably going to cause less
>>> irritation on balance.
>>
>> Ok. Probably something like this is what you want?
>>
>> Power10 behavior:
>>    - KVM does not support DAWR1: Boot the guest without DAWR1
>>      support (No warnings). Error out only if user tries with
>>      cap-dawr1=on.
>>    - KVM supports DAWR1: Boot the guest with DAWR1 support, unless
>>      user specifies cap-dawr1=off.
>>    - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
>>      DAWR0 (Should be fixed in future while adding PowerPC watch-
>>      point support in TCG mode)
>>
>> Power10 predecessor behavior:
>>    - KVM guest: Boot the guest without DAWR1 support. Error out
>>      if user tries with cap-dawr1=on.
>>    - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
>>      DAWR0 (Should be fixed in future while adding PowerPC watch-
>>      point support in TCG mode)
>>
>>> I'm wondering if we're actually just better off setting the pa feature
>>> just based on the guest CPU model.  TCG will be broken if you try to
>>> use it, but then, it already is.  AFAIK there's no inherent reason we
>>> couldn't implement DAWR support in TCG, it's just never been worth the
>>> trouble.
>>
>> Correct. Probably there is no practical usecase for DAWR in TCG mode.
> 
> What's the expected behavior ? Is it to generate a DSI if we have a DAWR
> match ?

Yes. DSI is the main thing. But many auxiliary stuff, off the top of my
head:
  - DAR needs to be set. Now, DAR value is set differently on p8 vs p10
    (not sure about p9 because there was hw bug and thus we needed to
    fully disable DAWR on p9).
  - DAWR matching criteria for quadword instruction are different for
    p8/p9 vs p10.
  - P10 supports 512 byte unaligned watchpoints but p8/p9 does not.

Kernel is aware of these differences and thus handles these scenarios,
sometimes as special case. i.e. Qemu will need to mimic the exact hw
behavior for the specific revision of processor.

Thanks,
Ravi


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-21  6:54         ` Ravi Bangoria
@ 2021-04-22  1:56           ` David Gibson
  2021-04-22  4:45             ` Richard Henderson
  0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2021-04-22  1:56 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: qemu-ppc, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	paulus, Cédric Le Goater, pbonzini


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

On Wed, Apr 21, 2021 at 12:24:22PM +0530, Ravi Bangoria wrote:
> Hi Cedric,
> 
> On 4/21/21 12:01 PM, Cédric Le Goater wrote:
> > On 4/21/21 8:20 AM, Ravi Bangoria wrote:
> > > Hi David,
> > > 
> > > On 4/19/21 10:23 AM, David Gibson wrote:
> > > > On Mon, Apr 12, 2021 at 05:14:33PM +0530, Ravi Bangoria wrote:
> > > > > As per the PAPR, bit 0 of byte 64 in pa-features property indicates
> > > > > availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
> > > > > DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
> > > > > find whether kvm supports 2nd DAWR or not. If it's supported, allow
> > > > > user to set the pa-feature bit in guest DT using cap-dawr1 machine
> > > > > capability. Though, watchpoint on powerpc TCG guest is not supported
> > > > > and thus 2nd DAWR is not enabled for TCG mode.
> > > > > 
> > > > > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > > > 
> > > > So, I'm actually not sure if using an spapr capability is what we want
> > > > to do here.  The problem is that presumably the idea is to at some
> > > > point make the DAWR1 capability default to on (on POWER10, at least).
> > > > But at that point you'll no longer to be able to start TCG guests
> > > > without explicitly disabling it.  That's technically correct, since we
> > > > don't implement DAWR1 in TCG, but then we also don't implement DAWR0
> > > > and we let that slide... which I think is probably going to cause less
> > > > irritation on balance.
> > > 
> > > Ok. Probably something like this is what you want?
> > > 
> > > Power10 behavior:
> > >    - KVM does not support DAWR1: Boot the guest without DAWR1
> > >      support (No warnings). Error out only if user tries with
> > >      cap-dawr1=on.
> > >    - KVM supports DAWR1: Boot the guest with DAWR1 support, unless
> > >      user specifies cap-dawr1=off.
> > >    - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
> > >      DAWR0 (Should be fixed in future while adding PowerPC watch-
> > >      point support in TCG mode)
> > > 
> > > Power10 predecessor behavior:
> > >    - KVM guest: Boot the guest without DAWR1 support. Error out
> > >      if user tries with cap-dawr1=on.
> > >    - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
> > >      DAWR0 (Should be fixed in future while adding PowerPC watch-
> > >      point support in TCG mode)
> > > 
> > > > I'm wondering if we're actually just better off setting the pa feature
> > > > just based on the guest CPU model.  TCG will be broken if you try to
> > > > use it, but then, it already is.  AFAIK there's no inherent reason we
> > > > couldn't implement DAWR support in TCG, it's just never been worth the
> > > > trouble.
> > > 
> > > Correct. Probably there is no practical usecase for DAWR in TCG mode.
> > 
> > What's the expected behavior ? Is it to generate a DSI if we have a DAWR
> > match ?
> 
> Yes. DSI is the main thing. But many auxiliary stuff, off the top of my
> head:
>  - DAR needs to be set. Now, DAR value is set differently on p8 vs p10
>    (not sure about p9 because there was hw bug and thus we needed to
>    fully disable DAWR on p9).
>  - DAWR matching criteria for quadword instruction are different for
>    p8/p9 vs p10.
>  - P10 supports 512 byte unaligned watchpoints but p8/p9 does not.
> 
> Kernel is aware of these differences and thus handles these scenarios,
> sometimes as special case. i.e. Qemu will need to mimic the exact hw
> behavior for the specific revision of processor.

I don't actually know if qemu has TCG watchpoint support on any
hardware.  Presumably it would mean instrumenting all the tcg loads
and stores.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-22  1:56           ` David Gibson
@ 2021-04-22  4:45             ` Richard Henderson
  0 siblings, 0 replies; 13+ messages in thread
From: Richard Henderson @ 2021-04-22  4:45 UTC (permalink / raw)
  To: David Gibson, Ravi Bangoria
  Cc: mikey, kvm, mst, mpe, cohuck, qemu-devel, groug, qemu-ppc,
	Cédric Le Goater, pbonzini, paulus

On 4/21/21 6:56 PM, David Gibson wrote:
> I don't actually know if qemu has TCG watchpoint support on any
> hardware.  Presumably it would mean instrumenting all the tcg loads
> and stores.

We tag the soft tlb for pages that contain watchpoints.

See include/hw/core/cpu.h:
   cpu_watchpoint_insert
   cpu_watchpoint_remove


r~


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10
  2021-04-21  6:20     ` Ravi Bangoria
  2021-04-21  6:31       ` Cédric Le Goater
@ 2021-05-05  5:50       ` David Gibson
  1 sibling, 0 replies; 13+ messages in thread
From: David Gibson @ 2021-05-05  5:50 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: qemu-ppc, mikey, kvm, mst, mpe, cohuck, qemu-devel, groug,
	paulus, clg, pbonzini


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

On Wed, Apr 21, 2021 at 11:50:40AM +0530, Ravi Bangoria wrote:
> Hi David,
> 
> On 4/19/21 10:23 AM, David Gibson wrote:
> > On Mon, Apr 12, 2021 at 05:14:33PM +0530, Ravi Bangoria wrote:
> > > As per the PAPR, bit 0 of byte 64 in pa-features property indicates
> > > availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
> > > DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
> > > find whether kvm supports 2nd DAWR or not. If it's supported, allow
> > > user to set the pa-feature bit in guest DT using cap-dawr1 machine
> > > capability. Though, watchpoint on powerpc TCG guest is not supported
> > > and thus 2nd DAWR is not enabled for TCG mode.
> > > 
> > > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > Reviewed-by: Greg Kurz <groug@kaod.org>
> > 
> > So, I'm actually not sure if using an spapr capability is what we want
> > to do here.  The problem is that presumably the idea is to at some
> > point make the DAWR1 capability default to on (on POWER10, at least).
> > But at that point you'll no longer to be able to start TCG guests
> > without explicitly disabling it.  That's technically correct, since we
> > don't implement DAWR1 in TCG, but then we also don't implement DAWR0
> > and we let that slide... which I think is probably going to cause less
> > irritation on balance.
> 
> Ok. Probably something like this is what you want?
> 
> Power10 behavior:
>   - KVM does not support DAWR1: Boot the guest without DAWR1
>     support (No warnings). Error out only if user tries with
>     cap-dawr1=on.
>   - KVM supports DAWR1: Boot the guest with DAWR1 support, unless
>     user specifies cap-dawr1=off.
>   - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
>     DAWR0 (Should be fixed in future while adding PowerPC watch-
>     point support in TCG mode)
> 
> Power10 predecessor behavior:
>   - KVM guest: Boot the guest without DAWR1 support. Error out
>     if user tries with cap-dawr1=on.
>   - TCG guest: Ignore cap-dawr1 i.e. boot as if there is only
>     DAWR0 (Should be fixed in future while adding PowerPC watch-
>     point support in TCG mode)

Sorry I've neglected this thread so long.  I'm afraid the logic above
won't work.  As a general rule we never want to change guest-visible
details of the platform based on KVM capabilities, because it makes a
total mess of migration across clusters.

So, if we'd introduced this along with initial POWER10 support, I
don't think we'd need a capability at all: we just present DAWR1 on
POWER10, don't otherwise.  The fact it doesn't work in TCG we just
treat as a TCG bug we'll probably never get around to fixing, just
like TCG not supporting DAWR0 is a TCG bug right now.

Since we have released versions with POWER10 support, but no DAWR1, in
theory we need a capability so new qemu with old machine types don't
gain guest visible features that the same machine types on older qemus
had.

Except.. there's a loophole we might use to sidestep that.  The
current POWER10 CPU modelled in qemu is a DD1 - which I strongly
suspect will never appear outside of IBM.  I'm pretty sure we want to
replace that with a DD2.

While the modelled CPU is DD1, I think it's pretty reasonable to say
our POWER10 support hasn't yet stabilized, and it would therefore be
ok to simply add DAWR1 on POWER10 unconditionally, as long as we do it
before we switch over to DD2.

> > I'm wondering if we're actually just better off setting the pa feature
> > just based on the guest CPU model.  TCG will be broken if you try to
> > use it, but then, it already is.  AFAIK there's no inherent reason we
> > couldn't implement DAWR support in TCG, it's just never been worth the
> > trouble.
> 
> Correct. Probably there is no practical usecase for DAWR in TCG mode.
> 
> Thanks,
> Ravi
> 

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 11:44 [PATCH v5 0/3] ppc: Enable 2nd DAWR support on Power10 Ravi Bangoria
2021-04-12 11:44 ` [PATCH v5 1/3] Linux headers: update from 5.12-rc3 Ravi Bangoria
2021-04-12 11:44 ` [PATCH v5 2/3] ppc: Rename current DAWR macros and variables Ravi Bangoria
2021-04-19  4:47   ` David Gibson
2021-04-12 11:44 ` [PATCH v5 3/3] ppc: Enable 2nd DAWR support on p10 Ravi Bangoria
2021-04-12 13:10   ` Cédric Le Goater
2021-04-19  4:53   ` David Gibson
2021-04-21  6:20     ` Ravi Bangoria
2021-04-21  6:31       ` Cédric Le Goater
2021-04-21  6:54         ` Ravi Bangoria
2021-04-22  1:56           ` David Gibson
2021-04-22  4:45             ` Richard Henderson
2021-05-05  5:50       ` David Gibson

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git