Re: [PATCH v3 13/19] xen/arm: Move fixmap definitions in a separate header

From: Julien Grall <julien@xen.org>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org, Julien Grall <jgrall@amazon.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH v3 13/19] xen/arm: Move fixmap definitions in a separate header
Date: Tue, 5 Apr 2022 22:47:07 +0100	[thread overview]
Message-ID: <ddfdefed-21e6-06ee-182b-731fd130794a@xen.org> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2204051411510.2910984@ubuntu-linux-20-04-desktop>

Hi Stefano,

On 05/04/2022 22:12, Stefano Stabellini wrote:
>> +/* Map a page in a fixmap entry */
>> +extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
>> +/* Remove a mapping from a fixmap entry */
>> +extern void clear_fixmap(unsigned map);
>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* __ASM_FIXMAP_H */
> 
> 
> It is a good idea to create fixmap.h, but I think it should be acpi.h to
> include fixmap.h, not the other way around.

As I wrote in the commit message, one definition in fixmap.h rely on 
define from acpi.h (i.e NUM_FIXMAP_ACPI_PAGES). So if we don't include 
it, then user of FIXMAP_PMAP_BEGIN (see next patch) will requires to 
include acpi.h in order to build.

Re-ordering the values would not help because the problem would exactly 
be the same but this time the acpi users would have to include pmap.h to 
define NUM_FIX_PMAP.

> 
> The appended changes build correctly on top of this patch.

That's expected because all the users of FIXMAP_ACPI_END will be 
including acpi.h. But after the next patch, we would need pmap.c to 
include acpi.h.

I don't think this would be right (and quite likely you would ask why
this is done). Hence this approach.

Cheers,

-- 
Julien Grall