* [Qemu-devel] [PATCH 1/2] virtio-input: use fixed device config space size
@ 2015-06-15 12:53 Gerd Hoffmann
2015-06-16 6:59 ` Michael S. Tsirkin
0 siblings, 1 reply; 2+ messages in thread
From: Gerd Hoffmann @ 2015-06-15 12:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin
virtio-input doesn't support legacy, so config spice doesn't live in IO
address space (on virtio-pci) and size is not something we have to worry
about. There is a full page (in mmio bar) available anyway.
Don't try to size it dynamically but simply use the full struct size
(136 bytes) unconditionally.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/input/virtio-input.c | 14 +++++++-------
include/hw/virtio/virtio-input.h | 1 -
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index c4f4b3c..c5c39fa 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -149,9 +149,9 @@ static void virtio_input_get_config(VirtIODevice *vdev, uint8_t *config_data)
config = virtio_input_find_config(vinput, vinput->cfg_select,
vinput->cfg_subsel);
if (config) {
- memcpy(config_data, config, vinput->cfg_size);
+ memcpy(config_data, config, sizeof(virtio_input_config));
} else {
- memset(config_data, 0, vinput->cfg_size);
+ memset(config_data, 0, sizeof(virtio_input_config));
}
}
@@ -206,6 +206,7 @@ static void virtio_input_device_realize(DeviceState *dev, Error **errp)
VirtIOInput *vinput = VIRTIO_INPUT(dev);
VirtIOInputConfig *cfg;
Error *local_err = NULL;
+ uint32_t cfg_size = 0;
if (vic->realize) {
vic->realize(dev, &local_err);
@@ -219,15 +220,14 @@ static void virtio_input_device_realize(DeviceState *dev, Error **errp)
vinput->input.serial);
QTAILQ_FOREACH(cfg, &vinput->cfg_list, node) {
- if (vinput->cfg_size < cfg->config.size) {
- vinput->cfg_size = cfg->config.size;
+ if (cfg_size < cfg->config.size) {
+ cfg_size = cfg->config.size;
}
}
- vinput->cfg_size += 8;
- assert(vinput->cfg_size <= sizeof(virtio_input_config));
+ assert(cfg_size + 8 <= sizeof(virtio_input_config));
virtio_init(vdev, "virtio-input", VIRTIO_ID_INPUT,
- vinput->cfg_size);
+ sizeof(virtio_input_config));
vinput->evt = virtio_add_queue(vdev, 64, virtio_input_handle_evt);
vinput->sts = virtio_add_queue(vdev, 64, virtio_input_handle_sts);
}
diff --git a/include/hw/virtio/virtio-input.h b/include/hw/virtio/virtio-input.h
index 8134178..7b46e62 100644
--- a/include/hw/virtio/virtio-input.h
+++ b/include/hw/virtio/virtio-input.h
@@ -71,7 +71,6 @@ struct VirtIOInput {
VirtIODevice parent_obj;
uint8_t cfg_select;
uint8_t cfg_subsel;
- uint32_t cfg_size;
QTAILQ_HEAD(, VirtIOInputConfig) cfg_list;
VirtQueue *evt, *sts;
virtio_input_conf input;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] virtio-input: use fixed device config space size
2015-06-15 12:53 [Qemu-devel] [PATCH 1/2] virtio-input: use fixed device config space size Gerd Hoffmann
@ 2015-06-16 6:59 ` Michael S. Tsirkin
0 siblings, 0 replies; 2+ messages in thread
From: Michael S. Tsirkin @ 2015-06-16 6:59 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Mon, Jun 15, 2015 at 02:53:08PM +0200, Gerd Hoffmann wrote:
> virtio-input doesn't support legacy, so config spice doesn't live in IO
> address space (on virtio-pci) and size is not something we have to worry
> about. There is a full page (in mmio bar) available anyway.
>
> Don't try to size it dynamically but simply use the full struct size
> (136 bytes) unconditionally.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
I'm not sure this is a good idea. Struct will grow with time as we add
features, and this will break compatibility. Size must depend on
host feature bits.
We really need to move the logic into core, too.
> ---
> hw/input/virtio-input.c | 14 +++++++-------
> include/hw/virtio/virtio-input.h | 1 -
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
> index c4f4b3c..c5c39fa 100644
> --- a/hw/input/virtio-input.c
> +++ b/hw/input/virtio-input.c
> @@ -149,9 +149,9 @@ static void virtio_input_get_config(VirtIODevice *vdev, uint8_t *config_data)
> config = virtio_input_find_config(vinput, vinput->cfg_select,
> vinput->cfg_subsel);
> if (config) {
> - memcpy(config_data, config, vinput->cfg_size);
> + memcpy(config_data, config, sizeof(virtio_input_config));
> } else {
> - memset(config_data, 0, vinput->cfg_size);
> + memset(config_data, 0, sizeof(virtio_input_config));
> }
> }
>
> @@ -206,6 +206,7 @@ static void virtio_input_device_realize(DeviceState *dev, Error **errp)
> VirtIOInput *vinput = VIRTIO_INPUT(dev);
> VirtIOInputConfig *cfg;
> Error *local_err = NULL;
> + uint32_t cfg_size = 0;
>
> if (vic->realize) {
> vic->realize(dev, &local_err);
> @@ -219,15 +220,14 @@ static void virtio_input_device_realize(DeviceState *dev, Error **errp)
> vinput->input.serial);
>
> QTAILQ_FOREACH(cfg, &vinput->cfg_list, node) {
> - if (vinput->cfg_size < cfg->config.size) {
> - vinput->cfg_size = cfg->config.size;
> + if (cfg_size < cfg->config.size) {
> + cfg_size = cfg->config.size;
> }
> }
> - vinput->cfg_size += 8;
> - assert(vinput->cfg_size <= sizeof(virtio_input_config));
> + assert(cfg_size + 8 <= sizeof(virtio_input_config));
>
> virtio_init(vdev, "virtio-input", VIRTIO_ID_INPUT,
> - vinput->cfg_size);
> + sizeof(virtio_input_config));
> vinput->evt = virtio_add_queue(vdev, 64, virtio_input_handle_evt);
> vinput->sts = virtio_add_queue(vdev, 64, virtio_input_handle_sts);
> }
> diff --git a/include/hw/virtio/virtio-input.h b/include/hw/virtio/virtio-input.h
> index 8134178..7b46e62 100644
> --- a/include/hw/virtio/virtio-input.h
> +++ b/include/hw/virtio/virtio-input.h
> @@ -71,7 +71,6 @@ struct VirtIOInput {
> VirtIODevice parent_obj;
> uint8_t cfg_select;
> uint8_t cfg_subsel;
> - uint32_t cfg_size;
> QTAILQ_HEAD(, VirtIOInputConfig) cfg_list;
> VirtQueue *evt, *sts;
> virtio_input_conf input;
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-06-16 6:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-15 12:53 [Qemu-devel] [PATCH 1/2] virtio-input: use fixed device config space size Gerd Hoffmann
2015-06-16 6:59 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).