Re: [PATCH v2 1/7] KVM: nVMX: Keep KVM updates to BNDCFGS ctrl bits across MSR write

Paolo Bonzini
2022-02-07T18:31:19Z

On 2/7/22 19:22, Oliver Upton wrote:
> Hi Paolo,
>
> On Mon, Feb 07, 2022 at 06:21:30PM +0100, Paolo Bonzini wrote:
>> On 2/4/22 21:46, Oliver Upton wrote:
>>> Since commit 5f76f6f5ff96 ("KVM: nVMX: Do not expose MPX VMX controls
>>> when guest MPX disabled"), KVM has taken ownership of the "load
>>> IA32_BNDCFGS" and "clear IA32_BNDCFGS" VMX entry/exit controls. The ABI
>>> is that these bits must be set in the IA32_VMX_TRUE_{ENTRY,EXIT}_CTLS
>>> MSRs if the guest's CPUID supports MPX, and clear otherwise.
>>>
>>> However, KVM will only do so if userspace sets the CPUID before writing
>>> to the corresponding MSRs. Of course, there are no ordering requirements
>>> between these ioctls. Uphold the ABI regardless of ordering by
>>> reapplying KVMs tweaks to the VMX control MSRs after userspace has
>>> written to them.
>>
>> I don't understand this patch. If you first write the CPUID and then the
>> MSR, the consistency is upheld by these checks:
>>
>> if (!is_bitwise_subset(data, supported, GENMASK_ULL(31, 0)))
>> return -EINVAL;
>>
>> if (!is_bitwise_subset(supported, data, GENMASK_ULL(63, 32)))
>> return -EINVAL;
>
> Right, this works if KVM chose to clear the bit, but userspace is trying
> to set it. If KVM chose to set the bit, and userspace attempts to clear
> it, these checks would pass.

Okay, that's what I expected too but I thought it would be okay that the
checks pass. Are you trying to undo an involuntary API change, and if
so why was the change a bug and not a fix?

Paolo