Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Fri, Feb 08, 2013 at 10:18:19PM +1100, Vadim Rozenfeld wrote: On Thu, 2013-02-07 at 14:33 +0100, Laszlo Ersek wrote: Apologies in advance for asking a possibly exorbitantly lame question... On 02/06/13 10:47, Vadim Rozenfeld wrote: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: 05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? If a driver receives descriptor with memory type resources it needs to map physical address to logical. This process requires the number of bytes to be mapped, and it is the best place for sanity check to verify that the resources are valid and not corrupted. We usually do it by comparing the resource size with some predefined value, that we expect to see. Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. In theory, would the following approach work? (1) insist on revision-id being 0, (2) only require a minimum size of VIRTIO_PCI_CONFIG_NOMSI == 20 bytes from the iomem BAR (= common virtio header, guaranteed by the virtio spec for revision-id==0) when loading the driver, (3) at first, only map this initial (guaranteed) part of the resource, (4) this should provide access to the feature bits, allowing verification of the full BAR size, including MSI-X fields and device type specific fields, (5a) remap the BAR with full size if there's a match, I would try avoiding any remapping in virtio block and scsi drivers, especially when they are operating in system installation, crash dump and hibernation modes. Best regards, Vadim. Looks like there's some misunderstanding. It's an IO BAR, not a memory BAR. As such I don't think there is any remapping done on x86 before access. (5b) if there's a mismatch, set permanent hardware failure or some such in the loaded drier. It would be probably overkill; I'm just wondering if it made sense in theory. Thanks Laszlo
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, 2013-02-07 at 14:33 +0100, Laszlo Ersek wrote: Apologies in advance for asking a possibly exorbitantly lame question... On 02/06/13 10:47, Vadim Rozenfeld wrote: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: 05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? If a driver receives descriptor with memory type resources it needs to map physical address to logical. This process requires the number of bytes to be mapped, and it is the best place for sanity check to verify that the resources are valid and not corrupted. We usually do it by comparing the resource size with some predefined value, that we expect to see. Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. In theory, would the following approach work? (1) insist on revision-id being 0, (2) only require a minimum size of VIRTIO_PCI_CONFIG_NOMSI == 20 bytes from the iomem BAR (= common virtio header, guaranteed by the virtio spec for revision-id==0) when loading the driver, (3) at first, only map this initial (guaranteed) part of the resource, (4) this should provide access to the feature bits, allowing verification of the full BAR size, including MSI-X fields and device type specific fields, (5a) remap the BAR with full size if there's a match, I would try avoiding any remapping in virtio block and scsi drivers, especially when they are operating in system installation, crash dump and hibernation modes. Best regards, Vadim. (5b) if there's a mismatch, set permanent hardware failure or some such in the loaded drier. It would be probably overkill; I'm just wondering if it made sense in theory. Thanks Laszlo
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, Feb 07, 2013 at 11:33:04AM +1030, Rusty Russell wrote: Vadim Rozenfeld vroze...@redhat.com writes: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. But it's also true that if we'd incremented revid you'd have the same failure in this case, right? We'd also break all linux guests, and maybe more. -- MST
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote: Vadim Rozenfeld vroze...@redhat.com writes: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. But it's also true that if we'd incremented revid you'd have the same failure in this case, right? It depends. If we have explicitly specified revision id in inf file and this id doesn't mach the new revision id, Windows will not try to load the incompatible driver, and finish up with device driver not found dialog. Best regards, Vadim. Cheers, Rusty.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, Feb 07, 2013 at 08:24:10PM +1100, Vadim Rozenfeld wrote: On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote: Vadim Rozenfeld vroze...@redhat.com writes: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. But it's also true that if we'd incremented revid you'd have the same failure in this case, right? It depends. If we have explicitly specified revision id in inf file and this id doesn't mach the new revision id, Windows will not try to load the incompatible driver, and finish up with device driver not found dialog. Best regards, Vadim. Cheers, Rusty. Well that's all in theory, in practice it does not look like revision ID is specified in the NetKVM inf so this won't work? From what I see this inf specifies: NetKVM/wlh/netkvm.inf:%kvmnet6.DeviceDesc%= kvmnet6.ndi, PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4 NetKVM/wxp/netkvm.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi, PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4 NetKVM/wxp/netkvm2k.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi, PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4 So we can tweak any of vendor device and subsystem id. Changing subsystem vendor ID actually will be completely transparent to linux which for some reason looks at the subsystem device ID (why? no idea) but not the subsystem vendor ID. Of course this requires a valid vendor ID, getting this costs $3000 I think. We could tweak device ID too but that might break some other guests which don't copy the crazy 'replace device id with subsystem device id' logic from Linux. -- MST
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, 2013-02-07 at 12:18 +0200, Michael S. Tsirkin wrote: On Thu, Feb 07, 2013 at 08:24:10PM +1100, Vadim Rozenfeld wrote: On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote: Vadim Rozenfeld vroze...@redhat.com writes: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. But it's also true that if we'd incremented revid you'd have the same failure in this case, right? It depends. If we have explicitly specified revision id in inf file and this id doesn't mach the new revision id, Windows will not try to load the incompatible driver, and finish up with device driver not found dialog. Best regards, Vadim. Cheers, Rusty. Well that's all in theory, in practice it does not look like revision ID is specified in the NetKVM inf so this won't work? From what I see this inf specifies: NetKVM/wlh/netkvm.inf:%kvmnet6.DeviceDesc%= kvmnet6.ndi, PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4 NetKVM/wxp/netkvm.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi, PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4 NetKVM/wxp/netkvm2k.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi, PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4 So we can tweak any of vendor device and subsystem id. Unfortunately, it won't. Only balloon has revision id, specified as a part of device HW descriptor. But it's only because virtio doesn't use revision ids. Otherwise it differential will be there. Changing subsystem vendor ID actually will be completely transparent to linux which for some reason looks at the subsystem device ID (why? no idea) but not the subsystem vendor ID. Of course this requires a valid vendor ID, getting this costs $3000 I think. We could tweak device ID too but that might break some other guests which don't copy the crazy 'replace device id with subsystem device id' logic from Linux.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, Feb 07, 2013 at 09:53:39PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 12:18 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 08:24:10PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote:
Vadim Rozenfeld vroze...@redhat.com writes:
On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote:
On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote:
Is it really
that bad that the config space size changed? Why it has this
effect?
Because in this case it's hard to distinguish between resource's
corruption and HW update.
But it's also true that if we'd incremented revid you'd have the same
failure in this case, right?
It depends. If we have explicitly specified revision id in inf file and
this id doesn't mach the new revision id, Windows will not try to load
the incompatible driver, and finish up with device driver not found
dialog.
Best regards,
Vadim.
Cheers,
Rusty.
Well that's all in theory, in practice it does not look like revision ID
is specified in the NetKVM inf so this won't work?
From what I see this inf specifies:
NetKVM/wlh/netkvm.inf:%kvmnet6.DeviceDesc%= kvmnet6.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm2k.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
So we can tweak any of vendor device and subsystem id.
Unfortunately, it won't. Only balloon has revision id, specified as a
part of device HW descriptor. But it's only because virtio doesn't use
revision ids. Otherwise it differential will be there.
So your driver will load and attempt to work on rev=2 devices?
If yes it's a bug. virtio spec specifies revision id as an ABI version.
Linux driver does:
if (pci_dev-revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR virtio_pci: expected ABI version %d, got %d\n,
VIRTIO_PCI_ABI_VERSION, pci_dev-revision);
return -ENODEV;
}
Changing subsystem vendor ID actually will be completely
transparent to linux which for some reason looks at the
subsystem device ID (why? no idea) but not the subsystem vendor ID.
Of course this requires a valid vendor ID, getting this
costs $3000 I think.
We could tweak device ID too but that might break some other guests
which don't copy the crazy 'replace device id with subsystem device id'
logic from Linux.
Apropos, would you guys like to start to copy your patches to
virtualizat...@lists.linux-foundation.org?
If you do, you might get some review and feedback, allowing
us to catch such forward compatibility issues earlier.
Of course it's your project so entirely up to you.
--
MST
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, 2013-02-07 at 13:33 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 09:53:39PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 12:18 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 08:24:10PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote:
Vadim Rozenfeld vroze...@redhat.com writes:
On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote:
On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote:
Is it really
that bad that the config space size changed? Why it has this
effect?
Because in this case it's hard to distinguish between resource's
corruption and HW update.
But it's also true that if we'd incremented revid you'd have the same
failure in this case, right?
It depends. If we have explicitly specified revision id in inf file and
this id doesn't mach the new revision id, Windows will not try to load
the incompatible driver, and finish up with device driver not found
dialog.
Best regards,
Vadim.
Cheers,
Rusty.
Well that's all in theory, in practice it does not look like revision ID
is specified in the NetKVM inf so this won't work?
From what I see this inf specifies:
NetKVM/wlh/netkvm.inf:%kvmnet6.DeviceDesc%= kvmnet6.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm2k.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
So we can tweak any of vendor device and subsystem id.
Unfortunately, it won't. Only balloon has revision id, specified as a
part of device HW descriptor. But it's only because virtio doesn't use
revision ids. Otherwise it differential will be there.
So your driver will load and attempt to work on rev=2 devices?
All virtio-win drivers (net, serial, block, and scsi), except for
balloon will.
If yes it's a bug. virtio spec specifies revision id as an ABI version.
Linux driver does:
if (pci_dev-revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR virtio_pci: expected ABI version %d, got
%d\n,
VIRTIO_PCI_ABI_VERSION, pci_dev-revision);
return -ENODEV;
}
Changing subsystem vendor ID actually will be completely
transparent to linux which for some reason looks at the
subsystem device ID (why? no idea) but not the subsystem vendor ID.
Of course this requires a valid vendor ID, getting this
costs $3000 I think.
We could tweak device ID too but that might break some other guests
which don't copy the crazy 'replace device id with subsystem device id'
logic from Linux.
Apropos, would you guys like to start to copy your patches to
virtualizat...@lists.linux-foundation.org?
If you do, you might get some review and feedback, allowing
us to catch such forward compatibility issues earlier.
Of course it's your project so entirely up to you.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Feb 7, 2013, at 1:46 PM, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 13:33 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 09:53:39PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 12:18 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 08:24:10PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote:
Vadim Rozenfeld vroze...@redhat.com writes:
On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote:
On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote:
Is it really
that bad that the config space size changed? Why it has this effect?
Because in this case it's hard to distinguish between resource's
corruption and HW update.
But it's also true that if we'd incremented revid you'd have the same
failure in this case, right?
It depends. If we have explicitly specified revision id in inf file and
this id doesn't mach the new revision id, Windows will not try to load
the incompatible driver, and finish up with device driver not found
dialog.
Best regards,
Vadim.
Cheers,
Rusty.
Well that's all in theory, in practice it does not look like revision ID
is specified in the NetKVM inf so this won't work?
From what I see this inf specifies:
NetKVM/wlh/netkvm.inf:%kvmnet6.DeviceDesc%= kvmnet6.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm2k.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
So we can tweak any of vendor device and subsystem id.
Unfortunately, it won't. Only balloon has revision id, specified as a
part of device HW descriptor. But it's only because virtio doesn't use
revision ids. Otherwise it differential will be there.
So your driver will load and attempt to work on rev=2 devices?
All virtio-win drivers (net, serial, block, and scsi), except for
balloon will.
If yes it's a bug. virtio spec specifies revision id as an ABI version.
Linux driver does:
if (pci_dev-revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR virtio_pci: expected ABI version %d, got
%d\n,
VIRTIO_PCI_ABI_VERSION, pci_dev-revision);
return -ENODEV;
}
We can add it from next release. Provided that everyone understand the
consequences especially for boot devices (virtio-block and virtio-scsi).
Changing subsystem vendor ID actually will be completely
transparent to linux which for some reason looks at the
subsystem device ID (why? no idea) but not the subsystem vendor ID.
Of course this requires a valid vendor ID, getting this
costs $3000 I think.
We could tweak device ID too but that might break some other guests
which don't copy the crazy 'replace device id with subsystem device id'
logic from Linux.
Apropos, would you guys like to start to copy your patches to
virtualizat...@lists.linux-foundation.org?
If you do, you might get some review and feedback, allowing
us to catch such forward compatibility issues earlier.
Of course it's your project so entirely up to you.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, Feb 07, 2013 at 03:02:45PM +0200, Yan Vugenfirer wrote:
On Feb 7, 2013, at 1:46 PM, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 13:33 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 09:53:39PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 12:18 +0200, Michael S. Tsirkin wrote:
On Thu, Feb 07, 2013 at 08:24:10PM +1100, Vadim Rozenfeld wrote:
On Thu, 2013-02-07 at 11:33 +1030, Rusty Russell wrote:
Vadim Rozenfeld vroze...@redhat.com writes:
On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote:
On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote:
Is it really
that bad that the config space size changed? Why it has this
effect?
Because in this case it's hard to distinguish between resource's
corruption and HW update.
But it's also true that if we'd incremented revid you'd have the same
failure in this case, right?
It depends. If we have explicitly specified revision id in inf file and
this id doesn't mach the new revision id, Windows will not try to load
the incompatible driver, and finish up with device driver not found
dialog.
Best regards,
Vadim.
Cheers,
Rusty.
Well that's all in theory, in practice it does not look like revision ID
is specified in the NetKVM inf so this won't work?
From what I see this inf specifies:
NetKVM/wlh/netkvm.inf:%kvmnet6.DeviceDesc%= kvmnet6.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
NetKVM/wxp/netkvm2k.inf:%kvmnet5.DeviceDesc%= kvmnet5.ndi,
PCI\VEN_1AF4DEV_1000SUBSYS_00011AF4
So we can tweak any of vendor device and subsystem id.
Unfortunately, it won't. Only balloon has revision id, specified as a
part of device HW descriptor. But it's only because virtio doesn't use
revision ids. Otherwise it differential will be there.
So your driver will load and attempt to work on rev=2 devices?
All virtio-win drivers (net, serial, block, and scsi), except for
balloon will.
If yes it's a bug. virtio spec specifies revision id as an ABI version.
Linux driver does:
if (pci_dev-revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR virtio_pci: expected ABI version %d, got
%d\n,
VIRTIO_PCI_ABI_VERSION, pci_dev-revision);
return -ENODEV;
}
We can add it from next release. Provided that everyone understand the
consequences especially for boot devices (virtio-block and virtio-scsi).
Same as Linux - guest won't boot.
Changing subsystem vendor ID actually will be completely
transparent to linux which for some reason looks at the
subsystem device ID (why? no idea) but not the subsystem vendor ID.
Of course this requires a valid vendor ID, getting this
costs $3000 I think.
We could tweak device ID too but that might break some other guests
which don't copy the crazy 'replace device id with subsystem device id'
logic from Linux.
Apropos, would you guys like to start to copy your patches to
virtualizat...@lists.linux-foundation.org?
If you do, you might get some review and feedback, allowing
us to catch such forward compatibility issues earlier.
Of course it's your project so entirely up to you.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Apologies in advance for asking a possibly exorbitantly lame question... On 02/06/13 10:47, Vadim Rozenfeld wrote: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: 05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? If a driver receives descriptor with memory type resources it needs to map physical address to logical. This process requires the number of bytes to be mapped, and it is the best place for sanity check to verify that the resources are valid and not corrupted. We usually do it by comparing the resource size with some predefined value, that we expect to see. Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. In theory, would the following approach work? (1) insist on revision-id being 0, (2) only require a minimum size of VIRTIO_PCI_CONFIG_NOMSI == 20 bytes from the iomem BAR (= common virtio header, guaranteed by the virtio spec for revision-id==0) when loading the driver, (3) at first, only map this initial (guaranteed) part of the resource, (4) this should provide access to the feature bits, allowing verification of the full BAR size, including MSI-X fields and device type specific fields, (5a) remap the BAR with full size if there's a match, (5b) if there's a mismatch, set permanent hardware failure or some such in the loaded drier. It would be probably overkill; I'm just wondering if it made sense in theory. Thanks Laszlo
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Thu, Feb 07, 2013 at 02:33:28PM +0100, Laszlo Ersek wrote: Apologies in advance for asking a possibly exorbitantly lame question... On 02/06/13 10:47, Vadim Rozenfeld wrote: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: 05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? If a driver receives descriptor with memory type resources it needs to map physical address to logical. This process requires the number of bytes to be mapped, and it is the best place for sanity check to verify that the resources are valid and not corrupted. We usually do it by comparing the resource size with some predefined value, that we expect to see. Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. In theory, would the following approach work? (1) insist on revision-id being 0, (2) only require a minimum size of VIRTIO_PCI_CONFIG_NOMSI == 20 bytes from the iomem BAR (= common virtio header, guaranteed by the virtio spec for revision-id==0) when loading the driver, (3) at first, only map this initial (guaranteed) part of the resource, (4) this should provide access to the feature bits, allowing verification of the full BAR size, including MSI-X fields and device type specific fields, (5a) remap the BAR with full size if there's a match, (5b) if there's a mismatch, set permanent hardware failure or some such in the loaded drier. It would be probably overkill; I'm just wondering if it made sense in theory. Thanks Laszlo Linux simply maps the whole BAR. It's an IO BAR so never bigger than 256 bytes. It then checks feature bits before access. We could also range-check versus the BAR size, though we don't at the moment. The API looks like this: if (virtio_config_val(vi-vdev, VIRTIO_NET_F_STATUS, offsetof(struct virtio_net_config, status), v) 0) -- MST
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: 05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: Vadim Rozenfeld vroze...@redhat.com writes: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx That's a useful link, thanks. I don't see anything in that link that would strictly require us to change the revision ID. It seems to focus on when the software interface changes. I take that to mean, change the revision ID if an old driver wouldn't work anymore which makes complete sense. Right, nobody can force you to change the revision id. It's just a good engineering practice to increase RevID every time the HW interface has changed, and you expect some compatibility issue between the new HW and old drivers. In this case, if Windows cannot locate the INF file which matches the new device identification strings, it just reports that it cannot find a suitable driver. But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? If a driver receives descriptor with memory type resources it needs to map physical address to logical. This process requires the number of bytes to be mapped, and it is the best place for sanity check to verify that the resources are valid and not corrupted. We usually do it by comparing the resource size with some predefined value, that we expect to see. Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. Is this size specified in the inf file somehow? Just for us to actually understand the issue? No, it is not specified in INF. We usually keep it hard-coded, but technically it can be specified in INF and the Registry. Best regards, Vadim. Thanks! /mjt This should be educational: https://github.com/YanVugenfirer/kvm-guest-drivers-windows/commit/10413d2bbef295cc0e0e75131147793ccc382155
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On 02/05/2013 11:49 PM, Lucas Meneghel Rodrigues wrote: On 02/05/2013 09:58 AM, Michael S. Tsirkin wrote: Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? Is it really that bad that the config space size changed? Why it has this effect? Is this size specified in the inf file somehow? Just for us to actually understand the issue? Thanks! /mjt This should be educational: https://github.com/YanVugenfirer/kvm-guest-drivers-windows/commit/10413d2bbef295cc0e0e75131147793ccc382155 I guess I chose a bad timing to do test grid maintenance. I've stopped the grid last Thursday to upgrade the test code, then when I resumed operations yesterday, I was checking out on the qemu.git results. I found out that all Windows tests were failing due to a failure to start the virtio driver. Attached there's an ogg video of what happens. I found this thread and then things make sense... Well, anyway, once a fix is in place, it'll be easy to verify whether the problem is solved. I'll keep checking on this thread for a fix and report whether it was successful or not. Just reporting that the latest build of the internal virtio drivers (-54) passed all tests on latest qemu.git, no more networking problems. Kernel (kvm.git): 02/06 18:31:29 INFO | git:0153| git commit ID is 949db153b6466c6f7cad5a427ecea94985927311 (tag kvm-3.8-1-13793-g949db15) QEMU (qemu.git): 02/06 18:46:06 INFO | git:0153| git commit ID is 5f876756c57c15f5e14d4136fc432b74f05f082b (tag v1.4.0-rc0-15-g5f87675) Windows Drivers build: 54 (Iso built automatically during the night).
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Vadim Rozenfeld vroze...@redhat.com writes: On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: Is it really that bad that the config space size changed? Why it has this effect? Because in this case it's hard to distinguish between resource's corruption and HW update. But it's also true that if we'd incremented revid you'd have the same failure in this case, right? Cheers, Rusty.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Michael S. Tsirkin m...@redhat.com writes: Maybe we should ask for some centrally assigned vendor id? We could ask IANA to keep the database. Or we could make it the task of the virtio spec. I don't think it's vital... Cheers, Rusty.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: Vadim Rozenfeld vroze...@redhat.com writes: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx That's a useful link, thanks. I don't see anything in that link that would strictly require us to change the revision ID. It seems to focus on when the software interface changes. I take that to mean, change the revision ID if an old driver wouldn't work anymore which makes complete sense. Right, nobody can force you to change the revision id. It's just a good engineering practice to increase RevID every time the HW interface has changed, and you expect some compatibility issue between the new HW and old drivers. In this case, if Windows cannot locate the INF file which matches the new device identification strings, it just reports that it cannot find a suitable driver. But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? Is it really that bad that the config space size changed? Why it has this effect? Is this size specified in the inf file somehow? Just for us to actually understand the issue? Thanks! /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: Vadim Rozenfeld vroze...@redhat.com writes: On Mon, 2013-02-04 at 14:22 +0200, Michael S. Tsirkin wrote: On Mon, Feb 04, 2013 at 02:00:46PM +0200, Yan Vugenfirer wrote: virtio spec is very explicit that revision never changes: The device must also have a Revision ID of 0 to match this specification. With all my respect to the virtio spec, let me point that Windows lives by different rules: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx That's a useful link, thanks. I don't see anything in that link that would strictly require us to change the revision ID. It seems to focus on when the software interface changes. I take that to mean, change the revision ID if an old driver wouldn't work anymore which makes complete sense. Right, nobody can force you to change the revision id. It's just a good engineering practice to increase RevID every time the HW interface has changed, and you expect some compatibility issue between the new HW and old drivers. In this case, if Windows cannot locate the INF file which matches the new device identification strings, it just reports that it cannot find a suitable driver. But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Best regards, Vadim. Regards, Anthony Liguori It also explicitly documents the use of feature bits for adding new fields: In particular, new fields in the device configuration header are indicated by offering a feature bit, so the guest can check before accessing that part of the configuration space. This allows for forwards and backwards compatibility: if the device is enhanced with a new feature bit, older guests will not write that feature bit back to the Guest Features field and it can go into backwards compatibility mode. Similarly, if a guest is enhanced with a feature that the device doesn't support, it will not see that feature bit in the Device Features field and can go into backwards compatibility mode (or, for poor implementations, set the FAILED Device Status bit). I really don't see how this can be interpreted except as a promise to add fields and a requirement for guest drivers to be forward compatible. It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: 05.02.2013 15:31, Vadim Rozenfeld wrote: On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: Vadim Rozenfeld vroze...@redhat.com writes: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx That's a useful link, thanks. I don't see anything in that link that would strictly require us to change the revision ID. It seems to focus on when the software interface changes. I take that to mean, change the revision ID if an old driver wouldn't work anymore which makes complete sense. Right, nobody can force you to change the revision id. It's just a good engineering practice to increase RevID every time the HW interface has changed, and you expect some compatibility issue between the new HW and old drivers. In this case, if Windows cannot locate the INF file which matches the new device identification strings, it just reports that it cannot find a suitable driver. But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. I agree, feature bits are good. The only one problem with them, is that driver usually doesn't have access to PCI space during the driver loading phase. Vadim, can you please describe in a bit more details what the actual issue here is, from the windows or windows driver point of view? Is it really that bad that the config space size changed? Why it has this effect? Is this size specified in the inf file somehow? Just for us to actually understand the issue? Thanks! /mjt This should be educational: https://github.com/YanVugenfirer/kvm-guest-drivers-windows/commit/10413d2bbef295cc0e0e75131147793ccc382155 -- MST
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Anthony Liguori anth...@codemonkey.ws writes: Rusty Russell ru...@rustcorp.com.au writes: If I could find a way, I'd like to create some code as an appendix to the virtio spec which would torture test each driver and/or device by configuring it in strange ways. But that's pure speculation at this point. It wouldn't be so hard to add a torture parameter to the virtio implementation in QEMU that would do silly things that were still within the bounds of the specification. Larger config sizes, advertisement of unknown feature bits, etc. My thought is that alongside the spec we provide two libraries for each device class: a device-side and a driver-side. Each one gets fed an integer (which controls which craziness it does) and you test against each variant. For testing qemu, we could either put sew this test code into Linux, or hack it into qemu and pretend it was guest-driven (handwave). In practice, we'd want to formalize it into a string and a version, and hope the version gets bumped appropriately. Because it'll actually be used for bug detection. Sure. But AFAICT that would be useless in this case. We really want to know about the guest before we even start it. We can't just prepare to fight yesterday's wars :-) s/just/even/. We'd want to know the string before we expose host features. That's easy. Absolutely. But we'd need a virtio2-like spec change, which insisted that the driver write this version number somewhere before inspecting any other field. Or? How we expose config space in virtio2 is a separate discussion. If we take a list approach like you've talked about before, it would be much harder for drivers to assume anything about the BAR size for config since the size would always be different. I think it's the same discussion. Strings are hard: how about a 16 bit implementation id (don't clash with others) and a 16 bit version number (increment as driver changes). Should we also loosen revid to be a version number for the device? It's only 8 bits though, so perhaps new config fields are better. Cheers, Rusty.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Wed, Feb 06, 2013 at 10:33:41AM +1030, Rusty Russell wrote: Anthony Liguori anth...@codemonkey.ws writes: Rusty Russell ru...@rustcorp.com.au writes: If I could find a way, I'd like to create some code as an appendix to the virtio spec which would torture test each driver and/or device by configuring it in strange ways. But that's pure speculation at this point. It wouldn't be so hard to add a torture parameter to the virtio implementation in QEMU that would do silly things that were still within the bounds of the specification. Larger config sizes, advertisement of unknown feature bits, etc. My thought is that alongside the spec we provide two libraries for each device class: a device-side and a driver-side. Each one gets fed an integer (which controls which craziness it does) and you test against each variant. For testing qemu, we could either put sew this test code into Linux, or hack it into qemu and pretend it was guest-driven (handwave). In practice, we'd want to formalize it into a string and a version, and hope the version gets bumped appropriately. Because it'll actually be used for bug detection. Sure. But AFAICT that would be useless in this case. We really want to know about the guest before we even start it. We can't just prepare to fight yesterday's wars :-) s/just/even/. We'd want to know the string before we expose host features. That's easy. Absolutely. But we'd need a virtio2-like spec change, which insisted that the driver write this version number somewhere before inspecting any other field. Or? How we expose config space in virtio2 is a separate discussion. If we take a list approach like you've talked about before, it would be much harder for drivers to assume anything about the BAR size for config since the size would always be different. I think it's the same discussion. Strings are hard: how about a 16 bit implementation id (don't clash with others) and a 16 bit version number (increment as driver changes). Should we also loosen revid to be a version number for the device? It's only 8 bits though, so perhaps new config fields are better. Cheers, Rusty. Maybe we should ask for some centrally assigned vendor id? We could ask IANA to keep the database. -- MST
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Feb 4, 2013, at 1:30 AM, Vadim Rozenfeld wrote: On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? I've got a patch that does exactly like this. It's pretty ugly though because of the relationship between virtio-pci and virtio-net. I'm going to try to see if I can find a cleaner way to do it on Monday. I'm also contemplating just disabling mq by default. Since a special command line is needed to enable it anyway (on the backend), having to specify mq=on for the device doesn't seem so bad. But yeah, we don't want Windows guests to break with -M pc by default so we should do something to work around it. N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not Why not just use revision id for that? That's what would be expected from real HW if size of the BAR is changed. It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Mon, Feb 04, 2013 at 02:00:46PM +0200, Yan Vugenfirer wrote: On Feb 4, 2013, at 1:30 AM, Vadim Rozenfeld wrote: On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? I've got a patch that does exactly like this. It's pretty ugly though because of the relationship between virtio-pci and virtio-net. I'm going to try to see if I can find a cleaner way to do it on Monday. I'm also contemplating just disabling mq by default. Since a special command line is needed to enable it anyway (on the backend), having to specify mq=on for the device doesn't seem so bad. But yeah, we don't want Windows guests to break with -M pc by default so we should do something to work around it. N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not Why not just use revision id for that? That's what would be expected from real HW if size of the BAR is changed. virtio spec is very explicit that revision never changes: The device must also have a Revision ID of 0 to match this specification. It also explicitly documents the use of feature bits for adding new fields: In particular, new fields in the device configuration header are indicated by offering a feature bit, so the guest can check before accessing that part of the configuration space. This allows for forwards and backwards compatibility: if the device is enhanced with a new feature bit, older guests will not write that feature bit back to the Guest Features field and it can go into backwards compatibility mode. Similarly, if a guest is enhanced with a feature that the device doesn't support, it will not see that feature bit in the Device Features field and can go into backwards compatibility mode (or, for poor implementations, set the FAILED Device Status bit). I really don't see how this can be interpreted except as a promise to add fields and a requirement for guest drivers to be forward compatible. It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Mon, 2013-02-04 at 14:22 +0200, Michael S. Tsirkin wrote: On Mon, Feb 04, 2013 at 02:00:46PM +0200, Yan Vugenfirer wrote: On Feb 4, 2013, at 1:30 AM, Vadim Rozenfeld wrote: On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? I've got a patch that does exactly like this. It's pretty ugly though because of the relationship between virtio-pci and virtio-net. I'm going to try to see if I can find a cleaner way to do it on Monday. I'm also contemplating just disabling mq by default. Since a special command line is needed to enable it anyway (on the backend), having to specify mq=on for the device doesn't seem so bad. But yeah, we don't want Windows guests to break with -M pc by default so we should do something to work around it. N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not Why not just use revision id for that? That's what would be expected from real HW if size of the BAR is changed. virtio spec is very explicit that revision never changes: The device must also have a Revision ID of 0 to match this specification. With all my respect to the virtio spec, let me point that Windows lives by different rules: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx It also explicitly documents the use of feature bits for adding new fields: In particular, new fields in the device configuration header are indicated by offering a feature bit, so the guest can check before accessing that part of the configuration space. This allows for forwards and backwards compatibility: if the device is enhanced with a new feature bit, older guests will not write that feature bit back to the Guest Features field and it can go into backwards compatibility mode. Similarly, if a guest is enhanced with a feature that the device doesn't support, it will not see that feature bit in the Device Features field and can go into backwards compatibility mode (or, for poor implementations, set the FAILED Device Status bit). I really don't see how this can be interpreted except as a promise to add fields and a requirement for guest drivers to be forward compatible. It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Rusty Russell ru...@rustcorp.com.au writes: Anthony Liguori anth...@codemonkey.ws writes: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. ... N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. Wow, yes. I never expected such a bug. I probably don't even want to know how this happened, right? If I could find a way, I'd like to create some code as an appendix to the virtio spec which would torture test each driver and/or device by configuring it in strange ways. But that's pure speculation at this point. It wouldn't be so hard to add a torture parameter to the virtio implementation in QEMU that would do silly things that were still within the bounds of the specification. Larger config sizes, advertisement of unknown feature bits, etc. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. In practice, we'd want to formalize it into a string and a version, and hope the version gets bumped appropriately. Because it'll actually be used for bug detection. Sure. But AFAICT that would be useless in this case. We really want to know about the guest before we even start it. We can't just prepare to fight yesterday's wars :-) We'd want to know the string before we expose host features. That's easy. We've had a lot of trouble with certain feature bits breaking drivers so masking certain features for known broken drivers would be really useful. How we expose config space in virtio2 is a separate discussion. If we take a list approach like you've talked about before, it would be much harder for drivers to assume anything about the BAR size for config since the size would always be different. Regards, Anthony Liguori Cheers, Rusty.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Vadim Rozenfeld vroze...@redhat.com writes: On Mon, 2013-02-04 at 14:22 +0200, Michael S. Tsirkin wrote: On Mon, Feb 04, 2013 at 02:00:46PM +0200, Yan Vugenfirer wrote: virtio spec is very explicit that revision never changes: The device must also have a Revision ID of 0 to match this specification. With all my respect to the virtio spec, let me point that Windows lives by different rules: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx That's a useful link, thanks. I don't see anything in that link that would strictly require us to change the revision ID. It seems to focus on when the software interface changes. I take that to mean, change the revision ID if an old driver wouldn't work anymore which makes complete sense. But adding feature bits an altering the config size doesn't constitute a change in the software interface IMHO. Regards, Anthony Liguori It also explicitly documents the use of feature bits for adding new fields: In particular, new fields in the device configuration header are indicated by offering a feature bit, so the guest can check before accessing that part of the configuration space. This allows for forwards and backwards compatibility: if the device is enhanced with a new feature bit, older guests will not write that feature bit back to the Guest Features field and it can go into backwards compatibility mode. Similarly, if a guest is enhanced with a feature that the device doesn't support, it will not see that feature bit in the Device Features field and can go into backwards compatibility mode (or, for poor implementations, set the FAILED Device Status bit). I really don't see how this can be interpreted except as a promise to add fields and a requirement for guest drivers to be forward compatible. It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Feb 3, 2013, at 1:07 AM, Anthony Liguori wrote: Vadim Rozenfeld vroze...@redhat.com writes: On Sat, 2013-02-02 at 20:42 +0800, Jason Wang wrote: Have a look at this issue. It was caused by multiqueue patch who adds a new field to virtio_net_cfg. Not sure multiqueue is the root cause since I also find even w/o multiqueue, adding any new field to virtio_net_cfg will break windows guest. Haven't had a clue on this, will continue investigate. cc'ing Yan, our NDIS guy. If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. Regards, Anthony Liguori That's exactly the problem. Best regards, Yan. Thank you, Vadim. Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? I've got a patch that does exactly like this. It's pretty ugly though because of the relationship between virtio-pci and virtio-net. I'm going to try to see if I can find a cleaner way to do it on Monday. I'm also contemplating just disabling mq by default. Since a special command line is needed to enable it anyway (on the backend), having to specify mq=on for the device doesn't seem so bad. But yeah, we don't want Windows guests to break with -M pc by default so we should do something to work around it. N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Sun, Feb 03, 2013 at 03:09:39PM -0600, Anthony Liguori wrote:
Michael Tokarev m...@tls.msk.ru writes:
03.02.2013 17:23, Yan Vugenfirer wrote:
If it helps, mq changes the config size from 8 to 16 bytes. If the
driver was making an assumption about an 8-byte config size, that's
likely what the problem is.
That's exactly the problem.
So what do we do? It isn't nice to break existing setups.
Maybe mq can be disabled by default (together with Antony's
patch) until fixed drivers will be more widely available?
I've got a patch that does exactly like this. It's pretty ugly though
because of the relationship between virtio-pci and virtio-net. I'm
going to try to see if I can find a cleaner way to do it on Monday.
I'm also contemplating just disabling mq by default. Since a special
command line is needed to enable it anyway (on the backend), having to
specify mq=on for the device doesn't seem so bad.
It's after midnight here so didn't test yet, but wouldn't the following
achieve this in a way that is a bit cleaner?
Signed-off-by: Michael S. Tsirkin m...@redhat.com
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5699f5e..3342a76 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice
*vdev, uint32_t features)
features |= (1 VIRTIO_NET_F_MAC);
+if (n-max_queues = 1) {
+features = ~(0x1 VIRTIO_NET_F_MQ);
+}
+
if (!peer_has_vnet_hdr(n)) {
features = ~(0x1 VIRTIO_NET_F_CSUM);
features = ~(0x1 VIRTIO_NET_F_HOST_TSO4);
@@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf
*conf,
int i;
n = (VirtIONet *)virtio_common_init(virtio-net, VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf-queues 1 ?
+sizeof(struct virtio_net_config) :
+sizeof(struct
virtio_net_config_compat),
sizeof(VirtIONet));
n-vdev.get_config = virtio_net_get_config;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1d5c721..95ad54f 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -69,6 +69,15 @@ typedef struct virtio_net_conf
/* Maximum packet size we can receive from tap device: header + 64k */
#define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 10))
+
+struct virtio_net_config_compat
+{
+/* The config defining mac address ($ETH_ALEN bytes) */
+uint8_t mac[ETH_ALEN];
+/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+uint16_t status;
+} QEMU_PACKED;
+
struct virtio_net_config
{
/* The config defining mac address ($ETH_ALEN bytes) */
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Michael S. Tsirkin m...@redhat.com writes:
On Sun, Feb 03, 2013 at 03:09:39PM -0600, Anthony Liguori wrote:
Michael Tokarev m...@tls.msk.ru writes:
03.02.2013 17:23, Yan Vugenfirer wrote:
If it helps, mq changes the config size from 8 to 16 bytes. If the
driver was making an assumption about an 8-byte config size, that's
likely what the problem is.
That's exactly the problem.
So what do we do? It isn't nice to break existing setups.
Maybe mq can be disabled by default (together with Antony's
patch) until fixed drivers will be more widely available?
I've got a patch that does exactly like this. It's pretty ugly though
because of the relationship between virtio-pci and virtio-net. I'm
going to try to see if I can find a cleaner way to do it on Monday.
I'm also contemplating just disabling mq by default. Since a special
command line is needed to enable it anyway (on the backend), having to
specify mq=on for the device doesn't seem so bad.
It's after midnight here so didn't test yet, but wouldn't the following
achieve this in a way that is a bit cleaner?
Signed-off-by: Michael S. Tsirkin m...@redhat.com
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5699f5e..3342a76 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice
*vdev, uint32_t features)
features |= (1 VIRTIO_NET_F_MAC);
+if (n-max_queues = 1) {
+features = ~(0x1 VIRTIO_NET_F_MQ);
+}
+
This is too late because the config size has already been set.
if (!peer_has_vnet_hdr(n)) {
features = ~(0x1 VIRTIO_NET_F_CSUM);
features = ~(0x1 VIRTIO_NET_F_HOST_TSO4);
@@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf
*conf,
int i;
n = (VirtIONet *)virtio_common_init(virtio-net, VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf-queues 1 ?
+sizeof(struct virtio_net_config)
:
+sizeof(struct
virtio_net_config_compat),
sizeof(VirtIONet));
This makes me a bit nervous. It relies on setting the config size to
one thing before we've masked a feature.
What do you think about just defaulting mq=off? The more I think about
it, the nicer of an option that is.
I'm not a huge fan of automagically changing the value of a feature bit
so I think defaulting it off is a bit more straight forward.
We can certainly revisit for 1.5+.
Regards,
Anthony Liguori
n-vdev.get_config = virtio_net_get_config;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1d5c721..95ad54f 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -69,6 +69,15 @@ typedef struct virtio_net_conf
/* Maximum packet size we can receive from tap device: header + 64k */
#define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 10))
+
+struct virtio_net_config_compat
+{
+/* The config defining mac address ($ETH_ALEN bytes) */
+uint8_t mac[ETH_ALEN];
+/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+uint16_t status;
+} QEMU_PACKED;
+
struct virtio_net_config
{
/* The config defining mac address ($ETH_ALEN bytes) */
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? I've got a patch that does exactly like this. It's pretty ugly though because of the relationship between virtio-pci and virtio-net. I'm going to try to see if I can find a cleaner way to do it on Monday. I'm also contemplating just disabling mq by default. Since a special command line is needed to enable it anyway (on the backend), having to specify mq=on for the device doesn't seem so bad. But yeah, we don't want Windows guests to break with -M pc by default so we should do something to work around it. N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not Why not just use revision id for that? It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Anthony Liguori anth...@codemonkey.ws writes: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. ... N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. Wow, yes. I never expected such a bug. I probably don't even want to know how this happened, right? If I could find a way, I'd like to create some code as an appendix to the virtio spec which would torture test each driver and/or device by configuring it in strange ways. But that's pure speculation at this point. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. In practice, we'd want to formalize it into a string and a version, and hope the version gets bumped appropriately. Because it'll actually be used for bug detection. But AFAICT that would be useless in this case. We really want to know about the guest before we even start it. Cheers, Rusty.
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Mon, Feb 04, 2013 at 10:30:47AM +1100, Vadim Rozenfeld wrote: On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 03.02.2013 17:23, Yan Vugenfirer wrote: If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. That's exactly the problem. So what do we do? It isn't nice to break existing setups. Maybe mq can be disabled by default (together with Antony's patch) until fixed drivers will be more widely available? I've got a patch that does exactly like this. It's pretty ugly though because of the relationship between virtio-pci and virtio-net. I'm going to try to see if I can find a cleaner way to do it on Monday. I'm also contemplating just disabling mq by default. Since a special command line is needed to enable it anyway (on the backend), having to specify mq=on for the device doesn't seem so bad. But yeah, we don't want Windows guests to break with -M pc by default so we should do something to work around it. N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net feature is going to trigger it (not just multiqueue). So while pc-1.3 will work forever with this guest image, it's pretty much guaranteed to break at some point in the future. It's easy to turn off mq by default and turn it on when needed... The problem now is that it isn't obvious why your existing setup breaks when you upgrade. While when you especially play with mq, you may look at options available around it... If we ever to get virtio2, a really handy feature to have would be a driver identification string. Even if was only informative (and not Why not just use revision id for that? This will break well-behaved linux guests. It really can be very useful, at least for virtio Windows drivers. BTW, Yan already fixed this problem in the Windows driver code. So we can make a new build and make it public. But it probably will not be WHQL'ed in the nearest future. authoritative, we've had a lot of issues like this and being able to make work arounds conditional on the driver identification string would be nice. Regards, Anthony Liguori How difficult it is to fix it in win driver? Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Sun, Feb 03, 2013 at 04:57:38PM -0600, Anthony Liguori wrote:
Michael S. Tsirkin m...@redhat.com writes:
On Sun, Feb 03, 2013 at 03:09:39PM -0600, Anthony Liguori wrote:
Michael Tokarev m...@tls.msk.ru writes:
03.02.2013 17:23, Yan Vugenfirer wrote:
If it helps, mq changes the config size from 8 to 16 bytes. If the
driver was making an assumption about an 8-byte config size, that's
likely what the problem is.
That's exactly the problem.
So what do we do? It isn't nice to break existing setups.
Maybe mq can be disabled by default (together with Antony's
patch) until fixed drivers will be more widely available?
I've got a patch that does exactly like this. It's pretty ugly though
because of the relationship between virtio-pci and virtio-net. I'm
going to try to see if I can find a cleaner way to do it on Monday.
I'm also contemplating just disabling mq by default. Since a special
command line is needed to enable it anyway (on the backend), having to
specify mq=on for the device doesn't seem so bad.
It's after midnight here so didn't test yet, but wouldn't the following
achieve this in a way that is a bit cleaner?
Signed-off-by: Michael S. Tsirkin m...@redhat.com
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5699f5e..3342a76 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice
*vdev, uint32_t features)
features |= (1 VIRTIO_NET_F_MAC);
+if (n-max_queues = 1) {
+features = ~(0x1 VIRTIO_NET_F_MQ);
+}
+
This is too late because the config size has already been set.
if (!peer_has_vnet_hdr(n)) {
features = ~(0x1 VIRTIO_NET_F_CSUM);
features = ~(0x1 VIRTIO_NET_F_HOST_TSO4);
@@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev,
NICConf *conf,
int i;
n = (VirtIONet *)virtio_common_init(virtio-net, VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf-queues 1 ?
+sizeof(struct
virtio_net_config) :
+sizeof(struct
virtio_net_config_compat),
sizeof(VirtIONet));
This makes me a bit nervous. It relies on setting the config size to
one thing before we've masked a feature.
What do you think about just defaulting mq=off? The more I think about
it, the nicer of an option that is.
I'm not a huge fan of automagically changing the value of a feature bit
so I think defaulting it off is a bit more straight forward.
We can certainly revisit for 1.5+.
Regards,
Anthony Liguori
Thinking about it some more, we have the queues option so
adding an mq option as well is not useful.
Let's clear it unless # of queues is set?
What do you think about the below?
This time tested with an sq guest but not an mq guest yet.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5699f5e..b8c8439 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice
*vdev, uint32_t features)
features |= (1 VIRTIO_NET_F_MAC);
+if (n-max_queues 1) {
+features |= (0x1 VIRTIO_NET_F_MQ);
+}
+
if (!peer_has_vnet_hdr(n)) {
features = ~(0x1 VIRTIO_NET_F_CSUM);
features = ~(0x1 VIRTIO_NET_F_HOST_TSO4);
@@ -1284,7 +1288,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf
*conf,
int i;
n = (VirtIONet *)virtio_common_init(virtio-net, VIRTIO_ID_NET,
-sizeof(struct virtio_net_config),
+conf-queues 1 ?
+sizeof(struct virtio_net_config) :
+sizeof(struct
virtio_net_config_compat),
sizeof(VirtIONet));
n-vdev.get_config = virtio_net_get_config;
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1d5c721..19f63cf 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -69,6 +69,14 @@ typedef struct virtio_net_conf
/* Maximum packet size we can receive from tap device: header + 64k */
#define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 10))
+struct virtio_net_config_compat
+{
+/* The config defining mac address ($ETH_ALEN bytes) */
+uint8_t mac[ETH_ALEN];
+/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+uint16_t status;
+} QEMU_PACKED;
+
struct virtio_net_config
{
/* The config defining mac address ($ETH_ALEN bytes) */
@@ -190,6 +199,5 @@ struct virtio_net_ctrl_mq {
DEFINE_PROP_BIT(ctrl_rx, _state, _field, VIRTIO_NET_F_CTRL_RX,
true), \
DEFINE_PROP_BIT(ctrl_vlan, _state,
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On 02/02/2013 05:13 AM, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:36, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:18, Michael Tokarev wrote: Just a heads-up for now, no real diagnostics or anything like that. Current git master (a9c87c586ba9ee290792a98dc126b2861b7f8b03), when booted a windows guest, results in no virtio-net inside. Neither winXP nor Win7, neither older nor latest (22 Jan 2013) virtio-net drivers works. Windows displays a yellow exclamation mark near the virtio-net device and says it can't start the device (Code 10). Linux guests work fine, quick test anyway. Cc'ing Jason since his virtio-net changes was last. But I repeat: no diagnostics as of yet, no bisection. Bisection was easy, since win works fine right before the multiqueue virtio-net series. This is the first bad commit: Adding Vadim and Michael. If you use -M pc-1.3 or explicitly disable multiqueue, does the driver work? Neither one of these nor both makes any visible difference. Neither does -M pc-1.1 (just in case). Hrm, then it's very likely not a driver problem. Thanks. Regards, Anthony Liguori Have a look at this issue. It was caused by multiqueue patch who adds a new field to virtio_net_cfg. Not sure multiqueue is the root cause since I also find even w/o multiqueue, adding any new field to virtio_net_cfg will break windows guest. Haven't had a clue on this, will continue investigate. Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Sat, 2013-02-02 at 20:42 +0800, Jason Wang wrote: On 02/02/2013 05:13 AM, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:36, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:18, Michael Tokarev wrote: Just a heads-up for now, no real diagnostics or anything like that. Current git master (a9c87c586ba9ee290792a98dc126b2861b7f8b03), when booted a windows guest, results in no virtio-net inside. Neither winXP nor Win7, neither older nor latest (22 Jan 2013) virtio-net drivers works. Windows displays a yellow exclamation mark near the virtio-net device and says it can't start the device (Code 10). Linux guests work fine, quick test anyway. Cc'ing Jason since his virtio-net changes was last. But I repeat: no diagnostics as of yet, no bisection. Bisection was easy, since win works fine right before the multiqueue virtio-net series. This is the first bad commit: Adding Vadim and Michael. If you use -M pc-1.3 or explicitly disable multiqueue, does the driver work? Neither one of these nor both makes any visible difference. Neither does -M pc-1.1 (just in case). Hrm, then it's very likely not a driver problem. Thanks. Regards, Anthony Liguori Have a look at this issue. It was caused by multiqueue patch who adds a new field to virtio_net_cfg. Not sure multiqueue is the root cause since I also find even w/o multiqueue, adding any new field to virtio_net_cfg will break windows guest. Haven't had a clue on this, will continue investigate. cc'ing Yan, our NDIS guy. Thank you, Vadim. Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Vadim Rozenfeld vroze...@redhat.com writes: On Sat, 2013-02-02 at 20:42 +0800, Jason Wang wrote: Have a look at this issue. It was caused by multiqueue patch who adds a new field to virtio_net_cfg. Not sure multiqueue is the root cause since I also find even w/o multiqueue, adding any new field to virtio_net_cfg will break windows guest. Haven't had a clue on this, will continue investigate. cc'ing Yan, our NDIS guy. If it helps, mq changes the config size from 8 to 16 bytes. If the driver was making an assumption about an 8-byte config size, that's likely what the problem is. Regards, Anthony Liguori Thank you, Vadim. Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
02.02.2013 00:18, Michael Tokarev пишет: Just a heads-up for now, no real diagnostics or anything like that. Current git master (a9c87c586ba9ee290792a98dc126b2861b7f8b03), when booted a windows guest, results in no virtio-net inside. Neither winXP nor Win7, neither older nor latest (22 Jan 2013) virtio-net drivers works. Windows displays a yellow exclamation mark near the virtio-net device and says it can't start the device (Code 10). Linux guests work fine, quick test anyway. Cc'ing Jason since his virtio-net changes was last. But I repeat: no diagnostics as of yet, no bisection. Bisection was easy, since win works fine right before the multiqueue virtio-net series. This is the first bad commit: commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:18, Michael Tokarev пишет: Just a heads-up for now, no real diagnostics or anything like that. Current git master (a9c87c586ba9ee290792a98dc126b2861b7f8b03), when booted a windows guest, results in no virtio-net inside. Neither winXP nor Win7, neither older nor latest (22 Jan 2013) virtio-net drivers works. Windows displays a yellow exclamation mark near the virtio-net device and says it can't start the device (Code 10). Linux guests work fine, quick test anyway. Cc'ing Jason since his virtio-net changes was last. But I repeat: no diagnostics as of yet, no bisection. Bisection was easy, since win works fine right before the multiqueue virtio-net series. This is the first bad commit: Adding Vadim and Michael. If you use -M pc-1.3 or explicitly disable multiqueue, does the driver work? Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
02.02.2013 00:36, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:18, Michael Tokarev wrote: Just a heads-up for now, no real diagnostics or anything like that. Current git master (a9c87c586ba9ee290792a98dc126b2861b7f8b03), when booted a windows guest, results in no virtio-net inside. Neither winXP nor Win7, neither older nor latest (22 Jan 2013) virtio-net drivers works. Windows displays a yellow exclamation mark near the virtio-net device and says it can't start the device (Code 10). Linux guests work fine, quick test anyway. Cc'ing Jason since his virtio-net changes was last. But I repeat: no diagnostics as of yet, no bisection. Bisection was easy, since win works fine right before the multiqueue virtio-net series. This is the first bad commit: Adding Vadim and Michael. If you use -M pc-1.3 or explicitly disable multiqueue, does the driver work? Neither one of these nor both makes any visible difference. Neither does -M pc-1.1 (just in case). Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:36, Anthony Liguori wrote: Michael Tokarev m...@tls.msk.ru writes: 02.02.2013 00:18, Michael Tokarev wrote: Just a heads-up for now, no real diagnostics or anything like that. Current git master (a9c87c586ba9ee290792a98dc126b2861b7f8b03), when booted a windows guest, results in no virtio-net inside. Neither winXP nor Win7, neither older nor latest (22 Jan 2013) virtio-net drivers works. Windows displays a yellow exclamation mark near the virtio-net device and says it can't start the device (Code 10). Linux guests work fine, quick test anyway. Cc'ing Jason since his virtio-net changes was last. But I repeat: no diagnostics as of yet, no bisection. Bisection was easy, since win works fine right before the multiqueue virtio-net series. This is the first bad commit: Adding Vadim and Michael. If you use -M pc-1.3 or explicitly disable multiqueue, does the driver work? Neither one of these nor both makes any visible difference. Neither does -M pc-1.1 (just in case). Hrm, then it's very likely not a driver problem. Thanks. Regards, Anthony Liguori Regards, Anthony Liguori commit fed699f9ca6ae8a0fb62803334cf46fa64d1eb91 Author: Jason Wang jasow...@redhat.com Date: Wed Jan 30 19:12:39 2013 +0800 virtio-net: multiqueue support This patch implements both userspace and vhost support for multiple queue virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of VirtIONetQueue to VirtIONet. Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Anthony Liguori aligu...@us.ibm.com After this commit, win guest (winXP and win7) shows yellow exclamation sign and is unable to start the device with code 10. FWIW. I'm not sure it is a good idea to make a release with such a breakage, even rc0. Thanks, /mjt
