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

[Julien Grall <julien@xxxxxxx>]

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