linux-kernel - Implementation details of PCI Managed (devres) Functions

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-next>] [day] [month] [year] [list]

Message-ID: <84be1049e41283cf8a110267646320af9ffe59fe.camel@redhat.com>
Date:   Tue, 07 Nov 2023 20:38:18 +0100
From:   Philipp Stanner <pstanner@...hat.com>
To:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Tejun Heo <htejun@...il.com>, dakr@...hat.com,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ben Dooks <ben.dooks@...ethink.co.uk>,
        Philipp Stanner <pstanner@...hat.com>, jeff@...zik.org,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Implementation details of PCI Managed (devres) Functions

Hi all,

I'm currently working on porting more drivers in DRM to managed pci-
functions. During this process I discovered something that might be
called an inconsistency in the implementation.

First, there would be the pcim_ functions being scattered across
several files. Some are implemented in drivers/pci/pci.c, others in
lib/devres.c, where they are guarded by #ifdef CONFIG_PCI
– this originates from an old cleanup, done in
5ea8176994003483a18c8fed580901e2125f8a83

Additionally, there is lib/pci_iomap.c, which contains the non-managed
pci_iomap() functions.

At first and second glance it's not obvious to me why these pci-
functions are scattered. Hints?

Second, it seems there are two competing philosophies behind managed
resource reservations. Some pci_ functions have pcim_ counterparts,
such as pci_iomap() <-> pcim_iomap(). So the API-user might expect that
relevant pci_ functions that do not have a managed counterpart do so
because no one bothered implementing them so far.

However, it turns out that for pci_request_region(), there is no
counterpart because a different mechanism / semantic was used to make
the function _sometimes_ managed:

   1. If you use pcim_enable_device(), the member is_managed in struct
      pci_dev is set to 1.
   2. This value is then evaluated in pci_request_region()'s call to
      find_pci_dr()

Effectively, this makes pci_request_region() sometimes managed.
Why has it been implemented that way and not as a separate function –
like, e.g., pcim_iomap()?

That's where an inconsistency lies. For things like iomappings there
are separate managed functions, for the region-request there's a
universal function doing managed or unmanaged, respectively.

Furthermore, look at pcim_iomap_regions() – that function uses a mix
between the obviously managed function pcim_iomap() and
pci_request_region(), which appears unmanaged judging by the name, but,
nevertheless, is (sometimes) managed below the surface.
Consequently, pcim_iomap_regions() could even be a little buggy: When
someone uses pci_enable_device() + pcim_iomap_regions(), wouldn't that
leak the resource regions?

The change to pci_request_region() hasn't grown historically but was
implemented that way in one run with the first set of managed functions
in commit 9ac7849e35f70. So this implies it has been implemented that
way on purpose.

What was that purpose?

Would be great if someone can give some hints :)

Regards,
P.