Hi Stefano,
On 23/11/2020 22:27, Stefano Stabellini wrote:
On Fri, 20 Nov 2020, Julien Grall wrote:
/*
* For arm32, page-tables are different on each CPUs. Yet, they
share
@@ -1265,14 +1287,43 @@ static int xen_pt_update(unsigned long virt,
spin_lock(&xen_pt_lock);
- for ( ; addr < addr_end; addr += PAGE_SIZE )
+ while ( left )
{
- rc = xen_pt_update_entry(root, addr, mfn, flags);
+ unsigned int order;
+ unsigned long mask;
+
+ /*
+ * Don't take into account the MFN when removing mapping (i.e
+ * MFN_INVALID) to calculate the correct target order.
+ *
+ * XXX: Support superpage mappings if nr is not aligned to a
+ * superpage size.
It would be good to add another sentence to explain that the checks
below are simply based on masks and rely on the mfn, vfn, and also
nr_mfn to be superpage aligned. (It took me some time to figure it out.)
I am not sure to understand what you wrote here. Could you suggest a sentence?
Something like the following:
/*
* Don't take into account the MFN when removing mapping (i.e
* MFN_INVALID) to calculate the correct target order.
*
* This loop relies on mfn, vfn, and nr_mfn, to be all superpage
* aligned, and it uses `mask' to check for that.
Unfortunately, I am still not sure to understand this comment.
The loop can deal with any (super)page size (4KB, 2MB, 1GB). There are
no assumption on any alignment for mfn, vfn and nr_mfn.
By OR-ing the 3 components together, we can use it to find out the
maximum size that can be used for the mapping.
So can you clarify what you mean?
*
* XXX: Support superpage mappings if nr_mfn is not aligned to a
* superpage size.
*/
Regarding the TODO itself, we have the exact same one in the P2M code. I
couldn't find a clever way to deal with it yet. Any idea how this could be
solved?
I was thinking of a loop that start with the highest possible superpage
size that virt and mfn are aligned to, and also smaller or equal to
nr_mfn. So rather than using the mask to also make sure nr_mfns is
aligned, I would only use the mask to check that mfn and virt are
aligned. Then, we only need to check that superpage_size <= left.
Concrete example: virt and mfn are 2MB aligned, nr_mfn is 5MB / 1280 4K
pages. We allocate 2MB superpages until onlt 1MB is left. At that point
superpage_size <= left fails and we go down to 4K allocations.
Would that work?
Unfortunately no, AFAICT, your assumption is that vfn/mfn are originally
aligned to higest possible superpage size. There are situation where
this is not the case.
To give a concrete example, at the moment the RAM is mapped using 1GB
superpage in Xen. But in the future, we will only want to map RAM
regions in the directmap that haven't been marked as reserved [1].
Those reserved regions don't have architectural alignment or placement.
I will use an over-exegerated example (or maybe not :)).
Imagine you have 4GB of RAM starting at 0. The HW/Software engineer
decided to place a 2MB reserved region start at 512MB.
As a result we would want to map two RAM regions:
1) 0 to 512MB
2) 514MB to 4GB
I will only focus on 2). In the ideal situation, we would want to map
a) 514MB to 1GB using 2MB superpage
b) 1GB to 4GB using 1GB superpage
We don't want be to use 2MB superpage because this will increase TLB
pressure (we want to avoid Xen using too much TLB entries) and also
increase the size of the page-tables.
Therefore, we want to select the best size for each iteration. For now,
the only solution I can come up with is to OR vfn/mfn and then use a
series of check to compare the mask and nr_mfn.
In addition to the "classic" mappings (i.e. 4KB, 2MB, 1GB). I would like
to explore contiguous mapping (e.g. 64KB, 32MB) to further reduce the
TLBs pressure. Note that a processor may or may not take advantage of
contiguous mapping to reduce the number of TLBs used.
This will unfortunately increase the numbers of check. I will try to
come up with a patch and we can discuss from there.
Cheers,
[1] Reserved region may be marked as uncacheable and therefore we
shouldn't map them in Xen address space to avoid break cache coherency.
--
Julien Grall
