[Qemu-devel] [PATCH for-4.1 2/2] qemu-tech: Fix dangling @menu entries

[Markus Armbruster]

Recent commit 2f2c4e4731 "Convert "translator internals" docs to RST,
move to devel manual" and commit 282d36b5e2 "qemu-tech.texi: Remove
"QEMU compared to other emulators" section" removed @node, but left
their @menu entries behind.  This broke building qemu-doc.info (but
not qemu-doc.{html,pdf,txt}; how odd).  Bury the dead @menu entries.

Reported-by: Philippe Mathieu-Daudé 
Fixes: 2f2c4e4731449449a2b1aafcd73e4f9ae107d78b
Fixes: 282d36b5e27ba86d42d0638430e439c2c257367b
Signed-off-by: Markus Armbruster 
---
 qemu-tech.texi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/qemu-tech.texi b/qemu-tech.texi
index 3451cfaa5b..0380de77b6 100644
--- a/qemu-tech.texi
+++ b/qemu-tech.texi
@@ -3,10 +3,7 @@
 
 @menu
 * CPU emulation::
-* Translator Internals::
-* QEMU compared to other emulators::
 * Managed start up options::
-* Bibliography::
 @end menu
 
 @node CPU emulation
-- 
2.21.0

[Qemu-devel] [PATCH for-4.1 1/2] Makefile: Fix missing dependency of on qemu-tech.texi

[Markus Armbruster]

The qemu-doc.{html,info,pdf,txt} depend on qemu-doc.texi and its
include files.  Except qemu-tech.texi is missing.  Has always been
missing as far as I can see.  Fix it.

Signed-off-by: Markus Armbruster 
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1fcbaed62c..8212a1446c 100644
--- a/Makefile
+++ b/Makefile
@@ -1020,7 +1020,8 @@ pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf 
docs/interop/qemu-ga-ref.pdf
 txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt
 
 qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
-   qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
+   qemu-img.texi qemu-nbd.texi qemu-options.texi \
+   qemu-tech.texi qemu-option-trace.texi \
qemu-deprecated.texi qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
qemu-monitor-info.texi docs/qemu-block-drivers.texi \
docs/qemu-cpu-models.texi docs/security.texi
-- 
2.21.0

[Qemu-devel] [PATCH for-4.1 0/2] qemu-doc build fixes

[Markus Armbruster]

*** BLURB HERE ***

Markus Armbruster (2):
  Makefile: Fix missing dependency of on qemu-tech.texi
  qemu-tech: Fix dangling @menu entries

 Makefile   | 3 ++-
 qemu-tech.texi | 3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)

-- 
2.21.0

Re: [Qemu-devel] Parallel make build fails on fast machine

[Markus Armbruster]

Mark Cave-Ayland  writes:

> On 11/07/2019 15:45, Markus Armbruster wrote:
>
>> Mark Cave-Ayland  writes:
>> 
>>> Something also looks a bit odd with distclean here on a fresh checkout:
>>>
>>> build@ezio:~/src/qemu/git/tmp/qemu$ make distclean
>>>   LD  recurse-clean.mo
>>> cc: fatal error: no input files
>>> compilation terminated.
>>> rules.mak:118: recipe for target 'recurse-clean.mo' failed
>>> make: *** [recurse-clean.mo] Error 1
>> 
>> This one should be fixed in master (commit 8d358a5ea08).  If it's still
>> broken for you, let me know.
>
> Yes, this is certainly looking much better! There still seems to be something 
> wrong
> with the tests/ subdirectory with your "make install" patch applied to git 
> master if
> I attempt a "make distclean" after a successful "make V=1 -j2 install" in an
> out-of-tree build:
>
> build@ezio:~/src/qemu/git/obj$ ../qemu/configure '--target-list=x86_64-softmmu
> sparc64-softmmu sparc-softmmu ppc-softmmu arm-softmmu'
> '--prefix=/home/build/rel-qemu-git' '--disable-pie' '--enable-debug'
> build@ezio:~/src/qemu/git/obj$ make V=1 -j2 install
>
> (lots of build output cut, but completes successfully)
>
> build@ezio:~/src/qemu/git/obj$ make distclean
> Makefile:85: rules.mak: No such file or directory
> Makefile:437: tests/Makefile.include: No such file or directory
> cat: VERSION: No such file or directory
> Makefile:1127: tests/docker/Makefile.include: No such file or directory
> Makefile:1128: tests/vm/Makefile.include: No such file or directory
> make: *** No rule to make target 'tests/vm/Makefile.include'.  Stop.

Double-checking: is this with my '[PATCH for-4.1] Makefile: Fix "make
install" when "make all" needs work' applied?

Re: [Qemu-devel] [BUG] 216 Alerts reported by LGTM for QEMU (some might be release critical)

[Markus Armbruster]

Stefan Weil  writes:

> Am 14.07.2019 um 19:30 schrieb Peter Maydell:
> [...]
>> "Analyzer thinks this multiply can overflow
>> but in fact it's not possible" is quite a common false
>> positive cause...
>
>
> The analysers don't complain because a multiply can overflow.
>
> They complain because the code indicates that a larger result is
> expected, for example uint64_t = uint32_t * uint32_t. They would not
> complain for the same multiplication if it were assigned to a uint32_t.

I agree this is an anti-pattern.

> So there is a simple solution to write the code in a way which avoids
> false positives...

You wrote elsewhere in this thread:

Either the assigned value should use the same data type as the
factors (possible when there is never an overflow, avoids a size
extension), or the multiplication could use the larger data type by
adding a type cast to one of the factors (then an overflow cannot
happen, static code analysers and human reviewers have an easier
job, but the multiplication costs more time).

Makes sense to me.

Re: [Qemu-devel] [PATCH 1/2] hw/gpio: Add basic Aspeed GPIO model

[Andrew Jeffery]

