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

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

On 2014/10/24 22:22, Jan Beulich wrote:
> > > > On 24.10.14 at 09:34, <tiejun.chen@xxxxxxxxx> wrote:
> > --- a/tools/firmware/hvmloader/util.c
> > +++ b/tools/firmware/hvmloader/util.c
> > @@ -828,6 +828,72 @@ int hpet_exists(unsigned long hpet_base)
> >       return ((hpet_id >> 16) == 0x8086);
> >   }
> >
> > +int get_reserved_device_memory_map(struct xen_mem_reserved_device_memory 
> > entries[],
> > +                                   uint32_t *max_entries)
> > +{
> > +    int rc;
> > +    struct xen_mem_reserved_device_memory_map memmap = {
> > +        .nr_entries = *max_entries
> > +    };
> > +
> > +    set_xen_guest_handle(memmap.buffer, entries);
> > +
> > +    rc = hypercall_memory_op(XENMEM_reserved_device_memory_map, &memmap);
> > +    if (rc == -ENOBUFS)
> Coding style.
    if ( rc == -ENOBUFS )

> > +        *max_entries = memmap.nr_entries;
> > +
> > +    return rc;
> > +}
> > +
> > +/* Getting all reserved device memory map info in case of hvmloader. */
> > +int hvm_get_reserved_device_memory_map(void)
> > +{
> > +    static uint32_t nr_entries = 0;
> > +    int rc = 0;
> > +
> > +    if ( !rdm_map )
> > +    {
> > +        /* Assume we have one entry if not enough we'll expand.*/
> > +        nr_entries = 1;
> > +        rdm_map = mem_alloc(nr_entries *
> > +                            sizeof(struct xen_mem_reserved_device_memory), 0);
> > +        if ( !rdm_map )
> > +        {
> > +            printf("No space to get reserved dev memory maps!\n");
> > +            return rc;
> > +        }
> > +
> > +        rc = get_reserved_device_memory_map(rdm_map, &nr_entries);
> > +        if ( rc == -ENOBUFS )
> > +        {
> > +            rdm_map = mem_alloc(nr_entries *
> > +                                sizeof(struct xen_mem_reserved_device_memory),
> > +                                0);
> > +            if ( rdm_map )
> > +            {
> > +                rc = get_reserved_device_memory_map(rdm_map, &nr_entries);
> > +                if ( rc )
> > +                {
> > +                    printf("Could not get reserved dev memory info on domain");
> > +                    return rc;
> > +                }
> > +            }
> > +            else
> > +            {
> > +                printf("No space to get reserved dev memory maps!\n");
> > +                return rc;
> > +            }
> > +        }
> > +        else if ( rc )
> > +        {
> > +            printf("Could not get reserved dev memory info on domain");
> > +            return rc;
> > +        }
> > +    }
> > +
> > +    return nr_entries;
> > +}
> I continue to think that adding these functions without user isn't
> really worthwhile in a separate patch.
These functions will be called in pci.c:pci_setup() and 
e820.c:build_e820_table(). If you really like one big patch I can squash 
these three patches as one.
> > --- a/tools/firmware/hvmloader/util.h
> > +++ b/tools/firmware/hvmloader/util.h
> > @@ -241,6 +241,12 @@ int build_e820_table(struct e820entry *e820,
> >                        unsigned int bios_image_base);
> >   void dump_e820_table(struct e820entry *e820, unsigned int nr);
> >
> > +#include <xen/memory.h>
> > +#define ENOBUFS     105 /* No buffer space available */
> This is a joke I hope? The #include belongs at the top (albeit afaict
> you don't really need it here), and the #define is completely
If without this line, #include <xen/memory.h>,

In file included from build.c:25:0:
../util.h:246:70: error: array type has incomplete element type
 int get_reserved_device_memory_map(struct xen_reserved_device_memory 
entries[],
                                                                      ^
make[8]: *** [build.o] Error 1

> misplaced here. While I generally wouldn't recommend doing this, I
> think in the case here including the hypervisor header that defines
> them would be okay. Perhaps not via relative path, but via having
Seems we just need to include this,

#include <errno.h>

> the Makefile symlink the hypervisor header here.
>
> > +struct xen_mem_reserved_device_memory *rdm_map;
> > +int get_reserved_device_memory_map(struct xen_mem_reserved_device_memory 
> > entries[],
> > +                                   uint32_t *max_entries);
> > +int hvm_get_reserved_device_memory_map(void);
> >   #ifndef NDEBUG
> Blank line missing after your additions.
Added.

Thanks
Tiejun

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