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

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

On Tue, Sep 08, 2026 at 11:14:14PM +0100, Jonathan Cameron wrote:
> On Tue,  8 Sep 2026 03:15:10 -0700
> Anisa Su <anisa.su887@gmail.com> 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.
> > 
> > The DCD event log is not drained here. cxl_event_drain_mask() reports
> > only the logs the driver can service, and the DCD bit is added by
> > the patch introducing DCD event handling. Until then a DCD interrupt
> > wakes the event thread, which finds no log it owns and returns.
> > 
> > Care is taken to preserve the interrupt policy set by the FW if FW first
> > has been selected by the BIOS.
> > 
> > Based on an original patch by Navneet Singh.
> > 
> > Signed-off-by: Ira Weiny <iweiny@kernel.org>
> > Signed-off-by: Anisa Su <anisa.su@samsung.com>
> > Tested-by: Wonjae Lee <wj28.lee@samsung.com>
> > Tested-by: Junhee Park <jh9934.park@samsung.com>
> > Tested-by: Heesoo Kim <habil.kim@samsung.com>
> > 
> 
> One minor thing inline.  Otherwise LGTM.
> 
> Jonathan
> 
> > ---
> >  drivers/cxl/cxl.h    |   4 +-
> >  drivers/cxl/cxlmem.h |   2 +
> >  drivers/cxl/pci.c    | 124 ++++++++++++++++++++++++++++++++++++-------
> >  3 files changed, 109 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index cab8ce39f465..d6e02bd0cf50 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -192,11 +192,13 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
> >  #define CXLDEV_EVENT_STATUS_WARN		BIT(1)
> >  #define CXLDEV_EVENT_STATUS_FAIL		BIT(2)
> >  #define CXLDEV_EVENT_STATUS_FATAL		BIT(3)
> > +#define CXLDEV_EVENT_STATUS_DCD			BIT(4)
> >  
> >  #define CXLDEV_EVENT_STATUS_ALL (CXLDEV_EVENT_STATUS_INFO |	\
> >  				 CXLDEV_EVENT_STATUS_WARN |	\
> >  				 CXLDEV_EVENT_STATUS_FAIL |	\
> > -				 CXLDEV_EVENT_STATUS_FATAL)
> > +				 CXLDEV_EVENT_STATUS_FATAL |	\
> > +				 CXLDEV_EVENT_STATUS_DCD)
> 
> See below. I think this define has become a bad idea, making things
> more confusing rather than simpler.
> 
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index 7b754af2168d..1ea505b1f40e 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -510,11 +510,23 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
> >  	return true;
> >  }
> >  
> > +/* Event logs the driver drains: standard logs when native_cxl */
> > +static u32 cxl_event_drain_mask(struct pci_host_bridge *host_bridge)
> > +{
> > +	if (host_bridge->native_cxl_error)
> > +		return CXLDEV_EVENT_STATUS_ALL & ~CXLDEV_EVENT_STATUS_DCD;
> 
> This seems a little backwards.    Better I think to just list the logs that
> are supported. There are other logs coming and I think we'd want to opt into
> them deliberately like you are doing for DCD rather than have them caught
> by the _ALL define.
> 
> 	if (host_bridge->native_cxl_error)
> 		return CXLDEV_EVENT_STATUS_INFO | CXLDEV_EVENT_STATUS_WARN |
> 		       CXLDEV_EVENT_STATUS_FAIL | CXLDEV_EVENT_STATUS_FATAL;
Applied.

Thanks,
Anisa
> > +
> > +	return 0;
> > +}
> 
>