Re: [PATCH v4 2/2] x86/ioreq: Extend ioreq server to support multiple ioreq pages

["Julian Vetter" <julian.vetter@xxxxxxxxxx>]

On 3/10/26 16:36, Jan Beulich wrote:
> On 05.03.2026 14:04, Julian Vetter wrote:
>> @@ -45,7 +52,8 @@ struct ioreq_server {
>>       /* Lock to serialize toolstack modifications */
>>       spinlock_t             lock;
>>   
>> -    struct ioreq_page      ioreq;
>> +    ioreq_t                *ioreq;
>> +    gfn_t                  ioreq_gfn;
>>       struct list_head       ioreq_vcpu_list;
>>       struct ioreq_page      bufioreq;
> 
> This change in data arrangement should in principle be independent of the
> step to supporting multiple pages. Hence it should be possible to separate
> out. Problem being that just by looking here and at hvm_{,un}map_ioreq_gfn()
> I can't conclude how you get away without the "page" field that struct
> ioreq_page had. If you can get away without, it's not quite clear why the
> field exists in the first place. If it's not needed, dropping it would be
> yet another separate, prereq change. At which point the remaining pair of
> fields could continue to be used, i.e. the change above then wouldn't be
> needed; va could be renamed if need be, and its type changed.
>

Thank you again Jan for your feedback! I don't need the page anymore. 
When I use vmap(), I don't need to keep track of it, because during 
teardown, I can recover it via vmap_to_page(). Currently it's necessary 
because in destroy_ring_for_helper we need the page, to be destroyed. 
But I see now, on X86 the map_domain_page_global called from 
prepare_ring_for_helper actually does vmap(&mfn, 1). So the page is also 
from the vmap range. So for the teardown I assume a vmap_to_page() could 
be used as well. But I also see there is a special case, if NDEBUG=1, 
map_domain_page_global short-circuits to mfn_to_virt() for low MFNs 
(putting the VA in the directmap range) and bypassing vmap. In that case 

vmap_to_page() would not work. So, this would be really messy. I would 
rather switch the bufioreq also to an explicitly vmap()'ed page, then we 
could remove the page pointer and both cases would be aligned again.

> And if the "page" field is still needed, then even more so you would imo
> want to retain the present data layout.
> 
> In any event, if at all possible a goal imo ought to be to avoid
> duplication of logic, like what is happening in the two named functions.
> I.e. preferably the bufio case would continue to be as similar as possible,
> just known to be single-page.

Yes, I agree. Thank you for you insight. I will have a look again how I 
could align them as much as possible.

Julian
> 
> Jan



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech