| Message ID | 1456412174-20162-9-git-send-email-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 25.02.16 at 15:56, <anthony.perard@citrix.com> wrote: > --- a/tools/firmware/hvmloader/hvmloader.c > +++ b/tools/firmware/hvmloader/hvmloader.c > @@ -254,10 +254,32 @@ static void acpi_enable_sci(void) > BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN)); > } > > +const struct hvm_modlist_entry *get_module_entry( > + const struct hvm_start_info *info, > + const char *name) > +{ > + const struct hvm_modlist_entry *modlist = > + (struct hvm_modlist_entry *)info->modlist_paddr; > + int i; unsigned int > + for ( i = 0; i < info->nr_modules; i++ ) > + { > + uint32_t module_name = modlist[i].cmdline_paddr; The right side is uint64_t, so you may be truncating here. Also blank line between declaration(s) and statement(s) please. > @@ -293,8 +315,16 @@ int main(void) > } > > printf("Loading %s ...\n", bios->name); > - if ( bios->bios_load ) > - bios->bios_load(bios); > + bios_module = get_module_entry(hvm_start_info, "bios"); > + if ( bios_module && bios->bios_load ) > + { > + uint32_t paddr = bios_module->paddr; > + bios->bios_load(bios, (void*)paddr, bios_module->size); bios_module->size is a 64-bit quantity too, so would also need checking. > + } > + else if ( bios->bios_load ) > + { > + bios->bios_load(bios, 0, 0); > + } > else > { > BUG_ON(bios->bios_address + bios->image_size > Is all of the remaining code (from the first "else if" above) intended to go away eventually? Jan
On Mon, Feb 29, 2016 at 09:56:48AM -0700, Jan Beulich wrote: > >>> On 25.02.16 at 15:56, <anthony.perard@citrix.com> wrote: > > --- a/tools/firmware/hvmloader/hvmloader.c > > +++ b/tools/firmware/hvmloader/hvmloader.c > > @@ -254,10 +254,32 @@ static void acpi_enable_sci(void) > > BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN)); > > } > > > > +const struct hvm_modlist_entry *get_module_entry( > > + const struct hvm_start_info *info, > > + const char *name) > > +{ > > + const struct hvm_modlist_entry *modlist = > > + (struct hvm_modlist_entry *)info->modlist_paddr; > > + int i; > > unsigned int ok > > + for ( i = 0; i < info->nr_modules; i++ ) > > + { > > + uint32_t module_name = modlist[i].cmdline_paddr; > > The right side is uint64_t, so you may be truncating here. Yes, I need to check that. > Also blank line between declaration(s) and statement(s) please. ok. > > @@ -293,8 +315,16 @@ int main(void) > > } > > > > printf("Loading %s ...\n", bios->name); > > - if ( bios->bios_load ) > > - bios->bios_load(bios); > > + bios_module = get_module_entry(hvm_start_info, "bios"); > > + if ( bios_module && bios->bios_load ) > > + { > > + uint32_t paddr = bios_module->paddr; > > + bios->bios_load(bios, (void*)paddr, bios_module->size); > > bios_module->size is a 64-bit quantity too, so would also need > checking. Yes. > > + } > > + else if ( bios->bios_load ) > > + { > > + bios->bios_load(bios, 0, 0); > > + } > > else > > { > > BUG_ON(bios->bios_address + bios->image_size > > > Is all of the remaining code (from the first "else if" above) intended > to go away eventually? The "else if" would be kept for the benefit of rombios (if it's compiled in), and the last "else" will be turned into a BUG.
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h index b838cf9..4c6d8ad 100644 --- a/tools/firmware/hvmloader/config.h +++ b/tools/firmware/hvmloader/config.h @@ -22,7 +22,7 @@ struct bios_config { /* ROMS */ void (*load_roms)(void); - void (*bios_load)(const struct bios_config *config); + void (*bios_load)(const struct bios_config *config, void *addr, uint32_t size); void (*bios_info_setup)(void); void (*bios_info_finish)(void); diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c index 20ec8dc..d319de0 100644 --- a/tools/firmware/hvmloader/hvmloader.c +++ b/tools/firmware/hvmloader/hvmloader.c @@ -254,10 +254,32 @@ static void acpi_enable_sci(void) BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN)); } +const struct hvm_modlist_entry *get_module_entry( + const struct hvm_start_info *info, + const char *name) +{ + const struct hvm_modlist_entry *modlist = + (struct hvm_modlist_entry *)info->modlist_paddr; + int i; + + for ( i = 0; i < info->nr_modules; i++ ) + { + uint32_t module_name = modlist[i].cmdline_paddr; + if ( !strcmp(name, (char*)module_name) ) + { + BUG_ON(modlist[i].paddr > UINT_MAX); + return &modlist[i]; + } + } + + return NULL; +} + int main(void) { const struct bios_config *bios; int acpi_enabled; + const struct hvm_modlist_entry *bios_module; /* Initialise hypercall stubs with RET, rendering them no-ops. */ memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, PAGE_SIZE); @@ -293,8 +315,16 @@ int main(void) } printf("Loading %s ...\n", bios->name); - if ( bios->bios_load ) - bios->bios_load(bios); + bios_module = get_module_entry(hvm_start_info, "bios"); + if ( bios_module && bios->bios_load ) + { + uint32_t paddr = bios_module->paddr; + bios->bios_load(bios, (void*)paddr, bios_module->size); + } + else if ( bios->bios_load ) + { + bios->bios_load(bios, 0, 0); + } else { BUG_ON(bios->bios_address + bios->image_size > diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c index db9fa7a..858a2d4 100644 --- a/tools/firmware/hvmloader/ovmf.c +++ b/tools/firmware/hvmloader/ovmf.c @@ -93,7 +93,8 @@ static void ovmf_finish_bios_info(void) info->checksum = -checksum; } -static void ovmf_load(const struct bios_config *config) +static void ovmf_load(const struct bios_config *config, + void *bios_addr, uint32_t bios_length) { xen_pfn_t mfn; uint64_t addr = OVMF_BEGIN; diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c index 1f15b94..2ded844 100644 --- a/tools/firmware/hvmloader/rombios.c +++ b/tools/firmware/hvmloader/rombios.c @@ -121,7 +121,8 @@ static void rombios_load_roms(void) option_rom_phys_addr + option_rom_sz - 1); } -static void rombios_load(const struct bios_config *config) +static void rombios_load(const struct bios_config *config, + void *unused_addr, uint32_t unused_size) { uint32_t bioshigh; struct rombios_info *info; diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h index 132d915..d45c107 100644 --- a/tools/firmware/hvmloader/util.h +++ b/tools/firmware/hvmloader/util.h @@ -25,6 +25,8 @@ #undef NULL #define NULL ((void*)0) +#define UINT_MAX (~0U) + void __assert_failed(char *assertion, char *file, int line) __attribute__((noreturn)); #define ASSERT(p) \
The BIOS can be found an entry called "bios" of the modlist of the hvm_start_info struct. The found BIOS blob is not loaded by this patch, but only passed as argument to bios_load() function. It is going to be used by the next few patches. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Changes in V3: - fix some codying style - use module.cmdline to look for a module name instead of the main cmdline from hvm_start_info. --- tools/firmware/hvmloader/config.h | 2 +- tools/firmware/hvmloader/hvmloader.c | 34 ++++++++++++++++++++++++++++++++-- tools/firmware/hvmloader/ovmf.c | 3 ++- tools/firmware/hvmloader/rombios.c | 3 ++- tools/firmware/hvmloader/util.h | 2 ++ 5 files changed, 39 insertions(+), 5 deletions(-)