Re: [Xen-devel] [PATCH v3 1/2] efi: remove old SetVirtualAddressMap() arrangement

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.10.2019 17:36, Marek Marczykowski-Górecki wrote:
> On Wed, Oct 23, 2019 at 05:15:42PM +0200, Jan Beulich wrote:
>> On 13.10.2019 00:11, Marek Marczykowski-Górecki  wrote:
>>> @@ -1099,9 +1096,6 @@ static void __init efi_exit_boot(EFI_HANDLE 
>>> ImageHandle, EFI_SYSTEM_TABLE *Syste
>>>  
>>>      /* Adjust pointers into EFI. */
>>>      efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START;
>>> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
>>> -    efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START;
>>> -#endif
>>
>> This doesn't get re-instated in any way by patch 2. 
> 
> This commit remove dead code.
> 
>> How come you
>> get away without? 
> 
> The second patch doesn't just fix what was under #ifdef
> USE_SET_VIRTUAL_ADDRESS_MAP. It does a completely different approach to
> using SetVirtualAddressMap. See below.
> 
> On Wed, Oct 23, 2019 at 05:26:48PM +0200, Jan Beulich wrote:
>> On 13.10.2019 00:11, Marek Marczykowski-Górecki  wrote:
>>> @@ -1591,10 +1576,6 @@ void __init efi_init_memory(void)
>>>          return;
>>>      }
>>>  
>>> -#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
>>> -    efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
>>> -                                 mdesc_ver, efi_memmap);
>>> -#else
>>>      /* Set up 1:1 page tables to do runtime calls in "physical" mode. */
>>
>> This comment, btw, also wants either adjusting or removing.
> 
> No, it still setup 1:1 page tables for the runtime calls, exactly as it
> was before.

But the "physical" is no longer correct.

> This is also why I don't need to adjust efi_rs.

Well, you may not _need_ to with the current code structure, but I
wonder if you better would. In fact I wonder whether the #ifdef
around the line further up shouldn't have been removed already
(and hence that's what you want to do): Take the processing of
XEN_EFI_query_variable_info - it could do the version check
outside of the efi_rs_{enter,exit}() region if efi_rs was properly
relocated. Right now it's a requirement to make all accesses to
efi_rs within such regions.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel