From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= Subject: Re: [PATCH v6 11/29] xen/x86: add bitmap of enabled emulated devices Date: Wed, 23 Sep 2015 14:35:05 +0200 Message-ID: <56029C79.8040505@citrix.com> References: <1441368548-43465-1-git-send-email-roger.pau@citrix.com> <1441368548-43465-12-git-send-email-roger.pau@citrix.com> <55F9577502000078000A3372@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZejGT-0001Fs-00 for xen-devel@lists.xenproject.org; Wed, 23 Sep 2015 12:35:13 +0000 In-Reply-To: <55F9577502000078000A3372@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Wei Liu , Ian Campbell , Stefano Stabellini , Andrew Cooper , Ian Jackson , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org El 16/09/15 a les 11.50, Jan Beulich ha escrit: >>>> On 04.09.15 at 14:08, wrote: >> --- a/tools/libxl/libxl_x86.c >> +++ b/tools/libxl/libxl_x86.c >> @@ -7,8 +7,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, >> libxl_domain_config *d_config, >> xc_domain_configuration_t *xc_config) >> { >> - /* No specific configuration right now */ >> - >> + if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM) >> + xc_config->emulation_flags = (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET | >> + XEN_X86_EMU_PMTIMER | XEN_X86_EMU_RTC | >> + XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC | >> + XEN_X86_EMU_PMU | XEN_X86_EMU_VGA | >> + XEN_X86_EMU_IOMMU); > > This calls for the elsewhere discussed XEN_X86_EMU_ALL to even be > exposed to the tool stack. Done. >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -555,6 +555,29 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, >> d->domain_id); >> } >> >> + if ( is_hvm_domain(d) ) >> + { >> + uint32_t emulation_mask = (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET | > > const With the introduction of XEN_X86_EMU_ALL this is no longer needed. > >> + XEN_X86_EMU_PMTIMER | XEN_X86_EMU_RTC | >> + XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC | >> + XEN_X86_EMU_PMU | XEN_X86_EMU_VGA | >> + XEN_X86_EMU_IOMMU); >> + if ( (config->emulation_flags & ~emulation_mask) != 0 ) > > Missing blank line between declaration and statements. emulation_mask is now gone, so no need for the newline any more. >> + { >> + printk(XENLOG_G_ERR "d%d: Invalid emulation bitmap: %#x.\n", > > Generally we have no full stops at the end of log messages. Ack, I've removed them here and below, but I've noticed that other printks in arch_domain_create have full stops (that's probably why I've added them here). > >> + d->domain_id, config->emulation_flags); >> + return -EINVAL; >> + } >> + if ( config->emulation_flags != emulation_mask ) >> + { >> + printk(XENLOG_G_ERR "d%d: Xen does not allow HVM creation with the " >> + "current selection of emulators: %#x.\n", d->domain_id, >> + config->emulation_flags); >> + return -EOPNOTSUPP; >> + } >> + d->arch.emulation_flags = config->emulation_flags; >> + } > > Isn't there an "else" missing here, validating that the flags are zero? The comment in xen/include/asm-x86/domain.h above the emulation_flags field already mentions that this field is ignored for PV guests. For example the x86 Dom0 building code calls arch_domain_create passing in a NULL xen_arch_domainconfig, so I think it's easier to just ignore this for PV guests. Roger.