netdev - Re: [PATCH mlx5-next 2/7] vfio: Add an API to check migration state transition validity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c7a18080-3ffc-488b-577c-1e3b356bf66e@nvidia.com>
Date:   Thu, 30 Sep 2021 19:51:22 +0300
From:   Max Gurtovoy <mgurtovoy@...dia.com>
To:     Jason Gunthorpe <jgg@...pe.ca>
CC:     Alex Williamson <alex.williamson@...hat.com>,
        Leon Romanovsky <leon@...nel.org>,
        Doug Ledford <dledford@...hat.com>,
        Yishai Hadas <yishaih@...dia.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Kirti Wankhede <kwankhede@...dia.com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <linux-rdma@...r.kernel.org>, <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...dia.com>,
        Cornelia Huck <cohuck@...hat.com>
Subject: Re: [PATCH mlx5-next 2/7] vfio: Add an API to check migration state
 transition validity


On 9/30/2021 7:24 PM, Jason Gunthorpe wrote:
> On Thu, Sep 30, 2021 at 06:32:07PM +0300, Max Gurtovoy wrote:
>>> Just prior to open device the vfio pci layer will generate a FLR to
>>> the function so we expect that post open_device has a fresh from reset
>>> fully running device state.
>> running also mean that the device doesn't have a clue on its internal state
>> ? or running means unfreezed and unquiesced ?
> The device just got FLR'd and it should be in a clean state and
> operating. Think the VM is booting for the first time.

During the resume phase in the dst, the VM is paused and not booting. 
Migration SW is waiting to get memory and state from SRC. The device 
will start from the exact point that was in the src.

it's exactly "000b => Device Stopped, not saving or resuming"

>
>>>>> driver will see RESUMING toggle off so it will trigger a
>>>>> de-serialization
>>>> You mean stop serialization ?
>>> No, I mean it will take all the migration data that has been uploaded
>>> through the migration region and de-serialize it into active device
>>> state.
>> you should feed the device way before that.
> I don't know what this means, when the resuming bit is set the
> migration data buffer is wiped and userspace should beging loading
> it. When the resuming bit is cleared whatever is in the migration
> buffer is deserialized into the current device internal state.

Well, this is your design for the driver implementation. Nobody is 
preventing other drivers to start deserializing device state into the 
device during RESUMING bit on.

Or is this a must ?

>
> It is the opposite of saving. When the saving bit is set the current
> device state is serialized into the migration buffer and userspace and
> reads it out.

This is not new.

>> 1. you initialize at  _RUNNING bit == 001b. No problem.
>>
>> 2. state stream arrives, migration SW raise _RESUMING bit. should it be 101b
>> or 100b ? for now it's 100b. But according to your statement is should be
>> 101b (invalid today) since device state can change. right ?
> Running means the device state chanages independently, the controlled
> change of the device state via deserializing the migration buffer is
> different. Both running and saving commands need running to be zero.
>
> ie commands that are marked invalid in the uapi comment are rejected
> at the start - and that is probably the core helper we should provide.
>
>> 3. Then you should indicate that all the state was serialized to the device
>> (actually to all the pci devices). 100b mean RESUMING and not RUNNING so
>> maybe this can say RESUMED and state can't change now ?
> State is not loaded into the device until the resuming bit is
> cleared. There is no RESUMED state until we incorporate Artem's
> proposal for an additional bit eg 1001b - running with DMA master
> disabled.

So if we moved from 100b to 010b somehow, one should deserialized its 
buffer to the device, and then serialize it to migration region again ?

I guess its doable since the device is freeze and quiesced. But moving 
from 100b to 011b is not possible, right ?

>
> Jason