On Mon, 06 Feb 2023 13:49:48 -0700
Dave Jiang <dave.jiang@xxxxxxxxx> 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@xxxxxxxxx>
Hi Dave,
Some comments on this follow on from previous patch so may not
be relevant once you've updated how that is done.
Jonathan
> ---
> drivers/cxl/core/pci.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 57764e9cd19d..a24dac36bedd 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -3,6 +3,7 @@
> #include <linux/io-64-nonatomic-lo-hi.h>
> #include <linux/device.h>
> #include <linux/delay.h>
> +#include <linux/acpi.h>
> #include <linux/pci.h>
> #include <linux/pci-doe.h>
> #include <cxlpci.h>
> @@ -592,6 +593,7 @@ void read_cdat_data(struct cxl_port *port)
> struct device *dev = &port->dev;
> struct device *uport = port->uport;
> size_t cdat_length;
> + acpi_status status;
> int rc;
>
> cdat_doe = find_cdat_doe(uport);
> @@ -620,5 +622,14 @@ void read_cdat_data(struct cxl_port *port)
> port->cdat.length = 0;
> dev_err(dev, "CDAT data read error\n");
> }
> +
> + status = acpi_ut_verify_cdat_checksum(port->cdat.table, port->cdat.length);
> + if (status != AE_OK) {
if (ACPI_FAILURE(acpi_ut...)) or better still put that in the wrapper I suggeste
in previous patch so that we have normal kernel return code handling out here.
> + /* Don't leave table data allocated on error */
> + devm_kfree(dev, port->cdat.table);
> + port->cdat.table = NULL;
> + port->cdat.length = 0;
I'd rather see us manipulate a local copy of cdat_length, and cdat_table
then only assign them to the port->cdat fields the success path rather
than setting them then unsetting the on error.
Diff will be bigger, but nicer resulting code (and hopefully diff
won't be too big!)
> + dev_err(dev, "CDAT data checksum error\n");
> + }
> }
> EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>
>
