Re: [RFC PATCH] xen/memory: Introduce a hypercall to provide unallocated space

[Oleksandr]

Hi Stefano


On 06.08.21 03:20, Stefano Stabellini wrote:
On Thu, 5 Aug 2021, Julien Grall wrote:
Hi Oleksandr,

On 05/08/2021 15:52, Oleksandr wrote:
On 05.08.21 01:00, Julien Grall wrote:

On 04/08/2021 21:56, Oleksandr wrote:
Hi Julien, Stefano.
Hi Oleksandr,

Hi, Julien


Thank you for the prompt reply and explanations.


On 02.08.21 22:12, Oleksandr wrote:
I have done some experiments with Xen and toolstack according to the
discussion above. So, I re-used DTB to pass a safe range to the domain.
For the range I borrowed some space from the second RAM bank.

-#define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 1016GB of RAM
@ 8GB */
-#define GUEST_RAM1_SIZE   xen_mk_ullong(0xfe00000000)
+#define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 888GB of RAM @
8GB */
+#define GUEST_RAM1_SIZE   xen_mk_ullong(0xDE00000000)
+
I am a bit split with reducing the amount of RAM. On one hand large guest
is not unheard on the server side (at least in the x86 world). On the
other hand, I am not aware of anyone using Xen on Arm in such setup.

So technically this will be a regression, but it may be OK.
I got it.



Regarding the range, this will be a problem as Xen configure the number of
the IPA bits based on the PA bits. The lowest possible address space ize
on 64-bit is 4GB.

 From my understanding, this is because the number of IPA bits supported is
contrained by the PA bits. So the position and the size of the region
would need to depend on the P2M configuration.
Indeed, I missed these bits that IPA bits on Arm64 might be < 40 bit, I
remember, we select p2m_ipa_bits in setup_virt_paging() depending on pabits,
moreover the p2m_ipa_bits might be even restricted by some external entity
(IOMMU, if P2M is shared).


For simplicity, this could be the last few X bytes of the supported
address space.
ok, agree. To summarize, so it sounds like we can't use the fixed safe range
as in my example, it must be variable. Well, I hope, we will be able to
achieve this without reducing the total amount of domain RAM in front
(GUEST_RAM1_SIZE). After all, we know the IPA size and the domain RAM in
advance, so we certainly can choose the start and size of the range. In
case, we won't be able to find a suitable large chunk (for example, when IPA
bits = 32, and domain has a lot of RAM assigned and as the result - almost
all address space below 4GB is in use), we won't expose a safe range to a
domain at all, and domain will just fall back to use real pages instead
(actually, how it currently behaves on Arm).
I think it would be fine for a first approach. We can refine it in the future.
What matters is that we correctly define the binding/hypercall.
I agree with Julien on both points. Looking at the existing device tree
binding, it is almost exactly what we need, so I think it would be OK to
use it.

ok, great.




For 32-bit domain, we also need to make sure the address is usable for
domain short page tables (not too long ago Debian was shipping the kernel
with them rather than LPAE). I haven't yet checked what's the limit here.
Hmm, I didn't take this use-case into the account. So, I assume we need the
safe range to be located below 4GB if is_32bit_domain() returns true.
Yes. Or we can say that if you are using a 32-bit domain then we don't (yet)
support a safe range for range.
So we would need some heuristic to decide whether to stole some RAM or use
the safe space.
Another possibility would be to add a new compatible in the DT that
indicates the region is "big" enough.
I like the last idea, did you perhaps mean new property (optional) rather
than new compatible? Let's say "xen, safe-range" or "xen, extended-regions"
...
I actually meant adding an extra compatible because this is technically a
change of the binding (even though it is backward compatible).

Although, I would be OK with the property. You may first want to ask the
Device-Tree folks how they expect a binding to be extended in a backward
compatible way.
I think we should expand the description in
Documentation/devicetree/bindings/arm/xen.txt to cover things other than
the grant table.

yes



I don't think we necessarely need a new compatible string because as you
said the change is fully compatible. At the same time it is trivial to
add compatible strings, so that could be done as well.

ok, please note, I have already asked DT folks for the clarification 
(how to make it properly), so let's wait for input.


--
Regards,

Oleksandr Tyshchenko