| Message ID | 20240405125143.1134539-1-dlemoal@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ata: ahci: Add mask_port_map module parameter | expand |
On Fri, Apr 05, 2024 at 09:51:43PM +0900, Damien Le Moal wrote: > Commits 0077a504e1a4 ("ahci: asm1166: correct count of reported ports") > and 9815e3961754 ("ahci: asm1064: correct count of reported ports") > attempted to limit the ports of the ASM1166 and ASM1064 AHCI controllers > to avoid long boot times caused by the fact that these adapters report > a port map larger than the number of physical ports. The excess ports > are "virtual" to hide port multiplier devices and probing these ports > takes time. However, these commits caused a regression for users that do > use PMP devices, as the ATA devices connected to the PMP cannot be > scanned. These commits have thus been reverted by commit 6cd8adc3e18 > ("ahci: asm1064: asm1166: don't limit reported ports") to allow the > discovery of devices connected through a port multiplier. But this > revert re-introduced the long boot times for users that do not use a > port multiplier setup. > > This patch adds the mask_port_map ahci module parameter to allow users > to manually specify port map masks for controllers. In the case of the > ASMedia 1166 and 1064 controllers, users that do not have port > multiplier devices can mask the excess virtual ports exposed by the > controller to speedup port scanning, thus reducing boot time. > > The mask_port_map parameter accepts 2 different formats: > - mask_port_map=<mask> > This applies the same mask to all AHCI controllers > present in the system. This format is convenient for small systems > that have only a single AHCI controller. > - mask_port_map=<pci_dev>=<mask>,<pci_dev>=mask,... > This applies the specified masks only to the PCI device listed. The > <pci_dev> field is a regular PCI device ID (domain:bus:dev.func). > This ID can be seen following "ahci" in the kernel messages. E.g. > for "ahci 0000:01:00.0: 2/2 ports implemented (port mask 0x3)", the > <pci_dev> field is "0000:01:00.0". > > When used, the kerenl messages indicate that a port map mask was forced. Nit: s/kerenl/kernel/ > E.g.: without a mask: > modrpobe ahci Nit: s/modrpobe/modprobe/ > dmesg | grep ahci > ... > ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode > ahci 0000:00:17.0: (0000:00:17.0) 8/8 ports implemented (port mask 0xff) > > With a mask: > > modrpobe ahci mask_port_map=0000:00:17.0=0x1 > dmesg | grep ahci > ... > ahci 0000:00:17.0: masking port_map 0xff -> 0x1 > ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode > ahci 0000:00:17.0: (0000:00:17.0) 1/8 ports implemented (port mask 0x1) > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- With nits fixed: Reviewed-by: Niklas Cassel <cassel@kernel.org>
On Fri, Apr 05, 2024 at 09:51:43PM +0900, Damien Le Moal wrote: > Commits 0077a504e1a4 ("ahci: asm1166: correct count of reported ports") > and 9815e3961754 ("ahci: asm1064: correct count of reported ports") > attempted to limit the ports of the ASM1166 and ASM1064 AHCI controllers > to avoid long boot times caused by the fact that these adapters report > a port map larger than the number of physical ports. The excess ports > are "virtual" to hide port multiplier devices and probing these ports > takes time. However, these commits caused a regression for users that do > use PMP devices, as the ATA devices connected to the PMP cannot be > scanned. These commits have thus been reverted by commit 6cd8adc3e18 > ("ahci: asm1064: asm1166: don't limit reported ports") to allow the > discovery of devices connected through a port multiplier. But this > revert re-introduced the long boot times for users that do not use a > port multiplier setup. > > This patch adds the mask_port_map ahci module parameter to allow users > to manually specify port map masks for controllers. In the case of the > ASMedia 1166 and 1064 controllers, users that do not have port > multiplier devices can mask the excess virtual ports exposed by the > controller to speedup port scanning, thus reducing boot time. > > The mask_port_map parameter accepts 2 different formats: > - mask_port_map=<mask> > This applies the same mask to all AHCI controllers > present in the system. This format is convenient for small systems > that have only a single AHCI controller. > - mask_port_map=<pci_dev>=<mask>,<pci_dev>=mask,... > This applies the specified masks only to the PCI device listed. The > <pci_dev> field is a regular PCI device ID (domain:bus:dev.func). > This ID can be seen following "ahci" in the kernel messages. E.g. > for "ahci 0000:01:00.0: 2/2 ports implemented (port mask 0x3)", the > <pci_dev> field is "0000:01:00.0". > > When used, the kerenl messages indicate that a port map mask was forced. > E.g.: without a mask: > modrpobe ahci > dmesg | grep ahci > ... > ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode > ahci 0000:00:17.0: (0000:00:17.0) 8/8 ports implemented (port mask 0xff) > > With a mask: > Perhaps drop this empty line. > modrpobe ahci mask_port_map=0000:00:17.0=0x1 s/modrpobe/modprobe/ here as well. (My R-b from previous email still applies of course.) > dmesg | grep ahci > ... > ahci 0000:00:17.0: masking port_map 0xff -> 0x1 > ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode > ahci 0000:00:17.0: (0000:00:17.0) 1/8 ports implemented (port mask 0x1) > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > ---
Hi Damien,
Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb:
><PATCH v2>
i did run a test on my hardware.
It seems to work and adjusting the port_map. But I noticed a difference,
that those virtual hostXY ports are not marked as DUMMY.
With the previous patch, only six ports reported "ahci" and rest
"DUMMY".
I am not sure, if that should also not happen with your patch?
Conrad
[ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f
[ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan
disabled
[ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6
Gbps 0x3f impl SATA mode
[ 13.397111] ahci 0000:09:00.0: flags: 64bit ncq sntf stag pm led only
pio sxs deso sadm sds apst
[ 13.593757] scsi host9: ahci
[ 13.597362] scsi host10: ahci
[ 13.600949] scsi host11: ahci
[ 13.604548] scsi host12: ahci
[ 13.612459] scsi host13: ahci
[ 13.616027] scsi host14: ahci
[ 13.616437] scsi host15: ahci
[ 13.616745] scsi host16: ahci
[ 13.617039] scsi host17: ahci
[ 13.617415] scsi host18: ahci
[ 13.617723] scsi host19: ahci
[ 13.637158] scsi host20: ahci
[ 13.640666] scsi host21: ahci
[ 13.651760] scsi host22: ahci
[ 13.652311] scsi host23: ahci
[ 13.652850] scsi host24: ahci
[ 13.656374] scsi host25: ahci
[ 13.664120] scsi host26: ahci
[ 13.664570] scsi host27: ahci
[ 13.686567] scsi host28: ahci
[ 13.690179] scsi host29: ahci
[ 13.697603] scsi host30: ahci
[ 13.698083] scsi host31: ahci
[ 13.698518] scsi host32: ahci
[ 13.701855] scsi host33: ahci
[ 13.702323] scsi host34: ahci
[ 13.702745] scsi host35: ahci
[ 13.721520] scsi host36: ahci
[ 13.725157] scsi host37: ahci
[ 13.736948] scsi host38: ahci
[ 13.737383] scsi host39: ahci
[ 13.748518] scsi host40: ahci
On 4/6/24 07:53, Conrad Kostecki wrote: > Hi Damien, > > Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb: > >> <PATCH v2> > i did run a test on my hardware. > It seems to work and adjusting the port_map. But I noticed a difference, that > those virtual hostXY ports are not marked as DUMMY. > With the previous patch, only six ports reported "ahci" and rest "DUMMY". > I am not sure, if that should also not happen with your patch? Let me check again. > Conrad > [ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f > [ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan disabled > [ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6 Gbps 0x3f > impl SATA mode > [ 13.397111] ahci 0000:09:00.0: flags: 64bit ncq sntf stag pm led only pio > sxs deso sadm sds apst > [ 13.593757] scsi host9: ahci > [ 13.597362] scsi host10: ahci > [ 13.600949] scsi host11: ahci > [ 13.604548] scsi host12: ahci > [ 13.612459] scsi host13: ahci > [ 13.616027] scsi host14: ahci > [ 13.616437] scsi host15: ahci > [ 13.616745] scsi host16: ahci > [ 13.617039] scsi host17: ahci > [ 13.617415] scsi host18: ahci > [ 13.617723] scsi host19: ahci > [ 13.637158] scsi host20: ahci > [ 13.640666] scsi host21: ahci > [ 13.651760] scsi host22: ahci > [ 13.652311] scsi host23: ahci > [ 13.652850] scsi host24: ahci > [ 13.656374] scsi host25: ahci > [ 13.664120] scsi host26: ahci > [ 13.664570] scsi host27: ahci > [ 13.686567] scsi host28: ahci > [ 13.690179] scsi host29: ahci > [ 13.697603] scsi host30: ahci > [ 13.698083] scsi host31: ahci > [ 13.698518] scsi host32: ahci > [ 13.701855] scsi host33: ahci > [ 13.702323] scsi host34: ahci > [ 13.702745] scsi host35: ahci > [ 13.721520] scsi host36: ahci > [ 13.725157] scsi host37: ahci > [ 13.736948] scsi host38: ahci > [ 13.737383] scsi host39: ahci > [ 13.748518] scsi host40: ahci
On 4/6/24 07:53, Conrad Kostecki wrote: > Hi Damien, > > Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb: > >> <PATCH v2> > i did run a test on my hardware. > It seems to work and adjusting the port_map. But I noticed a difference, that > those virtual hostXY ports are not marked as DUMMY. > With the previous patch, only six ports reported "ahci" and rest "DUMMY". > I am not sure, if that should also not happen with your patch? > Conrad > [ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f > [ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan disabled > [ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6 Gbps 0x3f > impl SATA mode > [ 13.397111] ahci 0000:09:00.0: flags: 64bit ncq sntf stag pm led only pio > sxs deso sadm sds apst > [ 13.593757] scsi host9: ahci > [ 13.597362] scsi host10: ahci > [ 13.600949] scsi host11: ahci > [ 13.604548] scsi host12: ahci > [ 13.612459] scsi host13: ahci > [ 13.616027] scsi host14: ahci > [ 13.616437] scsi host15: ahci > [ 13.616745] scsi host16: ahci > [ 13.617039] scsi host17: ahci > [ 13.617415] scsi host18: ahci > [ 13.617723] scsi host19: ahci > [ 13.637158] scsi host20: ahci > [ 13.640666] scsi host21: ahci > [ 13.651760] scsi host22: ahci > [ 13.652311] scsi host23: ahci > [ 13.652850] scsi host24: ahci > [ 13.656374] scsi host25: ahci > [ 13.664120] scsi host26: ahci > [ 13.664570] scsi host27: ahci > [ 13.686567] scsi host28: ahci > [ 13.690179] scsi host29: ahci > [ 13.697603] scsi host30: ahci > [ 13.698083] scsi host31: ahci > [ 13.698518] scsi host32: ahci > [ 13.701855] scsi host33: ahci > [ 13.702323] scsi host34: ahci > [ 13.702745] scsi host35: ahci > [ 13.721520] scsi host36: ahci > [ 13.725157] scsi host37: ahci > [ 13.736948] scsi host38: ahci > [ 13.737383] scsi host39: ahci > [ 13.748518] scsi host40: ahci These messages are normal. ata port stucture which leads to a scsi host are still created for dummy ports. So seeing all ports here as scsi hosts is normal. However, after these messages, you should see a "ataX: DUMMY" message. Example with a qemu VM with a 6-ports AHCI controller: Using the ahci driver without any port map mask, I get: [ 318.739513] ahci 0000:00:1f.2: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode [ 318.741283] ahci 0000:00:1f.2: 6/6 ports implemented (port mask 0x3f) [ 318.742619] ahci 0000:00:1f.2: flags: 64bit ncq only [ 318.759156] scsi host0: ahci [ 318.764098] scsi host1: ahci [ 318.788722] scsi host2: ahci [ 318.793502] scsi host3: ahci [ 318.797792] scsi host4: ahci [ 318.801843] scsi host5: ahci [ 318.804550] ata13: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16100 irq 40 lpm-pol 3 [ 318.805753] ata14: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16180 irq 40 lpm-pol 3 [ 318.807045] ata15: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16200 irq 40 lpm-pol 3 [ 318.808322] ata16: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16280 irq 40 lpm-pol 3 [ 318.809595] ata17: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16300 irq 40 lpm-pol 3 [ 318.810829] ata18: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16380 irq 40 lpm-pol 3 ... On the same VM, if I rmmod ahci and modprobe it with mask_port_map=0x1 (enabling only the first port), I get: [ 362.257874] ahci 0000:00:1f.2: masking port_map 0x3f -> 0x1 [ 362.260518] ahci 0000:00:1f.2: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode [ 362.262477] ahci 0000:00:1f.2: 1/6 ports implemented (port mask 0x1) [ 362.263917] ahci 0000:00:1f.2: flags: 64bit ncq only [ 362.281765] scsi host0: ahci [ 362.286353] scsi host1: ahci [ 362.290785] scsi host2: ahci [ 362.301082] scsi host3: ahci [ 362.324192] scsi host4: ahci [ 362.329560] scsi host5: ahci [ 362.332692] ata19: SATA max UDMA/133 abar m4096@0xfea16000 port 0xfea16100 irq 40 lpm-pol 3 [ 362.335041] ata20: DUMMY [ 362.335872] ata21: DUMMY [ 362.336828] ata22: DUMMY [ 362.337856] ata23: DUMMY [ 362.338660] ata24: DUMMY So the last 5 ports are now dummies... Can you confirm that you see this please ? Also, please confirm that boot time is OK and faster with the port map mask.
Hello Damien, please apologize, I wasn't able to answer earlier, but I had some health issues. Am 08.04.2024 04:26:06, "Damien Le Moal" <dlemoal@kernel.org> schrieb: >On 4/6/24 07:53, Conrad Kostecki wrote: >> Hi Damien, >> >> Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb: >> >>> <PATCH v2> >> i did run a test on my hardware. >> It seems to work and adjusting the port_map. But I noticed a difference, that >> those virtual hostXY ports are not marked as DUMMY. >> With the previous patch, only six ports reported "ahci" and rest "DUMMY". >> I am not sure, if that should also not happen with your patch? >> Conrad >> [ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f >> [ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan disabled >> [ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6 Gbps 0x3f >> impl SATA mode >> [ 13.397111] ahci 0000:09:00.0: flags: 64bit ncq sntf stag pm led only pio >> sxs deso sadm sds apst >> [ 13.593757] scsi host9: ahci >> [ 13.597362] scsi host10: ahci >> [ 13.600949] scsi host11: ahci >> [ 13.604548] scsi host12: ahci >> [ 13.612459] scsi host13: ahci >> [ 13.616027] scsi host14: ahci >> [ 13.616437] scsi host15: ahci >> [ 13.616745] scsi host16: ahci >> [ 13.617039] scsi host17: ahci >> [ 13.617415] scsi host18: ahci >> [ 13.617723] scsi host19: ahci >> [ 13.637158] scsi host20: ahci >> [ 13.640666] scsi host21: ahci >> [ 13.651760] scsi host22: ahci >> [ 13.652311] scsi host23: ahci >> [ 13.652850] scsi host24: ahci >> [ 13.656374] scsi host25: ahci >> [ 13.664120] scsi host26: ahci >> [ 13.664570] scsi host27: ahci >> [ 13.686567] scsi host28: ahci >> [ 13.690179] scsi host29: ahci >> [ 13.697603] scsi host30: ahci >> [ 13.698083] scsi host31: ahci >> [ 13.698518] scsi host32: ahci >> [ 13.701855] scsi host33: ahci >> [ 13.702323] scsi host34: ahci >> [ 13.702745] scsi host35: ahci >> [ 13.721520] scsi host36: ahci >> [ 13.725157] scsi host37: ahci >> [ 13.736948] scsi host38: ahci >> [ 13.737383] scsi host39: ahci >> [ 13.748518] scsi host40: ahci > >These messages are normal. ata port stucture which leads to a scsi host are >still created for dummy ports. So seeing all ports here as scsi hosts is >normal. However, after these messages, you should see a "ataX: DUMMY" message. That's what I mean. My asm1166 controller has only six ports. So for the list, most of them should been listed as DUMMY, but they are not. With my previous patch for asm1066, this was the case. >Can you confirm that you see this please ? Also, please confirm that boot time >is OK and faster with the port map mask. No, I am currently not able to confirm. It looks like, that's is still slow for me. Maybe a little bit faster, but I may be wrong. The main difference here is, that none of the asm1066 ports is being listed as DUMMY with your patch. As my "ahci" module is build into the kernel, I added to my kernel arguments: ahci.mask_port_map=0000:09:00.0=0x3f Based on the dmesg output, this should be correct? Cheers Conrad
On 2024/04/14 23:14, Conrad Kostecki wrote: > Hello Damien, > please apologize, I wasn't able to answer earlier, but I had some health > issues. > > > Am 08.04.2024 04:26:06, "Damien Le Moal" <dlemoal@kernel.org> schrieb: > >> On 4/6/24 07:53, Conrad Kostecki wrote: >>> Hi Damien, >>> >>> Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb: >>> >>>> <PATCH v2> >>> i did run a test on my hardware. >>> It seems to work and adjusting the port_map. But I noticed a difference, that >>> those virtual hostXY ports are not marked as DUMMY. >>> With the previous patch, only six ports reported "ahci" and rest "DUMMY". >>> I am not sure, if that should also not happen with your patch? >>> Conrad >>> [ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f >>> [ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan disabled >>> [ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6 Gbps 0x3f >>> impl SATA mode >>> [ 13.397111] ahci 0000:09:00.0: flags: 64bit ncq sntf stag pm led only pio >>> sxs deso sadm sds apst >>> [ 13.593757] scsi host9: ahci >>> [ 13.597362] scsi host10: ahci >>> [ 13.600949] scsi host11: ahci >>> [ 13.604548] scsi host12: ahci >>> [ 13.612459] scsi host13: ahci >>> [ 13.616027] scsi host14: ahci >>> [ 13.616437] scsi host15: ahci >>> [ 13.616745] scsi host16: ahci >>> [ 13.617039] scsi host17: ahci >>> [ 13.617415] scsi host18: ahci >>> [ 13.617723] scsi host19: ahci >>> [ 13.637158] scsi host20: ahci >>> [ 13.640666] scsi host21: ahci >>> [ 13.651760] scsi host22: ahci >>> [ 13.652311] scsi host23: ahci >>> [ 13.652850] scsi host24: ahci >>> [ 13.656374] scsi host25: ahci >>> [ 13.664120] scsi host26: ahci >>> [ 13.664570] scsi host27: ahci >>> [ 13.686567] scsi host28: ahci >>> [ 13.690179] scsi host29: ahci >>> [ 13.697603] scsi host30: ahci >>> [ 13.698083] scsi host31: ahci >>> [ 13.698518] scsi host32: ahci >>> [ 13.701855] scsi host33: ahci >>> [ 13.702323] scsi host34: ahci >>> [ 13.702745] scsi host35: ahci >>> [ 13.721520] scsi host36: ahci >>> [ 13.725157] scsi host37: ahci >>> [ 13.736948] scsi host38: ahci >>> [ 13.737383] scsi host39: ahci >>> [ 13.748518] scsi host40: ahci >> >> These messages are normal. ata port stucture which leads to a scsi host are >> still created for dummy ports. So seeing all ports here as scsi hosts is >> normal. However, after these messages, you should see a "ataX: DUMMY" message. > > That's what I mean. My asm1166 controller has only six ports. So for the > list, most of them should been listed as DUMMY, but they are not. > With my previous patch for asm1066, this was the case. OK. So despite what I thought, it seems that the mask should be applied to saved_port_map to modified that value permanently instead of using the mask to set ->port_map. I do not really understand why that would be needed. It seems that saved_port_map is being reused without the mask applied somewhere, which should not happen. I need to dig further into this. In the meantime, can you try to add this patch: diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 83431aae74d8..830a59f68620 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -546,6 +546,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv) port_map, port_map & hpriv->mask_port_map); port_map &= hpriv->mask_port_map; + hpriv->saved_port_map = port_map; } /* cross check port_map and cap.n_ports */ And test again ? > >> Can you confirm that you see this please ? Also, please confirm that boot time >> is OK and faster with the port map mask. > No, I am currently not able to confirm. It looks like, that's is still > slow for me. Maybe a little bit faster, but I may be wrong. > The main difference here is, that none of the asm1066 ports is being > listed as DUMMY with your patch. > > As my "ahci" module is build into the kernel, I added to my kernel > arguments: > ahci.mask_port_map=0000:09:00.0=0x3f > > Based on the dmesg output, this should be correct? Yes, this is correct and for your adapter with 6 ports, the mask 0x3f is correct as well, assuming that the 6 physical ports are the first ones (ports 0 to 5). > > Cheers > Conrad >
Hello Damien, Am 17.04.2024 01:13:08, "Damien Le Moal" <dlemoal@kernel.org> schrieb: >OK. So despite what I thought, it seems that the mask should be applied to >saved_port_map to modified that value permanently instead of using the mask to >set ->port_map. I do not really understand why that would be needed. It seems >that saved_port_map is being reused without the mask applied somewhere, which >should not happen. I need to dig further into this. > >In the meantime, can you try to add this patch: > >diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c >index 83431aae74d8..830a59f68620 100644 >--- a/drivers/ata/libahci.c >+++ b/drivers/ata/libahci.c >@@ -546,6 +546,7 @@ void ahci_save_initial_config(struct device *dev, struct >ahci_host_priv *hpriv) > port_map, > port_map & hpriv->mask_port_map); > port_map &= hpriv->mask_port_map; >+ hpriv->saved_port_map = port_map; > } > > /* cross check port_map and cap.n_ports */ > >And test again ? I can confirm, this works for me. The non physical ports are now marked again as DUMMY and booting is fast. Cheers Conrad
Hello Conrad, On Fri, Apr 05, 2024 at 10:53:55PM +0000, Conrad Kostecki wrote: > Hi Damien, > > Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb: > > > <PATCH v2> > i did run a test on my hardware. > It seems to work and adjusting the port_map. But I noticed a difference, > that those virtual hostXY ports are not marked as DUMMY. > With the previous patch, only six ports reported "ahci" and rest "DUMMY". > I am not sure, if that should also not happen with your patch? > Conrad > [ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f > [ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan disabled > [ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6 Gbps > 0x3f impl SATA mode This print above suggests that you are testing on a v6.8 based kernel. (The print has been improved in v6.9) I do not understand why things are not working for you. Could you please test with v6.9-rc4 + the attached debug patch. Please make sure that you don't have any other changes on top of v6.9-rc4 other than the debug patch. (mask_port_map is already included in v6.9-rc4.) Here is a how v6.9-rc4 + the attached debug patch looks for me with ahci.mask_port_map=0000:00:03.0=0xf added to the kernel command line. (If you use a /etc/modprobe.d/ahci.conf file instead, I assume that should look something like: options ahci mask_port_map=0000:00:03.0=0xf ) [ 0.538102] ahci 0000:00:03.0: masking port_map 0x3f -> 0xf [ 0.539063] ahci 0000:00:03.0: port 1/6 is implemented (port_map 0xf) [ 0.539933] ahci 0000:00:03.0: port 2/6 is implemented (port_map 0xf) [ 0.540750] ahci 0000:00:03.0: port 3/6 is implemented (port_map 0xf) [ 0.541663] ahci 0000:00:03.0: port 4/6 is implemented (port_map 0xf) [ 0.542990] ahci 0000:00:03.0: port 5/6 not implemented, mark as dummy (port_map 0xf) [ 0.544121] ahci 0000:00:03.0: port 6/6 not implemented, mark as dummy (port_map 0xf) [ 0.545766] ahci 0000:00:03.0: port 1/6 is implemented, calling init (port_map 0xf) [ 0.546718] ahci 0000:00:03.0: port 2/6 is implemented, calling init (port_map 0xf) [ 0.547642] ahci 0000:00:03.0: port 3/6 is implemented, calling init (port_map 0xf) [ 0.548399] ahci 0000:00:03.0: port 4/6 is implemented, calling init (port_map 0xf) [ 0.549418] ahci 0000:00:03.0: port 5/6 is not implemented, skipping init (port_map 0xf) [ 0.550650] ahci 0000:00:03.0: port 6/6 is not implemented, skipping init (port_map 0xf) [ 0.551306] ahci 0000:00:03.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode [ 0.551947] ahci 0000:00:03.0: 4/6 ports implemented (port mask 0xf) [ 0.552444] ahci 0000:00:03.0: flags: 64bit ncq only [ 0.553652] scsi host0: ahci [ 0.554138] scsi host1: ahci [ 0.554535] scsi host2: ahci [ 0.555332] scsi host3: ahci [ 0.555806] scsi host4: ahci [ 0.556212] scsi host5: ahci [ 0.556502] ata1: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1100 irq 43 lpm-pol 3 [ 0.557146] ata2: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1180 irq 43 lpm-pol 3 [ 0.557791] ata3: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1200 irq 43 lpm-pol 3 [ 0.558429] ata4: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1280 irq 43 lpm-pol 3 [ 0.559064] ata5: DUMMY [ 0.559260] ata6: DUMMY Please post your whole log, including both lines prefixed with "scsi" and "ata". As you can see, you should see, with your configuration, 32 "scsi: hostX: ahci" prints, 6 "ataX: SATA max ..." prints, 26 "ataX: DUMMY" prints. If your operating system is using systemd (considering your gentoo address, this is not a given), you could run: $ systemd-analyze both with and without the kernel module option. You should be able to see a difference. Or if you don't have systemd, please just upload the full dmesg with and without the kernel module option, so that we can look at the timestamps. Kind regards, Niklas From 6b5f6be5ade9bfed6765724877ea72524d965fbb Mon Sep 17 00:00:00 2001 From: Niklas Cassel <cassel@kernel.org> Date: Wed, 17 Apr 2024 15:41:24 +0200 Subject: [PATCH] mask_port_map debug prints Signed-off-by: Niklas Cassel <cassel@kernel.org> --- drivers/ata/ahci.c | 8 +++++++- drivers/ata/libahci.c | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 6548f10e61d9..6b54b128ac93 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -2011,8 +2011,14 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) ahci_update_initial_lpm_policy(ap); /* disabled/not-implemented port */ - if (!(hpriv->port_map & (1 << i))) + if (!(hpriv->port_map & (1 << i))) { ap->ops = &ata_dummy_port_ops; + dev_info(host->dev, "port %d/%d not implemented, mark as dummy (port_map %#x)\n", + i+1, host->n_ports, hpriv->port_map); + } else { + dev_info(host->dev, "port %d/%d is implemented (port_map %#x)\n", + i+1, host->n_ports, hpriv->port_map); + } } /* apply workaround for ASUS P5W DH Deluxe mainboard */ diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 83431aae74d8..11d97d4f44ad 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -1303,8 +1303,14 @@ void ahci_init_controller(struct ata_host *host) struct ata_port *ap = host->ports[i]; port_mmio = ahci_port_base(ap); - if (ata_port_is_dummy(ap)) + if (ata_port_is_dummy(ap)) { + dev_info(host->dev, "port %d/%d is not implemented, skipping init (port_map %#x)\n", + i+1, host->n_ports, hpriv->port_map); continue; + } else { + dev_info(host->dev, "port %d/%d is implemented, calling init (port_map %#x)\n", + i+1, host->n_ports, hpriv->port_map); + } ahci_port_init(host->dev, ap, i, mmio, port_mmio); }
Hello Niklas, please forgive me, I wasn't able to answer earlier. I have to roll back here :-) I don't know, what I exactly did wrong, but I did now tested again PATCH v2 and can confirm, it just works as it should. Tested on 6.8.9 and also 6.9-rc6 (with has PATCH v2 applied). My ports are being correct masked and DUMMY is being correctly shown. So all fine and I am happy with the patch as it is now. Sorry for the noise, I am really not sure, what I did wrong during first testing. Conrad Am 17.04.2024 19:21:21, "Niklas Cassel" <cassel@kernel.org> schrieb: >Hello Conrad, > >On Fri, Apr 05, 2024 at 10:53:55PM +0000, Conrad Kostecki wrote: >> Hi Damien, >> >> Am 05.04.2024 14:51:43, "Damien Le Moal" <dlemoal@kernel.org> schrieb: >> >> > <PATCH v2> >> i did run a test on my hardware. >> It seems to work and adjusting the port_map. But I noticed a difference, >> that those virtual hostXY ports are not marked as DUMMY. >> With the previous patch, only six ports reported "ahci" and rest "DUMMY". >> I am not sure, if that should also not happen with your patch? >> Conrad >> [ 13.365573] ahci 0000:09:00.0: masking port_map 0xffffff3f -> 0x3f >> [ 13.376511] ahci 0000:09:00.0: SSS flag set, parallel bus scan disabled >> [ 13.395670] ahci 0000:09:00.0: AHCI 0001.0301 32 slots 32 ports 6 Gbps >> 0x3f impl SATA mode > >This print above suggests that you are testing on a v6.8 based kernel. >(The print has been improved in v6.9) > >I do not understand why things are not working for you. > > >Could you please test with v6.9-rc4 + the attached debug patch. > >Please make sure that you don't have any other changes on top of v6.9-rc4 >other than the debug patch. (mask_port_map is already included in v6.9-rc4.) > > > >Here is a how v6.9-rc4 + the attached debug patch looks for me with >ahci.mask_port_map=0000:00:03.0=0xf >added to the kernel command line. > >(If you use a /etc/modprobe.d/ahci.conf file instead, I assume that should >look something like: >options ahci mask_port_map=0000:00:03.0=0xf >) > > >[ 0.538102] ahci 0000:00:03.0: masking port_map 0x3f -> 0xf >[ 0.539063] ahci 0000:00:03.0: port 1/6 is implemented (port_map 0xf) >[ 0.539933] ahci 0000:00:03.0: port 2/6 is implemented (port_map 0xf) >[ 0.540750] ahci 0000:00:03.0: port 3/6 is implemented (port_map 0xf) >[ 0.541663] ahci 0000:00:03.0: port 4/6 is implemented (port_map 0xf) >[ 0.542990] ahci 0000:00:03.0: port 5/6 not implemented, mark as dummy (port_map 0xf) >[ 0.544121] ahci 0000:00:03.0: port 6/6 not implemented, mark as dummy (port_map 0xf) >[ 0.545766] ahci 0000:00:03.0: port 1/6 is implemented, calling init (port_map 0xf) >[ 0.546718] ahci 0000:00:03.0: port 2/6 is implemented, calling init (port_map 0xf) >[ 0.547642] ahci 0000:00:03.0: port 3/6 is implemented, calling init (port_map 0xf) >[ 0.548399] ahci 0000:00:03.0: port 4/6 is implemented, calling init (port_map 0xf) >[ 0.549418] ahci 0000:00:03.0: port 5/6 is not implemented, skipping init (port_map 0xf) >[ 0.550650] ahci 0000:00:03.0: port 6/6 is not implemented, skipping init (port_map 0xf) >[ 0.551306] ahci 0000:00:03.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode >[ 0.551947] ahci 0000:00:03.0: 4/6 ports implemented (port mask 0xf) >[ 0.552444] ahci 0000:00:03.0: flags: 64bit ncq only >[ 0.553652] scsi host0: ahci >[ 0.554138] scsi host1: ahci >[ 0.554535] scsi host2: ahci >[ 0.555332] scsi host3: ahci >[ 0.555806] scsi host4: ahci >[ 0.556212] scsi host5: ahci >[ 0.556502] ata1: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1100 irq 43 lpm-pol 3 >[ 0.557146] ata2: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1180 irq 43 lpm-pol 3 >[ 0.557791] ata3: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1200 irq 43 lpm-pol 3 >[ 0.558429] ata4: SATA max UDMA/133 abar m4096@0xfebd1000 port 0xfebd1280 irq 43 lpm-pol 3 >[ 0.559064] ata5: DUMMY >[ 0.559260] ata6: DUMMY > >Please post your whole log, including both lines prefixed with "scsi" and >"ata". > >As you can see, you should see, with your configuration, >32 "scsi: hostX: ahci" prints, >6 "ataX: SATA max ..." prints, >26 "ataX: DUMMY" prints. > > >If your operating system is using systemd (considering your gentoo address, >this is not a given), you could run: > >$ systemd-analyze > >both with and without the kernel module option. > >You should be able to see a difference. > >Or if you don't have systemd, please just upload the full dmesg with and >without the kernel module option, so that we can look at the timestamps. > > >Kind regards, >Niklas
On Sun, May 05, 2024 at 07:42:55PM +0000, Conrad Kostecki wrote: > Hello Niklas, > please forgive me, I wasn't able to answer earlier. I have to roll back here > :-) No worries :) > I don't know, what I exactly did wrong, but I did now tested again PATCH v2 > and can confirm, it just works as it should. > Tested on 6.8.9 and also 6.9-rc6 (with has PATCH v2 applied). > > My ports are being correct masked and DUMMY is being correctly shown. > So all fine and I am happy with the patch as it is now. > > Sorry for the noise, I am really not sure, what I did wrong during first > testing. I'm glad that the patch/kernel module param works for you. (Even if we can all agree that we would prefer not needing a kernel module param at all :) ) Kind regards, Niklas
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 562302e2e57c..6548f10e61d9 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -666,6 +666,87 @@ static int mobile_lpm_policy = -1; module_param(mobile_lpm_policy, int, 0644); MODULE_PARM_DESC(mobile_lpm_policy, "Default LPM policy for mobile chipsets"); +static char *ahci_mask_port_map; +module_param_named(mask_port_map, ahci_mask_port_map, charp, 0444); +MODULE_PARM_DESC(mask_port_map, + "32-bits port map masks to ignore controllers ports. " + "Valid values are: " + "\"<mask>\" to apply the same mask to all AHCI controller " + "devices, and \"<pci_dev>=<mask>,<pci_dev>=<mask>,...\" to " + "specify different masks for the controllers specified, " + "where <pci_dev> is the PCI ID of an AHCI controller in the " + "form \"domain:bus:dev.func\""); + +static void ahci_apply_port_map_mask(struct device *dev, + struct ahci_host_priv *hpriv, char *mask_s) +{ + unsigned int mask; + + if (kstrtouint(mask_s, 0, &mask)) { + dev_err(dev, "Invalid port map mask\n"); + return; + } + + hpriv->mask_port_map = mask; +} + +static void ahci_get_port_map_mask(struct device *dev, + struct ahci_host_priv *hpriv) +{ + char *param, *end, *str, *mask_s; + char *name; + + if (!strlen(ahci_mask_port_map)) + return; + + str = kstrdup(ahci_mask_port_map, GFP_KERNEL); + if (!str) + return; + + /* Handle single mask case */ + if (!strchr(str, '=')) { + ahci_apply_port_map_mask(dev, hpriv, str); + goto free; + } + + /* + * Mask list case: parse the parameter to apply the mask only if + * the device name matches. + */ + param = str; + end = param + strlen(param); + while (param && param < end && *param) { + name = param; + param = strchr(name, '='); + if (!param) + break; + + *param = '\0'; + param++; + if (param >= end) + break; + + if (strcmp(dev_name(dev), name) != 0) { + param = strchr(param, ','); + if (param) + param++; + continue; + } + + mask_s = param; + param = strchr(mask_s, ','); + if (param) { + *param = '\0'; + param++; + } + + ahci_apply_port_map_mask(dev, hpriv, mask_s); + } + +free: + kfree(str); +} + static void ahci_pci_save_initial_config(struct pci_dev *pdev, struct ahci_host_priv *hpriv) { @@ -688,6 +769,10 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev, "Disabling your PATA port. Use the boot option 'ahci.marvell_enable=0' to avoid this.\n"); } + /* Handle port map masks passed as module parameter. */ + if (ahci_mask_port_map) + ahci_get_port_map_mask(&pdev->dev, hpriv); + ahci_save_initial_config(&pdev->dev, hpriv); }
Commits 0077a504e1a4 ("ahci: asm1166: correct count of reported ports") and 9815e3961754 ("ahci: asm1064: correct count of reported ports") attempted to limit the ports of the ASM1166 and ASM1064 AHCI controllers to avoid long boot times caused by the fact that these adapters report a port map larger than the number of physical ports. The excess ports are "virtual" to hide port multiplier devices and probing these ports takes time. However, these commits caused a regression for users that do use PMP devices, as the ATA devices connected to the PMP cannot be scanned. These commits have thus been reverted by commit 6cd8adc3e18 ("ahci: asm1064: asm1166: don't limit reported ports") to allow the discovery of devices connected through a port multiplier. But this revert re-introduced the long boot times for users that do not use a port multiplier setup. This patch adds the mask_port_map ahci module parameter to allow users to manually specify port map masks for controllers. In the case of the ASMedia 1166 and 1064 controllers, users that do not have port multiplier devices can mask the excess virtual ports exposed by the controller to speedup port scanning, thus reducing boot time. The mask_port_map parameter accepts 2 different formats: - mask_port_map=<mask> This applies the same mask to all AHCI controllers present in the system. This format is convenient for small systems that have only a single AHCI controller. - mask_port_map=<pci_dev>=<mask>,<pci_dev>=mask,... This applies the specified masks only to the PCI device listed. The <pci_dev> field is a regular PCI device ID (domain:bus:dev.func). This ID can be seen following "ahci" in the kernel messages. E.g. for "ahci 0000:01:00.0: 2/2 ports implemented (port mask 0x3)", the <pci_dev> field is "0000:01:00.0". When used, the kerenl messages indicate that a port map mask was forced. E.g.: without a mask: modrpobe ahci dmesg | grep ahci ... ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode ahci 0000:00:17.0: (0000:00:17.0) 8/8 ports implemented (port mask 0xff) With a mask: modrpobe ahci mask_port_map=0000:00:17.0=0x1 dmesg | grep ahci ... ahci 0000:00:17.0: masking port_map 0xff -> 0x1 ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode ahci 0000:00:17.0: (0000:00:17.0) 1/8 ports implemented (port mask 0x1) Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/ahci.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)