On Tue, May 13 2025, Eric Auger <eric.au...@redhat.com> wrote:
> Hi Connie,
>
> On 4/14/25 6:38 PM, Cornelia Huck wrote:
>> Add an helper to retrieve the writable id reg bitmask. The
>> status of the query is stored in the CPU struct so that an
>> an error, if any, can be reported on vcpu realize().
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Signed-off-by: Cornelia Huck <coh...@redhat.com>
>> ---
>> target/arm/cpu.h | 26 ++++++++++++++++++++++++++
>> target/arm/kvm.c | 32 ++++++++++++++++++++++++++++++++
>> target/arm/kvm_arm.h | 7 +++++++
>> 3 files changed, 65 insertions(+)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index d27134f4a025..bbee7ff2414a 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -856,6 +856,26 @@ typedef struct {
>> uint32_t map, init, supported;
>> } ARMVQMap;
>>
>> +typedef enum ARMIdRegsState {
>> + WRITABLE_ID_REGS_UNKNOWN,
>> + WRITABLE_ID_REGS_NOT_DISCOVERABLE,
>> + WRITABLE_ID_REGS_FAILED,
>> + WRITABLE_ID_REGS_AVAIL,
>> +} ARMIdRegsState;
>> +
>> +/*
>> + * The following structures are for the purpose of mapping the output of
>> + * KVM_ARM_GET_REG_WRITABLE_MASKS that also may cover id registers we do
>> + * not support in QEMU
>> + * ID registers in op0==3, op1=={0,1,3}, crn=0, crm=={0-7}, op2=={0-7},
>> + * as used by the KVM_ARM_GET_REG_WRITABLE_MASKS ioctl call.
>> + */
>> +#define NR_ID_REGS (3 * 8 * 8)
> We may rename this define to better associate to the KVM API. I tend to
> mix it with NUM_ID_IDX now ;-)
> maybe something like KVM_NR_EXPOSED_ID_REGS
The kernel calls it KVM_FEATURE_ID_RANGE_SIZE, but I'd like to avoid
adding 'KVM' in the name, as it is basically a range of registers and
nothing really KVM specific... maybe ID_REG_RANGE_SIZE?
>> +
>> +typedef struct IdRegMap {
>> + uint64_t regs[NR_ID_REGS];
>> +} IdRegMap;
> I would add a comment saying this is the mask array, just to prevent the
> reading from thinking it is the actual reg content.
"More comments" seems to be a theme :) I'll go ahead and add them where
it makes sense.
>> +
>> /* REG is ID_XXX */
>> #define FIELD_DP64_IDREG(ISAR, REG, FIELD, VALUE) \
>> ({ \
>> @@ -1044,6 +1064,12 @@ struct ArchCPU {
>> */
>> bool host_cpu_probe_failed;
>>
>> + /*
>> + * state of writable id regs query used to report an error, if any,
>> + * on KVM custom vcpu model realize
>> + */
>> + ARMIdRegsState writable_id_regs;
> maybe rename into writable_id_reg_status that would better reflect what
> it is.
Indeed.
