| Message ID | 167995345388.2857312.2421270054519644444.stgit@djiang5-mobl3 |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Add support for QTG ID retrieval for CXL subsystem | expand |
On Mon, Mar 27, 2023 at 02:44:13PM -0700, Dave Jiang wrote: > A CDAT table is available from a CXL device. The table is read by the > driver and cached in software. With the CXL subsystem needing to parse the > CDAT table, the checksum should be verified. Add checksum verification > after the CDAT table is read from device. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > > --- > v2: > - Drop ACPI checksum export and just use local verification. (Dan) > --- > drivers/cxl/core/pci.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 25b7e8125d5d..e0d5e6525c0d 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev, > return 0; > } > > +static unsigned char cdat_checksum(void *buf, size_t size) > +{ > + unsigned char sum, *data = buf; > + size_t i; > + > + for (sum = 0, i = 0; i < size; i++) > + sum += data[i]; > + return 0 - sum; This return value isn't obvious to me. What's happening here? Thanks for explaining, Alison > +} > + > /** > * read_cdat_data - Read the CDAT data on this port > * @port: Port to read data from > @@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port) > } > > port->cdat.table = cdat_table + sizeof(__le32); > + if (cdat_checksum(port->cdat.table, cdat_length)) { > + /* Don't leave table data allocated on error */ > + devm_kfree(dev, cdat_table); > + dev_err(dev, "CDAT data checksum error\n"); > + } > + > port->cdat.length = cdat_length; > } > EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL); > >
On 3/28/23 5:03 PM, Alison Schofield wrote: > On Mon, Mar 27, 2023 at 02:44:13PM -0700, Dave Jiang wrote: >> A CDAT table is available from a CXL device. The table is read by the >> driver and cached in software. With the CXL subsystem needing to parse the >> CDAT table, the checksum should be verified. Add checksum verification >> after the CDAT table is read from device. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> >> --- >> v2: >> - Drop ACPI checksum export and just use local verification. (Dan) >> --- >> drivers/cxl/core/pci.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 25b7e8125d5d..e0d5e6525c0d 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev, >> return 0; >> } >> >> +static unsigned char cdat_checksum(void *buf, size_t size) >> +{ >> + unsigned char sum, *data = buf; >> + size_t i; >> + >> + for (sum = 0, i = 0; i < size; i++) >> + sum += data[i]; >> + return 0 - sum; > > This return value isn't obvious to me. What's happening here? > Thanks for explaining, The expectation is that all the bytes add up to be equal 0 for the checksum verification. So if we get anything other than 0, the check would fail. DJ > > Alison > >> +} >> + >> /** >> * read_cdat_data - Read the CDAT data on this port >> * @port: Port to read data from >> @@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port) >> } >> >> port->cdat.table = cdat_table + sizeof(__le32); >> + if (cdat_checksum(port->cdat.table, cdat_length)) { >> + /* Don't leave table data allocated on error */ >> + devm_kfree(dev, cdat_table); >> + dev_err(dev, "CDAT data checksum error\n"); >> + } >> + >> port->cdat.length = cdat_length; >> } >> EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL); >> >>
Dave Jiang wrote: > A CDAT table is available from a CXL device. The table is read by the > driver and cached in software. With the CXL subsystem needing to parse the > CDAT table, the checksum should be verified. Add checksum verification > after the CDAT table is read from device. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 25b7e8125d5d..e0d5e6525c0d 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev, return 0; } +static unsigned char cdat_checksum(void *buf, size_t size) +{ + unsigned char sum, *data = buf; + size_t i; + + for (sum = 0, i = 0; i < size; i++) + sum += data[i]; + return 0 - sum; +} + /** * read_cdat_data - Read the CDAT data on this port * @port: Port to read data from @@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port) } port->cdat.table = cdat_table + sizeof(__le32); + if (cdat_checksum(port->cdat.table, cdat_length)) { + /* Don't leave table data allocated on error */ + devm_kfree(dev, cdat_table); + dev_err(dev, "CDAT data checksum error\n"); + } + port->cdat.length = cdat_length; } EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
A CDAT table is available from a CXL device. The table is read by the driver and cached in software. With the CXL subsystem needing to parse the CDAT table, the checksum should be verified. Add checksum verification after the CDAT table is read from device. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- v2: - Drop ACPI checksum export and just use local verification. (Dan) --- drivers/cxl/core/pci.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)