| Message ID | 1440419959-14315-2-git-send-email-qais.yousef@imgtec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 24 Aug 2015, Qais Yousef wrote: > Some drivers might require to send ipi to other cores. So export it. Which IPIs do you need to send from a driver which are not exposed by the SMP functions already? > This will be used later by AXD driver. That smells fishy and it wants a proper explanation WHY and not just a sloppy statement that it will be used later. I can figure that out myself as exporting a function without using it does not make any sense. Thanks, tglx
On 08/24/2015 01:49 PM, Thomas Gleixner wrote: > On Mon, 24 Aug 2015, Qais Yousef wrote: > >> Some drivers might require to send ipi to other cores. So export it. > Which IPIs do you need to send from a driver which are not exposed by > the SMP functions already? It's not an SMP IPI. We use GIC to exchange interrupts between AXD and the host system since AXD is another MIPS core in the cluster. >> This will be used later by AXD driver. > That smells fishy and it wants a proper explanation WHY and not just a > sloppy statement that it will be used later. I can figure that out > myself as exporting a function without using it does not make any sense. Sorry for the terse explanation. As pointed above AXD uses GIC to send and receive interrupts to the host core. Without this change I can't compile the driver as a driver module because the symbol is not exported. Does this make things clearer? Thanks, Qais > > Thanks, > > tglx
On 24/08/15 14:02, Qais Yousef wrote: > On 08/24/2015 01:49 PM, Thomas Gleixner wrote: >> On Mon, 24 Aug 2015, Qais Yousef wrote: >> >>> Some drivers might require to send ipi to other cores. So export it. >> Which IPIs do you need to send from a driver which are not exposed by >> the SMP functions already? > > It's not an SMP IPI. We use GIC to exchange interrupts between AXD and > the host system since AXD is another MIPS core in the cluster. So is this the case of another CPU in the system that is not under control of Linux, but that you need to signal anyway? How do you agree on the IPI number between the two systems? >>> This will be used later by AXD driver. >> That smells fishy and it wants a proper explanation WHY and not just a >> sloppy statement that it will be used later. I can figure that out >> myself as exporting a function without using it does not make any sense. > > Sorry for the terse explanation. As pointed above AXD uses GIC to send > and receive interrupts to the host core. Without this change I can't > compile the driver as a driver module because the symbol is not exported. > > Does this make things clearer? To me, it feels like this is yet another case of routing interrupts to another agent in the system, which is not a CPU under the kernel's control. There is at least two other platforms doing similar craziness (a Freescale platform, and at least one Nvidia). I'd rather see something more "architected" than this blind export, or at least some level of filtering (the idea random drivers can access such a low-level function doesn't make me feel very good). Thanks, M.
On 08/24/2015 02:32 PM, Marc Zyngier wrote: > On 24/08/15 14:02, Qais Yousef wrote: >> On 08/24/2015 01:49 PM, Thomas Gleixner wrote: >>> On Mon, 24 Aug 2015, Qais Yousef wrote: >>> >>>> Some drivers might require to send ipi to other cores. So export it. >>> Which IPIs do you need to send from a driver which are not exposed by >>> the SMP functions already? >> It's not an SMP IPI. We use GIC to exchange interrupts between AXD and >> the host system since AXD is another MIPS core in the cluster. > So is this the case of another CPU in the system that is not under > control of Linux, but that you need to signal anyway? How do you agree > on the IPI number between the two systems? When Linux loads AXD firmware into memory it places the GIC numbers to use at a specific offset there as part of the startup protocol. When AXD starts running it will see these values and use them to send and receive interrupts. > >>>> This will be used later by AXD driver. >>> That smells fishy and it wants a proper explanation WHY and not just a >>> sloppy statement that it will be used later. I can figure that out >>> myself as exporting a function without using it does not make any sense. >> Sorry for the terse explanation. As pointed above AXD uses GIC to send >> and receive interrupts to the host core. Without this change I can't >> compile the driver as a driver module because the symbol is not exported. >> >> Does this make things clearer? > To me, it feels like this is yet another case of routing interrupts to > another agent in the system, which is not a CPU under the kernel's > control. There is at least two other platforms doing similar craziness > (a Freescale platform, and at least one Nvidia). > > I'd rather see something more "architected" than this blind export, or > at least some level of filtering (the idea random drivers can access > such a low-level function doesn't make me feel very good). I don't know how to architect this better or how to perform the filtering, but I'm happy to hear suggestions and try them out. Keep in mind that detecting GIC and writing your own gic_send_ipi() is very simple. I have done this when the driver was out of tree. So restricting it by not exporting it will not prevent someone from really accessing the functionality, it's just they have to do it their own way. Thanks, Qais > > Thanks, > > M.
On Mon, 24 Aug 2015, Qais Yousef wrote: > On 08/24/2015 01:49 PM, Thomas Gleixner wrote: > > On Mon, 24 Aug 2015, Qais Yousef wrote: > > > > > Some drivers might require to send ipi to other cores. So export it. > > Which IPIs do you need to send from a driver which are not exposed by > > the SMP functions already? > > It's not an SMP IPI. We use GIC to exchange interrupts between AXD and the > host system since AXD is another MIPS core in the cluster. So that should have been in the changelog to begin with. > > > This will be used later by AXD driver. > > That smells fishy and it wants a proper explanation WHY and not just a > > sloppy statement that it will be used later. I can figure that out > > myself as exporting a function without using it does not make any sense. > > Sorry for the terse explanation. As pointed above AXD uses GIC to send and > receive interrupts to the host core. Without this change I can't compile the > driver as a driver module because the symbol is not exported. Really? Exporting it solves that problem then. That's interesting news for me. Thanks, tglx
On Mon, 24 Aug 2015, Qais Yousef wrote: > On 08/24/2015 02:32 PM, Marc Zyngier wrote: > > I'd rather see something more "architected" than this blind export, or > > at least some level of filtering (the idea random drivers can access > > such a low-level function doesn't make me feel very good). > > I don't know how to architect this better or how to perform the filtering, > but I'm happy to hear suggestions and try them out. > Keep in mind that detecting GIC and writing your own gic_send_ipi() is very > simple. I have done this when the driver was out of tree. So restricting it by > not exporting it will not prevent someone from really accessing the > functionality, it's just they have to do it their own way. Keep in mind that we are not talking about out of tree hackery. We talk about a kernel code submission and I doubt, that you will get away with a GIC detection/fiddling burried in your driver code. Keep in mind that just slapping an export to some random function is not much better than doing a GIC hack in the driver. Marcs concerns about blindly exposing IPI functionality to drivers is well justified and that kind of coprocessor stuff is not unique to your particular SoC. We're going to see such things more frequently in the not so distant future, so we better think now about proper solutions to that problem. There are a couple of issues to solve: 1) How is the IPI which is received by the coprocessor reserved in the system? 2) How is it associated to a particular driver? 3) How do we ensure that a driver cannot issue random IPIs and can only send the associated ones? None of these issues are handled by your export. So we need a core infrastructure which allows us to do that. The requirements are pretty clear from the above and Marc might have some further restrictions in mind. Thanks, tglx
On 08/24/2015 03:55 PM, Thomas Gleixner wrote: > On Mon, 24 Aug 2015, Qais Yousef wrote: >> On 08/24/2015 01:49 PM, Thomas Gleixner wrote: >>> On Mon, 24 Aug 2015, Qais Yousef wrote: >>> >>>> Some drivers might require to send ipi to other cores. So export it. >>> Which IPIs do you need to send from a driver which are not exposed by >>> the SMP functions already? >> It's not an SMP IPI. We use GIC to exchange interrupts between AXD and the >> host system since AXD is another MIPS core in the cluster. > So that should have been in the changelog to begin with. > OK sorry for the confusion. I'll amend the changelog and be more careful in the future. Thanks, Qais >>>> This will be used later by AXD driver. >>> That smells fishy and it wants a proper explanation WHY and not just a >>> sloppy statement that it will be used later. I can figure that out >>> myself as exporting a function without using it does not make any sense. >> Sorry for the terse explanation. As pointed above AXD uses GIC to send and >> receive interrupts to the host core. Without this change I can't compile the >> driver as a driver module because the symbol is not exported. > Really? Exporting it solves that problem then. That's interesting news > for me. > > Thanks, > > tglx
On 08/24/2015 04:07 PM, Thomas Gleixner wrote: > On Mon, 24 Aug 2015, Qais Yousef wrote: >> On 08/24/2015 02:32 PM, Marc Zyngier wrote: >>> I'd rather see something more "architected" than this blind export, or >>> at least some level of filtering (the idea random drivers can access >>> such a low-level function doesn't make me feel very good). >> I don't know how to architect this better or how to perform the filtering, >> but I'm happy to hear suggestions and try them out. >> Keep in mind that detecting GIC and writing your own gic_send_ipi() is very >> simple. I have done this when the driver was out of tree. So restricting it by >> not exporting it will not prevent someone from really accessing the >> functionality, it's just they have to do it their own way. > Keep in mind that we are not talking about out of tree hackery. We > talk about a kernel code submission and I doubt, that you will get > away with a GIC detection/fiddling burried in your driver code. > > Keep in mind that just slapping an export to some random function is > not much better than doing a GIC hack in the driver. > > Marcs concerns about blindly exposing IPI functionality to drivers is > well justified and that kind of coprocessor stuff is not unique to > your particular SoC. We're going to see such things more frequently in > the not so distant future, so we better think now about proper > solutions to that problem. Sure I'm not trying to argue against that. > > There are a couple of issues to solve: > > 1) How is the IPI which is received by the coprocessor reserved in the > system? > > 2) How is it associated to a particular driver? Shouldn't 'interrupts' property in DT take care of these 2 questions? Maybe we can give it an alias name to make it more readable that this interrupt is requested for external IPI. > > 3) How do we ensure that a driver cannot issue random IPIs and can > only send the associated ones? If we get the irq number from DT then I'm not sure how feasible it is to implement a generic_send_ipi() function that takes this number to generate an IPI. Do you think this approach would work? > > None of these issues are handled by your export. > > So we need a core infrastructure which allows us to do that. The > requirements are pretty clear from the above and Marc might have some > further restrictions in mind. Another issue I'm having which is related is that I need to communicate these GIC irq numbers to AXD core when it starts up. So the logic is that these IPIs are not hardwired and it's up to the system designer to allocate 2 free GIC irqs to be used for that purpose. At the moment I have my own DT property to take these numbers. Hopefully this link would explain the issue. See the question about gic-irq property. https://lkml.org/lkml/2015/8/24/459 From what I know there's no generic way for the driver to get the hw irq number from linux irq number unless I missed something. Is it possible to add something to support this? Or maybe there's something but I failed to find? Thanks, Qais > > Thanks, > > tglx
[adding Mark Rutland, as this is heading straight into uncharted DT territory] On 24/08/15 17:39, Qais Yousef wrote: > On 08/24/2015 04:07 PM, Thomas Gleixner wrote: >> On Mon, 24 Aug 2015, Qais Yousef wrote: >>> On 08/24/2015 02:32 PM, Marc Zyngier wrote: >>>> I'd rather see something more "architected" than this blind export, or >>>> at least some level of filtering (the idea random drivers can access >>>> such a low-level function doesn't make me feel very good). >>> I don't know how to architect this better or how to perform the filtering, >>> but I'm happy to hear suggestions and try them out. >>> Keep in mind that detecting GIC and writing your own gic_send_ipi() is very >>> simple. I have done this when the driver was out of tree. So restricting it by >>> not exporting it will not prevent someone from really accessing the >>> functionality, it's just they have to do it their own way. >> Keep in mind that we are not talking about out of tree hackery. We >> talk about a kernel code submission and I doubt, that you will get >> away with a GIC detection/fiddling burried in your driver code. >> >> Keep in mind that just slapping an export to some random function is >> not much better than doing a GIC hack in the driver. >> >> Marcs concerns about blindly exposing IPI functionality to drivers is >> well justified and that kind of coprocessor stuff is not unique to >> your particular SoC. We're going to see such things more frequently in >> the not so distant future, so we better think now about proper >> solutions to that problem. > > Sure I'm not trying to argue against that. > >> >> There are a couple of issues to solve: >> >> 1) How is the IPI which is received by the coprocessor reserved in the >> system? >> >> 2) How is it associated to a particular driver? > > Shouldn't 'interrupts' property in DT take care of these 2 questions? > Maybe we can give it an alias name to make it more readable that this > interrupt is requested for external IPI. The "interrupts" property has a rather different meaning, and isn't designed to hardcode IPIs. Also, this property describes an interrupt from a device to the CPU, not the other way around (I imagine you also have an interrupt coming from the AXD to the CPU, possibly using an IPI too). We can deal with these issues, but that's not something we can improvise. What I had in mind was something fairly generic: - interrupt-source: something generating an interrupt - interrupt-sink: something being targeted by an interrupt You could then express things like: intc: interrupt-controller@1000 { interrupt-controller; }; mydevice@f0000000 { interrupt-source = <&intc INT_SPEC 2 &inttarg1 &inttarg1>; }; inttarg1: mydevice@f1000000 { interrupt-sink = <&intc HWAFFINITY1>; }; inttarg2: cpu@1 { interrupt-sink = <&intc HWAFFINITY2>; }; You could also imagine having CPUs being both source and sink. >> >> 3) How do we ensure that a driver cannot issue random IPIs and can >> only send the associated ones? > > If we get the irq number from DT then I'm not sure how feasible it is to > implement a generic_send_ipi() function that takes this number to > generate an IPI. > > Do you think this approach would work? If you follow the above approach, it should be pretty easy to derive a source identifier and a sink identifier from the DT, and have the core code to route one to the other and do the right thing. The source identifier could also be used to describe an IPI in a fairly safe way (the target being fixed by DT, but the actual number used dynamically allocated by the kernel). This is just a 10 minutes braindump, so feel free to throw rocks at it and to come up with a better solution! :-) Thanks, M.
On 08/24/2015 06:17 PM, Marc Zyngier wrote: > [adding Mark Rutland, as this is heading straight into uncharted DT > territory] > > On 24/08/15 17:39, Qais Yousef wrote: >> On 08/24/2015 04:07 PM, Thomas Gleixner wrote: >>> On Mon, 24 Aug 2015, Qais Yousef wrote: >>>> On 08/24/2015 02:32 PM, Marc Zyngier wrote: >>>>> I'd rather see something more "architected" than this blind export, or >>>>> at least some level of filtering (the idea random drivers can access >>>>> such a low-level function doesn't make me feel very good). >>>> I don't know how to architect this better or how to perform the filtering, >>>> but I'm happy to hear suggestions and try them out. >>>> Keep in mind that detecting GIC and writing your own gic_send_ipi() is very >>>> simple. I have done this when the driver was out of tree. So restricting it by >>>> not exporting it will not prevent someone from really accessing the >>>> functionality, it's just they have to do it their own way. >>> Keep in mind that we are not talking about out of tree hackery. We >>> talk about a kernel code submission and I doubt, that you will get >>> away with a GIC detection/fiddling burried in your driver code. >>> >>> Keep in mind that just slapping an export to some random function is >>> not much better than doing a GIC hack in the driver. >>> >>> Marcs concerns about blindly exposing IPI functionality to drivers is >>> well justified and that kind of coprocessor stuff is not unique to >>> your particular SoC. We're going to see such things more frequently in >>> the not so distant future, so we better think now about proper >>> solutions to that problem. >> Sure I'm not trying to argue against that. >> >>> There are a couple of issues to solve: >>> >>> 1) How is the IPI which is received by the coprocessor reserved in the >>> system? >>> >>> 2) How is it associated to a particular driver? >> Shouldn't 'interrupts' property in DT take care of these 2 questions? >> Maybe we can give it an alias name to make it more readable that this >> interrupt is requested for external IPI. > The "interrupts" property has a rather different meaning, and isn't > designed to hardcode IPIs. Also, this property describes an interrupt > from a device to the CPU, not the other way around (I imagine you also > have an interrupt coming from the AXD to the CPU, possibly using an IPI > too). Yes we have an interrupt from AXD to the CPU. But the way I take care of the routing at the moment is that the CPU routes the interrupt it receives from AXD. And AXD routes the interrupt it receives from the CPU. This is useful because in MIPS GIC the routing is done per hw thread on the core so this gives the flexibility for each one to choose what it suits it best. > > We can deal with these issues, but that's not something we can improvise. > > What I had in mind was something fairly generic: > - interrupt-source: something generating an interrupt > - interrupt-sink: something being targeted by an interrupt > > You could then express things like: > > intc: interrupt-controller@1000 { > interrupt-controller; > }; > > mydevice@f0000000 { > interrupt-source = <&intc INT_SPEC 2 &inttarg1 &inttarg1>; > }; To make sure we're on the same page. INT_SPEC here refers to the arguments we pass to a standard interrupts property, right? > > inttarg1: mydevice@f1000000 { > interrupt-sink = <&intc HWAFFINITY1>; > }; > > inttarg2: cpu@1 { > interrupt-sink = <&intc HWAFFINITY2>; > }; And HWAFFINITY here is the core (or hardware thread) this interrupt will be routed to? So for my case where CPU is on core 0 and AXD is on core 1 my description will look like cpu: cpu@0 { interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING 1 &axd>; interrupt-sink = <&gic 0>; } axd: axd { interrupt-source = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING 1 &cpu>; interrupt-sink = <&gic 1>; } If I didn't misunderstand you, the issue I see with this is that the information about which IRQ to use to send an IPI to AXD is not present in the AXD node. We will need to search the cpu node for something that is meant to be routed to axd or have some logic to implicitly infer it from interrupt-sink in axd node. Not convenient IMO. Can we replace 'something' in interrupt-source and interrupt-sink definitions to 'host' or 'CPU' or do we really care about creating IPI between any 2 'things'? Changing the definition will also make interrupt-sink a synonym/alias to interrupts property. So the description will become axd: axd { interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING>; /* interrupt from CPU to AXD */ interrupt-sink = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING>; /* interrupt from AXD to CPU */ } But this assume Linux won't take care of the routing. If we want Linux to take care of the routing, maybe something like this then? axd: axd { interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING HWAFFINITY1>; /* interrupt from CPU to AXD@HWAFFINITY1*/ interrupt-sink = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING HWAFFINITY2>; /* interrupt from AXD to CPU@HWAFFINITY2 */ } I don't think it's necessary to specify the HWAFFINITY2 for interrupt-sink as linux can use SMP affinity to move it around but we can make it optional in case there's a need to hardcode it to a specific Linux core. Or maybe the driver can use affinity hint.. > > You could also imagine having CPUs being both source and sink. > >>> 3) How do we ensure that a driver cannot issue random IPIs and can >>> only send the associated ones? >> If we get the irq number from DT then I'm not sure how feasible it is to >> implement a generic_send_ipi() function that takes this number to >> generate an IPI. >> >> Do you think this approach would work? > If you follow the above approach, it should be pretty easy to derive a > source identifier and a sink identifier from the DT, and have the core > code to route one to the other and do the right thing. Do you think it's better for linux to take care of all the routing instead of each core doing its own routing? If Linux to do the routing for the other core (even if optionally), what's the mechanism to do that? We can't use irq_set_affinity() because we want to map something that is not part of Linux. A new mapping function in struct irq_domain_ops maybe? > > The source identifier could also be used to describe an IPI in a fairly > safe way (the target being fixed by DT, but the actual number used > dynamically allocated by the kernel). To be dynamic, then the interrupt controller must specify which interrupts are actually free to use. What if the DT doesn't describe all the hardawre that is connected to GIC and Linux thinks its free to use but actually it's connected to a real hardware but no one told us about? I think since this information will always have to be looked up maybe it's better to give the responsibility to the user to specify something they know will work explicitly. > > This is just a 10 minutes braindump, so feel free to throw rocks at it > and to come up with a better solution! :-) Thanks for that. My brain is tied down to my use case to come up with something generic easily :-) Any pointers on the best way to tie gic_send_ipi() with the driver/core code? The way it's currently tied to the core code is through SMP IPI functions which I don't think we can use. I'm thinking adding a pointer function in struct irq_chip would be the easiest approach maybe? Thanks, Qais > > Thanks, > > M.
On Wed, 26 Aug 2015, Qais Yousef wrote: > Can we replace 'something' in interrupt-source and interrupt-sink definitions > to 'host' or 'CPU' or do we really care about creating IPI between any 2 > 'things'? > > Changing the definition will also make interrupt-sink a synonym/alias to > interrupts property. So the description will become > > axd: axd { > interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING>; /* > interrupt from CPU to AXD */ > interrupt-sink = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING>; /* > interrupt from AXD to CPU */ > } > > But this assume Linux won't take care of the routing. If we want Linux to take > care of the routing, maybe something like this then? > > axd: axd { > interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING > HWAFFINITY1>; /* interrupt from CPU to > AXD@HWAFFINITY1*/ > interrupt-sink = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING > HWAFFINITY2>; /* interrupt from AXD to CPU@HWAFFINITY2 */ > } > > I don't think it's necessary to specify the HWAFFINITY2 for interrupt-sink as > linux can use SMP affinity to move it around but we can make it optional in > case there's a need to hardcode it to a specific Linux core. Or maybe the > driver can use affinity hint.. Wrong. You cannot move an IPI around with set_affinity. It's possible to send an IPI to more than one target CPU, but that has nothing to do with affinities. Are you talking about IPIs or about general interrupts which have an affinity setting? > Any pointers on the best way to tie gic_send_ipi() with the driver/core code? > The way it's currently tied to the core code is through SMP IPI functions > which I don't think we can use. I'm thinking adding a pointer function in > struct irq_chip would be the easiest approach maybe? That's the least of our worries. We need to get the high level interfaces and the devicetree mechanism straight before we talk about this kind of details. Thanks, tglx
On 08/26/2015 02:19 PM, Thomas Gleixner wrote: > On Wed, 26 Aug 2015, Qais Yousef wrote: >> Can we replace 'something' in interrupt-source and interrupt-sink definitions >> to 'host' or 'CPU' or do we really care about creating IPI between any 2 >> 'things'? >> >> Changing the definition will also make interrupt-sink a synonym/alias to >> interrupts property. So the description will become >> >> axd: axd { >> interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING>; /* >> interrupt from CPU to AXD */ >> interrupt-sink = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING>; /* >> interrupt from AXD to CPU */ >> } >> >> But this assume Linux won't take care of the routing. If we want Linux to take >> care of the routing, maybe something like this then? >> >> axd: axd { >> interrupt-source = <&gic GIC_SHARED 36 IRQ_TYPE_EDGE_RISING >> HWAFFINITY1>; /* interrupt from CPU to >> AXD@HWAFFINITY1*/ >> interrupt-sink = <&gic GIC_SHARED 37 IRQ_TYPE_EDGE_RISING >> HWAFFINITY2>; /* interrupt from AXD to CPU@HWAFFINITY2 */ >> } >> >> I don't think it's necessary to specify the HWAFFINITY2 for interrupt-sink as >> linux can use SMP affinity to move it around but we can make it optional in >> case there's a need to hardcode it to a specific Linux core. Or maybe the >> driver can use affinity hint.. > Wrong. You cannot move an IPI around with set_affinity. It's possible > to send an IPI to more than one target CPU, but that has nothing to do > with affinities. > > Are you talking about IPIs or about general interrupts which have an > affinity setting? Maybe my view of the world is limited. I wrote this because the mechanism to route an IPI and set affinities is the same. So specifying which core or hardware thread should Linux CPU route this IPI to is the same as setting the affinity, no? Linux will not move the IPI that is routed to the coprocessor core. Just the IPI it will receive. Also the way I see it is that this is an external interrupt whether it was asserted by real signal or through IPI mechanism and it should be treated as such in terms of moving inside Linux SMP, no? Again maybe my view of the world is limited but I can't see why migrating the interrupt would affect correctness unless there's a hardware limitation like only core 0 can read info from AXD (which is where my suggestion to using affinity hint above to accommodate such limitations). When you say 'It is possible to send an IPI to more than one target CPU', is it a case we need to cater for? The way I was seeing this problem is communication between single Linux SMP and a single coprocessor unit. I didn't think of it as single to many. Even if the coprocessor is a cluster I'd expect it to act as a single unit like Linux SMP. And if it wanted to send 2 different interrupts it will need to use 2 different IPIs. If I'm stating anything obvious above please bear with me. I'm just trying to be clear about my view of the world in case I'm missing something :-) > >> Any pointers on the best way to tie gic_send_ipi() with the driver/core code? >> The way it's currently tied to the core code is through SMP IPI functions >> which I don't think we can use. I'm thinking adding a pointer function in >> struct irq_chip would be the easiest approach maybe? > That's the least of our worries. We need to get the high level > interfaces and the devicetree mechanism straight before we talk about > this kind of details. Fair enough. The reason I asked is to help me start writing some test code but I'll wait. Thanks, Qais > > Thanks, > > tglx
On Wed, 26 Aug 2015, Qais Yousef wrote: > On 08/26/2015 02:19 PM, Thomas Gleixner wrote: > > Wrong. You cannot move an IPI around with set_affinity. It's possible > > to send an IPI to more than one target CPU, but that has nothing to do > > with affinities. > > > > Are you talking about IPIs or about general interrupts which have an > > affinity setting? > > Maybe my view of the world is limited. I wrote this because the mechanism to > route an IPI and set affinities is the same. That might be the case on your particular platform, but that's not generally true. > So specifying which core or hardware thread should Linux CPU route this IPI to > is the same as setting the affinity, no? Linux will not move the IPI that is > routed to the coprocessor core. Just the IPI it will receive. > > Also the way I see it is that this is an external interrupt whether it was > asserted by real signal or through IPI mechanism and it should be treated as > such in terms of moving inside Linux SMP, no? Again maybe my view of the world > is limited but I can't see why migrating the interrupt would affect > correctness unless there's a hardware limitation like only core 0 can read > info from AXD (which is where my suggestion to using affinity hint above to > accommodate such limitations). > > When you say 'It is possible to send an IPI to more than one target CPU', is > it a case we need to cater for? The way I was seeing this problem is > communication between single Linux SMP and a single coprocessor unit. I didn't > think of it as single to many. Even if the coprocessor is a cluster I'd expect > it to act as a single unit like Linux SMP. And if it wanted to send 2 > different interrupts it will need to use 2 different IPIs. You are confusing the terms. IPI = Inter Processor Interrupt As the name says that's an interrupt which goes from one cpu to another. So an IPI has a very clear target. Whether the platform implements IPIs via general interrupts which are made affine to a particular cpu or some other specialized mechanism is completely irrelevant. An IPI is not subject to affinity settings, period. So if you want to use an IPI then you need a target cpu for that IPI. If you want something which can be affined to any cpu, then you need a general interrupt and not an IPI. That's what I asked before and you still did not answer that question. > > Are you talking about IPIs or about general interrupts which have an > > affinity setting? Thanks, tglx
On 08/26/2015 04:08 PM, Thomas Gleixner wrote: > On Wed, 26 Aug 2015, Qais Yousef wrote: >> On 08/26/2015 02:19 PM, Thomas Gleixner wrote: >>> Wrong. You cannot move an IPI around with set_affinity. It's possible >>> to send an IPI to more than one target CPU, but that has nothing to do >>> with affinities. >>> >>> Are you talking about IPIs or about general interrupts which have an >>> affinity setting? >> Maybe my view of the world is limited. I wrote this because the mechanism to >> route an IPI and set affinities is the same. > That might be the case on your particular platform, but that's not > generally true. > >> So specifying which core or hardware thread should Linux CPU route this IPI to >> is the same as setting the affinity, no? Linux will not move the IPI that is >> routed to the coprocessor core. Just the IPI it will receive. >> >> Also the way I see it is that this is an external interrupt whether it was >> asserted by real signal or through IPI mechanism and it should be treated as >> such in terms of moving inside Linux SMP, no? Again maybe my view of the world >> is limited but I can't see why migrating the interrupt would affect >> correctness unless there's a hardware limitation like only core 0 can read >> info from AXD (which is where my suggestion to using affinity hint above to >> accommodate such limitations). >> >> When you say 'It is possible to send an IPI to more than one target CPU', is >> it a case we need to cater for? The way I was seeing this problem is >> communication between single Linux SMP and a single coprocessor unit. I didn't >> think of it as single to many. Even if the coprocessor is a cluster I'd expect >> it to act as a single unit like Linux SMP. And if it wanted to send 2 >> different interrupts it will need to use 2 different IPIs. > You are confusing the terms. > > IPI = Inter Processor Interrupt > > As the name says that's an interrupt which goes from one cpu to > another. So an IPI has a very clear target. OK understood. My interpretation of the processor here was the difference. I was viewing the whole linux cpus as one unit with regard to its coprocessors. > > Whether the platform implements IPIs via general interrupts which are > made affine to a particular cpu or some other specialized mechanism is > completely irrelevant. An IPI is not subject to affinity settings, > period. > > So if you want to use an IPI then you need a target cpu for that IPI. > > If you want something which can be affined to any cpu, then you need a > general interrupt and not an IPI. We are using IPIs to exchange interrupts. Affinity is not important to me. Thanks, Qais > > That's what I asked before and you still did not answer that question. > >>> Are you talking about IPIs or about general interrupts which have an >>> affinity setting? > Thanks, > > tglx
On Wed, 26 Aug 2015, Qais Yousef wrote: > On 08/26/2015 04:08 PM, Thomas Gleixner wrote: > > IPI = Inter Processor Interrupt > > > > As the name says that's an interrupt which goes from one cpu to > > another. So an IPI has a very clear target. > > OK understood. My interpretation of the processor here was the difference. I > was viewing the whole linux cpus as one unit with regard to its coprocessors. You can only view it this way if you talk about peripheral interrupts which are not used as per cpu interrupts and can be routed to a single cpu or a set of cpus via set_affinity. > > Whether the platform implements IPIs via general interrupts which are > > made affine to a particular cpu or some other specialized mechanism is > > completely irrelevant. An IPI is not subject to affinity settings, > > period. > > > > So if you want to use an IPI then you need a target cpu for that IPI. > > > > If you want something which can be affined to any cpu, then you need a > > general interrupt and not an IPI. > > We are using IPIs to exchange interrupts. Affinity is not important to me. That's a bold statement. If you chose CPU x as the target for the interrupts received from the coprocessor, then you have pinned the processing for this stuff on to CPU x. So you limit the freedom of moving stuff around on the linux cpus. And if your root irq controller supports sending normal device interrupts in the same or a similar way as it sends IPIs you can spare quite some extra handling on the linux side for receiving the coprocessor interrupt, i.e. you can use just the bog standard request_irq() mechanism and have the ability to set the affinity of that interrupt from user space so you can move it to the core on which your processing happens. Definitely simpler and more flexible, so I would go there if the hardware allows. But back to the IPIs. We need infrastructure and DT support to: 1) reserve an IPI 2) send an IPI 3) request/free an IPI #1 We have no infrastructure for that, but we definitely need one. We can look at the IPI as a single linux irq number which is replicated on all cpu cores. The replication can happen in hardware or by software, but that depends on the underlying root irq controller. How that is implemented does not matter for the reservation. The most flexible and platform independent solution would be to describe the IPI space as a seperate irq domain. In most cases this would be a hierarchical domain stacked on the root irq domain: [IPI-domain] --> [GIC-MIPS-domain] on x86 this would be: [IPI-domain] --> [vector-domain] That needs some change how the IPIs which are used by the kernel (rescheduling, function call ..) are set up, but we get a proper management and collision avoidance that way. Depending on the platform we could actually remove the whole IPI compile time reservation and hand out IPIs at boot time on demand and dynamically. So the reservation function would be something like: unsigned int irq_reserve_ipi(const struct cpumask *dest, void *devid); @dest contains the possible targets for the IPI. So for generic linux IPIs this would be cpu_possible_mask. For your coprocessor the target would be a cpumask with just the bit of the coprocessor core set. If you need to use an IPI for sending an interrupt from the coprocessor to a specific linux core then @dest will contain just that target cpu. @devid is stored in the IPI domain for sanity checks during operation. The function returns a linux irq number or 0 if allocation fails. We need a complementary interface as well, so you can hand back the IPI to the core when the coprocessor is disabled: void irq_destroy_ipi(unsigned int irq, void *devid); To configure your coprocessor proper, we need a translation mechanism from the linux interrupt number to the magic value which needs to be written into the trigger register when the coprocessor wants to send an interrupt or an IPI. int irq_get_irq_hwcfg(unsigned int irq, struct irq_hwcfg *cfg); struct irq_hwcfg needs to be defined, but it might look like this: { /* Generic fields */ x; ... union { mips_gic; ... }; }; The actual hw specific value(s) need to be filled in from the irq domain specific code. #2 We have no generic mechanism for that either. Something like this is needed: void irq_send_ipi(unsigned int irq, const struct cpumask *dest, void *devid); @dest is for generic linux IPIs and can be NULL so the IPI is sent to the core(s) which have been handed in at reservation time @devid is used to sanity check the driver call. So that finally will call down via a irq chip callback into the code which sends the IPI. #3 Now you get lucky, because we actually have an interface for this request_percpu_irq() free_percpu_irq() disable_percpu_irq() enable_percpu_irq() Though there is a caveat. enable/disable_percpu_irq() must be called from the target cpu, but that should be a solvable problem. And at the IPI-domain side we need sanity checks whether the cpu from which enable/disable is called is actually configured in the reservation mask. There are a few other nasty details, but that's not important for the big picture. As I said above, I really would recommend to avoid that if possible because a bog standard device interrupt is way simpler to deal with. That's certainly not the quick and dirty solution you are looking for, but exposing IPIs to drivers by anything else than a well thought out infrastructure is not going to happen. Thanks, tglx
On 2015/8/27 5:40, Thomas Gleixner wrote: > But back to the IPIs. We need infrastructure and DT support to: > > 1) reserve an IPI > > 2) send an IPI > > 3) request/free an IPI > > #1 We have no infrastructure for that, but we definitely need one. > > We can look at the IPI as a single linux irq number which is > replicated on all cpu cores. The replication can happen in hardware > or by software, but that depends on the underlying root irq > controller. How that is implemented does not matter for the > reservation. > > The most flexible and platform independent solution would be to > describe the IPI space as a seperate irq domain. In most cases this > would be a hierarchical domain stacked on the root irq domain: > > [IPI-domain] --> [GIC-MIPS-domain] > > on x86 this would be: > > [IPI-domain] --> [vector-domain] > > That needs some change how the IPIs which are used by the kernel > (rescheduling, function call ..) are set up, but we get a proper > management and collision avoidance that way. Depending on the > platform we could actually remove the whole IPI compile time > reservation and hand out IPIs at boot time on demand and > dynamically. Hi Thomas, Good point:) That will make the code more clear. Thanks! Gerry
On 08/26/2015 10:40 PM, Thomas Gleixner wrote: > On Wed, 26 Aug 2015, Qais Yousef wrote: >> On 08/26/2015 04:08 PM, Thomas Gleixner wrote: >>> IPI = Inter Processor Interrupt >>> >>> As the name says that's an interrupt which goes from one cpu to >>> another. So an IPI has a very clear target. >> OK understood. My interpretation of the processor here was the difference. I >> was viewing the whole linux cpus as one unit with regard to its coprocessors. > You can only view it this way if you talk about peripheral interrupts > which are not used as per cpu interrupts and can be routed to a single > cpu or a set of cpus via set_affinity. > >>> Whether the platform implements IPIs via general interrupts which are >>> made affine to a particular cpu or some other specialized mechanism is >>> completely irrelevant. An IPI is not subject to affinity settings, >>> period. >>> >>> So if you want to use an IPI then you need a target cpu for that IPI. >>> >>> If you want something which can be affined to any cpu, then you need a >>> general interrupt and not an IPI. >> We are using IPIs to exchange interrupts. Affinity is not important to me. > That's a bold statement. If you chose CPU x as the target for the > interrupts received from the coprocessor, then you have pinned the > processing for this stuff on to CPU x. So you limit the freedom of > moving stuff around on the linux cpus. I said that because I thought you were telling me if I'm expecting my IPIs to be movable then I must be using general interrupts. So what I was saying is that we use IPIs and if it's against the rule for them to have affinity we can live with that. > > And if your root irq controller supports sending normal device > interrupts in the same or a similar way as it sends IPIs you can spare > quite some extra handling on the linux side for receiving the > coprocessor interrupt, i.e. you can use just the bog standard > request_irq() mechanism and have the ability to set the affinity of > that interrupt from user space so you can move it to the core on which > your processing happens. Definitely simpler and more flexible, so I > would go there if the hardware allows. That's what I was trying to say but words failed me to explain it clearly maybe :( > > But back to the IPIs. We need infrastructure and DT support to: > > 1) reserve an IPI > > 2) send an IPI > > 3) request/free an IPI > > #1 We have no infrastructure for that, but we definitely need one. > > We can look at the IPI as a single linux irq number which is > replicated on all cpu cores. The replication can happen in hardware > or by software, but that depends on the underlying root irq > controller. How that is implemented does not matter for the > reservation. > > The most flexible and platform independent solution would be to > describe the IPI space as a seperate irq domain. In most cases this > would be a hierarchical domain stacked on the root irq domain: > > [IPI-domain] --> [GIC-MIPS-domain] > > on x86 this would be: > > [IPI-domain] --> [vector-domain] > > That needs some change how the IPIs which are used by the kernel > (rescheduling, function call ..) are set up, but we get a proper > management and collision avoidance that way. Depending on the > platform we could actually remove the whole IPI compile time > reservation and hand out IPIs at boot time on demand and > dynamically. > > So the reservation function would be something like: > > unsigned int irq_reserve_ipi(const struct cpumask *dest, void *devid); > > @dest contains the possible targets for the IPI. So for generic > linux IPIs this would be cpu_possible_mask. For your coprocessor > the target would be a cpumask with just the bit of the coprocessor > core set. If you need to use an IPI for sending an interrupt from > the coprocessor to a specific linux core then @dest will contain > just that target cpu. > > @devid is stored in the IPI domain for sanity checks during > operation. > > The function returns a linux irq number or 0 if allocation fails. > > We need a complementary interface as well, so you can hand back the > IPI to the core when the coprocessor is disabled: > > void irq_destroy_ipi(unsigned int irq, void *devid); > > > To configure your coprocessor proper, we need a translation > mechanism from the linux interrupt number to the magic value which > needs to be written into the trigger register when the coprocessor > wants to send an interrupt or an IPI. > > int irq_get_irq_hwcfg(unsigned int irq, struct irq_hwcfg *cfg); > > struct irq_hwcfg needs to be defined, but it might look like this: > > { > /* Generic fields */ > x; > ... > union { > mips_gic; > ... > }; > }; > > The actual hw specific value(s) need to be filled in from the irq > domain specific code. > > > #2 We have no generic mechanism for that either. > > Something like this is needed: > > void irq_send_ipi(unsigned int irq, const struct cpumask *dest, > void *devid); > > @dest is for generic linux IPIs and can be NULL so the IPI is sent to > the core(s) which have been handed in at reservation time > > @devid is used to sanity check the driver call. > > So that finally will call down via a irq chip callback into the > code which sends the IPI. > > > #3 Now you get lucky, because we actually have an interface for this > > request_percpu_irq() > free_percpu_irq() > disable_percpu_irq() > enable_percpu_irq() > > Though there is a caveat. enable/disable_percpu_irq() must be > called from the target cpu, but that should be a solvable problem. > > And at the IPI-domain side we need sanity checks whether the cpu > from which enable/disable is called is actually configured in the > reservation mask. > > There are a few other nasty details, but that's not important for > the big picture. > > As I said above, I really would recommend to avoid that if possible > because a bog standard device interrupt is way simpler to deal > with. Agreed. Something for us to think about and consider. But if not for AXD this kind of mechanism is important for us for other reasons so we probably want to see it through. > > That's certainly not the quick and dirty solution you are looking for, > but exposing IPIs to drivers by anything else than a well thought out > infrastructure is not going to happen. Thanks a lot for the detailed explanation. I wasn't looking for a quick and dirty solution but my view of the problem is much simpler than yours so my idea of a solution would look quick and dirty. I have a better appreciation of the problem now and a way to approach it :-) From DT point of view are we OK with this form then coprocessor { interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; } and if the root controller sends normal IPI as it sends normal device interrupts then interrupt-sink can be a standard interrupts property (like in my case) coprocessor { interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; interrupts = <INT_SPEC>; } Does this look right to you? Is there something else that needs to be covered still? One more thing I can think of now is that the coprocessor will need the raw irq numbers that are picked by linux so that it can use them to trigger the IPI. Are we ok to add a function that returns this raw irq number (as opposed to linux irq number) directly from DT? The way this is communicated to the coprocessor will be platform specific. Thanks, Qais > > Thanks, > > tglx
On Fri, 28 Aug 2015, Qais Yousef wrote: > Thanks a lot for the detailed explanation. I wasn't looking for a quick and > dirty solution but my view of the problem is much simpler than yours so my > idea of a solution would look quick and dirty. I have a better appreciation of > the problem now and a way to approach it :-) > > From DT point of view are we OK with this form then > > coprocessor { > interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; > interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; > } > > and if the root controller sends normal IPI as it sends normal device > interrupts then interrupt-sink can be a standard interrupts property (like in > my case) > > coprocessor { > interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; > interrupts = <INT_SPEC>; > } > > Does this look right to you? Is there something else that needs to be covered > still? I'm not an DT wizard. I leave that to the DT experts. > One more thing I can think of now is that the coprocessor will need the raw > irq numbers that are picked by linux so that it can use them to trigger the > IPI. Are we ok to add a function that returns this raw irq number (as opposed > to linux irq number) directly from DT? The way this is communicated to the > coprocessor will be platform specific. Why do you want that to be hacked into DT? > > To configure your coprocessor proper, we need a translation > > mechanism from the linux interrupt number to the magic value which > > needs to be written into the trigger register when the coprocessor > > wants to send an interrupt or an IPI. > > > > int irq_get_irq_hwcfg(unsigned int irq, struct irq_hwcfg *cfg); > > > > struct irq_hwcfg needs to be defined, but it might look like this: > > > > { > > /* Generic fields */ > > x; > > ... > > union { > > mips_gic; > > ... > > }; > > }; That function provides you the information which you have to hand over to your coprocessor firmware. Thanks, tglx
On 08/28/2015 03:22 PM, Thomas Gleixner wrote: >>> To configure your coprocessor proper, we need a translation >>> mechanism from the linux interrupt number to the magic value which >>> needs to be written into the trigger register when the coprocessor >>> wants to send an interrupt or an IPI. >>> >>> int irq_get_irq_hwcfg(unsigned int irq, struct irq_hwcfg *cfg); >>> >>> struct irq_hwcfg needs to be defined, but it might look like this: >>> >>> { >>> /* Generic fields */ >>> x; >>> ... >>> union { >>> mips_gic; >>> ... >>> }; >>> }; > That function provides you the information which you have to hand over > to your coprocessor firmware. > > Of course! * me slapping myself on the back * Thanks, Qais
On 08/28/2015 03:22 PM, Thomas Gleixner wrote: > On Fri, 28 Aug 2015, Qais Yousef wrote: >> Thanks a lot for the detailed explanation. I wasn't looking for a quick and >> dirty solution but my view of the problem is much simpler than yours so my >> idea of a solution would look quick and dirty. I have a better appreciation of >> the problem now and a way to approach it :-) >> >> From DT point of view are we OK with this form then >> >> coprocessor { >> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >> interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; >> } >> >> and if the root controller sends normal IPI as it sends normal device >> interrupts then interrupt-sink can be a standard interrupts property (like in >> my case) >> >> coprocessor { >> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >> interrupts = <INT_SPEC>; >> } >> >> Does this look right to you? Is there something else that needs to be covered >> still? > I'm not an DT wizard. I leave that to the DT experts. > Hi Marc Zyngier, Mark Rutland, Any comments about the DT binding for the IPIs? To recap, the proposal which is based on Marc Zyngier's is to use interrupt-source to represent an IPI from Linux CPU to a coprocessor and interrupt-sink to receive an IPI from coprocessor to Linux CPU. Hopefully the description above is self explanatory. Please let me know if you need more info. Thomas covered the routing, synthesising, and requesting parts in the core code. The remaining (high level) issue is how to describe the IPIs in DT. Thanks, Qais
On 02/09/15 10:33, Qais Yousef wrote: > On 08/28/2015 03:22 PM, Thomas Gleixner wrote: >> On Fri, 28 Aug 2015, Qais Yousef wrote: >>> Thanks a lot for the detailed explanation. I wasn't looking for a quick and >>> dirty solution but my view of the problem is much simpler than yours so my >>> idea of a solution would look quick and dirty. I have a better appreciation of >>> the problem now and a way to approach it :-) >>> >>> From DT point of view are we OK with this form then >>> >>> coprocessor { >>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >>> interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; >>> } >>> >>> and if the root controller sends normal IPI as it sends normal device >>> interrupts then interrupt-sink can be a standard interrupts property (like in >>> my case) >>> >>> coprocessor { >>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >>> interrupts = <INT_SPEC>; >>> } >>> >>> Does this look right to you? Is there something else that needs to be covered >>> still? >> I'm not an DT wizard. I leave that to the DT experts. >> > > Hi Marc Zyngier, Mark Rutland, > > Any comments about the DT binding for the IPIs? > > To recap, the proposal which is based on Marc Zyngier's is to use > interrupt-source to represent an IPI from Linux CPU to a coprocessor and > interrupt-sink to receive an IPI from coprocessor to Linux CPU. > Hopefully the description above is self explanatory. Please let me know > if you need more info. Thomas covered the routing, synthesising, and > requesting parts in the core code. The remaining (high level) issue is > how to describe the IPIs in DT. I'm definitely *not* a DT expert! ;-) My initial binding proposal was only for wired interrupts, not for IPIs. There is definitely some common aspects, except for one part: Who decides on the IPI number? So far, we've avoided encoding IPI numbers in the DT just like we don't encode MSIs, because they are programmable things. My feeling is that we shouldn't put the IPI number in the DT because the rest of the kernel uses them as well and could decide to use this particular IPI number for its own use: *clash*. The way I see it would be to have a pool of IPI numbers that the kernel requests for its own use first, leaving whatever remains to drivers. Mark (as *you* are the expert ;-), what do you think? M.
On 09/02/2015 10:55 AM, Marc Zyngier wrote: > On 02/09/15 10:33, Qais Yousef wrote: >> On 08/28/2015 03:22 PM, Thomas Gleixner wrote: >>> On Fri, 28 Aug 2015, Qais Yousef wrote: >>>> Thanks a lot for the detailed explanation. I wasn't looking for a quick and >>>> dirty solution but my view of the problem is much simpler than yours so my >>>> idea of a solution would look quick and dirty. I have a better appreciation of >>>> the problem now and a way to approach it :-) >>>> >>>> From DT point of view are we OK with this form then >>>> >>>> coprocessor { >>>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >>>> interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; >>>> } >>>> >>>> and if the root controller sends normal IPI as it sends normal device >>>> interrupts then interrupt-sink can be a standard interrupts property (like in >>>> my case) >>>> >>>> coprocessor { >>>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >>>> interrupts = <INT_SPEC>; >>>> } >>>> >>>> Does this look right to you? Is there something else that needs to be covered >>>> still? >>> I'm not an DT wizard. I leave that to the DT experts. >>> >> Hi Marc Zyngier, Mark Rutland, >> >> Any comments about the DT binding for the IPIs? >> >> To recap, the proposal which is based on Marc Zyngier's is to use >> interrupt-source to represent an IPI from Linux CPU to a coprocessor and >> interrupt-sink to receive an IPI from coprocessor to Linux CPU. >> Hopefully the description above is self explanatory. Please let me know >> if you need more info. Thomas covered the routing, synthesising, and >> requesting parts in the core code. The remaining (high level) issue is >> how to describe the IPIs in DT. > I'm definitely *not* a DT expert! ;-) My initial binding proposal was > only for wired interrupts, not for IPIs. There is definitely some common > aspects, except for one part: > > Who decides on the IPI number? So far, we've avoided encoding IPI > numbers in the DT just like we don't encode MSIs, because they are > programmable things. My feeling is that we shouldn't put the IPI number > in the DT because the rest of the kernel uses them as well and could > decide to use this particular IPI number for its own use: *clash*. I think this is covered in Thomas proposal to reserve IPIs. His thoughts is to use a separate irq-domain for IPIs and use irq_reserve_ipi() and irq_destroy_ipi() to get and release IPIs. > > The way I see it would be to have a pool of IPI numbers that the kernel > requests for its own use first, leaving whatever remains to drivers. That's what Thomas thinks too and he covered this by using irq_reserve_ipi() and irq_destroy_ipi(). https://lkml.org/lkml/2015/8/26/713 It's worth noting in the light of this that INT_SPEC should be optional since for hardware similar to mine there's not much to tell the controller if it's all dynamic except where we want the IPI to be routed to - the INT_SPEC is implicitly defined by the notion it's an IPI. Thanks, Qais > > Mark (as *you* are the expert ;-), what do you think? > > M.
On 02/09/15 11:48, Qais Yousef wrote: > On 09/02/2015 10:55 AM, Marc Zyngier wrote: >> On 02/09/15 10:33, Qais Yousef wrote: >>> On 08/28/2015 03:22 PM, Thomas Gleixner wrote: >>>> On Fri, 28 Aug 2015, Qais Yousef wrote: >>>>> Thanks a lot for the detailed explanation. I wasn't looking for a quick and >>>>> dirty solution but my view of the problem is much simpler than yours so my >>>>> idea of a solution would look quick and dirty. I have a better appreciation of >>>>> the problem now and a way to approach it :-) >>>>> >>>>> From DT point of view are we OK with this form then >>>>> >>>>> coprocessor { >>>>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >>>>> interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; >>>>> } >>>>> >>>>> and if the root controller sends normal IPI as it sends normal device >>>>> interrupts then interrupt-sink can be a standard interrupts property (like in >>>>> my case) >>>>> >>>>> coprocessor { >>>>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; >>>>> interrupts = <INT_SPEC>; >>>>> } >>>>> >>>>> Does this look right to you? Is there something else that needs to be covered >>>>> still? >>>> I'm not an DT wizard. I leave that to the DT experts. >>>> >>> Hi Marc Zyngier, Mark Rutland, >>> >>> Any comments about the DT binding for the IPIs? >>> >>> To recap, the proposal which is based on Marc Zyngier's is to use >>> interrupt-source to represent an IPI from Linux CPU to a coprocessor and >>> interrupt-sink to receive an IPI from coprocessor to Linux CPU. >>> Hopefully the description above is self explanatory. Please let me know >>> if you need more info. Thomas covered the routing, synthesising, and >>> requesting parts in the core code. The remaining (high level) issue is >>> how to describe the IPIs in DT. >> I'm definitely *not* a DT expert! ;-) My initial binding proposal was >> only for wired interrupts, not for IPIs. There is definitely some common >> aspects, except for one part: >> >> Who decides on the IPI number? So far, we've avoided encoding IPI >> numbers in the DT just like we don't encode MSIs, because they are >> programmable things. My feeling is that we shouldn't put the IPI number >> in the DT because the rest of the kernel uses them as well and could >> decide to use this particular IPI number for its own use: *clash*. > > I think this is covered in Thomas proposal to reserve IPIs. His thoughts > is to use a separate irq-domain for IPIs and use irq_reserve_ipi() and > irq_destroy_ipi() to get and release IPIs. > >> >> The way I see it would be to have a pool of IPI numbers that the kernel >> requests for its own use first, leaving whatever remains to drivers. > > That's what Thomas thinks too and he covered this by using > irq_reserve_ipi() and irq_destroy_ipi(). > > https://lkml.org/lkml/2015/8/26/713 Ah, I missed that, sorry for the noise. This looks very sensible. > It's worth noting in the light of this that INT_SPEC should be optional > since for hardware similar to mine there's not much to tell the > controller if it's all dynamic except where we want the IPI to be routed > to - the INT_SPEC is implicitly defined by the notion it's an IPI. Well, I'd think that the INT_SPEC should say that it is an IPI, and I don't believe we should omit it. On the ARM GIC side, our interrupts are typed (type 0 is a normal wired interrupt, type 1 a per-cpu interrupt, and we could allocate type 2 to identify an IPI). But we do need to identify it properly, as we should be able to cover both IPIs and normal wired interrupts. Thanks, M.
On Wed, Sep 02, 2015 at 10:55:20AM +0100, Marc Zyngier wrote: > On 02/09/15 10:33, Qais Yousef wrote: > > On 08/28/2015 03:22 PM, Thomas Gleixner wrote: > >> On Fri, 28 Aug 2015, Qais Yousef wrote: > >>> Thanks a lot for the detailed explanation. I wasn't looking for a quick and > >>> dirty solution but my view of the problem is much simpler than yours so my > >>> idea of a solution would look quick and dirty. I have a better appreciation of > >>> the problem now and a way to approach it :-) > >>> > >>> From DT point of view are we OK with this form then > >>> > >>> coprocessor { > >>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; > >>> interrupt-sink = <&intc INT_SPEC CPU_HWAFFINITY>; > >>> } > >>> > >>> and if the root controller sends normal IPI as it sends normal device > >>> interrupts then interrupt-sink can be a standard interrupts property (like in > >>> my case) > >>> > >>> coprocessor { > >>> interrupt-source = <&intc INT_SPEC COP_HWAFFINITY>; > >>> interrupts = <INT_SPEC>; > >>> } > >>> > >>> Does this look right to you? Is there something else that needs to be covered > >>> still? > >> I'm not an DT wizard. I leave that to the DT experts. > >> > > > > Hi Marc Zyngier, Mark Rutland, > > > > Any comments about the DT binding for the IPIs? > > > > To recap, the proposal which is based on Marc Zyngier's is to use > > interrupt-source to represent an IPI from Linux CPU to a coprocessor and > > interrupt-sink to receive an IPI from coprocessor to Linux CPU. > > Hopefully the description above is self explanatory. Please let me know > > if you need more info. Thomas covered the routing, synthesising, and > > requesting parts in the core code. The remaining (high level) issue is > > how to describe the IPIs in DT. > > I'm definitely *not* a DT expert! ;-) My initial binding proposal was > only for wired interrupts, not for IPIs. There is definitely some common > aspects, except for one part: > > Who decides on the IPI number? So far, we've avoided encoding IPI > numbers in the DT just like we don't encode MSIs, because they are > programmable things. My feeling is that we shouldn't put the IPI number > in the DT because the rest of the kernel uses them as well and could > decide to use this particular IPI number for its own use: *clash*. Agree. The best way I've found to design DT bindings is to imagine providing the DT to something other than Linux. The DT should *only* be describing the hardware. As such, I think we should be describing the connection here, and leaving the assignment up to the OS. thx, Jason.
On 09/02/2015 12:53 PM, Marc Zyngier wrote: > On 02/09/15 11:48, Qais Yousef wrote: >> It's worth noting in the light of this that INT_SPEC should be optional >> since for hardware similar to mine there's not much to tell the >> controller if it's all dynamic except where we want the IPI to be routed >> to - the INT_SPEC is implicitly defined by the notion it's an IPI. > Well, I'd think that the INT_SPEC should say that it is an IPI, and I > don't believe we should omit it. On the ARM GIC side, our interrupts are > typed (type 0 is a normal wired interrupt, type 1 a per-cpu interrupt, > and we could allocate type 2 to identify an IPI). I didn't mean to omit it completely, but just being optional so it's specified if the intc needs this info only. I'm assuming that INT_SPEC is interrupt controller specific. If not, then ignore me :-) > > But we do need to identify it properly, as we should be able to cover > both IPIs and normal wired interrupts. I'm a bit confused here. What do you mean by normal wired interrupts? I thought this DT binding is only to describe IPIs that needs reserving and routing. What am I missing? Thanks, Qais
On 02/09/15 14:25, Qais Yousef wrote: > On 09/02/2015 12:53 PM, Marc Zyngier wrote: >> On 02/09/15 11:48, Qais Yousef wrote: >>> It's worth noting in the light of this that INT_SPEC should be optional >>> since for hardware similar to mine there's not much to tell the >>> controller if it's all dynamic except where we want the IPI to be routed >>> to - the INT_SPEC is implicitly defined by the notion it's an IPI. >> Well, I'd think that the INT_SPEC should say that it is an IPI, and I >> don't believe we should omit it. On the ARM GIC side, our interrupts are >> typed (type 0 is a normal wired interrupt, type 1 a per-cpu interrupt, >> and we could allocate type 2 to identify an IPI). > > I didn't mean to omit it completely, but just being optional so it's > specified if the intc needs this info only. I'm assuming that INT_SPEC > is interrupt controller specific. If not, then ignore me :-) It is, but I don't think it can really be made optional. >> >> But we do need to identify it properly, as we should be able to cover >> both IPIs and normal wired interrupts. > > I'm a bit confused here. What do you mean by normal wired interrupts? I > thought this DT binding is only to describe IPIs that needs reserving > and routing. What am I missing? Look at my initial proposal, and the way I was describing a device having an interrupt source, and two possible interrupt sinks, one being a CPU and the other being another device. I'm looking at solving that case as well, possibly with the same infrastructure (the routing bit should be the same). Thanks, M.
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c index ff4be0515a0d..fc6fd506cd7e 100644 --- a/drivers/irqchip/irq-mips-gic.c +++ b/drivers/irqchip/irq-mips-gic.c @@ -227,6 +227,7 @@ void gic_send_ipi(unsigned int intr) { gic_write(GIC_REG(SHARED, GIC_SH_WEDGE), GIC_SH_WEDGE_SET(intr)); } +EXPORT_SYMBOL(gic_send_ipi); int gic_get_c0_compare_int(void) {
Some drivers might require to send ipi to other cores. So export it. This will be used later by AXD driver. Signed-off-by: Qais Yousef <qais.yousef@imgtec.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Jason Cooper <jason@lakedaemon.net> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: linux-kernel@vger.kernel.org Cc: linux-mips@linux-mips.org --- drivers/irqchip/irq-mips-gic.c | 1 + 1 file changed, 1 insertion(+)