diff mbox series

[v6,2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common

Message ID 20231113222118.825758-3-stewart.hildebrand@amd.com (mailing list archive)
State New, archived
Headers show
Series Kconfig for PCI passthrough on ARM | expand

Commit Message

Stewart Hildebrand Nov. 13, 2023, 10:21 p.m. UTC 
Both x86 and ARM need a way at domain creation time to specify whether
the domain needs vPCI emulation. Move the vPCI flag from x86
xen_domctl_createdomain.arch.emulation_flags to the common
xen_domctl_createdomain.flags.

Move has_vpci() macro to common header.

Bump XEN_DOMCTL_INTERFACE_VERSION since we're modifying flags inside
struct xen_domctl_createdomain and xen_arch_domainconfig.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
---
v5->v6:
* don't explicitly clear XEN_DOMCTL_CDF_vpci where unnecessary
* add comment about avoiding re-using bit 10 in emulation_flags
* don't #define _XEN_DOMCTL_CDF_vpci (underscore-prefixed), only
  introduce single new flag identifier
* move ( vpci && !hvm ) check to xen/arch/x86/domain.c
* simplify has_vpci()

v4->v5:
* move flags_optional change in xen/arch/arm/domain.c to next patch
* change latter 2 args to emulation_flags_ok() to unsigned int
* move vpci && !hvm check to common code
* remove stray spaces in emulation_flags_ok(), and a minor style fixup
* add CONFIG_HAS_VPCI check into has_vpci()
* add Christian's A-b (OCaml)

v3->v4:
* renamed, was:
  ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
* reworked: move x86 vPCI flag to common instead of adding another arch
  specific vPCI flag
* folded ("xen/arm: make has_vpci() depend on d->arch.has_vpci") into
  this patch (retain Signed-off-by's from [1] and [2])

v2->v3:
* new patch

[1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/27be1729ce8128dbe37275ce7946b2fbd2e5a382
[2] https://github.com/xen-troops/xen/commit/bf12185e6fb2e31db0d8e6ea9ccd8a02abadec17
---
 tools/libs/light/libxl_x86.c      |  2 +-
 tools/ocaml/libs/xc/xenctrl.ml    |  2 +-
 tools/ocaml/libs/xc/xenctrl.mli   |  2 +-
 tools/python/xen/lowlevel/xc/xc.c |  2 +-
 xen/arch/arm/include/asm/domain.h |  3 ---
 xen/arch/x86/domain.c             | 20 +++++++++++++++-----
 xen/arch/x86/include/asm/domain.h |  6 +-----
 xen/arch/x86/setup.c              |  4 ++--
 xen/common/domain.c               | 10 +++++++++-
 xen/include/public/arch-x86/xen.h |  9 +++++----
 xen/include/public/domctl.h       |  6 ++++--
 xen/include/xen/domain.h          |  2 ++
 12 files changed, 42 insertions(+), 26 deletions(-)

Comments

Jan Beulich Nov. 14, 2023, 9:11 a.m. UTC | #1 
On 13.11.2023 23:21, Stewart Hildebrand wrote:
> @@ -709,10 +710,17 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>          return -EINVAL;
>      }
>  
> +    if ( vpci && !hvm )
> +    {
> +        dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
> +        return -EINVAL;
> +    }
> +
>      return 0;
>  }

As said on the v5 thread, I think my comment was misguided (I'm sorry)
and this wants keeping in common code as you had it.

> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -283,15 +283,16 @@ struct xen_arch_domainconfig {
>  #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
>  #define _XEN_X86_EMU_USE_PIRQ       9
>  #define XEN_X86_EMU_USE_PIRQ        (1U<<_XEN_X86_EMU_USE_PIRQ)
> -#define _XEN_X86_EMU_VPCI           10
> -#define XEN_X86_EMU_VPCI            (1U<<_XEN_X86_EMU_VPCI)
> +/*
> + * Note: bit 10 was previously used for a XEN_X86_EMU_VPCI flag. This bit should
> + * not be re-used without careful consideration.
> + */

I think a multi-line comment is drawing overly much attention to this.
How about "Note: Bit 10 was previously used for XEN_X86_EMU_VPCI. Re-use
with care." which I think fits in a single line comment.

Jan
Stewart Hildebrand Nov. 29, 2023, 9:25 p.m. UTC | #2 
On 11/14/23 04:11, Jan Beulich wrote:
> On 13.11.2023 23:21, Stewart Hildebrand wrote:
>> @@ -709,10 +710,17 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>          return -EINVAL;
>>      }
>>  
>> +    if ( vpci && !hvm )
>> +    {
>> +        dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
>> +        return -EINVAL;
>> +    }
>> +
>>      return 0;
>>  }
> 
> As said on the v5 thread, I think my comment was misguided (I'm sorry)
> and this wants keeping in common code as you had it.

I'll move it back to xen/common/domain.c. No worries.

> 
>> --- a/xen/include/public/arch-x86/xen.h
>> +++ b/xen/include/public/arch-x86/xen.h
>> @@ -283,15 +283,16 @@ struct xen_arch_domainconfig {
>>  #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
>>  #define _XEN_X86_EMU_USE_PIRQ       9
>>  #define XEN_X86_EMU_USE_PIRQ        (1U<<_XEN_X86_EMU_USE_PIRQ)
>> -#define _XEN_X86_EMU_VPCI           10
>> -#define XEN_X86_EMU_VPCI            (1U<<_XEN_X86_EMU_VPCI)
>> +/*
>> + * Note: bit 10 was previously used for a XEN_X86_EMU_VPCI flag. This bit should
>> + * not be re-used without careful consideration.
>> + */
> 
> I think a multi-line comment is drawing overly much attention to this.
> How about "Note: Bit 10 was previously used for XEN_X86_EMU_VPCI. Re-use
> with care." which I think fits in a single line comment.

Sounds good.

> 
> Jan
Rahul Singh Dec. 15, 2023, 9:36 a.m. UTC | #3 
Hi Stewart,

> On 29 Nov 2023, at 9:25 pm, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
> 
> On 11/14/23 04:11, Jan Beulich wrote:
>> On 13.11.2023 23:21, Stewart Hildebrand wrote:
>>> @@ -709,10 +710,17 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>>         return -EINVAL;
>>>     }
>>> 
>>> +    if ( vpci && !hvm )
>>> +    {
>>> +        dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>>     return 0;
>>> }
>> 
>> As said on the v5 thread, I think my comment was misguided (I'm sorry)
>> and this wants keeping in common code as you had it.
> 
> I'll move it back to xen/common/domain.c. No worries.

I tested this patch and observed build failure when compiling the "x86_64” arch with
"CONFIG_HVM=n“ option.

x86_64-linux-gnu-ld    -melf_x86_64  -T arch/x86/xen.lds -N prelink.o --build-id=sha1 \
    ./common/symbols-dummy.o -o ./.xen-syms.0 
x86_64-linux-gnu-ld: prelink.o: in function `arch_iommu_hwdom_init’:
(.init.text+0x2192b): undefined reference to `vpci_is_mmcfg_address’
(.init.text+0x2192b): relocation truncated to fit: R_X86_64_PLT32 against undefined symbol `vpci_is_mmcfg_address'
x86_64-linux-gnu-ld: (.init.text+0x21947): undefined reference to `vpci_is_mmcfg_address'
(.init.text+0x21947): relocation truncated to fit: R_X86_64_PLT32 against undefined symbol `vpci_is_mmcfg_address'
x86_64-linux-gnu-ld: prelink.o: in function `do_physdev_op’: 
(.text.do_physdev_op+0x6db): undefined reference to `register_vpci_mmcfg_handler'
(.text.do_physdev_op+0x6db): relocation truncated to fit: R_X86_64_PLT32 against undefined symbol `register_vpci_mmcfg_handler'
x86_64-linux-gnu-ld: ./.xen-syms.0: hidden symbol `vpci_is_mmcfg_address' isn't defined
x86_64-linux-gnu-ld: final link failed: bad value                


Regards,
Rahul


> 
>> 
>>> --- a/xen/include/public/arch-x86/xen.h
>>> +++ b/xen/include/public/arch-x86/xen.h
>>> @@ -283,15 +283,16 @@ struct xen_arch_domainconfig {
>>> #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
>>> #define _XEN_X86_EMU_USE_PIRQ       9
>>> #define XEN_X86_EMU_USE_PIRQ        (1U<<_XEN_X86_EMU_USE_PIRQ)
>>> -#define _XEN_X86_EMU_VPCI           10
>>> -#define XEN_X86_EMU_VPCI            (1U<<_XEN_X86_EMU_VPCI)
>>> +/*
>>> + * Note: bit 10 was previously used for a XEN_X86_EMU_VPCI flag. This bit should
>>> + * not be re-used without careful consideration.
>>> + */
>> 
>> I think a multi-line comment is drawing overly much attention to this.
>> How about "Note: Bit 10 was previously used for XEN_X86_EMU_VPCI. Re-use
>> with care." which I think fits in a single line comment.
> 
> Sounds good.
> 
>> 
>> Jan
Stewart Hildebrand Dec. 19, 2023, 6:55 p.m. UTC | #4 
On 12/15/23 04:36, Rahul Singh wrote:
> Hi Stewart,
> 
>> On 29 Nov 2023, at 9:25 pm, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
>>
>> On 11/14/23 04:11, Jan Beulich wrote:
>>> On 13.11.2023 23:21, Stewart Hildebrand wrote:
>>>> @@ -709,10 +710,17 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>>>         return -EINVAL;
>>>>     }
>>>>
>>>> +    if ( vpci && !hvm )
>>>> +    {
>>>> +        dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>>     return 0;
>>>> }
>>>
>>> As said on the v5 thread, I think my comment was misguided (I'm sorry)
>>> and this wants keeping in common code as you had it.
>>
>> I'll move it back to xen/common/domain.c. No worries.
> 
> I tested this patch and observed build failure when compiling the "x86_64” arch with
> "CONFIG_HVM=n“ option.
> 
> x86_64-linux-gnu-ld    -melf_x86_64  -T arch/x86/xen.lds -N prelink.o --build-id=sha1 \
>     ./common/symbols-dummy.o -o ./.xen-syms.0 
> x86_64-linux-gnu-ld: prelink.o: in function `arch_iommu_hwdom_init’:
> (.init.text+0x2192b): undefined reference to `vpci_is_mmcfg_address’
> (.init.text+0x2192b): relocation truncated to fit: R_X86_64_PLT32 against undefined symbol `vpci_is_mmcfg_address'
> x86_64-linux-gnu-ld: (.init.text+0x21947): undefined reference to `vpci_is_mmcfg_address'
> (.init.text+0x21947): relocation truncated to fit: R_X86_64_PLT32 against undefined symbol `vpci_is_mmcfg_address'
> x86_64-linux-gnu-ld: prelink.o: in function `do_physdev_op’: 
> (.text.do_physdev_op+0x6db): undefined reference to `register_vpci_mmcfg_handler'
> (.text.do_physdev_op+0x6db): relocation truncated to fit: R_X86_64_PLT32 against undefined symbol `register_vpci_mmcfg_handler'
> x86_64-linux-gnu-ld: ./.xen-syms.0: hidden symbol `vpci_is_mmcfg_address' isn't defined
> x86_64-linux-gnu-ld: final link failed: bad value                

Ah, good catch. Before moving it, the flag was defined in xen/arch/x86/include/asm/domain.h:

#ifdef CONFIG_HVM
#define X86_EMU_VPCI     XEN_X86_EMU_VPCI
#else
#define X86_EMU_VPCI     0
#endif

#define has_vpci(d)        (!!((d)->arch.emulation_flags & X86_EMU_VPCI))


With CONFIG_HVM not set, in xen/drivers/passthrough/x86/iommu.c, the compiler optimized out the call to vpci_is_mmcfg_address():

    if ( has_vpci(d) && vpci_is_mmcfg_address(d, pfn_to_paddr(pfn)) )

After moving the flag, there are a couple of options to make the issue go away. I don't think #defining XEN_DOMCTL_CDF_vpci 0 in the !HVM case would be a good idea since that's a flag shared with the toolstack. We could change the definition of has_vpci():

#define has_vpci(d) (((d)->options & XEN_DOMCTL_CDF_vpci) && \
                     IS_ENABLED(CONFIG_HVM))

Or we could provide empty static inline definitions of vpci_is_mmcfg_address() and register_vpci_mmcfg_handler():

#ifdef CONFIG_HVM
bool vpci_is_mmcfg_address(const struct domain *d, paddr_t addr);
#else
static inline bool vpci_is_mmcfg_address(const struct domain *d, paddr_t addr)
{
    return false;
}
#endif

I don't have a strong preference for which way, but changing has_vpci() has a smaller diffstat, so I'll go with that for now. This is assuming that we still want to go with this approach. Given recent related discussions [1], it's possible we may not need a vPCI flag in struct xen_domctl_createdomain, but instead a flag internal to the hypervisor somewhere in struct domain.

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-12/msg00212.html
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index d16573e72cd4..4a42b7231f8b 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -8,7 +8,7 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
 {
     switch(d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        config->arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
+        config->arch.emulation_flags = XEN_X86_EMU_ALL;
         break;
     case LIBXL_DOMAIN_TYPE_PVH:
         config->arch.emulation_flags = XEN_X86_EMU_LAPIC;
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index d6c6eb73db44..6f3da9c6e064 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -46,7 +46,6 @@  type x86_arch_emulation_flags =
   | X86_EMU_IOMMU
   | X86_EMU_PIT
   | X86_EMU_USE_PIRQ
-  | X86_EMU_VPCI
 
 type x86_arch_misc_flags =
   | X86_MSR_RELAXED
@@ -70,6 +69,7 @@  type domain_create_flag =
   | CDF_IOMMU
   | CDF_NESTED_VIRT
   | CDF_VPMU
+  | CDF_VPCI
 
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 3bfc16edba96..e2dd02bec962 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -40,7 +40,6 @@  type x86_arch_emulation_flags =
   | X86_EMU_IOMMU
   | X86_EMU_PIT
   | X86_EMU_USE_PIRQ
-  | X86_EMU_VPCI
 
 type x86_arch_misc_flags =
   | X86_MSR_RELAXED
@@ -63,6 +62,7 @@  type domain_create_flag =
   | CDF_IOMMU
   | CDF_NESTED_VIRT
   | CDF_VPMU
+  | CDF_VPCI
 
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index d3ea350e07b9..b9f559a8e637 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -159,7 +159,7 @@  static PyObject *pyxc_domain_create(XcObject *self,
 
 #if defined (__i386) || defined(__x86_64__)
     if ( config.flags & XEN_DOMCTL_CDF_hvm )
-        config.arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
+        config.arch.emulation_flags = XEN_X86_EMU_ALL;
 #elif defined (__arm__) || defined(__aarch64__)
     config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
 #else
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 5fb8cd79c01a..3614562eaefe 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -298,9 +298,6 @@  static inline void arch_vcpu_block(struct vcpu *v) {}
 
 #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
 
-/* vPCI is not available on Arm */
-#define has_vpci(d)    ({ (void)(d); false; })
-
 struct arch_vcpu_io {
     struct instr_details dabt_instr; /* when the instruction is decoded */
 };
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 3712e36df930..bbb73abe88a1 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -636,6 +636,7 @@  int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
     bool hvm = config->flags & XEN_DOMCTL_CDF_hvm;
     bool hap = config->flags & XEN_DOMCTL_CDF_hap;
     bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
+    bool vpci = config->flags & XEN_DOMCTL_CDF_vpci;
     unsigned int max_vcpus;
 
     if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
@@ -709,10 +710,17 @@  int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( vpci && !hvm )
+    {
+        dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
+        return -EINVAL;
+    }
+
     return 0;
 }
 
-static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
+static bool emulation_flags_ok(const struct domain *d, unsigned int emflags,
+                               unsigned int cdf)
 {
 #ifdef CONFIG_HVM
     /* This doesn't catch !CONFIG_HVM case but it is better than nothing */
@@ -722,11 +730,13 @@  static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
     if ( is_hvm_domain(d) )
     {
         if ( is_hardware_domain(d) &&
-             emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
+             (!(cdf & XEN_DOMCTL_CDF_vpci) ||
+              emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
             return false;
         if ( !is_hardware_domain(d) &&
-             emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
-             emflags != X86_EMU_LAPIC )
+             ((cdf & XEN_DOMCTL_CDF_vpci) ||
+              (emflags != X86_EMU_ALL &&
+               emflags != X86_EMU_LAPIC)) )
             return false;
     }
     else if ( emflags != 0 && emflags != X86_EMU_PIT )
@@ -798,7 +808,7 @@  int arch_domain_create(struct domain *d,
         return -EINVAL;
     }
 
-    if ( !emulation_flags_ok(d, emflags) )
+    if ( !emulation_flags_ok(d, emflags, config->flags) )
     {
         printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation "
                "with the current selection of emulators: %#x\n",
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 619e667938ed..cb02a4d1ebb2 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -471,7 +471,6 @@  struct arch_domain
 #define X86_EMU_VGA      XEN_X86_EMU_VGA
 #define X86_EMU_IOMMU    XEN_X86_EMU_IOMMU
 #define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
-#define X86_EMU_VPCI     XEN_X86_EMU_VPCI
 #else
 #define X86_EMU_LAPIC    0
 #define X86_EMU_HPET     0
@@ -482,7 +481,6 @@  struct arch_domain
 #define X86_EMU_VGA      0
 #define X86_EMU_IOMMU    0
 #define X86_EMU_USE_PIRQ 0
-#define X86_EMU_VPCI     0
 #endif
 
 #define X86_EMU_PIT     XEN_X86_EMU_PIT
@@ -492,8 +490,7 @@  struct arch_domain
                                  X86_EMU_PM | X86_EMU_RTC |             \
                                  X86_EMU_IOAPIC | X86_EMU_PIC |         \
                                  X86_EMU_VGA | X86_EMU_IOMMU |          \
-                                 X86_EMU_PIT | X86_EMU_USE_PIRQ |       \
-                                 X86_EMU_VPCI)
+                                 X86_EMU_PIT | X86_EMU_USE_PIRQ)
 
 #define has_vlapic(d)      (!!((d)->arch.emulation_flags & X86_EMU_LAPIC))
 #define has_vhpet(d)       (!!((d)->arch.emulation_flags & X86_EMU_HPET))
@@ -505,7 +502,6 @@  struct arch_domain
 #define has_viommu(d)      (!!((d)->arch.emulation_flags & X86_EMU_IOMMU))
 #define has_vpit(d)        (!!((d)->arch.emulation_flags & X86_EMU_PIT))
 #define has_pirq(d)        (!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
-#define has_vpci(d)        (!!((d)->arch.emulation_flags & X86_EMU_VPCI))
 
 #define gdt_ldt_pt_idx(v) \
       ((v)->vcpu_id >> (PAGETABLE_ORDER - GDT_LDT_VCPU_SHIFT))
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a3d3f797bb1e..00dfcf231e21 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -890,12 +890,12 @@  static struct domain *__init create_dom0(const module_t *image,
 
     if ( opt_dom0_pvh )
     {
-        dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
+        dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_vpci |
                            ((hvm_hap_supported() && !opt_dom0_shadow) ?
                             XEN_DOMCTL_CDF_hap : 0));
 
         dom0_cfg.arch.emulation_flags |=
-            XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
+            XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC;
     }
 
     if ( iommu_enabled )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8f9ab01c0cb7..12dc27428972 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -509,12 +509,14 @@  static int sanitise_domain_config(struct xen_domctl_createdomain *config)
     bool hap = config->flags & XEN_DOMCTL_CDF_hap;
     bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
     bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
+    bool vpci = config->flags & XEN_DOMCTL_CDF_vpci;
 
     if ( config->flags &
          ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
            XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
            XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
-           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
+           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
+           XEN_DOMCTL_CDF_vpci) )
     {
         dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
         return -EINVAL;
@@ -575,6 +577,12 @@  static int sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( vpci && !IS_ENABLED(CONFIG_HAS_VPCI) )
+    {
+        dprintk(XENLOG_INFO, "vPCI requested but not enabled\n");
+        return -EINVAL;
+    }
+
     return arch_sanitise_domain_config(config);
 }
 
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index c0f4551247f4..abb808c9060b 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -283,15 +283,16 @@  struct xen_arch_domainconfig {
 #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
 #define _XEN_X86_EMU_USE_PIRQ       9
 #define XEN_X86_EMU_USE_PIRQ        (1U<<_XEN_X86_EMU_USE_PIRQ)
-#define _XEN_X86_EMU_VPCI           10
-#define XEN_X86_EMU_VPCI            (1U<<_XEN_X86_EMU_VPCI)
+/*
+ * Note: bit 10 was previously used for a XEN_X86_EMU_VPCI flag. This bit should
+ * not be re-used without careful consideration.
+ */
 
 #define XEN_X86_EMU_ALL             (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET |  \
                                      XEN_X86_EMU_PM | XEN_X86_EMU_RTC |      \
                                      XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC |  \
                                      XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU |   \
-                                     XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\
-                                     XEN_X86_EMU_VPCI)
+                                     XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ)
     uint32_t emulation_flags;
 
 /*
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a33f9ec32b08..8b3ea62cae06 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -21,7 +21,7 @@ 
 #include "hvm/save.h"
 #include "memory.h"
 
-#define XEN_DOMCTL_INTERFACE_VERSION 0x00000016
+#define XEN_DOMCTL_INTERFACE_VERSION 0x00000017
 
 /*
  * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
@@ -55,9 +55,11 @@  struct xen_domctl_createdomain {
 #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
 /* Should we expose the vPMU to the guest? */
 #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
+/* Should vPCI be enabled for the guest? */
+#define XEN_DOMCTL_CDF_vpci           (1U << 8)
 
 /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
-#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
+#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpci
 
     uint32_t flags;
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 54d88bf5e34b..17b3429240f3 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -51,6 +51,8 @@  void arch_get_domain_info(const struct domain *d,
 
 #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
 
+#define has_vpci(d) (!!((d)->options & XEN_DOMCTL_CDF_vpci))
+
 /*
  * Arch-specifics.
  */