Message-ID: <08fe3dc4-5849-e5b5-260e-31c0c70790d6@amazon.com>
Date:   Tue, 11 Aug 2020 10:58:11 +0300
From:   "Paraschiv, Andra-Irina" <andraprs@...zon.com>
To:     Alexander Graf <graf@...zon.de>,
        linux-kernel <linux-kernel@...r.kernel.org>
CC:     Anthony Liguori <aliguori@...zon.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Colm MacCarthaigh <colmmacc@...zon.com>,
        "David Duncan" <davdunc@...zon.com>,
        Bjoern Doebel <doebel@...zon.de>,
        "David Woodhouse" <dwmw@...zon.co.uk>,
        Frank van der Linden <fllinden@...zon.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Karen Noel <knoel@...hat.com>,
        "Martin Pohlack" <mpohlack@...zon.de>,
        Matt Wilson <msw@...zon.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Balbir Singh <sblbir@...zon.com>,
        Stefano Garzarella <sgarzare@...hat.com>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Stewart Smith <trawets@...zon.com>,
        Uwe Dannowski <uwed@...zon.de>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        kvm <kvm@...r.kernel.org>,
        ne-devel-upstream <ne-devel-upstream@...zon.com>
Subject: Re: [PATCH v6 07/18] nitro_enclaves: Init misc device providing the
 ioctl interface



