Re: [Xen-devel] [PATCH] VT-d: improve RMRR validity checking

[Weidong Han <weidong.han@xxxxxxxxx>]

Sander Eikelenboom wrote:
> Hello Weidong,
>
> Wouldn't it be more clear to add an option to iommu= for this case ?
>
> if iommu=on,..,..,security
>
> With the security option specified:
>      -it would be most strict in it's checks, since enforcing security with the 
> iommu requires that as you have pointed out.
>      -warn,fail or panic incase it can't enable all to enforce the security.
>   
iommu=force is for security. It does as you described above. So I think 
"security" option is not necessary.
> Without the security option specified (default)
>      - it tries to work as with the security option specified
>      - but incase of problems makes the assumption the iommu's main task is not 
> security, but making as much of vt-d working to keep the passthrough 
> functionality
>      - it will only warn, that you will lose the security part, that it would 
> be wise to let your bios be fixed, and not making it panic
>      - and keep vt-d enabled
>
>   
the default iommu=1 works like iommu=force if BIOS is correct. But in 
fact we encountered some buggy BIOS, and then we added some workarounds 
to make VT-d still be enabled,  or warn and disable VT-d if the issue is 
regarded as invalid and cannot be workarounded. These workarounds make 
Xen more defensive to VT-d BIOS issues. The panic only occurs when 
operating VT-d hardware fails, because it means the hardware is possibly 
malfunctional.
In short, default iommu=1 can workaround known VT-d BIOS issues we 
observed till now, while iommu=force ensures best security provided by VT-d.
Regards,
Weidong

> Regards,
>
> Sander
>
>
>
> Friday, January 22, 2010, 9:47:11 AM, you wrote:
>
>   
> > diff -r 207fba95a7d5 xen/drivers/passthrough/vtd/dmar.c
> > --- a/xen/drivers/passthrough/vtd/dmar.c        Fri Jan 22 13:12:45 2010 +0800
> > +++ b/xen/drivers/passthrough/vtd/dmar.c        Fri Jan 22 22:32:10 2010 +0800
> > @@ -396,8 +396,49 @@ acpi_parse_one_drhd(struct acpi_dmar_ent
> >  
> >      if ( ret )
> >          xfree(dmaru);
> > +    else if ( force_iommu || dmaru->include_all )
> > +        acpi_register_drhd_unit(dmaru);
> >      else
> > -        acpi_register_drhd_unit(dmaru);
> > +    {
> > +        u8 b, d, f;
> > +        int i, invalid_cnt = 0;
> > +
> > +        for ( i = 0; i < dmaru->scope.devices_cnt; i++ )
> > +        {
> > +            b = PCI_BUS(dmaru->scope.devices[i]);
> > +            d = PCI_SLOT(dmaru->scope.devices[i]);
> > +            f = PCI_FUNC(dmaru->scope.devices[i]);
> > +
> > +            if ( pci_device_detect(b, d, f) == 0 )
> > +            {
> > +                dprintk(XENLOG_WARNING VTDPREFIX,
> > +                    "  Non-existent device (%x:%x.%x) is reported "
> > +                    "in this DRHD's scope!\n", b, d, f);
> > +                invalid_cnt++;
> > +            }
> > +        }
> > +
> > +        if ( invalid_cnt )
> > +        {
> > +            xfree(dmaru);
> > +            if ( invalid_cnt == dmaru->scope.devices_cnt )
> > +            {
> > +                dprintk(XENLOG_WARNING VTDPREFIX,
> > +                    "  Ignore the DRHD due to all devices under "
> > +                    "its scope are not PCI discoverable!\n");
> > +            }
> > +            else
> > +            {
> > +                dprintk(XENLOG_WARNING VTDPREFIX,
> > +                    "  The DRHD is invalid due to some devices under "
> > +                    "its scope are not PCI discoverable!\n");
> > +                ret = -EINVAL;
> > +            }
> > +        }
> > +        else
> > +            acpi_register_drhd_unit(dmaru);
> > +    }
> > +
> >      return ret;
> >  }
> >  
> >     
>
>   
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel