Re: [Xen-devel] [PATCH 4/6] x86/boot: Clean up l?_bootmap[] construction

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/01/2020 16:30, Jan Beulich wrote:
>>>      for ( i = 0; i < 8; ++i )
>>>      {
>>>          unsigned int slot = (xen_phys_start >> L2_PAGETABLE_SHIFT) + i;
>>>          paddr_t addr = slot << L2_PAGETABLE_SHIFT;
>>>  
>>>          l2_identmap[slot] = l2e_from_paddr(addr, 
>>> PAGE_HYPERVISOR|_PAGE_PSE);
>>> -        slot &= L2_PAGETABLE_ENTRIES - 1;
>>>          l2_bootmap[slot] = l2e_from_paddr(addr, 
>>> __PAGE_HYPERVISOR|_PAGE_PSE);
>>>      }
>>> -    /* Initialise L3 boot-map page directory entries. */
>>> -    l3_bootmap[l3_table_offset(xen_phys_start)] =
>>> -        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
>>> -    l3_bootmap[l3_table_offset(xen_phys_start + (8 << L2_PAGETABLE_SHIFT) 
>>> - 1)] =
>>> -        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
>>> +
>>> +    /* Initialize L3 boot-map page directory entries. */
>>> +    for ( i = 0; i < 4; ++i )
>>> +        l3_bootmap[i] = l3e_from_paddr((UINTN)l2_bootmap + i * PAGE_SIZE,
>>> +                                       __PAGE_HYPERVISOR);
>> The idea behind the original code was to be immune to the number
>> of pages l2_bootmap[] covers, as long as it's at least one (which
>> it'll always be, I would say). The minimum requirement to any
>> change to this I have is that the build must break if the size
>> assumption here is violated. I.e. there may not be a literal 4 as
>> the upper loop bound here, or there would need to be a
>> BUILD_BUG_ON() right next to it. But I'd really prefer if the
>> code was left as is (perhaps with a comment added), unless you
>> can point out actual issues with it (which I can't see in the
>> description), or you can otherwise justify the change with better
>> than "the EFI side is further complicated by spraying non-identity
>> aliases into the mix."
> And if this change is to be made, won't it mean the code in setup.c
> commented with "Make boot page tables match non-EFI boot" can then
> go away at the same time?

When I've figured out why altering that causes the EFI boot to fail, yes
- that was the plan...

~Andrew

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