[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 09/16/2013 03:03 AM, Jan Beulich wrote:
On 14.09.13 at 01:30, <suravee.suthikulpanit@xxxxxxx> wrote:
From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

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@xxxxxxx>
---
 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.