On Mon, 15 Jul 2019, at 12:06, Rashmica Gupta wrote:
> Sorry for the late reply! I agree with most of your feedback and will
> send out
> a v2 shortly with those changes. I have a few replies below
> 
> [snip]
> 
> > > +static const struct AspeedGPIO gpios[0x1f0] = {
> > > +/* Set ABCD */
> > > +[GPIO_ABCD_DATA_VALUE] = {0, read_data_value,
> > > _write_data_value},
> > 
> > Maybe there would be less tedium (and risk of minor bugs) here if we
> > add
> > a table for looking up the read/write callbacks from a type, and just
> > stash
> > the type in struct AspeedGPIO (or a pointer to the corresponding type
> > ops),
> > e.g:
> > 
> > enum gpio_property { gpio_direction = 0, gpio_int_enable, ... };
> > 
> > struct aspeed_gpio_reg_ops {
> > int (*read)(...);
> > int (*write)(...);
> > };
> > 
> > struct aspeed_gpio_reg_ops gpio_reg_ops[] = {
> > [gpio_direction]  = { .read = read_direction, .write =
> > write_direction };
> > ...
> > };
> > 
> > Then we have:
> > 
> > static const struct AspeedGPIO gpios[...] = {
> >  [GPIO_ABCD_DIRECTION] = { .set = 0, .type = gpio_direction },
> >  ...
> > };
> > 
> > Not wedded to the idea, just thinking out loud. What do you think?
> > 
> 
> I'm not a huge fan of the extra indirection. It is less error prone but
> is this a place that is likely to be updated regularly?

Right - if you've written correctly already then there's not much benefit.

> 
> 
> > > +[GPIO_ABCD_DIRECTION] = {0, read_direction, _write_direction},
> > > +[GPIO_ABCD_INT_ENABLE] = {0, read_int_enable,
> > > _write_int_enable},
> > > +[GPIO_ABCD_INT_SENS_0] = {0, read_int_sens_0,
> > > _write_int_sens_0},
> > > +[GPIO_ABCD_INT_SENS_1] = {0, read_int_sens_1,
> > > _write_int_sens_1},
> > > +[GPIO_ABCD_INT_SENS_2] = {0, read_int_sens_2,
> > > _write_int_sens_2},
> > > +[GPIO_ABCD_INT_STATUS] = {0, read_int_status,
> > > _write_int_status},
> > > +[GPIO_ABCD_RESET_TOLERANT] = {0, read_reset_tol,
> > > _write_reset_tol},
> > > +[GPIO_ABCD_DEBOUNCE_1] = {0, read_debounce_1,
> > > _write_debounce_1},
> > > +[GPIO_ABCD_DEBOUNCE_2] = {0, read_debounce_2,
> > > _write_debounce_2},
> > > +[GPIO_ABCD_COMMAND_SRC_0] = {0, read_cmd_source_0, 
> > > _write_cmd_source_0},
> > > +[GPIO_ABCD_COMMAND_SRC_1] = {0, read_cmd_source_1, 
> > > _write_cmd_source_1},
> > > +[GPIO_ABCD_DATA_READ] = {0, read_data, NULL},
> > > +[GPIO_ABCD_INPUT_MASK] = {0, read_input_mask,
> > > _write_input_mask},
> > > +/* Set EFGH */
> > > +[GPIO_EFGH_DATA_VALUE] = {1, read_data_value,
> > > _write_data_value},
> > > +[GPIO_EFGH_DIRECTION] = {1, read_direction, _write_direction
> > > },
> > > +[GPIO_EFGH_INT_ENABLE] = {1, read_int_enable,
> > > _write_int_enable},
> > > +[GPIO_EFGH_INT_SENS_0] = {1, read_int_sens_0,
> > > _write_int_sens_0},
> > > +[GPIO_EFGH_INT_SENS_1] = {1, read_int_sens_1,
> > > _write_int_sens_1},
> > > +[GPIO_EFGH_INT_SENS_2] = {1, read_int_sens_2,
> > > _write_int_sens_2},
> > > +[GPIO_EFGH_INT_STATUS] = {1, read_int_status,
> > > _write_int_status},
> > > +[GPIO_EFGH_RESET_TOL] = {1,
> > > read_reset_tol,   _write_reset_tol},
> > > +[GPIO_EFGH_DEBOUNCE_1] = {1,
> > > read_debounce_1,  _write_debounce_1},
> > > +[GPIO_EFGH_DEBOUNCE_2] = {1,
> > > read_debounce_2,  _write_debounce_2},
> > > +[GPIO_EFGH_COMMAND_SRC_0] = {1, read_cmd_source_0,  
> > > _write_cmd_source_0},
> > > +[GPIO_EFGH_COMMAND_SRC_1] = {1, read_cmd_source_1,  
> > > _write_cmd_source_1},
> > > +[GPIO_EFGH_DATA_READ] = {1, read_data, NULL},
> > > +[GPIO_EFGH_INPUT_MASK] = {1,
> > > read_input_mask,  _write_input_mask},
> > > +/* Set IJKL */
> > > +[GPIO_IJKL_DATA_VALUE] = {2, read_data_value,
> > > _write_data_value},
> > > +[GPIO_IJKL_DIRECTION] = {2, read_direction, _write_direction},
> > > +[GPIO_IJKL_INT_ENABLE] = {2, read_int_enable,
> > > _write_int_enable},
> > > +[GPIO_IJKL_INT_SENS_0] = {2, read_int_sens_0,
> > > _write_int_sens_0},
> > > +[GPIO_IJKL_INT_SENS_1] = {2, read_int_sens_1,
> > > _write_int_sens_1},
> > > +[GPIO_IJKL_INT_SENS_2] = {2, read_int_sens_2,
> > > _write_int_sens_2},
> > > +[GPIO_IJKL_INT_STATUS] = {2, read_int_status,
> > > _write_int_status},
> > > +[GPIO_IJKL_RESET_TOLERANT] = {2, read_reset_tol,
> > > _write_reset_tol},
> > > +[GPIO_IJKL_DEBOUNCE_1] = {2, read_debounce_1,
> > > _write_debounce_1},
> > > +[GPIO_IJKL_DEBOUNCE_2] = {2, read_debounce_2,
> > > _write_debounce_2},
> > > +[GPIO_IJKL_COMMAND_SRC_0] = {2, read_cmd_source_0, 
> > > _write_cmd_source_0},
> > > +[GPIO_IJKL_COMMAND_SRC_1] = {2, read_cmd_source_1, 
> > > _write_cmd_source_1},
> > > +[GPIO_IJKL_DATA_READ] = {2, read_data, NULL},
> > > +[GPIO_IJKL_INPUT_MASK] = {2, read_input_mask,
> > > _write_input_mask},
> > > +/* Set MNOP */
> > > +[GPIO_MNOP_DATA_VALUE] = {3, read_data_value,
> > > _write_data_value},
> > > +[GPIO_MNOP_DIRECTION] 

Re: [Qemu-devel] [PATCH 1/2] hw/gpio: Add basic Aspeed GPIO model

[Rashmica Gupta]

Sorry for the late reply! I agree with most of your feedback and will
send out
a v2 shortly with those changes. I have a few replies below

[snip]

> > +static const struct AspeedGPIO gpios[0x1f0] = {
> > +/* Set ABCD */
> > +[GPIO_ABCD_DATA_VALUE] = {0, read_data_value,
> > _write_data_value},
> 
> Maybe there would be less tedium (and risk of minor bugs) here if we
> add
> a table for looking up the read/write callbacks from a type, and just
> stash
> the type in struct AspeedGPIO (or a pointer to the corresponding type
> ops),
> e.g:
> 
> enum gpio_property { gpio_direction = 0, gpio_int_enable, ... };
> 
> struct aspeed_gpio_reg_ops {
> int (*read)(...);
> int (*write)(...);
> };
> 
> struct aspeed_gpio_reg_ops gpio_reg_ops[] = {
> [gpio_direction]  = { .read = read_direction, .write =
> write_direction };
> ...
> };
> 
> Then we have:
> 
> static const struct AspeedGPIO gpios[...] = {
>  [GPIO_ABCD_DIRECTION] = { .set = 0, .type = gpio_direction },
>  ...
> };
> 
> Not wedded to the idea, just thinking out loud. What do you think?
> 

I'm not a huge fan of the extra indirection. It is less error prone but
is this a place that is likely to be updated regularly?


> > +[GPIO_ABCD_DIRECTION] = {0, read_direction, _write_direction},
> > +[GPIO_ABCD_INT_ENABLE] = {0, read_int_enable,
> > _write_int_enable},
> > +[GPIO_ABCD_INT_SENS_0] = {0, read_int_sens_0,
> > _write_int_sens_0},
> > +[GPIO_ABCD_INT_SENS_1] = {0, read_int_sens_1,
> > _write_int_sens_1},
> > +[GPIO_ABCD_INT_SENS_2] = {0, read_int_sens_2,
> > _write_int_sens_2},
> > +[GPIO_ABCD_INT_STATUS] = {0, read_int_status,
> > _write_int_status},
> > +[GPIO_ABCD_RESET_TOLERANT] = {0, read_reset_tol,
> > _write_reset_tol},
> > +[GPIO_ABCD_DEBOUNCE_1] = {0, read_debounce_1,
> > _write_debounce_1},
> > +[GPIO_ABCD_DEBOUNCE_2] = {0, read_debounce_2,
> > _write_debounce_2},
> > +[GPIO_ABCD_COMMAND_SRC_0] = {0, read_cmd_source_0, 
> > _write_cmd_source_0},
> > +[GPIO_ABCD_COMMAND_SRC_1] = {0, read_cmd_source_1, 
> > _write_cmd_source_1},
> > +[GPIO_ABCD_DATA_READ] = {0, read_data, NULL},
> > +[GPIO_ABCD_INPUT_MASK] = {0, read_input_mask,
> > _write_input_mask},
> > +/* Set EFGH */
> > +[GPIO_EFGH_DATA_VALUE] = {1, read_data_value,
> > _write_data_value},
> > +[GPIO_EFGH_DIRECTION] = {1, read_direction, _write_direction
> > },
> > +[GPIO_EFGH_INT_ENABLE] = {1, read_int_enable,
> > _write_int_enable},
> > +[GPIO_EFGH_INT_SENS_0] = {1, read_int_sens_0,
> > _write_int_sens_0},
> > +[GPIO_EFGH_INT_SENS_1] = {1, read_int_sens_1,
> > _write_int_sens_1},
> > +[GPIO_EFGH_INT_SENS_2] = {1, read_int_sens_2,
> > _write_int_sens_2},
> > +[GPIO_EFGH_INT_STATUS] = {1, read_int_status,
> > _write_int_status},
> > +[GPIO_EFGH_RESET_TOL] = {1,
> > read_reset_tol,   _write_reset_tol},
> > +[GPIO_EFGH_DEBOUNCE_1] = {1,
> > read_debounce_1,  _write_debounce_1},
> > +[GPIO_EFGH_DEBOUNCE_2] = {1,
> > read_debounce_2,  _write_debounce_2},
> > +[GPIO_EFGH_COMMAND_SRC_0] = {1, read_cmd_source_0,  
> > _write_cmd_source_0},
> > +[GPIO_EFGH_COMMAND_SRC_1] = {1, read_cmd_source_1,  
> > _write_cmd_source_1},
> > +[GPIO_EFGH_DATA_READ] = {1, read_data, NULL},
> > +[GPIO_EFGH_INPUT_MASK] = {1,
> > read_input_mask,  _write_input_mask},
> > +/* Set IJKL */
> > +[GPIO_IJKL_DATA_VALUE] = {2, read_data_value,
> > _write_data_value},
> > +[GPIO_IJKL_DIRECTION] = {2, read_direction, _write_direction},
> > +[GPIO_IJKL_INT_ENABLE] = {2, read_int_enable,
> > _write_int_enable},
> > +[GPIO_IJKL_INT_SENS_0] = {2, read_int_sens_0,
> > _write_int_sens_0},
> > +[GPIO_IJKL_INT_SENS_1] = {2, read_int_sens_1,
> > _write_int_sens_1},
> > +[GPIO_IJKL_INT_SENS_2] = {2, read_int_sens_2,
> > _write_int_sens_2},
> > +[GPIO_IJKL_INT_STATUS] = {2, read_int_status,
> > _write_int_status},
> > +[GPIO_IJKL_RESET_TOLERANT] = {2, read_reset_tol,
> > _write_reset_tol},
> > +[GPIO_IJKL_DEBOUNCE_1] = {2, read_debounce_1,
> > _write_debounce_1},
> > +[GPIO_IJKL_DEBOUNCE_2] = {2, read_debounce_2,
> > _write_debounce_2},
> > +[GPIO_IJKL_COMMAND_SRC_0] = {2, read_cmd_source_0, 
> > _write_cmd_source_0},
> > +[GPIO_IJKL_COMMAND_SRC_1] = {2, read_cmd_source_1, 
> > _write_cmd_source_1},
> > +[GPIO_IJKL_DATA_READ] = {2, read_data, NULL},
> > +[GPIO_IJKL_INPUT_MASK] = {2, read_input_mask,
> > _write_input_mask},
> > +/* Set MNOP */
> > +[GPIO_MNOP_DATA_VALUE] = {3, read_data_value,
> > _write_data_value},
> > +[GPIO_MNOP_DIRECTION] = {3, read_direction, _write_direction},
> > +[GPIO_MNOP_INT_ENABLE] = {3, read_int_enable,
> > _write_int_enable},
> > +[GPIO_MNOP_INT_SENS_0] = {3, read_int_sens_0,
> > _write_int_sens_0},
> > +[GPIO_MNOP_INT_SENS_1] = {3, read_int_sens_1,
> > _write_int_sens_1},
> > +[GPIO_MNOP_INT_SENS_2] = {3, read_int_sens_2,
> > _write_int_sens_2},
> > +[GPIO_MNOP_INT_STATUS] = 

Re: [Qemu-devel] [PATCH v2] migration/postcopy: fix document of postcopy_send_discard_bm_ram()

[Wei Yang]

On Mon, Jul 15, 2019 at 10:05:49AM +0800, Wei Yang wrote:
>Commit 6b6712efccd3 ('ram: Split dirty bitmap by RAMBlock') changes the
>parameter of postcopy_send_discard_bm_ram(), while left the document
>part untouched.
>
>This patch correct the document and fix two typo by hand.
>
>Signed-off-by: Wei Yang 

Oops, I missed this. Sorry Philippe :)

Reviewed-by: Philippe Mathieu-Daudé 

>---
>v2: fix typo in function name, pointed by Dave
>---
> migration/ram.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/migration/ram.c b/migration/ram.c
>index 246efe6939..e019c925b2 100644
>--- a/migration/ram.c
>+++ b/migration/ram.c
>@@ -2763,8 +2763,7 @@ void ram_postcopy_migrated_memory_release(MigrationState 
>*ms)
>  *
>  * @ms: current migration state
>  * @pds: state for postcopy
>- * @start: RAMBlock starting page
>- * @length: RAMBlock size
>+ * @block: RAMBlock to discard
>  */
> static int postcopy_send_discard_bm_ram(MigrationState *ms,
> PostcopyDiscardState *pds,
>@@ -2961,7 +2960,7 @@ static void postcopy_chunk_hostpages_pass(MigrationState 
>*ms, bool unsent_pass,
> }
> 
> /**
>- * postcopy_chuck_hostpages: discrad any partially sent host page
>+ * postcopy_chunk_hostpages: discard any partially sent host page
>  *
>  * Utility for the outgoing postcopy code.
>  *
>-- 
>2.17.1

-- 
Wei Yang
Help you, Help me

[Qemu-devel] [PATCH v2] migration/postcopy: fix document of postcopy_send_discard_bm_ram()

[Wei Yang]

Commit 6b6712efccd3 ('ram: Split dirty bitmap by RAMBlock') changes the
parameter of postcopy_send_discard_bm_ram(), while left the document
part untouched.

This patch correct the document and fix two typo by hand.

Signed-off-by: Wei Yang 
---
v2: fix typo in function name, pointed by Dave
---
 migration/ram.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 246efe6939..e019c925b2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2763,8 +2763,7 @@ void ram_postcopy_migrated_memory_release(MigrationState 
*ms)
  *
  * @ms: current migration state
  * @pds: state for postcopy
- * @start: RAMBlock starting page
- * @length: RAMBlock size
+ * @block: RAMBlock to discard
  */
 static int postcopy_send_discard_bm_ram(MigrationState *ms,
 PostcopyDiscardState *pds,
@@ -2961,7 +2960,7 @@ static void postcopy_chunk_hostpages_pass(MigrationState 
*ms, bool unsent_pass,
 }
 
 /**
- * postcopy_chuck_hostpages: discrad any partially sent host page
+ * postcopy_chunk_hostpages: discard any partially sent host page
  *
  * Utility for the outgoing postcopy code.
  *
-- 
2.17.1

Re: [Qemu-devel] [PATCH] migration: always initial RAMBlock.bmap to 1 for new migration

[Peter Xu]

On Sun, Jul 14, 2019 at 10:51:19PM +0800, Ivan Ren wrote:
> Reproduce the problem:
> migrate
> migrate_cancel
> migrate
> 
> Error happen for memory migration

Can mention "this mostly revert 0315851938 but with comments kept"
when merge...

> 
> The reason as follows:
> 1. qemu start, ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] all set to
>1 by a series of cpu_physical_memory_set_dirty_range
> 2. migration start:ram_init_bitmaps
>- memory_global_dirty_log_start: begin log diry
>- memory_global_dirty_log_sync: sync dirty bitmap to
>  ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]
>- migration_bitmap_sync_range: sync ram_list.
>  dirty_memory[DIRTY_MEMORY_MIGRATION] to RAMBlock.bmap
>  and ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] is set to zero
> 3. migration data...
> 4. migrate_cancel, will stop log dirty
> 5. migration start:ram_init_bitmaps
>- memory_global_dirty_log_start: begin log diry
>- memory_global_dirty_log_sync: sync dirty bitmap to
>  ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]
>- migration_bitmap_sync_range: sync ram_list.
>  dirty_memory[DIRTY_MEMORY_MIGRATION] to RAMBlock.bmap
>  and ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] is set to zero
> 
>Here RAMBlock.bmap only have new logged dirty pages, don't contain
>the whole guest pages.

Fixes: 03158519384f158

> 
> Signed-off-by: Ivan Ren 

Reviewed-by: Peter Xu 

I think this is a bit severe and should be rc2 material.  Dave/Juan?

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v7 00/13] Add migration support for VFIO device

[Yan Zhao]

On Sat, Jul 13, 2019 at 01:42:39AM +0800, Dr. David Alan Gilbert wrote:
> * Kirti Wankhede (kwankh...@nvidia.com) wrote:
> > 
> > 
> > On 7/11/2019 9:53 PM, Dr. David Alan Gilbert wrote:
> > > * Yan Zhao (yan.y.z...@intel.com) wrote:
> > >> On Thu, Jul 11, 2019 at 06:50:12PM +0800, Dr. David Alan Gilbert wrote:
> > >>> * Yan Zhao (yan.y.z...@intel.com) wrote:
> >  Hi Kirti,
> >  There are still unaddressed comments to your patches v4.
> >  Would you mind addressing them?
> > 
> >  1. should we register two migration interfaces simultaneously
> >  (https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04750.html)
> > >>>
> > >>> Please don't do this.
> > >>> As far as I'm aware we currently only have one device that does that
> > >>> (vmxnet3) and a patch has just been posted that fixes/removes that.
> > >>>
> > >>> Dave
> > >>>
> > >> hi Dave,
> > >> Thanks for notifying this. but if we want to support postcopy in future,
> > >> after device stops, what interface could we use to transfer data of
> > >> device state only?
> > >> for postcopy, when source device stops, we need to transfer only
> > >> necessary device state to target vm before target vm starts, and we
> > >> don't want to transfer device memory as we'll do that after target vm
> > >> resuming.
> > > 
> > > Hmm ok, lets see; that's got to happen in the call to:
> > > qemu_savevm_state_complete_precopy(fb, false, false);
> > > that's made from postcopy_start.
> > >  (the false's are iterable_only and inactivate_disks)
> > > 
> > > and at that time I believe the state is POSTCOPY_ACTIVE, so in_postcopy
> > > is true.
> > > 
> > > If you're doing postcopy, then you'll probably define a has_postcopy()
> > > function, so qemu_savevm_state_complete_precopy will skip the
> > > save_live_complete_precopy call from it's loop for at least two of the
> > > reasons in it's big if.
> > > 
> > > So you're right; you need the VMSD for this to happen in the second
> > > loop in qemu_savevm_state_complete_precopy.  Hmm.
> > > 
> > > Now, what worries me, and I don't know the answer, is how the section
> > > header for the vmstate and the section header for an iteration look
> > > on the stream; how are they different?
> > > 
> > 
> > I don't have way to test postcopy migration - is one of the major reason
> > I had not included postcopy support in this patchset and clearly called
> > out in cover letter.
> > This patchset is thoroughly tested for precopy migration.
> > If anyone have hardware that supports fault, then I would prefer to add
> > postcopy support as incremental change later which can be tested before
> > submitting.
> 
> Agreed; although I think Yan's right to think about how it's going to
> work.
> 
> > Just a suggestion, instead of using VMSD, is it possible to have some
> > additional check to call save_live_complete_precopy from
> > qemu_savevm_state_complete_precopy?
> 
> Probably yes; although as you can tell the logic in their is already
> pretty hairy.
> 
This might be a solution. but in that case, lots of modules' 
.save_live_complete_precopy needs to be rewritten, including that of
ram. and we need to redefine the interface of .save_live_complete_precopy.
maybe we can add a new interface like .save_live_pre_postcopy in SaveVMHandlers?

Thanks
Yan

> Dave
> 
> > 
> > >>
> >  2. in each save iteration, how much data is to be saved
> >  (https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04683.html)
> > 
> > > how big is the data_size ?
> > > if this size is too big, it may take too much time and block others.
> > 
> > I do had mentioned this in the comment about the structure in vfio.h
> > header. data_size will be provided by vendor driver and obviously will
> > not be greater that migration region size. Vendor driver should be
> > responsible to keep its solution optimized.
> > 
> > 
> >  3. do we need extra interface to get data for device state only
> >  (https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04812.html)
> > 
> > I don't think so. Opaque Device data from vendor driver can include
> > device state and device memory. Vendor driver who is managing his device
> > can decide how to place data over the stream.
> > 
> >  4. definition of dirty page copied_pfn
> >  (https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05592.html)
> > 
> > 
> > This was inline to discussion going with Alex. I addressed the concern
> > there. Please check current patchset, which addresses the concerns raised.
> > 
> >  Also, I'm glad to see that you updated code by following my comments 
> >  below,
> >  but please don't forget to reply my comments next time:)
> > 
> > I tried to reply top of threads and addressed common concerns raised in
> > that. Sorry If I missed any, I'll make sure to point you to my replies
> > going ahead.
> > 
> > Thanks,
> > Kirti
> > 
> >  https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05357.html
> >  

Re: [Qemu-devel] [RISU PATCH v3 00/18] Support for generating x86 SIMD test images

[Jan Bobek]

On 7/12/19 9:34 AM, Alex Bennée wrote:
> 
> Jan Bobek  writes:
> 
>> This is v3 of the patch series posted in [1] and [2]. Note that this
>> is the first fully-featured patch series implementing all desired
>> functionality, including (V)LDMXCSR and VSIB-based instructions like
>> VGATHER*.
>>
>> While implementing the last bits required in order to support VGATHERx
>> instructions, I ran into problems which required a larger redesign;
>> namely, there are no more !emit blocks as their functionality is now
>> implemented in regular !constraints blocks. Also, memory constraints
>> are specified in !memory blocks, similarly to other architectures.
>>
>> I tested these changes on my machine; both master and slave modes work
>> in both 32-bit and 64-bit modes.
> 
> Two things I've noticed:
> 
>   ./contrib/generate_all.sh -n 1 x86.risu testcases.x86
> 
> takes a very long time. I wonder if this is a consequence of constantly
> needing to re-query the random number generator?

I believe so. While other architectures can be as cheap as a single rand()
call per instruction, x86 does more like 5-10.

Even worse, there are some instructions which cannot be generated in
32-bit mode (those requiring REX.W prefix, e.g. MMX MOVQ). When I let
the script run for a little bit, risugen would get stuck in an
infinite loop, because it could only choose from a single instruction
which wasn't valid for 32-bit

> The other is:
> 
>   set -x RISU ./build/i686-linux-gnu/risu
>   ./contrib/record_traces.sh testcases.x86/*.risu.bin
> 
> fails on the first trace when validating the playback. Might want to
> check why that is.

The SIMD registers aren't getting initialized; both master and
apprentice need an --xfeatures=XXX parameter for that. Right now the
default is 'none'; unless the instructions are filtered, you'd need
--xfeatures=avx (or --xfeatures=sse, and that only works because on my
laptop, the upper part of ymm registers seems to be always zeroed when
risu starts).

>>
>> Cheers,
>>  -Jan
>>
>> Changes since v2:
>>   Too many to be listed individually; this patch series might be
>>   better reviewed on its own.
>>
>> References:
>>   1. https://lists.nongnu.org/archive/html/qemu-devel/2019-06/msg04123.html
>>   2. https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg1.html
>>
>> Jan Bobek (18):
>>   risugen_common: add helper functions insnv, randint
>>   risugen_common: split eval_with_fields into extract_fields and
>> eval_block
>>   risugen_x86_asm: add module
>>   risugen_x86_constraints: add module
>>   risugen_x86_memory: add module
>>   risugen_x86: add module
>>   risugen: allow all byte-aligned instructions
>>   risugen: add command-line flag --x86_64
>>   risugen: add --xfeatures option for x86
>>   x86.risu: add MMX instructions
>>   x86.risu: add SSE instructions
>>   x86.risu: add SSE2 instructions
>>   x86.risu: add SSE3 instructions
>>   x86.risu: add SSSE3 instructions
>>   x86.risu: add SSE4.1 and SSE4.2 instructions
>>   x86.risu: add AES and PCLMULQDQ instructions
>>   x86.risu: add AVX instructions
>>   x86.risu: add AVX2 instructions
>>
>>  risugen|   27 +-
>>  risugen_arm.pm |6 +-
>>  risugen_common.pm  |  117 +-
>>  risugen_m68k.pm|3 +-
>>  risugen_ppc64.pm   |6 +-
>>  risugen_x86.pm |  518 +
>>  risugen_x86_asm.pm |  918 
>>  risugen_x86_constraints.pm |  154 ++
>>  risugen_x86_memory.pm  |   87 +
>>  x86.risu   | 4499 
>>  10 files changed, 6293 insertions(+), 42 deletions(-)
>>  create mode 100644 risugen_x86.pm
>>  create mode 100644 risugen_x86_asm.pm
>>  create mode 100644 risugen_x86_constraints.pm
>>  create mode 100644 risugen_x86_memory.pm
>>  create mode 100644 x86.risu
> 
> 
> --
> Alex Bennée
> 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RISU PATCH v3 04/18] risugen_x86_constraints: add module

[Jan Bobek]

On 7/12/19 10:24 AM, Richard Henderson wrote:
> On 7/12/19 12:32 AM, Jan Bobek wrote:
>> +sub vex($%)
>> +{
>> +my ($insn, %vex) = @_;
>> +my $regidw = $is_x86_64 ? 4 : 3;
>> +
>> +# There is no point in randomizing other VEX fields, since
>> +# VEX.R/.X/.B are encoded automatically by risugen_x86_asm, and
>> +# VEX.M/.P are opcodes.
>> +$vex{l} = randint(width => 1) ? 256 : 128 unless defined $vex{l};
> 
> VEX.L is sort-of opcode-like as well.  It certainly differentiates AVX1 vs
> AVX2, and so probably should be constrained somehow.  I can't think of what's
> the best way to do that at the moment, since our existing --xstate=foo isn't 
> right.
> 
> Perhaps just a FIXME comment for now?

So, the instructions that use VEX.L specify it in the !constraints
block in the config file. Originally, I thought some instructions are
supposed to ignore it (denoted by LIG in the Intel manual -- it's the
scalar instructions like ADDSS), so it might be worth randomizing.
However, when I later read the manual pages of some of these
instructions, it said they are supposed to be encoded with VEX.L=0
anyway. I didn't check every single one of them, but right now they
are all encoded with VEX.L=0, so I suppose this line can be removed
and we can rely on the caller (the !constraints block) to always
specify it.

>> +sub modrm_($%)
>> +{
>> +my ($insn, %args) = @_;
>> +my $regidw = $is_x86_64 ? 4 : 3;
>> +
>> +my %modrm = ();
>> +if (defined $args{reg}) {
>> +# This makes the config file syntax a bit more accommodating
>> +# in cases where MODRM.REG is an opcode extension field.
>> +$modrm{reg} = $args{reg};
>> +} else {
>> +$modrm{reg} = randint(width => $regidw);
>> +}
>> +
>> +# There is also a displacement-only form, but we don't know
>> +# absolute address of the memblock, so we cannot test it.
> 
> 32-bit mode has displacement-only, aka absolute; 64-bit replaces that with
> rip-relative.  But agreed that the first is impossible to test and the second
> is difficult.
> 
>> +sub modrm($%)
>> +{
>> +my ($insn, %args) = @_;
>> +modrm_($insn, indexk => 'index', %args);
>> +}
> 
> How are you avoiding %rsp as index?
> I saw you die for that in the previous patch...

See write_mem_getoffset in risugen_x86.pm. I felt there's a better
place for it there, since that's when we actually need to write to it,
so the problem is more exposed.

-Jan

> 
> r~
> 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v9 0/2] Add Arm SBSA Reference Machine

[Radoslaw Biernacki]

If running full machine stack is an option, than I think that you will be
able to use SBSA machine in your tests once we have all the FW images
ready. But unfortunately we are not there yet.


niedz., 14 lip 2019, 23:57 użytkownik Guenter Roeck 
napisał:

> On 7/14/19 8:40 AM, Radoslaw Biernacki wrote:
> > This machine is not ment for direct kernel boot. Is main purpose is
> development of FW, kernel and other HW/SW parts for SBSA. We are currently
> working on UEFI and ATF for this machine.
> >
> > It might be somehow possible to run kernel with DT but we do not support
> it at this moment. If all you want is to boot kernel directly, it is far
> more convenient to use existing virt machine.
> >
>
> Too bad. As you may know, I am testing the Linux kernel by running it with
> as many qemu
> machines as possible. I already run several boot tests with the 'virt'
> machine, and
> I was trying to extend test coverage with the sbsa machine.
>
> Guenter
>
> > niedz., 14 lip 2019, 17:20 użytkownik Guenter Roeck  > napisał:
> >
> > Hi,
> >
> > On Sun, Jun 30, 2019 at 06:20:32PM +0800, Hongbo Zhang wrote:
> >  > For the Aarch64, there is one machine 'virt', it is primarily
> meant to
> >  > run on KVM and execute virtualization workloads, but we need an
> >  > environment as faithful as possible to physical hardware,  to
> support
> >  > firmware and OS development for pysical Aarch64 machines.
> >  >
> >
> > I tried to boot linux on this machine with -kernel command line
> argument,
> > but have not been successful. Can someone point me to a working
> command
> > line, one that lets me load the kernel directly ?
> >
> > Thanks,
> > Guenter
> >
>
>

Re: [Qemu-devel] [RISU PATCH v3 03/18] risugen_x86_asm: add module

[Jan Bobek]

On 7/12/19 10:11 AM, Richard Henderson wrote:
> On 7/12/19 12:32 AM, Jan Bobek wrote:
>> The module risugen_x86_asm.pm exports named register constants and
>> asm_insn_* family of functions, which greatly simplify emission of x86
>> instructions.
>>
>> Signed-off-by: Jan Bobek 
>> ---
>>  risugen_x86_asm.pm | 918 +
>>  1 file changed, 918 insertions(+)
>>  create mode 100644 risugen_x86_asm.pm
> 
> Clever use of token lists to make sure all state is processed as expected.  
> Kudos!

I was curious what you'll think of this part; thanks a lot, it's much
appreciated!

-Jan

> Reviewed-by: Richard Henderson 
> 
> 
> r~
> 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v9 0/2] Add Arm SBSA Reference Machine

[Guenter Roeck]

On 7/14/19 8:40 AM, Radoslaw Biernacki wrote:

This machine is not ment for direct kernel boot. Is main purpose is development 
of FW, kernel and other HW/SW parts for SBSA. We are currently working on UEFI 
and ATF for this machine.

It might be somehow possible to run kernel with DT but we do not support it at 
this moment. If all you want is to boot kernel directly, it is far more 
convenient to use existing virt machine.



Too bad. As you may know, I am testing the Linux kernel by running it with as 
many qemu
machines as possible. I already run several boot tests with the 'virt' machine, 
and
I was trying to extend test coverage with the sbsa machine.

Guenter


niedz., 14 lip 2019, 17:20 użytkownik Guenter Roeck mailto:li...@roeck-us.net>> napisał:

Hi,

On Sun, Jun 30, 2019 at 06:20:32PM +0800, Hongbo Zhang wrote:
 > For the Aarch64, there is one machine 'virt', it is primarily meant to
 > run on KVM and execute virtualization workloads, but we need an
 > environment as faithful as possible to physical hardware,  to support
 > firmware and OS development for pysical Aarch64 machines.
 >

I tried to boot linux on this machine with -kernel command line argument,
but have not been successful. Can someone point me to a working command
line, one that lets me load the kernel directly ?

Thanks,
Guenter

Re: [Qemu-devel] [RISU PATCH v3 01/18] risugen_common: add helper functions insnv, randint

[Jan Bobek]

On 7/12/19 1:48 AM, Richard Henderson wrote:
> On 7/12/19 12:32 AM, Jan Bobek wrote:
>> insnv allows emitting variable-length instructions in little-endian or
>> big-endian byte order; it subsumes functionality of former insn16()
>> and insn32() functions.
>>
>> randint can reliably generate signed or unsigned integers of arbitrary
>> width.
>>
>> Signed-off-by: Jan Bobek 
>> ---
>>  risugen_common.pm | 55 +--
>>  1 file changed, 48 insertions(+), 7 deletions(-)
>>
>> diff --git a/risugen_common.pm b/risugen_common.pm
>> index 71ee996..d63250a 100644
>> --- a/risugen_common.pm
>> +++ b/risugen_common.pm
>> @@ -23,8 +23,9 @@ BEGIN {
>>  require Exporter;
>>  
>>  our @ISA = qw(Exporter);
>> -our @EXPORT = qw(open_bin close_bin set_endian insn32 insn16 $bytecount
>> -   progress_start progress_update progress_end
>> +our @EXPORT = qw(open_bin close_bin set_endian insn32 insn16
>> +   $bytecount insnv randint progress_start
>> +   progress_update progress_end
>> eval_with_fields is_pow_of_2 sextract ctz
>> dump_insn_details);
>>  }
>> @@ -37,7 +38,7 @@ my $bigendian = 0;
>>  # (default is little endian, 0).
>>  sub set_endian
>>  {
>> -$bigendian = @_;
>> +($bigendian) = @_;
>>  }
>>  
>>  sub open_bin
>> @@ -52,18 +53,58 @@ sub close_bin
>>  close(BIN) or die "can't close output file: $!";
>>  }
>>  
>> +sub insnv(%)
>> +{
>> +my (%args) = @_;
>> +
>> +# Default to big-endian order, so that the instruction bytes are
>> +# emitted in the same order as they are written in the
>> +# configuration file.
>> +$args{bigendian} = 1 unless defined $args{bigendian};
>> +
>> +for (my $bitcur = 0; $bitcur < $args{width}; $bitcur += 8) {
>> +my $value = $args{value} >> ($args{bigendian}
>> + ? $args{width} - $bitcur - 8
>> + : $bitcur);
>> +
>> +print BIN pack("C", $value & 0xff);
>> +$bytecount += 1;
>> +}
> 
> Looks like bytecount is no longer used?

bytecount is an exported variable, a quick git grep shows that
it's being used in risugen_arm.pm (sub thumb_align4).

> Otherwise,
> Reviewed-by: Richard Henderson 
> 
> 
> r~
> 



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1836501] [NEW] cpu_address_space_init fails with assertion

[Lutz]

Public bug reported:

qemu-system-arm does not start with version >= 2.6 and KVM enabled.

  cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()'
failed.

Hardware is Odroid XU4 with Exynos with 4.9.61+ Tested with Debian
Stretch (9) or Buster (10).

Without KVM it is running fine but slow. I'm operating Debian Jessie
with qemu 2.1 for a long time with KVM virtualization working
flawlessly. When I upgraded to Stretch I ran into the trouble described
before. I tried Debian Stretch and Buster with all Kernels provided by
the Board manufacturer (Hardkernel).

It seems to be related to the feature introduced in Version 2.6:
https://wiki.qemu.org/ChangeLog/2.6
- Support for a separate EL3 address space

KVM is enabled, so I assume the adress space index asidx to be causing
the assert to fail.

dmesg | grep -i KVM
[0.741714] kvm [1]: 8-bit VMID
[0.741721] kvm [1]: IDMAP page: 40201000
[0.741729] kvm [1]: HYP VA range: c000:
[0.742543] kvm [1]: Hyp mode initialized successfully
[0.742600] kvm [1]: vgic-v2@10484000
[0.742924] kvm [1]: vgic interrupt IRQ16
[0.742943] kvm [1]: virtual timer IRQ60

Full command line is:
qemu-system-arm -M vexpress-a15 -smp 2 -m 512 -cpu host -enable-kvm -kernel 
vmlinuz -initrd initrd.gz -dtb vexpress-v2p-ca15-tc1.dtb -device 
virtio-blk-device,drive=inst-blk -drive 
file=PATHTOFILE,id=inst-blk,if=none,format=raw -append "vga=normal rw 
console=ttyAMA0" -nographic

Is there anything to do to understand, if this is a hardware related
failure or probably just a missing parameter?

Regards

Lutz

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1836501

Title:
  cpu_address_space_init fails with assertion

Status in QEMU:
  New

Bug description:
  qemu-system-arm does not start with version >= 2.6 and KVM enabled.

cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()'
  failed.

  Hardware is Odroid XU4 with Exynos with 4.9.61+ Tested with Debian
  Stretch (9) or Buster (10).

  Without KVM it is running fine but slow. I'm operating Debian Jessie
  with qemu 2.1 for a long time with KVM virtualization working
  flawlessly. When I upgraded to Stretch I ran into the trouble
  described before. I tried Debian Stretch and Buster with all Kernels
  provided by the Board manufacturer (Hardkernel).

  It seems to be related to the feature introduced in Version 2.6:
  https://wiki.qemu.org/ChangeLog/2.6
  - Support for a separate EL3 address space

  KVM is enabled, so I assume the adress space index asidx to be causing
  the assert to fail.

  dmesg | grep -i KVM
  [0.741714] kvm [1]: 8-bit VMID
  [0.741721] kvm [1]: IDMAP page: 40201000
  [0.741729] kvm [1]: HYP VA range: c000:
  [0.742543] kvm [1]: Hyp mode initialized successfully
  [0.742600] kvm [1]: vgic-v2@10484000
  [0.742924] kvm [1]: vgic interrupt IRQ16
  [0.742943] kvm [1]: virtual timer IRQ60

  Full command line is:
  qemu-system-arm -M vexpress-a15 -smp 2 -m 512 -cpu host -enable-kvm -kernel 
vmlinuz -initrd initrd.gz -dtb vexpress-v2p-ca15-tc1.dtb -device 
virtio-blk-device,drive=inst-blk -drive 
file=PATHTOFILE,id=inst-blk,if=none,format=raw -append "vga=normal rw 
console=ttyAMA0" -nographic

  Is there anything to do to understand, if this is a hardware related
  failure or probably just a missing parameter?

  Regards

  Lutz

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1836501/+subscriptions

Re: [Qemu-devel] [PATCH v5] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

[Arnd Bergmann]

On Sun, Jul 14, 2019 at 3:54 PM Laurent Vivier  wrote:
>
> From: Daniel P. Berrangé 
>
> The SIOCGSTAMP symbol was previously defined in the
> asm-generic/sockios.h header file. QEMU sees that header
> indirectly via sys/socket.h
>
> In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
> the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
> Instead it provides only SIOCGSTAMP_OLD, which only uses a
> 32-bit time_t on 32-bit architectures.
>
> The linux/sockios.h header then defines SIOCGSTAMP using
> either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
> SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
> on 32-bit architectures
>
> To cope with this we must now convert the old and new type from
> the target to the host one.
>
> Signed-off-by: Daniel P. Berrangé 
> Signed-off-by: Laurent Vivier 

Looks good to me now

Reviewed-by: Arnd Bergmann 

Re: [Qemu-devel] [BUG] 216 Alerts reported by LGTM for QEMU (some might be release critical)

[Stefan Weil]

Am 14.07.2019 um 19:30 schrieb Peter Maydell:
[...]
> "Analyzer thinks this multiply can overflow
> but in fact it's not possible" is quite a common false
> positive cause...


The analysers don't complain because a multiply can overflow.

They complain because the code indicates that a larger result is
expected, for example uint64_t = uint32_t * uint32_t. They would not
complain for the same multiplication if it were assigned to a uint32_t.

So there is a simple solution to write the code in a way which avoids
false positives...

Stefan

Re: [Qemu-devel] [BUG] 216 Alerts reported by LGTM for QEMU (some might be release critical)

[Peter Maydell]

On Sat, 13 Jul 2019 at 18:46, Stefan Weil  wrote:
> LGTM reports 16 errors, 81 warnings and 119 recommendations:
> https://lgtm.com/projects/g/qemu/qemu/alerts/?mode=list.

I had a look at some of these before, but mostly I came
to the conclusion that it wasn't worth trying to put the
effort into keeping up with the site because they didn't
seem to provide any useful way to mark things as false
positives. Coverity has its flaws but at least you can do
that kind of thing in its UI (it runs at about a 33% fp
rate, I think.) "Analyzer thinks this multiply can overflow
but in fact it's not possible" is quite a common false
positive cause...

Anyway, if you want to fish out specific issues, analyse
whether they're false positive or real, and report them
to the mailing list as followups to the patches which
introduced the issue, that's probably the best way for
us to make use of this analyzer. (That is essentially
what I do for coverity.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] linux-user: manage binfmt-misc preserve-arg[0] flag

[John Paul Adrian Glaubitz]

Hi!

> On Jul 14, 2019, at 3:40 PM, Laurent Vivier  wrote:
> 
> Add --preserve-arg0 in qemu-binfmt-conf.sh to configure the preserve-arg0
> flag.
> 
> Now, if QEMU is started with -0 or QEMU_ARGV0 and an empty parameter
> argv[0] (the full pathname provided by binfmt-misc) is removed and
> replaced by argv[1] (the original argv[0] provided by binfmt-misc when
> 'P'/preserve-arg[0] is set)
> 
> For instance:
> 
>  $ sudo QEMU_ARGV0= chroot m68k-chroot sh -c 'echo $0'
>  sh
> 
> without this patch:
> 
>  $ sudo chroot m68k-chroot sh -c 'echo $0'
>  /usr/bin/sh

As a regular user of qemu-user (we’re using qemu-user to run Debian’s buildds 
for m68k and sh4), I would like to add that the idea of having to pass 
additional environment variables to make qemu behave as expected, i.e. as the 
real hardware, is sub-optimal.

I would prefer that enabling the preserve flag with the qemu-binfmt.sh script 
would make qemu-user behave correctly.

If I understand correctly, the current design with the environment variable was 
chosen because my preferred approach would break compatibility in certain 
cases. However, I think that correct emulation is more important than 
compatibility to an old broken behavior and I would therefore be in favor to 
make the correct behavior default.

This will also be necessary when using qemu-user with Debian’s sbuild to 
“cross”-build packages with qemu-user. This particular bug was actually 
discovered while building Debian packages for m68k and sh4 using qemu-user.

Thanks,
Adrian

PS: I have disabled receiving messages for this list, so please keep me CC’ed.

Re: [Qemu-devel] [PATCH 1/2] linux-user: remove useless variable

[Philippe Mathieu-Daudé]

On 7/14/19 3:40 PM, Laurent Vivier wrote:
> filename is only used to open the file if AT_EXECFD is not provided.
> But exec_path already contains the path of the file to open.
> Remove filename as it is only used in main.c whereas exec_path is
> also used in syscall.c.
> 
> Fixes: d088d664f201 ("linux-user: identify running binary in /proc/self/exe")
> Signed-off-by: Laurent Vivier 
> ---
>  linux-user/main.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a59ae9439de1..ef8e8cb10eba 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -48,7 +48,6 @@
>  char *exec_path;
>  
>  int singlestep;
> -static const char *filename;
>  static const char *argv0;
>  static int gdbstub_port;
>  static envlist_t *envlist;
> @@ -580,7 +579,6 @@ static int parse_args(int argc, char **argv)
>  exit(EXIT_FAILURE);
>  }
>  
> -filename = argv[optind];
>  exec_path = argv[optind];
>  
>  return optind;
> @@ -651,9 +649,9 @@ int main(int argc, char **argv, char **envp)
>  
>  execfd = qemu_getauxval(AT_EXECFD);
>  if (execfd == 0) {
> -execfd = open(filename, O_RDONLY);
> +execfd = open(exec_path, O_RDONLY);
>  if (execfd < 0) {
> -printf("Error while loading %s: %s\n", filename, 
> strerror(errno));
> +printf("Error while loading %s: %s\n", exec_path, 
> strerror(errno));
>  _exit(EXIT_FAILURE);
>  }
>  }
> @@ -778,10 +776,10 @@ int main(int argc, char **argv, char **envp)
>  cpu->opaque = ts;
>  task_settid(ts);
>  
> -ret = loader_exec(execfd, filename, target_argv, target_environ, regs,
> +ret = loader_exec(execfd, exec_path, target_argv, target_environ, regs,
>  info, );
>  if (ret != 0) {
> -printf("Error while loading %s: %s\n", filename, strerror(-ret));
> +printf("Error while loading %s: %s\n", exec_path, strerror(-ret));
>  _exit(EXIT_FAILURE);
>  }
>  
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH-for-4.1 2/7] hw/usb: Bluetooth HCI USB depends on USB & BLUETOOTH

[Philippe Mathieu-Daudé]

Hi Paolo,

On 7/12/19 7:31 PM, Paolo Bonzini wrote:
> On 12/07/19 18:45, Philippe Mathieu-Daudé wrote:
>> On 7/12/19 4:58 PM, Paolo Bonzini wrote:
>>> The other is whether we want to enable USB_BLUETOOTH by default.  I
>>> wouldn't have any problem there, but if we disable it basically no one
>>> would ship/use it and we might as well delete the whole thing.
>>
>> The only user is the ARM Nokia N-series board, so if we set default=n,
>> the Bluetooth subsystem will be only be selected on arm-softmmu (and
>> aarch64-softmmu).

Using (1):

+default y if BLUETOOTH
 depends on USB
+select BLUETOOTH

I get:

KconfigDataError: cycle found including BLUETOOTH

This works but doesn't follow kconfig.rst (2):

+default y if BLUETOOTH
 depends on USB

This works (kconfig.rst compliant, 3):

+default n
 depends on USB
+select BLUETOOTH

Are you OK with (2) or you prefer (3)?

>>
>> This seems a sane cleanup. If another board wants to use the bluetooth
>> code, it should probably move it out of the orphan status.
> 
> Fair! ;)
> 
> Paolo
> 

Re: [Qemu-devel] [PATCH v9 0/2] Add Arm SBSA Reference Machine

[Radoslaw Biernacki]

This machine is not ment for direct kernel boot. Is main purpose is
development of FW, kernel and other HW/SW parts for SBSA. We are currently
working on UEFI and ATF for this machine.

It might be somehow possible to run kernel with DT but we do not support it
at this moment. If all you want is to boot kernel directly, it is far
more convenient to use existing virt machine.

niedz., 14 lip 2019, 17:20 użytkownik Guenter Roeck 
napisał:

> Hi,
>
> On Sun, Jun 30, 2019 at 06:20:32PM +0800, Hongbo Zhang wrote:
> > For the Aarch64, there is one machine 'virt', it is primarily meant to
> > run on KVM and execute virtualization workloads, but we need an
> > environment as faithful as possible to physical hardware,  to support
> > firmware and OS development for pysical Aarch64 machines.
> >
>
> I tried to boot linux on this machine with -kernel command line argument,
> but have not been successful. Can someone point me to a working command
> line, one that lets me load the kernel directly ?
>
> Thanks,
> Guenter
>

Re: [Qemu-devel] [PATCH 09/11] paaudio: fix playback glitches

[Zoltán Kővágó]

On 2019-07-10 21:58, Marc-André Lureau wrote:
> On Tue, Jul 9, 2019 at 10:49 PM Kővágó, Zoltán  wrote:
>>
>> Pulseaudio normally assumes that when the server wants it, the client
>> can generate the audio samples and send it right away.  Unfortunately
>> this is not the case with QEMU -- it's up to the emulated system when
>> does it generate the samples.  Buffering the samples and sending them
>> from a background thread is just a workaround, that doesn't work too
>> well.  Instead enable pa's compatibility support and let pa worry about
>> the details.
>>
>> Signed-off-by: Kővágó, Zoltán 
> 
> Could you explain how this is related to PA_STREAM_ADJUST_LATENCY ?

According to the pulseaudio documentation[1], you can't use
PA_STREAM_ADJUST_LATENCY and PA_STREAM_EARLY_REQUESTS at the same time.

[1]:
https://www.freedesktop.org/software/pulseaudio/doxygen/def_8h.html#a6966d809483170bc6d2e6c16188850fca98e436f686fc385697e565eb1ecb2609

>> ---
>>  audio/paaudio.c | 6 ++
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/audio/paaudio.c b/audio/paaudio.c
>> index 9d46f11b0a..d320d2e453 100644
>> --- a/audio/paaudio.c
>> +++ b/audio/paaudio.c
>> @@ -512,10 +512,8 @@ static pa_stream *qpa_simple_new (
>>
>>  flags =
>>  PA_STREAM_INTERPOLATE_TIMING
>> -#ifdef PA_STREAM_ADJUST_LATENCY
>> -| PA_STREAM_ADJUST_LATENCY
>> -#endif
>> -| PA_STREAM_AUTO_TIMING_UPDATE;
>> +| PA_STREAM_AUTO_TIMING_UPDATE
>> +| PA_STREAM_EARLY_REQUESTS;
>>
>>  if (dev) {
>>  /* don't move the stream if the user specified a sink/source */
>> --
>> 2.22.0
>>
>>
> 
> 
> --
> Marc-André Lureau
> 

Re: [Qemu-devel] [PATCH v9 0/2] Add Arm SBSA Reference Machine

[Guenter Roeck]

Hi,

On Sun, Jun 30, 2019 at 06:20:32PM +0800, Hongbo Zhang wrote:
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware,  to support
> firmware and OS development for pysical Aarch64 machines.
> 

I tried to boot linux on this machine with -kernel command line argument,
but have not been successful. Can someone point me to a working command
line, one that lets me load the kernel directly ?

Thanks,
Guenter

Re: [Qemu-devel] [PATCH 0/3] migration: fix migrate_cancel problems of multifd

[Ivan Ren]

The problem still exists in mainline, Ping for review

On Tue, Jun 25, 2019 at 9:18 PM Ivan Ren  wrote:

> The patches fix the problems encountered in multifd migration when try
> to cancel the migration by migrate_cancel.
>
> Ivan Ren (3):
>   migration: fix migrate_cancel leads live_migration thread endless loop
>   migration: fix migrate_cancel leads live_migration thread hung forever
>   migration: fix migrate_cancel multifd migration leads destination hung
> forever
>
>  migration/ram.c | 62 -
>  1 file changed, 51 insertions(+), 11 deletions(-)
>
> --
> 2.17.2 (Apple Git-113)
>
>

Re: [Qemu-devel] [Qemu-block] [PATCH v3] LUKS: support preallocation

[Maxim Levitsky]

On Thu, 2019-07-11 at 18:27 +0200, Stefano Garzarella wrote:
> On Thu, Jul 11, 2019 at 06:09:40PM +0300, Maxim Levitsky wrote:
> > preallocation=off and preallocation=metadata
> > both allocate luks header only, and preallocation=falloc/full
> > is passed to underlying file.
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
> > 
> > Signed-off-by: Maxim Levitsky 
> > 
> > ---
> > 
> > Note that QMP support was only compile tested, since I am still learning
> > on how to use it.
> > 
> > If there is some library/script/etc which makes it more high level,
> > I would more that glad to hear about it. So far I used the qmp-shell
> > 
> > Also can I use qmp's blockdev-create outside a vm running?
> > 
> >  block/crypto.c   | 29 ++---
> >  qapi/block-core.json |  5 -
> >  2 files changed, 30 insertions(+), 4 deletions(-)
> > 
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 8237424ae6..034a645652 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -74,6 +74,7 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> >  struct BlockCryptoCreateData {
> >  BlockBackend *blk;
> >  uint64_t size;
> > +PreallocMode prealloc;
> >  };
> >  
> >  
> > @@ -112,7 +113,7 @@ static ssize_t block_crypto_init_func(QCryptoBlock 
> > *block,
> >   * available to the guest, so we must take account of that
> >   * which will be used by the crypto header
> >   */
> > -return blk_truncate(data->blk, data->size + headerlen, 
> > PREALLOC_MODE_OFF,
> > +return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
> >  errp);
> >  }
> >  
> > @@ -251,6 +252,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat 
> > format,
> >  static int block_crypto_co_create_generic(BlockDriverState *bs,
> >int64_t size,
> >QCryptoBlockCreateOptions *opts,
> > +  PreallocMode prealloc,
> >Error **errp)
> >  {
> >  int ret;
> > @@ -266,9 +268,14 @@ static int 
> > block_crypto_co_create_generic(BlockDriverState *bs,
> >  goto cleanup;
> >  }
> >  
> > +if (prealloc == PREALLOC_MODE_METADATA) {
> > +prealloc = PREALLOC_MODE_OFF;
> > +}
> > +
> >  data = (struct BlockCryptoCreateData) {
> >  .blk = blk,
> >  .size = size,
> > +.prealloc = prealloc,
> >  };
> >  
> >  crypto = qcrypto_block_create(opts, NULL,
> > @@ -500,6 +507,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions 
> > *create_options, Error **errp)
> >  BlockdevCreateOptionsLUKS *luks_opts;
> >  BlockDriverState *bs = NULL;
> >  QCryptoBlockCreateOptions create_opts;
> > +PreallocMode preallocation = PREALLOC_MODE_OFF;
> >  int ret;
> >  
> >  assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
> > @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions 
> > *create_options, Error **errp)
> >  .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
> >  };
> >  
> > +if (luks_opts->has_preallocation)
> > +preallocation = luks_opts->preallocation;
> > +
> >  ret = block_crypto_co_create_generic(bs, luks_opts->size, _opts,
> > - errp);
> > + preallocation, errp);
> >  if (ret < 0) {
> >  goto fail;
> >  }
> > @@ -534,12 +545,24 @@ static int coroutine_fn 
> > block_crypto_co_create_opts_luks(const char *filename,
> >  QCryptoBlockCreateOptions *create_opts = NULL;
> >  BlockDriverState *bs = NULL;
> >  QDict *cryptoopts;
> > +PreallocMode prealloc;
> > +char *buf = NULL;
> >  int64_t size;
> >  int ret;
> > +Error *local_err = NULL;
> >  
> >  /* Parse options */
> >  size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
> >  
> > +buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> > +prealloc = qapi_enum_parse(_lookup, buf,
> > +   PREALLOC_MODE_OFF, _err);
> > +g_free(buf);
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +return -EINVAL;
> > +}
> > +
> >  cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> >   
> > _crypto_create_opts_luks,
> >   true);
> > @@ -565,7 +588,7 @@ static int coroutine_fn 
> > block_crypto_co_create_opts_luks(const char *filename,
> >  }
> >  
> >  /* Create format layer */
> > -ret = block_crypto_co_create_generic(bs, size, create_opts, errp);
> > +ret = block_crypto_co_create_generic(bs, size, create_opts, prealloc, 
> > errp);
> >  if (ret < 0) {
> >  goto fail;
> >  }
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 

