From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Chen, Tiejun" <tiejun.chen@intel.com>
Subject: Re: [RFC][PATCH 12/13] hvmloader/pci: skip reserved
	ranges
Date: Fri, 15 May 2015 11:18:52 +0800
Message-ID: <5555659C.4040104@intel.com>
References: <1428657724-3498-1-git-send-email-tiejun.chen@intel.com>
	<1428657724-3498-13-git-send-email-tiejun.chen@intel.com>
	<553527940200007800073DA5@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <553527940200007800073DA5@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: tim@xen.org, kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, yang.z.zhang@intel.com
List-Id: xen-devel@lists.xenproject.org

On 2015/4/20 22:21, Jan Beulich wrote:
>>>> On 10.04.15 at 11:22, <tiejun.chen@intel.com> wrote:
>> --- a/tools/firmware/hvmloader/pci.c
>> +++ b/tools/firmware/hvmloader/pci.c
>> @@ -59,8 +59,8 @@ void pci_setup(void)
>>           uint32_t bar_reg;
>>           uint64_t bar_sz;
>>       } *bars = (struct bars *)scratch_start;
>> -    unsigned int i, nr_bars = 0;
>> -    uint64_t mmio_hole_size = 0;
>> +    unsigned int i, j, nr_bars = 0;
>> +    uint64_t mmio_hole_size = 0, reserved_end;
>>
>>       const char *s;
>>       /*
>> @@ -393,8 +393,23 @@ void pci_setup(void)
>>           }
>>
>>           base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
>> + reallocate_mmio:
>>           bar_data |= (uint32_t)base;
>>           bar_data_upper = (uint32_t)(base >> 32);
>> +        for ( j = 0; j < memory_map.nr_map ; j++ )
>> +        {
>> +            if ( memory_map.map[j].type != E820_RAM )
>> +            {
>> +                reserved_end = memory_map.map[j].addr + memory_map.map[j].size;
>> +                if ( check_hole_conflict(base, bar_sz,
>> +                                         memory_map.map[j].addr,
>> +                                         memory_map.map[j].size) )
>> +                {
>> +                    base = (reserved_end  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
>> +                    goto reallocate_mmio;
>> +                }
>> +            }
>> +        }
>>           base += bar_sz;
>>
>>           if ( (base < resource->base) || (base > resource->max) )
>

Actually some original codes are missing here,

         if ( (base < resource->base) || (base > resource->max) )
         {
             printf("pci dev %02x:%x bar %02x size "PRIllx": no space for "
                    "resource!\n", devfn>>3, devfn&7, bar_reg,
                    PRIllx_arg(bar_sz));
             continue;
         }

I think this can guarantee the MMIO regions just fit in the available RAM.

Or am I wrong?

Thanks
Tiejun

> But you do nothing to make sure the MMIO regions all fit in the
> available window (see the code ahead of this relocating RAM if
> necessary).
>
> Jan
>
>