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

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

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>

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?

> +        break;
> +
>      case MSR_AMD64_DE_CFG:
>          if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
>              goto gp_fault;
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -287,6 +287,8 @@
>  
>  #define MSR_K7_HWCR                  0xc0010015

We could likely drop the K7 define here, as Xen won't be able to run
on K7 hardware anymore AFAICT.

Thanks, Roger.