| Message ID | 1469809747-11176-4-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/07/16 17:28, Roger Pau Monne wrote: > if ( is_hardware_domain(d) ) > - config->emulation_flags |= XEN_X86_EMU_PIT; > - if ( config->emulation_flags != 0 && > - (config->emulation_flags != > - (is_hvm_domain(d) ? XEN_X86_EMU_ALL : XEN_X86_EMU_PIT)) ) > + emflags |= XEN_X86_EMU_PIT; > + > + if ( (is_hardware_domain(d) ? > + (is_hvm_domain(d) && emflags != > + (XEN_X86_EMU_PIT|XEN_X86_EMU_LAPIC|XEN_X86_EMU_IOAPIC)) : > + (emflags && (is_hvm_domain(d) ? (emflags != XEN_X86_EMU_ALL) : > + (emflags != XEN_X86_EMU_PIT)))) ) This is getting very complicated. It is rather important (security wise) and not a hotpath. Could I please request that you move this logic to a helper such as: static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) and make this more readable. Any decent compiler will inline and simplify it appropriately. ~Andrew
On Fri, Jul 29, 2016 at 06:50:48PM +0100, Andrew Cooper wrote: > On 29/07/16 17:28, Roger Pau Monne wrote: > > if ( is_hardware_domain(d) ) > > - config->emulation_flags |= XEN_X86_EMU_PIT; > > - if ( config->emulation_flags != 0 && > > - (config->emulation_flags != > > - (is_hvm_domain(d) ? XEN_X86_EMU_ALL : XEN_X86_EMU_PIT)) ) > > + emflags |= XEN_X86_EMU_PIT; > > + > > + if ( (is_hardware_domain(d) ? > > + (is_hvm_domain(d) && emflags != > > + (XEN_X86_EMU_PIT|XEN_X86_EMU_LAPIC|XEN_X86_EMU_IOAPIC)) : > > + (emflags && (is_hvm_domain(d) ? (emflags != XEN_X86_EMU_ALL) : > > + (emflags != XEN_X86_EMU_PIT)))) ) > > This is getting very complicated. > > It is rather important (security wise) and not a hotpath. Could I > please request that you move this logic to a helper such as: > > static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) > > and make this more readable. Any decent compiler will inline and > simplify it appropriately. Thanks, I've now moved it to a helper as suggested: static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) { if ( is_hvm_domain(d) ) { if ( is_hardware_domain(d) && emflags != (XEN_X86_EMU_PIT|XEN_X86_EMU_LAPIC|XEN_X86_EMU_IOAPIC)) return false; if ( !is_hardware_domain(d) && emflags != XEN_X86_EMU_ALL && emflags != 0 ) return false; } else { /* PV or classic PVH. */ if ( is_hardware_domain(d) && emflags != XEN_X86_EMU_PIT ) return false; if ( !is_hardware_domain(d) && emflags != 0 ) return false; } return true; } Roger.
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 1133ea2..4d47228 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -545,25 +545,31 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, } else { - if ( (config->emulation_flags & ~XEN_X86_EMU_ALL) != 0 ) + uint32_t emflags = config->emulation_flags; + + if ( (emflags & ~XEN_X86_EMU_ALL) != 0 ) { printk(XENLOG_G_ERR "d%d: Invalid emulation bitmap: %#x\n", - d->domain_id, config->emulation_flags); + d->domain_id, emflags); return -EINVAL; } + if ( is_hardware_domain(d) ) - config->emulation_flags |= XEN_X86_EMU_PIT; - if ( config->emulation_flags != 0 && - (config->emulation_flags != - (is_hvm_domain(d) ? XEN_X86_EMU_ALL : XEN_X86_EMU_PIT)) ) + emflags |= XEN_X86_EMU_PIT; + + if ( (is_hardware_domain(d) ? + (is_hvm_domain(d) && emflags != + (XEN_X86_EMU_PIT|XEN_X86_EMU_LAPIC|XEN_X86_EMU_IOAPIC)) : + (emflags && (is_hvm_domain(d) ? (emflags != XEN_X86_EMU_ALL) : + (emflags != XEN_X86_EMU_PIT)))) ) { printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation " "with the current selection of emulators: %#x\n", d->domain_id, is_hvm_domain(d) ? "HVM" : "PV", - config->emulation_flags); + emflags); return -EOPNOTSUPP; } - d->arch.emulation_flags = config->emulation_flags; + d->arch.emulation_flags = emflags; } if ( has_hvm_container_domain(d) )
Allow the used of both the emulated local APIC and IO APIC for the hardware domain. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/domain.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)