Re: [Xen-devel] [PATCH] x86: ignore guest microcode loading attempts

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/03/18 10:13, Jan Beulich wrote:
> The respective MSRs are write-only, and hence attempts by guests to
> write to these are - as of 1f1d183d49 ("x86/HVM: don't give the wrong
> impression of WRMSR succeeding") no longer ignored. Restore original
> behavior for the two affected MSRs.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> While what is being logged for the current osstest failure on the 4.7
> branch (I have to admit that I don't understand why it's only that
> branch which shows a regression) doesn't fully prove this to be the
> problem, RCX holding 0x79 and there being a recorded hypervisor level
> #GP recovery immediately before the guest triple fault is sufficient
> indication imo.
> What I'm unsure about is whether we want to ignore such writes also for
> PV guests. If not, at least the WRMSR change would need to move into
> hvm/hvm.c.
>
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -147,6 +147,8 @@ int guest_rdmsr(const struct vcpu *v, ui
>  
>      switch ( msr )
>      {
> +    case MSR_AMD_PATCHLOADER:
> +    case MSR_IA32_UCODE_WRITE:
>      case MSR_PRED_CMD:

I've been keeping the labels in numeric order (grouped by
read/write-only status where applicable) on the expectation that you'd
insist on them being in numeric order.

>          /* Write-only */
>          goto gp_fault;
> @@ -200,6 +202,16 @@ int guest_wrmsr(struct vcpu *v, uint32_t
>          /* Read-only */
>          goto gp_fault;
>  
> +    case MSR_AMD_PATCHLOADER:
> +        if ( d->arch.cpuid->x86_vendor != X86_VENDOR_AMD )
> +            goto gp_fault;
> +        break;
> +
> +    case MSR_IA32_UCODE_WRITE:
> +        if ( d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL )
> +            goto gp_fault;

Can we leave a note here that Windows at least on some hardware loads
microcode before setting up an IDT/GDT, and will triple fault if we hand
it back #GP.  Ignoring the write means windows will see the same
microcode version after the load attempt, and conclude that it didn't
succeed?

~Andrew

> +        break;
> +
>      case MSR_SPEC_CTRL:
>          if ( !cp->feat.ibrsb )
>              goto gp_fault; /* MSR available? */
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel