* Re: Qemu-devel Digest, Vol 219, Issue 440
[not found] <mailman.14172.1624515218.20688.qemu-devel@nongnu.org>
@ 2021-06-24 11:15 ` Anat Heilper
0 siblings, 0 replies; only message in thread
From: Anat Heilper @ 2021-06-24 11:15 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 79241 bytes --]
On Thu, Jun 24, 2021, 09:15 <qemu-devel-request@nongnu.org> wrote:
> 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
> >
>
> 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 = rvalue_truncate(c, locp, src);
> > + } else {
> > + *src = rvalue_extend(c, 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 = rvalue_materialize(c, 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>
> 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>
>
> 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:
> 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
> 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 -> patchew/
> 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
>
> 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);
> >
[-- Attachment #2: Type: text/html, Size: 106537 bytes --]
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-06-24 11:50 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <mailman.14172.1624515218.20688.qemu-devel@nongnu.org>
2021-06-24 11:15 ` Qemu-devel Digest, Vol 219, Issue 440 Anat Heilper
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).