On 30/01/2018 19:49, Andrey Smirnov wrote:
> On Tue, Jan 30, 2018 at 5:18 AM, Marcel Apfelbaum
> <marcel.apfelb...@zoho.com> 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 <peter.mayd...@linaro.org>
>>> Cc: Jason Wang <jasow...@redhat.com>
>>> Cc: Philippe Mathieu-Daudé <f4...@amsat.org>
>>> Cc: qemu-devel@nongnu.org
>>> Cc: qemu-...@nongnu.org
>>> Cc: yurov...@gmail.com
>>> Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com>
>>> ---
>>> 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.
>
Not sure of a "better" way. The point was the "msi" naming
that suggests edge interrupts, while resulting into level interrupts.
I was wondering about the naming, nothing more.
>>>
>>> + }
>>> +}
>>> +
>>> +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.
>
OK, so I am pretty sure we call "memory_region_init_io" only once
in the init/realize part.
Then, if the address mappings is getting changed between the calls
you can use memory_region_del_subregion/memory_region_add_subregion on update.
If the mappings remains the same you can use memory_region_set_enabled
to disable/enable the memory region.
>> 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.
>
The same as above, I think memory_region_init_io should be used once.
>>> + 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.
My point was: is the bus PCIe or PCI? Because you used:
pci_bridge_initfn(dev, TYPE_PCI_BUS);
and not
pci_bridge_initfn(dev, TYPE_PCIE_BUS);
>
>>
>>> +
>>> + 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?
Sure, a Root Port is a type of PCI bridge, so I was expecting
the class id to be: PCI_CLASS_BRIDGE_PCI and not PCI_CLASS_BRIDGE_HOST.
>
>>> + 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.
>
Sorry for the noise
>>>
>>> +
>>> + /*
>>>
>>> + * 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.
>
Got it, thanks.
Marcel
> Thanks,
> Andrey Smirnov
>
