Re: [PATCH v1 3/6] xen/arm: create dom0less virtio-pci DT node

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 9/24/24 12:23, Edgar E. Iglesias wrote:
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index 09b65e44ae..dab24fa9e2 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -586,6 +586,189 @@ static int __init domain_handle_dtb_bootmodule(struct 
> domain *d,
>      return res;
>  }
>  
> +static int __init make_virtio_pci_domU_node(const struct kernel_info *kinfo)
> +{
> +    void *fdt = kinfo->fdt;
> +    /* reg is sized to be used for all the needed properties below */
> +    __be32 reg[(1 + (GUEST_ROOT_ADDRESS_CELLS * 2) + GUEST_ROOT_SIZE_CELLS)
> +               * 2];
> +    __be32 irq_map[4 * 4 * 8];
> +    __be32 *cells;
> +    char buf[22]; /* pcie@ + max 16 char address + '\0' */
> +    int res;
> +    int devfn, intx_pin;
> +    static const char compat[] = "pci-host-ecam-generic";
> +    static const char reg_names[] = "ecam";
> +
> +    if ( p2m_ipa_bits <= 40 ) {
> +        printk("PA bits %d is too small!\nvirtio-pci is only supported "
> +               "on platforms with PA bits > 40\n", p2m_ipa_bits);
> +        return -EINVAL;
> +    }
> +
> +    snprintf(buf, sizeof(buf), "pcie@%lx", kinfo->virtio_pci.ecam.base);
> +    dt_dprintk("Create virtio-pci node\n");
> +    res = fdt_begin_node(fdt, buf);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "compatible", compat, sizeof(compat));
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_string(fdt, "device_type", "pci");
> +    if ( res )
> +        return res;
> +
> +    /* Create reg property */
> +    cells = &reg[0];
> +    dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, 
> GUEST_ROOT_SIZE_CELLS,
> +                       kinfo->virtio_pci.ecam.base, 
> GUEST_VIRTIO_PCI_ECAM_SIZE);
> +
> +    res = fdt_property(fdt, "reg", reg,
> +                       (GUEST_ROOT_ADDRESS_CELLS +
> +                        GUEST_ROOT_SIZE_CELLS) * sizeof(*reg));
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "reg-names", reg_names, sizeof(reg_names));
> +    if ( res )
> +        return res;
> +
> +    /* Create bus-range property */
> +    cells = &reg[0];
> +    dt_set_cell(&cells, 1, 0);
> +    dt_set_cell(&cells, 1, 0xff);
> +    res = fdt_property(fdt, "bus-range", reg, 2 * sizeof(*reg));
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#address-cells", 3);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#size-cells", 2);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_string(fdt, "status", "okay");
> +    if ( res )
> +        return res;
> +
> +    /*
> +     * Create ranges property as:
> +     * <(PCI bitfield) (PCI address) (CPU address) (Size)>
> +     */
> +    cells = &reg[0];
> +    dt_set_cell(&cells, 1, GUEST_VIRTIO_PCI_MEM_TYPE);
> +    dt_set_cell(&cells, GUEST_ROOT_ADDRESS_CELLS, 
> kinfo->virtio_pci.mem.base);
> +    dt_set_cell(&cells, GUEST_ROOT_ADDRESS_CELLS, 
> kinfo->virtio_pci.mem.base);
> +    dt_set_cell(&cells, GUEST_ROOT_SIZE_CELLS, GUEST_VIRTIO_PCI_MEM_SIZE);
> +    dt_set_cell(&cells, 1, GUEST_VIRTIO_PCI_PREFETCH_MEM_TYPE);
> +    dt_set_cell(&cells, GUEST_ROOT_ADDRESS_CELLS, 
> kinfo->virtio_pci.pf_mem.base);
> +    dt_set_cell(&cells, GUEST_ROOT_ADDRESS_CELLS, 
> kinfo->virtio_pci.pf_mem.base);
> +    dt_set_cell(&cells, GUEST_ROOT_SIZE_CELLS, 
> GUEST_VIRTIO_PCI_PREFETCH_MEM_SIZE);
> +    res = fdt_property(fdt, "ranges", reg, 14 * sizeof(*reg));
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "dma-coherent", "", 0);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property_cell(fdt, "#interrupt-cells", 1);
> +    if ( res )
> +        return res;
> +
> +    /*
> +     * PCI-to-PCI bridge specification
> +     * 9.1: Interrupt routing. Table 9-1
> +     *
> +     * the PCI Express Base Specification, Revision 2.1
> +     * 2.2.8.1: INTx interrupt signaling - Rules
> +     *          the Implementation Note
> +     *          Table 2-20
> +     */
> +    cells = &irq_map[0];
> +    for (devfn = 0; devfn <= 0x18; devfn += 8) {
> +        for (intx_pin = 0; intx_pin < 4; intx_pin++) {
> +            int irq = GUEST_VIRTIO_PCI_SPI_FIRST - 32;
> +            irq += ((intx_pin + PCI_SLOT(devfn)) % 4);
> +
> +            dt_set_cell(&cells, 1, devfn << 8);
> +            dt_set_cell(&cells, 1, 0);
> +            dt_set_cell(&cells, 1, 0);
> +            dt_set_cell(&cells, 1, intx_pin + 1);
> +            dt_set_cell(&cells, 1, kinfo->phandle_gic);

Here we will also want a parent unit address (GIC unit address). The
number of cells is determined by the vGIC node's #address-cells. See
section 2.4.3 in [1].

Also take a look at EPAM's libxl implementation, in the function
create_virtio_pci_irq_map() [2].

[1] 
https://github.com/devicetree-org/devicetree-specification/releases/download/v0.4/devicetree-specification-v0.4.pdf
[2] 
https://lore.kernel.org/xen-devel/20231115112611.3865905-4-Sergiy_Kibrik@xxxxxxxx/

> +            /* 3 GIC cells.  */
> +            dt_set_cell(&cells, 1, 0);
> +            dt_set_cell(&cells, 1, irq);
> +            dt_set_cell(&cells, 1, DT_IRQ_TYPE_LEVEL_HIGH);
> +        }
> +    }
> +
> +    /* Assert we've sized irq_map correctly.  */
> +    BUG_ON(cells - &irq_map[0] != ARRAY_SIZE(irq_map));
> +
> +    res = fdt_property(fdt, "interrupt-map", irq_map, sizeof(irq_map));
> +    if ( res )
> +        return res;
> +
> +    cells = &reg[0];
> +    dt_set_cell(&cells, 1, cpu_to_be16(PCI_DEVFN(3, 0)));
> +    dt_set_cell(&cells, 1, 0x0);
> +    dt_set_cell(&cells, 1, 0x0);
> +    dt_set_cell(&cells, 1, 0x7);
> +    res = fdt_property(fdt, "interrupt-map-mask", reg, 4 * sizeof(*reg));
> +    if ( res )
> +        return res;
> +
> +    if ( kinfo->virtio_pci.mode == VIRTIO_PCI_GRANTS )
> +    {
> +        cells = &reg[0];
> +        dt_set_cell(&cells, 1, 0x0);
> +        dt_set_cell(&cells, 1, GUEST_PHANDLE_IOMMU);
> +        dt_set_cell(&cells, 1, 0x0);
> +        dt_set_cell(&cells, 1, 0x10000);
> +        res = fdt_property(fdt, "iommu-map", reg, 4 * sizeof(*reg));
> +        if ( res )
> +            return res;
> +    }
> +
> +    res = fdt_property_cell(fdt, "linux,pci-domain", 1);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_end_node(fdt);
> +    if ( res )
> +        return res;
> +
> +    if ( kinfo->virtio_pci.mode == VIRTIO_PCI_GRANTS )
> +    {
> +        snprintf(buf, sizeof(buf), "xen_iommu");
> +
> +        res = fdt_begin_node(fdt, buf);
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_string(fdt, "compatible", "xen,grant-dma");
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_cell(fdt, "#iommu-cells", 1);
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property_cell(fdt, "phandle", GUEST_PHANDLE_IOMMU);
> +        if ( res )
> +            return res;
> +
> +        res = fdt_end_node(fdt);
> +    }
> +
> +    return res;
> +}