Re: [Xen-devel] [PATCH 2/3] x86/pvh: Fixes to convert_pvh_info()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.01.19 at 16:37, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel