From: Borislav Petkov <bp@alien8.de>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>,
x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>, Kees Cook <keescook@chromium.org>,
"H . Peter Anvin" <hpa@zytor.com>, Joerg Roedel <jroedel@suse.de>,
"Gustavo A . R . Silva" <gustavoars@kernel.org>,
Jann Horn <jannh@google.com>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
Ricardo Neri <ricardo.neri-calderon@linux.intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] x86/uprobes: Fix not using prefixes.nbytes for loop over prefixes.bytes
Date: Fri, 4 Dec 2020 12:06:44 +0100 [thread overview]
Message-ID: <20201204110644.GB31534@zn.tnic> (raw)
In-Reply-To: <20201204095653.c948106a294fd6e731df5594@kernel.org>
On Fri, Dec 04, 2020 at 09:56:53AM +0900, Masami Hiramatsu wrote:
> Hmm, there is a difference between Intel SDM and AMD APM.
>
> Intel SDM vol.2
>
> 2.1.1 Instruction Prefixes
> Instruction prefixes are divided into four groups, each with a set of allowable prefix codes. For each instruction, it
> is only useful to include up to one prefix code from each of the four groups (Groups 1, 2, 3, 4).
>
> AMD APM vol.3
>
> 1.2.1 Summary of Legacy Prefixes
> Table 1-1 on page 7 shows the legacy prefixes. The legacy prefixes are organized into five groups, as
> shown in the left-most column of Table 1-1. An instruction encoding may include a maximum of one
> prefix from each of the five groups.
>
> So, Intel CPU doesn't accept LOCK-REP because those are in a same prefix
> group, but AMD says it is acceptable.
That would be a huge problem for code if both vendors would behave
differently wrt prefixes.
> Actually, insn.c only accepts the prefix up to 4, so if there is any
> instruction which has 5 prefixes, it will fail to parse.
Well, actually it looks more like a difference in how both vendors group
things:
AMD has 5 groups and Intel 4 by putting LOCK and REP together.
The most important aspect, however, is that you can have as many
prefixes as you want and there's no hardware limitation on the number -
I'm being told - just that you can overflow the instruction limit of 15
and then get a #GP for invalid insn. See here:
https://sandpile.org/x86/opc_enc.htm
note #1
with examples how you can overflow the 15 bytes limit even with a valid
insn.
> Note that anyway the same prefix can be repeated, we can see a good example
> in K8_NOP*.
Yap.
> In this case, insn.c just store the 1 osp in the prefixes.bytes[], and
> just increment prefixes.nbytes for the repeated prefixes.
>
> Anyway, if there is LOCK-REP prefix combination, I have to introduce new
> insn_field for legacy prefix.
Well, the legacy prefixes field needs to be of 4 fields because REP and
LOCK really are two separate but mutually exclusive groups. Why?
They're used by a disjoint set of instructions, see the AMD doc for both
REP and LOCK prefixes.
Which means, you can either have a REP (exclusive or) LOCK but not both.
Which means, as a stable@ fix I can use Tom's ARRAY_SIZE() suggestion
and then later on we can make the legacy prefixes a separate struct.
Maybe even a struct with a bitfield:
struct legacy_prefixes {
/* operand-size override: 0x66 */
u8 os_over: 1,
/* address-size override: 0x67 */
as_over: 1,
/*
* segment override: 0x2e(CS), 0x3e(DS), 0x26(ES), 0x64(FS), 0x65(GS),
* 0x36(SS)
*/
s_over: 1,
/* lock prefix: 0xf0 */
lock: 1,
/* repeat prefixes: 0xf2: REPNx, 0xf3: REPx */
rep: 1,
__resv: 3;
};
or so which you can set to denote when you've seen the respective
prefixes.
But that we can discuss later.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2020-12-04 11:07 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-03 4:50 [PATCH v2 0/3] x86/insn: Fix not using prefixes.nbytes for loop over prefixes.bytes Masami Hiramatsu
2020-12-03 4:50 ` [PATCH v2 1/3] x86/uprobes: " Masami Hiramatsu
2020-12-03 12:37 ` Borislav Petkov
2020-12-03 12:41 ` Borislav Petkov
2020-12-03 12:48 ` Borislav Petkov
2020-12-03 16:45 ` Tom Lendacky
2020-12-03 16:54 ` Borislav Petkov
2020-12-03 17:01 ` Borislav Petkov
2020-12-03 18:10 ` Tom Lendacky
2020-12-03 18:17 ` Borislav Petkov
2020-12-03 18:49 ` Tom Lendacky
2020-12-04 0:56 ` Masami Hiramatsu
2020-12-04 3:55 ` Masami Hiramatsu
2020-12-04 11:06 ` Borislav Petkov [this message]
2020-12-04 11:28 ` Masami Hiramatsu
2020-12-04 0:16 ` Masami Hiramatsu
2020-12-04 0:18 ` Masami Hiramatsu
2020-12-04 15:04 ` [tip: x86/urgent] x86/uprobes: Do not use prefixes.nbytes when looping " tip-bot2 for Masami Hiramatsu
2020-12-05 0:12 ` Masami Hiramatsu
2020-12-05 10:17 ` Borislav Petkov
2020-12-06 3:53 ` Masami Hiramatsu
2020-12-06 9:02 ` Borislav Petkov
2020-12-09 18:01 ` Arnaldo Carvalho de Melo
2020-12-10 10:36 ` Borislav Petkov
2020-12-09 18:05 ` Arnaldo Carvalho de Melo
2020-12-06 9:09 ` tip-bot2 for Masami Hiramatsu
2020-12-03 4:50 ` [PATCH v2 2/3] x86/insn-eval: Fix not using prefixes.nbytes for loop " Masami Hiramatsu
2020-12-04 15:04 ` [tip: x86/urgent] x86/insn-eval: Use new for_each_insn_prefix() macro to loop over prefixes bytes tip-bot2 for Masami Hiramatsu
2020-12-06 9:09 ` tip-bot2 for Masami Hiramatsu
2020-12-03 4:51 ` [PATCH v2 3/3] x86/sev-es: Fix not using prefixes.nbytes for loop over prefixes.bytes Masami Hiramatsu
2020-12-04 15:04 ` [tip: x86/urgent] x86/sev-es: Use new for_each_insn_prefix() macro to loop over prefixes bytes tip-bot2 for Masami Hiramatsu
2020-12-06 9:09 ` tip-bot2 for Masami Hiramatsu
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=20201204110644.GB31534@zn.tnic \
--to=bp@alien8.de \
--cc=gustavoars@kernel.org \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=jroedel@suse.de \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=ricardo.neri-calderon@linux.intel.com \
--cc=srikar@linux.vnet.ibm.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).