[PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Han, Weidong]

[-- Attachment #1: Type: text/plain, Size: 572 bytes --]

There are already PCI probe and remove callbacks in dom0 (c/s 593 in
linux-2.6.18-xen.hg). So it is not necessary to enumerate devices for
dom0 in Xen.

Espen had a patch (c/s 17975) to do the same thing, but his patch was
reverted in order to allow old dom0 kernels to work with iommu-capable
platforms. Xen 3.3 has been released for a few months, I think it's time
to clean up it. The rebased patch is attached.

Keir, what's your opinion?

Signed-off-by: Espen Skoglund <espen.skoglund@netronome.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>



[-- Attachment #2: remove-dom0-setup.patch --]
[-- Type: application/octet-stream, Size: 2579 bytes --]

diff -r d2f7243fc571 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Tue Oct 21 18:00:21 2008 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Wed Oct 22 16:19:53 2008 +0800
@@ -40,9 +40,6 @@ static spinlock_t domid_bitmap_lock;    
 static spinlock_t domid_bitmap_lock;    /* protect domain id bitmap */
 static int domid_bitmap_size;           /* domain id bitmap size in bits */
 static unsigned long *domid_bitmap;     /* iommu domain id bitmap */
-
-static void setup_dom0_devices(struct domain *d);
-static void setup_dom0_rmrr(struct domain *d);
 
 #define DID_FIELD_WIDTH 16
 #define DID_HIGH_OFFSET 8
@@ -1038,9 +1035,6 @@ static int intel_iommu_domain_init(struc
                 iommu_map_page(d, (i*tmp+j), (i*tmp+j));
         }
 
-        setup_dom0_devices(d);
-        setup_dom0_rmrr(d);
-
         iommu_flush_all();
 
         for_each_drhd_unit ( drhd )
@@ -1584,38 +1578,6 @@ static int intel_iommu_remove_device(str
     return domain_context_unmap(pdev->domain, pdev->bus, pdev->devfn);
 }
 
-static void setup_dom0_devices(struct domain *d)
-{
-    struct hvm_iommu *hd;
-    struct pci_dev *pdev;
-    int bus, dev, func;
-    u32 l;
-
-    hd = domain_hvm_iommu(d);
-
-    write_lock(&pcidevs_lock);
-    for ( bus = 0; bus < 256; bus++ )
-    {
-        for ( dev = 0; dev < 32; dev++ )
-        {
-            for ( func = 0; func < 8; func++ )
-            {
-                l = pci_conf_read32(bus, dev, func, PCI_VENDOR_ID);
-                /* some broken boards return 0 or ~0 if a slot is empty: */
-                if ( (l == 0xffffffff) || (l == 0x00000000) ||
-                     (l == 0x0000ffff) || (l == 0xffff0000) )
-                    continue;
-
-                pdev = alloc_pdev(bus, PCI_DEVFN(dev, func));
-                pdev->domain = d;
-                list_add(&pdev->domain_list, &d->arch.pdev_list);
-                domain_context_mapping(d, pdev->bus, pdev->devfn);
-            }
-        }
-    }
-    write_unlock(&pcidevs_lock);
-}
-
 void clear_fault_bits(struct iommu *iommu)
 {
     u64 val;
@@ -1680,20 +1642,6 @@ static int init_vtd_hw(void)
     return 0;
 }
 
-static void setup_dom0_rmrr(struct domain *d)
-{
-    struct acpi_rmrr_unit *rmrr;
-    u16 bdf;
-    int ret, i;
-
-    for_each_rmrr_device ( rmrr, bdf, i )
-    {
-        ret = iommu_prepare_rmrr_dev(d, rmrr, PCI_BUS(bdf), PCI_DEVFN2(bdf));
-        if ( ret )
-            gdprintk(XENLOG_ERR VTDPREFIX,
-                     "IOMMU: mapping reserved region failed\n");
-    }
-}
 
 int intel_vtd_setup(void)
 {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Zhao, Yu]

Han, Weidong wrote:
> There are already PCI probe and remove callbacks in dom0 (c/s 593 in linux-2.6.18-xen.hg). So it is not necessary to enumerate devices for dom0 in Xen.
> 
> Espen had a patch (c/s 17975) to do the same thing, but his patch was reverted in order to allow old dom0 kernels to work with iommu-capable platforms. Xen 3.3 has been released for a few months, I think it's time to clean up it. The rebased patch is attached.
> 
> Keir, what's your opinion?
> 
> Signed-off-by: Espen Skoglund <espen.skoglund@netronome.com>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> 
> 
Acked-by: Yu Zhao <yu.zhao@intel.com>

Re: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Keir Fraser]

On 22/10/08 09:59, "Han, Weidong" <weidong.han@intel.com> wrote:

> There are already PCI probe and remove callbacks in dom0 (c/s 593 in
> linux-2.6.18-xen.hg). So it is not necessary to enumerate devices for
> dom0 in Xen.
> 
> Espen had a patch (c/s 17975) to do the same thing, but his patch was
> reverted in order to allow old dom0 kernels to work with iommu-capable
> platforms. Xen 3.3 has been released for a few months, I think it's time
> to clean up it. The rebased patch is attached.
> 
> Keir, what's your opinion?

Xen 3.3 should continue to work with older dom0 kernels. However, now that
iommu is *disabled* by default, will this mean that it would continue to
work with older dom0 kernels so long as iommu is not enabled on the Xen
command line? This should be tested and confirmed with this patch applied.
If so, I would accept it.

 -- Keir

Re: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Espen Skoglund]