[Qemu-devel] [PATCH] migration: always initial RAMBlock.bmap to 1 for new migration

[Ivan Ren]

Reproduce the problem:
migrate
migrate_cancel
migrate

Error happen for memory migration

The reason as follows:
1. qemu start, ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] all set to
   1 by a series of cpu_physical_memory_set_dirty_range
2. migration start:ram_init_bitmaps
   - memory_global_dirty_log_start: begin log diry
   - memory_global_dirty_log_sync: sync dirty bitmap to
 ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]
   - migration_bitmap_sync_range: sync ram_list.
 dirty_memory[DIRTY_MEMORY_MIGRATION] to RAMBlock.bmap
 and ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] is set to zero
3. migration data...
4. migrate_cancel, will stop log dirty
5. migration start:ram_init_bitmaps
   - memory_global_dirty_log_start: begin log diry
   - memory_global_dirty_log_sync: sync dirty bitmap to
 ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]
   - migration_bitmap_sync_range: sync ram_list.
 dirty_memory[DIRTY_MEMORY_MIGRATION] to RAMBlock.bmap
 and ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] is set to zero

   Here RAMBlock.bmap only have new logged dirty pages, don't contain
   the whole guest pages.

Signed-off-by: Ivan Ren 
---
 migration/ram.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 908517fc2b..bbebaee0c1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3173,11 +3173,11 @@ static int ram_state_init(RAMState **rsp)
 QSIMPLEQ_INIT(&(*rsp)->src_page_requests);
 
 /*
+ * Count the total number of pages used by ram blocks not including any
+ * gaps due to alignment or unplugs.
  * This must match with the initial values of dirty bitmap.
- * Currently we initialize the dirty bitmap to all zeros so
- * here the total dirty page count is zero.
  */
