On Fri, May 12, 2017 at 04:21:58PM +0200, Maxime Coquelin wrote: > > > On 05/11/2017 08:25 PM, Michael S. Tsirkin wrote: > > On Thu, May 11, 2017 at 02:32:46PM +0200, Maxime Coquelin wrote: > > > This patch specifies and implements the master/slave communication > > > to support device IOTLB in slave. > > > > > > The vhost_iotlb_msg structure introduced for kernel backends is > > > re-used, making the design close between the two backends. > > > > > > An exception is the use of the secondary channel to enable the > > > slave to send IOTLB miss requests to the master. > > > > > > Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com> > > > --- > > > docs/specs/vhost-user.txt | 75 > > > +++++++++++++++++++++++++++++++++++++++++++++++ > > > hw/virtio/vhost-user.c | 31 ++++++++++++++++++++ > > > 2 files changed, 106 insertions(+) > > > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > > > index 5fa7016..4a1f0c3 100644 > > > --- a/docs/specs/vhost-user.txt > > > +++ b/docs/specs/vhost-user.txt > > > @@ -97,6 +97,23 @@ Depending on the request type, payload can be: > > > log offset: offset from start of supplied file descriptor > > > where logging starts (i.e. where guest address 0 would be logged) > > > + * An IOTLB message > > > + --------------------------------------------------------- > > > + | iova | size | user address | permissions flags | type | > > > + --------------------------------------------------------- > > > + > > > + IOVA: a 64-bit guest I/O virtual address > > > > guest -> VM > > Ok. > > > > > > + Size: a 64-bit size > > > > How do you specify "all memory"? give special meaning to size 0? > > Good point, it does not support all memory currently. > It is not vhost-user specific, but general to the vhost implementation.
But iommu needs it to support passthrough. > > > > + User address: a 64-bit user address > > > + Permissions flags: a 8-bit bit field: > > > + - Bit 0: Read access > > > + - Bit 1: Write access > > > > Can both bits be set? Can none? > > Both. I will change it by listing values directly: > - 0 : No access > - 1 : Read > - 2 : Write > - 3 : Read Write > > > > + Type: a 8-bit IOTLB message type: > > > + - 1: IOTLB miss > > > + - 2: IOTLB update > > > + - 3: IOTLB invalidate > > > + - 4: IOTLB access fail > > > + > > > In QEMU the vhost-user message is implemented with the following struct: > > > typedef struct VhostUserMsg { > > > @@ -109,6 +126,7 @@ typedef struct VhostUserMsg { > > > struct vhost_vring_addr addr; > > > VhostUserMemory memory; > > > VhostUserLog log; > > > + struct vhost_iotlb_msg iotlb; > > > }; > > > } QEMU_PACKED VhostUserMsg; > > > @@ -253,6 +271,31 @@ Once the source has finished migration, rings will > > > be stopped by > > > the source. No further update must be done before rings are > > > restarted. > > > +IOMMU support > > > +------------- > > > + > > > +When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated, the master > > > has > > > +to send IOTLB entries update & invalidation by sending > > > VHOST_USER_IOTLB_MSG > > > +requests to the slave with a struct vhost_iotlb_msg payload. > > > > Always? This seems a bit strange since iommu can be enabled/disabled > > dynamically. > Ok, what about: > > When the VIRTIO_F_IOMMU_PLATFORM feature has been negotiated and iommu > is enbaled, the master sends IOTLB entries update & invalidation via > VHOST_USER_IOTLB_MSG requests to the slave with a struct vhost_iotlb_msg > payload. > > > > Closing channel seems like a wrong thing to do for this. > > Sorry, I'm not sure to get your comment. What happens when guest disables the IOMMU? > > > For update events, > > > +the iotlb payload has to be filled with the update message type (2), the > > > I/O > > > +virtual address, the size, the user virtual address, and the permissions > > > +flags. For invalidation events, the iotlb payload has to be filled with > > > the > > > +invalidation message type (3), the I/O virtual address and the size. On > > > +success, the slave is expected to reply with a zero payload, non-zero > > > +otherwise. > > > + > > > +When the VHOST_USER_PROTOCOL_F_SLAVE_REQ is supported by the slave, and > > > the > > > +master initiated the slave to master communication channel using the > > > +VHOST_USER_SET_SLAVE_REQ_FD request, the slave can send IOTLB miss and > > > access > > > +failure events by sending VHOST_USER_SLAVE_IOTLB_MSG requests to the > > > master > > > +with a struct vhost_iotlb_msg payload. For miss events, the iotlb > > > payload has > > > +to be filled with the miss message type (1), the I/O virtual address and > > > the > > > +permissions flags. For access failure event, the iotlb payload has to be > > > +filled with the access failure message type (4), the I/O virtual address > > > and > > > +the permissions flags. For synchronization purpose, the slave may rely > > > on the > > > +reply-ack feature, so the master may send a reply when operation is > > > completed > > > +if the reply-ack feature is negotiated and slaves requests a reply. > > > + > > > Slave communication > > > ------------------- > > > @@ -514,6 +557,38 @@ Master message types > > > If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must > > > respond > > > with zero for success, non-zero otherwise. > > > + * VHOST_USER_IOTLB_MSG > > > + > > > + Id: 22 > > > + Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type) > > > + Master payload: struct vhost_iotlb_msg > > > + Slave payload: u64 > > > + > > > + Send IOTLB messages with struct vhost_iotlb_msg as payload. > > > + Master sends such requests to update and invalidate entries in the > > > device > > > + IOTLB. The slave has to acknowledge the request with sending zero > > > as u64 > > > + payload for success, non-zero otherwise. > > > + This request should be send only when VIRTIO_F_IOMMU_PLATFORM > > > feature > > > + has been successfully negotiated. > > > + > > > +Slave message types > > > +------------------- > > > + > > > + * VHOST_USER_SLAVE_IOTLB_MSG > > > + > > > + Id: 1 > > > + Equivalent ioctl: N/A (equivalent to VHOST_IOTLB_MSG message type) > > > + Slave payload: struct vhost_iotlb_msg > > > + Master payload: N/A > > > + > > > + Send IOTLB messages with struct vhost_iotlb_msg as payload. > > > + Slave sends such requests to notify of an IOTLB miss, or an IOTLB > > > + access failure. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, > > > + and slave set the VHOST_USER_NEED_REPLY flag, master must respond > > > with > > > + zero when operation is successfully completed, or non-zero > > > otherwise. > > > + This request should be send only when VIRTIO_F_IOMMU_PLATFORM > > > feature > > > + has been successfully negotiated. > > > + > > > > Are there limitations on number of messages in flight? > > I didn't think about this, I would say the maximum number of messages in > flight is dependent on the socket buffer size (which is kept to default > in this series). > > You question highlights a bug in by DPDK prototype, as the MISS request > can be sent by multiple threads, and I didn't protected this with a lock > to prevent concurrent read on the socket when waiting for the REPLY_ACK. > > Thanks, > Maxime A way to make it work without a lock might possibly be valuable. Can be done on top. -- MST