Re: [PATCH v7 5/5] arm/libxl: Emulated PCI device tree node in libxl

[Julien Grall <julien@xxxxxxx>]

Hi Bertrand,

On 15/10/2021 14:59, Bertrand Marquis wrote:
> From: Rahul Singh <rahul.singh@xxxxxxx>
>
> libxl will create an emulated PCI device tree node in the device tree to
> enable the guest OS to discover the virtual PCI during guest boot.
> Emulated PCI device tree node will only be created when there is any
> device assigned to guest.
>
> A new area has been reserved in the arm guest physical map at
> which the VPCI bus is declared in the device tree (reg and ranges
> parameters of the node).
>
> Note that currently we are using num_pcidevs instead of
> c_info->passthrough to decide whether to create a vPCI DT node.
> This will be insufficient if and when ARM does PCI hotplug.
> Add this note inside libxl_create.c where c_info->passthrough
> is set.
>
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
I believe you need to add your signed-off-by here as you sent the new 
version on xen-devel.
With one remark below (not to be handled for 4.16):

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

> +static int make_vpci_node(libxl__gc *gc, void *fdt,
> +                          const struct arch_info *ainfo,
> +                          struct xc_dom_image *dom)
> +{
> +    int res;
> +    const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
> +    const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
> +    const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
> +
> +    res = fdt_begin_node(fdt, name);
> +    if (res) return res;
> +
> +    res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
> +    if (res) return res;
> +
> +    res = fdt_property_string(fdt, "device_type", "pci");
> +    if (res) return res;
> +
> +    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> +            GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
> +    if (res) return res;
> +
> +    res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
> +    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;
> +
> +    res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> +        GUEST_ROOT_SIZE_CELLS, 2,
> +        GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
> +        GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
From my understanding, the prefetch memory region is optional. Is it 
going to be a problem to expose one for the vCPI when the host PCI 
hostbridge may not support it?
Cheers,

--
Julien Grall