Xen devel - Feb 2010 - [DOM0 KERNELS] pciback: Fix SR-IOV VF passthrough

Vendor/device and BAR fields in a VF''s host-level PCI config space are
dummy
and must always be virtualised/emulated. Implement this in pciback by always
extracting the values installed in dom0 kernel''s own PCI structures,
rather
than interrogating the underlying PCI config space directly.

AFAIK, this patch should apply to any kernel that implements pciback: That
includes pv_ops, SLES, and the XS/XCP kernels. It should be applied to all
of them. It is already applied to linux-2.6.18-xen.hg as 998:693c40564c8d.

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>



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

On Fri, Feb 26, 2010 at 05:25:45PM +0000, Keir Fraser
wrote:
> Vendor/device and BAR fields in a VF''s host-level PCI config space
are dummy
> and must always be virtualised/emulated. Implement this in pciback by
always
> extracting the values installed in dom0 kernel''s own PCI
structures, rather
> than interrogating the underlying PCI config space directly.
> 
> AFAIK, this patch should apply to any kernel that implements pciback: That
> includes pv_ops, SLES, and the XS/XCP kernels. It should be applied to all
> of them. It is already applied to linux-2.6.18-xen.hg as 998:693c40564c8d.
Goodies! Thanks for the patch.

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

>>> Keir Fraser <keir.fraser@eu.citrix.com> 26.02.10 18:25
>>>
>Vendor/device and BAR fields in a VF''s host-level PCI config space
are dummy
>and must always be virtualised/emulated. Implement this in pciback by always
>extracting the values installed in dom0 kernel''s own PCI
structures, rather
>than interrogating the underlying PCI config space directly.
>
>AFAIK, this patch should apply to any kernel that implements pciback: That
>includes pv_ops, SLES, and the XS/XCP kernels. It should be applied to all
>of them. It is already applied to linux-2.6.18-xen.hg as 998:693c40564c8d.
>
>Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
Some parts of this we had been given by Intel, but some were also
implemented differently there. I''m reproducing the patch below, and
I would appreciate clarification on the differences in the bar_read()/
bar_write()/rom_write() vs. read_dev_bar() modifications.

In any case I would think that the command_write() change would
be generally applicable.

Jan

Subject: guest SR-IOV support for PV guest

These changes are for PV guest to use Virtual Function. Because the
VF''s
vendor, device registers in cfg space are 0xffff, which are invalid and
ignored by PCI device scan. Values in ''struct pci_dev'' are
fixed up by
SR-IOV code, and using these values will present correct VID and DID to
PV guest kernel.

And command registers in the cfg space are read only 0, which means we
have to emulate MMIO enable bit (VF only uses MMIO  resource) so PV
kernel can work properly.

