Hello, On 2021-05-26 23:24, Axel Heider wrote:
GitHub offers a convenient platform here. So I wonder, what is preventing you from creating an account there and doing pull requests?
In the past I had DNS based ad/tracking filtering and that utterly broke the Github website. Since then I have always avoided Github and that's where my reluctance comes from. Nowadays there are decent in-browser plugins and it seems Github can stand on its own legs now, as it doesn't depend on external websites as much any more. Other than that I find the Github website not very pleasant as far as usability goes. On 2021-05-27 03:23, Gerwin Klein wrote:
It sounds like you might have already attached a patch to you message, but it didn't come through on the mailing list. This happened to me before as well -- I think the list is set up to strip attachments for security. If the GitHub option doesn't work for you, feel free to send me the patch directly and I'll raise a GitHub PR for you.
Indeed it got dropped silently. I'll just create a Github account, that is the easiest solution long-term.
For the Jira tracker, there should be an "Attach" button on the top left of the issue screen once an issue is created. That should allow you to attach a file. Does that button appear for you? (If not, there might be some configuration option we might be able to tweak)
I do not see such button. Or do only creators of an issue see it?
In terms of content: yes, halting the machine based on errors the user can trigger from user space is definitely not ideal, and we should do something about it. This case is at least somewhat constrained to users that have direct access to device memory, but if we can lock it down more, that would be good.
Further testing seems to indicate this only applies to writes to read-only registers on our platform. Reads of write-only registers cause a (synchronous) Data Abort exception. As the patch is small I'll attach it at the bottom here as well. With this patch, if users enable this feature, all user space fault handlers should check whether the fault is an SError or not (EC 0x2f) before deciding what to do, keeping in mind the address is a virtual address possibly for another task and that the PC is of the current task, not the instruction causing the fault. People who care deeply about this issue should make sure they have ARMv8.2 hardware with FEAT_RAS and use that to implement proper SError handling in seL4 for such systems (see e.g. Linux kernel). But that would be another patch, unrelated to this one. Greetings, Indan --- diff --git a/include/arch/arm/arch/64/mode/machine/registerset.h b/include/arch/arm/arch/64/mode/machine/registerset.h index ca040770..2bf6e674 100644 --- a/include/arch/arm/arch/64/mode/machine/registerset.h +++ b/include/arch/arm/arch/64/mode/machine/registerset.h @@ -38,6 +38,7 @@ #define ESR_EC_LEL_SVC64 0x15 // SVC from a lower EL in AArch64 state #define ESR_EC_LEL_HVC64 0x16 // HVC from EL1 in AArch64 state #define ESR_EL1_EC_ENFP 0x7 // Access to Advanced SIMD or floating-point registers +#define ESR_EC_LEL_SERROR 0x2f // SError /* ID_AA64PFR0_EL1 register */ diff --git a/src/arch/arm/64/traps.S b/src/arch/arm/64/traps.S index ed9aad3d..0f70cbc7 100644 --- a/src/arch/arm/64/traps.S +++ b/src/arch/arm/64/traps.S @@ -29,6 +29,9 @@ #endif +#ifdef CONFIG_AARCH64_PASS_SERROR +#define lower_el_serr lower_el_sync +#endif .macro lsp_i _tmp mrs \_tmp, TPIDR @@ -160,6 +163,10 @@ BEGIN_FUNC(lower_el_sync) b.eq lel_ia cmp x24, #ESR_EC_LEL_SVC64 b.eq lel_syscall +#ifdef CONFIG_AARCH64_PASS_SERROR + cmp x24, #ESR_EC_LEL_SERROR + b.eq lel_da +#endif #ifdef CONFIG_ARM_HYPERVISOR_SUPPORT cmp x24, #ESR_EC_LEL_HVC64 b.eq lel_syscall @@ -233,6 +240,8 @@ BEGIN_FUNC(lower_el_irq) b c_handle_interrupt END_FUNC(lower_el_irq) +#ifndef CONFIG_AARCH64_PASS_SERROR + BEGIN_FUNC(lower_el_serr) #ifdef CONFIG_PLAT_TX2 eret @@ -240,3 +249,5 @@ BEGIN_FUNC(lower_el_serr) b invalid_vector_entry #endif END_FUNC(lower_el_serr) + +#endif diff --git a/src/arch/arm/config.cmake b/src/arch/arm/config.cmake index 781e6833..d8770f49 100644 --- a/src/arch/arm/config.cmake +++ b/src/arch/arm/config.cmake @@ -179,6 +179,17 @@ config_option( DEFAULT_DISABLED OFF ) +config_option( + KernelAArch64PassSError AARCH64_PASS_SERROR + "By default any SError interrupt will halt the kernel. \ +SErrors may be caused by e.g. writes to read-only device registers or ECC errors. \ +When this option is enabled SErrors will be reported as a VM data fault for the currently running task. \ +Because SError is asynchronous it is not assured that the faulting task caused the error. " + DEFAULT OFF + DEPENDS "KernelSel4ArchAarch64;NOT KernelVerificationBuild" +) +mark_as_advanced(KernelAArch64PassSError) + if(KernelAArch32FPUEnableContextSwitch OR KernelSel4ArchAarch64) set(KernelHaveFPU ON) endif()