Re: [Xen-devel] [RFC PATCH 01/24] ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxx>
> > > ---
> > >  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,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel