From: George Dunlap <george.dunlap@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
George Dunlap <George.Dunlap@eu.citrix.com>,
Tiejun Chen <tiejun.chen@intel.com>
Cc: xen-devel@lists.xen.org
Subject: Re: [v10][PATCH 06/16] hvmloader/pci: Try to avoid placing BARs in RMRRs
Date: Mon, 20 Jul 2015 13:52:11 +0100 [thread overview]
Message-ID: <55ACEEFB.1050707@citrix.com> (raw)
In-Reply-To: <55ACF7EF020000780009305B@mail.emea.novell.com>
On 07/20/2015 12:30 PM, Jan Beulich wrote:
>>>> On 20.07.15 at 08:16, <tiejun.chen@intel.com> wrote:
>> --- a/tools/firmware/hvmloader/pci.c
>> +++ b/tools/firmware/hvmloader/pci.c
>> @@ -38,6 +38,43 @@ uint64_t pci_hi_mem_start = 0, pci_hi_mem_end = 0;
>> enum virtual_vga virtual_vga = VGA_none;
>> unsigned long igd_opregion_pgbase = 0;
>>
>> +/* Check if any conflicts with all reserved device memory. */
>
> /* Check if the specified range conflicts with any reserved device memory. */
>
> (and the "any" could perhaps be left out)
>
>> +static bool check_overlap_all(uint64_t start, uint64_t size)
>> +{
>> + unsigned int i;
>> +
>> + for ( i = 0; i < memory_map.nr_map; i++ )
>> + {
>> + if ( memory_map.map[i].type == E820_RESERVED &&
>> + check_overlap(start, size,
>> + memory_map.map[i].addr,
>> + memory_map.map[i].size) )
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +/* 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.
>> @@ -407,6 +456,19 @@ void pci_setup(void)
>> }
>>
>> base = (resource->base + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
>> +
>> + /* If we're using mem_resource, check for RMRR conflicts. */
>> + while ( resource == &mem_resource &&
>> + next_rmrr > 0 &&
>
>> = I think.
Yes, > is a typo; I certainly meant to type >=.
>
>> + check_overlap(base, bar_sz,
>> + memory_map.map[next_rmrr].addr,
>> + memory_map.map[next_rmrr].size) )
>> + {
>> + base = memory_map.map[next_rmrr].addr + memory_map.map[next_rmrr].size;
>> + base = (base + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
>
> Pointless cast.
This was copy-and-pasted from the line above. I had assumed that bar_sz
was uint32_t (perhaps it was at some point), but I guess now it's
pointless there too. :-)
-George
next prev parent reply other threads:[~2015-07-20 12:52 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-20 6:16 [v10][PATCH 00/16] Fix RMRR Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 01/16] xen: introduce XENMEM_reserved_device_memory_map Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 02/16] xen/vtd: create RMRR mapping Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 04/16] xen: enable XENMEM_memory_map in hvm Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 05/16] hvmloader: get guest memory map into memory_map[] Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 06/16] hvmloader/pci: Try to avoid placing BARs in RMRRs Tiejun Chen
2015-07-20 11:00 ` George Dunlap
2015-07-20 11:30 ` Jan Beulich
2015-07-20 12:52 ` George Dunlap [this message]
2015-07-20 14:06 ` Chen, Tiejun
2015-07-20 14:10 ` George Dunlap
2015-07-20 14:16 ` Jan Beulich
2015-07-20 14:32 ` Chen, Tiejun
2015-07-20 15:00 ` Jan Beulich
2015-07-21 0:53 ` Chen, Tiejun
2015-07-21 6:18 ` Jan Beulich
2015-07-21 9:42 ` George Dunlap
2015-07-20 6:16 ` [v10][PATCH 07/16] hvmloader/e820: construct guest e820 table Tiejun Chen
2015-07-20 11:56 ` Jan Beulich
2015-07-20 14:35 ` Chen, Tiejun
2015-07-20 15:03 ` Jan Beulich
2015-07-20 13:00 ` George Dunlap
2015-07-20 13:23 ` Chen, Tiejun
2015-07-20 13:50 ` George Dunlap
2015-07-20 13:57 ` Chen, Tiejun
2015-07-20 6:16 ` [v10][PATCH 08/16] tools/libxc: Expose new hypercall xc_reserved_device_memory_map Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 09/16] tools: extend xc_assign_device() to support rdm reservation policy Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 10/16] tools: introduce some new parameters to set rdm policy Tiejun Chen
2015-07-20 6:16 ` [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM Tiejun Chen
2015-07-20 13:32 ` Ian Jackson
2015-07-20 14:40 ` Chen, Tiejun
2015-07-20 14:53 ` Ian Jackson
2015-07-20 15:08 ` Chen, Tiejun
2015-07-20 15:24 ` Ian Jackson
2015-07-20 15:38 ` Ian Campbell
2015-07-21 6:44 ` Chen, Tiejun
2015-07-21 6:45 ` Chen, Tiejun
2015-07-21 6:38 ` Chen, Tiejun
2015-07-21 10:48 ` Ian Jackson
2015-07-21 11:12 ` Chen, Tiejun
2015-07-21 10:41 ` Ian Jackson
2015-07-21 11:04 ` Chen, Tiejun
2015-07-21 11:11 ` Ian Jackson
2015-07-21 11:23 ` Chen, Tiejun
2015-07-21 11:27 ` Ian Jackson
2015-07-21 11:45 ` Chen, Tiejun
2015-07-21 12:33 ` Ian Jackson
2015-07-21 13:29 ` Chen, Tiejun
2015-07-21 15:09 ` Ian Jackson
2015-07-21 15:42 ` Chen, Tiejun
2015-07-21 15:57 ` Ian Jackson
2015-07-21 15:57 ` Ian Jackson
2015-07-22 0:33 ` Chen, Tiejun
2015-07-22 8:43 ` Ian Campbell
2015-07-22 9:18 ` Chen, Tiejun
2015-07-22 10:28 ` Ian Jackson
2015-07-22 10:51 ` Ian Campbell
2015-07-21 13:41 ` Chen, Tiejun
2015-07-21 15:10 ` Ian Jackson
2015-07-21 15:22 ` Chen, Tiejun
2015-07-21 15:31 ` Ian Jackson
2015-07-21 12:04 ` Chen, Tiejun
2015-07-21 12:34 ` Ian Jackson
2015-07-20 6:16 ` [v10][PATCH 12/16] tools: introduce a new parameter to set a predefined rdm boundary Tiejun Chen
2015-07-20 6:17 ` [v10][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest Tiejun Chen
2015-07-20 13:34 ` Ian Jackson
2015-07-20 6:17 ` [v10][PATCH 14/16] xen/vtd: enable USB device assignment Tiejun Chen
2015-07-20 6:17 ` [v10][PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr Tiejun Chen
2015-07-20 6:17 ` [v10][PATCH 16/16] tools: parse to enable new rdm policy parameters Tiejun Chen
2015-07-20 13:43 ` Ian Jackson
2015-07-20 13:53 ` Chen, Tiejun
2015-07-20 10:37 ` [v10][PATCH 00/16] Fix RMRR George Dunlap
2015-07-20 12:39 ` Chen, Tiejun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55ACEEFB.1050707@citrix.com \
--to=george.dunlap@citrix.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=JBeulich@suse.com \
--cc=tiejun.chen@intel.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).