Re: [RFC PATCH 02/22] x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.10.2023 21:29, Edwin Török wrote:
> Dom0 should always be able to read this MSR: it is useful when
> investigating performance issues in production.

While I'm not outright opposed, I'm also not convinced. At the very least
...

> Although the count is Thread scoped, in practice all cores were observed
> to return the same count (perhaps due to implementation details of SMM),
> so do not require the cpu to be pinned in order to read it.

... this, even if matching your observations, is going to be properly
misleading in case counts end up diverging.

> This MSR exists on Intel since Nehalem.
> 
> Backport: 4.15+

If this was a backporting candidate, I think a Fixes: tag would need
to indicate what's being fixed here. Otherwise this is merely a new
feature.

> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -641,6 +641,9 @@
>  #define MSR_NHL_LBR_SELECT           0x000001c8
>  #define MSR_NHL_LASTBRANCH_TOS               0x000001c9
>  
> +/* Nehalem and newer other MSRs */
> +#define MSR_SMI_COUNT                       0x00000034

See my comment on the other patch regarding additions here.

> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -139,6 +139,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t 
> *val)
>          *val = msrs->misc_features_enables.raw;
>          break;
>  
> +    case MSR_SMI_COUNT:
> +        if ( cp->x86_vendor != X86_VENDOR_INTEL )
> +            goto gp_fault;
> +        if ( is_hardware_domain(d) && !rdmsr_safe(msr, *val) )
> +            break;
> +        return X86EMUL_UNHANDLEABLE;

Why #GP for non-Intel but UNHANDLEABLE for DomU?

Jan