Re: [RFC 0/2] type2 cxl initialization

[Dan Williams <dan.j.williams@intel.com>]

Ben Cheatham wrote:
> On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote:
> > From: Alejandro Lucero <alucerop@amd.com>
> > 
> > Avoiding to send the full type2 support patchset until there is an
> > agreement about how to do the cxl initialization from accel drivers.
> > 
> > Using the idea from fwctl for embedding cxl_dev_state in a private
> > accel driver struct and a macro helping for the allocation and
> > initialization of such struct.
> > 
> > The main problem is the amount of internal cxl structs which need to be
> > public for accel drivers which we tried to avoid since v1 of the type2
> > patchset history.
> 
> I have a suggestion or two in the next patch, but in my opinion the only
> potentially problematic exports are the cxl register structs. I'm assuming
> allowing accelerator drivers access to those may cause stale values in the
> CXL driver, but I haven't looked. The only suggestion I have there is to
> use pointers to the struct instead, but I imagine achieving could be a
> non-trivial amount of effort.

Hmm, say more about why you think those are a problem. Those are
"write-once at init" things, why would an accelerator driver have any
reason to play with those post init?

> The only other member that may be a problem is the DPA partition info,
> but I don't know enough about that to comment.

I think it is sufficient to mark the bulk of 'struct cxl_dev_state' as
private and then move things into the public space as needed, something
like:

struct cxl_dev_state {
/* public */
        struct device *dev;
        struct cxl_memdev *cxlmd;
/* private */
        struct cxl_register_map reg_map;
        struct cxl_regs regs;
        int cxl_dvsec;
        bool rcd;
        bool media_ready;
        struct resource dpa_res;
        struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
        unsigned int nr_partitions;
        u64 serial;
        enum cxl_devtype type;
        struct cxl_mailbox cxl_mbox;
};

...but otherwise let accelerator drivers see the full definition of that
struct for constainer_of() operations.