| Message ID | 52FD3593.3040109@samsung.com |
|---|---|
| State | New |
| Headers | show |
CCing the virtio maintainers. thanks -- PMM On 13 February 2014 21:13, Mario Smarduch <m.smarduch@samsung.com> wrote: > virtio: set virtio-net/virtio-mmio host features > > Patch sets 'virtio-net/virtio-mmio' host features to enable network > features based on peer capabilities. Currently host features turn > of all features by default. > > Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> > --- > hw/virtio/virtio-mmio.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 8829eb0..1d940b7 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -23,6 +23,7 @@ > #include "hw/virtio/virtio.h" > #include "qemu/host-utils.h" > #include "hw/virtio/virtio-bus.h" > +#include "hw/virtio/virtio-net.h" > > /* #define DEBUG_VIRTIO_MMIO */ > > @@ -92,6 +93,12 @@ typedef struct { > static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size, > VirtIOMMIOProxy *dev); > > +/* all possible virtio-net features supported */ > +static Property virtio_mmio_net_properties[] = { > + DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) > { > VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; > @@ -347,11 +354,33 @@ static void virtio_mmio_reset(DeviceState *d) > > /* virtio-mmio device */ > > +/* Walk virtio-net possible supported features and set host_features, this > + * should be done earlier when the object is instantiated but at that point > + * you don't know what type of device will be plugged in. > + */ > +static void virtio_mmio_set_net_features(Property *prop, uint32_t *features) > +{ > + for (; prop && prop->name; prop++) { > + if (prop->defval == true) { > + *features |= (1 << prop->bitnr); > + } else { > + *features &= ~(1 << prop->bitnr); > + } > + } > +} > + > /* This is called by virtio-bus just after the device is plugged. */ > static void virtio_mmio_device_plugged(DeviceState *opaque) > { > VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); > + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); > + Object *obj = OBJECT(vdev); > > + /* set host features only for virtio-net */ > + if (object_dynamic_cast(obj, TYPE_VIRTIO_NET)) { > + virtio_mmio_set_net_features(virtio_mmio_net_properties, > + &proxy->host_features); > + } > proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); > proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, > proxy->host_features); > -- > 1.7.9.5 >
Hello, any feedback on this patch, after a brief email exchange Anthony deferred to Peter. Lack of improper host features handling lowers 1g & 10g performance substantially on arm-kvm compared to xeon. We would like to have this fixed so we don't have to patch every new release of qemu, especially virtio stuff. - Mario On 02/14/2014 03:49 PM, Peter Maydell wrote: > CCing the virtio maintainers. > > thanks > -- PMM > > On 13 February 2014 21:13, Mario Smarduch <m.smarduch@samsung.com> wrote: >> virtio: set virtio-net/virtio-mmio host features >> >> Patch sets 'virtio-net/virtio-mmio' host features to enable network >> features based on peer capabilities. Currently host features turn >> of all features by default. >> >> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> >> --- >> hw/virtio/virtio-mmio.c | 29 +++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >> index 8829eb0..1d940b7 100644 >> --- a/hw/virtio/virtio-mmio.c >> +++ b/hw/virtio/virtio-mmio.c >> @@ -23,6 +23,7 @@ >> #include "hw/virtio/virtio.h" >> #include "qemu/host-utils.h" >> #include "hw/virtio/virtio-bus.h" >> +#include "hw/virtio/virtio-net.h" >> >> /* #define DEBUG_VIRTIO_MMIO */ >> >> @@ -92,6 +93,12 @@ typedef struct { >> static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size, >> VirtIOMMIOProxy *dev); >> >> +/* all possible virtio-net features supported */ >> +static Property virtio_mmio_net_properties[] = { >> + DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) >> { >> VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; >> @@ -347,11 +354,33 @@ static void virtio_mmio_reset(DeviceState *d) >> >> /* virtio-mmio device */ >> >> +/* Walk virtio-net possible supported features and set host_features, this >> + * should be done earlier when the object is instantiated but at that point >> + * you don't know what type of device will be plugged in. >> + */ >> +static void virtio_mmio_set_net_features(Property *prop, uint32_t *features) >> +{ >> + for (; prop && prop->name; prop++) { >> + if (prop->defval == true) { >> + *features |= (1 << prop->bitnr); >> + } else { >> + *features &= ~(1 << prop->bitnr); >> + } >> + } >> +} >> + >> /* This is called by virtio-bus just after the device is plugged. */ >> static void virtio_mmio_device_plugged(DeviceState *opaque) >> { >> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); >> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); >> + Object *obj = OBJECT(vdev); >> >> + /* set host features only for virtio-net */ >> + if (object_dynamic_cast(obj, TYPE_VIRTIO_NET)) { >> + virtio_mmio_set_net_features(virtio_mmio_net_properties, >> + &proxy->host_features); >> + } >> proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); >> proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, >> proxy->host_features); >> -- >> 1.7.9.5 >>
On 20 February 2014 18:12, Mario Smarduch <m.smarduch@samsung.com> wrote: > > Hello, > > any feedback on this patch, after a brief email exchange Anthony deferred to > Peter. > > Lack of improper host features handling lowers 1g & 10g performance > substantially on arm-kvm compared to xeon. > > We would like to have this fixed so we don't have to patch every new release > of qemu, especially virtio stuff. I don't know enough about virtio to review, really, but I'll have a go: >> On 13 February 2014 21:13, Mario Smarduch <m.smarduch@samsung.com> wrote: >>> virtio: set virtio-net/virtio-mmio host features >>> >>> Patch sets 'virtio-net/virtio-mmio' host features to enable network >>> features based on peer capabilities. Currently host features turn >>> of all features by default. >>> >>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> >>> --- >>> hw/virtio/virtio-mmio.c | 29 +++++++++++++++++++++++++++++ >>> 1 file changed, 29 insertions(+) >>> >>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >>> index 8829eb0..1d940b7 100644 >>> --- a/hw/virtio/virtio-mmio.c >>> +++ b/hw/virtio/virtio-mmio.c >>> @@ -23,6 +23,7 @@ >>> #include "hw/virtio/virtio.h" >>> #include "qemu/host-utils.h" >>> #include "hw/virtio/virtio-bus.h" >>> +#include "hw/virtio/virtio-net.h" >>> >>> /* #define DEBUG_VIRTIO_MMIO */ >>> >>> @@ -92,6 +93,12 @@ typedef struct { >>> static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size, >>> VirtIOMMIOProxy *dev); >>> >>> +/* all possible virtio-net features supported */ >>> +static Property virtio_mmio_net_properties[] = { >>> + DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) >>> { >>> VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; >>> @@ -347,11 +354,33 @@ static void virtio_mmio_reset(DeviceState *d) >>> >>> /* virtio-mmio device */ >>> >>> +/* Walk virtio-net possible supported features and set host_features, this >>> + * should be done earlier when the object is instantiated but at that point >>> + * you don't know what type of device will be plugged in. >>> + */ >>> +static void virtio_mmio_set_net_features(Property *prop, uint32_t *features) >>> +{ >>> + for (; prop && prop->name; prop++) { >>> + if (prop->defval == true) { >>> + *features |= (1 << prop->bitnr); >>> + } else { >>> + *features &= ~(1 << prop->bitnr); >>> + } >>> + } >>> +} >>> + >>> /* This is called by virtio-bus just after the device is plugged. */ >>> static void virtio_mmio_device_plugged(DeviceState *opaque) >>> { >>> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); >>> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); >>> + Object *obj = OBJECT(vdev); >>> >>> + /* set host features only for virtio-net */ >>> + if (object_dynamic_cast(obj, TYPE_VIRTIO_NET)) { >>> + virtio_mmio_set_net_features(virtio_mmio_net_properties, >>> + &proxy->host_features); >>> + } This looks weird. We already have a mechanism for saying "hey the thing we just plugged in gets to tell us about feature bits": >>> proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); >>> proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, >>> proxy->host_features); ...this is making an indirect call to the backend device's get_features method, which for virtio-net is virtio_net_get_features(). Why should the transport need special case code for virtio-net backends? thanks -- PMM
Peter thanks. Questionable in this patch - is cutting through several layers to set proxy properties. They should be set from "device" instance init before it's realized. The problem though is that unlike PCI that sets proxy and virtio-net properties via its virtio_net_properites[], the virtio-mmio version instantiates the proxy in the machine model, so it doesn't appear to be good place to set virtio-net host features since you don't know what virtio device will be plugged in later. It's virtio_net_properties[] can only set virtio-net properites when it's instantiated. I think the proper way would be to refactor virtio-mmio to similar structure PCI version uses then one virtio_net_properties[] can be used selecting PCI or virtio-mmio at compile time. But right now it's unclear to me how pci and virtio-mmio versions intend to co-exist. I'm fairly new to qemu community. - Mario On 02/20/2014 10:30 AM, Peter Maydell wrote: > On 20 February 2014 18:12, Mario Smarduch <m.smarduch@samsung.com> wrote: >> >> Hello, >> >> any feedback on this patch, after a brief email exchange Anthony deferred to >> Peter. >> >> Lack of improper host features handling lowers 1g & 10g performance >> substantially on arm-kvm compared to xeon. >> >> We would like to have this fixed so we don't have to patch every new release >> of qemu, especially virtio stuff. > > I don't know enough about virtio to review, really, but > I'll have a go: > >>> On 13 February 2014 21:13, Mario Smarduch <m.smarduch@samsung.com> wrote: >>>> virtio: set virtio-net/virtio-mmio host features >>>> >>>> Patch sets 'virtio-net/virtio-mmio' host features to enable network >>>> features based on peer capabilities. Currently host features turn >>>> of all features by default. >>>> >>>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> >>>> --- >>>> hw/virtio/virtio-mmio.c | 29 +++++++++++++++++++++++++++++ >>>> 1 file changed, 29 insertions(+) >>>> >>>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c >>>> index 8829eb0..1d940b7 100644 >>>> --- a/hw/virtio/virtio-mmio.c >>>> +++ b/hw/virtio/virtio-mmio.c >>>> @@ -23,6 +23,7 @@ >>>> #include "hw/virtio/virtio.h" >>>> #include "qemu/host-utils.h" >>>> #include "hw/virtio/virtio-bus.h" >>>> +#include "hw/virtio/virtio-net.h" >>>> >>>> /* #define DEBUG_VIRTIO_MMIO */ >>>> >>>> @@ -92,6 +93,12 @@ typedef struct { >>>> static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size, >>>> VirtIOMMIOProxy *dev); >>>> >>>> +/* all possible virtio-net features supported */ >>>> +static Property virtio_mmio_net_properties[] = { >>>> + DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features), >>>> + DEFINE_PROP_END_OF_LIST(), >>>> +}; >>>> + >>>> static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) >>>> { >>>> VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; >>>> @@ -347,11 +354,33 @@ static void virtio_mmio_reset(DeviceState *d) >>>> >>>> /* virtio-mmio device */ >>>> >>>> +/* Walk virtio-net possible supported features and set host_features, this >>>> + * should be done earlier when the object is instantiated but at that point >>>> + * you don't know what type of device will be plugged in. >>>> + */ >>>> +static void virtio_mmio_set_net_features(Property *prop, uint32_t *features) >>>> +{ >>>> + for (; prop && prop->name; prop++) { >>>> + if (prop->defval == true) { >>>> + *features |= (1 << prop->bitnr); >>>> + } else { >>>> + *features &= ~(1 << prop->bitnr); >>>> + } >>>> + } >>>> +} >>>> + >>>> /* This is called by virtio-bus just after the device is plugged. */ >>>> static void virtio_mmio_device_plugged(DeviceState *opaque) >>>> { >>>> VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); >>>> + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); >>>> + Object *obj = OBJECT(vdev); >>>> >>>> + /* set host features only for virtio-net */ >>>> + if (object_dynamic_cast(obj, TYPE_VIRTIO_NET)) { >>>> + virtio_mmio_set_net_features(virtio_mmio_net_properties, >>>> + &proxy->host_features); >>>> + } > > This looks weird. We already have a mechanism for saying > "hey the thing we just plugged in gets to tell us about > feature bits": > >>>> proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); >>>> proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, >>>> proxy->host_features); > > ...this is making an indirect call to the backend device's > get_features method, which for virtio-net is > virtio_net_get_features(). Why should the transport > need special case code for virtio-net backends? > > thanks > -- PMM >
On 20 February 2014 19:09, Mario Smarduch <m.smarduch@samsung.com> wrote: > Questionable in this patch - is cutting through several layers to set > proxy properties. They should be set from "device" instance init > before it's realized. The problem though is that unlike PCI > that sets proxy and virtio-net properties via its virtio_net_properites[], > the virtio-mmio version instantiates the proxy in the machine model, > so it doesn't appear to be good place to set virtio-net > host features since you don't know what virtio device will be plugged > in later. I think this function is the right place to set these properties, yes. What I'm saying is that I don't see why you're doing it this way rather than using the existing per-backend hook. Maybe there's a reason not to use that hook, but you don't say. > It's virtio_net_properties[] can only set virtio-net > properites when it's instantiated. I think the proper way would > be to refactor virtio-mmio to similar structure PCI version uses then > one virtio_net_properties[] can be used selecting PCI or virtio-mmio at > compile time. But right now it's unclear to me how pci and virtio-mmio > versions intend to co-exist. This is the result of a refactoring last year to allow virtio-mmio. Basically the underlying structure now is: [some virtio transport device] <- virtio bus -> [virtio backend] where the transport could be mmio, PCI, CCW, etc, and the backend is net, blk, balloon, etc. We also provide devices which are "virtio PCI transport plus a specific backend" for (a) user convenience and (b) backwards compatibility, since the pre-refactoring structure put the transport and backend all in one lump. The all-in-one-lump arrangement is legacy: we shouldn't break it, but it's not the model for virtio-mmio to follow either. The transport shouldn't know anything specific about the backend it's plugged into (or vice-versa), but it's fine for there to be hook functions so the transport and backend can call each other at the appropriate times. thanks -- PMM
On 02/20/2014 11:35 AM, Peter Maydell wrote: > On 20 February 2014 19:09, Mario Smarduch <m.smarduch@samsung.com> wrote: >> host features since you don't know what virtio device will be plugged >> in later. > > I think this function is the right place to set these properties, > yes. What I'm saying is that I don't see why you're doing it > this way rather than using the existing per-backend hook. > Maybe there's a reason not to use that hook, but you don't say. > Appears virtio-net beckend hooks are common to several transports, and would require virtio-mmio exception to set the host_features. If I'm missing something please recommend. >> It's virtio_net_properties[] can only set virtio-net >> properites when it's instantiated. I think the proper way would >> be to refactor virtio-mmio to similar structure PCI version uses then >> one virtio_net_properties[] can be used selecting PCI or virtio-mmio at >> compile time. But right now it's unclear to me how pci and virtio-mmio >> versions intend to co-exist. > > This is the result of a refactoring last year to allow virtio-mmio. > Basically the underlying structure now is: > > [some virtio transport device] <- virtio bus -> [virtio backend] > > where the transport could be mmio, PCI, CCW, etc, and the > backend is net, blk, balloon, etc. We also provide devices > which are "virtio PCI transport plus a specific backend" > for (a) user convenience and (b) backwards compatibility, since > the pre-refactoring structure put the transport and backend > all in one lump. The all-in-one-lump arrangement is legacy: > we shouldn't break it, but it's not the model for virtio-mmio > to follow either. > > The transport shouldn't know anything specific about the > backend it's plugged into (or vice-versa), but it's fine > for there to be hook functions so the transport and backend > can call each other at the appropriate times. > > thanks > -- PMM >
On 20 February 2014 21:59, Mario Smarduch <m.smarduch@samsung.com> wrote: > On 02/20/2014 11:35 AM, Peter Maydell wrote: >> On 20 February 2014 19:09, Mario Smarduch <m.smarduch@samsung.com> wrote: >>> host features since you don't know what virtio device will be plugged >>> in later. >> >> I think this function is the right place to set these properties, >> yes. What I'm saying is that I don't see why you're doing it >> this way rather than using the existing per-backend hook. >> Maybe there's a reason not to use that hook, but you don't say. >> > > Appears virtio-net beckend hooks are common to several transports, > and would require virtio-mmio exception to set the host_features. > If I'm missing something please recommend. Yes, they're supposed to be common across transports, because the backend isn't supposed to care about which transport in particular. If there's a condition where the backend needs to do something which only happens for one transport, maybe we need a new hook. thanks -- PMM
On 02/20/2014 02:10 PM, Peter Maydell wrote: > On 20 February 2014 21:59, Mario Smarduch <m.smarduch@samsung.com> wrote: >> On 02/20/2014 11:35 AM, Peter Maydell wrote: >>> On 20 February 2014 19:09, Mario Smarduch <m.smarduch@samsung.com> wrote: >>>> host features since you don't know what virtio device will be plugged >>>> in later. >>> >>> I think this function is the right place to set these properties, >>> yes. What I'm saying is that I don't see why you're doing it >>> this way rather than using the existing per-backend hook. >>> Maybe there's a reason not to use that hook, but you don't say. >>> >> >> Appears virtio-net beckend hooks are common to several transports, >> and would require virtio-mmio exception to set the host_features. >> If I'm missing something please recommend. > > Yes, they're supposed to be common across transports, because > the backend isn't supposed to care about which transport in > particular. If there's a condition where the backend needs to > do something which only happens for one transport, maybe we > need a new hook. So something like set_transport_features(...) in VirtiIODeviceClass, and call it from the realize hook where you can access the virtio-mmio transport class instance. > > thanks > -- PMM >
On 20 February 2014 22:36, Mario Smarduch <m.smarduch@samsung.com> wrote: > On 02/20/2014 02:10 PM, Peter Maydell wrote: >> On 20 February 2014 21:59, Mario Smarduch <m.smarduch@samsung.com> wrote: >>> On 02/20/2014 11:35 AM, Peter Maydell wrote: >>>> On 20 February 2014 19:09, Mario Smarduch <m.smarduch@samsung.com> wrote: >>>>> host features since you don't know what virtio device will be plugged >>>>> in later. >>>> >>>> I think this function is the right place to set these properties, >>>> yes. What I'm saying is that I don't see why you're doing it >>>> this way rather than using the existing per-backend hook. >>>> Maybe there's a reason not to use that hook, but you don't say. >>>> >>> >>> Appears virtio-net beckend hooks are common to several transports, >>> and would require virtio-mmio exception to set the host_features. >>> If I'm missing something please recommend. >> >> Yes, they're supposed to be common across transports, because >> the backend isn't supposed to care about which transport in >> particular. If there's a condition where the backend needs to >> do something which only happens for one transport, maybe we >> need a new hook. > > So something like set_transport_features(...) in VirtiIODeviceClass, > and call it from the realize hook where you can access > the virtio-mmio transport class instance. Not sure. You should probably also look at whether connecting a virtio-pci transport to a virtio-net backend gets these feature bits wrong (that's different from instantiating a single virtio-net-pci device). I suspect they both get this wrong and this isn't particularly virtio-mmio specific. thanks -- PMM
On 02/20/2014 02:47 PM, Peter Maydell wrote: > On 20 February 2014 22:36, Mario Smarduch <m.smarduch@samsung.com> wrote: >> On 02/20/2014 02:10 PM, Peter Maydell wrote: >>> On 20 February 2014 21:59, Mario Smarduch <m.smarduch@samsung.com> wrote: >>>> On 02/20/2014 11:35 AM, Peter Maydell wrote: >>>>> On 20 February 2014 19:09, Mario Smarduch <m.smarduch@samsung.com> wrote: >>>>>> host features since you don't know what virtio device will be plugged >>>>>> in later. >>>>> >>>>> I think this function is the right place to set these properties, >>>>> yes. What I'm saying is that I don't see why you're doing it >>>>> this way rather than using the existing per-backend hook. >>>>> Maybe there's a reason not to use that hook, but you don't say. >>>>> >>>> >>>> Appears virtio-net beckend hooks are common to several transports, >>>> and would require virtio-mmio exception to set the host_features. >>>> If I'm missing something please recommend. >>> >>> Yes, they're supposed to be common across transports, because >>> the backend isn't supposed to care about which transport in >>> particular. If there's a condition where the backend needs to >>> do something which only happens for one transport, maybe we >>> need a new hook. >> >> So something like set_transport_features(...) in VirtiIODeviceClass, >> and call it from the realize hook where you can access >> the virtio-mmio transport class instance. > > Not sure. You should probably also look at whether connecting > a virtio-pci transport to a virtio-net backend gets these feature > bits wrong (that's different from instantiating a single virtio-net-pci > device). I suspect they both get this wrong and this isn't particularly > virtio-mmio specific. > > thanks > -- PMM > It appears separate pci transport/virtio-net backend have the same issue. I'll look at this closer for a more generic solution. Thanks, Mario
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 8829eb0..1d940b7 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -23,6 +23,7 @@ #include "hw/virtio/virtio.h" #include "qemu/host-utils.h" #include "hw/virtio/virtio-bus.h" +#include "hw/virtio/virtio-net.h" /* #define DEBUG_VIRTIO_MMIO */ @@ -92,6 +93,12 @@ typedef struct { static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size, VirtIOMMIOProxy *dev); +/* all possible virtio-net features supported */ +static Property virtio_mmio_net_properties[] = { + DEFINE_VIRTIO_NET_FEATURES(VirtIOMMIOProxy, host_features), + DEFINE_PROP_END_OF_LIST(), +}; + static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) { VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; @@ -347,11 +354,33 @@ static void virtio_mmio_reset(DeviceState *d) /* virtio-mmio device */ +/* Walk virtio-net possible supported features and set host_features, this + * should be done earlier when the object is instantiated but at that point + * you don't know what type of device will be plugged in. + */ +static void virtio_mmio_set_net_features(Property *prop, uint32_t *features) +{ + for (; prop && prop->name; prop++) { + if (prop->defval == true) { + *features |= (1 << prop->bitnr); + } else { + *features &= ~(1 << prop->bitnr); + } + } +} + /* This is called by virtio-bus just after the device is plugged. */ static void virtio_mmio_device_plugged(DeviceState *opaque) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + Object *obj = OBJECT(vdev); + /* set host features only for virtio-net */ + if (object_dynamic_cast(obj, TYPE_VIRTIO_NET)) { + virtio_mmio_set_net_features(virtio_mmio_net_properties, + &proxy->host_features); + } proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, proxy->host_features);
virtio: set virtio-net/virtio-mmio host features Patch sets 'virtio-net/virtio-mmio' host features to enable network features based on peer capabilities. Currently host features turn of all features by default. Signed-off-by: Mario Smarduch <m.smarduch@samsung.com> --- hw/virtio/virtio-mmio.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)