[v4] Follow-up static shared memory PART I

[PATCH v4 00/10] Follow-up static shared memory PART I

Posted by Penny Zheng 7 months, 3 weeks ago

There are some unsolving issues on current 4.17 static shared memory
feature[1], including:
- In order to avoid keeping growing 'membank', having the shared memory
info in separate structures is preferred.
- Missing implementation on having the host address optional in
"xen,shared-mem" property
- Removing static shared memory from extended regions
- Missing reference release on foreign superpage
- Fix duplicated /reserved-memory node on Dom0
- Missing static shm node declaration on guest /memory node
- Missing "xen,offset" feature, which is introduced in Linux DOC[2]

All above objects have been divided into two parts to complete. And this
patch serie is PART I.

[1] https://lore.kernel.org/all/20220908135513.1800511-1-Penny.Zheng@arm.com/
[2] https://www.kernel.org/doc/Documentation/devicetree/bindings/reserved-memory/xen%2Cshared-memory.txt

Penny Zheng (10):
  xen/arm: remove stale addr_cells/size_cells in assign_shared_memory
  xen/arm: re-define a set of data structures for static shared memory
    region
  xen/arm: introduce allocate_domheap_memory and guest_physmap_memory
  xen/arm: use paddr_assigned to indicate whether host address is
    provided
  xen/arm: support static shared memory when host address not provided
  xen/arm: remove shm holes for extended regions
  xen/p2m: put reference for superpage
  xen/docs: refine docs about static shared memory
  xen/arm: fix duplicate /reserved-memory node in Dom0
  xen/arm: reate another /memory node for static shm

 docs/misc/arm/device-tree/booting.txt |  52 +-
 xen/arch/arm/bootfdt.c                | 125 ++++-
 xen/arch/arm/domain_build.c           | 780 ++++++++++++++++++++------
 xen/arch/arm/include/asm/kernel.h     |  11 +-
 xen/arch/arm/include/asm/setup.h      |  57 +-
 xen/arch/arm/p2m.c                    |  58 +-
 6 files changed, 856 insertions(+), 227 deletions(-)

-- 
2.25.1

Re: [PATCH v4 00/10] Follow-up static shared memory PART I

Posted by Michal Orzel 7 months, 3 weeks ago

Hi Penny,

On 11/09/2023 06:04, Penny Zheng wrote:
> 
> 
> There are some unsolving issues on current 4.17 static shared memory
> feature[1], including:
> - In order to avoid keeping growing 'membank', having the shared memory
> info in separate structures is preferred.
> - Missing implementation on having the host address optional in
> "xen,shared-mem" property
> - Removing static shared memory from extended regions
> - Missing reference release on foreign superpage
> - Fix duplicated /reserved-memory node on Dom0
> - Missing static shm node declaration on guest /memory node
> - Missing "xen,offset" feature, which is introduced in Linux DOC[2]
> 
> All above objects have been divided into two parts to complete. And this
> patch serie is PART I.

Just like I pointed out in the previous revision, there is a gitlab CI failure on shared-memory jobs:
https://gitlab.com/xen-project/patchew/xen/-/pipelines/999098293
Did you change the interface that could lead to this (I cannot spot any change in the docs refinment) ?
No Xen logs meaning the early boot failure. Please check.

~Michal

Re: [PATCH v4 00/10] Follow-up static shared memory PART I

Posted by Penny Zheng 7 months, 3 weeks ago

Hi Michal

On 2023/9/11 17:01, Michal Orzel wrote:
> Hi Penny,
> 
> On 11/09/2023 06:04, Penny Zheng wrote:
>>
>>
>> There are some unsolving issues on current 4.17 static shared memory
>> feature[1], including:
>> - In order to avoid keeping growing 'membank', having the shared memory
>> info in separate structures is preferred.
>> - Missing implementation on having the host address optional in
>> "xen,shared-mem" property
>> - Removing static shared memory from extended regions
>> - Missing reference release on foreign superpage
>> - Fix duplicated /reserved-memory node on Dom0
>> - Missing static shm node declaration on guest /memory node
>> - Missing "xen,offset" feature, which is introduced in Linux DOC[2]
>>
>> All above objects have been divided into two parts to complete. And this
>> patch serie is PART I.
> 
> Just like I pointed out in the previous revision, there is a gitlab CI failure on shared-memory jobs:
> https://gitlab.com/xen-project/patchew/xen/-/pipelines/999098293
> Did you change the interface that could lead to this (I cannot spot any change in the docs refinment) ?
> No Xen logs meaning the early boot failure. Please check.
> 

Soooooo sorry. I miss-looked that comment. I found that bug exists in 
bootfdt.c.
```
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 7d86dffd45..290dd27bf4 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -532,6 +532,8 @@ static int __init process_shm_node(const void *fdt, 
int node,
                             size, tot_size);
                      return -EINVAL;
                  }
+
+                break;
              }
              else if ( paddr_assigned )
              {
```
I accidentally delete a `break;` here, and I will also comment in the 
related commit and fix in next version!

