[PATCH v1 2/5] virtio-mem: Check that "memaddr" is multiples of the block size

David Hildenbrand posted 5 patches 5 years, 1 month ago
Maintainers: Igor Mammedov <imammedo@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
[PATCH v1 2/5] virtio-mem: Check that "memaddr" is multiples of the block size
Posted by David Hildenbrand 5 years, 1 month ago
The spec requires us to set the "addr" in guest physical address space to
multiples of the block size. In some cases, this is not the case right
now: For example, when starting a VM with 4 GiB boot memory and a
virtio-mem device with a block size of 2 GiB, "memaddr" will be
auto-assigned to 0x140000000 / 5 GiB.

We'll try to improve auto-assignment for memory devices next, to avoid
bailing out in case memory device code selects a bad address.

Note: The Linux driver doesn't support such big block sizes yet.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Wei Yang <richardw.yang@linux.intel.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/virtio/virtio-mem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 58098686ee..716eddd792 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -515,6 +515,11 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)
                    ")", VIRTIO_MEM_REQUESTED_SIZE_PROP,
                    VIRTIO_MEM_BLOCK_SIZE_PROP, vmem->block_size);
         return;
+    } else if (!QEMU_IS_ALIGNED(vmem->addr, vmem->block_size)) {
+        error_setg(errp, "'%s' property has to be multiples of '%s' (0x%" PRIx64
+                   ")", VIRTIO_MEM_ADDR_PROP, VIRTIO_MEM_BLOCK_SIZE_PROP,
+                   vmem->block_size);
+        return;
     } else if (!QEMU_IS_ALIGNED(memory_region_size(&vmem->memdev->mr),
                                 vmem->block_size)) {
         error_setg(errp, "'%s' property memdev size has to be multiples of"
-- 
2.26.2


Re: [PATCH v1 2/5] virtio-mem: Check that "memaddr" is multiples of the block size
Posted by Pankaj Gupta 5 years, 1 month ago
> The spec requires us to set the "addr" in guest physical address space to
> multiples of the block size. In some cases, this is not the case right
> now: For example, when starting a VM with 4 GiB boot memory and a
> virtio-mem device with a block size of 2 GiB, "memaddr" will be
> auto-assigned to 0x140000000 / 5 GiB.
>
> We'll try to improve auto-assignment for memory devices next, to avoid
> bailing out in case memory device code selects a bad address.
>
> Note: The Linux driver doesn't support such big block sizes yet.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Wei Yang <richardw.yang@linux.intel.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/virtio/virtio-mem.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index 58098686ee..716eddd792 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -515,6 +515,11 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)
>                     ")", VIRTIO_MEM_REQUESTED_SIZE_PROP,
>                     VIRTIO_MEM_BLOCK_SIZE_PROP, vmem->block_size);
>          return;
> +    } else if (!QEMU_IS_ALIGNED(vmem->addr, vmem->block_size)) {
> +        error_setg(errp, "'%s' property has to be multiples of '%s' (0x%" PRIx64
> +                   ")", VIRTIO_MEM_ADDR_PROP, VIRTIO_MEM_BLOCK_SIZE_PROP,
> +                   vmem->block_size);
> +        return;
>      } else if (!QEMU_IS_ALIGNED(memory_region_size(&vmem->memdev->mr),
>                                  vmem->block_size)) {
>          error_setg(errp, "'%s' property memdev size has to be multiples of"
> --
> 2.26.2

Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>