On 4/9/26 07:11, Andrew Cooper wrote:
On 09/04/2026 12:01 pm, Jan Beulich wrote:
On 09.04.2026 12:38, Andrew Cooper wrote:
Use IS_ENABLED() rather than #ifdef to give the compiler visibility into the
block, which in turn removes the #ifdef from the varaible block.
Just to mention, if it was just / mainly ...
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1335,9 +1335,7 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle,
EFI_SYSTEM_TABLE *Syste
EFI_STATUS status;
UINTN info_size = 0, map_key;
bool retry;
-#ifdef CONFIG_EFI_SET_VIRTUAL_ADDRESS_MAP
unsigned int i;
-#endif
... this to be got rid of, we could as well use ...
@@ -1371,31 +1369,32 @@ 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_EFI_SET_VIRTUAL_ADDRESS_MAP
- for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
for ( unsigned int i = 0; i < efi_memmap_size; i += efi_mdesc_size )
now. But yes, the typo aspect you mention can be avoided altogether by what
you change things to.
I originally had this change in the patch, but it interferes with diff
showing (just) an indentation change.
I'm not fussed either way.
+ if ( IS_ENABLED(CONFIG_EFI_SET_VIRTUAL_ADDRESS_MAP) )
{
- EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
+ for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
+ {
+ EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
- /*
- * Runtime services regions are always mapped here.
- * Attributes may be adjusted in efi_init_memory().
- */
- if ( (desc->Attribute & EFI_MEMORY_RUNTIME) ||
- desc->Type == EfiRuntimeServicesCode ||
- desc->Type == EfiRuntimeServicesData )
- 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);
+ /*
+ * Runtime services regions are always mapped here.
+ * Attributes may be adjusted in efi_init_memory().
+ */
+ if ( (desc->Attribute & EFI_MEMORY_RUNTIME) ||
+ desc->Type == EfiRuntimeServicesCode ||
+ desc->Type == EfiRuntimeServicesData )
+ 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);
Could I talk you into switching to
printk(XENLOG_ERR
"EFI: SetVirtualAddressMap() failed (%#lx), disabling runtime
services\n",
status);
to make the line at least a little less long?
Ok, but I'm not going to resend just for that.
~Andrew
I'm good with fix up on commit.
Acked-by: Daniel P. Smith <[email protected]>
