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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

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

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

Jan


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