Re: [PATCH v2 6/8] x86/pv: disallow access to unknown MSRs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.08.2020 17:08, Roger Pau Monne wrote:
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -972,9 +972,10 @@ static int read_msr(unsigned int reg, uint64_t *val,
>          }
>          /* fall through */
>      default:
> +        gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg);
> +        break;
> +
>      normal:
> -        /* Everyone can read the MSR space. */
> -        /* gdprintk(XENLOG_WARNING, "Domain attempted RDMSR %08x\n", reg); */
>          if ( rdmsr_safe(reg, *val) )
>              break;
>          return X86EMUL_OKAY;
> @@ -1141,14 +1142,15 @@ static int write_msr(unsigned int reg, uint64_t val,
>          }
>          /* fall through */
>      default:
> -        if ( rdmsr_safe(reg, temp) )
> -            break;
> +        gdprintk(XENLOG_WARNING,
> +                 "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
> +                 reg, val);
> +        break;
>  
> -        if ( val != temp )
>      invalid:
> -            gdprintk(XENLOG_WARNING,
> -                     "Domain attempted WRMSR %08x from 0x%016"PRIx64" to 
> 0x%016"PRIx64"\n",
> -                     reg, temp, val);
> +        gdprintk(XENLOG_WARNING,
> +                 "Domain attempted WRMSR %08x from 0x%016"PRIx64" to 
> 0x%016"PRIx64"\n",
> +                 reg, temp, val);
>          return X86EMUL_OKAY;
>      }

There some odd mix of whether or no 0x prefixes get logged. I'd
advocate for dropping all of them, but imo at the very least all
three messages should be consistent in this regard.

Jan