From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@citrix.com>
Subject: Re: [v10][PATCH 06/16] hvmloader/pci: Try to avoid
 placing BARs in RMRRs
Date: Mon, 20 Jul 2015 15:10:29 +0100
Message-ID: <55AD0155.1020108@citrix.com>
References: <1437373023-14884-1-git-send-email-tiejun.chen@intel.com>
	<1437373023-14884-7-git-send-email-tiejun.chen@intel.com>
	<55ACF7EF020000780009305B@mail.emea.novell.com>
	<55ACEEFB.1050707@citrix.com> <55AD004F.7060905@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <55AD004F.7060905@intel.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: "Chen, Tiejun" <tiejun.chen@intel.com>, Jan Beulich <JBeulich@suse.com>, George Dunlap <George.Dunlap@eu.citrix.com>
Cc: xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 07/20/2015 03:06 PM, Chen, Tiejun wrote:
>>>> +/* Find the lowest RMRR higher than base. */
>>>> +static int find_next_rmrr(uint32_t base)
>>>> +{
>>>> +    unsigned int i;
>>>> +    int next_rmrr = -1;
>>>> +    uint64_t min_base = (1ull << 32);
>>>> +
>>>> +    for ( i = 0; i < memory_map.nr_map ; i++ )
>>>> +    {
>>>> +        if ( memory_map.map[i].type == E820_RESERVED &&
>>>> +             memory_map.map[i].addr > base &&
>>>> +             memory_map.map[i].addr < min_base )
>>>> +        {
>>>> +            next_rmrr = i;
>>>> +            min_base = memory_map.map[i].addr;
>>>> +        }
>>>> +    }
>>>> +    return next_rmrr;
>>>> +}
>>>
>>> Considering _both_ callers, I think the function should actually return
>>> the lowest RMRR higher than or equal to base.
>>
>> You mean instead of strictly greater than the base.
>>
>>> Or wait - we actually
>>> need to find the lowest RMRR the _end_ of which is higher than base.
>>
>> Yes, you're right: there's always a risk that pci_mem_start will *start*
>> in the middle of a range.  Looking for the next *end* is more robust.
>>
> 
> Sorry this is not very clear to me so are you saying something like this?
> 
>     for ( i = 0; i < memory_map.nr_map ; i++ )
>     {
>         end = memory_map.map[i].addr + memory_map.map[i].size;
> 
>         if ( memory_map.map[i].type == E820_RESERVED &&
>              end > base &&
>              memory_map.map[i].addr < min_base )
>         {
>             next_rmrr = i;
>             min_base = memory_map.map[i].addr;
>         }
>     }

I think replace min_base with min_end:

         if ( memory_map.map[i].type == E820_RESERVED &&
              end > base &&
              end < min_end )
         {
             next_rmrr = i;
             min_end = end;
         }

Hmm... although I suppose that doesn't catch the possibility of a memory
range crossing the 4G boundary.

 -George