diff mbox

Is there any limitation on the firmware size in Xen?

Message ID 59159D610200007800159210@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich May 12, 2017, 9:32 a.m. UTC
>>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
> On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
>> Note that hvmloader's main() has
>> 
>>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
>> 
>> very early, so you having got past this means the corruption
>> occurred inside hvmloader (or at least while it was already
>> running). Could you comment out the call to perform_tests()
>> and try again?
>> 
> You got it. After commenting out perform_tests(), the grub2 menu showed
> and the system booted.
> 
> It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
> the members of hvm_start_info became 0 in my test.

So could you give the below/attached patch a try?

Jan
hvmloader: avoid tests when they would clobber used memory

First of all limit the memory range used for testing to 4Mb: There's no
point placing page tables right above 8Mb when they can equally well
live at the bottom of the chunk at 4Mb - rep_io_test() cares about the
5Mb...7Mb range only anyway. In a subsequent patch this will then also
allow simply looking for an unused 4Mb range (instead of using a build
time determined one).

Extend the "skip tests" condition beyond the "is there enough memory"
question.

Reported-by: Charles Arnold <carnold@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -19,7 +19,9 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "util.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -29,10 +31,12 @@
  * Memory layout during tests:
  *  4MB to 8MB is cleared.
  *  Page directory resides at 8MB.
- *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
- *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
+ *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
+ *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
  */
-#define PD_START (8ul << 20)
+#define TEST_MEM_BASE (4ul << 20)
+#define TEST_MEM_SIZE (4ul << 20)
+#define PD_START TEST_MEM_BASE
 #define PT_START (PD_START + 4096)
 
 static void setup_paging(void)
@@ -41,10 +45,10 @@ static void setup_paging(void)
     uint32_t *pt = (uint32_t *)PT_START;
     uint32_t i;
 
-    /* Identity map 0-16MB. */
-    for ( i = 0; i < 4; i++ )
+    /* Identity map 0-8MB. */
+    for ( i = 0; i < 2; i++ )
         pd[i] = (unsigned long)pt + (i<<12) + 3;
-    for ( i = 0; i < (4*1024); i++ )
+    for ( i = 0; i < 2 * 1024; i++ )
         pt[i] = (i << 12) + 3;
 
     /* Page at virtual 6MB maps to physical 5MB. */
@@ -112,7 +116,7 @@ static int rep_io_test(void)
     stop_paging();
 
     i = 0;
