[PATCH v3 09/14] pc-bios/s390-ccw: Introduce PCI device

jrossi@linux.ibm.com posted 14 patches 1 week, 5 days ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Jared Rossi <jrossi@linux.ibm.com>, Zhuoying Cai <zycai@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, John Snow <jsnow@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v3 09/14] pc-bios/s390-ccw: Introduce PCI device
Posted by jrossi@linux.ibm.com 1 week, 5 days ago
From: Jared Rossi <jrossi@linux.ibm.com>

Define selected s390x PCI instructions.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/Makefile |   2 +-
 pc-bios/s390-ccw/pci.c    | 194 ++++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/pci.h    |  80 ++++++++++++++++
 3 files changed, 275 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/s390-ccw/pci.c
 create mode 100644 pc-bios/s390-ccw/pci.h

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 9c29548f84..a62fc9d766 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -35,7 +35,7 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 
 OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
 	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
-	  virtio-ccw.o clp.o
+	  virtio-ccw.o clp.o pci.o
 
 SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
 
diff --git a/pc-bios/s390-ccw/pci.c b/pc-bios/s390-ccw/pci.c
new file mode 100644
index 0000000000..9656be45ca
--- /dev/null
+++ b/pc-bios/s390-ccw/pci.c
@@ -0,0 +1,194 @@
+/*
+ * s390x PCI funcionality
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include "pci.h"
+#include "bswap.h"
+#include <stdio.h>
+#include <stdbool.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;
+
+    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");
+    *status = req_off.even >> 24 & 0xff;
+    *data = __data;
+    return cc;
+}
+
+/* PCI store */
+static inline 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)
+        : [data] "d" (data)
+        : "cc");
+    *status = req_off.even >> 24 & 0xff;
+    return cc;
+}
+
+/* store PCI function controls */
+static inline 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 */
+static inline 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, uint8_t pcias, uint64_t data,
+              uint8_t len)
+{
+
+    uint64_t req = ZPCI_CREATE_REQ(fhandle, pcias, len);
+    uint8_t status;
+    int rc;
+
+    /* len must be non-zero power of 2 with a maximum of 8 bytes per write */
+    switch (len) {
+    case 1:
+    case 2:
+    case 4:
+    case 8:
+        rc = pcistg(data, req, offset, &status);
+        break;
+    default:
+        rc = -1;
+    }
+
+    /* Error condition detected */
+    if (rc == 1) {
+        printf("PCI store failed with status condition %d\n", status);
+        return -1;
+    }
+
+    return rc ? -1 : 0;
+}
+
+int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
+             uint8_t len)
+{
+    uint64_t req, data;
+    uint8_t status;
+    int rc;
+
+    req = ZPCI_CREATE_REQ(fhandle, pcias, len);
+    rc = pcilg(&data, req, offset, &status);
+
+    /* Error condition detected */
+    if (rc == 1) {
+        printf("PCI load failed with status condition %d\n", status);
+        return -1;
+    }
+
+    switch (len) {
+    case 1:
+        *(uint8_t *)buf = data;
+        break;
+    case 2:
+        *(uint16_t *)buf = data;
+        break;
+    case 4:
+        *(uint32_t *)buf = data;
+        break;
+    case 8:
+        *(uint64_t *)buf = data;
+        break;
+    default:
+        return -1;
+    }
+
+    return rc ? -1 : 0;
+}
+
+int pci_dev_enable(PciDevice *pcidev)
+{
+    int rc;
+
+    rc = enable_pci_function(&pcidev->fhandle);
+    if (rc) {
+        return rc;
+    }
+
+    pcidev->status = PCIST_ENABLED;
+
+    return pci_get_fib(&pcidev->fib, pcidev->fhandle);
+}
+
+int pci_get_fib(PciFib *fib, uint32_t fhandle)
+{
+    uint64_t req = ZPCI_CREATE_REQ(fhandle, 0, 0);
+    uint8_t status;
+    int rc;
+
+    rc = stpcifc(req, fib, &status);
+
+    if (rc == 1) {
+        return status;
+    } else if (rc) {
+        return rc;
+    }
+
+    return 0;
+}
+
+int pci_set_fib(PciFib *fib, uint32_t fhandle, uint8_t dma_as, uint8_t opcontrol)
+{
+    uint64_t req = ZPCI_CREATE_REQ(fhandle, dma_as, opcontrol);
+    uint8_t status;
+    int rc;
+
+    rc = mpcifc(req, fib, &status);
+
+    if (rc == 1) {
+        return status;
+    } else if (rc) {
+        return rc;
+    }
+
+    return 0;
+}
diff --git a/pc-bios/s390-ccw/pci.h b/pc-bios/s390-ccw/pci.h
new file mode 100644
index 0000000000..ff8d5687f4
--- /dev/null
+++ b/pc-bios/s390-ccw/pci.h
@@ -0,0 +1,80 @@
+/*
+ * s390x PCI definitions
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef PCI_H
+#define PCI_H
+
+#include <stdint.h>
+#include <stdbool.h>
+#include "clp.h"
+
+#define ZPCI_CREATE_REQ(handle, space, len)                    \
+    ((uint64_t) handle << 32 | space << 16 | len)
+
+union register_pair {
+    unsigned __int128 pair;
+    struct {
+        unsigned long even;
+        unsigned long odd;
+    };
+};
+
+#define PCIFIB_FC_ENABLED      0x80
+#define PCIFIB_FC_ERROR        0x40
+#define PCIFIB_FC_BLOCKED      0x20
+#define PCIFIB_FC_DMAREG       0x10
+
+#define PCIST_DISABLED         0x0
+#define PCIST_ENABLED          0x1
+
+#define PCI_CFGBAR             0xF  /* Base Address Register for config space */
+#define PCI_CAPABILITY_LIST    0x34 /* Offset of first capability list entry */
+
+struct PciFib {
+    uint32_t reserved0[2];
+    uint8_t fcflags;
+    uint8_t reserved1[3];
+    uint32_t reserved2;
+    uint64_t pba;
+    uint64_t pal;
+    uint64_t iota;
+    uint16_t isc:4;
+    uint16_t noi:12;
+    uint8_t reserved3:2;
+    uint8_t aibvo:6;
+    uint8_t s:1;
+    uint8_t reserved4:1;
+    uint8_t aisbo:6;
+    uint32_t reserved5;
+    uint64_t aibv;
+    uint64_t aisb;
+    uint64_t fmba;
+    uint32_t reserved6[2];
+};
+typedef struct PciFib PciFib;
+
+struct PciDevice {
+    uint16_t device_id;
+    uint16_t vendor_id;
+    uint32_t fid;
+    uint32_t fhandle;
+    uint8_t status;
+    PciFib fib;
+};
+typedef struct PciDevice PciDevice;
+
+int pci_dev_enable(PciDevice *pcidev);
+int pci_get_fib(PciFib *fib, uint32_t fhandle);
+int pci_set_fib(PciFib *fib, uint32_t fhandle, uint8_t dma_as, uint8_t opcontrol);
+int pci_write(uint32_t fhandle, uint64_t offset, uint8_t pcias, uint64_t data,
+              uint8_t len);
+int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
+             uint8_t len);
+
+#endif
-- 
2.52.0
Re: [PATCH v3 09/14] pc-bios/s390-ccw: Introduce PCI device
Posted by Farhan Ali 5 days, 6 hours ago
On 1/27/2026 8:15 AM, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
>
> Define selected s390x PCI instructions.
>
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/Makefile |   2 +-
>   pc-bios/s390-ccw/pci.c    | 194 ++++++++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/pci.h    |  80 ++++++++++++++++
>   3 files changed, 275 insertions(+), 1 deletion(-)
>   create mode 100644 pc-bios/s390-ccw/pci.c
>   create mode 100644 pc-bios/s390-ccw/pci.h
>
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index 9c29548f84..a62fc9d766 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -35,7 +35,7 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
>   
>   OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
>   	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
> -	  virtio-ccw.o clp.o
> +	  virtio-ccw.o clp.o pci.o
>   
>   SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
>   
> diff --git a/pc-bios/s390-ccw/pci.c b/pc-bios/s390-ccw/pci.c
> new file mode 100644
> index 0000000000..9656be45ca
> --- /dev/null
> +++ b/pc-bios/s390-ccw/pci.c
> @@ -0,0 +1,194 @@
> +/*
> + * s390x PCI funcionality
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "clp.h"
> +#include "pci.h"
> +#include "bswap.h"
> +#include <stdio.h>
> +#include <stdbool.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;

Nit: Setting cc = -1 doesn't make any difference since the cc gets 
overwritten and the pcilg will return cc > 0.


> +    uint64_t __data;
> +
> +    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");
> +    *status = req_off.even >> 24 & 0xff;
> +    *data = __data;
> +    return cc;
> +}
> +
> +/* PCI store */
> +static inline 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;

