Skip to content

Commit f960e57

Browse files
l1kdjbw
authored andcommitted
cxl/pci: Rightsize CDAT response allocation
Jonathan notes that cxl_cdat_get_length() and cxl_cdat_read_table() allocate 32 dwords for the DOE response even though it may be smaller. In the case of cxl_cdat_get_length(), only the second dword of the response is of interest (it contains the length). So reduce the allocation to 2 dwords and let DOE discard the remainder. In the case of cxl_cdat_read_table(), a correctly sized allocation for the full CDAT already exists. Let DOE write each table entry directly into that allocation. There's a snag in that the table entry is preceded by a Table Access Response Header (1 dword, CXL 3.0 table 8-14). Save the last dword of the previous table entry, let DOE overwrite it with the header of the next entry and restore it afterwards. The resulting CDAT is preceded by 4 unavoidable useless bytes. Increase the allocation size accordingly. The buffer overflow check in cxl_cdat_read_table() becomes unnecessary because the remaining bytes in the allocation are tracked in "length", which is passed to DOE and limits how many bytes it writes to the allocation. Additionally, cxl_cdat_read_table() bails out if the DOE response is truncated due to insufficient space. Tested-by: Ira Weiny <ira.weiny@intel.com> Signed-off-by: Lukas Wunner <lukas@wunner.de> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Cc: Dave Jiang <dave.jiang@intel.com> Link: https://lore.kernel.org/r/7a4e1f86958a79a70f29b96a92199522f08f8322.1678543498.git.lukas@wunner.de Signed-off-by: Dan Williams <dan.j.williams@intel.com>
1 parent 7a877c9 commit f960e57

1 file changed

Lines changed: 19 additions & 17 deletions

File tree

drivers/cxl/core/pci.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ static int cxl_cdat_get_length(struct device *dev,
453453
size_t *length)
454454
{
455455
__le32 request = CDAT_DOE_REQ(0);
456-
__le32 response[32];
456+
__le32 response[2];
457457
int rc;
458458

459459
rc = pci_doe(cdat_doe, PCI_DVSEC_VENDOR_ID_CXL,
@@ -464,7 +464,7 @@ static int cxl_cdat_get_length(struct device *dev,
464464
dev_err(dev, "DOE failed: %d", rc);
465465
return rc;
466466
}
467-
if (rc < 2 * sizeof(__le32))
467+
if (rc < sizeof(response))
468468
return -EIO;
469469

470470
*length = le32_to_cpu(response[1]);
@@ -477,28 +477,28 @@ static int cxl_cdat_read_table(struct device *dev,
477477
struct pci_doe_mb *cdat_doe,
478478
void *cdat_table, size_t *cdat_length)
479479
{
480-
size_t length = *cdat_length;
480+
size_t length = *cdat_length + sizeof(__le32);
481481
__le32 *data = cdat_table;
482482
int entry_handle = 0;
483+
__le32 saved_dw = 0;
483484

484485
do {
485486
__le32 request = CDAT_DOE_REQ(entry_handle);
486487
struct cdat_entry_header *entry;
487-
__le32 response[32];
488488
size_t entry_dw;
489489
int rc;
490490

491491
rc = pci_doe(cdat_doe, PCI_DVSEC_VENDOR_ID_CXL,
492492
CXL_DOE_PROTOCOL_TABLE_ACCESS,
493493
&request, sizeof(request),
494-
&response, sizeof(response));
494+
data, length);
495495
if (rc < 0) {
496496
dev_err(dev, "DOE failed: %d", rc);
497497
return rc;
498498
}
499499

500500
/* 1 DW Table Access Response Header + CDAT entry */
501-
entry = (struct cdat_entry_header *)(response + 1);
501+
entry = (struct cdat_entry_header *)(data + 1);
502502
if ((entry_handle == 0 &&
503503
rc != sizeof(__le32) + sizeof(struct cdat_header)) ||
504504
(entry_handle > 0 &&
@@ -508,21 +508,22 @@ static int cxl_cdat_read_table(struct device *dev,
508508

509509
/* Get the CXL table access header entry handle */
510510
entry_handle = FIELD_GET(CXL_DOE_TABLE_ACCESS_ENTRY_HANDLE,
511-
le32_to_cpu(response[0]));
511+
le32_to_cpu(data[0]));
512512
entry_dw = rc / sizeof(__le32);
513513
/* Skip Header */
514514
entry_dw -= 1;
515-
entry_dw = min(length / sizeof(__le32), entry_dw);
516-
/* Prevent length < 1 DW from causing a buffer overflow */
517-
if (entry_dw) {
518-
memcpy(data, entry, entry_dw * sizeof(__le32));
519-
length -= entry_dw * sizeof(__le32);
520-
data += entry_dw;
521-
}
515+
/*
516+
* Table Access Response Header overwrote the last DW of
517+
* previous entry, so restore that DW
518+
*/
519+
*data = saved_dw;
520+
length -= entry_dw * sizeof(__le32);
521+
data += entry_dw;
522+
saved_dw = *data;
522523
} while (entry_handle != CXL_DOE_TABLE_ACCESS_LAST_ENTRY);
523524

524525
/* Length in CDAT header may exceed concatenation of CDAT entries */
525-
*cdat_length -= length;
526+
*cdat_length -= length - sizeof(__le32);
526527

527528
return 0;
528529
}
@@ -559,7 +560,8 @@ void read_cdat_data(struct cxl_port *port)
559560
return;
560561
}
561562

562-
cdat_table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
563+
cdat_table = devm_kzalloc(dev, cdat_length + sizeof(__le32),
564+
GFP_KERNEL);
563565
if (!cdat_table)
564566
return;
565567

@@ -570,7 +572,7 @@ void read_cdat_data(struct cxl_port *port)
570572
dev_err(dev, "CDAT data read error\n");
571573
}
572574

573-
port->cdat.table = cdat_table;
575+
port->cdat.table = cdat_table + sizeof(__le32);
574576
port->cdat.length = cdat_length;
575577
}
576578
EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);

0 commit comments

Comments
 (0)