Re: [Xen-devel] [PATCH v3 2/2] xen/efi: optionally call SetVirtualAddressMap()

[Jan Beulich <jbeulich@suse.com>]

On 23.10.2019 18:07, Marek Marczykowski-Górecki wrote:
> On Wed, Oct 23, 2019 at 05:37:33PM +0200, Jan Beulich wrote:
>> On 13.10.2019 00:11, Marek Marczykowski-Górecki  wrote:
>>> @@ -1094,6 +1100,26 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste
>>>      if ( EFI_ERROR(status) )
>>>          PrintErrMesg(L"Cannot exit boot services", status);
>>>  
>>> +#ifdef CONFIG_SET_VIRTUAL_ADDRESS_MAP
>>> +    for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
>>> +    {
>>> +        EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
>>> +
>>> +        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
>>> +            desc->VirtualStart = desc->PhysicalStart;
>>> +        else
>>> +            desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
>>> +    }
>>> +    status = efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
>>> +                                          mdesc_ver, efi_memmap);
>>> +    if ( status != EFI_SUCCESS )
>>> +    {
>>> +        printk(XENLOG_ERR "EFI: SetVirtualAddressMap() failed (%#lx), disabling runtime services\n",
>>> +               status);
>>> +        __clear_bit(EFI_RS, &efi_flags);
>>> +    }
>>> +#endif
>>
>> This new placement undermines (or at least complicates afaict) the
>> original intention to allow picking virtual addresses which don't
>> match the directmap.
> 
> If I read it right, the original intention was to specifically use
> directmap, not some other virtual addresses. Which is flawed, because
> directmap is mapped with NX, so at least EfiRuntimeServicesCode will
> break. This means, even when using directmap, Xen would need to switch
> page tables for the runtime call time to allow executing that code.

Just FYI: The NX-ifying post-dates the EFI work by several years.

> There is of course an option to rewrite it completely differently,
> mapping EFI runtime regions somewhere else (not 1:1 and not re-use
> directmap). But I don't think it worth the effort, and also is definitely
> too complex this far in 4.13 release cycle.

Especially on this last point - fully agree.

Jan

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