On 10/08/2020 08:22, Alexander Graf wrote:
>
>
> On 05.08.20 11:10, Andra Paraschiv wrote:
>> The Nitro Enclaves driver provides an ioctl interface to the user space
>> for enclave lifetime management e.g. enclave creation / termination and
>> setting enclave resources such as memory and CPU.
>>
>> This ioctl interface is mapped to a Nitro Enclaves misc device.
>>
>> Signed-off-by: Andra Paraschiv <andraprs@...zon.com>
>> ---
>> Changelog
>>
>> v5 -> v6
>>
>> * Remove the ioctl to query API version.
>> * Update documentation to kernel-doc format.
>>
>> v4 -> v5
>>
>> * Update the size of the NE CPU pool string from 4096 to 512 chars.
>>
>> v3 -> v4
>>
>> * Use dev_err instead of custom NE log pattern.
>> * Remove the NE CPU pool init during kernel module loading, as the CPU
>>    pool is now setup at runtime, via a sysfs file for the kernel
>>    parameter.
>> * Add minimum enclave memory size definition.
>>
>> v2 -> v3
>>
>> * Remove the GPL additional wording as SPDX-License-Identifier is
>>    already in place.
>> * Remove the WARN_ON calls.
>> * Remove linux/bug and linux/kvm_host includes that are not needed.
>> * Remove "ratelimited" from the logs that are not in the ioctl call
>>    paths.
>> * Remove file ops that do nothing for now - open and release.
>>
>> v1 -> v2
>>
>> * Add log pattern for NE.
>> * Update goto labels to match their purpose.
>> * Update ne_cpu_pool data structure to include the global mutex.
>> * Update NE misc device mode to 0660.
>> * Check if the CPU siblings are included in the NE CPU pool, as full CPU
>>    cores are given for the enclave(s).
>> ---
>>   drivers/virt/nitro_enclaves/ne_misc_dev.c | 121 ++++++++++++++++++++++
>>   drivers/virt/nitro_enclaves/ne_pci_dev.c  |  11 ++
>>   2 files changed, 132 insertions(+)
>>   create mode 100644 drivers/virt/nitro_enclaves/ne_misc_dev.c
>>
>> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c 
>> b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> new file mode 100644
>> index 000000000000..472850250220
>> --- /dev/null
>> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
>> @@ -0,0 +1,121 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights 
>> Reserved.
>> + */
>> +
>> +/**
>> + * DOC: Enclave lifetime management driver for Nitro Enclaves (NE).
>> + * Nitro is a hypervisor that has been developed by Amazon.
>> + */
>> +
>> +#include <linux/anon_inodes.h>
>> +#include <linux/capability.h>
>> +#include <linux/cpu.h>
>> +#include <linux/device.h>
>> +#include <linux/file.h>
>> +#include <linux/hugetlb.h>
>> +#include <linux/list.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/mm.h>
>> +#include <linux/mman.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/nitro_enclaves.h>
>> +#include <linux/pci.h>
>> +#include <linux/poll.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +
>> +#include "ne_misc_dev.h"
>> +#include "ne_pci_dev.h"
>> +
>> +/**
>> + * NE_CPUS_SIZE - Size for max 128 CPUs, for now, in a cpu-list 
>> string, comma
>> + *          separated. The NE CPU pool includes CPUs from a single NUMA
>> + *          node.
>> + */
>> +#define NE_CPUS_SIZE        (512)
>> +
>> +/**
>> + * NE_EIF_LOAD_OFFSET - The offset where to copy the Enclave Image 
>> Format (EIF)
>> + *            image in enclave memory.
>> + */
>> +#define NE_EIF_LOAD_OFFSET    (8 * 1024UL * 1024UL)
>> +
>> +/**
>> + * NE_MIN_ENCLAVE_MEM_SIZE - The minimum memory size an enclave can 
>> be launched
>> + *                 with.
>> + */
>> +#define NE_MIN_ENCLAVE_MEM_SIZE    (64 * 1024UL * 1024UL)
>> +
>> +/**
>> + * NE_MIN_MEM_REGION_SIZE - The minimum size of an enclave memory 
>> region.
>> + */
>> +#define NE_MIN_MEM_REGION_SIZE    (2 * 1024UL * 1024UL)
>> +
>> +/*
>> + * TODO: Update logic to create new sysfs entries instead of using
>> + * a kernel parameter e.g. if multiple sysfs files needed.
>> + */
>> +static const struct kernel_param_ops ne_cpu_pool_ops = {
>> +    .get    = param_get_string,
>> +};
>> +
>> +static char ne_cpus[NE_CPUS_SIZE];
>> +static struct kparam_string ne_cpus_arg = {
>> +    .maxlen    = sizeof(ne_cpus),
>> +    .string    = ne_cpus,
>> +};
>> +
>> +module_param_cb(ne_cpus, &ne_cpu_pool_ops, &ne_cpus_arg, 0644);
>> +/* 
>> https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html#cpu-lists 
>> */
>> +MODULE_PARM_DESC(ne_cpus, "<cpu-list> - CPU pool used for Nitro 
>> Enclaves");
>> +
>> +/**
>> + * struct ne_cpu_pool - CPU pool used for Nitro Enclaves.
>> + * @avail_cores:    Available CPU cores in the pool.
>> + * @avail_cores_size:    The size of the available cores array.
>> + * @mutex:        Mutex for the access to the NE CPU pool.
>> + * @numa_node:        NUMA node of the CPUs in the pool.
>> + */
>> +struct ne_cpu_pool {
>> +    cpumask_var_t    *avail_cores;
>> +    unsigned int    avail_cores_size;
>> +    struct mutex    mutex;
>> +    int        numa_node;
>> +};
>> +
>> +static struct ne_cpu_pool ne_cpu_pool;
>> +
>> +static const struct file_operations ne_fops = {
>> +    .owner        = THIS_MODULE,
>> +    .llseek        = noop_llseek,
>> +};
>> +
>> +struct miscdevice ne_misc_dev = {
>> +    .minor    = MISC_DYNAMIC_MINOR,
>> +    .name    = "nitro_enclaves",
>> +    .fops    = &ne_fops,
>> +    .mode    = 0660,
>> +};
>> +
>> +static int __init ne_init(void)
>> +{
>> +    mutex_init(&ne_cpu_pool.mutex);
>> +
>> +    return pci_register_driver(&ne_pci_driver);
>> +}
>> +
>> +static void __exit ne_exit(void)
>> +{
>> +    pci_unregister_driver(&ne_pci_driver);
>> +}
>> +
>> +/* TODO: Handle actions such as reboot, kexec. */
>> +
>> +module_init(ne_init);
>> +module_exit(ne_exit);
>> +
>> +MODULE_AUTHOR("Amazon.com, Inc. or its affiliates");
>> +MODULE_DESCRIPTION("Nitro Enclaves Driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/virt/nitro_enclaves/ne_pci_dev.c 
>> b/drivers/virt/nitro_enclaves/ne_pci_dev.c
>> index a898fae066d9..1e434bf44c9d 100644
>> --- a/drivers/virt/nitro_enclaves/ne_pci_dev.c
>> +++ b/drivers/virt/nitro_enclaves/ne_pci_dev.c
>> @@ -527,6 +527,13 @@ static int ne_pci_probe(struct pci_dev *pdev, 
>> const struct pci_device_id *id)
>>           goto teardown_msix;
>>       }
>>   +    rc = misc_register(&ne_misc_dev);
>
> If you set ne_misc_dev.parent to &pdev->dev, you can establish a full 
> device path connection between the device node and the underlying NE 
> PCI device. That means that in the ioctl path, you can also just 
> access the device rather than search for it.
>
>

Thanks, that's a good option. I'll include this in v7.

Andra

>
>> +    if (rc < 0) {
>> +        dev_err(&pdev->dev, "Error in misc dev register [rc=%d]\n", 
>> rc);
>> +
>> +        goto disable_ne_pci_dev;
>> +    }
>> +
>>       atomic_set(&ne_pci_dev->cmd_reply_avail, 0);
>>       init_waitqueue_head(&ne_pci_dev->cmd_reply_wait_q);
>>       INIT_LIST_HEAD(&ne_pci_dev->enclaves_list);
>> @@ -536,6 +543,8 @@ static int ne_pci_probe(struct pci_dev *pdev, 
>> const struct pci_device_id *id)
>>         return 0;
>>   +disable_ne_pci_dev:
>> +    ne_pci_dev_disable(pdev);
>>   teardown_msix:
>>       ne_teardown_msix(pdev);
>>   iounmap_pci_bar:
>> @@ -561,6 +570,8 @@ static void ne_pci_remove(struct pci_dev *pdev)
>>   {
>>       struct ne_pci_dev *ne_pci_dev = pci_get_drvdata(pdev);
>>   +    misc_deregister(&ne_misc_dev);
>> +
>>       ne_pci_dev_disable(pdev);
>>         ne_teardown_msix(pdev);
>>




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.

Powered by blists - more mailing lists