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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.18 at 12:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

Okay, I'll add conditionals and commit.

Jan


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