<suravee.suthikulpanit@amd.com>
2013-Sep-25 19:15 UTC
[PATCH V2] x86/AMD-Vi: Fix IVRS HPET special->handle override
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. Also, it removes struct hpet_sbdf.iommu since it is not used anywhere in the code. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- Changes from V1: * Remove the struct hpet_sbdf.iommu xen/drivers/passthrough/amd/iommu_acpi.c | 21 +++++++++++++-------- xen/drivers/passthrough/amd/iommu_intr.c | 5 ++--- xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 1 - 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c index c3b9631..d1a82c9 100644 --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -789,20 +789,25 @@ 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)); + break; + } + /* set device id of hpet */ - if ( hpet_sbdf.iommu || - (hpet_sbdf.cmdline && hpet_sbdf.id != special->handle) ) + if ( hpet_sbdf.id != 0 ) { printk(XENLOG_WARNING "Only one IVHD HPET entry is supported\n"); break; } + hpet_sbdf.id = special->handle; - if ( !hpet_sbdf.cmdline ) - { - hpet_sbdf.bdf = bdf; - hpet_sbdf.seg = seg; - } - hpet_sbdf.iommu = iommu; + hpet_sbdf.bdf = bdf; + hpet_sbdf.seg = seg; break; default: printk(XENLOG_ERR "Unrecognized IVHD special variety %#x\n", diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c index 213f4d7..3e29824 100644 --- a/xen/drivers/passthrough/amd/iommu_intr.c +++ b/xen/drivers/passthrough/amd/iommu_intr.c @@ -598,10 +598,9 @@ int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) unsigned long flags; int rc = 0; - if ( msi_desc->hpet_id != hpet_sbdf.id || !hpet_sbdf.iommu ) + if ( msi_desc->hpet_id != hpet_sbdf.id ) { - AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping: %s\n", - hpet_sbdf.iommu ? "Wrong HPET" : "No IOMMU"); + AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping. Mismatch IVRS info.\n"); return -ENODEV; } diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h index 3e6961d..f14dc68 100644 --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h @@ -109,7 +109,6 @@ extern struct ioapic_sbdf { extern struct hpet_sbdf { u16 bdf, seg, id; bool_t cmdline; - struct amd_iommu *iommu; } hpet_sbdf; extern void *shared_intremap_table; -- 1.8.1.2
Jan Beulich
2013-Sep-26 06:52 UTC
Re: [PATCH V2] x86/AMD-Vi: Fix IVRS HPET special->handle override
>>> On 25.09.13 at 21:15, <suravee.suthikulpanit@amd.com> wrote: > --- a/xen/drivers/passthrough/amd/iommu_acpi.c > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c > @@ -789,20 +789,25 @@ 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)); > + break; > + } > + > /* set device id of hpet */ > - if ( hpet_sbdf.iommu || > - (hpet_sbdf.cmdline && hpet_sbdf.id != special->handle) ) > + if ( hpet_sbdf.id != 0 )Surely there''s no guarantee that special->handle in non-zero. After all that''s a firmware allocated value. What might be acceptable is assuming bdf to be non-zero, but even that would look sort of hackish. Perhaps you''d be better off converting .cmdline to an enum with states none, cmdline, and ivhd, and use a switch() here.> { > printk(XENLOG_WARNING "Only one IVHD HPET entry is supported\n"); > break; > } > + > hpet_sbdf.id = special->handle; > - if ( !hpet_sbdf.cmdline ) > - { > - hpet_sbdf.bdf = bdf; > - hpet_sbdf.seg = seg; > - } > - hpet_sbdf.iommu = iommu; > + hpet_sbdf.bdf = bdf; > + hpet_sbdf.seg = seg; > break; > default: > printk(XENLOG_ERR "Unrecognized IVHD special variety %#x\n", > --- a/xen/drivers/passthrough/amd/iommu_intr.c > +++ b/xen/drivers/passthrough/amd/iommu_intr.c > @@ -598,10 +598,9 @@ int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) > unsigned long flags; > int rc = 0; > > - if ( msi_desc->hpet_id != hpet_sbdf.id || !hpet_sbdf.iommu ) > + if ( msi_desc->hpet_id != hpet_sbdf.id ) > { > - AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping: %s\n", > - hpet_sbdf.iommu ? "Wrong HPET" : "No IOMMU"); > + AMD_IOMMU_DEBUG("Failed to setup HPET MSI remapping. Mismatch IVRS info.\n");This will need to get adjusted too - instead of !hpet_sbdf.iommu you''d now need to check the new enum to be other than "none". Jan