On 07.07.2021 15:39, Julien Grall wrote: > 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.
Well, I did say this would be better than is_pv_*(). I probably didn't pay enough attention to you already pointing out paging_mode_external() in the description; I'm sorry. On x86 both are in sync anyway, and I have to admit I don't see clearly enough which of the two would be the right one to use here, partly because I'm afraid I also don't see why steal_page() has such a restriction in the first place (which you now build upon). >>> @@ -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. Hmm, the public header is wrong then, as it documents the field as [OUT] only _despite_ the shouting warning in point 5 of the comment. I guess I never really understood why this sub-op differs from others in where the continuation indicator lives. Never mind then, indeed no code adjustment needed: Reviewed-by: Jan Beulich <jbeul...@suse.com> Jan
