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

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

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 <wei.w.wang@intel.com>
>>>>> ---
>>>>>     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.