Send Qemu-devel mailing list submissions to
qemu-devel@nongnu.org
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.nongnu.org/mailman/listinfo/qemu-devel
or, via email, send a message with subject or body 'help' to
qemu-devel-request@nongnu.org
You can reach the person managing the list at
qemu-devel-owner@nongnu.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Qemu-devel digest..."
Today's Topics:
1. RE: [PATCH v5 10/14] target/hexagon: import parser for
idef-parser (Taylor Simpson)
2. [RFC v1 0/1] ui: Add a Wayland backend for Qemu UI
(Vivek Kasireddy)
3. [RFC v1 1/1] ui: Add a plain Wayland backend for Qemu UI
(Vivek Kasireddy)
4. Re: [RFC v1 0/1] ui: Add a Wayland backend for Qemu UI
(no-reply@patchew.org)
5. Re: [PATCH v2 03/10] target/ppc: Push real-mode handling into
ppc_radix64_xlate (David Gibson)
6. Re: [PATCH v2 02/10] target/ppc: Use MMUAccessType with
*_handle_mmu_fault (David Gibson)
7. Re: [PATCH v2 04/10] target/ppc: Use bool success for
ppc_radix64_xlate (David Gibson)
8. Re: [PATCH v3 02/24] modules: collect module meta-data
(Paolo Bonzini)
9. Re: [PATCH v2 04/37] target/riscv: 8-bit Addition &
Subtraction Instruction (LIU Zhiwei)
10. Re: [PATCH v2 03/23] qapi/misc-target: Group SEV QAPI
definitions (Dov Murik)
----------------------------------------------------------------------
Message: 1
Date: Thu, 24 Jun 2021 03:55:35 +0000
From: Taylor Simpson <tsimpson@quicinc.com>
To: Alessandro Di Federico <ale.qemu@rev.ng>, "qemu-devel@nongnu.org"
<qemu-devel@nongnu.org>
Cc: Brian Cain <bcain@quicinc.com>, "babush@rev.ng" <babush@rev.ng>,
"nizzo@rev.ng" <nizzo@rev.ng>, "philmd@redhat.com"
<philmd@redhat.com>, "richard.henderson@linaro.org"
<richard.henderson@linaro.org>, Alessandro Di Federico <ale@rev.ng>
Subject: RE: [PATCH v5 10/14] target/hexagon: import parser for
idef-parser
Message-ID:
<BYAPR02MB488679E9F94D484852DD2398DE079@BYAPR02MB4886.namprd02.prod.outlook.com" target="_blank" rel="noreferrer">BYAPR02MB488679E9F94D484852DD2398DE079@BYAPR02MB4886.namprd02.prod.outlook.com>
Content-Type: text/plain; charset="us-ascii"
> -----Original Message-----
> From: Alessandro Di Federico <ale.qemu@rev.ng>
> Sent: Saturday, June 19, 2021 3:37 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson <tsimpson@quicinc.com>; Brian Cain
> <bcain@quicinc.com>; babush@rev.ng; nizzo@rev.ng; philmd@redhat.com;
> richard.henderson@linaro.org; Alessandro Di Federico <ale@rev.ng>
> Subject: [PATCH v5 10/14] target/hexagon: import parser for idef-parser
>
> From: Paolo Montesel <babush@rev.ng>
>
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> Signed-off-by: Paolo Montesel <babush@rev.ng>
> ---
> diff --git a/target/hexagon/idef-parser/idef-parser.y b/target/hexagon/idef-
> parser/idef-parser.y
> +for_statement : FOR '(' IMM '=' IMM ';' IMM '<' IMM ';' IMM PLUSPLUS ')'
> + {
> + @1.last_column = @13.last_column;
> + OUT(c, &@1, "for (int ", &$3, " = ", &$5, "; ",
> + &$7, " < ", &$9);
> + OUT(c, &@1, "; ", &$11, "++) {\n");
Increase indent level here
> + }
> + code_block
> + {
Decrease indent level
> + OUT(c, &@1, "}\n");
> + }
> + | FOR '(' IMM '=' IMM ';' IMM '<' IMM ';' IMM INC IMM ')'
> + {
> + @1.last_column = @14.last_column;
> + OUT(c, &@1, "for (int ", &$3, " = ", &$5, "; ",
> + &$7, " < ", &$9);
> + OUT(c, &@1, "; ", &$11, " += ", &$13, ") {\n");
Increase indent
> + }
> + code_block
> + {
Decrease indent
> + OUT(c, &@1, "}\n");
> + }
> + ;
> +
> +fpart1_statement : PART1
> + {
> + OUT(c, &@1, "if (insn->part1) {\n");
Increase indent
> + }
> + '(' statements ')'
> + {
> + @1.last_column = @3.last_column;
Emit the return first, then decrease indent before the curly
> + OUT(c, &@1, "return; }\n");
> + }
> + ;
> + | rvalue '[' rvalue ']'
> + {
> + @1.last_column = @4.last_column;
> + if ($3.type == IMMEDIATE) {
> + $$ = gen_tmp(c, &@1, $1.bit_width);
> + OUT(c, &@1, "tcg_gen_extract_i", &$$.bit_width, "(");
> + OUT(c, &@1, &$$, ", ", &$1, ", ", &$3, ", 1);\n");
> + } else {
> + HexValue _one_ = gen_imm_value(c, &@1, 1, $3.bit_width);
> + HexValue tmp = gen_bin_op(c, &@1, ASR_OP, &$1, &$3);
> + $$ = gen_bin_op(c, &@1, ANDB_OP, &tmp, &one);
Can this be done with a tcg_gen_extract_tl or tcg_gen_sextract_tl?
Do you need to worry about signed-ness?
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> new file mode 100644
> +const char *creg_str[] = {"HEX_REG_SP", "HEX_REG_FP", "HEX_REG_LR",
> + "HEX_REG_GP", "HEX_REG_LC0", "HEX_REG_LC1",
> + "HEX_REG_SA0", "HEX_REG_SA1"};
SP, FP, LR shouldn't in this array.
> +void reg_compose(Context *c, YYLTYPE *locp, HexReg *reg, char reg_id[5])
> +{
> + switch (reg->type) {
> + case GENERAL_PURPOSE:
> + reg_id[0] = 'R';
> + break;
> + case CONTROL:
> + reg_id[0] = 'C';
> + break;
> + case MODIFIER:
> + reg_id[0] = 'M';
> + break;
> + case DOTNEW:
> + /* The DOTNEW case is managed by the upper level function */
Should we raise an error if we get here?
> + break;
> + }
> + switch (reg->bit_width) {
> + case 32:
> + reg_id[1] = reg->id;
> + reg_id[2] = 'V';
> + break;
> + case 64:
> + reg_id[1] = reg->id;
> + reg_id[2] = reg->id;
> + reg_id[3] = 'V';
> + break;
> + default:
> + yyassert(c, locp, false, "Unhandled register bit width!\n");
> + }
> +}
> +
> +void reg_print(Context *c, YYLTYPE *locp, HexReg *reg)
> +{
> + if (reg->type == DOTNEW) {
> + EMIT(c, "N%cN", reg->id);
Why not handle this in reg_compose?
> + } else {
> + char reg_id[5] = { 0 };
> + reg_compose(c, locp, reg, reg_id);
> + EMIT(c, "%s", reg_id);
> + }
> +}
> +
> +void imm_print(Context *c, YYLTYPE *locp, HexImm *imm)
> +{
> + switch (imm->type) {
> + case I:
> + EMIT(c, "i");
> + break;
> + case VARIABLE:
> + EMIT(c, "%ciV", imm->id);
> + break;
> + case VALUE:
> + EMIT(c, "((int64_t)%" PRIu64 "ULL)", (int64_t)imm->value);
> + break;
> + case QEMU_TMP:
> + EMIT(c, "qemu_tmp_%" PRIu64, imm->index);
> + break;
> + case IMM_PC:
> + EMIT(c, "ctx->base.pc_next");
> + break;
> + case IMM_NPC:
> + EMIT(c, "ctx->npc");
> + break;
> + case IMM_CONSTEXT:
> + EMIT(c, "insn->extension_valid");
The extension_valid field is a bool indicating if the instruction has a constant extender. Don't you want the actual value here?
> + break;
> +
> +static HexValue get_ternary_cond(Context *c, YYLTYPE *locp)
> +{
> + yyassert(c, locp, is_inside_ternary(c), "unexisting condition");
> + Ternary *t = &g_array_index(c->ternary, Ternary, 0);
> + HexValue cond = t->cond;
> + if (t->state == IN_RIGHT) {
> + cond = gen_rvalue_notl(c, locp, &cond);
> + }
> + for (unsigned i = 1; i < c->ternary->len; ++i) {
> + Ternary *right = &g_array_index(c->ternary, Ternary, i);
> + HexValue other = right->cond;
> + /* Invert condition if we are on the right side */
> + if (right->state == IN_RIGHT) {
> + other = gen_rvalue_notl(c, locp, &other);
> + }
> + cond = gen_bin_op(c, locp, ANDL_OP, &cond, &other);
> + }
> + return cond;
> +}
> +
> +/* Temporary values creation */
> +HexValue gen_tmp(Context *c, YYLTYPE *locp, int bit_width)
> +{
> + HexValue rvalue;
> + rvalue.type = TEMP;
> + bit_width = (bit_width == 64) ? 64 : 32;
Better to assert it's either 64 or 32
> + rvalue.bit_width = bit_width;
> + rvalue.is_unsigned = false;
> + rvalue.is_dotnew = false;
> + rvalue.is_manual = false;
> + rvalue.tmp.index = c->inst.tmp_count;
> + OUT(c, locp, "TCGv_i", &bit_width, " tmp_", &c->inst.tmp_count,
> + " = tcg_temp_new_i", &bit_width, "();\n");
> + c->inst.tmp_count++;
> + return rvalue;
> +}
> +
> +
> +void rvalue_free(Context *c, YYLTYPE *locp, HexValue *rvalue)
Should be called gen_rvalue_free
> +{
> + if (rvalue->type == TEMP && !rvalue->is_manual) {
> + const char *bit_suffix = (rvalue->bit_width == 64) ? "i64" : "i32";
> + OUT(c, locp, "tcg_temp_free_", bit_suffix, "(", rvalue, ");\n");
> + }
> +}
> +HexValue rvalue_extend(Context *c, YYLTYPE *locp, HexValue *rvalue)
Should be called gen_rvalue_extend
> +{
> + if (rvalue->type == IMMEDIATE) {
> + HexValue res = *rvalue;
> + res.bit_width = 64;
> + return res;
> + } else {
> + if (rvalue->bit_width == 32) {
> + HexValue res = gen_tmp(c, locp, 64);
> + const char *sign_suffix = (rvalue->is_unsigned) ? "u" : "";
> + OUT(c, locp, "tcg_gen_ext", sign_suffix,
> + "_i32_i64(", &res, ", ", rvalue, ");\n");
> + rvalue_free(c, locp, rvalue);
> + return res;
> + }
> + }
> + return *rvalue;
> +}
> +
> +HexValue rvalue_truncate(Context *c, YYLTYPE *locp, HexValue *rvalue)
Should be called gen_rvalue_truncate
> +{
> + if (rvalue->type == IMMEDIATE) {
> + HexValue res = *rvalue;
> + res.bit_width = 32;
> + return res;
> + } else {
> + if (rvalue->bit_width == 64) {
> + HexValue res = gen_tmp(c, locp, 32);
> + OUT(c, locp, "tcg_gen_trunc_i64_tl(", &res, ", ", rvalue, ");\n");
> + rvalue_free(c, locp, rvalue);
> + return res;
> + }
> + }
> + return *rvalue;
> +}
> +
> +void varid_allocate(Context *c,
gen_varid_allocate
> + YYLTYPE *locp,
> + HexValue *varid,
> + int width,
> + bool is_unsigned)
> +{
> + varid->bit_width = width;
> + const char *bit_suffix = width == 64 ? "64" : "32";
> + int index = find_variable(c, locp, varid);
> + bool found = index != -1;
> + if (found) {
> + Var *other = &g_array_index(c->inst.allocated, Var, index);
> + varid->var.name = other->name;
> + varid->bit_width = other->bit_width;
> + varid->is_unsigned = other->is_unsigned;
> + } else {
> + EMIT_HEAD(c, "TCGv_i%s %s", bit_suffix, varid->var.name->str);
> + EMIT_HEAD(c, " = tcg_temp_local_new_i%s();\n", bit_suffix);
> + Var new_var = {
> + .name = varid->var.name,
> + .bit_width = width,
> + .is_unsigned = is_unsigned,
> + };
> + g_array_append_val(c->inst.allocated, new_var);
> + }
> +}
> +
> +void ea_free(Context *c, YYLTYPE *locp)
gen_ea_free
> +{
> + OUT(c, locp, "tcg_temp_free(EA);\n");
> +}
> +HexValue gen_bin_cmp(Context *c,
> + YYLTYPE *locp,
> + TCGCond type,
> + HexValue *op1_ptr,
> + HexValue *op2_ptr)
> +{
> + HexValue op1 = *op1_ptr;
> + HexValue op2 = *op2_ptr;
> + enum OpTypes op_types = (op1.type != IMMEDIATE) << 1
> + | (op2.type != IMMEDIATE);
> +
> + /* Find bit width of the two operands, if at least one is 64 bit use a */
> + /* 64bit operation, eventually extend 32bit operands. */
This is duplicated elsewhere (e.g., gen_bin_op) - should be pulled into a single function.
> + bool op_is64bit = op1.bit_width == 64 || op2.bit_width == 64;
> + const char *bit_suffix = op_is64bit ? "i64" : "i32";
> + int bit_width = (op_is64bit) ? 64 : 32;
> + if (op_is64bit) {
> + switch (op_types) {
> + case IMM_IMM:
> + break;
> + case IMM_REG:
> + op2 = rvalue_extend(c, locp, &op2);
> + break;
> + case REG_IMM:
> + op1 = rvalue_extend(c, locp, &op1);
> + break;
> + case REG_REG:
> + op1 = rvalue_extend(c, locp, &op1);
> + op2 = rvalue_extend(c, locp, &op2);
> + break;
> + }
> + }
> +static void gen_mini_op(Context *c, YYLTYPE *locp, unsigned bit_width,
> + HexValue *res, enum OpTypes op_types,
> + HexValue *op1_ptr, HexValue *op2_ptr)
> +{
> + HexValue op1 = *op1_ptr;
> + HexValue op2 = *op2_ptr;
> + const char *min = res->is_unsigned ? "tcg_gen_umin" : "tcg_gen_smin";
> + switch (op_types) {
> + case IMM_IMM:
> + OUT(c, locp, "int", &bit_width, "_t ", res, " = (", &op1, " <= ");
> + OUT(c, locp, &op2, ") ? ", &op1, " : ", &op2, ";\n");
> + break;
> + case IMM_REG:
> + op1.bit_width = bit_width;
> + op1 = rvalue_materialize(c, locp, &op1);
> + OUT(c, locp, min, "_i", &bit_width, "(");
> + OUT(c, locp, res, ", ", &op1, ", ", &op2, ");\n");
> + break;
> + case REG_IMM:
> + op2.bit_width = bit_width;
> + op2 = rvalue_materialize(c, locp, &op2);
> + /* Fallthrough */
> + case REG_REG:
> + OUT(c, locp, min, "_i", &bit_width, "(");
> + OUT(c, locp, res, ", ", &op1, ", ", &op2, ");\n");
> + break;
> + }
> + rvalue_free(c, locp, &op1);
> + rvalue_free(c, locp, &op2);
> +}
> +
> +static void gen_maxi_op(Context *c, YYLTYPE *locp, unsigned bit_width,
> + HexValue *res, enum OpTypes op_types,
> + HexValue *op1_ptr, HexValue *op2_ptr)
> +{
> + HexValue op1 = *op1_ptr;
> + HexValue op2 = *op2_ptr;
> + const char *min = res->is_unsigned ? "tcg_gen_umax" : "tcg_gen_smax";
> + switch (op_types) {
> + case IMM_IMM:
> + OUT(c, locp, "int", &bit_width, "_t ", res, " = (", &op1, " <= ");
> + OUT(c, locp, &op2, ") ? ", &op2, " : ", &op1, ";\n");
> + break;
> + case IMM_REG:
> + op1.bit_width = bit_width;
> + op1 = rvalue_materialize(c, locp, &op1);
> + OUT(c, locp, min, "_i", &bit_width, "(");
> + OUT(c, locp, res, ", ", &op1, ", ", &op2, ");\n");
> + break;
> + case REG_IMM:
> + op2.bit_width = bit_width;
> + op2 = rvalue_materialize(c, locp, &op2);
> + /* Fallthrough */
> + case REG_REG:
> + OUT(c, locp, min, "_i", &bit_width, "(");
> + OUT(c, locp, res, ", ", &op1, ", ", &op2, ");\n");
> + break;
> + }
> + rvalue_free(c, locp, &op1);
> + rvalue_free(c, locp, &op2);
> +}
These two look basically the same, create a single function with one extra are indicating min/max.
> +HexValue gen_cast_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *source,
> + unsigned target_width) {
Don't you need to worry about signed-ness of the result?
> + if (source->bit_width == target_width) {
> + return *source;
> + } else if (source->type == IMMEDIATE) {
> + HexValue res = *source;
> + res.bit_width = target_width;
> + return res;
> + } else {
> + HexValue res = gen_tmp(c, locp, target_width);
> + /* Truncate */
> + if (source->bit_width > target_width) {
> + OUT(c, locp, "tcg_gen_trunc_i64_tl(", &res, ", ", source, ");\n");
> + } else {
> + if (source->is_unsigned) {
> + /* Extend unsigned */
> + OUT(c, locp, "tcg_gen_extu_i32_i64(",
> + &res, ", ", source, ");\n");
> + } else {
> + /* Extend signed */
> + OUT(c, locp, "tcg_gen_ext_i32_i64(",
> + &res, ", ", source, ");\n");
> + }
> + }
> + rvalue_free(c, locp, source);
> + return res;
> + }
> +}
> +
> +HexValue gen_extend_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *src_width_ptr,
> + HexValue *dst_width_ptr,
> + HexValue *value_ptr,
> + bool is_unsigned) {
> + HexValue src_width = *src_width_ptr;
> + HexValue dst_width = *dst_width_ptr;
> + HexValue value = *value_ptr;
> + src_width = rvalue_extend(c, locp, &src_width);
> + value = rvalue_extend(c, locp, &value);
> + src_width = rvalue_materialize(c, locp, &src_width);
> + value = rvalue_materialize(c, locp, &value);
> +
> + HexValue res = gen_tmp(c, locp, 64);
> + HexValue shift = gen_tmp_value(c, locp, "64", 64);
> + HexValue zero = gen_tmp_value(c, locp, "0", 64);
> + OUT(c, locp, "tcg_gen_sub_i64(",
> + &shift, ", ", &shift, ", ", &src_width, ");\n");
> + if (is_unsigned) {
> + HexValue mask = gen_tmp_value(c, locp, "0xffffffffffffffff", 64);
> + OUT(c, locp, "tcg_gen_shr_i64(",
> + &mask, ", ", &mask, ", ", &shift, ");\n");
> + OUT(c, locp, "tcg_gen_and_i64(",
> + &res, ", ", &value, ", ", &mask, ");\n");
> + rvalue_free(c, locp, &mask);
Can't you do this with tcg_gen_extract?
> + } else {
> + OUT(c, locp, "tcg_gen_shl_i64(",
> + &res, ", ", &value, ", ", &shift, ");\n");
> + OUT(c, locp, "tcg_gen_sar_i64(",
> + &res, ", ", &res, ", ", &shift, ");\n");
Can't you do this with get_gen_sectract?
> + }
> + OUT(c, locp, "tcg_gen_movcond_i64(TCG_COND_EQ, ", &res, ", ");
> + OUT(c, locp, &src_width, ", ", &zero, ", ", &zero, ", ", &res, ");\n");
> +
> + rvalue_free(c, locp, &src_width);
> + rvalue_free(c, locp, &dst_width);
> + rvalue_free(c, locp, &value);
> + rvalue_free(c, locp, &shift);
> + rvalue_free(c, locp, &zero);
> +
> + res.is_unsigned = is_unsigned;
> + return res;
> +}
> +
> +void gen_rdeposit_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *dest,
> + HexValue *value,
> + HexValue *begin,
> + HexValue *width)
> +{
> + HexValue dest_m = *dest;
> + dest_m.is_manual = true;
> +
> + HexValue value_m = rvalue_extend(c, locp, value);
> + HexValue begin_m = rvalue_extend(c, locp, begin);
> + HexValue width_orig = *width;
> + width_orig.is_manual = true;
> + HexValue width_m = rvalue_extend(c, locp, &width_orig);
> + width_m = rvalue_materialize(c, locp, &width_m);
> +
> + HexValue mask = gen_tmp_value(c, locp, "0xffffffffffffffffUL", 64);
> + mask.is_unsigned = true;
> + HexValue k64 = gen_tmp_value(c, locp, "64", 64);
> + k64 = gen_bin_op(c, locp, SUB_OP, &k64, &width_m);
> + mask = gen_bin_op(c, locp, LSR_OP, &mask, &k64);
> + begin_m.is_manual = true;
> + mask = gen_bin_op(c, locp, ASL_OP, &mask, &begin_m);
> + mask.is_manual = true;
> + value_m = gen_bin_op(c, locp, ASL_OP, &value_m, &begin_m);
> + value_m = gen_bin_op(c, locp, ANDB_OP, &value_m, &mask);
> +
> + OUT(c, locp, "tcg_gen_not_i64(", &mask, ", ", &mask, ");\n");
> + mask.is_manual = false;
> + HexValue res = gen_bin_op(c, locp, ANDB_OP, &dest_m, &mask);
> + res = gen_bin_op(c, locp, ORB_OP, &res, &value_m);
> +
Can't you do this with tcg_gen_deposit?
> + if (dest->bit_width != res.bit_width) {
> + res = rvalue_truncate(c, locp, &res);
> + }
> +
> + HexValue zero = gen_tmp_value(c, locp, "0", res.bit_width);
> + OUT(c, locp, "tcg_gen_movcond_i", &res.bit_width, "(TCG_COND_NE, ",
> dest);
> + OUT(c, locp, ", ", &width_orig, ", ", &zero, ", ", &res, ", ", dest,
> + ");\n");
> +
> + rvalue_free(c, locp, &zero);
> + rvalue_free(c, locp, width);
> + rvalue_free(c, locp, &res);
> +}
> +
> +void gen_deposit_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *dest,
> + HexValue *value,
> + HexValue *index,
> + HexCast *cast)
What's the difference between this and the gen_rdeposit_op above?
> +{
> + yyassert(c, locp, index->type == IMMEDIATE,
> + "Deposit index must be immediate!\n");
> + HexValue value_m = *value;
> + int bit_width = (dest->bit_width == 64) ? 64 : 32;
> + int width = cast->bit_width;
> + /* If the destination value is 32, truncate the value, otherwise extend */
> + if (dest->bit_width != value->bit_width) {
> + if (bit_width == 32) {
> + value_m = rvalue_truncate(c, locp, &value_m);
> + } else {
> + value_m = rvalue_extend(c, locp, &value_m);
> + }
> + }
> + value_m = rvalue_materialize(c, locp, &value_m);
> + OUT(c, locp, "tcg_gen_deposit_i", &bit_width, "(", dest, ", ", dest, ", ");
> + OUT(c, locp, &value_m, ", ", index, " * ", &width, ", ", &width, ");\n");
> + rvalue_free(c, locp, index);
> + rvalue_free(c, locp, &value_m);
> +}
> +
> +HexValue gen_rextract_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *source,
> + int begin,
> + int width) {
> + int bit_width = (source->bit_width == 64) ? 64 : 32;
> + HexValue res = gen_tmp(c, locp, bit_width);
> + OUT(c, locp, "tcg_gen_extract_i", &bit_width, "(", &res);
> + OUT(c, locp, ", ", source, ", ", &begin, ", ", &width, ");\n");
> + rvalue_free(c, locp, source);
> + return res;
> +}
> +
> +HexValue gen_extract_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *source,
> + HexValue *index,
> + HexExtract *extract) {
What's the difference between this ant the gen_rextract_op above?
> + yyassert(c, locp, index->type == IMMEDIATE,
> + "Extract index must be immediate!\n");
> + int bit_width = (source->bit_width == 64) ? 64 : 32;
> + const char *sign_prefix = (extract->is_unsigned) ? "" : "s";
> + int width = extract->bit_width;
> + HexValue res = gen_tmp(c, locp, bit_width);
> + res.is_unsigned = extract->is_unsigned;
> + OUT(c, locp, "tcg_gen_", sign_prefix, "extract_i", &bit_width,
> + "(", &res, ", ", source);
> + OUT(c, locp, ", ", index, " * ", &width, ", ", &width, ");\n");
> +
> + /* Some extract operations have bit_width != storage_bit_width */
> + if (extract->storage_bit_width > bit_width) {
> + HexValue tmp = gen_tmp(c, locp, extract->storage_bit_width);
> + tmp.is_unsigned = extract->is_unsigned;
> + if (extract->is_unsigned) {
> + /* Extend unsigned */
> + OUT(c, locp, "tcg_gen_extu_i32_i64(",
> + &tmp, ", ", &res, ");\n");
> + } else {
> + /* Extend signed */
> + OUT(c, locp, "tcg_gen_ext_i32_i64(",
> + &tmp, ", ", &res, ");\n");
> + }
> + rvalue_free(c, locp, &res);
> + res = tmp;
> + }
> +
> + rvalue_free(c, locp, source);
> + rvalue_free(c, locp, index);
> + return res;
> +}
> +
> +HexValue gen_read_creg(Context *c, YYLTYPE *locp, HexValue *reg)
> +{
> + yyassert(c, locp, reg->type == REGISTER, "reg must be a register!");
> + if (reg->reg.id < 'a') {
What is this check telling us?
> + HexValue tmp = gen_tmp_value(c, locp, "0", 32);
> + const char *id = creg_str[(uint8_t)reg->reg.id];
> + OUT(c, locp, "READ_REG(", &tmp, ", ", id, ");\n");
Change READ_REG to gen_read_reg - that's what the macro is.
> + rvalue_free(c, locp, reg);
> + return tmp;
> + }
> + return *reg;
> +}
> +
> +/* Circular addressing mode with auto-increment */
> +void gen_circ_op(Context *c,
> + YYLTYPE *locp,
> + HexValue *addr,
> + HexValue *increment,
> + HexValue *modifier) {
> + HexValue increment_m = *increment;
> + HexValue cs = gen_tmp(c, locp, 32);
> + increment_m = rvalue_materialize(c, locp, &increment_m);
> + OUT(c, locp, "READ_REG(", &cs, ", HEX_REG_CS0 + MuN);\n");
Change READ_REG to gen_read_reg
> + OUT(c,
> + locp,
> + "gen_helper_fcircadd(",
> + addr,
> + ", ",
> + addr,
> + ", ",
> + &increment_m,
> + ", ",
> + modifier);
> + OUT(c, locp, ", ", &cs, ");\n");
> + rvalue_free(c, locp, &increment_m);
> + rvalue_free(c, locp, modifier);
> + rvalue_free(c, locp, &cs);
> +}
> +void gen_load(Context *c, YYLTYPE *locp, HexValue *num, HexValue *size,
> + bool is_unsigned, HexValue *ea, HexValue *dst)
> +{
> + /* Memop width is specified in the load macro */
> + int bit_width = (size->imm.value > 4) ? 64 : 32;
> + const char *sign_suffix = (size->imm.value > 4)
> + ? ""
> + : ((is_unsigned) ? "u" : "s");
> + char size_suffix[4] = { 0 };
> + /* Create temporary variable (if not present) */
> + if (dst->type == VARID) {
> + /* TODO: this is a common pattern, the parser should be varid-aware.
> */
> + varid_allocate(c, locp, dst, bit_width, is_unsigned);
> + }
> + snprintf(size_suffix, 4, "%" PRIu64, size->imm.value * 8);
> + if (bit_width == 32) {
> + *dst = rvalue_truncate(c, locp, dst);
> + } else {
> + *dst = rvalue_extend(c, locp, dst);
> + }
Why is the truncate/extend needed for the destination?
> + int var_id = find_variable(c, locp, ea);
> + yyassert(c, locp, var_id != -1, "Load variable must exist!\n");
> + /* We need to enforce the variable size */
> + ea->bit_width = g_array_index(c->inst.allocated, Var, var_id).bit_width;
> + if (ea->bit_width != 32) {
> + *ea = rvalue_truncate(c, locp, ea);
> + }
> + OUT(c, locp, "if (insn->slot == 0 && pkt->pkt_has_store_s1) {\n");
> + OUT(c, locp, "process_store(ctx, pkt, 1);\n");
Indent
> + OUT(c, locp, "}\n");
> + OUT(c, locp, "tcg_gen_qemu_ld", size_suffix, sign_suffix);
> + OUT(c, locp, "(", dst, ", ", ea, ", 0);\n");
> + /* If the var in EA was truncated it is now a tmp HexValue, so free it. */
> + rvalue_free(c, locp, ea);
> +}
> +
> +void gen_store(Context *c, YYLTYPE *locp, HexValue *num, HexValue
> *size,
> + HexValue *ea, HexValue *src)
> +{
> + /* Memop width is specified in the store macro */
> + int mem_width = size->imm.value;
> + /* Adjust operand bit width to memop bit width */
> + if (mem_width < 8) {
> + *src = "" locp, src);
> + } else {
> + *src = "" locp, src);
> + }
Why is this needed?
> + assert(ea->type == VARID);
> + int var_id = find_variable(c, locp, ea);
> + yyassert(c, locp, var_id != -1, "Load variable must exist!\n");
> + /* We need to enforce the variable size */
> + ea->bit_width = g_array_index(c->inst.allocated, Var, var_id).bit_width;
> + if (ea->bit_width != 32) {
> + *ea = rvalue_truncate(c, locp, ea);
> + }
How can ea be not 32 bits?
> + *src = "" locp, src);
> + OUT(c, locp, "gen_store", &mem_width, "(cpu_env, ", ea, ", ", src);
> + OUT(c, locp, ", ctx, insn->slot);\n");
> + rvalue_free(c, locp, src);
> + /* If the var in ea was truncated it is now a tmp HexValue, so free it. */
> + rvalue_free(c, locp, ea);
> +}
> +
> +void gen_setbits(Context *c, YYLTYPE *locp, HexValue *hi, HexValue *lo,
> + HexValue *dst, HexValue *val)
> +{
> + yyassert(c, locp, hi->type == IMMEDIATE &&
> + hi->imm.type == VALUE &&
> + lo->type == IMMEDIATE &&
> + lo->imm.type == VALUE,
> + "Range deposit needs immediate values!\n");
> +
> + *val = rvalue_truncate(c, locp, val);
> + unsigned len = hi->imm.value + 1 - lo->imm.value;
> + HexValue tmp = gen_tmp(c, locp, 32);
> + OUT(c, locp, "tcg_gen_neg_i32(", &tmp, ", ", val, ");\n");
> + OUT(c, locp, "tcg_gen_deposit_i32(", dst, ", ", dst, ", ", &tmp, ", ");
> + OUT(c, locp, lo, ", ", &len, ");\n");
This doesn't match the C semantics of fSETBITS
#define fSETBIT(N, DST, VAL) \
do { \
DST = (DST & ~(1ULL << (N))) | (((uint64_t)(VAL)) << (N)); \
} while (0)
#define fGETBIT(N, SRC) (((SRC) >> N) & 1)
#define fSETBITS(HI, LO, DST, VAL) \
do { \
int j; \
for (j = LO; j <= HI; j++) { \
fSETBIT(j, DST, VAL); \
} \
} while (0)
You need to put len copies of LSB(val), so emit something like this
TCGv zero = tcg_const_tl(0);
TCGv _ones_ = tcg_const_tl(~0);
tcg_gen_andi_tl(tmp, val, 1);
tcg_gen_movcond_tl(TCG_COND_NE, tmp, tmp, zero, ones, zero);
tcg_gen_deposit_tl(dst, dst, tmp, lo, len);
tcg_temp_free(zero);
tcg_temp_free(ones);
> +HexValue gen_rvalue_pow(Context *c, YYLTYPE *locp, HexValue *l,
> HexValue *r)
Which instruction calls this? I don't think there is one. If not, remove the POW token from the lexer and the associated rules from the parser.
> +HexValue gen_rvalue_abs(Context *c, YYLTYPE *locp, HexValue *v)
> +{
> + const char *bit_suffix = (v->bit_width == 64) ? "i64" : "i32";
> + int bit_width = (v->bit_width == 64) ? 64 : 32;
> + HexValue res;
> + res.is_unsigned = v->is_unsigned;
> + res.is_dotnew = false;
> + res.is_manual = false;
> + if (v->type == IMMEDIATE) {
> + res.type = IMMEDIATE;
> + res.imm.type = QEMU_TMP;
> + res.imm.index = c->inst.qemu_tmp_count;
> + OUT(c, locp, "int", &bit_width, "_t ", &res, " = abs(", v, ");\n");
> + c->inst.qemu_tmp_count++;
> + } else {
> + res = gen_tmp(c, locp, bit_width);
> + HexValue zero = gen_tmp_value(c, locp, "0", bit_width);
> + OUT(c, locp, "tcg_gen_neg_", bit_suffix, "(", &res, ", ", v, ");\n");
> + OUT(c, locp, "tcg_gen_movcond_i", &bit_width);
> + OUT(c, locp, "(TCG_COND_GT, ", &res, ", ", v, ", ", &zero);
tcg_gen_abs_i<bit_width>
> + OUT(c, locp, ", ", v, ", ", &res, ");\n");
> + rvalue_free(c, locp, &zero);
> + rvalue_free(c, locp, v);
> + }
> + return res;
> +}
> +
> +HexValue gen_rvalue_brev(Context *c, YYLTYPE *locp, HexValue *v)
> +{
> + yyassert(c, locp, v->bit_width <= 32,
> + "fbrev not implemented for 64-bit integers!");
> + HexValue res = gen_tmp(c, locp, v->bit_width);
> + *v = rvalue_materialize(c, locp, v);
> + OUT(c, locp, "gen_fbrev(", &res, ", ", v, ");\n");
gen_helper_fbrev
> diff --git a/target/hexagon/idef-parser/parser-helpers.h
> b/target/hexagon/idef-parser/parser-helpers.h
> new file mode 100644
> +#define OUT(c, locp, ...) FOR_EACH((c), (locp), OUT_IMPL, __VA_ARGS__)
You should be able to handle indenting here. Unfortunately, many of the C statements generated use multiple OUT invocations.
Create two macros
OUT prints indentation, then the text used for beginning a line of output
OUT_CONTINUE just print the text used for continuing a line
> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
> index 329219463f..a2257d41a5 100644
> --- a/target/hexagon/meson.build
> +++ b/target/hexagon/meson.build
> @@ -183,7 +183,7 @@ idef_parser_input_generated = custom_target(
> command: [python, files('gen_idef_parser_funcs.py'),
> semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'],
> )
>
> -idef_parser_input_generated_prep = custom_target(
> +preprocessed_idef_parser_input_generated = custom_target(
Don't change the name of this here, use the name you want in the patch where it was introduced.
------------------------------
Message: 2
Date: Wed, 23 Jun 2021 21:10:39 -0700
From: Vivek Kasireddy <vivek.kasireddy@intel.com>
To: qemu-devel@nongnu.org
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>, Gerd Hoffmann
<kraxel@redhat.com>, Marc-André Lureau <marcandre.lureau@redhat.com>,
Dongwon Kim <dongwon.kim@intel.com>, Tina Zhang <tina.zhang@intel.com>
Subject: [RFC v1 0/1] ui: Add a Wayland backend for Qemu UI
Message-ID: <20210624041040.1250631-1-vivek.kasireddy@intel.com" target="_blank" rel="noreferrer">20210624041040.1250631-1-vivek.kasireddy@intel.com>
Content-Type: text/plain; charset=UTF-8
Why does Qemu need a new Wayland UI backend?
The main reason why there needs to be a plain and simple Wayland backend
for Qemu UI is to eliminate the Blit (aka GPU copy) that happens if using
a toolkit like GTK or SDL (because they use EGL). The Blit can be eliminated
by sharing the dmabuf fd -- associated with the Guest scanout buffer --
directly with the Host compositor via the linux-dmabuf (unstable) protocol.
Once properly integrated, it would be potentially possible to have the
scanout buffer created by the Guest compositor be placed directly on a
hardware plane on the Host thereby improving performance. Only Guest
compositors that use multiple back buffers (at-least 1 front and 1 back)
and virtio-gpu would benefit from this work.
The patch(es) are still WIP and the only reason why I am sending them now
is to get feedback and see if anyone thinks this work is interesting. And,
even after this work is complete, it is not meant to be merged and can be
used for performance testing purposes. Given Qemu UI's new direction, the
proper way to add new backends is to create a separate UI/display module
that is part of the dbus/pipewire infrastructure that Marc-Andre is
working on:
https://lists.nongnu.org/archive/html/qemu-devel/2021-03/msg04331.html
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Cc: Tina Zhang <tina.zhang@intel.com>
Vivek Kasireddy (1):
ui: Add a plain Wayland backend for Qemu UI
configure | 17 ++
meson.build | 25 +++
meson_options.txt | 2 +
qapi/ui.json | 19 ++-
ui/meson.build | 52 ++++++
ui/wayland.c | 402 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 516 insertions(+), 1 deletion(-)
create mode 100644 ui/wayland.c
--
2.30.2
------------------------------
Message: 3
Date: Wed, 23 Jun 2021 21:10:40 -0700
From: Vivek Kasireddy <vivek.kasireddy@intel.com>
To: qemu-devel@nongnu.org
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>, Gerd Hoffmann
<kraxel@redhat.com>
Subject: [RFC v1 1/1] ui: Add a plain Wayland backend for Qemu UI
Message-ID: <20210624041040.1250631-2-vivek.kasireddy@intel.com" target="_blank" rel="noreferrer">20210624041040.1250631-2-vivek.kasireddy@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
configure | 17 ++
meson.build | 25 +++
meson_options.txt | 2 +
qapi/ui.json | 19 ++-
ui/meson.build | 52 ++++++
ui/wayland.c | 402 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 516 insertions(+), 1 deletion(-)
create mode 100644 ui/wayland.c
diff --git a/configure b/configure
index 8dcb9965b2..f6caaa6329 100755
--- a/configure
+++ b/configure
@@ -403,6 +403,7 @@ cfi_debug="false"
seccomp="auto"
glusterfs="auto"
gtk="auto"
+wayland="auto"
tls_priority="NORMAL"
gnutls="$default_feature"
nettle="$default_feature"
@@ -1372,6 +1373,10 @@ for opt do
;;
--enable-gtk) gtk="enabled"
;;
+ --disable-wayland) wayland="disabled"
+ ;;
+ --enable-wayland) wayland="enabled"
+ ;;
--tls-priority=*) tls_priority="$optarg"
;;
--disable-gnutls) gnutls="no"
@@ -1845,6 +1850,7 @@ disabled with --disable-FEATURE, default is enabled if available
sdl SDL UI
sdl-image SDL Image support for icons
gtk gtk UI
+ wayland Wayland UI
vte vte support for the gtk UI
curses curses UI
iconv font glyph conversion support
@@ -3616,6 +3622,11 @@ if $pkg_config gbm; then
gbm="yes"
fi
+if $pkg_config wayland-client; then
+ wayland_cflags="$($pkg_config --cflags wayland-client)"
+ wayland_libs="$($pkg_config --libs wayland-client)"
+fi
+
if test "$opengl" != "no" ; then
epoxy=no
if $pkg_config epoxy; then
@@ -5870,6 +5881,11 @@ if test "$gbm" = "yes" ; then
echo "GBM_CFLAGS=$gbm_cflags" >> $config_host_mak
fi
+if test "$wayland" = "enabled" ; then
+ echo "CONFIG_WAYLAND=y" >> $config_host_mak
+ echo "WAYLAND_LIBS=$wayland_libs" >> $config_host_mak
+ echo "WAYLAND_CFLAGS=$wayland_cflags" >> $config_host_mak
+fi
if test "$avx2_opt" = "yes" ; then
echo "CONFIG_AVX2_OPT=y" >> $config_host_mak
@@ -6436,6 +6452,7 @@ if test "$skip_meson" = no; then
-Dkvm=$kvm -Dhax=$hax -Dwhpx=$whpx -Dhvf=$hvf -Dnvmm=$nvmm \
-Dxen=$xen -Dxen_pci_passthrough=$xen_pci_passthrough -Dtcg=$tcg \
-Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
+ -Dwayland=$wayland \
-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
-Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
-Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
diff --git a/meson.build b/meson.build
index 626cf932c1..dbafe4a5d4 100644
--- a/meson.build
+++ b/meson.build
@@ -855,6 +855,29 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
+
+wayland = not_found
+if not get_option('wayland').auto()
+ wlclientdep = dependency('wayland-client', version: '>= 1.18.90',
+ method: 'pkg-config',
+ required: get_option('wayland'),
+ kwargs: static_kwargs)
+ wlprotocolsdep = dependency('wayland-protocols', version: '>= 1.14.91',
+ method: 'pkg-config',
+ required: get_option('wayland'),
+ kwargs: static_kwargs)
+
+ if not wlprotocolsdep.found()
+ wlproto_dir = subproject('wayland-protocols').get_variable('wayland_protocols_srcdir')
+ else
+ wlproto_dir = wlprotocolsdep.get_pkgconfig_variable('pkgdatadir')
+ endif
+
+ wayland = declare_dependency(dependencies: [wlclientdep, wlprotocolsdep],
+ compile_args: config_host['WAYLAND_CFLAGS'].split(),
+ link_args: config_host['WAYLAND_LIBS'].split())
+endif
+
vnc = not_found
png = not_found
jpeg = not_found
@@ -1146,6 +1169,7 @@ if glusterfs.found()
config_host_data.set('CONFIG_GLUSTERFS_IOCB_HAS_STAT', glusterfs_iocb_has_stat)
endif
config_host_data.set('CONFIG_GTK', gtk.found())
+config_host_data.set('CONFIG_WAYLAND', wayland.found())
config_host_data.set('CONFIG_LIBATTR', have_old_libattr)
config_host_data.set('CONFIG_LIBCAP_NG', libcap_ng.found())
config_host_data.set('CONFIG_EBPF', libbpf.found())
@@ -2695,6 +2719,7 @@ summary_info += {'SDL support': sdl.found()}
summary_info += {'SDL image support': sdl_image.found()}
# TODO: add back version
summary_info += {'GTK support': gtk.found()}
+summary_info += {'Wayland support': wayland.found()}
summary_info += {'pixman': pixman.found()}
# TODO: add back version
summary_info += {'VTE support': config_host.has_key('CONFIG_VTE')}
diff --git a/meson_options.txt b/meson_options.txt
index 3d304cac96..86066c63c9 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -86,6 +86,8 @@ option('rbd', type : 'feature', value : 'auto',
description: 'Ceph block device driver')
option('gtk', type : 'feature', value : 'auto',
description: 'GTK+ user interface')
+option('wayland', type : 'feature', value : 'auto',
+ description: 'Wayland user interface')
option('sdl', type : 'feature', value : 'auto',
description: 'SDL user interface')
option('sdl_image', type : 'feature', value : 'auto',
diff --git a/qapi/ui.json b/qapi/ui.json
index 1052ca9c38..55e5967889 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1057,6 +1057,20 @@
{ 'struct' : 'DisplayEGLHeadless',
'data' : { '*rendernode' : 'str' } }
+##
+# @DisplayWayland:
+#
+# Wayland display options.
+#
+# @rendernode: Which DRM render node should be used. Default is the first
+# available node on the host.
+#
+# Since: 3.1
+#
+##
+{ 'struct' : 'DisplayWayland',
+ 'data' : { '*rendernode' : 'str' } }
+
##
# @DisplayGLMode:
#
@@ -1108,6 +1122,8 @@
# DRI device. Graphical display need to be paired with
# VNC or Spice. (Since 3.1)
#
+# @wayland: The Wayland user interface.
+#
# @curses: Display video output via curses. For graphics device
# models which support a text mode, QEMU can display this
# output using a curses/ncurses interface. Nothing is
@@ -1128,7 +1144,7 @@
{ 'enum' : 'DisplayType',
'data' : [ 'default', 'none', 'gtk', 'sdl',
'egl-headless', 'curses', 'cocoa',
- 'spice-app'] }
+ 'wayland', 'spice-app'] }
##
# @DisplayOptions:
@@ -1154,6 +1170,7 @@
'discriminator' : 'type',
'data' : { 'gtk' : 'DisplayGTK',
'curses' : 'DisplayCurses',
+ 'wayland' : 'DisplayWayland',
'egl-headless' : 'DisplayEGLHeadless'} }
##
diff --git a/ui/meson.build b/ui/meson.build
index a3a187d633..fe255aec04 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -62,6 +62,58 @@ if config_host.has_key('CONFIG_OPENGL') and gbm.found()
ui_modules += {'egl-headless' : egl_headless_ss}
endif
+wayland_scanner = find_program('wayland-scanner')
+proto_sources = [
+ ['xdg-shell', 'stable', ],
+ ['fullscreen-shell', 'unstable', 'v1', ],
+ ['linux-dmabuf', 'unstable', 'v1', ],
+]
+wayland_headers = []
+wayland_proto_sources = []
+
+if wayland.found()
+ foreach p: proto_sources
+ proto_name = p.get(0)
+ proto_stability = p.get(1)
+
+ if proto_stability == 'stable'
+ output_base = proto_name
+ input = files(join_paths(wlproto_dir, '@0@/@1@/@2@.xml'.format(proto_stability, proto_name, output_base)))
+ else
+ proto_version = p.get(2)
+ output_base = '@0@-@1@-@2@'.format(proto_name, proto_stability, proto_version)
+ input = files(join_paths(wlproto_dir, '@0@/@1@/@2@.xml'.format(proto_stability, proto_name, output_base)))
+ endif
+
+ wayland_headers += custom_target('@0@ client header'.format(output_base),
+ input: input,
+ output: '@0@-client-protocol.h'.format(output_base),
+ command: [
+ wayland_scanner,
+ 'client-header',
+ '@INPUT@', '@OUTPUT@',
+ ], build_by_default: true
+ )
+
+ wayland_proto_sources += custom_target('@0@ source'.format(output_base),
+ input: input,
+ output: '@0@-protocol.c'.format(output_base),
+ command: [
+ wayland_scanner,
+ 'private-code',
+ '@INPUT@', '@OUTPUT@',
+ ], build_by_default: true
+ )
+ endforeach
+endif
+
+if wayland.found()
+ wayland_ss = ss.source_set()
+ wayland_ss.add(when: wayland, if_true: files('wayland.c', 'xdg-shell-protocol.c', 'fullscreen-shell-unstable-v1-protocol.c','linux-dmabuf-unstable-v1-protocol.c'))
+ #wayland_ss.add(when: wayland, if_true: files('wayland.c'), [wayland_proto_sources])
+ ui_modules += {'wayland' : wayland_ss}
+endif
+
if gtk.found()
softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('win32-kbd-hook.c'))
diff --git a/ui/wayland.c b/ui/wayland.c
new file mode 100644
index 0000000000..351d7e1146
--- /dev/null
+++ b/ui/wayland.c
@@ -0,0 +1,402 @@
+/*
+ * Wayland UI -- A simple Qemu UI backend to share buffers with Wayland compositors
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Mostly (boilerplate) based on cgit.freedesktop.org/wayland/weston/tree/clients/simple-dmabuf-egl.c
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/main-loop.h"
+#include "sysemu/sysemu.h"
+#include "ui/console.h"
+#include <wayland-client.h>
+#include "xdg-shell-client-protocol.h"
+#include "fullscreen-shell-unstable-v1-client-protocol.h"
+#include "linux-dmabuf-unstable-v1-client-protocol.h"
+
+#define MAX_BUFFERS 3
+
+typedef struct wayland_display {
+ struct wl_display *display;
+ struct wl_registry *registry;
+ struct wl_compositor *compositor;
+ struct xdg_wm_base *wm_base;
+ struct zwp_fullscreen_shell_v1 *fshell;
+ struct zwp_linux_dmabuf_v1 *dmabuf;
+} wayland_display;
+
+typedef struct wayland_buffer {
+ QemuConsole *con;
+ QemuDmaBuf *dmabuf;
+ struct wl_buffer *buffer;
+ bool busy;
+} wayland_buffer;
+
+typedef struct wayland_window {
+ wayland_display *display;
+ DisplayChangeListener dcl;
+ struct wl_surface *surface;
+ struct xdg_surface *xdg_surface;
+ struct xdg_toplevel *xdg_toplevel;
+ struct wl_callback *callback;
+ wayland_buffer buffers[MAX_BUFFERS];
+ wayland_buffer *new_buffer;
+ bool redraw;
+} wayland_window;
+
+static const struct wl_callback_listener frame_listener;
+
+static void
+xdg_surface_handle_configure(void *data, struct xdg_surface *surface,
+ uint32_t serial)
+{
+ xdg_surface_ack_configure(surface, serial);
+}
+
+static const struct xdg_surface_listener xdg_surface_listener = {
+ xdg_surface_handle_configure,
+};
+
+static void
+xdg_toplevel_handle_configure(void *data, struct xdg_toplevel *toplevel,
+ int32_t width, int32_t height,
+ struct wl_array *states)
+{
+}
+
+static void
+xdg_toplevel_handle_close(void *data, struct xdg_toplevel *xdg_toplevel)
+{
+}
+
+static const struct xdg_toplevel_listener xdg_toplevel_listener = {
+ xdg_toplevel_handle_configure,
+ xdg_toplevel_handle_close,
+};
+
+static void wayland_refresh(DisplayChangeListener *dcl)
+{
+ graphic_hw_update(dcl->con);
+}
+
+static QEMUGLContext wayland_create_context(DisplayChangeListener *dcl,
+ QEMUGLParams *params)
+{
+ return NULL;
+}
+
+static void wayland_destroy_context(DisplayChangeListener *dcl,
+ QEMUGLContext ctx)
+{
+}
+
+static int wayland_make_context_current(DisplayChangeListener *dcl,
+ QEMUGLContext ctx)
+{
+ return 0;
+}
+
+static void wayland_scanout_disable(DisplayChangeListener *dcl)
+{
+}
+
+static void wayland_scanout_texture(DisplayChangeListener *dcl,
+ uint32_t backing_id,
+ bool backing_y_0_top,
+ uint32_t backing_width,
+ uint32_t backing_height,
+ uint32_t x, uint32_t y,
+ uint32_t w, uint32_t h)
+{
+}
+
+static void wayland_release_dmabuf(DisplayChangeListener *dcl,
+ QemuDmaBuf *dmabuf)
+{
+}
+
+static void wayland_dispatch_handler(void *opaque)
+{
+ wayland_display *wldpy = opaque;
+
+ wl_display_prepare_read(wldpy->display);
+ wl_display_read_events(wldpy->display);
+ wl_display_dispatch_pending(wldpy->display);
+ wl_display_flush(wldpy->display);
+}
+
+static void wayland_window_redraw(void *data, struct wl_callback *callback,
+ uint32_t time)
+{
+ wayland_window *window = data;
+ QemuDmaBuf *dmabuf = window->new_buffer->dmabuf;
+
+ if (callback) {
+ wl_callback_destroy(callback);
+ window->callback = NULL;
+ }
+ if (!window->redraw) {
+ return;
+ }
+ window->callback = wl_surface_frame(window->surface);
+ wl_callback_add_listener(window->callback, &frame_listener, window);
+
+ wl_surface_attach(window->surface, window->new_buffer->buffer, 0, 0);
+ wl_surface_damage(window->surface, 0, 0, dmabuf->width, dmabuf->height);
+ wl_surface_commit(window->surface);
+ wl_display_flush(window->display->display);
+ window->redraw = false;
+}
+
+static const struct wl_callback_listener frame_listener = {
+ wayland_window_redraw
+};
+
+static void buffer_release(void *data, struct wl_buffer *buf)
+{
+ wayland_buffer *buffer = data;
+ QemuDmaBuf *dmabuf = buffer->dmabuf;
+
+ dmabuf->fence_fd = -1;
+ graphic_hw_gl_block(buffer->con, false);
+ graphic_hw_gl_flushed(buffer->con);
+ buffer->busy = false;
+ wl_buffer_destroy(buf);
+}
+
+static const struct wl_buffer_listener buffer_listener = {
+ buffer_release
+};
+
+static wayland_buffer *window_next_buffer(wayland_window *window)
+{
+ int i;
+
+ for (i = 0; i < MAX_BUFFERS; i++) {
+ if (!window->buffers[i].busy) {
+ return &window->buffers[i];
+ }
+ }
+ return NULL;
+}
+
+static void wayland_scanout_dmabuf(DisplayChangeListener *dcl,
+ QemuDmaBuf *dmabuf)
+{
+ wayland_window *window = container_of(dcl, wayland_window, dcl);
+ wayland_display *display = window->display;
+ wayland_buffer *buffer = window_next_buffer(window);
+ struct zwp_linux_buffer_params_v1 *params;
+
+ if (!buffer) {
+ error_report("Can't find free buffer\n");
+ exit(1);
+ }
+ params = zwp_linux_dmabuf_v1_create_params(display->dmabuf);
+ zwp_linux_buffer_params_v1_add(params, dmabuf->fd, 0, 0, dmabuf->stride,
+ 0, 0);
+ buffer->buffer = zwp_linux_buffer_params_v1_create_immed(params,
+ dmabuf->width,
+ dmabuf->height,
+ dmabuf->fourcc,
+ 0);
+ zwp_linux_buffer_params_v1_destroy(params);
+ buffer->dmabuf = dmabuf;
+ buffer->con = window->dcl.con;
+ window->new_buffer = buffer;
+ dmabuf->fence_fd = 1;
+ wl_buffer_add_listener(buffer->buffer, &buffer_listener, buffer);
+}
+
+static void wayland_scanout_flush(DisplayChangeListener *dcl,
+ uint32_t x, uint32_t y,
+ uint32_t w, uint32_t h)
+{
+ wayland_window *window = container_of(dcl, wayland_window, dcl);
+ static bool first = true;
+
+ if (!window->new_buffer->busy && !first) {
+ graphic_hw_gl_block(window->new_buffer->con, true);
+ }
+
+ window->redraw = true;
+ if (first || !window->callback) {
+ wayland_window_redraw(window, NULL, 0);
+ }
+ window->new_buffer->busy = true;
+ first = false;
+}
+
+static const DisplayChangeListenerOps wayland_ops = {
+ .dpy_name = "wayland",
+ .dpy_refresh = wayland_refresh,
+
+ .dpy_gl_ctx_create = wayland_create_context,
+ .dpy_gl_ctx_destroy = wayland_destroy_context,
+ .dpy_gl_ctx_make_current = wayland_make_context_current,
+
+ .dpy_gl_scanout_disable = wayland_scanout_disable,
+ .dpy_gl_scanout_texture = wayland_scanout_texture,
+ .dpy_gl_scanout_dmabuf = wayland_scanout_dmabuf,
+ .dpy_gl_release_dmabuf = wayland_release_dmabuf,
+ .dpy_gl_update = wayland_scanout_flush,
+};
+
+static void early_wayland_init(DisplayOptions *opts)
+{
+ display_opengl = 1;
+}
+
+static void
+dmabuf_modifier(void *data, struct zwp_linux_dmabuf_v1 *zwp_linux_dmabuf,
+ uint32_t format, uint32_t modifier_hi, uint32_t modifier_lo)
+{
+}
+
+static void
+dmabuf_format(void *data, struct zwp_linux_dmabuf_v1 *zwp_linux_dmabuf,
+ uint32_t format)
+{
+}
+
+static const struct zwp_linux_dmabuf_v1_listener dmabuf_listener = {
+ dmabuf_format,
+ dmabuf_modifier
+};
+
+static void
+xdg_wm_base_ping(void *data, struct xdg_wm_base *shell, uint32_t serial)
+{
+ xdg_wm_base_pong(shell, serial);
+}
+
+static const struct xdg_wm_base_listener wm_base_listener = {
+ xdg_wm_base_ping,
+};
+
+static void
+registry_handle_global(void *data, struct wl_registry *registry,
+ uint32_t id, const char *interface, uint32_t version)
+{
+ wayland_display *d = data;
+
+ if (strcmp(interface, "wl_compositor") == 0) {
+ d->compositor = wl_registry_bind(registry,
+ id, &wl_compositor_interface, 1);
+ } else if (strcmp(interface, "xdg_wm_base") == 0) {
+ d->wm_base = wl_registry_bind(registry,
+ id, &xdg_wm_base_interface, 1);
+ xdg_wm_base_add_listener(d->wm_base, &wm_base_listener, d);
+ } else if (strcmp(interface, "zwp_fullscreen_shell_v1") == 0) {
+ d->fshell = wl_registry_bind(registry,
+ id, &zwp_fullscreen_shell_v1_interface,
+ 1);
+ } else if (strcmp(interface, "zwp_linux_dmabuf_v1") == 0) {
+ d->dmabuf = wl_registry_bind(registry,
+ id, &zwp_linux_dmabuf_v1_interface, 3);
+ zwp_linux_dmabuf_v1_add_listener(d->dmabuf, &dmabuf_listener,
+ d);
+ }
+}
+
+static void
+registry_handle_global_remove(void *data, struct wl_registry *registry,
+ uint32_t name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+ registry_handle_global,
+ registry_handle_global_remove
+};
+
+static wayland_display *create_display(void)
+{
+ wayland_display *display;
+
+ display = g_new0(wayland_display, 1);
+ display->display = wl_display_connect(NULL);
+ assert(display->display);
+
+ display->registry = wl_display_get_registry(display->display);
+ wl_registry_add_listener(display->registry,
+ ®istry_listener, display);
+ wl_display_roundtrip(display->display);
+ if (display->dmabuf == NULL) {
+ error_report("No zwp_linux_dmabuf global\n");
+ exit(1);
+ }
+ return display;
+}
+
+static wayland_window *create_window(wayland_display *display)
+{
+ wayland_window *window;
+
+ window = g_new0(wayland_window, 1);
+ window->display = display;
+ window->surface = wl_compositor_create_surface(display->compositor);
+
+ if (display->wm_base) {
+ window->xdg_surface = xdg_wm_base_get_xdg_surface(display->wm_base,
+ window->surface);
+ assert(window->xdg_surface);
+ xdg_surface_add_listener(window->xdg_surface,
+ &xdg_surface_listener, window);
+ window->xdg_toplevel = xdg_surface_get_toplevel(window->xdg_surface);
+ assert(window->xdg_toplevel);
+ xdg_toplevel_add_listener(window->xdg_toplevel,
+ &xdg_toplevel_listener, window);
+ xdg_toplevel_set_title(window->xdg_toplevel, "qemu-wayland");
+ wl_surface_commit(window->surface);
+ } else if (display->fshell) {
+ zwp_fullscreen_shell_v1_present_surface(display->fshell,
+ window->surface,
+ ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_DEFAULT,
+ NULL);
+ } else {
+ assert(0);
+ }
+
+ return window;
+}
+
+static void wayland_init(DisplayState *ds, DisplayOptions *opts)
+{
+ QemuConsole *con;
+ wayland_display *wldpy;
+ wayland_window *wlwdw;
+ int idx;
+
+ wldpy = create_display();
+ for (idx = 0;; idx++) {
+ con = qemu_console_lookup_by_index(idx);
+ if (!con || !qemu_console_is_graphic(con)) {
+ break;
+ }
+
+ wlwdw = create_window(wldpy);
+ wlwdw->dcl.con = con;
+ wlwdw->dcl.ops = &wayland_ops;
+ register_displaychangelistener(&wlwdw->dcl);
+ }
+ wl_display_roundtrip(wldpy->display);
+ qemu_set_fd_handler(wl_display_get_fd(wldpy->display),
+ wayland_dispatch_handler, NULL, wldpy);
+}
+
+static QemuDisplay qemu_display_wayland = {
+ .type = DISPLAY_TYPE_WAYLAND,
+ .early_init = early_wayland_init,
+ .init = wayland_init,
+};
+
+static void register_wayland(void)
+{
+ qemu_display_register(&qemu_display_wayland);
+}
+
+type_init(register_wayland);
--
2.30.2
------------------------------
Message: 4
Date: Wed, 23 Jun 2021 21:30:46 -0700 (PDT)
From: no-reply@patchew.org
To: vivek.kasireddy@intel.com
Cc: qemu-devel@nongnu.org, marcandre.lureau@redhat.com,
dongwon.kim@intel.com, tina.zhang@intel.com,
vivek.kasireddy@intel.com, kraxel@redhat.com
Subject: Re: [RFC v1 0/1] ui: Add a Wayland backend for Qemu UI
Message-ID: <162450904493.16025.10486341594793128250@7c66fb7bc3ab>
Content-Type: text/plain; charset="utf-8"
Patchew URL: 20210624041040.1250631-1-vivek.kasireddy@intel.com/" rel="noreferrer noreferrer" target="_blank">https://patchew.org/QEMU/20210624041040.1250631-1-vivek.kasireddy@intel.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210624041040.1250631-1-vivek.kasireddy@intel.com" target="_blank" rel="noreferrer">20210624041040.1250631-1-vivek.kasireddy@intel.com
Subject: [RFC v1 0/1] ui: Add a Wayland backend for Qemu UI
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>From https://github.com/patchew-project/qemu
* [new tag] patchew/20210624041040.1250631-1-vivek.kasireddy@intel.com" target="_blank" rel="noreferrer">20210624041040.1250631-1-vivek.kasireddy@intel.com -> patchew/20210624041040.1250631-1-vivek.kasireddy@intel.com" target="_blank" rel="noreferrer">20210624041040.1250631-1-vivek.kasireddy@intel.com
Switched to a new branch 'test'
547ce45 ui: Add a plain Wayland backend for Qemu UI
=== OUTPUT BEGIN ===
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#260:
new file mode 100644
WARNING: line over 80 characters
#266: FILE: ui/wayland.c:2:
+ * Wayland UI -- A simple Qemu UI backend to share buffers with Wayland compositors
ERROR: use QEMU instead of Qemu or QEmu
#266: FILE: ui/wayland.c:2:
+ * Wayland UI -- A simple Qemu UI backend to share buffers with Wayland compositors
ERROR: line over 90 characters
#271: FILE: ui/wayland.c:7:
+ * Mostly (boilerplate) based on cgit.freedesktop.org/wayland/weston/tree/clients/simple-dmabuf-egl.c
ERROR: code indent should never use tabs
#318: FILE: ui/wayland.c:54:
+^I^I^I uint32_t serial)$
ERROR: code indent should never use tabs
#329: FILE: ui/wayland.c:65:
+^I^I^I int32_t width, int32_t height,$
ERROR: code indent should never use tabs
#330: FILE: ui/wayland.c:66:
+^I^I^I struct wl_array *states)$
ERROR: code indent should never use tabs
#459: FILE: ui/wayland.c:195:
+^Ierror_report("Can't find free buffer\n");$
ERROR: Error messages should not contain newlines
#459: FILE: ui/wayland.c:195:
+ error_report("Can't find free buffer\n");
ERROR: code indent should never use tabs
#519: FILE: ui/wayland.c:255:
+^I^I uint32_t format, uint32_t modifier_hi, uint32_t modifier_lo)$
ERROR: code indent should never use tabs
#552: FILE: ui/wayland.c:288:
+^I^I^I id, &wl_compositor_interface, 1);$
ERROR: code indent should never use tabs
#554: FILE: ui/wayland.c:290:
+^Id->wm_base = wl_registry_bind(registry,$
ERROR: code indent should never use tabs
#555: FILE: ui/wayland.c:291:
+^I^I^I^I id, &xdg_wm_base_interface, 1);$
ERROR: code indent should never use tabs
#556: FILE: ui/wayland.c:292:
+^Ixdg_wm_base_add_listener(d->wm_base, &wm_base_listener, d);$
ERROR: code indent should never use tabs
#558: FILE: ui/wayland.c:294:
+^Id->fshell = wl_registry_bind(registry,$
ERROR: code indent should never use tabs
#559: FILE: ui/wayland.c:295:
+^I id, &zwp_fullscreen_shell_v1_interface,$
ERROR: code indent should never use tabs
#560: FILE: ui/wayland.c:296:
+^I 1);$
ERROR: code indent should never use tabs
#562: FILE: ui/wayland.c:298:
+^Id->dmabuf = wl_registry_bind(registry,$
ERROR: code indent should never use tabs
#563: FILE: ui/wayland.c:299:
+^I id, &zwp_linux_dmabuf_v1_interface, 3);$
ERROR: code indent should never use tabs
#564: FILE: ui/wayland.c:300:
+^Izwp_linux_dmabuf_v1_add_listener(d->dmabuf, &dmabuf_listener,$
ERROR: code indent should never use tabs
#565: FILE: ui/wayland.c:301:
+^I d);$
ERROR: code indent should never use tabs
#593: FILE: ui/wayland.c:329:
+^Ierror_report("No zwp_linux_dmabuf global\n");$
ERROR: Error messages should not contain newlines
#593: FILE: ui/wayland.c:329:
+ error_report("No zwp_linux_dmabuf global\n");
ERROR: code indent should never use tabs
#594: FILE: ui/wayland.c:330:
+^Iexit(1);$
ERROR: code indent should never use tabs
#609: FILE: ui/wayland.c:345:
+^I window->surface);$
ERROR: code indent should never use tabs
#621: FILE: ui/wayland.c:357:
+^I window->surface,$
ERROR: line over 90 characters
#622: FILE: ui/wayland.c:358:
+ ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_DEFAULT,
ERROR: code indent should never use tabs
#622: FILE: ui/wayland.c:358:
+^I^I ZWP_FULLSCREEN_SHELL_V1_PRESENT_METHOD_DEFAULT,$
ERROR: code indent should never use tabs
#623: FILE: ui/wayland.c:359:
+^I^I NULL);$
total: 27 errors, 2 warnings, 607 lines checked
Commit 547ce45b800d (ui: Add a plain Wayland backend for Qemu UI) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
20210624041040.1250631-1-vivek.kasireddy@intel.com/testing.checkpatch/?type=message" rel="noreferrer noreferrer" target="_blank">http://patchew.org/logs/20210624041040.1250631-1-vivek.kasireddy@intel.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
------------------------------
Message: 5
Date: Thu, 24 Jun 2021 13:29:32 +1000
From: David Gibson <david@gibson.dropbear.id.au>
To: "Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br>
Cc: qemu-devel@nongnu.org, clg@kaod.org, Richard Henderson
<richard.henderson@linaro.org>, fernando.valle@eldorado.org.br,
matheus.ferst@eldorado.org.br, farosas@linux.ibm.com,
lucas.araujo@eldorado.org.br, luis.pires@eldorado.org.br,
qemu-ppc@nongnu.org, Greg Kurz <groug@kaod.org>
Subject: Re: [PATCH v2 03/10] target/ppc: Push real-mode handling into
ppc_radix64_xlate
Message-ID: <YNP8HPVUgSlFkyAm@yekko>
Content-Type: text/plain; charset="utf-8"
On Mon, Jun 21, 2021 at 09:51:08AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>
> This removes some incomplete duplication between
> ppc_radix64_handle_mmu_fault and ppc_radix64_get_phys_page_debug.
> The former was correct wrt SPR_HRMOR and the latter was not.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.1, thanks.
> ---
> target/ppc/mmu-radix64.c | 77 ++++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 43 deletions(-)
>
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index 1c707d387d..dd5ae69052 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -465,7 +465,6 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
> */
> static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> MMUAccessType access_type,
> - bool relocation,
> hwaddr *raddr, int *psizep, int *protp,
> bool guest_visible)
> {
> @@ -474,6 +473,37 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> ppc_v3_pate_t pate;
> int psize, prot;
> hwaddr g_raddr;
> + bool relocation;
> +
> + assert(!(msr_hv && cpu->vhyp));
> +
> + relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> +
> + /* HV or virtual hypervisor Real Mode Access */
> + if (!relocation && (msr_hv || cpu->vhyp)) {
> + /* In real mode top 4 effective addr bits (mostly) ignored */
> + *raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> +
> + /* In HV mode, add HRMOR if top EA bit is clear */
> + if (msr_hv || !env->has_hv_mode) {
Not in scope, because this is a code motion, but that test looks
bogus. If we don't have an HV mode, we won't have an HRMOR either.
> + if (!(eaddr >> 63)) {
> + *raddr |= env->spr[SPR_HRMOR];
> + }
> + }
> + *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> + *psizep = TARGET_PAGE_BITS;
> + return 0;
> + }
> +
> + /*
> + * Check UPRT (we avoid the check in real mode to deal with
> + * transitional states during kexec.
> + */
> + if (guest_visible && !ppc64_use_proc_tbl(cpu)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "LPCR:UPRT not set in radix mode ! LPCR="
> + TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
> + }
>
> /* Virtual Mode Access - get the fully qualified address */
> if (!ppc_radix64_get_fully_qualified_addr(&cpu->env, eaddr, &lpid, &pid)) {
> @@ -559,43 +589,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> MMUAccessType access_type, int mmu_idx)
> {
> CPUState *cs = CPU(cpu);
> - CPUPPCState *env = &cpu->env;
> int page_size, prot;
> - bool relocation;
> hwaddr raddr;
>
> - assert(!(msr_hv && cpu->vhyp));
> -
> - relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> - /* HV or virtual hypervisor Real Mode Access */
> - if (!relocation && (msr_hv || cpu->vhyp)) {
> - /* In real mode top 4 effective addr bits (mostly) ignored */
> - raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> -
> - /* In HV mode, add HRMOR if top EA bit is clear */
> - if (msr_hv || !env->has_hv_mode) {
> - if (!(eaddr >> 63)) {
> - raddr |= env->spr[SPR_HRMOR];
> - }
> - }
> - tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> - PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
> - TARGET_PAGE_SIZE);
> - return 0;
> - }
> -
> - /*
> - * Check UPRT (we avoid the check in real mode to deal with
> - * transitional states during kexec.
> - */
> - if (!ppc64_use_proc_tbl(cpu)) {
> - qemu_log_mask(LOG_GUEST_ERROR,
> - "LPCR:UPRT not set in radix mode ! LPCR="
> - TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
> - }
> -
> /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
> - if (ppc_radix64_xlate(cpu, eaddr, access_type, relocation, &raddr,
> + if (ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
> &page_size, &prot, true)) {
> return 1;
> }
> @@ -607,18 +605,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>
> hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
> {
> - CPUPPCState *env = &cpu->env;
> int psize, prot;
> hwaddr raddr;
>
> - /* Handle Real Mode */
> - if ((msr_dr == 0) && (msr_hv || cpu->vhyp)) {
> - /* In real mode top 4 effective addr bits (mostly) ignored */
> - return eaddr & 0x0FFFFFFFFFFFFFFFULL;
> - }
> -
> - if (ppc_radix64_xlate(cpu, eaddr, 0, msr_dr, &raddr, &psize,
> - &prot, false)) {
> + if (ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> + &psize, &prot, false)) {
> return -1;
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.nongnu.org/archive/html/qemu-devel/attachments/20210624/1d706e70/attachment.sig>
------------------------------
Message: 6
Date: Thu, 24 Jun 2021 13:19:28 +1000
From: David Gibson <david@gibson.dropbear.id.au>
To: "Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br>
Cc: qemu-devel@nongnu.org, clg@kaod.org, Richard Henderson
<richard.henderson@linaro.org>, fernando.valle@eldorado.org.br,
matheus.ferst@eldorado.org.br, farosas@linux.ibm.com,
lucas.araujo@eldorado.org.br, luis.pires@eldorado.org.br,
qemu-ppc@nongnu.org, Greg Kurz <groug@kaod.org>
Subject: Re: [PATCH v2 02/10] target/ppc: Use MMUAccessType with
*_handle_mmu_fault
Message-ID: <YNP5wNLe/7OqMMaT@yekko>
Content-Type: text/plain; charset="utf-8"
On Mon, Jun 21, 2021 at 09:51:07AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>
> These changes were waiting until we didn't need to match
> the function type of PowerPCCPUClass.handle_mmu_fault.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.1, thanks.
> ---
> target/ppc/mmu-hash32.c | 7 ++-----
> target/ppc/mmu-hash32.h | 4 ++--
> target/ppc/mmu-hash64.c | 6 +-----
> target/ppc/mmu-hash64.h | 4 ++--
> target/ppc/mmu-radix64.c | 7 ++-----
> target/ppc/mmu-radix64.h | 4 ++--
> 6 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 9f0a497657..8f19b43e47 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -415,8 +415,8 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
> return (rpn & ~mask) | (eaddr & mask);
> }
>
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> - int mmu_idx)
> +int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> + MMUAccessType access_type, int mmu_idx)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> @@ -425,11 +425,8 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> ppc_hash_pte32_t pte;
> int prot;
> int need_prot;
> - MMUAccessType access_type;
> hwaddr raddr;
>
> - assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> - access_type = rwx;
> need_prot = prot_for_access_type(access_type);
>
> /* 1. Handle real mode accesses */
> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
> index 898021f0d8..30e35718a7 100644
> --- a/target/ppc/mmu-hash32.h
> +++ b/target/ppc/mmu-hash32.h
> @@ -5,8 +5,8 @@
>
> hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
> hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
> - int mmu_idx);
> +int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> + MMUAccessType access_type, int mmu_idx);
>
> /*
> * Segment register definitions
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 708dffc31b..2febd369b1 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -874,7 +874,7 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
> }
>
> int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> - int rwx, int mmu_idx)
> + MMUAccessType access_type, int mmu_idx)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> @@ -884,13 +884,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> hwaddr ptex;
> ppc_hash_pte64_t pte;
> int exec_prot, pp_prot, amr_prot, prot;
> - MMUAccessType access_type;
> int need_prot;
> hwaddr raddr;
>
> - assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> - access_type = rwx;
> -
> /*
> * Note on LPCR usage: 970 uses HID4, but our special variant of
> * store_spr copies relevant fields into env->spr[SPR_LPCR].
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 4b8b8e7950..3e8a8eec1f 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -8,8 +8,8 @@ void dump_slb(PowerPCCPU *cpu);
>