Re: [PATCH v3 2/4] cxl: Update Soft Reserved resources upon region creation

[Bowman, Terry]

On 4/4/2025 8:32 AM, Jonathan Cameron wrote:
> On Thu, 3 Apr 2025 13:33:13 -0500
> Terry Bowman <terry.bowman@xxxxxxx> wrote:
> 
>> From: Nathan Fontenot <nathan.fontenot@xxxxxxx>
>>
>> Update handling of SOFT RESERVE iomem resources that intersect with
>> CXL region resources to remove intersections from the SOFT RESERVE
>> resources. The current approach of leaving SOFT RESERVE resources as
>> is can cause failures during hotplug replace of CXL devices because
>> the resource is not available for reuse after teardown of the CXL device.
>>
>> To accomplish this the cxl acpi driver creates a worker thread at the
> 
> Inconsistent in capitalization. I'd just use CXL ACPI here given you used CXL PCI
> below.
> 

Thanks. I will update.

>> end of cxl_acpi_probe(). This worker thread first waits for the CXL PCI
>> CXL mem drivers have loaded. The cxl core/suspend.c code is updated to
>> add a pci_loaded variable, in addition to the mem_active variable, that
>> is updated when the pci driver loads. Remove CONFIG_CXL_SUSPEND Kconfig as
>> it is no longer needed. A new cxl_wait_for_pci_mem() routine uses a
>> waitqueue for both these driver to be loaded. The need to add this
>> additional waitqueue is ensure the CXL PCI and CXL mem drivers have loaded
>> before we wait for their probe, without it the cxl acpi probe worker thread
>> calls wait_for_device_probe() before these drivers are loaded.
>>
>> After the CXL PCI and CXL mem drivers load the cxl acpi worker thread
> CXL ACPI
> 
>> uses wait_for_device_probe() to ensure device probe routines have
>> completed.
> 
> Does it matter if these drivers go away again?  Everything seems
> to be one way at the moment.
> 

There is a maximum timeout wait period. I'll add these details to the 
commit message here.

>>
>> Once probe completes and regions have been created, find all cxl
> 
> CXL
> 
>> regions that have been created and trim any SOFT RESERVE resources
>> that intersect with the region.
>>
>> Update cxl_acpi_exit() to cancel pending waitqueue work.
>>
>> Signed-off-by: Nathan Fontenot <nathan.fontenot@xxxxxxx>
>> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
> 
> 
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index be8a7dc77719..40835ec692c8 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -858,6 +858,7 @@ bool is_cxl_pmem_region(struct device *dev);
>>  struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
>>  int cxl_add_to_region(struct cxl_port *root,
>>  		      struct cxl_endpoint_decoder *cxled);
>> +int cxl_region_srmem_update(void);
> 
> As before: srmem is a bit obscure. Maybe spell it out more.
> 

Yes, will update.

-Terry

>>  struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
>>  u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
>>  #else
>> @@ -902,6 +903,8 @@ void cxl_coordinates_combine(struct access_coordinate *out,
>>  
>>  bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
>>  
>> +void cxl_wait_for_pci_mem(void);
>