Various docs and inthe field help patches.

RFC PATCH New Konrad Rzeszutek Wilk 5 participants 17 messages Aug 21, 2012 – Aug 22, 2012

Share & tools

Konrad Rzeszutek Wilk

author

2012-08-21 20:08 UTC #0 lore raw

RFC PATCH 1/1 Various docs and inthe field help patches.

Couple of patches that I've been having in my hg tree that I've neglected to send out. The vga_delay I posted some time ago and I have fixed it per review comments.

The other ones are newer and pertain to documentation and making in the field easier to figure out what is going wrong with a guest.

Konrad Rzeszutek Wilk

author

2012-08-21 20:08 UTC #1 lore raw

PATCH 1/1 xen/vga: Add 'vga_delay' parameter to delay screen output by X miliseconds per line

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 635917c6dac4ab8748572fcbeb3e745428684e15
# Parent e6ca45ca03c2e08af3a74b404166527b68fd1218
xen/vga: Add 'vga_delay' parameter to delay screen output by X miliseconds per line.

This is useful if you find yourself on machine that has no serial console, nor any PCI, PCIe to put in a serial card. Nothing really fancy except it allows to capture the screenshot of the screen using a camera.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

docs/misc/xen-command-line.markdown +9 -0

diff -r e6ca45ca03c2 -r 635917c6dac4 docs/misc/xen-command-line.markdown
@@ -606,6 +606,15 @@ The optional `keep` parameter causes Xen
 console even after dom0 has been started.  The default behaviour is to
 relinquish control to dom0.
 
+### vga_delay
+> `= <miliseconds>`
+
+> Default: `vga_delay=0`
+
+Defines the delay to print a line to the screen. '2' is a a good value
+to get a good screen output. Note: If you need to use this, do so with care
+as it will screw up time handling.
+
 ### vpid (Intel)
 > `= <boolean>`

xen/drivers/video/vga.c +11 -1

diff -r e6ca45ca03c2 -r 635917c6dac4 xen/drivers/video/vga.c
@@ -10,7 +10,7 @@
 #include <xen/mm.h>
 #include <xen/vga.h>
 #include <asm/io.h>
-
+#include <xen/delay.h>
 /* Filled in by arch boot code. */
 struct xen_vga_console_info vga_console_info;

@@ -49,6 +49,15 @@ void (*vga_puts)(const char *) = vga_noo
 static char __initdata opt_vga[30] = "";
 string_param("vga", opt_vga);
 
+/*
+ * 'vga_delay=miliseconds' which defines to delay to print a line
+ * to the screen. 2 is a a good value to get a good screen output.
+ * NOTE: If you need to use this, do so with care as it wil screw up
+ * time handling
+ */
+static unsigned int __read_mostly vga_delay;
+integer_param("vga_delay", vga_delay);
+
 /* VGA text-mode definitions. */
 static unsigned int columns, lines;
 #define ATTRIBUTE   7

@@ -135,6 +144,7 @@ static void vga_text_puts(const char *s)
                 ypos = lines - 1;
                 memmove(video, video + 2 * columns, ypos * 2 * columns);
                 memset(video + ypos * 2 * columns, 0, 2 * xpos);
+                mdelay(vga_delay);
             }
             xpos = 0;
         }

Konrad Rzeszutek Wilk

author

2012-08-21 20:08 UTC #2 lore raw

PATCH 1/1 get_page_type: Print out extra information when failing to get page_type

In reply to Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 8ed3eef706710c9c476a8d984bfb2861d92bedfb
# Parent 635917c6dac4ab8748572fcbeb3e745428684e15
get_page_type: Print out extra information when failing to get page_type.

When any reference to __get_page_type is called and it fails, we get:

(XEN) mm.c:2429:d0 Bad type (saw 1400000000000002 != exp 7000000000000000) for mfn 10e392 (pfn 1bf6c)

