Add BUS_UNMASK_INTR method

[wma_semihalf.com]

Summary

This commit allows other subsystem to explicitly unmask

previously configured interrupt on the current CPU.
Before it was not possible. For example, ARM timer should
unmask per-processor interrupts on each secondary CPU the
system boots. It cannot call bus_setup_intr then, because
was already invoked on primary CPU, thus the new method was
added.

Mostly will help to cleanup GIC init_secondary routines and move TMR PPI unmasking to timer drivers rather than using hardcoded values in the interrupt controller (future commits).

The armv8 function arm_unmask_irq is undefined yet and is waiting for https://reviews.freebsd.org/D3029 to process.

[wma_semihalf.com]

Any comments?

[zbb]

Please send your comments asap. We want to commit this soon.

[wma_semihalf.com]

Moved arm_unmask_irq here.

[mmel]

With full respect, I don’t think that this new bus method is right solution for our problem(s).
The proposed bus_unmask_intr() cannot be used from outside of secondary CPUs startup context. It cannot be used for SPI, it cannot be used of PPI after scheduler is started on given core.
Why you cannot simply call arm_unmask_irq() directly from driver?

[skra]

Well, I do not like to change bus interface so fast. What about to think of PPI on each core like separate interrupt with its irq number, counter, handler, and handler argument. This way bus_setup_intr will be called only once for PPI on each core. Of course, some coding is needed in nexus and/or interrupt framework.

Sadly, there is a new interrupt framework waiting for review for long time. Thus, this thing could be already discussed without such hurry.

[wma_semihalf.com]

I guess treating PPIs are separate IRQs number will cause a huge mess. I'm starting to port fbsd on 96 core armv8 platform and the idea of 1536 vectors wasted for PPIs is outrageous.

My point in adding separate interface is to support multi-PIC systems. Calling BUS_UNMASK_INTR with an appropriate resource will allow to pick up the correct PIC controller in a generic way (as I looked into intrng last time it was capable of associating device with specific PIC). Adding the same code to timer driver might be really inelegant, thus require a static map either in the driver or in intr_machdep file.

The proposed bus_unmask_intr() cannot be used from outside of secondary CPUs startup context. It cannot be used for SPI, it cannot be used of PPI after scheduler is started on given core.

Actually, I'm not quite sure I understand your point. Why cannot one use it for SPI (except it might not have a sense) or what the scheduler has to do with PPI? The only risk I see (in current implementation) is that someone calls unmask when irq is not configured.
The freebsd also lacks the fine-grained interrupt blocking mechanism, like, for example, mask/unmask pair exported as bus interface (that would require adding mask call here also). I bet if you ever tried to write a driver for some queer and exotic piece of hardware you wished these functions existed, like in other OSes. Or at least I have...

[skra]

In D3034#61087, @wma_semihalf.com wrote:

I guess treating PPIs are separate IRQs number will cause a huge mess. I'm starting to port fbsd on 96 core armv8 platform and the idea of 1536 vectors wasted for PPIs is outrageous.

In fact, PPIs are separate interrupts. So if you have 96 core armv8 platform with 16 PPIs on each core, then you have 1536 PPIs. If you have 96 core, then you have probably enough memory to run them all. And I certainly will not call using of 1536 IRQ numbers as wasting in this case. Moreover, 1536 IRQ numbers are allocated only if all of them are used.

Further, with such many cores, I can imagine more than before that same PPI could be served differently on various subsets of cores. However, I do not say that it's necessary to have an IRQ number for each PPI on each core. I just say that it should be so in principle. It could be optimalized according to usage.

Think about PPI statistics for example. Will be PPI interrupts counted on a single counter? (Just do not forget to use atomic increment in that case. I have confirmed that even two cores count badly without atomics.)

Anyhow, I still think that problem with PPIs you desribed can be dealt with either in nexus or in interrupt framework without a need to change bus interface. (You can change the way how bus_setup_intr() and bus_teardown_intr() are implemented for PPI kind of interrupts.)

My point in adding separate interface is to support multi-PIC systems. Calling BUS_UNMASK_INTR with an appropriate resource will allow to pick up the correct PIC controller in a generic way (as I looked into intrng last time it was capable of associating device with specific PIC). Adding the same code to timer driver might be really inelegant, thus require a static map either in the driver or in intr_machdep file.

Note that only root PIC is capable of PPIs and SGIs (IPIs).

However, if you really want to add some new bus interface, I would like to know more about it. For example, what about to add man pages for it? What about to add bus_mask_intr() too? What is difference between it and bus_activate_resource()? What about to add cpuset_t argument to it? (Which, I believe, was Michal's point for using with SPI.) Must be ensured that no cpu migration happens during the call? I have a really bad feeling with the change how it's introduced now.

The proposed bus_unmask_intr() cannot be used from outside of secondary CPUs startup context. It cannot be used for SPI, it cannot be used of PPI after scheduler is started on given core.

Actually, I'm not quite sure I understand your point. Why cannot one use it for SPI (except it might not have a sense) or what the scheduler has to do with PPI? The only risk I see (in current implementation) is that someone calls unmask when irq is not configured.
The freebsd also lacks the fine-grained interrupt blocking mechanism, like, for example, mask/unmask pair exported as bus interface (that would require adding mask call here also). I bet if you ever tried to write a driver for some queer and exotic piece of hardware you wished these functions existed, like in other OSes. Or at least I have...

[wma_semihalf.com]

Thanks! That was really helpful. However I'm still confused about the system I have. I didn't powered up the full configuration yet, but I'm afraid it has two root pics then (one per each 48-cpu partition). Nevertheless, I'm abandoning this review for a while and stick to hacking generic_timer. When I test it on actual hw I will get back to this patch. However I'm still thinking the generic mask/unmask pair might be a useful feature.

[andrew]

I would like to see something like how Linux handles this. When they create interrupts they can mark them as per-core, i.e. they will be unmasked on all cores. I think we could do this with a new flag in bus_setup_intr.

Care would need to be taken to handle devices created both before and after we have enabled the secondary cpus as I would expect the code paths to be slightly different, e.g. before enabling we would need to record the need to unmask when new cpus are brought up, after enabling we will also need to signal to the other cpus to unmask the interrupt.