Re: [PATCH v3 2/2] xen: move arm/include/asm/vm_event.h to asm-generic

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.08.2023 16:02, Oleksii wrote:
> On Thu, 2023-08-31 at 11:46 +0200, Jan Beulich wrote:
>> On 30.08.2023 18:57, Oleksii Kurochko wrote:
>>> ---
>>>  xen/arch/arm/include/asm/vm_event.h    | 66 ----------------------
>>> ----
>>>  xen/include/asm-generic/asm/vm_event.h | 55 +++++++++++++++++++++
>>>  2 files changed, 55 insertions(+), 66 deletions(-)
>>>  delete mode 100644 xen/arch/arm/include/asm/vm_event.h
>>>  create mode 100644 xen/include/asm-generic/asm/vm_event.h
>>
>> While it's a comment on the first patch, it's really better making
>> here:
>> Did you look at Linux? They don't put an intermediate asm/ here.
>> Instead
>> see their scripts/Makefile.asm-generic. That way an arch still has
>> control which generic headers it gets access to, without duplicating
>> any
>> of them.
> 
> IIUC scripts/Makefile.asm-generic script is needed to generate a
> wrapper for a header which will contain:
> #include <asm-generic/some.h>
> if it is mentioned generic-y += some.h in
> arch/${ARCH}/include/asm/Kbuild.
> 
> But do we need this generated wrapper header?
> Do we need all the support of generic-y in Kbuild, etc.?

For a single header we could likely get away without. But vm_event.h is
only the first example.

> In the previous patch of this patch series, it was added inclusion of
> $(srctree)/include/asm-generic after the inclusion of
> $(srctree)/arch/$(SRCARCH)/include so it will first look if the arch-
> specific header exists, and if not then use generic one.
> 
> Probably I misunderstood you and your suggestion was to have
> scripts/Makefile.asm-generic which will generate folder asm/ with
> necessary headers in arch specific folder?

Yes.

> So basically it was just a
> question if asm/ folder should exist in $(srctree)/include/asm-generic
> or not?

Not really, no. For "#include <asm/...>" to work, you can't simply omit
asm/ under asm-generic/. That's where the generated wrapper headers
come into play.

> One more thing I would like to clarify is the duplicating of the
> headers you mentioned above.
> But if the architecture doesn't want to use a generic header, then it
> still needs to add the header to arch/${ARCH}/include/asm and remove
> mention of the header from arch/${ARCH}/include/asm/Kbuild.

But then it's using its own custom header, not a duplication of whatever
is the (in the case here) stub one.

Jan