Hi Oleksandr,
On 04/10/2021 14:08, Oleksandr wrote:
On 04.10.21 09:59, Julien Grall wrote:
Hi Oleksandr,
Hi Julien
Hi Oleksandr,
I saw Stefano committed this patch on Friday. However, I didn't have a
chance go to through a second time and would like to request some
follow-up changes.
ok, do you prefer the follow-up patch to be pushed separately or within
the rest patches of this series (#1 and #3)? If the former, I will try
to push it today to close this question.
I don't mind. My main ask is they are addressed for 4.16.
On 30/09/2021 00:52, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <[email protected]>
The extended region (safe range) is a region of guest physical
address space which is unused and could be safely used to create
grant/foreign mappings instead of wasting real RAM pages from
the domain memory for establishing these mappings.
The extended regions are chosen at the domain creation time and
advertised to it via "reg" property under hypervisor node in
the guest device-tree. As region 0 is reserved for grant table
space (always present), the indexes for extended regions are 1...N.
If extended regions could not be allocated for some reason,
Xen doesn't fail and behaves as usual, so only inserts region 0.
Please note the following limitations:
- The extended region feature is only supported for 64-bit domain
currently.
- The ACPI case is not covered.
***
As Dom0 is direct mapped domain on Arm (e.g. MFN == GFN)
the algorithm to choose extended regions for it is different
in comparison with the algorithm for non-direct mapped DomU.
What is more, that extended regions should be chosen differently
whether IOMMU is enabled or not.
Provide RAM not assigned to Dom0 if IOMMU is disabled or memory
holes found in host device-tree if otherwise. Make sure that
extended regions are 2MB-aligned and located within maximum possible
addressable physical memory range. The minimum size of extended
region is 64MB.
You explained below why the 128 limits, but I don't see any
explanation on why 2MB and 64MB.
IIRC, 2MB was to potentally allow superpage mapping. I am not entirely
sure for 64MB.
Could you add an in-code comment explaining the two limits?
Yes. There was a discussion at [1]. 64MB was chosen as a reasonable
value to deal with between initial 2MB (we might end up having a lot of
small ranges which are not quite useful but increase bookkeeping) and
suggested 1GB (we might not be able find a suitable regions at all).
Ok. Please document in the code. Note that I don't think this choice
should be advertised to the OS. This would give us some flexibility to
change the size in the future (e.g. if we have platform where chunk of
less than 64MB is beneficial).
The code below looks like an open-coding version of
dt_for_each_range(). Can you try to re-use it please? This will help
to reduce the complexity of this function.
You are right, it makes sense, will definitely reuse. If I was aware of
that function before I would safe some time I spent on the investigation
how to parse that)
I have only skimmed through the diff below. This looks fine to me.
Please submit a formal patch.
Cheers,
--
Julien Grall
