----- "Dexuan Cui" <dexuan.cui@xxxxxxxxx> wrote:
> From: "Dexuan Cui" <dexuan.cui@xxxxxxxxx>
> To: "Miroslav Rezanina" <mrezanin@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx>
> Sent: Monday, October 19, 2009 9:38:12 AM GMT +01:00 Amsterdam / Berlin /
> Bern / Rome / Stockholm / Vienna
> Subject: [Xen-devel] RE: [Xen-changelog] [xen-unstable]vt-d: Fixpanic in
> msi_msg_read_remap_rte with acpi=off
>
> 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
>
Hi Dexuan,
you're right. We should print warning. In your patch, I do not understand
why you put comment only in setup_dom0_devices function. There is more
calling of domain_context_mapping and we check NULL also in domain_context_unmap
and reassign_device_ownership. We should put warning in there too, shouldn't we?
Regards,
Mirek
>
>
> -----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
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
--
Miroslav Rezanina
Software Engineer - Virtualization Team - XEN kernel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home