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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.10.2019 00:11, Marek Marczykowski-Górecki  wrote:
> Some UEFI implementations are not happy about running in 1:1 addressing,
> but really virtual address space.

I have to admit that I find this misleading. There's no true "physical
mode" on x86-64 anyway. What I assume happens is that people abuse the
address map change notification to do things beyond the necessary
ConvertPointer(() calls.

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -88,6 +88,19 @@ config KEXEC
>  
>         If unsure, say Y.
>  
> +config SET_VIRTUAL_ADDRESS_MAP

I'm of the strong opinion that this wants to have an EFI_ prefix.

> +    bool "EFI: call SetVirtualAddressMap()" if EXPERT = "y"
> +    default n

I don't think you need this line.

> @@ -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. I can accept this as an intended tradeoff (as
you validly mention in the other patch we don't honor the 1:1 map
requirement at the time of the call with its original placement),
but it should be mentioned in one of the two patch descriptions.

Jan

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