From: Roman Kagan <rvkagan@yandex-team.ru> To: qemu-devel@nongnu.org Cc: "Fam Zheng" <fam@euphon.net>, "Kevin Wolf" <kwolf@redhat.com>, "Stefano Stabellini" <sstabellini@kernel.org>, "Daniel P. Berrangé" <berrange@redhat.com>, "Eduardo Habkost" <ehabkost@redhat.com>, qemu-block@nongnu.org, "Paul Durrant" <paul@xen.org>, "John Snow" <jsnow@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Laurent Vivier" <laurent@vivier.eu>, "Max Reitz" <mreitz@redhat.com>, "Keith Busch" <kbusch@kernel.org>, "Gerd Hoffmann" <kraxel@redhat.com>, "Stefan Hajnoczi" <stefanha@redhat.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Anthony Perard" <anthony.perard@citrix.com>, xen-devel@lists.xenproject.org, "Philippe Mathieu-Daudé" <philmd@redhat.com> Subject: [PATCH v8 6/8] block: make BlockConf size props 32bit and accept size suffixes Date: Fri, 29 May 2020 01:55:14 +0300 [thread overview] Message-ID: <20200528225516.1676602-7-rvkagan@yandex-team.ru> (raw) In-Reply-To: <20200528225516.1676602-1-rvkagan@yandex-team.ru> Convert all size-related properties in BlockConf to 32bit. This will accommodate bigger block sizes (in a followup patch). This also allows to make them all accept size suffixes, either via DEFINE_PROP_BLOCKSIZE or via DEFINE_PROP_SIZE32. Also, since min_io_size is exposed to the guest by scsi and virtio-blk devices as an uint16_t in units of logical blocks, introduce an additional check in blkconf_blocksizes to prevent its silent truncation. Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru> --- v7 -> v8: - replace stringify with %u in the error message [Eric] - fix wording in the log [Eric] include/hw/block/block.h | 12 ++++++------ include/hw/qdev-properties.h | 2 +- hw/block/block.c | 10 ++++++++++ hw/core/qdev-properties.c | 4 ++-- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/include/hw/block/block.h b/include/hw/block/block.h index 784953a237..1e8b6253dd 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -18,9 +18,9 @@ typedef struct BlockConf { BlockBackend *blk; - uint16_t physical_block_size; - uint16_t logical_block_size; - uint16_t min_io_size; + uint32_t physical_block_size; + uint32_t logical_block_size; + uint32_t min_io_size; uint32_t opt_io_size; int32_t bootindex; uint32_t discard_granularity; @@ -51,9 +51,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) _conf.logical_block_size), \ DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ _conf.physical_block_size), \ - DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ - DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ - DEFINE_PROP_UINT32("discard_granularity", _state, \ + DEFINE_PROP_SIZE32("min_io_size", _state, _conf.min_io_size, 0), \ + DEFINE_PROP_SIZE32("opt_io_size", _state, _conf.opt_io_size, 0), \ + DEFINE_PROP_SIZE32("discard_granularity", _state, \ _conf.discard_granularity, -1), \ DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \ ON_OFF_AUTO_AUTO), \ diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index c03eadfad6..5252bb6b1a 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -200,7 +200,7 @@ extern const PropertyInfo qdev_prop_pcie_link_width; #define DEFINE_PROP_SIZE32(_n, _s, _f, _d) \ DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size32, uint32_t) #define DEFINE_PROP_BLOCKSIZE(_n, _s, _f) \ - DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t) + DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint32_t) #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress) #define DEFINE_PROP_OFF_AUTO_PCIBAR(_n, _s, _f, _d) \ diff --git a/hw/block/block.c b/hw/block/block.c index b22207c921..1e34573da7 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -96,6 +96,16 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp) return false; } + /* + * all devices which support min_io_size (scsi and virtio-blk) expose it to + * the guest as a uint16_t in units of logical blocks + */ + if (conf->min_io_size / conf->logical_block_size > UINT16_MAX) { + error_setg(errp, "min_io_size must not exceed %u logical blocks", + UINT16_MAX); + return false; + } + if (!QEMU_IS_ALIGNED(conf->opt_io_size, conf->logical_block_size)) { error_setg(errp, "opt_io_size must be a multiple of logical_block_size"); diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index c9af6a1341..bd4abdc1d1 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -782,7 +782,7 @@ static void set_blocksize(Object *obj, Visitor *v, const char *name, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint16_t *ptr = qdev_get_prop_ptr(dev, prop); + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); uint64_t value; Error *local_err = NULL; @@ -821,7 +821,7 @@ const PropertyInfo qdev_prop_blocksize = { .name = "size", .description = "A power of two between " MIN_BLOCK_SIZE_STR " and " MAX_BLOCK_SIZE_STR, - .get = get_uint16, + .get = get_uint32, .set = set_blocksize, .set_default_value = set_default_value_uint, }; -- 2.26.2
WARNING: multiple messages have this Message-ID (diff)
From: Roman Kagan <rvkagan@yandex-team.ru> To: qemu-devel@nongnu.org Cc: "Fam Zheng" <fam@euphon.net>, "Kevin Wolf" <kwolf@redhat.com>, "Stefano Stabellini" <sstabellini@kernel.org>, "Daniel P. Berrangé" <berrange@redhat.com>, "Eduardo Habkost" <ehabkost@redhat.com>, qemu-block@nongnu.org, "Paul Durrant" <paul@xen.org>, "John Snow" <jsnow@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Laurent Vivier" <laurent@vivier.eu>, "Eric Blake" <eblake@redhat.com>, "Max Reitz" <mreitz@redhat.com>, "Keith Busch" <kbusch@kernel.org>, "Gerd Hoffmann" <kraxel@redhat.com>, "Stefan Hajnoczi" <stefanha@redhat.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Anthony Perard" <anthony.perard@citrix.com>, xen-devel@lists.xenproject.org, "Philippe Mathieu-Daudé" <philmd@redhat.com> Subject: [PATCH v8 6/8] block: make BlockConf size props 32bit and accept size suffixes Date: Fri, 29 May 2020 01:55:14 +0300 [thread overview] Message-ID: <20200528225516.1676602-7-rvkagan@yandex-team.ru> (raw) In-Reply-To: <20200528225516.1676602-1-rvkagan@yandex-team.ru> Convert all size-related properties in BlockConf to 32bit. This will accommodate bigger block sizes (in a followup patch). This also allows to make them all accept size suffixes, either via DEFINE_PROP_BLOCKSIZE or via DEFINE_PROP_SIZE32. Also, since min_io_size is exposed to the guest by scsi and virtio-blk devices as an uint16_t in units of logical blocks, introduce an additional check in blkconf_blocksizes to prevent its silent truncation. Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru> --- v7 -> v8: - replace stringify with %u in the error message [Eric] - fix wording in the log [Eric] include/hw/block/block.h | 12 ++++++------ include/hw/qdev-properties.h | 2 +- hw/block/block.c | 10 ++++++++++ hw/core/qdev-properties.c | 4 ++-- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/include/hw/block/block.h b/include/hw/block/block.h index 784953a237..1e8b6253dd 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -18,9 +18,9 @@ typedef struct BlockConf { BlockBackend *blk; - uint16_t physical_block_size; - uint16_t logical_block_size; - uint16_t min_io_size; + uint32_t physical_block_size; + uint32_t logical_block_size; + uint32_t min_io_size; uint32_t opt_io_size; int32_t bootindex; uint32_t discard_granularity; @@ -51,9 +51,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) _conf.logical_block_size), \ DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ _conf.physical_block_size), \ - DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ - DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ - DEFINE_PROP_UINT32("discard_granularity", _state, \ + DEFINE_PROP_SIZE32("min_io_size", _state, _conf.min_io_size, 0), \ + DEFINE_PROP_SIZE32("opt_io_size", _state, _conf.opt_io_size, 0), \ + DEFINE_PROP_SIZE32("discard_granularity", _state, \ _conf.discard_granularity, -1), \ DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \ ON_OFF_AUTO_AUTO), \ diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index c03eadfad6..5252bb6b1a 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -200,7 +200,7 @@ extern const PropertyInfo qdev_prop_pcie_link_width; #define DEFINE_PROP_SIZE32(_n, _s, _f, _d) \ DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size32, uint32_t) #define DEFINE_PROP_BLOCKSIZE(_n, _s, _f) \ - DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t) + DEFINE_PROP_UNSIGNED(_n, _s, _f, 0, qdev_prop_blocksize, uint32_t) #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress) #define DEFINE_PROP_OFF_AUTO_PCIBAR(_n, _s, _f, _d) \ diff --git a/hw/block/block.c b/hw/block/block.c index b22207c921..1e34573da7 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -96,6 +96,16 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp) return false; } + /* + * all devices which support min_io_size (scsi and virtio-blk) expose it to + * the guest as a uint16_t in units of logical blocks + */ + if (conf->min_io_size / conf->logical_block_size > UINT16_MAX) { + error_setg(errp, "min_io_size must not exceed %u logical blocks", + UINT16_MAX); + return false; + } + if (!QEMU_IS_ALIGNED(conf->opt_io_size, conf->logical_block_size)) { error_setg(errp, "opt_io_size must be a multiple of logical_block_size"); diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index c9af6a1341..bd4abdc1d1 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -782,7 +782,7 @@ static void set_blocksize(Object *obj, Visitor *v, const char *name, { DeviceState *dev = DEVICE(obj); Property *prop = opaque; - uint16_t *ptr = qdev_get_prop_ptr(dev, prop); + uint32_t *ptr = qdev_get_prop_ptr(dev, prop); uint64_t value; Error *local_err = NULL; @@ -821,7 +821,7 @@ const PropertyInfo qdev_prop_blocksize = { .name = "size", .description = "A power of two between " MIN_BLOCK_SIZE_STR " and " MAX_BLOCK_SIZE_STR, - .get = get_uint16, + .get = get_uint32, .set = set_blocksize, .set_default_value = set_default_value_uint, }; -- 2.26.2
next prev parent reply other threads:[~2020-05-28 23:01 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-28 22:55 [PATCH v8 0/8] block: enhance handling of size-related BlockConf properties Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-05-28 22:55 ` [PATCH v8 1/8] virtio-blk: store opt_io_size with correct size Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-05-28 22:55 ` [PATCH v8 2/8] block: consolidate blocksize properties consistency checks Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-05-29 9:53 ` Markus Armbruster 2020-05-29 10:56 ` Roman Kagan 2020-06-02 11:42 ` Kevin Wolf 2020-05-28 22:55 ` [PATCH v8 3/8] qdev-properties: blocksize: use same limits in code and description Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-05-28 22:55 ` [PATCH v8 4/8] qdev-properties: add size32 property type Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-05-28 22:55 ` [PATCH v8 5/8] qdev-properties: make blocksize accept size suffixes Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-06-02 12:06 ` Philippe Mathieu-Daudé 2020-06-02 12:06 ` Philippe Mathieu-Daudé 2020-05-28 22:55 ` Roman Kagan [this message] 2020-05-28 22:55 ` [PATCH v8 6/8] block: make BlockConf size props 32bit and " Roman Kagan 2020-05-28 22:55 ` [PATCH v8 7/8] qdev-properties: add getter for size32 and blocksize Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-05-28 22:55 ` [PATCH v8 8/8] block: lift blocksize property limit to 2 MiB Roman Kagan 2020-05-28 22:55 ` Roman Kagan 2020-06-15 18:05 ` [PATCH v8 0/8] block: enhance handling of size-related BlockConf properties Roman Kagan 2020-06-16 16:09 ` Kevin Wolf 2020-06-16 16:09 ` Kevin Wolf
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200528225516.1676602-7-rvkagan@yandex-team.ru \ --to=rvkagan@yandex-team.ru \ --cc=anthony.perard@citrix.com \ --cc=berrange@redhat.com \ --cc=ehabkost@redhat.com \ --cc=fam@euphon.net \ --cc=jsnow@redhat.com \ --cc=kbusch@kernel.org \ --cc=kraxel@redhat.com \ --cc=kwolf@redhat.com \ --cc=laurent@vivier.eu \ --cc=mreitz@redhat.com \ --cc=mst@redhat.com \ --cc=paul@xen.org \ --cc=pbonzini@redhat.com \ --cc=philmd@redhat.com \ --cc=qemu-block@nongnu.org \ --cc=qemu-devel@nongnu.org \ --cc=sstabellini@kernel.org \ --cc=stefanha@redhat.com \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.