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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/03/18 11:36, Jan Beulich wrote:
>>>> On 15.03.18 at 12:14, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 15/03/18 11:09, 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>
>>> ---
>>> v2: Add comments.
>>> ---
>>> While what is being logged for the current osstest failures on the 4.7
>>> and 4.9 branches 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.
>> Sorry - I've raced with v2 of your patch.  The PV side currently always
>> yields #GP, because we have a read_safe() check on the wrmsr side.  With
>> that behaviour retained (i.e. with !is_pv_domain() checks in the hunks
>> below), Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Just to be sure we're on the same page (as the ! above is not what
> I've meant to do), this is the hunk in question:

LGTM.

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

>
> @@ -200,6 +202,28 @@ int guest_wrmsr(struct vcpu *v, uint32_t
>          /* Read-only */
>          goto gp_fault;
>  
> +    case MSR_AMD_PATCHLOADER:
> +        /*
> +         * See note on MSR_IA32_UCODE_WRITE below, which may or may not apply
> +         * to AMD CPUs as well (at least the architectural/CPUID part does).
> +         */
> +        if ( is_pv_domain(d) ||
> +             d->arch.cpuid->x86_vendor != X86_VENDOR_AMD )
> +            goto gp_fault;
> +        break;
> +
> +    case MSR_IA32_UCODE_WRITE:
> +        /*
> +         * Some versions of Windows at least on certain hardware try to load
> +         * microcode before setting up an IDT. Therefore we must not inject 
> #GP
> +         * for such attempts. Also the MSR is architectural and not qualified
> +         * by any CPUID bit.
> +         */
> +        if ( is_pv_domain(d) ||
> +             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