diff mbox

[v4,09/14] pci: Add support for Designware IP block

Message ID 20180116013709.13830-10-andrew.smirnov@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrey Smirnov Jan. 16, 2018, 1:37 a.m. UTC
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.maydell@linaro.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Cc: yurovsky@gmail.com
Signed-off-by: Andrey Smirnov <andrew.smirnov@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

Comments

Peter Maydell Jan. 16, 2018, 2:34 p.m. UTC | #1
On 16 January 2018 at 01:37, Andrey Smirnov <andrew.smirnov@gmail.com> 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.maydell@linaro.org>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org
> Cc: yurovsky@gmail.com
> Signed-off-by: Andrey Smirnov <andrew.smirnov@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

I'm not familiar enough with our PCI code to be able to review
this, I'm afraid. MST and Marcel are our PCI subsystem maintainers --
could one of you have a look at whether this seems to be a correct
implementation of a pcie host controller ?

I did notice it seems to be missing device state save/load support.

thanks
-- PMM
Marcel Apfelbaum Jan. 17, 2018, 3:23 p.m. UTC | #2
Hi Peter,

On 16/01/2018 16:34, Peter Maydell wrote:
> On 16 January 2018 at 01:37, Andrey Smirnov <andrew.smirnov@gmail.com> 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.maydell@linaro.org>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-arm@nongnu.org
>> Cc: yurovsky@gmail.com
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@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
> I'm not familiar enough with our PCI code to be able to review
> this, I'm afraid. MST and Marcel are our PCI subsystem maintainers --
> could one of you have a look at whether this seems to be a correct
> implementation of a pcie host controller ?

Sadly PCI Host bridges do not have a standard, each HW vendor
can do pretty much what they want.

That being said, if Andrey can point me to the PCI spec for the Designware
PCI host bridge and what parts they implemented for it I can have a look, sure.
(I will not be available for a week or so, but right after)

Thanks,
Marcel
> I did notice it seems to be missing device state save/load support.
>
> thanks
> -- PMM
>
Peter Maydell Jan. 17, 2018, 3:35 p.m. UTC | #3
On 17 January 2018 at 15:23, Marcel Apfelbaum <marcel.apfelbaum@zoho.com> wrote:
>
> On 16/01/2018 16:34, Peter Maydell wrote:
>> On 16 January 2018 at 01:37, Andrey Smirnov <andrew.smirnov@gmail.com>
>> I'm not familiar enough with our PCI code to be able to review
>> this, I'm afraid. MST and Marcel are our PCI subsystem maintainers --
>> could one of you have a look at whether this seems to be a correct
>> implementation of a pcie host controller ?
>
>
> Sadly PCI Host bridges do not have a standard, each HW vendor
> can do pretty much what they want.
>
> That being said, if Andrey can point me to the PCI spec for the Designware
> PCI host bridge and what parts they implemented for it I can have a look,
> sure.

I'm not so worried about whether it's implementing the spec for
the hardware (I trust Andrey has done enough testing for that
side of things), but whether the code seems to be structured
the way we expect a QEMU pcie host controller to be structured,
is using the right APIs, and so on.

thanks
-- PMM
Andrey Smirnov Jan. 17, 2018, 4:12 p.m. UTC | #4
On Wed, Jan 17, 2018 at 7:23 AM, Marcel Apfelbaum
<marcel.apfelbaum@zoho.com> wrote:
>
> Hi Peter,
>
>
> On 16/01/2018 16:34, Peter Maydell wrote:
>>
>> On 16 January 2018 at 01:37, Andrey Smirnov <andrew.smirnov@gmail.com>
>> 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.maydell@linaro.org>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Cc: qemu-devel@nongnu.org
>>> Cc: qemu-arm@nongnu.org
>>> Cc: yurovsky@gmail.com
>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@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
>>
>> I'm not familiar enough with our PCI code to be able to review
>> this, I'm afraid. MST and Marcel are our PCI subsystem maintainers --
>> could one of you have a look at whether this seems to be a correct
>> implementation of a pcie host controller ?
>
>
> Sadly PCI Host bridges do not have a standard, each HW vendor
> can do pretty much what they want.
>
> That being said, if Andrey can point me to the PCI spec for the Designware
> PCI host bridge and what parts they implemented for it I can have a look,
> sure.
> (I will not be available for a week or so, but right after)
>

Just in case you still want this:

To the best of my knowledge, Synposys does not provide specification
for their PCIe IP to general public and I am in no way affiliated with
them, so I don't have any backchannels to get it any other way.

The next best thing to an actual spec, that I found to be pretty
useful, is PCIe chapter of i.MX6Q Reference Manual
(https://www.nxp.com/docs/en/reference-manual/IMX6DQRM.pdf   page
4049), which is what I used to implement the code in question.

Last, and probably the most important, "source of truth" was actual
Linux PCIe driver for i.MX/Designware which I used as a sort of
inverse reference implementation.

