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?
Cheers,
--
Julien Grall
