Re: [PATCH 1/1] KVM: Add KVM_GET_REG_LIST ioctl for LoongArch

[liushuyu <liushuyu@aosc.io>]

Hi Bibo,

>
>
> On 2026/1/26 上午11:38, liushuyu wrote:
>> Hi Bibo,
>>>
>>>
>>> On 2026/1/25 下午1:43, Zixing Liu wrote:
>>>> This ioctl can be used by userspace applications to determine which
>>>> (special) registers are get/set-able.
>>>>
>>>> This can be very useful for cross-platform VMMs so that they do not
>>>> have
>>>> to hardcode register indices for each supported architectures.
>>>>
>>>> Signed-off-by: Zixing Liu <liushuyu@aosc.io>
>>>> ---
>>>>
>>>> For example, this ioctl could be used by rust-vmm/rust-kvm or maybe
>>>> VirtualBox-kvm in the future.
>>>>
>>>>    Documentation/virt/kvm/api.rst |  2 +-
>>>>    arch/loongarch/kvm/vcpu.c      | 69
>>>> ++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 70 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/virt/kvm/api.rst
>>>> b/Documentation/virt/kvm/api.rst
>>>> index 01a3abef8abb..f46dd8be282f 100644
>>>> --- a/Documentation/virt/kvm/api.rst
>>>> +++ b/Documentation/virt/kvm/api.rst
>>>> @@ -3603,7 +3603,7 @@ VCPU matching underlying host.
>>>>    ---------------------
>>>>      :Capability: basic
>>>> -:Architectures: arm64, mips, riscv, x86 (if KVM_CAP_ONE_REG)
>>>> +:Architectures: arm64, loongarch, mips, riscv, x86 (if
>>>> KVM_CAP_ONE_REG)
>>>>    :Type: vcpu ioctl
>>>>    :Parameters: struct kvm_reg_list (in/out)
>>>>    :Returns: 0 on success; -1 on error
>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
>>>> index 656b954c1134..b884eb9c76aa 100644
>>>> --- a/arch/loongarch/kvm/vcpu.c
>>>> +++ b/arch/loongarch/kvm/vcpu.c
>>>> @@ -1186,6 +1186,57 @@ static int kvm_loongarch_vcpu_set_attr(struct
>>>> kvm_vcpu *vcpu,
>>>>        return ret;
>>>>    }
>>>>    +static unsigned long kvm_loongarch_num_lbt_regs(void)
>>>> +{
>>>> +    /* +1 for the LBT_FTOP flag (inside arch.fpu) */
>>>> +    return sizeof(struct loongarch_lbt) / sizeof(unsigned long) + 1;
>>>> +}
>>>> +
>>>> +static unsigned long kvm_loongarch_num_regs(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +    /* +1 for the KVM_REG_LOONGARCH_COUNTER register */
>>>> +    unsigned long res = CSR_MAX_NUMS + KVM_MAX_CPUCFG_REGS + 1;
>>>> +
>>>> +    if (kvm_guest_has_lbt(&vcpu->arch))
>>>> +        res += kvm_loongarch_num_lbt_regs();
>>>> +
>>>> +    return res;
>>>> +}
>>>> +
>>>> +static int kvm_loongarch_copy_reg_indices(struct kvm_vcpu *vcpu,
>>>> +                      u64 __user *uindices)
>>>> +{
>>>> +    u64 reg;
>>>> +    unsigned int i;
>>>> +
>>>> +    for (i = 0; i < CSR_MAX_NUMS; i++) {
>>>> +        reg = KVM_IOC_CSRID(i);
>>>> +        if (put_user(reg, uindices++))
>>>> +            return -EFAULT;
>>>> +    }
>>> CSR_MAX_NUMS is max number of accessible CSR registers, instead only
>>> part of them is used by vCPU model. By my understanding, there will be
>>> no much meaning if CSR_MAX_NUMS is returned. And I think it will be
>>> better if real CSR register id and number is returned.
>>>
>> Did you mean we should only return the CSR registers initialized in this
>> function
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/loongarch/kvm/main.c?id=63804fed149a6750ffd28610c5c1c98cce6bd377#n48
>>
>> ?
>>
>> That looks like a very large list and the values of the register IDs are
>> not fully continuous. If that is the case, how do we maintain the
>> get/set-able CSR register list?
> It will be better if CSR register list can be categorized, for example
> with LoongArch CPU manual CSR register is split into 8 types from
> chapter 7.4 -- 7.11
>
> At the beginning, there is big array with size CSR_MAX_NUMS without
> any category, it can be fine-gained in late. 

Does that mean in the new `kvm_loongarch_copy_reg_indices` function, it
is also okay to embed this big list in there?

Or do you want to extract the list from the `kvm_init_gcsr_flag`
function and refactor both functions to share one single big list
(probably in the kvm_host.h header)?

Because from what I could understand, your previous messages point to
that we need to use this list to make out what CSR registers to return
to the user space VMMs.

>
> Regards
> Bibo Mao
>>
>>> Regards
>>> Bibo Mao
>>>> +
>>>> +    for (i = 0; i < KVM_MAX_CPUCFG_REGS; i++) {
>>>> +        reg = KVM_IOC_CPUCFG(i);
>>>> +        if (put_user(reg, uindices++))
>>>> +            return -EFAULT;
>>>> +    }
>>>> +
>>>> +    reg = KVM_REG_LOONGARCH_COUNTER;
>>>> +    if (put_user(reg, uindices++))
>>>> +        return -EFAULT;
>>>> +
>>>> +    if (!kvm_guest_has_lbt(&vcpu->arch))
>>>> +        return 0;
>>>> +
>>>> +    for (i = 1; i <= kvm_loongarch_num_lbt_regs(); i++) {
>>>> +        reg = (KVM_REG_LOONGARCH_LBT | KVM_REG_SIZE_U64 | i);
>>>> +        if (put_user(reg, uindices++))
>>>> +            return -EFAULT;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>    long kvm_arch_vcpu_ioctl(struct file *filp,
>>>>                 unsigned int ioctl, unsigned long arg)
>>>>    {
>>>> @@ -1251,6 +1302,24 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>>>>            r = kvm_loongarch_vcpu_set_attr(vcpu, &attr);
>>>>            break;
>>>>        }
>>>> +    case KVM_GET_REG_LIST: {
>>>> +        struct kvm_reg_list __user *user_list = argp;
>>>> +        struct kvm_reg_list reg_list;
>>>> +        unsigned n;
>>>> +
>>>> +        r = -EFAULT;
>>>> +        if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
>>>> +            break;
>>>> +        n = reg_list.n;
>>>> +        reg_list.n = kvm_loongarch_num_regs(vcpu);
>>>> +        if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
>>>> +            break;
>>>> +        r = -E2BIG;
>>>> +        if (n < reg_list.n)
>>>> +            break;
>>>> +        r = kvm_loongarch_copy_reg_indices(vcpu, user_list->reg);
>>>> +        break;
>>>> +    }
>>>>        default:
>>>>            r = -ENOIOCTLCMD;
>>>>            break;
>>>>
>>>
>> Thanks,
>> Zixing
>>
>
Thanks,
Zixing