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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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