From: "Paraschiv, Andra-Irina" <andraprs@amazon.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: <linux-kernel@vger.kernel.org>,
Anthony Liguori <aliguori@amazon.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Colm MacCarthaigh <colmmacc@amazon.com>,
Bjoern Doebel <doebel@amazon.de>,
David Woodhouse <dwmw@amazon.co.uk>,
Frank van der Linden <fllinden@amazon.com>,
"Alexander Graf" <graf@amazon.de>,
Martin Pohlack <mpohlack@amazon.de>, Matt Wilson <msw@amazon.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Balbir Singh <sblbir@amazon.com>,
Stefano Garzarella <sgarzare@redhat.com>,
"Stefan Hajnoczi" <stefanha@redhat.com>,
Stewart Smith <trawets@amazon.com>,
"Uwe Dannowski" <uwed@amazon.de>, <kvm@vger.kernel.org>,
<ne-devel-upstream@amazon.com>
Subject: Re: [PATCH v2 07/18] nitro_enclaves: Init misc device providing the ioctl interface
Date: Mon, 25 May 2020 23:49:50 +0300 [thread overview]
Message-ID: <fa3a72ef-ba0a-ada9-48bf-bd7cef0a8174@amazon.com> (raw)
In-Reply-To: <20200522070708.GC771317@kroah.com>
On 22/05/2020 10:07, Greg KH wrote:
> On Fri, May 22, 2020 at 09:29:35AM +0300, Andra Paraschiv wrote:
>> +static char *ne_cpus;
>> +module_param(ne_cpus, charp, 0644);
>> +MODULE_PARM_DESC(ne_cpus, "<cpu-list> - CPU pool used for Nitro Enclaves");
> This is not the 1990's, don't use module parameters if you can help it.
> Why is this needed, and where is it documented?
This is a CPU pool that can be set by the root user and that includes
CPUs set aside to be used for the enclave(s) setup; these CPUs are
offlined. From this CPU pool, the kernel logic chooses the CPUs that are
set for the created enclave(s).
The cpu-list format is matching the same that is documented here:
https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html
I've also thought of having a sysfs entry for the setup of this enclave
CPU pool.
>
>> +/* CPU pool used for Nitro Enclaves. */
>> +struct ne_cpu_pool {
>> + /* Available CPUs in the pool. */
>> + cpumask_var_t avail;
>> + struct mutex mutex;
>> +};
>> +
>> +static struct ne_cpu_pool ne_cpu_pool;
>> +
>> +static int ne_open(struct inode *node, struct file *file)
>> +{
>> + return 0;
>> +}
> If open does nothing, just don't even provide it.
I removed this and other file ops occurrences that do nothing for now.
>
>> +
>> +static long ne_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>> +{
>> + switch (cmd) {
>> +
>> + default:
>> + return -ENOTTY;
>> + }
>> +
>> + return 0;
>> +}
> Same for ioctl.
This logic is completed in the next patch in the series.
>
>> +
>> +static int ne_release(struct inode *inode, struct file *file)
>> +{
>> + return 0;
>> +}
> Same for release.
Done, I removed it for now.
>
>> +
>> +static const struct file_operations ne_fops = {
>> + .owner = THIS_MODULE,
>> + .llseek = noop_llseek,
>> + .unlocked_ioctl = ne_ioctl,
>> + .open = ne_open,
>> + .release = ne_release,
>> +};
>> +
>> +struct miscdevice ne_miscdevice = {
>> + .minor = MISC_DYNAMIC_MINOR,
>> + .name = NE_DEV_NAME,
>> + .fops = &ne_fops,
>> + .mode = 0660,
>> +};
>> +
>> +static int __init ne_init(void)
>> +{
>> + unsigned int cpu = 0;
>> + unsigned int cpu_sibling = 0;
>> + int rc = -EINVAL;
>> +
>> + memset(&ne_cpu_pool, 0, sizeof(ne_cpu_pool));
> Why did you just set a structure to 0 that was already initialized by
> the system to 0? Are you sure about this?
True, this is not needed. Removed the memset() call.
>
>> +
>> + if (!zalloc_cpumask_var(&ne_cpu_pool.avail, GFP_KERNEL))
>> + return -ENOMEM;
>> +
>> + mutex_init(&ne_cpu_pool.mutex);
>> +
>> + rc = cpulist_parse(ne_cpus, ne_cpu_pool.avail);
>> + if (rc < 0) {
>> + pr_err_ratelimited(NE "Error in cpulist parse [rc=%d]\n", rc);
> Again, drop all ratelimited stuff please.
Updated to pr_err().
Thank you.
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.
next prev parent reply other threads:[~2020-05-25 20:50 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-22 6:29 [PATCH v2 00/18] Add support for Nitro Enclaves Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 01/18] nitro_enclaves: Add ioctl interface definition Andra Paraschiv
2020-05-22 7:00 ` Greg KH
2020-05-22 8:16 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 02/18] nitro_enclaves: Define the PCI device interface Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 03/18] nitro_enclaves: Define enclave info for internal bookkeeping Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 04/18] nitro_enclaves: Init PCI device driver Andra Paraschiv
2020-05-22 7:04 ` Greg KH
2020-05-23 20:25 ` Alexander Graf
2020-05-24 6:32 ` Greg KH
2020-05-25 11:15 ` Paraschiv, Andra-Irina
2020-05-25 10:54 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 05/18] nitro_enclaves: Handle PCI device command requests Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 06/18] nitro_enclaves: Handle out-of-band PCI device events Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 07/18] nitro_enclaves: Init misc device providing the ioctl interface Andra Paraschiv
2020-05-22 7:07 ` Greg KH
2020-05-25 20:49 ` Paraschiv, Andra-Irina [this message]
2020-05-26 6:42 ` Greg KH
2020-05-26 8:17 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 08/18] nitro_enclaves: Add logic for enclave vm creation Andra Paraschiv
2020-05-22 7:08 ` Greg KH
2020-05-25 20:53 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 09/18] nitro_enclaves: Add logic for enclave vcpu creation Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 10/18] nitro_enclaves: Add logic for enclave image load metadata Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 11/18] nitro_enclaves: Add logic for enclave memory region set Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 12/18] nitro_enclaves: Add logic for enclave start Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 13/18] nitro_enclaves: Add logic for enclave termination Andra Paraschiv
2020-05-22 6:29 ` [PATCH v2 14/18] nitro_enclaves: Add Kconfig for the Nitro Enclaves driver Andra Paraschiv
2020-05-22 7:09 ` Greg KH
2020-05-25 21:00 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 15/18] nitro_enclaves: Add Makefile " Andra Paraschiv
2020-05-22 7:09 ` Greg KH
2020-05-25 21:02 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 16/18] nitro_enclaves: Add sample for ioctl interface usage Andra Paraschiv
2020-05-22 7:08 ` Greg KH
2020-05-25 20:57 ` Paraschiv, Andra-Irina
2020-05-26 6:41 ` Greg KH
2020-05-26 8:06 ` Paraschiv, Andra-Irina
2020-05-22 7:11 ` Greg KH
2020-05-25 21:10 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 17/18] nitro_enclaves: Add overview documentation Andra Paraschiv
2020-05-22 7:09 ` Greg KH
2020-05-25 21:04 ` Paraschiv, Andra-Irina
2020-05-22 6:29 ` [PATCH v2 18/18] MAINTAINERS: Add entry for the Nitro Enclaves driver Andra Paraschiv
2020-05-22 7:03 ` Joe Perches
2020-05-22 8:20 ` Paraschiv, Andra-Irina
2020-05-22 6:39 ` [PATCH v2 00/18] Add support for Nitro Enclaves Paraschiv, Andra-Irina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fa3a72ef-ba0a-ada9-48bf-bd7cef0a8174@amazon.com \
--to=andraprs@amazon.com \
--cc=aliguori@amazon.com \
--cc=benh@kernel.crashing.org \
--cc=colmmacc@amazon.com \
--cc=doebel@amazon.de \
--cc=dwmw@amazon.co.uk \
--cc=fllinden@amazon.com \
--cc=graf@amazon.de \
--cc=gregkh@linuxfoundation.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpohlack@amazon.de \
--cc=msw@amazon.com \
--cc=ne-devel-upstream@amazon.com \
--cc=pbonzini@redhat.com \
--cc=sblbir@amazon.com \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
--cc=trawets@amazon.com \
--cc=uwed@amazon.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).