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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/03/2018 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)

Differences in advertised viridian?

>  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.

Hmm - I'd very much like not to make this change, because it sets a bad
precedent for making the MSR handling sane.  We shouldn't be silently
dropping MSR writes, as that will cause more problems for the guests,
rather than less.

Given that it is appears to be just 4.7 which is affected, I think it is
worth trying to work out what is causing 4.8 and later to be fine, and
whether that is a better solution to backport.

>
> --- 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:
>          /* 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 )

If we do end up going with this patch, then there is probably a cp-> in
context.  Also, I'd need to double check the indices order.

~Andrew

> +            goto gp_fault;
> +        break;
> +
> +    case MSR_IA32_UCODE_WRITE:
> +        if ( d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL )
> +            goto gp_fault;
> +        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