LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v5 0/4] Merge contiguous physical memory regions
@ 2021-11-07 14:09 Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 1/4] nitro_enclaves: " Longpeng(Mike)
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Longpeng(Mike) @ 2021-11-07 14:09 UTC (permalink / raw)
  To: andraprs, lexnv, alcioa
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, Longpeng

From: Longpeng <longpeng2@huawei.com>

Hi guys,

This patchset try to merge the contiguous physical memory regions when
set user memory regions, you can see message in PATCH 1 for details.
Please review when you free, thank!

Changes v4 -> v5:
  Patch 1:
    - "Physical contiguous memory regions" -> "Contiguous physical memory 
      regions."  [Andra]
    - fix the warning of aligment that reported by the checkpath.pl  [Andra]
  Patch 4:
    - fix the warning of aligment that reported by the checkpath.pl  [Andra]
    - remove unnecessary comparison of NULL.  [Andra]

Changes v3 -> v4:
  Patch 1:
    - move "#include <linux/range.h>" according to the alphabetical order. [Andra]
    - rename several variables, parameters, structures and functions.  [Andra]
    - add missing "Context" in the comments.  [Andra]
    - some other changes to makes the code much neater.  [Andra]
  Patch 2:
    - add missing "Context" in the comments.  [Andra]
    - move the comment in ne_merge_phys_contig_memory_regions() before
      the "if (...)". [Andra]
  Patch 3:
    - Nitro enclaves -> Nitro Enclaves   [Andra]
    - check the return code of "ne_misc_dev_test_init()"  [Andra]
    - GPL-2.0-or-later -> GPL-2.0  [Andra]
  Patch 4:
    - "int expect_num" -> "unsigned long  expect_num"  [Andra]
    - rename several variables and structures  [Andra]
    - invoke "kunit_kfree" to free the "regions"  [Andra]

Changes v2 -> v3:
  Patch 1:
    - update the commit title and commit message.  [Andra]
    - use 'struct range' to instead of 'struct phys_mem_region'.  [Andra, Greg KH]
    - add comments before the function definition.  [Andra]
    - rename several variables, parameters and function.  [Andra]
  Patch 2:
    - update the commit title and commit message.  [Andra]
    - add comments before the function definition.  [Andra]
    - remove 'inline' attribute of ne_sanity_check_phys_mem_region. [Andra]
    - leave a blank line before return.  [Andra]
    - move sanity check in ne_merge_phys_contig_memory_regions to
      the beginning of the function.  [Andra]
    - double sanity checking after the merge of physical contiguous
      memory regions has been completed.  [Andra]
  Patch 3:
    - update the commit title and commit message.  [Andra]
    - use "misc_dev"/"misc device"/"MISC_DEV" to be more specific.  [Andra]
  Patch 4:
    - update the commit title and commit message.  [Andra]
    - align the fileds in 'struct phys_regions_test'.  [Andra]
    - rename 'phys_regions_testcases' to 'phys_regions_test_cases'.  [Andra]
    - add comments before each test cases.  [Andra]
    - initialize the variables in ne_misc_dev_test_merge_phys_contig_memory_regions.  [Andra]

Changes v1 -> v2:
  - update the commit message as Andra's suggestion  [Andra]
  - remove TODO completely in ne_set_user_memory_region_ioctl  [Andra]
  - extract the physical memory regions setup into individual
    function
  - add kunit tests  [Andra]

Longpeng (4):
  nitro_enclaves: Merge contiguous physical memory regions
  nitro_enclaves: Sanity check physical memory regions during merging
  nitro_enclaves: Add KUnit tests setup for the misc device
    functionality
  nitro_enclaves: Add KUnit tests for contiguous physical memory regions
    merging

 drivers/virt/nitro_enclaves/Kconfig            |   9 ++
 drivers/virt/nitro_enclaves/ne_misc_dev.c      | 174 ++++++++++++++++++-------
 drivers/virt/nitro_enclaves/ne_misc_dev_test.c | 157 ++++++++++++++++++++++
 3 files changed, 296 insertions(+), 44 deletions(-)
 create mode 100644 drivers/virt/nitro_enclaves/ne_misc_dev_test.c

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/4] nitro_enclaves: Merge contiguous physical memory regions
  2021-11-07 14:09 [PATCH v5 0/4] Merge contiguous physical memory regions Longpeng(Mike)
@ 2021-11-07 14:09 ` Longpeng(Mike)
  2021-11-08 10:23   ` Paraschiv, Andra-Irina
  2021-11-07 14:09 ` [PATCH v5 2/4] nitro_enclaves: Sanity check physical memory regions during merging Longpeng(Mike)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Longpeng(Mike) @ 2021-11-07 14:09 UTC (permalink / raw)
  To: andraprs, lexnv, alcioa
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, Longpeng

From: Longpeng <longpeng2@huawei.com>

There can be cases when there are more memory regions that need to be
set for an enclave than the maximum supported number of memory regions
per enclave. One example can be when the memory regions are backed by 2
MiB hugepages (the minimum supported hugepage size).

Let's merge the adjacent regions if they are physically contiguous. This
way the final number of memory regions is less than before merging and
could potentially avoid reaching maximum.

Signed-off-by: Longpeng <longpeng2@huawei.com>
---
Changes v4 -> v5:
  - "Physical contiguous memory regions" -> "Contiguous physical memory 
    regions."  [Andra]
  - fix the warning of aligment that reported by the checkpath.pl  [Andra]
  
Changes v3 -> v4:
  - move "#include <linux/range.h>" according to the alphabetical order. [Andra]
  - rename several variables, parameters, structures and functions.  [Andra]
  - add missing "Context" in the comments.  [Andra]
  - some other changes to makes the code much neater.  [Andra]

Changes v2 -> v3:
  - update the commit title and commit message.  [Andra]
  - use 'struct range' to instead of 'struct phys_mem_region'.  [Andra, Greg KH]
  - add comments before the function definition.  [Andra]
  - rename several variables, parameters and function.  [Andra]
---
 drivers/virt/nitro_enclaves/ne_misc_dev.c | 84 ++++++++++++++++++++-----------
 1 file changed, 56 insertions(+), 28 deletions(-)

diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index 8939612..ced58de 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -24,6 +24,7 @@
 #include <linux/nitro_enclaves.h>
 #include <linux/pci.h>
 #include <linux/poll.h>
+#include <linux/range.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <uapi/linux/vm_sockets.h>
@@ -126,6 +127,16 @@ struct ne_cpu_pool {
 static struct ne_cpu_pool ne_cpu_pool;
 
 /**
+ * struct ne_phys_contig_mem_regions - Contiguous physical memory regions.
+ * @num:	The number of regions that currently has.
+ * @regions:	The array of physical memory regions.
+ */
+struct ne_phys_contig_mem_regions {
+	unsigned long num;
+	struct range  *regions;
+};
+
+/**
  * ne_check_enclaves_created() - Verify if at least one enclave has been created.
  * @void:	No parameters provided.
  *
@@ -825,6 +836,33 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
 }
 
 /**
+ * ne_merge_phys_contig_memory_regions() - Add a memory region and merge the adjacent
+ *                                         regions if they are physically contiguous.
+ * @phys_contig_regions : Private data associated with the contiguous physical memory regions.
+ * @page_paddr :          Physical start address of the region to be added.
+ * @page_size :           Length of the region to be added.
+ *
+ * Context: Process context. This function is called with the ne_enclave mutex held.
+ */
+static void
+ne_merge_phys_contig_memory_regions(struct ne_phys_contig_mem_regions *phys_contig_regions,
+				    u64 page_paddr, u64 page_size)
+{
+	unsigned long num = phys_contig_regions->num;
+
+	/* Physically contiguous, just merge */
+	if (num && (phys_contig_regions->regions[num - 1].end + 1) == page_paddr) {
+		phys_contig_regions->regions[num - 1].end += page_size;
+
+		return;
+	}
+
+	phys_contig_regions->regions[num].start = page_paddr;
+	phys_contig_regions->regions[num].end = page_paddr + page_size - 1;
+	phys_contig_regions->num++;
+}
+
+/**
  * ne_set_user_memory_region_ioctl() - Add user space memory region to the slot
  *				       associated with the current enclave.
  * @ne_enclave :	Private data associated with the current enclave.
@@ -843,9 +881,8 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 	unsigned long max_nr_pages = 0;
 	unsigned long memory_size = 0;
 	struct ne_mem_region *ne_mem_region = NULL;
-	unsigned long nr_phys_contig_mem_regions = 0;
 	struct pci_dev *pdev = ne_devs.ne_pci_dev->pdev;
-	struct page **phys_contig_mem_regions = NULL;
+	struct ne_phys_contig_mem_regions phys_contig_mem_regions = {};
 	int rc = -EINVAL;
 
 	rc = ne_sanity_check_user_mem_region(ne_enclave, mem_region);
@@ -866,9 +903,10 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 		goto free_mem_region;
 	}
 
-	phys_contig_mem_regions = kcalloc(max_nr_pages, sizeof(*phys_contig_mem_regions),
-					  GFP_KERNEL);
-	if (!phys_contig_mem_regions) {
+	phys_contig_mem_regions.regions = kcalloc(max_nr_pages,
+						  sizeof(*phys_contig_mem_regions.regions),
+						  GFP_KERNEL);
+	if (!phys_contig_mem_regions.regions) {
 		rc = -ENOMEM;
 
 		goto free_mem_region;
@@ -901,26 +939,16 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 		if (rc < 0)
 			goto put_pages;
 
-		/*
-		 * TODO: Update once handled non-contiguous memory regions
-		 * received from user space or contiguous physical memory regions
-		 * larger than 2 MiB e.g. 8 MiB.
-		 */
-		phys_contig_mem_regions[i] = ne_mem_region->pages[i];
+		ne_merge_phys_contig_memory_regions(&phys_contig_mem_regions,
+						    page_to_phys(ne_mem_region->pages[i]),
+						    page_size(ne_mem_region->pages[i]));
 
 		memory_size += page_size(ne_mem_region->pages[i]);
 
 		ne_mem_region->nr_pages++;
 	} while (memory_size < mem_region.memory_size);
 
-	/*
-	 * TODO: Update once handled non-contiguous memory regions received
-	 * from user space or contiguous physical memory regions larger than
-	 * 2 MiB e.g. 8 MiB.
-	 */
-	nr_phys_contig_mem_regions = ne_mem_region->nr_pages;
-
-	if ((ne_enclave->nr_mem_regions + nr_phys_contig_mem_regions) >
+	if ((ne_enclave->nr_mem_regions + phys_contig_mem_regions.num) >
 	    ne_enclave->max_mem_regions) {
 		dev_err_ratelimited(ne_misc_dev.this_device,
 				    "Reached max memory regions %lld\n",
@@ -931,9 +959,9 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 		goto put_pages;
 	}
 
-	for (i = 0; i < nr_phys_contig_mem_regions; i++) {
-		u64 phys_region_addr = page_to_phys(phys_contig_mem_regions[i]);
-		u64 phys_region_size = page_size(phys_contig_mem_regions[i]);
+	for (i = 0; i < phys_contig_mem_regions.num; i++) {
+		u64 phys_region_addr = phys_contig_mem_regions.regions[i].start;
+		u64 phys_region_size = range_len(&phys_contig_mem_regions.regions[i]);
 
 		if (phys_region_size & (NE_MIN_MEM_REGION_SIZE - 1)) {
 			dev_err_ratelimited(ne_misc_dev.this_device,
@@ -959,13 +987,13 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 
 	list_add(&ne_mem_region->mem_region_list_entry, &ne_enclave->mem_regions_list);
 
-	for (i = 0; i < nr_phys_contig_mem_regions; i++) {
+	for (i = 0; i < phys_contig_mem_regions.num; i++) {
 		struct ne_pci_dev_cmd_reply cmd_reply = {};
 		struct slot_add_mem_req slot_add_mem_req = {};
 
 		slot_add_mem_req.slot_uid = ne_enclave->slot_uid;
-		slot_add_mem_req.paddr = page_to_phys(phys_contig_mem_regions[i]);
-		slot_add_mem_req.size = page_size(phys_contig_mem_regions[i]);
+		slot_add_mem_req.paddr = phys_contig_mem_regions.regions[i].start;
+		slot_add_mem_req.size = range_len(&phys_contig_mem_regions.regions[i]);
 
 		rc = ne_do_request(pdev, SLOT_ADD_MEM,
 				   &slot_add_mem_req, sizeof(slot_add_mem_req),
@@ -974,7 +1002,7 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 			dev_err_ratelimited(ne_misc_dev.this_device,
 					    "Error in slot add mem [rc=%d]\n", rc);
 
-			kfree(phys_contig_mem_regions);
+			kfree(phys_contig_mem_regions.regions);
 
 			/*
 			 * Exit here without put pages as memory regions may
@@ -987,7 +1015,7 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 		ne_enclave->nr_mem_regions++;
 	}
 
-	kfree(phys_contig_mem_regions);
+	kfree(phys_contig_mem_regions.regions);
 
 	return 0;
 
@@ -995,7 +1023,7 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 	for (i = 0; i < ne_mem_region->nr_pages; i++)
 		put_page(ne_mem_region->pages[i]);
 free_mem_region:
-	kfree(phys_contig_mem_regions);
+	kfree(phys_contig_mem_regions.regions);
 	kfree(ne_mem_region->pages);
 	kfree(ne_mem_region);
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 2/4] nitro_enclaves: Sanity check physical memory regions during merging
  2021-11-07 14:09 [PATCH v5 0/4] Merge contiguous physical memory regions Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 1/4] nitro_enclaves: " Longpeng(Mike)
@ 2021-11-07 14:09 ` Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 3/4] nitro_enclaves: Add KUnit tests setup for the misc device functionality Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 4/4] nitro_enclaves: Add KUnit tests for contiguous physical memory regions merging Longpeng(Mike)
  3 siblings, 0 replies; 7+ messages in thread
