netdev - Re: [PATCH v12 07/23] cxl: support dpa initialization without a mailbox

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c48a4a1-b23e-4648-8632-e1d068db9b08@amd.com>
Date: Thu, 10 Apr 2025 12:37:29 +0100
From: Alejandro Lucero Palau <alucerop@....com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 alejandro.lucero-palau@....com
Cc: linux-cxl@...r.kernel.org, netdev@...r.kernel.org,
 dan.j.williams@...el.com, edward.cree@....com, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
 dave.jiang@...el.com, Ben Cheatham <benjamin.cheatham@....com>
Subject: Re: [PATCH v12 07/23] cxl: support dpa initialization without a
 mailbox


On 4/7/25 11:53, Alejandro Lucero Palau wrote:
>
> On 4/4/25 17:05, Jonathan Cameron wrote:
>> On Mon, 31 Mar 2025 15:45:39 +0100
>> alejandro.lucero-palau@....com wrote:
>>
>>> From: Alejandro Lucero <alucerop@....com>
>>>
>>> Type3 relies on mailbox CXL_MBOX_OP_IDENTIFY command for initializing
>>> memdev state params which end up being used for dma initialization.
>> DMA
>>
>
> Ok
>
>
>>> Allow a Type2 driver to initialize dpa simply by giving the size of its
>> DPA
>
>
> Ok
>
>
>>> volatile and/or non-volatile hardware partitions.
>>>
>>> Export cxl_dpa_setup as well for initializing those added dpa 
>>> partitions
>>> with the proper resources.
>>>
>>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>>> Reviewed-by: Ben Cheatham <benjamin.cheatham@....com>
>>> ---
>>>   drivers/cxl/core/mbox.c | 17 ++++++++++++++---
>>>   drivers/cxl/cxlmem.h    | 13 -------------
>>>   include/cxl/cxl.h       | 14 ++++++++++++++
>>>   3 files changed, 28 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
>>> index ab994d459f46..e4610e778723 100644
>>> --- a/drivers/cxl/core/mbox.c
>>> +++ b/drivers/cxl/core/mbox.c
>>> @@ -1284,6 +1284,18 @@ static void add_part(struct cxl_dpa_info 
>>> *info, u64 start, u64 size, enum cxl_pa
>>>       info->nr_partitions++;
>>>   }
>>>   +void cxl_mem_dpa_init(struct cxl_dpa_info *info, u64 volatile_bytes,
>>> +              u64 persistent_bytes)
>>> +{
>>> +    if (!info->size)
>> Why?  What is this defending against?
>
>
> The new function is used by cxl_mem_dpa_fetch as well for avoiding 
> duplicated code, where size is initialized before the potential 
> invocation of cxl_mem_dpa_init.
>
>
> But with your heads up, I think I can just set the size 
> unconditionally and to change the caller in cxl_mem_dpa_fetch for 
> setting is if such invocation, because partition_align_bytes != 0, 
> does not happen.
>
>

After looking at this for implementing the discussed changes, I think it 
is better to keep the setting conditionally to the cxl_dpa_info state, 
that is if not size is set yet what is the case for Type2 without an mbox.

The reason is the values used for initializing size are different than 
in the Type3 case, or they could be. Currently the value is set to that 
one obtained from the total_capacity from the cxl_mbox_identify struct, 
so I'll preserve it.

I'm using though the new cxl_mem_dpa_init() for the other case inside 
cxl_mem_dpa_fetch() as you suggested.


Thank you


> Thanks!
>
>
>>> +        info->size = volatile_bytes + persistent_bytes;
>>> +
>>> +    add_part(info, 0, volatile_bytes, CXL_PARTMODE_RAM);
>>> +    add_part(info, volatile_bytes, persistent_bytes,
>>> +         CXL_PARTMODE_PMEM);
>>> +}
>>> +EXPORT_SYMBOL_NS_GPL(cxl_mem_dpa_init, "CXL");