Re: [PATCH v12 5/8] cxl/pci: Factor out interrupt policy check

On Fri, Jul 31, 2026 at 01:48:10AM -0700, Anisa Su wrote:
> From: Ira Weiny <[email protected]>
> 
> Dynamic Capacity Devices (DCD) require event interrupts to process
> memory addition or removal.  BIOS may have control over non-DCD event
> processing.  DCD interrupt configuration needs to be separate from
> memory event interrupt configuration.
> 
> Factor out event interrupt setting validation.

Hi Anisa,

"Validation" seems like the wrong term here. The helper checks for
firmware ownership of the memory event logs. It is not validating
the event interrupt settings.

Which leads to my next comment -

snip
> 
>  
> +static bool cxl_event_validate_mem_policy(struct cxl_memdev_state *mds,
> +					  struct cxl_event_interrupt_policy *policy)
> +{
> +	if (cxl_event_int_is_fw(policy->info_settings) ||
> +	    cxl_event_int_is_fw(policy->warn_settings) ||
> +	    cxl_event_int_is_fw(policy->failure_settings) ||
> +	    cxl_event_int_is_fw(policy->fatal_settings)) {
> +		dev_err(mds->cxlds.dev,
> +			"FW still in control of Event Logs despite _OSC settings\n");
> +		return false;
> +	}
> +
> +	return true;
> +}

Why such a general name here? This helper seems to have one very specific purpose.

cxl_event_validate_mem_policy() doesn't validate the policy. It checks whether
firmware still owns the standard memory event logs despite _OSC granting OS
control. I'd prefer naming it after that condition rather than "validate".
Something like cxl_event_fw_owns_mem_logs() reads much closer to what the
code actually does.

--Alison


> +
>  static int cxl_event_config(struct pci_host_bridge *host_bridge,
>  			    struct cxl_memdev_state *mds, bool irq_avail)
>  {
> @@ -661,14 +676,8 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
>  	if (rc)
>  		return rc;
>  
> -	if (cxl_event_int_is_fw(policy.info_settings) ||
> -	    cxl_event_int_is_fw(policy.warn_settings) ||
> -	    cxl_event_int_is_fw(policy.failure_settings) ||
> -	    cxl_event_int_is_fw(policy.fatal_settings)) {
> -		dev_err(mds->cxlds.dev,
> -			"FW still in control of Event Logs despite _OSC settings\n");
> +	if (!cxl_event_validate_mem_policy(mds, &policy))
>  		return -EBUSY;
> -	}
>  
>  	rc = cxl_event_config_msgnums(mds, &policy);
>  	if (rc)
> -- 
> 2.43.0
>