Re: [PATCH v4 12/14] cxl: Simplify cxl_rd_ops allocation and handling

[Robert Richter <rrichter@amd.com>]

Alison,

On 04.11.25 15:02:19, Alison Schofield wrote:
> On Mon, Nov 03, 2025 at 07:47:53PM +0100, Robert Richter wrote:
> > A root decoder's callback handlers are collected in struct cxl_rd_ops.
> > The structure is dynamically allocated, though it contains only a few
> > pointers in it. This also requires to check two pointes to check for
> > the existance of a callback.
> > 
> > Simplify the allocation, release and handler check by embedding the
> > ops statical in struct cxl_root_decoder.
> > 
> > Implementation is equivalent to how struct cxl_root_ops handles the
> > callbacks.
> 
> The allocation was intentionally dynamic because the root decoder ops
> only existed for CFMWS's defined w XOR Arithmetic.
> 
> From the commit msg:
> >> To avoid maintaining a static ops instance populated with mostly NULL
> >> pointers, allocate the ops structure dynamically only when a platform
> >> requires overrides (e.g. XOR interleave decoding).
> >> The setup can be extended as additional callbacks are added.
> See: 524b2b76f365 ("cxl: Move hpa_to_spa callback to a new root decoder ops structure")
> 
> Has the usage changed?

No, code did not change. In another patch I had to change struct
cxl_root and the cxl_root_ops allocation there. The pattern here is
the same and the code would benefit from the same change to simplify
it.

Dynamic allocation does not save much here (only a single pointer
entry) but requires resource allocation and a 2-level pointer check
which must be run even if the pointers are NULL. If there is at least
one pointer set (which becomes more likely with an increased number of
ops) you need to allocate the whole struct anyway and dynamic alloc
does net help then. The diffstat also shows the simplification:

 3 files changed, 8 insertions(+), 22 deletions(-)

-Robert