On Tue, Sep 24, 2024 at 04:55:15PM -0700, Stefano Stabellini wrote:
> On Tue, 24 Sep 2024, Edgar E. Iglesias wrote:
> > From: Stewart Hildebrand <stewart.hildebr...@amd.com>
> >
> > When virtio-pci is specified in the dom0less domU properties, create a
> > virtio-pci node in the guest's device tree. Set up an mmio handler with
> > a register for the guest to poll when the backend has connected and
> > virtio-pci bus is ready to be probed. Grant tables may be used by
> > specifying virtio-pci = "grants";.
> >
> > [Edgar: Use GPEX PCI INTX interrupt swizzling (from PCI specs).
> > Make grants iommu-map cover the entire PCI bus.
> > Add virtio-pci-ranges to specify memory-map for direct-mapped guests.
> > Document virtio-pci dom0less fdt bindings.]
> > Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@amd.com>
> > ---
> > docs/misc/arm/device-tree/booting.txt | 21 +++
> > xen/arch/arm/dom0less-build.c | 238 ++++++++++++++++++++++++++
> > xen/arch/arm/include/asm/kernel.h | 15 ++
> > 3 files changed, 274 insertions(+)
> >
> > diff --git a/docs/misc/arm/device-tree/booting.txt
> > b/docs/misc/arm/device-tree/booting.txt
> > index 3a04f5c57f..82f3bd7026 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -276,6 +276,27 @@ with the following properties:
> > passed through. This option is the default if this property is missing
> > and the user does not provide the device partial device tree for the
> > domain.
> >
> > +- virtio-pci
> > +
> > + A string property specifying whether virtio-pci is enabled for the
> > + domain and if grant table mappings should be used. If no value is set
> > + this property is treated as a boolean and the same way as if set to
> > + "enabled".
> > + Possible property values are:
> > +
> > + - "enabled"
> > + Virtio-pci is enabled for the domain.
> > +
> > + - "grants"
> > + Virtio-pci is enabled for the domain and an grants IOMMU node will be
> > + generated in the domains device-tree.
> > +
> > +- virtio-pci-ranges
> > +
> > + An optional array of 6 u32 values specifying the 2 cells base
> > addresses of
> > + the ECAM, Memory and Prefetchable-Memory regions for virtio-pci. This
> > is
> > + useful to avoid memory-map collisions when using direct-mapped guests.
> > +
> > Under the "xen,domain" compatible node, one or more sub-nodes are present
> > for the DomU kernel and ramdisk.
> >
> > 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 = ®[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 = ®[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 = ®[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);
> > + /* 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 = ®[0];
> > + dt_set_cell(&cells, 1, cpu_to_be16(PCI_DEVFN(3, 0)));
>
> Hi Edgar, what is this PCI_DEVFN(3, 0) device?
Hi Stefano,
This is for the interrupt-map-mask, since the swizzling pattern
repeats itself for every fourth device, we only need to describe
entries 0 - 3 and can mask out the rest.
I the next version (which will be quite different) I'll try to make
this clearer.
>
>
> > + 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 = ®[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");
>
> I don't think underscores are allowed in device tree node names
Will fix, thanks.
>
>
>
> > + 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;
> > +}
> > +
> > /*
> > * The max size for DT is 2MB. However, the generated DT is small (not
> > including
> > * domU passthrough DT nodes whose size we account separately), 4KB are
> > enough
> > @@ -693,6 +876,13 @@ static int __init prepare_dtb_domU(struct domain *d,
> > struct kernel_info *kinfo)
> > goto err;
> > }
> >
> > + if ( kinfo->virtio_pci.mode )
> > + {
> > + ret = make_virtio_pci_domU_node(kinfo);
> > + if ( ret )
> > + goto err;
> > + }
> > +
> > ret = fdt_end_node(kinfo->fdt);
> > if ( ret < 0 )
> > goto err;
> > @@ -744,11 +934,24 @@ static int __init alloc_xenstore_evtchn(struct domain
> > *d)
> > return 0;
> > }
> >
> > +static u64 combine_u64(u32 v[2])
>
> Let's make this a static inline or a macro. I can't believe we don't
> have one already.
Yes, I'll try what Stewart suggested.
Cheers,
Edgar
