| Message ID | 1366965244-20542-3-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 26.04.2013, at 10:34, Jason Wang wrote: > virtio-rng-s390 has zero config length, so no need to sync its config otherwise > qemu will crash since vdev->config is NULL. Why is it NULL? Alex > > Cc: Alexander Graf <agraf@suse.de> > Cc: Richard Henderson <rth@twiddle.net> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/s390x/s390-virtio-bus.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index dabbc2e..0f83516 100644 > --- a/hw/s390x/s390-virtio-bus.c > +++ b/hw/s390x/s390-virtio-bus.c > @@ -350,6 +350,10 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) > dev->feat_offs = cur_offs + dev->feat_len; > cur_offs += dev->feat_len * 2; > > + if (!dev->vdev->config_len) { > + return; > + } > + > /* Sync config space */ > if (dev->vdev->get_config) { > dev->vdev->get_config(dev->vdev, dev->vdev->config); > -- > 1.7.1 > >
On 04/27/2013 01:07 AM, Alexander Graf wrote: > On 26.04.2013, at 10:34, Jason Wang wrote: > >> virtio-rng-s390 has zero config length, so no need to sync its config otherwise >> qemu will crash since vdev->config is NULL. > Why is it NULL? As far as I see, virtio-rng's config_len is zero, so in its vdev->config were set to NULL in virtio_init(). > > > Alex > >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Richard Henderson <rth@twiddle.net> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/s390x/s390-virtio-bus.c | 4 ++++ >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c >> index dabbc2e..0f83516 100644 >> --- a/hw/s390x/s390-virtio-bus.c >> +++ b/hw/s390x/s390-virtio-bus.c >> @@ -350,6 +350,10 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) >> dev->feat_offs = cur_offs + dev->feat_len; >> cur_offs += dev->feat_len * 2; >> >> + if (!dev->vdev->config_len) { >> + return; >> + } >> + >> /* Sync config space */ >> if (dev->vdev->get_config) { >> dev->vdev->get_config(dev->vdev, dev->vdev->config); >> -- >> 1.7.1 >> >>
On Fri, Apr 26, 2013 at 04:34:04PM +0800, Jason Wang wrote: > virtio-rng-s390 has zero config length, so no need to sync its config otherwise > qemu will crash since vdev->config is NULL. > > Cc: Alexander Graf <agraf@suse.de> > Cc: Richard Henderson <rth@twiddle.net> > Signed-off-by: Jason Wang <jasowang@redhat.com> Actully, it validates get_config so what's the problem here? > --- > hw/s390x/s390-virtio-bus.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index dabbc2e..0f83516 100644 > --- a/hw/s390x/s390-virtio-bus.c > +++ b/hw/s390x/s390-virtio-bus.c > @@ -350,6 +350,10 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) > dev->feat_offs = cur_offs + dev->feat_len; > cur_offs += dev->feat_len * 2; > > + if (!dev->vdev->config_len) { > + return; > + } > + > /* Sync config space */ > if (dev->vdev->get_config) { > dev->vdev->get_config(dev->vdev, dev->vdev->config); > -- > 1.7.1
On 04/28/2013 04:31 PM, Michael S. Tsirkin wrote: > On Fri, Apr 26, 2013 at 04:34:04PM +0800, Jason Wang wrote: >> virtio-rng-s390 has zero config length, so no need to sync its config otherwise >> qemu will crash since vdev->config is NULL. >> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Richard Henderson <rth@twiddle.net> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > Actully, it validates get_config so what's the problem here? Yes, but the it will also pass vdev->config(NULL) to cpu_physical_memory_write(), but since the length is zero, we manage to survive here. I will drop this patch then. >> --- >> hw/s390x/s390-virtio-bus.c | 4 ++++ >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c >> index dabbc2e..0f83516 100644 >> --- a/hw/s390x/s390-virtio-bus.c >> +++ b/hw/s390x/s390-virtio-bus.c >> @@ -350,6 +350,10 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) >> dev->feat_offs = cur_offs + dev->feat_len; >> cur_offs += dev->feat_len * 2; >> >> + if (!dev->vdev->config_len) { >> + return; >> + } >> + >> /* Sync config space */ >> if (dev->vdev->get_config) { >> dev->vdev->get_config(dev->vdev, dev->vdev->config); >> -- >> 1.7.1
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index dabbc2e..0f83516 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -350,6 +350,10 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) dev->feat_offs = cur_offs + dev->feat_len; cur_offs += dev->feat_len * 2; + if (!dev->vdev->config_len) { + return; + } + /* Sync config space */ if (dev->vdev->get_config) { dev->vdev->get_config(dev->vdev, dev->vdev->config);
virtio-rng-s390 has zero config length, so no need to sync its config otherwise qemu will crash since vdev->config is NULL. Cc: Alexander Graf <agraf@suse.de> Cc: Richard Henderson <rth@twiddle.net> Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/s390x/s390-virtio-bus.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)