--- head-2009-07-28.orig/drivers/xen/pciback/conf_space_header.c	2009-07-28
12:01:32.000000000 +0200
+++ head-2009-07-28/drivers/xen/pciback/conf_space_header.c	2009-07-29
11:03:07.000000000 +0200
@@ -18,6 +18,25 @@ struct pci_bar_info {
 #define is_enable_cmd(value) ((value)&(PCI_COMMAND_MEMORY|PCI_COMMAND_IO))
 #define is_master_cmd(value) ((value)&PCI_COMMAND_MASTER)
 
+static int command_read(struct pci_dev *dev, int offset, u16 *value, void
*data)
+{
+	int i;
+	int ret;
+
+	ret = pciback_read_config_word(dev, offset, value, data);
+	if (!atomic_read(&dev->enable_cnt))
+		return ret;
+
+	for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+		if (dev->resource[i].flags & IORESOURCE_IO)
+			*value |= PCI_COMMAND_IO;
+		if (dev->resource[i].flags & IORESOURCE_MEM)
+			*value |= PCI_COMMAND_MEMORY;
+	}
+
+	return ret;
+}
+
 static int command_write(struct pci_dev *dev, int offset, u16 value, void
*data)
 {
 	int err;
@@ -141,10 +160,26 @@ static inline void read_dev_bar(struct p
 				struct pci_bar_info *bar_info, int offset,
 				u32 len_mask)
 {
-	pci_read_config_dword(dev, offset, &bar_info->val);
-	pci_write_config_dword(dev, offset, len_mask);
-	pci_read_config_dword(dev, offset, &bar_info->len_val);
-	pci_write_config_dword(dev, offset, bar_info->val);
+	int	pos;
+	struct resource	*res = dev->resource;
+
+	if (offset == PCI_ROM_ADDRESS || offset == PCI_ROM_ADDRESS1)
+		pos = PCI_ROM_RESOURCE;
+	else {
+		pos = (offset - PCI_BASE_ADDRESS_0) / 4;
+		if (pos && ((res[pos - 1].flags & (PCI_BASE_ADDRESS_SPACE |
+				PCI_BASE_ADDRESS_MEM_TYPE_MASK)) =+			   (PCI_BASE_ADDRESS_SPACE_MEMORY |
+				PCI_BASE_ADDRESS_MEM_TYPE_64))) {
+			bar_info->val = res[pos - 1].start >> 32;
+			bar_info->len_val = res[pos - 1].end >> 32;
+			return;
+		}
+	}
+
+	bar_info->val = res[pos].start |
+			(res[pos].flags & PCI_REGION_FLAG_MASK);
+	bar_info->len_val = res[pos].end - res[pos].start + 1;
 }
 
 static void *bar_init(struct pci_dev *dev, int offset)
@@ -185,6 +220,22 @@ static void bar_release(struct pci_dev *
 	kfree(data);
 }
 
+static int pciback_read_vendor(struct pci_dev *dev, int offset,
+			       u16 *value, void *data)
+{
+	*value = dev->vendor;
+
+	return 0;
+}
+
+static int pciback_read_device(struct pci_dev *dev, int offset,
+			       u16 *value, void *data)
+{
+	*value = dev->device;
+
+	return 0;
+}
+
 static int interrupt_read(struct pci_dev *dev, int offset, u8 * value,
 			  void *data)
 {
@@ -212,9 +263,19 @@ static int bist_write(struct pci_dev *de
 
 static const struct config_field header_common[] = {
 	{
+	 .offset    = PCI_VENDOR_ID,
+	 .size      = 2,
+	 .u.w.read  = pciback_read_vendor,
+	},
+	{
+	 .offset    = PCI_DEVICE_ID,
+	 .size      = 2,
+	 .u.w.read  = pciback_read_device,
+	},
+	{
 	 .offset    = PCI_COMMAND,
 	 .size      = 2,
-	 .u.w.read  = pciback_read_config_word,
+	 .u.w.read  = command_read,
 	 .u.w.write = command_write,
 	},
 	{



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

On 01/03/2010 09:06, "Jan Beulich" <JBeulich@novell.com> wrote:
>> AFAIK, this patch should apply to any kernel that implements pciback:
That
>> includes pv_ops, SLES, and the XS/XCP kernels. It should be applied to
all
>> of them. It is already applied to linux-2.6.18-xen.hg as
998:693c40564c8d.
>> 
>> Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
> 
> Some parts of this we had been given by Intel, but some were also
> implemented differently there. I''m reproducing the patch below,
and
> I would appreciate clarification on the differences in the bar_read()/
> bar_write()/rom_write() vs. read_dev_bar() modifications.
> 
> In any case I would think that the command_write() change would
> be generally applicable.
I never saw the Intel patch before. It looks fine to me.

 -- Keir



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

On Mon, Mar 01, 2010 at 09:06:39AM +0000, Jan Beulich
wrote:
> >>> Keir Fraser <keir.fraser@eu.citrix.com> 26.02.10 18:25
>>>
> >Vendor/device and BAR fields in a VF''s host-level PCI config
space are dummy
> >and must always be virtualised/emulated. Implement this in pciback by
always
> >extracting the values installed in dom0 kernel''s own PCI
structures, rather
> >than interrogating the underlying PCI config space directly.
> >
> >AFAIK, this patch should apply to any kernel that implements pciback:
That
> >includes pv_ops, SLES, and the XS/XCP kernels. It should be applied to
all
> >of them. It is already applied to linux-2.6.18-xen.hg as
998:693c40564c8d.
> >
> >Signed-off-by: Keir Fraser <keir.fraser@citrix.com>
> 
> Some parts of this we had been given by Intel, but some were also
> implemented differently there. I''m reproducing the patch below,
and
Could attach it as an attachment? I get:

patching file drivers/xen/pciback/conf_space_header.c
patch: **** malformed patch at line 139: *data)

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

On 01/03/2010 16:20, "Konrad Rzeszutek Wilk"
<konrad.wilk@oracle.com> wrote:
>> Some parts of this we had been given by Intel, but some were also
>> implemented differently there. I''m reproducing the patch
below, and
> 
> Could attach it as an attachment? I get:
> 
> patching file drivers/xen/pciback/conf_space_header.c
> patch: **** malformed patch at line 139: *data)
It applied for me oddly enough. The Intel patch is now in 2.6.18 as the
aggregate of changesets 998, 999 and 1003. It appears to be more
comprehensive than mine.

 -- Keir



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

On Mon, Mar 01, 2010 at 04:49:08PM +0000, Keir Fraser
wrote:
> On 01/03/2010 16:20, "Konrad Rzeszutek Wilk"
<konrad.wilk@oracle.com> wrote:
> 
> >> Some parts of this we had been given by Intel, but some were also
> >> implemented differently there. I''m reproducing the patch
below, and
> > 
> > Could attach it as an attachment? I get:
> > 
> > patching file drivers/xen/pciback/conf_space_header.c
> > patch: **** malformed patch at line 139: *data)
> 
> It applied for me oddly enough. The Intel patch is now in 2.6.18 as the
> aggregate of changesets 998, 999 and 1003. It appears to be more
> comprehensive than mine.
I took the patches out of the 2.6.18 and applied them and with some
compilation fixes made it work in the xen/master branch. Will soon push
them to Jeremy.

What do you use a DomU to test this? The only SR-IOV device I have is
the 82576 and I end up with this in 2.6.31.6 (xen/master):

[    1.053350] Intel(R) Virtual Function Network Driver - version 1.0.0-k0
[    1.053356] Copyright (c) 2009 Intel Corporation.
[    1.053795] igbvf 0000:01:10.0: enabling device (0000 -> 0002)
[    1.054007] igbvf 0000:01:10.0: Xen PCI enabling IRQ: 0
[    1.054007] igbvf 0000:01:10.0: enabling bus mastering
[    1.054007] igbvf 0000:01:10.0: setting latency timer to 64
[    1.054007]   alloc irq_desc for 103 on node 0
[    1.054007]   alloc kstat_irqs on node 0
[    1.054007] xen_allocate_pirq: returning irq 103 for gsi 103
[    1.054007] xen_allocate_pirq: returning irq 103 for gsi 103
[    1.094818] igbvf 0000:01:10.0: Intel(R) 82576 Virtual Function
[    1.094828] igbvf 0000:01:10.0: Address: 96:78:9d:7c:59:00
[    1.094834] igbvf 0000:01:10.0: MAC: 1
[    1.097275] initcall igbvf_init_module+0x0/0x70 [igbvf] returned 0 after
42891 usecs
[    1.097293] general protection fault: 0000 [#1] SMP 
[    1.097303] last sysfs file: /sys/devices/vfb-0/uevent
[    1.097308] CPU 0 
[    1.097314] Modules linked in: igbvf xen_blkfront xen_netfront xen_fbfront
fb_sys_fops sysimgblt sysfillrect syscopyarea xen_kbdfront
[    1.097343] Pid: 1113, comm: modprobe Tainted: G        W 
2.6.32NEB-00124-g402537c #49
[    1.097350] RIP: e030:[<ffffffff810b1c9e>]  [<ffffffff810b1c9e>]
remove_vm_area+0x3d/0x72
[    1.097363] RSP: e02b:ffff88001c28bec8  EFLAGS: 00010287
[    1.097368] RAX: c2c2c2c2c2c2c2c2 RBX: ffff88001cb0c940 RCX: 00000000c6e61701
[    1.097375] RDX: c2c2c2c2c2c2c2c2 RSI: ffffffff8168c1e0 RDI: ffffffff81572300
[    1.097381] RBP: ffff88001c28bed8 R08: 0000000000000000 R09: ffff88001fc0220a
[    1.097387] R10: ffff88001fc02200 R11: ffff88001c28be38 R12: ffff88001ab5da00
[    1.097393] R13: 0000000000000001 R14: 000000000000b4f0 R15: 0000000001f233f0
[    1.097404] FS:  00007f8c9ac3d6f0(0000) GS:ffff880005e62000(0000)
knlGS:0000000000000000
[    1.097411] CS:  e033 DS: 0000 ES: 0000 CR0: 000000008005003b
[    1.097417] CR2: 00000000014a9600 CR3: 000000001c2a2000 CR4: 0000000000002660
[    1.097423] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    1.097430] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[    1.097436] Process modprobe (pid: 1113, threadinfo ffff88001c28a000, task
ffff88001fda34f0)
[    1.097443] Stack:
[    1.097446]  ffffffffa002a920 0000000000000000 ffff88001c28bf18
ffffffff810b1dd0
[    1.097458] <0> ffff88001c28bf28 ffffffffa002e000 ffffffffa002a920
ffffffffa002a920
[    1.097473] <0> 0000000000000000 0000000001f29fb0 ffff88001c28bf28
ffffffff810b1eb1
[    1.097489] Call Trace:
[    1.097496]  [<ffffffff810b1dd0>] __vunmap+0x39/0xb8
[    1.097504]  [<ffffffffa002e000>] ? igbvf_probe+0x4ecf/0x4f3f [igbvf]
[    1.097512]  [<ffffffff810b1eb1>] vfree+0x29/0x2b
[    1.097520]  [<ffffffff8102a7a5>] module_free+0xc/0xe
[    1.097528]  [<ffffffff8106e7c4>] sys_init_module+0x1ec/0x230
[    1.097536]  [<ffffffff81011a02>] system_call_fastpath+0x16/0x1b
[    1.097542] Code: 74 54 f6 40 10 04 74 4e 48 c7 c7 00 23 57 81 48 8b 58 50 e8
a4 62 33 00 48 8b 05 4e 4c 65 00 48 c7 c2 e0 68 70 81 eb 06 48 89 c2 <48>
8b 00 48 39 d8 75 f5 48 8b 03 48 89 02 3e 81 05 49 06 4c 00
[    1.097767] RIP  [<ffffffff810b1c9e>] remove_vm_area+0x3d/0x72
[    1.097775]  RSP <ffff88001c28bec8>
[    1.097781] ---[ end trace 6f51333b6fdaedd4 ]---
[    1.097927] modprobe used greatest stack depth: 5144 bytes left
[    1.704018] usb usb2: suspend_rh (auto-stop)
[    1.704053] usb usb3: suspend_rh (auto-stop)

Updating to 2.6.32.8 with the pcifront back-port is not helping any
either (same error).

Does the 2.6.18 tree have a working DomU PV IGB driver?

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

On 01/03/2010 19:12, "Konrad Rzeszutek Wilk"
<konrad.wilk@oracle.com> wrote:
> What do you use a DomU to test this? The only SR-IOV device I have is
> the 82576 and I end up with this in 2.6.31.6 (xen/master):
>
> Updating to 2.6.32.8 with the pcifront back-port is not helping any
> either (same error).
> 
> Does the 2.6.18 tree have a working DomU PV IGB driver?
I built an out-of-tree ixgbevf driver, which drove a VF on an Intel Niantic
10G NIC.

 -- Keir



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

>>> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> 01.03.10 17:20
>>>
>On Mon, Mar 01, 2010 at 09:06:39AM +0000, Jan Beulich wrote:
>> Some parts of this we had been given by Intel, but some were also
>> implemented differently there. I''m reproducing the patch
below, and
>
>Could attach it as an attachment? I get:
>
>patching file drivers/xen/pciback/conf_space_header.c
>patch: **** malformed patch at line 139: *data)
Here you go.

Jan



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