Re: [Xen-devel] [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 13/09/17 22:13, Stefano Stabellini wrote:
> On Wed, 13 Sep 2017, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 09/12/2017 10:53 PM, Stefano Stabellini wrote:
> > > On Tue, 12 Sep 2017, Julien Grall wrote:
> > > > Currently, cpregs.h is included in pretty much every files even for
> > > > arm64. However, the only use for arm64 is when emulating co-processors.
> > > >
> > > > For arm32, all users of processor.h expect cpregs.h to be included in
> > > > order to access co-processors. So move the inclusion in
> > > > asm-arm/arm32/processor.h.
> > > >
> > > > cpregs.h will also be directly included in the co-processors emulation
> > > > to accommodate arm64.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > I can see that the patch works and does what you describe, but what is
> > > the benefit? OK, we remove #include <asm/cpregs.h> from
> > > asm-arm/processor.h, but then we have to add it to vcpreg.c, vgic-v3.c,
> > > vtimer.c, and arm32/processor.h. Is there a long term benefit? What
> > > prompted you into writing this patch?
> > asm/processor.h is included indirectly by every single file of the source
> > code. It seems that we use it as a placeholder for anything rather than
> > properly splitting in different headers. For instance it contains all the
> > definitions of the registers, the traps vector, SMC call, traps prototype...
> > For most of those stuff only a limited of files really care about it. So we
> > are polluting the name space of each file for no real benefits.
> >
> > On AArch64, cpregs.h is only useful when emulating co-processor gregisters. So
> > there are no point to include it in a main header that will be pulled by 100s
> > files just for the benetifs of 4 files.
> >
> > This patch is only a first attempt of clean-up processor.h. Ideally we should
> > have more patch to split it.
> All right.
>
> Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>
> Maybe add a couple of lines to the commit description saying that by
> removing the inclusion of cpregs.h from asm/processor.h we drastically
> reduce the exposure of cpregs.h to any source files on ARM64.
I will do that.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel