| Message ID | 20250410010545.99590-1-lizhijian@fujitsu.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] cxl/acpi: Verify CHBS length for CXL2.0 | expand |
Li Zhijian wrote: > Per CXL Spec r3.1 Table 9-21, both CXL1.1 and CXL2.0 have defined their > own length, verify it to avoid an invalid CHBS. > > Additionally, this patch also removes the redundant macro CXL_RCRB_SIZE, > favoring the use of `ACPI_CEDT_CHBS_LENGTH_CXL11`, which offers clearer > semantic meaning. Right, it was a mistake to allow the CXL_RCRB_SIZE definition to leak into the kernel in v6.2 when ACPI_CEDT_CHBS_LENGTH_CXL11 had existed since v5.14. > > Reviewed-by: Gregory Price <gourry@gourry.net> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > V3: > - say more words in removing CXL_RCRB_SIZE # Alison > - Collected Reviewed-by > V2: don't factor out, just validate # Dan > --- > drivers/cxl/acpi.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index cb14829bb9be..2e63e50b2c40 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -11,8 +11,6 @@ > #include "cxlpci.h" > #include "cxl.h" > > -#define CXL_RCRB_SIZE SZ_8K > - > struct cxl_cxims_data { > int nr_maps; > u64 xormaps[] __counted_by(nr_maps); > @@ -478,8 +476,10 @@ static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg, > > chbs = (struct acpi_cedt_chbs *) header; > > - if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && > - chbs->length != CXL_RCRB_SIZE) > + if ((chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && > + chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL11) || > + (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20 && > + chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL20)) > return 0; I have a small readability preference for: if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL11) return 0; if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20 && chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL20) return 0; ...but not enough to ask you to respin this again. Reviewed-by: Dan Williams <dan.j.williams@intel.com>
On 4/9/25 6:05 PM, Li Zhijian wrote: > Per CXL Spec r3.1 Table 9-21, both CXL1.1 and CXL2.0 have defined their > own length, verify it to avoid an invalid CHBS. > > Additionally, this patch also removes the redundant macro CXL_RCRB_SIZE, > favoring the use of `ACPI_CEDT_CHBS_LENGTH_CXL11`, which offers clearer > semantic meaning. > > Reviewed-by: Gregory Price <gourry@gourry.net> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> Applied to cxl/next with Dan's suggestion. > --- > V3: > - say more words in removing CXL_RCRB_SIZE # Alison > - Collected Reviewed-by > V2: don't factor out, just validate # Dan > --- > drivers/cxl/acpi.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index cb14829bb9be..2e63e50b2c40 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -11,8 +11,6 @@ > #include "cxlpci.h" > #include "cxl.h" > > -#define CXL_RCRB_SIZE SZ_8K > - > struct cxl_cxims_data { > int nr_maps; > u64 xormaps[] __counted_by(nr_maps); > @@ -478,8 +476,10 @@ static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg, > > chbs = (struct acpi_cedt_chbs *) header; > > - if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && > - chbs->length != CXL_RCRB_SIZE) > + if ((chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && > + chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL11) || > + (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20 && > + chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL20)) > return 0; > > if (!chbs->base)
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index cb14829bb9be..2e63e50b2c40 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -11,8 +11,6 @@ #include "cxlpci.h" #include "cxl.h" -#define CXL_RCRB_SIZE SZ_8K - struct cxl_cxims_data { int nr_maps; u64 xormaps[] __counted_by(nr_maps); @@ -478,8 +476,10 @@ static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg, chbs = (struct acpi_cedt_chbs *) header; - if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && - chbs->length != CXL_RCRB_SIZE) + if ((chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 && + chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL11) || + (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL20 && + chbs->length != ACPI_CEDT_CHBS_LENGTH_CXL20)) return 0; if (!chbs->base)