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.