-(*rsp)->migration_dirty_pages = 0;
+(*rsp)->migration_dirty_pages = ram_bytes_total() >> TARGET_PAGE_BITS;
 ram_state_reset(*rsp);
 
 return 0;
@@ -3196,12 +3196,13 @@ static void ram_list_init_bitmaps(void)
  * The initial dirty bitmap for migration must be set with all
  * ones to make sure we'll migrate every guest RAM page to
  * destination.
- * Here we didn't set RAMBlock.bmap simply because it is already
- * set in ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] in
- * ram_block_add, and that's where we'll sync the dirty bitmaps.
- * Here setting RAMBlock.bmap would be fine too but not necessary.
+ * Here we set RAMBlock.bmap all to 1 because when rebegin a
+ * new migration after a failed migration, ram_list.
+ * dirty_memory[DIRTY_MEMORY_MIGRATION] don't include the whole
+ * guest memory.
  */
 block->bmap = bitmap_new(pages);
+bitmap_set(block->bmap, 0, pages);
 if (migrate_postcopy_ram()) {
 block->unsentmap = bitmap_new(pages);
 bitmap_set(block->unsentmap, 0, pages);
-- 
2.17.2 (Apple Git-113)

[Qemu-devel] [PATCH v5] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

[Laurent Vivier]

From: Daniel P. Berrangé 

The SIOCGSTAMP symbol was previously defined in the
asm-generic/sockios.h header file. QEMU sees that header
indirectly via sys/socket.h

In linux kernel commit 0768e17073dc527ccd18ed5f96ce85f9985e9115
the asm-generic/sockios.h header no longer defines SIOCGSTAMP.
Instead it provides only SIOCGSTAMP_OLD, which only uses a
32-bit time_t on 32-bit architectures.

The linux/sockios.h header then defines SIOCGSTAMP using
either SIOCGSTAMP_OLD or SIOCGSTAMP_NEW as appropriate. If
SIOCGSTAMP_NEW is used, then the tv_sec field is 64-bit even
on 32-bit architectures

To cope with this we must now convert the old and new type from
the target to the host one.

Signed-off-by: Daniel P. Berrangé 
Signed-off-by: Laurent Vivier 
---

Notes:
v5: [lv] define special _OLD values for sh
define special case for SPARC64
define ioctl helpers
define target__kernel_sock_timeval and target__kernel_timespec and
converters to the host type
always use SIOCGSTAMP and SIOCGSTAMPNS on the host

v4: [lv] timeval64 and timespec64 are { long long , long }

v3: [lv] redefine TARGET_SIOCGSTAMP_NEW, TARGET_SIOCGSTAMPNS_NEW,
timeval64 and timespec64 to use 0x89 type and abi_llong[2]

v2: [dpb] implement _NEW and _OLD variants

 linux-user/ioctls.h|  11 ++-
 linux-user/syscall.c   | 140 +
 linux-user/syscall_defs.h  |  30 +++-
 linux-user/syscall_types.h |   6 --
 4 files changed, 149 insertions(+), 38 deletions(-)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 5e84dc7c3a77..9a4957840ac4 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -222,8 +222,15 @@
   IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
   IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
-  IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
-  IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
+
+  { TARGET_SIOCGSTAMP_OLD, SIOCGSTAMP, "IOCGSTAMP_OLD", IOC_R, \
+do_ioctl_SIOCGSTAMP },
+  { TARGET_SIOCGSTAMPNS_OLD, SIOCGSTAMPNS, "IOCGSTAMPNS_OLD", IOC_R, \
+do_ioctl_SIOCGSTAMPNS },
+  { TARGET_SIOCGSTAMP_NEW, SIOCGSTAMP, "IOCGSTAMP_NEW", IOC_R, \
+do_ioctl_SIOCGSTAMP },
+  { TARGET_SIOCGSTAMPNS_NEW, SIOCGSTAMPNS, "IOCGSTAMPNS_NEW", IOC_R, \
+do_ioctl_SIOCGSTAMPNS },
 
   IOCTL(RNDGETENTCNT, IOC_R, MK_PTR(TYPE_INT))
   IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 39a37496fed5..8367cb138dfe 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1126,8 +1127,9 @@ static inline abi_long copy_from_user_timeval(struct 
timeval *tv,
 {
 struct target_timeval *target_tv;
 
-if (!lock_user_struct(VERIFY_READ, target_tv, target_tv_addr, 1))
+if (!lock_user_struct(VERIFY_READ, target_tv, target_tv_addr, 1)) {
 return -TARGET_EFAULT;
+}
 
 __get_user(tv->tv_sec, _tv->tv_sec);
 __get_user(tv->tv_usec, _tv->tv_usec);
@@ -1142,8 +1144,26 @@ static inline abi_long copy_to_user_timeval(abi_ulong 
target_tv_addr,
 {
 struct target_timeval *target_tv;
 
-if (!lock_user_struct(VERIFY_WRITE, target_tv, target_tv_addr, 0))
+if (!lock_user_struct(VERIFY_WRITE, target_tv, target_tv_addr, 0)) {
+return -TARGET_EFAULT;
+}
+
+__put_user(tv->tv_sec, _tv->tv_sec);
+__put_user(tv->tv_usec, _tv->tv_usec);
+
+unlock_user_struct(target_tv, target_tv_addr, 1);
+
+return 0;
+}
+
+static inline abi_long copy_to_user_timeval64(abi_ulong target_tv_addr,
+ const struct timeval *tv)
+{
+struct target__kernel_sock_timeval *target_tv;
+
+if (!lock_user_struct(VERIFY_WRITE, target_tv, target_tv_addr, 0)) {
 return -TARGET_EFAULT;
+}
 
 __put_user(tv->tv_sec, _tv->tv_sec);
 __put_user(tv->tv_usec, _tv->tv_usec);
@@ -1153,6 +1173,48 @@ static inline abi_long copy_to_user_timeval(abi_ulong 
target_tv_addr,
 return 0;
 }
 
+static inline abi_long target_to_host_timespec(struct timespec *host_ts,
+   abi_ulong target_addr)
+{
+struct target_timespec *target_ts;
+
+if (!lock_user_struct(VERIFY_READ, target_ts, target_addr, 1)) {
+return -TARGET_EFAULT;
+}
+__get_user(host_ts->tv_sec, _ts->tv_sec);
+__get_user(host_ts->tv_nsec, _ts->tv_nsec);
+unlock_user_struct(target_ts, target_addr, 0);
+return 0;
+}
+
+static inline abi_long host_to_target_timespec(abi_ulong target_addr,
+   struct timespec *host_ts)
+{
+struct target_timespec *target_ts;
+
+if (!lock_user_struct(VERIFY_WRITE, target_ts, target_addr, 0)) {
+return -TARGET_EFAULT;
+}
+__put_user(host_ts->tv_sec, 

[Qemu-devel] [PATCH 2/2] linux-user: manage binfmt-misc preserve-arg[0] flag

[Laurent Vivier]

Add --preserve-arg0 in qemu-binfmt-conf.sh to configure the preserve-arg0
flag.

Now, if QEMU is started with -0 or QEMU_ARGV0 and an empty parameter
argv[0] (the full pathname provided by binfmt-misc) is removed and
replaced by argv[1] (the original argv[0] provided by binfmt-misc when
'P'/preserve-arg[0] is set)

For instance:

  $ sudo QEMU_ARGV0= chroot m68k-chroot sh -c 'echo $0'
  sh

without this patch:

  $ sudo chroot m68k-chroot sh -c 'echo $0'
  /usr/bin/sh

Signed-off-by: Laurent Vivier 
---
 linux-user/main.c   | 18 ++-
 scripts/qemu-binfmt-conf.sh | 44 +++--
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index ef8e8cb10eba..eeaba4a7b914 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -647,6 +647,9 @@ int main(int argc, char **argv, char **envp)
 
 init_qemu_uname_release();
 
+/*
+ * Manage binfmt-misc open-binary flag
+ */
 execfd = qemu_getauxval(AT_EXECFD);
 if (execfd == 0) {
 execfd = open(exec_path, O_RDONLY);
@@ -656,6 +659,19 @@ int main(int argc, char **argv, char **envp)
 }
 }
 
+/*
+ * Manage binfmt-misc preserve-arg[0] flag
+ *argv[optind] full path to the binary
+ *argv[optind + 1] original argv[0]
+ */
+if (optind + 1 < argc && argv0 != NULL && argv0[0] == 0) {
+/*
+ * argv0 with an empty string will set argv[optind + 1]
+ * as target_argv[0]
+ */
+optind++;
+}
+
 if (cpu_model == NULL) {
 cpu_model = cpu_get_model(get_elf_eflags(execfd));
 }
@@ -760,7 +776,7 @@ int main(int argc, char **argv, char **envp)
  * argv[0] pointer with the given one.
  */
 i = 0;
-if (argv0 != NULL) {
+if (argv0 != NULL && argv0[0] != 0) {
 target_argv[i++] = strdup(argv0);
 }
 for (; i < target_argc; i++) {
diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index b5a16742a149..7c9a4609c232 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -170,25 +170,27 @@ usage() {
 Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
[--help][--credential yes|no][--exportdir PATH]
[--persistent yes|no][--qemu-suffix SUFFIX]
+   [--preserve-arg0 yes|no]
 
Configure binfmt_misc to use qemu interpreter
 
-   --help:display this usage
-   --qemu-path:   set path to qemu interpreter ($QEMU_PATH)
-   --qemu-suffix: add a suffix to the default interpreter name
-   --debian:  don't write into /proc,
-  instead generate update-binfmts templates
-   --systemd: don't write into /proc,
-  instead generate file for systemd-binfmt.service
-  for the given CPU. If CPU is "ALL", generate a
-  file for all known cpus
-   --exportdir:   define where to write configuration files
-  (default: $SYSTEMDDIR or $DEBIANDIR)
-   --credential:  if yes, credential and security tokens are
-  calculated according to the binary to interpret
-   --persistent:  if yes, the interpreter is loaded when binfmt is
-  configured and remains in memory. All future uses
-  are cloned from the open file.
+   --help:  display this usage
+   --qemu-path: set path to qemu interpreter ($QEMU_PATH)
+   --qemu-suffix:   add a suffix to the default interpreter name
+   --debian:don't write into /proc,
+instead generate update-binfmts templates
+   --systemd:   don't write into /proc,
+instead generate file for systemd-binfmt.service
+for the given CPU. If CPU is "ALL", generate a
+file for all known cpus
+   --exportdir: define where to write configuration files
+(default: $SYSTEMDDIR or $DEBIANDIR)
+   --credential:if yes, credential and security tokens are
+calculated according to the binary to interpret
+   --persistent:if yes, the interpreter is loaded when binfmt is
+configured and remains in memory. All future uses
+are cloned from the open file.
+   --preserve-arg0  preserve arg[0]
 
 To import templates with update-binfmts, use :
 
@@ -261,6 +263,9 @@ qemu_generate_register() {
 if [ "$PERSISTENT" = "yes" ] ; then
 flags="${flags}F"
 fi
+if [ "$PRESERVE_ARG0" = "yes" ] ; then
+flags="${flags}P"
+fi
 
 echo ":qemu-$cpu:M::$magic:$mask:$qemu:$flags"
 }
@@ -322,9 +327,10 @@ DEBIANDIR="/usr/share/binfmts"
 QEMU_PATH=/usr/local/bin
 CREDENTIAL=no
 PERSISTENT=no
+PRESERVE_ARG0=no
 QEMU_SUFFIX=""
 
-options=$(getopt 

[Qemu-devel] [PATCH 1/2] linux-user: remove useless variable

[Laurent Vivier]

filename is only used to open the file if AT_EXECFD is not provided.
But exec_path already contains the path of the file to open.
Remove filename as it is only used in main.c whereas exec_path is
also used in syscall.c.

Fixes: d088d664f201 ("linux-user: identify running binary in /proc/self/exe")
Signed-off-by: Laurent Vivier 
---
 linux-user/main.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index a59ae9439de1..ef8e8cb10eba 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -48,7 +48,6 @@
 char *exec_path;
 
 int singlestep;
-static const char *filename;
 static const char *argv0;
 static int gdbstub_port;
 static envlist_t *envlist;
@@ -580,7 +579,6 @@ static int parse_args(int argc, char **argv)
 exit(EXIT_FAILURE);
 }
 
-filename = argv[optind];
 exec_path = argv[optind];
 
 return optind;
@@ -651,9 +649,9 @@ int main(int argc, char **argv, char **envp)
 
 execfd = qemu_getauxval(AT_EXECFD);
 if (execfd == 0) {
-execfd = open(filename, O_RDONLY);
+execfd = open(exec_path, O_RDONLY);
 if (execfd < 0) {
-printf("Error while loading %s: %s\n", filename, strerror(errno));
+printf("Error while loading %s: %s\n", exec_path, strerror(errno));
 _exit(EXIT_FAILURE);
 }
 }
@@ -778,10 +776,10 @@ int main(int argc, char **argv, char **envp)
 cpu->opaque = ts;
 task_settid(ts);
 
-ret = loader_exec(execfd, filename, target_argv, target_environ, regs,
+ret = loader_exec(execfd, exec_path, target_argv, target_environ, regs,
 info, );
 if (ret != 0) {
-printf("Error while loading %s: %s\n", filename, strerror(-ret));
+printf("Error while loading %s: %s\n", exec_path, strerror(-ret));
 _exit(EXIT_FAILURE);
 }
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

[Laurent Vivier]

Le 14/07/2019 à 13:33, Arnd Bergmann a écrit :
> On Sun, Jul 14, 2019 at 12:41 PM Richard Henderson
>  wrote:
>>
>> On 7/12/19 3:55 PM, Arnd Bergmann wrote:
>>> glibc will have to create a definition that matches the kernel, which uses
>>>
>>> struct __kernel_timespec {
>>> __s64 tv_sec;
>>> __s64 tv_nsec;
>>> };
>>>
>>> As posix requires tv_nsec to be 'long', you need padding between
>>> tv_sec and tv_nsec to have a libc definition matching the kernel's
>>> binary layout.
>>
>> Yes, but that's glibc's lookout.  All qemu cares about emulating is the 
>> kernel
>> interface.  So I think Laurent is right here, in that two reads handle the
>> above structure just fine.
> 
> But that only works if the structure defined by qemu matches the kernel's.
> 
> The structure that Laurent proposed
> 
> struct target_timeval64 {
> abi_llong tv_sec;
> abi_long tv_usec;
> };
> 
> is not compatible with the kernel or the glibc structure.

Yes, you're right. The next patch revision will change this to the
kernel structure one.

Thanks,
Laurent

Re: [Qemu-devel] [BUG] 216 Alerts reported by LGTM for QEMU (some might be release critical)

[Stefan Weil]

Am 13.07.2019 um 21:42 schrieb Paolo Bonzini:
> On 13/07/19 19:46, Stefan Weil wrote:
>> LGTM reports 16 errors, 81 warnings and 119 recommendations:
>> https://lgtm.com/projects/g/qemu/qemu/alerts/?mode=list.
>>
>> Some of them are already known (wrong format strings), others look like
>> real errors:
>>
>> - several multiplication results which don't work as they should in
>> contrib/vhost-user-gpu, block/* (m->nb_clusters * s->cluster_size only
>> 32 bit!),  target/i386/translate.c and other files
> m->nb_clusters here is limited by s->l2_slice_size (see for example
> handle_alloc) so I wouldn't be surprised if this is a false positive.  I
> couldn't find this particular multiplication in Coverity, but it has
> about 250 issues marked as intentional or false positive so there's
> probably a lot of overlap with what LGTM found.
>
> Paolo


>From other projects I know that there is a certain overlap between the
results from Coverity Scan an LGTM, but it is good to have both
analyzers, and the results from LGTM are typically quite reliable.

Even if we know that there is no multiplication overflow, the code could
be modified. Either the assigned value should use the same data type as
the factors (possible when there is never an overflow, avoids a size
extension), or the multiplication could use the larger data type by
adding a type cast to one of the factors (then an overflow cannot
happen, static code analysers and human reviewers have an easier job,
but the multiplication costs more time).

Stefan

[Qemu-devel] [PATCH-for-4.1] hw/lm32/Kconfig: Milkymist One provides a USB 1.1 Controller

[Philippe Mathieu-Daudé]

The Milkymist SoftUSB block provides the OHCI USB standard
(missed in 0858746b835).

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/lm32/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/lm32/Kconfig b/hw/lm32/Kconfig
index 3d09c2dd6f..ed2e3060b0 100644
--- a/hw/lm32/Kconfig
+++ b/hw/lm32/Kconfig
@@ -11,3 +11,4 @@ config MILKYMIST
 select PFLASH_CFI01
 select FRAMEBUFFER
 select SD
+select USB_OHCI
-- 
2.20.1

Re: [Qemu-devel] [PATCH v4] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

[Arnd Bergmann]

On Sun, Jul 14, 2019 at 12:41 PM Richard Henderson
 wrote:
>
> On 7/12/19 3:55 PM, Arnd Bergmann wrote:
> > glibc will have to create a definition that matches the kernel, which uses
> >
> > struct __kernel_timespec {
> > __s64 tv_sec;
> > __s64 tv_nsec;
> > };
> >
> > As posix requires tv_nsec to be 'long', you need padding between
> > tv_sec and tv_nsec to have a libc definition matching the kernel's
> > binary layout.
>
> Yes, but that's glibc's lookout.  All qemu cares about emulating is the kernel
> interface.  So I think Laurent is right here, in that two reads handle the
> above structure just fine.

But that only works if the structure defined by qemu matches the kernel's.

The structure that Laurent proposed

struct target_timeval64 {
abi_llong tv_sec;
abi_long tv_usec;
};

is not compatible with the kernel or the glibc structure.

  Arnd

[Qemu-devel] [PULL for-4.1 6/7] tcg: Remove duplicate #if !defined(CODE_ACCESS)

[Richard Henderson]

This code block is already surrounded by #ifndef CODE_ACCESS.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst_useronly_template.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/exec/cpu_ldst_useronly_template.h 
b/include/exec/cpu_ldst_useronly_template.h
index 8c7a2c6cd7..d663826ac2 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -118,11 +118,9 @@ static inline void
 glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr,
   RES_TYPE v)
 {
-#if !defined(CODE_ACCESS)
 trace_guest_mem_before_exec(
 env_cpu(env), ptr,
 trace_mem_build_info(SHIFT, false, MO_TE, true));
-#endif
 glue(glue(st, SUFFIX), _p)(g2h(ptr), v);
 }
 
-- 
2.17.1

[Qemu-devel] [PULL for-4.1 7/7] tcg: Release mmap_lock on translation fault

[Richard Henderson]

Turn helper_retaddr into a multi-state flag that may now also
indicate when we're performing a read on behalf of the translator.
In this case, release the mmap_lock before the longjmp back to
the main cpu loop, and thereby avoid a failing assert therein.

Fixes: https://bugs.launchpad.net/qemu/+bug/1832353
Tested-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst_useronly_template.h | 20 +--
 accel/tcg/user-exec.c | 66 ---
 2 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/include/exec/cpu_ldst_useronly_template.h 
b/include/exec/cpu_ldst_useronly_template.h
index d663826ac2..2378f2958c 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -64,12 +64,18 @@
 static inline RES_TYPE
 glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)
 {
-#if !defined(CODE_ACCESS)
+#ifdef CODE_ACCESS
+RES_TYPE ret;
+set_helper_retaddr(1);
+ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));
+clear_helper_retaddr();
+return ret;
+#else
 trace_guest_mem_before_exec(
 env_cpu(env), ptr,
 trace_mem_build_info(SHIFT, false, MO_TE, false));
-#endif
 return glue(glue(ld, USUFFIX), _p)(g2h(ptr));
+#endif
 }
 
 #ifndef CODE_ACCESS
@@ -90,12 +96,18 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), 
_ra)(CPUArchState *env,
 static inline int
 glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)
 {
-#if !defined(CODE_ACCESS)
+#ifdef CODE_ACCESS
+int ret;
+set_helper_retaddr(1);
+ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));
+clear_helper_retaddr();
+return ret;
+#else
 trace_guest_mem_before_exec(
 env_cpu(env), ptr,
 trace_mem_build_info(SHIFT, true, MO_TE, false));
-#endif
 return glue(glue(lds, SUFFIX), _p)(g2h(ptr));
+#endif
 }
 
 #ifndef CODE_ACCESS
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4384b59a4d..897d1571c4 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -64,27 +64,56 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t 
*info,
 CPUState *cpu = current_cpu;
 CPUClass *cc;
 unsigned long address = (unsigned long)info->si_addr;
-MMUAccessType access_type;
+MMUAccessType access_type = is_write ? MMU_DATA_STORE : MMU_DATA_LOAD;
 
-/* We must handle PC addresses from two different sources:
- * a call return address and a signal frame address.
- *
- * Within cpu_restore_state_from_tb we assume the former and adjust
- * the address by -GETPC_ADJ so that the address is within the call
- * insn so that addr does not accidentally match the beginning of the
- * next guest insn.
- *
- * However, when the PC comes from the signal frame, it points to
- * the actual faulting host insn and not a call insn.  Subtracting
- * GETPC_ADJ in that case may accidentally match the previous guest insn.
- *
- * So for the later case, adjust forward to compensate for what
- * will be done later by cpu_restore_state_from_tb.
- */
-if (helper_retaddr) {
+switch (helper_retaddr) {
+default:
+/*
+ * Fault during host memory operation within a helper function.
+ * The helper's host return address, saved here, gives us a
+ * pointer into the generated code that will unwind to the
+ * correct guest pc.
+ */
 pc = helper_retaddr;
-} else {
+break;
+
+case 0:
+/*
+ * Fault during host memory operation within generated code.
+ * (Or, a unrelated bug within qemu, but we can't tell from here).
+ *
+ * We take the host pc from the signal frame.  However, we cannot
+ * use that value directly.  Within cpu_restore_state_from_tb, we
+ * assume PC comes from GETPC(), as used by the helper functions,
+ * so we adjust the address by -GETPC_ADJ to form an address that
+ * is within the call insn, so that the address does not accidentially
+ * match the beginning of the next guest insn.  However, when the
+ * pc comes from the signal frame it points to the actual faulting
+ * host memory insn and not the return from a call insn.
+ *
+ * Therefore, adjust to compensate for what will be done later
+ * by cpu_restore_state_from_tb.
+ */
 pc += GETPC_ADJ;
+break;
+
+case 1:
+/*
+ * Fault during host read for translation, or loosely, "execution".
+ *
+ * The guest pc is already pointing to the start of the TB for which
+ * code is being generated.  If the guest translator manages the
+ * page crossings correctly, this is exactly the correct address
+ * (and if the translator doesn't handle page boundaries correctly
+ * there's little we can do about that here).  Therefore, do not
+  

[Qemu-devel] [PULL for-4.1 4/7] tcg: Introduce set/clear_helper_retaddr

[Richard Henderson]

At present we have a potential error in that helper_retaddr contains
data for handle_cpu_signal, but we have not ensured that those stores
will be scheduled properly before the operation that may fault.

It might be that these races are not in practice observable, due to
our use of -fno-strict-aliasing, but better safe than sorry.

Adjust all of the setters of helper_retaddr.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst.h   | 20 +++
 include/exec/cpu_ldst_useronly_template.h | 12 +++
 accel/tcg/user-exec.c | 11 +++---
 target/arm/helper-a64.c   |  8 ++---
 target/arm/sve_helper.c   | 43 +++
 5 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index a08b11bd2c..9de8c93303 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -89,6 +89,26 @@ typedef target_ulong abi_ptr;
 
 extern __thread uintptr_t helper_retaddr;
 
+static inline void set_helper_retaddr(uintptr_t ra)
+{
+helper_retaddr = ra;
+/*
+ * Ensure that this write is visible to the SIGSEGV handler that
+ * may be invoked due to a subsequent invalid memory operation.
+ */
+signal_barrier();
+}
+
+static inline void clear_helper_retaddr(void)
+{
+/*
+ * Ensure that previous memory operations have succeeded before
+ * removing the data visible to the signal handler.
+ */
+signal_barrier();
+helper_retaddr = 0;
+}
+
 /* In user-only mode we provide only the _code and _data accessors. */
 
 #define MEMSUFFIX _data
diff --git a/include/exec/cpu_ldst_useronly_template.h 
b/include/exec/cpu_ldst_useronly_template.h
index bc45e2b8d4..e65733f7e2 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -78,9 +78,9 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), 
_ra)(CPUArchState *env,
   uintptr_t retaddr)
 {
 RES_TYPE ret;
-helper_retaddr = retaddr;
+set_helper_retaddr(retaddr);
 ret = glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(env, ptr);
-helper_retaddr = 0;
+clear_helper_retaddr();
 return ret;
 }
 
@@ -102,9 +102,9 @@ glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), 
_ra)(CPUArchState *env,
   uintptr_t retaddr)
 {
 int ret;
-helper_retaddr = retaddr;
+set_helper_retaddr(retaddr);
 ret = glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(env, ptr);
-helper_retaddr = 0;
+clear_helper_retaddr();
 return ret;
 }
 #endif
@@ -128,9 +128,9 @@ glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), 
_ra)(CPUArchState *env,
   RES_TYPE v,
   uintptr_t retaddr)
 {
-helper_retaddr = retaddr;
+set_helper_retaddr(retaddr);
 glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(env, ptr, v);
-helper_retaddr = 0;
+clear_helper_retaddr();
 }
 #endif
 
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index cb5f4b19c5..4384b59a4d 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -134,7 +134,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t 
*info,
  * currently executing TB was modified and must be exited
  * immediately.  Clear helper_retaddr for next execution.
  */
-helper_retaddr = 0;
+clear_helper_retaddr();
 cpu_exit_tb_from_sighandler(cpu, old_set);
 /* NORETURN */
 
@@ -152,7 +152,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t 
*info,
  * an exception.  Undo signal and retaddr state prior to longjmp.
  */
 sigprocmask(SIG_SETMASK, old_set, NULL);
-helper_retaddr = 0;
+clear_helper_retaddr();
 
 cc = CPU_GET_CLASS(cpu);
 access_type = is_write ? MMU_DATA_STORE : MMU_DATA_LOAD;
@@ -682,14 +682,15 @@ static void *atomic_mmu_lookup(CPUArchState *env, 
target_ulong addr,
 if (unlikely(addr & (size - 1))) {
 cpu_loop_exit_atomic(env_cpu(env), retaddr);
 }
-helper_retaddr = retaddr;
-return g2h(addr);
+void *ret = g2h(addr);
+set_helper_retaddr(retaddr);
+return ret;
 }
 
 /* Macro to call the above, with local variables from the use context.  */
 #define ATOMIC_MMU_DECLS do {} while (0)
 #define ATOMIC_MMU_LOOKUP  atomic_mmu_lookup(env, addr, DATA_SIZE, GETPC())
-#define ATOMIC_MMU_CLEANUP do { helper_retaddr = 0; } while (0)
+#define ATOMIC_MMU_CLEANUP do { clear_helper_retaddr(); } while (0)
 
 #define ATOMIC_NAME(X)   HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
 #define EXTRA_ARGS
diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index 44e45a8037..060699b901 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -554,7 +554,7 @@ uint64_t HELPER(paired_cmpxchg64_le)(CPUARMState *env, 
uint64_t addr,
 

[Qemu-devel] [PULL for-4.1 2/7] tcg/aarch64: Fix output of extract2 opcodes

[Richard Henderson]

This patch fixes two problems:
(1) The inputs to the EXTR insn were reversed,
(2) The input constraints use rZ, which means that we need to use
the REG0 macro in order to supply XZR for a constant 0 input.

Fixes: 464c2969d5d
Reported-by: Peter Maydell 
Tested-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index b0f8106642..0713448bf5 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -2226,7 +2226,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 
 case INDEX_op_extract2_i64:
 case INDEX_op_extract2_i32:
-tcg_out_extr(s, ext, a0, a1, a2, args[3]);
+tcg_out_extr(s, ext, a0, REG0(2), REG0(1), args[3]);
 break;
 
 case INDEX_op_add2_i32:
-- 
2.17.1

[Qemu-devel] [PULL for-4.1 5/7] tcg: Remove cpu_ld*_code_ra

[Richard Henderson]

These functions are not used, and are not usable in the
context of code generation, because we never have a helper
return address to pass in to them.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst_useronly_template.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu_ldst_useronly_template.h 
b/include/exec/cpu_ldst_useronly_template.h
index e65733f7e2..8c7a2c6cd7 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -72,6 +72,7 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, 
abi_ptr ptr)
 return glue(glue(ld, USUFFIX), _p)(g2h(ptr));
 }
 
+#ifndef CODE_ACCESS
 static inline RES_TYPE
 glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
   abi_ptr ptr,
@@ -83,6 +84,7 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), 
_ra)(CPUArchState *env,
 clear_helper_retaddr();
 return ret;
 }
+#endif
 
 #if DATA_SIZE <= 2
 static inline int
@@ -96,6 +98,7 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, 
abi_ptr ptr)
 return glue(glue(lds, SUFFIX), _p)(g2h(ptr));
 }
 
