Hi Khushit,
On 9/30/26 9:34 AM, Khushit Shah wrote:
>
>> On 16 Sep 2026, at 8:15 PM, Eric Auger <[email protected]> wrote:
>>
>> !-------------------------------------------------------------------|
>> CAUTION: External Email
>>
>> |-------------------------------------------------------------------!
>>
>> Since legacy composite options (such as SVE, virtualization, ...)
>> and new SYSREG props coexist, add some basic consistency checks.
>>
>> Those checks are performed both in kvm_arch_init_vcpu() before
>> applying the SYSREG props at the end of the initialization chain.
>>
>> Some ID reg fields corresponding to legacy composite option scope
>> are not writable. In that case we just check that the field has
>> not become writable.
>>
>> Signed-off-by: Eric Auger <[email protected]>
> Hi Eric,
>
> Gave this another look. I suggest moving this to
> kvm_arm_expose_idreg_properties().
> What I suggest specifically is:
> - Have a list of ID register fields which conflicts with legacy props.
this depends on the definition of "conflict". For instance does
PMUVer conflicts with has_pmu?
Nevertheless your suggested approach could work for props below which in case
they show up would trigger
error_setg(errp, "%s is now exposed but qemu is not ready to support
it",
propname);
Thanks
Eric
> - in kvm_arm_expose_idreg_properties() (the new field based approach) never
> expose fields
> which are part of the above list.
>
> This way the same consistency logic you have will carry over to named models
> by just adding different
> setters for the above field.
>
> What are your thoughts on this?
>
> Thanks,
> Khushit
>> —
>> target/arm/kvm.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 105 insertions(+)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index ddf320d0e6..f52c03745e 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -340,6 +340,107 @@ static ARM64SysRegField *get_field(int i, ARM64SysReg
>> *reg)
>> return NULL;
>> }
>>
>> +
>> +/**
>> + * kvm_arm_vcpu_validate_sysreg:
>> + *
>> + * Validate a SYSREG field value is consistent with legacy composite options
>> + * Some checks are not implemented because the corresponding sysreg field
>> + * is not currently writable. In that case we make sure the field has not
>> + * become writable, which would mean the user had the capability to set the
>> + * corresponding property.
>> + *
>> + * return true if consistent. false if it is not, along with an error handle
>> + */
>> +static bool kvm_arm_vcpu_validate_sysreg(ARMCPU *cpu, ARM64SysRegField
>> *field,
>> + uint64_t value, Error **errp)
>> +{
>> + const char *fieldname = field->name;
>> + ARM64SysReg *sysregdesc = &arm64_id_regs[field->index];
>> + const char *regname = sysregdesc->name;
>> + g_autofree char *propname = g_strdup_printf("SYSREG_%s_%s", regname,
>> fieldname);
>> +
>> + /* virtualization */
>> + if (!strcmp(propname, "SYSREG_ID_AA64PFR0_EL1_EL2")) {
>> + /* consistency with machine virtualization property */
>> + if (cpu->has_el2 && value == 0) {
>> + error_setg(errp,
>> + "Inconsistent -machine virtualization=on and "
>> + "%s=0", propname);
>> + return false;
>> + } else if (!cpu->has_el2 && value > 0) {
>> + error_setg(errp,
>> + "Inconsistent -machine virtualization=off and "
>> + "%s > 0", propname);
>> + return false;
>> + }
>> + /* secure */
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64PFR0_EL1_EL3") && value >
>> 0) {
>> + /* consistency with machine secure property */
>> + error_setg(errp, "%s is set but qemu is not ready to support it",
>> + propname);
>> + return false;
>> + /* MTE */
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64PFR1_EL1_MTE")) {
>> + error_setg(errp, "%s is now exposed but qemu is not ready to
>> support it",
>> + propname);
>> + return false;
>> + /* SVE */
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64PFR0_EL1_SVE")) {
>> + error_setg(errp, "%s is now exposed but qemu is not ready to
>> support it",
>> + propname);
>> + return false;
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64ZFR0_EL1_SVEver")) {
>> + if (cpu_isar_feature(aa64_sve, cpu) && value == 0) {
>> + error_setg(errp, "sve is set but %s is set to 0", propname);
>> + return false;
>> + } else if (!cpu_isar_feature(aa64_sve, cpu) && value > 0) {
>> + error_setg(errp, "sve is not set but %s is greater than 0",
>> + propname);
>> + return false;
>> + }
>> + /* PAUTH */
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64ISAR1_EL1_APA")) {
>> + /* PAUTH QARMA5 address authentification */
>> + error_setg(errp, "%s is now exposed but qemu is not ready to
>> support it",
>> + propname);
>> + return false;
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64ISAR1_EL1_API")) {
>> + /* PAUTH Impl Defined address authentification */
>> + error_setg(errp, "%s is now exposed but qemu is not ready to
>> support it",
>> + propname);
>> + return false;
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64ISAR1_EL1_GPA")) {
>> + /* QARMA5 generic code authentification */
>> + error_setg(errp, "%s is now exposed but qemu is not ready to
>> support it",
>> + propname);
>> + return false;
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64ISAR1_EL1_GPI")) {
>> + /* QARMA5 generic code authentification */
>> + error_setg(errp, "%s is now exposed but qemu is not ready to
>> support it",
>> + propname);
>> + return false;
>> + /* PMU */
>> + } else if (!strcmp(propname, "SYSREG_ID_AA64DFR0_EL1_PMUVer")) {
>> + if (cpu->has_pmu && value == 0) {
>> + error_setg(errp, "%s is 0 whereas pmu is set", propname);
>> + return false;
>> + } else if (!cpu->has_pmu && value) {
>> + error_setg(errp, "%s is non null whereas pmu is unset",
>> propname);
>> + return false;
>> + }
>> + /* aarch64 false */
>> + } else if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
>> + if ((!strcmp(propname, "SYSREG_ID_AA64PFR0_EL1_EL0") ||
>> + !strcmp(propname, "SYSREG_ID_AA64PFR0_EL1_EL1") ||
>> + !strcmp(propname, "SYSREG_ID_AA64PFR0_EL1_EL2")) && value < 2)
>> + error_setg(errp, "%s is < 2 while aarch64 is set to off",
>> + propname);
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> #define MAKE_IDREG_KEY(reg_idx, field_shift) \
>> (((uint64_t)(reg_idx) << 8) | ((uint64_t)(field_shift) & 0xFF))
>>
>> @@ -2241,6 +2342,10 @@ static int kvm_arm_apply_sysreg_props(ARMCPU *cpu,
>> Error **errp)
>> uint64_t oldfv;
>> int ret;
>>
>> + if (!kvm_arm_vcpu_validate_sysreg(cpu, field, value, errp)) {
>> + return -1;
>> + }
>> +
>> mask = MAKE_64BIT_MASK(lower, length);
>> value = value << lower;
>>
>> --
>> 2.53.0
>>
