From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v7][RFC][PATCH 04/13] hvmloader/util: get reserved device memory maps Date: Mon, 03 Nov 2014 10:22:33 +0800 Message-ID: <5456E6E9.1080104@intel.com> References: <1414136077-18599-1-git-send-email-tiejun.chen@intel.com> <1414136077-18599-5-git-send-email-tiejun.chen@intel.com> <544A7CCB0200007800041FBA@mail.emea.novell.com> <544DB809.9020108@intel.com> <544E22410200007800042568@mail.emea.novell.com> <544F27BD.7060003@intel.com> <544F749A0200007800042B74@mail.emea.novell.com> <54508F1B.2030903@intel.com> <5450BBF50200007800043032@mail.emea.novell.com> <5451D2B5.50609@intel.com> <54520F2C0200007800043625@mail.emea.novell.com> <5452F207.1070105@intel.com> <545352F40200007800043D82@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: <545352F40200007800043D82@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, wei.liu2@citrix.com, "ian.campbell@citrix.com" , "stefano.stabellini@eu.citrix.com" , tim@xen.org, "ian.jackson@eu.citrix.com" , xen-devel@lists.xen.org, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2014/10/31 16:14, Jan Beulich wrote: >>>> On 31.10.14 at 03:20, wrote: >> On 2014/10/30 17:13, Jan Beulich wrote: >>>>>> On 30.10.14 at 06:55, wrote: >>>> On 2014/10/29 17:05, Jan Beulich wrote: >>>>>>>> On 29.10.14 at 07:54, wrote: >>>>>> Looks I can remove those stuff from util.h and just add 'extern' to them >>>>>> when we really need them. >>>>> >>>>> Please stop thinking this way. Declarations for things defined in .c >>>>> files are to be present in headers, and the defining .c file has to >>>>> include that header (making sure declaration and definition are and >>>>> remain in sync). I hate having to again repeat my remark that you >>>>> shouldn't forget it's not application code that you're modifying. >>>>> Robust and maintainable code are a requirement in the hypervisor >>>>> (and, as said it being an extension of it, hvmloader). Which - just >>>>> to avoid any misunderstanding - isn't to say that this shouldn't also >>>>> apply to application code. It's just that in the hypervisor and kernel >>>>> (and certain other code system components) the consequences of >>>>> being lax are much more severe. >>>> >>>> Okay. But currently, the pci.c file already include 'util.h' and >>>> ', >>>> >>>> #include "util.h" >>>> ... >>>> #include >>>> >>>> We can't redefine struct xen_reserved_device_memory in util.h. >>> >>> Redefine? I said forward declare. >> >> Seems we just need to declare hvm_get_reserved_device_memory_map() in >> the head file, tools/firmware/hvmloader/util.h, >> >> unsigned int hvm_get_reserved_device_memory_map(void); > > To me this looks very much like poor programming style, even if in > the context of hvmloader communicating information via global > variables rather than function arguments and return values is Do you mean you don't like a global variable? But it can be use to get RDM without more hypercall or function call in the context of hvmloader. > generally possible. The following is what I did: +struct xen_reserved_device_memory *rdm_map; +static int +get_reserved_device_memory_map(struct xen_reserved_device_memory entries[], + uint32_t *max_entries) +{ + int rc; + struct xen_reserved_device_memory_map xrdmmap = { + .nr_entries = *max_entries + }; + + set_xen_guest_handle(xrdmmap.buffer, entries); + + rc = hypercall_memory_op(XENMEM_reserved_device_memory_map, &xrdmmap); + if ( rc == -ENOBUFS ) + *max_entries = xrdmmap.nr_entries; + + return rc; +} + +/* + * Getting all reserved device memory map info in case of hvmloader. + * We just return zero for any failed cases, and this means we + * can't further handle any reserved device memory. + */ +unsigned int hvm_get_reserved_device_memory_map(void) +{ + ... +} So if you think they're not good, just please define these prototypes then I can finish them. Thanks Tiejun