Re: [PATCH v12 6/8] cxl/mem: Configure dynamic capacity interrupts

[Anisa Su <anisa.su887@gmail.com>]

On Mon, Aug 03, 2026 at 05:34:45PM -0700, Alison Schofield wrote:
> On Fri, Jul 31, 2026 at 01:48:11AM -0700, Anisa Su wrote:
> > From: Ira Weiny <iweiny@kernel.org>
> > 
> > Dynamic Capacity Devices (DCD) support extent change notifications
> > through the event log mechanism.  The interrupt mailbox commands were
> > extended in CXL 3.1 to support these notifications.  Firmware can't
> > configure DCD events to be FW controlled but can retain control of
> > memory events.
> > 
> > Configure DCD event log interrupts on devices supporting dynamic
> > capacity.  Disable DCD if interrupts are not supported.
> > 
> > Care is taken to preserve the interrupt policy set by the FW if FW first
> > has been selected by the BIOS.
> 
> Hi Anisa,
> 
> I gave one comment in response to a Sashiko comment, wrt 'drain'.
> Another below-
> 
> 
> > +
> > +	/*
> > +	 * A CXL 3.0+ device can carry dcd_settings field without DCD command
> > +	 * support, so size the request by the device's policy_size and only
> > +	 * enable the DCD interrupt when DCD commands are supported.
> > +	 */
> > +	if (cxl_dcd_supported(mds))
> > +		policy->dcd_settings = CXL_INT_MSI_MSIX;
> >  
> >  	mbox_cmd = (struct cxl_mbox_cmd) {
> >  		.opcode = CXL_MBOX_OP_SET_EVT_INT_POLICY,
> >  		.payload_in = policy,
> > -		.size_in = sizeof(*policy),
> > +		.size_in = policy_size,
> >  	};
> 
> Can you help me understand what happens if policy_size is only 4 bytes.
> dcd_settings is never sent in the Set command. How do we know cxl_irqsetup()
> isn't using a value the device never accepted?
> 
Hmmm... good catch. If a non spec-compliant device reports support for all four 48h commands in the
CEL but simultaneously reports a 4 byte policy_size, the device will never see
policy->dcd_settings. Then the re-read at the end of cxl_event_config_msgnums()
doesn't erase the value that was never really set on the device:

	/* Retrieve final interrupt settings */
	return cxl_event_get_int_policy(mds, policy, NULL);


^ reads into the same struct, the device returns 4 bytes again, and byte 5
keeps the value the driver just wrote. So "final interrupt settings" is
the driver's own value round-tripping back for that field, and
cxl_irqsetup() arms the DCD interrupt from it.

A spec-compliant device can't trigger this, since cxl_dcd_supported() requires all
four 48h commands in the CEL, DCD is CXL 3.x, and a 3.x Get Event
Interrupt Policy returns the 5 byte policy including dcd_settings. But to
enforce it explicitly in the driver, added the check below in cxl_event_config(),
before cxl_event_config_msgnums():

	/*
	 * dcd_settings cannot be sent to a device whose policy is too short
	 * to hold it, so the device would never accept the setting
	 * cxl_irqsetup() arms the DCD interrupt with.
	 */
	if (cxl_dcd_supported(mds) && policy_size < sizeof(policy)) {
		dev_warn(mds->cxlds.dev,
			 "DCD supported but interrupt policy is only %zu bytes\n",
			 policy_size);
		cxl_disable_dcd(mds);
	}

Disabling DCD makes cxl_dcd_supported() false, so the Set stops setting
dcd_settings and cxl_irqsetup() skips the DCD interrupt.

Thanks,
Anisa

> 
> >  	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> > @@ -596,7 +635,7 @@ static int cxl_event_config_msgnums(struct cxl_memdev_state *mds,
> >  	}
> >  
> >  	/* Retrieve final interrupt settings */
> > -	return cxl_event_get_int_policy(mds, policy);
> > +	return cxl_event_get_int_policy(mds, policy, NULL);
> >  }
> 
> snip
>