| Message ID | 1457978150-27201-10-git-send-email-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 14, 2016 at 05:55:44PM +0000, Anthony PERARD wrote: > As perform_tests() is going to clear memory past 4MB, we check that the > memory can be use or we skip the tests. I get the reason you want this - but if we have giant binary blobs and something else screws up what the tests are testing for- we don't have a regression test anymore. .. Oh my looking at rep_io_test and trying to make it more dynamic is tricky but should be managable? I presume that is the test that blows everything out of the sky? And also the placement of the page tables? > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > Changes in v4: > - move the check into the perform_test() function. > - skip tests instead of using BUG. > > New in V3 > --- > tools/firmware/hvmloader/tests.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/tools/firmware/hvmloader/tests.c b/tools/firmware/hvmloader/tests.c > index fea3ad3..7996206 100644 > --- a/tools/firmware/hvmloader/tests.c > +++ b/tools/firmware/hvmloader/tests.c > @@ -210,6 +210,26 @@ void perform_tests(void) > return; > } > > + /* Check that tests does not use memory where modules are stored */ > + if ( ((uint32_t)hvm_start_info + sizeof(struct hvm_start_info)) > 4 << 20 > + && (uint32_t)hvm_start_info < 8 << 20 ) > + { > + printf("Skipping tests due to memory used by hvm_start_info\n"); > + return; > + } > + for ( unsigned i = 0; i < hvm_start_info->nr_modules; i++ ) > + { > + const struct hvm_modlist_entry *modlist = > + (struct hvm_modlist_entry *)hvm_start_info->modlist_paddr; > + if ( modlist[i].paddr > + && modlist[i].paddr + modlist[i].size > 4ul << 20 > + && modlist[i].paddr < 8ul << 20 ) > + { > + printf("Skipping tests due to memory used by a module\n"); > + return; > + } > + } > + > passed = skipped = 0; > for ( i = 0; tests[i].test; i++ ) > { > -- > Anthony PERARD > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Tue, Mar 15, 2016 at 09:23:37PM -0400, Konrad Rzeszutek Wilk wrote: > On Mon, Mar 14, 2016 at 05:55:44PM +0000, Anthony PERARD wrote: > > As perform_tests() is going to clear memory past 4MB, we check that the > > memory can be use or we skip the tests. > > I get the reason you want this - but if we have giant binary blobs > and something else screws up what the tests are testing for- we > don't have a regression test anymore. > > .. Oh my looking at rep_io_test and trying to make it more dynamic > is tricky but should be managable? Maybe, it have been mention that the tests could be more dynamic. But they could also be moved to Andrew's Xen Test Framework. For now, the test are unlikely to be skipped. On my system, in the worse case, with rombios compiled in, and trying to boot with ovmf, the test are not skiped. So with seabios it should be fine for some time. Still, I'm not sure what to do about it. > I presume that is the test that blows everything out of the sky? Yes, the test. > And also the placement of the page tables? :(, I did not take the page tables into account will writing the check.
>>> On 14.03.16 at 18:55, <anthony.perard@citrix.com> wrote: > As perform_tests() is going to clear memory past 4MB, we check that the > memory can be use or we skip the tests. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > Changes in v4: > - move the check into the perform_test() function. > - skip tests instead of using BUG. > > New in V3 > --- > tools/firmware/hvmloader/tests.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/tools/firmware/hvmloader/tests.c > b/tools/firmware/hvmloader/tests.c > index fea3ad3..7996206 100644 > --- a/tools/firmware/hvmloader/tests.c > +++ b/tools/firmware/hvmloader/tests.c > @@ -210,6 +210,26 @@ void perform_tests(void) > return; > } > > + /* Check that tests does not use memory where modules are stored */ > + if ( ((uint32_t)hvm_start_info + sizeof(struct hvm_start_info)) > 4 << 20 if ( (uint32_t)(hvm_start_info + 1) > (4 << 20) > + && (uint32_t)hvm_start_info < 8 << 20 ) Missing parentheses around operands of <<. Also commonly the && would belong on the previous line. > + { > + printf("Skipping tests due to memory used by hvm_start_info\n"); > + return; > + } > + for ( unsigned i = 0; i < hvm_start_info->nr_modules; i++ ) Bogus shadowing of an outer scope variable. And we prefer not to use this C99 declaration style anyway. > + { > + const struct hvm_modlist_entry *modlist = > + (struct hvm_modlist_entry *)hvm_start_info->modlist_paddr; > + if ( modlist[i].paddr Blank line ... > + && modlist[i].paddr + modlist[i].size > 4ul << 20 > + && modlist[i].paddr < 8ul << 20 ) Parentheses ... Also, why the ul suffixed here but not above? Please be consistent. > + { > + printf("Skipping tests due to memory used by a module\n"); How about at least printing the module index as well? > + return; > + } > + } How about cmdline_paddr (both variant) and rsdp_paddr? Jan
diff --git a/tools/firmware/hvmloader/tests.c b/tools/firmware/hvmloader/tests.c index fea3ad3..7996206 100644 --- a/tools/firmware/hvmloader/tests.c +++ b/tools/firmware/hvmloader/tests.c @@ -210,6 +210,26 @@ void perform_tests(void) return; } + /* Check that tests does not use memory where modules are stored */ + if ( ((uint32_t)hvm_start_info + sizeof(struct hvm_start_info)) > 4 << 20 + && (uint32_t)hvm_start_info < 8 << 20 ) + { + printf("Skipping tests due to memory used by hvm_start_info\n"); + return; + } + for ( unsigned i = 0; i < hvm_start_info->nr_modules; i++ ) + { + const struct hvm_modlist_entry *modlist = + (struct hvm_modlist_entry *)hvm_start_info->modlist_paddr; + if ( modlist[i].paddr + && modlist[i].paddr + modlist[i].size > 4ul << 20 + && modlist[i].paddr < 8ul << 20 ) + { + printf("Skipping tests due to memory used by a module\n"); + return; + } + } + passed = skipped = 0; for ( i = 0; tests[i].test; i++ ) {
As perform_tests() is going to clear memory past 4MB, we check that the memory can be use or we skip the tests. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Changes in v4: - move the check into the perform_test() function. - skip tests instead of using BUG. New in V3 --- tools/firmware/hvmloader/tests.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)