Re: [PATCH v3 07/18] nitro_enclaves: Init misc device providing the ioctl interface

From: Paraschiv, Andra-Irina
Date: Tue May 26 2020 - 09:41:44 EST




On 26/05/2020 15:33, Greg KH wrote:
On Tue, May 26, 2020 at 01:42:41PM +0200, Alexander Graf wrote:

On 26.05.20 08:51, Greg KH wrote:
On Tue, May 26, 2020 at 01:13:23AM +0300, Andra Paraschiv wrote:
+#define NE "nitro_enclaves: "
Again, no need for this.

+#define NE_DEV_NAME "nitro_enclaves"
KBUILD_MODNAME?

+#define NE_IMAGE_LOAD_OFFSET (8 * 1024UL * 1024UL)
+
+static char *ne_cpus;
+module_param(ne_cpus, charp, 0644);
+MODULE_PARM_DESC(ne_cpus, "<cpu-list> - CPU pool used for Nitro Enclaves");
Again, please do not do this.
I actually asked her to put this one in specifically.

The concept of this parameter is very similar to isolcpus= and maxcpus= in
that it takes CPUs away from Linux and instead donates them to the
underlying hypervisor, so that it can spawn enclaves using them.

From an admin's point of view, this is a setting I would like to keep
persisted across reboots. How would this work with sysfs?
How about just as the "initial" ioctl command to set things up? Don't
grab any cpu pools until asked to. Otherwise, what happens when you
load this module on a system that can't support it?

module parameters are a major pain, you know this :)

So yes, let's give everyone in CC the change to review v3 properly first
before v4 goes out.

And get them to sign off on it too, showing they agree with the design
decisions here :)
I would expect a Reviewed-by tag as a result from the above would satisfy
this? :)
That would be most appreciated.

With regarding to reviewing, yes, the patch series went through several rounds before submitting it upstream.

I posted v3 shortly after v2 to have more visibility into the changelog for each patch in addition to the cover letter changelog. But no major design changes in there. :)

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.