with this patch we get some extra details such as:
(XEN) debug.c:127:d0 cr3: 10d80b000, searching for 10e392
(XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [258][272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PMD/L2: [258][511][511] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PGD/L4: [272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [511][511]

where it actually is in the pagetable of the guest. This is useful b/c we can figure out where it is, and use that to figure out where the OS thinks it is.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

xen/arch/x86/debug.c +119 -0

diff -r 635917c6dac4 -r 8ed3eef70671 xen/arch/x86/debug.c
@@ -70,8 +70,127 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct dom
     return mfn;
 }
 
+#define LEVEL_L4 4
+#define LEVEL_L3 3
+#define LEVEL_L2 2
+#define LEVEL_L1 1
+#define UNDEFINED 0
+static void dbg_print_mfn(struct domain *dp, unsigned long mfn,
+                          int l4i, int l3i, int l2i, int l1i)
+{
+	char s[32];
+	char *p;
+	static const char *const names[] = {
+		[LEVEL_L4] = "PGD/L4",
+		[LEVEL_L3] = "PUD/L3",
+		[LEVEL_L2] = "PMD/L2",
+		[LEVEL_L1] = "PTE/L1",
+		[UNDEFINED] = "unknown",
+	};
+	unsigned level = 0;
+	p = s;
+	if (l4i >= 0) {
+		p += snprintf(p, ARRAY_SIZE(s), "[%d]", l4i);
+		level = LEVEL_L4;
+	}
+	if (l3i >= 0) {
+		p += snprintf(p, ARRAY_SIZE(s) - (p - s), "[%d]", l3i);
+		level = LEVEL_L3;
+	}
+	if (l2i >= 0) {
+		p += snprintf(p, ARRAY_SIZE(s) - (p - s), "[%d]", l2i);
+		level = LEVEL_L2;
+	}
+	if (l1i >= 0) {
+		p += snprintf(p, ARRAY_SIZE(s) - (p - s), "[%d]", l1i);
+		level = LEVEL_L1;
+	}
+	gdprintk(XENLOG_WARNING , "cr3(%lx) has mfn(%lx) in level %s: %s\n",
+			dp->vcpu[0]->arch.cr3, mfn, names[level], s);
+#undef LEVEL_L4
+#undef LEVEL_L3
+#undef LEVEL_L2
+#undef LEVEL_L1
+#undef UNDEFINED
+}
+void
+dbg_pv_mfn(unsigned long find_mfn, struct domain *dp)
+{
 #if defined(__x86_64__)
+    l4_pgentry_t l4e, *l4t;
+#endif
+    l3_pgentry_t l3e, *l3t;
+    l2_pgentry_t l2e, *l2t;
+    l1_pgentry_t l1e, *l1t;
+    unsigned long cr3 = dp->vcpu[0]->arch.cr3;
+    int l4i, l3i, l2i, l1i;
+    unsigned long mfn;
 
+	gdprintk(XENLOG_WARNING , "cr3: %lx, searching for %lx\n",
+			dp->vcpu[0]->arch.cr3, find_mfn);
+
+	l4i = l3i = l2i = l1i = 0;
+#if defined(__x86_64__)
+	for ( l4i = 0; l4i < L4_PAGETABLE_ENTRIES; l4i++ )
+	{
+
+        l4t = map_domain_page(cr3 >> PAGE_SHIFT);
+        l4e = l4t[l4i];
+        mfn = l4e_get_pfn(l4e);
+        if (mfn == find_mfn)
+            dbg_print_mfn(dp, mfn, l4i, /* L3 */-1, /* L2 */-1, /* L1 */-1);
+
+        if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
+            continue;
+        mfn = l4e_get_pfn(l4e);
+        unmap_domain_page(l4t);
+        l3t = map_domain_page(mfn);
+#else
+		/* 32-bit start */
+        l3t = map_domain_page(cr3 >> PAGE_SHIFT);
+        l3t += (cr3 & 0xFE0UL) >> 3;
+#endif
+        for ( l3i = 0; l3i < L3_PAGETABLE_ENTRIES; l3i++ )
+        {
+            l3e = l3t[l3i];
+            mfn = l3e_get_pfn(l3e);
+            if ( mfn == find_mfn )
+                dbg_print_mfn(dp, mfn, l4i, l3i, /* L2 */-1, /* L1 */-1);
+
+            if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
+                continue;
+            l2t = map_domain_page(mfn);
+            for ( l2i = 0; l2i < L2_PAGETABLE_ENTRIES; l2i++ )
+            {
+                l2e = l2t[l2i];
+                mfn = l2e_get_pfn(l2e);
+                if (mfn == find_mfn )
+                    dbg_print_mfn(dp, mfn, l4i, l3i, l2i, /* L1 */-1);
+
+                if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) ||
+                    (l2e_get_flags(l2e) & _PAGE_PSE) )
+                    continue;
+                l1t = map_domain_page(mfn);
+                for ( l1i = 0; l1i < L1_PAGETABLE_ENTRIES; l1i ++ )
+                {
+                    l1e = l1t[l1i];
+                    mfn = l1e_get_pfn(l1e);
+                    if ( !mfn_valid(mfn) )
+                        continue;
+                    if ( mfn == find_mfn )
+                        dbg_print_mfn(dp, mfn, l4i, l3i, l2i, l1i);
+                }
+                unmap_domain_page(l1t);
+            }
+            unmap_domain_page(l2t);
+        }
+        unmap_domain_page(l3t);
+#if defined(__x86_64__)
+	}
+#endif
+}
+
+#if defined(__x86_64__)
 /* 
  * pgd3val: this is the value of init_mm.pgd[3] in a PV guest. It is optional.
  *          This to assist debug of modules in the guest. The kernel address

xen/arch/x86/mm.c +3 -0

diff -r 635917c6dac4 -r 8ed3eef70671 xen/arch/x86/mm.c
@@ -2422,6 +2422,8 @@ static int __put_page_type(struct page_i
 }
 
 
+extern void dbg_pv_mfn(unsigned long mfn, struct domain *d);
+
 static int __get_page_type(struct page_info *page, unsigned long type,
                            int preemptible)
 {

@@ -2503,6 +2505,7 @@ static int __get_page_type(struct page_i
                     "for mfn %lx (pfn %lx)",
                     x, type, page_to_mfn(page),
                     get_gpfn_from_mfn(page_to_mfn(page)));
+            dbg_pv_mfn(page_to_mfn(page), page_get_owner(page));
             return -EINVAL;
         }
         else if ( unlikely(!(x & PGT_validated)) )

Jan Beulich

2012-08-22 08:04 UTC #3 lore raw

Re: [PATCH 2 of 4] get_page_type: Print out extra information when failing to get page_type

Quoted 14 lines

Quoted 1 line
Quoted 1 line
Konrad Rzeszutek Wilk <[email protected]> 08/21/12 10:22 PM >>>
get_page_type: Print out extra information when failing to get page_type.
When any reference to __get_page_type is called and it fails, we get:
(XEN) mm.c:2429:d0 Bad type (saw 1400000000000002 != exp 7000000000000000) for mfn 10e392 (pfn 1bf6c)
with this patch we get some extra details such as:
(XEN) debug.c:127:d0 cr3: 10d80b000, searching for 10e392
(XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [258][272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PMD/L2: [258][511][511] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PGD/L4: [272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [511][511]

Quoted 3 lines

where it actually is in the pagetable of the guest. This is useful b/c we can figure out where it is, and use that to figure out where the OS thinks it is.

Hmm, not sure how useful this is: To me, it is first of all non-obvious why there are two instances of L3 here, what the non-consistent indexes really mean, and what the repeated printing of CR3 is good for.

xen/arch/x86/debug.c Tue Aug 21 16:08:29 2012 -0400

Quoted 2 lines

xen/arch/x86/debug.c Tue Aug 21 16:08:29 2012 -0400 +0 -0

Further, putting the code in this file is likely wrong - as it says close to its top, its a helper file for debuggers.

xen/arch/x86/mm.c Tue Aug 21 16:08:29 2012 -0400 static int __put_page_type(struct page_i · line 2429

Quoted 11 lines

xen/arch/x86/mm.c Tue Aug 21 16:08:29 2012 -0400 +2 -0

@@ -2422,6 +2422,8 @@ static int __put_page_type(struct page_i
 }
 
 
+extern void dbg_pv_mfn(unsigned long mfn, struct domain *d);
+
 static int __get_page_type(struct page_info *page, unsigned long type,
 int preemptible)
 {

No extern declarations in C files please.

Jan

Jan Beulich

2012-08-22 14:14 UTC #4 lore raw

Re: [PATCH 2 of 4] get_page_type: Print out extra information when failing to get page_type

In reply to Konrad Rzeszutek Wilk

Quoted 20 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 8ed3eef706710c9c476a8d984bfb2861d92bedfb
# Parent 635917c6dac4ab8748572fcbeb3e745428684e15
get_page_type: Print out extra information when failing to get page_type.
When any reference to __get_page_type is called and it fails, we get:
(XEN) mm.c:2429:d0 Bad type (saw 1400000000000002 != exp 7000000000000000) for mfn 10e392 (pfn 1bf6c)
with this patch we get some extra details such as:
(XEN) debug.c:127:d0 cr3: 10d80b000, searching for 10e392
(XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [258][272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PMD/L2: [258][511][511] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PGD/L4: [272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [511][511]

Quoted 3 lines

where it actually is in the pagetable of the guest. This is useful b/c we can figure out where it is, and use that to figure out where the OS thinks it is.

In addition to my earlier reply, I also think that the printing should be done at info level, so that nothing would get additionally printed without special command line options.

Jan

Konrad Rzeszutek Wilk

2012-08-22 14:24 UTC #5 lore raw

Re: [PATCH 2 of 4] get_page_type: Print out extra information when failing to get page_type

Quoted 20 lines · Jan Beulich

Quoted 20 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 8ed3eef706710c9c476a8d984bfb2861d92bedfb
# Parent 635917c6dac4ab8748572fcbeb3e745428684e15
get_page_type: Print out extra information when failing to get page_type.
When any reference to __get_page_type is called and it fails, we get:
(XEN) mm.c:2429:d0 Bad type (saw 1400000000000002 != exp 7000000000000000) for mfn 10e392 (pfn 1bf6c)
with this patch we get some extra details such as:
(XEN) debug.c:127:d0 cr3: 10d80b000, searching for 10e392
(XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [258][272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PMD/L2: [258][511][511] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PGD/L4: [272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3: [511][511]

Quoted 7 lines

Quoted 3 lines
where it actually is in the pagetable of the guest. This is useful b/c we can figure out where it is, and use that to figure out where the OS thinks it is.
In addition to my earlier reply, I also think that the printing should be done at info level, so that nothing would get additionally printed without special command line options.

I will be more than happy to make those changes. However I think you are feeling that this is not really that useful? Perhaps if I spiced it up by also dumping what those 'types' are and added some rudimentary logic troubleshooting (its an L2 type, and you are trying to add it as an L1 entry as writeable, so on..)

Quoted 9 lines

Jan
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Jan Beulich

2012-08-22 14:51 UTC #6 lore raw

Re: [PATCH 2 of 4] get_page_type: Print out extra information when failing to get page_type

Quoted 33 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
Quoted 8 lines · Jan Beulich
Quoted 8 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 8ed3eef706710c9c476a8d984bfb2861d92bedfb
# Parent 635917c6dac4ab8748572fcbeb3e745428684e15
get_page_type: Print out extra information when failing to get page_type.
Quoted 8 lines
Quoted 8 lines
When any reference to __get_page_type is called and it fails, we get:
(XEN) mm.c:2429:d0 Bad type (saw 1400000000000002 != exp 7000000000000000) for mfn 10e392 (pfn 1bf6c)
with this patch we get some extra details such as:
(XEN) debug.c:127:d0 cr3: 10d80b000, searching for 10e392 (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3:
[258][272]
Quoted 1 line
Quoted 1 line
(XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PMD/L2:
[258][511][511]
Quoted 2 lines
Quoted 2 lines
(XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PGD/L4: [272] (XEN) debug.c:111:d0 cr3(10d80b000) has mfn(10e392) in level PUD/L3:
[511][511]
Quoted 8 lines
Quoted 4 lines
where it actually is in the pagetable of the guest. This is useful b/c we can figure out where it is, and use that to figure out where the OS thinks it is.
In addition to my earlier reply, I also think that the printing should be done at info level, so that nothing would get additionally printed without special command line options.

Quoted 5 lines

I will be more than happy to make those changes. However I think you are feeling that this is not really that useful? Perhaps if I spiced it up by also dumping what those 'types' are and added some rudimentary logic troubleshooting (its an L2 type, and you are trying to add it as an L1 entry as writeable, so on..)

No, it doesn't need to go that far I think. My main concern is that if something like that should get added, it should reduce the need to consult the sources to understand the messages. Currently I'm rather suspecting the inverse.

Jan

Konrad Rzeszutek Wilk

author

2012-08-21 20:08 UTC #7 lore raw

PATCH 1/1 xen/pagetables: Document that all of the initial regions are mapped

In reply to Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 74bedb086c5b72447262e087c0218b89f8bc9140
# Parent 8ed3eef706710c9c476a8d984bfb2861d92bedfb xen/pagetables: Document that all of the initial regions are mapped.

The documentation states that the layout of the initial region looks as so:
a. relocated kernel image
b. initial ram disk [mod_start, mod_len]
c. list of allocated page frames [mfn_list, nr_pages]
(unless relocated due to XEN_ELFNOTE_INIT_P2M)

   d. start_info_t structure        [register ESI (x86)]
   e. bootstrap page tables         [pt_base, CR3 (x86)]
   f. bootstrap stack               [register ESP (x86)]

But it does not clarify that the virtual address to all of those areas is initially mapped by the pt_base (or CR3). Lets fix that.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

xen/include/public/xen.h +3 -0

diff -r 8ed3eef70671 -r 74bedb086c5b xen/include/public/xen.h
@@ -675,6 +675,9 @@ typedef struct shared_info shared_info_t
  *  8. There is guaranteed to be at least 512kB padding after the final
  *     bootstrap element. If necessary, the bootstrap virtual region is
  *     extended by an extra 4MB to ensure this.
+ *
+ *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
+ *  pagetables [pt_base, CR3 (x86)].
  */
 
 #define MAX_GUEST_CMDLINE 1024

Jan Beulich

2012-08-22 14:17 UTC #8 lore raw

Re: [PATCH 3 of 4] xen/pagetables: Document that all of the initial regions are mapped

Quoted 14 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 74bedb086c5b72447262e087c0218b89f8bc9140
# Parent 8ed3eef706710c9c476a8d984bfb2861d92bedfb xen/pagetables: Document that all of the initial regions are mapped.
The documentation states that the layout of the initial region looks as so:
a. relocated kernel image
b. initial ram disk [mod_start, mod_len]
c. list of allocated page frames [mfn_list, nr_pages]
(unless relocated due to XEN_ELFNOTE_INIT_P2M)

Quoted 7 lines

   d. start_info_t structure        [register ESI (x86)]
   e. bootstrap page tables         [pt_base, CR3 (x86)]
   f. bootstrap stack               [register ESP (x86)]
But it does not clarify that the virtual address to all of those areas is initially mapped by the pt_base (or CR3). Lets fix that.

To me this is already being said by "This the order of bootstrap elements in the initial virtual region".

Jan

Quoted 22 lines

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r 8ed3eef70671 -r 74bedb086c5b xen/include/public/xen.h
xen/include/public/xen.h +3 -0
@@ -675,6 +675,9 @@ typedef struct shared_info shared_info_t
  *  8. There is guaranteed to be at least 512kB padding after the final
  *     bootstrap element. If necessary, the bootstrap virtual region is
  *     extended by an extra 4MB to ensure this.
+ *
+ *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
+ *  pagetables [pt_base, CR3 (x86)].
  */
 
 #define MAX_GUEST_CMDLINE 1024
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Konrad Rzeszutek Wilk

2012-08-22 14:27 UTC #9 lore raw

Re: [PATCH 3 of 4] xen/pagetables: Document that all of the initial regions are mapped

Quoted 18 lines · Jan Beulich

Quoted 18 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 74bedb086c5b72447262e087c0218b89f8bc9140
# Parent 8ed3eef706710c9c476a8d984bfb2861d92bedfb xen/pagetables: Document that all of the initial regions are mapped.
The documentation states that the layout of the initial region looks as so:
a. relocated kernel image
b. initial ram disk [mod_start, mod_len]
c. list of allocated page frames [mfn_list, nr_pages]
(unless relocated due to XEN_ELFNOTE_INIT_P2M)
   d. start_info_t structure        [register ESI (x86)]
   e. bootstrap page tables         [pt_base, CR3 (x86)]
   f. bootstrap stack               [register ESP (x86)]

Quoted 6 lines

Quoted 3 lines
But it does not clarify that the virtual address to all of those areas is initially mapped by the pt_base (or CR3). Lets fix that.
To me this is already being said by "This the order of bootstrap elements in the initial virtual region".

Stefano wanted to make sure we have it written as clear as possible. I am going to be a good little submitter and let you guys sort this one out :-)

Quoted 34 lines

Jan
Quoted 18 lines
Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r 8ed3eef70671 -r 74bedb086c5b xen/include/public/xen.h
xen/include/public/xen.h +3 -0
@@ -675,6 +675,9 @@ typedef struct shared_info shared_info_t
  *  8. There is guaranteed to be at least 512kB padding after the final
  *     bootstrap element. If necessary, the bootstrap virtual region is
  *     extended by an extra 4MB to ensure this.
+ *
+ *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
+ *  pagetables [pt_base, CR3 (x86)].
  */
 
 #define MAX_GUEST_CMDLINE 1024
Quoted 4 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Stefano Stabellini

2012-08-22 15:24 UTC #10 lore raw

Re: [PATCH 3 of 4] xen/pagetables: Document that all of the initial regions are mapped

Quoted 23 lines · Konrad Rzeszutek Wilk

Quoted 22 lines · Jan Beulich
Quoted 21 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 74bedb086c5b72447262e087c0218b89f8bc9140
# Parent 8ed3eef706710c9c476a8d984bfb2861d92bedfb xen/pagetables: Document that all of the initial regions are mapped.
The documentation states that the layout of the initial region looks as so:
a. relocated kernel image
b. initial ram disk [mod_start, mod_len]
c. list of allocated page frames [mfn_list, nr_pages]
(unless relocated due to XEN_ELFNOTE_INIT_P2M)
   d. start_info_t structure        [register ESI (x86)]
   e. bootstrap page tables         [pt_base, CR3 (x86)]
   f. bootstrap stack               [register ESP (x86)]
But it does not clarify that the virtual address to all of those areas is initially mapped by the pt_base (or CR3). Lets fix that.

Quoted 6 lines

Quoted 2 lines
To me this is already being said by "This the order of bootstrap elements in the initial virtual region".
Stefano wanted to make sure we have it written as clear as possible. I am going to be a good little submitter and let you guys sort this one out :-)

Let's step back for a second and see if I understand correctly: your patch 6/11 removes the call to xen_map_identity_early on x86_64 because "Xen provides us with all the memory mapped that we need to function".

The original xen_map_identity_early maps up to max_pfn, that is xen_start_info->nr_pages, so I am assuming that what you meant is that "Xen provides us with all the memory already mapped in the bootstrap page tables".
And that is not written anywhere in the Xen headers.

Therefore, if I understand the issue correctly, I would add the following to xen.h:

"On x86_64 the bootstrap page tables map all the pages assigned to the domain."

Quoted 18 lines

Jan

Quoted 15 lines

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

diff -r 8ed3eef70671 -r 74bedb086c5b xen/include/public/xen.h

xen/include/public/xen.h +3 -0

@@ -675,6 +675,9 @@ typedef struct shared_info shared_info_t
  *  8. There is guaranteed to be at least 512kB padding after the final
  *     bootstrap element. If necessary, the bootstrap virtual region is
  *     extended by an extra 4MB to ensure this.
+ *
+ *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
+ *  pagetables [pt_base, CR3 (x86)].
  */
 
 #define MAX_GUEST_CMDLINE 1024

Jan Beulich

2012-08-22 15:35 UTC #11 lore raw

Re: [PATCH 3 of 4] xen/pagetables: Document that all of the initial regions are mapped

Quoted 1 line

Quoted 1 line
Quoted 1 line
On 22.08.12 at 17:24, Stefano Stabellini <[email protected]>

Quoted 41 lines · wrote:

Quoted 23 lines · Konrad Rzeszutek Wilk
Quoted 22 lines · Jan Beulich
Quoted 21 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 74bedb086c5b72447262e087c0218b89f8bc9140
# Parent 8ed3eef706710c9c476a8d984bfb2861d92bedfb xen/pagetables: Document that all of the initial regions are mapped.
The documentation states that the layout of the initial region looks as so:
a. relocated kernel image
b. initial ram disk [mod_start, mod_len]
c. list of allocated page frames [mfn_list, nr_pages]
(unless relocated due to XEN_ELFNOTE_INIT_P2M)
   d. start_info_t structure        [register ESI (x86)]
   e. bootstrap page tables         [pt_base, CR3 (x86)]
   f. bootstrap stack               [register ESP (x86)]
But it does not clarify that the virtual address to all of those areas is initially mapped by the pt_base (or CR3). Lets fix that.
Quoted 6 lines
Quoted 2 lines
To me this is already being said by "This the order of bootstrap elements in the initial virtual region".
Stefano wanted to make sure we have it written as clear as possible. I am going to be a good little submitter and let you guys sort this one out :-)
Let's step back for a second and see if I understand correctly: your patch 6/11 removes the call to xen_map_identity_early on x86_64 because "Xen provides us with all the memory mapped that we need to function".
The original xen_map_identity_early maps up to max_pfn, that is xen_start_info->nr_pages, so I am assuming that what you meant is that "Xen provides us with all the memory already mapped in the bootstrap page tables".
And that is not written anywhere in the Xen headers.

Quoted 5 lines

Therefore, if I understand the issue correctly, I would add the following to xen.h:
"On x86_64 the bootstrap page tables map all the pages assigned to the domain."

That certainly is not the case (and can't be - remember that the virtual space on x86-64 Linux'es initial mapping starts 2Gb from the end of address space, so how could all memory possibly be mapped [i.e. to what virtual addresses would those mappings be done]).

Jan

Konrad Rzeszutek Wilk

author

2012-08-22 16:15 UTC #12 lore raw

Re: [PATCH 3 of 4] xen/pagetables: Document that all of the initial regions are mapped

Quoted 29 lines · Jan Beulich

