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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.09.13 at 23:38, Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>
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


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