Re: [Xen-devel] [PATCH 57/57] ARM: VGIC: wire new VGIC(-v2) files into Xen build system

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 05/03/18 16:04, Andre Przywara wrote:
> Now that we have both the old VGIC prepared to cope with a sibling and
> the code for the new VGIC in place, lets add a Kconfig option to enable
> the new code and wire it into the Xen build system.
> This will add a compile time option to use either the "old" or the "new"
> VGIC.
> In the moment this is restricted to a vGIC-v2. To make the build system
> happy, we provide a temporary dummy implementation of
> vgic_v3_setup_hw() to allow building for now.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
> Changelog RFC ... v1:
> - no changes
>
>   xen/arch/arm/Kconfig     |  6 +++++-
>   xen/arch/arm/Makefile    | 10 +++++++++-
>   xen/arch/arm/vgic/vgic.c |  8 ++++++++
>   xen/common/Makefile      |  1 +
>   4 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2782ee6589..aad19927ce 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -48,7 +48,11 @@ config HAS_GICV3
>   config HAS_ITS
>           bool
>           prompt "GICv3 ITS MSI controller support" if EXPERT = "y"
> -        depends on HAS_GICV3
> +        depends on HAS_GICV3 && !NEW_VGIC
> +
> +config NEW_VGIC
> +        bool
> +        prompt "Use new VGIC implementation"
You potentially want to add a description encouraging people to test and 
making aware of this is new (not security supported). Or maybe we need 
to do that in SUPPORT.MD?
>   
>   config SBSA_VUART_CONSOLE
>         bool "Emulated SBSA UART console support"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 41d7366527..2a3ec94a18 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -16,7 +16,6 @@ obj-y += domain_build.o
>   obj-y += domctl.o
>   obj-$(EARLY_PRINTK) += early_printk.o
>   obj-y += gic.o
> -obj-y += gic-vgic.o
>   obj-y += gic-v2.o
>   obj-$(CONFIG_HAS_GICV3) += gic-v3.o
>   obj-$(CONFIG_HAS_ITS) += gic-v3-its.o
> @@ -47,10 +46,19 @@ obj-y += sysctl.o
>   obj-y += time.o
>   obj-y += traps.o
>   obj-y += vcpreg.o
> +ifeq ($(CONFIG_NEW_VGIC),y)
> +obj-y += vgic/vgic.o
> +obj-y += vgic/vgic-v2.o
> +obj-y += vgic/vgic-mmio.o
> +obj-y += vgic/vgic-mmio-v2.o
> +obj-y += vgic/vgic-init.o
Hmmm, I would much prefer to see a Makefile introduced in vgic.

> +else
> +obj-y += gic-vgic.o
>   obj-y += vgic.o
>   obj-y += vgic-v2.o
>   obj-$(CONFIG_HAS_GICV3) += vgic-v3.o
>   obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
> +endif
>   obj-y += vm_event.o
>   obj-y += vtimer.o
>   obj-$(CONFIG_SBSA_VUART_CONSOLE) += vpl011.o
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index f42092fec3..f7b4779a71 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -978,6 +978,14 @@ unsigned int domain_max_vcpus(const struct domain *d)
>       return min_t(unsigned int, MAX_VIRT_CPUS, vgic_vcpu_limit);
>   }
>   
> +void vgic_v3_setup_hw(paddr_t dbase,
> +                      unsigned int nr_rdist_regions,
> +                      const struct rdist_region *regions,
> +                      unsigned int intid_bits)
> +{
> +    /* Dummy implementation to allow building without actual vGICv3 support. */
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 3a349f478b..92a1d1fa58 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
This would require to CC the "REST".

> @@ -19,6 +19,7 @@ obj-y += keyhandler.o
>   obj-$(CONFIG_KEXEC) += kexec.o
>   obj-$(CONFIG_KEXEC) += kimage.o
>   obj-y += lib.o
> +obj-$(CONFIG_NEW_VGIC) += list_sort.
This does not look right to me. I would much prefer a new config or 
building by default. I would like to hear the opinion of others here.
>   obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
>   obj-y += lzo.o
>   obj-$(CONFIG_HAS_MEM_ACCESS) += mem_access.o
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel