[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 06/13] xen: detect pre-allocated memory interfering with e820 map
On 02/19/2015 07:07 PM, David Vrabel wrote:
On 18/02/2015 06:51, Juergen Gross wrote:
Currently especially for dom0 guest memory with guest pfns not
matching host areas populated with RAM are remapped to areas which
are RAM native as well. This is done to be able to use identity
mappings (pfn == mfn) for I/O areas.
Up to now it is not checked whether the remapped memory is already
in use. Remapping used memory will probably result in data corruption,
as the remapped memory will no longer be reserved. Any memory
allocation after the remap can claim that memory.
Add an infrastructure to check for conflicts of reserved memory areas
and in case of a conflict to react via an area specific function.
This function has 3 options:
- Panic
- Handle the conflict by moving the data to another memory area.
This is indicated by a return value other than 0.
- Just return 0. This will delay invalidating the conflicting memory
area to just before doing the remap. This option will be usable for
cases only where the memory will no longer be needed when the remap
operation will be started, e.g. for the p2m list, which is already
copied then.
When doing the remap, check for not remapping a reserved page.
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
arch/x86/xen/setup.c | 185
+++++++++++++++++++++++++++++++++++++++++++++++--
arch/x86/xen/xen-ops.h | 2 +
2 files changed, 182 insertions(+), 5 deletions(-)
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 0dda131..a0af554 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -59,6 +59,20 @@ static unsigned long xen_remap_mfn __initdata =
INVALID_P2M_ENTRY;
static unsigned long xen_remap_pfn;
static unsigned long xen_max_pfn;
+/*
+ * Areas with memblock_reserve()d memory to be checked against final
E820 map.
+ * Each area has an associated function to handle conflicts (by either
+ * removing the conflict or by just crashing with an appropriate
message).
+ * The array has a fixed size as there are only few areas of interest
which are
+ * well known: kernel, page tables, p2m, initrd.
+ */
+#define XEN_N_RESERVED_AREAS 4
+static struct {
+ phys_addr_t start;
+ phys_addr_t size;
+ int (*func)(phys_addr_t start, phys_addr_t size);
+} xen_reserved_area[XEN_N_RESERVED_AREAS] __initdata;
+
/*
* The maximum amount of extra memory compared to the base size. The
* main scaling factor is the size of struct page. At extreme ratios
@@ -365,10 +379,10 @@ static void __init
xen_set_identity_and_remap_chunk(unsigned long start_pfn,
unsigned long end_pfn, unsigned long *released, unsigned long
*remapped)
{
unsigned long pfn;
- unsigned long i = 0;
+ unsigned long i;
unsigned long n = end_pfn - start_pfn;
- while (i < n) {
+ for (i = 0; i < n; ) {
unsigned long cur_pfn = start_pfn + i;
unsigned long left = n - i;
unsigned long size = left;
@@ -411,6 +425,53 @@ static void __init
xen_set_identity_and_remap_chunk(unsigned long start_pfn,
(unsigned long)__va(pfn << PAGE_SHIFT),
mfn_pte(pfn, PAGE_KERNEL_IO), 0);
}
+/* Check to be remapped memory area for conflicts with reserved areas.
+ *
+ * Skip regions known to be reserved which are handled later. For these
+ * regions we have to increase the remapped counter in order to reserve
+ * extra memory space.
+ *
+ * In case a memory page already in use is to be remapped, just BUG().
+ */
+static void __init xen_set_identity_and_remap_chunk_chk(unsigned long
start_pfn,
+ unsigned long end_pfn, unsigned long *released, unsigned long
*remapped)
...remap_chunk_checked() ?
I just wanted to avoid the function name to be even longer. OTOH I
really don't mind to use your suggestion. :-)
+{
+ unsigned long pfn;
+ unsigned long area_start, area_end;
+ unsigned i;
+
+ for (i = 0; i < XEN_N_RESERVED_AREAS; i++) {
+
+ if (!xen_reserved_area[i].size)
+ break;
+
+ area_start = PFN_DOWN(xen_reserved_area[i].start);
+ area_end = PFN_UP(xen_reserved_area[i].start +
+ xen_reserved_area[i].size);
+ if (area_start >= end_pfn || area_end <= start_pfn)
+ continue;
+
+ if (area_start > start_pfn)
+ xen_set_identity_and_remap_chunk(start_pfn, area_start,
+ released, remapped);
+
+ if (area_end < end_pfn)
+ xen_set_identity_and_remap_chunk(area_end, end_pfn,
+ released, remapped);
+
+ *remapped += min(area_end, end_pfn) -
+ max(area_start, start_pfn);
+
+ return;
Why not defer the whole chunk that conflicts? Or for that matter defer
all this remapping to the last minute.
There are two problems arising from this:
- In the initrd case remapping would be deferred too long: the initrd
data is still in use when device initialization is running. And we
really want the remap to have happened before PCI space is being used.
- Delaying all remapping to the point where the new p2m list is in place
would either result in a p2m list with all memory holes covered with
individual entries as the new list is built with those holes still
populated, or we would have to perform the check for being able to use
the pre-built "invalid" or "identity" p2m page in set_phys_to_machine.
The first option could easily waste significant amounts of memory (on
my test machine with 1TB RAM this would have been about 1GB), while
the second option would be performance critical.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
