Hi Miroslav,
I agree we should program defensively.
However, here when acpi=off and iommu=1, or when iommu=0, with my "if (
acpi_disabled ) iommu_enabled = 0" checking, I believe xen's execution can't
reach the places you pointed out.
And when (by default acpi=on and ) iommu=1, normally the places can't return
NULL either, otherwise the BIOS is buggy or Xen has a bug; in these cases, we
can't simply ignore it silently -- I think we should at least print a warning
message.
So how about my attached new patch?
Thanks,
-- Dexuan
-----Original Message-----
From: Miroslav Rezanina [mailto:mrezanin@xxxxxxxxxx]
Sent: 2009年10月17日 1:40
To: Cui, Dexuan
Cc: Keir Fraser; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: RE: [Xen-changelog] [xen-unstable]vt-d: Fixpanic in
msi_msg_read_remap_rte with acpi=off
---- "Dexuan Cui" <dexuan.cui@xxxxxxxxx> wrote:
> From: "Dexuan Cui" <dexuan.cui@xxxxxxxxx>
> To: "Miroslav Rezanina" <mrezanin@xxxxxxxxxx>
> Cc: "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
> Sent: Friday, October 16, 2009 4:16:55 PM GMT +01:00 Amsterdam / Berlin /
> Bern / Rome / Stockholm / Vienna
> Subject: RE: [Xen-changelog] [xen-unstable]vt-d: Fixpanic in
> msi_msg_read_remap_rte with acpi=off
>
> Hi Miroslav and Keir,
> When acpi=off and we set iommu_enabled=0, the execution of xen can't
> reach the acpi_find_matched_drhd_unit Miroslav explicitly points out
> one by one.
> I think I can give a cleaner fix. Please see the attached patch.
>
> Thanks,
> -- Dexuan
>
Hi Dexuan,
acpi_find_matched_drhd_unit can theoretically return NULL. Is one check so big
overhead to risk
segmentation fault?
>
> diff -r 0705efd9c69e xen/drivers/passthrough/iommu.c
> --- a/xen/drivers/passthrough/iommu.c Fri Oct 16 09:04:53 2009 +0100
> +++ b/xen/drivers/passthrough/iommu.c Fri Oct 16 18:41:46 2009 +0800
> @@ -266,9 +266,13 @@ int iommu_setup(void)
> {
> int rc = -ENODEV;
>
> - rc = iommu_hardware_setup();
> -
> - iommu_enabled = (rc == 0);
> + if ( acpi_disabled )
> + iommu_enabled = 0;
> + else
> + {
> + rc = iommu_hardware_setup();
> + iommu_enabled = (rc == 0);
> + }
>
> if ( force_iommu && !iommu_enabled )
> panic("IOMMU setup failed, crash Xen for security purpose!\n");
I miss this part, so this one should be add. However, I would leave checks when
calling acpi_find_matched_drhd_unit.
--
Miroslav Rezanina
Software Engineer - Virtualization Team - XEN kernel
--
Miroslav Rezanina
Software Engineer - Virtualization Team - XEN kernel
disable_vtd_when_acpi_is_off_V2.patch
Description: disable_vtd_when_acpi_is_off_V2.patch
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home