[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] s390: Add PCI bus support
From: |
Frank Blaschka |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] s390: Add PCI bus support |
Date: |
Tue, 18 Nov 2014 13:50:59 +0100 |
User-agent: |
Mutt/1.5.17 (2007-11-01) |
On Mon, Nov 10, 2014 at 04:14:16PM +0100, Alexander Graf wrote:
>
>
> On 10.11.14 15:20, Frank Blaschka wrote:
> > From: Frank Blaschka <address@hidden>
> >
> > This patch implements a pci bus for s390x together with infrastructure
> > to generate and handle hotplug events, to configure/unconfigure via
> > sclp instruction, to do iommu translations and provide s390 support for
> > MSI/MSI-X notification processing.
> >
> > Signed-off-by: Frank Blaschka <address@hidden>
> > ---
> > default-configs/s390x-softmmu.mak | 1 +
> > hw/s390x/Makefile.objs | 1 +
> > hw/s390x/css.c | 5 +
> > hw/s390x/css.h | 1 +
> > hw/s390x/s390-pci-bus.c | 485
> > ++++++++++++++++++++++++++++++++++++++
> > hw/s390x/s390-pci-bus.h | 254 ++++++++++++++++++++
> > hw/s390x/s390-virtio-ccw.c | 3 +
> > hw/s390x/sclp.c | 10 +-
> > include/hw/s390x/sclp.h | 8 +
> > target-s390x/ioinst.c | 52 ++++
> > target-s390x/ioinst.h | 1 +
> > 11 files changed, 820 insertions(+), 1 deletion(-)
> > create mode 100644 hw/s390x/s390-pci-bus.c
> > create mode 100644 hw/s390x/s390-pci-bus.h
> >
> > diff --git a/default-configs/s390x-softmmu.mak
> > b/default-configs/s390x-softmmu.mak
> > index 126d88d..6ee2ff8 100644
> > --- a/default-configs/s390x-softmmu.mak
> > +++ b/default-configs/s390x-softmmu.mak
> > @@ -1,3 +1,4 @@
> > +include pci.mak
> > CONFIG_VIRTIO=y
> > CONFIG_SCLPCONSOLE=y
> > CONFIG_S390_FLIC=y
> > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> > index 1ba6c3a..428d957 100644
> > --- a/hw/s390x/Makefile.objs
> > +++ b/hw/s390x/Makefile.objs
> > @@ -8,3 +8,4 @@ obj-y += ipl.o
> > obj-y += css.o
> > obj-y += s390-virtio-ccw.o
> > obj-y += virtio-ccw.o
> > +obj-y += s390-pci-bus.o
> > diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> > index b67c039..7553085 100644
> > --- a/hw/s390x/css.c
> > +++ b/hw/s390x/css.c
> > @@ -1299,6 +1299,11 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t
> > chpid)
> > /* TODO */
> > }
> >
> > +void css_generate_css_crws(uint8_t cssid)
> > +{
> > + css_queue_crw(CRW_RSC_CSS, 0, 0, 0);
> > +}
> > +
> > int css_enable_mcsse(void)
> > {
> > trace_css_enable_facility("mcsse");
> > diff --git a/hw/s390x/css.h b/hw/s390x/css.h
> > index 33104ac..7e53148 100644
> > --- a/hw/s390x/css.h
> > +++ b/hw/s390x/css.h
> > @@ -101,6 +101,7 @@ void css_queue_crw(uint8_t rsc, uint8_t erc, int chain,
> > uint16_t rsid);
> > void css_generate_sch_crws(uint8_t cssid, uint8_t ssid, uint16_t schid,
> > int hotplugged, int add);
> > void css_generate_chp_crws(uint8_t cssid, uint8_t chpid);
> > +void css_generate_css_crws(uint8_t cssid);
> > void css_adapter_interrupt(uint8_t isc);
> >
> > #define CSS_IO_ADAPTER_VIRTIO 1
> > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > new file mode 100644
> > index 0000000..f2fa6ba
> > --- /dev/null
> > +++ b/hw/s390x/s390-pci-bus.c
> > @@ -0,0 +1,485 @@
> > +/*
> > + * s390 PCI BUS
> > + *
> > + * Copyright 2014 IBM Corp.
> > + * Author(s): Frank Blaschka <address@hidden>
> > + * Hong Bo Li <address@hidden>
> > + * Yi Min Zhao <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> > + * your option) any later version. See the COPYING file in the top-level
> > + * directory.
> > + */
> > +
> > +#include <hw/pci/pci.h>
> > +#include <hw/pci/pci_bus.h>
> > +#include <hw/s390x/css.h>
> > +#include <hw/s390x/sclp.h>
> > +#include <hw/pci/msi.h>
> > +#include "qemu/error-report.h"
> > +#include "s390-pci-bus.h"
> > +
> > +/* #define DEBUG_S390PCI_BUS */
> > +#ifdef DEBUG_S390PCI_BUS
> > +#define DPRINTF(fmt, ...) \
> > + do { fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); } while (0)
> > +#else
> > +#define DPRINTF(fmt, ...) \
> > + do { } while (0)
> > +#endif
> > +
> > +static const unsigned long be_to_le = BITS_PER_LONG - 1;
> > +static QTAILQ_HEAD(, SeiContainer) pending_sei =
> > + QTAILQ_HEAD_INITIALIZER(pending_sei);
> > +static QTAILQ_HEAD(, S390PCIBusDevice) device_list =
> > + QTAILQ_HEAD_INITIALIZER(device_list);
>
> Please get rid of all statics ;). All state has to live in objects.
>
be_to_le was misleading and unnecesary will remove this one but
static QTAILQ_HEAD seems to be a common practice for list anchors.
If you really want me to change this do you have any prefered way,
or can you point me to some code doing this?
> > +
> > +int chsc_sei_nt2_get_event(void *res)
> > +{
> > + ChscSeiNt2Res *nt2_res = (ChscSeiNt2Res *)res;
> > + PciCcdfAvail *accdf;
> > + PciCcdfErr *eccdf;
> > + int rc = 1;
> > + SeiContainer *sei_cont;
> > +
> > + sei_cont = QTAILQ_FIRST(&pending_sei);
> > + if (sei_cont) {
> > + QTAILQ_REMOVE(&pending_sei, sei_cont, link);
> > + nt2_res->nt = 2;
> > + nt2_res->cc = sei_cont->cc;
> > + switch (sei_cont->cc) {
> > + case 1: /* error event */
> > + eccdf = (PciCcdfErr *)nt2_res->ccdf;
> > + eccdf->fid = cpu_to_be32(sei_cont->fid);
> > + eccdf->fh = cpu_to_be32(sei_cont->fh);
> > + break;
> > + case 2: /* availability event */
> > + accdf = (PciCcdfAvail *)nt2_res->ccdf;
> > + accdf->fid = cpu_to_be32(sei_cont->fid);
> > + accdf->fh = cpu_to_be32(sei_cont->fh);
> > + accdf->pec = cpu_to_be16(sei_cont->pec);
> > + break;
> > + default:
> > + abort();
> > + }
> > + g_free(sei_cont);
> > + rc = 0;
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +int chsc_sei_nt2_have_event(void)
> > +{
> > + return !QTAILQ_EMPTY(&pending_sei);
> > +}
> > +
> > +S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
> > +{
> > + S390PCIBusDevice *pbdev;
> > +
> > + QTAILQ_FOREACH(pbdev, &device_list, next) {
> > + if (pbdev->fid == fid) {
> > + return pbdev;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +void s390_pci_sclp_configure(int configure, SCCB *sccb)
> > +{
> > + PciCfgSccb *psccb = (PciCfgSccb *)sccb;
> > + S390PCIBusDevice *pbdev =
> > s390_pci_find_dev_by_fid(be32_to_cpu(psccb->aid));
> > + uint16_t rc;
> > +
> > + if (pbdev) {
> > + if ((configure == 1 && pbdev->configured == true) ||
> > + (configure == 0 && pbdev->configured == false)) {
> > + rc = SCLP_RC_NO_ACTION_REQUIRED;
> > + } else {
> > + pbdev->configured = !pbdev->configured;
> > + rc = SCLP_RC_NORMAL_COMPLETION;
> > + }
> > + } else {
> > + DPRINTF("sclp config %d no dev found\n", configure);
> > + rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
> > + }
> > +
> > + psccb->header.response_code = cpu_to_be16(rc);
> > + return;
> > +}
> > +
> > +static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
> > +{
> > + return PCI_SLOT(pdev->devfn);
> > +}
> > +
> > +static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
> > +{
> > + return PCI_SLOT(pdev->devfn) | FH_VIRT;
> > +}
> > +
> > +S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
> > +{
> > + S390PCIBusDevice *dev;
> > + int i = 0;
> > +
> > + QTAILQ_FOREACH(dev, &device_list, next) {
> > + if (i == idx) {
> > + return dev;
> > + }
> > + i++;
> > + }
> > + return NULL;
> > +}
> > +
> > +S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
> > +{
> > + S390PCIBusDevice *pbdev;
> > +
> > + QTAILQ_FOREACH(pbdev, &device_list, next) {
> > + if (pbdev->fh == fh) {
> > + return pbdev;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +static void s390_pci_generate_plug_event(uint16_t pec, uint32_t fh,
> > + uint32_t fid)
> > +{
> > + SeiContainer *sei_cont = g_malloc0(sizeof(SeiContainer));
> > +
> > + sei_cont->fh = fh;
> > + sei_cont->fid = fid;
> > + sei_cont->cc = 2;
> > + sei_cont->pec = pec;
> > +
> > + QTAILQ_INSERT_TAIL(&pending_sei, sei_cont, link);
> > + css_generate_css_crws(0);
> > +}
> > +
> > +static void s390_pci_set_irq(void *opaque, int irq, int level)
> > +{
> > + /* nothing to do */
> > +}
> > +
> > +static int s390_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> > +{
> > + /* nothing to do */
> > + return 0;
> > +}
> > +
> > +void s390_pci_bus_init(void)
> > +{
> > + DeviceState *dev;
> > +
> > + dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> > + qdev_init_nofail(dev);
> > +}
> > +
> > +uint64_t s390_pci_get_table_origin(uint64_t iota)
> > +{
> > + return iota & ~ZPCI_IOTA_RTTO_FLAG;
> > +}
> > +
> > +static uint32_t s390_pci_get_p(uint64_t iota)
> > +{
> > + return iota & ~ZPCI_IOTA_RTTO_FLAG;
> > +}
> > +
> > +static uint32_t s390_pci_get_dt(uint64_t iota)
> > +{
> > + return (iota >> 2) & 0x7;
> > +}
> > +
> > +static uint32_t s390_pci_get_fs(uint64_t iota)
> > +{
> > + uint32_t dt = s390_pci_get_dt(iota);
> > +
> > + if (dt == 4 || dt == 5) {
> > + return iota & 0x3;
> > + } else {
> > + return ZPCI_IOTA_FS_4K;
> > + }
> > +}
> > +
> > +uint64_t s390_guest_io_table_walk(uint64_t guest_iota,
> > + uint64_t guest_dma_address)
> > +{
> > + uint64_t sto_a, pto_a, px_a;
> > + uint64_t sto, pto, pte;
> > + uint32_t rtx, sx, px;
> > +
> > + rtx = calc_rtx(guest_dma_address);
> > + sx = calc_sx(guest_dma_address);
> > + px = calc_px(guest_dma_address);
> > +
> > + sto_a = guest_iota + rtx * sizeof(uint64_t);
> > + cpu_physical_memory_rw(sto_a, (uint8_t *)&sto, sizeof(uint64_t), 0);
>
> This is not endian safe. Couldn't you just use ldq_be_phys() here?
>
> > + sto = (uint64_t)get_rt_sto(sto);
> > +
> > + pto_a = sto + sx * sizeof(uint64_t);
>
> Can there be invalid entries? How could the guest say "access not
> allowed for this region"?
>
> > + cpu_physical_memory_rw(pto_a, (uint8_t *)&pto, sizeof(uint64_t), 0);
>
> ldq_be_phys()
>
> > + pto = (uint64_t)get_st_pto(pto);
> > +
> > + px_a = pto + px * sizeof(uint64_t);
> > + cpu_physical_memory_rw(px_a, (uint8_t *)&pte, sizeof(uint64_t), 0);
>
> ldq_be_phys()
>
> > +
> > + return pte;
> > +}
> > +
> > +static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
> > + bool is_write)
> > +{
> > + IOMMUTLBEntry ret;
> > + uint32_t fs;
> > + uint64_t pte;
> > + BEntry *container = container_of(iommu, BEntry, mr);
> > + S390PCIBusDevice *pbdev = container->pbdev;
> > + S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
> > + ->qbus.parent);
> > +
> > + DPRINTF("iommu trans addr 0x%lx\n", addr);
> > +
> > + /* s390 does not have an APIC maped to main storage so we use
> > + * a separate AddressSpace only for msix notifications
> > + */
> > + if (addr == ZPCI_MSI_ADDR) {
> > + ret.target_as = &s->msix_notify_as;
> > + ret.iova = addr;
> > + ret.translated_addr = addr;
> > + ret.addr_mask = 0xfff;
> > + ret.perm = IOMMU_RW;
> > + return ret;
> > + }
> > +
> > + pte =
> > s390_guest_io_table_walk(s390_pci_get_table_origin(pbdev->g_iota),
> > + addr);
>
> Same question for the invalid entry part again. How can we inform the
> guest that a device tried to DMA to an invalid address? Shouldn't there
> be some error recovery interrupt and device shutdown in that case?
>
> > +
> > + ret.target_as = &address_space_memory;
> > + ret.iova = addr;
> > + ret.translated_addr = pte & ZPCI_PTE_ADDR_MASK;
> > + fs = s390_pci_get_fs(pbdev->g_iota);
> > + if (fs == ZPCI_IOTA_FS_4K) {
> > + ret.addr_mask = 0xfff;
> > + } else if (fs == ZPCI_IOTA_FS_1M) {
> > + ret.addr_mask = 0xfffff;
> > + } else if (fs == ZPCI_IOTA_FS_2G) {
> > + ret.addr_mask = 0x7fffffff;
> > + }
> > + if (s390_pci_get_p(pbdev->g_iota) == 1) {
> > + ret.perm = IOMMU_RO;
> > + } else {
> > + ret.perm = IOMMU_RW;
> > + }
> > + return ret;
> > +}
> > +
> > +static const MemoryRegionIOMMUOps s390_iommu_ops = {
> > + .translate = s390_translate_iommu,
> > +};
> > +
> > +static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int
> > devfn)
> > +{
> > + S390pciState *s = opaque;
> > +
> > + return &s->iommu[PCI_SLOT(devfn)].as;
> > +}
> > +
> > +static void s390_msi_ctrl_write(void *opaque, hwaddr addr, uint64_t data,
> > + unsigned int size)
> > +{
> > + S390PCIBusDevice *pbdev;
> > + unsigned long *aibv, *aisb;
> > + int summary_set;
> > + hwaddr aibv_len, aisb_len;
> > + uint32_t io_int_word;
> > + uint32_t hdata = le32_to_cpu(data);
>
> ^
>
> > + uint32_t fid = hdata >> ZPCI_MSI_VEC_BITS;
> > + uint32_t vec = hdata & ZPCI_MSI_VEC_MASK;
> > +
> > + DPRINTF("write_msix data 0x%lx fid %d vec 0x%x\n", data, fid, vec);
> > +
> > + pbdev = s390_pci_find_dev_by_fid(fid);
> > + if (!pbdev) {
> > + DPRINTF("msix_notify no dev\n");
> > + return;
> > + }
> > + aibv_len = aisb_len = 8;
> > + aibv = cpu_physical_memory_map(pbdev->routes.adapter.ind_addr,
> > + &aibv_len, 1);
> > + aisb = cpu_physical_memory_map(pbdev->routes.adapter.summary_addr,
> > + &aisb_len, 1);
>
> Please use ldq_le/be_phys.
>
> > +
> > + set_bit(vec ^ be_to_le, aibv);
>
> What is this be_to_le? Looking at the other endianness messup so far,
> I'm not convinced this is correct. Is there any way to let the
> infrastructure deal with endianness instead?
>
Ok this one was a mess all over the place. We can not use ldq_le/be_phys because
we need to atomically set the indicator bit in guest memory. virtio_ccw_notify
does something similar and uses a very smart approach to handle the
endianness as well. Will rewrite the code to use the same approach.
> > + summary_set = test_and_set_bit(pbdev->routes.adapter.summary_offset
> > + ^ be_to_le, aisb);
> > +
> > + if (!summary_set) {
> > + io_int_word = (pbdev->isc << 27) | IO_INT_WORD_AI;
> > + s390_io_interrupt(0, 0, 0, io_int_word);
> > + }
> > +
> > + cpu_physical_memory_unmap(aibv, aibv_len, 1, aibv_len);
> > + cpu_physical_memory_unmap(aisb, aisb_len, 1, aisb_len);
> > + return;
> > +}
> > +
> > +static uint64_t s390_msi_ctrl_read(void *opaque, hwaddr addr, unsigned
> > size)
> > +{
> > + return 0xffffffff;
> > +}
> > +
> > +static const MemoryRegionOps s390_msi_ctrl_ops = {
> > + .write = s390_msi_ctrl_write,
> > + .read = s390_msi_ctrl_read,
> > + .endianness = DEVICE_NATIVE_ENDIAN,
>
> If you're doing an LE conversion right after, it probably means the
> region is really a LITTLE_ENDIAN region, no?
>
> Also, le32_to_cpu is definitely wrong in this case, as that will byte
> swap on BE hosts, but not on LE hosts. The value you get back is always
> the same though, so you've successfully broken LE hosts.
>
> Keep in mind that there's always the slim chance that s390x supports LE
> one day, so getting endianness right from the start is pretty important,
> even though it seems useless to you today ;).
>
Ok, think I got this now. MR is LE
> > +};
> > +
> > +static void s390_pcihost_init_as(S390pciState *s)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < PCI_SLOT_MAX; i++) {
> > + memory_region_init_iommu(&s->iommu[i].mr, OBJECT(s),
> > + &s390_iommu_ops, "iommu-s390",
> > UINT64_MAX);
> > + address_space_init(&s->iommu[i].as, &s->iommu[i].mr, "iommu-pci");
> > + }
> > +
> > + memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
> > + &s390_msi_ctrl_ops, s, "msix-s390", UINT64_MAX);
> > + address_space_init(&s->msix_notify_as, &s->msix_notify_mr, "msix-pci");
> > +}
> > +
> > +static int s390_pcihost_init(SysBusDevice *dev)
> > +{
> > + PCIBus *b;
> > + BusState *bus;
> > + PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> > + S390pciState *s = S390_PCI_HOST_BRIDGE(dev);
> > +
> > + DPRINTF("host_init\n");
> > +
> > + b = pci_register_bus(DEVICE(dev), NULL,
> > + s390_pci_set_irq, s390_pci_map_irq, NULL,
> > + get_system_memory(), get_system_io(), 0, 64,
> > + TYPE_PCI_BUS);
> > + s390_pcihost_init_as(s);
> > + pci_setup_iommu(b, s390_pci_dma_iommu, s);
> > +
> > + bus = BUS(b);
> > + qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
> > + phb->bus = b;
> > + return 0;
> > +}
> > +
> > +static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
> > +{
> > + uint8_t pos;
> > + uint16_t ctrl;
> > + uint32_t table, pba;
> > +
> > + pos = pci_find_capability(pbdev->pdev, PCI_CAP_ID_MSIX);
> > + if (!pos) {
> > + pbdev->msix.available = false;
> > + return 0;
> > + }
> > +
> > + ctrl = pci_host_config_read_common(pbdev->pdev, pos + PCI_CAP_FLAGS,
> > + pci_config_size(pbdev->pdev), sizeof(ctrl));
> > + table = pci_host_config_read_common(pbdev->pdev, pos + PCI_MSIX_TABLE,
> > + pci_config_size(pbdev->pdev), sizeof(table));
> > + pba = pci_host_config_read_common(pbdev->pdev, pos + PCI_MSIX_PBA,
> > + pci_config_size(pbdev->pdev), sizeof(pba));
> > +
> > + pbdev->msix.table_bar = table & PCI_MSIX_FLAGS_BIRMASK;
> > + pbdev->msix.table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
> > + pbdev->msix.pba_bar = pba & PCI_MSIX_FLAGS_BIRMASK;
> > + pbdev->msix.pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
> > + pbdev->msix.entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
> > + pbdev->msix.available = true;
> > + return 0;
> > +}
> > +
> > +static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
> > + DeviceState *dev, Error **errp)
> > +{
> > + PCIDevice *pci_dev = PCI_DEVICE(dev);
> > + S390PCIBusDevice *pbdev;
> > + S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> > + ->qbus.parent);
> > +
> > + pbdev = g_malloc0(sizeof(*pbdev));
> > +
> > + pbdev->fid = s390_pci_get_pfid(pci_dev);
> > + pbdev->pdev = pci_dev;
> > + pbdev->configured = true;
> > +
> > + pbdev->fh = s390_pci_get_pfh(pci_dev);
> > +
> > + s->iommu[PCI_SLOT(pci_dev->devfn)].pbdev = pbdev;
> > + s390_pcihost_setup_msix(pbdev);
> > +
> > + QTAILQ_INSERT_TAIL(&device_list, pbdev, next);
> > + if (dev->hotplugged) {
> > + s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
> > + pbdev->fh, pbdev->fid);
> > + s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
> > + pbdev->fh, pbdev->fid);
> > + }
> > + return;
> > +}
> > +
> > +static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
> > + DeviceState *dev, Error **errp)
> > +{
> > + PCIDevice *pci_dev = PCI_DEVICE(dev);
> > + S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> > + ->qbus.parent);
> > + S390PCIBusDevice *pbdev = s->iommu[PCI_SLOT(pci_dev->devfn)].pbdev;
> > +
> > + if (pbdev->configured) {
> > + pbdev->configured = false;
> > + s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
> > + pbdev->fh, pbdev->fid);
> > + }
> > +
> > + QTAILQ_REMOVE(&device_list, pbdev, next);
> > + s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
> > + pbdev->fh, pbdev->fid);
> > + s->iommu[PCI_SLOT(pci_dev->devfn)].pbdev = NULL;
> > + object_unparent(OBJECT(pci_dev));
> > + g_free(pbdev);
> > +}
> > +
> > +static void s390_pcihost_class_init(ObjectClass *klass, void *data)
> > +{
> > + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> > +
> > + dc->cannot_instantiate_with_device_add_yet = true;
> > + k->init = s390_pcihost_init;
> > + hc->plug = s390_pcihost_hot_plug;
> > + hc->unplug = s390_pcihost_hot_unplug;
> > + msi_supported = true;
> > +}
> > +
> > +static const TypeInfo s390_pcihost_info = {
> > + .name = TYPE_S390_PCI_HOST_BRIDGE,
> > + .parent = TYPE_PCI_HOST_BRIDGE,
> > + .instance_size = sizeof(S390pciState),
> > + .class_init = s390_pcihost_class_init,
> > + .interfaces = (InterfaceInfo[]) {
> > + { TYPE_HOTPLUG_HANDLER },
> > + { }
> > + }
> > +};
> > +
> > +static void s390_pci_register_types(void)
> > +{
> > + type_register_static(&s390_pcihost_info);
> > +}
> > +
> > +type_init(s390_pci_register_types)
> > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
> > new file mode 100644
> > index 0000000..088f24f
> > --- /dev/null
> > +++ b/hw/s390x/s390-pci-bus.h
> > @@ -0,0 +1,254 @@
> > +/*
> > + * s390 PCI BUS definitions
> > + *
> > + * Copyright 2014 IBM Corp.
> > + * Author(s): Frank Blaschka <address@hidden>
> > + * Hong Bo Li <address@hidden>
> > + * Yi Min Zhao <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> > + * your option) any later version. See the COPYING file in the top-level
> > + * directory.
> > + */
> > +
> > +#ifndef HW_S390_PCI_BUS_H
> > +#define HW_S390_PCI_BUS_H
> > +
> > +#include <hw/pci/pci.h>
> > +#include <hw/pci/pci_host.h>
> > +#include "hw/s390x/sclp.h"
> > +#include "hw/s390x/s390_flic.h"
> > +#include "hw/s390x/css.h"
> > +
> > +#define TYPE_S390_PCI_HOST_BRIDGE "s390-pcihost"
> > +#define FH_VIRT 0x00ff0000
> > +#define ENABLE_BIT_OFFSET 31
> > +#define S390_PCIPT_ADAPTER 2
> > +
> > +#define S390_PCI_HOST_BRIDGE(obj) \
> > + OBJECT_CHECK(S390pciState, (obj), TYPE_S390_PCI_HOST_BRIDGE)
> > +
> > +#define HP_EVENT_TO_CONFIGURED 0x0301
> > +#define HP_EVENT_RESERVED_TO_STANDBY 0x0302
> > +#define HP_EVENT_CONFIGURED_TO_STBRES 0x0304
> > +#define HP_EVENT_STANDBY_TO_RESERVED 0x0308
> > +
> > +#define ZPCI_MSI_VEC_BITS 11
> > +#define ZPCI_MSI_VEC_MASK 0x7f
> > +
> > +#define ZPCI_MSI_ADDR 0xfe00000000000000
> > +#define ZPCI_SDMA_ADDR 0x100000000
> > +#define ZPCI_EDMA_ADDR 0x1ffffffffffffff
> > +
> > +#define PAGE_SHIFT 12
> > +#define PAGE_MASK (~(PAGE_SIZE-1))
> > +#define PAGE_DEFAULT_ACC 0
> > +#define PAGE_DEFAULT_KEY (PAGE_DEFAULT_ACC << 4)
> > +
> > +/* I/O Translation Anchor (IOTA) */
> > +enum ZpciIoatDtype {
> > + ZPCI_IOTA_STO = 0,
> > + ZPCI_IOTA_RTTO = 1,
> > + ZPCI_IOTA_RSTO = 2,
> > + ZPCI_IOTA_RFTO = 3,
> > + ZPCI_IOTA_PFAA = 4,
> > + ZPCI_IOTA_IOPFAA = 5,
> > + ZPCI_IOTA_IOPTO = 7
> > +};
> > +
> > +#define ZPCI_IOTA_IOT_ENABLED 0x800UL
> > +#define ZPCI_IOTA_DT_ST (ZPCI_IOTA_STO << 2)
> > +#define ZPCI_IOTA_DT_RT (ZPCI_IOTA_RTTO << 2)
> > +#define ZPCI_IOTA_DT_RS (ZPCI_IOTA_RSTO << 2)
> > +#define ZPCI_IOTA_DT_RF (ZPCI_IOTA_RFTO << 2)
> > +#define ZPCI_IOTA_DT_PF (ZPCI_IOTA_PFAA << 2)
> > +#define ZPCI_IOTA_FS_4K 0
> > +#define ZPCI_IOTA_FS_1M 1
> > +#define ZPCI_IOTA_FS_2G 2
> > +#define ZPCI_KEY (PAGE_DEFAULT_KEY << 5)
> > +
> > +#define ZPCI_IOTA_STO_FLAG (ZPCI_IOTA_IOT_ENABLED | ZPCI_KEY |
> > ZPCI_IOTA_DT_ST)
> > +#define ZPCI_IOTA_RTTO_FLAG (ZPCI_IOTA_IOT_ENABLED | ZPCI_KEY |
> > ZPCI_IOTA_DT_RT)
> > +#define ZPCI_IOTA_RSTO_FLAG (ZPCI_IOTA_IOT_ENABLED | ZPCI_KEY |
> > ZPCI_IOTA_DT_RS)
> > +#define ZPCI_IOTA_RFTO_FLAG (ZPCI_IOTA_IOT_ENABLED | ZPCI_KEY |
> > ZPCI_IOTA_DT_RF)
> > +#define ZPCI_IOTA_RFAA_FLAG (ZPCI_IOTA_IOT_ENABLED | ZPCI_KEY |\
> > + ZPCI_IOTA_DT_PF | ZPCI_IOTA_FS_2G)
> > +
> > +/* I/O Region and segment tables */
> > +#define ZPCI_INDEX_MASK 0x7ffUL
> > +
> > +#define ZPCI_TABLE_TYPE_MASK 0xc
> > +#define ZPCI_TABLE_TYPE_RFX 0xc
> > +#define ZPCI_TABLE_TYPE_RSX 0x8
> > +#define ZPCI_TABLE_TYPE_RTX 0x4
> > +#define ZPCI_TABLE_TYPE_SX 0x0
> > +
> > +#define ZPCI_TABLE_LEN_RFX 0x3
> > +#define ZPCI_TABLE_LEN_RSX 0x3
> > +#define ZPCI_TABLE_LEN_RTX 0x3
> > +
> > +#define ZPCI_TABLE_OFFSET_MASK 0xc0
> > +#define ZPCI_TABLE_SIZE 0x4000
> > +#define ZPCI_TABLE_ALIGN ZPCI_TABLE_SIZE
> > +#define ZPCI_TABLE_ENTRY_SIZE (sizeof(unsigned long))
> > +#define ZPCI_TABLE_ENTRIES (ZPCI_TABLE_SIZE / ZPCI_TABLE_ENTRY_SIZE)
> > +
> > +#define ZPCI_TABLE_BITS 11
> > +#define ZPCI_PT_BITS 8
> > +#define ZPCI_ST_SHIFT (ZPCI_PT_BITS + PAGE_SHIFT)
> > +#define ZPCI_RT_SHIFT (ZPCI_ST_SHIFT + ZPCI_TABLE_BITS)
> > +
> > +#define ZPCI_RTE_FLAG_MASK 0x3fffUL
> > +#define ZPCI_RTE_ADDR_MASK (~ZPCI_RTE_FLAG_MASK)
> > +#define ZPCI_STE_FLAG_MASK 0x7ffUL
> > +#define ZPCI_STE_ADDR_MASK (~ZPCI_STE_FLAG_MASK)
> > +
> > +/* I/O Page tables */
> > +#define ZPCI_PTE_VALID_MASK 0x400
> > +#define ZPCI_PTE_INVALID 0x400
> > +#define ZPCI_PTE_VALID 0x000
> > +#define ZPCI_PT_SIZE 0x800
> > +#define ZPCI_PT_ALIGN ZPCI_PT_SIZE
> > +#define ZPCI_PT_ENTRIES (ZPCI_PT_SIZE /
> > ZPCI_TABLE_ENTRY_SIZE)
> > +#define ZPCI_PT_MASK (ZPCI_PT_ENTRIES - 1)
> > +
> > +#define ZPCI_PTE_FLAG_MASK 0xfffUL
> > +#define ZPCI_PTE_ADDR_MASK (~ZPCI_PTE_FLAG_MASK)
> > +
> > +/* Shared bits */
> > +#define ZPCI_TABLE_VALID 0x00
> > +#define ZPCI_TABLE_INVALID 0x20
> > +#define ZPCI_TABLE_PROTECTED 0x200
> > +#define ZPCI_TABLE_UNPROTECTED 0x000
> > +
> > +#define ZPCI_TABLE_VALID_MASK 0x20
> > +#define ZPCI_TABLE_PROT_MASK 0x200
> > +
> > +typedef struct SeiContainer {
> > + QTAILQ_ENTRY(SeiContainer) link;
> > + uint32_t fid;
> > + uint32_t fh;
> > + uint8_t cc;
> > + uint16_t pec;
> > +} SeiContainer;
> > +
> > +typedef struct PciCcdfErr {
> > + uint32_t reserved1;
> > + uint32_t fh;
> > + uint32_t fid;
> > + uint32_t reserved2;
> > + uint64_t faddr;
> > + uint32_t reserved3;
> > + uint16_t reserved4;
> > + uint16_t pec;
> > +} QEMU_PACKED PciCcdfErr;
> > +
> > +typedef struct PciCcdfAvail {
> > + uint32_t reserved1;
> > + uint32_t fh;
> > + uint32_t fid;
> > + uint32_t reserved2;
> > + uint32_t reserved3;
> > + uint32_t reserved4;
> > + uint32_t reserved5;
> > + uint16_t reserved6;
> > + uint16_t pec;
> > +} QEMU_PACKED PciCcdfAvail;
> > +
> > +typedef struct ChscSeiNt2Res {
> > + uint16_t length;
> > + uint16_t code;
> > + uint16_t reserved1;
> > + uint8_t reserved2;
> > + uint8_t nt;
> > + uint8_t flags;
> > + uint8_t reserved3;
> > + uint8_t reserved4;
> > + uint8_t cc;
> > + uint32_t reserved5[13];
> > + uint8_t ccdf[4016];
> > +} QEMU_PACKED ChscSeiNt2Res;
> > +
> > +typedef struct PciCfgSccb {
> > + SCCBHeader header;
> > + uint8_t atype;
> > + uint8_t reserved1;
> > + uint16_t reserved2;
> > + uint32_t aid;
> > +} QEMU_PACKED PciCfgSccb;
> > +
> > +typedef struct S390MsixInfo {
> > + bool available;
> > + uint8_t table_bar;
> > + uint8_t pba_bar;
> > + uint16_t entries;
> > + uint32_t table_offset;
> > + uint32_t pba_offset;
> > +} S390MsixInfo;
> > +
> > +typedef struct S390PCIBusDevice {
> > + PCIDevice *pdev;
> > + bool configured;
> > + uint32_t fh;
> > + uint32_t fid;
> > + uint64_t g_iota;
> > + uint8_t isc;
> > + S390MsixInfo msix;
> > + AdapterRoutes routes;
> > + QTAILQ_ENTRY(S390PCIBusDevice) next;
> > +} S390PCIBusDevice;
> > +
> > +typedef struct BEntry {
> > + AddressSpace as;
> > + MemoryRegion mr;
> > + S390PCIBusDevice *pbdev;
> > +} BEntry;
> > +
> > +typedef struct S390pciState {
> > + PCIHostState parent_obj;
> > + BEntry iommu[PCI_SLOT_MAX];
> > + AddressSpace msix_notify_as;
> > + MemoryRegion msix_notify_mr;
> > +} S390pciState;
> > +
> > +static inline unsigned int calc_rtx(dma_addr_t ptr)
> > +{
> > + return ((unsigned long) ptr >> ZPCI_RT_SHIFT) & ZPCI_INDEX_MASK;
> > +}
> > +
> > +static inline unsigned int calc_sx(dma_addr_t ptr)
> > +{
> > + return ((unsigned long) ptr >> ZPCI_ST_SHIFT) & ZPCI_INDEX_MASK;
> > +}
> > +
> > +static inline unsigned int calc_px(dma_addr_t ptr)
> > +{
> > + return ((unsigned long) ptr >> PAGE_SHIFT) & ZPCI_PT_MASK;
> > +}
> > +
> > +static inline unsigned long *get_rt_sto(unsigned long entry)
> > +{
> > + return ((entry & ZPCI_TABLE_TYPE_MASK) == ZPCI_TABLE_TYPE_RTX)
> > + ? (unsigned long *) (entry & ZPCI_RTE_ADDR_MASK)
> > + : NULL;
> > +}
> > +
> > +static inline unsigned long *get_st_pto(unsigned long entry)
> > +{
> > + return ((entry & ZPCI_TABLE_TYPE_MASK) == ZPCI_TABLE_TYPE_SX)
> > + ? (unsigned long *) (entry & ZPCI_STE_ADDR_MASK)
> > + : NULL;
> > +}
>
> Are these static inlines used outside of a single place? If not, please
> move them into the .c file they get called from.
>
> > +
> > +int chsc_sei_nt2_get_event(void *res);
> > +int chsc_sei_nt2_have_event(void);
> > +void s390_pci_sclp_configure(int configure, SCCB *sccb);
> > +S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
> > +S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
> > +S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
>
> I think it makes sense to pass the PHB device as parameter on these.
> Don't assume you only have one.
We need to lookup our device mainly in the instruction handlers and there
we do not have a PHB available. Also having one list for our S390PCIBusDevices
devices does not prevent us from supporting more PHBs.
>
> > +void s390_pci_bus_init(void);
> > +uint64_t s390_pci_get_table_origin(uint64_t iota);
> > +uint64_t s390_guest_io_table_walk(uint64_t guest_iota,
> > + uint64_t guest_dma_address);
>
> Why are these exported?
>
> > +
> > +#endif
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index bc4dc2a..2e25834 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -18,6 +18,7 @@
> > #include "css.h"
> > #include "virtio-ccw.h"
> > #include "qemu/config-file.h"
> > +#include "s390-pci-bus.h"
> >
> > #define TYPE_S390_CCW_MACHINE "s390-ccw-machine"
> >
> > @@ -127,6 +128,8 @@ static void ccw_init(MachineState *machine)
> > machine->initrd_filename, "s390-ccw.img");
> > s390_flic_init();
> >
> > + s390_pci_bus_init();
>
> Please just inline that function here.
>
What do you mean by "just inline"?
> > +
> > /* register hypercalls */
> > virtio_ccw_register_hcalls();
> >
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index a759da7..a969975 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -20,6 +20,7 @@
> > #include "qemu/config-file.h"
> > #include "hw/s390x/sclp.h"
> > #include "hw/s390x/event-facility.h"
> > +#include "hw/s390x/s390-pci-bus.h"
> >
> > static inline SCLPEventFacility *get_event_facility(void)
> > {
> > @@ -62,7 +63,8 @@ static void read_SCP_info(SCCB *sccb)
> > read_info->entries[i].type = 0;
> > }
> >
> > - read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
> > + read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
> > + SCLP_HAS_PCI_RECONFIG);
> >
> > /*
> > * The storage increment size is a multiple of 1M and is a power of 2.
> > @@ -350,6 +352,12 @@ static void sclp_execute(SCCB *sccb, uint32_t code)
> > case SCLP_UNASSIGN_STORAGE:
> > unassign_storage(sccb);
> > break;
> > + case SCLP_CMDW_CONFIGURE_PCI:
> > + s390_pci_sclp_configure(1, sccb);
> > + break;
> > + case SCLP_CMDW_DECONFIGURE_PCI:
> > + s390_pci_sclp_configure(0, sccb);
> > + break;
> > default:
> > efc->command_handler(ef, sccb, code);
> > break;
> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> > index ec07a11..e8a64e2 100644
> > --- a/include/hw/s390x/sclp.h
> > +++ b/include/hw/s390x/sclp.h
> > @@ -43,14 +43,22 @@
> > #define SCLP_CMDW_CONFIGURE_CPU 0x00110001
> > #define SCLP_CMDW_DECONFIGURE_CPU 0x00100001
> >
> > +/* SCLP PCI codes */
> > +#define SCLP_HAS_PCI_RECONFIG 0x0000000040000000ULL
> > +#define SCLP_CMDW_CONFIGURE_PCI 0x001a0001
> > +#define SCLP_CMDW_DECONFIGURE_PCI 0x001b0001
> > +#define SCLP_RECONFIG_PCI_ATPYE 2
> > +
> > /* SCLP response codes */
> > #define SCLP_RC_NORMAL_READ_COMPLETION 0x0010
> > #define SCLP_RC_NORMAL_COMPLETION 0x0020
> > #define SCLP_RC_SCCB_BOUNDARY_VIOLATION 0x0100
> > +#define SCLP_RC_NO_ACTION_REQUIRED 0x0120
> > #define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0
> > #define SCLP_RC_CONTAINED_EQUIPMENT_CHECK 0x0340
> > #define SCLP_RC_INSUFFICIENT_SCCB_LENGTH 0x0300
> > #define SCLP_RC_STANDBY_READ_COMPLETION 0x0410
> > +#define SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED 0x09f0
> > #define SCLP_RC_INVALID_FUNCTION 0x40f0
> > #define SCLP_RC_NO_EVENT_BUFFERS_STORED 0x60f0
> > #define SCLP_RC_INVALID_SELECTION_MASK 0x70f0
> > diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
> > index b8a6486..d969f8f 100644
> > --- a/target-s390x/ioinst.c
> > +++ b/target-s390x/ioinst.c
> > @@ -14,6 +14,7 @@
> > #include "cpu.h"
> > #include "ioinst.h"
> > #include "trace.h"
> > +#include "hw/s390x/s390-pci-bus.h"
> >
> > int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int
> > *ssid,
> > int *schid)
> > @@ -398,6 +399,7 @@ typedef struct ChscResp {
> > #define CHSC_SCPD 0x0002
> > #define CHSC_SCSC 0x0010
> > #define CHSC_SDA 0x0031
> > +#define CHSC_SEI 0x000e
> >
> > #define CHSC_SCPD_0_M 0x20000000
> > #define CHSC_SCPD_0_C 0x10000000
> > @@ -566,6 +568,53 @@ out:
> > res->param = 0;
> > }
> >
> > +static int chsc_sei_nt0_get_event(void *res)
> > +{
> > + /* no events yet */
> > + return 1;
> > +}
> > +
> > +static int chsc_sei_nt0_have_event(void)
> > +{
> > + /* no events yet */
> > + return 0;
> > +}
> > +
> > +#define CHSC_SEI_NT0 (1ULL << 63)
> > +#define CHSC_SEI_NT2 (1ULL << 61)
> > +static void ioinst_handle_chsc_sei(ChscReq *req, ChscResp *res)
> > +{
> > + uint64_t selection_mask = be64_to_cpu(*(uint64_t *)&req->param1);
>
> ldq_p(&req->param1) I guess?
>
>
> Alex
Thx for your help,
Frank
>
> > + uint8_t *res_flags = (uint8_t *)res->data;
> > + int have_event = 0;
> > + int have_more = 0;
> > +
> > + /* regarding architecture nt0 can not be masked */
> > + have_event = !chsc_sei_nt0_get_event(res);
> > + have_more = chsc_sei_nt0_have_event();
> > +
> > + if (selection_mask & CHSC_SEI_NT2) {
> > + if (!have_event) {
> > + have_event = !chsc_sei_nt2_get_event(res);
> > + }
> > +
> > + if (!have_more) {
> > + have_more = chsc_sei_nt2_have_event();
> > + }
> > + }
> > +
> > + if (have_event) {
> > + res->code = cpu_to_be16(0x0001);
> > + if (have_more) {
> > + (*res_flags) |= 0x80;
> > + } else {
> > + (*res_flags) &= ~0x80;
> > + }
> > + } else {
> > + res->code = cpu_to_be16(0x0004);
> > + }
> > +}
> > +
> > static void ioinst_handle_chsc_unimplemented(ChscResp *res)
> > {
> > res->len = cpu_to_be16(CHSC_MIN_RESP_LEN);
> > @@ -617,6 +666,9 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
> > case CHSC_SDA:
> > ioinst_handle_chsc_sda(req, res);
> > break;
> > + case CHSC_SEI:
> > + ioinst_handle_chsc_sei(req, res);
> > + break;
> > default:
> > ioinst_handle_chsc_unimplemented(res);
> > break;
> > diff --git a/target-s390x/ioinst.h b/target-s390x/ioinst.h
> > index 29f6423..1efe16c 100644
> > --- a/target-s390x/ioinst.h
> > +++ b/target-s390x/ioinst.h
> > @@ -204,6 +204,7 @@ typedef struct CRW {
> >
> > #define CRW_RSC_SUBCH 0x3
> > #define CRW_RSC_CHP 0x4
> > +#define CRW_RSC_CSS 0xb
> >
> > /* I/O interruption code */
> > typedef struct IOIntCode {
> >
>
>
[Qemu-devel] [PATCH 3/3] kvm: extend kvm_irqchip_add_msi_route to work on s390, Frank Blaschka, 2014/11/10
[Qemu-devel] [PATCH 2/3] s390: implement pci instructions, Frank Blaschka, 2014/11/10