Hi all,
On 16/04/2023 15:32, Julien Grall wrote:
From: Julien Grall <jgr...@amazon.com>
At the moment, the temporary mapping is only used when the virtual
runtime region of Xen is clashing with the physical region.
In follow-up patches, we will rework how secondary CPU bring-up works
and it will be convenient to use the fixmap area for accessing
the root page-table (it is per-cpu).
Rework the code to use temporary mapping when the Xen physical address
is not overlapping with the temporary mapping.
This also has the advantage to simplify the logic to identity map
Xen.
I had to revert this patch a couple of months ago because Xen was not
booting anymore on the Arndale. I finally managed to figure out what was
the issue. It was an interesting read through the Arm Arm.
In switch_to_runtime_mapping we have the following code:
flush_xen_tlb_local r0
/* Map boot_second into boot_pgtable */
mov_w r0, XEN_VIRT_START
create_table_entry boot_pgtable, boot_second, r0, 1
/* Ensure any page table updates are visible before continuing */
dsb nsh
ready_to_switch:
mov pc, lr
Per Arm Arm (Armv7 ARM DDI 0406C.d, section A3-148):
"The DMB and DSB memory barriers affect reads and writes to the memory
system generated by load/store
instructions and data or unified cache maintenance operations being
executed by the processor. Instruction fetches
or accesses caused by a hardware translation table access are not
explicit accesses."
In the example above, 'lr' points to a region covered by the mapping we
created just above. As the 'dsb' doesn't affect instruction fetch, it
means it could happen before the page-table entry was observed and
therefore result to a translation fault.
There is another situation where this could happen (taken from Linux
commit d0b7a302d58a "Revert "arm64: Remove unnecessary ISBs from
set_{pte,pmd,pud}"):
MOV X0, <valid pte>
STR X0, [Xptep] // Store new PTE to page table
DSB ISHST
LDR X1, [X2] // Translates using the new PTE
The dsb needs to be followed by an isb otherwise, a translation fault
could occur.
There are a few places in where where the isb is missing. I think we
didn't notice it before because we don't often create a new PTE and then
directly access it.
Note that this issue is not in this patch but in fbd9b5fb4c26
("xen/arm32: head: Remove restriction where to load Xen"). We didn't
notice it because the temporary mapping wasn't much used before.
I will prepare a patch series to add the missing isb.
Cheers,
--
Julien Grall
