Re: [PATCH RFC 2/2] cxl/memdev: Remove temporary variables from cxl_memdev_state
From: Ira Weiny
Date: Wed Jan 29 2025 - 16:17:13 EST
Alejandro Lucero Palau wrote:
>
> <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.
But in the case of no mbox where does the type2 get those values from
within cxl_dev_state_identify()?
The idea with this patch change is to only call cxl_dev_state_identify IFF
the device has a mailbox. Are you saying the same thing?
> 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.
Agreed. But without an mbox I'm hoping you don't need to call
cxl_dev_state_identify at all if you don't need to query the device... ie
don't have a mailbox.
>
>
> >> 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.
Even if type 2 has a mailbox the partition information may or may not need
to be in the identify command.
Why force any type 2 to call that mailbox command?
Ira