RE: [PATCH] contrib/plugins/execlog: Fix compiler warning
Hi Pierrick,
thanks for your reply, and I will modify and push the patch later.
thanks
Xingtao
> -Original Message-
> From: Pierrick Bouvier
> Sent: Monday, March 25, 2024 12:31 PM
> To: Yao, Xingtao/姚 幸涛 ; Peter Maydell
>
> Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com;
> qemu-devel@nongnu.org
> Subject: Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
>
> On 3/25/24 07:00, Xingtao Yao (Fujitsu) wrote:
> > Pete:
> > Thanks for your comment.
> >
> > I also find a similar patch written by Pierrick:
> > https://lore.kernel.org/qemu-devel/20240118032400.3762658-15-pierrick.
> > bouv...@linaro.org/ but for some reason, the patch was not merged yet.
> >
> > shall I need to continue tracking the fixes of this bug?
> >
>
> Hi Xingtao,
>
> you're doing the right thing here. In my original patch, there was no
> consideration for backward compatibility, to the opposite of what you did.
>
> Alex will be out for several weeks, so it might take some time to get this
> merged,
> but I'm definitely voting for this .
>
> Pierrick
>
> >> -Original Message-
> >> From: Peter Maydell
> >> Sent: Friday, March 22, 2024 7:50 PM
> >> To: Yao, Xingtao/姚 幸涛
> >> Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com;
> >> pierrick.bouv...@linaro.org; qemu-devel@nongnu.org
> >> Subject: Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
> >>
> >> On Wed, 20 Mar 2024 at 02:05, Yao Xingtao via
> >> wrote:
> >>>
> >>> 1. The g_pattern_match_string() is deprecated when glib2 version >=
> 2.70.
> >>> Use g_pattern_spec_match_string() instead to avoid this problem.
> >>>
> >>> 2. The type of second parameter in g_ptr_array_add() is
> >>> 'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
> >>> Cast the type of reg->name to 'gpointer' to avoid this problem.
> >>>
> >>> compiler warning message:
> >>> /root/qemu/contrib/plugins/execlog.c:330:17: warning:
> >> ‘g_pattern_match_string’
> >>> is deprecated: Use 'g_pattern_spec_match_string'
> >>> instead [-Wdeprecated-declarations]
> >>>330 | if (g_pattern_match_string(pat, rd->name) ||
> >>>| ^~
> >>> In file included from /usr/include/glib-2.0/glib.h:67,
> >>> from /root/qemu/contrib/plugins/execlog.c:9:
> >>> /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
> >>> 57 | gboolean g_pattern_match_string (GPatternSpec
> *pspec,
> >>>| ^~
> >>> /root/qemu/contrib/plugins/execlog.c:331:21: warning:
> >> ‘g_pattern_match_string’
> >>> is deprecated: Use 'g_pattern_spec_match_string'
> >>> instead [-Wdeprecated-declarations]
> >>>331 | g_pattern_match_string(pat, rd_lower)) {
> >>>| ^~
> >>> /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
> >>> 57 | gboolean g_pattern_match_string (GPatternSpec
> *pspec,
> >>>| ^~
> >>> /root/qemu/contrib/plugins/execlog.c:339:63: warning: passing
> >>> argument
> >>> 2 of ‘g_ptr_array_add’ discards ‘const’ qualifier from pointer
> >>> target type [-Wdiscarded-qualifiers]
> >>>339 | g_ptr_array_add(all_reg_names,
> >> reg->name);
> >>>|
> >> ~~~^~
> >>> In file included from /usr/include/glib-2.0/glib.h:33:
> >>> /usr/include/glib-2.0/glib/garray.h:198:62: note: expected ‘gpointer’
> >>> {aka ‘void *’} but argument is of type ‘const char *’
> >>>198 |gpointer
> >> data);
> >>>|
> >> ~~^~~~
> >>>
> >>
> >> Hi; thanks for this patch.
> >>
> >> This fixes a bug reported in the bug tracker so we should put
> >>
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
> >>
> >> in the commit message just above your signed-off-by tag.
> >>
> >>
> >>> Signed-off-by: Yao Xingtao
> > I will if needed.
> >
> >>> ---
> >>> contrib/plugins/execlog.c | 7 ++-
> >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> >>> index a1dfd59ab7..41f6774116 100644
> >>> --- a/contrib/plugins/execlog.c
> >>> +++ b/contrib/plugins/execlog.c
> >>> @@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
> >>> for (int p = 0; p < rmatches->len; p++) {
> >>> g_autoptr(GPatternSpec) pat =
> >> g_pattern_spec_new(rmatches->pdata[p]);
> >>> g_autofree gchar *rd_lower =
> >>> g_utf8_strdown(rd->name, -1);
> >>> +#if GLIB_VERSION_MAX_ALLOWED < G_ENCODE_VERSION(2, 70)
> >>
> >> Elsewhere we do glib version checks with
> >>
> >> #if GLIB_CHECK_VERSION(2, 70, 0)
> >> code for 2.70.0 and up;
> >> #else
> >> code for older versions
> >> #endif
> >>
> >> so I think we should probably do that here too.
> >>
> >>> if
Re: [PATCH 04/12] qapi: Tidy up indentation of add_client's example
Markus Armbruster writes:
> Commit d23055b8db8 (qapi: Require descriptions and tagged sections to
> be indented) indented add_client's example too much. Revert that.
>
> Signed-off-by: Markus Armbruster
> ---
> qapi/misc.json | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 1b0c5dad88..ec30e5c570 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -32,9 +32,9 @@
> #
> # Example:
> #
> -# -> { "execute": "add_client", "arguments": { "protocol": "vnc",
> -# "fdname": "myclient"
> } }
> -# <- { "return": {} }
> +# -> { "execute": "add_client", "arguments": { "protocol": "vnc",
> +# "fdname": "myclient" } }
> +# <- { "return": {} }
> ##
> { 'command': 'add_client',
>'data': { 'protocol': 'str', 'fdname': 'str', '*skipauth': 'bool',
> @@ -142,7 +142,7 @@
> # option was passed on the command line.
> #
> # In the "suspended" state, it will completely stop the VM and
> -# cause a transition to the "paused" state. (Since 9.0)
> +# cause a transition to the "paused" state. (Since 9.0)
> #
> # Example:
> #
This hunk belongs to PATCH 11.
Re: [PATCH v8] arm/kvm: Enable support for KVM_ARM_VCPU_PMU_V3_FILTER
Hi Daniel, Thanks for your reviewing. I see your comments in the v7. I have some doubts about what you said about the QAPI. Do you want me to convert the current design into the QAPI parsing like the IOThreadVirtQueueMapping? And we need to add new json definition in the qapi/ directory? Thanks, Shaoqin On 3/22/24 22:53, Daniel P. Berrangé wrote: On Tue, Mar 12, 2024 at 03:48:49AM -0400, Shaoqin Huang wrote: The KVM_ARM_VCPU_PMU_V3_FILTER provides the ability to let the VMM decide which PMU events are provided to the guest. Add a new option `kvm-pmu-filter` as -cpu sub-option to set the PMU Event Filtering. Without the filter, all PMU events are exposed from host to guest by default. The usage of the new sub-option can be found from the updated document (docs/system/arm/cpu-features.rst). Here is an example which shows how to use the PMU Event Filtering, when we launch a guest by use kvm, add such command line: # qemu-system-aarch64 \ -accel kvm \ -cpu host,kvm-pmu-filter="D:0x11-0x11" I mistakenly sent some comments to the older v7 (despite this v8 already existing) about the design of this syntax So for linking up the threads: https://lists.nongnu.org/archive/html/qemu-devel/2024-03/msg04703.html With regards, Daniel -- Shaoqin
Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
On 3/25/24 07:00, Xingtao Yao (Fujitsu) wrote:
Pete:
Thanks for your comment.
I also find a similar patch written by Pierrick:
https://lore.kernel.org/qemu-devel/20240118032400.3762658-15-pierrick.bouv...@linaro.org/
but for some reason, the patch was not merged yet.
shall I need to continue tracking the fixes of this bug?
Hi Xingtao,
you're doing the right thing here. In my original patch, there was no
consideration for backward compatibility, to the opposite of what you did.
Alex will be out for several weeks, so it might take some time to get
this merged, but I'm definitely voting for this .
Pierrick
-Original Message-
From: Peter Maydell
Sent: Friday, March 22, 2024 7:50 PM
To: Yao, Xingtao/姚 幸涛
Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com;
pierrick.bouv...@linaro.org; qemu-devel@nongnu.org
Subject: Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
On Wed, 20 Mar 2024 at 02:05, Yao Xingtao via
wrote:
1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
Use g_pattern_spec_match_string() instead to avoid this problem.
2. The type of second parameter in g_ptr_array_add() is
'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
Cast the type of reg->name to 'gpointer' to avoid this problem.
compiler warning message:
/root/qemu/contrib/plugins/execlog.c:330:17: warning:
‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
330 | if (g_pattern_match_string(pat, rd->name) ||
| ^~
In file included from /usr/include/glib-2.0/glib.h:67,
from /root/qemu/contrib/plugins/execlog.c:9:
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
| ^~
/root/qemu/contrib/plugins/execlog.c:331:21: warning:
‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
331 | g_pattern_match_string(pat, rd_lower)) {
| ^~
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
| ^~
/root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument
2 of ‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target
type [-Wdiscarded-qualifiers]
339 | g_ptr_array_add(all_reg_names,
reg->name);
|
~~~^~
In file included from /usr/include/glib-2.0/glib.h:33:
/usr/include/glib-2.0/glib/garray.h:198:62: note: expected ‘gpointer’
{aka ‘void *’} but argument is of type ‘const char *’
198 |gpointer
data);
|
~~^~~~
Hi; thanks for this patch.
This fixes a bug reported in the bug tracker so we should put
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
in the commit message just above your signed-off-by tag.
Signed-off-by: Yao Xingtao
I will if needed.
---
contrib/plugins/execlog.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..41f6774116 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
for (int p = 0; p < rmatches->len; p++) {
g_autoptr(GPatternSpec) pat =
g_pattern_spec_new(rmatches->pdata[p]);
g_autofree gchar *rd_lower = g_utf8_strdown(rd->name,
-1);
+#if GLIB_VERSION_MAX_ALLOWED < G_ENCODE_VERSION(2, 70)
Elsewhere we do glib version checks with
#if GLIB_CHECK_VERSION(2, 70, 0)
code for 2.70.0 and up;
#else
code for older versions
#endif
so I think we should probably do that here too.
if (g_pattern_match_string(pat, rd->name) ||
g_pattern_match_string(pat, rd_lower)) {
+#else
+if (g_pattern_spec_match_string(pat, rd->name) ||
+g_pattern_spec_match_string(pat, rd_lower)) {
+#endif
thanks, I got it.
Rather than putting this ifdef in the middle of this function, I think it would
be
easier to read if we abstract out a function which does the pattern matching
and whose body calls the right glib function depending on glib version. We
generally call these functions the same as the glib function but with a _qemu
suffix (compare the ones in include/glib-compat.h), so here that would be
g_pattern_spec_match_string_qemu().
Register *reg = init_vcpu_register(rd);
g_ptr_array_add(registers, reg);
@@ -336,7 +341,7 @@ static GPtrArray *registers_init(int vcpu_index)
if (disas_assist) {
[PATCH 3/3] target/hppa: Fix unit carry conditions
Split do_unit_cond to do_unit_zero_cond to only handle
conditions versus zero. These are the only ones that
are legal for UXOR. Simplify trans_uxor accordingly.
Rename do_unit to do_unit_addsub, since xor has been split.
Properly compute carry-out bits for add and subtract,
mirroring the code in do_add and do_sub.
Signed-off-by: Richard Henderson
---
target/hppa/translate.c | 214
1 file changed, 109 insertions(+), 105 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 3fc3e7754c..2bf213c938 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -936,98 +936,44 @@ static DisasCond do_sed_cond(DisasContext *ctx, unsigned
orig, bool d,
return do_log_cond(ctx, c * 2 + f, d, res);
}
-/* Similar, but for unit conditions. */
-
-static DisasCond do_unit_cond(unsigned cf, bool d, TCGv_i64 res,
- TCGv_i64 in1, TCGv_i64 in2)
+/* Similar, but for unit zero conditions. */
+static DisasCond do_unit_zero_cond(unsigned cf, bool d, TCGv_i64 res)
{
-DisasCond cond;
-TCGv_i64 tmp, cb = NULL;
+TCGv_i64 tmp;
uint64_t d_repl = d ? 0x00010001ull : 1;
-
-if (cf & 8) {
-/* Since we want to test lots of carry-out bits all at once, do not
- * do our normal thing and compute carry-in of bit B+1 since that
- * leaves us with carry bits spread across two words.
- */
-cb = tcg_temp_new_i64();
-tmp = tcg_temp_new_i64();
-tcg_gen_or_i64(cb, in1, in2);
-tcg_gen_and_i64(tmp, in1, in2);
-tcg_gen_andc_i64(cb, cb, res);
-tcg_gen_or_i64(cb, cb, tmp);
-}
+uint64_t ones = 0, sgns = 0;
switch (cf >> 1) {
-case 0: /* never / TR */
-cond = cond_make_f();
-break;
-
case 1: /* SBW / NBW */
if (d) {
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x0001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
-} else {
-/* undefined */
-cond = cond_make_f();
+ones = d_repl;
+sgns = d_repl << 31;
}
break;
-
case 2: /* SBZ / NBZ */
-/* See hasless(v,1) from
- * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
- */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x01010101u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x01010101u;
+sgns = ones << 7;
break;
-
case 3: /* SHZ / NHZ */
-tmp = tcg_temp_new_i64();
-tcg_gen_subi_i64(tmp, res, d_repl * 0x00010001u);
-tcg_gen_andc_i64(tmp, tmp, res);
-tcg_gen_andi_i64(tmp, tmp, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, tmp);
+ones = d_repl * 0x00010001u;
+sgns = ones << 15;
break;
-
-case 4: /* SDC / NDC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0xu);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 5: /* SWC / NWC */
-if (d) {
-tcg_gen_andi_i64(cb, cb, d_repl * 0x8000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-} else {
-/* undefined */
-cond = cond_make_f();
-}
-break;
-
-case 6: /* SBC / NBC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80808080u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-case 7: /* SHC / NHC */
-tcg_gen_andi_i64(cb, cb, d_repl * 0x80008000u);
-cond = cond_make_0(TCG_COND_NE, cb);
-break;
-
-default:
-g_assert_not_reached();
}
-if (cf & 1) {
-cond.c = tcg_invert_cond(cond.c);
+if (ones == 0) {
+/* Undefined, or 0/1 (never/always). */
+return cf & 1 ? cond_make_t() : cond_make_f();
}
-return cond;
+/*
+ * See hasless(v,1) from
+ * https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
+ */
+tmp = tcg_temp_new_i64();
+tcg_gen_subi_i64(tmp, res, ones);
+tcg_gen_andc_i64(tmp, tmp, res);
+
+return cond_make_tmp(cf & 1 ? TCG_COND_TSTEQ : TCG_COND_TSTNE,
+ tmp, tcg_constant_i64(sgns));
}
static TCGv_i64 get_carry(DisasContext *ctx, bool d,
@@ -1330,34 +1276,82 @@ static bool do_log_reg(DisasContext *ctx, arg_rrr_cf_d
*a,
return nullify_end(ctx);
}
-static void do_unit(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
-TCGv_i64 in2, unsigned cf, bool d, bool is_tc,
-void (*fn)(TCGv_i64, TCGv_i64, TCGv_i64))
+static void do_unit_addsub(DisasContext *ctx, unsigned rt, TCGv_i64 in1,
+ TCGv_i64 in2, unsigned cf, bool d,
+
[PATCH 2/3] target/hppa: Optimize UADDCM with no condition
With r1 as zero is by far the only usage of UADDCM, as the easiest
way to invert a register. The compiler does occasionally use the
addition step as well, and we can simplify that to avoid a temp
and write directly into the destination.
Signed-off-by: Richard Henderson
---
target/hppa/translate.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a3f425d861..3fc3e7754c 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2763,9 +2763,29 @@ static bool do_uaddcm(DisasContext *ctx, arg_rrr_cf_d
*a, bool is_tc)
{
TCGv_i64 tcg_r1, tcg_r2, tmp;
-if (a->cf) {
-nullify_over(ctx);
+if (a->cf == 0) {
+tcg_r2 = load_gpr(ctx, a->r2);
+tmp = dest_gpr(ctx, a->t);
+
+if (a->r1 == 0) {
+/* UADDCM r0,src,dst is the common idiom for dst = ~src. */
+tcg_gen_not_i64(tmp, tcg_r2);
+} else {
+/*
+ * Recall that r1 - r2 == r1 + ~r2 + 1.
+ * Thus r1 + ~r2 == r1 - r2 - 1,
+ * which does not require an extra temporary.
+ */
+tcg_r1 = load_gpr(ctx, a->r1);
+tcg_gen_sub_i64(tmp, tcg_r1, tcg_r2);
+tcg_gen_subi_i64(tmp, tmp, 1);
+}
+save_gpr(ctx, a->t, tmp);
+cond_free(>null_cond);
+return true;
}
+
+nullify_over(ctx);
tcg_r1 = load_gpr(ctx, a->r1);
tcg_r2 = load_gpr(ctx, a->r2);
tmp = tcg_temp_new_i64();
--
2.34.1
[PATCH for-9.0 0/3] target/hppa: Fix DCOR, UADDCM conditions
Two problems, both related to the reconstruction and computation of carry bits. Simplify UXOR a bit, since no carry is involved. While in the area, optimize UADDCM without condition, as that's the common case for inverting a register. r~ Richard Henderson (3): targt/hppa: Fix DCOR reconstruction of carry bits target/hppa: Optimize UADDCM with no condition target/hppa: Fix unit carry conditions target/hppa/translate.c | 240 ++-- 1 file changed, 132 insertions(+), 108 deletions(-) -- 2.34.1
[PATCH 1/3] targt/hppa: Fix DCOR reconstruction of carry bits
The carry bits for each nibble N are located in bit (N+1)*4,
so the shift by 3 was off by one. Furthermore, the carry bit
for the most significant carry bit is indeed located in bit 64,
which is located in a different storage word.
Use a double-word shift-right to reassemble into a single word
and place them all at bit 0 of their respective nibbles.
Signed-off-by: Richard Henderson
---
target/hppa/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index e041310207..a3f425d861 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2791,7 +2791,7 @@ static bool do_dcor(DisasContext *ctx, arg_rr_cf_d *a,
bool is_i)
nullify_over(ctx);
tmp = tcg_temp_new_i64();
-tcg_gen_shri_i64(tmp, cpu_psw_cb, 3);
+tcg_gen_extract2_i64(tmp, cpu_psw_cb, cpu_psw_cb_msb, 4);
if (!is_i) {
tcg_gen_not_i64(tmp, tmp);
}
--
2.34.1
RE: [PATCH] contrib/plugins/execlog: Fix compiler warning
Pete:
Thanks for your comment.
I also find a similar patch written by Pierrick:
https://lore.kernel.org/qemu-devel/20240118032400.3762658-15-pierrick.bouv...@linaro.org/
but for some reason, the patch was not merged yet.
shall I need to continue tracking the fixes of this bug?
> -Original Message-
> From: Peter Maydell
> Sent: Friday, March 22, 2024 7:50 PM
> To: Yao, Xingtao/姚 幸涛
> Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com;
> pierrick.bouv...@linaro.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
>
> On Wed, 20 Mar 2024 at 02:05, Yao Xingtao via
> wrote:
> >
> > 1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
> >Use g_pattern_spec_match_string() instead to avoid this problem.
> >
> > 2. The type of second parameter in g_ptr_array_add() is
> >'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
> >Cast the type of reg->name to 'gpointer' to avoid this problem.
> >
> > compiler warning message:
> > /root/qemu/contrib/plugins/execlog.c:330:17: warning:
> ‘g_pattern_match_string’
> > is deprecated: Use 'g_pattern_spec_match_string'
> > instead [-Wdeprecated-declarations]
> > 330 | if (g_pattern_match_string(pat, rd->name) ||
> > | ^~
> > In file included from /usr/include/glib-2.0/glib.h:67,
> > from /root/qemu/contrib/plugins/execlog.c:9:
> > /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
> >57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
> > | ^~
> > /root/qemu/contrib/plugins/execlog.c:331:21: warning:
> ‘g_pattern_match_string’
> > is deprecated: Use 'g_pattern_spec_match_string'
> > instead [-Wdeprecated-declarations]
> > 331 | g_pattern_match_string(pat, rd_lower)) {
> > | ^~
> > /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
> >57 | gboolean g_pattern_match_string (GPatternSpec *pspec,
> > | ^~
> > /root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument
> > 2 of ‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target
> > type [-Wdiscarded-qualifiers]
> > 339 | g_ptr_array_add(all_reg_names,
> reg->name);
> > |
> ~~~^~
> > In file included from /usr/include/glib-2.0/glib.h:33:
> > /usr/include/glib-2.0/glib/garray.h:198:62: note: expected ‘gpointer’
> > {aka ‘void *’} but argument is of type ‘const char *’
> > 198 |gpointer
> data);
> > |
> ~~^~~~
> >
>
> Hi; thanks for this patch.
>
> This fixes a bug reported in the bug tracker so we should put
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
>
> in the commit message just above your signed-off-by tag.
>
>
> > Signed-off-by: Yao Xingtao
I will if needed.
> > ---
> > contrib/plugins/execlog.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> > index a1dfd59ab7..41f6774116 100644
> > --- a/contrib/plugins/execlog.c
> > +++ b/contrib/plugins/execlog.c
> > @@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
> > for (int p = 0; p < rmatches->len; p++) {
> > g_autoptr(GPatternSpec) pat =
> g_pattern_spec_new(rmatches->pdata[p]);
> > g_autofree gchar *rd_lower = g_utf8_strdown(rd->name,
> > -1);
> > +#if GLIB_VERSION_MAX_ALLOWED < G_ENCODE_VERSION(2, 70)
>
> Elsewhere we do glib version checks with
>
> #if GLIB_CHECK_VERSION(2, 70, 0)
> code for 2.70.0 and up;
> #else
> code for older versions
> #endif
>
> so I think we should probably do that here too.
>
> > if (g_pattern_match_string(pat, rd->name) ||
> > g_pattern_match_string(pat, rd_lower)) {
> > +#else
> > +if (g_pattern_spec_match_string(pat, rd->name) ||
> > +g_pattern_spec_match_string(pat, rd_lower)) {
> > +#endif
thanks, I got it.
>
> Rather than putting this ifdef in the middle of this function, I think it
> would be
> easier to read if we abstract out a function which does the pattern matching
> and whose body calls the right glib function depending on glib version. We
> generally call these functions the same as the glib function but with a _qemu
> suffix (compare the ones in include/glib-compat.h), so here that would be
> g_pattern_spec_match_string_qemu().
>
> > Register *reg = init_vcpu_register(rd);
> > g_ptr_array_add(registers, reg);
> >
> > @@ -336,7 +341,7 @@ static GPtrArray *registers_init(int vcpu_index)
> > if (disas_assist) {
> > g_mutex_lock(_reg_name_lock);
> >
[PATCH v3] target/riscv: Fix the element agnostic function problem
In RVV and vcrypto instructions, the masked and tail elements are set to 1s
using vext_set_elems_1s function if the vma/vta bit is set. It is the element
agnostic policy.
However, this function can't deal the big endian situation. This patch fixes
the problem by adding handling of such case.
Signed-off-by: Huang Tao
Suggested-by: Richard Henderson
Reviewed-by: LIU Zhiwei
---
Changes in v3:
- use "if (HOST_BIG_ENDIAN)" instead of "#if HOST_BIG_ENDIAN"
Changes in v2:
- Keep the api of vext_set_elems_1s
- Reduce the number of patches.
---
target/riscv/vector_internals.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/target/riscv/vector_internals.c b/target/riscv/vector_internals.c
index 12f5964fbb..36635a1138 100644
--- a/target/riscv/vector_internals.c
+++ b/target/riscv/vector_internals.c
@@ -30,6 +30,28 @@ void vext_set_elems_1s(void *base, uint32_t is_agnostic,
uint32_t cnt,
if (tot - cnt == 0) {
return ;
}
+
+if (HOST_BIG_ENDIAN) {
+/*
+ * Deal the situation when the elements are insdie
+ * only one uint64 block including setting the
+ * masked-off element.
+ */
+if (((tot - 1) ^ cnt) < 8) {
+memset(base + H1(tot - 1), -1, tot - cnt);
+return;
+}
+/*
+ * Otherwise, at least cross two uint64_t blocks.
+ * Set first unaligned block.
+ */
+if (cnt % 8 != 0) {
+uint32_t j = ROUND_UP(cnt, 8);
+memset(base + H1(j - 1), -1, j - cnt);
+cnt = j;
+}
+/* Set other 64bit aligend blocks */
+}
memset(base + cnt, -1, tot - cnt);
}
--
2.41.0
Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor
On Sun, 24 Mar 2024, Dr. David Alan Gilbert wrote:
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
Replace qemu_printf() by monitor_printf() / monitor_puts() in monitor.
Signed-off-by: Philippe Mathieu-Daudé
---
target/m68k/cpu.h | 2 +-
target/m68k/helper.c | 126 +-
target/m68k/monitor.c | 4 +-
3 files changed, 67 insertions(+), 65 deletions(-)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 346427e144..4e4307956d 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -620,6 +620,6 @@ static inline void cpu_get_tb_cpu_state(CPUM68KState *env,
vaddr *pc,
}
}
-void dump_mmu(CPUM68KState *env);
+void dump_mmu(Monitor *mon, CPUM68KState *env);
#endif
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 1a475f082a..310e26dfa1 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -25,7 +25,7 @@
#include "exec/helper-proto.h"
#include "gdbstub/helpers.h"
#include "fpu/softfloat.h"
-#include "qemu/qemu-print.h"
+#include "monitor/monitor.h"
#define SIGNBIT (1u << 31)
@@ -455,28 +455,30 @@ void m68k_switch_sp(CPUM68KState *env)
#if !defined(CONFIG_USER_ONLY)
/* MMU: 68040 only */
-static void print_address_zone(uint32_t logical, uint32_t physical,
+static void print_address_zone(Monitor *mon,
+ uint32_t logical, uint32_t physical,
uint32_t size, int attr)
{
-qemu_printf("%08x - %08x -> %08x - %08x %c ",
-logical, logical + size - 1,
-physical, physical + size - 1,
-attr & 4 ? 'W' : '-');
+monitor_printf(mon, "%08x - %08x -> %08x - %08x %c ",
+ logical, logical + size - 1,
+ physical, physical + size - 1,
+ attr & 4 ? 'W' : '-');
size >>= 10;
if (size < 1024) {
-qemu_printf("(%d KiB)\n", size);
+monitor_printf(mon, "(%d KiB)\n", size);
} else {
size >>= 10;
if (size < 1024) {
-qemu_printf("(%d MiB)\n", size);
+monitor_printf(mon, "(%d MiB)\n", size);
} else {
size >>= 10;
-qemu_printf("(%d GiB)\n", size);
+monitor_printf(mon, "(%d GiB)\n", size);
}
}
}
-static void dump_address_map(CPUM68KState *env, uint32_t root_pointer)
+static void dump_address_map(Monitor *mon, CPUM68KState *env,
+ uint32_t root_pointer)
{
int i, j, k;
int tic_size, tic_shift;
@@ -545,7 +547,7 @@ static void dump_address_map(CPUM68KState *env, uint32_t
root_pointer)
if (first_logical != 0x) {
size = last_logical + (1 << tic_shift) -
first_logical;
-print_address_zone(first_logical,
+print_address_zone(mon, first_logical,
first_physical, size, last_attr);
}
first_logical = logical;
@@ -556,125 +558,125 @@ static void dump_address_map(CPUM68KState *env,
uint32_t root_pointer)
}
if (first_logical != logical || (attr & 4) != (last_attr & 4)) {
size = logical + (1 << tic_shift) - first_logical;
-print_address_zone(first_logical, first_physical, size, last_attr);
+print_address_zone(mon, first_logical, first_physical, size,
last_attr);
}
}
#define DUMP_CACHEFLAGS(a) \
switch (a & M68K_DESC_CACHEMODE) { \
case M68K_DESC_CM_WRTHRU: /* cacheable, write-through */ \
-qemu_printf("T"); \
+monitor_puts(mon, "T"); \
break; \
case M68K_DESC_CM_COPYBK: /* cacheable, copyback */ \
-qemu_printf("C"); \
+monitor_puts(mon, "C"); \
break; \
case M68K_DESC_CM_SERIAL: /* noncachable, serialized */ \
-qemu_printf("S"); \
+monitor_puts(mon, "S"); \
break; \
case M68K_DESC_CM_NCACHE: /* noncachable */ \
-qemu_printf("N"); \
+monitor_puts(mon, "N"); \
break; \
}
-static void dump_ttr(uint32_t ttr)
+static void dump_ttr(Monitor *mon, uint32_t ttr)
{
if ((ttr & M68K_TTR_ENABLED) == 0) {
-qemu_printf("disabled\n");
+monitor_puts(mon, "disabled\n");
return;
}
-qemu_printf("Base: 0x%08x Mask: 0x%08x Control: ",
-ttr & M68K_TTR_ADDR_BASE,
-(ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
+monitor_printf(mon, "Base: 0x%08x Mask: 0x%08x Control: ",
+ ttr & M68K_TTR_ADDR_BASE,
+ (ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
switch (ttr & M68K_TTR_SFIELD) {
case M68K_TTR_SFIELD_USER:
-qemu_printf("U");
+monitor_puts(mon, "U");
break;
case M68K_TTR_SFIELD_SUPER:
-qemu_printf("S");
+monitor_puts(mon, "S");
break;
default:
-
Re: [PATCH v4 0/2] Add support for LAM in QEMU
Ping... On 1/22/2024 4:55 PM, Binbin Wu wrote: Gentle ping... Please help to review and consider applying the patch series. (The KVM part has been merged). On 1/12/2024 2:00 PM, Binbin Wu wrote: Linear-address masking (LAM) [1], modifies the checking that is applied to *64-bit* linear addresses, allowing software to use of the untranslated address bits for metadata and masks the metadata bits before using them as linear addresses to access memory. When the feature is virtualized and exposed to guest, it can be used for efficient address sanitizers (ASAN) implementation and for optimizations in JITs and virtual machines. The KVM patch series can be found in [2]. [1] Intel ISE https://cdrdv2.intel.com/v1/dl/getContent/671368 Chapter Linear Address Masking (LAM) [2] https://lore.kernel.org/kvm/20230913124227.12574-1-binbin...@linux.intel.com --- Changelog v4: - Add a reviewed-by from Xiaoyao for patch 1. - Mask out LAM bit on CR4 if vcpu doesn't support LAM in cpu_x86_update_cr4() (Xiaoyao) v3: - https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg04160.html Binbin Wu (1): target/i386: add control bits support for LAM Robert Hoo (1): target/i386: add support for LAM in CPUID enumeration target/i386/cpu.c | 2 +- target/i386/cpu.h | 9 - target/i386/helper.c | 4 3 files changed, 13 insertions(+), 2 deletions(-) base-commit: f614acb7450282a119d85d759f27eae190476058
Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> Replace qemu_printf() by monitor_printf() / monitor_puts() in monitor.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> target/m68k/cpu.h | 2 +-
> target/m68k/helper.c | 126 +-
> target/m68k/monitor.c | 4 +-
> 3 files changed, 67 insertions(+), 65 deletions(-)
>
> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> index 346427e144..4e4307956d 100644
> --- a/target/m68k/cpu.h
> +++ b/target/m68k/cpu.h
> @@ -620,6 +620,6 @@ static inline void cpu_get_tb_cpu_state(CPUM68KState
> *env, vaddr *pc,
> }
> }
>
> -void dump_mmu(CPUM68KState *env);
> +void dump_mmu(Monitor *mon, CPUM68KState *env);
>
> #endif
> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
> index 1a475f082a..310e26dfa1 100644
> --- a/target/m68k/helper.c
> +++ b/target/m68k/helper.c
> @@ -25,7 +25,7 @@
> #include "exec/helper-proto.h"
> #include "gdbstub/helpers.h"
> #include "fpu/softfloat.h"
> -#include "qemu/qemu-print.h"
> +#include "monitor/monitor.h"
>
> #define SIGNBIT (1u << 31)
>
> @@ -455,28 +455,30 @@ void m68k_switch_sp(CPUM68KState *env)
> #if !defined(CONFIG_USER_ONLY)
> /* MMU: 68040 only */
>
> -static void print_address_zone(uint32_t logical, uint32_t physical,
> +static void print_address_zone(Monitor *mon,
> + uint32_t logical, uint32_t physical,
> uint32_t size, int attr)
> {
> -qemu_printf("%08x - %08x -> %08x - %08x %c ",
> -logical, logical + size - 1,
> -physical, physical + size - 1,
> -attr & 4 ? 'W' : '-');
> +monitor_printf(mon, "%08x - %08x -> %08x - %08x %c ",
> + logical, logical + size - 1,
> + physical, physical + size - 1,
> + attr & 4 ? 'W' : '-');
> size >>= 10;
> if (size < 1024) {
> -qemu_printf("(%d KiB)\n", size);
> +monitor_printf(mon, "(%d KiB)\n", size);
> } else {
> size >>= 10;
> if (size < 1024) {
> -qemu_printf("(%d MiB)\n", size);
> +monitor_printf(mon, "(%d MiB)\n", size);
> } else {
> size >>= 10;
> -qemu_printf("(%d GiB)\n", size);
> +monitor_printf(mon, "(%d GiB)\n", size);
> }
> }
> }
>
> -static void dump_address_map(CPUM68KState *env, uint32_t root_pointer)
> +static void dump_address_map(Monitor *mon, CPUM68KState *env,
> + uint32_t root_pointer)
> {
> int i, j, k;
> int tic_size, tic_shift;
> @@ -545,7 +547,7 @@ static void dump_address_map(CPUM68KState *env, uint32_t
> root_pointer)
> if (first_logical != 0x) {
> size = last_logical + (1 << tic_shift) -
> first_logical;
> -print_address_zone(first_logical,
> +print_address_zone(mon, first_logical,
> first_physical, size, last_attr);
> }
> first_logical = logical;
> @@ -556,125 +558,125 @@ static void dump_address_map(CPUM68KState *env,
> uint32_t root_pointer)
> }
> if (first_logical != logical || (attr & 4) != (last_attr & 4)) {
> size = logical + (1 << tic_shift) - first_logical;
> -print_address_zone(first_logical, first_physical, size, last_attr);
> +print_address_zone(mon, first_logical, first_physical, size,
> last_attr);
> }
> }
>
> #define DUMP_CACHEFLAGS(a) \
> switch (a & M68K_DESC_CACHEMODE) { \
> case M68K_DESC_CM_WRTHRU: /* cacheable, write-through */ \
> -qemu_printf("T"); \
> +monitor_puts(mon, "T"); \
> break; \
> case M68K_DESC_CM_COPYBK: /* cacheable, copyback */ \
> -qemu_printf("C"); \
> +monitor_puts(mon, "C"); \
> break; \
> case M68K_DESC_CM_SERIAL: /* noncachable, serialized */ \
> -qemu_printf("S"); \
> +monitor_puts(mon, "S"); \
> break; \
> case M68K_DESC_CM_NCACHE: /* noncachable */ \
> -qemu_printf("N"); \
> +monitor_puts(mon, "N"); \
> break; \
> }
>
> -static void dump_ttr(uint32_t ttr)
> +static void dump_ttr(Monitor *mon, uint32_t ttr)
> {
> if ((ttr & M68K_TTR_ENABLED) == 0) {
> -qemu_printf("disabled\n");
> +monitor_puts(mon, "disabled\n");
> return;
> }
> -qemu_printf("Base: 0x%08x Mask: 0x%08x Control: ",
> -ttr & M68K_TTR_ADDR_BASE,
> -(ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
> +monitor_printf(mon, "Base: 0x%08x Mask: 0x%08x Control: ",
> + ttr & M68K_TTR_ADDR_BASE,
> + (ttr & M68K_TTR_ADDR_MASK) << M68K_TTR_ADDR_MASK_SHIFT);
> switch (ttr & M68K_TTR_SFIELD) {
> case M68K_TTR_SFIELD_USER:
>
Re: [PATCH 3/3] target/hppa: fix building gva for wide mode
On 3/24/24 11:39, Richard Henderson wrote:
On 3/23/24 22:09, Sven Schnelle wrote:
64 Bit hppa no longer has a fixed 32/32 bit split between space and
offset. Instead it uses 42 bits for the offset. The lower 10 bits of
the space are always zero, leaving 22 bits actually used. Simply or
the values together to build the gva.
Signed-off-by: Sven Schnelle
---
target/hppa/mem_helper.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 84785b5a5c..6f895fced7 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr,
target_ulong reg)
}
static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
- target_ulong r2, vaddr va_b)
+ target_ulong r2, uint64_t spc, uint64_t off)
{
HPPATLBEntry *ent;
- vaddr va_e;
+ vaddr va_b, va_e;
uint64_t va_size;
int mask_shift;
+ va_b = off & gva_offset_mask(env->psw);
+ va_b |= spc << 32;
Actually, no, these instructions don't form a GVA in the normal fashion:
space ← ISR;
offset ← cat(ISR{32..63},IOR{32..63});
VIRTUAL_ADDR ← (space<<32) | (offset);
- vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
But this is wrong too.
Actually, no. The
VIRTUAL_ADDR = (space << 32) | offset
line constructs a 96-bit virtual address. The low 32 bits of ISR have already replaced
the high 32 bits of IOR, so this line really only adds the high 32 bits of space as bits
[96:64] of the full virtual address.
Truncated to 64 bits, the deposit as written is exactly right.
r~
Re: [PATCH 2/3] target/hppa: mask offset bits in gva
On 3/24/24 08:41, Sven Schnelle wrote: 7f09e0: val=000fffb0301f r2=110e0f01 r1=01fff600 phys=fffb 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 'val' is the value constructed from IOR/ISR, Is this byte swapped in some weird way? I do not see how 'val' corresponds to any of the addresses we're talking about. From here, the string "301f" appears to an out-of-context grep hit. r~
Re: [PATCH 3/3] target/hppa: fix building gva for wide mode
On 3/23/24 22:09, Sven Schnelle wrote:
64 Bit hppa no longer has a fixed 32/32 bit split between space and
offset. Instead it uses 42 bits for the offset. The lower 10 bits of
the space are always zero, leaving 22 bits actually used. Simply or
the values together to build the gva.
Signed-off-by: Sven Schnelle
---
target/hppa/mem_helper.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 84785b5a5c..6f895fced7 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong
addr, target_ulong reg)
}
static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
- target_ulong r2, vaddr va_b)
+ target_ulong r2, uint64_t spc, uint64_t off)
{
HPPATLBEntry *ent;
-vaddr va_e;
+vaddr va_b, va_e;
uint64_t va_size;
int mask_shift;
+va_b = off & gva_offset_mask(env->psw);
+va_b |= spc << 32;
Actually, no, these instructions don't form a GVA in the normal fashion:
space ← ISR;
offset ← cat(ISR{32..63},IOR{32..63});
VIRTUAL_ADDR ← (space<<32) | (offset);
-vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
But this is wrong too.
Since the input here is from the result of a previous memory access, which populated
ISR+IOR, I presume any masking would be done from the prior memory access.
We do need to fix the merge, per the docs though.
r~
Re: [PATCH] docs/system/ppc/amigang.rst: Fix formatting
On Sun, 24 Mar 2024 at 16:13, BALATON Zoltan wrote: > > Add missing space to fix character formatting where it was missed in > two places. > > Fixes: 623d9065b6 (docs/system/ppc: Document running Linux on AmigaNG > machines) > Signed-off-by: BALATON Zoltan > --- Reviewed-by: Peter Maydell thanks -- PMM
[PATCH v3 10/17] esp.c: don't assert() if FIFO empty when executing non-DMA SELATNS
The current logic assumes that at least 1 byte is present in the FIFO when executing a non-DMA SELATNS command, but this may not be the case if the guest executes an invalid ESP command sequence. Reported-by: Chuhong Yuan Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index 1aac8f5564..f3aa5364cf 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -762,7 +762,8 @@ static void esp_do_nodma(ESPState *s) case CMD_SELATNS: /* Copy one byte from FIFO into cmdfifo */ -len = esp_fifo_pop_buf(s, buf, 1); +len = esp_fifo_pop_buf(s, buf, + MIN(fifo8_num_used(>fifo), 1)); len = MIN(fifo8_num_free(>cmdfifo), len); fifo8_push_all(>cmdfifo, buf, len); -- 2.39.2
[PATCH v3 02/17] esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in do_command_phase()
The aim is to restrict the esp_fifo_*() functions so that they only operate on
the hardware FIFO. When reading from cmdfifo in do_command_phase() use the
underlying esp_fifo8_pop_buf() function directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 1b7b118a0b..ff51145da7 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -280,7 +280,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
-esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+esp_fifo8_pop_buf(>cmdfifo, buf, cmdlen);
current_lun = scsi_device_find(>bus, 0, s->current_dev->id, s->lun);
if (!current_lun) {
--
2.39.2
[PATCH v3 08/17] esp.c: change esp_fifo_pop_buf() to take ESPState
Now that all users of esp_fifo_pop_buf() operate on the main FIFO there is no
need to pass the FIFO explicitly.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8d2d36d56c..83b621ee0f 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -155,9 +155,9 @@ static uint32_t esp_fifo8_pop_buf(Fifo8 *fifo, uint8_t
*dest, int maxlen)
return n;
}
-static uint32_t esp_fifo_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
+static uint32_t esp_fifo_pop_buf(ESPState *s, uint8_t *dest, int maxlen)
{
-return esp_fifo8_pop_buf(fifo, dest, maxlen);
+return esp_fifo8_pop_buf(>fifo, dest, maxlen);
}
static uint32_t esp_get_tc(ESPState *s)
@@ -459,7 +459,7 @@ static void esp_do_dma(ESPState *s)
s->dma_memory_read(s->dma_opaque, buf, len);
esp_set_tc(s, esp_get_tc(s) - len);
} else {
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
esp_raise_drq(s);
}
@@ -515,7 +515,7 @@ static void esp_do_dma(ESPState *s)
fifo8_push_all(>cmdfifo, buf, len);
esp_set_tc(s, esp_get_tc(s) - len);
} else {
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
esp_raise_drq(s);
@@ -549,7 +549,7 @@ static void esp_do_dma(ESPState *s)
/* Copy FIFO data to device */
len = MIN(s->async_len, ESP_FIFO_SZ);
len = MIN(len, fifo8_num_used(>fifo));
-len = esp_fifo_pop_buf(>fifo, s->async_buf, len);
+len = esp_fifo_pop_buf(s, s->async_buf, len);
esp_raise_drq(s);
}
@@ -713,7 +713,7 @@ static void esp_nodma_ti_dataout(ESPState *s)
}
len = MIN(s->async_len, ESP_FIFO_SZ);
len = MIN(len, fifo8_num_used(>fifo));
-esp_fifo_pop_buf(>fifo, s->async_buf, len);
+esp_fifo_pop_buf(s, s->async_buf, len);
s->async_buf += len;
s->async_len -= len;
s->ti_size += len;
@@ -738,7 +738,7 @@ static void esp_do_nodma(ESPState *s)
switch (s->rregs[ESP_CMD]) {
case CMD_SELATN:
/* Copy FIFO into cmdfifo */
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
@@ -757,7 +757,7 @@ static void esp_do_nodma(ESPState *s)
case CMD_SELATNS:
/* Copy one byte from FIFO into cmdfifo */
-len = esp_fifo_pop_buf(>fifo, buf, 1);
+len = esp_fifo_pop_buf(s, buf, 1);
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
@@ -774,7 +774,7 @@ static void esp_do_nodma(ESPState *s)
case CMD_TI:
/* Copy FIFO into cmdfifo */
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
@@ -792,7 +792,7 @@ static void esp_do_nodma(ESPState *s)
switch (s->rregs[ESP_CMD]) {
case CMD_TI:
/* Copy FIFO into cmdfifo */
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
@@ -821,7 +821,7 @@ static void esp_do_nodma(ESPState *s)
case CMD_SEL | CMD_DMA:
case CMD_SELATN | CMD_DMA:
/* Copy FIFO into cmdfifo */
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
@@ -836,7 +836,7 @@ static void esp_do_nodma(ESPState *s)
case CMD_SEL:
case CMD_SELATN:
/* FIFO already contain entire CDB: copy to cmdfifo and execute */
-len = esp_fifo_pop_buf(>fifo, buf, fifo8_num_used(>fifo));
+len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
--
2.39.2
[PATCH v3 07/17] esp.c: use esp_fifo_push() instead of fifo8_push()
There are still a few places that use fifo8_push() instead of esp_fifo_push() in
order to push a value into the FIFO. Update those places to use esp_fifo_push()
instead.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d474268438..8d2d36d56c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -858,7 +858,7 @@ static void esp_do_nodma(ESPState *s)
return;
}
if (fifo8_is_empty(>fifo)) {
-fifo8_push(>fifo, s->async_buf[0]);
+esp_fifo_push(s, s->async_buf[0]);
s->async_buf++;
s->async_len--;
s->ti_size--;
@@ -881,7 +881,7 @@ static void esp_do_nodma(ESPState *s)
case STAT_ST:
switch (s->rregs[ESP_CMD]) {
case CMD_ICCS:
-fifo8_push(>fifo, s->status);
+esp_fifo_push(s, s->status);
esp_set_phase(s, STAT_MI);
/* Process any message in phase data */
@@ -893,7 +893,7 @@ static void esp_do_nodma(ESPState *s)
case STAT_MI:
switch (s->rregs[ESP_CMD]) {
case CMD_ICCS:
-fifo8_push(>fifo, 0);
+esp_fifo_push(s, 0);
/* Raise end of command interrupt */
s->rregs[ESP_RINTR] |= INTR_FC;
--
2.39.2
[PATCH v3 06/17] esp.c: change esp_fifo_pop() to take ESPState
Now that all users of esp_fifo_pop() operate on the main FIFO there is no need
to pass the FIFO explicitly.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 7e3338815b..d474268438 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -116,13 +116,13 @@ static void esp_fifo_push(ESPState *s, uint8_t val)
fifo8_push(>fifo, val);
}
-static uint8_t esp_fifo_pop(Fifo8 *fifo)
+static uint8_t esp_fifo_pop(ESPState *s)
{
-if (fifo8_is_empty(fifo)) {
+if (fifo8_is_empty(>fifo)) {
return 0;
}
-return fifo8_pop(fifo);
+return fifo8_pop(>fifo);
}
static uint32_t esp_fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
@@ -217,7 +217,7 @@ static uint8_t esp_pdma_read(ESPState *s)
{
uint8_t val;
-val = esp_fifo_pop(>fifo);
+val = esp_fifo_pop(s);
return val;
}
@@ -1184,7 +1184,7 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
switch (saddr) {
case ESP_FIFO:
-s->rregs[ESP_FIFO] = esp_fifo_pop(>fifo);
+s->rregs[ESP_FIFO] = esp_fifo_pop(s);
val = s->rregs[ESP_FIFO];
break;
case ESP_RINTR:
--
2.39.2
[PATCH v3 11/17] esp.c: rework esp_cdb_length() into esp_cdb_ready()
The esp_cdb_length() function is only used as part of a calculation to determine
whether the cmdfifo contains an entire SCSI CDB. Rework esp_cdb_length() into a
new esp_cdb_ready() function which both enables us to handle the case where
scsi_cdb_length() returns -1, plus simplify the logic for its callers.
Suggested-by: Paolo Bonzini
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f3aa5364cf..f47abc36d6 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -425,20 +425,20 @@ static void write_response(ESPState *s)
}
}
-static int esp_cdb_length(ESPState *s)
+static bool esp_cdb_ready(ESPState *s)
{
+int len = fifo8_num_used(>cmdfifo) - s->cmdfifo_cdb_offset;
const uint8_t *pbuf;
-int cmdlen, len;
+int cdblen;
-cmdlen = fifo8_num_used(>cmdfifo);
-if (cmdlen < s->cmdfifo_cdb_offset) {
-return 0;
+if (len <= 0) {
+return false;
}
-pbuf = fifo8_peek_buf(>cmdfifo, cmdlen, NULL);
-len = scsi_cdb_length((uint8_t *)[s->cmdfifo_cdb_offset]);
+pbuf = fifo8_peek_buf(>cmdfifo, len, NULL);
+cdblen = scsi_cdb_length((uint8_t *)[s->cmdfifo_cdb_offset]);
-return len;
+return cdblen < 0 ? false : (len >= cdblen);
}
static void esp_dma_ti_check(ESPState *s)
@@ -806,10 +806,9 @@ static void esp_do_nodma(ESPState *s)
trace_esp_handle_ti_cmd(cmdlen);
/* CDB may be transferred in one or more TI commands */
-if (esp_cdb_length(s) && esp_cdb_length(s) ==
-fifo8_num_used(>cmdfifo) - s->cmdfifo_cdb_offset) {
-/* Command has been received */
-do_cmd(s);
+if (esp_cdb_ready(s)) {
+/* Command has been received */
+do_cmd(s);
} else {
/*
* If data was transferred from the FIFO then raise bus
@@ -832,10 +831,9 @@ static void esp_do_nodma(ESPState *s)
fifo8_push_all(>cmdfifo, buf, len);
/* Handle when DMA transfer is terminated by non-DMA FIFO write */
-if (esp_cdb_length(s) && esp_cdb_length(s) ==
-fifo8_num_used(>cmdfifo) - s->cmdfifo_cdb_offset) {
-/* Command has been received */
-do_cmd(s);
+if (esp_cdb_ready(s)) {
+/* Command has been received */
+do_cmd(s);
}
break;
--
2.39.2
[PATCH v3 15/17] esp.c: update esp_fifo_{push, pop}() to call esp_update_drq()
This ensures that the DRQ line is always set correctly when reading/writing
single bytes to/from the FIFO.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 6fd1a12f23..4895181ec1 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -170,10 +170,11 @@ static void esp_fifo_push(ESPState *s, uint8_t val)
{
if (fifo8_num_used(>fifo) == s->fifo.capacity) {
trace_esp_error_fifo_overrun();
-return;
+} else {
+fifo8_push(>fifo, val);
}
-fifo8_push(>fifo, val);
+esp_update_drq(s);
}
static void esp_fifo_push_buf(ESPState *s, uint8_t *buf, int len)
@@ -184,11 +185,16 @@ static void esp_fifo_push_buf(ESPState *s, uint8_t *buf,
int len)
static uint8_t esp_fifo_pop(ESPState *s)
{
+uint8_t val;
+
if (fifo8_is_empty(>fifo)) {
-return 0;
+val = 0;
+} else {
+val = fifo8_pop(>fifo);
}
-return fifo8_pop(>fifo);
+esp_update_drq(s);
+return val;
}
static uint32_t esp_fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
--
2.39.2
[PATCH v3 16/17] esp.c: ensure esp_pdma_write() always calls esp_fifo_push()
This ensures that esp_update_drq() is called via esp_fifo_push() whenever the
host uses PDMA to transfer data to a SCSI device.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 4895181ec1..04dfd90090 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -282,14 +282,12 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
{
uint32_t dmalen = esp_get_tc(s);
-if (dmalen == 0) {
-return;
-}
-
esp_fifo_push(s, val);
-dmalen--;
-esp_set_tc(s, dmalen);
+if (dmalen && s->drq_state) {
+dmalen--;
+esp_set_tc(s, dmalen);
+}
}
static int esp_select(ESPState *s)
--
2.39.2
[PATCH v3 14/17] esp.c: introduce esp_update_drq() and update esp_fifo_{push, pop}_buf() to use it
This new function sets the DRQ line correctly according to the current transfer
mode, direction and FIFO contents. Update esp_fifo_push_buf() and
esp_fifo_pop_buf()
to use it so that DRQ is always set correctly when reading/writing multiple
bytes
to/from the FIFO.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 48 +++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9e35c00927..6fd1a12f23 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -124,6 +124,48 @@ void esp_request_cancelled(SCSIRequest *req)
}
}
+static void esp_update_drq(ESPState *s)
+{
+bool to_device;
+
+switch (esp_get_phase(s)) {
+case STAT_MO:
+case STAT_CD:
+case STAT_DO:
+to_device = true;
+break;
+
+case STAT_DI:
+case STAT_ST:
+case STAT_MI:
+to_device = false;
+break;
+
+default:
+return;
+}
+
+if (s->dma) {
+/* DMA request so update DRQ according to transfer direction */
+if (to_device) {
+if (fifo8_num_free(>fifo) < 2) {
+esp_lower_drq(s);
+} else {
+esp_raise_drq(s);
+}
+} else {
+if (fifo8_num_used(>fifo) < 2) {
+esp_lower_drq(s);
+} else {
+esp_raise_drq(s);
+}
+}
+} else {
+/* Not a DMA request */
+esp_lower_drq(s);
+}
+}
+
static void esp_fifo_push(ESPState *s, uint8_t val)
{
if (fifo8_num_used(>fifo) == s->fifo.capacity) {
@@ -137,6 +179,7 @@ static void esp_fifo_push(ESPState *s, uint8_t val)
static void esp_fifo_push_buf(ESPState *s, uint8_t *buf, int len)
{
fifo8_push_all(>fifo, buf, len);
+esp_update_drq(s);
}
static uint8_t esp_fifo_pop(ESPState *s)
@@ -180,7 +223,10 @@ static uint32_t esp_fifo8_pop_buf(Fifo8 *fifo, uint8_t
*dest, int maxlen)
static uint32_t esp_fifo_pop_buf(ESPState *s, uint8_t *dest, int maxlen)
{
-return esp_fifo8_pop_buf(>fifo, dest, maxlen);
+uint32_t len = esp_fifo8_pop_buf(>fifo, dest, maxlen);
+
+esp_update_drq(s);
+return len;
}
static uint32_t esp_get_tc(ESPState *s)
--
2.39.2
[PATCH v3 04/17] esp.c: replace cmdfifo use of esp_fifo_pop() in do_message_phase()
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9386704a58..5b169b3720 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -315,7 +315,8 @@ static void do_command_phase(ESPState *s)
static void do_message_phase(ESPState *s)
{
if (s->cmdfifo_cdb_offset) {
-uint8_t message = esp_fifo_pop(>cmdfifo);
+uint8_t message = fifo8_is_empty(>cmdfifo) ? 0 :
+ fifo8_pop(>cmdfifo);
trace_esp_do_identify(message);
s->lun = message & 7;
--
2.39.2
[PATCH v3 09/17] esp.c: introduce esp_fifo_push_buf() function for pushing to the FIFO
Instead of pushing data into the FIFO directly with fifo8_push_all(), add a new
esp_fifo_push_buf() function and use it accordingly.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 83b621ee0f..1aac8f5564 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -116,6 +116,11 @@ static void esp_fifo_push(ESPState *s, uint8_t val)
fifo8_push(>fifo, val);
}
+static void esp_fifo_push_buf(ESPState *s, uint8_t *buf, int len)
+{
+fifo8_push_all(>fifo, buf, len);
+}
+
static uint8_t esp_fifo_pop(ESPState *s)
{
if (fifo8_is_empty(>fifo)) {
@@ -601,7 +606,7 @@ static void esp_do_dma(ESPState *s)
} else {
/* Copy device data to FIFO */
len = MIN(len, fifo8_num_free(>fifo));
-fifo8_push_all(>fifo, s->async_buf, len);
+esp_fifo_push_buf(s, s->async_buf, len);
esp_raise_drq(s);
}
@@ -650,7 +655,7 @@ static void esp_do_dma(ESPState *s)
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, buf, len);
} else {
-fifo8_push_all(>fifo, buf, len);
+esp_fifo_push_buf(s, buf, len);
}
esp_set_tc(s, esp_get_tc(s) - len);
@@ -685,7 +690,7 @@ static void esp_do_dma(ESPState *s)
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, buf, len);
} else {
-fifo8_push_all(>fifo, buf, len);
+esp_fifo_push_buf(s, buf, len);
}
esp_set_tc(s, esp_get_tc(s) - len);
--
2.39.2
[PATCH v3 17/17] esp.c: remove explicit setting of DRQ within ESP state machine
Now the esp_update_drq() is called for all reads/writes to the FIFO, there is
no need to manually raise and lower the DRQ signal.
Signed-off-by: Mark Cave-Ayland
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/611
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1831
---
hw/scsi/esp.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 04dfd90090..5d9b52632e 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -506,7 +506,6 @@ static void esp_dma_ti_check(ESPState *s)
if (esp_get_tc(s) == 0 && fifo8_num_used(>fifo) < 2) {
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
-esp_lower_drq(s);
}
}
@@ -526,7 +525,6 @@ static void esp_do_dma(ESPState *s)
} else {
len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
-esp_raise_drq(s);
}
fifo8_push_all(>cmdfifo, buf, len);
@@ -583,7 +581,6 @@ static void esp_do_dma(ESPState *s)
len = esp_fifo_pop_buf(s, buf, fifo8_num_used(>fifo));
len = MIN(fifo8_num_free(>cmdfifo), len);
fifo8_push_all(>cmdfifo, buf, len);
-esp_raise_drq(s);
}
trace_esp_handle_ti_cmd(cmdlen);
s->ti_size = 0;
@@ -615,7 +612,6 @@ static void esp_do_dma(ESPState *s)
len = MIN(s->async_len, ESP_FIFO_SZ);
len = MIN(len, fifo8_num_used(>fifo));
len = esp_fifo_pop_buf(s, s->async_buf, len);
-esp_raise_drq(s);
}
s->async_buf += len;
@@ -667,7 +663,6 @@ static void esp_do_dma(ESPState *s)
/* Copy device data to FIFO */
len = MIN(len, fifo8_num_free(>fifo));
esp_fifo_push_buf(s, s->async_buf, len);
-esp_raise_drq(s);
}
s->async_buf += len;
@@ -733,7 +728,6 @@ static void esp_do_dma(ESPState *s)
if (fifo8_num_used(>fifo) < 2) {
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
-esp_lower_drq(s);
}
break;
}
@@ -1021,9 +1015,6 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
-/* Ensure DRQ is set correctly for TC underflow or normal completion */
-esp_dma_ti_check(s);
-
if (s->current_req) {
scsi_req_unref(s->current_req);
s->current_req = NULL;
--
2.39.2
[PATCH v3 03/17] esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in do_message_phase()
The aim is to restrict the esp_fifo_*() functions so that they only operate on
the hardware FIFO. When reading from cmdfifo in do_message_phase() use the
underlying esp_fifo8_pop_buf() function directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index ff51145da7..9386704a58 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -325,7 +325,7 @@ static void do_message_phase(ESPState *s)
/* Ignore extended messages for now */
if (s->cmdfifo_cdb_offset) {
int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(>cmdfifo));
-esp_fifo_pop_buf(>cmdfifo, NULL, len);
+esp_fifo8_pop_buf(>cmdfifo, NULL, len);
s->cmdfifo_cdb_offset = 0;
}
}
--
2.39.2
[PATCH v3 12/17] esp.c: prevent cmdfifo overflow in esp_cdb_ready()
During normal use the cmdfifo will never wrap internally and cmdfifo_cdb_offset
will always indicate the start of the SCSI CDB. However it is possible that a
malicious guest could issue an invalid ESP command sequence such that cmdfifo
wraps internally and cmdfifo_cdb_offset could point beyond the end of the FIFO
data buffer.
Add an extra check to fifo8_peek_buf() to ensure that if the cmdfifo has wrapped
internally then esp_cdb_ready() will exit rather than allow scsi_cdb_length() to
access data outside the cmdfifo data buffer.
Reported-by: Chuhong Yuan
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f47abc36d6..d8db33b921 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -429,13 +429,23 @@ static bool esp_cdb_ready(ESPState *s)
{
int len = fifo8_num_used(>cmdfifo) - s->cmdfifo_cdb_offset;
const uint8_t *pbuf;
+uint32_t n;
int cdblen;
if (len <= 0) {
return false;
}
-pbuf = fifo8_peek_buf(>cmdfifo, len, NULL);
+pbuf = fifo8_peek_buf(>cmdfifo, len, );
+if (n < len) {
+/*
+ * In normal use the cmdfifo should never wrap, but include this check
+ * to prevent a malicious guest from reading past the end of the
+ * cmdfifo data buffer below
+ */
+return false;
+}
+
cdblen = scsi_cdb_length((uint8_t *)[s->cmdfifo_cdb_offset]);
return cdblen < 0 ? false : (len >= cdblen);
--
2.39.2
[PATCH v3 13/17] esp.c: move esp_set_phase() and esp_get_phase() towards the beginning of the file
This allows these functions to be used earlier in the file without needing a
separate forward declaration.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d8db33b921..9e35c00927 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -79,6 +79,24 @@ static void esp_lower_drq(ESPState *s)
}
}
+static const char *esp_phase_names[8] = {
+"DATA OUT", "DATA IN", "COMMAND", "STATUS",
+"(reserved)", "(reserved)", "MESSAGE OUT", "MESSAGE IN"
+};
+
+static void esp_set_phase(ESPState *s, uint8_t phase)
+{
+s->rregs[ESP_RSTAT] &= ~7;
+s->rregs[ESP_RSTAT] |= phase;
+
+trace_esp_set_phase(esp_phase_names[phase]);
+}
+
+static uint8_t esp_get_phase(ESPState *s)
+{
+return s->rregs[ESP_RSTAT] & 7;
+}
+
void esp_dma_enable(ESPState *s, int irq, int level)
{
if (level) {
@@ -200,24 +218,6 @@ static uint32_t esp_get_stc(ESPState *s)
return dmalen;
}
-static const char *esp_phase_names[8] = {
-"DATA OUT", "DATA IN", "COMMAND", "STATUS",
-"(reserved)", "(reserved)", "MESSAGE OUT", "MESSAGE IN"
-};
-
-static void esp_set_phase(ESPState *s, uint8_t phase)
-{
-s->rregs[ESP_RSTAT] &= ~7;
-s->rregs[ESP_RSTAT] |= phase;
-
-trace_esp_set_phase(esp_phase_names[phase]);
-}
-
-static uint8_t esp_get_phase(ESPState *s)
-{
-return s->rregs[ESP_RSTAT] & 7;
-}
-
static uint8_t esp_pdma_read(ESPState *s)
{
uint8_t val;
--
2.39.2
[PATCH v3 00/17] [for-9.0] esp: avoid explicit setting of DRQ within ESP state machine
[MCA: Since v1 I've received a few reports of FIFO assert()s being triggered
and a
cmdfifo buffer overflow discovered by fuzzing the updated ESP code. The
updating of
all FIFO push/pop operations to use the esp_fifo_*() functions in this series
provides protection against this, and in conjunction with new patches 9-11 is
enough to prevent all qtest reproducers that I have been sent.
For these reasons I would recommend that this series be applied for 9.0. Many
thanks
to Chuhong Yuan for reporting the issues and providing
qtest
reproducers.]
The ESP device has a DRQ (DMA request) signal that is used to handle flow
control
during DMA transfers. At the moment the DRQ signal is explicitly raised and
lowered at various points in the ESP state machine as required, rather than
implementing the logic described in the datasheet:
"DREQ will remain true as long as either the FIFO contains at least one word (or
one byte if 8-bit mode) to send to memory during DMA read, or has room for one
more
word (or byte if 8-bit mode) in the FIFO during DMA write."
This series updates the ESP device to use the same logic as described above
which
also fixes a couple of outstanding GitLab issues related to recovery handling
during FIFO overflow on 68k Macs using PDMA.
Patches 1-4 update existing users of esp_fifo_pop_buf() and esp_fifo_pop() with
the internal cmdfifo to use the underlying Fifo8 device directly. The aim is
to ensure that all the esp_fifo_*() functions only operate on the ESP's hardware
FIFO.
Patches 5-6 update esp_fifo_push() and esp_fifo_pop() to take ESPState directly
as a parameter to prevent any usage that doesn't reference the ESP hardware
FIFO.
Patch 7 ensures that any usage of fifo8_push() for the ESP hardware FIFO is
updated to use esp_fifo_push() instead.
Patch 8 updates esp_fifo_pop_buf() to take ESPState directly as a parameter
to prevent any usage that doesn't reference the ESP hardware FIFO.
Patch 9 introduces the esp_fifo_push_buf() function for pushing multiple bytes
to the ESP hardware FIFO, and updates callers to use it accordingly.
Patches 10-12 incorporate additional protection to prevent FIFO assert()s and a
cmdfifo buffer overflow discovered via fuzzing.
Patch 13 is just code movement which avoids the use of a forward declaration
whilst
also making it easier to locate the mapping between ESP SCSI phases and their
names.
Patches 14 introduce a new esp_update_drq() function that implements the above
DRQ logic which is called by both esp_fifo_{push,pop}_buf().
Patch 15 updates esp_fifo_push() and esp_fifo_pop() to use the new
esp_update_drq()
function. At this point all reads/writes to the ESP FIFO use the esp_fifo_*()
functions and will set DRQ correctly.
Patch 16 is a small update to the logic in esp_pdma_write() to ensure that
esp_fifo_push() is always called for PDMA writes to the FIFO, thereby ensuring
that esp_update_drq() remains correct even in the case of FIFO overflow.
Finally patch 17 removes all manual calls to esp_raise_drq() and esp_lower_drq()
since the DRQ signal is now updated correctly upon each FIFO read/write access.
Signed-off-by: Mark Cave-Ayland
v3:
- Rebase onto master
- Add patch 1 to move the internals of esp_fifo_pop_buf() to a new
esp_fifo8_pop_buf() function. This allows the FIFO wrap logic to still be
used for managing cmdfifo
- Rework esp_cdb_length() into esp_cdb_ready() as suggested by Paolo in patch
11
- Update the logic in patch 12 to use the new esp_cdb_ready() function
v2:
- Rebase onto master
- Add patches 9-12 to handle FIFO assert()s and cmdfifo overflow as reported by
Chuhong Yuan
Mark Cave-Ayland (17):
esp.c: move esp_fifo_pop_buf() internals to new esp_fifo8_pop_buf()
function
esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in
do_command_phase()
esp.c: replace esp_fifo_pop_buf() with esp_fifo8_pop_buf() in
do_message_phase()
esp.c: replace cmdfifo use of esp_fifo_pop() in do_message_phase()
esp.c: change esp_fifo_push() to take ESPState
esp.c: change esp_fifo_pop() to take ESPState
esp.c: use esp_fifo_push() instead of fifo8_push()
esp.c: change esp_fifo_pop_buf() to take ESPState
esp.c: introduce esp_fifo_push_buf() function for pushing to the FIFO
esp.c: don't assert() if FIFO empty when executing non-DMA SELATNS
esp.c: rework esp_cdb_length() into esp_cdb_ready()
esp.c: prevent cmdfifo overflow in esp_cdb_ready()
esp.c: move esp_set_phase() and esp_get_phase() towards the beginning
of the file
esp.c: introduce esp_update_drq() and update esp_fifo_{push,pop}_buf()
to use it
esp.c: update esp_fifo_{push,pop}() to call esp_update_drq()
esp.c: ensure esp_pdma_write() always calls esp_fifo_push()
esp.c: remove explicit setting of DRQ within ESP state machine
hw/scsi/esp.c | 223 --
1 file changed, 142 insertions(+), 81 deletions(-)
--
2.39.2
[PATCH v3 01/17] esp.c: move esp_fifo_pop_buf() internals to new esp_fifo8_pop_buf() function
Update esp_fifo_pop_buf() to be a simple wrapper onto the new
esp_fifo8_pop_buf()
function.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..1b7b118a0b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -125,7 +125,7 @@ static uint8_t esp_fifo_pop(Fifo8 *fifo)
return fifo8_pop(fifo);
}
-static uint32_t esp_fifo_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
+static uint32_t esp_fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
{
const uint8_t *buf;
uint32_t n, n2;
@@ -155,6 +155,11 @@ static uint32_t esp_fifo_pop_buf(Fifo8 *fifo, uint8_t
*dest, int maxlen)
return n;
}
+static uint32_t esp_fifo_pop_buf(Fifo8 *fifo, uint8_t *dest, int maxlen)
+{
+return esp_fifo8_pop_buf(fifo, dest, maxlen);
+}
+
static uint32_t esp_get_tc(ESPState *s)
{
uint32_t dmalen;
--
2.39.2
[PATCH v3 05/17] esp.c: change esp_fifo_push() to take ESPState
Now that all users of esp_fifo_push() operate on the main FIFO there is no need
to pass the FIFO explicitly.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
---
hw/scsi/esp.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5b169b3720..7e3338815b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -106,14 +106,14 @@ void esp_request_cancelled(SCSIRequest *req)
}
}
-static void esp_fifo_push(Fifo8 *fifo, uint8_t val)
+static void esp_fifo_push(ESPState *s, uint8_t val)
{
-if (fifo8_num_used(fifo) == fifo->capacity) {
+if (fifo8_num_used(>fifo) == s->fifo.capacity) {
trace_esp_error_fifo_overrun();
return;
}
-fifo8_push(fifo, val);
+fifo8_push(>fifo, val);
}
static uint8_t esp_fifo_pop(Fifo8 *fifo)
@@ -229,7 +229,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
return;
}
-esp_fifo_push(>fifo, val);
+esp_fifo_push(s, val);
dmalen--;
esp_set_tc(s, dmalen);
@@ -1240,7 +1240,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t
val)
break;
case ESP_FIFO:
if (!fifo8_is_full(>fifo)) {
-esp_fifo_push(>fifo, val);
+esp_fifo_push(s, val);
}
esp_do_nodma(s);
break;
--
2.39.2
Re: [PATCH 2/3] target/hppa: mask offset bits in gva
Hi Richard, Richard Henderson writes: > In particular Figure 2-14 for "data translation disabled" may be > instructive. Suppose the cpu does not implement all of the physical > address lines (true for all extant pa-risc cpus; qemu implements 40 > bits to match pa-8500 iirc). Suppose when reporting a trap with > translation disabled, it is a truncated physical address that is used > as input to Figure 2-14. > > If that is so, then the fix might be in hppa_set_ior_and_isr. Perhaps > > -env->cr[CR_ISR] &= 0x3fff; > +env->cr[CR_ISR] &= 0x301f; > > Though my argument would suggest the mask should be 0xff for the > 40-bit physical address, which is not what you see at all, so perhaps > the thing is moot. I am at a loss to explain why or how HP-UX gets a > 7-bit hole in the ISR result. > > On the other hand, there are some not-well-documented shenanigans (aka > implementation defined behaviour) between Figure H-8 and Figure H-11, > where the 62-bit absolute address is expanded to a 64-bit logical > physical address and then compacted to a 40-bit implementation > physical address. > > We've already got hacks in place for this in hppa_abs_to_phys_pa2_w1, > which just truncates everything down to 40 bits. But that's probably > not what the processor is really doing. > > Anyhow, will you please try the hppa_set_ior_and_isr change and see if > that fixes your HP-UX problems? The problem occurs with data address translation - it's working without, which is not suprising because no exception can happen there. But as soon as the kernel enables address translation it will hit a data tlb miss exception because it can't find 0xfffb0500 in the page tables. Trying to truncate the ISR in hppa_set_ior_and_isr() for the data translation enabled case leads to this loop: hppa_tlb_fill_excp env=0x55bf06e976e0 addr=0x3ffb0500 size=4 type=0 mmu_idx=9 hppa_tlb_find_entry env=0x55bf06e976e0 ent=0x55bf06e97b30 valid=1 va_b=0x20 va_e=0x2f pa=0x20 hppa_tlb_get_physical_address env=0x55bf06e976e0 ret=-1 prot=5 addr=0x26170c phys=0x26170c hppa_tlb_flush_ent env=0x55bf06e976e0 ent=0x55bf06e97bf0 va_b=0x301b va_e=0x301b0fff pa=0xfffb hppa_tlb_itlba env=0x55bf06e976e0 ent=0x55bf06e97bf0 va_b=0x301b va_e=0x301b0fff pa=0xfffb hppa_tlb_itlbp env=0x55bf06e976e0 ent=0x55bf06e97bf0 access_id=0 u=1 pl2=0 pl1=0 type=1 b=0 d=0 t=0 So qemu is looking up 0x3ffb0500 in the TLB, can't find it, raises an exception, HP-UX says: "ah nice, i have a translation for you", but that doesn't match because we're only stripping the bits in the ISR. As i was a bit puzzled in the beginning what's going on, i dumped the pagetables and wrote a small dump program: 68: val=000f47ff301f r2=110e0f01 r1=01e8ffe0 phys=f47ff000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 680020: val=000f47fe301f r2=110e0f01 r1=01e8ffc0 phys=f47fe000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 680060: val=000f47fc301f r2=110e0f01 r1=01e8ff80 phys=f47fc000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5860: val=000fed3c301f r2=010e0001 r1=01fda780 phys=fed3c000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d58e0: val=000fed38301f r2=010e0001 r1=01fda700 phys=fed38000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d59a0: val=000fed32301f r2=010e0001 r1=01fda640 phys=fed32000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d59e0: val=000fed30301f r2=110e0f01 r1=01fda600 phys=fed3 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5a00: val=000fed2f301f r2=010e0001 r1=01fda5e0 phys=fed2f000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5a20: val=000fed2e301f r2=010e0001 r1=01fda5c0 phys=fed2e000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5a40: val=000fed2d301f r2=010e0001 r1=01fda5a0 phys=fed2d000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5a60: val=000fed2c301f r2=010e0001 r1=01fda580 phys=fed2c000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5a80: val=000fed2b301f r2=010e0001 r1=01fda560 phys=fed2b000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5aa0: val=000fed2a301f r2=010e0001 r1=01fda540 phys=fed2a000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5ac0: val=000fed29301f r2=010e0001 r1=01fda520 phys=fed29000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5ae0: val=000fed28301f r2=010e0001 r1=01fda500 phys=fed28000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5b00: val=000fed27301f r2=010e0001 r1=01fda4e0 phys=fed27000 4K aid=1 pl1=0, pl2=0 type=1 (DATA RW) 7d5b20: val=000fed26301f r2=010e0001 r1=01fda4c0 phys=fed26000 4K aid=1 pl1=0, pl2=0 type=1
Re: [PATCH 2/3] target/hppa: mask offset bits in gva
On 3/23/24 22:09, Sven Schnelle wrote:
The CPU seems to mask a few bits in the offset when running
under HP-UX. ISR/IOR register contents for an address in
the processor HPA (0xfffa) on my C8000 and J6750:
running on Linux: 3fff c000fffa0500
running on HP-UX: 301f c000fffa0500
I haven't found how this is switched (guess some diag in the
firmware), but linux + seabios seems to handle that as well,
so lets mask out the additional bits.
Signed-off-by: Sven Schnelle
---
target/hppa/cpu.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a072d0bb63..9bc4d208fa 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -283,12 +283,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
void hppa_translate_init(void);
+#define HPPA_GVA_OFFSET_MASK64 0x301f
#define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
static inline uint64_t gva_offset_mask(target_ulong psw)
{
return (psw & PSW_W
-? MAKE_64BIT_MASK(0, 62)
+? HPPA_GVA_OFFSET_MASK64
: MAKE_64BIT_MASK(0, 32));
}
I'm not keen on this, because it contradicts the manual for forming an address.
Where I can imagine this sort of thing creeping in is the fact that you're getting a
result from trap registers. The cpu does not actually retain the original {space, offset}
tuple that formed the GVA to fill the trap registers, but takes bits [62:32] and
back-computes a space, and subtracts to re-form an offset. See "Interruption Parameter
Registers" in the pa20 manual.
In particular Figure 2-14 for "data translation disabled" may be instructive. Suppose the
cpu does not implement all of the physical address lines (true for all extant pa-risc
cpus; qemu implements 40 bits to match pa-8500 iirc). Suppose when reporting a trap with
translation disabled, it is a truncated physical address that is used as input to Figure 2-14.
If that is so, then the fix might be in hppa_set_ior_and_isr. Perhaps
-env->cr[CR_ISR] &= 0x3fff;
+env->cr[CR_ISR] &= 0x301f;
Though my argument would suggest the mask should be 0xff for the 40-bit physical address,
which is not what you see at all, so perhaps the thing is moot. I am at a loss to explain
why or how HP-UX gets a 7-bit hole in the ISR result.
On the other hand, there are some not-well-documented shenanigans (aka implementation
defined behaviour) between Figure H-8 and Figure H-11, where the 62-bit absolute address
is expanded to a 64-bit logical physical address and then compacted to a 40-bit
implementation physical address.
We've already got hacks in place for this in hppa_abs_to_phys_pa2_w1, which just truncates
everything down to 40 bits. But that's probably not what the processor is really doing.
Anyhow, will you please try the hppa_set_ior_and_isr change and see if that fixes your
HP-UX problems?
r~
Re: [PULL 00/15] riscv-to-apply queue
On 3/24/24 12:07, Michael Tokarev wrote: 22.03.2024 22:46, Daniel Henrique Barboza : On 3/22/24 14:16, Michael Tokarev wrote: 22.03.2024 11:53, Alistair Francis : RISC-V PR for 9.0 * Do not enable all named features by default * A range of Vector fixes * Update APLIC IDC after claiming iforce register * Remove the dependency of Zvfbfmin to Zfbfmin * Fix mode in riscv_tlb_fill * Fix timebase-frequency when using KVM acceleration Should something from there be picked up for stable (8.2 and probably 7.2)? Ignore the "Do not enable all named features by default" since it's fixing something that were added in 9.0. The rest you can pick it up to 8.2 at least. Thanks, Unfortunately this doesn't quite work, the following changes fail to apply to 8.2: 929e521a47 target/riscv: always clear vstart for ldst_whole insns b46631f122 target/riscv: remove 'over' brconds from vector trans d57dfe4b37 trans_rvv.c.inc: remove redundant mark_vs_dirty() calls bac802ada8 target/riscv: enable 'vstart_eq_zero' in the end of insns 385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM acceleration I tried to back-port at least the first one but it turned out to be another failure. Didn't try looking at the rest. This particular code (vector emulation) has been going through a lot of changes in the last couple of releases, so I'm not surprised with the difficulty with backporting these. If these really should be in 8.2 (it's your guys to decide, not me), I need help with back-porting these to 8.2 (and/or cherry-picking additional patches from master). The amount of work can be non-trivial for this backport, so I'd say we should leave it aside for now. If someone has a good argument for this work then we can re-evaluate. Thanks, Daniel Thanks, /mjt Daniel Henrique Barboza (10): target/riscv: do not enable all named features by default target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX() trans_rvv.c.inc: set vstart = 0 in int scalar move insns target/riscv/vector_helper.c: fix 'vmvr_v' memcpy endianess target/riscv: always clear vstart in whole vec move insns target/riscv: always clear vstart for ldst_whole insns target/riscv/vector_helpers: do early exit when vstart >= vl target/riscv: remove 'over' brconds from vector trans trans_rvv.c.inc: remove redundant mark_vs_dirty() calls target/riscv/vector_helper.c: optimize loops in ldst helpers Frank Chang (1): hw/intc: Update APLIC IDC after claiming iforce register Irina Ryapolova (1): target/riscv: Fix mode in riscv_tlb_fill Ivan Klokov (1): target/riscv: enable 'vstart_eq_zero' in the end of insns Max Chou (1): target/riscv: rvv: Remove the dependency of Zvfbfmin to Zfbfmin Yong-Xuan Wang (1): target/riscv/kvm: fix timebase-frequency when using KVM acceleration
Re: [PATCH 1/3] target/hppa: use gva_offset_mask() everywhere
On 3/23/24 22:09, Sven Schnelle wrote: move it to cpu.h, so it can also be used in hppa_form_gva_psw() Signed-off-by: Sven Schnelle --- target/hppa/cpu.h | 10 -- target/hppa/translate.c | 12 +++- 2 files changed, 11 insertions(+), 11 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction
On 3/24/24 18:13, Richard Henderson wrote:
On 3/23/24 11:15, Helge Deller wrote:
The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
triggers a trap if the condition is true, and stores the result of the
addition in the target register otherwise.
It does not use the condition to nullify the following instruction, so
drop the calculated condition and do not install it as null_cond.
That's not what the manual says:
if (trapc == TC && cond_satisfied)
conditional_trap;
else {
GR[t] ← res;
if (cond_satisfied) PSW[N] ← 1;
}
The above is from the 64-bit manual.
My mail was based on the info from the 32-bit manual, which says:
res ← GR[r1] + ∼GR[r2];
if (cond_satisfied)
conditional_trap;
else
GR[t] ← res
But basically it doesn't matter, since as you explain below,
if it traps, it won't return anyway and as such PSW[N] will
not be modified.
We have implemented this like so:
if (trapc == TC)
conditional_trap(cond_satisfied);
GR[t] ← res;
if (cond_satisfied) PSW[N] ← 1;
because the conditional trap step does not return if it traps.
Yes.
Thanks for review anyway!
Helge
This patch is not tested and as such sent as RFC.
I just stumbled over the apparently wrong behaviour while debugging the
uaddcm instruction.
Under separate cover you said the condition might be computed incorrectly. I
think that is more likely the root cause of the wrong behaviour.
r~
Re: [PATCH 3/3] target/hppa: fix building gva for wide mode
On 3/23/24 22:09, Sven Schnelle wrote:
64 Bit hppa no longer has a fixed 32/32 bit split between space and
offset. Instead it uses 42 bits for the offset. The lower 10 bits of
the space are always zero, leaving 22 bits actually used. Simply or
the values together to build the gva.
Signed-off-by: Sven Schnelle
---
target/hppa/mem_helper.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 84785b5a5c..6f895fced7 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong
addr, target_ulong reg)
}
static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
- target_ulong r2, vaddr va_b)
+ target_ulong r2, uint64_t spc, uint64_t off)
{
HPPATLBEntry *ent;
-vaddr va_e;
+vaddr va_b, va_e;
uint64_t va_size;
int mask_shift;
+va_b = off & gva_offset_mask(env->psw);
+va_b |= spc << 32;
Indeed, this ought to be using
va_b = hppa_form_gva_psw(env->psw, off, spc << 32);
With that,
Reviewed-by: Richard Henderson
r~
Re: [PATCH][RFC] target/hppa: Avoid nullification for uaddcmt instruction
On 3/23/24 11:15, Helge Deller wrote:
The uaddcmt (UNIT ADD COMPLEMENT AND TRAP ON CONDITION) instruction
triggers a trap if the condition is true, and stores the result of the
addition in the target register otherwise.
It does not use the condition to nullify the following instruction, so
drop the calculated condition and do not install it as null_cond.
That's not what the manual says:
if (trapc == TC && cond_satisfied)
conditional_trap;
else {
GR[t] ← res;
if (cond_satisfied) PSW[N] ← 1;
}
We have implemented this like so:
if (trapc == TC)
conditional_trap(cond_satisfied);
GR[t] ← res;
if (cond_satisfied) PSW[N] ← 1;
because the conditional trap step does not return if it traps.
This patch is not tested and as such sent as RFC.
I just stumbled over the apparently wrong behaviour while debugging the
uaddcm instruction.
Under separate cover you said the condition might be computed incorrectly. I think that
is more likely the root cause of the wrong behaviour.
r~
[PATCH v2 6/6] tests/qtest: Add tests for the STM32L4x5 USART
Test:
- read/write from/to the usart registers
- send/receive a character/string over the serial port
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
tests/qtest/meson.build| 3 +-
tests/qtest/stm32l4x5_usart-test.c | 326 +
2 files changed, 328 insertions(+), 1 deletion(-)
create mode 100644 tests/qtest/stm32l4x5_usart-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 36c5c13a7b..e0d72ee91e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -205,7 +205,8 @@ qtests_stm32l4x5 = \
['stm32l4x5_exti-test',
'stm32l4x5_syscfg-test',
'stm32l4x5_rcc-test',
- 'stm32l4x5_gpio-test']
+ 'stm32l4x5_gpio-test',
+ 'stm32l4x5_usart-test']
qtests_arm = \
(config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
diff --git a/tests/qtest/stm32l4x5_usart-test.c
b/tests/qtest/stm32l4x5_usart-test.c
new file mode 100644
index 00..2d42f053f6
--- /dev/null
+++ b/tests/qtest/stm32l4x5_usart-test.c
@@ -0,0 +1,326 @@
+/*
+ * QTest testcase for STML4X5_USART
+ *
+ * Copyright (c) 2023 Arnaud Minier
+ * Copyright (c) 2023 Inès Varhol
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/misc/stm32l4x5_rcc_internals.h"
+#include "hw/registerfields.h"
+
+#define RCC_BASE_ADDR 0x40021000
+/* Use USART 1 ADDR, assume the others work the same */
+#define USART1_BASE_ADDR 0x40013800
+
+/* See stm32l4x5_usart for definitions */
+REG32(CR1, 0x00)
+FIELD(CR1, M1, 28, 1)
+FIELD(CR1, OVER8, 15, 1)
+FIELD(CR1, M0, 12, 1)
+FIELD(CR1, PCE, 10, 1)
+FIELD(CR1, TXEIE, 7, 1)
+FIELD(CR1, RXNEIE, 5, 1)
+FIELD(CR1, TE, 3, 1)
+FIELD(CR1, RE, 2, 1)
+FIELD(CR1, UE, 0, 1)
+REG32(CR2, 0x04)
+REG32(CR3, 0x08)
+FIELD(CR3, OVRDIS, 12, 1)
+REG32(BRR, 0x0C)
+REG32(GTPR, 0x10)
+REG32(RTOR, 0x14)
+REG32(RQR, 0x18)
+REG32(ISR, 0x1C)
+FIELD(ISR, TXE, 7, 1)
+FIELD(ISR, RXNE, 5, 1)
+FIELD(ISR, ORE, 3, 1)
+REG32(ICR, 0x20)
+REG32(RDR, 0x24)
+REG32(TDR, 0x28)
+
+#define NVIC_ISPR1 0XE000E204
+#define NVIC_ICPR1 0xE000E284
+#define USART1_IRQ 37
+
+static bool check_nvic_pending(QTestState *qts, unsigned int n)
+{
+/* No USART interrupts are less than 32 */
+assert(n > 32);
+n -= 32;
+return qtest_readl(qts, NVIC_ISPR1) & (1 << n);
+}
+
+static bool clear_nvic_pending(QTestState *qts, unsigned int n)
+{
+/* No USART interrupts are less than 32 */
+assert(n > 32);
+n -= 32;
+qtest_writel(qts, NVIC_ICPR1, (1 << n));
+return true;
+}
+
+/*
+ Tests should never need to sleep(), because while it might be plenty of time
on a
+ fast development machine, it can cause intermittent failures due
+ to timeouts if the test is on some heavily-loaded slow CI runner.
+ */
+static bool usart_wait_for_flag(QTestState *qts, uint32_t event_addr, uint32_t
flag)
+{
+/* Wait at most 10 minutes */
+for (int i = 0; i < 60; i++) {
+if ((qtest_readl(qts, event_addr) & flag)) {
+return true;
+}
+g_usleep(1000);
+}
+
+return false;
+}
+
+static void usart_receive_string(QTestState *qts, int sock_fd, const char *in,
+ char *out)
+{
+int i, in_len = strlen(in);
+
+g_assert_true(send(sock_fd, in, in_len, 0) == in_len);
+for (i = 0; i < in_len; i++) {
+g_assert_true(usart_wait_for_flag(qts,
+USART1_BASE_ADDR + A_ISR, R_ISR_RXNE_MASK));
+out[i] = qtest_readl(qts, USART1_BASE_ADDR + A_RDR);
+}
+out[i] = '\0';
+}
+
+static void usart_send_string(QTestState *qts, const char *in)
+{
+int i, in_len = strlen(in);
+
+for (i = 0; i < in_len; i++) {
+qtest_writel(qts, USART1_BASE_ADDR + A_TDR, in[i]);
+g_assert_true(usart_wait_for_flag(qts,
+USART1_BASE_ADDR + A_ISR, R_ISR_TXE_MASK));
+}
+}
+
+/* Init the RCC clocks to run at 80 MHz */
+static void init_clocks(QTestState *qts)
+{
+uint32_t value;
+
+/* MSIRANGE can be set only when MSI is OFF or READY */
+qtest_writel(qts, (RCC_BASE_ADDR + A_CR), R_CR_MSION_MASK);
+
+/* Clocking from MSI, in case MSI was not the default source */
+qtest_writel(qts, (RCC_BASE_ADDR + A_CFGR), 0);
+
+/*
+ * Update PLL and set MSI as the source clock.
+ * PLLM = 1 --> 000
+ * PLLN = 40 --> 40
+ * PPLLR = 2 --> 00
+ * PLLDIV = unused, PLLP = unused (SAI3), PLLQ = unused (48M1)
+ * SRC = MSI --> 01
+ */
+qtest_writel(qts, (RCC_BASE_ADDR + A_PLLCFGR), R_PLLCFGR_PLLREN_MASK |
+(40 << R_PLLCFGR_PLLN_SHIFT) |
+(0b01 << R_PLLCFGR_PLLSRC_SHIFT));
+
+/* PLL activation */
+
+value = qtest_readl(qts, (RCC_BASE_ADDR + A_CR));
+qtest_writel(qts, (RCC_BASE_ADDR + A_CR), value | R_CR_PLLON_MASK);
+
+/* RCC_CFGR is OK by defaut */
+qtest_writel(qts,
[PATCH v2 4/6] hw/char/stm32l4x5_usart: Add options for serial parameters setting
Add a function to change the settings of the
serial connection.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
hw/char/stm32l4x5_usart.c | 98 +++
hw/char/trace-events | 1 +
2 files changed, 99 insertions(+)
diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
index ec8c2f6e63..b4d11dd826 100644
--- a/hw/char/stm32l4x5_usart.c
+++ b/hw/char/stm32l4x5_usart.c
@@ -267,6 +267,92 @@ static void usart_cancel_transmit(Stm32l4x5UsartBaseState
*s)
}
}
+static void stm32l4x5_update_params(Stm32l4x5UsartBaseState *s)
+{
+int speed, parity, data_bits, stop_bits;
+uint32_t value, usart_div;
+QEMUSerialSetParams ssp;
+
+/* Select the parity type */
+if (s->cr1 & R_CR1_PCE_MASK) {
+if (s->cr1 & R_CR1_PS_MASK) {
+parity = 'O';
+} else {
+parity = 'E';
+}
+} else {
+parity = 'N';
+}
+
+/* Select the number of stop bits */
+switch (FIELD_EX32(s->cr2, CR2, STOP)) {
+case 0:
+stop_bits = 1;
+break;
+case 2:
+stop_bits = 2;
+break;
+default:
+qemu_log_mask(LOG_UNIMP,
+"UNIMPLEMENTED: fractionnal stop bits; CR2[13:12] = %x",
+FIELD_EX32(s->cr2, CR2, STOP));
+return;
+}
+
+/* Select the length of the word */
+switch ((FIELD_EX32(s->cr1, CR1, M1) << 1) | FIELD_EX32(s->cr1, CR1, M0)) {
+case 0:
+data_bits = 8;
+break;
+case 1:
+data_bits = 9;
+break;
+case 2:
+data_bits = 7;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+"UNDEFINED: invalid word length, CR1.M = 0b11");
+return;
+}
+
+/* Select the baud rate */
+value = FIELD_EX32(s->brr, BRR, BRR);
+if (value < 16) {
+qemu_log_mask(LOG_GUEST_ERROR,
+"UNDEFINED: BRR lesser than 16: %u", value);
+return;
+}
+
+if (FIELD_EX32(s->cr1, CR1, OVER8) == 0) {
+/*
+ * Oversampling by 16
+ * BRR = USARTDIV
+ */
+usart_div = value;
+} else {
+/*
+ * Oversampling by 8
+ * - BRR[2:0] = USARTDIV[3:0] shifted 1 bit to the right.
+ * - BRR[3] must be kept cleared.
+ * - BRR[15:4] = USARTDIV[15:4]
+ * - The frequency is multiplied by 2
+ */
+usart_div = ((value & 0xFFF0) | ((value & 0x0007) << 1)) / 2;
+}
+
+speed = clock_get_hz(s->clk) / usart_div;
+
+ssp.speed = speed;
+ssp.parity= parity;
+ssp.data_bits = data_bits;
+ssp.stop_bits = stop_bits;
+
+qemu_chr_fe_ioctl(>chr, CHR_IOCTL_SERIAL_SET_PARAMS, );
+
+trace_stm32l4x5_usart_update_params(speed, parity, data_bits, stop_bits);
+}
+
static void stm32l4x5_usart_base_reset_hold(Object *obj)
{
Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
@@ -365,16 +451,19 @@ static void stm32l4x5_usart_base_write(void *opaque,
hwaddr addr,
switch (addr) {
case A_CR1:
s->cr1 = value;
+stm32l4x5_update_params(s);
stm32l4x5_update_irq(s);
return;
case A_CR2:
s->cr2 = value;
+stm32l4x5_update_params(s);
return;
case A_CR3:
s->cr3 = value;
return;
case A_BRR:
s->brr = value;
+stm32l4x5_update_params(s);
return;
case A_GTPR:
s->gtpr = value;
@@ -443,10 +532,19 @@ static void stm32l4x5_usart_base_init(Object *obj)
s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
}
+static int stm32l4x5_usart_base_post_load(void *opaque, int version_id)
+{
+Stm32l4x5UsartBaseState *s = (Stm32l4x5UsartBaseState *)opaque;
+
+stm32l4x5_update_params(s);
+return 0;
+}
+
static const VMStateDescription vmstate_stm32l4x5_usart_base = {
.name = TYPE_STM32L4X5_USART_BASE,
.version_id = 1,
.minimum_version_id = 1,
+.post_load = stm32l4x5_usart_base_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT32(cr1, Stm32l4x5UsartBaseState),
VMSTATE_UINT32(cr2, Stm32l4x5UsartBaseState),
diff --git a/hw/char/trace-events b/hw/char/trace-events
index f22f0ee2bc..8875758076 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -116,6 +116,7 @@ stm32l4x5_usart_irq_raised(uint32_t reg) "USART: IRQ
raised: 0x%08"PRIx32
stm32l4x5_usart_irq_lowered(void) "USART: IRQ lowered"
stm32l4x5_usart_overrun_detected(uint8_t current, uint8_t received) "USART:
Overrun detected, RDR='0x%x', received 0x%x"
stm32l4x5_usart_receiver_not_enabled(uint8_t ue_bit, uint8_t re_bit) "USART:
Receiver not enabled, UE=0x%x, RE=0x%x"
+stm32l4x5_usart_update_params(int speed, uint8_t parity, int data, int stop)
"USART: speed: %d, parity: %c, data bits: %d, stop bits: %d"
# xen_console.c
xen_console_connect(unsigned int idx, unsigned int ring_ref, unsigned int
port, unsigned int limit) "idx %u ring_ref %u port %u limit %u"
--
[PATCH v2 5/6] hw/arm: Add the USART to the stm32l4x5 SoC
Add the USART to the SoC and connect it to the other implemented devices.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
docs/system/arm/b-l475e-iot01a.rst | 2 +-
hw/arm/Kconfig | 1 +
hw/arm/stm32l4x5_soc.c | 82 +++---
include/hw/arm/stm32l4x5_soc.h | 13 +
4 files changed, 91 insertions(+), 7 deletions(-)
diff --git a/docs/system/arm/b-l475e-iot01a.rst
b/docs/system/arm/b-l475e-iot01a.rst
index 0afef8e4f4..a76c9976c5 100644
--- a/docs/system/arm/b-l475e-iot01a.rst
+++ b/docs/system/arm/b-l475e-iot01a.rst
@@ -19,13 +19,13 @@ Currently B-L475E-IOT01A machine's only supports the
following devices:
- STM32L4x5 SYSCFG (System configuration controller)
- STM32L4x5 RCC (Reset and clock control)
- STM32L4x5 GPIOs (General-purpose I/Os)
+- STM32L4x5 USARTs, UARTs and LPUART (Serial ports)
Missing devices
"""
The B-L475E-IOT01A does *not* support the following devices:
-- Serial ports (UART)
- Analog to Digital Converter (ADC)
- SPI controller
- Timer controller (TIMER)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 893a7bff66..098d043375 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -477,6 +477,7 @@ config STM32L4X5_SOC
select STM32L4X5_SYSCFG
select STM32L4X5_RCC
select STM32L4X5_GPIO
+select STM32L4X5_USART
config XLNX_ZYNQMP_ARM
bool
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 40e294f838..ae0868dcab 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -28,6 +28,7 @@
#include "sysemu/sysemu.h"
#include "hw/or-irq.h"
#include "hw/arm/stm32l4x5_soc.h"
+#include "hw/char/stm32l4x5_usart.h"
#include "hw/gpio/stm32l4x5_gpio.h"
#include "hw/qdev-clock.h"
#include "hw/misc/unimp.h"
@@ -116,6 +117,22 @@ static const struct {
{ 0x48001C00, 0x000F, 0x, 0x },
};
+static const hwaddr usart_addr[] = {
+0x40013800, /* "USART1", 0x400 */
+0x40004400, /* "USART2", 0x400 */
+0x40004800, /* "USART3", 0x400 */
+};
+static const hwaddr uart_addr[] = {
+0x40004C00, /* "UART4" , 0x400 */
+0x40005000 /* "UART5" , 0x400 */
+};
+
+#define LPUART_BASE_ADDRESS 0x40008000
+
+static const int usart_irq[] = { 37, 38, 39 };
+static const int uart_irq[] = { 52, 53 };
+#define LPUART_IRQ 70
+
static void stm32l4x5_soc_initfn(Object *obj)
{
Stm32l4x5SocState *s = STM32L4X5_SOC(obj);
@@ -132,6 +149,18 @@ static void stm32l4x5_soc_initfn(Object *obj)
g_autofree char *name = g_strdup_printf("gpio%c", 'a' + i);
object_initialize_child(obj, name, >gpio[i], TYPE_STM32L4X5_GPIO);
}
+
+for (int i = 0; i < STM_NUM_USARTS; i++) {
+object_initialize_child(obj, "usart[*]", >usart[i],
+TYPE_STM32L4X5_USART);
+}
+
+for (int i = 0; i < STM_NUM_UARTS; i++) {
+object_initialize_child(obj, "uart[*]", >uart[i],
+TYPE_STM32L4X5_UART);
+}
+object_initialize_child(obj, "lpuart1", >lpuart,
+TYPE_STM32L4X5_LPUART);
}
static void stm32l4x5_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -279,6 +308,53 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
Error **errp)
sysbus_mmio_map(busdev, 0, RCC_BASE_ADDRESS);
sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, RCC_IRQ));
+/* USART devices */
+for (int i = 0; i < STM_NUM_USARTS; i++) {
+dev = DEVICE(&(s->usart[i]));
+qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+g_autofree char *name = g_strdup_printf("usart%d-out", i + 1);
+qdev_connect_clock_in(dev, "clk",
+qdev_get_clock_out(DEVICE(&(s->rcc)), name));
+busdev = SYS_BUS_DEVICE(dev);
+if (!sysbus_realize(busdev, errp)) {
+return;
+}
+sysbus_mmio_map(busdev, 0, usart_addr[i]);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, usart_irq[i]));
+}
+
+/*
+ * TODO: Connect the USARTs, UARTs and LPUART to the EXTI once the EXTI
+ * can handle other gpio-in than the gpios. (e.g. Direct Lines for the
usarts)
+ */
+
+/* UART devices */
+for (int i = 0; i < STM_NUM_UARTS; i++) {
+dev = DEVICE(&(s->uart[i]));
+qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS + i));
+g_autofree char *name = g_strdup_printf("uart%d-out", STM_NUM_USARTS +
i + 1);
+qdev_connect_clock_in(dev, "clk",
+qdev_get_clock_out(DEVICE(&(s->rcc)), name));
+busdev = SYS_BUS_DEVICE(dev);
+if (!sysbus_realize(busdev, errp)) {
+return;
+}
+sysbus_mmio_map(busdev, 0, uart_addr[i]);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(armv7m, uart_irq[i]));
+}
+
+/* LPUART device*/
+dev = DEVICE(&(s->lpuart));
+qdev_prop_set_chr(dev, "chardev", serial_hd(STM_NUM_USARTS +
STM_NUM_UARTS));
+qdev_connect_clock_in(dev, "clk",
+
[PATCH v2 3/6] hw/char/stm32l4x5_usart: Enable serial read and write
Implement the ability to read and write characters to the
usart using the serial port.
The character transmission is based on the
cmsdk-apb-uart implementation.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
hw/char/stm32l4x5_usart.c | 139 ++
hw/char/trace-events | 7 ++
include/hw/char/stm32l4x5_usart.h | 1 +
3 files changed, 147 insertions(+)
diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
index 46e69bb096..ec8c2f6e63 100644
--- a/hw/char/stm32l4x5_usart.c
+++ b/hw/char/stm32l4x5_usart.c
@@ -154,6 +154,119 @@ REG32(RDR, 0x24)
REG32(TDR, 0x28)
FIELD(TDR, TDR, 0, 9)
+static void stm32l4x5_update_irq(Stm32l4x5UsartBaseState *s)
+{
+if (((s->isr & R_ISR_WUF_MASK) && (s->cr3 & R_CR3_WUFIE_MASK))||
+((s->isr & R_ISR_CMF_MASK) && (s->cr1 & R_CR1_CMIE_MASK)) ||
+((s->isr & R_ISR_ABRF_MASK) && (s->cr1 & R_CR1_RXNEIE_MASK)) ||
+((s->isr & R_ISR_EOBF_MASK) && (s->cr1 & R_CR1_EOBIE_MASK)) ||
+((s->isr & R_ISR_RTOF_MASK) && (s->cr1 & R_CR1_RTOIE_MASK)) ||
+((s->isr & R_ISR_CTSIF_MASK) && (s->cr3 & R_CR3_CTSIE_MASK)) ||
+((s->isr & R_ISR_LBDF_MASK) && (s->cr2 & R_CR2_LBDIE_MASK)) ||
+((s->isr & R_ISR_TXE_MASK) && (s->cr1 & R_CR1_TXEIE_MASK))||
+((s->isr & R_ISR_TC_MASK) && (s->cr1 & R_CR1_TCIE_MASK)) ||
+((s->isr & R_ISR_RXNE_MASK) && (s->cr1 & R_CR1_RXNEIE_MASK)) ||
+((s->isr & R_ISR_IDLE_MASK) && (s->cr1 & R_CR1_IDLEIE_MASK)) ||
+((s->isr & R_ISR_ORE_MASK) &&
+((s->cr1 & R_CR1_RXNEIE_MASK) || (s->cr3 & R_CR3_EIE_MASK))) ||
+/* TODO: Handle NF ? */
+((s->isr & R_ISR_FE_MASK) && (s->cr3 & R_CR3_EIE_MASK)) ||
+((s->isr & R_ISR_PE_MASK) && (s->cr1 & R_CR1_PEIE_MASK))) {
+qemu_irq_raise(s->irq);
+trace_stm32l4x5_usart_irq_raised(s->isr);
+} else {
+qemu_irq_lower(s->irq);
+trace_stm32l4x5_usart_irq_lowered();
+}
+}
+
+static int stm32l4x5_usart_base_can_receive(void *opaque)
+{
+Stm32l4x5UsartBaseState *s = opaque;
+
+if (!(s->isr & R_ISR_RXNE_MASK)) {
+return 1;
+}
+
+return 0;
+}
+
+static void stm32l4x5_usart_base_receive(void *opaque, const uint8_t *buf, int
size)
+{
+Stm32l4x5UsartBaseState *s = opaque;
+
+if (!((s->cr1 & R_CR1_UE_MASK) && (s->cr1 & R_CR1_RE_MASK))) {
+trace_stm32l4x5_usart_receiver_not_enabled(
+FIELD_EX32(s->cr1, CR1, UE), FIELD_EX32(s->cr1, CR1, RE));
+return;
+}
+
+/* Check if overrun detection is enabled and if there is an overrun */
+if (!(s->cr3 & R_CR3_OVRDIS_MASK) && (s->isr & R_ISR_RXNE_MASK)) {
+/*
+ * A character has been received while
+ * the previous has not been read = Overrun.
+ */
+s->isr |= R_ISR_ORE_MASK;
+trace_stm32l4x5_usart_overrun_detected(s->rdr, *buf);
+} else {
+/* No overrun */
+s->rdr = *buf;
+s->isr |= R_ISR_RXNE_MASK;
+trace_stm32l4x5_usart_rx(s->rdr);
+}
+
+stm32l4x5_update_irq(s);
+}
+
+/* Try to send tx data, and arrange to be called back later if
+ * we can't (ie the char backend is busy/blocking).
+ */
+static gboolean usart_transmit(void *do_not_use, GIOCondition cond, void
*opaque)
+{
+Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(opaque);
+int ret;
+/* TODO: Handle 9 bits transmission */
+uint8_t ch = s->tdr;
+
+s->watch_tag = 0;
+
+if (!(s->cr1 & R_CR1_TE_MASK) || (s->isr & R_ISR_TXE_MASK)) {
+return G_SOURCE_REMOVE;
+}
+
+ret = qemu_chr_fe_write(>chr, , 1);
+if (ret <= 0) {
+s->watch_tag = qemu_chr_fe_add_watch(>chr, G_IO_OUT | G_IO_HUP,
+ usart_transmit, s);
+if (!s->watch_tag) {
+/* Most common reason to be here is "no chardev backend":
+ * just insta-drain the buffer, so the serial output
+ * goes into a void, rather than blocking the guest.
+ */
+goto buffer_drained;
+}
+/* Transmit pending */
+trace_stm32l4x5_usart_tx_pending();
+return G_SOURCE_REMOVE;
+}
+
+buffer_drained:
+/* Character successfully sent */
+trace_stm32l4x5_usart_tx(ch);
+s->isr |= R_ISR_TC_MASK | R_ISR_TXE_MASK;
+stm32l4x5_update_irq(s);
+return G_SOURCE_REMOVE;
+}
+
+static void usart_cancel_transmit(Stm32l4x5UsartBaseState *s)
+{
+if (s->watch_tag) {
+g_source_remove(s->watch_tag);
+s->watch_tag = 0;
+}
+}
+
static void stm32l4x5_usart_base_reset_hold(Object *obj)
{
Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
@@ -167,6 +280,22 @@ static void stm32l4x5_usart_base_reset_hold(Object *obj)
s->isr = 0x02C0;
s->rdr = 0x;
s->tdr = 0x;
+
+usart_cancel_transmit(s);
+
[PATCH v2 1/6] hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock
The "clock_set_mul_div" function doesn't propagate the clock period to
the children if it is changed (e.g. by enabling/disabling
a clock multiplexer).
This was overlooked during the implementation due to late changes.
This commit propagates the change if the multiplier or divider changes.
The usart tests will ensure that this behavior will not regress.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
hw/misc/stm32l4x5_rcc.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/misc/stm32l4x5_rcc.c b/hw/misc/stm32l4x5_rcc.c
index bc2d63528b..4725ba4f1c 100644
--- a/hw/misc/stm32l4x5_rcc.c
+++ b/hw/misc/stm32l4x5_rcc.c
@@ -59,7 +59,12 @@ static void clock_mux_update(RccClockMuxState *mux, bool
bypass_source)
freq_multiplier = mux->divider;
}
-clock_set_mul_div(mux->out, freq_multiplier, mux->multiplier);
+if ((mux->out->multiplier != freq_multiplier) ||
+mux->out->divider != mux->multiplier) {
+clock_set_mul_div(mux->out, freq_multiplier, mux->multiplier);
+clock_propagate(mux->out);
+}
+
clock_update(mux->out, clock_get(current_source));
src_freq = clock_get_hz(current_source);
--
2.34.1
[PATCH v2 2/6] hw/char: Implement STM32L4x5 USART skeleton
Add the basic infrastructure (register read/write, type...)
to implement the STM32L4x5 USART.
Also create different types for the USART, UART and LPUART
of the STM32L4x5 to deduplicate code and enable the
implementation of different behaviors depending on the type.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
---
MAINTAINERS | 1 +
hw/char/Kconfig | 3 +
hw/char/meson.build | 1 +
hw/char/stm32l4x5_usart.c | 395 ++
hw/char/trace-events | 4 +
include/hw/char/stm32l4x5_usart.h | 66 +
6 files changed, 470 insertions(+)
create mode 100644 hw/char/stm32l4x5_usart.c
create mode 100644 include/hw/char/stm32l4x5_usart.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 409d7db4d4..deba4a54ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1128,6 +1128,7 @@ M: Inès Varhol
L: qemu-...@nongnu.org
S: Maintained
F: hw/arm/stm32l4x5_soc.c
+F: hw/char/stm32l4x5_usart.c
F: hw/misc/stm32l4x5_exti.c
F: hw/misc/stm32l4x5_syscfg.c
F: hw/misc/stm32l4x5_rcc.c
diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 6b6cf2fc1d..4fd74ea878 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -41,6 +41,9 @@ config VIRTIO_SERIAL
config STM32F2XX_USART
bool
+config STM32L4X5_USART
+bool
+
config CMSDK_APB_UART
bool
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 006d20f1e2..e5b13b6958 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -31,6 +31,7 @@ system_ss.add(when: 'CONFIG_RENESAS_SCI', if_true:
files('renesas_sci.c'))
system_ss.add(when: 'CONFIG_SIFIVE_UART', if_true: files('sifive_uart.c'))
system_ss.add(when: 'CONFIG_SH_SCI', if_true: files('sh_serial.c'))
system_ss.add(when: 'CONFIG_STM32F2XX_USART', if_true:
files('stm32f2xx_usart.c'))
+system_ss.add(when: 'CONFIG_STM32L4X5_USART', if_true:
files('stm32l4x5_usart.c'))
system_ss.add(when: 'CONFIG_MCHP_PFSOC_MMUART', if_true:
files('mchp_pfsoc_mmuart.c'))
system_ss.add(when: 'CONFIG_HTIF', if_true: files('riscv_htif.c'))
system_ss.add(when: 'CONFIG_GOLDFISH_TTY', if_true: files('goldfish_tty.c'))
diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
new file mode 100644
index 00..46e69bb096
--- /dev/null
+++ b/hw/char/stm32l4x5_usart.c
@@ -0,0 +1,395 @@
+/*
+ * STM32L4X5 USART (Universal Synchronous Asynchronous Receiver Transmitter)
+ *
+ * Copyright (c) 2023 Arnaud Minier
+ * Copyright (c) 2023 Inès Varhol
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * The STM32L4X5 USART is heavily inspired by the stm32f2xx_usart
+ * by Alistair Francis.
+ * The reference used is the STMicroElectronics RM0351 Reference manual
+ * for STM32L4x5 and STM32L4x6 advanced Arm ® -based 32-bit MCUs.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "chardev/char-fe.h"
+#include "chardev/char-serial.h"
+#include "migration/vmstate.h"
+#include "hw/char/stm32l4x5_usart.h"
+#include "hw/clock.h"
+#include "hw/irq.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "hw/registerfields.h"
+#include "trace.h"
+
+
+REG32(CR1, 0x00)
+FIELD(CR1, M1, 28, 1)/* Word length (part 2, see M0)*/
+FIELD(CR1, EOBIE, 27, 1) /* End of Block interrupt enable */
+FIELD(CR1, RTOIE, 26, 1) /* Receiver timeout interrupt enable */
+FIELD(CR1, DEAT, 21, 5) /* Driver Enable assertion time */
+FIELD(CR1, DEDT, 16, 5) /* Driver Enable de-assertion time */
+FIELD(CR1, OVER8, 15, 1) /* Oversampling mode */
+FIELD(CR1, CMIE, 14, 1) /* Character match interrupt enable */
+FIELD(CR1, MME, 13, 1) /* Mute mode enable */
+FIELD(CR1, M0, 12, 1)/* Word length (part 1, see M1) */
+FIELD(CR1, WAKE, 11, 1) /* Receiver wakeup method */
+FIELD(CR1, PCE, 10, 1) /* Parity control enable */
+FIELD(CR1, PS, 9, 1) /* Parity selection */
+FIELD(CR1, PEIE, 8, 1) /* PE interrupt enable */
+FIELD(CR1, TXEIE, 7, 1) /* TXE interrupt enable */
+FIELD(CR1, TCIE, 6, 1) /* Transmission complete interrupt enable */
+FIELD(CR1, RXNEIE, 5, 1) /* RXNE interrupt enable */
+FIELD(CR1, IDLEIE, 4, 1) /* IDLE interrupt enable */
+FIELD(CR1, TE, 3, 1) /* Transmitter enable */
+FIELD(CR1, RE, 2, 1) /* Receiver enable */
+FIELD(CR1, UESM, 1, 1) /* USART enable in Stop mode */
+FIELD(CR1, UE, 0, 1) /* USART enable */
+REG32(CR2, 0x04)
+FIELD(CR2, ADD_1, 28, 4)/* ADD[7:4] */
+FIELD(CR2, ADD_0, 24, 1)/* ADD[3:0] */
+FIELD(CR2, RTOEN, 23, 1)/* Receiver timeout enable */
+FIELD(CR2, ABRMOD, 21, 2) /* Auto baud rate mode */
+FIELD(CR2, ABREN, 20, 1)/* Auto baud rate enable */
+FIELD(CR2, MSBFIRST, 19, 1) /* Most significant bit first
[PATCH v2 0/6] hw/char: Implement the STM32L4x5 USART, UART and LPUART
This patch adds the STM32L4x5 USART (Universal Synchronous/Asynchronous Receiver/Transmitter) device and is part of a series implementing the STM32L4x5 with a few peripherals. It implements the necessary functionalities to receive/send characters over the serial port, which are useful to communicate with the program currently running. Many thanks Peter for your review, I think I addressed almost everything. I'm just unsure about how to handle the waiting time in the tests. I understand your concerns about the unreliability of using the wallclock time but I don't understand how using clock_step() would make it more reliable. We will always be waiting on something that is out of our control (i.e. the OS). I increased the delay from 5s to 10min to match the microbit test and added a comment (I paraphrased your comment, is that okay ?). I also saw that Philippe Mathieu-Daudé have sent a patchset with the first commit of this patchset and another commit to make clock_set_mul_div() return a boolean. If this is merged before this patchset (which will probably be the case), I will remove the first commit. Changes from v1 to v2: - Use asynchronous transmission for serial communication (based on cmsdk-apb-uart implementation) - Use qemu_log_mask instead of error_report - Squash the commit that renamed the base struct - Use switch statements where appropriate - Fix RDR and TDR mask size - Increase time limit in tests - Remove the global qtest in the tests - Use assert when checking the interrupt number in the tests - Correct usage of g_autofree in the SoC Arnaud Minier (6): hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock hw/char: Implement STM32L4x5 USART skeleton hw/char/stm32l4x5_usart: Enable serial read and write hw/char/stm32l4x5_usart: Add options for serial parameters setting hw/arm: Add the USART to the stm32l4x5 SoC tests/qtest: Add tests for the STM32L4x5 USART MAINTAINERS| 1 + docs/system/arm/b-l475e-iot01a.rst | 2 +- hw/arm/Kconfig | 1 + hw/arm/stm32l4x5_soc.c | 82 +++- hw/char/Kconfig| 3 + hw/char/meson.build| 1 + hw/char/stm32l4x5_usart.c | 632 + hw/char/trace-events | 12 + hw/misc/stm32l4x5_rcc.c| 7 +- include/hw/arm/stm32l4x5_soc.h | 13 + include/hw/char/stm32l4x5_usart.h | 67 +++ tests/qtest/meson.build| 3 +- tests/qtest/stm32l4x5_usart-test.c | 326 +++ 13 files changed, 1141 insertions(+), 9 deletions(-) create mode 100644 hw/char/stm32l4x5_usart.c create mode 100644 include/hw/char/stm32l4x5_usart.h create mode 100644 tests/qtest/stm32l4x5_usart-test.c -- 2.34.1
Re: [PATCH 3/3] target/hppa: fix building gva for wide mode
On 3/24/24 09:09, Sven Schnelle wrote:
64 Bit hppa no longer has a fixed 32/32 bit split between space and
offset. Instead it uses 42 bits for the offset. The lower 10 bits of
the space are always zero, leaving 22 bits actually used. Simply or
the values together to build the gva.
Signed-off-by: Sven Schnelle
Reviewed-by: Helge Deller
Tested-by: Helge Deller
Helge
---
target/hppa/mem_helper.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 84785b5a5c..6f895fced7 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong
addr, target_ulong reg)
}
static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
- target_ulong r2, vaddr va_b)
+ target_ulong r2, uint64_t spc, uint64_t off)
{
HPPATLBEntry *ent;
-vaddr va_e;
+vaddr va_b, va_e;
uint64_t va_size;
int mask_shift;
+va_b = off & gva_offset_mask(env->psw);
+va_b |= spc << 32;
+
mask_shift = 2 * (r1 & 0xf);
va_size = (uint64_t)TARGET_PAGE_SIZE << mask_shift;
va_b &= -va_size;
@@ -569,14 +572,12 @@ static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
void HELPER(idtlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
{
-vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
-itlbt_pa20(env, r1, r2, va_b);
+itlbt_pa20(env, r1, r2, env->cr[CR_ISR], env->cr[CR_IOR]);
}
void HELPER(iitlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
{
-vaddr va_b = deposit64(env->cr[CR_IIAOQ], 32, 32, env->cr[CR_IIASQ]);
-itlbt_pa20(env, r1, r2, va_b);
+itlbt_pa20(env, r1, r2, env->cr[CR_IIASQ], env->cr[CR_IIAOQ]);
}
/* Purge (Insn/Data) TLB. */
Re: [PATCH 2/3] target/hppa: mask offset bits in gva
On 3/24/24 09:09, Sven Schnelle wrote:
The CPU seems to mask a few bits in the offset when running
under HP-UX. ISR/IOR register contents for an address in
the processor HPA (0xfffa) on my C8000 and J6750:
running on Linux: 3fff c000fffa0500
running on HP-UX: 301f c000fffa0500
I haven't found how this is switched (guess some diag in the
firmware), but linux + seabios seems to handle that as well,
so lets mask out the additional bits.
Signed-off-by: Sven Schnelle
I've seen the issue on HP-UX too, and can confirm the patch does
not break existing 32- and 64-bit Linux installations, so:
Tested-by: Helge Deller
Thanks!
Helge
---
target/hppa/cpu.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a072d0bb63..9bc4d208fa 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -283,12 +283,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
void hppa_translate_init(void);
+#define HPPA_GVA_OFFSET_MASK64 0x301f
#define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
static inline uint64_t gva_offset_mask(target_ulong psw)
{
return (psw & PSW_W
-? MAKE_64BIT_MASK(0, 62)
+? HPPA_GVA_OFFSET_MASK64
: MAKE_64BIT_MASK(0, 32));
}
[PATCH] docs/system/ppc/amigang.rst: Fix formatting
Add missing space to fix character formatting where it was missed in two places. Fixes: 623d9065b6 (docs/system/ppc: Document running Linux on AmigaNG machines) Signed-off-by: BALATON Zoltan --- docs/system/ppc/amigang.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/system/ppc/amigang.rst b/docs/system/ppc/amigang.rst index ba1a3d80b9..e2c9cb74b7 100644 --- a/docs/system/ppc/amigang.rst +++ b/docs/system/ppc/amigang.rst @@ -16,7 +16,7 @@ firmware to support AmigaOS 4. Emulated devices - * PowerPC 7457 CPU (can also use``-cpu g3, 750cxe, 750fx`` or ``750gx``) + * PowerPC 7457 CPU (can also use ``-cpu g3, 750cxe, 750fx`` or ``750gx``) * Articia S north bridge * VIA VT82C686B south bridge * PCI VGA compatible card (guests may need other card instead) @@ -73,7 +73,7 @@ https://www.powerdeveloper.org/platforms/pegasos/schematics. Emulated devices - * PowerPC 7457 CPU (can also use``-cpu g3`` or ``750cxe``) + * PowerPC 7457 CPU (can also use ``-cpu g3`` or ``750cxe``) * Marvell MV64361 Discovery II north bridge * VIA VT8231 south bridge * PCI VGA compatible card (guests may need other card instead) -- 2.30.9
Re: [PATCH 1/3] target/hppa: use gva_offset_mask() everywhere
On 3/24/24 09:09, Sven Schnelle wrote:
move it to cpu.h, so it can also be used in hppa_form_gva_psw()
Signed-off-by: Sven Schnelle
Reviewed-by: Helge Deller
Helge
---
target/hppa/cpu.h | 10 --
target/hppa/translate.c | 12 +++-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a92dc352cb..a072d0bb63 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -285,14 +285,20 @@ void hppa_translate_init(void);
#define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
+static inline uint64_t gva_offset_mask(target_ulong psw)
+{
+return (psw & PSW_W
+? MAKE_64BIT_MASK(0, 62)
+: MAKE_64BIT_MASK(0, 32));
+}
+
static inline target_ulong hppa_form_gva_psw(target_ulong psw, uint64_t spc,
target_ulong off)
{
#ifdef CONFIG_USER_ONLY
return off;
#else
-off &= psw & PSW_W ? MAKE_64BIT_MASK(0, 62) : MAKE_64BIT_MASK(0, 32);
-return spc | off;
+return spc | (off & gva_offset_mask(psw));
#endif
}
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 19594f917e..0af125ed74 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -586,17 +586,10 @@ static bool nullify_end(DisasContext *ctx)
return true;
}
-static uint64_t gva_offset_mask(DisasContext *ctx)
-{
-return (ctx->tb_flags & PSW_W
-? MAKE_64BIT_MASK(0, 62)
-: MAKE_64BIT_MASK(0, 32));
-}
-
static void copy_iaoq_entry(DisasContext *ctx, TCGv_i64 dest,
uint64_t ival, TCGv_i64 vval)
{
-uint64_t mask = gva_offset_mask(ctx);
+uint64_t mask = gva_offset_mask(ctx->tb_flags);
if (ival != -1) {
tcg_gen_movi_i64(dest, ival & mask);
@@ -1403,7 +1396,8 @@ static void form_gva(DisasContext *ctx, TCGv_i64 *pgva,
TCGv_i64 *pofs,
*pofs = ofs;
*pgva = addr = tcg_temp_new_i64();
-tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base, gva_offset_mask(ctx));
+tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base,
+ gva_offset_mask(ctx->tb_flags));
#ifndef CONFIG_USER_ONLY
if (!is_phys) {
tcg_gen_or_i64(addr, addr, space_select(ctx, sp, base));
[PATCH] chardev/char-win-stdio: Fix keyboard input after exit Qemu on
After exit Qemu need to return the terminal to the default state.
Signed-off-by: Irina Ryapolova
---
chardev/char-win-stdio.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c
index 1a18999e78..4fa2c3de8b 100644
--- a/chardev/char-win-stdio.c
+++ b/chardev/char-win-stdio.c
@@ -220,6 +220,7 @@ err1:
static void char_win_stdio_finalize(Object *obj)
{
WinStdioChardev *stdio = WIN_STDIO_CHARDEV(obj);
+DWORD dwMode;
if (stdio->hInputReadyEvent != INVALID_HANDLE_VALUE) {
CloseHandle(stdio->hInputReadyEvent);
@@ -230,6 +231,10 @@ static void char_win_stdio_finalize(Object *obj)
if (stdio->hInputThread != INVALID_HANDLE_VALUE) {
TerminateThread(stdio->hInputThread, 0);
}
+
+GetConsoleMode(stdio->hStdIn, );
+dwMode &= ~ENABLE_VIRTUAL_TERMINAL_INPUT;
+SetConsoleMode(stdio->hStdIn, dwMode);
}
static int win_stdio_write(Chardev *chr, const uint8_t *buf, int len)
--
2.25.1
[PATCH v3 1/2] target/riscv/csr.c: Add functional of hvictl CSR
CSR hvictl (Hypervisor Virtual Interrupt Control) provides further flexibility
for injecting interrupts into VS level in situations not fully supported by the
facilities described thus far, but only with more active involvement of the
hypervisor.
A hypervisor must use hvictl for any of the following:
• asserting for VS level a major interrupt not supported by hvien and hvip;
• implementing configurability of priorities at VS level for major interrupts
beyond those sup-
ported by hviprio1 and hviprio2; or
• emulating an external interrupt controller for a virtual hart without the use
of an IMSIC’s
guest interrupt file, while also supporting configurable priorities both for
external interrupts
and for major interrupts to the virtual hart.
All hvictl fields together can affect the value of CSR vstopi (Virtual
Supervisor Top Interrupt)
and therefore the interrupt identity reported in vscause when an interrupt
traps to VS-mode.
When hvictl.VTI = 1, the absence of an interrupt for VS level can be indicated
only by setting
hvictl.IID = 9. Software might want to use the pair IID = 9, IPRIO = 0
generally to represent
no interrupt in hvictl.
(See riscv-interrupts-1.0: Interrupts at VS level)
Signed-off-by: Irina Ryapolova
---
Changes for v2:
-added more information in commit message
Changes for v3:
-applied patch in master
---
target/riscv/csr.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 726096444f..4c2cbcd59f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3613,6 +3613,21 @@ static RISCVException write_hvictl(CPURISCVState *env,
int csrno,
target_ulong val)
{
env->hvictl = val & HVICTL_VALID_MASK;
+if (env->hvictl & HVICTL_VTI)
+{
+uint32_t hviid = get_field(env->hvictl, HVICTL_IID);
+uint32_t hviprio = get_field(env->hvictl, HVICTL_IPRIO);
+/* the pair IID = 9, IPRIO = 0 generally to represent no interrupt in
hvictl. */
+if (!(hviid == IRQ_S_EXT && hviprio == 0)) {
+uint64_t new_val = BIT(hviid) ;
+ if (new_val & S_MODE_INTERRUPTS) {
+rmw_hvip64(env, csrno, NULL, new_val << 1, new_val << 1);
+} else if (new_val & LOCAL_INTERRUPTS) {
+rmw_hvip64(env, csrno, NULL, new_val, new_val);
+}
+}
+}
+
return RISCV_EXCP_NONE;
}
--
2.25.1
[PATCH v3 2/2] target/riscv/csr: Added the ability to delegate LCOFI to VS
From: Vadim Shakirov In the AIA specification in the paragraph "Virtual interrupts for VS level" it is indicated for interrupts 13-63: if the bit in hideleg is enabled, then the corresponding vsip and vsie bits are aliases to sip and sie Signed-off-by: Vadim Shakirov Reviewed-by: Alistair Francis --- target/riscv/csr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 4c2cbcd59f..38548a01d9 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -1150,7 +1150,7 @@ static RISCVException write_stimecmph(CPURISCVState *env, int csrno, static const uint64_t delegable_ints = S_MODE_INTERRUPTS | VS_MODE_INTERRUPTS | MIP_LCOFIP; static const uint64_t vs_delegable_ints = -(VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & ~MIP_LCOFIP; +VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS; static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS | HS_MODE_INTERRUPTS | LOCAL_INTERRUPTS; #define DELEGABLE_EXCPS ((1ULL << (RISCV_EXCP_INST_ADDR_MIS)) | \ -- 2.25.1
[PATCH] target/riscv: Fix mode in riscv_tlb_fill
Need to convert mmu_idx to privilege mode for PMP function. Signed-off-by: Irina Ryapolova --- target/riscv/cpu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index ce7322011d..fc090d729a 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -1315,7 +1315,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, bool two_stage_lookup = mmuidx_2stage(mmu_idx); bool two_stage_indirect_error = false; int ret = TRANSLATE_FAIL; -int mode = mmu_idx; +int mode = mmuidx_priv(mmu_idx); /* default TLB page size */ target_ulong tlb_size = TARGET_PAGE_SIZE; -- 2.25.1
Re: [PULL 00/15] riscv-to-apply queue
22.03.2024 22:46, Daniel Henrique Barboza : On 3/22/24 14:16, Michael Tokarev wrote: 22.03.2024 11:53, Alistair Francis : RISC-V PR for 9.0 * Do not enable all named features by default * A range of Vector fixes * Update APLIC IDC after claiming iforce register * Remove the dependency of Zvfbfmin to Zfbfmin * Fix mode in riscv_tlb_fill * Fix timebase-frequency when using KVM acceleration Should something from there be picked up for stable (8.2 and probably 7.2)? Ignore the "Do not enable all named features by default" since it's fixing something that were added in 9.0. The rest you can pick it up to 8.2 at least. Thanks, Unfortunately this doesn't quite work, the following changes fail to apply to 8.2: 929e521a47 target/riscv: always clear vstart for ldst_whole insns b46631f122 target/riscv: remove 'over' brconds from vector trans d57dfe4b37 trans_rvv.c.inc: remove redundant mark_vs_dirty() calls bac802ada8 target/riscv: enable 'vstart_eq_zero' in the end of insns 385e575cd5 target/riscv/kvm: fix timebase-frequency when using KVM acceleration I tried to back-port at least the first one but it turned out to be another failure. Didn't try looking at the rest. If these really should be in 8.2 (it's your guys to decide, not me), I need help with back-porting these to 8.2 (and/or cherry-picking additional patches from master). Thanks, /mjt Daniel Henrique Barboza (10): target/riscv: do not enable all named features by default target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX() trans_rvv.c.inc: set vstart = 0 in int scalar move insns target/riscv/vector_helper.c: fix 'vmvr_v' memcpy endianess target/riscv: always clear vstart in whole vec move insns target/riscv: always clear vstart for ldst_whole insns target/riscv/vector_helpers: do early exit when vstart >= vl target/riscv: remove 'over' brconds from vector trans trans_rvv.c.inc: remove redundant mark_vs_dirty() calls target/riscv/vector_helper.c: optimize loops in ldst helpers Frank Chang (1): hw/intc: Update APLIC IDC after claiming iforce register Irina Ryapolova (1): target/riscv: Fix mode in riscv_tlb_fill Ivan Klokov (1): target/riscv: enable 'vstart_eq_zero' in the end of insns Max Chou (1): target/riscv: rvv: Remove the dependency of Zvfbfmin to Zfbfmin Yong-Xuan Wang (1): target/riscv/kvm: fix timebase-frequency when using KVM acceleration
Re: [PATCH v2 1/3] ui/cocoa: Fix aspect ratio
On Sat, 23 Mar 2024 at 06:20, Akihiko Odaki wrote:
>
> [NSWindow setContentAspectRatio:] does not trigger window resize itself,
> so the wrong aspect ratio will persist if nothing resizes the window.
> Call [NSWindow setContentSize:] in such a case.
>
> Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen")
> Signed-off-by: Akihiko Odaki
> ---
Reviewed-by: Peter Maydell
thanks
-- PMM
[PATCH v1 1/1] virtio-snd: rewrite invalid tx/rx message handling
The current handling of invalid virtqueue elements inside the TX/RX virt
queue handlers is wrong.
They are added in a per-stream invalid queue to be processed after the
handler is done examining each message, but the invalid message might
not be specifying any stream_id; which means it's invalid to add it to
any stream->invalid queue since stream could be NULL at this point.
This commit moves the invalid queue to the VirtIOSound struct which
guarantees there will always be a valid temporary place to store them
inside the tx/rx handlers. The queue will be emptied before the handler
returns, so the queue must be empty at any other point of the device's
lifetime.
Signed-off-by: Manos Pitsidianakis
---
include/hw/audio/virtio-snd.h | 16 +++-
hw/audio/virtio-snd.c | 137 +++---
2 files changed, 77 insertions(+), 76 deletions(-)
diff --git a/include/hw/audio/virtio-snd.h b/include/hw/audio/virtio-snd.h
index 3d79181364..8dafedb276 100644
--- a/include/hw/audio/virtio-snd.h
+++ b/include/hw/audio/virtio-snd.h
@@ -151,7 +151,6 @@ struct VirtIOSoundPCMStream {
QemuMutex queue_mutex;
bool active;
QSIMPLEQ_HEAD(, VirtIOSoundPCMBuffer) queue;
-QSIMPLEQ_HEAD(, VirtIOSoundPCMBuffer) invalid;
};
/*
@@ -223,6 +222,21 @@ struct VirtIOSound {
QemuMutex cmdq_mutex;
QTAILQ_HEAD(, virtio_snd_ctrl_command) cmdq;
bool processing_cmdq;
+/*
+ * Convenience queue to keep track of invalid tx/rx queue messages inside
+ * the tx/rx callbacks.
+ *
+ * In the callbacks as a first step we are emptying the virtqueue to handle
+ * each message and we cannot add an invalid message back to the queue: we
+ * would re-process it in subsequent loop iterations.
+ *
+ * Instead, we add them to this queue and after finishing examining every
+ * virtqueue element, we inform the guest for each invalid message.
+ *
+ * This queue must be empty at all times except for inside the tx/rx
+ * callbacks.
+ */
+QSIMPLEQ_HEAD(, VirtIOSoundPCMBuffer) invalid;
};
struct virtio_snd_ctrl_command {
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 30493f06a8..90d9a2796e 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -456,7 +456,6 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s,
uint32_t stream_id)
stream->s = s;
qemu_mutex_init(>queue_mutex);
QSIMPLEQ_INIT(>queue);
-QSIMPLEQ_INIT(>invalid);
/*
* stream_id >= s->snd_conf.streams was checked before so this is
@@ -611,9 +610,6 @@ static size_t
virtio_snd_pcm_get_io_msgs_count(VirtIOSoundPCMStream *stream)
QSIMPLEQ_FOREACH_SAFE(buffer, >queue, entry, next) {
count += 1;
}
-QSIMPLEQ_FOREACH_SAFE(buffer, >invalid, entry, next) {
-count += 1;
-}
}
return count;
}
@@ -831,47 +827,36 @@ static void virtio_snd_handle_event(VirtIODevice *vdev,
VirtQueue *vq)
trace_virtio_snd_handle_event();
}
+/*
+ * Must only be called if vsnd->invalid is not empty.
+ */
static inline void empty_invalid_queue(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOSoundPCMBuffer *buffer = NULL;
-VirtIOSoundPCMStream *stream = NULL;
virtio_snd_pcm_status resp = { 0 };
VirtIOSound *vsnd = VIRTIO_SND(vdev);
-bool any = false;
-for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
-stream = vsnd->pcm->streams[i];
-if (stream) {
-any = false;
-WITH_QEMU_LOCK_GUARD(>queue_mutex) {
-while (!QSIMPLEQ_EMPTY(>invalid)) {
-buffer = QSIMPLEQ_FIRST(>invalid);
-if (buffer->vq != vq) {
-break;
-}
-any = true;
-resp.status = cpu_to_le32(VIRTIO_SND_S_BAD_MSG);
-iov_from_buf(buffer->elem->in_sg,
- buffer->elem->in_num,
- 0,
- ,
- sizeof(virtio_snd_pcm_status));
-virtqueue_push(vq,
- buffer->elem,
- sizeof(virtio_snd_pcm_status));
-QSIMPLEQ_REMOVE_HEAD(>invalid, entry);
-virtio_snd_pcm_buffer_free(buffer);
-}
-if (any) {
-/*
- * Notify vq about virtio_snd_pcm_status responses.
- * Buffer responses must be notified separately later.
- */
-virtio_notify(vdev, vq);
-}
-}
-}
+g_assert(!QSIMPLEQ_EMPTY(>invalid));
+
+while (!QSIMPLEQ_EMPTY(>invalid)) {
+buffer = QSIMPLEQ_FIRST(>invalid);
+/* If buffer->vq != vq, our logic is fundamentally wrong, so bail out
*/
+
[PATCH v1 0/1] virtio-snd: fix invalid tx/rx message handling logic
This is a logic fix for the error handling in the TX/RX virt queue handlers. A potential invalid address dereference was reported and fixed by Zheyu Ma in <20240322110827.568412-1-zheyum...@gmail.com>. This patch moves the invalid message storage from the stream structs to the virtio device struct to 1. make such bug impossible 2. reply to invalid messages again with VIRTIO_SND_S_BAD_MSG, which was not possible before. Patch based on master base-commit: 853546f8128476eefb701d4a55b2781bb3a46faa with the following patch applied: Subject: [PATCH v2] virtio-snd: Enhance error handling for invalid transfers From: Zheyu Ma Date: Fri, 22 Mar 2024 12:08:27 +0100 Message-Id: <20240322110827.568412-1-zheyum...@gmail.com> Manos Pitsidianakis (1): virtio-snd: rewrite invalid tx/rx message handling include/hw/audio/virtio-snd.h | 16 +++- hw/audio/virtio-snd.c | 137 +++--- 2 files changed, 77 insertions(+), 76 deletions(-) base-commit: 853546f8128476eefb701d4a55b2781bb3a46faa prerequisite-patch-id: 8209301569bd30ba806d06b3452a2f3156503a7a -- γαῖα πυρί μιχθήτω
Re: [PATCH-for-9.1 25/27] target/tricore: Convert to TCGCPUOps::get_cpu_state()
On Tue, Mar 19, 2024 at 04:42:54PM +0100, Philippe Mathieu-Daudé wrote: > Convert cpu_get_tb_cpu_state() to TCGCPUOps::get_cpu_state(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > target/tricore/cpu.h | 12 > target/tricore/cpu.c | 13 + > 2 files changed, 13 insertions(+), 12 deletions(-) Reviewed-by: Bastian Koppelmann Cheers, Bastian
Re: [PATCH] target/tricore/helper: Use correct string format in cpu_tlb_fill()
On Tue, Mar 19, 2024 at 06:14:13AM +0100, Philippe Mathieu-Daudé wrote:
> 'address' got converted from target_ulong to vaddr in commit
> 68d6eee73c ("target/tricore: Convert to CPUClass::tlb_fill").
> Use the corresponding format string to avoid casting.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> target/tricore/helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Bastian Koppelmann
Cheers,
Bastian
[PATCH] hw/net/net_tx_pkt: Fix virtio header without checksum offloading
It is incorrect to have the VIRTIO_NET_HDR_F_NEEDS_CSUM set when
checksum offloading is disabled so clear the bit. Set the
VIRTIO_NET_HDR_F_DATA_VALID bit instead to tell the checksum is valid.
TCP/UDP checksum is usually offloaded when the peer requires virtio
headers because they can instruct the peer to compute checksum. However,
igb disables TX checksum offloading when a VF is enabled whether the
peer requires virtio headers because a transmitted packet can be routed
to it and it expects the packet has a proper checksum. Therefore, it
is necessary to have a correct virtio header even when checksum
offloading is disabled.
A real TCP/UDP checksum will be computed and saved in the buffer when
checksum offloading is disabled. The virtio specification requires to
set the packet checksum stored in the buffer to the TCP/UDP pseudo
header when the VIRTIO_NET_HDR_F_NEEDS_CSUM bit is set so the bit must
be cleared in that case.
The VIRTIO_NET_HDR_F_NEEDS_CSUM bit also tells to skip checksum
validation. Even if checksum offloading is disabled, it is desirable to
skip checksum validation because the checksum is always correct. Use the
VIRTIO_NET_HDR_F_DATA_VALID bit to claim the validity of the checksum.
Fixes: ffbd2dbd8e64 ("e1000e: Perform software segmentation for loopback")
Buglink: https://issues.redhat.com/browse/RHEL-23067
Signed-off-by: Akihiko Odaki
---
hw/net/net_tx_pkt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index 2e5f58b3c9cc..c225cf706513 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -833,6 +833,9 @@ bool net_tx_pkt_send_custom(struct NetTxPkt *pkt, bool
offload,
if (offload || gso_type == VIRTIO_NET_HDR_GSO_NONE) {
if (!offload && pkt->virt_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+pkt->virt_hdr.flags =
+(pkt->virt_hdr.flags & ~VIRTIO_NET_HDR_F_NEEDS_CSUM) |
+VIRTIO_NET_HDR_F_DATA_VALID;
net_tx_pkt_do_sw_csum(pkt, >vec[NET_TX_PKT_L2HDR_FRAG],
pkt->payload_frags +
NET_TX_PKT_PL_START_FRAG - 1,
pkt->payload_len);
---
base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
change-id: 20240324-tx-c57d3c22ad73
Best regards,
--
Akihiko Odaki
[PATCH 3/3] target/hppa: fix building gva for wide mode
64 Bit hppa no longer has a fixed 32/32 bit split between space and
offset. Instead it uses 42 bits for the offset. The lower 10 bits of
the space are always zero, leaving 22 bits actually used. Simply or
the values together to build the gva.
Signed-off-by: Sven Schnelle
---
target/hppa/mem_helper.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 84785b5a5c..6f895fced7 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong
addr, target_ulong reg)
}
static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
- target_ulong r2, vaddr va_b)
+ target_ulong r2, uint64_t spc, uint64_t off)
{
HPPATLBEntry *ent;
-vaddr va_e;
+vaddr va_b, va_e;
uint64_t va_size;
int mask_shift;
+va_b = off & gva_offset_mask(env->psw);
+va_b |= spc << 32;
+
mask_shift = 2 * (r1 & 0xf);
va_size = (uint64_t)TARGET_PAGE_SIZE << mask_shift;
va_b &= -va_size;
@@ -569,14 +572,12 @@ static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
void HELPER(idtlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
{
-vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);
-itlbt_pa20(env, r1, r2, va_b);
+itlbt_pa20(env, r1, r2, env->cr[CR_ISR], env->cr[CR_IOR]);
}
void HELPER(iitlbt_pa20)(CPUHPPAState *env, target_ulong r1, target_ulong r2)
{
-vaddr va_b = deposit64(env->cr[CR_IIAOQ], 32, 32, env->cr[CR_IIASQ]);
-itlbt_pa20(env, r1, r2, va_b);
+itlbt_pa20(env, r1, r2, env->cr[CR_IIASQ], env->cr[CR_IIAOQ]);
}
/* Purge (Insn/Data) TLB. */
--
2.43.2
[PATCH 0/3] few hppa fixes for 64bit mode
Hi, in preparation of getting 64bit HP-UX running in qemu, here are a few fixes to make HP-UX progress a bit further. Sven Schnelle (3): target/hppa: use gva_offset_mask() everywhere target/hppa: mask offset bits in gva target/hppa: fix building gva for wide mode target/hppa/cpu.h| 11 +-- target/hppa/mem_helper.c | 13 +++-- target/hppa/translate.c | 12 +++- 3 files changed, 19 insertions(+), 17 deletions(-) -- 2.43.2
[PATCH 2/3] target/hppa: mask offset bits in gva
The CPU seems to mask a few bits in the offset when running
under HP-UX. ISR/IOR register contents for an address in
the processor HPA (0xfffa) on my C8000 and J6750:
running on Linux: 3fff c000fffa0500
running on HP-UX: 301f c000fffa0500
I haven't found how this is switched (guess some diag in the
firmware), but linux + seabios seems to handle that as well,
so lets mask out the additional bits.
Signed-off-by: Sven Schnelle
---
target/hppa/cpu.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a072d0bb63..9bc4d208fa 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -283,12 +283,13 @@ static inline int HPPA_BTLB_ENTRIES(CPUHPPAState *env)
void hppa_translate_init(void);
+#define HPPA_GVA_OFFSET_MASK64 0x301f
#define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
static inline uint64_t gva_offset_mask(target_ulong psw)
{
return (psw & PSW_W
-? MAKE_64BIT_MASK(0, 62)
+? HPPA_GVA_OFFSET_MASK64
: MAKE_64BIT_MASK(0, 32));
}
--
2.43.2
[PATCH 1/3] target/hppa: use gva_offset_mask() everywhere
move it to cpu.h, so it can also be used in hppa_form_gva_psw()
Signed-off-by: Sven Schnelle
---
target/hppa/cpu.h | 10 --
target/hppa/translate.c | 12 +++-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index a92dc352cb..a072d0bb63 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -285,14 +285,20 @@ void hppa_translate_init(void);
#define CPU_RESOLVING_TYPE TYPE_HPPA_CPU
+static inline uint64_t gva_offset_mask(target_ulong psw)
+{
+return (psw & PSW_W
+? MAKE_64BIT_MASK(0, 62)
+: MAKE_64BIT_MASK(0, 32));
+}
+
static inline target_ulong hppa_form_gva_psw(target_ulong psw, uint64_t spc,
target_ulong off)
{
#ifdef CONFIG_USER_ONLY
return off;
#else
-off &= psw & PSW_W ? MAKE_64BIT_MASK(0, 62) : MAKE_64BIT_MASK(0, 32);
-return spc | off;
+return spc | (off & gva_offset_mask(psw));
#endif
}
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 19594f917e..0af125ed74 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -586,17 +586,10 @@ static bool nullify_end(DisasContext *ctx)
return true;
}
-static uint64_t gva_offset_mask(DisasContext *ctx)
-{
-return (ctx->tb_flags & PSW_W
-? MAKE_64BIT_MASK(0, 62)
-: MAKE_64BIT_MASK(0, 32));
-}
-
static void copy_iaoq_entry(DisasContext *ctx, TCGv_i64 dest,
uint64_t ival, TCGv_i64 vval)
{
-uint64_t mask = gva_offset_mask(ctx);
+uint64_t mask = gva_offset_mask(ctx->tb_flags);
if (ival != -1) {
tcg_gen_movi_i64(dest, ival & mask);
@@ -1403,7 +1396,8 @@ static void form_gva(DisasContext *ctx, TCGv_i64 *pgva,
TCGv_i64 *pofs,
*pofs = ofs;
*pgva = addr = tcg_temp_new_i64();
-tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base, gva_offset_mask(ctx));
+tcg_gen_andi_i64(addr, modify <= 0 ? ofs : base,
+ gva_offset_mask(ctx->tb_flags));
#ifndef CONFIG_USER_ONLY
if (!is_phys) {
tcg_gen_or_i64(addr, addr, space_select(ctx, sp, base));
--
2.43.2
