Re: [PATCH 2/4] hw/pci-bridge/cxl_upstream: Fix bandwidth entry base unit for SSLBIS
On 9/22/23 13:08, Michael Tokarev wrote:
> 04.09.2023 16:28, Jonathan Cameron:
>> From: Dave Jiang
>>
>> 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
>> Signed-off-by: Jonathan Cameron
>> ---
>> 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
04.09.2023 16:28, Jonathan Cameron:
From: Dave Jiang
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
Signed-off-by: Jonathan Cameron
---
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
On Mon, Sep 04, 2023 at 02:28:04PM +0100, Jonathan Cameron wrote:
> From: Dave Jiang
>
> 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
> Signed-off-by: Jonathan Cameron
Reviewed-by: Fan Ni
> ---
> 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
On 4/9/23 15:28, Jonathan Cameron wrote:
From: Dave Jiang
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
Signed-off-by: Jonathan Cameron
---
hw/pci-bridge/cxl_upstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé
