From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v6 01/19] common/symbols: Export hypervisor symbols to privileged guest Date: Fri, 16 May 2014 12:12:53 -0400 Message-ID: <53763905.9020101@oracle.com> References: <1399996413-1998-1-git-send-email-boris.ostrovsky@oracle.com> <1399996413-1998-2-git-send-email-boris.ostrovsky@oracle.com> <5375E2D30200007800012E89@mail.emea.novell.com> <5376279D.8000503@oracle.com> <537647F70200007800013318@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: <537647F70200007800013318@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: Tim Deegan , kevin.tian@intel.com, keir@xen.org, jun.nakajima@intel.com, andrew.cooper3@citrix.com, Ian Jackson , donald.d.dugger@intel.com, xen-devel@lists.xen.org, Ian Campbell , dietmar.hahn@ts.fujitsu.com, suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org On 05/16/2014 11:16 AM, Jan Beulich wrote: >>>> On 16.05.14 at 16:58, wrote: >> On 05/16/2014 04:05 AM, Jan Beulich wrote: >>> Considering that xensyms_read() is there only to implement this sub- >>> hypercall, I wonder whether you wouldn't be better of passing it the >>> handle, and have it do the copying. It's holding a lock itself, so the >>> static buffer could be placed there, along with the other statics it >>> already uses (which btw should go into the only function that make >>> use of them). >> I didn't want to expose xensyms_read() to any handles to keep it a >> purely "internal" (for the lack of a better term) routine. >> >> But if 'name' is to become static I don't want to introduce another lock >> (or move existing one up from xensyms_read()) so I guess I would have to >> pass the handle. > No, even in the caller you're already protected by a lock. Ah, yes. Then I should be able to simply make 'name' static in do_platform_op(). >>>> --- a/xen/arch/x86/x86_64/platform_hypercall.c >>>> +++ b/xen/arch/x86/x86_64/platform_hypercall.c >>>> @@ -32,6 +32,8 @@ CHECK_pf_pcpu_version; >>>> CHECK_pf_enter_acpi_sleep; >>>> #undef xen_pf_enter_acpi_sleep >>>> >>>> +#define xenpf_symdata compat_pf_symdata >>> Did you check that you really need this? There's no explicit instance of >>> the structure, so I would think it's not needed. >> Isn't this required for auto-building compat interfaces? > That depends, and it seems to me that it's not needed here. OK, I'll see if I really need it. -boris