diff mbox series

[v5,10/11] arm/libxl: Emulated PCI device tree node in libxl

Message ID b81b5dea800c8fe47071f3dbd20588b1e472fb99.1633540842.git.rahul.singh@arm.com (mailing list archive)
State New, archived
Headers show
Series PCI devices passthrough on Arm | expand

Commit Message

Rahul Singh Oct. 6, 2021, 5:40 p.m. UTC
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>
---
Change in v5:
- Move setting the arch_arm.vpci and XEN_DOMCTL_CDF_vpci to libxl_arm.c
Change in v4:
- Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
Change in v3:
- Make GUEST_VPCI_MEM_ADDR address 2MB aligned
Change in v2:
- enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
---
---
 tools/include/libxl.h            |   6 ++
 tools/libs/light/libxl_arm.c     | 111 +++++++++++++++++++++++++++++++
 tools/libs/light/libxl_types.idl |   1 +
 xen/include/public/arch-arm.h    |  10 +++
 4 files changed, 128 insertions(+)

Comments

Julien Grall Oct. 6, 2021, 6:01 p.m. UTC | #1
Hi Rahul,

On 06/10/2021 19:40, Rahul Singh wrote:
> 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),

I have posted some comments regarding the field in v4. To summarize, 
AFAICT, this option is meant to be only set by libxl but you still let 
the toolstack (e.g. xl, libvirt) to set it.

If you still want to expose to the toolstack, then I think the option 
should be outside of arch_arm. Otherwise, this should be moved in an 
internal structure (Ian, do you have any suggestion?).

Cheers,
Stefano Stabellini Oct. 7, 2021, 12:26 a.m. UTC | #2
On Wed, 6 Oct 2021, Julien Grall wrote:
> Hi Rahul,
> 
> On 06/10/2021 19:40, Rahul Singh wrote:
> > 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),
> 
> I have posted some comments regarding the field in v4. To summarize, AFAICT,
> this option is meant to be only set by libxl but you still let the toolstack
> (e.g. xl, libvirt) to set it.
> 
> If you still want to expose to the toolstack, then I think the option should
> be outside of arch_arm. Otherwise, this should be moved in an internal
> structure (Ian, do you have any suggestion?).


First let me premise that the patch is much better already and Rahul
addessed my request well. However, Julien's point about not wanting to
make a potentially breaking ABI change in libxl is a good one. FYI we
had a few libvirt breakages due to this kind of changes in the past and
I would certainly be happier if we didn't cause another one. And in
general, it is better to avoid changes to the libxl ABI if we can.

I think in this case we can: I looked at the patch and
b_info.arch_arm.vpci is only used within tools/libs/light/libxl_arm.c.
Also, we don't need b_info.arch_arm.vpci if we can access
d_config->num_pcidevs given that the check is just based on
d_config->num_pcidevs.

So the only issue is how to check on d_config->num_pcidevs in
libxl__prepare_dtb. libxl__prepare_dtb takes libxl_domain_build_info as
parameter but with container_of we can retrieve libxl_domain_config and
from there check on num_pcidevs.

Something like the appended (untested). It doesn't need any libxl struct
changes but it requires the introduction of container_of (which is a
simple macro). Alternatively, it would be just as simple to change
libxl__arch_domain_init_hw_description and libxl__prepare_dtb to take a
libxl_domain_config *d_config parameter instead of a
libxl_domain_build_info *info parameter.

Ian, any thoughts?


diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 2be208b99b..ee1176519c 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -102,10 +102,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
     }
 
     /* Enable VPCI support. */
-    if (d_config->num_pcidevs) {
+    if (d_config->num_pcidevs)
         config->flags |= XEN_DOMCTL_CDF_vpci;
-        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
-    }
 
     return 0;
 }
@@ -976,6 +974,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
 
     const libxl_version_info *vers;
     const struct arch_info *ainfo;
+    libxl_domain_config *d_config = container_of(info, libxl_domain_config, b_info);
 
     vers = libxl_get_version_info(CTX);
     if (vers == NULL) return ERROR_FAIL;
@@ -1076,7 +1075,7 @@ next_resize:
         if (info->tee == LIBXL_TEE_TYPE_OPTEE)
             FDT( make_optee_node(gc, fdt) );
 
-        if (libxl_defbool_val(info->arch_arm.vpci))
+        if (d_config->num_pcidevs)
             FDT( make_vpci_node(gc, fdt, ainfo, dom) );
 
         if (pfdt)
