Re: [Xen-devel] [PATCH 1/6] arm: make it possible to disable more kconfig options

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 19 Apr 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 18/04/18 23:15, Stefano Stabellini wrote:
> > Make it possible to disable the following existing kconfig options:
> >    HAS_GICV3
> >    HAS_ARM_HDLCD
> >    HAS_MEM_ACCESS
> > 
> > Today they are silent option. This patch adds one line descriptions and
> > make them de/selectable.
> > 
> > Also, do not select VIDEO: make HAS_ARM_HDLCD select VIDEO instead. In
> > fact, VIDEO is only needed by HAS_ARM_HDLCD.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > ---
> >   xen/arch/arm/Kconfig      | 15 +++++++++++----
> >   xen/drivers/video/Kconfig |  8 +++++++-
> >   2 files changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 8174c0c..83963c8 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -12,17 +12,13 @@ config ARM_32
> >   config ARM_64
> >     def_bool y
> >     depends on 64BIT
> > -   select HAS_GICV3
> >     config ARM
> >     def_bool y
> >     select HAS_ALTERNATIVE
> > -   select HAS_ARM_HDLCD
> >     select HAS_DEVICE_TREE
> > -   select HAS_MEM_ACCESS
> >     select HAS_PASSTHROUGH
> >     select HAS_PDX
> > -   select VIDEO
> >     config ARCH_DEFCONFIG
> >     string
> > @@ -44,6 +40,17 @@ config ACPI
> >     config HAS_GICV3
> >     bool
> > +   prompt "GICv3 driver"
> > +   default y
> 
> That's quite different for the existing config. Now you select GICv3 for arm32
> which we know does not work.

Ah! I'll fix that.


> Also, to bike-shed a bit, I feel HAS_* is more to say "we support" over this
> is selectable by the user. So probably some renaming is required here.

OK, should I just remove the HAS_ prefix (for example HAS_GICV3 ->
GICV3) or do you have a better suggestion? In any case, I'll do in a
separate patch to make it easier to review.


> > +
> > +config HAS_MEM_ACCESS
> > +   bool
> > +   prompt "Memory Access and VM events"
> > +   default y
> 
> Most of the memaccess code is not protected by HAS_MEM_ACCESS.  So you are
> going to drop just a couple of hundreds line. Not sure if it is worth it in
> the actual state.

Yes, the LOC count it is not worth it today, but I would still like to
make it selectable because I don't want Xen to come to rely on having
HAS_MEM_ACCESS enabled all the time. !MEM_ACCESS is a good and valid
configuration. Also, we can go forward in making more stuff protected by
HAS_MEM_ACCESS soon.


> > +   ---help---
> > +
> > +     Framework to configure memory access types for guests and receive
> > +     related events in userspace.
> >     config HAS_ITS
> >           bool
> > diff --git a/xen/drivers/video/Kconfig b/xen/drivers/video/Kconfig
> > index 52e8ce6..26daf9a 100644
> > --- a/xen/drivers/video/Kconfig
> > +++ b/xen/drivers/video/Kconfig
> > @@ -13,4 +13,10 @@ config VGA
> >       If unsure, say Y.
> >     config HAS_ARM_HDLCD
> 
> To be honest I would just rip off the driver. I doubt anybody has been using
> it for the past 5 years and only targets vexpress.

It is true, the only reason for keeping it would be as a reference
example, but I don't know if it is worth having it in the tree just for
that. I'll remove it completely, unless you tell me otherwise.


> > -   bool
> > +   bool "ARM HDLCD driver"
> > +   default n
> > +   select VIDEO
> > +   ---help---
> > +     Enable Xen video output to ARM HDLCD graphic controllers.
> > +
> > +     if unsure, say N.
> > 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel