Hi Stefano,
> On 9 Sep 2021, at 11:54 pm, Stefano Stabellini <[email protected]> wrote:
>
> On Thu, 19 Aug 2021, Rahul Singh wrote:
>> XEN during boot will read the PCI device tree node “reg” property
>> and will map the PCI config space to the XEN memory.
>>
>> As of now "pci-host-ecam-generic" compatible board is supported.
>>
>> "linux,pci-domain" device tree property assigns a fixed PCI domain
>> number to a host bridge, otherwise an unstable (across boots) unique
>> number will be assigned by Linux.This property has to be in sync with
>> XEN to access the PCI devices.
>>
>> XEN will read the “linux,pci-domain” property from the device tree node
>> and configure the host bridge segment number accordingly. If this
>> property is not available XEN will allocate the unique segment number
>> to the host bridge.
>>
>> dt_get_pci_domain_nr(..) and dt_pci_bus_find_domain_nr(..) are directly
>> imported from the Linux source tree.
>>
>> Signed-off-by: Rahul Singh <[email protected]>
>> ---
>> xen/arch/arm/pci/Makefile | 2 +
>> xen/arch/arm/pci/pci-host-common.c | 261 ++++++++++++++++++++++++++++
>> xen/arch/arm/pci/pci-host-generic.c | 55 ++++++
>> xen/include/asm-arm/pci.h | 28 +++
>> 4 files changed, 346 insertions(+)
>> create mode 100644 xen/arch/arm/pci/pci-host-common.c
>> create mode 100644 xen/arch/arm/pci/pci-host-generic.c
>>
>> diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile
>> index a9ee0b9b44..f3d97f859e 100644
>> --- a/xen/arch/arm/pci/Makefile
>> +++ b/xen/arch/arm/pci/Makefile
>> @@ -1,2 +1,4 @@
>> obj-y += pci.o
>> obj-y += pci-access.o
>> +obj-y += pci-host-generic.o
>> +obj-y += pci-host-common.o
>> diff --git a/xen/arch/arm/pci/pci-host-common.c
>> b/xen/arch/arm/pci/pci-host-common.c
>> new file mode 100644
>> index 0000000000..9dd9b02271
>> --- /dev/null
>> +++ b/xen/arch/arm/pci/pci-host-common.c
>> @@ -0,0 +1,261 @@
>> +/*
>> + * Copyright (C) 2021 Arm Ltd.
>> + *
>> + * Based on Linux drivers/pci/ecam.c
>> + * Copyright 2016 Broadcom.
>> + *
>> + * Based on Linux drivers/pci/controller/pci-host-common.c
>> + * Based on Linux drivers/pci/controller/pci-host-generic.c
>> + * Copyright (C) 2014 ARM Limited Will Deacon <[email protected]>
>> + *
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program 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 General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <xen/init.h>
>> +#include <xen/pci.h>
>> +#include <asm/pci.h>
>> +#include <xen/rwlock.h>
>> +#include <xen/sched.h>
>> +#include <xen/vmap.h>
>> +
>> +/*
>> + * List for all the pci host bridges.
>> + */
>> +
>> +static LIST_HEAD(pci_host_bridges);
>> +
>> +static atomic_t domain_nr = ATOMIC_INIT(-1);
>> +
>> +bool dt_pci_parse_bus_range(struct dt_device_node *dev,
>> + struct pci_config_window *cfg)
>> +{
>> + const __be32 *cells;
>> + uint32_t len;
>> +
>> + cells = dt_get_property(dev, "bus-range", &len);
>> + /* bus-range should at least be 2 cells */
>> + if ( !cells || (len < (sizeof(*cells) * 2)) )
>> + return false;
>> +
>> + cfg->busn_start = dt_next_cell(1, &cells);
>> + cfg->busn_end = dt_next_cell(1, &cells);
>> +
>> + return true;
>> +}
>> +
>> +static inline void __iomem *pci_remap_cfgspace(paddr_t start, size_t len)
>> +{
>> + return ioremap_nocache(start, len);
>> +}
>> +
>> +static void pci_ecam_free(struct pci_config_window *cfg)
>> +{
>> + if ( cfg->win )
>> + iounmap(cfg->win);
>> +
>> + xfree(cfg);
>> +}
>> +
>> +static struct pci_config_window *gen_pci_init(struct dt_device_node *dev,
>> + int ecam_reg_idx)
>
> If it is only called at init time, then the function should be __init
Ack.
>
>
>> +{
>> + int err;
>> + struct pci_config_window *cfg;
>> + paddr_t addr, size;
>> +
>> + cfg = xzalloc(struct pci_config_window);
>> + if ( !cfg )
>> + return NULL;
>> +
>> + err = dt_pci_parse_bus_range(dev, cfg);
>> + if ( !err ) {
>> + cfg->busn_start = 0;
>> + cfg->busn_end = 0xff;
>> + printk(XENLOG_ERR "%s:No bus range found for pci controller\n",
>> + dt_node_full_name(dev));
>> + } else {
>> + if ( cfg->busn_end > cfg->busn_start + 0xff )
>> + cfg->busn_end = cfg->busn_start + 0xff;
>
> Is this a hard limit in the specification? Or is it a limit in the Xen
> implementation?
I just take the reference from Linux code.
https://elixir.bootlin.com/linux/latest/source/drivers/pci/of.c#L306
>
>
>> + }
>> +
>> + /* Parse our PCI ecam register address*/
> ^ space
>
Ack.
>> + err = dt_device_get_address(dev, ecam_reg_idx, &addr, &size);
>> + if ( err )
>> + goto err_exit;
>> +
>> + cfg->phys_addr = addr;
>> + cfg->size = size;
>> +
>> + /*
>> + * On 64-bit systems, we do a single ioremap for the whole config space
>> + * since we have enough virtual address range available. On 32-bit, we
>> + * ioremap the config space for each bus individually.
>> + *
>> + * As of now only 64-bit is supported 32-bit is not supported.
>> + */
>> + cfg->win = pci_remap_cfgspace(cfg->phys_addr, cfg->size);
>> + if ( !cfg->win )
>> + goto err_exit_remap;
>> +
>> + printk("ECAM at [mem %lx-%lx] for [bus %x-%x] \n",cfg->phys_addr,
>> + cfg->phys_addr + cfg->size - 1, cfg->busn_start, cfg->busn_end);
>> +
>> + return cfg;
>> +
>> +err_exit_remap:
>> + printk(XENLOG_ERR "ECAM ioremap failed\n");
>> +err_exit:
>> + pci_ecam_free(cfg);
>> + return NULL;
>> +}
>> +
>> +struct pci_host_bridge *pci_alloc_host_bridge(void)
>> +{
>> + struct pci_host_bridge *bridge = xzalloc(struct pci_host_bridge);
>> +
>> + if ( !bridge )
>> + return NULL;
>> +
>> + INIT_LIST_HEAD(&bridge->node);
>> + bridge->bus_start = ~0;
>> + bridge->bus_end = ~0;
>
> Please use INVALID_PADDR instead of ~0
bridge->bus_start is if type uint8_t if I use INVALID_PADDR I will get
overflow error.
I will use U8_MAX instead of INVALID_PADDR.
>
>
>> + return bridge;
>> +}
>> +
>> +void pci_add_host_bridge(struct pci_host_bridge *bridge)
>> +{
>> + list_add_tail(&bridge->node, &pci_host_bridges);
>> +}
>> +
>> +static int pci_get_new_domain_nr(void)
>> +{
>> + return atomic_inc_return(&domain_nr);
>> +}
>> +
>> +/*
>> + * This function will try to obtain the host bridge domain number by
>> + * finding a property called "linux,pci-domain" of the given device node.
>> + *
>> + * @node: device tree node with the domain information
>> + *
>> + * Returns the associated domain number from DT in the range [0-0xffff], or
>> + * a negative value if the required property is not found.
>> + */
>> +static int dt_get_pci_domain_nr(struct dt_device_node *node)
>> +{
>> + u32 domain;
>> + int error;
>> +
>> + error = dt_property_read_u32(node, "linux,pci-domain", &domain);
>> + if ( !error )
>> + return -EINVAL;
>> +
>> + return (u16)domain;
>
> Let's check that domain <= UINT16_MAX
>
Ack
Regards,
Rahul