Ian Jackson Oct. 7, 2021, 10:53 a.m. UTC | #3
Julien Grall writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl"):
> On 06/10/2021 19:40, Rahul Singh wrote:
> > 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),
> 
> I have posted some comments regarding the field in v4. To summarize, 
> AFAICT, this option is meant to be only set by libxl but you still let 
> the toolstack (e.g. xl, libvirt) to set it.
> 
> If you still want to expose to the toolstack, then I think the option 
> should be outside of arch_arm. Otherwise, this should be moved in an 
> internal structure (Ian, do you have any suggestion?).

If it should be in an internal structure, probably the libxl create
context.

But I'm not convinced yet.  In particular, if enabling VPCI is
necessary on ARM for hotplugged PCI devices[1], then there has to be
a way for the admin to say "while this domain may not have any PCI
devices right now, I may wish to hotplug some".  That's what the
"passthrough=" option is for.

See my other mail.

[1] I think this is all true even if PCI hotplug for ARM is not
currently implemented.

Ian.
Rahul Singh Oct. 7, 2021, 3:29 p.m. UTC | #4
Hi Ian,

> On 7 Oct 2021, at 11:53 am, Ian Jackson <iwj@xenproject.org> wrote:
> 
> Julien Grall writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl"):
>> On 06/10/2021 19:40, Rahul Singh wrote:
>>> 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),
>> 
>> I have posted some comments regarding the field in v4. To summarize, 
>> AFAICT, this option is meant to be only set by libxl but you still let 
>> the toolstack (e.g. xl, libvirt) to set it.
>> 
>> If you still want to expose to the toolstack, then I think the option 
>> should be outside of arch_arm. Otherwise, this should be moved in an 
>> internal structure (Ian, do you have any suggestion?).
> 
> If it should be in an internal structure, probably the libxl create
> context.

As Stefano suggested in another email that we can remove the vpci option, if we 
reach to conclusion that we need vpci option I will move it to internal structure.
 
>  
> But I'm not convinced yet.  In particular, if enabling VPCI is
> necessary on ARM for hotplugged PCI devices[1], then there has to be
> a way for the admin to say "while this domain may not have any PCI
> devices right now, I may wish to hotplug some".  That's what the
> "passthrough=" option is for.

Yes I agree with you VPCI is necessary for hot plugged PCI device and once we 
implement the hotplug in future we will use the passthrough= option to enable VPCI.

Regards,
Rahul  

> 
> See my other mail.
> 
> [1] I think this is all true even if PCI hotplug for ARM is not
> currently implemented.
> 
> Ian.
Rahul Singh Oct. 7, 2021, 3:31 p.m. UTC | #5
Hi Stefano,

> On 7 Oct 2021, at 1:26 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Wed, 6 Oct 2021, Julien Grall wrote:
>> Hi Rahul,
>> 
>> On 06/10/2021 19:40, Rahul Singh wrote:
>>> 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),
>> 
>> I have posted some comments regarding the field in v4. To summarize, AFAICT,
>> this option is meant to be only set by libxl but you still let the toolstack
>> (e.g. xl, libvirt) to set it.
>> 
>> If you still want to expose to the toolstack, then I think the option should
>> be outside of arch_arm. Otherwise, this should be moved in an internal
>> structure (Ian, do you have any suggestion?).
> 
> 
> First let me premise that the patch is much better already and Rahul
> addessed my request well. However, Julien's point about not wanting to
> make a potentially breaking ABI change in libxl is a good one. FYI we
> had a few libvirt breakages due to this kind of changes in the past and
> I would certainly be happier if we didn't cause another one. And in
> general, it is better to avoid changes to the libxl ABI if we can.
> 
> I think in this case we can: I looked at the patch and
> b_info.arch_arm.vpci is only used within tools/libs/light/libxl_arm.c.
> Also, we don't need b_info.arch_arm.vpci if we can access
> d_config->num_pcidevs given that the check is just based on
> d_config->num_pcidevs.
> 
> So the only issue is how to check on d_config->num_pcidevs in
> libxl__prepare_dtb. libxl__prepare_dtb takes libxl_domain_build_info as
> parameter but with container_of we can retrieve libxl_domain_config and
> from there check on num_pcidevs.
> 
> Something like the appended (untested). It doesn't need any libxl struct
> changes but it requires the introduction of container_of (which is a
> simple macro). Alternatively, it would be just as simple to change
> libxl__arch_domain_init_hw_description and libxl__prepare_dtb to take a
> libxl_domain_config *d_config parameter instead of a
> libxl_domain_build_info *info parameter.

Thanks for sharing the ideas to remove the arch_arm.vpci field. 
I am ok with any options, but I feel second option is simple and better to avoid  
introduction of container_of(). I tested the second option and is working fine.
If everyone agree that we don’t need vpci option I will send the patch for review
In next version.

Regards,
Rahul
> 
> Ian, any thoughts?
> 
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 2be208b99b..ee1176519c 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -102,10 +102,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>     }
> 
>     /* Enable VPCI support. */
> -    if (d_config->num_pcidevs) {
> +    if (d_config->num_pcidevs)
>         config->flags |= XEN_DOMCTL_CDF_vpci;
> -        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
> -    }
> 
>     return 0;
> }
> @@ -976,6 +974,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
> 
>     const libxl_version_info *vers;
>     const struct arch_info *ainfo;
> +    libxl_domain_config *d_config = container_of(info, libxl_domain_config, b_info);
> 
>     vers = libxl_get_version_info(CTX);
>     if (vers == NULL) return ERROR_FAIL;
> @@ -1076,7 +1075,7 @@ next_resize:
>         if (info->tee == LIBXL_TEE_TYPE_OPTEE)
>             FDT( make_optee_node(gc, fdt) );
> 
> -        if (libxl_defbool_val(info->arch_arm.vpci))
> +        if (d_config->num_pcidevs)
>             FDT( make_vpci_node(gc, fdt, ainfo, dom) );
> 
>         if (pfdt)
Ian Jackson Oct. 7, 2021, 4:11 p.m. UTC | #6
Rahul Singh writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl"):
> As Stefano suggested in another email that we can remove the vpci
> option, if we reach to conclusion that we need vpci option I will
> move it to internal structure.
...
> Yes I agree with you VPCI is necessary for hot plugged PCI device
> and once we implement the hotplug in future we will use the
> passthrough= option to enable VPCI.

So, to summarise, I think the situation is:

 * VCPI is necessry for passthrough on ARM, whether coldplug or
   hotplug.  It's part of the way that PCI-PT works on ARM.

 * Hotplug is not yet implemented.

 * VPCI is not necessary on x86 (evidently, since we don't have it
   there but we do have passthrough).

So when hotplug is added, vpci will need to be turned on when
passthrough=yes is selected.  I don't fully understand the other
possible values for passthrough= but maybe we can defer the question
of whether they apply to ARM ?

I think that means that yes, this should be an internal variable.
Probably in libxl__domain_create_state.  We don't currently arrange to
elide arch-specific state in there, so perhaps it's fine just to
invent a member called `arm_vpci`.

Maybe you could leave a comment somewhere so that if and when PCI PT
hotplug is implemented for ARM, the implementor remembers to wire this
up.

Ian.
Roger Pau Monné Oct. 11, 2021, 1:46 p.m. UTC | #7
On Thu, Oct 07, 2021 at 05:11:47PM +0100, Ian Jackson wrote:
> Rahul Singh writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl"):
> > As Stefano suggested in another email that we can remove the vpci
> > option, if we reach to conclusion that we need vpci option I will
> > move it to internal structure.
> ...
> > Yes I agree with you VPCI is necessary for hot plugged PCI device
> > and once we implement the hotplug in future we will use the
> > passthrough= option to enable VPCI.
> 
> So, to summarise, I think the situation is:
> 
>  * VCPI is necessry for passthrough on ARM, whether coldplug or
>    hotplug.  It's part of the way that PCI-PT works on ARM.
> 
>  * Hotplug is not yet implemented.
> 
>  * VPCI is not necessary on x86 (evidently, since we don't have it
>    there but we do have passthrough).
> 
> So when hotplug is added, vpci will need to be turned on when
> passthrough=yes is selected.  I don't fully understand the other
> possible values for passthrough= but maybe we can defer the question
> of whether they apply to ARM ?
> 
> I think that means that yes, this should be an internal variable.
> Probably in libxl__domain_create_state.  We don't currently arrange to
> elide arch-specific state in there, so perhaps it's fine just to
> invent a member called `arm_vpci`.

Seeing as we might want to also use this on x86, I wonder whether we
should allow to specify a backend to use for each passthrough device,
ie:

pci = [ '36:00.0,backend=vpci',
        '36:01.0,backend=qemu',
	... ]

In principle we should support different backends on a per-device
basis, albeit if not currently possible.

Iff we have to introduce a new struct member for vPCI it should be
shared between all arches, as it's likely x86 will also want to at
least have the option to use vPCI for passthrough.

Thanks, Roger.
Ian Jackson Oct. 12, 2021, 3:03 p.m. UTC | #8
Rahul Singh writes ("[PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl"):
> 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).

I think this series is targeted for 4.16.  Stefano drew this patch to
my attention.  I have read the thread on this patch and it is not
clear to me that it has converged.  Code freeze is imminent.

Is there some way I can help here ?

Ian.
Bertrand Marquis Oct. 14, 2021, 5:16 p.m. UTC | #9
Hi Ian,

