[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.