On 20/10/2025 18.20, [email protected] wrote:
From: Jared Rossi <[email protected]>
Define selected s390x PCI instructions and extend IPLB to allow PCI devices.
Signed-off-by: Jared Rossi <[email protected]>
---
include/hw/s390x/ipl/qipl.h | 9 ++
pc-bios/s390-ccw/pci.h | 77 +++++++++++++++
pc-bios/s390-ccw/pci.c | 191 ++++++++++++++++++++++++++++++++++++
pc-bios/s390-ccw/Makefile | 2 +-
4 files changed, 278 insertions(+), 1 deletion(-)
create mode 100644 pc-bios/s390-ccw/pci.h
create mode 100644 pc-bios/s390-ccw/pci.c
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index aadab87c2e..efd7b3797c 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -104,6 +104,14 @@ struct IplBlockQemuScsi {
} QEMU_PACKED;
typedef struct IplBlockQemuScsi IplBlockQemuScsi;
+struct IplBlockPci {
+ uint32_t reserved0[80];
+ uint8_t opt;
+ uint8_t reserved1[3];
+ uint32_t fid;
+} QEMU_PACKED;
Looks like all members of this struct are naturally aligned ... I think you
could likely drop the QEMU_PACKED here.
+typedef struct IplBlockPci IplBlockPci;
+
union IplParameterBlock {
struct {
uint32_t len;
@@ -119,6 +127,7 @@ union IplParameterBlock {
IplBlockFcp fcp;
IPLBlockPV pv;
IplBlockQemuScsi scsi;
+ IplBlockPci pci;
};
} QEMU_PACKED;
struct {
...
diff --git a/pc-bios/s390-ccw/pci.c b/pc-bios/s390-ccw/pci.c
new file mode 100644
index 0000000000..f776bc064c
--- /dev/null
+++ b/pc-bios/s390-ccw/pci.c
@@ -0,0 +1,191 @@
+/*
+ * s390x PCI funcionality
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <[email protected]>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include "pci.h"
+#include <stdio.h>
+
+/* PCI load */
+static inline int pcilg(uint64_t *data, uint64_t req, uint64_t offset, uint8_t
*status)
+{
+ union register_pair req_off = {.even = req, .odd = offset};
+ int cc = -1;
+ uint64_t __data = 0x92;
+
+ asm volatile (
+ " .insn rre,0xb9d20000,%[data],%[req_off]\n"
+ " ipm %[cc]\n"
+ " srl %[cc],28\n"
+ : [cc] "+d" (cc), [data] "=d" (__data),
+ [req_off] "+&d" (req_off.pair) :: "cc");
What's the "&" good for here?
+ *status = req_off.even >> 24 & 0xff;
+ *data = __data;
+ return cc;
+}
+
+/* PCI store */
+int pcistg(uint64_t data, uint64_t req, uint64_t offset, uint8_t *status)
+{
+ union register_pair req_off = {.even = req, .odd = offset};
+ int cc = -1;
+
+ asm volatile (
+ " .insn rre,0xb9d00000,%[data],%[req_off]\n"
+ " ipm %[cc]\n"
+ " srl %[cc],28\n"
+ : [cc] "+d" (cc), [req_off] "+&d" (req_off.pair)
dito
+ : [data] "d" (data)
+ : "cc");
+ *status = req_off.even >> 24 & 0xff;
+ return cc;
+}
+
+/* store PCI function controls */
+int stpcifc(uint64_t req, PciFib *fib, uint8_t *status)
+{
+ uint8_t cc;
+
+ asm volatile (
+ " .insn rxy,0xe300000000d4,%[req],%[fib]\n"
+ " ipm %[cc]\n"
+ " srl %[cc],28\n"
+ : [cc] "=d" (cc), [req] "+d" (req), [fib] "+Q" (*fib)
+ : : "cc");
+ *status = req >> 24 & 0xff;
+ return cc;
+}
+
+/* modify PCI function controls */
+int mpcifc(uint64_t req, PciFib *fib, uint8_t *status)
+{
+ uint8_t cc;
+
+ asm volatile (
+ " .insn rxy,0xe300000000d0,%[req],%[fib]\n"
+ " ipm %[cc]\n"
+ " srl %[cc],28\n"
+ : [cc] "=d" (cc), [req] "+d" (req), [fib] "+Q" (*fib)
+ : : "cc");
+ *status = req >> 24 & 0xff;
+ return cc;
+}
+
+int pci_write(uint32_t fhandle, uint64_t offset, uint64_t data, uint8_t len)
+{
+
+ uint64_t req = ZPCI_CREATE_REQ(fhandle, 4, len);
+ uint8_t status;
+ int rc;
+
+ rc = pcistg(data, req, offset, &status);
+ if (rc == 1) {
+ return status;
+ } else if (rc) {
+ return rc;
+ }
+
+ return 0;
+}
+
+int pci_read(uint32_t fhandle, uint64_t offset, uint8_t picas, void *buf,
uint8_t len)
+{
+ uint64_t req;
+ uint64_t data;
+ uint8_t status;
+ int readlen;
+ int i = 0;
+ int rc = 0;
+
+ while (len > 0 && !rc) {
+ data = 0;
+ readlen = len > 8 ? 8 : len;
+ req = ZPCI_CREATE_REQ(fhandle, picas, readlen);
+ rc = pcilg(&data, req, offset + (i * 8), &status);
+ ((uint64_t *)buf)[i] = data;
This looks somewhat dangerous ... what if buf points to a buffer where its
lengths is not divisible by 8? ... you'll happily overwrite the data that is
right behind the buffer in memory.
+ len -= readlen;
+ i++;
+ }
+
+ if (rc == 1) {
+ return status;
+ } else if (rc) {
+ return rc;
+ }
+
+ return 0;
+}
+
+/*
+ * Find the position of the capability config within PCI configuration
+ * space for a given cfg type. Return the position if found, otherwise 0.
+ */
+uint8_t find_cap_pos(uint32_t fhandle, uint64_t cfg_type) {
Curly bracket on the next line, please.
+ uint64_t req, next, cfg;
+ uint8_t status;
+ int rc;
+
+ req = ZPCI_CREATE_REQ(fhandle, 0xf, 1);
+ rc = pcilg(&next, req, PCI_CAPABILITY_LIST, &status);
+ rc = pcilg(&cfg, req, next + 3, &status);
Assigning rc just to discard the value again in the next line does not make
sense... if you're lazy, use "rc |= ..." in the second line. Otherwise
please explicitly check the "rc" after the first call.
+ while (!rc && (cfg != cfg_type) && next) {
+ rc = pcilg(&next, req, next + 1, &status);
+ rc = pcilg(&cfg, req, next + 3, &status);
dito
+ }
+
+ return rc ? 0 : next;
+}
Thomas
