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

[Oleksandr <olekstysh@gmail.com>]

On 05.08.21 20:25, Julien Grall wrote:
> Hi Oleksandr,


Hi Julien, all


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

The RFC patch (which is one of the prereq patches for safe range) is 
already pushed:
https://lore.kernel.org/xen-devel/1628897304-20793-1-git-send-email-olekstysh@gmail.com/
What we need is to reach an agreement regarding the interface. We can 
discuss it there.


Thanks!


>
>>>
>>>
>>> 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,
>
-- 
Regards,

Oleksandr Tyshchenko