+#ifndef CODE_ACCESS
 static inline int
 glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
   abi_ptr ptr,
@@ -107,7 +110,8 @@ glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), 
_ra)(CPUArchState *env,
 clear_helper_retaddr();
 return ret;
 }
-#endif
+#endif /* CODE_ACCESS */
+#endif /* DATA_SIZE <= 2 */
 
 #ifndef CODE_ACCESS
 static inline void
-- 
2.17.1

[Qemu-devel] [PULL for-4.1 3/7] include/qemu/atomic.h: Add signal_barrier

[Richard Henderson]

We have some potential race conditions vs our user-exec signal
handler that will be solved with this barrier.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/qemu/atomic.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index a6ac188188..f9cd24c899 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -88,6 +88,13 @@
 #define smp_read_barrier_depends()   barrier()
 #endif
 
+/*
+ * A signal barrier forces all pending local memory ops to be observed before
+ * a SIGSEGV is delivered to the *same* thread.  In practice this is exactly
+ * the same as barrier(), but since we have the correct builtin, use it.
+ */
+#define signal_barrier()__atomic_signal_fence(__ATOMIC_SEQ_CST)
+
 /* Sanity check that the size of an atomic operation isn't "overly large".
  * Despite the fact that e.g. i686 has 64-bit atomic operations, we do not
  * want to use them because we ought not need them, and this lets us do a
@@ -308,6 +315,10 @@
 #define smp_read_barrier_depends()   barrier()
 #endif
 
+#ifndef signal_barrier
+#define signal_barrier()barrier()
+#endif
+
 /* These will only be atomic if the processor does the fetch or store
  * in a single issue memory operation
  */
-- 
2.17.1

[Qemu-devel] [PULL for-4.1 1/7] tcg: Fix constant folding of INDEX_op_extract2_i32

[Richard Henderson]

On a 64-bit host, discard any replications of the 32-bit
sign bit when performing the shift and merge.

Fixes: https://bugs.launchpad.net/bugs/1834496
Tested-by: Christophe Lyon 
Tested-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 tcg/optimize.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index d7c71a6085..d2424de4af 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -1213,8 +1213,8 @@ void tcg_optimize(TCGContext *s)
 if (opc == INDEX_op_extract2_i64) {
 tmp = (v1 >> op->args[3]) | (v2 << (64 - op->args[3]));
 } else {
-tmp = (v1 >> op->args[3]) | (v2 << (32 - op->args[3]));
-tmp = (int32_t)tmp;
+tmp = (int32_t)(((uint32_t)v1 >> op->args[3]) |
+((uint32_t)v2 << (32 - op->args[3])));
 }
 tcg_opt_gen_movi(s, op, op->args[0], tmp);
 break;
-- 
2.17.1

[Qemu-devel] [PULL for-4.1 0/7] tcg patch queue

[Richard Henderson]

The following changes since commit 1316b1ddc8a05e418c8134243f8bff8cccbbccb1:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2019-07-12 15:38:22 +0100)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-tcg-20190714

