Re: [Xen-devel] [v7][RFC][PATCH 04/13] hvmloader/util: get reserved device memory maps

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2014/11/3 16:53, Jan Beulich wrote:
> > > > On 03.11.14 at 03:22, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2014/10/31 16:14, Jan Beulich wrote:
> > > > > > On 31.10.14 at 03:20, <tiejun.chen@xxxxxxxxx> wrote:
> > > > On 2014/10/30 17:13, Jan Beulich wrote:
> > > > > > > > On 30.10.14 at 06:55, <tiejun.chen@xxxxxxxxx> wrote:
> > > > > > On 2014/10/29 17:05, Jan Beulich wrote:
> > > > > > > > > > On 29.10.14 at 07:54, <tiejun.chen@xxxxxxxxx> 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
> > > > > > '<xen/memory.h>,
> > > > > >
> > > > > > #include "util.h"
> > > > > > ...
> > > > > > #include <xen/memory.h>
> > > > > >
> > > > > > 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.
> This argument which you brought up before, and which we commented
> on before, is pretty pointless. We don't really care much about doing
> one or two more hypercalls from hvmloader, unless these would be
> long-running ones.
Another benefit to use a global variable is that we wouldn't allocate 
xen_reserved_device_memory * N each time, and reduce some duplicated 
codes, unless you mean I should define that as static inside in local.
Thanks
Tiejun


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel