linux-kernel - Re: [PATCH v2] ata: libahci_platform: support non-consecutive port numbers

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: <bcfa145c3227b13e7b9d8bb3b0f92c678464cfdc.camel@gmail.com>
Date: Wed, 05 Feb 2025 19:03:47 +0100
From: Klaus Kudielka <klaus.kudielka@...il.com>
To: Josua Mayer <josua@...id-run.com>, Damien Le Moal <dlemoal@...nel.org>, 
 Niklas Cassel <cassel@...nel.org>, Hans de Goede <hdegoede@...hat.com>
Cc: Jon Nettleton <jon@...id-run.com>, Mikhail Anikin	
 <mikhail.anikin@...id-run.com>, Yazan Shhady <yazan.shhady@...id-run.com>, 
 Rabeeh Khoury <rabeeh@...id-run.com>, linux-ide@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ata: libahci_platform: support non-consecutive port
 numbers

On Wed, 2025-01-01 at 13:13 +0100, Josua Mayer wrote:
> So far ahci_platform relied on number of child nodes in firmware to
> allocate arrays and expected port numbers to start from 0 without holes.
> This number of ports is then set in private structure for use when
> configuring phys and regulators.
> 
> Some platforms may not use every port of an ahci controller.
> E.g. SolidRUN CN9130 Clearfog uses only port 1 but not port 0, leading
> to the following errors during boot:
> [    1.719476] ahci f2540000.sata: invalid port number 1
> [    1.724562] ahci f2540000.sata: No port enabled
> 
> Update all accessesors of ahci_host_priv phys and target_pwrs arrays to
> support holes. Access is gated by hpriv->mask_port_map which has a bit
> set for each enabled port.
> 
> Update ahci_platform_get_resources to ignore holes in the port numbers
> and enable ports defined in firmware by their reg property only.
> 
> When firmware does not define children it is assumed that there is
> exactly one port, using index 0.
> 

[...]

> @@ -625,6 +651,8 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
>  		 * If no sub-node was found, keep this for device tree
>  		 * compatibility
>  		 */
> +		hpriv->mask_port_map |= BIT(0);
> +
>  		rc = ahci_platform_get_phy(hpriv, 0, dev, dev->of_node);
>  		if (rc)
>  			goto err_out;
> 

This very last hunk (when firmware does not define children...) causes some change in behaviour on my Turris Omnia
(Armada 385):

6.13.0 bootlog
==============

Feb 05 18:30:45 xxxx kernel: ahci-mvebu f10a8000.sata: AHCI vers 0001.0000, 32 command slots, 6 Gbps, platform mode
Feb 05 18:30:45 xxxx kernel: ahci-mvebu f10a8000.sata: 2/2 ports implemented (port mask 0x3)
Feb 05 18:30:45 xxxx kernel: ahci-mvebu f10a8000.sata: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 
Feb 05 18:30:45 xxxx kernel: scsi host0: ahci-mvebu
Feb 05 18:30:45 xxxx kernel: scsi host1: ahci-mvebu
Feb 05 18:30:45 xxxx kernel: ata1: SATA max UDMA/133 mmio [mem 0xf10a8000-0xf10a9fff] port 0x100 irq 40 lpm-pol 0
Feb 05 18:30:45 xxxx kernel: ata2: SATA max UDMA/133 mmio [mem 0xf10a8000-0xf10a9fff] port 0x180 irq 40 lpm-pol 0

Previously, both detected ports were automatically used, and no warning was emitted.
(hpriv->mask_port_map was 0x0)


6.14.0-rc1 bootlog
==================

Feb 05 18:36:40 xxxx kernel: ahci-mvebu f10a8000.sata: masking port_map 0x3 -> 0x1
Feb 05 18:36:40 xxxx kernel: ahci-mvebu f10a8000.sata: AHCI vers 0001.0000, 32 command slots, 6 Gbps, platform mode
Feb 05 18:36:40 xxxx kernel: ahci-mvebu f10a8000.sata: 1/2 ports implemented (port mask 0x1)
Feb 05 18:36:40 xxxx kernel: ahci-mvebu f10a8000.sata: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 
Feb 05 18:36:40 xxxx kernel: scsi host0: ahci-mvebu
Feb 05 18:36:40 xxxx kernel: scsi host1: ahci-mvebu
Feb 05 18:36:40 xxxx kernel: ata1: SATA max UDMA/133 mmio [mem 0xf10a8000-0xf10a9fff] port 0x100 irq 40 lpm-pol 0
Feb 05 18:36:40 xxxx kernel: ata2: DUMMY

Now, hpriv->mask_port_map is forced to 0x1, resulting in a kernel warning, and no more ata2 available.
In my particular case it is not a big deal, since nothing is connected to the 2nd port, and I can live with this obscure warning.

But that might not be the case for everybody?

Regards, Klaus