[PATCH v2] ata: ahci: use hweight_long() to count port_map bits

[PATCH v2] ata: ahci: use hweight_long() to count port_map bits

[kensanya]

From: TanZheng <tanzheng@kylinos.cn>

Replace the open loop used to calculate the number of set bits
in the port mapping with the `hweight_long()` function, which
simplifies the code without altering its functionality.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: TanZheng <tanzheng@kylinos.cn>
---
v2:
- Add the prefix "ata:" to the patch title

---
 drivers/ata/libahci.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index c79abdfcd7a9..15d44266c9df 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -552,11 +552,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
 
 	/* cross check port_map and cap.n_ports */
 	if (port_map) {
-		int map_ports = 0;
-
-		for (i = 0; i < AHCI_MAX_PORTS; i++)
-			if (port_map & (1 << i))
-				map_ports++;
+		int map_ports = hweight_long(port_map);
 
 		/* If PI has more ports than n_ports, whine, clear
 		 * port_map and let it be generated from n_ports.
-- 
2.25.1

Re: [PATCH v2] ata: ahci: use hweight_long() to count port_map bits

[Hannes Reinecke]

On 5/28/26 08:00, kensanya@163.com wrote:
> From: TanZheng <tanzheng@kylinos.cn>
> 
> Replace the open loop used to calculate the number of set bits
> in the port mapping with the `hweight_long()` function, which
> simplifies the code without altering its functionality.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: TanZheng <tanzheng@kylinos.cn>
> ---
> v2:
> - Add the prefix "ata:" to the patch title
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v2] ata: ahci: use hweight_long() to count port_map bits

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Signed integer overflow and undefined behavior in bit shift operations.
- [High] Strict validation zeroes out `port_map` for non-compliant devices without fallback for AHCI 1.3+, completely disabling the controller.
--

commit 3665ac64e718a0f079a1778f11d8d96122c49025
Author: TanZheng <tanzheng@kylinos.cn>

ata: ahci: use hweight_long() to count port_map bits

This commit simplifies the calculation of set bits in the port mapping
by replacing an open loop with the hweight_long() function.

> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index c79abdfcd7a9b..15d44266c9df8 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -552,11 +552,7 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
>  
>  	/* cross check port_map and cap.n_ports */
>  	if (port_map) {
> -		int map_ports = 0;
> -
> -		for (i = 0; i < AHCI_MAX_PORTS; i++)
> -			if (port_map & (1 << i))
> -				map_ports++;
> +		int map_ports = hweight_long(port_map);
>  
>  		/* If PI has more ports than n_ports, whine, clear
>  		 * port_map and let it be generated from n_ports.

[Severity: High]
This is a pre-existing issue, but zeroing out the port map based on this
check can permanently disable the controller for newer devices.

If a device reports extra unimplemented ports in its port map, the code
zeroes out port_map. The inline comment indicates it will let it be generated
from n_ports. However, the subsequent fallback logic that regenerates
port_map is conditionally guarded for older versions:

ahci_save_initial_config() {
    ...
    if (!port_map && vers < 0x10300) {
        port_map = (1 << ahci_nr_ports(cap)) - 1;
    ...
}

For AHCI 1.3+ devices, port_map is never regenerated and remains 0.

Does this cause all legitimate ports to be assigned ata_dummy_port_ops,
breaking otherwise functional devices?

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing
undefined behavior regression with the port mask generation nearby.

When regenerating the port map for devices prior to AHCI 1.3, the fallback
logic uses a signed 32-bit integer:

ahci_save_initial_config() {
    ...
    if (!port_map && vers < 0x10300) {
        port_map = (1 << ahci_nr_ports(cap)) - 1;
    ...
}

Since ahci_nr_ports(cap) can return up to 32, shifting a 32-bit integer by 32
is undefined behavior. On architectures like x86, the shift count is masked
to 5 bits, so 1 << 32 evaluates to 1 << 0 = 1.

This results in 1 - 1 = 0, causing port_map to be incorrectly cleared to 0.

Could this unintentionally disable all ports on a fully populated 32-port
AHCI controller? Is it possible to use 1UL instead of 1 to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528060050.743599-1-kensanya@163.com?part=1

Re: [PATCH v2] ata: ahci: use hweight_long() to count port_map bits

[Niklas Cassel]

On Thu, 28 May 2026 14:00:50 +0800, kensanya@163.com wrote:
> Replace the open loop used to calculate the number of set bits
> in the port mapping with the `hweight_long()` function, which
> simplifies the code without altering its functionality.

Applied to libata/linux.git (for-next), thanks!

[1/1] ata: ahci: use hweight_long() to count port_map bits
      https://git.kernel.org/libata/linux/c/b7ed008e

Kind regards,
Niklas

Re: [PATCH v2] ata: ahci: use hweight_long() to count port_map bits

[Damien Le Moal]

On 2026/05/28 15:00, kensanya@163.com wrote:
> From: TanZheng <tanzheng@kylinos.cn>
> 
> Replace the open loop used to calculate the number of set bits
> in the port mapping with the `hweight_long()` function, which
> simplifies the code without altering its functionality.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: TanZheng <tanzheng@kylinos.cn>

Title: I suggested:

ata: libahci: ...

You did not fix that.


-- 
Damien Le Moal
Western Digital Research