On 6/2/26 2:22 AM, Anisa Su wrote:
> On Wed, May 27, 2026 at 05:16:44PM -0700, Dave Jiang wrote:
>>
>>
>> On 5/23/26 2:43 AM, Anisa Su wrote:
>>> From: Ira Weiny <[email protected]>
< --snip -->
>>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>>> index a7f71f36531f..2d33001dac26 100644
>>> --- a/drivers/cxl/core/port.c
>>> +++ b/drivers/cxl/core/port.c
>>> @@ -337,6 +337,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
>>> &dev_attr_qos_class.attr,
>>> SET_CXL_REGION_ATTR(create_pmem_region)
>>> SET_CXL_REGION_ATTR(create_ram_region)
>>> + SET_CXL_REGION_ATTR(create_dynamic_ram_a_region)
>>
>> With this add, may need to add checks in cxl_root_decoder_visible() for
>> dynamic_ram for create and also delete.
>>
> So for this check, since there's no CXL_DECODER_F_ bit defined for DCD, I
> considered
> traversing through all endpoints and seeing if they have a DYNAMIC_RAM_A
> partition, but that traversal already happens in the store_targetN() path,
> which also includes the mode mismatch check.
>
> Specifically, in cxl_region_attach:
>
> if (cxlds->part[cxled->part].mode != cxlr->mode) {
> dev_dbg(&cxlr->dev, "%s region mode: %d mismatch\n",
> dev_name(&cxled->cxld.dev), cxlr->mode);
> return -EINVAL;
> }
>
> Is it sufficient here to prohibit creating a dynamic ram region if the root
> decoder does not support ram?
>
> if (a == CXL_REGION_ATTR(create_dynamic_ram_a_region) &&
> !can_create_ram(cxlrd))
> return 0;
>
I think so.
>>> SET_CXL_REGION_ATTR(delete_region)
>>> NULL,
>>> };
>>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>>> index edc267c6cf77..7561bf3d8af8 100644
>>> --- a/drivers/cxl/core/region.c
>>> +++ b/drivers/cxl/core/region.c
>>> @@ -493,6 +493,11 @@ static int set_interleave_ways(struct cxl_region
>>> *cxlr, int val)
>>> int save, rc;
>>> u8 iw;
>>>
>>> + if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM_A && val != 1) {
>>> + dev_err(&cxlr->dev, "Interleaving and DCD not supported\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> rc = ways_to_eiw(val, &iw);
>>> if (rc)
>>> return rc;
>>> @@ -2389,6 +2394,7 @@ static size_t store_targetN(struct cxl_region *cxlr,
>>> const char *buf, int pos,
>>> if (sysfs_streq(buf, "\n"))
>>> rc = detach_target(cxlr, pos);
>>> else {
>>> + struct cxl_endpoint_decoder *cxled;
>>> struct device *dev;
>>>
>>> dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf);
>>> @@ -2400,8 +2406,14 @@ static size_t store_targetN(struct cxl_region *cxlr,
>>> const char *buf, int pos,
>>> goto out;
>>> }
>>>
>>> - rc = attach_target(cxlr, to_cxl_endpoint_decoder(dev), pos,
>>> - TASK_INTERRUPTIBLE);
>>> + cxled = to_cxl_endpoint_decoder(dev);
>>> + if (cxlr->mode == CXL_PARTMODE_DYNAMIC_RAM_A &&
>>> + !cxl_dcd_supported(cxled_to_mds(cxled))) {
>>
>> cxled_to_mds() can return NULL with the earlier change suggested. Need to
>> handle that
>>
> Fixed
>> DJ
>>
> Thanks,
> Anisa
>
> Also, for potential future support for multiple DC partitions not to be
> awkward, I
> think it would make sense to rename dynamic_ram_a to dynamic_ram_1. Any
> objections?
No objections from me. Seems reasonable.
