From 190dcbde2872b57600088606785b75259b69efce Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Thu, 9 Dec 2021 12:23:39 +0000 Subject: allow DMA funcs to be called in quiesced state (#635) Signed-off-by: Thanos Makatos Reviewed-by: John Levon --- include/libvfio-user.h | 41 +++++++++++-- lib/libvfio-user.c | 2 - test/py/libvfio_user.py | 30 +++++++--- test/py/test_migration.py | 2 +- test/py/test_quiesce.py | 147 +++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 204 insertions(+), 18 deletions(-) diff --git a/include/libvfio-user.h b/include/libvfio-user.h index a67fe30..7a0000b 100644 --- a/include/libvfio-user.h +++ b/include/libvfio-user.h @@ -347,8 +347,10 @@ typedef enum vfu_reset_type { * Device callback for quiescing the device. * * vfu_run_ctx uses this callback to request from the device to quiesce its - * operation. A quiesced device cannot use the following functions: - * vfu_addr_to_sg, vfu_map_sg, vfu_unmap_sg, vfu_dma_read, and vfu_dma_write. + * operation. A quiesced device must call the following functions: + * - vfu_dma_read and vfu_dma_write, + * - vfu_addr_to_sg, vfu_map_sg, and vfu_unmap_sg, unless it does so from a + * device callback. * * The callback can return two values: * 1) 0: this indicates that the device was quiesced. vfu_run_ctx then continues @@ -362,9 +364,38 @@ typedef enum vfu_reset_type { * vfu_device_quiesced returns the device is no longer quiesced. * * A quiesced device should expect for any of the following callbacks to be - * executed: - * vfu_dma_register_cb_t, vfu_unregister_cb_t, vfu_reset_cb_t, and transition. - * These callbacks are only called after the device has been quiesced. + * executed: vfu_dma_register_cb_t, vfu_unregister_cb_t, vfu_reset_cb_t, and + * the migration transition callback. These callbacks are only called after the + * device has been quiesced. + * + * The following example demonstrates how a device can use vfu_map_sg and + * friends while quiesced: + * + * A DMA region is mapped, libvfio-user calls the quiesce callback but the + * device cannot immediately quiesce: + * + * int quiesce_cb(vfu_ctx_t *vfu_ctx) { + * errno = EBUSY; + * return -1; + * } + * + * While quiescing, the device can continue to operate as normal, including + * calling functions such as vfu_map_sg. Then, the device finishes quiescing: + * + * vfu_quiesce_done(vfu_ctx, 0); + * + * At this point, the device must have stopped using functions like + * vfu_map_sg(), for example by pausing any I/O threads. libvfio-user + * eventually calls the dma_register device callback before vfu_quiesce_done + * returns. In this callback the device is allowed to call functions such as + * vfu_map_sg: + * + * void (dma_register_cb(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info) { + * vfu_map_sg(ctx, ...); + * } + * + * Once vfu_quiesce_done returns, the device is unquiesced. + * * * @vfu_ctx: the libvfio-user context * diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 0a61f13..f1c99ec 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -1983,8 +1983,6 @@ vfu_map_sg(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, struct iovec *iov, int cnt, return ERROR_INT(EINVAL); } - assert(vfu_ctx->pending.state != VFU_CTX_PENDING_MSG); - ret = dma_map_sg(vfu_ctx->dma, sg, iov, cnt); if (ret < 0) { return -1; diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 60c5fbf..4b4a96a 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -672,9 +672,17 @@ def get_reply(sock, expect=0): return buf[16:] -def msg(ctx, sock, cmd, payload, expect_reply_errno=0, fds=None, rsp=True, - expect_run_ctx_errno=None): - """Round trip a request and reply to the server.""" +def msg(ctx, sock, cmd, payload=bytearray(), expect_reply_errno=0, fds=None, + rsp=True, expect_run_ctx_errno=None): + """ + Round trip a request and reply to the server. vfu_run_ctx will be + called once for the server to process the incoming message, + @expect_run_ctx_errrno checks the return value of vfu_run_ctx. If a + response is not expected then @rsp must be set to False, otherwise this + function will block indefinitely. + """ + # FIXME if expect_run_ctx_errno == errno.EBUSY then shouldn't it implied + # that rsp == False? hdr = vfio_user_header(cmd, size=len(payload)) if fds: @@ -821,7 +829,7 @@ def dma_unregister(ctx, info): @vfu_dma_unregister_cb_t def __dma_unregister(ctx, info): - dma_unregister(ctx, info) + dma_unregister(ctx, copy.copy(info.contents)) def quiesce_cb(ctx): @@ -856,7 +864,7 @@ def vfu_setup_device_reset_cb(ctx, cb=_reset_cb): def prepare_ctx_for_dma(dma_register=__dma_register, dma_unregister=__dma_unregister, quiesce=_quiesce_cb, - reset=_reset_cb): + reset=_reset_cb, migration_callbacks=False): ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -883,6 +891,10 @@ def prepare_ctx_for_dma(dma_register=__dma_register, fd=f.fileno()) assert ret == 0 + if migration_callbacks: + ret = vfu_setup_device_migration_callbacks(ctx) + assert ret == 0 + ret = vfu_realize_ctx(ctx) assert ret == 0 @@ -949,10 +961,10 @@ def vfu_attach_ctx(ctx, expect=0): def vfu_run_ctx(ctx, expect_errno=None): ret = lib.vfu_run_ctx(ctx) - if expect_errno is not None: + if expect_errno: assert ret < 0 and expect_errno == c.get_errno(), \ - "expected '%s' (%d), actual '%s' (%s)" % \ - (os.strerror(expect_errno), expect_errno, + "ret=%s, expected '%s' (%d), actual '%s' (%s)" % \ + (ret, os.strerror(expect_errno), expect_errno, os.strerror(c.get_errno()), c.get_errno()) return ret @@ -1105,7 +1117,7 @@ def __migr_data_written_cb(ctx, count): return migr_data_written_cb(ctx, count) -def vfu_setup_device_migration_callbacks(ctx, cbs=None, offset=0): +def vfu_setup_device_migration_callbacks(ctx, cbs=None, offset=0x4000): assert ctx is not None if not cbs: diff --git a/test/py/test_migration.py b/test/py/test_migration.py index d70be5e..094a693 100644 --- a/test/py/test_migration.py +++ b/test/py/test_migration.py @@ -46,7 +46,7 @@ def setup_function(function): flags=VFU_REGION_FLAG_RW) assert ret == 0 - ret = vfu_setup_device_migration_callbacks(ctx, offset=0x4000) + ret = vfu_setup_device_migration_callbacks(ctx) assert ret == 0 vfu_setup_device_quiesce_cb(ctx) diff --git a/test/py/test_quiesce.py b/test/py/test_quiesce.py index cf41e36..f231170 100644 --- a/test/py/test_quiesce.py +++ b/test/py/test_quiesce.py @@ -29,6 +29,7 @@ from libvfio_user import * import errno +from unittest import mock from unittest.mock import patch @@ -37,7 +38,7 @@ ctx = None def setup_function(function): global ctx, sock - ctx = prepare_ctx_for_dma() + ctx = prepare_ctx_for_dma(migration_callbacks=True) assert ctx is not None sock = connect_client(ctx) @@ -106,4 +107,148 @@ def test_device_quiesce_error_after_busy(mock_quiesce, mock_dma_register): assert c.get_errno() == errno.ENOENT +# DMA map/unmap, migration device state transition, and reset callbacks +# have the same function signature in Python +def _side_effect(ctx, _): + count, sgs = vfu_addr_to_sg(ctx, 0x10000, 0x1000) + assert count == 1 + sg = sgs[0] + assert sg.dma_addr == 0x10000 and sg.region == 0 \ + and sg.length == 0x1000 and sg.offset == 0 and sg.writeable + iovec = iovec_t() + ret = vfu_map_sg(ctx, sg, iovec) + assert ret == 0, "%s" % c.get_errno() + assert iovec.iov_base != 0 + assert iovec.iov_len == 0x1000 + assert ret == 0 + vfu_unmap_sg(ctx, sg, iovec) + return 0 + + +def _map_dma_region(ctx, sock, expect_run_ctx_errno=0): + rsp = expect_run_ctx_errno != errno.EBUSY + f = tempfile.TemporaryFile() + f.truncate(0x1000) + map_payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), + flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), + offset=0, addr=0x10000, size=0x1000) + msg(ctx, sock, VFIO_USER_DMA_MAP, map_payload, rsp=rsp, + expect_run_ctx_errno=expect_run_ctx_errno, fds=[f.fileno()]) + + +def _unmap_dma_region(ctx, sock, expect_run_ctx_errno=0): + rsp = expect_run_ctx_errno != errno.EBUSY + unmap_payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), + addr=0x10000, size=0x1000) + msg(ctx, sock, VFIO_USER_DMA_UNMAP, unmap_payload, rsp=rsp, + expect_run_ctx_errno=expect_run_ctx_errno) + + +@patch('libvfio_user.dma_register', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesced_dma_register(mock_quiesce, + mock_dma_register): + + global ctx, sock + + # FIXME assert quiesce callback is called + _map_dma_region(ctx, sock) + # FIXME it's difficult to check that mock_dma_register has been called with + # the expected DMA info because we don't know the vaddr and the mapping + # (2nd and 3rd arguments of vfu_dma_info_t) as they're values returned from + # mmap(0) so they can't be predicted. Using mock.ANY in their place fails + # with "TypeError: cannot be converted to pointer". In any case this is + # tested by other unit tests. + mock_dma_register.assert_called_once_with(ctx, mock.ANY) + + +@patch('libvfio_user.dma_unregister', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesced_dma_unregister(mock_quiesce, + mock_dma_unregister): + + global ctx, sock + _map_dma_region(ctx, sock) + _unmap_dma_region(ctx, sock) + mock_dma_unregister.assert_called_once_with(ctx, mock.ANY) + + +@patch('libvfio_user.dma_register', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb', side_effect=fail_with_errno(errno.EBUSY)) +def test_allowed_funcs_in_quiesced_dma_register_busy(mock_quiesce, + mock_dma_register): + + global ctx, sock + _map_dma_region(ctx, sock, errno.EBUSY) + ret = vfu_device_quiesced(ctx, 0) + assert ret == 0 + mock_dma_register.assert_called_once_with(ctx, mock.ANY) + + +@patch('libvfio_user.dma_unregister', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesced_dma_unregister_busy(mock_quiesce, + mock_dma_unregister): + + global ctx, sock + _map_dma_region(ctx, sock) + mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) + _unmap_dma_region(ctx, sock, expect_run_ctx_errno=errno.EBUSY) + ret = vfu_device_quiesced(ctx, 0) + assert ret == 0 + mock_dma_unregister.assert_called_once_with(ctx, mock.ANY) + + +@patch('libvfio_user.migr_trans_cb', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesed_migration(mock_quiesce, + mock_trans): + + global ctx, sock + _map_dma_region(ctx, sock) + data = VFIO_DEVICE_STATE_SAVING.to_bytes(c.sizeof(c.c_int), 'little') + write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + count=len(data), data=data) + mock_trans.assert_called_once_with(ctx, VFIO_DEVICE_STATE_SAVING) + + +@patch('libvfio_user.migr_trans_cb', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesed_migration_busy(mock_quiesce, + mock_trans): + + global ctx, sock + _map_dma_region(ctx, sock) + mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) + data = VFIO_DEVICE_STATE_STOP.to_bytes(c.sizeof(c.c_int), 'little') + write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + count=len(data), data=data, rsp=False, + expect_run_ctx_errno=errno.EBUSY) + ret = vfu_device_quiesced(ctx, 0) + assert ret == 0 + mock_trans.assert_called_once_with(ctx, VFIO_DEVICE_STATE_STOP) + + +@patch('libvfio_user.reset_cb', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesced_reset(mock_quiesce, mock_reset): + global ctx, sock + _map_dma_region(ctx, sock) + msg(ctx, sock, VFIO_USER_DEVICE_RESET) + mock_reset.assert_called_once_with(ctx, VFU_RESET_DEVICE) + + +@patch('libvfio_user.reset_cb', side_effect=_side_effect) +@patch('libvfio_user.quiesce_cb') +def test_allowed_funcs_in_quiesced_reset_busy(mock_quiesce, mock_reset): + global ctx, sock + _map_dma_region(ctx, sock) + mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) + msg(ctx, sock, VFIO_USER_DEVICE_RESET, rsp=False, + expect_run_ctx_errno=errno.EBUSY) + ret = vfu_device_quiesced(ctx, 0) + assert ret == 0 + mock_reset.assert_called_once_with(ctx, VFU_RESET_DEVICE) + + # ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: # -- cgit v1.1