On Tue, 2021-10-12 at 19:03 +0200, Paolo Bonzini wrote:
> On 12/10/21 18:57, Jarkko Sakkinen wrote:
> > > +
> > > static const struct file_operations sgx_vepc_fops = {
> > > .owner = THIS_MODULE,
> > > .open = sgx_vepc_open,
> > > + .unlocked_ioctl = sgx_vepc_ioctl,
> > > + .compat_ioctl = sgx_vepc_ioctl,
> > > .release = sgx_vepc_release,
> > > .mmap = sgx_vepc_mmap,
> > > };
> > I went through this a few times, the code change is sound and
> > reasoning makes sense in the commit message.
> >
> > The only thing that I think that is IMHO lacking is a simple
> > kselftest. I think a trivial test for SGX_IOC_VEP_REMOVE_ALL
> > would do.
>
> Yeah, a trivial test wouldn't cover a lot; it would be much better to at
> least set up a SECS, and check that the first call returns 1 and the
> second returns 0. There is no existing test for /dev/sgx_vepc at all.
>
> Right now I'm relying on Yang for testing this in QEMU, but I'll look
> into adding a selftest that does the full setup and runs an enclave in a
> guest.
This having a regression would not working would not cause that much collateral
damage, especially since it would be probably quickly noticed by someone, so I
think that this is not absolutely mandatory. So you can ignore kselftest part,
and thus
Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
Thank you, this work helps me a lot, given that my SGX testing is based around
using QEMU ATM.
/Jarkko
