On Thu, Nov 15, 2018 at 06:04:22PM +0000, Dave Martin wrote:
> On Fri, Nov 02, 2018 at 09:32:27AM +0100, Christoffer Dall wrote:
> > On Fri, Sep 28, 2018 at 02:39:24PM +0100, Dave Martin wrote:
> > > To enable arm64-specific vm ioctls to be added cleanly, this patch
> > > adds a kvm_arm_arch_vm_ioctl() hook so that these don't pollute the
> > > common code.
> >
> > Hmmm, I don't really see the strenght of that argument, and have the
> > same concern as before. I'd like to avoid the additional indirection
> > and instead just follow the existing pattern with a dummy implementation
> > on the 32-bit side that returns an error.
>
> So for this and the similar comment on patch 18, this was premature (or
> at least, overzealous) factoring on my part.
>
> I'm happy to merge this back together for arm and arm64 as you prefer.
>
> Do we have a nice way of writing the arch check, e.g.
>
> case KVM_ARM_SVE_CONFIG:
> if (!IS_ENABLED(ARM64))
> return -EINVAL;
> else
> return kvm_vcpu_sve_config(NULL, userp);
>
> should work, but looks a bit strange. Maybe I'm just being fussy.
I prefer just doing:
case KVM_ARM_SVE_CONFIG:
return kvm_vcpu_sve_config(NULL, userp);
And having this in arch/arm/include/asm/kvm_foo.h:
static inline int kvm_vcpu_sve_config(...)
{
return -EINVAL;
}
Thanks,
Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
