linux-kernel - RE: [PATCH] Drivers: hv: Kconfig: select CPUMASK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <SN6PR02MB41573212FCDBDF58305881ECD4512@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Mon, 19 Feb 2024 17:53:25 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Saurabh Singh Sengar <ssengar@...ux.microsoft.com>
CC: "kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
	<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
	"decui@...rosoft.com" <decui@...rosoft.com>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "ssengar@...rosoft.com"
	<ssengar@...rosoft.com>
Subject: RE: [PATCH] Drivers: hv: Kconfig: select CPUMASK_OFFSTACK for Hyper-V

From: Saurabh Singh Sengar <ssengar@...ux.microsoft.com> Sent: Saturday, February 17, 2024 11:17 PM
> > >
> > > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > > index 0024210..bc3f496 100644
> > > --- a/drivers/hv/Kconfig
> > > +++ b/drivers/hv/Kconfig
> > > @@ -9,6 +9,7 @@ config HYPERV
> > >  	select PARAVIRT
> > >  	select X86_HV_CALLBACK_VECTOR if X86
> > >  	select OF_EARLY_FLATTREE if OF
> > > +	select CPUMASK_OFFSTACK
> > >  	help
> > >  	  Select this option to run Linux as a Hyper-V client operating
> > >  	  system.
> > > --
> > > 1.8.3.1
> > >
> >
> > I'm not sure that enabling CPUMASK_OFFSTACK for Hyper-V
> > guests is the right thing to do, as there's additional runtime
> > cost when CPUMASK_OFFSTACK is enabled.  I agree that for
> > the most general case, you want NR_CPUS to be 2048, which
> > requires CPUMASK_OFFSTACK.  But it would be legitimate to
> > build a kernel with NR_CPUS set to something like 64 or 256
> > for a more limited Hyper-V guest use case, and to not want to
> > incur the cost of CPUMASK_OFFSTACK.
> >
> > You could consider doing something like this:
> >
> > 	select CPUMASK_OFFSTACK if NR_CPUS > 512
> 
> Thanks for your review.
> 
> This was my first thought as well, but for x86, NR_CPUS itself depends
> on CPUMASK_OFFSTACK and this creates some kind of circular dependency
> and doesn't work effectively.
> 
> Here are few key points to note:
> 
> 1. In ARM64 as well for enabling CPUMASK_OFFSTACK we need to enable
>    DEBUG_PER_CPU_MAPS and that will have additional overhead.
>    This dependency is for all the archs. There was an earlier attempt
>    to decouple it: https://lore.kernel.org/lkml/20220412231508.32629-1-libo.chen@oracle.com/ 
> 
> 2. However, for ARM64, NR_CPUS doesn't have dependency on CPUMASK_OFFSTACK.
>    In ARM64 NR_CPUS is quite independent from any policy, we can choose any
>    value for NR_CPUS freely, things are simple. This problem specificaly
>    to be solved for x86.
> 
> 3. If we have to select more then 512 CPUs on x86, CPUMASK_OFFSTACK
>    needto be enabled, so this additional runtime cost is unavoidable
>    for NR_CPUS > 512. There is no way today to enable CPUMASK_OFFSTACK
>    apart from enabling MAXSMP or DEBUG_PER_CPU_MAPS. Both of these
>    options we don't want to use.
> 
> I agree that we possibly don't want to enable this option for HyperV VMs
> where NR_CPUS < 512. I have two thoughts here:
> 
> 1. Enable it only for VTL platforms, as current requirement for minimal kernel
>    is only for VTL platforms only.
> 
> 2. Fix this for all of x86. I couldn't find any reson why CPUMASK_OFFSTACK
>    dependency is there on x86 for having more than 512 CPUs. What is special
>    in x86 to have this restriction ? If there is no reason we should relax
>    the restriction of CPUMASK_OFFSTACK for NR_CPUs similar to ARM and other
>    archs.
> 

You've done some deeper research than I did. :-(  What a mess.

ARM64 seems to have it right.  On x86, the dependency between NR_CPUS
and CPUMASK_OFFSTACK seems to flow the wrong direction. I would think
you would select NR_CPUS first, and then if the number is large, select
CPUMASK_OFFSTACK.

And the display of CPUMASK_OFFSTACK in config tools should not be
dependent on DEBUG_PER_CPU_MAPS.   It should be easy to independently
select CPUMASK_OFFSTACK (modulo architectures that don't support it).
In the Libo Chen thread, I don't understand the reluctance to make
CPUMASK_OFFSTACK independent of DEBUG_PER_CPU_MAPS.

I don't have any great suggestions for the path forward. :-(  Maybe
revive the Libo Chen thread, with a better justification for removing
the dependency between CPUMASK_OFFSTACK and
DEBUG_PER_CPU_MAPS?  Or at least clarify why the dependency
should be kept?

Michael