Cui, Dexuan
2009-Oct-16 10:03 UTC
[Xen-devel] RE: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off
> Xen panics when "acpi=off noacpi" is set.(looks xen has not such a parameter "noacpi".) I guess you meant: when "iommu=1 acpi=off", Xen panics. But when I use c/s 20330 and use "iommu=1 acpi=off", I still get the the panic: (XEN) Xen call trace: (XEN) [<ffff82c4801401b5>] ats_device+0x53/0x10e (XEN) [<ffff82c48013c8e5>] intel_iommu_domain_init+0xa8/0x200 (XEN) [<ffff82c4801379dd>] iommu_domain_init+0x74/0x76 (XEN) [<ffff82c48014c437>] arch_domain_create+0x544/0x94a (XEN) [<ffff82c48010676a>] domain_create+0x26c/0x3fd (XEN) [<ffff82c48024cdb5>] __start_xen+0x5264/0x557f Actually when iommu=1 and acpi=off, in __start_xen() -> acpi_boot_init(), acpi_dmar_init() can''t be invoked at all and hence parse_dmar_table() can''t be invoked, as a result, we should not try to use VT-d at all, however, the global variable iommu_enabled is left set to 1. So I don''t think this changeset is the right fix. I think the correct one should be: in disable_acpi(), we force iommu_enabled to 0. Thanks, -- Dexuan -----Original Message----- From: xen-changelog-bounces@lists.xensource.com [mailto:xen-changelog-bounces@lists.xensource.com] On Behalf Of Xen patchbot-unstable Sent: 2009?10?16? 16:10 To: xen-changelog@lists.xensource.com Subject: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off # HG changeset patch # User Keir Fraser <keir.fraser@citrix.com> # Date 1255678127 -3600 # Node ID 2370e16ab6d3a1c9de43babec48c8f14121d19bc # Parent 648c674fcc96b09fc8fb9428843fec08c8d2fc4e vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off Xen panics when "acpi=off noacpi" is set. Problem is caused by dereferencing NULL pointer in drhd after calling acpi_find_matched_drhd_unit. As acpi_find_matched_drhd_unit can return NULL, checks has to be done before returned value is used. From: Miroslav Rezanina <mrezanin@redhat.com> Signed-off-by: Keir Fraser <keir.fraser@eu.citrix.com> --- xen/drivers/passthrough/vtd/intremap.c | 6 ++++-- xen/drivers/passthrough/vtd/iommu.c | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff -r 648c674fcc96 -r 2370e16ab6d3 xen/drivers/passthrough/vtd/intremap.c --- a/xen/drivers/passthrough/vtd/intremap.c Fri Oct 16 08:25:17 2009 +0100 +++ b/xen/drivers/passthrough/vtd/intremap.c Fri Oct 16 08:28:47 2009 +0100 @@ -563,7 +563,8 @@ void msi_msg_read_remap_rte( struct iommu *iommu = NULL; struct ir_ctrl *ir_ctrl; - drhd = acpi_find_matched_drhd_unit(pdev); + if ( (drhd = acpi_find_matched_drhd_unit(pdev)) == NULL ) + return; iommu = drhd->iommu; ir_ctrl = iommu_ir_ctrl(iommu); @@ -581,7 +582,8 @@ void msi_msg_write_remap_rte( struct iommu *iommu = NULL; struct ir_ctrl *ir_ctrl; - drhd = acpi_find_matched_drhd_unit(pdev); + if ( (drhd = acpi_find_matched_drhd_unit(pdev)) == NULL ) + return; iommu = drhd->iommu; ir_ctrl = iommu_ir_ctrl(iommu); diff -r 648c674fcc96 -r 2370e16ab6d3 xen/drivers/passthrough/vtd/iommu.c --- a/xen/drivers/passthrough/vtd/iommu.c Fri Oct 16 08:25:17 2009 +0100 +++ b/xen/drivers/passthrough/vtd/iommu.c Fri Oct 16 08:28:47 2009 +0100 @@ -1343,7 +1343,8 @@ static int reassign_device_ownership( if (!pdev) return -ENODEV; - drhd = acpi_find_matched_drhd_unit(pdev); + if ( (drhd = acpi_find_matched_drhd_unit(pdev)) == NULL ) + return -ENODEV; pdev_iommu = drhd->iommu; domain_context_unmap(source, bus, devfn); @@ -1357,7 +1358,7 @@ static int reassign_device_ownership( for_each_pdev ( source, pdev ) { drhd = acpi_find_matched_drhd_unit(pdev); - if ( drhd->iommu == pdev_iommu ) + if ( drhd && drhd->iommu == pdev_iommu ) { found = 1; break; _______________________________________________ Xen-changelog mailing list Xen-changelog@lists.xensource.com http://lists.xensource.com/xen-changelog _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Oct-16 10:22 UTC
[Xen-devel] Re: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off
On 16/10/2009 11:03, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:> Actually when iommu=1 and acpi=off, in __start_xen() -> acpi_boot_init(), > acpi_dmar_init() can''t be invoked at all and hence parse_dmar_table() can''t be > invoked, as a result, we should not try to use VT-d at all, however, the > global variable iommu_enabled is left set to 1. > > So I don''t think this changeset is the right fix. I think the correct one > should be: in disable_acpi(), we force iommu_enabled to 0.Sounds better to me. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Cui, Dexuan
2009-Oct-16 10:36 UTC
[Xen-devel] RE: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off
A better fix is: At the begnning of in iommu_setup(), if acpi_pci_disabled, we set iommu_enabled to 0. Thanks, -- Dexuan -----Original Message----- From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] Sent: 2009?10?16? 18:22 To: Cui, Dexuan; xen-devel@lists.xensource.com; Miroslav Rezanina Subject: Re: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off On 16/10/2009 11:03, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:> Actually when iommu=1 and acpi=off, in __start_xen() -> acpi_boot_init(), > acpi_dmar_init() can''t be invoked at all and hence parse_dmar_table() can''t be > invoked, as a result, we should not try to use VT-d at all, however, the > global variable iommu_enabled is left set to 1. > > So I don''t think this changeset is the right fix. I think the correct one > should be: in disable_acpi(), we force iommu_enabled to 0.Sounds better to me. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Cui, Dexuan
2009-Oct-16 10:39 UTC
[Xen-devel] RE: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off
Oh, I meant acpi_disabled, not acpi_pci_disabled. :-) Thanks, -- Dexuan -----Original Message----- From: Cui, Dexuan Sent: 2009?10?16? 18:37 To: ''Keir Fraser''; xen-devel@lists.xensource.com; Miroslav Rezanina Subject: RE: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off A better fix is: At the begnning of in iommu_setup(), if acpi_pci_disabled, we set iommu_enabled to 0. Thanks, -- Dexuan -----Original Message----- From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] Sent: 2009?10?16? 18:22 To: Cui, Dexuan; xen-devel@lists.xensource.com; Miroslav Rezanina Subject: Re: [Xen-changelog] [xen-unstable] vt-d: Fixpanic in msi_msg_read_remap_rte with acpi=off On 16/10/2009 11:03, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:> Actually when iommu=1 and acpi=off, in __start_xen() -> acpi_boot_init(), > acpi_dmar_init() can''t be invoked at all and hence parse_dmar_table() can''t be > invoked, as a result, we should not try to use VT-d at all, however, the > global variable iommu_enabled is left set to 1. > > So I don''t think this changeset is the right fix. I think the correct one > should be: in disable_acpi(), we force iommu_enabled to 0.Sounds better to me. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel