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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-08-31 at 11:46 +0200, Jan Beulich wrote:
> On 30.08.2023 18:57, Oleksii Kurochko wrote:
> > asm/vm_event.h is common for ARM and RISC-V so it will be moved to
> > asm-generic dir.
> > 
> > Original asm/vm_event.h from ARM was updated:
> >  * use SPDX-License-Identifier.
> >  * update comment messages of stubs.
> >  * update #ifdef.
> >  * change public/domctl.h to public/vm_event.h.
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > ---
> > Changes in V3:
> >  - add Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> for
> > "xen: move arm/include/asm/vm_event.h to asm-generic"
> >  - update SPDX tag.
> >  - move asm/vm_event.h to asm-generic.
> > ---
> > Changes in V2:
> >  - change public/domctl.h to public/vm_event.h.
> >  - update commit message of [PATCH v2 2/2] xen: move
> > arm/include/asm/vm_event.h to stubs
> > ---
> >  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.?

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

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.

~ Oleksii