for you to fetch changes up to 52ba13f042714c4086416973fb88e2465e0888a1:

  tcg: Release mmap_lock on translation fault (2019-07-14 12:19:01 +0200)


Fixes for 3 tcg bugs


Richard Henderson (7):
  tcg: Fix constant folding of INDEX_op_extract2_i32
  tcg/aarch64: Fix output of extract2 opcodes
  include/qemu/atomic.h: Add signal_barrier
  tcg: Introduce set/clear_helper_retaddr
  tcg: Remove cpu_ld*_code_ra
  tcg: Remove duplicate #if !defined(CODE_ACCESS)
  tcg: Release mmap_lock on translation fault

 include/exec/cpu_ldst.h   | 20 
 include/exec/cpu_ldst_useronly_template.h | 40 ++--
 include/qemu/atomic.h | 11 +
 accel/tcg/user-exec.c | 77 +--
 target/arm/helper-a64.c   |  8 ++--
 target/arm/sve_helper.c   | 43 +
 tcg/aarch64/tcg-target.inc.c  |  2 +-
 tcg/optimize.c|  4 +-
 8 files changed, 139 insertions(+), 66 deletions(-)

Re: [Qemu-devel] [PATCH-for-4.1? 0/7] vl: Allow building with CONFIG_BLUETOOTH disabled

[Richard Henderson]

On 7/12/19 3:39 PM, Philippe Mathieu-Daudé wrote:
> A series of obvious patches to build without the deprecated
> bluetooth devices. Still worth for 4.1 or too late?
> It is clearly not a bugfix.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (7):
>   hw/arm: Nokia N-series tablet requires Bluetooth
>   hw/usb: Bluetooth HCI USB depends on USB & BLUETOOTH
>   MAINTAINERS: Add a Bluetooth entry
>   vl: Fix 'braces' coding style issues
>   vl: Use qemu_strtoi() instead of strtol()
>   vl: Extract bt_parse() into its own file
>   hw/bt: Allow building with CONFIG_BLUETOOTH disabled

All of this looks plausible to me to go along with the 4.1 deprecation.

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v4] linux-user: fix to handle variably sized SIOCGSTAMP with new kernels

[Richard Henderson]

On 7/12/19 3:55 PM, Arnd Bergmann wrote:
> glibc will have to create a definition that matches the kernel, which uses
> 
> struct __kernel_timespec {
> __s64 tv_sec;
> __s64 tv_nsec;
> };
> 
> As posix requires tv_nsec to be 'long', you need padding between
> tv_sec and tv_nsec to have a libc definition matching the kernel's
> binary layout.

Yes, but that's glibc's lookout.  All qemu cares about emulating is the kernel
interface.  So I think Laurent is right here, in that two reads handle the
above structure just fine.


r~