Re: [Xen-devel] [PATCH] x86/cpuid: fix dom0 crash on skylake machine

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.06.16 at 13:45, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 01/06/16 12:38, Jan Beulich wrote:
>>>>> On 01.06.16 at 13:27, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 01/06/16 10:43, Jan Beulich wrote:
>>>>>>> On 01.06.16 at 11:34, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> On 01/06/16 10:17, Jan Beulich wrote:
>>>>>>>>> On 01.06.16 at 11:03, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>> While this does work, it undoes some of the work I started with my cpuid
>>>>>>> improvements in 4.7
>>>>>>>
>>>>>>> Does the attached patch also resolve your issue?
>>>>>> While that's much better than the original, I don't think it's quite
>>>>>> enough. The rest of the domain policy should be taken into account
>>>>>> (and I think I had suggested to do so during review of your CPUID
>>>>>> rework series), i.e. this can't be calculated once for every domain.
>>>>> Like the current use of {hvm,pv}_featureset, as an upper bound, this is
>>>>> just a stopgap fix.
>>>>>
>>>>> Fixing this in a properly per-domain way is part of my further plans for
>>>>> cpuid improvements.  The reason it isn't done like this yet is because
>>>>> there is a substantial quantity of work required to make this function.
>>>> What substantial work other than XSTATE leaf handling inquiring
>>>> other leaves do you see?
>>> I want to adjust the representation of cpuid information in struct
>>> domain. The current loop in domain_cpuid() causes an O(N) overhead for
>>> every query, which is very poor for actions which really should be a
>>> single bit test at a fixed offset.
>>>
>>> This needs to be combined with properly splitting the per-domain and
>>> per-vcpu information, which requires knowing the expected vcpu topology
>>> during domain creation.
>>>
>>> On top of that, there needs to be verification logic to check the
>>> correctness of information passed from the toolstack.
>>>
>>> All of these areas are covered in the "known issues" section of the
>>> feature doc, and I do plan to fix them all.  However, it isn't a couple
>>> of hours worth of work.
>> All understood, yet not to the point: The original remark was that
>> the very XSTATE handling could be done better with far not as much
>> of a change, at least afaict without having tried.
> 
> In which case I don't know what you were suggesting.

Make {hvm,pv}_cpuid() invoke themselves recursively to
determine what bits to mask off from CPUID[0xd].EAX.

>>> I am also wondering whether we should disable MPX by default for
>>> domains.  We know it won't work properly for anything which ends up
>>> being emulated.  Users who specifically want to try it should be able to
>>> turn it on in their domain configuration file, but it really shouldn't
>>> be on my default.
>> For guests being introspected I agree. For others I'm not sure, as
>> normally we shouldn't hit the instruction emulator for branches (the
>> main exception being PAE mode shadow guests).
> 
> MPX can be used for any memory reference, and one stated purpose is for
> buffer overflow detection.  While the instruction emulator doesn't
> understand MPX, any MMIO or shadow pagetable updates are liable to be
> incorrect.

No, memory references are completely unaffected by MPX. Code
needs to explicitly use BNDC{L,N,U} to invoke bounds checking.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel