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 <frank.blasc...@de.ibm.com> > > > > 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 <frank.blasc...@de.ibm.com> > > --- > > 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 <frank.blasc...@de.ibm.com> > > + * Hong Bo Li <lih...@cn.ibm.com> > > + * Yi Min Zhao <zyi...@cn.ibm.com> > > + * > > + * 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 <frank.blasc...@de.ibm.com> > > + * Hong Bo Li <lih...@cn.ibm.com> > > + * Yi Min Zhao <zyi...@cn.ibm.com> > > + * > > + * 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 { > > > >