-    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
+    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
     {
         uint32_t expected = 0;
         if ( check[i].addr == (unsigned long)p )
@@ -144,12 +148,12 @@ static int shadow_gs_test(void)
     if ( !(edx & (1u<<29)) )
         return TEST_SKIP;
 
-    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
+    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
     *pd = (unsigned long)pd + 0x1007; /* Level 4 */
     pd += 512;
     *pd = (unsigned long)pd + 0x1007; /* Level 3 */
     pd += 512;
-    for ( i = 0; i < 8; i++ )         /* Level 2 */
+    for ( i = 0; i < 4; i++ )         /* Level 2 */
         *pd++ = (i << 21) + 0x1e3;
 
     asm volatile (
@@ -191,8 +195,7 @@ static int shadow_gs_test(void)
 
 void perform_tests(void)
 {
-    int i, passed, skipped;
-
+    unsigned int i, passed, skipped;
     static struct {
         int (* const test)(void);
         const char *description;
@@ -204,12 +207,73 @@ void perform_tests(void)
 
     printf("Testing HVM environment:\n");
 
-    if ( hvm_info->low_mem_pgend < 0x1000 )
+    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
+    if ( hvm_info->low_mem_pgend <
+         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
+    {
+        printf("Skipping tests due to insufficient memory (<%luMB)\n",
+               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
+        return;
+    }
+
+    if ( hvm_start_info->cmdline_paddr &&
+         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+         ((hvm_start_info->cmdline_paddr +
+           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
+          TEST_MEM_BASE) )
     {
-        printf("Skipping tests due to insufficient memory (<16MB)\n");
+        printf("Skipping tests due to overlap with command line\n");
         return;
     }
 
+    if ( hvm_start_info->rsdp_paddr )
+    {
+        printf("Skipping tests due to non-zero RSDP address\n");
+        return;
+    }
+
+    if ( hvm_start_info->nr_modules )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
+             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
+              hvm_start_info->nr_modules) )
+        {
+            printf("Skipping tests due to inaccessible module list\n");
+            return;
+        }
+
+        if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
+             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
+        {
+            printf("Skipping tests due to overlap with module list\n");
+            return;
+        }
+
+        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
+        {
+            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
+                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
+            {
+                printf("Skipping tests due to overlap with module %u\n", i);
+                return;
+            }
+
+            if ( modlist[i].cmdline_paddr &&
+                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+                 ((modlist[i].cmdline_paddr +
+                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
+                  TEST_MEM_BASE) )
+            {
+                printf("Skipping tests due to overlap with module %u cmdline\n",
+                       i);
+                return;
+            }
+        }
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {

Comments

Andrew Cooper May 12, 2017, 10:11 a.m. UTC | #1
On 12/05/17 10:32, Jan Beulich wrote:
>>>> On 11.05.17 at 11:02, <GLin@suse.com> wrote:
>> On Thu, May 11, 2017 at 06:14:42PM +1000, Jan Beulich wrote:
>>> Note that hvmloader's main() has
>>>
>>>     BUG_ON(hvm_start_info->magic != XEN_HVM_START_MAGIC_VALUE);
>>>
>>> very early, so you having got past this means the corruption
>>> occurred inside hvmloader (or at least while it was already
>>> running). Could you comment out the call to perform_tests()
>>> and try again?
>>>
>> You got it. After commenting out perform_tests(), the grub2 menu showed
>> and the system booted.
>>
>> It seems that perform_tests() cleared 0x400000~0x800000, and that's why 
>> the members of hvm_start_info became 0 in my test.
> So could you give the below/attached patch a try?
>
> Jan
>
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c

In the presence of XTF regression tests being run in OSSTest, this
entire mechanism in HVMLoader ought to be dropped.

~Andrew
Jan Beulich May 12, 2017, 12:26 p.m. UTC | #2
>>> On 12.05.17 at 12:11, <andrew.cooper3@citrix.com> wrote:
> On 12/05/17 10:32, Jan Beulich wrote:
>> --- a/tools/firmware/hvmloader/tests.c
>> +++ b/tools/firmware/hvmloader/tests.c
> 
> In the presence of XTF regression tests being run in OSSTest, this
> entire mechanism in HVMLoader ought to be dropped.

Well, maybe, but rather not at this point of the release cycle. Even
after 4.9 was branched I'm not entirely convinced that these aren't
at least reasonable as immediate smoke tests.

Jan
diff mbox

Patch

--- a/tools/firmware/hvmloader/tests.c
+++ b/tools/firmware/hvmloader/tests.c
@@ -19,7 +19,9 @@ 
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "util.h"
+#include <xen/arch-x86/hvm/start_info.h>
 
 #define TEST_FAIL 0
 #define TEST_PASS 1
@@ -29,10 +31,12 @@ 
  * Memory layout during tests:
  *  4MB to 8MB is cleared.
  *  Page directory resides at 8MB.
- *  4 page table pages reside at 8MB+4kB to 8MB+20kB.
- *  Pagetables identity-map 0-16MB, except 4kB at va 6MB maps to pa 5MB.
+ *  2 page table pages reside at 4MB+4kB to 4MB+12kB.
+ *  Pagetables identity-map 0-8MB, except 4kB at va 6MB maps to pa 5MB.
  */
-#define PD_START (8ul << 20)
+#define TEST_MEM_BASE (4ul << 20)
+#define TEST_MEM_SIZE (4ul << 20)
+#define PD_START TEST_MEM_BASE
 #define PT_START (PD_START + 4096)
 
 static void setup_paging(void)
@@ -41,10 +45,10 @@  static void setup_paging(void)
     uint32_t *pt = (uint32_t *)PT_START;
     uint32_t i;
 
-    /* Identity map 0-16MB. */
-    for ( i = 0; i < 4; i++ )
+    /* Identity map 0-8MB. */
+    for ( i = 0; i < 2; i++ )
         pd[i] = (unsigned long)pt + (i<<12) + 3;
-    for ( i = 0; i < (4*1024); i++ )
+    for ( i = 0; i < 2 * 1024; i++ )
         pt[i] = (i << 12) + 3;
 
     /* Page at virtual 6MB maps to physical 5MB. */
@@ -112,7 +116,7 @@  static int rep_io_test(void)
     stop_paging();
 
     i = 0;
-    for ( p = (uint32_t *)0x400000ul; p < (uint32_t *)0x700000ul; p++ )
+    for ( p = (uint32_t *)0x4ff000ul; p < (uint32_t *)0x602000ul; p++ )
     {
         uint32_t expected = 0;
         if ( check[i].addr == (unsigned long)p )
@@ -144,12 +148,12 @@  static int shadow_gs_test(void)
     if ( !(edx & (1u<<29)) )
         return TEST_SKIP;
 
-    /* Long mode pagetable setup: Identity map 0-16MB with 2MB mappings. */
+    /* Long mode pagetable setup: Identity map 0-8MB with 2MB mappings. */
     *pd = (unsigned long)pd + 0x1007; /* Level 4 */
     pd += 512;
     *pd = (unsigned long)pd + 0x1007; /* Level 3 */
     pd += 512;
-    for ( i = 0; i < 8; i++ )         /* Level 2 */
+    for ( i = 0; i < 4; i++ )         /* Level 2 */
         *pd++ = (i << 21) + 0x1e3;
 
     asm volatile (
@@ -191,8 +195,7 @@  static int shadow_gs_test(void)
 
 void perform_tests(void)
 {
-    int i, passed, skipped;
-
+    unsigned int i, passed, skipped;
     static struct {
         int (* const test)(void);
         const char *description;
@@ -204,12 +207,73 @@  void perform_tests(void)
 
     printf("Testing HVM environment:\n");
 
-    if ( hvm_info->low_mem_pgend < 0x1000 )
+    BUILD_BUG_ON(SCRATCH_PHYSICAL_ADDRESS > HVMLOADER_PHYSICAL_ADDRESS);
+    if ( hvm_info->low_mem_pgend <
+         ((TEST_MEM_BASE + TEST_MEM_SIZE) >> PAGE_SHIFT) )
+    {
+        printf("Skipping tests due to insufficient memory (<%luMB)\n",
+               (TEST_MEM_BASE + TEST_MEM_SIZE) >> 20);
+        return;
+    }
+
+    if ( hvm_start_info->cmdline_paddr &&
+         hvm_start_info->cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+         ((hvm_start_info->cmdline_paddr +
+           strlen((char *)(uintptr_t)hvm_start_info->cmdline_paddr)) >=
+          TEST_MEM_BASE) )
     {
-        printf("Skipping tests due to insufficient memory (<16MB)\n");
+        printf("Skipping tests due to overlap with command line\n");
         return;
     }
 
+    if ( hvm_start_info->rsdp_paddr )
+    {
+        printf("Skipping tests due to non-zero RSDP address\n");
+        return;
+    }
+
+    if ( hvm_start_info->nr_modules )
+    {
+        const struct hvm_modlist_entry *modlist =
+            (void *)(uintptr_t)hvm_start_info->modlist_paddr;
+
+        if ( hvm_start_info->modlist_paddr > UINTPTR_MAX ||
+             ((UINTPTR_MAX - (uintptr_t)modlist) / sizeof(*modlist) <
+              hvm_start_info->nr_modules) )
+        {
+            printf("Skipping tests due to inaccessible module list\n");
+            return;
+        }
+
+        if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) &&
+             (uintptr_t)modlist < TEST_MEM_BASE + TEST_MEM_SIZE )
+        {
+            printf("Skipping tests due to overlap with module list\n");
+            return;
+        }
+
+        for ( i = 0; i < hvm_start_info->nr_modules; ++i )
+        {
+            if ( TEST_MEM_BASE < modlist[i].paddr + modlist[i].size &&
+                 modlist[i].paddr < TEST_MEM_BASE + TEST_MEM_SIZE )
+            {
+                printf("Skipping tests due to overlap with module %u\n", i);
+                return;
+            }
+
+            if ( modlist[i].cmdline_paddr &&
+                 modlist[i].cmdline_paddr < TEST_MEM_BASE + TEST_MEM_SIZE &&
+                 ((modlist[i].cmdline_paddr +
+                   strlen((char *)(uintptr_t)modlist[i].cmdline_paddr)) >=
+                  TEST_MEM_BASE) )
+            {
+                printf("Skipping tests due to overlap with module %u cmdline\n",
+                       i);
+                return;
+            }
+        }
+    }
+
     passed = skipped = 0;
     for ( i = 0; tests[i].test; i++ )
     {