Re: [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h

From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: xen-devel@lists.xenproject.org,
	"Julien Grall" <jgrall@amazon.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"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>, "Wei Liu" <wl@xen.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Paul Durrant" <paul@xen.org>,
	"Jun Nakajima" <jun.nakajima@intel.com>,
	"Kevin Tian" <kevin.tian@intel.com>
Subject: Re: [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h
Date: Tue, 18 Aug 2020 17:34:56 +0100	[thread overview]
Message-ID: <24005b88-ac1d-926d-ba86-67641375f610@xen.org> (raw)
In-Reply-To: <bd328401-e825-900e-5945-a5fc7cc519cd@suse.com>

Hi Jan,

On 18/08/2020 17:23, Jan Beulich wrote:
> On 18.08.2020 18:20, Julien Grall wrote:
>>
>>
>> On 18/08/2020 17:04, Jan Beulich wrote:
>>> On 18.08.2020 15:14, Julien Grall wrote:
>>>>
>>>>
>>>> On 18/08/2020 12:32, Jan Beulich wrote:
>>>>> On 18.08.2020 10:58, Julien Grall wrote:
>>>>>>> One option. Personally I'd prefer to avoid introduction of yet another
>>>>>>> constant, by leveraging __XEN_GUEST_ACCESS_H__ instead.
>>>>>>
>>>>>> I thought about it but it doesn't prevent new inclusions of asm/guest_access.h. For instance, the following would still compile:
>>>>>>
>>>>>> #include <xen/guest_access.h>
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>> #include <asm/guest_access.h>
>>>>>
>>>>> But where's the problem with this? The first #include will already
>>>>> have resulted in the inclusion of asm/guest_access.h, so the second
>>>>> #include is simply a no-op.
>>>>
>>>> A couple of reasons:
>>>>      1) I don't consider this solving completely your original request [1]
>>>>      2) I don't see how this is more important to prevent including <asm/guest_access.h> before and not after. Both will still compile fine, we just want to avoid it.
>>>>
>>>>
>>>> [1] "Is there any chance you could take measures to avoid new inclusions of asm/guest_access.h to appear?"
>>>
>>> Is
>>>
>>> #include <xen/guest_access.h>
>>> [...]
>>> #include <asm/guest_access.h>
>>>
>>> actually a problem (as opposed to an asm/ include without any include
>>> of the xen/ one at all)?
>>
>> Neither of them are really a problem today. So it is not entirely clear why we would want to prevent one and not the other.
> 
> If neither is a problem, why the conversion?

As I wrote in the commit message, some of the helpers will be moved from 
asm/guest_access.h. So existing users of asm/guest_access.h may not 
compiled anymore.

If you are not using any helpers from xen/guest_access.h, then you could 
theoritically only include asm/guest_access.h. But they are quite 
limited or maybe don't even exist.

Actually xen/guest_access.h was included from asm/guest_access.h. But 
there are compilation issues if you try to include xen/guest_access.h 
from asm/guest_access.h.

Therefore it is better to request everyone to include 
<xen/guest_access.h>. With that you get all the guest access helpers 
rather than just the arch specific one.

Cheers,

-- 
Julien Grall