Quoted 1 line
Quoted 1 line
Quoted 1 line
On 22.08.12 at 17:24, Stefano Stabellini <[email protected]>
Quoted 27 lines · wrote:
Quoted 26 lines · Konrad Rzeszutek Wilk
Quoted 24 lines · Jan Beulich
Quoted 14 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID 74bedb086c5b72447262e087c0218b89f8bc9140
# Parent 8ed3eef706710c9c476a8d984bfb2861d92bedfb xen/pagetables: Document that all of the initial regions are mapped.
The documentation states that the layout of the initial region looks as so:
a. relocated kernel image
b. initial ram disk [mod_start, mod_len]
c. list of allocated page frames [mfn_list, nr_pages]
(unless relocated due to XEN_ELFNOTE_INIT_P2M)
Quoted 7 lines
   d. start_info_t structure        [register ESI (x86)]
   e. bootstrap page tables         [pt_base, CR3 (x86)]
   f. bootstrap stack               [register ESP (x86)]
But it does not clarify that the virtual address to all of those areas is initially mapped by the pt_base (or CR3). Lets fix that.
To me this is already being said by "This the order of bootstrap elements in the initial virtual region".

Quoted 7 lines

Quoted 7 lines
Quoted 3 lines
Stefano wanted to make sure we have it written as clear as possible. I am going to be a good little submitter and let you guys sort this one out :-)
Let's step back for a second and see if I understand correctly: your patch 6/11 removes the call to xen_map_identity_early on x86_64 because "Xen provides us with all the memory mapped that we need to function".

Hm, it should have said: "all the bootstrap memory we need.." and perhaps include that it covers the kernel, ramdisk, p2m list and the pagetables provided by the kernel.

Quoted 19 lines

Quoted 7 lines
The original xen_map_identity_early maps up to max_pfn, that is xen_start_info->nr_pages, so I am assuming that what you meant is that "Xen provides us with all the memory already mapped in the bootstrap page tables".
And that is not written anywhere in the Xen headers.
Quoted 5 lines
Therefore, if I understand the issue correctly, I would add the following to xen.h:
"On x86_64 the bootstrap page tables map all the pages assigned to the domain."
That certainly is not the case (and can't be - remember that the virtual space on x86-64 Linux'es initial mapping starts 2Gb from the end of address space, so how could all memory possibly be mapped [i.e. to what virtual addresses would those mappings be done]).
Jan

Konrad Rzeszutek Wilk

author

2012-08-21 20:08 UTC #13 lore raw

PATCH 1/1 xen/pagetables: Document pt_base inconsistency when running in COMPAT mode

In reply to Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID b992f8e613a3401b9ddd140ce436c840d412beb7
# Parent 74bedb086c5b72447262e087c0218b89f8bc9140 xen/pagetables: Document pt_base inconsistency when running in COMPAT mode.

c/s 13257 added the COMPAT mode wherein a 64-bit hypervisor can run with a 32-bit initial domain. One of the things it added was that the pt_base is offset by two pages. This inconsistency is only present in the COMPAT mode so lets document it.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

xen/include/public/xen.h +4 -1

diff -r 74bedb086c5b -r b992f8e613a3 xen/include/public/xen.h
@@ -663,7 +663,7 @@ typedef struct shared_info shared_info_t
  *      c. list of allocated page frames [mfn_list, nr_pages]
  *         (unless relocated due to XEN_ELFNOTE_INIT_P2M)
  *      d. start_info_t structure        [register ESI (x86)]
- *      e. bootstrap page tables         [pt_base, CR3 (x86)]
+ *      e. bootstrap page tables         [pt_base, CR3 (x86)] *1
  *      f. bootstrap stack               [register ESP (x86)]
  *  4. Bootstrap elements are packed together, but each is 4kB-aligned.
  *  5. The initial ram disk may be omitted.

@@ -678,6 +678,9 @@ typedef struct shared_info shared_info_t
  *
  *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
  *  pagetables [pt_base, CR3 (x86)].
+ *
+ *  *1: When booting under a 64-bit hypervisor with a 32-bit initial domain
+ *  it is offset by two pages (pt_base += PAGE_SIZE*2).
  */
 
 #define MAX_GUEST_CMDLINE 1024

Ian Campbell

2012-08-22 09:21 UTC #14 lore raw

Re: [PATCH 4 of 4] xen/pagetables: Document pt_base inconsistency when running in COMPAT mode

Quoted 27 lines · Konrad Rzeszutek Wilk

# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID b992f8e613a3401b9ddd140ce436c840d412beb7
# Parent 74bedb086c5b72447262e087c0218b89f8bc9140 xen/pagetables: Document pt_base inconsistency when running in COMPAT mode.
c/s 13257 added the COMPAT mode wherein a 64-bit hypervisor can run with a 32-bit initial domain. One of the things it added was that the pt_base is offset by two pages. This inconsistency is only present in the COMPAT mode so lets document it.
Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r 74bedb086c5b -r b992f8e613a3 xen/include/public/xen.h
xen/include/public/xen.h +1 -1
@@ -663,7 +663,7 @@ typedef struct shared_info shared_info_t
  *      c. list of allocated page frames [mfn_list, nr_pages]
  *         (unless relocated due to XEN_ELFNOTE_INIT_P2M)
  *      d. start_info_t structure        [register ESI (x86)]
