| Message ID | 20160928182457.12433-2-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 28 Sep 2016, Andre Przywara wrote: > Parse the DT GIC subnodes to find every ITS MSI controller the hardware > offers. Store that information in a list to both propagate all of them > later to Dom0, but also to be able to iterate over all ITSes. > This introduces an ITS Kconfig option. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Looks good to me > xen/arch/arm/Kconfig | 5 ++++ > xen/arch/arm/Makefile | 1 + > xen/arch/arm/gic-its.c | 67 +++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 6 ++++ > xen/include/asm-arm/gic-its.h | 57 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 136 insertions(+) > create mode 100644 xen/arch/arm/gic-its.c > create mode 100644 xen/include/asm-arm/gic-its.h > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 797c91f..9fe3b8e 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -45,6 +45,11 @@ config ACPI > config HAS_GICV3 > bool > > +config HAS_ITS > + bool "GICv3 ITS MSI controller support" > + depends on ARM_64 > + depends on HAS_GICV3 > + > config ALTERNATIVE > bool > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 64fdf41..c2c4daa 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -18,6 +18,7 @@ obj-$(EARLY_PRINTK) += early_printk.o > obj-y += gic.o > obj-y += gic-v2.o > obj-$(CONFIG_HAS_GICV3) += gic-v3.o > +obj-$(CONFIG_HAS_ITS) += gic-its.o > obj-y += guestcopy.o > obj-y += hvm.o > obj-y += io.o > diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > new file mode 100644 > index 0000000..0f42a77 > --- /dev/null > +++ b/xen/arch/arm/gic-its.c > @@ -0,0 +1,67 @@ > +/* > + * xen/arch/arm/gic-its.c > + * > + * ARM Generic Interrupt Controller ITS support > + * > + * Copyright (C) 2016 - ARM Ltd > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <xen/config.h> > +#include <xen/lib.h> > +#include <xen/device_tree.h> > +#include <xen/libfdt/libfdt.h> > +#include <asm/gic.h> > +#include <asm/gic_v3_defs.h> > +#include <asm/gic-its.h> > + > +void gicv3_its_dt_init(const struct dt_device_node *node) > +{ > + const struct dt_device_node *its = NULL; > + struct host_its *its_data; > + > + /* > + * Check for ITS MSI subnodes. If any, add the ITS register > + * frames to the ITS list. > + */ > + dt_for_each_child_node(node, its) > + { > + paddr_t addr, size; > + > + if ( !dt_device_is_compatible(its, "arm,gic-v3-its") ) > + continue; > + > + if ( dt_device_get_address(its, 0, &addr, &size) ) > + panic("GICv3: Cannot find a valid ITS frame address"); > + > + its_data = xzalloc(struct host_its); > + if ( !its_data ) > + panic("GICv3: Cannot allocate memory for ITS frame"); > + > + its_data->addr = addr; > + its_data->size = size; > + its_data->dt_node = its; > + > + printk("GICv3: Found ITS @0x%lx\n", addr); > + > + list_add_tail(&its_data->entry, &host_its_list); > + } > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index b8be395..238da84 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -43,9 +43,12 @@ > #include <asm/device.h> > #include <asm/gic.h> > #include <asm/gic_v3_defs.h> > +#include <asm/gic-its.h> > #include <asm/cpufeature.h> > #include <asm/acpi.h> > > +LIST_HEAD(host_its_list); > + > /* Global state */ > static struct { > void __iomem *map_dbase; /* Mapped address of distributor registers */ > @@ -1229,6 +1232,9 @@ static void __init gicv3_dt_init(void) > > dt_device_get_address(node, 1 + gicv3.rdist_count + 2, > &vbase, &vsize); > + > + /* Check for ITS child nodes and build the host ITS list accordingly. */ > + gicv3_its_dt_init(node); > } > > static int gicv3_iomem_deny_access(const struct domain *d) > diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h > new file mode 100644 > index 0000000..2f5c51c > --- /dev/null > +++ b/xen/include/asm-arm/gic-its.h > @@ -0,0 +1,57 @@ > +/* > + * ARM GICv3 ITS support > + * > + * Andre Przywara <andre.przywara@arm.com> > + * Copyright (c) 2016 ARM Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#ifndef __ASM_ARM_ITS_H__ > +#define __ASM_ARM_ITS_H__ > + > +#ifndef __ASSEMBLY__ > +#include <xen/device_tree.h> > + > +/* data structure for each hardware ITS */ > +struct host_its { > + struct list_head entry; > + const struct dt_device_node *dt_node; > + paddr_t addr; > + paddr_t size; > +}; > + > +extern struct list_head host_its_list; > + > +#ifdef CONFIG_HAS_ITS > + > +/* Parse the host DT and pick up all host ITSes. */ > +void gicv3_its_dt_init(const struct dt_device_node *node); > + > +#else > + > +static inline void gicv3_its_dt_init(const struct dt_device_node *node) > +{ > +} > + > +#endif /* CONFIG_HAS_ITS */ > + > +#endif /* __ASSEMBLY__ */ > +#endif > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > -- > 2.9.0 >
Hi Andre, On 28/09/2016 19:24, Andre Przywara wrote: > Parse the DT GIC subnodes to find every ITS MSI controller the hardware > offers. Store that information in a list to both propagate all of them > later to Dom0, but also to be able to iterate over all ITSes. > This introduces an ITS Kconfig option. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/Kconfig | 5 ++++ > xen/arch/arm/Makefile | 1 + > xen/arch/arm/gic-its.c | 67 +++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 6 ++++ > xen/include/asm-arm/gic-its.h | 57 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 136 insertions(+) > create mode 100644 xen/arch/arm/gic-its.c > create mode 100644 xen/include/asm-arm/gic-its.h > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 797c91f..9fe3b8e 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -45,6 +45,11 @@ config ACPI > config HAS_GICV3 > bool > > +config HAS_ITS > + bool "GICv3 ITS MSI controller support" > + depends on ARM_64 HAS_GICV3 will only be selected for 64-bit. It would need some rework to be supported on 32-bit. So I would drop this dependency. > + depends on HAS_GICV3 > + I am not convinced that we should (currently) let the user selecting the ITS support. It increases the test coverage (we have to test with and without). Do we expect people using GICv3 without ITS? Regards,
Hi, On 01/11/16 15:13, Julien Grall wrote: > Hi Andre, > > On 28/09/2016 19:24, Andre Przywara wrote: >> Parse the DT GIC subnodes to find every ITS MSI controller the hardware >> offers. Store that information in a list to both propagate all of them >> later to Dom0, but also to be able to iterate over all ITSes. >> This introduces an ITS Kconfig option. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> xen/arch/arm/Kconfig | 5 ++++ >> xen/arch/arm/Makefile | 1 + >> xen/arch/arm/gic-its.c | 67 >> +++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/gic-v3.c | 6 ++++ >> xen/include/asm-arm/gic-its.h | 57 ++++++++++++++++++++++++++++++++++++ >> 5 files changed, 136 insertions(+) >> create mode 100644 xen/arch/arm/gic-its.c >> create mode 100644 xen/include/asm-arm/gic-its.h >> >> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig >> index 797c91f..9fe3b8e 100644 >> --- a/xen/arch/arm/Kconfig >> +++ b/xen/arch/arm/Kconfig >> @@ -45,6 +45,11 @@ config ACPI >> config HAS_GICV3 >> bool >> >> +config HAS_ITS >> + bool "GICv3 ITS MSI controller support" >> + depends on ARM_64 > > HAS_GICV3 will only be selected for 64-bit. It would need some rework to > be supported on 32-bit. So I would drop this dependency. OK, makes sense. >> + depends on HAS_GICV3 >> + > > I am not convinced that we should (currently) let the user selecting the > ITS support. It increases the test coverage (we have to test with and > without). Do we expect people using GICv3 without ITS? My concern was more that if it breaks something, people can just disable it. But I have to go through the patches again to see if disabling it really brings us something (because thinking about it I don't think so). So given the test coverage argument I think we should at least enable it by default for ARM64. Is there some "expert options" group somewhere where we could insert the option to turn it off? Cheers, Andre.
On 14/11/16 17:35, Andre Przywara wrote: > Hi, Hi Andre, > On 01/11/16 15:13, Julien Grall wrote: >> On 28/09/2016 19:24, Andre Przywara wrote: >>> Parse the DT GIC subnodes to find every ITS MSI controller the hardware >>> offers. Store that information in a list to both propagate all of them >>> later to Dom0, but also to be able to iterate over all ITSes. >>> This introduces an ITS Kconfig option. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> xen/arch/arm/Kconfig | 5 ++++ >>> xen/arch/arm/Makefile | 1 + >>> xen/arch/arm/gic-its.c | 67 >>> +++++++++++++++++++++++++++++++++++++++++++ >>> xen/arch/arm/gic-v3.c | 6 ++++ >>> xen/include/asm-arm/gic-its.h | 57 ++++++++++++++++++++++++++++++++++++ >>> 5 files changed, 136 insertions(+) >>> create mode 100644 xen/arch/arm/gic-its.c >>> create mode 100644 xen/include/asm-arm/gic-its.h >>> >>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig >>> index 797c91f..9fe3b8e 100644 >>> --- a/xen/arch/arm/Kconfig >>> +++ b/xen/arch/arm/Kconfig >>> @@ -45,6 +45,11 @@ config ACPI >>> config HAS_GICV3 >>> bool >>> >>> +config HAS_ITS >>> + bool "GICv3 ITS MSI controller support" >>> + depends on ARM_64 >> >> HAS_GICV3 will only be selected for 64-bit. It would need some rework to >> be supported on 32-bit. So I would drop this dependency. > > OK, makes sense. > >>> + depends on HAS_GICV3 >>> + >> >> I am not convinced that we should (currently) let the user selecting the >> ITS support. It increases the test coverage (we have to test with and >> without). Do we expect people using GICv3 without ITS? > > My concern was more that if it breaks something, people can just disable > it. But I have to go through the patches again to see if disabling it > really brings us something (because thinking about it I don't think so). > > So given the test coverage argument I think we should at least enable it > by default for ARM64. Is there some "expert options" group somewhere > where we could insert the option to turn it off? You can use "if EXPERT=y", see how we handle ACPI for instance. Thinking a bit more about this, I would like to see ITS as a technical preview at the beginning. This would let us a bit of time to stabilize the code. Any opinions? Regards,
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 797c91f..9fe3b8e 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -45,6 +45,11 @@ config ACPI config HAS_GICV3 bool +config HAS_ITS + bool "GICv3 ITS MSI controller support" + depends on ARM_64 + depends on HAS_GICV3 + config ALTERNATIVE bool diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 64fdf41..c2c4daa 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -18,6 +18,7 @@ obj-$(EARLY_PRINTK) += early_printk.o obj-y += gic.o obj-y += gic-v2.o obj-$(CONFIG_HAS_GICV3) += gic-v3.o +obj-$(CONFIG_HAS_ITS) += gic-its.o obj-y += guestcopy.o obj-y += hvm.o obj-y += io.o diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c new file mode 100644 index 0000000..0f42a77 --- /dev/null +++ b/xen/arch/arm/gic-its.c @@ -0,0 +1,67 @@ +/* + * xen/arch/arm/gic-its.c + * + * ARM Generic Interrupt Controller ITS support + * + * Copyright (C) 2016 - ARM Ltd + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <xen/config.h> +#include <xen/lib.h> +#include <xen/device_tree.h> +#include <xen/libfdt/libfdt.h> +#include <asm/gic.h> +#include <asm/gic_v3_defs.h> +#include <asm/gic-its.h> + +void gicv3_its_dt_init(const struct dt_device_node *node) +{ + const struct dt_device_node *its = NULL; + struct host_its *its_data; + + /* + * Check for ITS MSI subnodes. If any, add the ITS register + * frames to the ITS list. + */ + dt_for_each_child_node(node, its) + { + paddr_t addr, size; + + if ( !dt_device_is_compatible(its, "arm,gic-v3-its") ) + continue; + + if ( dt_device_get_address(its, 0, &addr, &size) ) + panic("GICv3: Cannot find a valid ITS frame address"); + + its_data = xzalloc(struct host_its); + if ( !its_data ) + panic("GICv3: Cannot allocate memory for ITS frame"); + + its_data->addr = addr; + its_data->size = size; + its_data->dt_node = its; + + printk("GICv3: Found ITS @0x%lx\n", addr); + + list_add_tail(&its_data->entry, &host_its_list); + } +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index b8be395..238da84 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -43,9 +43,12 @@ #include <asm/device.h> #include <asm/gic.h> #include <asm/gic_v3_defs.h> +#include <asm/gic-its.h> #include <asm/cpufeature.h> #include <asm/acpi.h> +LIST_HEAD(host_its_list); + /* Global state */ static struct { void __iomem *map_dbase; /* Mapped address of distributor registers */ @@ -1229,6 +1232,9 @@ static void __init gicv3_dt_init(void) dt_device_get_address(node, 1 + gicv3.rdist_count + 2, &vbase, &vsize); + + /* Check for ITS child nodes and build the host ITS list accordingly. */ + gicv3_its_dt_init(node); } static int gicv3_iomem_deny_access(const struct domain *d) diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h new file mode 100644 index 0000000..2f5c51c --- /dev/null +++ b/xen/include/asm-arm/gic-its.h @@ -0,0 +1,57 @@ +/* + * ARM GICv3 ITS support + * + * Andre Przywara <andre.przywara@arm.com> + * Copyright (c) 2016 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __ASM_ARM_ITS_H__ +#define __ASM_ARM_ITS_H__ + +#ifndef __ASSEMBLY__ +#include <xen/device_tree.h> + +/* data structure for each hardware ITS */ +struct host_its { + struct list_head entry; + const struct dt_device_node *dt_node; + paddr_t addr; + paddr_t size; +}; + +extern struct list_head host_its_list; + +#ifdef CONFIG_HAS_ITS + +/* Parse the host DT and pick up all host ITSes. */ +void gicv3_its_dt_init(const struct dt_device_node *node); + +#else + +static inline void gicv3_its_dt_init(const struct dt_device_node *node) +{ +} + +#endif /* CONFIG_HAS_ITS */ + +#endif /* __ASSEMBLY__ */ +#endif + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */
Parse the DT GIC subnodes to find every ITS MSI controller the hardware offers. Store that information in a list to both propagate all of them later to Dom0, but also to be able to iterate over all ITSes. This introduces an ITS Kconfig option. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/Kconfig | 5 ++++ xen/arch/arm/Makefile | 1 + xen/arch/arm/gic-its.c | 67 +++++++++++++++++++++++++++++++++++++++++++ xen/arch/arm/gic-v3.c | 6 ++++ xen/include/asm-arm/gic-its.h | 57 ++++++++++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+) create mode 100644 xen/arch/arm/gic-its.c create mode 100644 xen/include/asm-arm/gic-its.h