All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey.Brodkin@synopsys.com (Alexey Brodkin)
To: linux-snps-arc@lists.infradead.org
Subject: [PATCH] ARC: Change ld.as instruction to regular ld.
Date: Tue, 16 Aug 2016 13:15:33 +0000	[thread overview]
Message-ID: <1471353226.3980.42.camel@synopsys.com> (raw)
In-Reply-To: <1471334135-21801-1-git-send-email-liavr@mellanox.com>

Hi Liav,

On Tue, 2016-08-16@10:55 +0300, Liav Rehana wrote:
> From: Liav Rehana <liavr at mellanox.com>
> 
> The instruction ld.as takes as operands a base address and an offset,
> and doesn't access the sum of these two, but the sum of the base
> address and a shifted version of the offset.
> This isn't what we want in that case, since it causes a bug during
> the push and pop of r25, since his actual offset is given during
> resume_user_mode_begin.
> Thus, the use of ld solves this problem.
> 
> Signed-off-by: Liav Rehana <liavr at mellanox.com>
> ---

Very nice catch!

But IMHO description could be improved a little bit.
Probably something like that:
--------------------->8---------------------
"PT_user_r25" is offset in bytes within pt_regs structure.

In its turn what "ld.as r1, [r2, x]" really does is
r1 <- load_from(r2 + (x << data_size)) = load_from(r2 + x*4).

But the code in question is supposed to load_from(r2 + x).

This leads to obvious stack corruption.
--------------------->8---------------------

Reviewed-by: Alexey Brodkin <abrodkin at synopsys.com>

WARNING: multiple messages have this Message-ID (diff)
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: "liavr@mellanox.com" <liavr@mellanox.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"eladkan@mellanox.com" <eladkan@mellanox.com>,
	"noamca@mellanox.com" <noamca@mellanox.com>,
	Vineet Gupta <Vineet.Gupta1@synopsys.com>,
	"linux-snps-arc@lists.infradead.org" 
	<linux-snps-arc@lists.infradead.org>
Subject: Re: [PATCH] ARC: Change ld.as instruction to regular ld.
Date: Tue, 16 Aug 2016 13:15:33 +0000	[thread overview]
Message-ID: <1471353226.3980.42.camel@synopsys.com> (raw)
In-Reply-To: <1471334135-21801-1-git-send-email-liavr@mellanox.com>

Hi Liav,

On Tue, 2016-08-16 at 10:55 +0300, Liav Rehana wrote:
> From: Liav Rehana <liavr@mellanox.com>
> 
> The instruction ld.as takes as operands a base address and an offset,
> and doesn't access the sum of these two, but the sum of the base
> address and a shifted version of the offset.
> This isn't what we want in that case, since it causes a bug during
> the push and pop of r25, since his actual offset is given during
> resume_user_mode_begin.
> Thus, the use of ld solves this problem.
> 
> Signed-off-by: Liav Rehana <liavr@mellanox.com>
> ---

Very nice catch!

But IMHO description could be improved a little bit.
Probably something like that:
--------------------->8---------------------
"PT_user_r25" is offset in bytes within pt_regs structure.

In its turn what "ld.as r1, [r2, x]" really does is
r1 <- load_from(r2 + (x << data_size)) = load_from(r2 + x*4).

But the code in question is supposed to load_from(r2 + x).

This leads to obvious stack corruption.
--------------------->8---------------------

Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>

  reply	other threads:[~2016-08-16 13:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-16  7:55 [PATCH] ARC: Change ld.as instruction to regular ld Liav Rehana
2016-08-16  7:55 ` Liav Rehana
2016-08-16 13:15 ` Alexey Brodkin [this message]
2016-08-16 13:15   ` Alexey Brodkin
2016-08-16 15:47   ` Vineet Gupta
2016-08-16 15:47     ` Vineet Gupta
  -- strict thread matches above, loose matches on Subject: below --
2016-08-17  6:23 Liav Rehana
2016-08-17  6:23 ` Liav Rehana
2016-08-17 17:16 ` Vineet Gupta
2016-08-17 17:16   ` Vineet Gupta
2016-08-25 12:05 ` Alexey Brodkin
2016-08-25 12:05   ` Alexey Brodkin
2016-08-25 17:40   ` Vineet Gupta
2016-08-25 17:40     ` Vineet Gupta

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=1471353226.3980.42.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=linux-snps-arc@lists.infradead.org \
    /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: link
Be 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.