On Thu, Jun 23, 2022 at 04:47:22PM +0200, Jan Beulich wrote:
> On 23.06.2022 10:24, Roger Pau Monne wrote:
> > Allow selecting the default x2APIC destination mode from Kconfig.
> > Note the default destination mode is still Logical (Cluster) mode.
> > 
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> > ---
> >  xen/arch/x86/Kconfig          | 29 +++++++++++++++++++++++++++++
> >  xen/arch/x86/genapic/x2apic.c |  6 ++++--
> >  2 files changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> > index 1e31edc99f..f560dc13f4 100644
> > --- a/xen/arch/x86/Kconfig
> > +++ b/xen/arch/x86/Kconfig
> > @@ -226,6 +226,35 @@ config XEN_ALIGN_2M
> >  
> >  endchoice
> >  
> > +choice
> > +   prompt "x2APIC default destination mode"
> 
> What's the point of using "choice" here, and not a single "bool"?

I think choice better reflects the purpose of the option, it's
selecting between two different modes.  It could be expressed with a
bool, but I think it's less clear.

> > +   default X2APIC_LOGICAL
> > +   ---help---
> 
> Nit: Please don't use ---help--- anymore - we're trying to phase out its
> use as Linux has dropped it altogether (and hence once we update our
> Kconfig, we'd like to change as few places as possible), leaving just
> "help".
> 
> One downside of "choice" (iirc) is that the individual sub-options' help
> text is inaccessible from at least the command line version of kconfig.

Hm, I usually use menuconfig when wanting to poke at options help.

I guess I could introduce a single X2APIC_PHYSICAL bool that starts
with default false and notes that otherwise the destination mode is
logical.

> > +     Specify default destination mode for x2APIC.
> > +
> > +     If unsure, choose "Logical".
> > +
> > +config X2APIC_LOGICAL
> > +   bool "Logical mode"
> > +   ---help---
> > +     Use Logical Destination mode.
> > +
> > +     When using this mode APICs are addressed using the Logical
> > +     Destination mode, which allows for optimized IPI sending,
> > +     but also reduces the amount of vectors available for external
> > +     interrupts when compared to physical mode.
> > +
> > +config X2APIC_PHYS
> 
> X2APIC_PHYSICAL (to be in line with X2APIC_LOGICAL)?

Right, was about to expand it but did consider PHYS to be clear enough
(opposed to using LOG or LOGIC), will expand in next version.

Thanks, Roger.

Reply via email to