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.zh...@arm.com/ > [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
