linux-kernel - Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align

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: <mhng-730176d6-cea5-4f58-99fa-63f6f32df543@palmer-si-x1c4>
Date:   Fri, 23 Jun 2017 19:01:51 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     Arnd Bergmann <arnd@...db.de>
CC:     linux-pci@...r.kernel.org
Subject:     Re: [PATCH 06/17] pci: Add generic pcibios_{fixup_bus,align_resource}

On Thu, 08 Jun 2017 01:35:29 PDT (-0700), Arnd Bergmann wrote:
> On Thu, Jun 8, 2017 at 10:12 AM, Christoph Hellwig <hch@...radead.org> wrote:
>> On Wed, Jun 07, 2017 at 09:19:49AM +0200, Geert Uytterhoeven wrote:
>>> CC pci folks
>>
>> Ok, replying with pci folks in Cc then :)
>>
>> Weak symbols have (rightly) gotten a bad reputation, so maybe
>> we should approach this without them.
>
> Agreed, I would almost never recommend using a __weak symbol,
> but they are already widely used in the PCI subsystem, so I suggested
> using them here for consistency.
>
> We have a struct pci_host_bridge now, and we should eventually
> move most of the 38 pci specific weak  per-architecture symbols
> into per-host driver callbacks, but that I think that would be too much
> to ask for when adding an architecture port.

I agree :)

>> It seems we have a large number of emptry pcibios_fixup_bus calls
>> alreayd, so I think we should simply have the architectures
>> that do define it define a Kconfig or header symbol and not call
>> it at all otherwise.
>
> I would argue that most of them should not be per-architecture
> in the first place, the current state is mostly an artifact of the
> times when each architecture had just one PCI implementation.
>
> The ones that have multiple implementations (arm, powerpc, ...)
> tend to actually override the weak functions with their own
> per-host multiplexers again.
>
>> For the ones that exist as lot just seem to call pci_read_bridge_bases
>> and/or pcibios_fixup_device_resources in one form or another,
>> and I wonder why we even need the arch indirection for that.
>>
>> Similarly for pcibios_align_resource: a lot of architetures seem
>> to have a noop, and the once that don't mostly seem copy and
>> paste code, so we should again have a symbol for architectures
>> to opt into it, and we probably should have a generic helper
>> for the VGA window mirroring code instead of duplicating it multiple
>> times.
>
> I now remember that we already have a host_bridge->align_resource
> callback pointer, so the generic function should definitely try
> to use that.
>
> We could just use the version from arch/mips/pci/pci-generic.c
> and remove that in the process. I'm not sure about why mips calls
> pci_read_bridge_bases() in pcibios_fixup_bus() though, or whether
> this make sense to put in the generic version.

I'm splitting this off into another patch set and sending it to a bunch of PCI
people.