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

[Julien Grall <julien@xen.org>]

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.

> 
> A side note: we would likely need the toolstack (that generates 
> device-tree for guests) to query IPA size, or similar.

I think we can use XEN_SYSCTL_* (or possibly hypfs) for that.

>>
>>
>> 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.

Cheers,

-- 
Julien Grall