Re: [PATCH v2 2/2][4.15] x86/AMD: expose HWCR.TscFreqSel to guests

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Mar 08, 2021 at 12:47:44PM +0100, Jan Beulich wrote:
> On 08.03.2021 12:37, Roger Pau Monné wrote:
> > On Fri, Mar 05, 2021 at 10:50:54AM +0100, Jan Beulich wrote:
> >> Linux has been warning ("firmware bug") about this bit being clear for a
> >> long time. While writable in older hardware it has been readonly on more
> >> than just most recent hardware. For simplicitly report it always set (if
> >> anything we may want to log the issue ourselves if it turns out to be
> >> clear on older hardware).
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> > One question below.
> > 
> >> ---
> >> v2: New.
> >> ---
> >> There are likely more bits worthwhile to expose, but for about every one
> >> of them there would be the risk of a lengthy discussion, as there are
> >> clear downsides to exposing such information, the more that it would be
> >> tbd whether the hardware values should be surfaced, and if so what
> >> should happen when the guest gets migrated.
> >>
> >> The main risk with making the read not fault here is that guests might
> >> imply they can also write this MSR then.
> >>
> >> --- a/xen/arch/x86/msr.c
> >> +++ b/xen/arch/x86/msr.c
> >> @@ -315,6 +315,12 @@ int guest_rdmsr(struct vcpu *v, uint32_t
> >>          *val = msrs->tsc_aux;
> >>          break;
> >>  
> >> +    case MSR_K8_HWCR:
> >> +        if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> >> +            goto gp_fault;
> >> +        *val = K8_HWCR_TSC_FREQ_SEL;
> > 
> > I've been only able to find information about this MSR up to family
> > 10h, but I think in theory Xen might also run on family 0Fh, do you
> > know if the MSR is present there, and the bit has the same meaning?
> 
> From its name (and its K7 alternative name) it's clear the register
> had been there at that point. And indeed the bit has a different
> meaning there (its the bottom bit of a 6-bit START_FID field if the
> BKDG I'm looking at can be trusted.

OK, I cannot seem to find the BKDG for family 0Fh. The oldest BKDG I
can find is for Family 10h [0].

> Since I don't think it matters
> much whether we expose a value of 0x00 or a value of 0x01 there,
> and since we likely don't want to make #GP raising dependent upon
> family when we don't _really_ need to, I would want to propose that
> the value used is good enough uniformly.

I would be fine with setting it to 0 if Fam < 10h if you think that's
acceptable. I think the chances of someone running Xen >= 4.15 on such
old hardware are quite dim.

Thanks, Roger.

[0] https://developer.amd.com/resources/developer-guides-manuals/