|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Is there any limitation on the firmware size in Xen?
On Fri, May 12, 2017 at 10:31:28PM +1000, Jan Beulich wrote: > >>> On 12.05.17 at 12:06, <GLin@xxxxxxxx> wrote: > > On Fri, May 12, 2017 at 07:32:49PM +1000, Jan Beulich wrote: > >> >>> On 11.05.17 at 11:02, <GLin@xxxxxxxx> 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? > >> > > It won't compile. > > > > tests.c: In function 'perform_tests': > > tests.c:248:50: error: 'i' may be used uninitialized in this function > > [-Werror=maybe-uninitialized] > > if ( TEST_MEM_BASE < (uintptr_t)(modlist + i) && > > ^ > > cc1: all warnings being treated as errors > > Oops - quite obviously. No idea why neither of the two gcc versions > I've built this with caught the issue. Below/attached a better one (also > with a few other changes). > This patch works for me :) Thanks, Gary Lin > Jan > > --- 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 > @@ -28,11 +30,13 @@ > /* > * 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. > + * Page directory resides at 4MB. > + * 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,80 @@ 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 ( (unsigned long)_end > TEST_MEM_BASE ) > + { > + printf("Skipping tests due to overlap with base image\n"); > + 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 overlap with command line\n"); > + return; > + } > + > + if ( hvm_start_info->rsdp_paddr ) > { > - printf("Skipping tests due to insufficient memory (<16MB)\n"); > + 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 + > + hvm_start_info->nr_modules) && > + (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++ ) > { > > > 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@xxxxxxxx> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > --- 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 > @@ -28,11 +30,13 @@ > /* > * 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. > + * Page directory resides at 4MB. > + * 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,80 @@ 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 ( (unsigned long)_end > TEST_MEM_BASE ) > + { > + printf("Skipping tests due to overlap with base image\n"); > + 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 overlap with command line\n"); > + return; > + } > + > + if ( hvm_start_info->rsdp_paddr ) > { > - printf("Skipping tests due to insufficient memory (<16MB)\n"); > + 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 + > + hvm_start_info->nr_modules) && > + (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++ ) > { _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |