Hi,
On 21/06/2023 10:33, Michal Orzel wrote:
On 19/06/2023 19:01, Julien Grall wrote:
From: Julien Grall <[email protected]>
On older version of the Arm Arm (ARM DDI 0487E.a, B2-125) there were
the following paragraph:
"DMB and DSB instructions affect reads and writes to the memory system
generated by Load/Store instructions and data or unified cache
maintenance instructions being executed by the PE. Instruction fetches
or accesses caused by a hardware translation table access are not
explicit accesses."
Newer revision (e.g. ARM DDI 0487J.a) doesn't have the second sentence
(it might be somewhere else in the Arm Arm). But the interpretation is
not much different.
In setup_fixmap(), we write the fixmap area and may be used soon after,
for instance, to write to the UART. IOW, there could be hardware
translation table access. So we need to ensure the 'dsb' has completed
before continuing. Therefore add an 'isb'.
Fixes: 2b11c3646105 ("xen/arm64: head: Remove 1:1 mapping as soon as it is not
used")
Signed-off-by: Julien Grall <[email protected]>
Reviewed-by: Michal Orzel <[email protected]>
I'm happy with the whole series but I do not see a point in flooding each patch
with my tag
since you already got two (from Henry and Luca).
Thanks. To clarify, shall I add it in each patch or only this one?
When it comes to essential isb() after dsb() in arm64 head.S, I can see that we
are missing one in enable_mmu()
after TLB invalidation. On HW without FEAT_ETS the TLB is "guaranteed to be
complete after the execution of
DSB by that PE, followed by a Context synchronization event", so I view isb as
necessary there.
While there is no ISB directly after DSB NSH, there are one right after
MSR. I don't think we need one before because nothing will use the TLBs
between before the ISB.
/*
* The state of the TLBs is unknown before turning on the MMU.
* Flush them to avoid stale one.
*/
tlbi alle2 /* Flush hypervisor TLBs */
dsb nsh
/* Write Xen's PT's paddr into TTBR0_EL2 */
load_paddr x0, boot_pgtable
msr TTBR0_EL2, x0
isb
We could also
introduce (just like for arm32) flush_xen_tlb_local macro and use it there +
remove opencoding it.
That would be good. But I don't think this is necessary here (see above).
Cheers,
--
Julien Grall
