Skip to content

Commit

Permalink
arm: don't do S1 translation in Arch_setMRs_fault
Browse files Browse the repository at this point in the history
For a VM fault in a hypervisor context, 32-bit Arm translated the IP
address into an IPA, while 64-bit Arm did not. The previous commit
made these consistent by performing the translation on both.

After investigation and discussion, the 32-bit Arm behaviour was
declared a bug: reporting an IPA (instead of a VA) to the VMM is not
very useful and can cause issues when the fault message is not sent
immediately (SELFOUR-1602). This commit, therefore, removes all stage 1
translation from Arch_setMRs_fault on Arm platforms.

Signed-off-by: Rafal Kolanski <[email protected]>
  • Loading branch information
Xaphiosis committed Jun 6, 2022
1 parent fce5cec commit b8ef00e
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 14 deletions.
4 changes: 3 additions & 1 deletion CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ description indicates whether it is SOURCE-COMPATIBLE, BINARY-COMPATIBLE, or BRE
Further information about [seL4 releases](https://docs.sel4.systems/sel4_release/) is available.

---
Upcoming release: BINARY COMPATIBLE
Upcoming release: BREAKING

## Changes

Expand All @@ -42,6 +42,8 @@ Upcoming release: BINARY COMPATIBLE
* Removed obsolete define `HAVE_AUTOCONF`
* Removed user address space reserved slots restriction on 40bit PA platforms when KernelArmHypervisorSupport is set.
This change is reflected in the definition of the seL4_UserTop constant that holds the largest user virtual address.
* aarch32 VM fault messages now deliver original (untranslated) faulting IP in a hypervisor context, matching
aarch64 behaviour.

## Upgrade Notes
---
Expand Down
5 changes: 0 additions & 5 deletions include/arch/arm/arch/64/mode/machine.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,5 @@ void arch_clean_invalidate_L1_caches(word_t type);

static inline paddr_t addressTranslateS1(vptr_t vaddr)
{
#ifdef CONFIG_ARM_HYPERVISOR_SUPPORT
return ats1e1r(vaddr);
#else
/* shouldn't be called on non-hyp, added for consistency with AArch32 */
return vaddr;
#endif
}
9 changes: 1 addition & 8 deletions src/arch/arm/api/faults.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,7 @@ word_t Arch_setMRs_fault(tcb_t *sender, tcb_t *receiver, word_t *receiveIPCBuffe
{
switch (faultType) {
case seL4_Fault_VMFault: {
if (config_set(CONFIG_ARM_HYPERVISOR_SUPPORT)) {
word_t ipa, va;
va = getRestartPC(sender);
ipa = (addressTranslateS1(va) & ~MASK(PAGE_BITS)) | (va & MASK(PAGE_BITS));
setMR(receiver, receiveIPCBuffer, seL4_VMFault_IP, ipa);
} else {
setMR(receiver, receiveIPCBuffer, seL4_VMFault_IP, getRestartPC(sender));
}
setMR(receiver, receiveIPCBuffer, seL4_VMFault_IP, getRestartPC(sender));
setMR(receiver, receiveIPCBuffer, seL4_VMFault_Addr,
seL4_Fault_VMFault_get_address(sender->tcbFault));
setMR(receiver, receiveIPCBuffer, seL4_VMFault_PrefetchFault,
Expand Down

0 comments on commit b8ef00e

Please sign in to comment.