[virtio-dev] Re: [virtio-comment] [PATCH 5/5] virtio-pci: implement VIRTIO_F_QUEUE_STATE

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 9/12/2023 2:11 PM, Parav Pandit wrote:
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Tuesday, September 12, 2023 9:48 AM
>>
>> On Mon, Sep 11, 2023 at 2:47 PM Parav Pandit <parav@nvidia.com> wrote:
>>>
>>>
>>>> From: Jason Wang <jasowang@redhat.com>
>>>> Sent: Monday, September 11, 2023 12:01 PM
>>>>
>>>> On Mon, Sep 11, 2023 at 12:12 PM Parav Pandit <parav@nvidia.com>
>> wrote:
>>>>> Hi Michael,
>>>>>
>>>>>> From: virtio-comment@lists.oasis-open.org
>>>>>> <virtio-comment@lists.oasis- open.org> On Behalf Of Jason Wang
>>>>>> Sent: Monday, September 11, 2023 8:31 AM
>>>>>>
>>>>>> On Wed, Sep 6, 2023 at 4:33 PM Michael S. Tsirkin
>>>>>> <mst@redhat.com>
>>>> wrote:
>>>>>>> On Wed, Sep 06, 2023 at 04:16:37PM +0800, Zhu Lingshan wrote:
>>>>>>>> This patch adds two new le16 fields to common configuration
>>>>>>>> structure to support VIRTIO_F_QUEUE_STATE in PCI transport layer.
>>>>>>>>
>>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>>>>
>>>>>>> I do not see why this would be pci specific at all.
>>>>>> This is the PCI interface for live migration. The facility is not specific to
>> PCI.
>>>>>> It can choose to reuse the common configuration or not, but the
>>>>>> semantic is general enough to be used by other transports. We
>>>>>> can introduce one for MMIO for sure.
>>>>>>
>>>>>>> But besides I thought work on live migration will use admin queue.
>>>>>>> This was explicitly one of the motivators.
>>>>> Please find the proposal that uses administration commands for
>>>>> device
>>>> migration at [1] for passthrough devices.
>>>>> [1]
>>>>> https://lists.oasis-open.org/archives/virtio-comment/202309/msg000
>>>>> 61.h
>>>>> tml
>>>> This proposal couples live migration with several requirements, and
>>>> suffers from the exact issues I've mentioned below.
>>>>
>>> It does not.
>>> Can you please list which one?
>>>
>>>> In some cases, it's even worse (coupling with PCI/SR-IOV, second
>>>> state machine other than the device status).
>>>>
>>> There is no state machine in [1].
>> Isn't the migration modes of "active, stop, freeze" a state machine?
>>
> Huh, no. Each mode stops/starts specific thing.
> Just because one series is missing this and did only suspend/resume and other series covered P2P mode with modes, it does not make it state machine.
> If you call suspend resume as states, it is still 2 state state machines. :)
Why need P2P for Live Migration?
>
>>> It is not coupled with PCI/SR-IOV either.
>>> It supports PCI/SR-IOV transport and in future other transports too when they
>> evolve.
>> For example:
>>
>> +struct virtio_dev_ctx_pci_vq_cfg {
>> + le16 vq_index;
>> +        le16 queue_size;
>> +        le16 queue_msix_vector;
>> + le64 queue_descÍ
>> +        le64 queue_driverÍ
>> +        le64 queue_deviceÍ
>> +};
>> +\end{lstlisting}
>>
>> And does this mean we will have commands for MMIO and other transport?
> There are transports so yes, field structures from the device context will have PCI specific items.
>
>> (Most of the fields except the msix are general enough). And it's just a partial
>> implementation of the queue related functionality of the common cfg, so I
>> wonder how it can work.
>>
> As I already explained in cover letter, device context will evolve in v0->v1 to cover more.
> True, most fields are general, and it has some pci specific fields, which were not worth taking out to a different structure.
> And replicating small number of structs for MMIO is not a problem either as it is not complicating the transport either.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org