From: Longpeng(Mike) @ 2021-11-07 14:09 UTC (permalink / raw)
  To: andraprs, lexnv, alcioa
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, Longpeng

From: Longpeng <longpeng2@huawei.com>

Sanity check the physical memory regions during the merge of contiguous
regions. Thus we can test the physical memory regions setup logic
individually, including the error cases coming from the sanity checks.

Signed-off-by: Longpeng <longpeng2@huawei.com>
Reviewed-by: Andra Paraschiv <andraprs@amazon.com>
---
 drivers/virt/nitro_enclaves/ne_misc_dev.c | 77 +++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 25 deletions(-)

diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index ced58de..83ed9b5 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -836,6 +836,37 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
 }
 
 /**
+ * ne_sanity_check_phys_mem_region() - Sanity check the start address and the size
+ *                                     of a physical memory region.
+ * @phys_mem_region_paddr : Physical start address of the region to be sanity checked.
+ * @phys_mem_region_size  : Length of the region to be sanity checked.
+ *
+ * Context: Process context. This function is called with the ne_enclave mutex held.
+ * Return:
+ * * 0 on success.
+ * * Negative return value on failure.
+ */
+static int ne_sanity_check_phys_mem_region(u64 phys_mem_region_paddr,
+					   u64 phys_mem_region_size)
+{
+	if (phys_mem_region_size & (NE_MIN_MEM_REGION_SIZE - 1)) {
+		dev_err_ratelimited(ne_misc_dev.this_device,
+				    "Physical mem region size is not multiple of 2 MiB\n");
+
+		return -EINVAL;
+	}
+
+	if (!IS_ALIGNED(phys_mem_region_paddr, NE_MIN_MEM_REGION_SIZE)) {
+		dev_err_ratelimited(ne_misc_dev.this_device,
+				    "Physical mem region address is not 2 MiB aligned\n");
+
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
  * ne_merge_phys_contig_memory_regions() - Add a memory region and merge the adjacent
  *                                         regions if they are physically contiguous.
  * @phys_contig_regions : Private data associated with the contiguous physical memory regions.
@@ -843,23 +874,31 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
  * @page_size :           Length of the region to be added.
  *
  * Context: Process context. This function is called with the ne_enclave mutex held.
+ * Return:
+ * * 0 on success.
+ * * Negative return value on failure.
  */
-static void
+static int
 ne_merge_phys_contig_memory_regions(struct ne_phys_contig_mem_regions *phys_contig_regions,
 				    u64 page_paddr, u64 page_size)
 {
 	unsigned long num = phys_contig_regions->num;
+	int rc = 0;
+
+	rc = ne_sanity_check_phys_mem_region(page_paddr, page_size);
+	if (rc < 0)
+		return rc;
 
 	/* Physically contiguous, just merge */
 	if (num && (phys_contig_regions->regions[num - 1].end + 1) == page_paddr) {
 		phys_contig_regions->regions[num - 1].end += page_size;
-
-		return;
+	} else {
+		phys_contig_regions->regions[num].start = page_paddr;
+		phys_contig_regions->regions[num].end = page_paddr + page_size - 1;
+		phys_contig_regions->num++;
 	}
 
-	phys_contig_regions->regions[num].start = page_paddr;
-	phys_contig_regions->regions[num].end = page_paddr + page_size - 1;
-	phys_contig_regions->num++;
+	return 0;
 }
 
 /**
@@ -939,9 +978,11 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 		if (rc < 0)
 			goto put_pages;
 
-		ne_merge_phys_contig_memory_regions(&phys_contig_mem_regions,
-						    page_to_phys(ne_mem_region->pages[i]),
-						    page_size(ne_mem_region->pages[i]));
+		rc = ne_merge_phys_contig_memory_regions(&phys_contig_mem_regions,
+							 page_to_phys(ne_mem_region->pages[i]),
+							 page_size(ne_mem_region->pages[i]));
+		if (rc < 0)
+			goto put_pages;
 
 		memory_size += page_size(ne_mem_region->pages[i]);
 
@@ -963,23 +1004,9 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
 		u64 phys_region_addr = phys_contig_mem_regions.regions[i].start;
 		u64 phys_region_size = range_len(&phys_contig_mem_regions.regions[i]);
 
-		if (phys_region_size & (NE_MIN_MEM_REGION_SIZE - 1)) {
-			dev_err_ratelimited(ne_misc_dev.this_device,
-					    "Physical mem region size is not multiple of 2 MiB\n");
-
-			rc = -EINVAL;
-
-			goto put_pages;
-		}
-
-		if (!IS_ALIGNED(phys_region_addr, NE_MIN_MEM_REGION_SIZE)) {
-			dev_err_ratelimited(ne_misc_dev.this_device,
-					    "Physical mem region address is not 2 MiB aligned\n");
-
-			rc = -EINVAL;
-
+		rc = ne_sanity_check_phys_mem_region(phys_region_addr, phys_region_size);
+		if (rc < 0)
 			goto put_pages;
-		}
 	}
 
 	ne_mem_region->memory_size = mem_region.memory_size;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 3/4] nitro_enclaves: Add KUnit tests setup for the misc device functionality
  2021-11-07 14:09 [PATCH v5 0/4] Merge contiguous physical memory regions Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 1/4] nitro_enclaves: " Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 2/4] nitro_enclaves: Sanity check physical memory regions during merging Longpeng(Mike)
@ 2021-11-07 14:09 ` Longpeng(Mike)
  2021-11-07 14:09 ` [PATCH v5 4/4] nitro_enclaves: Add KUnit tests for contiguous physical memory regions merging Longpeng(Mike)
  3 siblings, 0 replies; 7+ messages in thread
From: Longpeng(Mike) @ 2021-11-07 14:09 UTC (permalink / raw)
  To: andraprs, lexnv, alcioa
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, Longpeng

From: Longpeng <longpeng2@huawei.com>

Add the initial setup for the KUnit tests that will target the Nitro
Enclaves misc device functionality.

Signed-off-by: Longpeng <longpeng2@huawei.com>
Reviewed-by: Andra Paraschiv <andraprs@amazon.com>
---
 drivers/virt/nitro_enclaves/Kconfig            |  9 ++++++++
 drivers/virt/nitro_enclaves/ne_misc_dev.c      | 31 ++++++++++++++++++++++++++
 drivers/virt/nitro_enclaves/ne_misc_dev_test.c | 17 ++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 drivers/virt/nitro_enclaves/ne_misc_dev_test.c

diff --git a/drivers/virt/nitro_enclaves/Kconfig b/drivers/virt/nitro_enclaves/Kconfig
index f53740b..2d3d981 100644
--- a/drivers/virt/nitro_enclaves/Kconfig
+++ b/drivers/virt/nitro_enclaves/Kconfig
@@ -14,3 +14,12 @@ config NITRO_ENCLAVES
 
 	  To compile this driver as a module, choose M here.
 	  The module will be called nitro_enclaves.
+
+config NITRO_ENCLAVES_MISC_DEV_TEST
+	bool "Tests for the misc device functionality of the Nitro Enclaves"
+	depends on NITRO_ENCLAVES && KUNIT=y
+	help
+	  Enable KUnit tests for the misc device functionality of the Nitro
+	  Enclaves. Select this option only if you will boot the kernel for
+	  the purpose of running unit tests (e.g. under UML or qemu). If
+	  unsure, say N.
diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index 83ed9b5..51ba4ca 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -1756,8 +1756,37 @@ static long ne_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return 0;
 }
 
+#if defined(CONFIG_NITRO_ENCLAVES_MISC_DEV_TEST)
+#include "ne_misc_dev_test.c"
+
+static inline int ne_misc_dev_test_init(void)
+{
+	return __kunit_test_suites_init(ne_misc_dev_test_suites);
+}
+
+static inline void ne_misc_dev_test_exit(void)
+{
+	__kunit_test_suites_exit(ne_misc_dev_test_suites);
+}
+#else
+static inline int ne_misc_dev_test_init(void)
+{
+	return 0;
+}
+
+static inline void ne_misc_dev_test_exit(void)
+{
+}
+#endif
+
 static int __init ne_init(void)
 {
+	int rc = 0;
+
+	rc = ne_misc_dev_test_init();
+	if (rc < 0)
+		return rc;
+
 	mutex_init(&ne_cpu_pool.mutex);
 
 	return pci_register_driver(&ne_pci_driver);
@@ -1768,6 +1797,8 @@ static void __exit ne_exit(void)
 	pci_unregister_driver(&ne_pci_driver);
 
 	ne_teardown_cpu_pool();
+
+	ne_misc_dev_test_exit();
 }
 
 module_init(ne_init);
diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
new file mode 100644
index 0000000..6862e99
--- /dev/null
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <kunit/test.h>
+
+static struct kunit_case ne_misc_dev_test_cases[] = {
+	{}
+};
+
+static struct kunit_suite ne_misc_dev_test_suite = {
+	.name = "ne_misc_dev_test",
+	.test_cases = ne_misc_dev_test_cases,
+};
+
+static struct kunit_suite *ne_misc_dev_test_suites[] = {
+	&ne_misc_dev_test_suite,
+	NULL
+};
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 4/4] nitro_enclaves: Add KUnit tests for contiguous physical memory regions merging
  2021-11-07 14:09 [PATCH v5 0/4] Merge contiguous physical memory regions Longpeng(Mike)
                   ` (2 preceding siblings ...)
  2021-11-07 14:09 ` [PATCH v5 3/4] nitro_enclaves: Add KUnit tests setup for the misc device functionality Longpeng(Mike)
@ 2021-11-07 14:09 ` Longpeng(Mike)
  2021-11-08 10:25   ` Paraschiv, Andra-Irina
  3 siblings, 1 reply; 7+ messages in thread
From: Longpeng(Mike) @ 2021-11-07 14:09 UTC (permalink / raw)
  To: andraprs, lexnv, alcioa
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, Longpeng

From: Longpeng <longpeng2@huawei.com>

Add KUnit tests for the contiguous physical memory regions merging
functionality from the Nitro Enclaves misc device logic.

We can build the test binary with the following configuration:
  CONFIG_KUNIT=y
  CONFIG_NITRO_ENCLAVES=m
  CONFIG_NITRO_ENCLAVES_MISC_DEV_TEST=y
and install the nitro_enclaves module to run the testcases.

We'll see the following message using dmesg if everything goes well:

[...]     # Subtest: ne_misc_dev_test
[...]     1..1
[...] (NULL device *): Physical mem region address is not 2 MiB aligned
[...] (NULL device *): Physical mem region size is not multiple of 2 MiB
[...] (NULL device *): Physical mem region address is not 2 MiB aligned
[...]     ok 1 - ne_misc_dev_test_merge_phys_contig_memory_regions
[...] ok 1 - ne_misc_dev_test

Signed-off-by: Longpeng <longpeng2@huawei.com>
---
Changes v4 -> v5:
  - fix the warning of aligment that reported by the checkpath.pl  [Andra]
  - remove unnecessary comparison of NULL.  [Andra]

Changes v3 -> v4:
  - "int expect_num" -> "unsigned long  expect_num"  [Andra]
  - rename several variables and structures  [Andra]
  - invoke "kunit_kfree" to free the "regions"  [Andra]

Changes v2 -> v3:
  - update the commit title and commit message.  [Andra]
  - align the fileds in 'struct phys_regions_test'.  [Andra]
  - rename 'phys_regions_testcases' to 'phys_regions_test_cases'.  [Andra]
  - add comments before each test cases.  [Andra]
  - initialize the variables in ne_misc_dev_test_merge_phys_contig_memory_regions.  [Andra]
---
 drivers/virt/nitro_enclaves/ne_misc_dev_test.c | 140 +++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
index 6862e99..265797b 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
@@ -2,7 +2,147 @@
 
 #include <kunit/test.h>
 
+#define MAX_PHYS_REGIONS	16
+#define INVALID_VALUE		(~0ull)
+
+struct ne_phys_regions_test {
+	u64           paddr;
+	u64           size;
+	int           expect_rc;
+	unsigned long expect_num;
+	u64           expect_last_paddr;
+	u64           expect_last_size;
+} phys_regions_test_cases[] = {
+	/*
+	 * Add the region from 0x1000 to (0x1000 + 0x200000 - 1):
+	 *   Expected result:
+	 *       Failed, start address is not 2M-aligned
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 0
+	 *   regions = {}
+	 */
+	{0x1000, 0x200000, -EINVAL, 0, INVALID_VALUE, INVALID_VALUE},
+
+	/*
+	 * Add the region from 0x200000 to (0x200000 + 0x1000 - 1):
+	 *   Expected result:
+	 *       Failed, size is not 2M-aligned
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 0
+	 *   regions = {}
+	 */
+	{0x200000, 0x1000, -EINVAL, 0, INVALID_VALUE, INVALID_VALUE},
+
+	/*
+	 * Add the region from 0x200000 to (0x200000 + 0x200000 - 1):
+	 *   Expected result:
+	 *       Successful
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 1
+	 *   regions = {
+	 *       {start=0x200000, end=0x3fffff}, // len=0x200000
+	 *   }
+	 */
+	{0x200000, 0x200000, 0, 1, 0x200000, 0x200000},
+
+	/*
+	 * Add the region from 0x0 to (0x0 + 0x200000 - 1):
+	 *   Expected result:
+	 *       Successful
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 2
+	 *   regions = {
+	 *       {start=0x200000, end=0x3fffff}, // len=0x200000
+	 *       {start=0x0,      end=0x1fffff}, // len=0x200000
+	 *   }
+	 */
+	{0x0, 0x200000, 0, 2, 0x0, 0x200000},
+
+	/*
+	 * Add the region from 0x600000 to (0x600000 + 0x400000 - 1):
+	 *   Expected result:
+	 *       Successful
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 3
+	 *   regions = {
+	 *       {start=0x200000, end=0x3fffff}, // len=0x200000
+	 *       {start=0x0,      end=0x1fffff}, // len=0x200000
+	 *       {start=0x600000, end=0x9fffff}, // len=0x400000
+	 *   }
+	 */
+	{0x600000, 0x400000, 0, 3, 0x600000, 0x400000},
+
+	/*
+	 * Add the region from 0xa00000 to (0xa00000 + 0x400000 - 1):
+	 *   Expected result:
+	 *       Successful, merging case!
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 3
+	 *   regions = {
+	 *       {start=0x200000, end=0x3fffff}, // len=0x200000
+	 *       {start=0x0,      end=0x1fffff}, // len=0x200000
+	 *       {start=0x600000, end=0xdfffff}, // len=0x800000
+	 *   }
+	 */
+	{0xa00000, 0x400000, 0, 3, 0x600000, 0x800000},
+
+	/*
+	 * Add the region from 0x1000 to (0x1000 + 0x200000 - 1):
+	 *   Expected result:
+	 *       Failed, start address is not 2M-aligned
+	 *
+	 * Now the instance of struct ne_phys_contig_mem_regions is:
+	 *   num = 3
+	 *   regions = {
+	 *       {start=0x200000, end=0x3fffff}, // len=0x200000
+	 *       {start=0x0,      end=0x1fffff}, // len=0x200000
+	 *       {start=0x600000, end=0xdfffff}, // len=0x800000
+	 *   }
+	 */
+	{0x1000, 0x200000, -EINVAL, 3, 0x600000, 0x800000},
+};
+
+static void ne_misc_dev_test_merge_phys_contig_memory_regions(struct kunit *test)
+{
+	struct ne_phys_contig_mem_regions phys_contig_mem_regions = {};
+	int rc = 0;
+	int i = 0;
+
+	phys_contig_mem_regions.regions = kunit_kcalloc(test, MAX_PHYS_REGIONS,
+							sizeof(*phys_contig_mem_regions.regions),
+							GFP_KERNEL);
+	KUNIT_ASSERT_TRUE(test, phys_contig_mem_regions.regions);
+
+	for (i = 0; i < ARRAY_SIZE(phys_regions_test_cases); i++) {
+		struct ne_phys_regions_test *test_case = &phys_regions_test_cases[i];
+		unsigned long num = 0;
+
+		rc = ne_merge_phys_contig_memory_regions(&phys_contig_mem_regions,
+							 test_case->paddr, test_case->size);
+		KUNIT_EXPECT_EQ(test, rc, test_case->expect_rc);
+		KUNIT_EXPECT_EQ(test, phys_contig_mem_regions.num, test_case->expect_num);
+
+		if (test_case->expect_last_paddr == INVALID_VALUE)
+			continue;
+
+		num = phys_contig_mem_regions.num;
+		KUNIT_EXPECT_EQ(test, phys_contig_mem_regions.regions[num - 1].start,
+				test_case->expect_last_paddr);
+		KUNIT_EXPECT_EQ(test, range_len(&phys_contig_mem_regions.regions[num - 1]),
+				test_case->expect_last_size);
+	}
+
+	kunit_kfree(test, phys_contig_mem_regions.regions);
+}
+
 static struct kunit_case ne_misc_dev_test_cases[] = {
+	KUNIT_CASE(ne_misc_dev_test_merge_phys_contig_memory_regions),
 	{}
 };
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 1/4] nitro_enclaves: Merge contiguous physical memory regions
  2021-11-07 14:09 ` [PATCH v5 1/4] nitro_enclaves: " Longpeng(Mike)
@ 2021-11-08 10:23   ` Paraschiv, Andra-Irina
  0 siblings, 0 replies; 7+ messages in thread
