xen/arch/x86/domain.c | 78 +++++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 18 deletions(-)
From: Denis Mukhin <dmukhin@ford.com>
Rewrite emulation_flags_ok() to simplify future modifications.
No functional change intended.
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v6:
- simplified checks for PV further
Link to v6: https://lore.kernel.org/xen-devel/20250610004216.3012253-1-dmukhin@ford.com/
Link to CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1862559089
---
xen/arch/x86/domain.c | 78 +++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 7536b6c871..fdbd064ebf 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -743,32 +743,74 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
return 0;
}
+/*
+ * Verify that the domain's emulation flags resolve to a supported configuration.
+ *
+ * This ensures we only allow a known, safe subset of emulation combinations
+ * (for both functionality and security). Arbitrary mixes are likely to cause
+ * errors (e.g. null pointer dereferences).
+ *
+ * NB: use the internal X86_EMU_XXX symbols, not the public XEN_X86_EMU_XXX
+ * symbols.
+ */
static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
{
+ enum {
+ CAP_PV = BIT(0, U),
+ CAP_HVM = BIT(1, U),
+ CAP_HWDOM = BIT(2, U),
+ CAP_DOMU = BIT(3, U),
+ };
+ static const struct {
+ unsigned int caps;
+ uint32_t min;
+ uint32_t opt;
+ } configs[] = {
+#ifdef CONFIG_PV
+ /* PV dom0 and domU */
+ {
+ .caps = CAP_PV | CAP_HWDOM | CAP_DOMU,
+ .opt = X86_EMU_PIT,
+ },
+#endif /* #ifdef CONFIG_PV */
+
+#ifdef CONFIG_HVM
+ /* PVH dom0 */
+ {
+ .caps = CAP_HVM | CAP_HWDOM,
+ .min = X86_EMU_LAPIC | X86_EMU_IOAPIC | X86_EMU_VPCI,
+ },
+
+ /* PVH domU */
+ {
+ .caps = CAP_HVM | CAP_DOMU,
+ .min = X86_EMU_LAPIC,
+ },
+
+ /* HVM domU */
+ {
+ .caps = CAP_HVM | CAP_DOMU,
+ .min = X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ),
+ /* HVM PIRQ feature is user-selectable. */
+ .opt = X86_EMU_USE_PIRQ,
+ },
+#endif /* #ifdef CONFIG_HVM */
+ };
+ unsigned int i;
+ unsigned int caps = (is_pv_domain(d) ? CAP_PV : CAP_HVM) |
+ (is_hardware_domain(d) ? CAP_HWDOM : CAP_DOMU);
+
#ifdef CONFIG_HVM
/* This doesn't catch !CONFIG_HVM case but it is better than nothing */
BUILD_BUG_ON(X86_EMU_ALL != XEN_X86_EMU_ALL);
#endif
- if ( is_hvm_domain(d) )
- {
- if ( is_hardware_domain(d) &&
- emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
- return false;
- if ( !is_hardware_domain(d) &&
- /* HVM PIRQ feature is user-selectable. */
- (emflags & ~X86_EMU_USE_PIRQ) !=
- (X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ)) &&
- emflags != X86_EMU_LAPIC )
- return false;
- }
- else if ( emflags != 0 && emflags != X86_EMU_PIT )
- {
- /* PV or classic PVH. */
- return false;
- }
+ for ( i = 0; i < ARRAY_SIZE(configs); i++ )
+ if ( (caps & configs[i].caps) == caps &&
+ (emflags & ~configs[i].opt) == configs[i].min )
+ return true;
- return true;
+ return false;
}
void __init arch_init_idle_domain(struct domain *d)
--
2.34.1On Tue, Jun 10, 2025 at 02:45:12PM +0000, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
>
> Rewrite emulation_flags_ok() to simplify future modifications.
>
> No functional change intended.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v6:
> - simplified checks for PV further
>
> Link to v6: https://lore.kernel.org/xen-devel/20250610004216.3012253-1-dmukhin@ford.com/
> Link to CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1862559089
> ---
> xen/arch/x86/domain.c | 78 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 60 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 7536b6c871..fdbd064ebf 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -743,32 +743,74 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> return 0;
> }
>
> +/*
> + * Verify that the domain's emulation flags resolve to a supported configuration.
> + *
> + * This ensures we only allow a known, safe subset of emulation combinations
> + * (for both functionality and security). Arbitrary mixes are likely to cause
> + * errors (e.g. null pointer dereferences).
> + *
> + * NB: use the internal X86_EMU_XXX symbols, not the public XEN_X86_EMU_XXX
> + * symbols.
Not sure if it's worth expanding the sentence a bit to add the reason
why X86_EMU_XXX should be used (so that we take build-time config
options into account for short-circuited emulations).
> + */
> static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
> {
> + enum {
> + CAP_PV = BIT(0, U),
> + CAP_HVM = BIT(1, U),
> + CAP_HWDOM = BIT(2, U),
> + CAP_DOMU = BIT(3, U),
> + };
You probably want to name this type.
> + static const struct {
> + unsigned int caps;
So it can be used here
> + uint32_t min;
> + uint32_t opt;
> + } configs[] = {
> +#ifdef CONFIG_PV
> + /* PV dom0 and domU */
> + {
> + .caps = CAP_PV | CAP_HWDOM | CAP_DOMU,
> + .opt = X86_EMU_PIT,
> + },
> +#endif /* #ifdef CONFIG_PV */
> +
> +#ifdef CONFIG_HVM
> + /* PVH dom0 */
> + {
> + .caps = CAP_HVM | CAP_HWDOM,
> + .min = X86_EMU_LAPIC | X86_EMU_IOAPIC | X86_EMU_VPCI,
> + },
> +
> + /* PVH domU */
> + {
> + .caps = CAP_HVM | CAP_DOMU,
> + .min = X86_EMU_LAPIC,
> + },
> +
> + /* HVM domU */
> + {
> + .caps = CAP_HVM | CAP_DOMU,
> + .min = X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ),
> + /* HVM PIRQ feature is user-selectable. */
> + .opt = X86_EMU_USE_PIRQ,
> + },
> +#endif /* #ifdef CONFIG_HVM */
> + };
> + unsigned int i;
> + unsigned int caps = (is_pv_domain(d) ? CAP_PV : CAP_HVM) |
> + (is_hardware_domain(d) ? CAP_HWDOM : CAP_DOMU);
And here instead of using unsigned int?
> +
> #ifdef CONFIG_HVM
> /* This doesn't catch !CONFIG_HVM case but it is better than nothing */
> BUILD_BUG_ON(X86_EMU_ALL != XEN_X86_EMU_ALL);
> #endif
>
> - if ( is_hvm_domain(d) )
> - {
> - if ( is_hardware_domain(d) &&
> - emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
> - return false;
> - if ( !is_hardware_domain(d) &&
> - /* HVM PIRQ feature is user-selectable. */
> - (emflags & ~X86_EMU_USE_PIRQ) !=
> - (X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ)) &&
> - emflags != X86_EMU_LAPIC )
> - return false;
> - }
> - else if ( emflags != 0 && emflags != X86_EMU_PIT )
> - {
> - /* PV or classic PVH. */
> - return false;
> - }
> + for ( i = 0; i < ARRAY_SIZE(configs); i++ )
> + if ( (caps & configs[i].caps) == caps &&
> + (emflags & ~configs[i].opt) == configs[i].min )
> + return true;
>
> - return true;
> + return false;
> }
>
> void __init arch_init_idle_domain(struct domain *d)
> --
> 2.34.1
>
>
On Mon, Jun 16, 2025 at 04:03:14PM +0200, Roger Pau Monné wrote: > On Tue, Jun 10, 2025 at 02:45:12PM +0000, dmkhn@proton.me wrote: > > From: Denis Mukhin <dmukhin@ford.com> > > > > Rewrite emulation_flags_ok() to simplify future modifications. > > > > No functional change intended. > > > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> > > --- > > Changes since v6: > > - simplified checks for PV further > > > > Link to v6: https://lore.kernel.org/xen-devel/20250610004216.3012253-1-dmukhin@ford.com/ > > Link to CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1862559089 > > --- > > xen/arch/x86/domain.c | 78 +++++++++++++++++++++++++++++++++---------- > > 1 file changed, 60 insertions(+), 18 deletions(-) > > > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > > index 7536b6c871..fdbd064ebf 100644 > > --- a/xen/arch/x86/domain.c > > +++ b/xen/arch/x86/domain.c > > @@ -743,32 +743,74 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) > > return 0; > > } > > > > +/* > > + * Verify that the domain's emulation flags resolve to a supported configuration. > > + * > > + * This ensures we only allow a known, safe subset of emulation combinations > > + * (for both functionality and security). Arbitrary mixes are likely to cause > > + * errors (e.g. null pointer dereferences). > > + * > > + * NB: use the internal X86_EMU_XXX symbols, not the public XEN_X86_EMU_XXX > > + * symbols. > > Not sure if it's worth expanding the sentence a bit to add the reason > why X86_EMU_XXX should be used (so that we take build-time config > options into account for short-circuited emulations). With this comment adjusted as you have in v8: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
© 2016 - 2025 Red Hat, Inc.