hw/cxl: Minor CXL emulation fixes and cleanup

[PATCH 2/4] hw/pci-bridge/cxl_upstream: Fix bandwidth entry base unit for SSLBIS

Posted by Jonathan Cameron via 1 year, 2 months ago

From: Dave Jiang <dave.jiang@intel.com>

According to ACPI spec 6.5 5.2.28.4 System Locality Latency and Bandwidth
Information Structure, if the "Entry Base Unit" is 1024 for BW and the
matrix entry has the value of 100, the BW is 100 GB/s. So the
entry_base_unit should be changed from 1000 to 1024 given the comment notes
it's 16GB/s for .latency_bandwidth.

Fixes: 882877fc359d ("hw/pci-bridge/cxl-upstream: Add a CDAT table access DOE")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 hw/pci-bridge/cxl_upstream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index 9159f48a8c..2b9cf0cc97 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -262,7 +262,7 @@ static int build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
                 .length = sslbis_size,
             },
             .data_type = HMATLB_DATA_TYPE_ACCESS_BANDWIDTH,
-            .entry_base_unit = 1000,
+            .entry_base_unit = 1024,
         },
     };
 
-- 
2.39.2

Re: [PATCH 2/4] hw/pci-bridge/cxl_upstream: Fix bandwidth entry base unit for SSLBIS

Posted by Michael Tokarev 1 year, 2 months ago

04.09.2023 16:28, Jonathan Cameron:
> From: Dave Jiang <dave.jiang@intel.com>
> 
> According to ACPI spec 6.5 5.2.28.4 System Locality Latency and Bandwidth
> Information Structure, if the "Entry Base Unit" is 1024 for BW and the
> matrix entry has the value of 100, the BW is 100 GB/s. So the
> entry_base_unit should be changed from 1000 to 1024 given the comment notes
> it's 16GB/s for .latency_bandwidth.
> 
> Fixes: 882877fc359d ("hw/pci-bridge/cxl-upstream: Add a CDAT table access DOE")
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   hw/pci-bridge/cxl_upstream.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
> index 9159f48a8c..2b9cf0cc97 100644
> --- a/hw/pci-bridge/cxl_upstream.c
> +++ b/hw/pci-bridge/cxl_upstream.c
> @@ -262,7 +262,7 @@ static int build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
>                   .length = sslbis_size,
>               },
>               .data_type = HMATLB_DATA_TYPE_ACCESS_BANDWIDTH,
> -            .entry_base_unit = 1000,
> +            .entry_base_unit = 1024,
>           },
>       };

BTW, is this one stable-worthly?  How it's been found, - due to some real
issue or just by code review?

Thanks,

/mjt

Re: [PATCH 2/4] hw/pci-bridge/cxl_upstream: Fix bandwidth entry base unit for SSLBIS

Posted by Dave Jiang 1 year, 2 months ago


On 9/22/23 13:08, Michael Tokarev wrote:
> 04.09.2023 16:28, Jonathan Cameron:
>> From: Dave Jiang <dave.jiang@intel.com>
>>
>> According to ACPI spec 6.5 5.2.28.4 System Locality Latency and Bandwidth
>> Information Structure, if the "Entry Base Unit" is 1024 for BW and the
>> matrix entry has the value of 100, the BW is 100 GB/s. So the
>> entry_base_unit should be changed from 1000 to 1024 given the comment notes
>> it's 16GB/s for .latency_bandwidth.
>>
>> Fixes: 882877fc359d ("hw/pci-bridge/cxl-upstream: Add a CDAT table access DOE")
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> ---
>>   hw/pci-bridge/cxl_upstream.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
>> index 9159f48a8c..2b9cf0cc97 100644
>> --- a/hw/pci-bridge/cxl_upstream.c
>> +++ b/hw/pci-bridge/cxl_upstream.c
>> @@ -262,7 +262,7 @@ static int build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
>>                   .length = sslbis_size,
>>               },
>>               .data_type = HMATLB_DATA_TYPE_ACCESS_BANDWIDTH,
>> -            .entry_base_unit = 1000,
>> +            .entry_base_unit = 1024,
>>           },
>>       };
> 
> BTW, is this one stable-worthly?  How it's been found, - due to some real
> issue or just by code review?

Code review. I was doing CXL CDAT parsing. So I guess I'm the first user. It's small enough that it won't make much of a difference for the resulting computed data. Mostly just correctness fixing. 
> 
> Thanks,
> 
> /mjt
> 
>

Re: [PATCH 2/4] hw/pci-bridge/cxl_upstream: Fix bandwidth entry base unit for SSLBIS

Posted by Fan Ni 1 year, 2 months ago

On Mon, Sep 04, 2023 at 02:28:04PM +0100, Jonathan Cameron wrote:
> From: Dave Jiang <dave.jiang@intel.com>
> 
> According to ACPI spec 6.5 5.2.28.4 System Locality Latency and Bandwidth
> Information Structure, if the "Entry Base Unit" is 1024 for BW and the
> matrix entry has the value of 100, the BW is 100 GB/s. So the
> entry_base_unit should be changed from 1000 to 1024 given the comment notes
> it's 16GB/s for .latency_bandwidth.
> 
> Fixes: 882877fc359d ("hw/pci-bridge/cxl-upstream: Add a CDAT table access DOE")
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Fan Ni <fan.ni@samsung.com>

> ---
>  hw/pci-bridge/cxl_upstream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
> index 9159f48a8c..2b9cf0cc97 100644
> --- a/hw/pci-bridge/cxl_upstream.c
> +++ b/hw/pci-bridge/cxl_upstream.c
> @@ -262,7 +262,7 @@ static int build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
>                  .length = sslbis_size,
>              },
>              .data_type = HMATLB_DATA_TYPE_ACCESS_BANDWIDTH,
> -            .entry_base_unit = 1000,
> +            .entry_base_unit = 1024,
>          },
>      };
>  
> -- 
> 2.39.2
>

Re: [PATCH 2/4] hw/pci-bridge/cxl_upstream: Fix bandwidth entry base unit for SSLBIS

Posted by Philippe Mathieu-Daudé 1 year, 2 months ago

On 4/9/23 15:28, Jonathan Cameron wrote:
> From: Dave Jiang <dave.jiang@intel.com>
> 
> According to ACPI spec 6.5 5.2.28.4 System Locality Latency and Bandwidth
> Information Structure, if the "Entry Base Unit" is 1024 for BW and the
> matrix entry has the value of 100, the BW is 100 GB/s. So the
> entry_base_unit should be changed from 1000 to 1024 given the comment notes
> it's 16GB/s for .latency_bandwidth.
> 
> Fixes: 882877fc359d ("hw/pci-bridge/cxl-upstream: Add a CDAT table access DOE")
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>   hw/pci-bridge/cxl_upstream.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>