> On 16 Oct 2026, at 5:57 PM, Eric Auger <[email protected]> wrote:
>
> !-------------------------------------------------------------------|
> CAUTION: External Email
>
> |-------------------------------------------------------------------!
>
> Hi Khushit
> On 9/21/26 2:32 PM, Khushit Shah wrote:
>> Named CPU models zero the ID registers QEMU knows about and then apply
>> SYSREG_ properties. However, the host and KVM might expose ID
>> registers that QEMU does not know about yet. In this scenario, named
> this may also happen with reserved fields becoming named, no?
Correct, Thanks, I will add a check that all the RESx fields.
>> CPU models might
> \n
>> leak host features to the guest.
>>
>> Part of the problem can be addressed because the Arm Architecture
>> Reference Manual specifies that unused encodings in the space (op1==0,
>> CRn==0, CRm=={2-7}) may be treated as RES0. This range includes all
>> ID_AA64* registers.
> actually the KVM writable API covers AArch64 System register space
>
> with op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
> so do you mean you only check part of the space. In the positive don't we
> need to be more precise and check for a given opcode if it is known
Sorry, I do not understand. This is exactly what this commit does.
>
>>
>> Error out if KVM returns a non-zero value for a register in the above
>> space which QEMU does not know about yet.
>>
>> get_sysreg_idx() now returns -1 for encodings that are not known ID
>> registers, and kvm_arm_read_sys_reg64() exposes a KVM GET_ONE_REG
>> read for this probe.
>>
>> Signed-off-by: Khushit Shah <[email protected]>
>> ---
>> target/arm/cpu-models.c | 74 ++++++++++++++++++++++++++++++++++++++++
>> target/arm/cpu-sysregs.h | 2 +-
>> target/arm/cpu64.c | 3 +-
>> target/arm/kvm-stub.c | 7 ++++
>> target/arm/kvm.c | 7 ++++
>> target/arm/kvm_arm.h | 12 +++++++
>> 6 files changed, 103 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/cpu-models.c b/target/arm/cpu-models.c
>> index 4e92909755..855eb5ee6b 100644
>> --- a/target/arm/cpu-models.c
>> +++ b/target/arm/cpu-models.c
>> @@ -362,6 +362,78 @@ static void reset_res1_fields(ARMCPU *cpu)
>> }
>> }
>>
>> +/*
>> + * From ARM ARM:
>> + *
>> + * For unused encodings in the part of the space that has op1==0 and
>> + * CRm=={2-7}, direct reads that are not trapped behave as Reserved,
>> + * RES0 to ensure correct behavior if the encodings are used for ID
>> + * registers in the future.
>> + *
>> + * It includes every ID_AA64* register.
>> + *
>> + * For named CPU models, we can check if any of the unknown ID registers
>> + * that fall into this space is non-zero on the host. If so, the named CPU
>> + * model leaks those host features. Error out in such case.
>> + *
>> + * We cannot do the same for the remaining Feature ID space (op1==0 and
>> + * CRm==0, or op1=={1,3}). Unused encodings there are UNDEFINED. The
>> + * implemented registers in that space (MIDR_EL1, CTR_EL0, DCZID_EL0,
>> + * CCSIDR_EL1, ...) are identification or cache geometry.
>> + */
>> +static void arm_named_cpu_zero_unknown_feature_ids(ARMCPU *cpu, Error
>> **errp)
>> +{
>> + uint32_t init_features0 = 0;
>> + static bool checked;
>> + uint32_t target;
>> + int fdarray[3];
>> + int crm, op2;
>> + int fd;
>> +
>> + if (checked) {
>> + return;
>> + }
>> +
>> + if (!kvm_arm_create_scratch_max_host_vcpu(fdarray, &init_features0,
>> + &target, errp)) {
> can't we do that when we retrieve the host features on the 1st created
> scratch vcpu. We shall avoid creating too many of them if possible.
I will try to check if there is an elegant way to fold this into host probe.
As we don’t want to stop -cpu host guest in this case.
Thanks,
Khushit
>> + return;
>> + }
>> +
>> + fd = fdarray[2];
>> + for (crm = 2; crm <= 7; crm++) {
>> + for (op2 = 0; op2 <= 7; op2++) {
>> + uint64_t val = 0;
>> + int ret;
>> +
>> + if (get_sysreg_idx(ENCODE_ID_REG(3, 0, 0, crm, op2)) != -1) {
>> + continue;
>> + }
>> +
>> + ret = kvm_arm_read_sys_reg64(fd, &val, 3, 0, 0, crm, op2);
>> + if (ret) {
>> + continue;
>> + }
>> +
>> + /*
>> + * TODO: if this encoding is writable, try to write 0?
>> + */
>> + if (val) {
>> + error_setg(errp,
>> + "unknown Feature ID register "
>> + "(op0=3, op1=0, CRn=0, CRm=%d, op2=%d) "
>> + "reads as 0x%" PRIx64 " on the host; named CPU "
>> + "models cannot hide it",
>> + crm, op2, val);
>> + kvm_arm_destroy_scratch_host_vcpu(fdarray);
>> + return;
>> + }
>> + }
>> + }
>> +
>> + kvm_arm_destroy_scratch_host_vcpu(fdarray);
>> + checked = true;
>> +}
>> +
>> static void arm_named_cpu_initfn(Object *obj)
>> {
>> ARMCPU *cpu = ARM_CPU(obj);
>> @@ -401,6 +473,8 @@ static void arm_named_cpu_initfn(Object *obj)
>>
>> /* 3. Realize the model chain. */
>> arm_realize_model_chain(obj, model, &error_abort);
>> +
>> + arm_named_cpu_zero_unknown_feature_ids(cpu, &error_abort);
>> }
>>
>> void arm_register_named_cpu_models(void)
>> diff --git a/target/arm/cpu-sysregs.h b/target/arm/cpu-sysregs.h
>> index 7877a3b06a..d48b44705f 100644
>> --- a/target/arm/cpu-sysregs.h
>> +++ b/target/arm/cpu-sysregs.h
>> @@ -37,6 +37,6 @@ typedef enum ARMSysRegs {
>>
>> extern const uint32_t id_register_sysreg[NUM_ID_IDX];
>>
>> -int get_sysreg_idx(ARMSysRegs sysreg);
>> +int get_sysreg_idx(ARMSysRegs sysreg); /* -1 if @sysreg is not a known ID
>> register */
>>
>> #endif /* ARM_CPU_SYSREGS_H */
>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>> index a0114b0fa2..3cc698d65a 100644
>> --- a/target/arm/cpu64.c
>> +++ b/target/arm/cpu64.c
>> @@ -57,8 +57,9 @@ int get_sysreg_idx(ARMSysRegs sysreg)
>> {
>> switch (sysreg) {
>> #include "cpu-sysregs.h.inc"
>> + default:
>> + return -1;
>> }
>> - g_assert_not_reached();
>> }
>>
>> #undef DEF
>> diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c
>> index eae2e1ec2c..d263adc921 100644
>> --- a/target/arm/kvm-stub.c
>> +++ b/target/arm/kvm-stub.c
>> @@ -50,6 +50,13 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>> g_assert_not_reached();
>> }
>>
>> +int kvm_arm_read_sys_reg64(int fd, uint64_t *val,
>> + uint8_t op0, uint8_t op1,
>> + uint8_t crn, uint8_t crm, uint8_t op2)
>> +{
>> + g_assert_not_reached();
>> +}
>> +
>> void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
>> {
>> g_assert_not_reached();
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index 70d9a07580..6f704c0509 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -263,6 +263,13 @@ static int read_sys_reg64(int fd, uint64_t *pret,
>> uint64_t id)
>> return ioctl(fd, KVM_GET_ONE_REG, &idreg);
>> }
>>
>> +int kvm_arm_read_sys_reg64(int fd, uint64_t *val,
>> + uint8_t op0, uint8_t op1,
>> + uint8_t crn, uint8_t crm, uint8_t op2)
>> +{
>> + return read_sys_reg64(fd, val, ARM64_SYS_REG(op0, op1, crn, crm, op2));
>> +}
>> +
>> static uint64_t idregs_sysreg_to_kvm_reg(ARMSysRegs sysreg)
>> {
>> return ARM64_SYS_REG((sysreg & CP_REG_ARM64_SYSREG_OP0_MASK) >>
>> CP_REG_ARM64_SYSREG_OP0_SHIFT,
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index 997120966f..4e563fab96 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -154,6 +154,18 @@ bool kvm_arm_create_scratch_max_host_vcpu(int *fdarray,
>> */
>> void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu);
>>
>> +/**
>> + * kvm_arm_read_sys_reg64:
>> + * @fd: vCPU fd
>> + * @val: filled with the register value on success
>> + * @op0/@op1/@crn/@crm/@op2: System ID register encoding
>> + *
>> + * Returns: 0 on success, -1 on failure.
>> + */
>> +int kvm_arm_read_sys_reg64(int fd, uint64_t *val,
>> + uint8_t op0, uint8_t op1,
>> + uint8_t crn, uint8_t crm, uint8_t op2);
>> +
>> /**
>> * kvm_arm_add_vcpu_properties:
>> * @cpu: The CPU object to add the properties to
> Thanks
>
> Eric
