Re: [PATCH v5 13/26] KVM: arm64/sve: System register context switch and access support

[Julien Grall <julien.grall@arm.com>]

Hi Dave,

On 18/02/2019 19:52, Dave Martin wrote:
> @@ -1091,6 +1088,95 @@ static int reg_from_user(u64 *val, const void __user *uaddr, u64 id);
>   static int reg_to_user(void __user *uaddr, const u64 *val, u64 id);
>   static u64 sys_reg_to_index(const struct sys_reg_desc *reg);
>   
> +static unsigned int sve_restrictions(const struct kvm_vcpu *vcpu,
> +				     const struct sys_reg_desc *rd)
> +{
> +	return vcpu_has_sve(vcpu) ? 0 : REG_NO_USER | REG_NO_GUEST;
> +}
> +
> +static unsigned int sve_id_restrictions(const struct kvm_vcpu *vcpu,
> +					const struct sys_reg_desc *rd)
> +{
> +	return vcpu_has_sve(vcpu) ? 0 : REG_NO_USER;
> +}
> +
> +static int get_zcr_el1(struct kvm_vcpu *vcpu,
> +		       const struct sys_reg_desc *rd,
> +		       const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> +	if (WARN_ON(!vcpu_has_sve(vcpu)))
> +		return -ENOENT;
> +
> +	return reg_to_user(uaddr, &vcpu->arch.ctxt.sys_regs[ZCR_EL1],
> +			   reg->id);
> +}
> +
> +static int set_zcr_el1(struct kvm_vcpu *vcpu,
> +		       const struct sys_reg_desc *rd,
> +		       const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> +	if (WARN_ON(!vcpu_has_sve(vcpu)))
> +		return -ENOENT;
> +
> +	return reg_from_user(&vcpu->arch.ctxt.sys_regs[ZCR_EL1], uaddr,
> +			     reg->id);
> +}
> +
> +/* Generate the emulated ID_AA64ZFR0_EL1 value exposed to the guest */
> +static u64 guest_id_aa64zfr0_el1(const struct kvm_vcpu *vcpu)
> +{
> +	if (!vcpu_has_sve(vcpu))
> +		return 0;
> +
> +	return read_sanitised_ftr_reg(SYS_ID_AA64ZFR0_EL1);
> +}
> +
> +static bool access_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> +				   struct sys_reg_params *p,
> +				   const struct sys_reg_desc *rd)
> +{
> +	if (p->is_write)
> +		return write_to_read_only(vcpu, p, rd);
> +
> +	p->regval = guest_id_aa64zfr0_el1(vcpu);
> +	return true;
> +}
> +
> +static int get_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> +		const struct sys_reg_desc *rd,
> +		const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> +	u64 val;
> +
> +	if (!vcpu_has_sve(vcpu))
> +		return -ENOENT;
> +
> +	val = guest_id_aa64zfr0_el1(vcpu);
> +	return reg_to_user(uaddr, &val, reg->id);
> +}
> +
> +static int set_id_aa64zfr0_el1(struct kvm_vcpu *vcpu,
> +		const struct sys_reg_desc *rd,
> +		const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> +	const u64 id = sys_reg_to_index(rd);
> +	int err;
> +	u64 val;
> +
> +	if (!vcpu_has_sve(vcpu))
> +		return -ENOENT;
> +
> +	err = reg_from_user(&val, uaddr, id);
> +	if (err)
> +		return err;
> +
> +	/* This is what we mean by invariant: you can't change it. */
> +	if (val != guest_id_aa64zfr0_el1(vcpu))
> +		return -EINVAL;
> +
> +	return 0;
> +}

We seem to already have code for handling invariant registers as well as reading 
ID register. I guess the only reason you can't use them is because of the check 
the vcpu is using SVE.

However, AFAICT the restrictions callback would prevent you to enter the {get, 
set}_id if the vCPU does not support SVE. So the check should not be reachable.

Did I miss anything?

Cheers,

-- 
Julien Grall