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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/03/18 11:13, Jan Beulich wrote:
>>>> On 15.03.18 at 12:03, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 15/03/18 10:40, Jan Beulich wrote:
>>>>> @@ -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.
>>> Will do.
>>>
>>>>  Ignoring the write means windows will see the same
>>>> microcode version after the load attempt, and conclude that it didn't
>>>> succeed?
>>> That's what I imply. After all things have worked before, where
>>> we also silently dropped these writes.
>> Actually, on further investigation, we've always had a read_safe() test
>> for PV, which means that PV guests have always unilaterally seen #GP. 
>> Can we retain that behaviour please?
> Well, that's exactly what I wanted to gather opinions on by having
> written:
>
> "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."
>
> I've just sent v2, but I can certainly send v3 with the WRMSR side
> code moved (to be honest I'm not convinced we want all sorts of
> is_{hvm,pv}_{domain,vcpu}() checks in guest_{rd,wr}msr(), but if
> that was your plan, then the code could also stay where it is).

Yeah - I noticed and replied there.  If you agree with my suggestion
then there is no need to post a v3.

I was always expecting to have some pv/hvm split in the
guest_{rd,wr}msr() side of things, similar to how we do the legacy CPUID
handling.

However, I want everything to be implemented in guest_{rd,wr}msr() to
set the expectation that new additions are symmetric WRT guest types,
and I hope that the overwhelming majority of code will be based on
proper architectural information in the CPUID/MSR policies, rather than
on domain type specifically.

~Andrew

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