On Wed, Jul 21, 2021, Tom Lendacky wrote:...
On 7/20/21 7:01 PM, Sean Christopherson wrote:
On Wed, Jul 07, 2021, Brijesh Singh wrote:
From: Tom Lendacky <thomas.lendacky@xxxxxxx>
This seems like it's missing a big pile of sanity checks. E.g. KVM should reject
SVM_VMGEXIT_AP_CREATE if the target vCPU is already "created", including the case
where it was "created_on_init" but hasn't yet received INIT-SIPI.
Why? If the guest wants to call it multiple times I guess I don't see a
reason that it would need to call DESTROY first and then CREATE. I don't
know why a guest would want to do that, but I don't think we should
prevent it.
Because "creating" a vCPU that already exists is non-sensical. Ditto for
onlining a vCPU that is already onlined. E.g. from the guest's perspective, I
would fully expect a SVM_VMGEXIT_AP_CREATE to fail, not effectively send the vCPU
to an arbitrary state.
Any ambiguity as to the legality of CREATE/DESTROY absolutely needs to be clarified
in the GHCB.
+
+ target_svm->snp_vmsa_gpa = 0;
+ target_svm->snp_vmsa_update_on_init = false;
+
+ /* Interrupt injection mode shouldn't change for AP creation */
+ if (request < SVM_VMGEXIT_AP_DESTROY) {
+ u64 sev_features;
+
+ sev_features = vcpu->arch.regs[VCPU_REGS_RAX];
+ sev_features ^= sev->sev_features;
+ if (sev_features & SVM_SEV_FEATURES_INT_INJ_MODES) {
Why is only INT_INJ_MODES checked? The new comment in sev_es_sync_vmsa() explicitly
states that sev_features are the same for all vCPUs, but that's not enforced here.
At a bare minimum I would expect this to sanity check SVM_SEV_FEATURES_SNP_ACTIVE.
That's because we can't really enforce it. The SEV_FEATURES value is part
of the VMSA, of which the hypervisor has no insight into (its encrypted).
The interrupt injection mechanism was specifically requested as a sanity
check type of thing during the GHCB review, and as there were no
objections, it was added (see the end of section 4.1.9).
I can definitely add the check for the SNP_ACTIVE bit, but it isn't required.
I'm confused. If we've no insight into what the guest is actually using, what's
the point of the INT_INJ_MODES check?