On Wed, Mar 13, 2024 at 11:20:18PM +0100, Hans de Goede wrote: > Hi, > > On 3/13/24 10:52 PM, Conrad Kostecki wrote: > > Hello folks, > > > > Am 13.03.2024 22:21:51, "Hans de Goede" <hdegoede@xxxxxxxxxx> schrieb: > > > >> So on this 4 port controller we actually get 4 + 16 ports. > >> which isuggests that port multipliers are handled transparently > >> inside the controller and that ata15-ata18 are likely the ports > >> on a 1:4 multiplier on ata7, ata19-ata22 are the ports on a > >> 1:4 multiplier on ata8, etc. > >> > > do you have any idea, if we could make somehow an non-default option to disable such ports? > > > > The initial problem will now persist again. As for example a 16 port x4 pcie card (4x ASM1064, each connted to one pcie lane, so only providing 16 physical real ports, no SATA PMP) will take about 3-4 minutes to slow down boot, as 128 ports are being detected and waiting to timeout to continue further. > > I think you can already do this, see: > > https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html > > and look for libata.force > > So lets say the virtual ports for the transparent PMP support are ata8-ata31, then > you could do: > > libata.force=8:disable,9:disable,10:disable,...,31:disable Too long. libata.force=8-31:disable is better, but unsupported. > Although I have to admit that that is very verbose and it relies on > the probe order to be constant which is not guaranteed. > > So being able to specify some sort of port-mask override to disable > the "virtual" ports would be better. > > I guess you could even add a disable_transparent_pmp_ports bool module > parameter to the ahci.c code which defaults to false and then simple > change: > > if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA) { > switch (pdev->device) { > case 0x1166: > dev_info(&pdev->dev, "ASM1166 has only six ports\n"); > hpriv->saved_port_map = 0x3f; > break; > > to: > > if (disable_transparent_pmp_ports && pdev->vendor == PCI_VENDOR_ID_ASMEDIA) { > switch (pdev->device) { > case 0x1166: > dev_info(&pdev->dev, "Limiting ASM1166 to its six physical ports\n"); > hpriv->saved_port_map = 0x3f; > break; > > And then you can activate the behavior with ahci.disable_transparent_pmp_ports=1 > on the kernel cmdline. Non-standard vendor extensions should be enabled with user attention, so - invert logic and param name: if (!enable_transparent_pmp_ports && pdev->vendor == PCI_VENDOR_ID_ASMEDIA) { switch (pdev->device) { case 0x1166: dev_info(&pdev->dev, "Limiting ASM1166 to its six physical ports\n"); hpriv->saved_port_map = 0x3f; break; > Although some generic mechanism to set an override for the port-mask on a > per controller basis would perhaps be better. > > Niklas, do you have any remarks / ideas ? > > >> Conrad as the author of the patch adding the original port limiting for > >> the ASM1166, can you submit a patch upstream to drop the port-limiting for > >> both the ASM1164 and ASM1166 for now, with the following tags added to this > >> patch: > >> > >> Fixes: 0077a504e1a4 ("ahci: asm1166: correct count of reported ports") > >> Fixes: 9815e3961754 ("ahci: asm1064: correct count of reported ports") > >> Cc: stable@xxxxxxxxxxxxxxx > >> > > I've send the patch
