On Fri, Apr 05, 2019 at 11:45:56AM +0200, Andrew Jones wrote:
> On Fri, Apr 05, 2019 at 10:35:45AM +0100, Dave Martin wrote:
> > On Thu, Apr 04, 2019 at 04:08:32PM +0200, Andrew Jones wrote:
> > > On Fri, Mar 29, 2019 at 01:00:44PM +0000, 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>
> > > > Tested-by: zhang.lei <zhang.lei@xxxxxxxxxxxxxx>
> > > >
> > > > ---
[...]
> > > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
[...]
> > > > +static int copy_sve_reg_indices(const struct kvm_vcpu *vcpu,
> > > > + u64 __user *uindices)
> > > > +{
> > > > + /* Only the first slice ever exists, for now */
[...]
> > > > + const unsigned int slices = vcpu_sve_slices(vcpu);
> > > > + u64 reg;
> > > > + unsigned int i, n;
> > > > + int num_regs = 0;
> > > > +
> > > > + if (!vcpu_has_sve(vcpu))
> > > > + return 0;
> > > > +
> > > > + for (i = 0; i < slices; i++) {
> > > > + for (n = 0; n < SVE_NUM_ZREGS; n++) {
> > > > + reg = KVM_REG_ARM64_SVE_ZREG(n, i);
> > > > + if (put_user(reg, uindices++))
> > > > + return -EFAULT;
> > > > +
> > > > + num_regs++;
> > > > + }
> > > > +
> > > > + for (n = 0; n < SVE_NUM_PREGS; n++) {
> > > > + reg = KVM_REG_ARM64_SVE_PREG(n, i);
> > > > + if (put_user(reg, uindices++))
> > > > + return -EFAULT;
> > > > +
> > > > + num_regs++;
> > > > + }
> > > > +
> > > > + reg = KVM_REG_ARM64_SVE_FFR(i);
> > > > + if (put_user(reg, uindices++))
> > > > + return -EFAULT;
> > > > +
> > > > + num_regs++;
> > > > + }
> > >
> > > nit: the extra blank lines above the num_regs++'s give the code an odd
> > > look (to me)
> >
> > There's no guaranteed fall-through onto the increments: the blank line
> > was there to highlight the fact that we may jump out using a return
> > instead.
> >
> > But I'm happy enough to change it if you have a strong preference or
> > you feel the code is equally clear without.
>
> It's just a nit, so I don't have a strong preference :)
OK, well given that you mentioned it and there are other tweaks to make
on top of this patch anyway, I'll go ahead and make the change.
This saves a few lines, if nothing else.
Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
