From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v7 12/19] x86/VPMU: Initialize PMU for PV guests Date: Wed, 11 Jun 2014 08:49:56 -0400 Message-ID: <53985074.4010205@oracle.com> References: <1402076415-26475-1-git-send-email-boris.ostrovsky@oracle.com> <1402076415-26475-13-git-send-email-boris.ostrovsky@oracle.com> <539849960200007800019D87@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <539849960200007800019D87@mail.emea.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: kevin.tian@intel.com, keir@xen.org, suravee.suthikulpanit@amd.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, jun.nakajima@intel.com List-Id: xen-devel@lists.xenproject.org On 06/11/2014 06:20 AM, Jan Beulich wrote: >>>> On 06.06.14 at 19:40, wrote: >> Code for initializing/tearing down PMU for PV guests >> ... >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -4031,7 +4031,8 @@ static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = { >> [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op, >> HYPERCALL(hvm_op), >> HYPERCALL(sysctl), >> - HYPERCALL(domctl) >> + HYPERCALL(domctl), >> + HYPERCALL(xenpmu_op) > How is this change related to the subject and description? Yes, this is result of my eliminating PVH patch as a standalone one and spreading its content over other patches. I'll update the text. > >> +static int pvpmu_init(struct domain *d, xen_pmu_params_t *params) >> +{ >> + struct vcpu *v; >> + struct page_info *page; >> + uint64_t gfn = params->d.val; >> + >> + if ( !params || params->vcpu >= d->max_vcpus ) >> + return -EINVAL; >> + >> + page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC); >> + if ( !page ) >> + return -EINVAL; >> + >> + if ( !get_page_type(page, PGT_writable_page) ) >> + { >> + put_page(page); >> + return -EINVAL; >> + } >> + >> + v = d->vcpu[params->vcpu]; > I'm sure I asked this before - what guarantees (now as well as > going forward) that you get non-NULL here? Various existing > code paths properly verify both d->vcpu and d->vcpu[...] not > being NULL, so you should do this consistently too. Ah, I thought you were referring to params check last time. I'll add vcpu checks as well. -boris