Re: [PATCH v6 08/18] nitro_enclaves: Add logic for creating an enclave VM

[Paraschiv, Andra-Irina]

On 10/08/2020 09:11, Alexander Graf wrote:
> On 05.08.20 11:10, Andra Paraschiv wrote:
> > Add ioctl command logic for enclave VM creation. It triggers a slot
> > allocation. The enclave resources will be associated with this slot and
> > it will be used as an identifier for triggering enclave run.
> >
> > Return a file descriptor, namely enclave fd. This is further used by the
> > associated user space enclave process to set enclave resources and
> > trigger enclave termination.
> >
> > The poll function is implemented in order to notify the enclave process
> > when an enclave exits without a specific enclave termination command
> > trigger e.g. when an enclave crashes.
> >
> > Signed-off-by: Alexandru Vasile <lexnv@xxxxxxxxxx>
> > Signed-off-by: Andra Paraschiv <andraprs@xxxxxxxxxx>
> > Reviewed-by: Alexander Graf <graf@xxxxxxxxxx>
> > ---
> > Changelog
> >
> > v5 -> v6
> >
> > * Update the code base to init the ioctl function in this patch.
> > * Update documentation to kernel-doc format.
> >
> > v4 -> v5
> >
> > * Release the reference to the NE PCI device on create VM error.
> > * Close enclave fd on copy_to_user() failure; rename fd to enclave fd
> >    while at it.
> > * Remove sanity checks for situations that shouldn't happen, only if
> >    buggy system or broken logic at all.
> > * Remove log on copy_to_user() failure.
> >
> > v3 -> v4
> >
> > * Use dev_err instead of custom NE log pattern.
> > * Update the NE ioctl call to match the decoupling from the KVM API.
> > * Add metadata for the NUMA node for the enclave memory and CPUs.
> >
> > v2 -> v3
> >
> > * Remove the WARN_ON calls.
> > * Update static calls sanity checks.
> > * Update kzfree() calls to kfree().
> > * Remove file ops that do nothing for now - open.
> >
> > v1 -> v2
> >
> > * Add log pattern for NE.
> > * Update goto labels to match their purpose.
> > * Remove the BUG_ON calls.
> > ---
> >   drivers/virt/nitro_enclaves/ne_misc_dev.c | 229 ++++++++++++++++++++++
> >   1 file changed, 229 insertions(+)
> >
> > diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c 
> > b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> > index 472850250220..6c8c12f65666 100644
> > --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> > +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>
> [...]
>
> > +/**
> > + * ne_ioctl() - Ioctl function provided by the NE misc device.
> > + * @file:    File associated with this ioctl function.
> > + * @cmd:    The command that is set for the ioctl call.
> > + * @arg:    The argument that is provided for the ioctl call.
> > + *
> > + * Context: Process context.
> > + * Return:
> > + * * Ioctl result (e.g. enclave file descriptor) on success.
> > + * * Negative return value on failure.
> > + */
> > +static long ne_ioctl(struct file *file, unsigned int cmd, unsigned 
> > long arg)
> > +{
> > +    switch (cmd) {
> > +    case NE_CREATE_VM: {
> > +        int enclave_fd = -1;
> > +        struct file *enclave_file = NULL;
> > +        struct ne_pci_dev *ne_pci_dev = NULL;
> > +        /* TODO: Find another way to get the NE PCI device 
> > reference. */
> > +        struct pci_dev *pdev = pci_get_device(PCI_VENDOR_ID_AMAZON,
> > +                              PCI_DEVICE_ID_NE, NULL);
>
> This should go away if you set misc_dev.parent.
>
> > +        int rc = -EINVAL;
> > +        u64 slot_uid = 0;
> > +
> > +        ne_pci_dev = pci_get_drvdata(pdev);
> > +
> > +        mutex_lock(&ne_pci_dev->enclaves_list_mutex);
> > +
> > +        enclave_fd = ne_create_vm_ioctl(pdev, ne_pci_dev, &slot_uid);
> > +        if (enclave_fd < 0) {
> > +            rc = enclave_fd;
> > +
> > + mutex_unlock(&ne_pci_dev->enclaves_list_mutex);
> > +
> > +            pci_dev_put(pdev);
>
> This should also disappear.

Correct. I'll follow the misc dev parent approach to get the PCI device 
and include all the necessary code base updates in v7.

Thanks,
Andra



Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.