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

[Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>]

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

On 09/16/2013 03:03 AM, Jan Beulich wrote:
>>>> On 14.09.13 at 01:30, <suravee.suthikulpanit@amd.com> wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>
>> The current logic does not handle the case when HPET special->handle
>> is invalid in IVRS. On such system, the following message is shown:
>>
>> (XEN) AMD-Vi: Failed to setup HPET MSI remapping: Wrong HPET
>>
>> This patch will allow the ivrs_hpet[<handle>]=<sbdf> to override the
>> IVRS.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> ---
>>   xen/drivers/passthrough/amd/iommu_acpi.c | 22 +++++++++++++++-------
>>   1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
>> b/xen/drivers/passthrough/amd/iommu_acpi.c
>> index 0778af0..bfcc2eb 100644
>> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
>> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
>> @@ -671,6 +671,7 @@ static void __init parse_ivrs_hpet(char *str)
>>       if ( !s || *s )
>>           return;
>>   
>> +    hpet_sbdf.id = id;
>>       hpet_sbdf.bdf = PCI_BDF(bus, dev, func);
>>       hpet_sbdf.seg = seg;
>>       hpet_sbdf.cmdline = 1;
>> @@ -787,19 +788,26 @@ static u16 __init parse_ivhd_device_special(
>>           }
>>           break;
>>       case ACPI_IVHD_HPET:
>> +        if ( hpet_sbdf.cmdline )
>> +        {
>> +            AMD_IOMMU_DEBUG("IVHD: Command line override present for HPET %#x "
>> +                            "(IVRS: %#x devID %04x:%02x:%02x.%u)\n",
>> +                            hpet_sbdf.id, special->handle, seg, PCI_BUS(bdf),
>> +                            PCI_SLOT(bdf), PCI_FUNC(bdf));
>> +            hpet_sbdf.iommu = iommu;
> 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.
>
> Jan
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.

Suravee

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

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

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