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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 22 Mar 2023 10:33:00 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> 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>

As a side note, I don't think the CDAT spec doesn't say which version of ACPI we
should refer to for meaning of HMAT entries.  It should set a minimum given there were breaking
changes in the HMAT spec from versions 1 to 2.

Perhaps worth poking the author of that doc?  Posting here in the hope that
someone (maybe me) remembers to do so.

I'll pick this up for my qemu tree and roll into a misc fixes series once I get
/ make time to pull one together.

Thanks,

Jonathan




> ---
>  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 2a00b708e560..d6f19c859a3d 100644
> --- a/hw/pci-bridge/cxl_upstream.c
> +++ b/hw/pci-bridge/cxl_upstream.c
> @@ -297,7 +297,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,
>          },
>      };
>  
> 
> 
>