On 8/13/2020 5:09 PM, Andrew Jones wrote:
> On Thu, Aug 13, 2020 at 02:05:16PM +0800, Peng Liang wrote:
>> It's time to make ID registers configurable. When userspace (but not
>> guest) want to set the values of ID registers, save the value in
>> kvm_arch_vcpu so that guest can read the modified values.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@xxxxxxxxxx>
>> Signed-off-by: Peng Liang <liangpeng10@xxxxxxxxxx>
>> ---
>> arch/arm64/kvm/sys_regs.c | 23 ++++++++++++++++-------
>> 1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 776c2757a01e..f98635489966 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1111,6 +1111,14 @@ static u64 kvm_get_id_reg(struct kvm_vcpu *vcpu, u64 id)
>> return ri->sys_val;
>> }
>>
>> +static void kvm_set_id_reg(struct kvm_vcpu *vcpu, u64 id, u64 value)
>> +{
>> + struct id_reg_info *ri = kvm_id_reg(vcpu, id);
>> +
>> + BUG_ON(!ri);
>> + ri->sys_val = value;
>> +}
>> +
>> /* Read a sanitised cpufeature ID register by sys_reg_desc */
>> static u64 read_id_reg(struct kvm_vcpu *vcpu,
>> struct sys_reg_desc const *r, bool raz)
>> @@ -1252,10 +1260,6 @@ static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
>>
>> /*
>> * cpufeature ID register user accessors
>> - *
>> - * For now, these registers are immutable for userspace, so no values
>> - * are stored, and for set_id_reg() we don't allow the effective value
>> - * to be changed.
>> */
>> static int __get_id_reg(struct kvm_vcpu *vcpu,
>> const struct sys_reg_desc *rd, void __user *uaddr,
>> @@ -1279,9 +1283,14 @@ static int __set_id_reg(struct kvm_vcpu *vcpu,
>> if (err)
>> return err;
>>
>> - /* This is what we mean by invariant: you can't change it. */
>> - if (val != read_id_reg(vcpu, rd, raz))
>> - return -EINVAL;
>> + if (raz) {
>> + if (val != read_id_reg(vcpu, rd, raz))
>> + return -EINVAL;
>> + } else {
>> + u32 reg_id = sys_reg((u32)rd->Op0, (u32)rd->Op1, (u32)rd->CRn,
>> + (u32)rd->CRm, (u32)rd->Op2);
>> + kvm_set_id_reg(vcpu, reg_id, val);
>
> So userspace can set the ID registers to whatever they want? I think each
> register should have its own sanity checks applied before accepting the
> input.
>
> Thanks,
> drew
>
>> + }
>>
>> return 0;
>> }
>> --
>> 2.18.4
>>
>
> .
>
Yea, sanity checkers are necessary and I'm working on it. I think we should make
sure that every ID fields should be checked to match the HW capabilities so that
guest will not be confused because of a careless hypervisor.
Thanks,
Peng
