On Wed, Sep 13, 2023 at 08:18:57AM +0200, Jan Beulich wrote:
> On 12.09.2023 18:35, Andrew Cooper wrote:
> > On 12/09/2023 5:23 pm, Roger Pau Monne wrote:
> >> --- a/xen/arch/x86/msr.c
> >> +++ b/xen/arch/x86/msr.c
> >> @@ -240,8 +240,12 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr,
> >> uint64_t *val)
> >> case MSR_K8_HWCR:
> >> if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> >> goto gp_fault;
> >> - *val = get_cpu_family(cp->basic.raw_fms, NULL, NULL) >= 0x10
> >> - ? K8_HWCR_TSC_FREQ_SEL : 0;
> >> + /*
> >> + * OpenBSD 7.3 accesses HWCR unconditionally if the TSC is
> >> reported as
> >> + * Invariant. Do not set TSC_FREQ_SEL as that would trigger
> >> OpenBSD to
> >> + * also poke at PSTATE0.
> >> + */
> >
> > While this is true, the justification for removing this is because
> > TSC_FREQ_SEL is a model specific bit, not an architectural bit in HWCR.
> >
> > Also because it's addition without writing into the migration stream was
> > bogus irrespective of the specifics of the bit.
> >
> > I'm still of the opinion that it's buggy for OpenBSD to be looking at
> > model specific bits when virtualised, but given my latest reading of the
> > AMD manuals, I think OpenBSD *is* well behaved looking at PSTATE0 if it
> > can see TSC_FREQ_SEL.
>
> I'm afraid I'm still at a loss seeing what wording in which PPR makes you
> see a connection. If there was a connection, I'd like to ask that we at
> least properly consider exposing PSTATE0 (or if necessary all PSTATEn) as
> well, with zero value (i.e. in particular with the valid bit clear),
> rather than exposing a r/o bit with the wrong value, upsetting Linux.
But PSTATEn is also non-architectural, so the bit meaning could in
theory change between models.
There seems to be no bit anywhere that signals whether PSTATEn is
available, as it's my reading that you can have PSTATEn without the
architectural PSTATE_{LIMIT,CTRL,STATUS} MSRs that are signaled by
HW_PSTATE CPUID bit.
Thanks, Roger.
