Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 1 Apr 2023 10:00:40 +0300
From:   Zhi Wang <zhi.wang.linux@gmail.com>
To:     Isaku Yamahata <isaku.yamahata@gmail.com>
Cc:     isaku.yamahata@intel.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        erdemaktas@google.com, Sean Christopherson <seanjc@google.com>,
        Sagi Shahar <sagis@google.com>,
        David Matlack <dmatlack@google.com>,
        Kai Huang <kai.huang@intel.com>
Subject: Re: [PATCH v13 018/113] KVM: x86, tdx: Make KVM_CAP_MAX_VCPUS
 backend specific
Message-ID: <20230401100040.0000515c@gmail.com>
In-Reply-To: <20230329233258.GC1108448@ls.amr.corp.intel.com>
References: <cover.1678643051.git.isaku.yamahata@intel.com>
        <87f6baf8419103077f0a42859a0a847c695f5f59.1678643052.git.isaku.yamahata@intel.com>
        <20230325201326.00002d4d@gmail.com>
        <20230329233258.GC1108448@ls.amr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 29 Mar 2023 16:32:58 -0700
Isaku Yamahata <isaku.yamahata@gmail.com> wrote:

> On Sat, Mar 25, 2023 at 08:13:26PM +0200,
> Zhi Wang <zhi.wang.linux@gmail.com> wrote:
> 
> > On Sun, 12 Mar 2023 10:55:42 -0700
> > isaku.yamahata@intel.com wrote:
> > 
> > > From: Isaku Yamahata <isaku.yamahata@intel.com>
> > > 
> > > TDX has its own limitation on the maximum number of vcpus that the guest
> > > can accommodate.  Allow x86 kvm backend to implement its own KVM_ENABLE_CAP
> > > handler and implement TDX backend for KVM_CAP_MAX_VCPUS.  user space VMM,
> > > e.g. qemu, can specify its value instead of KVM_MAX_VCPUS.
> > > 
> > 
> > I think enabling the cap here is actually "configuring the cap". KVM_CAP_MAX
> > _VCPUS is actually always enabled whether userspace enables it or not. It
> > would be nice to configure of the max VCPUS in kvm_arch_vm_ioctl() where
> > routines of configuring a VM should belong. E.g. KVM_SET_MAX_VCPUS.
> 
> I'm not sure I understand your point.  Although KVM_ENABLE_CAP sounds like
> only on/off feature, but it isn't. It's also used to set parameters. For
> example, KVM_CAP_MAX_VCPU_ID.
>

Yes, I understand your point. But what has been there might not be right as
well. That doesn't smell right as "enable" is for something which is previously
"disabled". I understand that there can be some caps require configuration
when being enabled. But later, for those caps which don't have "on/off"
state, KVM_ENABLE_CAP doesn't actually enable a feature, it is just
configuring a feature. It seems like the KVM_ENABLE_CAP has been mis-used
little by little in the history. Also, I don't find KVM_DISABLE_CAP
accordingly. So KVM_ENABLE_CAP is actually used as "KVM_SET_CAP". 

I realize it is not realistic to solve the problem in this patch series.
You can keep the current approach. 

> KVM_SET_XXX is for run time feature. But the maxium number of vcpus is not
> runtime changable. Set it at vm creation.