| Message ID | 20170705133635.11850-5-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 05, 2017 at 09:36:33PM +0800, Fam Zheng wrote: > +static void nvme_dma_map(BlockDriverState *bs, void *host, size_t size) > +{ > + BDRVNVMeState *s = bs->opaque; > + > + nvme_vfio_dma_map(s->vfio, host, size, false, NULL); Since temporary=false repeated calls to map/unmap will run out of space and stop working after some time?
On 10/07/2017 16:59, Stefan Hajnoczi wrote: >> +static void nvme_dma_map(BlockDriverState *bs, void *host, size_t size) >> +{ >> + BDRVNVMeState *s = bs->opaque; >> + >> + nvme_vfio_dma_map(s->vfio, host, size, false, NULL); > Since temporary=false repeated calls to map/unmap will run out of space > and stop working after some time? Yes, the point of bdrv_dma_map/unmap is to add a permanent mapping. Temporary mappings are only valid inside nvme.c, because the corresponding iova is not recorded anywhere. Instead, bdrv_dma_map/unmap cache the iova just like we do for RAMBlock areas during system emulation. The solution is simply not to do that, just like img_bench only calls map/unmap once. If it happens, things just become slower as the driver falls back to temporary mappings. Paolo
On Mon, Jul 10, 2017 at 05:09:25PM +0200, Paolo Bonzini wrote: > On 10/07/2017 16:59, Stefan Hajnoczi wrote: > >> +static void nvme_dma_map(BlockDriverState *bs, void *host, size_t size) > >> +{ > >> + BDRVNVMeState *s = bs->opaque; > >> + > >> + nvme_vfio_dma_map(s->vfio, host, size, false, NULL); > > Since temporary=false repeated calls to map/unmap will run out of space > > and stop working after some time? > > Yes, the point of bdrv_dma_map/unmap is to add a permanent mapping. > Temporary mappings are only valid inside nvme.c, because the > corresponding iova is not recorded anywhere. Instead, > bdrv_dma_map/unmap cache the iova just like we do for RAMBlock areas > during system emulation. > > The solution is simply not to do that, just like img_bench only calls > map/unmap once. If it happens, things just become slower as the driver > falls back to temporary mappings. The constraints need to be documented. Someone might try to use blk_dma_map() and waste time debugging poor performance in the future. Stefan
diff --git a/block/nvme.c b/block/nvme.c index eb999a1..7913017 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -1056,6 +1056,20 @@ static void nvme_aio_unplug(BlockDriverState *bs) } } +static void nvme_dma_map(BlockDriverState *bs, void *host, size_t size) +{ + BDRVNVMeState *s = bs->opaque; + + nvme_vfio_dma_map(s->vfio, host, size, false, NULL); +} + +static void nvme_dma_unmap(BlockDriverState *bs, void *host) +{ + BDRVNVMeState *s = bs->opaque; + + nvme_vfio_dma_unmap(s->vfio, host); +} + static BlockDriver bdrv_nvme = { .format_name = "nvme", .protocol_name = "nvme", @@ -1081,6 +1095,9 @@ static BlockDriver bdrv_nvme = { .bdrv_io_plug = nvme_aio_plug, .bdrv_io_unplug = nvme_aio_unplug, + + .bdrv_dma_map = nvme_dma_map, + .bdrv_dma_unmap = nvme_dma_unmap, }; static void bdrv_nvme_init(void)
Forward these two calls to the IOVA manager. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/nvme.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)