diff mbox series

[v2,2/5] xen/vioapic: add support for the extended destination ID field

Message ID 20220216103026.11533-3-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86: extended destination ID support | expand

Commit Message

Roger Pau Monné Feb. 16, 2022, 10:30 a.m. UTC
Such field uses bits 55:48, but for the purposes the register will be
used use bits 55:49 instead. Bit 48 is used to signal an RTE entry is
in remappable format which is not supported by the vIO-APIC.

Use the extended destination ID to store the high bits from the
destination ID, thus expanding the size of the destination ID field to
15 bits, allowing an IO-APIC to target APIC IDs up to 32768. A
description of the feature can be found at:

http://david.woodhou.se/15-bit-msi.pdf

Note this is already supported by QEMU/KVM and HyperV.

Introduce an option in xl.cfg to allow switching off the feature.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Expand commit message, add reference document.
 - Add xl option to disable the feature.
---
 docs/man/xl.cfg.5.pod.in               | 10 ++++++++++
 tools/include/libxl.h                  |  8 ++++++++
 tools/libs/light/libxl_create.c        |  6 ++++++
 tools/libs/light/libxl_types.idl       |  1 +
 tools/libs/light/libxl_x86.c           | 12 ++++++++++++
 tools/xl/xl_parse.c                    |  3 +++
 xen/arch/x86/domain.c                  | 10 +++++++++-
 xen/arch/x86/hvm/vioapic.c             |  3 +++
 xen/arch/x86/include/asm/domain.h      |  3 +++
 xen/arch/x86/setup.c                   |  1 +
 xen/include/public/arch-x86/hvm/save.h |  4 +++-
 xen/include/public/arch-x86/xen.h      |  2 ++
 xen/include/public/domctl.h            |  2 +-
 13 files changed, 62 insertions(+), 3 deletions(-)

Comments

Jan Beulich Feb. 16, 2022, 3:54 p.m. UTC | #1
On 16.02.2022 11:30, Roger Pau Monne wrote:
> Such field uses bits 55:48, but for the purposes the register will be
> used use bits 55:49 instead. Bit 48 is used to signal an RTE entry is
> in remappable format which is not supported by the vIO-APIC.

Nit: The first sentence looks to have some stray words.

> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -527,6 +527,14 @@
>   */
>  #define LIBXL_HAVE_MAX_GRANT_VERSION 1
>  
> +/*
> + * LIBXL_HAVE_X86_EXT_DEST_ID indicates the toolstack can signal to the
> + * hypervisor whether the domain wants to use the extended destination ID mode
> + * for interrupt messages. This is done by setting the libxl_domain_build_info
> + * arch_x86.ext_dest_id field.
> + */
> +#define LIBXL_HAVE_X86_EST_DEST_ID 1

Did you mean LIBXL_HAVE_X86_EXT_DEST_ID, as the comment has it?

> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -648,6 +648,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                 ("vuart", libxl_vuart_type),
>                                ])),
>      ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
> +                               ("ext_dest_id", libxl_defbool),

Let's hope there's not going to appear any other meaning of "dest ID".
I would have suggested to add "apic" to the name, but this would get
it a little longish for my taste.

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -782,6 +782,7 @@ static struct domain *__init create_dom0(const module_t *image,
>  
>          dom0_cfg.arch.emulation_flags |=
>              XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
> +        dom0_cfg.arch.misc_flags |= XEN_X86_EXT_DEST_ID;
>      }

Without any way to suppress this?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -38,7 +38,7 @@
>  #include "hvm/save.h"
>  #include "memory.h"
>  
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000014
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000015

I'm struggling to figure which binary incompatible change in here
requires this bumping. Does this perhaps belong in a later patch?

Jan
diff mbox series

Patch

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index b98d161398..349e0b432a 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2901,6 +2901,16 @@  option.
 
 If using this option is necessary to fix an issue, please report a bug.
 
