Re: [PATCH v3 2/3] xen/x86: use trampoline e820 buffer for BIOS interface only

[Juergen Gross <jgross@suse.com>]

On 24/03/17 12:24, Jan Beulich wrote:
>>>> On 24.03.17 at 08:48, <jgross@suse.com> wrote:
>> Instead of using the E820 raw buffer for BIOS, EFI and multiboot based
>> memory map information use it for the BIOS interface only. This will
>> enable us to support more E820 entries than the limited trampoline
>> located buffer can.
>>
>> Add a new raw e820 table for common purpose and copy the BIOS buffer
>> to it. Doing the copying in assembly avoids the need to export the
>> symbols for the BIOS E820 buffer and number of entries.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one question:
> 
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -636,7 +636,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>  {
>>      char *memmap_type = NULL;
>>      char *cmdline, *kextra, *loader;
>> -    unsigned int initrdidx, domcr_flags = DOMCRF_s3_integrity;
>> +    unsigned int nr_e820, initrdidx, domcr_flags = DOMCRF_s3_integrity;
> 
> Is this variable really needed?
> 
>> @@ -782,14 +782,17 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>      }
>>      else if ( efi_enabled(EFI_BOOT) )
>>          memmap_type = "EFI";
>> -    else if ( e820_raw_nr != 0 )
>> +    else if ( (nr_e820 = copy_bios_e820(e820_raw.map,
>> +                                        ARRAY_SIZE(e820_raw.map))) != 0 )
>>      {
>>          memmap_type = "Xen-e820";
>> +        e820_raw.nr_map = nr_e820;
>>      }
> 
> I.e. can't we store the result of the function call in e820_raw.nr_map
> right away? (The change could of course be done while committing.)

You are right, the variable can be omitted.


Juergen


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