Re: [PATCH v30 2/3] mailbox: pcc: functions for reading and writing PCC extended data

Sudeep Holla
2025-10-24T13:50:27Z

On Tue, Oct 21, 2025 at 01:20:50PM -0400, Adam Young wrote:

[...]

> > > Because the PCC spec is wonky.
> > > https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html#extended-pcc-subspace-shared-memory-region
> > >
> > > "Length of payload being transmitted including command field."  Thus in
> > > order to copy all of the data, including  the PCC header, I need to drop the
> > > length (- sizeof(u32) ) and then add the entire header. Having all the PCC
> > > data in the buffer allows us to see it in networking tools. It is also
> > > parallel with how the messages are sent, where the PCC header is written by
> > > the driver and then the whole message is mem-copies in one io/read or write.
> > >
> > No you have misread this part.
> > Communication subspace(only part and last entry in shared memory at offset of
> > 16 bytes) - "Memory region for reading/writing PCC data. The maximum size of
> > this region is 16 bytes smaller than the size of the shared memory region
> > (specified in the Master slave Communications Subspace structure). When a
> > command is sent to or received from the platform, the size of the data in
> > this space will be Length (expressed above) minus the 4 bytes taken up by
> > the command."
> >
> > The keyword is "this space/region" which refers to only the communication
> > subspace which is at offset 16 bytes in the shmem.
> >
> > It should be just length - sizeof(command) i.e. length - 4
>
>
> I just want to make sure I have this correct.  I want to copy the entire PCC
> buffer, not just the payload, into the sk_buff.  If I wanted the payload, I
> would use the length field.  However, I want the PCC header as well, which
> is the length field, plus sizeof (header).  But that double counts the
> command field, which is part of the header, and thus I subtract this out.  I
> think my math is correct. What you wrote would be for the case where I want
> only the PCC payload.
>

Why ? How does sk_buff interpret that as PCC header. Something doesn't align
well here or I might be missing something.

I started writing some helpers and this comment made me to rethink my
approach. I don't have any to share and I will be away for a while. I will
try to review any further changes from you but expect delays.

--
Regards,
Sudeep