Re: [PATCH] KVM: x86: Ignore MSR_AMD64_BU_CFG access

[Paolo Bonzini]

On Fri, Oct 6, 2023 at 2:44 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > We already do similar ignoring in KVM for MSR_AMD64_BU_CFG2, MSR_AMD64_DC_CFG
> > and MSR_F15H_EX_CFG, so doing this {BU_CFG2,TW_CFG} MSR filtering in QEMU would
> > be inconsistent with these.
>
> Not if QEMU filters those too. :-)
>
> The MSR filter mechanism wasn't a thing back when KVM added "support" for those
> MSR, so I don't feel that punting to userspace would be inconsistent.  It's more
> along the lines of asking/requiring userspace to utilize a new tool to solve a
> problem that is best solved in userspace, with a few outliers that got
> grandfathered in.

As long as it is enough to ignore the value and read as zero, IMO there
is an advantage in doing so in KVM, because it centralizes the update to
one place instead of requiring changes to all userspace implementations
(those that can run Windows can be counted on one hand, granted, but
still).

If we get into giving semantics to really-model-specific registers, the
disadvantages would be way more pronounced, however. I don't think we
should get any of those MSRs into KVM_GET_MSR_INDEX_LIST ever, and
we shouldn't implement them in KVM if KVM_SET_MSR has to do anything.
Also, we probably shouldn't implement them in KVM if KVM_GET_MSR
has to ever return anything but zero.

We almost have one of those, a "feature MSR" bit that is used to pass
down information on whether LFENCE serializes execution; but it's not
supported by KVM_GET_MSR/KVM_SET_MSR. I'd retcon this happily as
"we don't want any stateful chicken bit MSRs in the kernel", and I'm
in favor of removing it if there are no complaints, now that AMD has
defined a bit in 0x80000021 for this purpose.

We have the microcode revision, which we had to add because
of the side-channel mess of a few years ago.
And we also have MSR_AMD64_OSVW_ID_LENGTH and
MSR_AMD64_OSVW_STATUS, but those technically are vendor-
specific but not model-specific and have an associated CPUID bit. I
think those are as close as we get to being a mistake, but still (barely
so) on the acceptable side.

Paolo