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

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

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.
> 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

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