* [PATCH 1/4] ACPICA: Fix PCC OperationRegion command offsets
2026-07-17 8:10 [PATCH 0/4] ACPI/PCC: Correct PCC OperationRegion handling Sudeep Holla
@ 2026-07-17 8:10 ` Sudeep Holla
2026-07-17 8:10 ` [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler Sudeep Holla
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-07-17 8:10 UTC (permalink / raw)
To: linux-acpi, linux-kernel
Cc: Sudeep Holla, acpica-devel, Rafael J . Wysocki, Saket Dumbre
ACPI 6.3, section 5.5.2.4.7.3, states that the PCC Operation
Region is associated with the region of shared memory that follows the
PCC signature.
The generic and extended PCC shared memory layouts include the 4-byte
signature at offset 0, so their raw shared memory COMMAND fields are at
offsets 4 and 12 respectively. Since AML field offsets for the PCC
OperationRegion are relative to the region after that signature, ACPICA
must look for those COMMAND fields at OperationRegion offsets 0 and 8.
Adjust the generic and master subspace command checks to use those
OperationRegion-relative offsets. Otherwise writes to the COMMAND field
can fail to invoke the PCC address space handler at the offset described
by the PCC OperationRegion definition.
Fixes: aa6ec56b574d ("ACPICA: ACPI 6.3: add PCC operation region support for AML interpreter")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/acpi/acpica/exfield.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c
index 9a55524ed8f4..a7fac63938b3 100644
--- a/drivers/acpi/acpica/exfield.c
+++ b/drivers/acpi/acpica/exfield.c
@@ -45,12 +45,13 @@ static const u8 acpi_protocol_lengths[] = {
/*
* The following macros determine a given offset is a COMD field.
- * According to the specification, generic subspaces (types 0-2) contains a
- * 2-byte COMD field at offset 4 and master subspaces (type 3) contains a 4-byte
- * COMD field starting at offset 12.
+ * According to the specification, the PCC OperationRegion begins after
+ * the PCC signature. The raw shared memory COMD offsets of 4 for generic
+ * subspaces (types 0-2) and 12 for master subspaces (type 3) therefore
+ * appear at OperationRegion offsets 0 and 8.
*/
-#define GENERIC_SUBSPACE_COMMAND(a) (4 == a || a == 5)
-#define MASTER_SUBSPACE_COMMAND(a) (12 <= a && a <= 15)
+#define GENERIC_SUBSPACE_COMMAND(a) ((a) < 2)
+#define MASTER_SUBSPACE_COMMAND(a) (((a) - 8) < 4)
/*******************************************************************************
*
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler
2026-07-17 8:10 [PATCH 0/4] ACPI/PCC: Correct PCC OperationRegion handling Sudeep Holla
2026-07-17 8:10 ` [PATCH 1/4] ACPICA: Fix PCC OperationRegion command offsets Sudeep Holla
@ 2026-07-17 8:10 ` Sudeep Holla
2026-07-21 11:43 ` lihuisong (C)
2026-07-17 8:10 ` [PATCH 3/4] ACPI: PCC: Free channel on OpRegion deactivation Sudeep Holla
2026-07-17 8:10 ` [PATCH 4/4] ACPI: PCC: Cache OpRegion command timeout Sudeep Holla
3 siblings, 1 reply; 12+ messages in thread
From: Sudeep Holla @ 2026-07-17 8:10 UTC (permalink / raw)
To: linux-acpi, linux-kernel
Cc: Sudeep Holla, acpica-devel, Rafael J . Wysocki, Saket Dumbre
ACPI 6.3 introduced PCC OperationRegions. Section 5.5.2.4.7.3,
"Declaring message fields within a PCC OperationRegion", states that,
for all PCC subspace types, the PCC Operation Region pertains to the
region of PCC subspace that succeeds the PCC signature.
The PCC address space handler currently copies the OperationRegion
buffer to and from the start of the PCC shared memory region. That can
overwrite or expose the signature at byte offset 0, and it also misses
the last 4 bytes of the actual PCC OperationRegion data.
Offset OperationRegion copies by the size of the signature and reject
regions that do not fit in the shared memory after that signature. AML
that sizes the PCC OperationRegion to include the signature is not
conforming to the PCC OperationRegion definition.
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/acpi/acpi_pcc.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index 438c67189511..9881c9ee293d 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -28,6 +28,7 @@
* to PCC commands
*/
#define PCC_CMD_WAIT_RETRIES_NUM 500ULL
+#define PCC_SIGNATURE_SIZE sizeof(u32)
struct pcc_data {
struct pcc_mbox_chan *pcc_chan;
@@ -74,6 +75,14 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function,
}
pcc_chan = data->pcc_chan;
+ if (pcc_chan->shmem_size < PCC_SIGNATURE_SIZE ||
+ ctx->length > pcc_chan->shmem_size - PCC_SIGNATURE_SIZE) {
+ pr_err("PCC channel-%d shared memory is too small.\n",
+ ctx->subspace_id);
+ ret = AE_AML_REGION_LIMIT;
+ goto err_free_channel;
+ }
+
if (!pcc_chan->mchan->mbox->txdone_irq) {
pr_err("This channel-%d does not support interrupt.\n",
ctx->subspace_id);
@@ -97,14 +106,17 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
u32 bits, acpi_integer *value,
void *handler_context, void *region_context)
{
- int ret;
struct pcc_data *data = region_context;
+ void __iomem *pcc_opregion;
u64 usecs_lat;
+ int ret;
+
+ pcc_opregion = data->pcc_chan->shmem + PCC_SIGNATURE_SIZE;
reinit_completion(&data->done);
- /* Write to Shared Memory */
- memcpy_toio(data->pcc_chan->shmem, (void *)value, data->ctx.length);
+ /* Write to the PCC OperationRegion after the shared memory signature. */
+ memcpy_toio(pcc_opregion, (void *)value, data->ctx.length);
ret = mbox_send_message(data->pcc_chan->mchan, NULL);
if (ret < 0)
@@ -125,7 +137,7 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
mbox_chan_txdone(data->pcc_chan->mchan, ret);
- memcpy_fromio(value, data->pcc_chan->shmem, data->ctx.length);
+ memcpy_fromio(value, pcc_opregion, data->ctx.length);
return AE_OK;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler
2026-07-17 8:10 ` [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler Sudeep Holla
@ 2026-07-21 11:43 ` lihuisong (C)
2026-07-21 12:27 ` Sudeep Holla
0 siblings, 1 reply; 12+ messages in thread
From: lihuisong (C) @ 2026-07-21 11:43 UTC (permalink / raw)
To: Sudeep Holla, linux-acpi, linux-kernel
Cc: acpica-devel, Rafael J . Wysocki, Saket Dumbre, lihuisong
On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> ACPI 6.3 introduced PCC OperationRegions. Section 5.5.2.4.7.3,
> "Declaring message fields within a PCC OperationRegion", states that,
> for all PCC subspace types, the PCC Operation Region pertains to the
> region of PCC subspace that succeeds the PCC signature.
>
> The PCC address space handler currently copies the OperationRegion
> buffer to and from the start of the PCC shared memory region. That can
> overwrite or expose the signature at byte offset 0, and it also misses
> the last 4 bytes of the actual PCC OperationRegion data.
>
> Offset OperationRegion copies by the size of the signature and reject
> regions that do not fit in the shared memory after that signature. AML
> that sizes the PCC OperationRegion to include the signature is not
> conforming to the PCC OperationRegion definition.
>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> ---
The below commit description said the PCC operationRegion contains the
signature field.
"77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC
Type 3 subtype")"
And I find that the definition of this OperationRegion in ACPI spec 6.3
is the same with spec 6.5 and 6.6.
That is, the original implementation already didn't comply with ACPI spec.
Patch 1/4 also requires the change of platform AML about PCC
OperationRegion.
These modification may lead to the existed PCC OperationRegion method in
some drivers doesn't work.
Because the platform AML still contains signature field in PCC
OperationRegion method.
I don't know how many drivers in linux have used this method.
IMO, we at least need to reclarify this usage change and provide a new
usage example in the commit log like the original commit did.
Anyway, this patch also makes the driver more compliant with the
specifications.
So I agree.
Reviewed-by: Huisong Li <lihuisong@huawei.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler
2026-07-21 11:43 ` lihuisong (C)
@ 2026-07-21 12:27 ` Sudeep Holla
2026-07-21 19:20 ` Rafael J. Wysocki (Intel)
0 siblings, 1 reply; 12+ messages in thread
From: Sudeep Holla @ 2026-07-21 12:27 UTC (permalink / raw)
To: lihuisong (C)
Cc: linux-acpi, linux-kernel, Sudeep Holla, acpica-devel,
Rafael J . Wysocki, Saket Dumbre
On Tue, Jul 21, 2026 at 07:43:53PM +0800, lihuisong (C) wrote:
>
> On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> > ACPI 6.3 introduced PCC OperationRegions. Section 5.5.2.4.7.3,
> > "Declaring message fields within a PCC OperationRegion", states that,
> > for all PCC subspace types, the PCC Operation Region pertains to the
> > region of PCC subspace that succeeds the PCC signature.
> >
> > The PCC address space handler currently copies the OperationRegion
> > buffer to and from the start of the PCC shared memory region. That can
> > overwrite or expose the signature at byte offset 0, and it also misses
> > the last 4 bytes of the actual PCC OperationRegion data.
> >
> > Offset OperationRegion copies by the size of the signature and reject
> > regions that do not fit in the shared memory after that signature. AML
> > that sizes the PCC OperationRegion to include the signature is not
> > conforming to the PCC OperationRegion definition.
> >
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> > Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> > ---
> The below commit description said the PCC operationRegion contains the
> signature field.
> "77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC
> Type 3 subtype")"
> And I find that the definition of this OperationRegion in ACPI spec 6.3 is
> the same with spec 6.5 and 6.6.
> That is, the original implementation already didn't comply with ACPI spec.
>
> Patch 1/4 also requires the change of platform AML about PCC
> OperationRegion.
> These modification may lead to the existed PCC OperationRegion method in
> some drivers doesn't work.
> Because the platform AML still contains signature field in PCC
> OperationRegion method.
> I don't know how many drivers in linux have used this method.
> IMO, we at least need to reclarify this usage change and provide a new usage
> example in the commit log like the original commit did.
>
Good point, I didn't realise I had wrong example in the original commit.
I will add the example of correct usage in this change. Thanks for pointing
it out.
> Anyway, this patch also makes the driver more compliant with the
> specifications.
> So I agree.
> Reviewed-by: Huisong Li <lihuisong@huawei.com>
>
Thanks again for all the review.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler
2026-07-21 12:27 ` Sudeep Holla
@ 2026-07-21 19:20 ` Rafael J. Wysocki (Intel)
2026-07-22 8:47 ` Sudeep Holla
0 siblings, 1 reply; 12+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-07-21 19:20 UTC (permalink / raw)
To: Sudeep Holla
Cc: lihuisong (C), linux-acpi, linux-kernel, acpica-devel,
Rafael J . Wysocki, Saket Dumbre
On Tue, Jul 21, 2026 at 2:27 PM Sudeep Holla <sudeep.holla@kernel.org> wrote:
>
> On Tue, Jul 21, 2026 at 07:43:53PM +0800, lihuisong (C) wrote:
> >
> > On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> > > ACPI 6.3 introduced PCC OperationRegions. Section 5.5.2.4.7.3,
> > > "Declaring message fields within a PCC OperationRegion", states that,
> > > for all PCC subspace types, the PCC Operation Region pertains to the
> > > region of PCC subspace that succeeds the PCC signature.
> > >
> > > The PCC address space handler currently copies the OperationRegion
> > > buffer to and from the start of the PCC shared memory region. That can
> > > overwrite or expose the signature at byte offset 0, and it also misses
> > > the last 4 bytes of the actual PCC OperationRegion data.
> > >
> > > Offset OperationRegion copies by the size of the signature and reject
> > > regions that do not fit in the shared memory after that signature. AML
> > > that sizes the PCC OperationRegion to include the signature is not
> > > conforming to the PCC OperationRegion definition.
> > >
> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > > Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> > > Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> > > ---
> > The below commit description said the PCC operationRegion contains the
> > signature field.
> > "77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC
> > Type 3 subtype")"
> > And I find that the definition of this OperationRegion in ACPI spec 6.3 is
> > the same with spec 6.5 and 6.6.
> > That is, the original implementation already didn't comply with ACPI spec.
> >
> > Patch 1/4 also requires the change of platform AML about PCC
> > OperationRegion.
> > These modification may lead to the existed PCC OperationRegion method in
> > some drivers doesn't work.
> > Because the platform AML still contains signature field in PCC
> > OperationRegion method.
> > I don't know how many drivers in linux have used this method.
> > IMO, we at least need to reclarify this usage change and provide a new usage
> > example in the commit log like the original commit did.
> >
>
> Good point, I didn't realise I had wrong example in the original commit.
> I will add the example of correct usage in this change. Thanks for pointing
> it out.
>
> > Anyway, this patch also makes the driver more compliant with the
> > specifications.
> > So I agree.
> > Reviewed-by: Huisong Li <lihuisong@huawei.com>
> >
>
> Thanks again for all the review.
I gather that there will be a v2 then.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler
2026-07-21 19:20 ` Rafael J. Wysocki (Intel)
@ 2026-07-22 8:47 ` Sudeep Holla
0 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-07-22 8:47 UTC (permalink / raw)
To: Rafael J. Wysocki (Intel)
Cc: lihuisong (C), linux-acpi, Sudeep Holla, linux-kernel,
acpica-devel, Saket Dumbre
On Tue, Jul 21, 2026 at 09:20:15PM +0200, Rafael J. Wysocki (Intel) wrote:
> On Tue, Jul 21, 2026 at 2:27 PM Sudeep Holla <sudeep.holla@kernel.org> wrote:
> >
> > On Tue, Jul 21, 2026 at 07:43:53PM +0800, lihuisong (C) wrote:
> > >
> > > On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> > > > ACPI 6.3 introduced PCC OperationRegions. Section 5.5.2.4.7.3,
> > > > "Declaring message fields within a PCC OperationRegion", states that,
> > > > for all PCC subspace types, the PCC Operation Region pertains to the
> > > > region of PCC subspace that succeeds the PCC signature.
> > > >
> > > > The PCC address space handler currently copies the OperationRegion
> > > > buffer to and from the start of the PCC shared memory region. That can
> > > > overwrite or expose the signature at byte offset 0, and it also misses
> > > > the last 4 bytes of the actual PCC OperationRegion data.
> > > >
> > > > Offset OperationRegion copies by the size of the signature and reject
> > > > regions that do not fit in the shared memory after that signature. AML
> > > > that sizes the PCC OperationRegion to include the signature is not
> > > > conforming to the PCC OperationRegion definition.
> > > >
> > > > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > > > Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> > > > ---
> > > The below commit description said the PCC operationRegion contains the
> > > signature field.
> > > "77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC
> > > Type 3 subtype")"
> > > And I find that the definition of this OperationRegion in ACPI spec 6.3 is
> > > the same with spec 6.5 and 6.6.
> > > That is, the original implementation already didn't comply with ACPI spec.
> > >
> > > Patch 1/4 also requires the change of platform AML about PCC
> > > OperationRegion.
> > > These modification may lead to the existed PCC OperationRegion method in
> > > some drivers doesn't work.
> > > Because the platform AML still contains signature field in PCC
> > > OperationRegion method.
> > > I don't know how many drivers in linux have used this method.
> > > IMO, we at least need to reclarify this usage change and provide a new usage
> > > example in the commit log like the original commit did.
> > >
> >
> > Good point, I didn't realise I had wrong example in the original commit.
> > I will add the example of correct usage in this change. Thanks for pointing
> > it out.
> >
> > > Anyway, this patch also makes the driver more compliant with the
> > > specifications.
> > > So I agree.
> > > Reviewed-by: Huisong Li <lihuisong@huawei.com>
> > >
> >
> > Thanks again for all the review.
>
> I gather that there will be a v2 then.
Yes, just with the improved commit message giving illustration of AML code
like the original commit that this change fixes. No change is the code as
such.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] ACPI: PCC: Free channel on OpRegion deactivation
2026-07-17 8:10 [PATCH 0/4] ACPI/PCC: Correct PCC OperationRegion handling Sudeep Holla
2026-07-17 8:10 ` [PATCH 1/4] ACPICA: Fix PCC OperationRegion command offsets Sudeep Holla
2026-07-17 8:10 ` [PATCH 2/4] ACPI: PCC: Preserve shared memory signature in OpRegion handler Sudeep Holla
@ 2026-07-17 8:10 ` Sudeep Holla
2026-07-22 1:15 ` lihuisong (C)
2026-07-17 8:10 ` [PATCH 4/4] ACPI: PCC: Cache OpRegion command timeout Sudeep Holla
3 siblings, 1 reply; 12+ messages in thread
From: Sudeep Holla @ 2026-07-17 8:10 UTC (permalink / raw)
To: linux-acpi, linux-kernel
Cc: Sudeep Holla, acpica-devel, Rafael J . Wysocki, Saket Dumbre
ACPICA calls the address space setup callback with
ACPI_REGION_DEACTIVATE when a PCC OperationRegion is torn down.
The PCC setup callback currently allocates a fresh pcc_data and requests
the mailbox channel before looking at the function argument. If ACPICA
deactivates a region, this can leave the existing region context and
mailbox channel unreleased, and may also request a channel during
teardown.
Handle ACPI_REGION_DEACTIVATE before allocation. Free the PCC mailbox
channel, release the region context and clear the context pointer.
Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/acpi/acpi_pcc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index 9881c9ee293d..57d13b25c1d6 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -55,6 +55,19 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function,
struct pcc_mbox_chan *pcc_chan;
acpi_status ret;
+ if (function == ACPI_REGION_DEACTIVATE) {
+ data = *region_context;
+ if (data) {
+ pcc_mbox_free_channel(data->pcc_chan);
+ kfree(data);
+ *region_context = NULL;
+ }
+ return AE_OK;
+ }
+
+ if (function != ACPI_REGION_ACTIVATE)
+ return AE_BAD_PARAMETER;
+
data = kzalloc_obj(*data);
if (!data)
return AE_NO_MEMORY;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] ACPI: PCC: Free channel on OpRegion deactivation
2026-07-17 8:10 ` [PATCH 3/4] ACPI: PCC: Free channel on OpRegion deactivation Sudeep Holla
@ 2026-07-22 1:15 ` lihuisong (C)
2026-07-22 8:45 ` Sudeep Holla
0 siblings, 1 reply; 12+ messages in thread
From: lihuisong (C) @ 2026-07-22 1:15 UTC (permalink / raw)
To: Sudeep Holla
Cc: acpica-devel, Rafael J . Wysocki, Saket Dumbre, linux-acpi,
linux-kernel, lihuisong
On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> ACPICA calls the address space setup callback with
> ACPI_REGION_DEACTIVATE when a PCC OperationRegion is torn down.
>
> The PCC setup callback currently allocates a fresh pcc_data and requests
> the mailbox channel before looking at the function argument. If ACPICA
> deactivates a region, this can leave the existing region context and
> mailbox channel unreleased, and may also request a channel during
> teardown.
>
> Handle ACPI_REGION_DEACTIVATE before allocation. Free the PCC mailbox
> channel, release the region context and clear the context pointer.
>
> Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> ---
How to trigger this region deactivate at runtime?
Overall looks good to me.
Reviewed-by: Huisong Li <lihuisong@huawei.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] ACPI: PCC: Free channel on OpRegion deactivation
2026-07-22 1:15 ` lihuisong (C)
@ 2026-07-22 8:45 ` Sudeep Holla
0 siblings, 0 replies; 12+ messages in thread
From: Sudeep Holla @ 2026-07-22 8:45 UTC (permalink / raw)
To: lihuisong (C)
Cc: acpica-devel, Rafael J . Wysocki, Sudeep Holla, Saket Dumbre,
linux-acpi, linux-kernel
On Wed, Jul 22, 2026 at 09:15:40AM +0800, lihuisong (C) wrote:
>
> On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> > ACPICA calls the address space setup callback with
> > ACPI_REGION_DEACTIVATE when a PCC OperationRegion is torn down.
> >
> > The PCC setup callback currently allocates a fresh pcc_data and requests
> > the mailbox channel before looking at the function argument. If ACPICA
> > deactivates a region, this can leave the existing region context and
> > mailbox channel unreleased, and may also request a channel during
> > teardown.
> >
> > Handle ACPI_REGION_DEACTIVATE before allocation. Free the PCC mailbox
> > channel, release the region context and clear the context pointer.
> >
> > Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> > Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> > ---
> How to trigger this region deactivate at runtime?
>
I haven't tried it myself. I wasn't even aware of ACPI_REGION_DEACTIVATE
until sashiko pointed it out and then did some digging in the code.
> Overall looks good to me.
> Reviewed-by: Huisong Li <lihuisong@huawei.com>
Thanks!
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] ACPI: PCC: Cache OpRegion command timeout
2026-07-17 8:10 [PATCH 0/4] ACPI/PCC: Correct PCC OperationRegion handling Sudeep Holla
` (2 preceding siblings ...)
2026-07-17 8:10 ` [PATCH 3/4] ACPI: PCC: Free channel on OpRegion deactivation Sudeep Holla
@ 2026-07-17 8:10 ` Sudeep Holla
2026-07-21 11:53 ` lihuisong (C)
3 siblings, 1 reply; 12+ messages in thread
From: Sudeep Holla @ 2026-07-17 8:10 UTC (permalink / raw)
To: linux-acpi, linux-kernel
Cc: Sudeep Holla, acpica-devel, Rafael J . Wysocki, Saket Dumbre
The PCC OperationRegion handler computes the same command completion
wait timeout each time it sends a command. The timeout is derived from
static channel properties, so compute it once when the PCC channel is
set up and store the millisecond value in the mailbox client timeout
field.
Use the cached timeout when waiting for the OperationRegion command to
complete. This keeps the timeout calculation in one place and avoids
recomputing it for every command.
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/acpi/acpi_pcc.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
index 57d13b25c1d6..345f233d77cd 100644
--- a/drivers/acpi/acpi_pcc.c
+++ b/drivers/acpi/acpi_pcc.c
@@ -50,10 +50,11 @@ static acpi_status
acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function,
void *handler_context, void **region_context)
{
- struct pcc_data *data;
struct acpi_pcc_info *ctx = handler_context;
struct pcc_mbox_chan *pcc_chan;
+ struct pcc_data *data;
acpi_status ret;
+ u64 usecs_lat;
if (function == ACPI_REGION_DEACTIVATE) {
data = *region_context;
@@ -103,6 +104,16 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function,
goto err_free_channel;
}
+ /*
+ * pcc_chan->latency is just a Nominal value. In reality the remote
+ * processor could be much slower to reply. So add an arbitrary
+ * amount of wait on top of Nominal.
+ */
+ usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * pcc_chan->latency;
+ data->cl.tx_tout = DIV_ROUND_UP_ULL(usecs_lat, 1000);
+ if (!data->cl.tx_tout)
+ data->cl.tx_tout = 1;
+
*region_context = data;
return AE_OK;
@@ -121,7 +132,6 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
{
struct pcc_data *data = region_context;
void __iomem *pcc_opregion;
- u64 usecs_lat;
int ret;
pcc_opregion = data->pcc_chan->shmem + PCC_SIGNATURE_SIZE;
@@ -135,14 +145,8 @@ acpi_pcc_address_space_handler(u32 function, acpi_physical_address addr,
if (ret < 0)
return AE_ERROR;
- /*
- * pcc_chan->latency is just a Nominal value. In reality the remote
- * processor could be much slower to reply. So add an arbitrary
- * amount of wait on top of Nominal.
- */
- usecs_lat = PCC_CMD_WAIT_RETRIES_NUM * data->pcc_chan->latency;
ret = wait_for_completion_timeout(&data->done,
- usecs_to_jiffies(usecs_lat));
+ msecs_to_jiffies(data->cl.tx_tout));
if (ret == 0) {
pr_err("PCC command executed timeout!\n");
return AE_TIME;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/4] ACPI: PCC: Cache OpRegion command timeout
2026-07-17 8:10 ` [PATCH 4/4] ACPI: PCC: Cache OpRegion command timeout Sudeep Holla
@ 2026-07-21 11:53 ` lihuisong (C)
0 siblings, 0 replies; 12+ messages in thread
From: lihuisong (C) @ 2026-07-21 11:53 UTC (permalink / raw)
To: Sudeep Holla, linux-acpi, linux-kernel
Cc: acpica-devel, Rafael J . Wysocki, Saket Dumbre, lihuisong
On 7/17/2026 4:10 PM, Sudeep Holla wrote:
> The PCC OperationRegion handler computes the same command completion
> wait timeout each time it sends a command. The timeout is derived from
> static channel properties, so compute it once when the PCC channel is
> set up and store the millisecond value in the mailbox client timeout
> field.
>
> Use the cached timeout when waiting for the OperationRegion command to
> complete. This keeps the timeout calculation in one place and avoids
> recomputing it for every command.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> ---
LGTM,
Reviewed-by: Huisong Li <lihuisong@huawei.com>
^ permalink raw reply [flat|nested] 12+ messages in thread