Re: [PATCH RFC 2/2] cxl/memdev: Remove temporary variables from cxl_memdev_state

Alejandro Lucero Palau
2025-01-29T18:17:54Z

<snip>

>
>> FWIW, I had (as part of current in-progress v10) similar struct than
>> used here for Type2 initialization when there is no mailbox.
>>
>>
>> I had added a specific function for initialising that struct, but my
>> idea now with this change is to have cxl_mem_dev_info initialized by the
>> driver before calling cxl_dev_state_identify,
> Why is the accelerator calling cxl_dev_state_identify? I did not see that
> in v9. My idea was that was a mailbox only call which is only needed for
> memdevs. And cxl_add_partition() can be called by accelerators as a
> convenience function to aid in creating cxl_dpa_info. (This and cxl_test
> needed that function shared so it just got left in mbox.c)

No in v9 ... that predates Dan's DPA patches.

Type2 without an mbox needs to give those values obtained from
CXL_MBOX_OP_IDENTIFY. I'm using an struct for passing those values to
cxl_dev_state_identify for having same function serving Type2 with and
without mbox. I could have another function instead and calling that one
for Type2 with mbox. My idea is to keep similar initialization than
Type3 pci driver.

>> and inside that function
> I'm not clear which 'that function' you are referring to here.

cxl_dev_state_identify

>> checking if total_bytes already != 0 for avoiding call the mbox command
>> for getting the info. This will support both cases for Type2, with and
>> without mailbox.
> I think I agree with you except the != 0 to avoid mailbox commands.
>
> Unless I am miss-understanding Dan we need to get to a place where mailbox
> commands stop filling in structures unless those work for both type 2 and
> 3 __and__ are optional. Because putting in special checks for the type
> within a cxl/core/mailbox call is wrong IMO.

As I said, I can avoid that check with a wrapper for Type2, then only
Type2 with mbox and supporting that command (is it mandatory if an
mbox?) will end up calling cxl_dev_state_identify.

> Ira