On 3/31/2023 8:44 PM, Zhi Wang wrote:
On Thu, 30 Mar 2023 17:18:03 -0700
Isaku Yamahata <isaku.yamahata@xxxxxxxxx> wrote:
On Wed, Mar 29, 2023 at 04:17:22PM -0700,
Isaku Yamahata <isaku.yamahata@xxxxxxxxx> wrote:
On Sat, Mar 25, 2023 at 10:43:06AM +0200,
Zhi Wang <zhi.wang.linux@xxxxxxxxx> wrote:
On Sun, 12 Mar 2023 10:55:40 -0700
isaku.yamahata@xxxxxxxxx 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?
We only need global parameters of the TDX module, and we don't interact with TDX
module at this point. One alternative is to export those parameters via sysfs.
Also the existence of the sysfs node indicates that the TDX module is
loaded(initialized?) or not in addition to boot log. Thus we can drop system
scope one.
What do you think?
I like this idea and the patch below, it feels right for me now. It would be nice
if more folks can chime in and comment.
SYSFS option requires CONFIG_SYSFS, which reqiures CONFIG_KVM_TDX to
select CONFIG_SYSFS.
Regarding to other TDX KVM specific ioctls (KVM_TDX_INIT_VM, KVM_TDX_INIT_VCPU,
KVM_TDX_INIT_MEM_REGION, and KVM_TDX_FINALIZE_VM), they are specific to KVM. So
I don't think it can be split out to independent driver.
They can stay in KVM as they are KVM-specific. SNP also has KVM-specific ioctls
which wraps the SEV driver calls. At this level, both TDX and SNP go their specific
implementation without more abstraction other than KVM_ENCRYPT_MEMORY_OP. Their
strategies are aligned.
The problem of the previous approach was the abstraction that no other implementation
is using it. It is like, TDX wants a higher abstraction to cover both TDX and SNP,
but SNP is not using it, which makes the abstraction looks strange.
Note, before this TDX enabling series, KVM_MEMORY_ENCRYPT_OP is a VM
scope ioctl, that only serves for SEV and no other implementation uses
it. I see no reason why cannot introduce a new IOCTL in x86 KVM that
serves only one vendor.
We choose KVM_MEMORY_ENCRYPT_OP for TDX platform scope, just because we
reuse KVM_MEMORY_ENCRYPT_OP for TDX VM-scope and extend it to TDX vcpu
scope. It's just to avoid defining a new IOCTL number.
We can rename it to KVM_GET_CC_CAPABILITIES, and even return different
capabilities based on VM type. And even, if SNP wants to use it, I think
it can wrap SNP driver calls inside this IOCTL?
kvm.ko is special that it needs to serve two vendors. Sometime it's
unaviodable that an interface is only used by one vendor.
