Re: [Qemu-devel] [PATCH 4/6 Resend] Vhost-pci RFC: Detailed Description in the Virtio Specification Format

[Wang, Wei W]

On Thu 6/2/2016 7:13 PM, Xiao Guangrong wrote:
> On 06/02/2016 04:43 PM, Wang, Wei W wrote:
> > On Thu 6/2/2016 11:52 AM,  Xiao Guangrong wrote:
> >> On 06/02/2016 11:15 AM, Wang, Wei W wrote:
> >>> On Wed 6/1/2016 4:15 PM, Xiao Guangrong wrote:
> >>>> On 05/29/2016 04:11 PM, Wei Wang wrote:
> >>>>> Signed-off-by: Wei Wang <address@hidden>
> >>>>> ---
> >>>>>     Details | 324
> >>>>
> >>
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>     1 file changed, 324 insertions(+)
> >>>>>     create mode 100644 Details
> >>>>>
> >>>>> diff --git a/Details b/Details
> >>>>> new file mode 100644
> >>>>> index 0000000..4ea2252
> >>>>> --- /dev/null
> >>>>> +++ b/Details
> >>>>> @@ -0,0 +1,324 @@
> >>>>> +1 Device ID
> >>>>> +TBD
> >>>>> +
> >>>>> +2 Virtqueues
> >>>>> +0 controlq
> >>>>> +
> >>>>> +3 Feature Bits
> >>>>> +3.1 Local Feature Bits
> >>>>> +Currently no local feature bits are defined, so the standard
> >>>>> +virtio feature bits negation will always be successful and complete.
> >>>>> +
> >>>>> +3.2 Remote Feature Bits
> >>>>> +The remote feature bits are obtained from the frontend virtio
> >>>>> +device and negotiated with the vhost-pci driver via the controlq.
> >>>>> +The negotiation steps are described in 4.5 Device Initialization.
> >>>>> +
> >>>>> +4 Device Configuration Layout
> >>>>> +struct vhost_pci_config {
> >>>>> +       #define VHOST_PCI_CONTROLQ_MEMORY_INFO_ACK 0
> >>>>> +       #define VHOST_PCI_CONTROLQ_DEVICE_INFO_ACK 1
> >>>>> +       #define VHOST_PCI_CONTROLQ_FEATURE_BITS_ACK 2
> >>>>> +       u32 ack_type;
> >>>>> +       u32 ack_device_type;
> >>>>> +       u64 ack_device_id;
> >>>>> +       union {
> >>>>> +               #define VHOST_PCI_CONTROLQ_ACK_ADD_DONE 0
> >>>>> +               #define VHOST_PCI_CONTROLQ_ACK_ADD_FAIL 1
> >>>>> +               #define VHOST_PCI_CONTROLQ_ACK_DEL_DONE 2
> >>>>> +               #define VHOST_PCI_CONTROLQ_ACK_DEL_FAIL 3
> >>>>> +               u64 ack_memory_info;
> >>>>> +               u64 ack_device_info;
> >>>>> +               u64 ack_feature_bits;
> >>>>> +       };
> >>>>> +};
> >>>>
> >>>> Do you need to write all these 4 field to ack the operation? It
> >>>> seems it is not efficient and it is not flexible if the driver need
> >>>> to offer more data to the device in the further. Can we dedicate a
> >>>> vq for this purpose？
> >>>
> >>> Yes, all the 4 fields are required to be written. The vhost-pci
> >>> server usually
> >> connects to multiple clients, and the "ack_device_type" and "ack_device_id"
> >> fields are used to identify them.
> >>>
> >>> Agree, another controlq for the guest->host direction looks better,
> >>> and the
> >> above fileds can be converted to be the controlq message header.
> >>>
> >>
> >> Thanks.
> >>
> >>>>
> >>>> BTW, current approach can not handle the case if there are multiple
> >>>> same kind of requests in the control queue, e.g, if there are two
> >>>> memory-add request in the control queue.
> >>>
> >>> A vhost-pci device corresponds to a driver VM. The two memory-add
> >>> requests
> >> on the controlq are all for the same driver VM.  Memory-add requests
> >> for different driver VMs  couldn’t be present on the same controlq. I
> >> haven’t seen the issue yet. Can you please explain more? Thanks.
> >>
> >> The issue is caused by "The two memory-add requests on the controlq
> >> are all for the same driver VM", the driver need to ACK these request
> >> respectively, however, these two requests have the same ack_type,
> >> device_type, device_id, ack_memory_info, then QEMU is not able to
> >> figure out which request has been acked.
> >
> > Normally pieces of memory info should be combined into one message (the
> structure includes multiple memory regions) and sent by the client. In a rare 
> case
> like this: the driver VM hot-adds 1GB memory, followed by hot-adding another
> 1GB memory. The first piece of memory info is passed via the socket and
> controlq to the vhost-pci driver, then the second. Normally they won't get an
> opportunity to be put on the controlq at the same time.
> > Even the implementation batches the controlq messages, there will be a
> sequence difference between the two messages on the controlq, right?
> 
> That assumes the driver should serially handle the control messages...
> 
> >
> >  From the QEMU's (vhost-pci server) perspective, it just sends back an ACK 
> > to
> the client whenever it receives an ACK from the vhost-pci driver.
> >  From the client's perspective, it will receive two ACK messages in this 
> > example.
> > Since the two have a sequence difference, the client should be able to
> distinguish the two (first sent, first acked), right?
> 
> That assumes that the vhost-pci server and remote virtio device should use 
> serial
> mode too.

Before adding more fields to support that, I have another two questions to 
discuss here:

In my understanding, socket messages are sent one by one. How would a client 
send
two messages in parallel to the server?

Regarding the controlq, I understand that there are optimizations to 
parallelize ring 
operations, but that are mostly done to increase the data plane performance. 
Is it necessary to do such parallelism for the control message operations, 
which would
complicate the design?

Best,
Wei