| Date | Wed, 10 Apr 2024 16:23:51 -0700 | From | Alison Schofield <> | Subject | Re: [PATCH 13/26] cxl/mem: Configure dynamic capacity interrupts |
| |
On Sun, Mar 24, 2024 at 04:18:16PM -0700, Ira Weiny wrote: > From: Navneet Singh <navneet.singh@intel.com> > > 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. Split irq configuration of memory events and > DCD events to allow for FW control of memory events while DCD is host > controlled. > > Configure DCD event log interrupts on devices supporting dynamic > capacity. Disable DCD if interrupts are not supported. > > Signed-off-by: Navneet Singh <navneet.singh@intel.com> > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > ---
snip
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 12cd5d399230..ef482eae09e9 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c
snip
> > +static int cxl_irqsetup(struct cxl_memdev_state *mds, > + struct cxl_event_interrupt_policy *policy, > + bool native_cxl) > +{ > + struct cxl_dev_state *cxlds = &mds->cxlds; > + int rc; > + > + if (native_cxl) { > + rc = cxl_event_irqsetup(mds, policy); > + if (rc) > + return rc; > + } > + > + if (cxl_dcd_supported(mds)) { > + rc = cxl_event_req_irq(cxlds, policy->dcd_settings); > + if (rc) { > + dev_err(cxlds->dev, "Failed to get interrupt for DCD event log\n"); move this..
> + cxl_disable_dcd(mds);
to after you've done the disabling...
dev_err(cxlds->dev, "DCD disabled: failed to get interrupt for event log\n"); > + return rc;
not sure I got the words right.
> + } > + } > + > + return 0; > +} > + > static bool cxl_event_int_is_fw(u8 setting) > { > u8 mode = FIELD_GET(CXLDEV_EVENT_INT_MODE_MASK, setting); > @@ -757,17 +793,25 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge, > struct cxl_memdev_state *mds, bool irq_avail) > { > struct cxl_event_interrupt_policy policy = { 0 }; > + bool native_cxl = host_bridge->native_cxl_error; > int rc; > > /* > * When BIOS maintains CXL error reporting control, it will process > * event records. Only one agent can do so. > + * > + * If BIOS has control of events and DCD is not supported skip event > + * configuration. > */ > - if (!host_bridge->native_cxl_error) > + if (!native_cxl && !cxl_dcd_supported(mds)) > return 0; > > if (!irq_avail) { > dev_info(mds->cxlds.dev, "No interrupt support, disable event processing.\n"); > + if (cxl_dcd_supported(mds)) { > + dev_info(mds->cxlds.dev, "DCD requires interrupts, disable DCD\n");
Similar here - Maybe better to disable, and just say it's done because this sounds a bit like a request to the user.
> + cxl_disable_dcd(mds);
dev_info(mds->cxlds.dev, "DCD disabled: no interrupt support\n");
How come this one is dev_info() and prior case of disabling was a dev_err()/
snip to end
-- Alison
|