From: Paraschiv, Andra-Irina @ 2021-11-08 10:23 UTC (permalink / raw)
  To: Longpeng(Mike)
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, lexnv, alcioa



On 07/11/2021 16:09, Longpeng(Mike) wrote:
> 
> From: Longpeng <longpeng2@huawei.com>
> 
> There can be cases when there are more memory regions that need to be
> set for an enclave than the maximum supported number of memory regions
> per enclave. One example can be when the memory regions are backed by 2
> MiB hugepages (the minimum supported hugepage size).
> 
> Let's merge the adjacent regions if they are physically contiguous. This
> way the final number of memory regions is less than before merging and
> could potentially avoid reaching maximum.
> 
> Signed-off-by: Longpeng <longpeng2@huawei.com>
> ---
> Changes v4 -> v5:
>    - "Physical contiguous memory regions" -> "Contiguous physical memory
>      regions."  [Andra]
>    - fix the warning of aligment that reported by the checkpath.pl  [Andra]
> 
> Changes v3 -> v4:
>    - move "#include <linux/range.h>" according to the alphabetical order. [Andra]
>    - rename several variables, parameters, structures and functions.  [Andra]
>    - add missing "Context" in the comments.  [Andra]
>    - some other changes to makes the code much neater.  [Andra]
> 
> Changes v2 -> v3:
>    - update the commit title and commit message.  [Andra]
>    - use 'struct range' to instead of 'struct phys_mem_region'.  [Andra, Greg KH]
>    - add comments before the function definition.  [Andra]
>    - rename several variables, parameters and function.  [Andra]
> ---
>   drivers/virt/nitro_enclaves/ne_misc_dev.c | 84 ++++++++++++++++++++-----------
>   1 file changed, 56 insertions(+), 28 deletions(-)
> 

Reviewed-by: Andra Paraschiv <andraprs@amazon.com>

Thanks,
Andra

> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> index 8939612..ced58de 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> @@ -24,6 +24,7 @@
>   #include <linux/nitro_enclaves.h>
>   #include <linux/pci.h>
>   #include <linux/poll.h>
> +#include <linux/range.h>
>   #include <linux/slab.h>
>   #include <linux/types.h>
>   #include <uapi/linux/vm_sockets.h>
> @@ -126,6 +127,16 @@ struct ne_cpu_pool {
>   static struct ne_cpu_pool ne_cpu_pool;
> 
>   /**
> + * struct ne_phys_contig_mem_regions - Contiguous physical memory regions.
> + * @num:       The number of regions that currently has.
> + * @regions:   The array of physical memory regions.
> + */
> +struct ne_phys_contig_mem_regions {
> +       unsigned long num;
> +       struct range  *regions;
> +};
> +
> +/**
>    * ne_check_enclaves_created() - Verify if at least one enclave has been created.
>    * @void:      No parameters provided.
>    *
> @@ -825,6 +836,33 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
>   }
> 
>   /**
> + * ne_merge_phys_contig_memory_regions() - Add a memory region and merge the adjacent
> + *                                         regions if they are physically contiguous.
> + * @phys_contig_regions : Private data associated with the contiguous physical memory regions.
> + * @page_paddr :          Physical start address of the region to be added.
> + * @page_size :           Length of the region to be added.
> + *
> + * Context: Process context. This function is called with the ne_enclave mutex held.
> + */
> +static void
> +ne_merge_phys_contig_memory_regions(struct ne_phys_contig_mem_regions *phys_contig_regions,
> +                                   u64 page_paddr, u64 page_size)
> +{
> +       unsigned long num = phys_contig_regions->num;
> +
> +       /* Physically contiguous, just merge */
> +       if (num && (phys_contig_regions->regions[num - 1].end + 1) == page_paddr) {
> +               phys_contig_regions->regions[num - 1].end += page_size;
> +
> +               return;
> +       }
> +
> +       phys_contig_regions->regions[num].start = page_paddr;
> +       phys_contig_regions->regions[num].end = page_paddr + page_size - 1;
> +       phys_contig_regions->num++;
> +}
> +
> +/**
>    * ne_set_user_memory_region_ioctl() - Add user space memory region to the slot
>    *                                    associated with the current enclave.
>    * @ne_enclave :       Private data associated with the current enclave.
> @@ -843,9 +881,8 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>          unsigned long max_nr_pages = 0;
>          unsigned long memory_size = 0;
>          struct ne_mem_region *ne_mem_region = NULL;
> -       unsigned long nr_phys_contig_mem_regions = 0;
>          struct pci_dev *pdev = ne_devs.ne_pci_dev->pdev;
> -       struct page **phys_contig_mem_regions = NULL;
> +       struct ne_phys_contig_mem_regions phys_contig_mem_regions = {};
>          int rc = -EINVAL;
> 
>          rc = ne_sanity_check_user_mem_region(ne_enclave, mem_region);
> @@ -866,9 +903,10 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>                  goto free_mem_region;
>          }
> 
> -       phys_contig_mem_regions = kcalloc(max_nr_pages, sizeof(*phys_contig_mem_regions),
> -                                         GFP_KERNEL);
> -       if (!phys_contig_mem_regions) {
> +       phys_contig_mem_regions.regions = kcalloc(max_nr_pages,
> +                                                 sizeof(*phys_contig_mem_regions.regions),
> +                                                 GFP_KERNEL);
> +       if (!phys_contig_mem_regions.regions) {
>                  rc = -ENOMEM;
> 
>                  goto free_mem_region;
> @@ -901,26 +939,16 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>                  if (rc < 0)
>                          goto put_pages;
> 
> -               /*
> -                * TODO: Update once handled non-contiguous memory regions
> -                * received from user space or contiguous physical memory regions
> -                * larger than 2 MiB e.g. 8 MiB.
> -                */
> -               phys_contig_mem_regions[i] = ne_mem_region->pages[i];
> +               ne_merge_phys_contig_memory_regions(&phys_contig_mem_regions,
> +                                                   page_to_phys(ne_mem_region->pages[i]),
> +                                                   page_size(ne_mem_region->pages[i]));
> 
>                  memory_size += page_size(ne_mem_region->pages[i]);
> 
>                  ne_mem_region->nr_pages++;
>          } while (memory_size < mem_region.memory_size);
> 
> -       /*
> -        * TODO: Update once handled non-contiguous memory regions received
> -        * from user space or contiguous physical memory regions larger than
> -        * 2 MiB e.g. 8 MiB.
> -        */
> -       nr_phys_contig_mem_regions = ne_mem_region->nr_pages;
> -
> -       if ((ne_enclave->nr_mem_regions + nr_phys_contig_mem_regions) >
> +       if ((ne_enclave->nr_mem_regions + phys_contig_mem_regions.num) >
>              ne_enclave->max_mem_regions) {
>                  dev_err_ratelimited(ne_misc_dev.this_device,
>                                      "Reached max memory regions %lld\n",
> @@ -931,9 +959,9 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>                  goto put_pages;
>          }
> 
> -       for (i = 0; i < nr_phys_contig_mem_regions; i++) {
> -               u64 phys_region_addr = page_to_phys(phys_contig_mem_regions[i]);
> -               u64 phys_region_size = page_size(phys_contig_mem_regions[i]);
> +       for (i = 0; i < phys_contig_mem_regions.num; i++) {
> +               u64 phys_region_addr = phys_contig_mem_regions.regions[i].start;
> +               u64 phys_region_size = range_len(&phys_contig_mem_regions.regions[i]);
> 
>                  if (phys_region_size & (NE_MIN_MEM_REGION_SIZE - 1)) {
>                          dev_err_ratelimited(ne_misc_dev.this_device,
> @@ -959,13 +987,13 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
> 
>          list_add(&ne_mem_region->mem_region_list_entry, &ne_enclave->mem_regions_list);
> 
> -       for (i = 0; i < nr_phys_contig_mem_regions; i++) {
> +       for (i = 0; i < phys_contig_mem_regions.num; i++) {
>                  struct ne_pci_dev_cmd_reply cmd_reply = {};
>                  struct slot_add_mem_req slot_add_mem_req = {};
> 
>                  slot_add_mem_req.slot_uid = ne_enclave->slot_uid;
> -               slot_add_mem_req.paddr = page_to_phys(phys_contig_mem_regions[i]);
> -               slot_add_mem_req.size = page_size(phys_contig_mem_regions[i]);
> +               slot_add_mem_req.paddr = phys_contig_mem_regions.regions[i].start;
> +               slot_add_mem_req.size = range_len(&phys_contig_mem_regions.regions[i]);
> 
>                  rc = ne_do_request(pdev, SLOT_ADD_MEM,
>                                     &slot_add_mem_req, sizeof(slot_add_mem_req),
> @@ -974,7 +1002,7 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>                          dev_err_ratelimited(ne_misc_dev.this_device,
>                                              "Error in slot add mem [rc=%d]\n", rc);
> 
> -                       kfree(phys_contig_mem_regions);
> +                       kfree(phys_contig_mem_regions.regions);
> 
>                          /*
>                           * Exit here without put pages as memory regions may
> @@ -987,7 +1015,7 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>                  ne_enclave->nr_mem_regions++;
>          }
> 
> -       kfree(phys_contig_mem_regions);
> +       kfree(phys_contig_mem_regions.regions);
> 
>          return 0;
> 
> @@ -995,7 +1023,7 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
>          for (i = 0; i < ne_mem_region->nr_pages; i++)
>                  put_page(ne_mem_region->pages[i]);
>   free_mem_region:
> -       kfree(phys_contig_mem_regions);
> +       kfree(phys_contig_mem_regions.regions);
>          kfree(ne_mem_region->pages);
>          kfree(ne_mem_region);
> 
> --
> 1.8.3.1
> 



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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] nitro_enclaves: Add KUnit tests for contiguous physical memory regions merging
  2021-11-07 14:09 ` [PATCH v5 4/4] nitro_enclaves: Add KUnit tests for contiguous physical memory regions merging Longpeng(Mike)
@ 2021-11-08 10:25   ` Paraschiv, Andra-Irina
  0 siblings, 0 replies; 7+ messages in thread
From: Paraschiv, Andra-Irina @ 2021-11-08 10:25 UTC (permalink / raw)
  To: Longpeng(Mike)
  Cc: arei.gonglei, gregkh, kamal, pbonzini, sgarzare, stefanha,
	vkuznets, linux-kernel, ne-devel-upstream, lexnv, alcioa



On 07/11/2021 16:09, Longpeng(Mike) wrote:
> 
> From: Longpeng <longpeng2@huawei.com>
> 
> Add KUnit tests for the contiguous physical memory regions merging
> functionality from the Nitro Enclaves misc device logic.
> 
> We can build the test binary with the following configuration:
>    CONFIG_KUNIT=y
>    CONFIG_NITRO_ENCLAVES=m
>    CONFIG_NITRO_ENCLAVES_MISC_DEV_TEST=y
> and install the nitro_enclaves module to run the testcases.
> 
> We'll see the following message using dmesg if everything goes well:
> 
> [...]     # Subtest: ne_misc_dev_test
> [...]     1..1
> [...] (NULL device *): Physical mem region address is not 2 MiB aligned
> [...] (NULL device *): Physical mem region size is not multiple of 2 MiB
> [...] (NULL device *): Physical mem region address is not 2 MiB aligned
> [...]     ok 1 - ne_misc_dev_test_merge_phys_contig_memory_regions
> [...] ok 1 - ne_misc_dev_test
> 
> Signed-off-by: Longpeng <longpeng2@huawei.com>
> ---
> Changes v4 -> v5:
>    - fix the warning of aligment that reported by the checkpath.pl  [Andra]
>    - remove unnecessary comparison of NULL.  [Andra]
> 
> Changes v3 -> v4:
>    - "int expect_num" -> "unsigned long  expect_num"  [Andra]
>    - rename several variables and structures  [Andra]
>    - invoke "kunit_kfree" to free the "regions"  [Andra]
> 
> Changes v2 -> v3:
>    - update the commit title and commit message.  [Andra]
>    - align the fileds in 'struct phys_regions_test'.  [Andra]
>    - rename 'phys_regions_testcases' to 'phys_regions_test_cases'.  [Andra]
>    - add comments before each test cases.  [Andra]
>    - initialize the variables in ne_misc_dev_test_merge_phys_contig_memory_regions.  [Andra]
> ---
>   drivers/virt/nitro_enclaves/ne_misc_dev_test.c | 140 +++++++++++++++++++++++++
>   1 file changed, 140 insertions(+)
> 

Reviewed-by: Andra Paraschiv <andraprs@amazon.com>

Thanks,
Andra

> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
> index 6862e99..265797b 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
> @@ -2,7 +2,147 @@
> 
>   #include <kunit/test.h>
> 
> +#define MAX_PHYS_REGIONS       16
> +#define INVALID_VALUE          (~0ull)
> +
> +struct ne_phys_regions_test {
> +       u64           paddr;
> +       u64           size;
> +       int           expect_rc;
> +       unsigned long expect_num;
> +       u64           expect_last_paddr;
> +       u64           expect_last_size;
> +} phys_regions_test_cases[] = {
> +       /*
> +        * Add the region from 0x1000 to (0x1000 + 0x200000 - 1):
> +        *   Expected result:
> +        *       Failed, start address is not 2M-aligned
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 0
> +        *   regions = {}
> +        */
> +       {0x1000, 0x200000, -EINVAL, 0, INVALID_VALUE, INVALID_VALUE},
> +
> +       /*
> +        * Add the region from 0x200000 to (0x200000 + 0x1000 - 1):
> +        *   Expected result:
> +        *       Failed, size is not 2M-aligned
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 0
> +        *   regions = {}
> +        */
> +       {0x200000, 0x1000, -EINVAL, 0, INVALID_VALUE, INVALID_VALUE},
> +
> +       /*
> +        * Add the region from 0x200000 to (0x200000 + 0x200000 - 1):
> +        *   Expected result:
> +        *       Successful
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 1
> +        *   regions = {
> +        *       {start=0x200000, end=0x3fffff}, // len=0x200000
> +        *   }
> +        */
> +       {0x200000, 0x200000, 0, 1, 0x200000, 0x200000},
> +
> +       /*
> +        * Add the region from 0x0 to (0x0 + 0x200000 - 1):
> +        *   Expected result:
> +        *       Successful
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 2
> +        *   regions = {
> +        *       {start=0x200000, end=0x3fffff}, // len=0x200000
> +        *       {start=0x0,      end=0x1fffff}, // len=0x200000
> +        *   }
> +        */
> +       {0x0, 0x200000, 0, 2, 0x0, 0x200000},
> +
> +       /*
> +        * Add the region from 0x600000 to (0x600000 + 0x400000 - 1):
> +        *   Expected result:
> +        *       Successful
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 3
> +        *   regions = {
> +        *       {start=0x200000, end=0x3fffff}, // len=0x200000
> +        *       {start=0x0,      end=0x1fffff}, // len=0x200000
> +        *       {start=0x600000, end=0x9fffff}, // len=0x400000
> +        *   }
> +        */
> +       {0x600000, 0x400000, 0, 3, 0x600000, 0x400000},
> +
> +       /*
> +        * Add the region from 0xa00000 to (0xa00000 + 0x400000 - 1):
> +        *   Expected result:
> +        *       Successful, merging case!
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 3
> +        *   regions = {
> +        *       {start=0x200000, end=0x3fffff}, // len=0x200000
> +        *       {start=0x0,      end=0x1fffff}, // len=0x200000
> +        *       {start=0x600000, end=0xdfffff}, // len=0x800000
> +        *   }
> +        */
> +       {0xa00000, 0x400000, 0, 3, 0x600000, 0x800000},
> +
> +       /*
> +        * Add the region from 0x1000 to (0x1000 + 0x200000 - 1):
> +        *   Expected result:
> +        *       Failed, start address is not 2M-aligned
> +        *
> +        * Now the instance of struct ne_phys_contig_mem_regions is:
> +        *   num = 3
> +        *   regions = {
> +        *       {start=0x200000, end=0x3fffff}, // len=0x200000
> +        *       {start=0x0,      end=0x1fffff}, // len=0x200000
> +        *       {start=0x600000, end=0xdfffff}, // len=0x800000
> +        *   }
> +        */
> +       {0x1000, 0x200000, -EINVAL, 3, 0x600000, 0x800000},
> +};
> +
> +static void ne_misc_dev_test_merge_phys_contig_memory_regions(struct kunit *test)
> +{
> +       struct ne_phys_contig_mem_regions phys_contig_mem_regions = {};
> +       int rc = 0;
> +       int i = 0;
> +
> +       phys_contig_mem_regions.regions = kunit_kcalloc(test, MAX_PHYS_REGIONS,
> +                                                       sizeof(*phys_contig_mem_regions.regions),
> +                                                       GFP_KERNEL);
> +       KUNIT_ASSERT_TRUE(test, phys_contig_mem_regions.regions);
> +
> +       for (i = 0; i < ARRAY_SIZE(phys_regions_test_cases); i++) {
> +               struct ne_phys_regions_test *test_case = &phys_regions_test_cases[i];
> +               unsigned long num = 0;
> +
> +               rc = ne_merge_phys_contig_memory_regions(&phys_contig_mem_regions,
> +                                                        test_case->paddr, test_case->size);
> +               KUNIT_EXPECT_EQ(test, rc, test_case->expect_rc);
> +               KUNIT_EXPECT_EQ(test, phys_contig_mem_regions.num, test_case->expect_num);
> +
> +               if (test_case->expect_last_paddr == INVALID_VALUE)
> +                       continue;
> +
> +               num = phys_contig_mem_regions.num;
> +               KUNIT_EXPECT_EQ(test, phys_contig_mem_regions.regions[num - 1].start,
> +                               test_case->expect_last_paddr);
> +               KUNIT_EXPECT_EQ(test, range_len(&phys_contig_mem_regions.regions[num - 1]),
> +                               test_case->expect_last_size);
> +       }
> +
> +       kunit_kfree(test, phys_contig_mem_regions.regions);
> +}
> +
>   static struct kunit_case ne_misc_dev_test_cases[] = {
> +       KUNIT_CASE(ne_misc_dev_test_merge_phys_contig_memory_regions),
>          {}
>   };
> 
> --
> 1.8.3.1
> 



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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-08 10:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-07 14:09 [PATCH v5 0/4] Merge contiguous physical memory regions Longpeng(Mike)
2021-11-07 14:09 ` [PATCH v5 1/4] nitro_enclaves: " Longpeng(Mike)
2021-11-08 10:23   ` Paraschiv, Andra-Irina
2021-11-07 14:09 ` [PATCH v5 2/4] nitro_enclaves: Sanity check physical memory regions during merging Longpeng(Mike)
2021-11-07 14:09 ` [PATCH v5 3/4] nitro_enclaves: Add KUnit tests setup for the misc device functionality Longpeng(Mike)
2021-11-07 14:09 ` [PATCH v5 4/4] nitro_enclaves: Add KUnit tests for contiguous physical memory regions merging Longpeng(Mike)
2021-11-08 10:25   ` Paraschiv, Andra-Irina

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).