When host address is not provided in "xen,shared-mem", we let Xen
automatically allocate requested static shared memory from heap, and it
stands good chance of having multiple host memory banks allocated for the
requested static shared memory as a result. Therefore current membank is not
going to cover it.

This commit introduces a new field "mem" to cover both scenarios.
"struct membank" is used when host address is provided, whereas
"struct meminfo" shall be used when host address not provided.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
 xen/arch/arm/bootfdt.c           |  6 +++---
 xen/arch/arm/domain_build.c      | 16 ++++++++--------
 xen/arch/arm/include/asm/setup.h | 17 ++++++++++++++++-
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index ccf281cd37..2f34a8ea83 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -458,8 +458,8 @@ static int __init process_shm_node(const void *fdt, int node,
 
     for ( i = 0; i < shm_mem->nr_banks; i++ )
     {
-        paddr_t bank_start = shm_mem->bank[i].membank->start;
-        paddr_t bank_size = shm_mem->bank[i].membank->size;
+        paddr_t bank_start = shm_mem->bank[i].mem.bank->start;
+        paddr_t bank_size = shm_mem->bank[i].mem.bank->size;
 
         /*
          * Meet the following check:
@@ -523,7 +523,7 @@ static int __init process_shm_node(const void *fdt, int node,
             mem->nr_banks++;
 
             safe_strcpy(shm_mem->bank[i].shm_id, shm_id);
-            shm_mem->bank[i].membank = membank;
+            shm_mem->bank[i].mem.bank = membank;
             shm_mem->nr_banks++;
         }
         else
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 92763e96fc..fbb196d8a4 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -818,7 +818,7 @@ static bool __init is_shm_allocated_to_domio(struct shm_membank *shm_membank)
     struct page_info *page;
     struct domain *d;
 
-    page = maddr_to_page(shm_membank->membank->start);
+    page = maddr_to_page(shm_membank->mem.bank->start);
     d = page_get_owner_and_reference(page);
     if ( d == NULL )
         return false;
@@ -878,8 +878,8 @@ static int __init assign_shared_memory(struct domain *d,
     struct page_info *page;
     paddr_t pbase, psize;
 
-    pbase = shm_membank->membank->start;
-    psize = shm_membank->membank->size;
+    pbase = shm_membank->mem.bank->start;
+    psize = shm_membank->mem.bank->size;
 
     printk("%pd: allocate static shared memory BANK %#"PRIpaddr"-%#"PRIpaddr".\n",
            d, pbase, pbase + psize);
@@ -951,9 +951,9 @@ static int __init append_shm_bank_to_domain(struct kernel_info *kinfo,
     if ( membank == NULL )
         return -ENOMEM;
 
-    kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].membank = membank;
-    kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].membank->start = start;
-    kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].membank->size = size;
+    kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].mem.bank = membank;
+    kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].mem.bank->start = start;
+    kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].mem.bank->size = size;
     safe_strcpy(kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].shm_id, shm_id);
     kinfo->shm_mem.nr_banks++;
 
@@ -1422,8 +1422,8 @@ static int __init make_shm_memory_node(const struct domain *d,
 
     for ( ; i < mem->nr_banks; i++ )
     {
-        uint64_t start = mem->bank[i].membank->start;
-        uint64_t size = mem->bank[i].membank->size;
+        uint64_t start = mem->bank[i].mem.bank->start;
+        uint64_t size = mem->bank[i].mem.bank->size;
         /* Placeholder for xen-shmem@ + a 64-bit number + \0 */
         char buf[27];
         const char compat[] = "xen,shared-memory-v1";
diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
index 2d4ae0f00a..c54ffc8a5b 100644
--- a/xen/arch/arm/include/asm/setup.h
+++ b/xen/arch/arm/include/asm/setup.h
@@ -60,7 +60,22 @@ struct meminfo {
 struct shm_membank {
     char shm_id[MAX_SHM_ID_LENGTH];
     unsigned int nr_shm_borrowers;
-    struct membank *membank;
+    struct {
+        /*
+         * When host address is provided in "xen,shared-mem", then only one
+         * consistent host memory bank is behind each shared memory node.
+         */
+        struct membank *bank;
+        struct {
+            /*
+             * When host address is not provided in "xen,shared-mem", then
+             * we let Xen allocate requested memory from heap, and a shared
+             * memory bank could be consisted of multiple host memory banks.
+             */
+            struct meminfo *meminfo;
+            unsigned long total_size;
+        } banks;
+    } mem;
 };
 
 struct shm_meminfo {
-- 
2.25.1

Hi Penny,

On 15/11/2022 02:52, Penny Zheng wrote:
> When host address is not provided in "xen,shared-mem", we let Xen
> automatically allocate requested static shared memory from heap, and it
> stands good chance of having multiple host memory banks allocated for the
> requested static shared memory as a result. Therefore current membank is not
> going to cover it.
> 
> This commit introduces a new field "mem" to cover both scenarios.
> "struct membank" is used when host address is provided, whereas
> "struct meminfo" shall be used when host address not provided.

 From this patch, it is not clear to me how a user can know which part 
of the union should be used.

However... I am not entirely sure why you need to create a union because 
in your new structure you can fit one bank.

Cheers,

-- 
Julien Grall

On 08/01/2023 12:13, Julien Grall wrote:
> Hi Penny,
> 
> On 15/11/2022 02:52, Penny Zheng wrote:
>> When host address is not provided in "xen,shared-mem", we let Xen
>> automatically allocate requested static shared memory from heap, and it
>> stands good chance of having multiple host memory banks allocated for the
>> requested static shared memory as a result. Therefore current membank 
>> is not
>> going to cover it.
>>
>> This commit introduces a new field "mem" to cover both scenarios.
>> "struct membank" is used when host address is provided, whereas
>> "struct meminfo" shall be used when host address not provided.
> 
>  From this patch, it is not clear to me how a user can know which part 
> of the union should be used.

Ah it is a struct rather than an union. Yet...

> 
> However... I am not entirely sure why you need to create a union because 
> in your new structure you can fit one bank.

... my point here stands.

Cheers,

-- 
Julien Grall