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