Re: [PATCH 3/3] x86/AMD-Vi: Fix IVRS HPET special->handle override

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2437 bytes --]

On 23/09/13 17:48, Suravee Suthikulpanit wrote:
> On 9/23/2013 1:52 AM, Jan Beulich wrote:
>>>>> On 20.09.13 at 23:38, Suravee Suthikulanit <suravee.suthikulpanit@amd.com>
>> wrote:
>>> On 9/17/2013 10:30 AM, Jan Beulich wrote:
>>>>>> This (unconditional) assignment is what the earlier logic attempted
>>>>>>>> to avoid: We must not blindly set this (and in particular not blindly
>>>>>>>> overwrite a previously set valid value), and in order to do so we
>>>>>>>> need to know whether to trust devid or handle. I'm therefore going
>>>>>>>> to apply only the first hunk - being a clear and obvious bug fix - for
>>>>>>>> the time being.
>>>>>>>>
>>>>>> But since we only allow one HPET in the system, and if users want to
>>>>>> override the one that is in the IVRS (the buggy one)
>>>>>> we should allow this, right?  Otherwise, if the special->handle doesn't
>>>>>> match, it would end up causing this logic to complain
>>>>>> about multiple HPETs.
>>>> We must not allow multiple HPET entries in the ACPI tables to
>>>> confuse us and store a bad IOMMU pointer.
>>> I understand this part and agree.  Currently, the patch does not allow 
>>> multiple HPET in the system.
>>> If user specifies the ivrs_hpet, that will be the one that get used, and 
>>> ignore the one that
>>> is in the IVRS.  Although, it will be pointing to the IOMMU which lists 
>>> HPET in the IVRS.
>>>
>>> However, if IVRS is listing multiple HPETs in different IOMMUs, then it 
>>> will just default to the first IOMMU.
>>> I don't see this case happening though since HPET is in the Southbridge 
>>> which only has one in the system.
>>>
>>> Am I missing any thing?
>> Yes - there's no guarantee that (especially in a multi-node
>> system) there's just one HPET. Nor do the ACPI tables have
>> any indication there this would always be the case. Even if
>> _all_ current systems only have a single HPET (which I don't
>> think you can guarantee), we shouldn't code in a latent bug
>> like this.
>>
>> Jan
> I am a bit confused on what you want to do.  I believe all the systems
> at this point
> should have only one HPET.  That's why the code only has one data
> structure for
> initialize one HPET.  Are you expecting that certain systems could
> have more than
> one HPETs?

Amongst the test servers we have, I have plenty of examples of AMD
hardware with anything up to 8 HPETs in the system.  (See for reference
my HPET stack overflow thread)

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 3899 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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