+=item B<ext_dest_id=BOOLEAN>
+
+(HVM/PVH only) Signal the hypervisor whether the guest should be allowed to use
+extended destination ID for interrupt messages. Such feature allow expanding
+the target APIC ID from 8 to 15bits without requiring the usage of an emulated
+IOMMU with interrupt remapping. Note this requires guest support and might not
+be exposed to the guest even if explicitly set due to other constrains.
+
+Default: enabled
+
 =back
 
 =head1 SEE ALSO
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 51a9b6cfac..35329bca50 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -527,6 +527,14 @@ 
  */
 #define LIBXL_HAVE_MAX_GRANT_VERSION 1
 
+/*
+ * LIBXL_HAVE_X86_EXT_DEST_ID indicates the toolstack can signal to the
+ * hypervisor whether the domain wants to use the extended destination ID mode
+ * for interrupt messages. This is done by setting the libxl_domain_build_info
+ * arch_x86.ext_dest_id field.
+ */
+#define LIBXL_HAVE_X86_EST_DEST_ID 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 7499922088..66ecfbdf4d 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -2330,6 +2330,12 @@  int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
      */
     libxl_defbool_setdefault(&d_config->b_info.arch_x86.msr_relaxed, true);
 
+    /*
+     * Do not enable the extended destination ID for restored domains unless
+     * explicitly set.
+     */
+    libxl_defbool_setdefault(&d_config->b_info.arch_x86.ext_dest_id, false);
+
     return do_domain_create(ctx, d_config, domid, restore_fd, send_back_fd,
                             params, ao_how, aop_console_how);
 }
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 2a42da2f7d..bbfe218c48 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -648,6 +648,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
                                ("vuart", libxl_vuart_type),
                               ])),
     ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
+                               ("ext_dest_id", libxl_defbool),
                               ])),
     # Alternate p2m is not bound to any architecture or guest type, as it is
     # supported by x86 HVM and ARM support is planned.
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index 1feadebb18..68f4de7ea9 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -14,6 +14,11 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         config->arch.emulation_flags = 0;
+        if (libxl_defbool_val(d_config->b_info.arch_x86.ext_dest_id))
+        {
+            LOG(ERROR, "Extended Destination ID not supported for PV guests");
+            return ERROR_INVAL;
+        }
         break;
     default:
         abort();
@@ -22,6 +27,8 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
     config->arch.misc_flags = 0;
     if (libxl_defbool_val(d_config->b_info.arch_x86.msr_relaxed))
         config->arch.misc_flags |= XEN_X86_MSR_RELAXED;
+    if (libxl_defbool_val(d_config->b_info.arch_x86.ext_dest_id))
+        config->arch.misc_flags |= XEN_X86_EXT_DEST_ID;
 
     return 0;
 }
@@ -824,6 +831,8 @@  void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
 {
     libxl_defbool_setdefault(&b_info->acpi, true);
     libxl_defbool_setdefault(&b_info->arch_x86.msr_relaxed, false);
+    libxl_defbool_setdefault(&b_info->arch_x86.ext_dest_id,
+                             b_info->type != LIBXL_DOMAIN_TYPE_PV);
 }
 
 int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc,
