Re: [PATCH v5 1/4] xen: XENMEM_exchange should only be used/compiled for arch supporting PV guest

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 05/07/2021 09:41, Jan Beulich wrote:
> On 03.07.2021 19:11, Julien Grall wrote:
> > Changes in v5:
> >      - Removed the #ifdef CONFIG_X86 as they are not necessary anymore
> >      - Used paging_mode_translate() rather than is_pv_domain()
> Is there a particular reason you use this in favor of steal_page()'s
> paging_mode_external()?
This is what you suggested in v4 [1]. I can switch to 
paging_mode_external() if this is what you now prefer.
> > @@ -815,6 +812,9 @@ static long 
> > memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
> >       if ( __copy_field_to_guest(arg, &exch, nr_exchanged) )
> >           rc = -EFAULT;
> I'm afraid that for correctness of the interface you need to keep
> this part even in the !PV case.
Xen never initializes the field nr_exchanged. Instead, it expects the 
guest to set to 0. So I am not quite to sure why we would need to keep 
this line.
> Seeing the 2nd use of steal_page() I wonder if it was too much to
> ask you to do a similar transformation for gnttab_transfer(), even
> if it's not directly related to the immediate purpose of this
> series. If you're not going to do so, I guess I'd put it on my list
> (perhaps together with moving x86'es steal_page() and dropping
> Arm's stub).
I will have a look to #ifdef the code in gnttab_transfer() and drop 
steal_page() on Arm.
Cheers,

[1] <834db49d-dda5-784c-1135-8427086a04eb@xxxxxxxx>

--
Julien Grall