Re: [v4][PATCH 4/9] tools:libxc: check if mmio BAR is out of RMRR mappings

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

On 2014/8/27 4:36, Ian Campbell wrote:
> On Fri, 2014-08-22 at 18:09 +0800, Tiejun Chen wrote:
>
>> +    /* We should check if mmio range is out of RMRR mapping.
>> +     *
>> +     * Assume we have one entry if not enough we'll expand.
>> +     */
>
> The usual approach with such hypervisor interfaces (which I suppose
> xc_reserved_device_memory_map turns into) is to first call it with NULL
> to get the required size and then allocate a suitable buffer and call a
> second time.

Ofentimes, RMRR should be rare with one or two entries, even zero. So I 
think its reasonable to start posting one entry since this can cover 
such a scenario the platform really owns one entry.

>
>
>> +    for ( i = 0; i < rc; i++ )
>> +    {
>> +        rmrr_start = map[i].pfn << PAGE_SHIFT;
>> +        rmrr_end = rmrr_start + map[i].count * PAGE_SIZE;
>> +        if ( check_mmio_hole(rmrr_start, map[i].count * PAGE_SIZE,
>
> Adding rmrr_size = map... & PAGE_SIZE could be used twice here.
>

Yes, so I think the follows should be expected:

--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -251,7 +251,7 @@ static int check_rmrr_overlap(xc_interface *xch, 
uint64_t mmio_start,
                                uint64_t mmio_size)
  {
      struct xen_mem_reserved_device_memory *map = NULL;
-    uint64_t rmrr_start = 0, rmrr_end = 0;
+    uint64_t rmrr_start = 0, rmrr_end = 0, rmrr_size = 0;
      unsigned int i = 0;
      int rc = 0;
      /* Assume we have one entry if not enough we'll expand.*/
@@ -295,9 +295,9 @@ static int check_rmrr_overlap(xc_interface *xch, 
uint64_t mmio_start,
      for ( i = 0; i < rc; i++ )
      {
          rmrr_start = map[i].start_pfn << PAGE_SHIFT;
-        rmrr_end = rmrr_start + map[i].nr_pages * PAGE_SIZE;
-        if ( check_mmio_hole(rmrr_start, map[i].nr_pages * PAGE_SIZE,
-                             mmio_start, mmio_size) )
+        rmrr_size = map[i].nr_pages * PAGE_SIZE;
+        rmrr_end = rmrr_start + rmrr_size;
+        if ( check_mmio_hole(rmrr_start, rmrr_size, mmio_start, 
mmio_size) )
          {
              PERROR("MMIO: [%lx]<->[%lx] overlap RMRR [%lx]<->[%lx]\n",
                     mmio_start, (mmio_start + mmio_size), rmrr_start, 
rmrr_end);

Thanks
Tiejun

> Ian.
>
>