Re: [PATCH 6/6] ACPI: PCC: Preserve shared memory signature in OpRegion handler

[Sudeep Holla]

On Wed, Jul 01, 2026 at 02:34:10PM +0100, Sudeep Holla wrote:
> On Wed, Jul 01, 2026 at 12:44:30PM +0200, Rafael J. Wysocki (Intel) wrote:
> > On Tue, Jun 30, 2026 at 7:00 PM Sudeep Holla <sudeep.holla@xxxxxxxxxx> wrote:
> > >
> > > On Tue, Jun 30, 2026 at 02:33:08PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > > On Sat, Jun 27, 2026 at 6:39 PM Sudeep Holla <sudeep.holla@xxxxxxxxxx> wrote:
> > > > >
> > > > > ACPI defines a PCC OperationRegion as the shared memory fields that
> > > > > follow the PCC signature. The OperationRegion length is likewise the
> > > > > total size of the fields that succeed the signature in shared memory.
> > > > >
> > > > > ACPI 6.6 adds clarification that the PCC shared memory signature is
> > > > > populated by the platform and verified by OSPM.
> > > > >
> > > > > 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.
> > > > >
> > > > > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > > > > Fixes: 77e2a04745ff ("ACPI: PCC: Implement OperationRegion handler for the PCC Type 3 subtype")
> > > > > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxxxxx>
> > > >
> > > > Does this depend on anything else in the series or is it independent?
> > > >
> > >
> > > It is independent. This just fixes the issue of including the signature in
> > > the opregion data. It can go independently as all other patches as well.
> > 
> > So I've looked at Sashiko's feedback on this change:
> > 
> > https://sashiko.dev/#/patchset/20260627-acpi_pcc_signature-v1-0-c1b7268d4fdc%40kernel.org
> > 
> > and I'm a bit worried.
> > 
> > Of course, the preexisting issues found by it will need to be
> > addressed separately, but what about firmware released before the
> > clarification in ACPI 6.6 and the possible misalignment issue
> > mentioned by it?
> 
> I haven't analysed all the report from Sashiko yet. But unlike other patches
> in the series, this one is not strictly to validate the signature.
> 
> Even before v6.6, the PCC Opregion didn't include the signature which is
> at offset 0. The code always started copying to/from offset 0 missing the
> last 32-bit field(word). Since v6.3 when PCC Opregion was added we have
> this in the specification:
> 
>   | <section> Declaring message fields within a PCC OperationRegion
>   |
>   | For all PCC subspace types, the PCC Operation Region pertains to the
>   | region of PCC subspace that succeeds the PCC signature.
> 
> So, this patch addresses that issue of wrong starting offset used by this
> driver.
> 
> Other patches I agree, it is more likely we just need to warn and not
> reject absence of wrong PCC signatures.
> 
> I will take a look at the other issues reported by Sashiko soon.
> 

Forgot to add that let us wait at-least until I go through the report
understand and conclude it is not fatal/critical before we can merge this.
Indeed a quick read concerned as much that I would like to spend time
analysing it carefully.

-- 
Regards,
Sudeep