Re: [PATCH v3 5/6] arm64/xen: introduce CONFIG_XEN and hypercall.S on ARM64

Stefano Stabellini
2013-06-05T13:04:57Z

On Wed, 5 Jun 2013, Arnd Bergmann wrote:
> On Wednesday 05 June 2013 13:15:29 Stefano Stabellini wrote:
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index c95c5cb..79dd13d 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -37,6 +37,7 @@ TEXT_OFFSET := 0x00080000
> > export TEXT_OFFSET GZFLAGS
> >
> > core-y += arch/arm64/kernel/ arch/arm64/mm/
> > +core-$(CONFIG_XEN) += arch/arm64/xen/
> > libs-y := arch/arm64/lib/ $(libs-y)
> > libs-y += $(LIBGCC)
> >
> > diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
> > new file mode 100644
> > index 0000000..be24040
> > --- /dev/null
> > +++ b/arch/arm64/xen/Makefile
> > @@ -0,0 +1,2 @@
> > +xen-arm-y += $(addprefix ../../arm/xen/, enlighten.o grant-table.o)
> > +obj-y := xen-arm.o hypercall.o
>
> I think it would be nicer to redirect the entire directory, not just
> the enlighten.o and grant-table.o files. You could do in arch/arm64/Makefile:
>
> core-(CONFIG_XEN) += arch/arm/xen/
>
> That leaves a small difference in hypercall.o, which I think you can
> handle with an #ifdef.

That is nicer.

The implementation of hypercall.S is significantly different between
arm32 and arm64, so hypercall.S is going to be harder to read.
However hypercall.S is just one file and it's not going to grow much,
while we might have many more C source files in common between the two
architecures, so I think that your approach is going to be cleaner in
the long term.

I'll make the change.