linux-kernel - Re: [PATCH v6 10/43] arm64: kvm: Allow passing machine type in KVM creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1356de81-2fa1-4ad5-80bd-d02440603288@arm.com>
Date: Thu, 30 Jan 2025 14:14:43 +0000
From: Steven Price <steven.price@....com>
To: Gavin Shan <gshan@...hat.com>, kvm@...r.kernel.org, kvmarm@...ts.linux.dev
Cc: Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
 Will Deacon <will@...nel.org>, James Morse <james.morse@....com>,
 Oliver Upton <oliver.upton@...ux.dev>,
 Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu
 <yuzenghui@...wei.com>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Joey Gouly <joey.gouly@....com>,
 Alexandru Elisei <alexandru.elisei@....com>,
 Christoffer Dall <christoffer.dall@....com>, Fuad Tabba <tabba@...gle.com>,
 linux-coco@...ts.linux.dev,
 Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>,
 Shanker Donthineni <sdonthineni@...dia.com>, Alper Gun
 <alpergun@...gle.com>, "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>
Subject: Re: [PATCH v6 10/43] arm64: kvm: Allow passing machine type in KVM
 creation

On 29/01/2025 04:07, Gavin Shan wrote:
> On 12/13/24 1:55 AM, Steven Price wrote:
>> Previously machine type was used purely for specifying the physical
>> address size of the guest. Reserve the higher bits to specify an ARM
>> specific machine type and declare a new type 'KVM_VM_TYPE_ARM_REALM'
>> used to create a realm guest.
>>
>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@....com>
>> Signed-off-by: Steven Price <steven.price@....com>
>> ---
>>   arch/arm64/kvm/arm.c     | 17 +++++++++++++++++
>>   arch/arm64/kvm/mmu.c     |  3 ---
>>   include/uapi/linux/kvm.h | 19 +++++++++++++++----
>>   3 files changed, 32 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index c505ec61180a..73016e1e0067 100644
>> --- a/arch/arm64/kvm/arm.c
>> +++ b/arch/arm64/kvm/arm.c
>> @@ -207,6 +207,23 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
>> long type)
>>       mutex_unlock(&kvm->lock);
>>   #endif
>>   +    if (type & ~(KVM_VM_TYPE_ARM_MASK |
>> KVM_VM_TYPE_ARM_IPA_SIZE_MASK))
>> +        return -EINVAL;
>> +
>> +    switch (type & KVM_VM_TYPE_ARM_MASK) {
>> +    case KVM_VM_TYPE_ARM_NORMAL:
>> +        break;
>> +    case KVM_VM_TYPE_ARM_REALM:
>> +        kvm->arch.is_realm = true;
>> +        if (!kvm_is_realm(kvm)) {
>> +            /* Realm support unavailable */
>> +            return -EINVAL;
>> +        }
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>>       kvm_init_nested(kvm);
>>         ret = kvm_share_hyp(kvm, kvm + 1);
> 
> Corresponding to comments for PATCH[6], the block of the code can be
> modified
> to avoid using kvm_is_realm() here. In this way, kvm_is_realm() can be
> simplifed
> as I commented for PATCH[6].
> 
>     case KVM_VM_TYPE_ARM_REALM:
>         if (static_branch_unlikely(&kvm_rme_is_available))
>             return -EPERM;    /* -EPERM may be more suitable than -
> EINVAL */
> 
>         kvm->arch.is_realm = true;
>         break;

Yes that's more readable. I'd used kvm_is_realm() because I wanted to
keep the check on kvm_rme_is_available to one place, but coming back to
the code there's definitely a "Huh?" moment from setting 'is_realm' and
then testing if it's a realm!

I also agree -EPERM is probably better to signify that the kernel
supports realms but the hardware doesn't.

Thanks,

Steve