Re: [PATCH v2 1/7] xen/mm: lift 32 item limit from mfn/gfn arrays

[Michał Leszczyński <michal.leszczynski@xxxxxxx>]

----- 19 cze 2020 o 14:39, Jan Beulich jbeulich@xxxxxxxx napisał(a):

> On 19.06.2020 14:35, Michał Leszczyński wrote:
>> ----- 19 cze 2020 o 13:34, Roger Pau Monné roger.pau@xxxxxxxxxx napisał(a):
>> 
>>> On Fri, Jun 19, 2020 at 01:38:00AM +0200, Michał Leszczyński wrote:
>>>> Replace on-stack array allocation with heap allocation
>>>> in order to lift the limit of 32 items in mfn/gfn arrays
>>>> when calling acquire_resource.
>>>
>>> I'm afraid this is not correct, you cannot allow unbounded amounts of
>>> items to be processed like this, it's likely that you manage to
>>> trigger the watchdog if the list is long enough, specially when doing
>>> set_foreign_p2m_entry.
>>>
>>> You need to process the items in batches (32 was IMO a good start), and
>>> then add support for hypercall continuations. Take a look at how
>>> XENMEM_populate_physmap just a couple of lines below makes use of
>>> hypercall_create_continuation.
>>>
>>> After processing every batch you need to check if
>>> hypercall_preempt_check returns true and if so use
>>> hypercall_create_continuation in order to encode a continuation.
>> 
>> One more question. Are these continuations transparent from the caller side,
>> or do I also need to add something on the invoker side to properly handle 
>> these
>> continuations?
> 
> They are (mostly) transparent to the guest, yes. "Mostly" because we
> have cases (iirc) where the continuation data is stored in a way that
> a guest could observe it. But it still wouldn't need to do anything
> in order for the hypercall to get continued until it completes (which
> may be "fails", faod).
> 
> Jan


Okay, I've managed to implement continuations while still having these array 
small.
The operation could simply process max. 32 elements at the time and creates 
continuation
until everything gets processed.

This will be in patch v3.


Best regards,
Michał Leszczyński
CERT Polska