Re: [Xen-devel] [PATCH v8 07/13] hvmloader: Locate the BIOS blob

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/08/16 16:39, Jan Beulich wrote:
>>>> On 18.08.16 at 16:13, <wei.liu2@xxxxxxxxxx> wrote:
>> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>>
>> The BIOS blob can be found an entry called "firmware" 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.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> provided Andrew confirms his concerns have got addressed and a
> few minor things get taken care of:

They haven't, but I am planning some fixes anyway, so I will adjust
things there.  They are not worth letting this series drag on even longer.

>
>> --- a/tools/firmware/hvmloader/hvmloader.c
>> +++ b/tools/firmware/hvmloader/hvmloader.c
>> @@ -266,10 +266,57 @@ 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 *)(uintptr_t)info->modlist_paddr;
>> +    unsigned int i;
>> +
>> +    if ( !modlist ||
>> +         info->modlist_paddr > UINTPTR_MAX ||
>> +         (info->modlist_paddr + info->nr_modules * sizeof(*modlist) - 1)
>> +            > UINTPTR_MAX
>> +         )
> Urgh?
>
>> +        return NULL;
>> +
>> +    for ( i = 0; i < info->nr_modules; i++ )
>> +    {
>> +        char *module_name = (char*)(uintptr_t)modlist[i].cmdline_paddr;
>> +
>> +        /* Skip if the module or its cmdline is missing. */
>> +        if ( !module_name || !modlist[i].paddr )
>> +            continue;
>> +
>> +        /* Skip if the cmdline can not be read. */
> cannot (afaik)
>
>> +        if ( modlist[i].cmdline_paddr > UINTPTR_MAX ||
>> +             (modlist[i].cmdline_paddr + strlen(name)) > UINTPTR_MAX )
>> +            continue;
>> +
>> +        if ( !strcmp(name, module_name) )
>> +        {
>> +            if ( modlist[i].paddr > UINTPTR_MAX ||
>> +                 modlist[i].size > UINTPTR_MAX ||
>> +                 (modlist[i].paddr + modlist[i].size - 1) > UINTPTR_MAX )
>> +            {
>> +                printf("Can not load \"%s\" from 0x"PRIllx" (0x"PRIllx")\n",
> Same here.

"can not" is valid, but it is very uncommon to use.  The double n is
awkward to pronounce, which is why "cannot" (with only a single n to
pronounce) is the more common form.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel