On 4/24/22 15:01, Paul Brook wrote:
Add CHECK_AVX* macros, and use them to validate VEX encoded AVX instructions
All AVX instructions require both CPU and OS support, this is encapsulated
by HF_AVX_EN.
Some also require specific values in the VEX.L and VEX.V fields.
Some (mostly integer operations) also require AVX2
Signed-off-by: Paul Brook <p...@nowt.org>
---
target/i386/tcg/translate.c | 159 +++++++++++++++++++++++++++++++++---
1 file changed, 149 insertions(+), 10 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 66ba690b7d..2f5cc24e0c 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -3185,10 +3185,54 @@ static const struct SSEOpHelper_table7
sse_op_table7[256] = {
goto illegal_op; \
} while (0)
+/*
+ * VEX encodings require AVX
+ * Allow legacy SSE encodings even if AVX not enabled
+ */
+#define CHECK_AVX(s) do { \
+ if ((s->prefix & PREFIX_VEX) \
+ && !(env->hflags & HF_AVX_EN_MASK)) \
+ goto illegal_op; \
+ } while (0)
+
+/* If a VEX prefix is used then it must have V=1111b */
+#define CHECK_AVX_V0(s) do { \
+ CHECK_AVX(s); \
+ if ((s->prefix & PREFIX_VEX) && (s->vex_v != 0)) \
+ goto illegal_op; \
+ } while (0)
+
+/* If a VEX prefix is used then it must have L=0 */
+#define CHECK_AVX_128(s) do { \
+ CHECK_AVX(s); \
+ if ((s->prefix & PREFIX_VEX) && (s->vex_l != 0)) \
+ goto illegal_op; \
+ } while (0)
+
+/* If a VEX prefix is used then it must have V=1111b and L=0 */
+#define CHECK_AVX_V0_128(s) do { \
+ CHECK_AVX(s); \
+ if ((s->prefix & PREFIX_VEX) && (s->vex_v != 0 || s->vex_l != 0)) \
+ goto illegal_op; \
+ } while (0)
These predicates have some overlap, but awkwardly. It leaves you with cases
like
+ if (op6.flags & SSE_OPF_V0) {
+ CHECK_AVX_V0(s);
+ } else {
+ CHECK_AVX(s);
+ }
this, where clearly the CHECK_AVX is common across the IF, and would be better
written as
CHECK_AVX(s);
if (flags & SSE_OPF_V0) {
CHECK_V0(s);
}
+ CHECK_AVX(s);
+ scalar_op = (s->prefix & PREFIX_VEX)
+ && (op7.flags & SSE_OPF_SCALAR)
+ && !(op7.flags & SSE_OPF_CMP);
+ if (is_xmm && (op7.flags & SSE_OPF_MMX)) {
+ CHECK_AVX2_256(s);
+ }
+ if (op7.flags & SSE_OPF_AVX2) {
+ CHECK_AVX2(s);
+ }
+ if ((op7.flags & SSE_OPF_V0) && !scalar_op) {
+ CHECK_AVX_V0(s);
+ }
And these. Also, it would appear as if there's overlap between the AVX2 checks. Is this
clearer as
CHECK_AVX(s);
if (v0 && !scalar) {
CHECK_V0(s);
}
if ((flags & AVX2) || ((flags & MMX) && s->vex_l)) {
CHECK_AVX2(s);
}
and perhaps these could be broken out into helpers, so that
if (is_xmm) {
+ scalar_op = (s->prefix & PREFIX_VEX)
+ && (sse_op.flags & SSE_OPF_SCALAR)
+ && !(sse_op.flags & SSE_OPF_CMP)
+ && (b1 == 2 || b1 == 3);
+ /* VEX encoded scalar ops always have 3 operands! */
+ if ((sse_op.flags & SSE_OPF_V0) && !scalar_op) {
+ CHECK_AVX_V0(s);
+ } else {
+ CHECK_AVX(s);
+ }
+ if (sse_op.flags & SSE_OPF_MMX) {
+ CHECK_AVX2_256(s);
+ }
... you don't have to keep repeating stuff. This is where a better decoder
could really help.
r~
