On 14/06/2024 08:54, Luca Fancellu wrote:
Hi Julien,
Hi Luca,
On 13 Jun 2024, at 12:31, Julien Grall <jul...@xen.org> wrote:
Hi,
On 11/06/2024 13:42, Michal Orzel wrote:
We would like this serie to be in Xen 4.19, there was a misunderstanding on our
side because we thought
that since the serie was sent before the last posting date, it could have been
a candidate for merging in the
new release, instead after speaking with Julien and Oleksii we are now aware
that we need to provide a
justification for its presence.
Pros to this serie is that we are closing the circle for static shared memory,
allowing it to use memory from
the host or from Xen, it is also a feature that is not enabled by default, so
it should not cause too much
disruption in case of any bugs that escaped the review, however we’ve tested
many configurations for that
with/without enabling the feature if that can be an additional value.
Cons: we are touching some common code related to p2m, but also there the
impact should be minimal because
the new code is subject to l2 foreign mapping (to be confirmed maybe from a p2m
expert like Julien).
The comments on patch 3 of this serie are addressed by this patch:
https://patchwork.kernel.org/project/xen-devel/patch/20240528125603.2467640-1-luca.fance...@arm.com/
And the serie is fully reviewed.
So our request is to allow this serie in 4.19, Oleksii, ARM maintainers, do you
agree on that?
As a main reviewer of this series I'm ok to have this series in. It is nicely
encapsulated and the feature itself
is still in unsupported state. I don't foresee any issues with it.
There are changes in the p2m code and the memory allocation for boot domains.
So is it really encapsulated?
For me there are two risks:
* p2m (already mentioned by Luca): We modify the code to put foreign mapping.
The worse that can happen if we don't release the foreign mapping. This would
mean the page will not be freed. AFAIK, we don't exercise this path in the CI.
* domain allocation: This mainly look like refactoring. And the path is
exercised in the CI.
So I am not concerned with the domain allocation one. @Luca, would it be
possible to detail how did you test the foreign pages were properly removed?
So at first we tested the code, with/without the static shared memory feature
enabled, creating/destroying guest from Dom0 and checking that everything
was ok.
Just to details a bit more for the other. In a normal setup, guest would
be using PV block/network which are based on grant table. To exercise
the foreing mapping path, you would need a backend that map using the
GFN directly.
This is the case for virtio MMIO. Other users are IIRC XenFB, IOREQ (if
you emulate a device).
After a chat on Matrix with Julien he suggested that using a virtio-mmio disk
was better to stress out the foreign mapping looking for
regressions.
Luckily I’ve found this slide deck from @Oleksandr:
https://static.linaro.org/connect/lvc21/presentations/lvc21-314.pdf
So I did a setup using fvp-base, having a disk with two partitions containing
Dom0 rootfs and DomU rootfs, Dom0 sees
this disk using VirtIO block.
Thanks for the testing! I am satisfied with the result. I have committed
the series.
Cheers,
--
Julien Grall
