Re: [PATCH 1/3] cxl: Set target type of root decoder based on CFMWS restrictions

["Huang, Ying" <ying.huang@intel.com>]

Hi, Jonathan,

Jonathan Cameron <Jonathan.Cameron@Huawei.com> writes:

> On Mon, 29 Jul 2024 16:46:09 +0800
> Huang Ying <ying.huang@intel.com> wrote:
>
>> Now, the target type of root decoder is hard-coded to HOSTONLYMEM,
>> because only type3 expanders are supported.  To support type2
>> accelerators, set the target type of root decoder based on the
>> window restrictions field of CFMWS entry.
>> 
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Cc: Davidlohr Bueso <dave@stgolabs.net>
>> Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
>> Cc: Dave Jiang <dave.jiang@intel.com>
>> Cc: Alison Schofield <alison.schofield@intel.com>
>> Cc: Vishal Verma <vishal.l.verma@intel.com>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Cc: Alejandro Lucero <alucerop@amd.com>
>> ---
>>  drivers/cxl/acpi.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index 82b78e331d8e..40c92ad29122 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -382,7 +382,10 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>>  
>>  	cxld = &cxlrd->cxlsd.cxld;
>>  	cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
>> -	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
>> +	if (cxld->flags & CXL_DECODER_F_TYPE2)
>
> These flags need updating or we are going to run into problems
> long term.
>
> As of more recent specs, the distinction is messier than it was and
> it's device coherent HDM-D / HDM-DB (second one being type2 or type3 with
> BI support) and/or Host only coherent HDM-H.

I got your idea.  Previously, Device Coherent (HDM-D/DB) means type2
devices, while Host-only Coherent (HDM-H) means type3 devices.  But in
recent specs, type3 devices could be HDM-DB too.  So, we should rename
ACPI_CEDT_CFMWS_RESTRICT_TYPEX and CXL_DECODER_F_TYPEX.  What's your
suggestion for the new name? _DEVMEM and _HOSTONLYMEM?

> I'm curious on whether anyone is support both on same CFWMS?
> I believe it is possible and the spec doesn't rule it out.

This sounds possible.

> Jonathan
>
>
>> +		cxld->target_type = CXL_DECODER_DEVMEM;
>> +	else
>> +		cxld->target_type = CXL_DECODER_HOSTONLYMEM;
>>  	cxld->hpa_range = (struct range) {
>>  		.start = cfmws->base_hpa,
>>  		.end = cfmws->base_hpa + cfmws->window_size - 1,

--
Best Regards,
Huang, Ying