Re: [PATCH v4 5/9] cxl/region, dax/hmem: Arbitrate Soft Reserved ownership with cxl_regions_fully_map()

[<dan.j.williams@intel.com>]

Smita Koralahalli wrote:
> Introduce cxl_regions_fully_map() to check whether CXL regions form a
> single contiguous, non-overlapping cover of a given Soft Reserved range.
> 
> Use this helper to decide whether Soft Reserved memory overlapping CXL
> regions should be owned by CXL or registered by HMEM.
> 
> If the span is fully covered by CXL regions, treat the Soft Reserved
> range as owned by CXL and have HMEM skip registration. Else, let HMEM
> claim the range and register the corresponding devdax for it.

This all feels a bit too custom when helpers like resource_contains()
exist.

Also remember that the default list of soft-reserved ranges that dax
grabs is filtered by the ACPI HMAT. So while there is a chance that one
EFI memory map entry spans multiple CXL regions, there is a lower chance
that a single ACPI HMAT range spans multiple CXL regions.

I think it is fair for Linux to be simple and require that an algorithm
of:

cxl_contains_soft_reserve()
    for_each_cxl_intersecting_hmem_resource()
        found = false
        for_each_region()
           if (resource_contains(cxl_region_resource, hmem_resource))
               found = true
        if (!found)
            return false
    return true

...should be good enough, otherwise fallback to pure hmem operation, and
do not worry about the corner cases.

If Linux really needs to understand that ACPI HMAT ranges may span
multiple CXL regions then I would want to understand more what is
driving that configuration.

Btw, I do not see a:

    guard(rwsem_read)(&cxl_rwsem.region)

...anywhere in the proposed patch. That needs to be held be sure the
region's resource settings are not changed out from underneath you. This
should probably also be checking that the region is in the commit state
because it may still be racing regions under creation post
wait_for_device_probe().

>  void cxl_endpoint_parse_cdat(struct cxl_port *port);
> diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
> index f70a0688bd11..db4c46337ac3 100644
> --- a/drivers/dax/hmem/hmem.c
> +++ b/drivers/dax/hmem/hmem.c
> @@ -3,6 +3,8 @@
>  #include <linux/memregion.h>
>  #include <linux/module.h>
>  #include <linux/dax.h>
> +
> +#include "../../cxl/cxl.h"
>  #include "../bus.h"
>  
>  static bool region_idle;
> @@ -150,7 +152,17 @@ static int hmem_register_device(struct device *host, int target_nid,
>  static int handle_deferred_cxl(struct device *host, int target_nid,
>  			       const struct resource *res)
>  {
> -	/* TODO: Handle region assembly failures */
> +	if (region_intersects(res->start, resource_size(res), IORESOURCE_MEM,
> +			      IORES_DESC_CXL) != REGION_DISJOINT) {
> +
> +		if (cxl_regions_fully_map(res->start, res->end))
> +			dax_cxl_mode = DAX_CXL_MODE_DROP;
> +		else
> +			dax_cxl_mode = DAX_CXL_MODE_REGISTER;
> +
> +		hmem_register_device(host, target_nid, res);
> +	}
> +

I think there is enough content to just create the new
cxl_contains_soft_reserve() ABI, and then hookup handle_deferred_cxl in
a follow-on patch.