On Tue, 2007-07-31 at 13:35 +0200, Andi Kleen wrote:
> > static unsigned long dma_reserve __initdata;
> > +/* Flag indicating EFI runtime executable code area */
> > +static int efi_runtime_code_area;
>
> We don't normally use globals to modify function behaviour.
>
> >
> > DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
> >
> > @@ -199,7 +202,7 @@
> > static __meminit void unmap_low_page(void *adr)
> > {
> >
> > - if (after_bootmem)
> > + if (after_bootmem || efi_runtime_code_area)
> > return;
> >
> > early_iounmap(adr, PAGE_SIZE);
> > @@ -259,16 +262,21 @@
> > pmd_t *pmd = pmd_page + pmd_index(address);
> >
> > if (address >= end) {
> > - if (!after_bootmem)
> > + if (!after_bootmem && !efi_runtime_code_area)
>
>
> This one seems also weird. Are you sure this doesn't remove _NX from
> more than the intended area?
>
> > for (; i < PTRS_PER_PMD; i++, pmd++)
> > set_pmd(pmd, __pmd(0));
> > break;
> > }
> >
> > - if (pmd_val(*pmd))
> > + if (pmd_val(*pmd) && !efi_runtime_code_area)
> > continue;
> >
> > - entry = _PAGE_NX|_PAGE_PSE|_KERNPG_TABLE|_PAGE_GLOBAL|address;
> > + if (efi_runtime_code_area) {
> > + entry = pmd_val(*pmd);
> > + entry &= ~_PAGE_NX;
> > + } else
> > + entry = _PAGE_NX | _PAGE_PSE | _KERNPG_TABLE | \
> > + _PAGE_GLOBAL | address;
>
> This doesn't look correct. PSE/KERNPG/GLOBAL/address surely need to be set
> for EFI areas too.
>
> The changes to this file are quite messy.
> Perhaps it would be better if you just use change_page_attr() afterwards.
> This would make it using 4K pages instead of 2MB, but that wouldn't be a catastrophe.
The memory area mapping must be changed before time_init, where the
first EFI runtime serivce (efi_get_time) is called. But
"change_page_attr" can not be used there, because "alloc_pages" is used
by "change_page_attr".
Should I change "change_page_attr" to make it work before "mem_init"? Or
Should I change "init_memory_mapping" to make it can be used to change
mapping attributes? Which one is better?
> What exactly are you trying to do here? Just remove _NX for some areas?
Yes. I just want to remove _NX for some areas.
Best Regards,
Huang Ying
>
> Should I change "change_page_attr" to make it work before "mem_init"? Or
> Should I change "init_memory_mapping" to make it can be used to change
> mapping attributes? Which one is better?
It's probably better to change init_memory_mapping. Just do it cleanly
and correctly please. As in only change the pages that straddle the
EFI areas and don't use globals.
BTW i don't see any such code on i386. How does the EFI support
there handle it?
-Andi