On Fri, Sep 29, 2023, Jim Mattson wrote:
> On Thu, Sep 28, 2023 at 5:56 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > ---
> > arch/x86/kvm/x86.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 791a644dd481..4dd64d359142 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -3700,11 +3700,19 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > data &= ~(u64)0x100; /* ignore ignne emulation enable */
> > data &= ~(u64)0x8; /* ignore TLB cache disable */
> >
> > - /* Handle McStatusWrEn */
> > - if (data & ~BIT_ULL(18)) {
> > + /*
> > + * Allow McStatusWrEn and TscFreqSel, some guests whine if they
> > + * aren't set.
> > + */
>
> The whining is only about TscFreqSel. KVM actually supports the
> functionality of McStatusWrEn (i.e. allows the guest to write to the
> MCi_STATUS MSRs).
Ugh, I missed the usage can_set_mci_status(). Stupid open coded bit numbers.
> How about...
>
> /*
> * Allow McStatusWrEn and TscFreqSel. (Linux guests from v3.2 through
> * at least v6.6 whine if TscFreqSel is clear, depending on F/M/S.
> */
>
> > + if (data & ~(BIT_ULL(18) | BIT_ULL(24))) {
> > kvm_pr_unimpl_wrmsr(vcpu, msr, data);
> > return 1;
> > }
> > +
> > + /* TscFreqSel is architecturally read-only, writes are ignored */
>
> This isn't true. TscFreqSel is not architectural at all.
Doh, right, the whole source of this mess is the uarch specific nature of the MSR.
> On Family
> 10h, per https://www.amd.com/content/dam/amd/en/documents/archived-tech-docs/programmer-references/31116.pdf,
> it was R/W and powered on as 0. In Family 15h, one of the "changes
> relative to Family 10H Revision D processors," per
> https://www.amd.com/content/dam/amd/en/documents/archived-tech-docs/programmer-references/42301_15h_Mod_00h-0Fh_BKDG.pdf,
> was:
>
> • MSRC001_0015 [Hardware Configuration (HWCR)]:
> • Dropped TscFreqSel; TSC can no longer be selected to run at NB P0-state.
>
> Despite the "Dropped" above, that same document later describes
> HWCR[bit 24] as follows:
>
> TscFreqSel: TSC frequency select. Read-only. Reset: 1. 1=The TSC
> increments at the P0 frequency
>
> So, perhaps this block of code can just be dropped? Who really cares
> if the guest changes the value (unless it goes on to kexec a new
> kernel, which will whine about the bit being clear)?
Works for me. If userspace wants to precisely emulate model specific behavior,
then userspace can darn well intercept writes to the MSR.