- *      e. bootstrap page tables         [pt_base, CR3 (x86)]
+ *      e. bootstrap page tables         [pt_base, CR3 (x86)] *1
  *      f. bootstrap stack               [register ESP (x86)]
  *  4. Bootstrap elements are packed together, but each is 4kB-aligned.
  *  5. The initial ram disk may be omitted.

xen/include/public/xen.h typedef struct shared_info shared_info_t · line 683

Quoted 6 lines

xen/include/public/xen.h +3 -0

@@ -678,6 +678,9 @@ typedef struct shared_info shared_info_t
  *
  *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
  *  pagetables [pt_base, CR3 (x86)].
+ *
+ *  *1: When booting under a 64-bit hypervisor with a 32-bit initial domain
+ *  it is offset by two pages (pt_base += PAGE_SIZE*2).

"it" here being the page which you would have to load into cr3?

What, if anything, ends up in the other two pages?

Ian.

Konrad Rzeszutek Wilk

2012-08-22 14:06 UTC #15 lore raw

Re: [PATCH 4 of 4] xen/pagetables: Document pt_base inconsistency when running in COMPAT mode

Quoted 28 lines · Ian Campbell

Quoted 27 lines · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID b992f8e613a3401b9ddd140ce436c840d412beb7
# Parent 74bedb086c5b72447262e087c0218b89f8bc9140 xen/pagetables: Document pt_base inconsistency when running in COMPAT mode.
c/s 13257 added the COMPAT mode wherein a 64-bit hypervisor can run with a 32-bit initial domain. One of the things it added was that the pt_base is offset by two pages. This inconsistency is only present in the COMPAT mode so lets document it.
Signed-off-by Konrad Rzeszutek Wilk <[email protected]>
diff -r 74bedb086c5b -r b992f8e613a3 xen/include/public/xen.h
xen/include/public/xen.h +1 -1
@@ -663,7 +663,7 @@ typedef struct shared_info shared_info_t
  *      c. list of allocated page frames [mfn_list, nr_pages]
  *         (unless relocated due to XEN_ELFNOTE_INIT_P2M)
  *      d. start_info_t structure        [register ESI (x86)]
- *      e. bootstrap page tables         [pt_base, CR3 (x86)]
+ *      e. bootstrap page tables         [pt_base, CR3 (x86)] *1
  *      f. bootstrap stack               [register ESP (x86)]
  *  4. Bootstrap elements are packed together, but each is 4kB-aligned.
  *  5. The initial ram disk may be omitted.
@@ -678,6 +678,9 @@ typedef struct shared_info shared_info_t

Quoted 8 lines

xen/include/public/xen.h typedef struct shared_info shared_info_t · line 683
Quoted 6 lines
xen/include/public/xen.h +3 -0
  *
  *  NOTE: The initial virtual region (3a -> 3f) are all mapped by the initial
  *  pagetables [pt_base, CR3 (x86)].
+ *
+ *  *1: When booting under a 64-bit hypervisor with a 32-bit initial domain
+ *  it is offset by two pages (pt_base += PAGE_SIZE*2).
"it" here being the page which you would have to load into cr3?

No. The pt_base value.

Quoted 2 lines

What, if anything, ends up in the other two pages?

The L3 and L1. The pt_base has incorrect data and points to second L1 instead of the L3.

Its laid out like this: [L3] [L1] [L1] ... [L2]
a b c

and pt_base has the phys_addr of 'c' instead of 'a'.

Jan Beulich

2012-08-22 14:17 UTC #16 lore raw

Re: [PATCH 4 of 4] xen/pagetables: Document pt_base inconsistency when running in COMPAT mode

In reply to Konrad Rzeszutek Wilk

Quoted 8 lines

Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1345579709 14400
# Node ID b992f8e613a3401b9ddd140ce436c840d412beb7
# Parent 74bedb086c5b72447262e087c0218b89f8bc9140 xen/pagetables: Document pt_base inconsistency when running in COMPAT mode.

Quoted 4 lines

c/s 13257 added the COMPAT mode wherein a 64-bit hypervisor can run with a 32-bit initial domain. One of the things it added was that the pt_base is offset by two pages. This inconsistency is only present in the COMPAT mode so lets document it.

Let's first understand whether that's really a bug.

Jan