> On 7 Oct 2021, at 17:11, Ian Jackson <iwj@xenproject.org> wrote:
> 
> Rahul Singh writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl"):
>> As Stefano suggested in another email that we can remove the vpci
>> option, if we reach to conclusion that we need vpci option I will
>> move it to internal structure.
> ...
>> Yes I agree with you VPCI is necessary for hot plugged PCI device
>> and once we implement the hotplug in future we will use the
>> passthrough= option to enable VPCI.
> 
> So, to summarise, I think the situation is:
> 
> * VCPI is necessry for passthrough on ARM, whether coldplug or
>   hotplug.  It's part of the way that PCI-PT works on ARM.
> 
> * Hotplug is not yet implemented.
> 
> * VPCI is not necessary on x86 (evidently, since we don't have it
>   there but we do have passthrough).
> 
> So when hotplug is added, vpci will need to be turned on when
> passthrough=yes is selected.  I don't fully understand the other
> possible values for passthrough= but maybe we can defer the question
> of whether they apply to ARM ?
> 
> I think that means that yes, this should be an internal variable.
> Probably in libxl__domain_create_state.  We don't currently arrange to
> elide arch-specific state in there, so perhaps it's fine just to
> invent a member called `arm_vpci`.
> 
> Maybe you could leave a comment somewhere so that if and when PCI PT
> hotplug is implemented for ARM, the implementor remembers to wire this
> up.

Sorry for missing this on the v6 serie.

Now you suggest to add a new field arm_vpci in libxl__domain_create_state.

Once we have done that I will need to access this structure to know if I need
to add the DT part and somehow to give it a value depending something which
for now would the number of pcidevs as there will be no user parameter anymore.

I had quite an understanding of the solution using libxl_domain_config and changing
The arguments of libxl__arch_domain_init_hw_description and libxl__prepare_dtb
Suggested by Stefano but I am a bit lost in this solution.

The following might be a stupid question but I did not dig a lot in libxl so:
If we add a parameter in the state structure how should we access it ?

Thank
Bertrand



> 
> Ian.
diff mbox series

Patch

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index b9ba16d698..3362073b21 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -358,6 +358,12 @@ 
  */
 #define LIBXL_HAVE_BUILDINFO_ARM_VUART 1
 
+/*
+ * LIBXL_HAVE_BUILDINFO_ARM_VPCI indicates that the toolstack supports virtual
+ * PCI for ARM.
+ */
+#define LIBXL_HAVE_BUILDINFO_ARM_VPCI 1
+
 /*
  * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
  * has the max_grant_frames and max_maptrack_frames fields.
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6e00..2be208b99b 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -101,6 +101,12 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
         return ERROR_FAIL;
     }
 
+    /* Enable VPCI support. */
+    if (d_config->num_pcidevs) {
+        config->flags |= XEN_DOMCTL_CDF_vpci;
+        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
+    }
+
     return 0;
 }
 
@@ -269,6 +275,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 = &regs[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, uint32_t);
+        set_cell(&cells, 1, arg);
+
+        /* 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);
+    }
+    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 +726,53 @@  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, 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,
+        GUEST_VPCI_PREFETCH_MEM_SIZE);
+    if (res) return res;
+
+    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 +1076,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 +1297,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/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 44be337dec..45aac5d18f 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -433,6 +433,11 @@  typedef uint64_t xen_callback_t;
 #define GUEST_PL011_BASE    xen_mk_ullong(0x22000000)
 #define GUEST_PL011_SIZE    xen_mk_ullong(0x00001000)
 
+/* Guest PCI-PCIe memory space where config space and BAR will be available.*/
+#define GUEST_VPCI_ADDR_TYPE_MEM            xen_mk_ullong(0x02000000)
+#define GUEST_VPCI_MEM_ADDR                 xen_mk_ullong(0x23000000)
+#define GUEST_VPCI_MEM_SIZE                 xen_mk_ullong(0x10000000)
+
 /*
  * 16MB == 4096 pages reserved for guest to use as a region to map its
  * grant table in.
@@ -448,6 +453,11 @@  typedef uint64_t xen_callback_t;
 #define GUEST_RAM0_BASE   xen_mk_ullong(0x40000000) /* 3GB of low RAM @ 1GB */
 #define GUEST_RAM0_SIZE   xen_mk_ullong(0xc0000000)
 
+/* 4GB @ 4GB Prefetch Memory for VPCI */
+#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM   xen_mk_ullong(0x42000000)
+#define GUEST_VPCI_PREFETCH_MEM_ADDR        xen_mk_ullong(0x100000000)
+#define GUEST_VPCI_PREFETCH_MEM_SIZE        xen_mk_ullong(0x100000000)
+
 #define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 1016GB of RAM @ 8GB */
 #define GUEST_RAM1_SIZE   xen_mk_ullong(0xfe00000000)