From patchwork Fri Feb 17 17:40:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Ostrovsky X-Patchwork-Id: 9580459 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 22AA86043A for ; Fri, 17 Feb 2017 17:45:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 133BD28762 for ; Fri, 17 Feb 2017 17:45:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0521228764; Fri, 17 Feb 2017 17:45:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2E8DC28762 for ; Fri, 17 Feb 2017 17:45:00 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cemYO-00038a-H3; Fri, 17 Feb 2017 17:42:44 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cemYM-00037w-Ee for xen-devel@lists.xen.org; Fri, 17 Feb 2017 17:42:42 +0000 Received: from [85.158.137.68] by server-8.bemta-3.messagelabs.com id 09/04-31649-11637A85; Fri, 17 Feb 2017 17:42:41 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupkkeJIrShJLcpLzFFi42LpnVTnqitgtjz CYP9fM4slHxezODB6HN39mymAMYo1My8pvyKBNePSr3b2gtPxFTfen2ZqYHzk2cXIxSEkMJFJ 4sOSy4wQzjdGibM3D7NBOBsYJW5u3MHaxcgJ5PQwSrTs5gSx2QSMJM4enc4IYosISEtc+wzRz SwwhVFiwZH5YAlhAXeJjb9nsIPYLAKqEjtOTgcbxCvgKdF18D8biC0hoCAx5eF7ZhCbU8BL4t 3tH4wQyzwllm6+wwJRYyjxeeNS5gmMfAsYGVYxahSnFpWlFukamuglFWWmZ5TkJmbm6BoaGOv lphYXJ6an5iQmFesl5+duYgQGCwMQ7GBcsd3zEKMkB5OSKO8jruURQnxJ+SmVGYnFGfFFpTmp xYcYZTg4lCR4n5sA5QSLUtNTK9Iyc4BhC5OW4OBREuG9DJLmLS5IzC3OTIdInWLU5Th14/RLJ iGWvPy8VClx3jcgRQIgRRmleXAjYDF0iVFWSpiXEegoIZ6C1KLczBJU+VeM4hyMSsK8M0Cm8G TmlcBtegV0BBPQEZ0RS0GOKElESEk1MLJe6VVXc4jOM3mz0PHRu5PeVyydlI72ra5TObTgWW7 gxasr/mtHd6fsl3p/P9A51OEU29cmlWUnPpySOb2iabn7k5qFfjsYfJ8EaC4/tr3m2KSulM8J LzUObNzmtHet3np3xr1LRApdv83WSzxYt3450/XAvxrK7pdOJX366N3TtbROJshwzS8lluKMR EMt5qLiRADY3cwCnAIAAA== X-Env-Sender: boris.ostrovsky@oracle.com X-Msg-Ref: server-10.tower-31.messagelabs.com!1487353358!85485227!1 X-Originating-IP: [141.146.126.69] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTQxLjE0Ni4xMjYuNjkgPT4gMjc3MjE4\n X-StarScan-Received: X-StarScan-Version: 9.2.3; banners=-,-,- X-VirusChecked: Checked Received: (qmail 7656 invoked from network); 17 Feb 2017 17:42:40 -0000 Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by server-10.tower-31.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 17 Feb 2017 17:42:40 -0000 Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v1HHgXeK030452 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 17 Feb 2017 17:42:33 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v1HHgWNn016582 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 17 Feb 2017 17:42:33 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id v1HHgUaA015108; Fri, 17 Feb 2017 17:42:31 GMT Received: from ovs104.us.oracle.com (/10.149.76.204) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 17 Feb 2017 09:42:30 -0800 From: Boris Ostrovsky To: xen-devel@lists.xen.org Date: Fri, 17 Feb 2017 12:40:10 -0500 Message-Id: <1487353212-4395-2-git-send-email-boris.ostrovsky@oracle.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1487353212-4395-1-git-send-email-boris.ostrovsky@oracle.com> References: <1487353212-4395-1-git-send-email-boris.ostrovsky@oracle.com> X-Source-IP: userv0022.oracle.com [156.151.31.74] Cc: Boris Ostrovsky , andrew.cooper3@citrix.com, kevin.tian@intel.com, jun.nakajima@intel.com, jbeulich@suse.com Subject: [Xen-devel] [PATCH v2 1/3] x86/vpmu: Add get/put_vpmu() and VPMU_AVAILABLE X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP vpmu_enabled() (used by hvm/pv_cpuid() to properly report 0xa leaf for Intel processors) is based on the value of VPMU_CONTEXT_ALLOCATED bit. This is problematic: * For HVM guests VPMU context is allocated lazily, during the first access to VPMU MSRs. Since the leaf is typically queried before guest attempts to read or write the MSRs it is likely that CPUID will report no PMU support * For PV guests the context is allocated eagerly but only in responce to guest's XENPMU_init hypercall. There is a chance that the guest will try to read CPUID before making this hypercall. This patch introduces VPMU_AVAILABLE flag which is set (subject to vpmu_mode constraints) during VCPU initialization for both PV and HVM guests. Since this flag is expected to be managed together with vpmu_count, get/put_vpmu() are added to simplify code. vpmu_enabled() (renamed to vpmu_available()) can now use this new flag. (As a side affect this patch also fixes a race in pvpmu_init() where we increment vcpu_count in vpmu_initialise() after checking vpmu_mode) Signed-off-by: Boris Ostrovsky --- v2: * Rename VPMU_ENABLED to VPMU_AVAILABLE (and vpmu_enabled() to vpmu_available() * Check VPMU_AVAILABLE flag in put_vpmu() under lock xen/arch/x86/cpu/vpmu.c | 111 ++++++++++++++++++++++++++++++--------------- xen/arch/x86/cpuid.c | 8 ++-- xen/arch/x86/domain.c | 5 ++ xen/arch/x86/hvm/svm/svm.c | 5 -- xen/arch/x86/hvm/vmx/vmx.c | 5 -- xen/include/asm-x86/vpmu.h | 6 ++- 6 files changed, 88 insertions(+), 52 deletions(-) diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c index 35a9403..b30769d 100644 --- a/xen/arch/x86/cpu/vpmu.c +++ b/xen/arch/x86/cpu/vpmu.c @@ -456,32 +456,21 @@ int vpmu_load(struct vcpu *v, bool_t from_guest) return 0; } -void vpmu_initialise(struct vcpu *v) +static int vpmu_arch_initialise(struct vcpu *v) { struct vpmu_struct *vpmu = vcpu_vpmu(v); uint8_t vendor = current_cpu_data.x86_vendor; int ret; - bool_t is_priv_vpmu = is_hardware_domain(v->domain); BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ); BUILD_BUG_ON(sizeof(struct xen_pmu_amd_ctxt) > XENPMU_CTXT_PAD_SZ); BUILD_BUG_ON(sizeof(struct xen_pmu_regs) > XENPMU_REGS_PAD_SZ); BUILD_BUG_ON(sizeof(struct compat_pmu_regs) > XENPMU_REGS_PAD_SZ); - ASSERT(!vpmu->flags && !vpmu->context); + ASSERT(!(vpmu->flags & ~VPMU_AVAILABLE) && !vpmu->context); - if ( !is_priv_vpmu ) - { - /* - * Count active VPMUs so that we won't try to change vpmu_mode while - * they are in use. - * vpmu_mode can be safely updated while dom0's VPMUs are active and - * so we don't need to include it in the count. - */ - spin_lock(&vpmu_lock); - vpmu_count++; - spin_unlock(&vpmu_lock); - } + if ( !vpmu_available(v) ) + return 0; switch ( vendor ) { @@ -501,7 +490,7 @@ void vpmu_initialise(struct vcpu *v) opt_vpmu_enabled = 0; vpmu_mode = XENPMU_MODE_OFF; } - return; /* Don't bother restoring vpmu_count, VPMU is off forever */ + return -EINVAL; } vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | APIC_LVT_MASKED; @@ -509,15 +498,63 @@ void vpmu_initialise(struct vcpu *v) if ( ret ) printk(XENLOG_G_WARNING "VPMU: Initialization failed for %pv\n", v); - /* Intel needs to initialize VPMU ops even if VPMU is not in use */ - if ( !is_priv_vpmu && - (ret || (vpmu_mode == XENPMU_MODE_OFF) || - (vpmu_mode == XENPMU_MODE_ALL)) ) + return ret; +} + +static void get_vpmu(struct vcpu *v) +{ + spin_lock(&vpmu_lock); + + /* + * Count active VPMUs so that we won't try to change vpmu_mode while + * they are in use. + * vpmu_mode can be safely updated while dom0's VPMUs are active and + * so we don't need to include it in the count. + */ + if ( !is_hardware_domain(v->domain) && + (vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) ) + { + vpmu_count++; + vpmu_set(vcpu_vpmu(v), VPMU_AVAILABLE); + } + else if ( is_hardware_domain(v->domain) && + (vpmu_mode != XENPMU_MODE_OFF) ) + vpmu_set(vcpu_vpmu(v), VPMU_AVAILABLE); + + spin_unlock(&vpmu_lock); +} + +static void put_vpmu(struct vcpu *v) +{ + spin_lock(&vpmu_lock); + + if ( !vpmu_is_set(vcpu_vpmu(v), VPMU_AVAILABLE) ) + goto out; + + if ( !is_hardware_domain(v->domain) && + (vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) ) { - spin_lock(&vpmu_lock); vpmu_count--; - spin_unlock(&vpmu_lock); + vpmu_reset(vcpu_vpmu(v), VPMU_AVAILABLE); } + else if ( is_hardware_domain(v->domain) && + (vpmu_mode != XENPMU_MODE_OFF) ) + vpmu_reset(vcpu_vpmu(v), VPMU_AVAILABLE); + + out: + spin_unlock(&vpmu_lock); +} + +void vpmu_initialise(struct vcpu *v) +{ + get_vpmu(v); + + /* + * Guests without LAPIC (i.e. PV) call vpmu_arch_initialise() + * from pvpmu_init(). + */ + if ( has_vlapic(v->domain) && vpmu_arch_initialise(v) ) + put_vpmu(v); } static void vpmu_clear_last(void *arg) @@ -526,7 +563,7 @@ static void vpmu_clear_last(void *arg) this_cpu(last_vcpu) = NULL; } -void vpmu_destroy(struct vcpu *v) +static void vpmu_arch_destroy(struct vcpu *v) { struct vpmu_struct *vpmu = vcpu_vpmu(v); @@ -551,11 +588,13 @@ void vpmu_destroy(struct vcpu *v) vpmu_save_force, v, 1); vpmu->arch_vpmu_ops->arch_vpmu_destroy(v); } +} - spin_lock(&vpmu_lock); - if ( !is_hardware_domain(v->domain) ) - vpmu_count--; - spin_unlock(&vpmu_lock); +void vpmu_destroy(struct vcpu *v) +{ + vpmu_arch_destroy(v); + + put_vpmu(v); } static int pvpmu_init(struct domain *d, xen_pmu_params_t *params) @@ -565,13 +604,15 @@ static int pvpmu_init(struct domain *d, xen_pmu_params_t *params) struct page_info *page; uint64_t gfn = params->val; - if ( (vpmu_mode == XENPMU_MODE_OFF) || - ((vpmu_mode & XENPMU_MODE_ALL) && !is_hardware_domain(d)) ) - return -EINVAL; - if ( (params->vcpu >= d->max_vcpus) || (d->vcpu[params->vcpu] == NULL) ) return -EINVAL; + v = d->vcpu[params->vcpu]; + vpmu = vcpu_vpmu(v); + + if ( !vpmu_available(v) ) + return -ENOENT; + page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); if ( !page ) return -EINVAL; @@ -582,9 +623,6 @@ static int pvpmu_init(struct domain *d, xen_pmu_params_t *params) return -EINVAL; } - v = d->vcpu[params->vcpu]; - vpmu = vcpu_vpmu(v); - spin_lock(&vpmu->vpmu_lock); if ( v->arch.vpmu.xenpmu_data ) @@ -602,7 +640,8 @@ static int pvpmu_init(struct domain *d, xen_pmu_params_t *params) return -ENOMEM; } - vpmu_initialise(v); + if ( vpmu_arch_initialise(v) ) + put_vpmu(v); spin_unlock(&vpmu->vpmu_lock); @@ -626,7 +665,7 @@ static void pvpmu_finish(struct domain *d, xen_pmu_params_t *params) vpmu = vcpu_vpmu(v); spin_lock(&vpmu->vpmu_lock); - vpmu_destroy(v); + vpmu_arch_destroy(v); xenpmu_data = vpmu->xenpmu_data; vpmu->xenpmu_data = NULL; diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index e0a387e..6d1946a 100644 --- a/xen/arch/x86/cpuid.c +++ b/xen/arch/x86/cpuid.c @@ -713,7 +713,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res) } } - if ( vpmu_enabled(curr) && + if ( vpmu_available(curr) && vpmu_is_set(vcpu_vpmu(curr), VPMU_CPU_HAS_DS) ) { res->d |= cpufeat_mask(X86_FEATURE_DS); @@ -726,7 +726,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res) case 0x0000000a: /* Architectural Performance Monitor Features (Intel) */ if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || - !vpmu_enabled(curr) ) + !vpmu_available(curr) ) goto unsupported; /* Report at most version 3 since that's all we currently emulate. */ @@ -793,7 +793,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res) if ( !hap_enabled(d) && !hvm_pae_enabled(v) ) res->d &= ~cpufeat_mask(X86_FEATURE_PSE36); - if ( vpmu_enabled(v) && + if ( vpmu_available(v) && vpmu_is_set(vcpu_vpmu(v), VPMU_CPU_HAS_DS) ) { res->d |= cpufeat_mask(X86_FEATURE_DS); @@ -811,7 +811,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res) break; case 0x0000000a: /* Architectural Performance Monitor Features (Intel) */ - if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || !vpmu_enabled(v) ) + if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || !vpmu_available(v) ) { *res = EMPTY_LEAF; break; diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 71c0e3c..292442a 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -458,6 +458,8 @@ int vcpu_initialise(struct vcpu *v) if ( is_pv_domain(d) ) xfree(v->arch.pv_vcpu.trap_ctxt); } + else if ( !is_idle_domain(v->domain) ) + vpmu_initialise(v); return rc; } @@ -475,6 +477,9 @@ void vcpu_destroy(struct vcpu *v) vcpu_destroy_fpu(v); + if ( !is_idle_domain(v->domain) ) + vpmu_destroy(v); + if ( has_hvm_container_vcpu(v) ) hvm_vcpu_destroy(v); else diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index ca2785c..27bfb3c 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1169,10 +1169,6 @@ static int svm_vcpu_initialise(struct vcpu *v) return rc; } - /* PVH's VPMU is initialized via hypercall */ - if ( has_vlapic(v->domain) ) - vpmu_initialise(v); - svm_guest_osvw_init(v); return 0; @@ -1180,7 +1176,6 @@ static int svm_vcpu_initialise(struct vcpu *v) static void svm_vcpu_destroy(struct vcpu *v) { - vpmu_destroy(v); svm_destroy_vmcb(v); passive_domain_destroy(v); } diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 42f4fbd..d446e9e 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -330,10 +330,6 @@ static int vmx_vcpu_initialise(struct vcpu *v) } } - /* PVH's VPMU is initialized via hypercall */ - if ( has_vlapic(v->domain) ) - vpmu_initialise(v); - vmx_install_vlapic_mapping(v); /* %eax == 1 signals full real-mode support to the guest loader. */ @@ -354,7 +350,6 @@ static void vmx_vcpu_destroy(struct vcpu *v) */ vmx_vcpu_disable_pml(v); vmx_destroy_vmcs(v); - vpmu_destroy(v); passive_domain_destroy(v); } diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h index e50618f..5e778ab 100644 --- a/xen/include/asm-x86/vpmu.h +++ b/xen/include/asm-x86/vpmu.h @@ -25,7 +25,7 @@ #define vcpu_vpmu(vcpu) (&(vcpu)->arch.vpmu) #define vpmu_vcpu(vpmu) container_of((vpmu), struct vcpu, arch.vpmu) -#define vpmu_enabled(vcpu) vpmu_is_set(vcpu_vpmu(vcpu), VPMU_CONTEXT_ALLOCATED) +#define vpmu_available(vcpu) vpmu_is_set(vcpu_vpmu(vcpu), VPMU_AVAILABLE) #define MSR_TYPE_COUNTER 0 #define MSR_TYPE_CTRL 1 @@ -74,6 +74,7 @@ struct vpmu_struct { #define VPMU_PASSIVE_DOMAIN_ALLOCATED 0x20 /* PV(H) guests: VPMU registers are accessed by guest from shared page */ #define VPMU_CACHED 0x40 +#define VPMU_AVAILABLE 0x80 /* Intel-specific VPMU features */ #define VPMU_CPU_HAS_DS 0x100 /* Has Debug Store */ @@ -89,7 +90,8 @@ static inline void vpmu_reset(struct vpmu_struct *vpmu, const u32 mask) } static inline void vpmu_clear(struct vpmu_struct *vpmu) { - vpmu->flags = 0; + /* VPMU_AVAILABLE should be altered by get/put_vpmu(). */ + vpmu->flags &= VPMU_AVAILABLE; } static inline bool_t vpmu_is_set(const struct vpmu_struct *vpmu, const u32 mask) {