linux-kernel - Re: [PATCH v2 1/1] cxl/feature: Using Full Data Transfer only when offset is 0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9c835759-8a67-4479-854f-4a96edee2933@zohomail.com>
Date: Wed, 16 Apr 2025 09:14:23 +0800
From: Li Ming <ming.li@...omail.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: dave@...olabs.net, dave.jiang@...el.com, alison.schofield@...el.com,
 vishal.l.verma@...el.com, ira.weiny@...el.com, dan.j.williams@...el.com,
 linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
 shiju.jose@...wei.com, John Groves <John@...ves.net>
Subject: Re: [PATCH v2 1/1] cxl/feature: Using Full Data Transfer only when
 offset is 0

On 4/15/2025 10:36 PM, Jonathan Cameron wrote:
> On Thu, 10 Apr 2025 10:47:26 +0800
> Li Ming <ming.li@...omail.com> wrote:
>
>> Per CXL r3.2 section 8.2.10.6.3 Set Feature(Opcode 0502h)
>>
>> "If the Feature data is transferred in its entirely, the caller makes
> entirety 
>
>> one call to Set Feature with Action = Full Data Transfer. The Offset
>> field is not used and shall be ignored."
>>
>> It implies if using Full Data Transfer, the received data will be
>> updated from offset 0 on device side.
>>
>> Current driver implementation is if feature data size is less than mbox
>> payload - set feature header, driver will use Full Data Transfer even if
>> user provides an offset. Per above description, feature data will be
>> written from offset 0 rather than the offset value user provided, the
>> result will not be what user expects.
>>
>> The changes is checking if the offset caller provides is equal to 0, if
>> yes, using Full Data Transfer, if not, using Initiate Data Transfer -
>> Finish Data Transfer.
>>
>> After the changes, all Set Feature transfer scenarios are below:
>>
>> 1. data size + header is less than mbox payload and offset is 0
>> 	Full Data Transfer
>>
>> 2. data size + header is less than mbox payload and offset is not 0
> Is it valid to call this function with a none zero offset? I'm not sure.
> In my possibly entirely incorrect mental model the offset should be
> an internal detail of how we get the features that we should not expose to
> userspace (oops).  The specification is confusing on this point so I think
> we should ask for a clarification.
>
> There is text that for example says
> "Once the _entire_ Feature data is fully transferred to the device (i.e.
>  Action = Full Data Transfer or Action = Finish Data Transfer"), the
>  device shall update the attribute(s) of the Feature."
>
> What does 'entire Feature data mean here?'  The whole thing or just
> the bit from offset used in Initiate Data Transfer onwards?
>
> I'd read entire as meaning all of it so effectively the offset passed
> to this function should always be zero.
>
> I'll send a query and let you know the answer. Maybe I'm reading too
> much into that 'entire'.
>
> J

Thanks for that.

I have the same doubt too. 'entire' can be whole feature data or just mean a part of feature data needed to be updated.

If 'entire' means whole feature data, it will require Initiate Data Transfer to be with an zero offset. But I can't find any description of SPEC including this hint.

And current set feature implementation does not have such limitation either, so I made current solution. I agree with you that we should have an accurate answer.


Ming

[snip]