Re: [Xen-devel] [PATCH] PCI/MSI: don't disable AMD IOMMU MSI on Xen dom0

[Wei Wang <wei.wang2@xxxxxxx>]

On 06/21/2012 02:06 PM, Jan Beulich wrote:
> > > > On 21.06.12 at 13:21, Wei Wang<wei.wang2@xxxxxxx>  wrote:
> > On 06/21/2012 11:59 AM, Jan Beulich wrote:
> > > > > > On 14.06.12 at 17:15, Wei Wang<wei.wang2@xxxxxxx>   wrote:
> > > > Am 14.06.2012 16:18, schrieb Jan Beulich:
> > > > > Have you at all considered getting this fixed on the kernel side?
> > > > > As I don't have direct access to any AMD IOMMU capable
> > > > > system - can one, other than by enumerating the respective
> > > > > PCI IDs or reading ACPI tables, reasonably easily identify the
> > > > > devices in question (e.g. via vendor/class/sub-class or some
> > > > > such)? That might permit skipping those in the offending kernel
> > > > > code...
> > > > AMD IOMMUs (both v1 and v2) uses class id 08 (System Base Peripheral)
> > > > and sub class id 06 (IOMMU). Combined with PCI_VENDEOR_ID_AMD, this
> > > > should be enough to identify amd iommu device. I could send you a kernel
> > > > patch for review using this approach. I would believe that fixing this
> > > > issue in 4.2, no matter how, is really important for amd iommu.
> > > As you didn't come forward with anything, here's my first
> > > take on this:
> > Hi Jan
> > Thanks a lot for the patch. Actually I plan to send my version today,
> > which is based on 3.4 pv_ops but looks very similar to yours. So, Acked!
> >
> > I also evaluated the possibility of hiding iommu device from dom0. I
> > think the change is no quite a lot, at least, for io based pcicfg
> > access. A proof-of-concept patch is attached.
> This completely hides the device from Dom0, but only when
> config space is accessed via method 1. Did you not see my
> earlier patch doing this for MCFG as well
Could you please provide a particular c/s number?... (I saw too many c/s 
might be related to this topic). so that I could work out a patch to 
support both i/o and mmcfg.
(albeit only disallowing
> writes, so allowing the device to still be seen by Dom0)?
Sounds better to me...this still allows user to check iommu status from 
lspci.
> Whether completely hiding the device is actually okay I can't
> easily tell: Would IOMMUs always be either at func 0 of a single-
> unction device, or at a non-zero func of a multi-function one? If
> not, other devices may get hidden implicitly.
AMD IOMMU is an independent pci-e endpoint, and this function will not 
be used for other purposes other than containing an iommu. So I don't 
see that iommu will share bdf value with other devices.
Thanks,
Wei

> Also I noticed just now that guest_io_read() wouldn't really
> behave correctly when pci_cfg_ok() returned false - it might
> pass back 0xff even for a multi-byte read. I'll send a fix shortly.
>
> Jan
>
> > --- a/xen/arch/x86/traps.c      Thu Jun 21 11:30:59 2012 +0200
> > +++ b/xen/arch/x86/traps.c      Thu Jun 21 13:19:02 2012 +0200
> > @@ -73,6 +73,7 @@
> >    #include<asm/hpet.h>
> >    #include<public/arch-x86/cpuid.h>
> >    #include<xsm/xsm.h>
> > +#include<asm/hvm/svm/amd-iommu-proto.h>
> >
> >    /*
> >     * opt_nmi: one of 'ignore', 'dom0', or 'fatal'.
> > @@ -1686,10 +1687,19 @@ static int pci_cfg_ok(struct domain *d,
> >    {
> >        uint32_t machine_bdf;
> >        uint16_t start, end;
> > +    struct amd_iommu *iommu;
> > +
> >        if (!IS_PRIV(d))
> >            return 0;
> >
> >        machine_bdf = (d->arch.pci_cf8>>  8)&  0xFFFF;
> > +
> > +    for_each_amd_iommu ( iommu )
> > +    {
> > +        if ( machine_bdf == iommu->bdf )
> > +            return 0;
> > +    }
> > +
> >        start = d->arch.pci_cf8&  0xFF;
> >        end = start + size - 1;
> >        if (xsm_pci_config_permission(d, machine_bdf, start, end, write))


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel