[v1] virtio-gpu: fix unmap the already mapped items

[PATCH] virtio-gpu: fix unmap the already mapped items

Posted by Li Zhijian 3 years, 8 months ago

we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider
to unmap the 'i'th item as well when the 'i'th item is not nil

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 hw/display/virtio-gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f0dd7c150..1f777e43ff 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -656,7 +656,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
             qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
                           " resource %d element %d\n",
                           __func__, ab->resource_id, i);
-            virtio_gpu_cleanup_mapping_iov(g, *iov, i);
+            virtio_gpu_cleanup_mapping_iov(g, *iov, i + !!(*iov)[i].iov_base);
             g_free(ents);
             *iov = NULL;
             if (addr) {
-- 
2.17.1

Re: [PATCH] virtio-gpu: fix unmap the already mapped items

Posted by Gerd Hoffmann 3 years, 8 months ago

On Fri, Aug 21, 2020 at 04:49:45PM +0800, Li Zhijian wrote:
> we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider
> to unmap the 'i'th item as well when the 'i'th item is not nil
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  hw/display/virtio-gpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 5f0dd7c150..1f777e43ff 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -656,7 +656,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>              qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
>                            " resource %d element %d\n",
>                            __func__, ab->resource_id, i);
> -            virtio_gpu_cleanup_mapping_iov(g, *iov, i);
> +            virtio_gpu_cleanup_mapping_iov(g, *iov, i + !!(*iov)[i].iov_base);

Cute trick, but the code should be readable without having to dig out
the commit message which explains it.  Can we have something simpler
along the lines of "if (iov_base) { i++; /* cleanup partial map */ }"
please?

thanks,
  Gerd

Re: [PATCH] virtio-gpu: fix unmap the already mapped items

Posted by Li Zhijian 3 years, 8 months ago


On 8/21/20 6:07 PM, Gerd Hoffmann wrote:
> On Fri, Aug 21, 2020 at 04:49:45PM +0800, Li Zhijian wrote:
>> we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider
>> to unmap the 'i'th item as well when the 'i'th item is not nil
>>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> ---
>>   hw/display/virtio-gpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index 5f0dd7c150..1f777e43ff 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -656,7 +656,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
>>               qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
>>                             " resource %d element %d\n",
>>                             __func__, ab->resource_id, i);
>> -            virtio_gpu_cleanup_mapping_iov(g, *iov, i);
>> +            virtio_gpu_cleanup_mapping_iov(g, *iov, i + !!(*iov)[i].iov_base);
> Cute trick, but the code should be readable without having to dig out
> the commit message which explains it.  Can we have something simpler
> along the lines of "if (iov_base) { i++; /* cleanup partial map */ }"
> please?
make sense !

Thanks
Zhijian

>
> thanks,
>    Gerd
>
>
>