Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
Hi Thomas, On 11/16/2015 05:17 PM, Thomas Gleixner wrote: 1) IPI as per_cpu interrupts Single hwirq represented by a single irq descriptor 2) IPI with consecutive mapping space No extra mapping from virq base to target cpu required as its just linear. Everything can be handled via the base virq. I think I am seeing a major issue with this approach. Take the case where we reserve an IPI with ipi_mask that has cpu 5 and 6 set only. When allocating a per_cpu or consectuve mapping, we will require 2 consecutive virqs and hwirqs. But since the cpu location is not starting from 0, we can't use the cpu as an offset anymore. So when a user wants to send an IPI to cpu 6 only, the code can't easily tell what's the correct offset from base virq or hwirq to use. Same applies when doing the reverse mapping. In other words, the ipi_mask won't always necessarily be linear to facilitate the 1:1 mapping that this approach assumes. It is a solvable problem, but I think we're losing the elegance that promoted going into this direction and I think sticking to using struct ipi_mapping (with some enhancements to how it's exposed an integrated by/into generic code) is a better approach. Thoughts? I still don't have a working implementation otherwise I would have sent my patches, but I thought I'd raise this up before I spend more time on it unnecessarily. Thanks, Qais -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
Hi Thomas, On 11/16/2015 05:17 PM, Thomas Gleixner wrote: 1) IPI as per_cpu interrupts Single hwirq represented by a single irq descriptor 2) IPI with consecutive mapping space No extra mapping from virq base to target cpu required as its just linear. Everything can be handled via the base virq. I think I am seeing a major issue with this approach. Take the case where we reserve an IPI with ipi_mask that has cpu 5 and 6 set only. When allocating a per_cpu or consectuve mapping, we will require 2 consecutive virqs and hwirqs. But since the cpu location is not starting from 0, we can't use the cpu as an offset anymore. So when a user wants to send an IPI to cpu 6 only, the code can't easily tell what's the correct offset from base virq or hwirq to use. Same applies when doing the reverse mapping. In other words, the ipi_mask won't always necessarily be linear to facilitate the 1:1 mapping that this approach assumes. It is a solvable problem, but I think we're losing the elegance that promoted going into this direction and I think sticking to using struct ipi_mapping (with some enhancements to how it's exposed an integrated by/into generic code) is a better approach. Thoughts? I still don't have a working implementation otherwise I would have sent my patches, but I thought I'd raise this up before I spend more time on it unnecessarily. Thanks, Qais -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Tue, 17 Nov 2015, Qais Yousef wrote:
> On 11/16/2015 05:24 PM, Thomas Gleixner wrote:
> >
> > int ipi_get_hw_irq(int irq)
> > {
> > struct irq_data *d = irq_get_irq_data(irq);
> > return d ? irqd_to_hwirq(d);
> > }
> > Hmm?
> >
>
> We need cpu as an argument too.
Indeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/17/2015 10:11 AM, Thomas Gleixner wrote: Right, I was assuming a consecutive available space and your hardware folks should really avoid to break that assumption. Now you still need some DT support to describe the space which is available for IPIs and that should be part of that series. It's a simple change that shouldn't be a problem adding it to this series. I just wanted to avoid having to take more acks from more maintainers for this series to go in. But if it's needed, then it is what it is. Maybe I'm better off sending this change separately actually as it's independent from other changes and could be merged in first. Thanks for being patient and persistant on that! Thanks a lot for your help and patience too! Qais -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/16/2015 05:24 PM, Thomas Gleixner wrote:
int ipi_get_hw_irq(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
return d ? irqd_to_hwirq(d);
}
Hmm?
We need cpu as an argument too.
Taking your other comments into account and ignoring the random mapping
space for now. I think I can expand that to do the right thing for when
the IPI domain is per cpu or consecutive.
Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Tue, 17 Nov 2015, Qais Yousef wrote: > On 11/16/2015 05:17 PM, Thomas Gleixner wrote: > > On Mon, 9 Nov 2015, Qais Yousef wrote: > > > On 11/07/2015 02:51 PM, Thomas Gleixner wrote: > > > Generally it's hard to know whether a real device is connected to a hwirq > > > or > > > not. I am saving a patch where we get a set of free hwirqs from DT as only > > > the > > > SoC designer knows what hwirq are actually free and safe to use for IPI. > > > I'll > > > send this patch with the DT IPI changes or the rproc driver that I will be > > > send once these changes are merged. > > > > > > The current code assumes that the last 2 * NR_CPUs hwirqs are always free > > > to > > > use for Linux SMP. > > So what you're saying is that you cannot rely on the last X hwirqs > > being available for IPIs. That's insane and to my knowledge there is > > no hardware out there which does not reserve a consecutive IPI space. > > If I read the code you were suggesting correctly, you were trying to fit the > IPIs in any available non allocated area in the GIC space. What I am trying to > say is that we can only work on a limited subset of this space that we are > told explicitly it's safe to use for IPIs. Most likely it's consecutive, but I > don't feel brave enough to make this assumption personally - maybe I'm over > paranoid.. I'm more keen on anything that would simplify this patch series now > though. Right, I was assuming a consecutive available space and your hardware folks should really avoid to break that assumption. Now you still need some DT support to describe the space which is available for IPIs and that should be part of that series. > I'll do my best with the next series but maybe we'd need to iterate > this more than once till I get it right. Thanks for being patient and persistant on that! tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/16/2015 05:17 PM, Thomas Gleixner wrote: On Mon, 9 Nov 2015, Qais Yousef wrote: On 11/07/2015 02:51 PM, Thomas Gleixner wrote: Generally it's hard to know whether a real device is connected to a hwirq or not. I am saving a patch where we get a set of free hwirqs from DT as only the SoC designer knows what hwirq are actually free and safe to use for IPI. I'll send this patch with the DT IPI changes or the rproc driver that I will be send once these changes are merged. The current code assumes that the last 2 * NR_CPUs hwirqs are always free to use for Linux SMP. So what you're saying is that you cannot rely on the last X hwirqs being available for IPIs. That's insane and to my knowledge there is no hardware out there which does not reserve a consecutive IPI space. If I read the code you were suggesting correctly, you were trying to fit the IPIs in any available non allocated area in the GIC space. What I am trying to say is that we can only work on a limited subset of this space that we are told explicitly it's safe to use for IPIs. Most likely it's consecutive, but I don't feel brave enough to make this assumption personally - maybe I'm over paranoid.. I'm more keen on anything that would simplify this patch series now though. I'll do my best with the next series but maybe we'd need to iterate this more than once till I get it right. Thanks, Qais -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/16/2015 05:17 PM, Thomas Gleixner wrote: On Mon, 9 Nov 2015, Qais Yousef wrote: On 11/07/2015 02:51 PM, Thomas Gleixner wrote: Generally it's hard to know whether a real device is connected to a hwirq or not. I am saving a patch where we get a set of free hwirqs from DT as only the SoC designer knows what hwirq are actually free and safe to use for IPI. I'll send this patch with the DT IPI changes or the rproc driver that I will be send once these changes are merged. The current code assumes that the last 2 * NR_CPUs hwirqs are always free to use for Linux SMP. So what you're saying is that you cannot rely on the last X hwirqs being available for IPIs. That's insane and to my knowledge there is no hardware out there which does not reserve a consecutive IPI space. If I read the code you were suggesting correctly, you were trying to fit the IPIs in any available non allocated area in the GIC space. What I am trying to say is that we can only work on a limited subset of this space that we are told explicitly it's safe to use for IPIs. Most likely it's consecutive, but I don't feel brave enough to make this assumption personally - maybe I'm over paranoid.. I'm more keen on anything that would simplify this patch series now though. I'll do my best with the next series but maybe we'd need to iterate this more than once till I get it right. Thanks, Qais -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/16/2015 05:24 PM, Thomas Gleixner wrote:
int ipi_get_hw_irq(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
return d ? irqd_to_hwirq(d);
}
Hmm?
We need cpu as an argument too.
Taking your other comments into account and ignoring the random mapping
space for now. I think I can expand that to do the right thing for when
the IPI domain is per cpu or consecutive.
Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/17/2015 10:11 AM, Thomas Gleixner wrote: Right, I was assuming a consecutive available space and your hardware folks should really avoid to break that assumption. Now you still need some DT support to describe the space which is available for IPIs and that should be part of that series. It's a simple change that shouldn't be a problem adding it to this series. I just wanted to avoid having to take more acks from more maintainers for this series to go in. But if it's needed, then it is what it is. Maybe I'm better off sending this change separately actually as it's independent from other changes and could be merged in first. Thanks for being patient and persistant on that! Thanks a lot for your help and patience too! Qais -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Tue, 17 Nov 2015, Qais Yousef wrote:
> On 11/16/2015 05:24 PM, Thomas Gleixner wrote:
> >
> > int ipi_get_hw_irq(int irq)
> > {
> > struct irq_data *d = irq_get_irq_data(irq);
> > return d ? irqd_to_hwirq(d);
> > }
> > Hmm?
> >
>
> We need cpu as an argument too.
Indeed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Tue, 17 Nov 2015, Qais Yousef wrote: > On 11/16/2015 05:17 PM, Thomas Gleixner wrote: > > On Mon, 9 Nov 2015, Qais Yousef wrote: > > > On 11/07/2015 02:51 PM, Thomas Gleixner wrote: > > > Generally it's hard to know whether a real device is connected to a hwirq > > > or > > > not. I am saving a patch where we get a set of free hwirqs from DT as only > > > the > > > SoC designer knows what hwirq are actually free and safe to use for IPI. > > > I'll > > > send this patch with the DT IPI changes or the rproc driver that I will be > > > send once these changes are merged. > > > > > > The current code assumes that the last 2 * NR_CPUs hwirqs are always free > > > to > > > use for Linux SMP. > > So what you're saying is that you cannot rely on the last X hwirqs > > being available for IPIs. That's insane and to my knowledge there is > > no hardware out there which does not reserve a consecutive IPI space. > > If I read the code you were suggesting correctly, you were trying to fit the > IPIs in any available non allocated area in the GIC space. What I am trying to > say is that we can only work on a limited subset of this space that we are > told explicitly it's safe to use for IPIs. Most likely it's consecutive, but I > don't feel brave enough to make this assumption personally - maybe I'm over > paranoid.. I'm more keen on anything that would simplify this patch series now > though. Right, I was assuming a consecutive available space and your hardware folks should really avoid to break that assumption. Now you still need some DT support to describe the space which is available for IPIs and that should be part of that series. > I'll do my best with the next series but maybe we'd need to iterate > this more than once till I get it right. Thanks for being patient and persistant on that! tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Thu, 12 Nov 2015, Qais Yousef wrote:
> Issues I'm seeing:
>
> - Device domain would be identical to GIC domain and it would defer
> everything to the parent domain except for the extra level of indirection. No?
It's not identical. It's a subset of the GIC domain and it has
different semantics than the IPI domain.
> - The race condition I mentioned in my earlier email where we must be told
> what hwirqs are available because we can't guarantee there's no real device
> connected to it which could interfere with the operation. We have always to
> work on a pre reserved set defined by the system. Currently GIC hard codes
> this set, but I'll be making it a DT property in the future.
We do that better now as we really don't want to start over when it
turns out that the DT property imposes other issues on it.
> - If we remove the mapping, how can a coprocessor drivers find out the
> reverse mapping to pass the hwirq to the firmware so that it can send and
> listen on the correct hwirqs? I have to say my current patches missed dealing
> with this problem. Now I have something to test my rproc driver on I came to
> realise I haven't added the function to do the reverse mapping.
int ipi_get_hw_irq(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
return d ? irqd_to_hwirq(d);
}
Hmm?
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Mon, 9 Nov 2015, Qais Yousef wrote: > On 11/07/2015 02:51 PM, Thomas Gleixner wrote: > Generally it's hard to know whether a real device is connected to a hwirq or > not. I am saving a patch where we get a set of free hwirqs from DT as only the > SoC designer knows what hwirq are actually free and safe to use for IPI. I'll > send this patch with the DT IPI changes or the rproc driver that I will be > send once these changes are merged. > > The current code assumes that the last 2 * NR_CPUs hwirqs are always free to > use for Linux SMP. So what you're saying is that you cannot rely on the last X hwirqs being available for IPIs. That's insane and to my knowledge there is no hardware out there which does not reserve a consecutive IPI space. But nevertheless, lets look at the various (possible) requirements we have: 1) IPI as per_cpu interrupts Single hwirq represented by a single irq descriptor 2) IPI with consecutive mapping space No extra mapping from virq base to target cpu required as its just linear. Everything can be handled via the base virq. 3) IPI with random mapping space Seperate mapping virq base to target cpu is required. The obvious place to store it are the irq descriptors. That needs a bit different machinery for ipi_send_mask(), but it's not rocket science. > > That makes a lot of things simpler. You don't have to keep a mapping > > of the hwirq to the target cpu. You just can use the base hwirq and > > calculate the destination hwirq from there when sending an IPI > > (general Linux ones). The coprocessor one will just be a natural > > fallout. > > Are you suggesting here to remove the whole new mapping API from the > generic code or just that it's not necessary to use it in my case? Err. I'm saying that you did not make use of hierarchical domains. You just glued the IPI stuff sideways on the GIC. We certainly want the generic code for managing the allocation etc. > I'm confused here as well. Is this a complementary API or are you suggesting > replacing the one this patch introduces? Those are replacements. We just need to handle the random mapping case if we really need it. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Mon, 9 Nov 2015, Qais Yousef wrote: > On 11/07/2015 02:51 PM, Thomas Gleixner wrote: > Generally it's hard to know whether a real device is connected to a hwirq or > not. I am saving a patch where we get a set of free hwirqs from DT as only the > SoC designer knows what hwirq are actually free and safe to use for IPI. I'll > send this patch with the DT IPI changes or the rproc driver that I will be > send once these changes are merged. > > The current code assumes that the last 2 * NR_CPUs hwirqs are always free to > use for Linux SMP. So what you're saying is that you cannot rely on the last X hwirqs being available for IPIs. That's insane and to my knowledge there is no hardware out there which does not reserve a consecutive IPI space. But nevertheless, lets look at the various (possible) requirements we have: 1) IPI as per_cpu interrupts Single hwirq represented by a single irq descriptor 2) IPI with consecutive mapping space No extra mapping from virq base to target cpu required as its just linear. Everything can be handled via the base virq. 3) IPI with random mapping space Seperate mapping virq base to target cpu is required. The obvious place to store it are the irq descriptors. That needs a bit different machinery for ipi_send_mask(), but it's not rocket science. > > That makes a lot of things simpler. You don't have to keep a mapping > > of the hwirq to the target cpu. You just can use the base hwirq and > > calculate the destination hwirq from there when sending an IPI > > (general Linux ones). The coprocessor one will just be a natural > > fallout. > > Are you suggesting here to remove the whole new mapping API from the > generic code or just that it's not necessary to use it in my case? Err. I'm saying that you did not make use of hierarchical domains. You just glued the IPI stuff sideways on the GIC. We certainly want the generic code for managing the allocation etc. > I'm confused here as well. Is this a complementary API or are you suggesting > replacing the one this patch introduces? Those are replacements. We just need to handle the random mapping case if we really need it. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Thu, 12 Nov 2015, Qais Yousef wrote:
> Issues I'm seeing:
>
> - Device domain would be identical to GIC domain and it would defer
> everything to the parent domain except for the extra level of indirection. No?
It's not identical. It's a subset of the GIC domain and it has
different semantics than the IPI domain.
> - The race condition I mentioned in my earlier email where we must be told
> what hwirqs are available because we can't guarantee there's no real device
> connected to it which could interfere with the operation. We have always to
> work on a pre reserved set defined by the system. Currently GIC hard codes
> this set, but I'll be making it a DT property in the future.
We do that better now as we really don't want to start over when it
turns out that the DT property imposes other issues on it.
> - If we remove the mapping, how can a coprocessor drivers find out the
> reverse mapping to pass the hwirq to the firmware so that it can send and
> listen on the correct hwirqs? I have to say my current patches missed dealing
> with this problem. Now I have something to test my rproc driver on I came to
> realise I haven't added the function to do the reverse mapping.
int ipi_get_hw_irq(int irq)
{
struct irq_data *d = irq_get_irq_data(irq);
return d ? irqd_to_hwirq(d);
}
Hmm?
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
Hi Thomas,
On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
On Tue, 3 Nov 2015, Qais Yousef wrote:
Add a new ipi domain on top of the normal domain.
MIPS GIC now supports dynamic allocation of an IPI.
I don't think you make use of the power of hierarchical irq
domains. You just whacked the current code into submission.
This time I'm having problems digesting your suggestion. I can't see how
it would make things simpler to be honest.
Issues I'm seeing:
- Device domain would be identical to GIC domain and it would defer
everything to the parent domain except for the extra level of
indirection. No?
- The race condition I mentioned in my earlier email where we must
be told what hwirqs are available because we can't guarantee there's no
real device connected to it which could interfere with the operation. We
have always to work on a pre reserved set defined by the system.
Currently GIC hard codes this set, but I'll be making it a DT property
in the future.
- If we remove the mapping, how can a coprocessor drivers find out
the reverse mapping to pass the hwirq to the firmware so that it can
send and listen on the correct hwirqs? I have to say my current patches
missed dealing with this problem. Now I have something to test my rproc
driver on I came to realise I haven't added the function to do the
reverse mapping.
In summary. I can't see how adding the device domain would help in
making things simpler and without having generic explicit cpu mapping I
don't know how I can implement a generic reverse mapping function to get
the hwirq to pass to the coprocessor firmware.
If I misunderstood your suggestion, mind rephrasing it please?
I can see though if I use irq_*_alloc_parent() I can probably get rid
off the below since I'd be able to use gic_irq_domain all the time to do
the revmap.
+ if (test_bit(intr, ipi_intrs)) {
+ virq = irq_linear_revmap(gic_ipi_domain,
+ GIC_SHARED_TO_HWIRQ(intr));
+ } else {
+ virq = irq_linear_revmap(gic_irq_domain,
+ GIC_SHARED_TO_HWIRQ(intr));
+ }
+
Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
Hi Thomas,
On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
On Tue, 3 Nov 2015, Qais Yousef wrote:
Add a new ipi domain on top of the normal domain.
MIPS GIC now supports dynamic allocation of an IPI.
I don't think you make use of the power of hierarchical irq
domains. You just whacked the current code into submission.
This time I'm having problems digesting your suggestion. I can't see how
it would make things simpler to be honest.
Issues I'm seeing:
- Device domain would be identical to GIC domain and it would defer
everything to the parent domain except for the extra level of
indirection. No?
- The race condition I mentioned in my earlier email where we must
be told what hwirqs are available because we can't guarantee there's no
real device connected to it which could interfere with the operation. We
have always to work on a pre reserved set defined by the system.
Currently GIC hard codes this set, but I'll be making it a DT property
in the future.
- If we remove the mapping, how can a coprocessor drivers find out
the reverse mapping to pass the hwirq to the firmware so that it can
send and listen on the correct hwirqs? I have to say my current patches
missed dealing with this problem. Now I have something to test my rproc
driver on I came to realise I haven't added the function to do the
reverse mapping.
In summary. I can't see how adding the device domain would help in
making things simpler and without having generic explicit cpu mapping I
don't know how I can implement a generic reverse mapping function to get
the hwirq to pass to the coprocessor firmware.
If I misunderstood your suggestion, mind rephrasing it please?
I can see though if I use irq_*_alloc_parent() I can probably get rid
off the below since I'd be able to use gic_irq_domain all the time to do
the revmap.
+ if (test_bit(intr, ipi_intrs)) {
+ virq = irq_linear_revmap(gic_ipi_domain,
+ GIC_SHARED_TO_HWIRQ(intr));
+ } else {
+ virq = irq_linear_revmap(gic_irq_domain,
+ GIC_SHARED_TO_HWIRQ(intr));
+ }
+
Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
On Tue, 3 Nov 2015, Qais Yousef wrote:
Add a new ipi domain on top of the normal domain.
MIPS GIC now supports dynamic allocation of an IPI.
I don't think you make use of the power of hierarchical irq
domains. You just whacked the current code into submission.
Let me explain it to you how that should look like and how that's
going to make the code way simpler.
The root domain is the GIC itself. It provides an allocation mechanism
for all GIC interrupts, global, ipi and per cpu plus the basic
management of them.
So the GIC domain looks at the complete hardware irq space. Now that
irq space is first partioned into local and shared interrupts.
- hwirq MAX
Shared interrupts
- hwirq 6
Local interrupts
- hwirq 0
So that shared interrupt space is where your device interrupts and the
ipi interrupts come from. That local interrupt space seems to be
hardwired, so it'd be overkill to handle that in an extra domain.
I assume that that shared interrupt space is partitioned as well
because the potential device interrupts on the SoC are hardwired. So
the picture looks like this:
- hwirq MAX
Shared assignable interrupts
- hwirq X
Shared device interrupts
- hwirq 6
Local interrupts
- hwirq 0
So if we look at the resulting hierarchy it looks like this:
|- [IPI domain]
[ GIC domain] -
|- [device domain]
The GIC domain manages a bitmap of the full irq space. The IPI domain
and the device domain request N interrupts from the GIC domain at
allocation time.
So when you allocate from the device domain, you tell the parent
domain, that this is actually a device interrupt, and from the IPI
domain you tell it it's an IPI.
So the allocator in the root domain can decide from which space of the
bitmap to allocate.
if (device) {
hwirq = translate_from_dt(arg);
if (test_and_set_bit(_irqs, hwirq))
return -EBUSY;
} else {
start = first_ipi_irq;
end = last_ipi_irq + 1;
hwirq = bitmap_find_next_zero_area(allocated_irqs, start, end,
nrirqs, 0);
}
So that gives you a consecutive hw irq space for your IPI.
This is potentially dangerous. If a device allocation happens after an
IPI allocation, and that device wants the hwirq at the region that IPI
just allocated because it thought it's free, it'd fail.
Generally it's hard to know whether a real device is connected to a
hwirq or not. I am saving a patch where we get a set of free hwirqs from
DT as only the SoC designer knows what hwirq are actually free and safe
to use for IPI. I'll send this patch with the DT IPI changes or the
rproc driver that I will be send once these changes are merged.
The current code assumes that the last 2 * NR_CPUs hwirqs are always
free to use for Linux SMP.
That makes a lot of things simpler. You don't have to keep a mapping
of the hwirq to the target cpu. You just can use the base hwirq and
calculate the destination hwirq from there when sending an IPI
(general Linux ones). The coprocessor one will just be a natural
fallout.
Are you suggesting here to remove the whole new mapping API from the
generic code or just that it's not necessary to use it in my case?
So if you have the following in the generic ipi code:
void ipi_send_single(unsigned int irq, unsigned int cpu)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
if (chip->ipi_send_single)
chip->ipi_send_single(data, cpu);
else
chip->ipi_send_mask(data, cpumask_of(cpu));
}
void ipi_send_mask(unsigned int irq, const struct cpumask *dest)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
int cpu;
if (!chip->ipi_send_mask) {
for_each_cpu(cpu, dest)
chip->ipi_send_single(data, cpu);
} else {
chip->ipi_send_mask(data, dest);
}
}
void ipi_send_coproc_mask(unsigned int irq, const struct ipi_mask *dest)
{
Fill in the obvious code.
}
And your ipi_send_single() callback just boils down to:
{
target = data->hw_irq + cpu;
tweak_chip_regs(target);
}
Sanity checks omitted for brevity.
I'm confused here as well. Is this a complementary API or are you
suggesting replacing the one this patch introduces?
And that whole thing works for your case and for the case where we
only have a single per cpu interrupt descriptor allocated. The
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On 11/07/2015 02:51 PM, Thomas Gleixner wrote:
On Tue, 3 Nov 2015, Qais Yousef wrote:
Add a new ipi domain on top of the normal domain.
MIPS GIC now supports dynamic allocation of an IPI.
I don't think you make use of the power of hierarchical irq
domains. You just whacked the current code into submission.
Let me explain it to you how that should look like and how that's
going to make the code way simpler.
The root domain is the GIC itself. It provides an allocation mechanism
for all GIC interrupts, global, ipi and per cpu plus the basic
management of them.
So the GIC domain looks at the complete hardware irq space. Now that
irq space is first partioned into local and shared interrupts.
- hwirq MAX
Shared interrupts
- hwirq 6
Local interrupts
- hwirq 0
So that shared interrupt space is where your device interrupts and the
ipi interrupts come from. That local interrupt space seems to be
hardwired, so it'd be overkill to handle that in an extra domain.
I assume that that shared interrupt space is partitioned as well
because the potential device interrupts on the SoC are hardwired. So
the picture looks like this:
- hwirq MAX
Shared assignable interrupts
- hwirq X
Shared device interrupts
- hwirq 6
Local interrupts
- hwirq 0
So if we look at the resulting hierarchy it looks like this:
|- [IPI domain]
[ GIC domain] -
|- [device domain]
The GIC domain manages a bitmap of the full irq space. The IPI domain
and the device domain request N interrupts from the GIC domain at
allocation time.
So when you allocate from the device domain, you tell the parent
domain, that this is actually a device interrupt, and from the IPI
domain you tell it it's an IPI.
So the allocator in the root domain can decide from which space of the
bitmap to allocate.
if (device) {
hwirq = translate_from_dt(arg);
if (test_and_set_bit(_irqs, hwirq))
return -EBUSY;
} else {
start = first_ipi_irq;
end = last_ipi_irq + 1;
hwirq = bitmap_find_next_zero_area(allocated_irqs, start, end,
nrirqs, 0);
}
So that gives you a consecutive hw irq space for your IPI.
This is potentially dangerous. If a device allocation happens after an
IPI allocation, and that device wants the hwirq at the region that IPI
just allocated because it thought it's free, it'd fail.
Generally it's hard to know whether a real device is connected to a
hwirq or not. I am saving a patch where we get a set of free hwirqs from
DT as only the SoC designer knows what hwirq are actually free and safe
to use for IPI. I'll send this patch with the DT IPI changes or the
rproc driver that I will be send once these changes are merged.
The current code assumes that the last 2 * NR_CPUs hwirqs are always
free to use for Linux SMP.
That makes a lot of things simpler. You don't have to keep a mapping
of the hwirq to the target cpu. You just can use the base hwirq and
calculate the destination hwirq from there when sending an IPI
(general Linux ones). The coprocessor one will just be a natural
fallout.
Are you suggesting here to remove the whole new mapping API from the
generic code or just that it's not necessary to use it in my case?
So if you have the following in the generic ipi code:
void ipi_send_single(unsigned int irq, unsigned int cpu)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
if (chip->ipi_send_single)
chip->ipi_send_single(data, cpu);
else
chip->ipi_send_mask(data, cpumask_of(cpu));
}
void ipi_send_mask(unsigned int irq, const struct cpumask *dest)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
int cpu;
if (!chip->ipi_send_mask) {
for_each_cpu(cpu, dest)
chip->ipi_send_single(data, cpu);
} else {
chip->ipi_send_mask(data, dest);
}
}
void ipi_send_coproc_mask(unsigned int irq, const struct ipi_mask *dest)
{
Fill in the obvious code.
}
And your ipi_send_single() callback just boils down to:
{
target = data->hw_irq + cpu;
tweak_chip_regs(target);
}
Sanity checks omitted for brevity.
I'm confused here as well. Is this a complementary API or are you
suggesting replacing the one this patch introduces?
And that whole thing works for your case and for the case where we
only have a single per cpu interrupt descriptor allocated. The
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Tue, 3 Nov 2015, Qais Yousef wrote:
> Add a new ipi domain on top of the normal domain.
>
> MIPS GIC now supports dynamic allocation of an IPI.
I don't think you make use of the power of hierarchical irq
domains. You just whacked the current code into submission.
Let me explain it to you how that should look like and how that's
going to make the code way simpler.
The root domain is the GIC itself. It provides an allocation mechanism
for all GIC interrupts, global, ipi and per cpu plus the basic
management of them.
So the GIC domain looks at the complete hardware irq space. Now that
irq space is first partioned into local and shared interrupts.
- hwirq MAX
Shared interrupts
- hwirq 6
Local interrupts
- hwirq 0
So that shared interrupt space is where your device interrupts and the
ipi interrupts come from. That local interrupt space seems to be
hardwired, so it'd be overkill to handle that in an extra domain.
I assume that that shared interrupt space is partitioned as well
because the potential device interrupts on the SoC are hardwired. So
the picture looks like this:
- hwirq MAX
Shared assignable interrupts
- hwirq X
Shared device interrupts
- hwirq 6
Local interrupts
- hwirq 0
So if we look at the resulting hierarchy it looks like this:
|- [IPI domain]
[ GIC domain] -
|- [device domain]
The GIC domain manages a bitmap of the full irq space. The IPI domain
and the device domain request N interrupts from the GIC domain at
allocation time.
So when you allocate from the device domain, you tell the parent
domain, that this is actually a device interrupt, and from the IPI
domain you tell it it's an IPI.
So the allocator in the root domain can decide from which space of the
bitmap to allocate.
if (device) {
hwirq = translate_from_dt(arg);
if (test_and_set_bit(_irqs, hwirq))
return -EBUSY;
} else {
start = first_ipi_irq;
end = last_ipi_irq + 1;
hwirq = bitmap_find_next_zero_area(allocated_irqs, start, end,
nrirqs, 0);
}
So that gives you a consecutive hw irq space for your IPI.
That makes a lot of things simpler. You don't have to keep a mapping
of the hwirq to the target cpu. You just can use the base hwirq and
calculate the destination hwirq from there when sending an IPI
(general Linux ones). The coprocessor one will just be a natural
fallout.
So if you have the following in the generic ipi code:
void ipi_send_single(unsigned int irq, unsigned int cpu)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
if (chip->ipi_send_single)
chip->ipi_send_single(data, cpu);
else
chip->ipi_send_mask(data, cpumask_of(cpu));
}
void ipi_send_mask(unsigned int irq, const struct cpumask *dest)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
int cpu;
if (!chip->ipi_send_mask) {
for_each_cpu(cpu, dest)
chip->ipi_send_single(data, cpu);
} else {
chip->ipi_send_mask(data, dest);
}
}
void ipi_send_coproc_mask(unsigned int irq, const struct ipi_mask *dest)
{
Fill in the obvious code.
}
And your ipi_send_single() callback just boils down to:
{
target = data->hw_irq + cpu;
tweak_chip_regs(target);
}
Sanity checks omitted for brevity.
And that whole thing works for your case and for the case where we
only have a single per cpu interrupt descriptor allocated. The irq
descriptor based variants are exactly the same thing.
So now for the irq chips of your device and IPI domains. You can
either set the proper GIC chip variant or in case you need some extra
magic for one of the domains, you implement your own special chip
which can have some of its callback implemented by the existing
irq_***_parent() variants.
That gets rid of all your extra mappings, bitmaps and whatever add ons
you have duct taped into the existing GIC code.
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] irqchip/mips-gic: Add a IPI hierarchy domain
On Tue, 3 Nov 2015, Qais Yousef wrote:
> Add a new ipi domain on top of the normal domain.
>
> MIPS GIC now supports dynamic allocation of an IPI.
I don't think you make use of the power of hierarchical irq
domains. You just whacked the current code into submission.
Let me explain it to you how that should look like and how that's
going to make the code way simpler.
The root domain is the GIC itself. It provides an allocation mechanism
for all GIC interrupts, global, ipi and per cpu plus the basic
management of them.
So the GIC domain looks at the complete hardware irq space. Now that
irq space is first partioned into local and shared interrupts.
- hwirq MAX
Shared interrupts
- hwirq 6
Local interrupts
- hwirq 0
So that shared interrupt space is where your device interrupts and the
ipi interrupts come from. That local interrupt space seems to be
hardwired, so it'd be overkill to handle that in an extra domain.
I assume that that shared interrupt space is partitioned as well
because the potential device interrupts on the SoC are hardwired. So
the picture looks like this:
- hwirq MAX
Shared assignable interrupts
- hwirq X
Shared device interrupts
- hwirq 6
Local interrupts
- hwirq 0
So if we look at the resulting hierarchy it looks like this:
|- [IPI domain]
[ GIC domain] -
|- [device domain]
The GIC domain manages a bitmap of the full irq space. The IPI domain
and the device domain request N interrupts from the GIC domain at
allocation time.
So when you allocate from the device domain, you tell the parent
domain, that this is actually a device interrupt, and from the IPI
domain you tell it it's an IPI.
So the allocator in the root domain can decide from which space of the
bitmap to allocate.
if (device) {
hwirq = translate_from_dt(arg);
if (test_and_set_bit(_irqs, hwirq))
return -EBUSY;
} else {
start = first_ipi_irq;
end = last_ipi_irq + 1;
hwirq = bitmap_find_next_zero_area(allocated_irqs, start, end,
nrirqs, 0);
}
So that gives you a consecutive hw irq space for your IPI.
That makes a lot of things simpler. You don't have to keep a mapping
of the hwirq to the target cpu. You just can use the base hwirq and
calculate the destination hwirq from there when sending an IPI
(general Linux ones). The coprocessor one will just be a natural
fallout.
So if you have the following in the generic ipi code:
void ipi_send_single(unsigned int irq, unsigned int cpu)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
if (chip->ipi_send_single)
chip->ipi_send_single(data, cpu);
else
chip->ipi_send_mask(data, cpumask_of(cpu));
}
void ipi_send_mask(unsigned int irq, const struct cpumask *dest)
{
struct irq_desc *desc = irq_to_desc(irq);
struct irq_data *data = irq_desc_get_irq_data(desc);
struct irq_chip *chip = irq_data_get_irq_chip(data);
int cpu;
if (!chip->ipi_send_mask) {
for_each_cpu(cpu, dest)
chip->ipi_send_single(data, cpu);
} else {
chip->ipi_send_mask(data, dest);
}
}
void ipi_send_coproc_mask(unsigned int irq, const struct ipi_mask *dest)
{
Fill in the obvious code.
}
And your ipi_send_single() callback just boils down to:
{
target = data->hw_irq + cpu;
tweak_chip_regs(target);
}
Sanity checks omitted for brevity.
And that whole thing works for your case and for the case where we
only have a single per cpu interrupt descriptor allocated. The irq
descriptor based variants are exactly the same thing.
So now for the irq chips of your device and IPI domains. You can
either set the proper GIC chip variant or in case you need some extra
magic for one of the domains, you implement your own special chip
which can have some of its callback implemented by the existing
irq_***_parent() variants.
That gets rid of all your extra mappings, bitmaps and whatever add ons
you have duct taped into the existing GIC code.
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
