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

From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Julien Grall" <jgrall@amazon.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	xen-devel@lists.xenproject.org,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h
Date: Wed, 8 Apr 2020 23:05:45 +0100	[thread overview]
Message-ID: <041a9f9f-cc9e-eac5-cdd2-555fb1c88e6f@xen.org> (raw)
In-Reply-To: <e2588f6e-1f13-b66f-8e3d-b8568f67b62a@suse.com>

Hi Jan,

On 07/04/2020 09:14, Jan Beulich wrote:
> On 04.04.2020 15:10, 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()
>>        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.
> 
> While I'm fine in principle with such consolidation, I'm afraid I
> really need to ask for some historical background to be added
> here. It may very well be that there's a reason for the separation
> (likely to be found in the removed ia64 or ppc ports), which may
> then provide a hint at why future ports may want to have these
> separated. If such reasons exist, I'd prefer to avoid the back and
> forth between headers. What we could do in such a case is borrow
> Linux'es asm-generic/ concept, and move the "typical"
> implementation there. (And of course if there were no noticable
> reasons for the split, the change as it is would be fine in
> general; saying so without having looked at the details of it,
> yet).

Looking at the history, ia64 and ppc used to include a common header 
called xen/xencomm.h from asm/guest_access.h.

This has now disappeared with the removal of the two ports.

Regarding future arch, the fact arm and x86 gives me some confidence we 
are unlikely going to get a new ABI for an arch. Do you see any reason to?

Cheers,

-- 
Julien Grall