| Message ID | 20230904132806.6094-3-Jonathan.Cameron@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/cxl: Minor CXL emulation fixes and cleanup | expand |
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>
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 >
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
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 > >
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, }, };