On 2015/4/20 19:32, Cornelia Huck wrote:
> On Mon, 20 Apr 2015 16:20:00 +0800
> shannon.z...@linaro.org wrote:
>
>> From: Shannon Zhao <shannon.z...@linaro.org>
>>
>> Move DEFINE_VIRTIO_NET_FEATURES to the backend virtio-net.
>> The transports just sync the host features from backend.
>>
>> Signed-off-by: Shannon Zhao <zhaoshengl...@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.z...@linaro.org>
>> ---
>> hw/net/virtio-net.c | 4 ++++
>> hw/s390x/s390-virtio-bus.c | 1 -
>> hw/s390x/virtio-ccw.c | 1 -
>> hw/virtio/virtio-pci.c | 1 -
>> include/hw/virtio/virtio-net.h | 1 +
>> 5 files changed, 5 insertions(+), 3 deletions(-)
>
> I need the following change to make this work for virtio-ccw:
>
Maybe we can use following patch. This moves virtio_net_set_config_size to
virtio_net_device_realize function. As the features are moved to virtio-net,
so we should set the config_size in virtio-net too. And this can be useful to
virtio-mmio which now doesn't call virtio_net_set_config_size in
virtio-mmio's realize function.
Cornelia, could you check if this works on s390? Thanks.
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 27ec5b1..36ba027 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1588,6 +1588,7 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
NetClientState *nc;
int i;
+ virtio_net_set_config_size(n, n->host_features);
virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
n->max_queues = MAX(n->nic_conf.peers.queues, 1);
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 1987873..b893e02 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -145,7 +145,6 @@ static void s390_virtio_net_realize(VirtIOS390Device
*s390_dev, Error **errp)
DeviceState *vdev = DEVICE(&dev->vdev);
Error *err = NULL;
- virtio_net_set_config_size(&dev->vdev, s390_dev->host_features);
virtio_net_set_netclient_name(&dev->vdev, qdev->id,
object_get_typename(OBJECT(qdev)));
qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 803526a..1252162 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -779,7 +779,6 @@ static void virtio_ccw_net_realize(VirtioCcwDevice
*ccw_dev, Error **errp)
DeviceState *vdev = DEVICE(&dev->vdev);
Error *err = NULL;
- virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features[0]);
virtio_net_set_netclient_name(&dev->vdev, qdev->id,
object_get_typename(OBJECT(qdev)));
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 772244e..c6b99f9 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1369,7 +1369,6 @@ static void virtio_net_pci_realize(VirtIOPCIProxy
*vpci_dev, Error **errp)
VirtIONetPCI *dev = VIRTIO_NET_PCI(vpci_dev);
DeviceState *vdev = DEVICE(&dev->vdev);
- virtio_net_set_config_size(&dev->vdev, vpci_dev->host_features);
virtio_net_set_netclient_name(&dev->vdev, qdev->id,
object_get_typename(OBJECT(qdev)));
qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 2252789..7a2bdff 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -779,7 +779,6 @@ static void virtio_ccw_net_realize(VirtioCcwDevice
> *ccw_dev, Error **errp)
> DeviceState *vdev = DEVICE(&dev->vdev);
> Error *err = NULL;
>
> - virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features[0]);
> virtio_net_set_netclient_name(&dev->vdev, qdev->id,
> object_get_typename(OBJECT(qdev)));
> qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
> @@ -790,6 +789,7 @@ static void virtio_ccw_net_realize(VirtioCcwDevice
> *ccw_dev, Error **errp)
> }
>
> virtio_ccw_device_realize(ccw_dev, VIRTIO_DEVICE(vdev), errp);
> + virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features[0]);
> }
>
> static void virtio_ccw_net_instance_init(Object *obj)
>
> host_features used to be statically populated, so
> virtio_net_set_config_size() was able to use the various feature bits
> for its decisions.
>
> It does not seem quite right, however, since the set of feature bits
> had not been through virtio-net's ->get_features() routine (or the
> feature bit manipulations in virtio-ccw's realize() routine) - it was
> just good enough.
>
> Maybe the right place for calling set_config_size() would be in a
> virtio-net specific ->plugged() callback?
>
> I'm not sure why virtio-pci works, but they have a different topology
> with pci device and virtio-pci device separate, so it might work out
> there.
>
