[PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs

Marc Zyngier posted 3 patches 3 months, 2 weeks ago
drivers/pci/controller/pci-host-common.c |  4 +-
drivers/pci/controller/pcie-apple.c      | 53 ++++++++++++++++++++++--
drivers/pci/ecam.c                       |  2 -
3 files changed, 51 insertions(+), 8 deletions(-)
[PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs
Posted by Marc Zyngier 3 months, 2 weeks ago
Geert reports that some drivers do rely on the device driver_data
field containing a pointer to the bridge structure at the point of
initialising the root port, while this has been recently changed to
contain some other data for the benefit of the Apple PCIe driver.

This small series builds on top of Geert previously posted (and
included as a prefix for reference) fix for the Microchip driver,
which breaks the Apple driver. This is basically swapping a regression
for another, which isn't a massive deal at this stage, as the
follow-up patch fixes things for the Apple driver by adding extra
tracking.

Finally, we can revert a one-liner that glued the whole thing
together, and that isn't needed anymore.

All of this is candidate for 6.16, as we have regressed the Microchip
driver in -rc1, and that fixing it breaks the Apple driver.

Geert Uytterhoeven (1):
  PCI: host-generic: Set driver_data before calling gen_pci_init()

Marc Zyngier (2):
  PCI: apple: Add tracking of probed root ports
  Revert "PCI: ecam: Allow cfg->priv to be pre-populated from the root
    port device"

 drivers/pci/controller/pci-host-common.c |  4 +-
 drivers/pci/controller/pcie-apple.c      | 53 ++++++++++++++++++++++--
 drivers/pci/ecam.c                       |  2 -
 3 files changed, 51 insertions(+), 8 deletions(-)

-- 
2.39.2
Re: [PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs
Posted by Bjorn Helgaas 3 months, 1 week ago
On Wed, Jun 25, 2025 at 12:18:03PM +0100, Marc Zyngier wrote:
> Geert reports that some drivers do rely on the device driver_data
> field containing a pointer to the bridge structure at the point of
> initialising the root port, while this has been recently changed to
> contain some other data for the benefit of the Apple PCIe driver.
> 
> This small series builds on top of Geert previously posted (and
> included as a prefix for reference) fix for the Microchip driver,
> which breaks the Apple driver. This is basically swapping a regression
> for another, which isn't a massive deal at this stage, as the
> follow-up patch fixes things for the Apple driver by adding extra
> tracking.

Is there a bisection hole between patches 1 and 2?

  1: PCI: host-generic: Set driver_data before calling gen_pci_init()
  2: PCI: apple: Add tracking of probed root ports

If so, would it be practical to avoid the hole by reordering those
patches?

> Finally, we can revert a one-liner that glued the whole thing
> together, and that isn't needed anymore.
> 
> All of this is candidate for 6.16, as we have regressed the Microchip
> driver in -rc1, and that fixing it breaks the Apple driver.
> 
> Geert Uytterhoeven (1):
>   PCI: host-generic: Set driver_data before calling gen_pci_init()
> 
> Marc Zyngier (2):
>   PCI: apple: Add tracking of probed root ports
>   Revert "PCI: ecam: Allow cfg->priv to be pre-populated from the root
>     port device"
> 
>  drivers/pci/controller/pci-host-common.c |  4 +-
>  drivers/pci/controller/pcie-apple.c      | 53 ++++++++++++++++++++++--
>  drivers/pci/ecam.c                       |  2 -
>  3 files changed, 51 insertions(+), 8 deletions(-)
> 
> -- 
> 2.39.2
>
Re: [PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs
Posted by Marc Zyngier 3 months, 1 week ago
On Mon, 30 Jun 2025 18:06:01 +0100,
Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> On Wed, Jun 25, 2025 at 12:18:03PM +0100, Marc Zyngier wrote:
> > Geert reports that some drivers do rely on the device driver_data
> > field containing a pointer to the bridge structure at the point of
> > initialising the root port, while this has been recently changed to
> > contain some other data for the benefit of the Apple PCIe driver.
> > 
> > This small series builds on top of Geert previously posted (and
> > included as a prefix for reference) fix for the Microchip driver,
> > which breaks the Apple driver. This is basically swapping a regression
> > for another, which isn't a massive deal at this stage, as the
> > follow-up patch fixes things for the Apple driver by adding extra
> > tracking.
> 
> Is there a bisection hole between patches 1 and 2?
> 
>   1: PCI: host-generic: Set driver_data before calling gen_pci_init()
>   2: PCI: apple: Add tracking of probed root ports
> 
> If so, would it be practical to avoid the hole by reordering those
> patches?

Sure, but you said you already had queued patch #1, and what is in
-rc1 already breaks Geert's box. So no matter the order, we break
something at some point.

If you want to only break one thing, then yes, swapping these two
patches is the correct thing to do.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs
Posted by Bjorn Helgaas 3 months, 1 week ago
On Mon, Jun 30, 2025 at 06:23:00PM +0100, Marc Zyngier wrote:
> On Mon, 30 Jun 2025 18:06:01 +0100,
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> > 
> > On Wed, Jun 25, 2025 at 12:18:03PM +0100, Marc Zyngier wrote:
> > > Geert reports that some drivers do rely on the device driver_data
> > > field containing a pointer to the bridge structure at the point of
> > > initialising the root port, while this has been recently changed to
> > > contain some other data for the benefit of the Apple PCIe driver.
> > > 
> > > This small series builds on top of Geert previously posted (and
> > > included as a prefix for reference) fix for the Microchip driver,
> > > which breaks the Apple driver. This is basically swapping a regression
> > > for another, which isn't a massive deal at this stage, as the
> > > follow-up patch fixes things for the Apple driver by adding extra
> > > tracking.
> > 
> > Is there a bisection hole between patches 1 and 2?
> > 
> >   1: PCI: host-generic: Set driver_data before calling gen_pci_init()
> >   2: PCI: apple: Add tracking of probed root ports
> > 
> > If so, would it be practical to avoid the hole by reordering those
> > patches?
> 
> Sure, but you said you already had queued patch #1, and what is in
> -rc1 already breaks Geert's box. So no matter the order, we break
> something at some point.

I did, but when I saw your problem report and subsequent updates, I
put Geert's patch on hold.

> If you want to only break one thing, then yes, swapping these two
> patches is the correct thing to do.

I swapped them and put them back on pci/for-linus for v6.16:

  https://git.kernel.org/cgit/linux/kernel/git/pci/pci.git/log/?h=for-linus&id=ba74278c638d
Re: [PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs
Posted by Marc Zyngier 3 months, 1 week ago
On Mon, 30 Jun 2025 18:34:15 +0100,
Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> On Mon, Jun 30, 2025 at 06:23:00PM +0100, Marc Zyngier wrote:
> > On Mon, 30 Jun 2025 18:06:01 +0100,
> > Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > 
> > > On Wed, Jun 25, 2025 at 12:18:03PM +0100, Marc Zyngier wrote:
> > > > Geert reports that some drivers do rely on the device driver_data
> > > > field containing a pointer to the bridge structure at the point of
> > > > initialising the root port, while this has been recently changed to
> > > > contain some other data for the benefit of the Apple PCIe driver.
> > > > 
> > > > This small series builds on top of Geert previously posted (and
> > > > included as a prefix for reference) fix for the Microchip driver,
> > > > which breaks the Apple driver. This is basically swapping a regression
> > > > for another, which isn't a massive deal at this stage, as the
> > > > follow-up patch fixes things for the Apple driver by adding extra
> > > > tracking.
> > > 
> > > Is there a bisection hole between patches 1 and 2?
> > > 
> > >   1: PCI: host-generic: Set driver_data before calling gen_pci_init()
> > >   2: PCI: apple: Add tracking of probed root ports
> > > 
> > > If so, would it be practical to avoid the hole by reordering those
> > > patches?
> > 
> > Sure, but you said you already had queued patch #1, and what is in
> > -rc1 already breaks Geert's box. So no matter the order, we break
> > something at some point.
> 
> I did, but when I saw your problem report and subsequent updates, I
> put Geert's patch on hold.
> 
> > If you want to only break one thing, then yes, swapping these two
> > patches is the correct thing to do.
> 
> I swapped them and put them back on pci/for-linus for v6.16:
> 
>   https://git.kernel.org/cgit/linux/kernel/git/pci/pci.git/log/?h=for-linus&id=ba74278c638d
> 

LGTM, thanks for picking these up!

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH 0/3] PCI: host-generic: Fix driver_data overwriting bugs
Posted by Geert Uytterhoeven 3 months, 2 weeks ago
Hi Marc,

On Wed, 25 Jun 2025 at 13:18, Marc Zyngier <maz@kernel.org> wrote:
> Geert reports that some drivers do rely on the device driver_data
> field containing a pointer to the bridge structure at the point of
> initialising the root port, while this has been recently changed to
> contain some other data for the benefit of the Apple PCIe driver.
>
> This small series builds on top of Geert previously posted (and
> included as a prefix for reference) fix for the Microchip driver,
> which breaks the Apple driver. This is basically swapping a regression
> for another, which isn't a massive deal at this stage, as the
> follow-up patch fixes things for the Apple driver by adding extra
> tracking.
>
> Finally, we can revert a one-liner that glued the whole thing
> together, and that isn't needed anymore.

Thanks, works fine on Icicle.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds