|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v8][PATCH 09/17] hvmloader/ram: check if guest memory is out of reserved device memory maps
> From: Chen, Tiejun
> Sent: Monday, December 01, 2014 5:24 PM
>
> We need to check to reserve all reserved device memory maps in e820
> to avoid any potential guest memory conflict.
>
> Currently, if we can't insert RDM entries directly, we may need to handle
> several ranges as follows:
> a. Fixed Ranges --> BUG()
> lowmem_reserved_base-0xA0000: reserved by BIOS implementation,
> BIOS region,
> RESERVED_MEMBASE ~ 0x100000000,
> b. RAM or RAM:Hole -> Try to reserve
>
> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> ---
> tools/firmware/hvmloader/e820.c | 168
> ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 168 insertions(+)
>
> diff --git a/tools/firmware/hvmloader/e820.c
> b/tools/firmware/hvmloader/e820.c
> index 2e05e93..ef87e41 100644
> --- a/tools/firmware/hvmloader/e820.c
> +++ b/tools/firmware/hvmloader/e820.c
> @@ -22,6 +22,7 @@
>
> #include "config.h"
> #include "util.h"
> +#include <xen/memory.h>
>
> void dump_e820_table(struct e820entry *e820, unsigned int nr)
> {
> @@ -68,12 +69,173 @@ void dump_e820_table(struct e820entry *e820,
> unsigned int nr)
> }
> }
>
> +extern struct xen_reserved_device_memory *rdm_map;
> +static unsigned int construct_rdm_e820_maps(unsigned int
> next_e820_entry_index,
> + uint32_t nr_map,
> + struct
> xen_reserved_device_memory *map,
> + struct e820entry
> *e820,
> + unsigned int
> lowmem_reserved_base,
> + unsigned int
> bios_image_base)
> +{
> + unsigned int i, j, sum_nr;
> + uint64_t start, end, next_start, rdm_start, rdm_end;
> + uint32_t type;
> + int err = 0;
> +
> + for ( i = 0; i < nr_map; i++ )
> + {
> + rdm_start = (uint64_t)map[i].start_pfn << PAGE_SHIFT;
> + rdm_end = rdm_start + ((uint64_t)map[i].nr_pages <<
> PAGE_SHIFT);
> +
> + for ( j = 0; j < next_e820_entry_index - 1; j++ )
> + {
> + sum_nr = next_e820_entry_index + nr_map;
need a check whether sum_nr exceeds max #entries...
> + start = e820[j].addr;
> + end = e820[j].addr + e820[j].size;
> + type = e820[j].type;
> + next_start = e820[j+1].addr;
> +
> + if ( rdm_start >= start && rdm_start <= end )
> + {
what about other confliction pattern here, e.g. rdm_end between [start,end]?
> + /*
> + * lowmem_reserved_base-0xA0000: reserved by BIOS
> + * implementation.
> + * Or BIOS region.
> + */
> + if ( (lowmem_reserved_base < 0xA0000 &&
> + start == lowmem_reserved_base) ||
> + start == bios_image_base )
> + {
> + err = -1;
> + break;
> + }
> + }
> +
> + /* Just amid those remaining e820 entries. */
> + if ( (rdm_start > end) && (rdm_end < next_start) )
>= and <=?
> + {
> + memmove(&e820[j+2], &e820[j+1],
> + (sum_nr - j - 1) * sizeof(struct e820entry));
seems you just need (next_e820_entry_index - j - 1)
> +
> + /* Then fill RMRR into that entry. */
> + e820[j+1].addr = rdm_start;
> + e820[j+1].size = rdm_end - rdm_start;
> + e820[j+1].type = E820_RESERVED;
> + next_e820_entry_index++;
> + continue;
continue->break? otherwise the newly added rmrr entry will be checked next
for same rmrr entry, and then catch an unnecessary confliction.
> + }
> +
> + /* Already at the end. */
> + if ( (rdm_start > end) && !next_start )
> + {
> + e820[next_e820_entry_index].addr = rdm_start;
> + e820[next_e820_entry_index].size = rdm_end -
> rdm_start;
> + e820[next_e820_entry_index].type = E820_RESERVED;
> + next_e820_entry_index++;
> + continue;
break though it's the last one.
> + }
> +
> + if ( type == E820_RAM )
> + {
> + /* If coincide with one RAM range. */
> + if ( rdm_start == start && rdm_end == end)
> + {
> + e820[j].type = E820_RESERVED;
> + continue;
> + }
> +
> + /* If we're just aligned with start of one RAM range. */
> + if ( rdm_start == start && rdm_end < end )
> + {
> + memmove(&e820[j+1], &e820[j],
> + (sum_nr - j) * sizeof(struct e820entry));
> +
> + e820[j+1].addr = rdm_end;
> + e820[j+1].size = e820[j].addr + e820[j].size -
> rdm_end;
> + e820[j+1].type = E820_RAM;
> + next_e820_entry_index++;
> +
> + e820[j].addr = rdm_start;
> + e820[j].size = rdm_end - rdm_start;
> + e820[j].type = E820_RESERVED;
> + continue;
> + }
> +
> + /* If we're just aligned with end of one RAM range. */
> + if ( rdm_start > start && rdm_end == end )
> + {
> + memmove(&e820[j+1], &e820[j],
> + (sum_nr - j) * sizeof(struct e820entry));
> +
> + e820[j].size = rdm_start - e820[j].addr;
> + e820[j].type = E820_RAM;
> +
> + e820[j+1].addr = rdm_start;
> + e820[j+1].size = rdm_end - rdm_start;
> + e820[j+1].type = E820_RESERVED;
> + next_e820_entry_index++;
> + continue;
> + }
> +
> + /* If we're just in of one RAM range */
> + if ( rdm_start > start && rdm_end < end )
> + {
> + memmove(&e820[j+2], &e820[j],
> + (sum_nr - j) * sizeof(struct e820entry));
> +
> + e820[j+2].addr = rdm_end;
> + e820[j+2].size = e820[j].addr + e820[j].size -
> rdm_end;
> + e820[j+2].type = E820_RAM;
> + next_e820_entry_index++;
> +
> + e820[j+1].addr = rdm_start;
> + e820[j+1].size = rdm_end - rdm_start;
> + e820[j+1].type = E820_RESERVED;
> + next_e820_entry_index++;
> +
> + e820[j].size = rdm_start - e820[j].addr;
> + e820[j].type = E820_RAM;
> + continue;
> + }
> +
> + /* If we're going last RAM:Hole range */
> + if ( end < next_start && rdm_start > start &&
> + rdm_end < next_start )
> + {
> + memmove(&e820[j+1], &e820[j],
> + (sum_nr - j) * sizeof(struct e820entry));
> +
> + e820[j].size = rdm_start - e820[j].addr;
> + e820[j].type = E820_RAM;
> +
> + e820[j+1].addr = rdm_start;
> + e820[j+1].size = rdm_end - rdm_start;
> + e820[j+1].type = E820_RESERVED;
> + next_e820_entry_index++;
> + continue;
above logic looks incomplete. e.g. what about a RMRR region conflicts with
multiple
e820 entries, etc.
also you need detect confliction with igd_opregion too, since although it's
marked
as reserved, it means some useful thing so confliction with RMRR also means
problem.
ideally like Yang suggested before, opregion base can be adjusted dynamically
but
here at least an error should be thrown out as the 1st step.
> + }
> + }
> + }
here also need a check on 'err'. You don't want to continue the outer loop too
when
you already see an error condition.
> + }
> +
> + /* These overlap may issue guest can't work well. */
> + if ( err )
> + {
> + printf("Guest can't work with some reserved device memory
> overlap!\n");
> + BUG();
> + }
> +
> + /* Fine to construct RDM mappings into e820. */
> + return next_e820_entry_index;
> +}
> +
> /* Create an E820 table based on memory parameters provided in hvm_info.
> */
> int build_e820_table(struct e820entry *e820,
> unsigned int lowmem_reserved_base,
> unsigned int bios_image_base)
> {
> unsigned int nr = 0;
> + unsigned int nr_entries = 0;
>
> if ( !lowmem_reserved_base )
> lowmem_reserved_base = 0xA0000;
> @@ -169,6 +331,12 @@ int build_e820_table(struct e820entry *e820,
> nr++;
> }
>
> + nr_entries = hvm_get_reserved_device_memory_map();
> + if ( nr_entries )
> + nr = construct_rdm_e820_maps(nr, nr_entries, rdm_map, e820,
> + lowmem_reserved_base,
> + bios_image_base);
> +
> return nr;
> }
>
> --
> 1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |