Re: [PATCH v3 0/8] Follow-up static shared memory PART I

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Penny,

On 21/08/2023 06:00, 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
> - 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@xxxxxxx/
> [2] 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/reserved-memory/xen%2Cshared-memory.txt

It looks like there is a problem with the changes introduced in this series.
The gitlab static shared memory tests failed:
https://gitlab.com/xen-project/patchew/xen/-/pipelines/973985190
No Xen logs meaning the failure occurred before serial console initialization.

Now, I would like to share some observations after playing around with the 
current static shared mem code today.
1) Static shared memory region is advertised to a domain by creating a child 
node under reserved-memory.
/reserved-memory is nothing but a way to carve out a region from the normal 
memory specified in /memory node.
For me, such regions should be described in domain's /memory node as well. This 
is not the case at the moment
for static shm unlike to other sub-nodes of /reserved-memory (present in host 
dtb) for which Xen creates separate
/memory nodes.

2) Domain dtb parsing issue with two /reserved-memory nodes present.
In case there is a /reserved-memory node already present in the host dtb, Xen 
would create yet another /reserved-memory
node for the static shm (to be observed in case of dom0). This is a bug as 
there can be only one /reserved-memory node.
This leads to an error when dumping with dtc and leads to a shm node not being 
visible to a domain (guest OS relies on
a presence of a single /reserved-memory node). The issue is because in 
make_resv_memory_node(), you are not checking if
such node already exists.

I haven't looked closely at this series yet. It might be that these issues are 
fixed. If not, I would definitely
suggest to fix them in the first place.

~Michal