[Keir Fraser]
> On 22/10/08 09:59, "Han, Weidong" <weidong.han@intel.com> wrote:
>> There are already PCI probe and remove callbacks in dom0 (c/s 593 in
>> linux-2.6.18-xen.hg). So it is not necessary to enumerate devices for
>> dom0 in Xen.
>> 
>> Espen had a patch (c/s 17975) to do the same thing, but his patch was
>> reverted in order to allow old dom0 kernels to work with iommu-capable
>> platforms. Xen 3.3 has been released for a few months, I think it's time
>> to clean up it. The rebased patch is attached.
>> 
>> Keir, what's your opinion?

> Xen 3.3 should continue to work with older dom0 kernels. However,
> now that iommu is *disabled* by default, will this mean that it
> would continue to work with older dom0 kernels so long as iommu is
> not enabled on the Xen command line? This should be tested and
> confirmed with this patch applied.  If so, I would accept it.

Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be able
to find the PCI device unless it has been registered by dom0.  Apart
from the iommu code, the only user affected would be the MSI code
which looks up the PCI device before enabling MSIs on it.  As such,
you would not be able to enable MSIs if you have an old dom0 kernel.

	eSk

Re: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Keir Fraser]

On 22/10/08 13:38, "Espen Skoglund" <espen.skoglund@netronome.com> wrote:

>> Xen 3.3 should continue to work with older dom0 kernels. However,
>> now that iommu is *disabled* by default, will this mean that it
>> would continue to work with older dom0 kernels so long as iommu is
>> not enabled on the Xen command line? This should be tested and
>> confirmed with this patch applied.  If so, I would accept it.
> 
> Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be able
> to find the PCI device unless it has been registered by dom0.  Apart
> from the iommu code, the only user affected would be the MSI code
> which looks up the PCI device before enabling MSIs on it.  As such,
> you would not be able to enable MSIs if you have an old dom0 kernel.

Is there any disadvantage to keeping this legacy code in Xen, apart from it
annoyingly sitting there?

 -- Keir

Re: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Espen Skoglund]

[Keir Fraser]
> On 22/10/08 13:38, "Espen Skoglund" <espen.skoglund@netronome.com> wrote:

>>> Xen 3.3 should continue to work with older dom0 kernels. However,
>>> now that iommu is *disabled* by default, will this mean that it
>>> would continue to work with older dom0 kernels so long as iommu is
>>> not enabled on the Xen command line? This should be tested and
>>> confirmed with this patch applied.  If so, I would accept it.
>> 
>> Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be
>> able to find the PCI device unless it has been registered by dom0.
>> Apart from the iommu code, the only user affected would be the MSI
>> code which looks up the PCI device before enabling MSIs on it.  As
>> such, you would not be able to enable MSIs if you have an old dom0
>> kernel.

> Is there any disadvantage to keeping this legacy code in Xen, apart
> from it annoyingly sitting there?

You would get a list of PCI devices in Xen containing entries that are
not necessarily in use.  Some of the entries might even be bogus.
Apart from some annoying extra code and additional entries in the
IOMMU page tables, I don't think there is much of a problem.  There
could be some potential problems if Xen at a future stage decides to
do some in-hypervisor cleverness with all known PCI devices (a not
very likely scenario IMHO).

	eSk

RE: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Han, Weidong]

