Re: [PATCH 19/26] KVM: arm64: Make struct kvm_regs userspace-only

Marc Zyngier
2020-05-27T10:23:04Z

On 2020-05-26 17:29, James Morse wrote:
> Hi Marc,
>
> On 22/04/2020 13:00, Marc Zyngier wrote:
>> struct kvm_regs is used by userspace to indicate which register gets
>> accessed by the {GET,SET}_ONE_REG API. But as we're about to refactor
>> the layout of the in-kernel register structures, we need the kernel to
>> move away from it.
>>
>> Let's make kvm_regs userspace only, and let the kernel map it to its
>> own
>> internal representation.
>
>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>> index 23ebe51410f06..9fec9231b63e2 100644
>> --- a/arch/arm64/kvm/guest.c
>> +++ b/arch/arm64/kvm/guest.c
>> @@ -102,6 +102,55 @@ static int core_reg_size_from_offset(const struct
>> kvm_vcpu *vcpu, u64 off)
>> return size;
>> }
>>
>> +static void *core_reg_addr(struct kvm_vcpu *vcpu, const struct
>> kvm_one_reg *reg)
>> +{
>> + u64 off = core_reg_offset_from_id(reg->id);
>> +
>> + switch (off) {
>
>> + default:
>> + return NULL;
>
> Doesn't this switch statement catch an out of range offset, and a
> misaligned offset?
>
> ... We still test for those explicitly in the caller. Better safe than
> implicit?

Indeed, this is not supposed to happen at all. Maybe I should just fold
validate_core_offset offset there, and make this NULL value the error
case.

>
>> + }
>> +}
>
> With the reset thing reported by Zenghui and Zengtao on the previous
> patch fixed:
> Reviewed-by: James Morse <james.morse@arm.com>
>
> (otherwise struct kvm_regs isn't userspace-only!)

Indeed!

Thanks,

M.
--
Jazz is not dead. It just smells funny...