Re: [PATCH v6 06/43] arm64: RME: Check for RME support at KVM init

[Steven Price <steven.price@arm.com>]

On 29/01/2025 03:57, Gavin Shan wrote:
> On 12/13/24 1:55 AM, Steven Price wrote:
>> Query the RMI version number and check if it is a compatible version. A
>> static key is also provided to signal that a supported RMM is available.
>>
>> Functions are provided to query if a VM or VCPU is a realm (or rec)
>> which currently will always return false.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Changes since v5:
>>   * Reword "unsupported" message from "host supports" to "we want" to
>>     clarify that 'we' are the 'host'.
>> Changes since v2:
>>   * Drop return value from kvm_init_rme(), it was always 0.
>>   * Rely on the RMM return value to identify whether the RSI ABI is
>>     compatible.
>> ---
>>   arch/arm64/include/asm/kvm_emulate.h | 18 +++++++++
>>   arch/arm64/include/asm/kvm_host.h    |  4 ++
>>   arch/arm64/include/asm/kvm_rme.h     | 56 ++++++++++++++++++++++++++++
>>   arch/arm64/include/asm/virt.h        |  1 +
>>   arch/arm64/kvm/Makefile              |  3 +-
>>   arch/arm64/kvm/arm.c                 |  6 +++
>>   arch/arm64/kvm/rme.c                 | 50 +++++++++++++++++++++++++
>>   7 files changed, 137 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm64/include/asm/kvm_rme.h
>>   create mode 100644 arch/arm64/kvm/rme.c
>>
> 
> [...]
> 
>> +
>> +static inline bool kvm_is_realm(struct kvm *kvm)
>> +{
>> +    if (static_branch_unlikely(&kvm_rme_is_available) && kvm)
>> +        return kvm->arch.is_realm;
>> +    return false;
>> +}
>> +
> 
> kvm->arch.is_realm won't be true when kvm_rme_is_available is false.
> It's set
> in kvm_arch_init_vm() of PATCH[10]. So it's safe to be simplified as
> below if
> the changes to kvm_arch_init_vm() is slightly modified, more details can be
> found from the comments to PATCH[10]
> 
> With the changes, we don't have to check kvm_rme_is_available every time.

This is true, however, kvm_rme_is_available is a static key, which means
that the kernel uses runtime patching to change the value. So by
checking kvm_rme_is_available this check is short-circuited and there's
no (data) memory access.

I have to admit I've no idea whether this actually makes any practical
difference to any benchmark but the intention was to have the minimum
possible overhead on systems which don't have the RME hardware.

Steve

> static inline bool kvm_is_realm(struct kvm *kvm)
> {
>     return (kvm && kvm->arch.is_realm);
> }
> 
> Thanks,
> Gavin
>