Re: [Xen-devel] [RFC 27/29] build: convert HAS_GICV3 use to Kconfig

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-10-06 at 10:56 +0100, George Dunlap wrote:
> On Mon, Oct 5, 2015 at 11:25 PM, Julien Grall <julien.grall@xxxxxxxxxx>
> wrote:
> > Hi,
> > 
> > On 05/10/2015 23:03, Doug Goldstein wrote:
> > > 
> > > Use the Kconfig generated CONFIG_HAS_GICV3 defines in the code base.
> > 
> > 
> > If you are going to rename all HAS_* to CONFIG_HAS_, please drop the
> > HAS
> > which is now redundant.
> > 
> > > 
> > > Signed-off-by: Doug Goldstein <cardoe@xxxxxxxxxx>
> > > ---
> > >   xen/arch/arm/Kconfig         | 4 ++++
> > >   xen/arch/arm/Makefile        | 2 +-
> > >   xen/arch/arm/Rules.mk        | 2 --
> > >   xen/arch/arm/vgic.c          | 2 +-
> > >   xen/include/asm-arm/domain.h | 3 ++-
> > >   xen/include/asm-arm/gic.h    | 4 ++--
> > >   xen/include/asm-arm/vgic.h   | 2 +-
> > >   7 files changed, 11 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > index f100f17..01744c7 100644
> > > --- a/xen/arch/arm/Kconfig
> > > +++ b/xen/arch/arm/Kconfig
> > > @@ -28,6 +28,10 @@ config ARCH_DEFCONFIG
> > >         default "arch/arm/arm32_defconfig" if ARM_32
> > >         default "arch/arm/arm64_defconfig" if ARM_64
> > > 
> > > +# Select HAS_GICV3 if Generic Interrupt Connect (GICv3) is supported
> > 
> > 
> > s/Connect/Controller/ although saying GICv3 is enough. No need to spell
> > out
> > the acronym.
> > 
> > If you really want to spell it it should be Generic Interrupt
> > Controller v3.
> 
> Do remember that those comments are for users, not developers.

They really shouldn't be, the choice of inclusion of GICv3 is a choice for
the (sub)arch maintainer, not the end user.

And this is is a code comment in the Kconfig, not the help text for a user
facing option, so Doug hasn't actually made it a user facing option, which
is good and proper.

When this switch was first proposed I raised a concern about the
proliferation of user facing options and therefore of Xen binaries which
support different sets of h/w or feature support. I remain concerned about
that and your comment seems to me to be the first instance of that worry
becoming realised, even if it was based on a misunderstanding of where the
comment would appear.

>   *You*
> may know without thinking that "GIC" is an interrupt controller, but
> many users would appreciate at least a hint so that they know whether
> they can safely ignore it or go look further into it.

No user of Xen should ever be asked to make this decision.

Ian.

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