Re: [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h

From: Julien Grall <julien@xen.org>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org,
	"Julien Grall" <jgrall@amazon.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>, "Wei Liu" <wl@xen.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h
Date: Fri, 14 Aug 2020 20:10:26 +0100	[thread overview]
Message-ID: <1f397467-5e57-7a8a-f121-19eef25dd26d@xen.org> (raw)
In-Reply-To: <alpine.DEB.2.21.2007301234180.1767@sstabellini-ThinkPad-T480s>

Hi Stefano,

On 30/07/2020 20:37, Stefano Stabellini wrote:
> On Thu, 30 Jul 2020, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Most of the helpers to access guest memory are implemented the same way
>> on Arm and x86. The only differences are:
>>      - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE()
> 
> It is actually just guest_handle_to_param() ?

Yes. I forgot Jan recently removed the helper in commit dd5520f9df05 
"x86: adjustments to guest handle treatment". I will update the commit 
message.

> 
> 
>>        and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It
>>        is still fine to use the Arm implementation on x86.
>>      - __clear_guest_offset(): Interestingly the prototype does not match
>>        between the x86 and Arm. However, the Arm one is bogus. So the x86
>>        implementation can be used.
>>      - guest_handle{,_subrange}_okay(): They are validly differing
>>        because Arm is only supporting auto-translated guest and therefore
>>        handles are always valid.
>>
>> In the past, the ia64 and ppc64 port use a different model to access
>> guest parameter. They have been long gone now.
>>
>> Given Xen currently only support 2 archictures, it is too soon to have a
>> directory asm-generic as it is not possible to differentiate it with the
>> existing directory xen/. If/When there is a 3rd port, we can decide to
>> create the new directory if that new port decide to use a different way
>> to access guest parameter.
>>
>> For now, consolidate it in xen/guest_access.h.
>>
>> While it would be possible to adjust the coding style at the same, this
>> is left for a follow-up patch so 'diff' can be used to check the
>> consolidation was done correctly.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Looks good to me
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thank you!

Cheers,

-- 
Julien Grall