lkml.org 
[lkml]   [2021]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v4 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller
Date
On 07/04/2021 03.16, Marc Zyngier wrote:
> Hi Hector,
>
> On Fri, 02 Apr 2021 10:05:39 +0100,
> Hector Martin <marcan@marcan.st> wrote:
>> + /*
>> + * In EL1 the non-redirected registers are the guest's,
>> + * not EL2's, so remap the hwirqs to match.
>> + */
>> + if (!is_kernel_in_hyp_mode()) {
>> + switch (fwspec->param[1]) {
>> + case AIC_TMR_GUEST_PHYS:
>> + *hwirq = ic->nr_hw + AIC_TMR_HV_PHYS;
>> + break;
>> + case AIC_TMR_GUEST_VIRT:
>> + *hwirq = ic->nr_hw + AIC_TMR_HV_VIRT;
>> + break;
>> + case AIC_TMR_HV_PHYS:
>> + case AIC_TMR_HV_VIRT:
>> + return -ENOENT;
>> + default:
>> + break;
>> + }
>> + }
>
> Urgh, this is nasty. You are internally remapping the hwirq from one
> timer to another in order to avoid accessing the enable register
> which happens to be an EL2 only register?

The remapping is to make the IRQs route properly at all.

There are EL2 and EL0 timers, and on GIC each timer goes to its own IRQ.
But here there are no real IRQs, everything's a FIQ. However, thanks to
VHE, the EL2 timer shows up as the EL0 timer, and the EL0 timer is
accessed via EL02 registers, when in EL2. So in EL2/VHE mode, "HV" means
EL0 and "guest" means EL02, while in EL1, there is no HV and "guest"
means EL0. And since we figure out which IRQ fired by reading timer
registers, this is what matters. So I map the guest IRQs to the HV
hwirqs in EL1 mode, which makes this all work out. Then the timer code
goes and ends up undoing all this logic again, so we map to separate
fake "IRQs" only to end up right back at using the same timer registers
anuway :-)

Really, the ugliness here is that the constant meaning is overloaded. In
fwspec context they mean what they say on the tin, while in hwirq
context "HV" means EL0 and "guest" means EL02 (other FIQs would be
passed through unchanged). Perhaps some additional defines might help
clarify this? Say, at the top of this file (not in the binding),

/*
* Pass-through mapping from real timers to the correct registers to
* access them in EL2/VHE mode. When running in EL1, this gets
* overridden to access the guest timer using EL0 registers.
*/
#define AIC_TMR_EL0_PHYS AIC_TMR_HV_PHYS
#define AIC_TMR_EL0_VIRT AIC_TMR_HV_VIRT
#define AIC_TMR_EL02_PHYS AIC_TMR_GUEST_PHYS
#define AIC_TMR_EL02_VIRT AIC_TMR_GUEST_VIRT

Then the irqchip/FIQ dispatch side can use the EL* constants, the
default pass-through mapping is appropriate for VHE/EL2 mode, and
translation can adjust it for EL1 mode.

--
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

\
 
 \ /
  Last update: 2021-04-06 21:22    [W:0.101 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site