Keir Fraser wrote:
> On 22/10/08 13:38, "Espen Skoglund" <espen.skoglund@netronome.com>
> wrote: 
> 
>>> Xen 3.3 should continue to work with older dom0 kernels. However,
>>> now that iommu is *disabled* by default, will this mean that it
>>> would continue to work with older dom0 kernels so long as iommu is
>>> not enabled on the Xen command line? This should be tested and
>>> confirmed with this patch applied.  If so, I would accept it.
>> 
>> Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be able
>> to find the PCI device unless it has been registered by dom0.  Apart
>> from the iommu code, the only user affected would be the MSI code
>> which looks up the PCI device before enabling MSIs on it.  As such,
>> you would not be able to enable MSIs if you have an old dom0 kernel.
> 
> Is there any disadvantage to keeping this legacy code in Xen, apart
> from it annoyingly sitting there?
> 

 At present, I don't see any other disadvantage on functionality, apart
from it annoyingly sitting there and it allocates two pdev for the same
bdf.

Regards,
Weidong

RE: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Espen Skoglund]

[Weidong Han]
> Keir Fraser wrote:
>> On 22/10/08 13:38, "Espen Skoglund" <espen.skoglund@netronome.com>
>> wrote: 
>>> Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be
>>> able to find the PCI device unless it has been registered by dom0.
>>> Apart from the iommu code, the only user affected would be the MSI
>>> code which looks up the PCI device before enabling MSIs on it.  As
>>> such, you would not be able to enable MSIs if you have an old dom0
>>> kernel.
>> 
>> Is there any disadvantage to keeping this legacy code in Xen, apart
>> from it annoyingly sitting there?
>> 

> At present, I don't see any other disadvantage on functionality,
> apart from it annoyingly sitting there and it allocates two pdev for
> the same bdf.

Allocates two pdevs?  How?  alloc_pdev() will not allocate a new pdev
if the bdf already exists.  It will just return the existing one.

	eSk

RE: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Han, Weidong]

Espen Skoglund wrote:
> [Weidong Han]
>> Keir Fraser wrote:
>>> On 22/10/08 13:38, "Espen Skoglund" <espen.skoglund@netronome.com>
>>> wrote:
>>>> Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be
>>>> able to find the PCI device unless it has been registered by dom0.
>>>> Apart from the iommu code, the only user affected would be the MSI
>>>> code which looks up the PCI device before enabling MSIs on it.  As
>>>> such, you would not be able to enable MSIs if you have an old dom0
>>>> kernel.
>>> 
>>> Is there any disadvantage to keeping this legacy code in Xen, apart
>>> from it annoyingly sitting there?
>>> 
> 
>> At present, I don't see any other disadvantage on functionality,
>> apart from it annoyingly sitting there and it allocates two pdev for
>> the same bdf.
> 
> Allocates two pdevs?  How?  alloc_pdev() will not allocate a new pdev
> if the bdf already exists.  It will just return the existing one.
> 

You are right, it won't allocate twice for one bdf. I made a mistake -:(

Regards,
Weidong

Re: [PATCH] [VTD] Remove PCI device enumeration for dom0 in Xen

[Yu Zhao]

Espen Skoglund wrote:
> [Keir Fraser]
>> On 22/10/08 13:38, "Espen Skoglund" <espen.skoglund@netronome.com> wrote:
> 
>>>> Xen 3.3 should continue to work with older dom0 kernels. However,
>>>> now that iommu is *disabled* by default, will this mean that it
>>>> would continue to work with older dom0 kernels so long as iommu is
>>>> not enabled on the Xen command line? This should be tested and
>>>> confirmed with this patch applied.  If so, I would accept it.
>>> Users of pci_lock_pdev() and pci_lock_domain_pdev() would not be
>>> able to find the PCI device unless it has been registered by dom0.
>>> Apart from the iommu code, the only user affected would be the MSI
>>> code which looks up the PCI device before enabling MSIs on it.  As
>>> such, you would not be able to enable MSIs if you have an old dom0
>>> kernel.
> 
>> Is there any disadvantage to keeping this legacy code in Xen, apart
>> from it annoyingly sitting there?
> 
> You would get a list of PCI devices in Xen containing entries that are
> not necessarily in use.  Some of the entries might even be bogus.
> Apart from some annoying extra code and additional entries in the
> IOMMU page tables, I don't think there is much of a problem.  There
> could be some potential problems if Xen at a future stage decides to
> do some in-hypervisor cleverness with all known PCI devices (a not
> very likely scenario IMHO).

We would have bus number rebalance (hotplug, etc.) and PCI domain 
support in the future, which may require hypervisor to keep consistent 
device list with dom0 kernel. If we don't remove the code, we still have 
to fix the list up.

Thanks,
Yu