Re: [PATCH 1/6] mailbox: pcc: Validate shared memory signature on request

[Sudeep Holla]

On Tue, Jun 30, 2026 at 05:18:39PM +0800, lihuisong (C) wrote:
> 
> On 6/30/2026 5:02 PM, lihuisong (C) wrote:
> > 
> > On 6/28/2026 12:37 AM, Sudeep Holla wrote:
> > > ACPI defines the PCC shared memory signature as the bitwise OR of
> > > 0x50434300 and the PCC subspace ID. The signature is located at byte
> > > offset 0 for the generic, extended and reduced PCC shared memory region
> > > layouts.
> > > 
> > > Validate the signature when a client requests a PCC mailbox channel,
> > > after
> > > mapping the shared memory and before binding the mailbox client. This
> > > keeps PCC shared memory validation in the PCC mailbox controller instead
> > > of duplicating it in individual clients.
> > > 
> > > ACPI 6.6 added clarification that the signature is populated by the
> > > platform and verified by OSPM.
> > > 
> > > Cc: Jassi Brar <jassisinghbrar@xxxxxxxxx>
> > > Cc: Huisong Li <lihuisong@xxxxxxxxxx>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxxxxx>
> > > ---
> > >   drivers/mailbox/pcc.c | 38 +++++++++++++++++++++++++++++++++-----
> > >   1 file changed, 33 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> > > index 636879ae1db7..6fbc17d41774 100644
> > > --- a/drivers/mailbox/pcc.c
> > > +++ b/drivers/mailbox/pcc.c
> > > @@ -345,6 +345,28 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
> > >       return IRQ_HANDLED;
> > >   }
> > >   +static int pcc_mbox_validate_signature(struct pcc_mbox_chan
> > > *pcc_mchan,
> > > +                       int subspace_id)
> > > +{
> > > +    u32 expected_signature = PCC_SIGNATURE | subspace_id;
> > > +    u32 signature;
> > > +
> > > +    if (pcc_mchan->shmem_size < sizeof(signature)) {
> > > +        pr_err("PCC subspace %d shared memory is too small\n",
> > > +               subspace_id);
> > > +        return -EINVAL;
> > > +    }
> > > +
> > > +    signature = ioread32(pcc_mchan->shmem);
> > > +    if (signature != expected_signature) {
> > > +        pr_err("PCC subspace %d has invalid signature: %#x expected
> > > %#x\n",
> > > +               subspace_id, signature, expected_signature);
> > > +        return -EINVAL;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > If the signature is incorrect, there must be an issue with the PCCT
> > table reporting.
> > So how about place this verification in pcc_mbox_probe?
> > We can mark the channel as unavailable so that other normal channels can
> > continue to be used.
> > And the impact is minimal.
> It can only be seen after being mapped here.

Indeed, we still fail to allocate the channel in case of invalid signature.

> Sorry, please ignore this.

No worries.

-- 
Regards,
Sudeep