linux-kernel - Re: [RFC PATCH 15/21] PCI: Make PCI saved state and capability structs public

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: <20251020234934.GB648579.vipinsh@google.com>
Date: Mon, 20 Oct 2025 16:49:34 -0700
From: Vipin Sharma <vipinsh@...gle.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Lukas Wunner <lukas@...ner.de>, bhelgaas@...gle.com,
	alex.williamson@...hat.com, pasha.tatashin@...een.com,
	dmatlack@...gle.com, 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 15/21] PCI: Make PCI saved state and capability
 structs public

On 2025-10-18 20:11:26, Jason Gunthorpe wrote:
> On Sat, Oct 18, 2025 at 03:36:20PM -0700, Vipin Sharma wrote:
> 
> > Having __packed in my version of struct, I can build validation like
> > hardcoded offset of members. I can add version number (not added in this
> > series) for checking compatbility in the struct for serialization and
> > deserialization. Overall, it is providing some freedom to how to pass
> > data to next kernel without changing or modifying the PCI state
> > structs.
> 
> I keep saying this, and this series really strongly shows why, we need
> to have a dedicated header directroy for LUO "ABI" structs. Putting
> this random struct in some random header and then declaring it is part
> of the luo ABI is really bad.

Now that we have PCI, IOMMU, and VFIO series out. What should be the
strategy for LUO "ABI" structs? I would like some more clarity on how
you are visioning this.

Are you suggesting that each subsystem create a separate header file for
their serialization structs or we can have one common header file used
by all subsystems as dumping ground for their structs?

> 
> All the information in the abi headers needs to have detailed comments
> explaining what it is and so on so people can evaluate if it is
> suitable or not.

I agree. I should have at least written comments in my *_ser structs on
why that particular field is there and what it is enabling. I will do
that in next version.

> 
> But, it is also not clear why pci serialization structs should leak
> out of the PCI layer.
> 

When PCI device is opened for the first time, VFIO driver asks for this state
from PCI and saves it in struct vfio_pci_core_device{.pci_saved_state}
field. It loads this value back to pci device after last device FD is
closed. 

PCI layer will not have access to this value as it can be changed once
VFIO has start using this device. Therefore, I thought this should be
saved.

May be serialization and deserialization logic can be put in PCI and
that way it can stay in PCI?

> The design of luo was to allow each layer to contribute its own
> tags/etc to the serialization so there is no reason to have vfio
> piggback on pci structs or something.
> 
> Jason