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:   Fri, 31 Mar 2023 15:31:07 +0300
From:   Zhi Wang <zhi.wang.linux@gmail.com>
To:     Xiaoyao Li <xiaoyao.li@intel.com>
Cc:     isaku.yamahata@intel.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com,
        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>,
        Sean Christopherson <sean.j.christopherson@intel.com>
Subject: Re: [PATCH v13 016/113] KVM: TDX: x86: Add ioctl to get TDX
 systemwide parameters
Message-ID: <20230331153107.0000029d@gmail.com>
In-Reply-To: <fc642000-7db8-9944-e57e-db54f0d1336d@intel.com>
References: <cover.1678643051.git.isaku.yamahata@intel.com>
        <cb0ae8b4941aaa45e1e5856dde644f9b2f53d9a6.1678643052.git.isaku.yamahata@intel.com>
        <20230325104306.00004585@gmail.com>
        <fc642000-7db8-9944-e57e-db54f0d1336d@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 31 Mar 2023 14:59:18 +0800
Xiaoyao Li <xiaoyao.li@intel.com> wrote:

> On 3/25/2023 4:43 PM, Zhi Wang wrote:
> > On Sun, 12 Mar 2023 10:55:40 -0700
> > isaku.yamahata@intel.com  wrote:
> > 
> > Does this have to be a new generic ioctl with a dedicated new x86_ops? SNP
> > does not use it at all and all the system-scoped ioctl of SNP going through
> > the CCP driver. So getting system-scope information of TDX/SNP will end up
> > differently.
> > 
> > Any thought, Sean? Moving getting SNP system-wide information to
> > KVM dev ioctl seems not ideal and TDX does not have a dedicated driver like
> > CCP. Maybe make this ioctl TDX-specific? KVM_TDX_DEV_OP?
> 
> What's the real different of it? For me, it's just renaming 
> KVM_MEMORY_ENCRYPT_OP to KVM_TDX_DEV_OP and maybe add some error message 
> if the IOCTL is issued for AMD plaform.
> 

Hi:

The ioctl is the API for the userspace. The purpose is to be orthogonal,
avoid confusion and reflect its nature. A "generic" name with only one
implementation is fine in the early design. But if the other implementation
at the same level is pretty sure not going to use it, then the abstraction,
which is only abstracted for one implementation, is just confusing.

The possible strategies are:

1) Re-factor the other implementation to fit the current abstraction.
2) Give up the abstraction. Go "specific".

For 1), it seems not realistic due to the efforts of re-factoring the SEV
driver.

For 2), there can be several ways: a. renaming it, let the name reflect
its nature. IMO, KVM_TDX_DEV_OP is not ideal as well, but I don't have a
better one. b. moving it to a proper layer of the implementation. But
it is also not realistic to have a "TDX" driver because of it. That's why
I am torn here.