@@ -880,6 +889,9 @@  void libxl__arch_update_domain_config(libxl__gc *gc,
      */
     libxl_defbool_setdefault(&dst->b_info.arch_x86.msr_relaxed,
                     libxl_defbool_val(src->b_info.arch_x86.msr_relaxed));
+    /* Force Extended Destination ID so it's part of the migration data. */
+    libxl_defbool_setdefault(&dst->b_info.arch_x86.ext_dest_id,
+                    libxl_defbool_val(src->b_info.arch_x86.ext_dest_id));
 }
 
 /*
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 117fcdcb2b..b609b76779 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2761,6 +2761,9 @@  skip_usbdev:
 
     xlu_cfg_get_defbool(config, "vpmu", &b_info->vpmu, 0);
 
+    xlu_cfg_get_defbool(config, "ext_dest_id", &b_info->arch_x86.ext_dest_id,
+                        0);
+
     xlu_cfg_destroy(config);
 }
 
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ef1812dc14..9764f42878 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -685,12 +685,19 @@  int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
         }
     }
 
-    if ( config->arch.misc_flags & ~XEN_X86_MSR_RELAXED )
+    if ( config->arch.misc_flags &
+         ~(XEN_X86_MSR_RELAXED | XEN_X86_EXT_DEST_ID) )
     {
         dprintk(XENLOG_INFO, "Invalid arch misc flags %#x\n",
                 config->arch.misc_flags);
         return -EINVAL;
     }
+    if ( !hvm && (config->arch.misc_flags & XEN_X86_EXT_DEST_ID) )
+    {
+        dprintk(XENLOG_INFO,
+                "Extended Destination ID only supported for HVM\n");
+        return -EINVAL;
+    }
 
     return 0;
 }
@@ -862,6 +869,7 @@  int arch_domain_create(struct domain *d,
     domain_cpu_policy_changed(d);
 
     d->arch.msr_relaxed = config->arch.misc_flags & XEN_X86_MSR_RELAXED;
+    d->arch.ext_dest_id = config->arch.misc_flags & XEN_X86_EXT_DEST_ID;
 
     return 0;
 
diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
index 553c0f76ef..a02bd16b4b 100644
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -424,6 +424,9 @@  static void vioapic_deliver(struct hvm_vioapic *vioapic, unsigned int pin)
 
     ASSERT(spin_is_locked(&d->arch.hvm.irq_lock));
 
+    if ( d->arch.ext_dest_id )
+        dest |= vioapic->redirtbl[pin].fields.virt_ext_dest_id << 8;
+
     HVM_DBG_LOG(DBG_LEVEL_IOAPIC,
                 "dest=%x dest_mode=%x delivery_mode=%x "
                 "vector=%x trig_mode=%x",
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index e62e109598..83a1608212 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -440,6 +440,9 @@  struct arch_domain
     /* Don't unconditionally inject #GP for unhandled MSRs. */
     bool msr_relaxed;
 
+    /* Use the Extended Destination ID to expand APIC ID. */
+    bool ext_dest_id;
+
     /* Emulated devices enabled bitmap. */
     uint32_t emulation_flags;
 } __cacheline_aligned;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 115f8f6517..4aaa6ebc07 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -782,6 +782,7 @@  static struct domain *__init create_dom0(const module_t *image,
 
         dom0_cfg.arch.emulation_flags |=
             XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
+        dom0_cfg.arch.misc_flags |= XEN_X86_EXT_DEST_ID;
     }
 
     if ( iommu_enabled )
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 773a380bc2..253dc89a04 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -376,7 +376,9 @@  union vioapic_redir_entry
         uint8_t trig_mode:1;
         uint8_t mask:1;
         uint8_t reserve:7;
-        uint8_t reserved[4];
+        uint8_t reserved[3];
+        uint8_t :1;
+        uint8_t virt_ext_dest_id:7;
         uint8_t dest_id;
     } fields;
 };
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 7acd94c8eb..06d64a309f 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -317,6 +317,8 @@  struct xen_arch_domainconfig {
  * doesn't allow the guest to read or write to the underlying MSR.
  */
 #define XEN_X86_MSR_RELAXED (1u << 0)
+/* Select whether to use Extended Destination ID for interrupt messages. */
+#define XEN_X86_EXT_DEST_ID (1u << 1)
     uint32_t misc_flags;
 };
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index b85e6170b0..31ec083cb0 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -38,7 +38,7 @@ 
 #include "hvm/save.h"
 #include "memory.h"
 
-#define XEN_DOMCTL_INTERFACE_VERSION 0x00000014
+#define XEN_DOMCTL_INTERFACE_VERSION 0x00000015
 
 /*
  * NB. xen_domctl.domain is an IN/OUT parameter for this operation.