[Xen-devel,v2,45/45] ARM: VGIC: wire new VGIC(-v2) files into Xen build system

Message ID 20180315203050.19791-46-andre.przywara@linaro.org
State New
Headers show
Series
  • New VGIC(-v2) implementation
Related show

Commit Message

Andre Przywara March 15, 2018, 8:30 p.m.
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 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       | 17 ++++++++++++++++-
 xen/arch/arm/Makefile      |  5 ++++-
 xen/arch/arm/vgic/Makefile |  5 +++++
 xen/arch/arm/vgic/vgic.c   | 10 ++++++++++
 xen/common/Makefile        |  1 +
 5 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/arm/vgic/Makefile

Comments

Jan Beulich March 16, 2018, 10:48 a.m. | #1
>>> On 15.03.18 at 21:30, <andre.przywara@linaro.org> wrote:
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -19,6 +19,7 @@ obj-y += keyhandler.o
>  obj-$(CONFIG_KEXEC) += kexec.o
>  obj-$(CONFIG_KEXEC) += kimage.o
>  obj-y += lib.o
> +obj-y += list_sort.o

Why here rather than in patch 17? And why also for x86? I think you
want a promptless Kconfig option that an arch can select if it needs
this code, unless or until common code makes use of it.

Jan
Andre Przywara March 16, 2018, 11:10 a.m. | #2
Hi,

On 16/03/18 10:48, Jan Beulich wrote:
>>>> On 15.03.18 at 21:30, <andre.przywara@linaro.org> wrote:
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -19,6 +19,7 @@ obj-y += keyhandler.o
>>  obj-$(CONFIG_KEXEC) += kexec.o
>>  obj-$(CONFIG_KEXEC) += kimage.o
>>  obj-y += lib.o
>> +obj-y += list_sort.o
> 
> Why here rather than in patch 17? And why also for x86? I think you
> want a promptless Kconfig option that an arch can select if it needs
> this code, unless or until common code makes use of it.

Yeah, I had obj-$(CONFIG_NEW_VGIC) before, but Julien disliked it.
Promptless Kconfig sounds good to me.

Thanks!
Andre
Jan Beulich March 16, 2018, 11:32 a.m. | #3
>>> On 16.03.18 at 12:10, <andre.przywara@linaro.org> wrote:
> On 16/03/18 10:48, Jan Beulich wrote:
>>>>> On 15.03.18 at 21:30, <andre.przywara@linaro.org> wrote:
>>> --- a/xen/common/Makefile
>>> +++ b/xen/common/Makefile
>>> @@ -19,6 +19,7 @@ obj-y += keyhandler.o
>>>  obj-$(CONFIG_KEXEC) += kexec.o
>>>  obj-$(CONFIG_KEXEC) += kimage.o
>>>  obj-y += lib.o
>>> +obj-y += list_sort.o
>> 
>> Why here rather than in patch 17? And why also for x86? I think you
>> want a promptless Kconfig option that an arch can select if it needs
>> this code, unless or until common code makes use of it.
> 
> Yeah, I had obj-$(CONFIG_NEW_VGIC) before, but Julien disliked it.
> Promptless Kconfig sounds good to me.

And note I'm not asking for a VGIC option, but for a LIST_SORT one.

Jan
Andre Przywara March 16, 2018, 3:13 p.m. | #4
Hi,

On 16/03/18 11:32, Jan Beulich wrote:
>>>> On 16.03.18 at 12:10, <andre.przywara@linaro.org> wrote:
>> On 16/03/18 10:48, Jan Beulich wrote:
>>>>>> On 15.03.18 at 21:30, <andre.przywara@linaro.org> wrote:
>>>> --- a/xen/common/Makefile
>>>> +++ b/xen/common/Makefile
>>>> @@ -19,6 +19,7 @@ obj-y += keyhandler.o
>>>>  obj-$(CONFIG_KEXEC) += kexec.o
>>>>  obj-$(CONFIG_KEXEC) += kimage.o
>>>>  obj-y += lib.o
>>>> +obj-y += list_sort.o
>>>
>>> Why here rather than in patch 17? And why also for x86? I think you
>>> want a promptless Kconfig option that an arch can select if it needs
>>> this code, unless or until common code makes use of it.
>>
>> Yeah, I had obj-$(CONFIG_NEW_VGIC) before, but Julien disliked it.
>> Promptless Kconfig sounds good to me.
> 
> And note I'm not asking for a VGIC option, but for a LIST_SORT one.

Yeah, I got that ;-)
CONFIG_LIST_SORT or CONFIG_HAS_LIST_SORT?

Cheers,
Andre.
Jan Beulich March 16, 2018, 3:34 p.m. | #5
>>> On 16.03.18 at 16:13, <andre.przywara@linaro.org> wrote:
> Hi,
> 
> On 16/03/18 11:32, Jan Beulich wrote:
>>>>> On 16.03.18 at 12:10, <andre.przywara@linaro.org> wrote:
>>> On 16/03/18 10:48, Jan Beulich wrote:
>>>>>>> On 15.03.18 at 21:30, <andre.przywara@linaro.org> wrote:
>>>>> --- a/xen/common/Makefile
>>>>> +++ b/xen/common/Makefile
>>>>> @@ -19,6 +19,7 @@ obj-y += keyhandler.o
>>>>>  obj-$(CONFIG_KEXEC) += kexec.o
>>>>>  obj-$(CONFIG_KEXEC) += kimage.o
>>>>>  obj-y += lib.o
>>>>> +obj-y += list_sort.o
>>>>
>>>> Why here rather than in patch 17? And why also for x86? I think you
>>>> want a promptless Kconfig option that an arch can select if it needs
>>>> this code, unless or until common code makes use of it.
>>>
>>> Yeah, I had obj-$(CONFIG_NEW_VGIC) before, but Julien disliked it.
>>> Promptless Kconfig sounds good to me.
>> 
>> And note I'm not asking for a VGIC option, but for a LIST_SORT one.
> 
> Yeah, I got that ;-)
> CONFIG_LIST_SORT or CONFIG_HAS_LIST_SORT?

The former (or CONFIG_NEEDS_LIST_SORT or CONFIG_WANT_LIST_SORT).

Jan
Julien Grall March 20, 2018, 3:13 a.m. | #6
Hi Andre,

On 03/15/2018 08:30 PM, Andre Przywara wrote:
> 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 4b9664f313..342b95be31 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -968,6 +968,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

Why not just avoid selecting HAS_GICV3?

Cheers,
Andre Przywara March 20, 2018, 3:57 p.m. | #7
Hi,

On 20/03/18 03:13, Julien Grall wrote:
> Hi Andre,
> 
> On 03/15/2018 08:30 PM, Andre Przywara wrote:
>> 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 4b9664f313..342b95be31 100644
>> --- a/xen/arch/arm/vgic/vgic.c
>> +++ b/xen/arch/arm/vgic/vgic.c
>> @@ -968,6 +968,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
> 
> Why not just avoid selecting HAS_GICV3?

Because "config ARM_64" selects HAS_GICV3, and I didn't dare to touch
this. Shouldn't be around for long anyways.

Cheers,
Andre.

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2782ee6589..310f909768 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -48,7 +48,22 @@  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"
+        ---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 4b9664f313..342b95be31 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -968,6 +968,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
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 3a349f478b..1668e14c4b 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -19,6 +19,7 @@  obj-y += keyhandler.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_KEXEC) += kimage.o
 obj-y += lib.o
+obj-y += list_sort.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
 obj-y += lzo.o
 obj-$(CONFIG_HAS_MEM_ACCESS) += mem_access.o