Nit: Same as pcilg.


> +
> +    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)
> +        : [data] "d" (data)
> +        : "cc");
> +    *status = req_off.even >> 24 & 0xff;
> +    return cc;
> +}
> +
> +/* store PCI function controls */
> +static inline 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 */
> +static inline 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, uint8_t pcias, uint64_t data,
> +              uint8_t len)
> +{
> +
> +    uint64_t req = ZPCI_CREATE_REQ(fhandle, pcias, len);
> +    uint8_t status;
> +    int rc;
> +
> +    /* len must be non-zero power of 2 with a maximum of 8 bytes per write */
> +    switch (len) {
> +    case 1:
> +    case 2:
> +    case 4:
> +    case 8:
> +        rc = pcistg(data, req, offset, &status);
> +        break;
> +    default:
> +        rc = -1;
> +    }
> +
> +    /* Error condition detected */
> +    if (rc == 1) {

According to the architecture a return code of 0 would indicate normal 
completion. But we do have error scenarios that can return error code > 
1. For example an error code of 3 would indicate 'Unrecognized handle'. 
May we should check for non-zero rc in this if condition...


> +        printf("PCI store failed with status condition %d\n", status);

... and print both the rc and status here.


> +        return -1;
> +    }
> +
> +    return rc ? -1 : 0;
> +}
> +
> +int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
> +             uint8_t len)
> +{
> +    uint64_t req, data;
> +    uint8_t status;
> +    int rc;
> +
> +    req = ZPCI_CREATE_REQ(fhandle, pcias, len);
> +    rc = pcilg(&data, req, offset, &status);
> +
> +    /* Error condition detected */
> +    if (rc == 1) {

Similar to pci_write we should check for non-zero return code here? And 
should do the same in pci_get_fib (stpcifc) and pci_set_fib (mpcifc)?


> +        printf("PCI load failed with status condition %d\n", status);
> +        return -1;
> +    }
> +
> +    switch (len) {
> +    case 1:
> +        *(uint8_t *)buf = data;
> +        break;
> +    case 2:
> +        *(uint16_t *)buf = data;
> +        break;
> +    case 4:
> +        *(uint32_t *)buf = data;
> +        break;
> +    case 8:
> +        *(uint64_t *)buf = data;
> +        break;
> +    default:
> +        return -1;
> +    }
> +
> +    return rc ? -1 : 0;
> +}
> +

<...snip...>
Re: [PATCH v3 09/14] pc-bios/s390-ccw: Introduce PCI device
Posted by Eric Farman 1 week, 2 days ago
On Tue, 2026-01-27 at 11:15 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Define selected s390x PCI instructions.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/Makefile |   2 +-
>  pc-bios/s390-ccw/pci.c    | 194 ++++++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/pci.h    |  80 ++++++++++++++++
>  3 files changed, 275 insertions(+), 1 deletion(-)
>  create mode 100644 pc-bios/s390-ccw/pci.c
>  create mode 100644 pc-bios/s390-ccw/pci.h
> 
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index 9c29548f84..a62fc9d766 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -35,7 +35,7 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
>  
>  OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
>  	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
> -	  virtio-ccw.o clp.o
> +	  virtio-ccw.o clp.o pci.o
>  
>  SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
>  
> diff --git a/pc-bios/s390-ccw/pci.c b/pc-bios/s390-ccw/pci.c
> new file mode 100644
> index 0000000000..9656be45ca
> --- /dev/null
> +++ b/pc-bios/s390-ccw/pci.c
> @@ -0,0 +1,194 @@
> +/*
> + * s390x PCI funcionality

functionality

> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "clp.h"
> +#include "pci.h"
> +#include "bswap.h"
> +#include <stdio.h>
> +#include <stdbool.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;
> +
> +    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");
> +    *status = req_off.even >> 24 & 0xff;
> +    *data = __data;
> +    return cc;
> +}
> +
> +/* PCI store */
> +static inline 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)
> +        : [data] "d" (data)
> +        : "cc");
> +    *status = req_off.even >> 24 & 0xff;
> +    return cc;
> +}
> +
> +/* store PCI function controls */
> +static inline 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 */
> +static inline 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, uint8_t pcias, uint64_t data,
> +              uint8_t len)
> +{
> +
> +    uint64_t req = ZPCI_CREATE_REQ(fhandle, pcias, len);
> +    uint8_t status;
> +    int rc;
> +
> +    /* len must be non-zero power of 2 with a maximum of 8 bytes per write */
> +    switch (len) {
> +    case 1:
> +    case 2:
> +    case 4:
> +    case 8:
> +        rc = pcistg(data, req, offset, &status);
> +        break;
> +    default:
> +        rc = -1;
> +    }
> +
> +    /* Error condition detected */
> +    if (rc == 1) {
> +        printf("PCI store failed with status condition %d\n", status);
> +        return -1;
> +    }
> +
> +    return rc ? -1 : 0;
> +}
> +
> +int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
> +             uint8_t len)
> +{
> +    uint64_t req, data;
> +    uint8_t status;
> +    int rc;
> +
> +    req = ZPCI_CREATE_REQ(fhandle, pcias, len);
> +    rc = pcilg(&data, req, offset, &status);
> +
> +    /* Error condition detected */
> +    if (rc == 1) {
> +        printf("PCI load failed with status condition %d\n", status);
> +        return -1;
> +    }
> +
> +    switch (len) {
> +    case 1:
> +        *(uint8_t *)buf = data;
> +        break;
> +    case 2:
> +        *(uint16_t *)buf = data;
> +        break;
> +    case 4:
> +        *(uint32_t *)buf = data;
> +        break;
> +    case 8:
> +        *(uint64_t *)buf = data;
> +        break;
> +    default:
> +        return -1;
> +    }
> +
> +    return rc ? -1 : 0;
> +}
> +
> +int pci_dev_enable(PciDevice *pcidev)
> +{
> +    int rc;
> +
> +    rc = enable_pci_function(&pcidev->fhandle);
> +    if (rc) {
> +        return rc;
> +    }
> +
> +    pcidev->status = PCIST_ENABLED;
> +
> +    return pci_get_fib(&pcidev->fib, pcidev->fhandle);

Is pcidev->status meant to be left "enabled" if this fails? Seems that the error cases would be
unexpected, but severe.

Oh... But pci_dev_enable doesn't seem to be called anywhere (enable_pci_function is called by
virtio_pci_setup_device, in patch 11). Nor pci_get/set_fib below... So maybe all these chunks are
not needed? (Ditto the related structs in pci.h, below)

