From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?Um9nZXIgUGF1IE1vbm7DqQ==?= Subject: Re: [PATCH v3 11/32] xen/x86: add bitmap of enabled emulated devices Date: Thu, 23 Jul 2015 12:29:37 +0200 Message-ID: <55B0C211.6030703@citrix.com> References: <1435923310-9019-1-git-send-email-roger.pau@citrix.com> <1435923310-9019-12-git-send-email-roger.pau@citrix.com> <559A8C3A.7090406@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZIDl3-0007YM-NO for xen-devel@lists.xenproject.org; Thu, 23 Jul 2015 10:29:45 +0000 In-Reply-To: <559A8C3A.7090406@citrix.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: Andrew Cooper , xen-devel@lists.xenproject.org Cc: Wei Liu , Ian Jackson , Ian Campbell , Jan Beulich , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org El 06/07/15 a les 16.10, Andrew Cooper ha escrit: > On 03/07/15 12:34, Roger Pau Monne wrote: >> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c >> index a112953..d684f49 100644 >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -528,6 +528,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, >> { >> int i, paging_initialised = 0; >> int rc = -ENOMEM; >> + uint32_t emulation_mask; >> >> d->arch.s3_integrity = !!(domcr_flags & DOMCRF_s3_integrity); >> >> @@ -550,6 +551,20 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags, >> d->domain_id); >> } >> >> + if ( is_hvm_domain(d) ) >> + { >> + emulation_mask = (EMU_LAPIC | EMU_HPET | EMU_PMTIMER | EMU_RTC | >> + EMU_IOAPIC | EMU_PIC | EMU_PMU | EMU_VGA | EMU_IOMMU); >> + if ( (config->emulation_flags & emulation_mask) != emulation_mask ) >> + { >> + printk(XENLOG_G_ERR "Xen does not allow HVM creation with the " >> + "current selection of emulators: 0x%x.\n", >> + config->emulation_flags); > > Please put "d%d" in the error message to identify which domain has the > bad configuration. > >> + return -EPERM; > > EPERM feels wrong here. It is not a lack of permissions causing the > issue. EINVAL/EOPNOTSUPP perhaps? EOPNOTSUPP looks better indeed. >> + } >> + d->arch.emulation_flags = config->emulation_flags; >> + } >> + >> if ( has_hvm_container_domain(d) ) >> { >> d->arch.hvm_domain.hap_enabled = >> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h >> index 96bde65..502379e 100644 >> --- a/xen/include/asm-x86/domain.h >> +++ b/xen/include/asm-x86/domain.h >> @@ -356,8 +356,21 @@ struct arch_domain >> >> /* Mem_access emulation control */ >> bool_t mem_access_emulate_enabled; >> + >> + /* Emulated devices enabled bitmap. */ >> + uint32_t emulation_flags; >> } __cacheline_aligned; >> >> +#define has_vlapic(d) (d->arch.emulation_flags & EMU_LAPIC) >> +#define has_vhpet(d) (d->arch.emulation_flags & EMU_HPET) >> +#define has_vpmtimer(d) (d->arch.emulation_flags & EMU_PMTIMER) >> +#define has_vrtc(d) (d->arch.emulation_flags & EMU_RTC) >> +#define has_vioapic(d) (d->arch.emulation_flags & EMU_IOAPIC) >> +#define has_vpic(d) (d->arch.emulation_flags & EMU_PIC) >> +#define has_vpmu(d) (d->arch.emulation_flags & EMU_PMU) >> +#define has_vvga(d) (d->arch.emulation_flags & EMU_VGA) >> +#define has_viommu(d) (d->arch.emulation_flags & EMU_IOMMU) > > (d) should be bracketed in the expansion. > >> + >> #define has_arch_pdevs(d) (!list_empty(&(d)->arch.pdev_list)) >> >> #define gdt_ldt_pt_idx(v) \ >> diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h >> index 2ecc9c9..6b387a3 100644 >> --- a/xen/include/public/arch-x86/xen.h >> +++ b/xen/include/public/arch-x86/xen.h >> @@ -268,7 +268,25 @@ typedef struct arch_shared_info arch_shared_info_t; >> * XEN_DOMCTL_INTERFACE_VERSION. >> */ >> struct xen_arch_domainconfig { >> - char dummy; >> +#define _EMU_LAPIC 0 >> +#define EMU_LAPIC (1U<<_EMU_LAPIC) >> +#define _EMU_HPET 1 >> +#define EMU_HPET (1U<<_EMU_HPET) >> +#define _EMU_PMTIMER 2 >> +#define EMU_PMTIMER (1U<<_EMU_PMTIMER) >> +#define _EMU_RTC 3 >> +#define EMU_RTC (1U<<_EMU_RTC) >> +#define _EMU_IOAPIC 4 >> +#define EMU_IOAPIC (1U<<_EMU_IOAPIC) >> +#define _EMU_PIC 5 >> +#define EMU_PIC (1U<<_EMU_PIC) >> +#define _EMU_PMU 6 >> +#define EMU_PMU (1U<<_EMU_PMU) >> +#define _EMU_VGA 7 >> +#define EMU_VGA (1U<<_EMU_VGA) >> +#define _EMU_IOMMU 8 >> +#define EMU_IOMMU (1U<<_EMU_IOMMU) > > These are all in the public API, so absolutely needs a XEN_ prefix, and > likely an X86 one as well. > >> + uint32_t emulation_flags; > > Either state that this is ignored for domain types other than HVM, or > have arch_domain_create() confirm that it is 0. IMHO adding a check to arch_domain_create is TRTTD. Roger.