Re: [XEN PATCH 3/6] x86/vm_event: add missing include

[Julien Grall <julien@xxxxxxx>]

Hi,

On 12/08/2023 10:53, Nicola Vetrini wrote:
> > > diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
> > > index 7027c08a926b..499b6b349d79 100644
> > > --- a/xen/arch/x86/vm_event.c
> > > +++ b/xen/arch/x86/vm_event.c
> > > @@ -20,6 +20,7 @@
> > >
> > >  #include <xen/sched.h>
> > >  #include <xen/mem_access.h>
> > > +#include <xen/vm_event.h>
> > >  #include <asm/vm_event.h>
> > >
> > >  /* Implicitly serialized by the domctl lock. */
> > I think the problem here is that ./arch/x86/include/asm/vm_event.h,
> > differently from ./arch/arm/include/asm/vm_event.h, doesn't #include
> > <xen/vm_event.h>
> I see your point. Do you think it would be better to include xen/vm_event.h
> in asm/vm_event.h for x86 or move the inclusion of xen/vm_event.h for 
> arm to
> the source file, as done in the patch?
I think it is a bit odd require the C file to include the arch-specific 
header and the common one. It would be better to include only one.
My preference would be to include <asm/...> from <xen/...> and then only 
include the latter in the C file.
Cheers,

--
Julien Grall