Re: [PATCH 4/5] cxl: Set type of region to that of the first endpoint
From: Dan Williams
Date: Fri Oct 18 2024 - 17:20:08 EST
Huang, Ying wrote:
> Dan Williams <dan.j.williams@xxxxxxxxx> writes:
>
> > Huang Ying wrote:
>
> [snip]
>
> >> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> >> index 21b877d8582f..d709738ada61 100644
> >> --- a/drivers/cxl/core/region.c
> >> +++ b/drivers/cxl/core/region.c
> >> @@ -1926,7 +1926,10 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> >> return -ENXIO;
> >> }
> >>
> >> - if (cxled->cxld.target_type != cxlr->type) {
> >> + /* Set the type of region to that of the first endpoint */
> >> + if (cxlr->type == CXL_DECODER_INVALID) {
> >> + cxlr->type = cxled->cxld.target_type;
> >> + } else if (cxled->cxld.target_type != cxlr->type) {
> >
> > No, the type of the region is determined by the caller and should be
> > gated by the region capability. For type-2 region creation I doubt
> > userspace is going to be creating those vs the accelerator so this all
> > seems backwards to me.
>
> How do we determine the type of the endpoint? Specify it in type2/type3
> device driver?
Why does the endpoint type matter? Memory expansion can be supported by
HDM-D[B], and an accelerator could have one or more HDM-H decoders.
> If so, we will specify the type of both the endpoint and the region in
> type2/type3 device driver. Then, why not only specify the type of the
> endpoint? The type of region can be determined from the type of the
> endpoint.
Because CXL HDM protocol is per decoder not per device.