> ~Michal

Re: [PATCH v4 00/10] Follow-up static shared memory PART I

Posted by Michal Orzel 5 months ago

Hi Penny,

On 11/09/2023 12:04, Penny Zheng wrote:
> 
> 
> Hi Michal
> 
> On 2023/9/11 17:01, Michal Orzel wrote:
>> Hi Penny,
>>
>> On 11/09/2023 06:04, Penny Zheng wrote:
>>>
>>>
>>> There are some unsolving issues on current 4.17 static shared memory
>>> feature[1], including:
>>> - In order to avoid keeping growing 'membank', having the shared memory
>>> info in separate structures is preferred.
>>> - Missing implementation on having the host address optional in
>>> "xen,shared-mem" property
>>> - Removing static shared memory from extended regions
>>> - Missing reference release on foreign superpage
>>> - Fix duplicated /reserved-memory node on Dom0
>>> - Missing static shm node declaration on guest /memory node
>>> - Missing "xen,offset" feature, which is introduced in Linux DOC[2]
>>>
>>> All above objects have been divided into two parts to complete. And this
>>> patch serie is PART I.
>>
>> Just like I pointed out in the previous revision, there is a gitlab CI failure on shared-memory jobs:
>> https://gitlab.com/xen-project/patchew/xen/-/pipelines/999098293
>> Did you change the interface that could lead to this (I cannot spot any change in the docs refinment) ?
>> No Xen logs meaning the early boot failure. Please check.
>>
> 
> Soooooo sorry. I miss-looked that comment. I found that bug exists in
> bootfdt.c.
> ```
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 7d86dffd45..290dd27bf4 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -532,6 +532,8 @@ static int __init process_shm_node(const void *fdt,
> int node,
>                              size, tot_size);
>                       return -EINVAL;
>                   }
> +
> +                break;
>               }
>               else if ( paddr_assigned )
>               {
> ```
> I accidentally delete a `break;` here, and I will also comment in the
> related commit and fix in next version!
> 
While searching for the pending series, I noticed this one.
If you have time and want us to review the series, please send an updated version
based on the recent Luca's dom0less features modularization.

~Michal

Re: [PATCH v4 00/10] Follow-up static shared memory PART I

Posted by Penny Zheng 5 months ago

Thanks for the reminder!
I’ll send the updated version ASAP :)

Thanks,
Penny

> 在 2023年11月30日，18:09，Michal Orzel <michal.orzel@amd.com> 写道：
> 
> Hi Penny,
> 
>> On 11/09/2023 12:04, Penny Zheng wrote:
>> 
>> 
>> Hi Michal
>> 
>>> On 2023/9/11 17:01, Michal Orzel wrote:
>>> Hi Penny,
>>> 
>>> On 11/09/2023 06:04, Penny Zheng wrote:
>>>> 
>>>> 
>>>> There are some unsolving issues on current 4.17 static shared memory
>>>> feature[1], including:
>>>> - In order to avoid keeping growing 'membank', having the shared memory
>>>> info in separate structures is preferred.
>>>> - Missing implementation on having the host address optional in
>>>> "xen,shared-mem" property
>>>> - Removing static shared memory from extended regions
>>>> - Missing reference release on foreign superpage
>>>> - Fix duplicated /reserved-memory node on Dom0
>>>> - Missing static shm node declaration on guest /memory node
>>>> - Missing "xen,offset" feature, which is introduced in Linux DOC[2]
>>>> 
>>>> All above objects have been divided into two parts to complete. And this
>>>> patch serie is PART I.
>>> 
>>> Just like I pointed out in the previous revision, there is a gitlab CI failure on shared-memory jobs:
>>> https://gitlab.com/xen-project/patchew/xen/-/pipelines/999098293
>>> Did you change the interface that could lead to this (I cannot spot any change in the docs refinment) ?
>>> No Xen logs meaning the early boot failure. Please check.
>>> 
>> 
>> Soooooo sorry. I miss-looked that comment. I found that bug exists in
>> bootfdt.c.
>> ```
>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
>> index 7d86dffd45..290dd27bf4 100644
>> --- a/xen/arch/arm/bootfdt.c
>> +++ b/xen/arch/arm/bootfdt.c
>> @@ -532,6 +532,8 @@ static int __init process_shm_node(const void *fdt,
>> int node,
>>                             size, tot_size);
>>                      return -EINVAL;
>>                  }
>> +
>> +                break;
>>              }
>>              else if ( paddr_assigned )
>>              {
>> ```
>> I accidentally delete a `break;` here, and I will also comment in the
>> related commit and fix in next version!
>> 
> While searching for the pending series, I noticed this one.
> If you have time and want us to review the series, please send an updated version
> based on the recent Luca's dom0less features modularization.
> 
> ~Michal