Skip to content

Commit fbaa382

Browse files
l1kdjbw
authored andcommitted
cxl/pci: Fix CDAT retrieval on big endian
The CDAT exposed in sysfs differs between little endian and big endian arches: On big endian, every 4 bytes are byte-swapped. PCI Configuration Space is little endian (PCI r3.0 sec 6.1). Accessors such as pci_read_config_dword() implicitly swap bytes on big endian. That way, the macros in include/uapi/linux/pci_regs.h work regardless of the arch's endianness. For an example of implicit byte-swapping, see ppc4xx_pciex_read_config(), which calls in_le32(), which uses lwbrx (Load Word Byte-Reverse Indexed). DOE Read/Write Data Mailbox Registers are unlike other registers in Configuration Space in that they contain or receive a 4 byte portion of an opaque byte stream (a "Data Object" per PCIe r6.0 sec 7.9.24.5f). They need to be copied to or from the request/response buffer verbatim. So amend pci_doe_send_req() and pci_doe_recv_resp() to undo the implicit byte-swapping. The CXL_DOE_TABLE_ACCESS_* and PCI_DOE_DATA_OBJECT_DISC_* macros assume implicit byte-swapping. Byte-swap requests after constructing them with those macros and byte-swap responses before parsing them. Change the request and response type to __le32 to avoid sparse warnings. Per a request from Jonathan, replace sizeof(u32) with sizeof(__le32) for consistency. Fixes: c970060 ("cxl/port: Read CDAT table") Tested-by: Ira Weiny <[email protected]> Signed-off-by: Lukas Wunner <[email protected]> Reviewed-by: Dan Williams <[email protected]> Cc: [email protected] # v6.0+ Reviewed-by: Jonathan Cameron <[email protected]> Link: https://lore.kernel.org/r/3051114102f41d19df3debbee123129118fc5e6d.1678543498.git.lukas@wunner.de Signed-off-by: Dan Williams <[email protected]>
1 parent e8d018d commit fbaa382

File tree

3 files changed

+33
-26
lines changed

3 files changed

+33
-26
lines changed

drivers/cxl/core/pci.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ static struct pci_doe_mb *find_cdat_doe(struct device *uport)
462462
return NULL;
463463
}
464464

465-
#define CDAT_DOE_REQ(entry_handle) \
465+
#define CDAT_DOE_REQ(entry_handle) cpu_to_le32 \
466466
(FIELD_PREP(CXL_DOE_TABLE_ACCESS_REQ_CODE, \
467467
CXL_DOE_TABLE_ACCESS_REQ_CODE_READ) | \
468468
FIELD_PREP(CXL_DOE_TABLE_ACCESS_TABLE_TYPE, \
@@ -475,8 +475,8 @@ static void cxl_doe_task_complete(struct pci_doe_task *task)
475475
}
476476

477477
struct cdat_doe_task {
478-
u32 request_pl;
479-
u32 response_pl[32];
478+
__le32 request_pl;
479+
__le32 response_pl[32];
480480
struct completion c;
481481
struct pci_doe_task task;
482482
};
@@ -510,10 +510,10 @@ static int cxl_cdat_get_length(struct device *dev,
510510
return rc;
511511
}
512512
wait_for_completion(&t.c);
513-
if (t.task.rv < sizeof(u32))
513+
if (t.task.rv < sizeof(__le32))
514514
return -EIO;
515515

516-
*length = t.response_pl[1];
516+
*length = le32_to_cpu(t.response_pl[1]);
517517
dev_dbg(dev, "CDAT length %zu\n", *length);
518518

519519
return 0;
@@ -524,13 +524,13 @@ static int cxl_cdat_read_table(struct device *dev,
524524
struct cxl_cdat *cdat)
525525
{
526526
size_t length = cdat->length;
527-
u32 *data = cdat->table;
527+
__le32 *data = cdat->table;
528528
int entry_handle = 0;
529529

530530
do {
531531
DECLARE_CDAT_DOE_TASK(CDAT_DOE_REQ(entry_handle), t);
532532
size_t entry_dw;
533-
u32 *entry;
533+
__le32 *entry;
534534
int rc;
535535

536536
rc = pci_doe_submit_task(cdat_doe, &t.task);
@@ -540,21 +540,21 @@ static int cxl_cdat_read_table(struct device *dev,
540540
}
541541
wait_for_completion(&t.c);
542542
/* 1 DW header + 1 DW data min */
543-
if (t.task.rv < (2 * sizeof(u32)))
543+
if (t.task.rv < (2 * sizeof(__le32)))
544544
return -EIO;
545545

546546
/* Get the CXL table access header entry handle */
547547
entry_handle = FIELD_GET(CXL_DOE_TABLE_ACCESS_ENTRY_HANDLE,
548-
t.response_pl[0]);
548+
le32_to_cpu(t.response_pl[0]));
549549
entry = t.response_pl + 1;
550-
entry_dw = t.task.rv / sizeof(u32);
550+
entry_dw = t.task.rv / sizeof(__le32);
551551
/* Skip Header */
552552
entry_dw -= 1;
553-
entry_dw = min(length / sizeof(u32), entry_dw);
553+
entry_dw = min(length / sizeof(__le32), entry_dw);
554554
/* Prevent length < 1 DW from causing a buffer overflow */
555555
if (entry_dw) {
556-
memcpy(data, entry, entry_dw * sizeof(u32));
557-
length -= entry_dw * sizeof(u32);
556+
memcpy(data, entry, entry_dw * sizeof(__le32));
557+
length -= entry_dw * sizeof(__le32);
558558
data += entry_dw;
559559
}
560560
} while (entry_handle != CXL_DOE_TABLE_ACCESS_LAST_ENTRY);

drivers/pci/doe.c

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ static int pci_doe_send_req(struct pci_doe_mb *doe_mb,
128128
return -EIO;
129129

130130
/* Length is 2 DW of header + length of payload in DW */
131-
length = 2 + task->request_pl_sz / sizeof(u32);
131+
length = 2 + task->request_pl_sz / sizeof(__le32);
132132
if (length > PCI_DOE_MAX_LENGTH)
133133
return -EIO;
134134
if (length == PCI_DOE_MAX_LENGTH)
@@ -141,9 +141,9 @@ static int pci_doe_send_req(struct pci_doe_mb *doe_mb,
141141
pci_write_config_dword(pdev, offset + PCI_DOE_WRITE,
142142
FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH,
143143
length));
144-
for (i = 0; i < task->request_pl_sz / sizeof(u32); i++)
144+
for (i = 0; i < task->request_pl_sz / sizeof(__le32); i++)
145145
pci_write_config_dword(pdev, offset + PCI_DOE_WRITE,
146-
task->request_pl[i]);
146+
le32_to_cpu(task->request_pl[i]));
147147

148148
pci_doe_write_ctrl(doe_mb, PCI_DOE_CTRL_GO);
149149

@@ -195,11 +195,11 @@ static int pci_doe_recv_resp(struct pci_doe_mb *doe_mb, struct pci_doe_task *tas
195195

196196
/* First 2 dwords have already been read */
197197
length -= 2;
198-
payload_length = min(length, task->response_pl_sz / sizeof(u32));
198+
payload_length = min(length, task->response_pl_sz / sizeof(__le32));
199199
/* Read the rest of the response payload */
200200
for (i = 0; i < payload_length; i++) {
201-
pci_read_config_dword(pdev, offset + PCI_DOE_READ,
202-
&task->response_pl[i]);
201+
pci_read_config_dword(pdev, offset + PCI_DOE_READ, &val);
202+
task->response_pl[i] = cpu_to_le32(val);
203203
/* Prior to the last ack, ensure Data Object Ready */
204204
if (i == (payload_length - 1) && !pci_doe_data_obj_ready(doe_mb))
205205
return -EIO;
@@ -217,7 +217,7 @@ static int pci_doe_recv_resp(struct pci_doe_mb *doe_mb, struct pci_doe_task *tas
217217
if (FIELD_GET(PCI_DOE_STATUS_ERROR, val))
218218
return -EIO;
219219

220-
return min(length, task->response_pl_sz / sizeof(u32)) * sizeof(u32);
220+
return min(length, task->response_pl_sz / sizeof(__le32)) * sizeof(__le32);
221221
}
222222

223223
static void signal_task_complete(struct pci_doe_task *task, int rv)
@@ -317,14 +317,16 @@ static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u16 *vid,
317317
{
318318
u32 request_pl = FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX,
319319
*index);
320+
__le32 request_pl_le = cpu_to_le32(request_pl);
321+
__le32 response_pl_le;
320322
u32 response_pl;
321323
DECLARE_COMPLETION_ONSTACK(c);
322324
struct pci_doe_task task = {
323325
.prot.vid = PCI_VENDOR_ID_PCI_SIG,
324326
.prot.type = PCI_DOE_PROTOCOL_DISCOVERY,
325-
.request_pl = &request_pl,
327+
.request_pl = &request_pl_le,
326328
.request_pl_sz = sizeof(request_pl),
327-
.response_pl = &response_pl,
329+
.response_pl = &response_pl_le,
328330
.response_pl_sz = sizeof(response_pl),
329331
.complete = pci_doe_task_complete,
330332
.private = &c,
@@ -340,6 +342,7 @@ static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u16 *vid,
340342
if (task.rv != sizeof(response_pl))
341343
return -EIO;
342344

345+
response_pl = le32_to_cpu(response_pl_le);
343346
*vid = FIELD_GET(PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID, response_pl);
344347
*protocol = FIELD_GET(PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL,
345348
response_pl);
@@ -533,8 +536,8 @@ int pci_doe_submit_task(struct pci_doe_mb *doe_mb, struct pci_doe_task *task)
533536
* DOE requests must be a whole number of DW and the response needs to
534537
* be big enough for at least 1 DW
535538
*/
536-
if (task->request_pl_sz % sizeof(u32) ||
537-
task->response_pl_sz < sizeof(u32))
539+
if (task->request_pl_sz % sizeof(__le32) ||
540+
task->response_pl_sz < sizeof(__le32))
538541
return -EINVAL;
539542

540543
if (test_bit(PCI_DOE_FLAG_DEAD, &doe_mb->flags))

include/linux/pci-doe.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ struct pci_doe_mb;
3434
* @work: Used internally by the mailbox
3535
* @doe_mb: Used internally by the mailbox
3636
*
37+
* Payloads are treated as opaque byte streams which are transmitted verbatim,
38+
* without byte-swapping. If payloads contain little-endian register values,
39+
* the caller is responsible for conversion with cpu_to_le32() / le32_to_cpu().
40+
*
3741
* The payload sizes and rv are specified in bytes with the following
3842
* restrictions concerning the protocol.
3943
*
@@ -45,9 +49,9 @@ struct pci_doe_mb;
4549
*/
4650
struct pci_doe_task {
4751
struct pci_doe_protocol prot;
48-
u32 *request_pl;
52+
__le32 *request_pl;
4953
size_t request_pl_sz;
50-
u32 *response_pl;
54+
__le32 *response_pl;
5155
size_t response_pl_sz;
5256
int rv;
5357
void (*complete)(struct pci_doe_task *task);

0 commit comments

Comments
 (0)