| Message ID | 1467745519-9868-17-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote: > --- 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; > + > + if ( is_hardware_domain(d) ) > + config->emulation_flags |= XEN_X86_EMU_PIT; > + > + emflags = config->emulation_flags; > + if ( (emflags & ~XEN_X86_EMU_ALL) != 0 ) In cases like this please consider dropping the pointless != 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)) ) > + > + /* PVHv2 guests can request emulated APIC */ Comment style. With at least this one fixed Reviewed-by: Jan Beulich <jbeulich@suse.com> > + if ( emflags && > + (is_hvm_domain(d) ? ((emflags != XEN_X86_EMU_ALL) && > + (emflags != XEN_X86_EMU_LAPIC)) : > + (emflags != XEN_X86_EMU_PIT)) ) I guess sooner or later this will need re-arranging so that it's easier to maintain when more variations are permitted for HVM, but for now I think it's fine. Jan
On 03/08/16 17:11, Jan Beulich wrote: >>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote: >> --- 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; >> + >> + if ( is_hardware_domain(d) ) >> + config->emulation_flags |= XEN_X86_EMU_PIT; >> + >> + emflags = config->emulation_flags; >> + if ( (emflags & ~XEN_X86_EMU_ALL) != 0 ) > In cases like this please consider dropping the pointless != 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)) ) >> + >> + /* PVHv2 guests can request emulated APIC */ > Comment style. With at least this one fixed > Reviewed-by: Jan Beulich <jbeulich@suse.com> > >> + if ( emflags && >> + (is_hvm_domain(d) ? ((emflags != XEN_X86_EMU_ALL) && >> + (emflags != XEN_X86_EMU_LAPIC)) : >> + (emflags != XEN_X86_EMU_PIT)) ) > I guess sooner or later this will need re-arranging so that it's easier > to maintain when more variations are permitted for HVM, but for > now I think it's fine. Roger already did this at my request on a different thread. Message: <20160801112343.7ndtre6tfzxk3xjd@mac>, subject "[PATCH RFC 03/12] xen/x86: allow the emulated APICs to be enbled for the hardware domain" And on that note, Roger: you have a typo in your patch subject. ~Andrew
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 989bc74..c495a29 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; + + if ( is_hardware_domain(d) ) + config->emulation_flags |= XEN_X86_EMU_PIT; + + 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)) ) + + /* PVHv2 guests can request emulated APIC */ + if ( emflags && + (is_hvm_domain(d) ? ((emflags != XEN_X86_EMU_ALL) && + (emflags != XEN_X86_EMU_LAPIC)) : + (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); + d->domain_id, is_hvm_domain(d) ? "HVM" : "PV", emflags); return -EOPNOTSUPP; } - d->arch.emulation_flags = config->emulation_flags; + d->arch.emulation_flags = emflags; } if ( has_hvm_container_domain(d) )
PVHv2 guests may request LAPIC emulation (and nothing else) Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- Changes in v1: * Refer to PVHv2 (not hvmlite) xen/arch/x86/domain.c | 26 ++++++++++++++++---------- 1 files changed, 16 insertions(+), 10 deletions(-)