On Thu, 13 Dec 2018 at 09:54, Julien Thierry [off-list ref] wrote:

On 12/12/2018 18:10, Ard Biesheuvel wrote:

quoted

On Wed, 12 Dec 2018 at 18:59, Julien Thierry [off-list ref] wrote:

quoted

On 12/12/2018 17:27, Ard Biesheuvel wrote:

quoted

On Wed, 12 Dec 2018 at 17:48, Julien Thierry [off-list ref] wrote:

quoted

Instead disabling interrupts by setting the PSR.I bit, use a priority
higher than the one used for interrupts to mask them via PMR.

When using PMR to disable interrupts, the value of PMR will be used
instead of PSR.[DAIF] for the irqflags.

Signed-off-by: Julien Thierry <redacted>
Suggested-by: Daniel Thompson <redacted>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <redacted>
Cc: Ard Biesheuvel <redacted>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 arch/arm64/include/asm/efi.h      |   5 +-
 arch/arm64/include/asm/irqflags.h | 123 +++++++++++++++++++++++++++++---------
 2 files changed, 99 insertions(+), 29 deletions(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 7ed3208..a9d3ebc 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h

@@ -42,7 +42,10 @@

 efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);

-#define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
+#define ARCH_EFI_IRQ_FLAGS_MASK                                                \
+       (system_uses_irq_prio_masking() ?                               \
+               GIC_PRIO_IRQON :                                        \
+               (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT))

This mask is used to determine whether we return from a firmware call
with a different value for the I flag than we entered it with. So
instead of changing the mask, we should change the way we record DAIF,
given that the firmware is still going to poke the I bit if it
misbehaves, regardless of whether the OS happens to use priorities for
interrupt masking.

Thanks for pointing that out, so this change makes little sense...

The annoying part is that the flag checking takes place in the arch
agnostic code.

Would introducing some overriddable efi_get_flags() or efi_save_flags()
that default to local_save_flags() seem like an acceptable solution?

This way I could override it for arm64 and still return the DAIF bits.

I don't follow the reasoning below about irqflags exactly, but is
there any way we could simply but both PMR and DAIF in flags? We could
even update the mask here to ensure that the firmware doesn't corrupt
the PMR.

So, that was the case in my previous versions of the series, and as you
said, that covered checking both DAIF bits and PMR on return from EFI
services. But Catalin suggested that irqflags could just use PMR when we
enable the priority masking feature. Catalin's suggestion does simplify
things, except for this part.

However, it doesn't seem to far-fetched to me that the architecture
could have a more generic way to tell the EFI driver "this is the set of
stuff that I care about and you should return from runtime services with
this stuff in the same state as before" without the "set of stuff" being
limited to irqflags.

But maybe this would be over-engineering just to deal with my use-case...

No, that makes sense. As you said, you can just create a
efi_get_irqflags() helper that defaults to what we are using now, and
can be overridden to just return DAIF in our case.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel