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.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
hw/ppc/spapr.c | 7 ++++++-
hw/ppc/spapr_caps.c | 36 ++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_hcall.c | 25 ++++++++++++++++---------
include/hw/ppc/spapr.h | 6 +++++-
target/ppc/kvm.c | 12 ++++++++++++
target/ppc/kvm_ppc.h | 12 ++++++++++++
6 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e8dabc8614..91a97d72e7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -262,7 +262,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;
@@ -303,6 +303,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)));
}
@@ -2138,6 +2141,7 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_cap_fwnmi,
&vmstate_spapr_fwnmi,
&vmstate_spapr_cap_rpt_invalidate,
+ &vmstate_spapr_cap_dawr1,
NULL
}
};
@@ -4717,6 +4721,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
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_RPT_INVALIDATE] = SPAPR_CAP_OFF;
+ smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
/*
* This cap specifies whether the AIL 3 mode for
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index e889244e52..677f17cea6 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -655,6 +655,32 @@ static void cap_ail_mode_3_apply(SpaprMachineState *spapr,
}
}
+static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
+ Error **errp)
+{
+ ERRP_GUARD();
+
+ if (!val) {
+ return; /* Disable by default */
+ }
+
+ if (!ppc_type_check_compat(MACHINE(spapr)->cpu_type,
+ CPU_POWERPC_LOGICAL_3_10, 0,
+ spapr->max_compat_pvr)) {
+ warn_report("DAWR1 supported only on POWER10 and later CPUs");
+ }
+
+ 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");
+ } 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");
+ }
+ }
+}
+
SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
@@ -781,6 +807,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
.type = "bool",
.apply = cap_ail_mode_3_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,
@@ -923,6 +958,7 @@ 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(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
+SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
void spapr_caps_init(SpaprMachineState *spapr)
{
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index fcefd1d1c7..34c1c77c95 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -814,11 +814,12 @@ static target_ulong h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
return H_SUCCESS;
}
-static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
- SpaprMachineState *spapr,
- target_ulong mflags,
- target_ulong value1,
- target_ulong value2)
+static target_ulong h_set_mode_resource_set_dawr(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
+ target_ulong mflags,
+ target_ulong resource,
+ target_ulong value1,
+ target_ulong value2)
{
CPUPPCState *env = &cpu->env;
@@ -831,8 +832,13 @@ static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
return H_P4;
}
- ppc_store_dawr0(env, value1);
- ppc_store_dawrx0(env, value2);
+ if (resource == H_SET_MODE_RESOURCE_SET_DAWR0) {
+ ppc_store_dawr0(env, value1);
+ ppc_store_dawrx0(env, value2);
+ } else {
+ ppc_store_dawr1(env, value1);
+ ppc_store_dawrx1(env, value2);
+ }
return H_SUCCESS;
}
@@ -911,8 +917,9 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, SpaprMachineState *spapr,
args[3]);
break;
case H_SET_MODE_RESOURCE_SET_DAWR0:
- ret = h_set_mode_resource_set_dawr0(cpu, spapr, args[0], args[2],
- args[3]);
+ case H_SET_MODE_RESOURCE_SET_DAWR1:
+ ret = h_set_mode_resource_set_dawr(cpu, spapr, args[0], args[1],
+ args[2], args[3]);
break;
case H_SET_MODE_RESOURCE_LE:
ret = h_set_mode_resource_le(cpu, spapr, args[0], args[2], args[3]);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e91791a1a9..2b13c9a00e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -80,8 +80,10 @@ typedef enum {
#define SPAPR_CAP_RPT_INVALIDATE 0x0B
/* Support for AIL modes */
#define SPAPR_CAP_AIL_MODE_3 0x0C
+/* DAWR1 */
+#define SPAPR_CAP_DAWR1 0x0D
/* Num Caps */
-#define SPAPR_CAP_NUM (SPAPR_CAP_AIL_MODE_3 + 1)
+#define SPAPR_CAP_NUM (SPAPR_CAP_DAWR1 + 1)
/*
* Capability Values
@@ -403,6 +405,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
@@ -986,6 +989,7 @@ extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
extern const VMStateDescription vmstate_spapr_cap_fwnmi;
extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
extern const VMStateDescription vmstate_spapr_wdt;
+extern const VMStateDescription vmstate_spapr_cap_dawr1;
static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
{
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 26fa9d0575..3d8a8f35b6 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -89,6 +89,7 @@ static int cap_large_decr;
static int cap_fwnmi;
static int cap_rpt_invalidate;
static int cap_ail_mode_3;
+static int cap_dawr1;
static uint32_t debug_inst_opcode;
@@ -143,6 +144,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.
@@ -2109,6 +2111,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 1975fb5ee6..493d6bb477 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -68,6 +68,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);
@@ -377,6 +379,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;Hi Shiva,
On 2/1/24 20:16, Shivaprasad G Bhat 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.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
> hw/ppc/spapr.c | 7 ++++++-
> hw/ppc/spapr_caps.c | 36 ++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_hcall.c | 25 ++++++++++++++++---------
> include/hw/ppc/spapr.h | 6 +++++-
> target/ppc/kvm.c | 12 ++++++++++++
> target/ppc/kvm_ppc.h | 12 ++++++++++++
> 6 files changed, 87 insertions(+), 11 deletions(-)
>
<snipped>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index fcefd1d1c7..34c1c77c95 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -814,11 +814,12 @@ static target_ulong h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
> return H_SUCCESS;
> }
>
> -static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
> - SpaprMachineState *spapr,
> - target_ulong mflags,
> - target_ulong value1,
> - target_ulong value2)
> +static target_ulong h_set_mode_resource_set_dawr(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + target_ulong mflags,
> + target_ulong resource,
> + target_ulong value1,
> + target_ulong value2)
> {
> CPUPPCState *env = &cpu->env;
>
> @@ -831,8 +832,13 @@ static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
> return H_P4;
> }
>
> - ppc_store_dawr0(env, value1);
> - ppc_store_dawrx0(env, value2);
> + if (resource == H_SET_MODE_RESOURCE_SET_DAWR0) {
> + ppc_store_dawr0(env, value1);
> + ppc_store_dawrx0(env, value2);
> + } else {
Should we explicitly check here for resource ==
H_SET_MODE_RESOURCE_SET_DAWR0 ?
> + ppc_store_dawr1(env, value1);
> + ppc_store_dawrx1(env, value2);
and then do an else g_assert_not_reached() ?
With suggested changes after addressing other review comments from
Nick/David:
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> + }
>
> return H_SUCCESS;
> }
> @@ -911,8 +917,9 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, SpaprMachineState *spapr,
> args[3]);
> break;
> case H_SET_MODE_RESOURCE_SET_DAWR0:
> - ret = h_set_mode_resource_set_dawr0(cpu, spapr, args[0], args[2],
> - args[3]);
> + case H_SET_MODE_RESOURCE_SET_DAWR1:
> + ret = h_set_mode_resource_set_dawr(cpu, spapr, args[0], args[1],
> + args[2], args[3]);
> break;
> case H_SET_MODE_RESOURCE_LE:
> ret = h_set_mode_resource_le(cpu, spapr, args[0], args[2], args[3]);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index e91791a1a9..2b13c9a00e 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -80,8 +80,10 @@ typedef enum {
> #define SPAPR_CAP_RPT_INVALIDATE 0x0B
> /* Support for AIL modes */
> #define SPAPR_CAP_AIL_MODE_3 0x0C
> +/* DAWR1 */
> +#define SPAPR_CAP_DAWR1 0x0D
> /* Num Caps */
> -#define SPAPR_CAP_NUM (SPAPR_CAP_AIL_MODE_3 + 1)
> +#define SPAPR_CAP_NUM (SPAPR_CAP_DAWR1 + 1)
>
> /*
> * Capability Values
> @@ -403,6 +405,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
> @@ -986,6 +989,7 @@ extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
> extern const VMStateDescription vmstate_spapr_cap_fwnmi;
> extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
> extern const VMStateDescription vmstate_spapr_wdt;
> +extern const VMStateDescription vmstate_spapr_cap_dawr1;
>
> static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
> {
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 26fa9d0575..3d8a8f35b6 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -89,6 +89,7 @@ static int cap_large_decr;
> static int cap_fwnmi;
> static int cap_rpt_invalidate;
> static int cap_ail_mode_3;
> +static int cap_dawr1;
>
> static uint32_t debug_inst_opcode;
>
> @@ -143,6 +144,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.
> @@ -2109,6 +2111,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 1975fb5ee6..493d6bb477 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -68,6 +68,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);
> @@ -377,6 +379,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;
>
>
>
Typo corrected below.
On 12/20/24 15:27, Harsh Prateek Bora wrote:
> Hi Shiva,
>
> On 2/1/24 20:16, Shivaprasad G Bhat 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.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
>> ---
>> hw/ppc/spapr.c | 7 ++++++-
>> hw/ppc/spapr_caps.c | 36 ++++++++++++++++++++++++++++++++++++
>> hw/ppc/spapr_hcall.c | 25 ++++++++++++++++---------
>> include/hw/ppc/spapr.h | 6 +++++-
>> target/ppc/kvm.c | 12 ++++++++++++
>> target/ppc/kvm_ppc.h | 12 ++++++++++++
>> 6 files changed, 87 insertions(+), 11 deletions(-)
>>
>
> <snipped>
>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index fcefd1d1c7..34c1c77c95 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -814,11 +814,12 @@ static target_ulong
>> h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
>> return H_SUCCESS;
>> }
>> -static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
>> - SpaprMachineState
>> *spapr,
>> - target_ulong mflags,
>> - target_ulong value1,
>> - target_ulong value2)
>> +static target_ulong h_set_mode_resource_set_dawr(PowerPCCPU *cpu,
>> +
>> SpaprMachineState *spapr,
>> + target_ulong
>> mflags,
>> + target_ulong
>> resource,
>> + target_ulong
>> value1,
>> + target_ulong
>> value2)
>> {
>> CPUPPCState *env = &cpu->env;
>> @@ -831,8 +832,13 @@ static target_ulong
>> h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
>> return H_P4;
>> }
>> - ppc_store_dawr0(env, value1);
>> - ppc_store_dawrx0(env, value2);
>> + if (resource == H_SET_MODE_RESOURCE_SET_DAWR0) {
>> + ppc_store_dawr0(env, value1);
>> + ppc_store_dawrx0(env, value2);
>> + } else {
>
> Should we explicitly check here for resource ==
> H_SET_MODE_RESOURCE_SET_DAWR1 ?
Copy paste error corrected. Was referring to _DAWR1 here.
>
>> + ppc_store_dawr1(env, value1);
>> + ppc_store_dawrx1(env, value2);
>
> and then do an else g_assert_not_reached() ?
>
> With suggested changes after addressing other review comments from
> Nick/David:
>
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
>> + }
>> return H_SUCCESS;
>> }
>> @@ -911,8 +917,9 @@ static target_ulong h_set_mode(PowerPCCPU *cpu,
>> SpaprMachineState *spapr,
>> args[3]);
>> break;
>> case H_SET_MODE_RESOURCE_SET_DAWR0:
>> - ret = h_set_mode_resource_set_dawr0(cpu, spapr, args[0],
>> args[2],
>> - args[3]);
>> + case H_SET_MODE_RESOURCE_SET_DAWR1:
>> + ret = h_set_mode_resource_set_dawr(cpu, spapr, args[0], args[1],
>> + args[2], args[3]);
>> break;
>> case H_SET_MODE_RESOURCE_LE:
>> ret = h_set_mode_resource_le(cpu, spapr, args[0], args[2],
>> args[3]);
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index e91791a1a9..2b13c9a00e 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -80,8 +80,10 @@ typedef enum {
>> #define SPAPR_CAP_RPT_INVALIDATE 0x0B
>> /* Support for AIL modes */
>> #define SPAPR_CAP_AIL_MODE_3 0x0C
>> +/* DAWR1 */
>> +#define SPAPR_CAP_DAWR1 0x0D
>> /* Num Caps */
>> -#define SPAPR_CAP_NUM (SPAPR_CAP_AIL_MODE_3 + 1)
>> +#define SPAPR_CAP_NUM (SPAPR_CAP_DAWR1 + 1)
>> /*
>> * Capability Values
>> @@ -403,6 +405,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
>> @@ -986,6 +989,7 @@ extern const VMStateDescription
>> vmstate_spapr_cap_ccf_assist;
>> extern const VMStateDescription vmstate_spapr_cap_fwnmi;
>> extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
>> extern const VMStateDescription vmstate_spapr_wdt;
>> +extern const VMStateDescription vmstate_spapr_cap_dawr1;
>> static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
>> {
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 26fa9d0575..3d8a8f35b6 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -89,6 +89,7 @@ static int cap_large_decr;
>> static int cap_fwnmi;
>> static int cap_rpt_invalidate;
>> static int cap_ail_mode_3;
>> +static int cap_dawr1;
>> static uint32_t debug_inst_opcode;
>> @@ -143,6 +144,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.
>> @@ -2109,6 +2111,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 1975fb5ee6..493d6bb477 100644
>> --- a/target/ppc/kvm_ppc.h
>> +++ b/target/ppc/kvm_ppc.h
>> @@ -68,6 +68,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);
>> @@ -377,6 +379,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;
>>
>>
>>
>
On Fri Feb 2, 2024 at 12:46 AM AEST, Shivaprasad G Bhat 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.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
> hw/ppc/spapr.c | 7 ++++++-
> hw/ppc/spapr_caps.c | 36 ++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_hcall.c | 25 ++++++++++++++++---------
> include/hw/ppc/spapr.h | 6 +++++-
> target/ppc/kvm.c | 12 ++++++++++++
> target/ppc/kvm_ppc.h | 12 ++++++++++++
> 6 files changed, 87 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e8dabc8614..91a97d72e7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -262,7 +262,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;
> @@ -303,6 +303,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)));
> }
> @@ -2138,6 +2141,7 @@ static const VMStateDescription vmstate_spapr = {
> &vmstate_spapr_cap_fwnmi,
> &vmstate_spapr_fwnmi,
> &vmstate_spapr_cap_rpt_invalidate,
> + &vmstate_spapr_cap_dawr1,
> NULL
> }
> };
> @@ -4717,6 +4721,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> 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_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> + smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
>
> /*
> * This cap specifies whether the AIL 3 mode for
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index e889244e52..677f17cea6 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -655,6 +655,32 @@ static void cap_ail_mode_3_apply(SpaprMachineState *spapr,
> }
> }
>
> +static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
> + Error **errp)
> +{
> + ERRP_GUARD();
> +
> + if (!val) {
> + return; /* Disable by default */
> + }
> +
> + if (!ppc_type_check_compat(MACHINE(spapr)->cpu_type,
> + CPU_POWERPC_LOGICAL_3_10, 0,
> + spapr->max_compat_pvr)) {
> + warn_report("DAWR1 supported only on POWER10 and later CPUs");
> + }
Should this be an error?
Should the dawr1 cap be enabled by default for POWER10 machines?
> +
> + 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");
> + } 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");
> + }
> + }
> +}
> +
> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> [SPAPR_CAP_HTM] = {
> .name = "htm",
> @@ -781,6 +807,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .type = "bool",
> .apply = cap_ail_mode_3_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,
> @@ -923,6 +958,7 @@ 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(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
> +SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
>
> void spapr_caps_init(SpaprMachineState *spapr)
> {
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index fcefd1d1c7..34c1c77c95 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -814,11 +814,12 @@ static target_ulong h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
> return H_SUCCESS;
> }
>
> -static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
> - SpaprMachineState *spapr,
> - target_ulong mflags,
> - target_ulong value1,
> - target_ulong value2)
> +static target_ulong h_set_mode_resource_set_dawr(PowerPCCPU *cpu,
> + SpaprMachineState *spapr,
> + target_ulong mflags,
> + target_ulong resource,
> + target_ulong value1,
> + target_ulong value2)
Did the text alignment go wrong here?
Aside from those things,
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks,
Nick
On Tue, Feb 27, 2024 at 10:21:23PM +1000, Nicholas Piggin wrote:
> On Fri Feb 2, 2024 at 12:46 AM AEST, Shivaprasad G Bhat 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.
> >
> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> > ---
> > hw/ppc/spapr.c | 7 ++++++-
> > hw/ppc/spapr_caps.c | 36 ++++++++++++++++++++++++++++++++++++
> > hw/ppc/spapr_hcall.c | 25 ++++++++++++++++---------
> > include/hw/ppc/spapr.h | 6 +++++-
> > target/ppc/kvm.c | 12 ++++++++++++
> > target/ppc/kvm_ppc.h | 12 ++++++++++++
> > 6 files changed, 87 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e8dabc8614..91a97d72e7 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -262,7 +262,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;
> > @@ -303,6 +303,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)));
> > }
> > @@ -2138,6 +2141,7 @@ static const VMStateDescription vmstate_spapr = {
> > &vmstate_spapr_cap_fwnmi,
> > &vmstate_spapr_fwnmi,
> > &vmstate_spapr_cap_rpt_invalidate,
> > + &vmstate_spapr_cap_dawr1,
> > NULL
> > }
> > };
> > @@ -4717,6 +4721,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > 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_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> > + smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
> >
> > /*
> > * This cap specifies whether the AIL 3 mode for
> > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > index e889244e52..677f17cea6 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -655,6 +655,32 @@ static void cap_ail_mode_3_apply(SpaprMachineState *spapr,
> > }
> > }
> >
> > +static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
> > + Error **errp)
> > +{
> > + ERRP_GUARD();
> > +
> > + if (!val) {
> > + return; /* Disable by default */
> > + }
> > +
> > + if (!ppc_type_check_compat(MACHINE(spapr)->cpu_type,
> > + CPU_POWERPC_LOGICAL_3_10, 0,
> > + spapr->max_compat_pvr)) {
> > + warn_report("DAWR1 supported only on POWER10 and later CPUs");
> > + }
>
> Should this be an error?
Yes, it should. If you can't supply the cap requested, you *must*
fail to start. Near enough is not good enough when it comes to the
guest visible properties of the virtual machine, or you'll end up with
no end of migration headaches.
> Should the dawr1 cap be enabled by default for POWER10 machines?
>
> > +
> > + 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");
> > + } 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");
> > + }
> > + }
> > +}
> > +
> > SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> > [SPAPR_CAP_HTM] = {
> > .name = "htm",
> > @@ -781,6 +807,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> > .type = "bool",
> > .apply = cap_ail_mode_3_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,
> > @@ -923,6 +958,7 @@ 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(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
> > +SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
> >
> > void spapr_caps_init(SpaprMachineState *spapr)
> > {
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index fcefd1d1c7..34c1c77c95 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -814,11 +814,12 @@ static target_ulong h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
> > return H_SUCCESS;
> > }
> >
> > -static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
> > - SpaprMachineState *spapr,
> > - target_ulong mflags,
> > - target_ulong value1,
> > - target_ulong value2)
> > +static target_ulong h_set_mode_resource_set_dawr(PowerPCCPU *cpu,
> > + SpaprMachineState *spapr,
> > + target_ulong mflags,
> > + target_ulong resource,
> > + target_ulong value1,
> > + target_ulong value2)
>
> Did the text alignment go wrong here?
>
> Aside from those things,
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Thanks,
> Nick
>
--
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
Hi David, Nick,
Sorry about not getting back on this for long!
On 2/28/24 2:22 AM, David Gibson wrote:
> On Tue, Feb 27, 2024 at 10:21:23PM +1000, Nicholas Piggin wrote:
>> On Fri Feb 2, 2024 at 12:46 AM AEST, Shivaprasad G Bhat 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.
>>>
>>> Signed-off-by: Ravi Bangoria<ravi.bangoria@linux.ibm.com>
>>> Signed-off-by: Shivaprasad G Bhat<sbhat@linux.ibm.com>
>>> ---
>>> hw/ppc/spapr.c | 7 ++++++-
>>> hw/ppc/spapr_caps.c | 36 ++++++++++++++++++++++++++++++++++++
>>> hw/ppc/spapr_hcall.c | 25 ++++++++++++++++---------
>>> include/hw/ppc/spapr.h | 6 +++++-
>>> target/ppc/kvm.c | 12 ++++++++++++
>>> target/ppc/kvm_ppc.h | 12 ++++++++++++
>>> 6 files changed, 87 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index e8dabc8614..91a97d72e7 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -262,7 +262,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;
>>> @@ -303,6 +303,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)));
>>> }
>>> @@ -2138,6 +2141,7 @@ static const VMStateDescription vmstate_spapr = {
>>> &vmstate_spapr_cap_fwnmi,
>>> &vmstate_spapr_fwnmi,
>>> &vmstate_spapr_cap_rpt_invalidate,
>>> + &vmstate_spapr_cap_dawr1,
>>> NULL
>>> }
>>> };
>>> @@ -4717,6 +4721,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>> 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_RPT_INVALIDATE] = SPAPR_CAP_OFF;
>>> + smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
>>>
>>> /*
>>> * This cap specifies whether the AIL 3 mode for
>>> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
>>> index e889244e52..677f17cea6 100644
>>> --- a/hw/ppc/spapr_caps.c
>>> +++ b/hw/ppc/spapr_caps.c
>>> @@ -655,6 +655,32 @@ static void cap_ail_mode_3_apply(SpaprMachineState *spapr,
>>> }
>>> }
>>>
>>> +static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
>>> + Error **errp)
>>> +{
>>> + ERRP_GUARD();
>>> +
>>> + if (!val) {
>>> + return; /* Disable by default */
>>> + }
>>> +
>>> + if (!ppc_type_check_compat(MACHINE(spapr)->cpu_type,
>>> + CPU_POWERPC_LOGICAL_3_10, 0,
>>> + spapr->max_compat_pvr)) {
>>> + warn_report("DAWR1 supported only on POWER10 and later CPUs");
>>> + }
>> Should this be an error?
Changing this to error_report().
> Yes, it should. If you can't supply the cap requested, you *must*
> fail to start. Near enough is not good enough when it comes to the
> guest visible properties of the virtual machine, or you'll end up with
> no end of migration headaches.
This was made to warn_report() as suggested[1] by Greg to avoid "make test"
failures once we enable dawr1 caps ON by default with POWER9 being the
default
cpu then.
However, now that we have moved to P10 as the default CPU, I tested with
default
ON on P10, and OFF otherwise, "make test" passes. The problem was only
applicable
before as the default cpu was P9 back then.
>> Should the dawr1 cap be enabled by default for POWER10 machines?
Yes. Made it default enabled on Power10 cpu, and disable for Power9 and
below.
>>> +
>>> + 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");
>>> + } 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");
>>> + }
>>> + }
>>> +}
>>> +
<snip>
>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>> index fcefd1d1c7..34c1c77c95 100644
>>> --- a/hw/ppc/spapr_hcall.c
>>> +++ b/hw/ppc/spapr_hcall.c
>>> @@ -814,11 +814,12 @@ static target_ulong h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
>>> return H_SUCCESS;
>>> }
>>>
>>> -static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
>>> - SpaprMachineState *spapr,
>>> - target_ulong mflags,
>>> - target_ulong value1,
>>> - target_ulong value2)
>>> +static target_ulong h_set_mode_resource_set_dawr(PowerPCCPU *cpu,
>>> + SpaprMachineState *spapr,
>>> + target_ulong mflags,
>>> + target_ulong resource,
>>> + target_ulong value1,
>>> + target_ulong value2)
>> Did the text alignment go wrong here?
Yes. Fixed that.
>> Aside from those things,
>>
>> Reviewed-by: Nicholas Piggin<npiggin@gmail.com>
Since its been a while, I am not sure if the Reviewed-by is valid anymore.
I am keeping it anyway, please let me know if you have any further
comments in the v9 posted here [2]
I have addressed the comments from Harsh as well in the v9.
References:
[1] - https://lore.kernel.org/qemu-devel/20230708082536.73a2bfd8@bahia/
[2] -
https://lore.kernel.org/all/173708679976.1678.10844458987521427074.stgit@linux.ibm.com/
Thanks,
Shiva
© 2016 - 2026 Red Hat, Inc.