>>> On 21.01.19 at 16:37, <andrew.coop...@citrix.com> wrote: > --- a/xen/arch/x86/guest/pvh-boot.c > +++ b/xen/arch/x86/guest/pvh-boot.c > @@ -38,12 +38,20 @@ static const char *__initdata pvh_loader = "PVH > Directboot"; > static void __init convert_pvh_info(multiboot_info_t **mbi, > module_t **mod) > { > - const struct hvm_start_info *pvh_info = __va(pvh_start_info_pa); > + struct hvm_start_info *pvh_info = __va(pvh_start_info_pa); > const struct hvm_modlist_entry *entry; > unsigned int i; > > if ( pvh_info->magic != XEN_HVM_START_MAGIC_VALUE ) > - panic("Magic value is wrong: %x\n", pvh_info->magic); > + panic("PVH magic value is wrong: %x\n", pvh_info->magic); > + > + /* Check consistency between the modlist and number of modules. */ > + if ( (pvh_info->modlist_paddr == 0) != (pvh_info->nr_modules == 0) ) > + { > + printk("PVH module mismatch: pa %08"PRIx64", nr %u - Ignoring\n", > + pvh_info->modlist_paddr, pvh_info->nr_modules); > + pvh_info->modlist_paddr = pvh_info->nr_modules = 0; > + }
While we don't consume memmap_{paddr,entries} (yet), wouldn't it make sense to also check those for similar consistency? Furthermore I'm not convinced the check above is correct: I don't see anything wrong with a random modlist_paddr as long as nr_modules is zero. In particular it is not uncommon for placement implementations to assign the next sequential address to the next item to process before looking at or iterating over the number of associated entries. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel