[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 09/14] pci: Add support for Design
From: |
Andrey Smirnov |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 09/14] pci: Add support for Designware IP block |
Date: |
Tue, 30 Jan 2018 09:49:55 -0800 |
On Tue, Jan 30, 2018 at 5:18 AM, Marcel Apfelbaum
<address@hidden> wrote:
> Hi Andrei,
>
> Sorry for letting you wait,
> I have some comments/questions below.
>
>
> On 16/01/2018 3:37, Andrey Smirnov wrote:
>>
>> Add code needed to get a functional PCI subsytem when using in
>> conjunction with upstream Linux guest (4.13+). Tested to work against
>> "e1000e" (network adapter, using MSI interrupts) as well as
>> "usb-ehci" (USB controller, using legacy PCI interrupts).
>>
>> Cc: Peter Maydell <address@hidden>
>> Cc: Jason Wang <address@hidden>
>> Cc: Philippe Mathieu-Daudé <address@hidden>
>> Cc: address@hidden
>> Cc: address@hidden
>> Cc: address@hidden
>> Signed-off-by: Andrey Smirnov <address@hidden>
>> ---
>> default-configs/arm-softmmu.mak | 2 +
>> hw/pci-host/Makefile.objs | 2 +
>> hw/pci-host/designware.c | 618
>> +++++++++++++++++++++++++++++++++++++++
>> include/hw/pci-host/designware.h | 93 ++++++
>> include/hw/pci/pci_ids.h | 2 +
>> 5 files changed, 717 insertions(+)
>> create mode 100644 hw/pci-host/designware.c
>> create mode 100644 include/hw/pci-host/designware.h
>>
>> diff --git a/default-configs/arm-softmmu.mak
>> b/default-configs/arm-softmmu.mak
>> index b0d6e65038..0c5ae914ed 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -132,3 +132,5 @@ CONFIG_GPIO_KEY=y
>> CONFIG_MSF2=y
>> CONFIG_FW_CFG_DMA=y
>> CONFIG_XILINX_AXI=y
>> +CONFIG_PCI_DESIGNWARE=y
>> +
>> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
>> index 9c7909cf44..0e2c0a123b 100644
>> --- a/hw/pci-host/Makefile.objs
>> +++ b/hw/pci-host/Makefile.objs
>> @@ -17,3 +17,5 @@ common-obj-$(CONFIG_PCI_PIIX) += piix.o
>> common-obj-$(CONFIG_PCI_Q35) += q35.o
>> common-obj-$(CONFIG_PCI_GENERIC) += gpex.o
>> common-obj-$(CONFIG_PCI_XILINX) += xilinx-pcie.o
>> +
>> +common-obj-$(CONFIG_PCI_DESIGNWARE) += designware.o
>> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
>> new file mode 100644
>> index 0000000000..98fff5e5f3
>> --- /dev/null
>> +++ b/hw/pci-host/designware.c
>> @@ -0,0 +1,618 @@
>> +/*
>> + * Copyright (c) 2017, Impinj, Inc.
>
> 2018 :)
>
>> + *
>> + * Designware PCIe IP block emulation
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see
>> + * <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/pci/msi.h"
>> +#include "hw/pci/pci_bridge.h"
>> +#include "hw/pci/pci_host.h"
>> +#include "hw/pci/pcie_port.h"
>> +#include "hw/pci-host/designware.h"
>> +
>> +#define PCIE_PORT_LINK_CONTROL 0x710
>> +
>> +#define PCIE_PHY_DEBUG_R1 0x72C
>> +#define PCIE_PHY_DEBUG_R1_XMLH_LINK_UP BIT(4)
>> +
>> +#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
>> +
>> +#define PCIE_MSI_ADDR_LO 0x820
>> +#define PCIE_MSI_ADDR_HI 0x824
>> +#define PCIE_MSI_INTR0_ENABLE 0x828
>> +#define PCIE_MSI_INTR0_MASK 0x82C
>> +#define PCIE_MSI_INTR0_STATUS 0x830
>> +
>> +#define PCIE_ATU_VIEWPORT 0x900
>> +#define PCIE_ATU_REGION_INBOUND (0x1 << 31)
>> +#define PCIE_ATU_REGION_OUTBOUND (0x0 << 31)
>> +#define PCIE_ATU_REGION_INDEX2 (0x2 << 0)
>> +#define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
>> +#define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
>> +#define PCIE_ATU_CR1 0x904
>> +#define PCIE_ATU_TYPE_MEM (0x0 << 0)
>> +#define PCIE_ATU_TYPE_IO (0x2 << 0)
>> +#define PCIE_ATU_TYPE_CFG0 (0x4 << 0)
>> +#define PCIE_ATU_TYPE_CFG1 (0x5 << 0)
>> +#define PCIE_ATU_CR2 0x908
>> +#define PCIE_ATU_ENABLE (0x1 << 31)
>> +#define PCIE_ATU_BAR_MODE_ENABLE (0x1 << 30)
>> +#define PCIE_ATU_LOWER_BASE 0x90C
>> +#define PCIE_ATU_UPPER_BASE 0x910
>> +#define PCIE_ATU_LIMIT 0x914
>> +#define PCIE_ATU_LOWER_TARGET 0x918
>> +#define PCIE_ATU_BUS(x) (((x) >> 24) & 0xff)
>> +#define PCIE_ATU_DEVFN(x) (((x) >> 16) & 0xff)
>> +#define PCIE_ATU_UPPER_TARGET 0x91C
>> +
>> +static DesignwarePCIEHost *
>> +designware_pcie_root_to_host(DesignwarePCIERoot *root)
>> +{
>> + BusState *bus = qdev_get_parent_bus(DEVICE(root));
>> + return DESIGNWARE_PCIE_HOST(bus->parent);
>> +}
>> +
>> +static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned len)
>> +{
>> + DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(opaque);
>> + DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
>> +
>> + root->msi.intr[0].status |= (1 << val) & root->msi.intr[0].enable;
>> +
>> + if (root->msi.intr[0].status & ~root->msi.intr[0].mask) {
>> + qemu_set_irq(host->pci.irqs[0], 1);
>
>
> Sorry for the possibly dumb question, but "msi_write"
> will result in raising an INTx ?
Correct, that's the intention. I wouldn't be surprised if I missed a
better/canonical way to implement this.
>>
>> + }
>> +}
>> +
>> +const MemoryRegionOps designware_pci_host_msi_ops = {
>> + .write = designware_pcie_root_msi_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>
>
> You may want to limit the access size.
Good point, will do.
>>
>> +};
>> +
>> +static void designware_pcie_root_update_msi_mapping(DesignwarePCIERoot
>> *root)
>> +
>> +{
>> + DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
>> + MemoryRegion *address_space = &host->pci.memory;
>> + MemoryRegion *mem = &root->msi.iomem;
>> + const uint64_t base = root->msi.base;
>> + const bool enable = root->msi.intr[0].enable;
>> +
>> + if (memory_region_is_mapped(mem)) {
>> + memory_region_del_subregion(address_space, mem);
>> + object_unparent(OBJECT(mem));
>> + }
>> +
>> + if (enable) {
>> + memory_region_init_io(mem, OBJECT(root),
>> + &designware_pci_host_msi_ops,
>> + root, "pcie-msi", 0x1000);
>> +
>> + memory_region_add_subregion(address_space, base, mem);
>
>
> What happens if is enabled twice?
Ideally this shouldn't happen since this function is only called when
PCIE_MSI_INTR0_ENABLE transitions from "All IRQs disabled" to "At
least one IRQ enabled" or the inverse (via "update_msi_mapping" in
designware_pcie_root_config_write).
But, assuming I got my logic wrong there, my thinking was that if it
gets enabled for the second time, first "if" statement in
designware_pcie_root_update_msi_mapping() would remove old
MemoryRegion and second one would add it back, so it end up being a
"no-op". I might be violating some API usage rules, so, please don't
hesitate to call me out on this if I do.
> Is it possible to be also disabled?
>
Yes, similar to the case above, except the "if (enabled)" is not going
to be executed and there'd be no MemoryRegion to trigger MSI
interrupt.
>
>> + }
>> +}
>> +
>> +static DesignwarePCIEViewport *
>> +designware_pcie_root_get_current_viewport(DesignwarePCIERoot *root)
>> +{
>> + const unsigned int idx = root->atu_viewport & 0xF;
>> + const unsigned int dir = !!(root->atu_viewport &
>> PCIE_ATU_REGION_INBOUND);
>> + return &root->viewports[dir][idx];
>> +}
>> +
>> +static uint32_t
>> +designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
>> +{
>> + DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(d);
>> + DesignwarePCIEViewport *viewport =
>> + designware_pcie_root_get_current_viewport(root);
>> +
>> + uint32_t val;
>> +
>> + switch (address) {
>> + case PCIE_PORT_LINK_CONTROL:
>> + case PCIE_LINK_WIDTH_SPEED_CONTROL:
>> + val = 0xDEADBEEF;
>> + /* No-op */
>
>
> Not really a no-op
>
What I meant by "no-op" is that those registers do not implement their
actual function and instead return obviously bogus value. I'll change
the comment to state that in a more explicit way.
>> + break;
>> +
>> + case PCIE_MSI_ADDR_LO:
>> + val = root->msi.base;
>> + break;
>> +
>> + case PCIE_MSI_ADDR_HI:
>> + val = root->msi.base >> 32;
>> + break;
>> +
>> + case PCIE_MSI_INTR0_ENABLE:
>> + val = root->msi.intr[0].enable;
>> + break;
>> +
>> + case PCIE_MSI_INTR0_MASK:
>> + val = root->msi.intr[0].mask;
>> + break;
>> +
>> + case PCIE_MSI_INTR0_STATUS:
>> + val = root->msi.intr[0].status;
>> + break;
>> +
>> + case PCIE_PHY_DEBUG_R1:
>> + val = PCIE_PHY_DEBUG_R1_XMLH_LINK_UP;
>> + break;
>> +
>> + case PCIE_ATU_VIEWPORT:
>> + val = root->atu_viewport;
>> + break;
>> +
>> + case PCIE_ATU_LOWER_BASE:
>> + val = viewport->base;
>> + break;
>> +
>> + case PCIE_ATU_UPPER_BASE:
>> + val = viewport->base >> 32;
>> + break;
>> +
>> + case PCIE_ATU_LOWER_TARGET:
>> + val = viewport->target;
>> + break;
>> +
>> + case PCIE_ATU_UPPER_TARGET:
>> + val = viewport->target >> 32;
>> + break;
>> +
>> + case PCIE_ATU_LIMIT:
>> + val = viewport->limit;
>> + break;
>> +
>> + case PCIE_ATU_CR1:
>> + case PCIE_ATU_CR2: /* FALLTHROUGH */
>> + val = viewport->cr[(address - PCIE_ATU_CR1) / sizeof(uint32_t)];
>> + break;
>> +
>> + default:
>> + val = pci_default_read_config(d, address, len);
>> + break;
>> + }
>> +
>> + return val;
>> +}
>> +
>> +static uint64_t designware_pcie_root_data_read(void *opaque,
>> + hwaddr addr, unsigned len)
>> +{
>> + DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(opaque);
>> + DesignwarePCIEViewport *viewport =
>> + designware_pcie_root_get_current_viewport(root);
>> +
>> + const uint8_t busnum = PCIE_ATU_BUS(viewport->target);
>> + const uint8_t devfn = PCIE_ATU_DEVFN(viewport->target);
>> + PCIBus *pcibus = pci_get_bus(PCI_DEVICE(root));
>> + PCIDevice *pcidev = pci_find_device(pcibus, busnum, devfn);
>> +
>> + if (pcidev) {
>> + addr &= PCI_CONFIG_SPACE_SIZE - 1;
>> +
>> + return pci_host_config_read_common(pcidev, addr,
>> + PCI_CONFIG_SPACE_SIZE, len);
>> + }
>
> You can use "pci_data_read" instead
Good to know, will change.
>>
>> +
>> + return UINT64_MAX;
>> +}
>> +
>> +static void designware_pcie_root_data_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned len)
>> +{
>> + DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(opaque);
>> + DesignwarePCIEViewport *viewport =
>> + designware_pcie_root_get_current_viewport(root);
>> + const uint8_t busnum = PCIE_ATU_BUS(viewport->target);
>> + const uint8_t devfn = PCIE_ATU_DEVFN(viewport->target);
>> + PCIBus *pcibus = pci_get_bus(PCI_DEVICE(root));
>> + PCIDevice *pcidev = pci_find_device(pcibus, busnum, devfn);
>> +
>> + if (pcidev) {
>> + addr &= PCI_CONFIG_SPACE_SIZE - 1;
>> + pci_host_config_write_common(pcidev, addr,
>> + PCI_CONFIG_SPACE_SIZE,
>> + val, len);
>> + }
>
> You can use pci_data_write instead.
>
Ditto.
>> +}
>> +
>> +const MemoryRegionOps designware_pci_host_conf_ops = {
>> + .read = designware_pcie_root_data_read,
>> + .write = designware_pcie_root_data_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>
>
> Maybe you want to limit the access size also here.
>
OK, will do.
>> +};
>> +
>> +static void designware_pcie_update_viewport(DesignwarePCIERoot *root,
>> + DesignwarePCIEViewport
>> *viewport)
>> +{
>> + DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
>> +
>> + MemoryRegion *mem = &viewport->memory;
>> + const uint64_t target = viewport->target;
>> + const uint64_t base = viewport->base;
>> + const uint64_t size = (uint64_t)viewport->limit - base + 1;
>> + const bool inbound = viewport->inbound;
>> +
>> + MemoryRegion *source, *destination;
>> + const char *direction;
>> + char *name;
>> +
>> + if (inbound) {
>> + source = &host->pci.address_space_root;
>> + destination = get_system_memory();
>> + direction = "Inbound";
>> + } else {
>> + source = get_system_memory();
>> + destination = &host->pci.memory;
>> + direction = "Outbound";
>> + }
>> +
>> + if (memory_region_is_mapped(mem)) {
>> + /* Before we modify anything, unmap and destroy the region */
>
>
> I saw this also before. Can you please explain a little
> why/when do you need to destroy prev mappings?
>
They are going to be updated every time a viewport (inbound or
outbound) in address translation unit (iATU) is reconfigured. Because
PCIE_ATU_*_TARGET register is used to configure which deivce/bus to
address outgoing configuration TLP to, they (viewports) get
reconfigured quite a bit. Corresponding functions in Linux kernel
would be dw_pcie_prog_outbound_atu() and dw_pcie_rd_other_conf(). I
wouldn't be surprised that the way I went about implementing it is far
from optimal, so let me know if it is.
>> + memory_region_del_subregion(source, mem);
>> + object_unparent(OBJECT(mem));
>> + }
>> +
>> + name = g_strdup_printf("PCI %s Viewport %p", direction, viewport);
>> +
>> + switch (viewport->cr[0]) {
>> + case PCIE_ATU_TYPE_MEM:
>> + memory_region_init_alias(mem, OBJECT(root), name,
>> + destination, target, size);
>> + break;
>> + case PCIE_ATU_TYPE_CFG0:
>> + case PCIE_ATU_TYPE_CFG1: /* FALLTHROUGH */
>> + if (inbound) {
>> + goto exit;
>> + }
>> +
>> + memory_region_init_io(mem, OBJECT(root),
>> + &designware_pci_host_conf_ops,
>> + root, name, size);
>> + break;
>> + }
>> +
>> + if (inbound) {
>> + memory_region_add_subregion_overlap(source, base,
>> + mem, -1);
>> + } else {
>> + memory_region_add_subregion(source, base, mem);
>> + }
>> +
>> + exit:
>> + g_free(name);
>> +}
>> +
>> +static void designware_pcie_root_config_write(PCIDevice *d, uint32_t
>> address,
>> + uint32_t val, int len)
>> +{
>> + DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(d);
>> + DesignwarePCIEHost *host = designware_pcie_root_to_host(root);
>> + DesignwarePCIEViewport *viewport =
>> + designware_pcie_root_get_current_viewport(root);
>> +
>> + switch (address) {
>> + case PCIE_PORT_LINK_CONTROL:
>> + case PCIE_LINK_WIDTH_SPEED_CONTROL:
>> + case PCIE_PHY_DEBUG_R1:
>> + /* No-op */
>> + break;
>> +
>> + case PCIE_MSI_ADDR_LO:
>> + root->msi.base &= 0xFFFFFFFF00000000ULL;
>> + root->msi.base |= val;
>> + break;
>> +
>> + case PCIE_MSI_ADDR_HI:
>> + root->msi.base &= 0x00000000FFFFFFFFULL;
>> + root->msi.base |= (uint64_t)val << 32;
>> + break;
>> +
>> + case PCIE_MSI_INTR0_ENABLE: {
>> + const bool update_msi_mapping = !root->msi.intr[0].enable ^
>> !!val;
>> +
>> + root->msi.intr[0].enable = val;
>> +
>> + if (update_msi_mapping) {
>> + designware_pcie_root_update_msi_mapping(root);
>> + }
>> + break;
>> + }
>> +
>> + case PCIE_MSI_INTR0_MASK:
>> + root->msi.intr[0].mask = val;
>> + break;
>> +
>> + case PCIE_MSI_INTR0_STATUS:
>> + root->msi.intr[0].status ^= val;
>> + if (!root->msi.intr[0].status) {
>> + qemu_set_irq(host->pci.irqs[0], 0);
>> + }
>> + break;
>> +
>> + case PCIE_ATU_VIEWPORT:
>> + root->atu_viewport = val;
>> + break;
>> +
>> + case PCIE_ATU_LOWER_BASE:
>> + viewport->base &= 0xFFFFFFFF00000000ULL;
>> + viewport->base |= val;
>> + break;
>> +
>> + case PCIE_ATU_UPPER_BASE:
>> + viewport->base &= 0x00000000FFFFFFFFULL;
>> + viewport->base |= (uint64_t)val << 32;
>> + break;
>> +
>> + case PCIE_ATU_LOWER_TARGET:
>> + viewport->target &= 0xFFFFFFFF00000000ULL;
>> + viewport->target |= val;
>> + break;
>> +
>> + case PCIE_ATU_UPPER_TARGET:
>> + viewport->target &= 0x00000000FFFFFFFFULL;
>> + viewport->target |= val;
>> + break;
>> +
>> + case PCIE_ATU_LIMIT:
>> + viewport->limit = val;
>> + break;
>> +
>> + case PCIE_ATU_CR1:
>> + viewport->cr[0] = val;
>> + break;
>> + case PCIE_ATU_CR2:
>> + viewport->cr[1] = val;
>> +
>> + if (viewport->cr[1] & PCIE_ATU_ENABLE) {
>> + designware_pcie_update_viewport(root, viewport);
>> + }
>> + break;
>> +
>> + default:
>> + pci_bridge_write_config(d, address, val, len);
>> + break;
>> + }
>> +}
>> +
>> +static int designware_pcie_root_init(PCIDevice *dev)
>> +{
>
>
> Please use "realize" function rather than init.
> We want to get rid of it eventually.
OK, will do.
>>
>> + DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(dev);
>> + PCIBridge *br = PCI_BRIDGE(dev);
>> + DesignwarePCIEViewport *viewport;
>> + size_t i;
>> +
>> + br->bus_name = "dw-pcie";
>> +
>> + pci_set_word(dev->config + PCI_COMMAND,
>> + PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>> +
>> + pci_config_set_interrupt_pin(dev->config, 1);
>> + pci_bridge_initfn(dev, TYPE_PCI_BUS);
>
> So this is a PCI Express Root Port "sitting" on PCI bus?
Yes, it is a built-in PCIe bridge, whose configuration space is mapped
into CPU's address space (designware_pci_host_conf_ops) and the rest
of PCIe hierarchy is presented through it.
>
>> +
>> + pcie_port_init_reg(dev);
>> +
>> + pcie_cap_init(dev, 0x70, PCI_EXP_TYPE_ROOT_PORT,
>> + 0, &error_fatal);
>> +
>> + msi_nonbroken = true;
>> + msi_init(dev, 0x50, 32, true, true, &error_fatal);
>> +
>> + for (i = 0; i < DESIGNWARE_PCIE_NUM_VIEWPORTS; i++) {
>> + viewport = &root->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][i];
>> + viewport->inbound = true;
>> + }
>> +
>> + /*
>> + * If no inbound iATU windows are configured, HW defaults to
>> + * letting inbound TLPs to pass in. We emulate that by exlicitly
>> + * configuring first inbound window to cover all of target's
>> + * address space.
>> + *
>> + * NOTE: This will not work correctly for the case when first
>> + * configured inbound window is window 0
>> + */
>> + viewport = &root->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][0];
>> + viewport->base = 0x0000000000000000ULL;
>> + viewport->target = 0x0000000000000000ULL;
>> + viewport->limit = UINT32_MAX;
>> + viewport->cr[0] = PCIE_ATU_TYPE_MEM;
>> +
>> + designware_pcie_update_viewport(root, viewport);
>> +
>> + return 0;
>> +}
>> +
>> +static void designware_pcie_set_irq(void *opaque, int irq_num, int level)
>> +{
>> + DesignwarePCIEHost *host = DESIGNWARE_PCIE_HOST(opaque);
>> +
>> + qemu_set_irq(host->pci.irqs[irq_num], level);
>> +}
>> +
>> +static const char *designware_pcie_host_root_bus_path(PCIHostState
>> *host_bridge,
>> + PCIBus *rootbus)
>> +{
>> + return "0000:00";
>> +}
>> +
>> +
>> +static void designware_pcie_root_class_init(ObjectClass *klass, void
>> *data)
>> +{
>> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> +
>> + k->vendor_id = PCI_VENDOR_ID_SYNOPSYS;
>> + k->device_id = 0xABCD;
>> + k->revision = 0;
>> + k->class_id = PCI_CLASS_BRIDGE_HOST;
>
> So is a Root Port with call is "BRIDGE_HOST" ?
>
I think I am missing some PCI subsystem knowledge to understand that
question, would you mind re-phrasing it?
>> + k->is_express = true;
>> + k->is_bridge = true;
>> + k->init = designware_pcie_root_init;
>> + k->exit = pci_bridge_exitfn;
>> + dc->reset = pci_bridge_reset;
>> + k->config_read = designware_pcie_root_config_read;
>> + k->config_write = designware_pcie_root_config_write;
>
>
> Please add category:
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
It's already there, line 4 of the function.
>>
>> +
>> + /*
>>
>> + * PCI-facing part of the host bridge, not usable without the
>> + * host-facing part, which can't be device_add'ed, yet.
>> + */
>> + dc->user_creatable = false;
>> +}
>> +
>> +static uint64_t designware_pcie_host_mmio_read(void *opaque, hwaddr addr,
>> + unsigned int size)
>> +{
>> + PCIHostState *pci = PCI_HOST_BRIDGE(opaque);
>> + PCIDevice *device = pci_find_device(pci->bus, 0, 0);
>> +
>> + return pci_host_config_read_common(device,
>> + addr,
>> + pci_config_size(device),
>> + size);
>> +}
>> +
>> +static void designware_pcie_host_mmio_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned int
>> size)
>> +{
>> + PCIHostState *pci = PCI_HOST_BRIDGE(opaque);
>> + PCIDevice *device = pci_find_device(pci->bus, 0, 0);
>> +
>> + return pci_host_config_write_common(device,
>> + addr,
>> + pci_config_size(device),
>> + val, size);
>> +}
>> +
>> +static const MemoryRegionOps designware_pci_mmio_ops = {
>> + .read = designware_pcie_host_mmio_read,
>> + .write = designware_pcie_host_mmio_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static AddressSpace *designware_pcie_host_set_iommu(PCIBus *bus, void
>> *opaque,
>> + int devfn)
>> +{
>> + DesignwarePCIEHost *s = DESIGNWARE_PCIE_HOST(opaque);
>> +
>> + return &s->pci.address_space;
>> +}
>> +
>> +static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
>> +{
>> + PCIHostState *pci = PCI_HOST_BRIDGE(dev);
>> + DesignwarePCIEHost *s = DESIGNWARE_PCIE_HOST(dev);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + size_t i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(s->pci.irqs); i++) {
>> + sysbus_init_irq(sbd, &s->pci.irqs[i]);
>> + }
>> +
>> + memory_region_init_io(&s->mmio,
>> + OBJECT(s),
>> + &designware_pci_mmio_ops,
>> + s,
>> + "pcie.reg", 4 * 1024);
>> + sysbus_init_mmio(sbd, &s->mmio);
>> +
>> + memory_region_init(&s->pci.io, OBJECT(s), "pcie-pio", 16);
>> + memory_region_init(&s->pci.memory, OBJECT(s),
>> + "pcie-bus-memory",
>> + UINT64_MAX);
>> +
>> + pci->bus = pci_register_root_bus(dev, "pcie",
>> + designware_pcie_set_irq,
>> + pci_swizzle_map_irq_fn,
>> + s,
>> + &s->pci.memory,
>> + &s->pci.io,
>> + 0, 4,
>> + TYPE_PCIE_BUS);
>> +
>> + memory_region_init(&s->pci.address_space_root,
>> + OBJECT(s),
>> + "pcie-bus-address-space-root",
>> + UINT64_MAX);
>> + memory_region_add_subregion(&s->pci.address_space_root,
>> + 0x0, &s->pci.memory);
>> + address_space_init(&s->pci.address_space,
>> + &s->pci.address_space_root,
>> + "pcie-bus-address-space");
>> + pci_setup_iommu(pci->bus, designware_pcie_host_set_iommu, s);
>> +
>> + qdev_set_parent_bus(DEVICE(&s->root), BUS(pci->bus));
>> + qdev_init_nofail(DEVICE(&s->root));
>> +}
>> +
>> +static void designware_pcie_host_class_init(ObjectClass *klass, void
>> *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
>> +
>> + hc->root_bus_path = designware_pcie_host_root_bus_path;
>> + dc->realize = designware_pcie_host_realize;
>> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> + dc->fw_name = "pci";
>> +}
>> +
>> +static void designware_pcie_host_init(Object *obj)
>> +{
>> + DesignwarePCIEHost *s = DESIGNWARE_PCIE_HOST(obj);
>> + DesignwarePCIERoot *root = &s->root;
>> +
>> + object_initialize(root, sizeof(*root), TYPE_DESIGNWARE_PCIE_ROOT);
>> + object_property_add_child(obj, "root", OBJECT(root), NULL);
>> + qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
>> + qdev_prop_set_bit(DEVICE(root), "multifunction", false);
>> +}
>> +
>> +static const TypeInfo designware_pcie_root_info = {
>> + .name = TYPE_DESIGNWARE_PCIE_ROOT,
>> + .parent = TYPE_PCI_BRIDGE,
>> + .instance_size = sizeof(DesignwarePCIERoot),
>> + .class_init = designware_pcie_root_class_init,
>> + .interfaces = (InterfaceInfo[]) {
>> + { INTERFACE_PCIE_DEVICE },
>> + { }
>> + },
>> +};
>> +
>> +static const TypeInfo designware_pcie_host_info = {
>> + .name = TYPE_DESIGNWARE_PCIE_HOST,
>> + .parent = TYPE_PCI_HOST_BRIDGE,
>> + .instance_size = sizeof(DesignwarePCIEHost),
>> + .instance_init = designware_pcie_host_init,
>> + .class_init = designware_pcie_host_class_init,
>> +};
>> +
>> +static void designware_pcie_register(void)
>> +{
>> + type_register_static(&designware_pcie_root_info);
>> + type_register_static(&designware_pcie_host_info);
>> +}
>> +type_init(designware_pcie_register)
>> +
>> +/* 00:00.0 Class 0604: 16c3:abcd */
>> diff --git a/include/hw/pci-host/designware.h
>> b/include/hw/pci-host/designware.h
>> new file mode 100644
>> index 0000000000..55e45fcba0
>> --- /dev/null
>> +++ b/include/hw/pci-host/designware.h
>> @@ -0,0 +1,93 @@
>> +/*
>> + * Copyright (c) 2017, Impinj, Inc.
>> + *
>> + * Designware PCIe IP block emulation
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see
>> + * <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef DESIGNWARE_H
>> +#define DESIGNWARE_H
>> +
>> +#include "hw/hw.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/pci/pci.h"
>> +#include "hw/pci/pci_bus.h"
>> +#include "hw/pci/pcie_host.h"
>> +#include "hw/pci/pci_bridge.h"
>> +
>> +#define TYPE_DESIGNWARE_PCIE_HOST "designware-pcie-host"
>> +#define DESIGNWARE_PCIE_HOST(obj) \
>> + OBJECT_CHECK(DesignwarePCIEHost, (obj), TYPE_DESIGNWARE_PCIE_HOST)
>> +
>> +#define TYPE_DESIGNWARE_PCIE_ROOT "designware-pcie-root"
>> +#define DESIGNWARE_PCIE_ROOT(obj) \
>> + OBJECT_CHECK(DesignwarePCIERoot, (obj), TYPE_DESIGNWARE_PCIE_ROOT)
>> +
>> +typedef struct DesignwarePCIEViewport {
>> + MemoryRegion memory;
>> +
>> + uint64_t base;
>> + uint64_t target;
>> + uint32_t limit;
>> + uint32_t cr[2];
>> +
>> + bool inbound;
>> +} DesignwarePCIEViewport;
>> +
>> +typedef struct DesignwarePCIERoot {
>> + PCIBridge parent_obj;
>> +
>> + uint32_t atu_viewport;
>> +
>> +#define DESIGNWARE_PCIE_VIEWPORT_OUTBOUND 0
>> +#define DESIGNWARE_PCIE_VIEWPORT_INBOUND 1
>> +#define DESIGNWARE_PCIE_NUM_VIEWPORTS 4
>> +
>> + DesignwarePCIEViewport viewports[2][DESIGNWARE_PCIE_NUM_VIEWPORTS];
>> +
>> + struct {
>> + uint64_t base;
>> + MemoryRegion iomem;
>> +
>> + struct {
>> + uint32_t enable;
>> + uint32_t mask;
>> + uint32_t status;
>> + } intr[1];
>> + } msi;
>
>
> I think I didn't understand the msi struct above.
> Is it some special msi implementation?
> (related to the dumb question above)
Yes, it represents Designware specific MSI registers (part of a block
of vendor specific registers in configuration space of the root PCIe
bridge) as follows:
- PCIE_PL_MSICA, PCIE_PL_MSICUA via msi.base
- PCIE_PL_MSICI0_ENB via msi.intr[0].enable
- PCIE_PL_MSICI0_MASK via msi.intr[0].mask
- PCIE_PL_MSICI0_STATUS via msi.intr[0].status
i.MX6/7 specifies 8 sets of PCIE_PL_MSICn_* registers, so I defined it
as an array, but since Linux only uses first set I limited the number
of elements to 1.
Thanks,
Andrey Smirnov
- Re: [Qemu-arm] [PATCH v4 11/14] ARM: Add basic code to emulate A7MPCore DAP block, (continued)
Re: [Qemu-arm] [Qemu-devel] [PATCH v4 09/14] pci: Add support for Designware IP block, Marcel Apfelbaum, 2018/01/30
[Qemu-arm] [PATCH v4 13/14] hw/arm: Move virt's PSCI DT fixup code to arm/boot.c, Andrey Smirnov, 2018/01/15
[Qemu-arm] [PATCH v4 12/14] i.MX: Add i.MX7 SOC implementation., Andrey Smirnov, 2018/01/15
Message not available
Re: [Qemu-arm] [PATCH v4 00/14] Initial i.MX7 support, Peter Maydell, 2018/01/16
Re: [Qemu-arm] [PATCH v4 00/14] Initial i.MX7 support, Philippe Mathieu-Daudé, 2018/01/31