From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4 13/17] x86/VPMU: Add privileged PMU mode Date: Tue, 04 Feb 2014 10:53:49 -0500 Message-ID: <52F10D0D.2050908@oracle.com> References: <1390331342-3967-1-git-send-email-boris.ostrovsky@oracle.com> <1390331342-3967-14-git-send-email-boris.ostrovsky@oracle.com> <52F0DDBE0200007800118F62@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52F0DDBE0200007800118F62@nat28.tlf.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: keir@xen.org, suravee.suthikulpanit@amd.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, jun.nakajima@intel.com List-Id: xen-devel@lists.xenproject.org On 02/04/2014 06:31 AM, Jan Beulich wrote: >>>> On 21.01.14 at 20:08, Boris Ostrovsky wrote: >> @@ -152,33 +162,62 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs) >> err = vpmu->arch_vpmu_ops->arch_vpmu_save(v); >> vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED); >> >> - /* Store appropriate registers in xenpmu_data */ >> - if ( is_pv_32bit_domain(current->domain) ) >> + if ( !is_hvm_domain(current->domain) ) >> { >> - /* >> - * 32-bit dom0 cannot process Xen's addresses (which are 64 bit) >> - * and therefore we treat it the same way as a non-priviledged >> - * PV 32-bit domain. >> - */ >> - struct compat_cpu_user_regs *cmp; >> - >> - gregs = guest_cpu_user_regs(); >> - >> - cmp = (struct compat_cpu_user_regs *) >> - &v->arch.vpmu.xenpmu_data->pmu.r.regs; >> - XLAT_cpu_user_regs(cmp, gregs); >> + uint16_t cs = (current->arch.flags & TF_kernel_mode) ? 0 : 0x3; > The surrounding if checks !hvm, i.e. both PV and PVH can make it > here. But TF_kernel_mode is meaningful for PV only. As of this patch PVH doesn't work and you won't get into this code path. And the later patch (#16) addresses this. (Although it unnecessary calculates cs in the line above for PVH, only to call hvm_get_segment_register() later. I'll move it to !pvh clause there). > >> + >> + /* Store appropriate registers in xenpmu_data */ >> + if ( is_pv_32bit_domain(current->domain) ) >> + { >> + gregs = guest_cpu_user_regs(); >> + >> + if ( (vpmu_mode & XENPMU_MODE_PRIV) && >> + !is_pv_32bit_domain(v->domain) ) >> + memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs, >> + gregs, sizeof(struct cpu_user_regs)); >> + else >> + { >> + /* >> + * 32-bit dom0 cannot process Xen's addresses (which are >> + * 64 bit) and therefore we treat it the same way as a >> + * non-priviledged PV 32-bit domain. >> + */ >> + >> + struct compat_cpu_user_regs *cmp; >> + >> + cmp = (struct compat_cpu_user_regs *) >> + &v->arch.vpmu.xenpmu_data->pmu.r.regs; >> + XLAT_cpu_user_regs(cmp, gregs); >> + } >> + } >> + else if ( !is_control_domain(current->domain) && >> + !is_idle_vcpu(current) ) >> + { >> + /* PV guest */ >> + gregs = guest_cpu_user_regs(); >> + memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs, >> + gregs, sizeof(struct cpu_user_regs)); >> + } >> + else >> + memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs, >> + regs, sizeof(struct cpu_user_regs)); >> + >> + gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs; >> + gregs->cs = cs; > And now you store a NUL selector (i.e. just the RPL bits) into the > output field? >> } >> - else if ( !is_control_domain(current->domain) && >> - !is_idle_vcpu(current) ) >> + else >> { >> - /* PV guest */ >> + /* HVM guest */ >> + struct segment_register cs; >> + >> gregs = guest_cpu_user_regs(); >> memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs, >> gregs, sizeof(struct cpu_user_regs)); >> + >> + hvm_get_segment_register(current, x86_seg_cs, &cs); >> + gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs; >> + gregs->cs = cs.attr.fields.dpl; > And here too? If that's intended, a code comment is a must. This is HVM-only path, PVH or PV don't go here so cs should be valid. -boris