| Message ID | 20180321163235.12529-40-andre.przywara@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi Andre, On 03/21/2018 04:32 PM, Andre Przywara wrote: > diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c > index 131358a5a1..22c70ff7cd 100644 > --- a/xen/arch/arm/vgic/vgic.c > +++ b/xen/arch/arm/vgic/vgic.c > @@ -981,6 +981,16 @@ unsigned int vgic_max_vcpus(const struct domain *d) > return min_t(unsigned int, MAX_VIRT_CPUS, vgic_vcpu_limit); > } > > +#ifdef CONFIG_HAS_GICV3 > +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. */ One major inconvenience with that solution is GICv3 driver is going to be initialized but then you hit the BUG_ON() in domain_vgic_register. This is really not nice for the user but it is not obvious why the BUG_ON() is hit. I am ok if you don't want to touch the Kconfig. But I would at least implement that helper with a panic("vGICv3 not yet supported with the new vGIC"); Cheers,
Hi, On 22/03/18 08:16, Julien Grall wrote: > Hi Andre, > > On 03/21/2018 04:32 PM, Andre Przywara wrote: >> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c >> index 131358a5a1..22c70ff7cd 100644 >> --- a/xen/arch/arm/vgic/vgic.c >> +++ b/xen/arch/arm/vgic/vgic.c >> @@ -981,6 +981,16 @@ unsigned int vgic_max_vcpus(const struct domain *d) >> return min_t(unsigned int, MAX_VIRT_CPUS, vgic_vcpu_limit); >> } >> +#ifdef CONFIG_HAS_GICV3 >> +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. */ > > One major inconvenience with that solution is GICv3 driver is going to > be initialized but then you hit the BUG_ON() in domain_vgic_register. > This is really not nice for the user but it is not obvious why the > BUG_ON() is hit. > > I am ok if you don't want to touch the Kconfig. But I would at least > implement that helper with a panic("vGICv3 not yet supported with the > new vGIC"); Yes, that's a good point (and easy to implement!) ;-) Also I think we should have something saying that we are using the new VGIC, so we have it in the logs. Just realised this when I was wondering if my machine is currently using the new or the old VGIC ;-) I put something in the vgic_v2_setup_hw() implementation. Cheers, Andre.
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 2782ee6589..8174c0c635 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -48,7 +48,23 @@ 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" + select NEEDS_LIST_SORT + ---help--- + + This is an alternative implementation of the ARM GIC interrupt + controller emulation, based on the Linux/KVM VGIC. It has a better + design and fixes many shortcomings of the existing GIC emulation in + Xen. It will eventually replace the existing/old VGIC. + However at the moment it lacks support for Dom0 using the ITS for + using MSIs. + Say Y if you want to help testing this new code or if you experience + problems with the standard emulation. + At the moment this implementation is not security supported. config SBSA_VUART_CONSOLE bool "Emulated SBSA UART console support" diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 41d7366527..a9533b107e 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,14 @@ obj-y += sysctl.o obj-y += time.o obj-y += traps.o obj-y += vcpreg.o +subdir-$(CONFIG_NEW_VGIC) += vgic +ifneq ($(CONFIG_NEW_VGIC),y) +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/Makefile b/xen/arch/arm/vgic/Makefile new file mode 100644 index 0000000000..806826948e --- /dev/null +++ b/xen/arch/arm/vgic/Makefile @@ -0,0 +1,5 @@ +obj-y += vgic.o +obj-y += vgic-v2.o +obj-y += vgic-mmio.o +obj-y += vgic-mmio-v2.o +obj-y += vgic-init.o diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c index 131358a5a1..22c70ff7cd 100644 --- a/xen/arch/arm/vgic/vgic.c +++ b/xen/arch/arm/vgic/vgic.c @@ -981,6 +981,16 @@ unsigned int vgic_max_vcpus(const struct domain *d) return min_t(unsigned int, MAX_VIRT_CPUS, vgic_vcpu_limit); } +#ifdef CONFIG_HAS_GICV3 +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. */ +} +#endif + /* * Local variables: * mode: C
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@linaro.org> --- Changelog v2 ... v3: - fix indentation of Kconfig entry - select NEEDS_LIST_SORT - drop unconditional list_sort.o inclusion Changelog v1 ... v2: - add Kconfig help text - use separate Makefile in vgic/ directory - protect compilation without GICV3 support - always include list_sort() in build xen/arch/arm/Kconfig | 18 +++++++++++++++++- xen/arch/arm/Makefile | 5 ++++- xen/arch/arm/vgic/Makefile | 5 +++++ xen/arch/arm/vgic/vgic.c | 10 ++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 xen/arch/arm/vgic/Makefile