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

["lihuisong (C)" <lihuisong@huawei.com>]

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@gmail.com>
>> Cc: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
>> ---
>>   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.
Sorry, please ignore this.
>>   /**
>>    * pcc_mbox_request_channel - PCC clients call this function to
>>    *        request a pointer to their PCC subspace, from which they
>> @@ -381,14 +403,20 @@ pcc_mbox_request_channel(struct mbox_client 
>> *cl, int subspace_id)
>>       if (!pcc_mchan->shmem)
>>           return ERR_PTR(-ENXIO);
>>   +    rc = pcc_mbox_validate_signature(pcc_mchan, subspace_id);
>> +    if (rc)
>> +        goto err_unmap_shmem;
>> +
>>       rc = mbox_bind_client(chan, cl);
>> -    if (rc) {
>> -        iounmap(pcc_mchan->shmem);
>> -        pcc_mchan->shmem = NULL;
>> -        return ERR_PTR(rc);
>> -    }
>> +    if (rc)
>> +        goto err_unmap_shmem;
>>         return pcc_mchan;
>> +
>> +err_unmap_shmem:
>> +    iounmap(pcc_mchan->shmem);
>> +    pcc_mchan->shmem = NULL;
>> +    return ERR_PTR(rc);
>>   }
>>   EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
>>