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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 24/09/2024 17:23, Edgar E. Iglesias wrote:
> From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>
> 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.hildebrand@xxxxxxx>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxx>
> ---
>   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
Similar question to the other patches, why is this specific to virtio 
PCI? QEMU (or another device module) is free to emulate whatever it 
wants behind the PCI hosbtridge.
> +
> +    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;
> +    }
Please add a comment explaining where does this requires come from. If 
this is the Address layout, then probably be to avoid relying on 
hardcoded number of bits.
Cheers,

--
Julien Grall