Re: [RFC PATCH 25/25] block/nvme: Fix use of write-only doorbells page on Aarch64 arch

[Auger Eric]

Hi,

On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote:
> qemu_vfio_pci_map_bar() calls mmap(), and mmap(2) states:
> 
>   'offset' must be a multiple of the page size as returned
>    by sysconf(_SC_PAGE_SIZE).
> 
> In commit f68453237b9 we started to use an offset of 4K which
> broke this contract on Aarch64 arch.
> 
> Fix by mapping at offset 0, and and accessing doorbells at offset=4K.
> 
> Fixes: f68453237b9 ("block/nvme: Map doorbells pages write-only")
> Reported-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> ---
>  block/nvme.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index c1c52bae44f..ff645eefe6a 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -94,6 +94,7 @@ typedef struct {
>  struct BDRVNVMeState {
>      AioContext *aio_context;
>      QEMUVFIOState *vfio;
> +    void *bar0_wo_map;
>      /* Memory mapped registers */
>      volatile struct {
>          uint32_t sq_tail;
> @@ -778,8 +779,10 @@ static int nvme_init(BlockDriverState *bs, const char 
> *device, int namespace,
>          }
>      }
>  
> -    s->doorbells = qemu_vfio_pci_map_bar(s->vfio, 0, sizeof(NvmeBar),
> -                                         NVME_DOORBELL_SIZE, PROT_WRITE, 
> errp);
> +    s->bar0_wo_map = qemu_vfio_pci_map_bar(s->vfio, 0, 0,
> +                                           sizeof(NvmeBar) + 
> NVME_DOORBELL_SIZE,
> +                                           PROT_WRITE, errp);
> +    s->doorbells = (void *)((uintptr_t)s->bar0_wo_map + sizeof(NvmeBar));
>      if (!s->doorbells) {
>          ret = -EINVAL;
>          goto out;
> @@ -913,8 +916,8 @@ static void nvme_close(BlockDriverState *bs)
>                             &s->irq_notifier[MSIX_SHARED_IRQ_IDX],
>                             false, NULL, NULL);
>      event_notifier_cleanup(&s->irq_notifier[MSIX_SHARED_IRQ_IDX]);
> -    qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->doorbells,
> -                            sizeof(NvmeBar), NVME_DOORBELL_SIZE);
> +    qemu_vfio_pci_unmap_bar(s->vfio, 0, s->bar0_wo_map,
> +                            0, sizeof(NvmeBar) + NVME_DOORBELL_SIZE);
>      qemu_vfio_close(s->vfio);
>  
>      g_free(s->device);
>