[PATCH] Update virtio to latest ABI

[PATCH] Update virtio to latest ABI

[Anthony Liguori]

This patch updates KVM's virtio implementation to the latest virtio ABI.  This
includes a change in the network header and support for reset.

With this patch, the block and network driver from Rusty's latest queue are
functioning.  Module unload and reload work and I no longer see an error when
repeatedly bringing a network interface up and down.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 296edf6..86f9e5a 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -23,11 +23,8 @@
 
 /* The feature bitmap for virtio net */
 #define VIRTIO_NET_F_NO_CSUM	0
-#define VIRTIO_NET_F_TSO4	1
-#define VIRTIO_NET_F_UFO	2
-#define VIRTIO_NET_F_TSO4_ECN	3
-#define VIRTIO_NET_F_TSO6	4
 #define VIRTIO_NET_F_MAC	5
+#define VIRTIO_NET_F_GS0	6
 
 #define TX_TIMER_INTERVAL (1000 / 500)
 
@@ -49,7 +46,9 @@ struct virtio_net_hdr
 #define VIRTIO_NET_HDR_GSO_TCPV4_ECN	2	// GSO frame, IPv4 TCP w/ ECN
 #define VIRTIO_NET_HDR_GSO_UDP		3	// GSO frame, IPv4 UDP (UFO)
 #define VIRTIO_NET_HDR_GSO_TCPV6	4	// GSO frame, IPv6 TCP
+#define VIRTIO_NET_HDR_GSO_ECN		0x80	// TCP has ECN set
     uint8_t gso_type;
+    uint16_t hdr_len;
     uint16_t gso_size;
     uint16_t csum_start;
     uint16_t csum_offset;
diff --git a/qemu/hw/virtio.c b/qemu/hw/virtio.c
index b78c2c5..bbcb44c 100644
--- a/qemu/hw/virtio.c
+++ b/qemu/hw/virtio.c
@@ -210,6 +210,8 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 	break;
     case VIRTIO_PCI_STATUS:
 	vdev->status = val & 0xFF;
+	if (vdev->status == 0)
+	    virtio_reset(vdev);
 	break;
     }
 }

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Update virtio to latest ABI

[Avi Kivity]

Anthony Liguori wrote:
> This patch updates KVM's virtio implementation to the latest virtio ABI.  This
> includes a change in the network header and support for reset.
>
> With this patch, the block and network driver from Rusty's latest queue are
> functioning.  Module unload and reload work and I no longer see an error when
> repeatedly bringing a network interface up and down.
>
>   

Applied, thanks.

This ABI breakage is worrying.  While it is expected to take some time 
for the ABI to congeal, we need some way to prevent mismatched guests 
and hosts from running.  Perhaps something like the kvm abi version, 
storedin the pci revision field?  Keep incrementing it until we are 
satisfied, then use feature bits to add functionality.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Update virtio to latest ABI

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> This patch updates KVM's virtio implementation to the latest virtio 
>> ABI.  This
>> includes a change in the network header and support for reset.
>>
>> With this patch, the block and network driver from Rusty's latest 
>> queue are
>> functioning.  Module unload and reload work and I no longer see an 
>> error when
>> repeatedly bringing a network interface up and down.
>>
>>   
>
> Applied, thanks.
>
> This ABI breakage is worrying.  While it is expected to take some time 
> for the ABI to congeal, we need some way to prevent mismatched guests 
> and hosts from running.  Perhaps something like the kvm abi version, 
> storedin the pci revision field?  Keep incrementing it until we are 
> satisfied, then use feature bits to add functionality.

This breakage wasn't actually necessary.  We could have used a feature 
bit to indicate the additional net config field.  In the very least, the 
additional net config field could have been added to the end of the 
config space so that old guests kept working that didn't use GSO.

I don't think we need another mechanism to avoid breakages in the 
future.  I think we have to declare that whatever ships in 2.6.25 is the 
stable ABI and make sure not to break it again in the future.

Regards,

Anthony Liguori



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Update virtio to latest ABI

[Avi Kivity]

Anthony Liguori wrote:
>>
>> This ABI breakage is worrying.  While it is expected to take some 
>> time for the ABI to congeal, we need some way to prevent mismatched 
>> guests and hosts from running.  Perhaps something like the kvm abi 
>> version, storedin the pci revision field?  Keep incrementing it until 
>> we are satisfied, then use feature bits to add functionality.
>
> This breakage wasn't actually necessary.  We could have used a feature 
> bit to indicate the additional net config field.  In the very least, 
> the additional net config field could have been added to the end of 
> the config space so that old guests kept working that didn't use GSO.
>
> I don't think we need another mechanism to avoid breakages in the 
> future.  I think we have to declare that whatever ships in 2.6.25 is 
> the stable ABI and make sure not to break it again in the future.

I'm worried about the ramp up to 2.6.25 causing confusion among users as 
before that things will break left and right, if we don't provide a 
tighter check.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Update virtio to latest ABI

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>>
>>> This ABI breakage is worrying.  While it is expected to take some 
>>> time for the ABI to congeal, we need some way to prevent mismatched 
>>> guests and hosts from running.  Perhaps something like the kvm abi 
>>> version, storedin the pci revision field?  Keep incrementing it 
>>> until we are satisfied, then use feature bits to add functionality.
>>
>> This breakage wasn't actually necessary.  We could have used a 
>> feature bit to indicate the additional net config field.  In the very 
>> least, the additional net config field could have been added to the 
>> end of the config space so that old guests kept working that didn't 
>> use GSO.
>>
>> I don't think we need another mechanism to avoid breakages in the 
>> future.  I think we have to declare that whatever ships in 2.6.25 is 
>> the stable ABI and make sure not to break it again in the future.
>
> I'm worried about the ramp up to 2.6.25 causing confusion among users 
> as before that things will break left and right, if we don't provide a 
> tighter check.
>

Well, using the PCI revision ID isn't a bad idea.  It wouldn't have 
helped this last problem because that was an ABI break in the network 
driver and we really can't indicate a ABI break in the PCI driver for 
any possibly virtio device.  However, if we change the PCI ABI, we can 
be a little more friendly about it.

Just sent out patches.

Regards,

Anthony Liguori

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Update virtio to latest ABI

[Avi Kivity]

Anthony Liguori wrote:
>>
>> I'm worried about the ramp up to 2.6.25 causing confusion among users 
>> as before that things will break left and right, if we don't provide 
>> a tighter check.
>>
>
> Well, using the PCI revision ID isn't a bad idea.  It wouldn't have 
> helped this last problem because that was an ABI break in the network 
> driver and we really can't indicate a ABI break in the PCI driver for 
> any possibly virtio device.


Why not?  we have one pci device per virtio device, and in the same way 
the pci id is device-specific, the revision id can be device specific as 
well.  We just need a virtio revision field, and a pci-virtio binding 
for that field.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] Update virtio to latest ABI

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>>
>>> I'm worried about the ramp up to 2.6.25 causing confusion among 
>>> users as before that things will break left and right, if we don't 
>>> provide a tighter check.
>>>
>>
>> Well, using the PCI revision ID isn't a bad idea.  It wouldn't have 
>> helped this last problem because that was an ABI break in the network 
>> driver and we really can't indicate a ABI break in the PCI driver for 
>> any possibly virtio device.
>
>
> Why not?  we have one pci device per virtio device, and in the same 
> way the pci id is device-specific, the revision id can be device 
> specific as well.  We just need a virtio revision field, and a 
> pci-virtio binding for that field.

I hadn't thought of that, but that's clever :-)  If we need to bump it, 
we can add a field to virtio_init_pci() to indicate ABI version.  I 
don't think it's necessary to add that until we need to use it though.

Regards,

Anthony Liguori


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/