From: wuyun.wu@huawei•com (Abel)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 02/20] arm64: initial support for GICv3
Date: Wed, 11 Jun 2014 09:15:07 +0800 [thread overview]
Message-ID: <5397AD9B.8000304@huawei.com> (raw)
In-Reply-To: <87ppihf3io.fsf@approximate.cambridge.arm.com>
On 2014/6/10 18:43, Marc Zyngier wrote:
> On Tue, Jun 10 2014 at 4:57:03 am BST, Abel <wuyun.wu@huawei•com> wrote:
>> On 2014/6/9 16:41, Marc Zyngier wrote:
>>
>>> On Mon, Jun 09 2014 at 5:10:30 am BST, Abel <wuyun.wu@huawei•com> wrote:
>>>> As for the interrupts here, the ones entered into this 'if' clause, I
>>>> think they are expected already been mapped into this irq_domain.
>>>
>>> Mapped or not is not the problem. Think of a (broken) implementation
>>> that would generate random IDs in the irq_nr..1021 range. You'd end up
>>> with the deadlock situation I've outlined above.
>>>
>>> And actually, irq_find_mapping can fail, and I should handle this.
>>
>>
>> Yes, makes sense. This case should be properly handled.
>> And I still have two questions.
>> a) Does this scenario really exist or necessary? I mean, part of interrupts
>> mapped while others which belongs to the same interrupt controller are not.
>
> Not that I know of. But consider this cheap defensive programming. It is
> actually cheaper to check against a constant boundary than do a load and
> check against a variable. Safer, faster.
Yes, sure.
>
>> b) Why 1021?
>
> The maximum SPI ID is 1020.
Oooh.. SPI ends at 1019, and 1020 is for special purpose.
>
>>
>>>>>
>>>>>>> + irqnr = irq_find_mapping(gic_data.domain, irqnr);
>>>>>>> + handle_IRQ(irqnr, regs);
>>>>>>> + continue;
>>>>>>> + }
>>>>>>> + if (irqnr < 16) {
>>>>>>> + gic_write_eoir(irqnr);
>>>>>>> +#ifdef CONFIG_SMP
>>>>>>> + handle_IPI(irqnr, regs);
>>>>>>> +#else
>>>>>>> + WARN_ONCE(true, "Unexpected SGI received!\n");
>>>>>>> +#endif
>>>>>>> + continue;
>>>>>>> + }
>>>>>>> + } while (irqnr != 0x3ff);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void __init gic_dist_init(void)
>>>>>>> +{
>>>>>>> + unsigned int i;
>>>>>>> + u64 affinity;
>>>>>>> + void __iomem *base = gic_data.dist_base;
>>>>>>> +
>>>>>>> + /* Disable the distributor */
>>>>>>> + writel_relaxed(0, base + GICD_CTLR);
>>>>>>
>>>>>>
>>>>>> I'm not sure whether waiting for write pending here is
>>>>>> necessary. What do you think?
>>>>>
>>>>> Hmmm. That's a good point. The spec says the RWP tracks the completion
>>>>> of writes that change (among other things) the state of an interrupt
>>>>> group enable. But the next line will perform a wait_for_rwp anyway after
>>>>> having disabled all SPIs. So no, I don't think we need to wait. We
>>>>> guaranteed to get a consistent state after the call to gic_dist_config.
>>>>
>>>>
>>>> The spec also says that software must ensure the interrupt is disabled before
>>>> changing its routing/priority/group information, see chapter 4.5.5 in v18.
>>>
>>> That's only to ensure consistency when a single interrupt is being
>>> reconfigured, and that's what the code already does. In this particular
>>> context, waiting for RWP looks completely superfluous.
>>
>> It's reasonable to think GIC having been enabled by boot loader or
>> something else in the boot procedure. And here not waiting for RWP may
>> cause hardware implementation defined behavior, that is configuring an
>> enabled interrupt. It's beyond the scope of the GIC architecture, and
>> vendors also don't want to see this happen.
>
> Well, I guess that since this is a one-off thing, it doesn't really add
> an extra synchronization. As long as it gives you peace of mind... ;-)
>
OK, then let's keep it what it was. :)
Thanks,
Abel.
next prev parent reply other threads:[~2014-06-11 1:15 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-15 17:58 [PATCH v4 00/20] arm64: GICv3 support Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 01/20] ARM: GIC: move some bits of GICv2 to a library-type file Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 02/20] arm64: initial support for GICv3 Marc Zyngier
2014-05-23 16:40 ` Jean-Philippe Brucker
2014-05-27 8:17 ` Marc Zyngier
2014-06-05 7:47 ` Abel
2014-06-05 8:44 ` Marc Zyngier
2014-06-09 4:10 ` Abel
2014-06-09 8:41 ` Marc Zyngier
2014-06-10 3:57 ` Abel
2014-06-10 10:43 ` Marc Zyngier
2014-06-11 1:15 ` Abel [this message]
2014-05-15 17:58 ` [PATCH v4 03/20] arm64: GICv3 device tree binding documentation Marc Zyngier
2014-05-20 14:58 ` Andre Przywara
2014-05-20 15:32 ` Marc Zyngier
2014-05-20 16:21 ` Andre Przywara
2014-05-15 17:58 ` [PATCH v4 04/20] arm64: boot protocol documentation update for GICv3 Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 05/20] KVM: arm/arm64: vgic: move GICv2 registers to their own structure Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 06/20] KVM: ARM: vgic: introduce vgic_ops and LR manipulation primitives Marc Zyngier
2014-05-20 12:33 ` Christoffer Dall
2014-05-15 17:58 ` [PATCH v4 07/20] KVM: ARM: vgic: abstract access to the ELRSR bitmap Marc Zyngier
2014-05-20 12:35 ` Christoffer Dall
2014-05-15 17:58 ` [PATCH v4 08/20] KVM: ARM: vgic: abstract EISR bitmap access Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 09/20] KVM: ARM: vgic: abstract MISR decoding Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 10/20] KVM: ARM: vgic: move underflow handling to vgic_ops Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 11/20] KVM: ARM: vgic: abstract VMCR access Marc Zyngier
2014-05-20 12:41 ` Christoffer Dall
2014-05-15 17:58 ` [PATCH v4 12/20] KVM: ARM: vgic: introduce vgic_enable Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 13/20] KVM: ARM: introduce vgic_params structure Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 14/20] KVM: ARM: vgic: split GICv2 backend from the main vgic code Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 15/20] KVM: ARM: vgic: revisit implementation of irqchip_in_kernel Marc Zyngier
2014-05-20 12:50 ` Christoffer Dall
2014-05-15 17:58 ` [PATCH v4 16/20] arm64: KVM: remove __kvm_hyp_code_{start, end} from hyp.S Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 17/20] arm64: KVM: split GICv2 world switch from hyp code Marc Zyngier
2014-05-20 12:53 ` Christoffer Dall
2014-05-15 17:58 ` [PATCH v4 18/20] arm64: KVM: move HCR_EL2.{IMO, FMO} manipulation into the vgic switch code Marc Zyngier
2014-05-20 12:58 ` Christoffer Dall
2014-05-15 17:58 ` [PATCH v4 19/20] KVM: ARM: vgic: add the GICv3 backend Marc Zyngier
2014-05-20 13:09 ` Christoffer Dall
2014-05-20 13:29 ` Marc Zyngier
2014-05-15 17:58 ` [PATCH v4 20/20] arm64: KVM: vgic: add GICv3 world switch Marc Zyngier
2014-05-28 19:11 ` Will Deacon
2014-06-02 15:09 ` Marc Zyngier
2014-05-30 23:06 ` [PATCH v4 00/20] arm64: GICv3 support Radha Mohan
2014-05-31 1:14 ` Chalamarla, Tirumalesh
2014-06-02 12:59 ` Marc Zyngier
2014-06-02 12:57 ` Marc Zyngier
2014-06-11 1:49 ` Abel
2014-06-11 2:58 ` Abel
[not found] <CALicx6vz6JZsMDmEVUMarsTd67xK1r+HjVTD0Jg94U6LD9bk3w@mail.gmail.com>
2014-06-11 16:16 ` [PATCH v4 02/20] arm64: initial support for GICv3 Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5397AD9B.8000304@huawei.com \
--to=wuyun.wu@huawei$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox