Page MenuHomeFreeBSD

GIC interrupt controller reworked for new ARM interrupt framework
ClosedPublic

Authored by ian on Mar 11 2015, 4:12 PM.
Tags
None
Referenced Files
F102391970: D2048.id4183.diff
Mon, Nov 11, 3:31 PM
F102387023: D2048.id7985.diff
Mon, Nov 11, 2:01 PM
F102386749: D2048.id4260.diff
Mon, Nov 11, 1:56 PM
F102385494: D2048.id.diff
Mon, Nov 11, 1:31 PM
F102385401: D2048.id.diff
Mon, Nov 11, 1:29 PM
F102384033: D2048.diff
Mon, Nov 11, 1:02 PM
F102380972: D2048.diff
Mon, Nov 11, 11:55 AM
Unknown Object (File)
Tue, Nov 5, 12:06 PM

Details

Summary

Utilization of new ARM interrupt framework by GIC implementation. Some things are dealt with better now. Some things are dealt now which could not be dealt before.

PANDABOARD switching to the framework is added too.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

skra retitled this revision from to GIC interrupt controller reworked for new ARM interrupt framework.
skra updated this object.
skra edited the test plan for this revision. (Show Details)
skra added reviewers: andrew, ian, imp.
skra set the repository for this revision to rS FreeBSD src repository - subversion.
skra added a parent revision: D2047: New ARM interrupt framework.
skra added a subscriber: ARM.
skra set the repository for this revision to rS FreeBSD src repository - subversion.

Do enable IPIs on other CPUs. It's IMPLEMENTATION DEFINED if masking IPI on GIC is supported.
Some small optimalization in gic_bind().

ian edited reviewers, added: skra; removed: ian.

Commandeer, because apparently that's the only way a collaborator is allowed to update the diff.

ian set the repository for this revision to rS FreeBSD src repository - subversion.

Update the diff so it applies to -current as of r286786, 2015/08/16.

Also, fix a little bug: in gic_map_fdt() the offsets for FIRST_PPI and FIRST_SGI based on isrc_cells[0] were reversed.

sys/arm/arm/gic.c
504 ↗(On Diff #7985)

If we ever allow an interrupt to be handled by mulitple CPUs, this printf will have to go because spurious interrupts will become a normal thing.

sys/arm/arm/gic.c
504 ↗(On Diff #7985)

IMO, it's two things. One thing is that we shuffle interrupts among cores, but each interrupt can fire only on one core. Thus, the situation will be same as it's now, just interrupt load could be balanced among cores. Another thing is, and certainly wanted one, that we allow an interrupt to be fired on whatever core it wants - the one free for interrupt dispatch. In that case, it depends on a controller if it's able to ensure that the very same interrupt does not fire on more than one core. If not, it's question if we disable such a feature or implement some locking to prevent such unwanted thing - an interrupt being served on more cores at same time.

Note that this is the reason for ARM_ISRCF_BOUND flag - a controller should know which interrupts must be bound to some core, and which interrupts are free to shuffle them among cores or enable them on all cores depending on its will and capability.

sys/arm/arm/gic.c
504 ↗(On Diff #7985)

With GIC, the IRQ line into the cores will be signaled for all cores eligible to receive the interrupt. One of those cores will receive the interrupt number when reading the IAR, and the others will get 0x3ff. So GIC g'tees only one core processes an interrupt, we just have to stop whining about when it signals to other cores that they lost the race.

sys/arm/arm/gic.c
453 ↗(On Diff #7985)

I really don't like this (void*) cast because of an intrng pic filter taking different args than a regular filter function. What if we say pic filter functions always take a pointer to a struct like:

struct arm_picfilter_data {
	void * pic_arg;
	void * trap_frame;
};

Where pic_arg is the arg value passed to bus_setup_intr() and trap_frame is what needs to be passed back to arm_irq_dispatch().

sys/arm/arm/gic.c
453 ↗(On Diff #7985)

It's associated with INTR_SOLO, so at least it should be more like this:

struct intr_solofilter_data {
           void * solo_arg;
           struct trapframe * solo_tf; 
};

But to be honest, my vote is for (void*) cast if I could choose from these two possibilities.

In fact, I would like to see this INTR_SOLO stuff pushed to MI interrupt framework, i.e. intergrated to intr_event struct . ;)

This revision was automatically updated to reflect the committed changes.