| Message ID | 1447251876-4811-1-git-send-email-victork@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi ----- Original Message ----- > Unlike the kernel, vhost-user application accesses log table by > mmaping it to its user space. This change adds two new fields to > VhostUserMsg payload: mmap_size, and mmap_offset and make QEMU to > pass the to vhost-user application in VHOST_USER_SET_LOG_BASE > request. > > Signed-off-by: Victor Kaplansky <victork@redhat.com> What's the motivation for doing this? The offset is always 0, and the size must at least match the size of VM memory. > > --- > hw/virtio/vhost-user.c | 11 +++++++++-- > tests/vhost-user-test.c | 8 ++++++++ > docs/specs/vhost-user.txt | 8 +++++++- > 3 files changed, 24 insertions(+), 3 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 83c84f1..46c63bc 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -75,6 +75,11 @@ typedef struct VhostUserMemory { > VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS]; > } VhostUserMemory; > > +typedef struct VhostUserLog { > + uint64_t mmap_size; > + uint64_t mmap_offset; > +} VhostUserLog; > + > typedef struct VhostUserMsg { > VhostUserRequest request; > > @@ -89,6 +94,7 @@ typedef struct VhostUserMsg { > struct vhost_vring_state state; > struct vhost_vring_addr addr; > VhostUserMemory memory; > + VhostUserLog log; > } payload; > } QEMU_PACKED VhostUserMsg; > > @@ -200,8 +206,9 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, > uint64_t base, > VhostUserMsg msg = { > .request = VHOST_USER_SET_LOG_BASE, > .flags = VHOST_USER_VERSION, > - .payload.u64 = base, > - .size = sizeof(msg.payload.u64), > + .payload.log.mmap_size = log->size, > + .payload.log.mmap_offset = 0, > + .size = sizeof(msg.payload.log), > }; > > if (shmfd && log->fd != -1) { > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c > index b6dde75..f005ecf 100644 > --- a/tests/vhost-user-test.c > +++ b/tests/vhost-user-test.c > @@ -86,6 +86,11 @@ typedef struct VhostUserMemory { > VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS]; > } VhostUserMemory; > > +typedef struct VhostUserLog { > + uint64_t mmap_size; > + uint64_t mmap_offset; > +} VhostUserLog; > + > typedef struct VhostUserMsg { > VhostUserRequest request; > > @@ -94,10 +99,13 @@ typedef struct VhostUserMsg { > uint32_t flags; > uint32_t size; /* the following payload size */ > union { > +#define VHOST_USER_VRING_IDX_MASK (0xff) > +#define VHOST_USER_VRING_NOFD_MASK (0x1<<8) > uint64_t u64; > struct vhost_vring_state state; > struct vhost_vring_addr addr; > VhostUserMemory memory; > + VhostUserLog log; > } payload; > } QEMU_PACKED VhostUserMsg; > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index e0d71e2..eb8f2b2 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -98,6 +98,7 @@ typedef struct VhostUserMsg { > struct vhost_vring_state state; > struct vhost_vring_addr addr; > VhostUserMemory memory; > + VhostUserLog log; > }; > } QEMU_PACKED VhostUserMsg; > > @@ -282,7 +283,12 @@ Message types > Master payload: u64 > Slave payload: N/A > > - Sets the logging base address. > + Sets logging shared memory space. > + When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol > + feature, the log memory fd is provided in the ancillary data of > + VHOST_USER_SET_LOG_BASE message, the size and offset of shared > + memory area provided in the message. > + > > * VHOST_USER_SET_LOG_FD > > -- > --Victor >
On Wed, Nov 11, 2015 at 09:32:17AM -0500, Marc-André Lureau wrote: > Hi > > ----- Original Message ----- > > Unlike the kernel, vhost-user application accesses log table by > > mmaping it to its user space. This change adds two new fields to > > VhostUserMsg payload: mmap_size, and mmap_offset and make QEMU to > > pass the to vhost-user application in VHOST_USER_SET_LOG_BASE > > request. > > > > Signed-off-by: Victor Kaplansky <victork@redhat.com> > > What's the motivation for doing this? The offset is always 0, and the size must at least match the size of VM memory. Remote doesn't know the size though, and offset is only there in the current implementation. > > > > --- > > hw/virtio/vhost-user.c | 11 +++++++++-- > > tests/vhost-user-test.c | 8 ++++++++ > > docs/specs/vhost-user.txt | 8 +++++++- > > 3 files changed, 24 insertions(+), 3 deletions(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index 83c84f1..46c63bc 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -75,6 +75,11 @@ typedef struct VhostUserMemory { > > VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS]; > > } VhostUserMemory; > > > > +typedef struct VhostUserLog { > > + uint64_t mmap_size; > > + uint64_t mmap_offset; > > +} VhostUserLog; > > + > > typedef struct VhostUserMsg { > > VhostUserRequest request; > > > > @@ -89,6 +94,7 @@ typedef struct VhostUserMsg { > > struct vhost_vring_state state; > > struct vhost_vring_addr addr; > > VhostUserMemory memory; > > + VhostUserLog log; > > } payload; > > } QEMU_PACKED VhostUserMsg; > > > > @@ -200,8 +206,9 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, > > uint64_t base, > > VhostUserMsg msg = { > > .request = VHOST_USER_SET_LOG_BASE, > > .flags = VHOST_USER_VERSION, > > - .payload.u64 = base, > > - .size = sizeof(msg.payload.u64), > > + .payload.log.mmap_size = log->size, > > + .payload.log.mmap_offset = 0, > > + .size = sizeof(msg.payload.log), > > }; > > > > if (shmfd && log->fd != -1) { > > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c > > index b6dde75..f005ecf 100644 > > --- a/tests/vhost-user-test.c > > +++ b/tests/vhost-user-test.c > > @@ -86,6 +86,11 @@ typedef struct VhostUserMemory { > > VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS]; > > } VhostUserMemory; > > > > +typedef struct VhostUserLog { > > + uint64_t mmap_size; > > + uint64_t mmap_offset; > > +} VhostUserLog; > > + > > typedef struct VhostUserMsg { > > VhostUserRequest request; > > > > @@ -94,10 +99,13 @@ typedef struct VhostUserMsg { > > uint32_t flags; > > uint32_t size; /* the following payload size */ > > union { > > +#define VHOST_USER_VRING_IDX_MASK (0xff) > > +#define VHOST_USER_VRING_NOFD_MASK (0x1<<8) > > uint64_t u64; > > struct vhost_vring_state state; > > struct vhost_vring_addr addr; > > VhostUserMemory memory; > > + VhostUserLog log; > > } payload; > > } QEMU_PACKED VhostUserMsg; > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > > index e0d71e2..eb8f2b2 100644 > > --- a/docs/specs/vhost-user.txt > > +++ b/docs/specs/vhost-user.txt > > @@ -98,6 +98,7 @@ typedef struct VhostUserMsg { > > struct vhost_vring_state state; > > struct vhost_vring_addr addr; > > VhostUserMemory memory; > > + VhostUserLog log; > > }; > > } QEMU_PACKED VhostUserMsg; > > > > @@ -282,7 +283,12 @@ Message types > > Master payload: u64 > > Slave payload: N/A > > > > - Sets the logging base address. > > + Sets logging shared memory space. > > + When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol > > + feature, the log memory fd is provided in the ancillary data of > > + VHOST_USER_SET_LOG_BASE message, the size and offset of shared > > + memory area provided in the message. > > + > > > > * VHOST_USER_SET_LOG_FD > > > > -- > > --Victor > >
Hi On Wed, Nov 11, 2015 at 4:17 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > Remote doesn't know the size though, and offset is only there > in the current implementation. It can compute the size based on the mem tables. But it could be more future-proof to add these informations.
On Wed, Nov 11, 2015 at 04:23:33PM +0100, Marc-André Lureau wrote: > Hi > > On Wed, Nov 11, 2015 at 4:17 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > Remote doesn't know the size though, and offset is only there > > in the current implementation. > > It can compute the size based on the mem tables. But it could be more > future-proof to add these informations. Right. Doing it now means we won't need a separate feature flag for it. > -- > Marc-André Lureau
Hi On Wed, Nov 11, 2015 at 3:26 PM, Victor Kaplansky <victork@redhat.com> wrote: > > - Sets the logging base address. > + Sets logging shared memory space. > + When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol > + feature, the log memory fd is provided in the ancillary data of > + VHOST_USER_SET_LOG_BASE message, the size and offset of shared > + memory area provided in the message. I think this extra payload needs a better description, in payload description above. Something like * A pair of 64-bit integers ------------- | u64 | u64 | ------------- u64: a 64-bit unsigned integer "the size and offset of shared memory area provided in the message as a pair of 64-bit integers."
On Wed, Nov 11, 2015 at 04:33:05PM +0100, Marc-André Lureau wrote: > Hi > > On Wed, Nov 11, 2015 at 3:26 PM, Victor Kaplansky <victork@redhat.com> wrote: > > > > - Sets the logging base address. > > + Sets logging shared memory space. > > + When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol > > + feature, the log memory fd is provided in the ancillary data of > > + VHOST_USER_SET_LOG_BASE message, the size and offset of shared > > + memory area provided in the message. > > I think this extra payload needs a better description, in payload > description above. Something like > > * A pair of 64-bit integers > ------------- > | u64 | u64 | > ------------- > > u64: a 64-bit unsigned integer > > "the size and offset of shared memory area provided in the message as > a pair of 64-bit integers." Sounds good. I've queued this one up, pls tweak docs with patches on top. > -- > Marc-André Lureau
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 83c84f1..46c63bc 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -75,6 +75,11 @@ typedef struct VhostUserMemory { VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS]; } VhostUserMemory; +typedef struct VhostUserLog { + uint64_t mmap_size; + uint64_t mmap_offset; +} VhostUserLog; + typedef struct VhostUserMsg { VhostUserRequest request; @@ -89,6 +94,7 @@ typedef struct VhostUserMsg { struct vhost_vring_state state; struct vhost_vring_addr addr; VhostUserMemory memory; + VhostUserLog log; } payload; } QEMU_PACKED VhostUserMsg; @@ -200,8 +206,9 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base, VhostUserMsg msg = { .request = VHOST_USER_SET_LOG_BASE, .flags = VHOST_USER_VERSION, - .payload.u64 = base, - .size = sizeof(msg.payload.u64), + .payload.log.mmap_size = log->size, + .payload.log.mmap_offset = 0, + .size = sizeof(msg.payload.log), }; if (shmfd && log->fd != -1) { diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c index b6dde75..f005ecf 100644 --- a/tests/vhost-user-test.c +++ b/tests/vhost-user-test.c @@ -86,6 +86,11 @@ typedef struct VhostUserMemory { VhostUserMemoryRegion regions[VHOST_MEMORY_MAX_NREGIONS]; } VhostUserMemory; +typedef struct VhostUserLog { + uint64_t mmap_size; + uint64_t mmap_offset; +} VhostUserLog; + typedef struct VhostUserMsg { VhostUserRequest request; @@ -94,10 +99,13 @@ typedef struct VhostUserMsg { uint32_t flags; uint32_t size; /* the following payload size */ union { +#define VHOST_USER_VRING_IDX_MASK (0xff) +#define VHOST_USER_VRING_NOFD_MASK (0x1<<8) uint64_t u64; struct vhost_vring_state state; struct vhost_vring_addr addr; VhostUserMemory memory; + VhostUserLog log; } payload; } QEMU_PACKED VhostUserMsg; diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index e0d71e2..eb8f2b2 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -98,6 +98,7 @@ typedef struct VhostUserMsg { struct vhost_vring_state state; struct vhost_vring_addr addr; VhostUserMemory memory; + VhostUserLog log; }; } QEMU_PACKED VhostUserMsg; @@ -282,7 +283,12 @@ Message types Master payload: u64 Slave payload: N/A - Sets the logging base address. + Sets logging shared memory space. + When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol + feature, the log memory fd is provided in the ancillary data of + VHOST_USER_SET_LOG_BASE message, the size and offset of shared + memory area provided in the message. + * VHOST_USER_SET_LOG_FD
Unlike the kernel, vhost-user application accesses log table by mmaping it to its user space. This change adds two new fields to VhostUserMsg payload: mmap_size, and mmap_offset and make QEMU to pass the to vhost-user application in VHOST_USER_SET_LOG_BASE request. Signed-off-by: Victor Kaplansky <victork@redhat.com> --- hw/virtio/vhost-user.c | 11 +++++++++-- tests/vhost-user-test.c | 8 ++++++++ docs/specs/vhost-user.txt | 8 +++++++- 3 files changed, 24 insertions(+), 3 deletions(-)