| Message ID | 20201026105504.4023620-4-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | util/vfio-helpers: Allow using multiple MSIX IRQs | expand |
Hi Philippe, On 10/26/20 11:54 AM, Philippe Mathieu-Daudé wrote: > Introduce device/iommu 'page_size_min' variables to make > the code clearer. I am unclear how much the device and the iommu page size must equal. For instance, in [RFC 0/5] NVMe passthrough: Support 64kB page host, I have a 64kB host page and an MPS set to 4kB. Thanks Eric > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/nvme.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/block/nvme.c b/block/nvme.c > index aa290996679..5abd7257cac 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -690,6 +690,8 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > uint64_t deadline, now; > Error *local_err = NULL; > volatile NvmeBar *regs = NULL; > + size_t device_page_size_min; > + size_t iommu_page_size_min = 4096; > > qemu_co_mutex_init(&s->dma_map_lock); > qemu_co_queue_init(&s->dma_flush_queue); > @@ -724,7 +726,8 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > goto out; > } > > - s->page_size = MAX(4096, 1u << (12 + NVME_CAP_MPSMIN(cap))); > + device_page_size_min = 1u << (12 + NVME_CAP_MPSMIN(cap)); > + s->page_size = MAX(iommu_page_size_min, device_page_size_min); > s->doorbell_scale = (4 << NVME_CAP_DSTRD(cap)) / sizeof(uint32_t); > bs->bl.opt_mem_alignment = s->page_size; > bs->bl.request_alignment = s->page_size; >
On Mon, Oct 26, 2020 at 11:54:48AM +0100, Philippe Mathieu-Daudé wrote: > Introduce device/iommu 'page_size_min' variables to make > the code clearer. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/nvme.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/block/nvme.c b/block/nvme.c > index aa290996679..5abd7257cac 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -690,6 +690,8 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > uint64_t deadline, now; > Error *local_err = NULL; > volatile NvmeBar *regs = NULL; > + size_t device_page_size_min; > + size_t iommu_page_size_min = 4096; > > qemu_co_mutex_init(&s->dma_map_lock); > qemu_co_queue_init(&s->dma_flush_queue); > @@ -724,7 +726,8 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > goto out; > } > > - s->page_size = MAX(4096, 1u << (12 + NVME_CAP_MPSMIN(cap))); > + device_page_size_min = 1u << (12 + NVME_CAP_MPSMIN(cap)); > + s->page_size = MAX(iommu_page_size_min, device_page_size_min); It's not clear to me that the 4096 value is related to the IOMMU page size here. The MAX(4096) expression seems like a sanity-check to me. An MPS value of 0 is a 4KB page size, so it's never possible to express a smaller page size. I guess MAX() was used in case a device incorrectly reports MPSMIN. I think introducing the concept of IOMMU page size is premature and maybe not the intention of the existing code, but the concept will be needed soon, so this patch is okay: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/block/nvme.c b/block/nvme.c index aa290996679..5abd7257cac 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -690,6 +690,8 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, uint64_t deadline, now; Error *local_err = NULL; volatile NvmeBar *regs = NULL; + size_t device_page_size_min; + size_t iommu_page_size_min = 4096; qemu_co_mutex_init(&s->dma_map_lock); qemu_co_queue_init(&s->dma_flush_queue); @@ -724,7 +726,8 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, goto out; } - s->page_size = MAX(4096, 1u << (12 + NVME_CAP_MPSMIN(cap))); + device_page_size_min = 1u << (12 + NVME_CAP_MPSMIN(cap)); + s->page_size = MAX(iommu_page_size_min, device_page_size_min); s->doorbell_scale = (4 << NVME_CAP_DSTRD(cap)) / sizeof(uint32_t); bs->bl.opt_mem_alignment = s->page_size; bs->bl.request_alignment = s->page_size;
Introduce device/iommu 'page_size_min' variables to make the code clearer. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- block/nvme.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)