[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251018224401.GE1034710.vipinsh@google.com>
Date: Sat, 18 Oct 2025 15:44:01 -0700
From: Vipin Sharma <vipinsh@...gle.com>
To: Lukas Wunner <lukas@...ner.de>
Cc: bhelgaas@...gle.com, alex.williamson@...hat.com,
pasha.tatashin@...een.com, dmatlack@...gle.com, jgg@...pe.ca,
graf@...zon.com, pratyush@...nel.org, gregkh@...uxfoundation.org,
chrisl@...nel.org, rppt@...nel.org, skhawaja@...gle.com,
parav@...dia.com, saeedm@...dia.com, kevin.tian@...el.com,
jrhilke@...gle.com, david@...hat.com, jgowans@...zon.com,
dwmw2@...radead.org, epetron@...zon.de, junaids@...gle.com,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
kvm@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [RFC PATCH 16/21] vfio/pci: Save and restore the PCI state of
the VFIO device
On 2025-10-18 09:25:30, Lukas Wunner wrote:
> On Fri, Oct 17, 2025 at 05:07:08PM -0700, Vipin Sharma wrote:
> > Save and restore the PCI state of the VFIO device which in the normal
> > flow is recorded by VFIO when the device FD is opened for the first time
> > and then reapplied to PCI device when the last opened device FD is
> > closed.
> >
> > Introduce "_ser" version of the struct pci_saved_state{} and struct
> > pci_cap_saved_data{} to serialized saved PCI state for liveupdate. Store
> > PCI state in VFIO in a separate folio as the size is indeterministic at
> > build time to reserve space in struct vfio_pci_core_device_ser{}.
>
> Unfortunately this commit message is of the type "summarize the code
> changes without explaining the reason for these changes".
>
> Comparing the pci_saved_state_ser and pci_cap_saved_data_ser structs
> which you're introducing here with the existing pci_saved_state and
> pci_cap_saved_data structs, the only difference seems to be that
> you're adding __packed to your new structs. Is that all? Is that
> the only reason why these structs need to be duplicated? Maybe
> it would make more sense to add __packed to the existing structs,
> though the gain seems minimal.
>
It allows (in future) to build more validation and compatibility between
layout changes of struct across kernel version. We can add more fields
in the *_ser version which can act as metadata to support in
deserialization.
I do agree in the current form (with the assumption of no layout
changes) we can get away with using the existing structs. I also think
this should be taken care by PCI series instead of VFIO series.
Lets see what others also think, I am open to not adding these *_ser
structs if we should wait for a proper support for struct serialization
and work under assumption that these won't change.
Powered by blists - more mailing lists