linux-kernel - Re: [PATCH 12/14] hwmon: (xgene-hwmon) Simplify PCC shared memory region handling

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: <afb9a22f-7d21-42a2-a8dc-87537caad027@roeck-us.net>
Date: Mon, 3 Mar 2025 05:55:57 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Sudeep Holla <sudeep.holla@....com>, linux-acpi@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: Jassi Brar <jassisinghbrar@...il.com>, Huisong Li <lihuisong@...wei.com>,
 Adam Young <admiyo@...amperecomputing.com>, Jean Delvare
 <jdelvare@...e.com>, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH 12/14] hwmon: (xgene-hwmon) Simplify PCC shared memory
 region handling

On 3/3/25 02:51, Sudeep Holla wrote:
> The PCC driver now handles mapping and unmapping of shared memory
> areas as part of pcc_mbox_{request,free}_channel(). Without these before,
> this xgene hwmon driver did handling of those mappings like several
> other PCC mailbox client drivers.
> 
> There were redundant operations, leading to unnecessary code. Maintaining
> the consistency across these driver was harder due to scattered handling
> of shmem.
> 
> Just use the mapped shmem and remove all redundant operations from this
> driver.
> 
> Cc: Jean Delvare <jdelvare@...e.com>
> Cc: Guenter Roeck <linux@...ck-us.net>
> Cc: linux-hwmon@...r.kernel.org
> Signed-off-by: Sudeep Holla <sudeep.holla@....com>

Acked-by: Guenter Roeck <linux@...ck-us.net>

Note that I'll apply a fix this week which will cause a context conflict.
See below.

> ---
>   drivers/hwmon/xgene-hwmon.c | 40 ++++------------------------------------
...
> @@ -685,34 +681,6 @@ static int xgene_hwmon_probe(struct platform_device *pdev)
>   			goto out;
>   		}
>   
> -		/*
> -		 * This is the shared communication region
> -		 * for the OS and Platform to communicate over.
> -		 */
> -		ctx->comm_base_addr = pcc_chan->shmem_base_addr;
> -		if (ctx->comm_base_addr) {
> -			if (version == XGENE_HWMON_V2)
> -				ctx->pcc_comm_addr = (void __force *)devm_ioremap(&pdev->dev,
> -								  ctx->comm_base_addr,
> -								  pcc_chan->shmem_size);
> -			else
> -				ctx->pcc_comm_addr = devm_memremap(&pdev->dev,
> -								   ctx->comm_base_addr,
> -								   pcc_chan->shmem_size,
> -								   MEMREMAP_WB);
> -		} else {
> -			dev_err(&pdev->dev, "Failed to get PCC comm region\n");
> -			rc = -ENODEV;
> -			goto out;
> -		}
> -
> -		if (!ctx->pcc_comm_addr) {

This needed to be IS_ERR_OR_NULL() since devm_memremap() returns an ERR_PTR.

Thanks,
Guenter