On 12.06.2023 15:48, Jan Beulich wrote: > On 06.06.2023 21:55, Oleksii Kurochko wrote: >> -void __init noreturn noinline enable_mmu() >> +/* >> + * enable_mmu() can't be __init because __init section isn't part of >> identity >> + * mapping so it will cause an issue after MMU will be enabled. >> + */ > > As hinted at above already - perhaps the identity mapping wants to be > larger, up to covering the entire Xen image? Since it's temporary > only anyway, you could even consider using a large page (and RWX > permission). You already require no overlap of link and load addresses, > so at least small page mappings ought to be possible for the entire > image.
To expand on that: Assume a future change on this path results in a call to memcpy() or memset() being introduced by the compiler (and then let's further assume this only occurs for a specific compiler version). Right now such a case would be noticed simply because we don't build those library functions yet. But it'll likely be a perplexing crash once a full hypervisor can be built, the more that exception handlers also aren't mapped. >> - mmu_is_enabled: >> /* >> - * Stack should be re-inited as: >> - * 1. Right now an address of the stack is relative to load time >> - * addresses what will cause an issue in case of load start address >> - * isn't equal to linker start address. >> - * 2. Addresses in stack are all load time relative which can be an >> - * issue in case when load start address isn't equal to linker >> - * start address. >> - * >> - * We can't return to the caller because the stack was reseted >> - * and it may have stash some variable on the stack. >> - * Jump to a brand new function as the stack was reseted >> + * id_addrs should be in sync with id mapping in >> + * setup_initial_pagetables() > > What is "id" meant to stand for here? Also if things need keeping in > sync, then a similar comment should exist on the other side. I guess it's meant to stand for "identity mapping", but the common use of "id" makes we wonder if the variable wouldn't better be ident_addrs[]. Jan
