[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH-4.16 v3] xen/efi: Fix Grub2 boot on arm64


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Luca Fancellu <luca.fancellu@xxxxxxx>
  • Date: Fri, 5 Nov 2021 13:07:04 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ZhDtJbfn4DvtUyAyZ93g4S+X0I+6c+d11iSdcoIXeSQ=; b=SAM04hFCKUgDv6PHZtA+Gk5WFryfY5s27S4AMZohHU/lOW8BsAo+M6fYr6WupTjDQ9jLB1+cLt8BxtoU8LZY0n3x0mamybF2diWcHymYAE/X5FmvZRI2cgdcddqzbRVYQV9w9PvfaqAOtPbqVriDRI6BiCh9I+n9nzW4MdWW8dJ/AI4y13JbpUTTKjcjA6xhQe9QHbZMJWjuN6CfEjezhMg5z1DyedXRlhq6THsohPIjEwgyUtf3VWF21/PGGLONZSZsIY6QRnO4Z1D8dEtHf3th4oDAeuMMGSrhu5qxMxqWWvzdOD1DV3ARdMQzZ7+WW+COVlb8kdoci/hk4ASfDQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iaODJGSMSGnu/pga5TcXR8CQ37eH+PP+4E7mhappLvUzrRiSefl9+PNRE3Q1oKvBi5VX1eu7Or/P0WJbvSA99dNxEx3YSHB181HkcABsrj5RTJ3+c3dSosW7/amVFkRVIKdY+em3kdHHHBm1AlVVZhdsa25Ht0Pwisr9SVHHu39ZXWua7hjOx+ALhYqpzhvinF9g4tIElLFDbtnVX+r9y2H+XU2Bk8HdEs4KdhfaU1uSNTLGnPh24mY2tUoro2HnZbQRaOkqnelEB1yrNt2paQk00RcmHjzptiq93f0R+6a+4hplsi7p9KS0vG6QwAGeDweLTbt3B+qm7MWTW/WmkQ==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>, wei.chen@xxxxxxx, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 05 Nov 2021 13:07:48 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;


> On 5 Nov 2021, at 12:20, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 05.11.2021 12:21, Luca Fancellu wrote:
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -121,6 +121,8 @@ static char *get_value(const struct file *cfg, const 
>> char *section,
>> static char *split_string(char *s);
>> static CHAR16 *s2w(union string *str);
>> static char *w2s(const union string *str);
>> +static EFI_FILE_HANDLE get_parent_handle(EFI_LOADED_IMAGE *loaded_image,
>> +                                         CHAR16 **leaf);
>> static bool read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>                       struct file *file, const char *options);
>> static bool read_section(const EFI_LOADED_IMAGE *image, const CHAR16 *name,
>> @@ -167,7 +169,7 @@ static void __init PrintErr(const CHAR16 *s)
>> }
>> 
>> #ifndef CONFIG_HAS_DEVICE_TREE
>> -static int __init efi_check_dt_boot(EFI_FILE_HANDLE dir_handle)
>> +static int __init efi_check_dt_boot(EFI_LOADED_IMAGE *loaded_image)
>> {
>>     return 0;
>> }
>> @@ -1225,9 +1227,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>> 
>>     efi_arch_relocate_image(0);
>> 
>> -    /* Get the file system interface. */
>> -    dir_handle = get_parent_handle(loaded_image, &file_name);
>> -
>>     if ( use_cfg_file )
>>     {
>>         UINTN depth, cols, rows, size;
> 
> With the dir_handle declaration also moved back here (as I did
> indicated in reply to Stefano's proposal), ...

Ops.. sorry forgot that, will fix it

> 
>> @@ -1240,6 +1239,9 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>> 
>>         gop = efi_get_gop();
>> 
>> +        /* Get the file system interface. */
>> +        dir_handle = get_parent_handle(loaded_image, &file_name);
>> +
>>         /* Read and parse the config file. */
>>         if ( read_section(loaded_image, L"config", &cfg, NULL) )
>>             PrintStr(L"Using builtin config file\r\n");
>> @@ -1362,14 +1364,14 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>> *SystemTable)
>>         efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>>         cfg.addr = 0;
>> 
>> +        dir_handle->Close(dir_handle);
>> +
>>         if ( gop && !base_video )
>>             gop_mode = efi_find_gop_mode(gop, cols, rows, depth);
>>     }
>> 
>>     /* Get the number of boot modules specified on the DT or an error (<0) */
>> -    dt_modules_found = efi_check_dt_boot(dir_handle);
>> -
>> -    dir_handle->Close(dir_handle);
>> +    dt_modules_found = efi_check_dt_boot(loaded_image);
>> 
>>     if ( dt_modules_found < 0 )
>>         /* efi_check_dt_boot throws some error */
>> 
> 
> ... all of the quoted part
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thank you for your time, will push very soon v4 

> 
> Down the road we will want to constify efi_check_dt_boot()'s parameter,
> but that will require changes elsewhere as well.
> 
> Jan
> 




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.