| Message ID | 20201027135547.374946-26-philmd@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block/nvme: Fix Aarch64 host | expand |
On Tue, Oct 27, 2020 at 02:55:47PM +0100, 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> > --- > block/nvme.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
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); >
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);
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> --- block/nvme.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)