Re: [v4][PATCH 14/19] tools/libxl: detect and avoid conflicts with RDM

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Jun 26, 2015 at 01:45:02PM +0800, Chen, Tiejun wrote:
> On 2015/6/25 19:23, Wei Liu wrote:
> >On Tue, Jun 23, 2015 at 05:57:25PM +0800, Tiejun Chen wrote:
> >>While building a VM, HVM domain builder provides struct hvm_info_table{}
> >>to help hvmloader. Currently it includes two fields to construct guest
> >>e820 table by hvmloader, low_mem_pgend and high_mem_pgend. So we should
> >>check them to fix any conflict with RAM.
> >>
> >
> >RAM -> RDM?
> 
> Fixed.
> 
> >
> >>RMRR can reside in address space beyond 4G theoretically, but we never
> 
> [snip]
> 
> >>+static struct xen_reserved_device_memory
> >>+*xc_device_get_rdm(libxl__gc *gc,
> >>+                   uint32_t flag,
> >>+                   uint16_t seg,
> >>+                   uint8_t bus,
> >>+                   uint8_t devfn,
> >>+                   unsigned int *nr_entries)
> >
> >I just notice this function lives in libxl_dm.c. The function should be
> >renamed to libxl__xc_device_get_rdm.
> >
> >This function should return proper libxl error code (ERROR_FAIL or
> >something more appropriate). The allocated RDM entries should be
> 
> ERROR_FAIL is better.
> 
> So refactor this function after address your all comments,
> 
> static int
> libxl__xc_device_get_rdm(libxl__gc *gc,
>                          uint32_t flag,
>                          uint16_t seg,
>                          uint8_t bus,
>                          uint8_t devfn,
>                          unsigned int *nr_entries,
>                          struct xen_reserved_device_memory *xrdm)

Shouldn't this be struct xen_reserved_device_memory **xrdm? That is,
pointer to a pointer. The code structure looks OK.

Wei.