qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gustavo Romero <gustavo.romero@linaro.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>, qemu-devel@nongnu.org
Cc: "Andrey Smirnov" <andrew.smirnov@gmail.com>,
	qemu-arm@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH 8/8] hw/pci-host/designware: Create ViewPorts during host bridge realization
Date: Mon, 19 Aug 2024 01:23:49 -0300	[thread overview]
Message-ID: <9ce96a75-ad19-2d3e-50a3-640aae63dc4e@linaro.org> (raw)
In-Reply-To: <20231012121857.31873-9-philmd@linaro.org>

Hi Phil,

On 10/12/23 9:18 AM, Philippe Mathieu-Daudé wrote:
> ViewPorts are managed by the host bridge part, so create them
> when the host bridge is realized. The host bridge become the

nit: becomes


> owner of the memory regions.
> 
> The PCI root function realize() method now only contains PCI
> specific code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>


Cheers,
Gustavo

> ---
>   hw/pci-host/designware.c | 207 +++++++++++++++++++--------------------
>   1 file changed, 102 insertions(+), 105 deletions(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 6cb8655a75..e5dc9b4b8d 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -384,22 +384,10 @@ static char *designware_pcie_viewport_name(const char *direction,
>   static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
>   {
>       DesignwarePCIERoot *root = DESIGNWARE_PCIE_ROOT(dev);
> -    DesignwarePCIEHost *host = DESIGNWARE_PCIE_HOST(
> -                                    qdev_get_parent_bus(DEVICE(dev))->parent);
> -    MemoryRegion *host_mem = get_system_memory();
> -    MemoryRegion *address_space = &host->pci.memory;
>       PCIBridge *br = PCI_BRIDGE(dev);
> -    DesignwarePCIEViewport *viewport;
> -    /*
> -     * Dummy values used for initial configuration of MemoryRegions
> -     * that belong to a given viewport
> -     */
> -    const hwaddr dummy_offset = 0;
> -    const uint64_t dummy_size = 4;
> -    size_t i;
>   
>       br->bus_name  = "dw-pcie";
> -    root->host = host;
> +    root->host = DESIGNWARE_PCIE_HOST(qdev_get_parent_bus(DEVICE(dev))->parent);
>   
>       pci_set_word(dev->config + PCI_COMMAND,
>                    PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> @@ -414,97 +402,6 @@ static void designware_pcie_root_realize(PCIDevice *dev, Error **errp)
>   
>       msi_nonbroken = true;
>       msi_init(dev, 0x50, 32, true, true, &error_fatal);
> -
> -    for (i = 0; i < DESIGNWARE_PCIE_NUM_VIEWPORTS; i++) {
> -        MemoryRegion *source, *destination, *mem;
> -        const char *direction;
> -        char *name;
> -
> -        viewport = &host->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][i];
> -        viewport->inbound = true;
> -        viewport->base    = 0x0000000000000000ULL;
> -        viewport->target  = 0x0000000000000000ULL;
> -        viewport->limit   = UINT32_MAX;
> -        viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
> -
> -        source      = &host->pci.address_space_root;
> -        destination = host_mem;
> -        direction   = "Inbound";
> -
> -        /*
> -         * Configure MemoryRegion implementing PCI -> CPU memory
> -         * access
> -         */
> -        mem  = &viewport->mem;
> -        name = designware_pcie_viewport_name(direction, i, "MEM");
> -        memory_region_init_alias(mem, OBJECT(root), name, destination,
> -                                 dummy_offset, dummy_size);
> -        memory_region_add_subregion_overlap(source, dummy_offset, mem, -1);
> -        memory_region_set_enabled(mem, false);
> -        g_free(name);
> -
> -        viewport = &host->viewports[DESIGNWARE_PCIE_VIEWPORT_OUTBOUND][i];
> -        viewport->host    = host;
> -        viewport->inbound = false;
> -        viewport->base    = 0x0000000000000000ULL;
> -        viewport->target  = 0x0000000000000000ULL;
> -        viewport->limit   = UINT32_MAX;
> -        viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
> -
> -        destination = &host->pci.memory;
> -        direction   = "Outbound";
> -        source      = host_mem;
> -
> -        /*
> -         * Configure MemoryRegion implementing CPU -> PCI memory
> -         * access
> -         */
> -        mem  = &viewport->mem;
> -        name = designware_pcie_viewport_name(direction, i, "MEM");
> -        memory_region_init_alias(mem, OBJECT(root), name, destination,
> -                                 dummy_offset, dummy_size);
> -        memory_region_add_subregion(source, dummy_offset, mem);
> -        memory_region_set_enabled(mem, false);
> -        g_free(name);
> -
> -        /*
> -         * Configure MemoryRegion implementing access to configuration
> -         * space
> -         */
> -        mem  = &viewport->cfg;
> -        name = designware_pcie_viewport_name(direction, i, "CFG");
> -        memory_region_init_io(&viewport->cfg, OBJECT(root),
> -                              &designware_pci_host_conf_ops,
> -                              viewport, name, dummy_size);
> -        memory_region_add_subregion(source, dummy_offset, mem);
> -        memory_region_set_enabled(mem, false);
> -        g_free(name);
> -    }
> -
> -    /*
> -     * If no inbound iATU windows are configured, HW defaults to
> -     * letting inbound TLPs to pass in. We emulate that by explicitly
> -     * 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 = &host->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][0];
> -    viewport->cr[1] = DESIGNWARE_PCIE_ATU_ENABLE;
> -    designware_pcie_update_viewport(root, viewport);
> -
> -    memory_region_init_io(&host->msi.iomem, OBJECT(root),
> -                          &designware_pci_host_msi_ops,
> -                          root, "pcie-msi", 0x4);
> -    /*
> -     * We initially place MSI interrupt I/O region at address 0 and
> -     * disable it. It'll be later moved to correct offset and enabled
> -     * in designware_pcie_root_update_msi_mapping() as a part of
> -     * initialization done by guest OS
> -     */
> -    memory_region_add_subregion(address_space, dummy_offset, &host->msi.iomem);
> -    memory_region_set_enabled(&host->msi.iomem, false);
>   }
>   
>   static void designware_pcie_set_irq(void *opaque, int irq_num, int level)
> @@ -590,7 +487,7 @@ static void designware_pcie_root_class_init(ObjectClass *klass, void *data)
>       dc->reset = pci_bridge_reset;
>       /*
>        * PCI-facing part of the host bridge, not usable without the
> -     * host-facing part, which can't be device_add'ed, yet.
> +     * host-facing part.
>        */
>       dc->user_creatable = false;
>       dc->vmsd = &vmstate_designware_pcie_root;
> @@ -650,8 +547,17 @@ 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);
> +    MemoryRegion *host_mem = get_system_memory();
> +    DesignwarePCIEViewport *viewport;
>       size_t i;
>   
> +    /*
> +     * Dummy values used for initial configuration of MemoryRegions
> +     * that belong to a given viewport
> +     */
> +    const hwaddr dummy_offset = 0;
> +    const uint64_t dummy_size = 4;
> +
>       for (i = 0; i < ARRAY_SIZE(s->pci.irqs); i++) {
>           sysbus_init_irq(sbd, &s->pci.irqs[i]);
>       }
> @@ -694,6 +600,97 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
>       qdev_prop_set_int32(DEVICE(&s->root), "addr", PCI_DEVFN(0, 0));
>       qdev_prop_set_bit(DEVICE(&s->root), "multifunction", false);
>       qdev_realize(DEVICE(&s->root), BUS(pci->bus), &error_fatal);
> +
> +    memory_region_init_io(&s->msi.iomem, OBJECT(s),
> +                          &designware_pci_host_msi_ops,
> +                          s, "pcie-msi", 0x4);
> +    /*
> +     * We initially place MSI interrupt I/O region at address 0 and
> +     * disable it. It'll be later moved to correct offset and enabled
> +     * in designware_pcie_host_update_msi_mapping() as a part of
> +     * initialization done by guest OS
> +     */
> +    memory_region_add_subregion(&s->pci.memory, dummy_offset, &s->msi.iomem);
> +    memory_region_set_enabled(&s->msi.iomem, false);
> +
> +    for (i = 0; i < DESIGNWARE_PCIE_NUM_VIEWPORTS; i++) {
> +        MemoryRegion *source, *destination, *mem;
> +        const char *direction;
> +        char *name;
> +
> +        viewport = &s->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][i];
> +        viewport->inbound = true;
> +        viewport->base    = 0x0000000000000000ULL;
> +        viewport->target  = 0x0000000000000000ULL;
> +        viewport->limit   = UINT32_MAX;
> +        viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
> +
> +        source      = &s->pci.address_space_root;
> +        destination = host_mem;
> +        direction   = "Inbound";
> +
> +        /*
> +         * Configure MemoryRegion implementing PCI -> CPU memory
> +         * access
> +         */
> +        mem  = &viewport->mem;
> +        name = designware_pcie_viewport_name(direction, i, "MEM");
> +        memory_region_init_alias(mem, OBJECT(s), name, destination,
> +                                 dummy_offset, dummy_size);
> +        memory_region_add_subregion_overlap(source, dummy_offset, mem, -1);
> +        memory_region_set_enabled(mem, false);
> +        g_free(name);
> +
> +        viewport = &s->viewports[DESIGNWARE_PCIE_VIEWPORT_OUTBOUND][i];
> +        viewport->host    = s;
> +        viewport->inbound = false;
> +        viewport->base    = 0x0000000000000000ULL;
> +        viewport->target  = 0x0000000000000000ULL;
> +        viewport->limit   = UINT32_MAX;
> +        viewport->cr[0]   = DESIGNWARE_PCIE_ATU_TYPE_MEM;
> +
> +        destination = &s->pci.memory;
> +        direction   = "Outbound";
> +        source      = host_mem;
> +
> +        /*
> +         * Configure MemoryRegion implementing CPU -> PCI memory
> +         * access
> +         */
> +        mem  = &viewport->mem;
> +        name = designware_pcie_viewport_name(direction, i, "MEM");
> +        memory_region_init_alias(mem, OBJECT(s), name, destination,
> +                                 dummy_offset, dummy_size);
> +        memory_region_add_subregion(source, dummy_offset, mem);
> +        memory_region_set_enabled(mem, false);
> +        g_free(name);
> +
> +        /*
> +         * Configure MemoryRegion implementing access to configuration
> +         * space
> +         */
> +        mem  = &viewport->cfg;
> +        name = designware_pcie_viewport_name(direction, i, "CFG");
> +        memory_region_init_io(&viewport->cfg, OBJECT(s),
> +                              &designware_pci_host_conf_ops,
> +                              viewport, name, dummy_size);
> +        memory_region_add_subregion(source, dummy_offset, mem);
> +        memory_region_set_enabled(mem, false);
> +        g_free(name);
> +    }
> +
> +    /*
> +     * If no inbound iATU windows are configured, HW defaults to
> +     * letting inbound TLPs to pass in. We emulate that by explicitly
> +     * 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 = &s->viewports[DESIGNWARE_PCIE_VIEWPORT_INBOUND][0];
> +    viewport->cr[1] = DESIGNWARE_PCIE_ATU_ENABLE;
> +    designware_pcie_update_viewport(&s->root, viewport);
>   }
>   
>   static const VMStateDescription vmstate_designware_pcie_host = {
> 


  reply	other threads:[~2024-08-19  4:27 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-12 12:18 [PATCH 0/8] hw/pci-host/designware: QOM shuffling (Host bridge <-> Root function) Philippe Mathieu-Daudé
2023-10-12 12:18 ` [PATCH 1/8] hw/pci-host/designware: Declare CPU QOM types using DEFINE_TYPES() macro Philippe Mathieu-Daudé
2023-10-17 16:31   ` Peter Maydell
2024-08-19  4:21   ` Gustavo Romero
2024-09-10 14:33     ` Philippe Mathieu-Daudé
2023-10-12 12:18 ` [PATCH 2/8] hw/pci-host/designware: Initialize root function in host bridge realize Philippe Mathieu-Daudé
2023-10-17 16:32   ` Peter Maydell
2024-02-06 16:35     ` Philippe Mathieu-Daudé
2024-08-19  4:21   ` Gustavo Romero
2023-10-12 12:18 ` [PATCH 3/8] hw/pci-host/designware: Add 'host_mem' variable for clarity Philippe Mathieu-Daudé
2023-10-17 16:33   ` Peter Maydell
2024-08-19  4:21   ` Gustavo Romero
2023-10-12 12:18 ` [PATCH 4/8] hw/pci-host/designware: Hoist host controller in root function #0 Philippe Mathieu-Daudé
2024-08-19  4:22   ` Gustavo Romero
2023-10-12 12:18 ` [PATCH 5/8] hw/pci-host/designware: Keep host reference in DesignwarePCIEViewport Philippe Mathieu-Daudé
2024-08-19  4:22   ` Gustavo Romero
2023-10-12 12:18 ` [PATCH 6/8] hw/pci-host/designware: Move viewports from root func to host bridge Philippe Mathieu-Daudé
2024-08-19  4:23   ` Gustavo Romero
2023-10-12 12:18 ` [PATCH 7/8] hw/pci-host/designware: Move MSI registers " Philippe Mathieu-Daudé
2024-08-19  4:23   ` Gustavo Romero
2023-10-12 12:18 ` [PATCH 8/8] hw/pci-host/designware: Create ViewPorts during host bridge realization Philippe Mathieu-Daudé
2024-08-19  4:23   ` Gustavo Romero [this message]
2023-10-27 12:18 ` [PATCH 0/8] hw/pci-host/designware: QOM shuffling (Host bridge <-> Root function) Peter Maydell
2023-11-15 14:47 ` [PATCH-for-9.0 " Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ce96a75-ad19-2d3e-50a3-640aae63dc4e@linaro.org \
    --to=gustavo.romero@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=andrew.smirnov@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).