On Wed, Mar 27, 2019 at 09:47:42AM +0000, Julien Thierry wrote:
> Hi Dave,
>
> On 19/03/2019 17:52, Dave Martin wrote:
> > This patch includes the SVE register IDs in the list returned by
> > KVM_GET_REG_LIST, as appropriate.
> >
> > On a non-SVE-enabled vcpu, no new IDs are added.
> >
> > On an SVE-enabled vcpu, IDs for the FPSIMD V-registers are removed
> > from the list, since userspace is required to access the Z-
> > registers instead in order to access the V-register content. For
> > the variably-sized SVE registers, the appropriate set of slice IDs
> > are enumerated, depending on the maximum vector length for the
> > vcpu.
> >
> > As it currently stands, the SVE architecture never requires more
> > than one slice to exist per register, so this patch adds no
> > explicit support for enumerating multiple slices. The code can be
> > extended straightforwardly to support this in the future, if
> > needed.
> >
> > Signed-off-by: Dave Martin <Dave.Martin@xxxxxxx>
> >
>
> Reviewed-by: Julien Thierry <julien.thierry@xxxxxxx>
Thanks, although...
> > ---
> >
> > Changes since v5:
> >
> > (Dropped Julien Thierry's Reviewed-by due to non-trivial rebasing)
> >
> > * Move mis-split reword to prevent put_user()s being accidentally the
> > correct size from KVM: arm64/sve: Add pseudo-register for the guest's
> > vector lengths.
> > ---
> > arch/arm64/kvm/guest.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 56 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index 736d8cb..585c31e5 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -411,6 +411,56 @@ static int get_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> > return copy_to_user(uaddr, &val, KVM_REG_SIZE(reg->id)) ? -EFAULT : 0;
> > }
> >
> > +static unsigned long num_sve_regs(const struct kvm_vcpu *vcpu)
> > +{
> > + /* Only the first slice ever exists, for now */
> > + const unsigned int slices = 1;
>
> Nit: Might be worth introducing a macro/inline function for the number
> of slices supported. This way, the day we need to change that, we only
> need to look for that identifier.
... Reasonable point, but I wanted to avoid inventing anything
prematurely, partly because sve_reg_to_region() will need work in order
to support multiple slices (though it's not rocket science).
I could introduce something like the following:
static unsigned int sve_num_slices(const struct kvm_vcpu *vcpu)
{
unsigned int slice_size = KVM_REG_SIZE(KVM_REG_ARM64_SVE_ZREG(0, 0));
unsigned int slices = DIV_ROUND_UP(vcpu->arch.sve_max_vl, slice_size);
/*
* For now, the SVE register ioctl access code won't work
* properly with multiple register slices. KVM should prevent
* configuration of a vcpu with a maximum vector length large
* enough to trigger this:
*/
if (WARN_ON_ONCE(slices > 1))
return 1;
return slices;
}
This may be clearer, but felt a bit like overkill...
Thoughts?
Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