> +}
> +
> +int pci_get_fib(PciFib *fib, uint32_t fhandle)
> +{
> +    uint64_t req = ZPCI_CREATE_REQ(fhandle, 0, 0);
> +    uint8_t status;
> +    int rc;
> +
> +    rc = stpcifc(req, fib, &status);
> +
> +    if (rc == 1) {
> +        return status;
> +    } else if (rc) {
> +        return rc;
> +    }
> +
> +    return 0;
> +}
> +
> +int pci_set_fib(PciFib *fib, uint32_t fhandle, uint8_t dma_as, uint8_t opcontrol)
> +{
> +    uint64_t req = ZPCI_CREATE_REQ(fhandle, dma_as, opcontrol);
> +    uint8_t status;
> +    int rc;
> +
> +    rc = mpcifc(req, fib, &status);
> +
> +    if (rc == 1) {
> +        return status;
> +    } else if (rc) {
> +        return rc;
> +    }
> +
> +    return 0;
> +}
> diff --git a/pc-bios/s390-ccw/pci.h b/pc-bios/s390-ccw/pci.h
> new file mode 100644
> index 0000000000..ff8d5687f4
> --- /dev/null
> +++ b/pc-bios/s390-ccw/pci.h
> @@ -0,0 +1,80 @@
> +/*
> + * s390x PCI definitions
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef PCI_H
> +#define PCI_H
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include "clp.h"
> +
> +#define ZPCI_CREATE_REQ(handle, space, len)                    \
> +    ((uint64_t) handle << 32 | space << 16 | len)
> +
> +union register_pair {
> +    unsigned __int128 pair;
> +    struct {
> +        unsigned long even;
> +        unsigned long odd;
> +    };
> +};
> +
> +#define PCIFIB_FC_ENABLED      0x80
> +#define PCIFIB_FC_ERROR        0x40
> +#define PCIFIB_FC_BLOCKED      0x20
> +#define PCIFIB_FC_DMAREG       0x10
> +
> +#define PCIST_DISABLED         0x0
> +#define PCIST_ENABLED          0x1
> +
> +#define PCI_CFGBAR             0xF  /* Base Address Register for config space */
> +#define PCI_CAPABILITY_LIST    0x34 /* Offset of first capability list entry */
> +
> +struct PciFib {
> +    uint32_t reserved0[2];
> +    uint8_t fcflags;
> +    uint8_t reserved1[3];
> +    uint32_t reserved2;
> +    uint64_t pba;
> +    uint64_t pal;
> +    uint64_t iota;
> +    uint16_t isc:4;
> +    uint16_t noi:12;
> +    uint8_t reserved3:2;
> +    uint8_t aibvo:6;
> +    uint8_t s:1;
> +    uint8_t reserved4:1;
> +    uint8_t aisbo:6;
> +    uint32_t reserved5;
> +    uint64_t aibv;
> +    uint64_t aisb;
> +    uint64_t fmba;
> +    uint32_t reserved6[2];
> +};
> +typedef struct PciFib PciFib;
> +
> +struct PciDevice {
> +    uint16_t device_id;
> +    uint16_t vendor_id;
> +    uint32_t fid;
> +    uint32_t fhandle;
> +    uint8_t status;
> +    PciFib fib;
> +};
> +typedef struct PciDevice PciDevice;
> +
> +int pci_dev_enable(PciDevice *pcidev);
> +int pci_get_fib(PciFib *fib, uint32_t fhandle);
> +int pci_set_fib(PciFib *fib, uint32_t fhandle, uint8_t dma_as, uint8_t opcontrol);
> +int pci_write(uint32_t fhandle, uint64_t offset, uint8_t pcias, uint64_t data,
> +              uint8_t len);
> +int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
> +             uint8_t len);
> +
> +#endif
Re: [PATCH v3 09/14] pc-bios/s390-ccw: Introduce PCI device
Posted by Thomas Huth 1 week, 3 days ago
On 27/01/2026 17.15, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Define selected s390x PCI instructions.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
...
> diff --git a/pc-bios/s390-ccw/pci.h b/pc-bios/s390-ccw/pci.h
> new file mode 100644
> index 0000000000..ff8d5687f4
> --- /dev/null
> +++ b/pc-bios/s390-ccw/pci.h
> @@ -0,0 +1,80 @@
> +/*
> + * s390x PCI definitions
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef PCI_H
> +#define PCI_H
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include "clp.h"
> +
> +#define ZPCI_CREATE_REQ(handle, space, len)                    \
> +    ((uint64_t) handle << 32 | space << 16 | len)
> +
> +union register_pair {
> +    unsigned __int128 pair;
> +    struct {
> +        unsigned long even;
> +        unsigned long odd;
> +    };
> +};
> +
> +#define PCIFIB_FC_ENABLED      0x80
> +#define PCIFIB_FC_ERROR        0x40
> +#define PCIFIB_FC_BLOCKED      0x20
> +#define PCIFIB_FC_DMAREG       0x10
> +
> +#define PCIST_DISABLED         0x0
> +#define PCIST_ENABLED          0x1
> +
> +#define PCI_CFGBAR             0xF  /* Base Address Register for config space */
> +#define PCI_CAPABILITY_LIST    0x34 /* Offset of first capability list entry */
> +
> +struct PciFib {
> +    uint32_t reserved0[2];
> +    uint8_t fcflags;
> +    uint8_t reserved1[3];
> +    uint32_t reserved2;
> +    uint64_t pba;
> +    uint64_t pal;
> +    uint64_t iota;
> +    uint16_t isc:4;
> +    uint16_t noi:12;
> +    uint8_t reserved3:2;
> +    uint8_t aibvo:6;
> +    uint8_t s:1;
> +    uint8_t reserved4:1;
> +    uint8_t aisbo:6;
> +    uint32_t reserved5;
> +    uint64_t aibv;
> +    uint64_t aisb;
> +    uint64_t fmba;
> +    uint32_t reserved6[2];
> +};
> +typedef struct PciFib PciFib;

I can't comment on the gory details here since I don't have the spec, but 
from a distance, the patch looks fine to me now.

Acked-by: Thomas Huth <thuth@redhat.com>