Thanks,
Andrey Smirnov
Marcel Apfelbaum Jan. 17, 2018, 4:12 p.m. UTC | #5
On 17/01/2018 17:35, Peter Maydell wrote:
> On 17 January 2018 at 15:23, Marcel Apfelbaum <marcel.apfelbaum@zoho.com> wrote:
>>
>> On 16/01/2018 16:34, Peter Maydell wrote:
>>> On 16 January 2018 at 01:37, Andrey Smirnov <andrew.smirnov@gmail.com>
>>> I'm not familiar enough with our PCI code to be able to review
>>> this, I'm afraid. MST and Marcel are our PCI subsystem maintainers --
>>> could one of you have a look at whether this seems to be a correct
>>> implementation of a pcie host controller ?
>>
>>
>> Sadly PCI Host bridges do not have a standard, each HW vendor
>> can do pretty much what they want.
>>
>> That being said, if Andrey can point me to the PCI spec for the Designware
>> PCI host bridge and what parts they implemented for it I can have a look,
>> sure.
> 
> I'm not so worried about whether it's implementing the spec for
> the hardware (I trust Andrey has done enough testing for that
> side of things), but whether the code seems to be structured
> the way we expect a QEMU pcie host controller to be structured,
> is using the right APIs, and so on.
> 

Got it, I'll review in a week.

Thanks,
Marcel

> thanks
> -- PMM
>
Marcel Apfelbaum Jan. 17, 2018, 4:17 p.m. UTC | #6
On 17/01/2018 18:12, Andrey Smirnov wrote:
> On Wed, Jan 17, 2018 at 7:23 AM, Marcel Apfelbaum
> <marcel.apfelbaum@zoho.com> wrote:
>>
>> Hi Peter,
>>
>>
>> On 16/01/2018 16:34, Peter Maydell wrote:
>>>
>>> On 16 January 2018 at 01:37, Andrey Smirnov <andrew.smirnov@gmail.com>
>>> 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.maydell@linaro.org>
>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Cc: qemu-devel@nongnu.org
>>>> Cc: qemu-arm@nongnu.org
>>>> Cc: yurovsky@gmail.com
>>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@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
>>>
>>> I'm not familiar enough with our PCI code to be able to review
>>> this, I'm afraid. MST and Marcel are our PCI subsystem maintainers --
>>> could one of you have a look at whether this seems to be a correct
>>> implementation of a pcie host controller ?
>>
>>
>> Sadly PCI Host bridges do not have a standard, each HW vendor
>> can do pretty much what they want.
>>
>> That being said, if Andrey can point me to the PCI spec for the Designware
>> PCI host bridge and what parts they implemented for it I can have a look,
>> sure.
>> (I will not be available for a week or so, but right after)
>>
> 
> Just in case you still want this:
> 
> To the best of my knowledge, Synposys does not provide specification
> for their PCIe IP to general public and I am in no way affiliated with
> them, so I don't have any backchannels to get it any other way.
> 
> The next best thing to an actual spec, that I found to be pretty
> useful, is PCIe chapter of i.MX6Q Reference Manual
> (https://www.nxp.com/docs/en/reference-manual/IMX6DQRM.pdf   page
> 4049), which is what I used to implement the code in question.
> 

Appreciated.

> Last, and probably the most important, "source of truth" was actual
> Linux PCIe driver for i.MX/Designware which I used as a sort of
> inverse reference implementation.
> 

We did the same for our PVRDMA device implementation :)


Thanks,
Marcel

> Thanks,
> Andrey Smirnov
>
Philippe Mathieu-Daudé Jan. 17, 2018, 4:45 p.m. UTC | #7
Hi Andrey,

On Wed, Jan 17, 2018 at 1:12 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> On Wed, Jan 17, 2018 at 7:23 AM, Marcel Apfelbaum
> <marcel.apfelbaum@zoho.com> wrote:
>> That being said, if Andrey can point me to the PCI spec for the Designware
>> PCI host bridge and what parts they implemented for it I can have a look,
>> sure.
>> (I will not be available for a week or so, but right after)
>>
>
> Just in case you still want this:
>
> To the best of my knowledge, Synposys does not provide specification
> for their PCIe IP to general public and I am in no way affiliated with
> them, so I don't have any backchannels to get it any other way.
>
> The next best thing to an actual spec, that I found to be pretty
> useful, is PCIe chapter of i.MX6Q Reference Manual
> (https://www.nxp.com/docs/en/reference-manual/IMX6DQRM.pdf   page
> 4049), which is what I used to implement the code in question.

Please add a comment about it in your respin, such:

Based on "i.MX6 Applications Processor Reference Manual" (Document
Number: IMX6DQRM Rev. 4)

> Last, and probably the most important, "source of truth" was actual
> Linux PCIe driver for i.MX/Designware which I used as a sort of
> inverse reference implementation.

Same here:

Reversed from Linux v4.9 drivers/pci/host/pcie-designware.c
Marcel Apfelbaum Jan. 30, 2018, 1:18 p.m. UTC | #8
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.maydell@linaro.org>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org
> Cc: yurovsky@gmail.com
> Signed-off-by: Andrey Smirnov <andrew.smirnov@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 ?
> +    }
> +}
> +
> +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.
> +};
> +
> +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?
Is it possible to be also disabled?

> +    }
> +}
> +
> +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
> +        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
> +
> +    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.

> +}
> +
> +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.
> +};
> +
> +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?
> +        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.
> +    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?
> +
> +    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" ?

> +    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);
> +
> +    /*
> +     * 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)



> +} DesignwarePCIERoot;
> +
> +typedef struct DesignwarePCIEHost {
> +    PCIHostState parent_obj;
> +
> +    bool link_up;
> +
> +    DesignwarePCIERoot root;
> +
> +    struct {
> +        AddressSpace address_space;
> +        MemoryRegion address_space_root;
> +
> +        MemoryRegion memory;
> +        MemoryRegion io;
> +
> +        qemu_irq     irqs[4];
> +    } pci;
> +
> +    MemoryRegion mmio;
> +} DesignwarePCIEHost;
> +
> +#endif  /* DESIGNWARE_H */
> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
> index 35df1874a9..23fefe1bc6 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -266,4 +266,6 @@
>   #define PCI_VENDOR_ID_TEWS               0x1498
>   #define PCI_DEVICE_ID_TEWS_TPCI200       0x30C8
>   
> +#define PCI_VENDOR_ID_SYNOPSYS           0x16C3
> +
>   #endif


The host-bridge part looks OK to me, the Root Port part looks strange but it may

be because of the chipset.

Thanks,

Marcel
Andrey Smirnov Jan. 30, 2018, 5:49 p.m. UTC | #9
On Tue, Jan 30, 2018 at 5:18 AM, Marcel Apfelbaum
<marcel.apfelbaum@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.maydell@linaro.org>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Cc: qemu-devel@nongnu.org
>> Cc: qemu-arm@nongnu.org
>> Cc: yurovsky@gmail.com
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@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.

>>
>> +    }
>> +}
>> +
>> +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
Marcel Apfelbaum Jan. 31, 2018, 12:13 p.m. UTC | #10
On 30/01/2018 19:49, Andrey Smirnov wrote:
> On Tue, Jan 30, 2018 at 5:18 AM, Marcel Apfelbaum
> <marcel.apfelbaum@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.maydell@linaro.org>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Cc: qemu-devel@nongnu.org
>>> Cc: qemu-arm@nongnu.org
>>> Cc: yurovsky@gmail.com
>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@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
>
Andrey Smirnov Feb. 7, 2018, 4:10 a.m. UTC | #11
On Wed, Jan 31, 2018 at 4:13 AM, Marcel Apfelbaum <marcel@redhat.com> wrote:
> On 30/01/2018 19:49, Andrey Smirnov wrote:
>> On Tue, Jan 30, 2018 at 5:18 AM, Marcel Apfelbaum
>> <marcel.apfelbaum@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.maydell@linaro.org>
>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Cc: qemu-devel@nongnu.org
>>>> Cc: qemu-arm@nongnu.org
>>>> Cc: yurovsky@gmail.com
>>>> Signed-off-by: Andrey Smirnov <andrew.smirnov@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.
>

Sounds good. Will do in v5.

>>> 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.
>

Will do in v5.

>
>>>> +        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);
>

Oops, my bad. Will fix in v5.

>>
>>>
>>>> +
>>>> +    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.
>

Yeah, that just mistake on my part since real HW reports
0604/PCI_CLASS_BRIDGE_PCI. Will fix in v5.

Thanks,
Andrey Smirnov
diff mbox

Patch

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.
+ *
+ * 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);
+    }
+}
+
+const MemoryRegionOps designware_pci_host_msi_ops = {
+    .write = designware_pcie_root_msi_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+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);
+    }
+}
+
+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 */
+        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);
+    }
+
+    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);
+    }
+}
+
+const MemoryRegionOps designware_pci_host_conf_ops = {
+    .read = designware_pcie_root_data_read,
+    .write = designware_pcie_root_data_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+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 */
+        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)
+{
+    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);
+
+    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;
+    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;
+
+    /*
+     * 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;
+} DesignwarePCIERoot;
+
+typedef struct DesignwarePCIEHost {
+    PCIHostState parent_obj;
+
+    bool link_up;
+
+    DesignwarePCIERoot root;
+
+    struct {
+        AddressSpace address_space;
+        MemoryRegion address_space_root;
+
+        MemoryRegion memory;
+        MemoryRegion io;
+
+        qemu_irq     irqs[4];
+    } pci;
+
+    MemoryRegion mmio;
+} DesignwarePCIEHost;
+
+#endif  /* DESIGNWARE_H */
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index 35df1874a9..23fefe1bc6 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -266,4 +266,6 @@ 
 #define PCI_VENDOR_ID_TEWS               0x1498
 #define PCI_DEVICE_ID_TEWS_TPCI200       0x30C8
 
+#define PCI_VENDOR_ID_SYNOPSYS           0x16C3
+
 #endif