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

Paraschiv, Andra-Irina
2020-08-11T08:17:36Z

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@amazon.com>
>> Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
>> Reviewed-by: Alexander Graf <graf@amazon.com>
>> ---
>> 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