virtio-gpu: fix bug in host memory calculation.
PATCH 1/1 virtio-gpu: fix bug in host memory calculation.
The old code treats bits as bytes when calculating host memory usage. Change it to be consistent with allocation logic in pixman library.
--- hw/display/virtio-gpu.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
hw/display/virtio-gpu.c +11 -2
@@ -322,6 +322,15 @@ static pixman_format_code_t get_pixman_format(uint32_t virtio_gpu_format) } } +static uint32_t calc_image_hostmem (pixman_format_code_t pformat, +uint32_t width,uint32_t height) +{ +/* copied from pixman/pixman-bits-image.c, skip integer overflow check. */ +int bpp =PIXMAN_FORMAT_BPP (pformat); +int stride = ((width * bpp +0x1f ) >>5 ) *sizeof (uint32_t ); +return height * stride; +} +static void virtio_gpu_resource_create_2d (VirtIOGPU *g,struct virtio_gpu_ctrl_command *cmd) {
@@ -366,7 +375,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,return ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * c2d.width * c2d.height; + res->hostmem =calc_image_hostmem (pformat, c2d.width, c2d.height);if (res->hostmem + g->hostmem < g->conf.max_hostmem) { res->image = pixman_image_create_bits(pformat, c2d.width,
@@ -1087,7 +1096,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,return -EINVAL ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * res->width * res->height; + res->hostmem =calc_image_hostmem (pformat, res->width, res->height); res->addrs =g_new (uint64_t, res->iov_cnt); res->iov = g_new(struct iovec, res->iov_cnt);
Signature · 1 line
2.15.0.403.gc27cc4dac6-goog
Hi
----- Original Message -----
Quoted 3 lines
The old code treats bits as bytes when calculating host memory usage. Change it to be consistent with allocation logic in pixman library.
Good catch
Quoted 10 lines
Signed-off-by Tao Wu <[email protected]>--- hw/display/virtio-gpu.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)hw/display/virtio-gpu.c +0
-0
hw/display/virtio-gpu.c static pixman_format_code_t get_pixman_format(uint32_t · line 330
Quoted 9 lines
hw/display/virtio-gpu.c +4
-0@@ -322,6 +322,15 @@ static pixman_format_code_t get_pixman_format(uint32_t virtio_gpu_format) } } +static uint32_t calc_image_hostmem (pixman_format_code_t pformat, +uint32_t width,uint32_t height) +{ +/* copied from pixman/pixman-bits-image.c, skip integer overflow check. */
So we rely on pixman create_bits() to fail if overflow happened? perhaps it's worth a comment.
Quoted 29 lines
hw/display/virtio-gpu.c +5
-0+int bpp =PIXMAN_FORMAT_BPP (pformat); +int stride = ((width * bpp +0x1f ) >>5 ) *sizeof (uint32_t ); +return height * stride; +} + static void virtio_gpu_resource_create_2d(VirtIOGPU *g,struct virtio_gpu_ctrl_command *cmd){
hw/display/virtio-gpu.c +2
-2@@ -366,7 +375,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,return ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * c2d.width * c2d.height; + res->hostmem =calc_image_hostmem (pformat, c2d.width, c2d.height);if (res->hostmem + g->hostmem < g->conf.max_hostmem) { res->image = pixman_image_create_bits(pformat, c2d.width,@@ -1087,7 +1096,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque,size_t size, return -EINVAL; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * res->width * res->height; + res->hostmem =calc_image_hostmem (pformat, res->width, res->height);res->addrs = g_new(uint64_t, res->iov_cnt); res->iov = g_new(struct iovec, res->iov_cnt);
Quoted 3 lines
--
2.15.0.403.gc27cc4dac6-goog
looks good otherwise,
Quoted 1 line
PATCH 1/1 virtio-gpu: fix bug in host memory calculation.
The old code treats bits as bytes when calculating host memory usage. Change it to be consistent with allocation logic in pixman library.
--- hw/display/virtio-gpu.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
hw/display/virtio-gpu.c +14 -2
@@ -322,6 +322,18 @@ static pixman_format_code_t get_pixman_format(uint32_t virtio_gpu_format) } } +static uint32_t calc_image_hostmem (pixman_format_code_t pformat, +uint32_t width,uint32_t height) +{ +/* Copied from pixman/pixman-bits-image.c, skip integer overflow check. +* pixman_image_create_bits will fail in case it overflow. +*/ + +int bpp =PIXMAN_FORMAT_BPP (pformat); +int stride = ((width * bpp +0x1f ) >>5 ) *sizeof (uint32_t ); +return height * stride; +} +static void virtio_gpu_resource_create_2d (VirtIOGPU *g,struct virtio_gpu_ctrl_command *cmd) {
@@ -366,7 +378,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,return ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * c2d.width * c2d.height; + res->hostmem =calc_image_hostmem (pformat, c2d.width, c2d.height);if (res->hostmem + g->hostmem < g->conf.max_hostmem) { res->image = pixman_image_create_bits(pformat, c2d.width,
@@ -1087,7 +1099,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,return -EINVAL ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * res->width * res->height; + res->hostmem =calc_image_hostmem (pformat, res->width, res->height); res->addrs =g_new (uint64_t, res->iov_cnt); res->iov = g_new(struct iovec, res->iov_cnt);
Signature · 1 line
2.15.0.448.gf294e3d99a-goog
Quoted 4 lines · Tao Wu via Qemu-devel
The old code treats bits as bytes when calculating host memory usage. Change it to be consistent with allocation logic in pixman library.
Signed-off-by Tao Wu <[email protected]>
Quoted 49 lines
--- hw/display/virtio-gpu.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)hw/display/virtio-gpu.c +14
-3@@ -322,6 +322,18 @@ static pixman_format_code_t get_pixman_format(uint32_t virtio_gpu_format) } } +static uint32_t calc_image_hostmem (pixman_format_code_t pformat, +uint32_t width,uint32_t height) +{ +/* Copied from pixman/pixman-bits-image.c, skip integer overflow check. +* pixman_image_create_bits will fail in case it overflow. +*/ + +int bpp =PIXMAN_FORMAT_BPP (pformat); +int stride = ((width * bpp +0x1f ) >>5 ) *sizeof (uint32_t ); +return height * stride; +} +static void virtio_gpu_resource_create_2d (VirtIOGPU *g,struct virtio_gpu_ctrl_command *cmd) {@@ -366,7 +378,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,return ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * c2d.width * c2d.height; + res->hostmem =calc_image_hostmem (pformat, c2d.width, c2d.height);if (res->hostmem + g->hostmem < g->conf.max_hostmem) { res->image = pixman_image_create_bits(pformat, c2d.width,@@ -1087,7 +1099,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,return -EINVAL ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * res->width * res->height; + res->hostmem =calc_image_hostmem (pformat, res->width, res->height); res->addrs =g_new (uint64_t, res->iov_cnt); res->iov = g_new(struct iovec, res->iov_cnt); --2.15.0.448.gf294e3d99a-goog
Signature · 1 line
Marc-André Lureau
Thanks. Sent out a new version add comments to say that we rely on pixman create_bits to fail.
Quoted 69 lines · Marc-André Lureau
Hi
----- Original Message -----
Quoted 3 lines
The old code treats bits as bytes when calculating host memory usage. Change it to be consistent with allocation logic in pixman library.
Good catch
Quoted 10 lines
Signed-off-by Tao Wu <[email protected]>--- hw/display/virtio-gpu.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)hw/display/virtio-gpu.c +0
-0hw/display/virtio-gpu.c static pixman_format_code_t get_pixman_format(uint32_t · line 330
Quoted 9 lines
hw/display/virtio-gpu.c +4
-0@@ -322,6 +322,15 @@ static pixman_format_code_t get_pixman_format(uint32_t virtio_gpu_format) } } +static uint32_t calc_image_hostmem (pixman_format_code_t pformat, +uint32_t width,uint32_t height) +{ +/* copied from pixman/pixman-bits-image.c, skip integer overflow check. */ So we rely on pixman
create_bits()to fail if overflow happened? perhaps it's worth a comment.Quoted 29 lines
hw/display/virtio-gpu.c +5
-0+int bpp =PIXMAN_FORMAT_BPP (pformat); +int stride = ((width * bpp +0x1f ) >>5 ) *sizeof (uint32_t ); +return height * stride; +} + static void virtio_gpu_resource_create_2d(VirtIOGPU *g,struct virtio_gpu_ctrl_command *cmd){
hw/display/virtio-gpu.c +2
-2@@ -366,7 +375,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,return ; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * c2d.width * c2d.height; + res->hostmem =calc_image_hostmem (pformat, c2d.width, c2d.height);if (res->hostmem + g->hostmem < g->conf.max_hostmem) { res->image = pixman_image_create_bits(pformat, c2d.width,@@ -1087,7 +1096,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque,size_t size, return -EINVAL; } - res->hostmem =PIXMAN_FORMAT_BPP (pformat) * res->width * res->height; + res->hostmem =calc_image_hostmem (pformat, res->width, res->height);res->addrs = g_new(uint64_t, res->iov_cnt); res->iov = g_new(struct iovec, res->iov_cnt);Quoted 3 lines
--
2.15.0.403.gc27cc4dac6-googlooks good otherwise,
Reviewed-by Marc-André Lureau <[email protected]>Quoted 1 line