linux-kernel - Re: [PATCH] [RFC] x86: kvm: remove KVM_SOFT_MAX

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130916114725.GA14981@hawk.usersys.redhat.com>
Date:	Mon, 16 Sep 2013 13:47:26 +0200
From:	Andrew Jones <drjones@...hat.com>
To:	Gleb Natapov <gleb@...hat.com>
Cc:	kvm@...r.kernel.org, pbonzini@...hat.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [RFC] x86: kvm: remove KVM_SOFT_MAX_VCPUS

On Mon, Sep 16, 2013 at 11:55:17AM +0300, Gleb Natapov wrote:
> On Mon, Sep 16, 2013 at 10:22:09AM +0200, Andrew Jones wrote:
> > > > [1] Actually, until 972fc544b6034a in uq/master is merged there won't be
> > > >     any warnings either.
> > > > 
> > > > Signed-off-by: Andrew Jones <drjones@...hat.com>
> > > > ---
> > > >  arch/x86/include/asm/kvm_host.h | 1 -
> > > >  arch/x86/kvm/x86.c              | 2 +-
> > > >  2 files changed, 1 insertion(+), 2 deletions(-)
> > > > 
> > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > > index c76ff74a98f2e..9236c63315a9b 100644
> > > > --- a/arch/x86/include/asm/kvm_host.h
> > > > +++ b/arch/x86/include/asm/kvm_host.h
> > > > @@ -32,7 +32,6 @@
> > > >  #include <asm/asm.h>
> > > >  
> > > >  #define KVM_MAX_VCPUS 255
> > > > -#define KVM_SOFT_MAX_VCPUS 160
> > > >  #define KVM_USER_MEM_SLOTS 125
> > > >  /* memory slots that are not exposed to userspace */
> > > >  #define KVM_PRIVATE_MEM_SLOTS 3
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index e5ca72a5cdb6d..d9d3e2ed68ee9 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -2604,7 +2604,7 @@ int kvm_dev_ioctl_check_extension(long ext)
> > > >  		r = !kvm_x86_ops->cpu_has_accelerated_tpr();
> > > >  		break;
> > > >  	case KVM_CAP_NR_VCPUS:
> > > > -		r = KVM_SOFT_MAX_VCPUS;
> > > > +		r = min(num_online_cpus(), KVM_MAX_VCPUS);
> > > s/KVM_MAX_VCPUS/KVM_SOFT_MAX_VCPUS/.  Also what about hotplug cpus?
> > 
> > I'll send a v2 with this change.
> > 
> > I thought a bit about hotplug, and thus using num_possible_cpus()
> > instead, but then decided it made more sense to stick to what's online now
> > for the recommended number. It's just a recommendation anyway. So as long
> > as KVM_MAX_VCPUS is >= num_possible_cpus(), then one can still configure
> > more vcpus to count for all hotplugable cpus, if they wish.
> > 
> It is just recommended, but we do warn about it, so it is user visible.
> Well, the whole point of it existence is to be user visible ;). If user
> creates a guest with max cpus greater than current number if online
> cpus, taking into account feature grows, he will get a warning, but we
> should not warn about it.

Even it if means the user may end up running, e.g. 128 vcpus on 96 pcpus
indefinitely? I'd rather warn about it, which could remind them to offline
32 vcpus for the time being. Although, as we're just discussing when or
when not to output a warning, then I'm not really stressed about it either
way. I can certainly change this to num_possible_cpus(), if all are in
agreement that that is a better recommendation.

drew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/