Messages in this thread | | | Subject | Re: [PATCH v4 07/18] nitro_enclaves: Init misc device providing the ioctl interface | From | "Paraschiv, Andra-Irina" <> | Date | Mon, 6 Jul 2020 16:09:26 +0300 |
| |
On 06/07/2020 11:01, Alexander Graf wrote: > > > On 06.07.20 09:49, Paraschiv, Andra-Irina wrote: >> >> >> On 06/07/2020 10:13, Alexander Graf wrote: >>> >>> >>> On 22.06.20 22:03, 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@amazon.com> >>>> --- >>>> Changelog >>>> >>>> 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 | 133 >>>> ++++++++++++++++++++++ >>>> drivers/virt/nitro_enclaves/ne_pci_dev.c | 11 ++ >>>> 2 files changed, 144 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..628fb10c2b36 >>>> --- /dev/null >>>> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c >>>> @@ -0,0 +1,133 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights >>>> Reserved. >>>> + */ >>>> + >>>> +/** >>>> + * 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" >>>> + >>>> +#define NE_EIF_LOAD_OFFSET (8 * 1024UL * 1024UL) >>>> + >>>> +#define NE_MIN_ENCLAVE_MEM_SIZE (64 * 1024UL * 1024UL) >>>> + >>>> +#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 = { >>> >>> Adding an empty ops struct looks very odd. If you fill it in a later >>> patch, please indicate so in a comment here. >> >> True, I already updated this in v5, to have the .get function here >> and the .set one in a later patch. >> >>> >>>> +}; >>>> + >>>> +static char ne_cpus[PAGE_SIZE]; >>> >>> PAGE_SIZE is a bit excessive, no? Even if you list every single CPU >>> of a 256 CPU system you are <1024. >> >> It is a bit too much, I was thinking of it while declaring this. I >> can update to 1024 in v5. > > The largest NUMA node CPU count I'm aware of today is 64. Since we > limit the pool to a single node, we can't go beyond that. Let's be a > bit future proof and double that number: 128. Then we get to 401 > characters if you pass in every single CPU as comma separated. I would > seriously hope most people would just pass ranges though. > > So how about we make it 512 for now?
We can set it like this, I changed to 512 and updated the comment as well.
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.
| |