On 4/11/2024 10:22 PM, Sean Christopherson wrote:
On Thu, Apr 11, 2024, Rick P Edgecombe wrote:
On Tue, 2024-04-09 at 09:26 -0700, Sean Christopherson wrote:
Haha, if this is the confusion, I see why you reacted that way to "JSON".
That would be quite the curious choice for a TDX module API.
So it is easy to convert it to a C struct and embed it in KVM. It's just not
that useful because it will not necessarily be valid for future TDX modules.
No, I don't want to embed anything in KVM, that's the exact same as hardcoding
crud into KVM, which is what I want to avoid. I want to be able to roll out a
new TDX module with any kernel changes, and I want userspace to be able to
assert
that, for a given TDX module, the effective guest CPUID configuration aligns
with
userspace's desired the vCPU model, i.e. that the value of fixed bits match up
with the guest CPUID that userspace wants to define.
Maybe that just means converting the JSON file into some binary format that
the
kernel can already parse. But I want Intel to commit to providing that
metadata
along with every TDX module.
Oof. It turns out in one of the JSON files there is a description of a different
interface (TDX module runtime interface) that provides a way to read CPUID data
that is configured in a TD, including fixed bits. It works like:
1. VMM queries which CPUID bits are directly configurable.
2. VMM provides directly configurable CPUID bits, along with XFAM and
ATTRIBUTES, via TDH.MNG.INIT. (KVM_TDX_INIT_VM)
3. Then VMM can use this other interface via TDH.MNG.RD, to query the resulting
values of specific CPUID leafs.
This does not provide a way to query the fixed bits specifically, it tells you
what ended up getting configuring in a specific TD, which includes the fixed
bits and anything else. So we need to do KVM_TDX_INIT_VM before KVM_SET_CPUID in
order to have something to check against. But there was discussion of
KVM_SET_CPUID on CPU0 having the CPUID state to pass to KVM_TDX_INIT_VM. So that
would need to be sorted.
If we pass the directly configurable values with KVM_TDX_INIT_VM, like we do
today, then the data provided by this interface should allow us to check
consistency between KVM_SET_CPUID and the actual configured TD CPUID behavior.
I think it would be a good (optional?) sanity check, e.g. KVM_BUG_ON() if the
post-KVM_TDX_INIT_VM CPUID set doesn't match KVM's internal data. But that alone
provides a terrible experience for userspace.
- The VMM would still need to hardcode knowledge of fixed bits, without a way
to do a sanity check of its own.
Maybe we can do it this way to avoid hardcode:
1. KVM can get the configurable CPUID bits from TDX module with
TDH.SYS.RD (they are the old info of TD_SYSINFO.CPUID_CONFIG[]), and
report them to userspace;
2. userspace configures the configurable CPUID bits and pass them to KVM
to init TD.
3. After TD is initialized via TDH.MNG.INIT, KVM can get a full CPUID
list of TD via TDH.MNG.RD. KVM provides interface to report the full
CPUID list to userspace.
4. Userspace can sanity check the full CPUID list.
- the configurable bits reported in #1 should be what they have been
configured;
- the dynamic bits and other special bits will be checked case by case;
- the rest bits should be fixed. If the value is not what user
wants, userspace prints error to user and stop.
Does it sounds reasonable?
- Lack of a sanity check means the VMM can't fail VM creation early.
- KVM_SET_CPUID2 doesn't have a way to inform userspace _which_ CPUID bits are
"bad".
- Neither userspace nor KVM can programming detect when bits are fixed vs.
flexible. E.g. it's not impossible that userspace would want to do X if a
feature is fixed, but Y if it's flexible.
flexible (configurable) bits is known to VMM (KVM and userspace) because
TDX module has interface to report them. So we can treat a bit as fixed
if it is not reported in the flexible group. (of course the dynamic bits
are special and excluded.)
