On Wed, Oct 6, 2021 at 9:11 PM Julien Grall <[email protected]> wrote: > Hi Oleksandr, >
Hi Julien [Sorry for the possible format issues] > > 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. > Already submitted, please take a look at [1]. [1] https://lore.kernel.org/xen-devel/[email protected]/ -- Regards, Oleksandr Tyshchenko
