From: Marc Zyngier <marc.zyngier@arm.com> To: Robin Murphy <robin.murphy@arm.com>, robh+dt@kernel.org, frowand.list@gmail.com, grant.likely@linaro.org, devicetree@vger.kernel.org Cc: mark.rutland@arm.com, david.daney@cavium.com, stuart.yoder@nxp.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base Date: Tue, 9 Feb 2016 12:06:06 +0000 [thread overview] Message-ID: <56B9D62E.8080003@arm.com> (raw) In-Reply-To: <9f6845195d03b0e0b0d187bb510fbf7bd497e836.1455015344.git.robin.murphy@arm.com> Hi Robin, On 09/02/16 11:04, Robin Murphy wrote: > The existing msi-map code is fine for shifting the entire RID space > upwards, but attempting finer-grained remapping reveals a bug. It turns > out that we are mistakenly treating the msi-base part as an offset, not > as a new base to remap onto, so things get squiffy when rid-base is > nonzero. Fix this, and at the same time add a sanity check against > having msi-map-mask clash with a nonzero rid-base, as that's another > thing one can easily get wrong. > > CC: <stable@vger.kernel.org> > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Looks like Stuart and you both found the same bug at the same time: https://lkml.org/lkml/2016/2/8/1066 but yours seem more correct to me (the rid_base masking in Stuart's version seems odd). > --- > drivers/of/irq.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > index 7ee21ae..e7bfc17 100644 > --- a/drivers/of/irq.c > +++ b/drivers/of/irq.c > @@ -635,6 +635,13 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np, > msi_base = be32_to_cpup(msi_map + 2); > rid_len = be32_to_cpup(msi_map + 3); > > + if (rid_base & ~map_mask) { > + dev_err(parent_dev, > + "Invalid msi-map translation - msi-map-mask (0x%x) ignores rid-base (0x%x)\n", > + map_mask, rid_base); > + return rid_out; > + } > + > msi_controller_node = of_find_node_by_phandle(phandle); > > matched = (masked_rid >= rid_base && > @@ -654,7 +661,7 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np, > if (!matched) > return rid_out; > > - rid_out = masked_rid + msi_base; > + rid_out = masked_rid - rid_base + msi_base; > dev_dbg(dev, > "msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: %08x, length: %08x, rid: %08x -> %08x\n", > dev_name(parent_dev), map_mask, rid_base, msi_base, > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> M. -- Jazz is not dead. It just smells funny...
WARNING: multiple messages have this Message-ID (diff)
From: marc.zyngier@arm.com (Marc Zyngier) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base Date: Tue, 9 Feb 2016 12:06:06 +0000 [thread overview] Message-ID: <56B9D62E.8080003@arm.com> (raw) In-Reply-To: <9f6845195d03b0e0b0d187bb510fbf7bd497e836.1455015344.git.robin.murphy@arm.com> Hi Robin, On 09/02/16 11:04, Robin Murphy wrote: > The existing msi-map code is fine for shifting the entire RID space > upwards, but attempting finer-grained remapping reveals a bug. It turns > out that we are mistakenly treating the msi-base part as an offset, not > as a new base to remap onto, so things get squiffy when rid-base is > nonzero. Fix this, and at the same time add a sanity check against > having msi-map-mask clash with a nonzero rid-base, as that's another > thing one can easily get wrong. > > CC: <stable@vger.kernel.org> > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Looks like Stuart and you both found the same bug at the same time: https://lkml.org/lkml/2016/2/8/1066 but yours seem more correct to me (the rid_base masking in Stuart's version seems odd). > --- > drivers/of/irq.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > index 7ee21ae..e7bfc17 100644 > --- a/drivers/of/irq.c > +++ b/drivers/of/irq.c > @@ -635,6 +635,13 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np, > msi_base = be32_to_cpup(msi_map + 2); > rid_len = be32_to_cpup(msi_map + 3); > > + if (rid_base & ~map_mask) { > + dev_err(parent_dev, > + "Invalid msi-map translation - msi-map-mask (0x%x) ignores rid-base (0x%x)\n", > + map_mask, rid_base); > + return rid_out; > + } > + > msi_controller_node = of_find_node_by_phandle(phandle); > > matched = (masked_rid >= rid_base && > @@ -654,7 +661,7 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np, > if (!matched) > return rid_out; > > - rid_out = masked_rid + msi_base; > + rid_out = masked_rid - rid_base + msi_base; > dev_dbg(dev, > "msi-map at: %s, using mask %08x, rid-base: %08x, msi-base: %08x, length: %08x, rid: %08x -> %08x\n", > dev_name(parent_dev), map_mask, rid_base, msi_base, > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> M. -- Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2016-02-09 12:06 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-02-09 11:04 [PATCH] of/irq: Fix msi-map calculation for nonzero rid-base Robin Murphy 2016-02-09 11:04 ` Robin Murphy 2016-02-09 12:06 ` Marc Zyngier [this message] 2016-02-09 12:06 ` Marc Zyngier 2016-02-09 15:56 ` Stuart Yoder 2016-02-09 15:56 ` Stuart Yoder 2016-02-09 15:56 ` Stuart Yoder 2016-02-09 15:56 ` Stuart Yoder 2016-02-09 16:08 ` Mark Rutland 2016-02-09 16:08 ` Mark Rutland 2016-02-09 16:08 ` Mark Rutland 2016-02-09 16:17 ` Robin Murphy 2016-02-09 16:17 ` Robin Murphy 2016-02-09 16:17 ` Robin Murphy 2016-02-09 18:19 ` Mark Rutland 2016-02-09 18:19 ` Mark Rutland 2016-02-09 18:19 ` Mark Rutland 2016-02-09 16:53 ` Stuart Yoder 2016-02-09 16:53 ` Stuart Yoder 2016-02-09 16:53 ` Stuart Yoder 2016-02-09 17:03 ` David Daney 2016-02-09 17:03 ` David Daney 2016-02-09 17:03 ` David Daney 2016-02-09 18:12 ` Stuart Yoder 2016-02-09 18:12 ` Stuart Yoder 2016-02-09 18:12 ` Stuart Yoder 2016-02-09 18:12 ` Stuart Yoder 2016-02-11 11:04 ` Marc Zyngier 2016-02-11 11:04 ` Marc Zyngier 2016-02-11 18:10 ` Frank Rowand 2016-02-11 18:10 ` Frank Rowand 2016-02-11 23:15 ` Rob Herring 2016-02-11 23:15 ` Rob Herring 2016-02-11 23:15 ` Rob Herring 2016-02-11 23:15 ` Rob Herring 2016-02-12 8:32 ` Marc Zyngier 2016-02-12 8:32 ` Marc Zyngier 2016-02-12 8:32 ` Marc Zyngier 2016-02-12 8:32 ` Marc Zyngier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=56B9D62E.8080003@arm.com \ --to=marc.zyngier@arm.com \ --cc=david.daney@cavium.com \ --cc=devicetree@vger.kernel.org \ --cc=frowand.list@gmail.com \ --cc=grant.likely@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=robh+dt@kernel.org \ --cc=robin.murphy@arm.com \ --cc=stable@vger.kernel.org \ --cc=stuart.yoder@nxp.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.