From: Stefano Stabellini <sstabellini@kernel.org>
To: Rahul Singh <rahul.singh@arm.com>
Cc: xen-devel@lists.xenproject.org, bertrand.marquis@arm.com,
Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
Anthony PERARD <anthony.perard@citrix.com>,
Juergen Gross <jgross@suse.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <george.dunlap@citrix.com>,
Jan Beulich <jbeulich@suse.com>, Julien Grall <julien@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v1 12/14] arm/libxl: Emulated PCI device tree node in libxl
Date: Thu, 9 Sep 2021 17:51:58 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.21.2109091742210.10523@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <31e48fe4b219ebaf5f1d9f3f7ca9b433caadc5b2.1629366665.git.rahul.singh@arm.com>
On Thu, 19 Aug 2021, Rahul Singh wrote:
> 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).
>
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
> tools/libs/light/libxl_arm.c | 109 ++++++++++++++++++++++++++
> tools/libs/light/libxl_types.idl | 1 +
> tools/xl/xl_parse.c | 2 +
> xen/include/public/arch-arm.h | 11 +++
> xen/include/public/device_tree_defs.h | 1 +
> 5 files changed, 124 insertions(+)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index e3140a6e00..a091e97e76 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
> return fdt_property(fdt, "reg", regs, sizeof(regs));
> }
>
> +static int fdt_property_values(libxl__gc *gc, void *fdt,
> + const char *name, unsigned num_cells, ...)
> +{
> + uint32_t prop[num_cells];
> + be32 *cells = &prop[0];
> + int i;
> + va_list ap;
> + uint32_t arg;
> +
> + va_start(ap, num_cells);
> + for (i = 0 ; i < num_cells; i++) {
> + arg = va_arg(ap, uint32_t);
> + set_cell(&cells, 1, arg);
> + }
> + va_end(ap);
> +
> + return fdt_property(fdt, name, prop, sizeof(prop));
> +}
> +
> +static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
> + unsigned addr_cells,
> + unsigned size_cells,
> + unsigned num_regs, ...)
> +{
> + uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
> + be32 *cells = ®s[0];
> + int i;
> + va_list ap;
> + uint64_t arg;
> +
> + va_start(ap, num_regs);
> + for (i = 0 ; i < num_regs; i++) {
> + /* Set the memory bit field */
> + arg = va_arg(ap, uint64_t);
> + set_cell(&cells, 1, arg);
Shouldn't this be uint32_t given that it is 1 cell exactly?
> + /* Set the vpci bus address */
> + arg = addr_cells ? va_arg(ap, uint64_t) : 0;
> + set_cell(&cells, addr_cells , arg);
> +
> + /* Set the cpu bus address where vpci address is mapped */
> + set_cell(&cells, addr_cells, arg);
> +
> + /* Set the vpci size requested */
> + arg = size_cells ? va_arg(ap, uint64_t) : 0;
> + set_cell(&cells, size_cells,arg);
^ space
> + }
> + va_end(ap);
> +
> + return fdt_property(fdt, "ranges", regs, sizeof(regs));
> +}
> +
> static int make_root_properties(libxl__gc *gc,
> const libxl_version_info *vers,
> void *fdt)
> @@ -668,6 +720,57 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
> return 0;
> }
>
> +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,17);
^ space
> + 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,
> + GUEST_VPCI_PREFETCH_MEM_SIZE);
> + if (res) return res;
> +
> + res = fdt_property_values(gc, fdt, "msi-map", 4, 0, GUEST_PHANDLE_ITS,
> + 0, 0x10000);
> + if (res) return res;
I agree with Julien that we shouldn't add it now if it is not working.
One question: what about legacy interrupts? If they are supported,
shouldn't we have interrupts and interrupt-parent properties?
> + res = fdt_end_node(fdt);
> + if (res) return res;
> +
> + return 0;
> +}
> +
> static const struct arch_info *get_arch_info(libxl__gc *gc,
> const struct xc_dom_image *dom)
> {
> @@ -971,6 +1074,9 @@ next_resize:
> if (info->tee == LIBXL_TEE_TYPE_OPTEE)
> FDT( make_optee_node(gc, fdt) );
>
> + if (libxl_defbool_val(info->arch_arm.vpci))
> + FDT( make_vpci_node(gc, fdt, ainfo, dom) );
> +
> if (pfdt)
> FDT( copy_partial_fdt(gc, fdt, pfdt) );
>
> @@ -1189,6 +1295,9 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> /* ACPI is disabled by default */
> libxl_defbool_setdefault(&b_info->acpi, false);
>
> + /* VPCI is disabled by default */
> + libxl_defbool_setdefault(&b_info->arch_arm.vpci, false);
> +
> if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
> return;
>
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index 3f9fff653a..78b1ddf0b8 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -644,6 +644,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>
> ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> ("vuart", libxl_vuart_type),
> + ("vpci", libxl_defbool),
> ])),
> ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> ])),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 17dddb4cd5..ffafbeffb4 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1497,6 +1497,8 @@ void parse_config_data(const char *config_source,
> }
> if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
> libxl_defbool_set(&b_info->u.pv.e820_host, true);
> + if (d_config->num_pcidevs)
> + libxl_defbool_set(&b_info->arch_arm.vpci, true);
> }
>
> if (!xlu_cfg_get_list (config, "dtdev", &dtdevs, 0, 0)) {
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 0a9749e768..01d13e669e 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -426,6 +426,17 @@ typedef uint64_t xen_callback_t;
> #define GUEST_VPCI_ECAM_BASE xen_mk_ullong(0x10000000)
> #define GUEST_VPCI_ECAM_SIZE xen_mk_ullong(0x10000000)
>
> +/* PCI-PCIe memory space types */
> +#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM xen_mk_ullong(0x42000000)
> +#define GUEST_VPCI_ADDR_TYPE_MEM xen_mk_ullong(0x02000000)
> +
> +/* Guest PCI-PCIe memory space where config space and BAR will be available.*/
> +#define GUEST_VPCI_PREFETCH_MEM_ADDR xen_mk_ullong(0x900000000)
> +#define GUEST_VPCI_MEM_ADDR xen_mk_ullong(0x20000000)
> +
> +#define GUEST_VPCI_PREFETCH_MEM_SIZE xen_mk_ullong(0x2000000000)
> +#define GUEST_VPCI_MEM_SIZE xen_mk_ullong(0x04000000)
Now I get it: GUEST_VPCI_ECAM_BASE was just for the config space, not
the aperture. The apertures are starting at 0x20000000 and 0x900000000.
Speaking of which, isn't GUEST_VPCI_MEM_SIZE a bit too small? There
could be PCI devices with a <4GB requirement. Maybe 256MB?
next prev parent reply other threads:[~2021-09-10 0:52 UTC|newest]
Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-19 12:02 [PATCH v1 00/14] PCI devices passthrough on Arm Rahul Singh
2021-08-19 12:02 ` [PATCH v1 01/14] xen/pci: Refactor MSI code that implements MSI functionality within XEN Rahul Singh
2021-08-19 12:18 ` Julien Grall
2021-08-19 14:16 ` Rahul Singh
2021-09-07 10:01 ` Julien Grall
2021-08-24 15:53 ` Jan Beulich
2021-08-31 12:31 ` Rahul Singh
2021-08-31 13:00 ` Jan Beulich
2021-08-26 13:23 ` Daniel P. Smith
2021-08-19 12:02 ` [PATCH v1 02/14] xen/pci: solve compilation error on ARM with HAS_PCI enabled Rahul Singh
2021-08-19 12:28 ` Julien Grall
2021-08-20 10:30 ` Rahul Singh
2021-08-20 11:37 ` Julien Grall
2021-08-20 11:55 ` Jan Beulich
2021-08-20 12:10 ` Julien Grall
2021-08-20 7:01 ` Jan Beulich
2021-08-20 11:21 ` Rahul Singh
2021-09-09 13:16 ` Julien Grall
2021-08-19 12:02 ` [PATCH v1 03/14] xen/pci: solve compilation error on ARM with ACPI && HAS_PCI Rahul Singh
2021-08-20 7:06 ` Jan Beulich
2021-08-20 11:41 ` Rahul Singh
2021-08-20 11:54 ` Jan Beulich
2021-09-09 1:11 ` Stefano Stabellini
2021-09-10 10:22 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 04/14] xen/arm: Add support for PCI init to initialize the PCI driver Rahul Singh
2021-09-07 8:20 ` Julien Grall
2021-09-10 10:47 ` Rahul Singh
2021-09-09 1:16 ` Stefano Stabellini
2021-09-10 10:32 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 05/14] xen/arm: PCI host bridge discovery within XEN on ARM Rahul Singh
2021-09-07 9:05 ` Julien Grall
2021-09-10 11:22 ` Rahul Singh
2021-09-10 11:53 ` Julien Grall
2021-09-09 22:54 ` Stefano Stabellini
2021-09-10 11:53 ` Rahul Singh
2021-09-13 14:52 ` Oleksandr Andrushchenko
2021-09-13 20:23 ` Stefano Stabellini
2021-09-14 4:35 ` Oleksandr Andrushchenko
2021-09-14 7:53 ` Oleksandr Andrushchenko
2021-08-19 12:02 ` [PATCH v1 06/14] xen/arm: Add support for PCI ecam operations Rahul Singh
2021-09-09 11:32 ` Julien Grall
2021-09-14 8:13 ` Rahul Singh
2021-09-09 23:21 ` Stefano Stabellini
2021-09-14 11:13 ` Rahul Singh
2021-09-14 23:06 ` Stefano Stabellini
2021-09-15 16:38 ` Rahul Singh
2021-09-15 20:45 ` Stefano Stabellini
2021-09-16 16:51 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 07/14] xen/arm: Add support for Xilinx ZynqMP PCI host controller Rahul Singh
2021-09-09 23:34 ` Stefano Stabellini
2021-09-10 12:01 ` Rahul Singh
2021-09-13 14:46 ` Oleksandr Andrushchenko
2021-09-13 21:02 ` Stefano Stabellini
2021-09-14 4:31 ` Oleksandr Andrushchenko
2021-09-17 7:39 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 08/14] xen:arm: Implement pci access functions Rahul Singh
2021-09-09 23:41 ` Stefano Stabellini
2021-09-14 16:05 ` Rahul Singh
2021-09-14 22:40 ` Stefano Stabellini
2021-09-15 7:54 ` Oleksandr Andrushchenko
2021-09-15 10:47 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 09/14] xen/arm: Add cmdline boot option "pci=on" Rahul Singh
2021-08-19 12:09 ` Jan Beulich
2021-08-19 12:31 ` Julien Grall
2021-08-20 12:19 ` Rahul Singh
2021-08-20 14:34 ` Julien Grall
2021-08-20 14:37 ` Jan Beulich
2021-09-09 23:46 ` Stefano Stabellini
2021-09-09 23:48 ` Stefano Stabellini
2021-08-19 12:02 ` [PATCH v1 10/14] xen/arm: Discovering PCI devices and add the PCI devices in XEN Rahul Singh
2021-08-19 12:35 ` Julien Grall
2021-08-19 13:40 ` Jan Beulich
2021-08-20 13:05 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 11/14] xen/arm: Enable the existing x86 virtual PCI support for ARM Rahul Singh
2021-08-24 16:09 ` Jan Beulich
2021-08-25 5:44 ` Oleksandr Andrushchenko
2021-08-25 6:35 ` Jan Beulich
2021-09-09 13:50 ` Julien Grall
2021-09-16 10:46 ` Rahul Singh
2021-09-10 0:26 ` Stefano Stabellini
2021-09-16 11:01 ` Rahul Singh
2021-09-16 20:26 ` Stefano Stabellini
2021-09-21 13:49 ` Rahul Singh
2021-09-21 21:38 ` Stefano Stabellini
2021-08-19 12:02 ` [PATCH v1 12/14] arm/libxl: Emulated PCI device tree node in libxl Rahul Singh
2021-08-19 13:00 ` Julien Grall
2021-08-20 16:03 ` Rahul Singh
2021-09-09 13:59 ` Julien Grall
2021-09-16 16:16 ` Rahul Singh
2021-09-10 0:51 ` Stefano Stabellini [this message]
2021-09-16 16:35 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 13/14] xen/arm: Fixed error when PCI device is assigned to guest Rahul Singh
2021-08-19 12:12 ` Jan Beulich
2021-08-19 12:40 ` Julien Grall
2021-08-20 17:01 ` Rahul Singh
2021-08-19 12:02 ` [PATCH v1 14/14] xen/arm: Add linux,pci-domain property for hwdom if not available Rahul Singh
2021-09-10 1:00 ` Stefano Stabellini
2021-09-16 16:36 ` Rahul Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.2109091742210.10523@sstabellini-ThinkPad-T480s \
--to=sstabellini@kernel.org \
--cc=Volodymyr_Babchuk@epam.com \
--cc=andrew.cooper3@citrix.com \
--cc=anthony.perard@citrix.com \
--cc=bertrand.marquis@arm.com \
--cc=george.dunlap@citrix.com \
--cc=iwj@xenproject.org \
--cc=jbeulich@suse.com \
--cc=jgross@suse.com \
--cc=julien@xen.org \
--cc=rahul.singh@arm.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).