From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v10 09/20] x86/VPMU: Add public xenpmu.h Date: Fri, 12 Sep 2014 07:50:38 +0100 Message-ID: <5412B3DE02000078000344A0@mail.emea.novell.com> References: <1409802080-6160-1-git-send-email-boris.ostrovsky@oracle.com> <1409802080-6160-10-git-send-email-boris.ostrovsky@oracle.com> <54108045020000780003362D@mail.emea.novell.com> <5410890B.9010900@oracle.com> <54115FCA0200007800033A6C@mail.emea.novell.com> <5411A97C.7090100@oracle.com> <5411D40F0200007800033FDA@mail.emea.novell.com> <5411BF12.6090600@oracle.com> <5411E2F502000078000340BA@mail.emea.novell.com> <5411D31B.90403@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5411D31B.90403@oracle.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: tim@xen.org, kevin.tian@intel.com, keir@xen.org, suravee.suthikulpanit@amd.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, xen-devel@lists.xen.org, Aravind.Gopalakrishnan@amd.com, jun.nakajima@intel.com List-Id: xen-devel@lists.xenproject.org >>> On 11.09.14 at 18:51, wrote: > On 09/11/2014 11:59 AM, Jan Beulich wrote: >>>>> On 11.09.14 at 17:26, wrote: >>> On 09/11/2014 10:55 AM, Jan Beulich wrote: >>>>>>> On 11.09.14 at 15:54, wrote: >>>>> On 09/11/2014 02:39 AM, Jan Beulich wrote: >>>>>>>>> On 10.09.14 at 19:23, wrote: >>>>>>> On 09/10/2014 10:45 AM, Jan Beulich wrote: >>>>>>>>>>> On 04.09.14 at 05:41, wrote: >>>>>>>>> +struct xen_pmu_arch { >>>>>>>>> + union { >>>>>>>>> + struct cpu_user_regs regs; >>>>>>>>> + uint8_t pad[256]; >>>>>>>>> + } r; >>>>>>>> Can you remind me again what you need the union and padding for >>>>>>>> here? >>>>>>> This structure is laid out in a shared page with a (possibly 32-bit) >>>>>>> guest who need to access fields that follow this union. >>>>>> Hmm, okay. But how would such a guest make reasonable use of >>>>>> the regs field then? >>>>> When hypervisor is preparing this data for 32-bit consumer in >>>>> vpmu_do_interrupts() it translates registers to 32-bit version: >>>>> >>>>> struct compat_cpu_user_regs *cmp; >>>>> gregs = guest_cpu_user_regs(); >>>>> cmp = (void *)&vpmu->xenpmu_data->pmu.r.regs; >>>>> XLAT_cpu_user_regs(cmp, gregs); >>>>> >>>>> I remember struggling trying to figure a better way of presenting this >>>>> but ended up with the (void *) cast. IIRC I tried putting >>>>> compat_cpu_user_regs into the union but something didn't quite work >>>>> (with compilation). >>>> Of course that can't work - the compat structure simply doesn't >>>> exist for public headers. >>>> >>>>>> And then - why 256 and not 200? struct >>>>>> cpu_user_regs can't change size anyway. Plus, finally, why do >>>>>> you expose the GPRs but not any of the other register state? >>>>> I wanted to leave some padding in case we decide to add non-GPR >>>>> registers and keep major version of the interface unchanged (only minor >>>>> version will bumped). TBH though, I can't think of any non-GPR registers >>>>> to be ever useful. >>>> Then what do you need the GPRs for here? I don't think they're >>>> any better or worse than, say, XMM ones. I could see you needing/ >>>> wanting some basic stuff like CS:RIP and SS:RSP and maybe EFLAGS, >>>> but that's about it. >>> I believe some perf sub-tools (tracing-related if I am not mistaken) >>> want to have access to traced function's arguments. >> And function arguments on x86-64 can very well live in XMM >> registers... Hence no, I still don't see why the registers get >> exposed here in an incomplete/inconsistent fashion. > > Linux perf handler takes struct pt_regs as the its sole argument. If we > pass only few selected registers from hypervisor to the guest then I > will be passing garbage (partly) to perf. Okay, so you're tailoring the hypervisor interface to Linux. That's not what we generally aim for, and hence I continue to think this isn't the right approach. Jan