Re: [PATCH v3 9/9] RISC-V: KVM: Upgrade the supported SBI version to 3.0

[Atish Patra <atish.patra@linux.dev>]

On 5/28/25 12:21 PM, Atish Patra wrote:
> <Removing Palmer's rivos email address to avoid bouncing>
> 
> On 5/28/25 8:09 AM, Andrew Jones wrote:
>> On Wed, May 28, 2025 at 07:16:11AM -0700, Atish Patra wrote:
>>> On 5/26/25 4:13 AM, Andrew Jones wrote:
>>>> On Mon, May 26, 2025 at 11:00:30AM +0200, Radim Krčmář wrote:
>>>>> 2025-05-23T10:16:11-07:00, Atish Patra <atish.patra@linux.dev>:
>>>>>> On 5/23/25 6:31 AM, Radim Krčmář wrote:
>>>>>>> 2025-05-22T12:03:43-07:00, Atish Patra <atishp@rivosinc.com>:
>>>>>>>> Upgrade the SBI version to v3.0 so that corresponding features
>>>>>>>> can be enabled in the guest.
>>>>>>>>
>>>>>>>> Signed-off-by: Atish Patra <atishp@rivosinc.com>
>>>>>>>> ---
>>>>>>>> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/ 
>>>>>>>> include/asm/kvm_vcpu_sbi.h
>>>>>>>> -#define KVM_SBI_VERSION_MAJOR 2
>>>>>>>> +#define KVM_SBI_VERSION_MAJOR 3
>>>>>>> I think it's time to add versioning to KVM SBI implementation.
>>>>>>> Userspace should be able to select the desired SBI version and 
>>>>>>> KVM would
>>>>>>> tell the guest that newer features are not supported.
>>>> We need new code for this, but it's a good idea.
>>>>
>>>>>> We can achieve that through onereg interface by disabling 
>>>>>> individual SBI
>>>>>> extensions.
>>>>>> We can extend the existing onereg interface to disable a specific SBI
>>>>>> version directly
>>>>>> instead of individual ones to save those IOCTL as well.
>>>>> Yes, I am all in favor of letting userspace provide all values in the
>>>>> BASE extension.
>>> We already support vendorid/archid/impid through one reg. I think we 
>>> just
>>> need to add the SBI version support to that so that user space can 
>>> set it.
>>>
>>>> This is covered by your recent patch that provides userspace_sbi.
>>> Why do we need to invent new IOCTL for this ? Once the user space 
>>> sets the
>>> SBI version, KVM can enforce it.
>> If an SBI spec version provides an extension that can be emulated by
>> userspace, then userspace could choose to advertise that spec version,
>> implement a BASE probe function that advertises the extension, and
>> implement the extension, even if the KVM version running is older
>> and unaware of it. But, in order to do that, we need KVM to exit to
>> userspace for all unknown SBI calls and to allow BASE to be overridden
> You mean only the version field in BASE - Correct ?
> 
> We already support vendorid/archid/impid through one reg. I don't see the
> point of overriding SBI implementation ID & version.
> 
>> by userspace. The new KVM CAP ioctl allows opting into that new behavior.
> 
> But why we need a new IOCTL for that ? We can achieve that with existing
> one reg interface with improvements.
> 
>> The old KVM with new VMM configuration isn't totally far-fetched. While
>> host kernels tend to get updated regularly to include security fixes,
>> enterprise kernels tend to stop adding features at some point in order
>> to maximize stability. While enterprise VMMs would also eventually stop
>> adding features, enterprise consumers are always free to use their own
>> VMMs (at their own risk). So, there's a real chance we could have
> 
> I think we are years away from that happening (if it happens). My 
> suggestion was not to
> try to build a world where no body lives ;). When we get to that 

We also support KVM as a kernel module. So it is relatively easier to 
update the RISC-V KVM module for enterprise consumers.

> scenario, the default KVM
> shipped will have many extension implemented. So there won't be much 
> advantage to
> reimplement them in the user space. We can also take an informed 
> decision at that time
> if the current selective forwarding approach is better or we need to 
> blindly forward any
> unknown SBI calls to the user space.
> 
>> deployments with older, stable KVM where users want to enable later SBI
>> extensions, and, in some cases, that should be possible by just updating
>> the VMM -- but only if KVM is only acting as an SBI implementation
>> accelerator and not as a userspace SBI implementation gatekeeper.
> 
> But some of the SBI extensions are so fundamental that it must be 
> implemented in KVM
> for various reasons pointed by Anup on other thread.
> 
>> Thanks,
>> drew
>>
>>>> With that, userspace can disable all extensions that aren't
>>>> supported by a given spec version, disable BASE and then provide
>>>> a BASE that advertises the version it wants. The new code is needed
>>>> for extensions that userspace still wants KVM to accelerate, but then
>>>> KVM needs to be informed it should deny all functions not included in
>>>> the selected spec version.
>>>>
>>>> Thanks,
>>>> drew
>>>>
>>>> _______________________________________________
>>>> linux-riscv mailing list
>>>> linux-riscv@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv