Re: [virtio-dev] Re: [PATCH 0/5] virtio: introduce SUSPEND bit and vq state

Zhu, Lingshan
2023-09-20T12:16:20Z

On 9/20/2023 8:05 PM, Michael S. Tsirkin wrote:
> On Wed, Sep 20, 2023 at 07:22:32PM +0800, Zhu, Lingshan wrote:
>>
>> On 9/20/2023 6:36 PM, Michael S. Tsirkin wrote:
>>> On Wed, Sep 20, 2023 at 02:06:13PM +0800, Zhu, Lingshan wrote:
>>>> On 9/19/2023 2:49 AM, Michael S. Tsirkin wrote:
>>>>> On Mon, Sep 18, 2023 at 06:41:55PM +0000, Parav Pandit wrote:
>>>>>>> Please refer to the code for setting FEATURES_OK.
>>>>>> It wont work when one needs to suspend the device.
>>>>>> There is no point of doing such work over registers as fundamental framework is over the AQ.
>>>>> Well not really. It's over admin commands. When these were built the
>>>>> intent always was that it's possible to use admin commands through
>>>>> another interface, other than admin queue. Is there a problem
>>>>> implementing admin commands over a memory BAR? For example, I can see
>>>>> an "admin command" capability pointing at a BAR where
>>>>> commands are supplied, and using a new group type referring to
>>>>> device itself.
>>>> I am not sure, if a bar cap would be implemented as a proxy for the admin vq
>>>> based live migration.
>>> Not a proxy for a vq in that there's no vq then.
>> I think if the driver sends admin commands through a VF's bar, then
>> VF forwards the admin commands to the PF, it acts like a proxy,
>> or an agent. Anyway it takes admin commands.
> Why send them to the PF? They are controlling the VF anyway.
I think its still too heavy compared to this series proposal
>
>> So the problems we have discussed still exist.
>>>> then the problems of admin vq LM that we have
>>>> discussed
>>>> still exist.
>>> I freely admit the finer points of this extended flamewar have been lost
>>> on me, and I wager I'm not the only one. I thought you wanted to migrate
>>> the device just by accessing the device itself (e.g. the VF) without
>>> accessing other devices (e.g. the PF), while Parav wants it in a
>>> separate device so the whole of the device itself can passed through to
>>> guest. Isn't this, fundamentally, the issue?
>> we are implementing basic facilities for live migration.
>>
>> We have pointed out lots of issues, there are many discussions with
>> Jason and Parav about the problems in migration by admin vq, for example:
>> security, QOS and nested.
> /me shrugs
> Thanks for the summary I guess. Same applies to almost any proposal.
> What would help make progress is an explanation why this has grown into
> a megathread. Do you understand Parav's thoughts well enough to
> summarize them?
as far as I see, I don't see admin vq as must for live migration.
and it does not serve nested for sure.
>
>>>> the bar is only a proxy, doesn't fix anything. and even larger
>>>> side channel attacking surface: vf-->pf-->vf
>>> In this model there's no pf. BAR belongs to vf itself
>>> and you submit commands for the VF through its BAR.
>>> Just separate from the pci config space.
>> If using the bar to process admin commands,
>> is this solution too heavy compared to my proposal in this series?
> somewhat - because it's more comprehensive - you can actually
> migrate a device using it.
> this series just begins to define how to poke at some
> of the vq state - it's a subset of the necessary functionality.
>
> And it will give you a bunch of side benefits, such as
> support for legacy compat commands that were merged.
next version will include in-flight descriptors and dirty page tracking.
I failed to process the comments for legacy.
legacy devices are defined by code than the spec, if wants to migrate legacy
devices, maybe working on QEMU first
>
>
>
>>> The whole attacking surface discussion is also puzzling. We either are
>>> or are not discussing confidential computing/TDI. I couldn't figure
>>> it out. This needs a separate thread I think.
>> I agree confidential computing is out of spec. Parva mentioned TDISP and
>> even
>> in TDISP spec, it explicitly defined some attacking model, and PF is an
>> example.
>>
>> It is out of spec anyway.
> OK so we are ignoring TDISP applications for now? Everyone agrees on
> that?
sure