Re: [PATCH v2] ata: ahci: Add mask_port_map module parameter

[Damien Le Moal <dlemoal@kernel.org>]

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
> 

-- 
Damien Le Moal
Western Digital Research