Re: [Qemu-devel] [for-4.2 PATCH 0/6] Block-related record/replay fixes

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/156438176555.22071.10523120047318890136.stgit@pasha-Precision-3630-Tower/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  ui/x_keymap.o
  CC  ui/gtk.o
  CC  ui/curses.o
/tmp/qemu-test/src/replay/replay-events.c:141:23: error: implicit declaration 
of function 'replay_get_current_icount' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]
uint64_t id = replay_get_current_icount();
  ^
/tmp/qemu-test/src/replay/replay-events.c:141:23: note: did you mean 
'replay_get_current_step'?
/tmp/qemu-test/src/include/sysemu/replay.h:78:10: note: 
'replay_get_current_step' declared here
uint64_t replay_get_current_step(void);
 ^
/tmp/qemu-test/src/replay/replay-events.c:141:23: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
uint64_t id = replay_get_current_icount();
  ^
2 errors generated.


The full log is available at
http://patchew.org/logs/156438176555.22071.10523120047318890136.stgit@pasha-Precision-3630-Tower/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH 2/3] memory-device: break the loop if no hint is provided

[Igor Mammedov]

On Sun, 28 Jul 2019 21:13:03 +0800
Wei Yang  wrote:

> When there is no hint, the first un-overlapped range is the proper one.
> Just break the loop instead of iterate the whole list.
could it change default pc-dimm mapping (will address assignment stay
the same as before this change)?

In commit message I'd suggest to replace 'the proper one' with more
verbose explanation why it is safe to break earlier.

otherwise patch look good to me

> 
> Signed-off-by: Wei Yang 
> ---
>  hw/mem/memory-device.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index df3261b32a..413b514586 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState 
> *ms,
>  range_make_empty(&new);
>  break;
>  }
> +} else if (!hint) {
> +break;
>  }
>  }
>  

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Markus Armbruster]

Peter Maydell  writes:

> On Sat, 27 Jul 2019 at 13:24, Paolo Bonzini  wrote:
>>
>> On 27/07/19 09:16, Markus Armbruster wrote:
>> > We started with a single trace-events.  That wasn't good, so we split it
>> > up into one per directory.  That isn't good, so what about splitting it
>> > up into one per source file?  Pass -DTRACE_HEADER='"trace-DIR-FOO.h"
>> > instead of -DTRACE_HEADER='"trace-DIR.h"' when compiling DIR/FOO.c.
>>
>> For Make this would all work great, however not for Meson because it
>> doesn't allow per-file compile flags.
>
> Apologies for randomly parachuting into this email thread, but if
> Meson doesn't support per-file compile flags then what's the plan
> for handling the cases where we currently need per-file compile flags ?
> (This is one of the things that I thought was quite a nice move
> forward in our make infrastructure -- we now have clean syntax
> for saying "these files need to be built with these warnings disabled
> or these extra include paths or whatever" and also "these files
> imply we're going to need to link against library X".)

I vaguely remember from my review that Meson lets us express "these
files imply we're going to need to link against library X" even more
clearly.  I can't point you to an example, though.

Aside: I can't apply this series anymore.  I tried master, failed, tried
a merge commit shortly before its posting date, failed, gave up.  Can I
git-pull it from somewhere?

Losing the ability to add compiler flags per file with minimal fuss
would be regrettable.  How regrettable?  I append results of a quick
grep.

>> Meson maintainers suggest building a static library for each special set
>> of compile flags; we could do that automatically per-directory(*) but it
>> would be harder to scale that to per-file.

This is clearly not "minimal fuss", not even per directory, let alone
per file.

It's pretty lame even for the large sets we have (per target,
target-independent): I guess we'd either throw away the .a unused, or
link with --wholearchive.

The grep below shows multiple instances of per-file.

We should explore how to handle typical instances before we commit to
the conversion project.

>> (*) Still, I'd rather go on with the forwarding headers and look into
>> that later, to ease review.

For me, forwarding headers are just fine for a PoC, quite tolerable
while a conversion is in progress, and perhaps even tolerable as a
permanent work-around.  My *actual* question is how we could do per-file
rather than per-directory trace.h with Meson.  Quoting myself:

We have trace-events with hundreds of tracepoints for tens of source
files.  The generated trace.h clock in at hundreds of KiB for me.
Messing with tracepoints in one source file recompiles most of the
directory.  This is so much better than it used to be, but clearly not
as good as it could be.

The worst of the lot is trace-root.h, which gets included for >1300 .o
in my "build everything" tree, mostly because it contains tracepoints
for static inline functions in widely included headers.  See also
"[PATCH 07/28] trace: Do not include qom/cpu.h into generated trace.h",
Message-Id: <20190726120542.9894-8-arm...@redhat.com>.

We started with a single trace-events.  That wasn't good, so we split it
up into one per directory.  That isn't good, so what about splitting it
up into one per source file?

Any ideas?


$ git-grep -F o-cflags
Makefile.objs:bt-host.o-cflags := $(BLUEZ_CFLAGS)
Makefile.objs:vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS)
Makefile.objs:qemu-seccomp.o-cflags := $(SECCOMP_CFLAGS)
Makefile.objs:vhost-user-scsi.o-cflags := $(LIBISCSI_CFLAGS)
audio/Makefile.objs:sdl.mo-cflags := $(SDL_CFLAGS)
block/Makefile.objs:iscsi.o-cflags := $(LIBISCSI_CFLAGS)
block/Makefile.objs:curl.o-cflags  := $(CURL_CFLAGS)
block/Makefile.objs:rbd.o-cflags   := $(RBD_CFLAGS)
block/Makefile.objs:gluster.o-cflags   := $(GLUSTERFS_CFLAGS)
block/Makefile.objs:ssh.o-cflags   := $(LIBSSH_CFLAGS)
block/Makefile.objs:parallels.o-cflags := $(LIBXML2_CFLAGS)
chardev/Makefile.objs:baum.o-cflags := $(SDL_CFLAGS)
contrib/vhost-user-gpu/Makefile.objs:main.o-cflags := $(PIXMAN_CFLAGS) 
$(GBM_CFLAGS)
contrib/vhost-user-gpu/Makefile.objs:virgl.o-cflags := $(VIRGL_CFLAGS) 
$(GBM_CFLAGS)
contrib/vhost-user-gpu/Makefile.objs:vugbm.o-cflags := $(GBM_CFLAGS)
disas/Makefile.objs:arm-a64.o-cflags := -I$(libvixldir) -Wno-sign-compare
docs/devel/build-system.txt:  curl.o-cflags  := $(CURL_CFLAGS)
docs/devel/build-system.txt:  iscsi.o-cflags := $(LIBISCSI_CFLAGS)
docs/devel/build-system.txt:  curl.o-cflags  := $(CURL_CFLAGS)
hw/display/Makefile.objs:milkymist-tmu2.o-cflags := $(X11_CFLAGS) 
$(OPENGL_CFLAGS)
hw/display/Makefile.objs:virtio-gpu.o-cflags := $(VIRGL_CFLAGS)
hw/display/Makefile.objs:virtio-gpu-3d.o-cflags := $(VIRGL_CFLAGS)
hw/usb/Makefile.objs:smartcard.mo-cflags := $(SMARTCARD_CFLAGS)
hw/usb/Makefile.objs:redirect.o-cflags = $(USB_REDIR_CFLAGS)

Re: [Qemu-devel] [PATCH 1/3] memory-device: not necessary to use goto for the last check

[David Hildenbrand]

On 28.07.19 15:13, Wei Yang wrote:
> We are already at the last condition check.
> 
> Signed-off-by: Wei Yang 
> ---
>  hw/mem/memory-device.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index 5f2c408036..df3261b32a 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -186,7 +186,6 @@ static uint64_t memory_device_get_free_addr(MachineState 
> *ms,
>  if (!range_contains_range(&as, &new)) {
>  error_setg(errp, "could not find position in guest address space for 
> "
> "memory device - memory fragmented due to alignments");
> -goto out;
>  }
>  out:
>  g_slist_free(list);
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 09/17] ppc/xive: Extend XiveTCTX with a XiveRouter pointer

[Cédric Le Goater]

On 29/07/2019 08:11, David Gibson wrote:
> On Sun, Jul 28, 2019 at 11:06:27AM +0200, Cédric Le Goater wrote:
>> On 28/07/2019 09:46, David Gibson wrote:
>>> On Thu, Jul 18, 2019 at 01:54:12PM +0200, Cédric Le Goater wrote:
 This is to perform lookups in the NVT table when a vCPU is dispatched
 and possibily resend interrupts.

 Future XIVE chip will use a different class for the model of the
 interrupt controller and we might need to change the type of
 'XiveRouter *' to 'Object *'

 Signed-off-by: Cédric Le Goater 
>>>
>>> Hrm.  This still bothers me. 
>>
>> Your feeling is right. There should be a good reason to link two objects 
>> together as it can be an issue later on (such P10). It should not be an 
>> hidden parameter to function calls. this is more or less the case. 
>>  
>> See below for more explanation.
>>
>>> AIUI there can be multiple XiveRouters in the system, yes?  
>>
>> yes and it works relatively well with 4 chips. I say relatively because 
>> the presenter model is taking a shortcut we should fix. 
>>
>>> And at least theoretically can present irqs from multiple routers? 
>>
>> Yes. the console being the most simple example. We only have one device 
>> per system on the LPC bus of chip 0. 
>>  
>>> In which case what's the rule for which one should be associated with 
>>> a specific.
>>> I guess it's the one on the same chip, but that needs to be explained
>>> up front, with some justification of why that's the relevant one.
>>
>> Yes. we try to minimize the traffic on the PowerBUS so generally CPU 
>> targets are on the same IC. The EAT on POWER10 should be on the same
>> HW chip.
>>
>>
>> I think we can address the proposed changes from another perspective, 
>> from the presenter one. this is cleaner and reflects better the HW design. 
>>
>> The thread contexts are owned by the presenter. It can scan its list 
>> when doing CAM matching and when the thread context registers are being 
>> accessed by a CPU. Adding a XiveRouter parameter to all the TIMA 
>> operations seems like a better option and solves the problem.
>>  
>>
>> The thread context registers are modeled under the CPU object today 
>> because it was practical for sPAPR but on HW, these are SRAM entries,
>> one for each HW thread of the chip. So may be, we should have introduced
>> an other table under the XiveRouter to model the contexts but there
>> was no real need for the XIVE POWER9 IC of the pseries machine. This 
>> design might be reaching its limits with PowerNV and POWER10.  
>>
>>
>> Looking at :
>>  
>>   [PATCH v2 15/17] ppc/pnv: Grab the XiveRouter object from XiveTCTX in 
>> pnv_xive_get_tctx()
>>
>> we see that the code adds an extra check on the THREAD_ENABLE registers 
>> and for that, its needs the IC to which belongs the thread context. This 
>> code is wrong. We should not be need to find a XiveRouter object from a 
>> XiveRouter handler.
>>
>> This is because the xive_presenter_match() routine does:
>>
>> CPU_FOREACH(cs) {
>> XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
>>  
>> we should be, instead, looping on the different IC of the system 
>> looking for a match. Something else to fix. I think I will use the
>> PIR to match the CPU of a chip.
>>
>>
>> Looking at POWER10, XIVE internal structures have changed and we will
>> need to introduce new IC models, one for PowerNV obviously but surely 
>> also one for pseries. A third one ... yes, sorry about that. If we go 
>> in that direction, it would be good to have a common XiveTCTX and not 
>> link it to a specific XiveRouter (P9 or P10). Another good reason not
>> to use that link.
>>
>>
>> So I will rework the end of that patchset. Thanks for having given me 
>> time to think more about it before merging. I did more experiments and
>> the models are now more precise, specially with guest and multichip
>> support.
> 
> Ok, good to hear.  I will wait for the respin.
> 

Could you check the patch adding the powernv{8,9} machines ?  I am 
rebasing the respin on top of this patch and introducing a new handler 
to the machine to perform the CAM line matching.

Thanks,

C. 

Re: [Qemu-devel] [PATCH 2/3] memory-device: break the loop if no hint is provided

[David Hildenbrand]

On 28.07.19 15:13, Wei Yang wrote:
> When there is no hint, the first un-overlapped range is the proper one.
> Just break the loop instead of iterate the whole list.
> 
> Signed-off-by: Wei Yang 
> ---
>  hw/mem/memory-device.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index df3261b32a..413b514586 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState 
> *ms,
>  range_make_empty(&new);
>  break;
>  }
> +} else if (!hint) {
> +break;
>  }
>  }
>  
> 

I think

a) This is fine. I was not able to construct a counter-example where
this would not work. Whenever we modify the range, we check against the
next one in the sorted list. If there is no overlap, it fits. And, it
won't overlap with any other range (and therefore never be changed again)

b) This should therefore not change the assignment order / break migration.

Maybe mention that this will not change the assigned addresses compared
to old code in all scenarios.

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 3/3] memory-device: break the loop if tmp exceed the hinted range

[David Hildenbrand]

On 28.07.19 15:13, Wei Yang wrote:
> The memory-device list built by memory_device_build_list is ordered by
> its address, this means if the tmp range exceed the hinted range, all
> the following range will not overlap with it.
> 
> Signed-off-by: Wei Yang 
> ---
>  hw/mem/memory-device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index 413b514586..aea47ab3e8 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -180,7 +180,7 @@ static uint64_t memory_device_get_free_addr(MachineState 
> *ms,
>  range_make_empty(&new);
>  break;
>  }
> -} else if (!hint) {
> +} else if (!hint || range_lob(&tmp) > range_upb(&new)) {
>  break;
>  }
>  }
> 

Lower bound is inclusive, upper bound is exclusive. Shouldn't this be

range_lob(&tmp) >= range_upb(&new)

Also, I wonder if patch #2 is now really needed?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] memory-device: break the loop if no hint is provided

[Wei Yang]

On Mon, Jul 29, 2019 at 09:04:01AM +0200, Igor Mammedov wrote:
>On Sun, 28 Jul 2019 21:13:03 +0800
>Wei Yang  wrote:
>
>> When there is no hint, the first un-overlapped range is the proper one.
>> Just break the loop instead of iterate the whole list.
>could it change default pc-dimm mapping (will address assignment stay
>the same as before this change)?

Yes, it stays the same as before.

>
>In commit message I'd suggest to replace 'the proper one' with more
>verbose explanation why it is safe to break earlier.
>

ok, let me re-phrase it. Thanks

>otherwise patch look good to me
>
>> 
>> Signed-off-by: Wei Yang 
>> ---
>>  hw/mem/memory-device.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index df3261b32a..413b514586 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState 
>> *ms,
>>  range_make_empty(&new);
>>  break;
>>  }
>> +} else if (!hint) {
>> +break;
>>  }
>>  }
>>  

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 2/3] memory-device: break the loop if no hint is provided

[Wei Yang]

On Mon, Jul 29, 2019 at 09:45:24AM +0200, David Hildenbrand wrote:
>On 28.07.19 15:13, Wei Yang wrote:
>> When there is no hint, the first un-overlapped range is the proper one.
>> Just break the loop instead of iterate the whole list.
>> 
>> Signed-off-by: Wei Yang 
>> ---
>>  hw/mem/memory-device.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index df3261b32a..413b514586 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState 
>> *ms,
>>  range_make_empty(&new);
>>  break;
>>  }
>> +} else if (!hint) {
>> +break;
>>  }
>>  }
>>  
>> 
>
>I think
>
>a) This is fine. I was not able to construct a counter-example where
>this would not work. Whenever we modify the range, we check against the
>next one in the sorted list. If there is no overlap, it fits. And, it
>won't overlap with any other range (and therefore never be changed again)
>
>b) This should therefore not change the assignment order / break migration.
>
>Maybe mention that this will not change the assigned addresses compared
>to old code in all scenarios.
>

Thanks, let me add this in change log.

>Reviewed-by: David Hildenbrand 
>
>-- 
>
>Thanks,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 3/3] memory-device: break the loop if tmp exceed the hinted range

[David Hildenbrand]

On 29.07.19 09:49, David Hildenbrand wrote:
> On 28.07.19 15:13, Wei Yang wrote:
>> The memory-device list built by memory_device_build_list is ordered by
>> its address, this means if the tmp range exceed the hinted range, all
>> the following range will not overlap with it.
>>
>> Signed-off-by: Wei Yang 
>> ---
>>  hw/mem/memory-device.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index 413b514586..aea47ab3e8 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -180,7 +180,7 @@ static uint64_t memory_device_get_free_addr(MachineState 
>> *ms,
>>  range_make_empty(&new);
>>  break;
>>  }
>> -} else if (!hint) {
>> +} else if (!hint || range_lob(&tmp) > range_upb(&new)) {
>>  break;
>>  }
>>  }
>>
> 
> Lower bound is inclusive, upper bound is exclusive. Shouldn't this be
> 
> range_lob(&tmp) >= range_upb(&new)

Confused by the description of range_set_bounds1().

Both bounds are inclusive and this is correct.

> 
> Also, I wonder if patch #2 is now really needed?
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Pino Toscano]

On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
> On 7/26/19 9:09 AM, Pino Toscano wrote:
> > Add a 'private-key' option which represents the path of a private key
> > to use for authentication, and 'private-key-secret' as the name of an
> > object with its passphrase.
> > 
> > Signed-off-by: Pino Toscano 
> 
> > +++ b/qapi/block-core.json
> > @@ -3226,6 +3226,11 @@
> >  # @password-secret: ID of a QCryptoSecret object providing a password
> >  #   for authentication (since 4.2)
> >  #
> > +# @private-key: path to the private key (since 4.2)
> > +#
> > +# @private-key-secret:  ID of a QCryptoSecret object providing the 
> > passphrase
> > +#   for 'private-key' (since 4.2)
> 
> Is password-secret intended to be mutually-exclusive with
> private-key/private-key-secret?

My initial thought was to allow users to specify data for all the
authentication methods possible.  Either ways (all of them, or a single
one) are fine for me.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.

[Qemu-devel] [PATCH v2 2/3] migration: add speed limit for multifd migration

[Ivan Ren]

Limit the speed of multifd migration through common speed limitation
qemu file.

Signed-off-by: Ivan Ren 
---
 migration/ram.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 889148dd84..88ddd2bbe2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -922,7 +922,7 @@ struct {
  * false.
  */
 
-static int multifd_send_pages(void)
+static int multifd_send_pages(RAMState *rs)
 {
 int i;
 static int next_channel;
@@ -954,6 +954,7 @@ static int multifd_send_pages(void)
 multifd_send_state->pages = p->pages;
 p->pages = pages;
 transferred = ((uint64_t) pages->used) * TARGET_PAGE_SIZE + p->packet_len;
+qemu_file_update_transfer(rs->f, transferred);
 ram_counters.multifd_bytes += transferred;
 ram_counters.transferred += transferred;;
 qemu_mutex_unlock(&p->mutex);
@@ -962,7 +963,7 @@ static int multifd_send_pages(void)
 return 1;
 }
 
-static int multifd_queue_page(RAMBlock *block, ram_addr_t offset)
+static int multifd_queue_page(RAMState *rs, RAMBlock *block, ram_addr_t offset)
 {
 MultiFDPages_t *pages = multifd_send_state->pages;
 
@@ -981,12 +982,12 @@ static int multifd_queue_page(RAMBlock *block, ram_addr_t 
offset)
 }
 }
 
-if (multifd_send_pages() < 0) {
+if (multifd_send_pages(rs) < 0) {
 return -1;
 }
 
 if (pages->block != block) {
-return  multifd_queue_page(block, offset);
+return  multifd_queue_page(rs, block, offset);
 }
 
 return 1;
@@ -1054,7 +1055,7 @@ void multifd_save_cleanup(void)
 multifd_send_state = NULL;
 }
 
-static void multifd_send_sync_main(void)
+static void multifd_send_sync_main(RAMState *rs)
 {
 int i;
 
@@ -1062,7 +1063,7 @@ static void multifd_send_sync_main(void)
 return;
 }
 if (multifd_send_state->pages->used) {
-if (multifd_send_pages() < 0) {
+if (multifd_send_pages(rs) < 0) {
 error_report("%s: multifd_send_pages fail", __func__);
 return;
 }
@@ -1083,6 +1084,7 @@ static void multifd_send_sync_main(void)
 p->packet_num = multifd_send_state->packet_num++;
 p->flags |= MULTIFD_FLAG_SYNC;
 p->pending_job++;
+qemu_file_update_transfer(rs->f, p->packet_len);
 qemu_mutex_unlock(&p->mutex);
 qemu_sem_post(&p->sem);
 }
@@ -2079,7 +2081,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus 
*pss, bool last_stage)
 static int ram_save_multifd_page(RAMState *rs, RAMBlock *block,
  ram_addr_t offset)
 {
-if (multifd_queue_page(block, offset) < 0) {
+if (multifd_queue_page(rs, block, offset) < 0) {
 return -1;
 }
 ram_counters.normal++;
@@ -3482,7 +3484,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 ram_control_before_iterate(f, RAM_CONTROL_SETUP);
 ram_control_after_iterate(f, RAM_CONTROL_SETUP);
 
-multifd_send_sync_main();
+multifd_send_sync_main(*rsp);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
 
@@ -3570,7 +3572,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 ram_control_after_iterate(f, RAM_CONTROL_ROUND);
 
 out:
-multifd_send_sync_main();
+multifd_send_sync_main(rs);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
 ram_counters.transferred += 8;
@@ -3629,7 +3631,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
 
 rcu_read_unlock();
 
-multifd_send_sync_main();
+multifd_send_sync_main(rs);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
 
-- 
2.17.2 (Apple Git-113)

[Qemu-devel] [PATCH v2 0/3] migration: add speed limit for multifd migration

[Ivan Ren]

Currently multifd migration has not been limited and it will consume
the whole bandwidth of Nic. These two patches add speed limitation to
it.

This is the v2 patches, differences with v1:
1. change qemu_file_update_rate_transfer interface name
   to qemu_file_update_transfer
2. add a new patch to update ram_counters for multifd sync packet

Ivan Ren (3):
  migration: add qemu_file_update_transfer interface
  migration: add speed limit for multifd migration
  migration: update ram_counters for multifd sync packet

 migration/qemu-file.c |  5 +
 migration/qemu-file.h |  1 +
 migration/ram.c   | 24 ++--
 3 files changed, 20 insertions(+), 10 deletions(-)

-- 
2.17.2 (Apple Git-113)

[Qemu-devel] [PATCH v2 1/3] migration: add qemu_file_update_transfer interface

[Ivan Ren]

Add qemu_file_update_transfer for just update bytes_xfer for speed
limitation. This will be used for further migration feature such as
multifd migration.

Signed-off-by: Ivan Ren 
---
 migration/qemu-file.c | 5 +
 migration/qemu-file.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 0431585502..18f480529a 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -615,6 +615,11 @@ void qemu_file_reset_rate_limit(QEMUFile *f)
 f->bytes_xfer = 0;
 }
 
+void qemu_file_update_transfer(QEMUFile *f, int64_t len)
+{
+f->bytes_xfer += len;
+}
+
 void qemu_put_be16(QEMUFile *f, unsigned int v)
 {
 qemu_put_byte(f, v >> 8);
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 13baf896bd..5de9fa2e96 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -147,6 +147,7 @@ int qemu_peek_byte(QEMUFile *f, int offset);
 void qemu_file_skip(QEMUFile *f, int size);
 void qemu_update_position(QEMUFile *f, size_t size);
 void qemu_file_reset_rate_limit(QEMUFile *f);
+void qemu_file_update_transfer(QEMUFile *f, int64_t len);
 void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
 void qemu_file_set_error(QEMUFile *f, int ret);
-- 
2.17.2 (Apple Git-113)

[Qemu-devel] [PATCH v2 3/3] migration: update ram_counters for multifd sync packet

[Ivan Ren]

Multifd sync will send MULTIFD_FLAG_SYNC flag info to destination, add
these bytes to ram_counters record.

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

diff --git a/migration/ram.c b/migration/ram.c
index 88ddd2bbe2..20b6eebb7c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1085,6 +1085,8 @@ static void multifd_send_sync_main(RAMState *rs)
 p->flags |= MULTIFD_FLAG_SYNC;
 p->pending_job++;
 qemu_file_update_transfer(rs->f, p->packet_len);
+ram_counters.multifd_bytes += p->packet_len;
+ram_counters.transferred += p->packet_len;
 qemu_mutex_unlock(&p->mutex);
 qemu_sem_post(&p->sem);
 }
-- 
2.17.2 (Apple Git-113)

[Qemu-devel] [Bug 1838228] Re: Slirp Broadcast traffic

[elmarco]

slirp has been moved to a standalone project, can you report here:
https://gitlab.freedesktop.org/slirp/libslirp/issues

I don't have an answer off the top of my head, but I would suggest
looking/tweaking at the network mask. And for the receive side,
debugging from sorecvfrom().

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

Title:
  Slirp Broadcast traffic

Status in QEMU:
  New

Bug description:
  Hi all,

  Version: QEMU emulator version 3.1.0 (Debian 1:3.1+dfsg-7+build1)

  I'm running some DHCP traffic to a *custom* DHCP server with user-mode
  networking in QEMU. I'm using port 30067 for the server, so this does
  not conflict with the built-in DHCP server.

  DHCP broadcasts to and from the server, and I'm observing issues with
  both sending and receiving packets.

  Firstly, from the VM, a packet sent to IPv4 x.x.x.2:30067 (gateway)
  makes it to the server, but a packet sent to 255.255.255.255 does not.
  I'd suspect that Slirp has to support sending to the broadcast IP
  address? Or is this something I can turn on with a configuration
  option? (My QEMU version too old?)

  Secondly, the source address in a DHCP IPv4 packet must be 0.0.0.0 (by
  RFC). That means that any return packet will have 0.0.0.0 swapped in
  as its destination address. However, that packet doesn't make it into
  the VM at all. I know that if you deliver this packet to Linux, a raw
  socket will spit it back out. The packets' destination address should
  not prevent the packet from being delivered to the right VM, since
  Slirp (should?) know exactly which VM the session belongs to. (It's a
  proxy, not a router.)

  WDYT? Did I miss some configuration options or use too old a version?

  Thanks,
  Chris

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

Re: [Qemu-devel] [Virtio-fs] [PATCH 2/5] virtiofsd: prevent lo_lookup() NULL pointer dereference

[Stefan Hajnoczi]

On Fri, Jul 26, 2019 at 02:26:14PM -0700, Liu Bo wrote:
> On Fri, Jul 26, 2019 at 10:11:00AM +0100, Stefan Hajnoczi wrote:
> > Most lo_do_lookup() have already checked that the parent inode exists.
> > lo_lookup() hasn't and can therefore hit a NULL pointer dereference when
> > lo_inode(req, parent) returns NULL.
> >
> 
> Sigh...this one has been fixed by 3 different developers...Me, Pengtao and 
> Stefan.
> 
> The following one on the ML did the exactly same thing.
> ---
> Subject: [Virtio-fs] [PATCH] virtiofsd: fix lo_do_lookup panic
> 
> It needs to check for invalid parent dir.
> 
> Signed-off-by: Peng Tao 
> ---

One of the previously posted patches will be merged before mine.  It's a
shame that work has been duplicated.  As a contributor I will send a
ping email if there has been no response to a patch after a few days.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [BUG] VM abort after migration

[Jason Wang]

On 2019/7/27 下午2:10, Longpeng (Mike) wrote:

在 2019/7/10 11:57, Jason Wang 写道:

On 2019/7/10 上午11:36, Longpeng (Mike) wrote:

在 2019/7/10 11:25, Jason Wang 写道:

On 2019/7/8 下午5:47, Dr. David Alan Gilbert wrote:

* longpeng (longpe...@huawei.com) wrote:

Hi guys,

We found a qemu core in our testing environment, the assertion
'assert(bus->irq_count[i] == 0)' in pcibus_reset() was triggered and
the bus->irq_count[i] is '-1'.

Through analysis, it was happened after VM migration and we think
it was caused by the following sequence:

*Migration Source*
1. save bus pci.0 state, including irq_count[x] ( =0 , old )
2. save E1000:
  e1000_pre_save
   e1000_mit_timer
    set_interrupt_cause
     pci_set_irq --> update pci_dev->irq_state to 1 and
     update bus->irq_count[x] to 1 ( new )
   the irq_state sent to dest.

*Migration Dest*
1. Receive the irq_count[x] of pci.0 is 0 , but the irq_state of e1000 is 1.
2. If the e1000 need change irqline , it would call to pci_irq_handler(),
     the irq_state maybe change to 0 and bus->irq_count[x] will become
     -1 in this situation.
3. do VM reboot then the assertion will be triggered.

We also found some guys faced the similar problem:
[1] https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02525.html
[2] https://bugs.launchpad.net/qemu/+bug/1702621

Is there some patches to fix this problem ?

I don't remember any.


Can we save pcibus state after all the pci devs are saved ?

Does this problem only happen with e1000? I think so.
If it's only e1000 I think we should fix it - I think once the VM is
stopped for doing the device migration it shouldn't be raising
interrupts.

I wonder maybe we can simply fix this by no setting ICS on pre_save() but
scheduling mit timer unconditionally in post_load().


I also think this is a bug of e1000 because we find more cores with the same
frame thease days.

I'm not familiar with e1000 so hope someone could fix it, thanks. :)


Draft a path in attachment, please test.


Hi Jason,

We've tested the patch for about two weeks, everything went well, thanks!

Feel free to add my:
Reported-and-tested-by: Longpeng 



Applied.

Thanks



Thanks



Thanks



Dave


Thanks,
Longpeng(Mike)

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

.

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Daniel P . Berrangé]

On Sat, Jul 27, 2019 at 07:20:15PM +0100, Peter Maydell wrote:
> On Sat, 27 Jul 2019 at 13:24, Paolo Bonzini  wrote:
> >
> > On 27/07/19 09:16, Markus Armbruster wrote:
> > > We started with a single trace-events.  That wasn't good, so we split it
> > > up into one per directory.  That isn't good, so what about splitting it
> > > up into one per source file?  Pass -DTRACE_HEADER='"trace-DIR-FOO.h"
> > > instead of -DTRACE_HEADER='"trace-DIR.h"' when compiling DIR/FOO.c.
> >
> > For Make this would all work great, however not for Meson because it
> > doesn't allow per-file compile flags.
> 
> Apologies for randomly parachuting into this email thread, but if
> Meson doesn't support per-file compile flags then what's the plan
> for handling the cases where we currently need per-file compile flags ?

I'd suggest we don't actually /need/ per-file compiler flags in most
cases. eg when we add  $foo.o-libs += $(FOO_LIBS) that's not really
a per-file setting when it gets expanded onto the final linker line.
Its just a "-lfoo" that gets used for the library as a while.

> (This is one of the things that I thought was quite a nice move
> forward in our make infrastructure -- we now have clean syntax
> for saying "these files need to be built with these warnings disabled
> or these extra include paths or whatever" and also "these files
> imply we're going to need to link against library X".)

You can disable warnings selectively per file using a Pragma in the
source.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [for-4.2 PATCH 2/2] hw/i386: AMD-Vi IVRS DMA alias support

[Peter Xu]

On Fri, Jul 26, 2019 at 06:55:53PM -0600, Alex Williamson wrote:
> When we account for DMA aliases in the PCI address space, we can no
> longer use a single IVHD entry in the IVRS covering all devices.  We
> instead need to walk the PCI bus and create alias ranges when we find
> a conventional bus.  These alias ranges cannot overlap with a "Select
> All" range (as currently implemented), so we also need to enumerate
> each device with IVHD entries.
> 
> Importantly, the IVHD entries used here include a Device ID, which is
> simply the PCI BDF (Bus/Device/Function).  The guest firmware is
> responsible for programming bus numbers, so the final revision of this
> table depends on the update mechanism (acpi_build_update) to be called
> after guest PCI enumeration.

Ouch... so the ACPI build procedure is after those guest PCI code!
Could I ask how do you find this? :) It seems much easier for sure
this way...

This looks very nice to me already, though I still have got a few
questions, please see below.

[...]

> +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
> +PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(dev));
> +uint8_t sec = pci_bus_num(sec_bus);
> +uint8_t sub = dev->config[PCI_SUBORDINATE_BUS];
> +
> +if (pci_bus_is_express(sec_bus)) {
> +/*
> + * Walk the bus if there are subordinates, otherwise use a range
> + * to cover an entire leaf bus.  We could potentially also use a
> + * range for traversed buses, but we'd need to take care not to
> + * create both Select and Range entries covering the same device.
> + * This is easier and potentially more compact.
> + *
> + * An example bare metal system seems to use Select entries for
> + * root ports without a slot (ie. built-ins) and Range entries
> + * when there is a slot.  The same system also only hard-codes
> + * the alias range for an onboard PCIe-to-PCI bridge, apparently
> + * making no effort to support nested bridges.  We attempt to
> + * be more thorough here.
> + */
> +if (sec == sub) { /* leaf bus */
> +/* "Start of Range" IVHD entry, type 0x3 */
> +entry = PCI_BUILD_BDF(sec, PCI_DEVFN(0, 0)) << 8 | 0x3;
> +build_append_int_noprefix(table_data, entry, 4);
> +/* "End of Range" IVHD entry, type 0x4 */
> +entry = PCI_BUILD_BDF(sub, PCI_DEVFN(31, 7)) << 8 | 0x4;
> +build_append_int_noprefix(table_data, entry, 4);
> +} else {
> +pci_for_each_device(sec_bus, sec, insert_ivhd, table_data);
> +}
> +} else {
> +/*
> + * If the secondary bus is conventional, then we need to create 
> an
> + * Alias range for everything downstream.  The range covers the
> + * first devfn on the secondary bus to the last devfn on the
> + * subordinate bus.  The alias target depends on legacy versus
> + * express bridges, just as in pci_device_iommu_address_space().
> + * DeviceIDa vs DeviceIDb as per the AMD IOMMU spec.
> + */
> +uint16_t dev_id_a, dev_id_b;
> +
> +dev_id_a = PCI_BUILD_BDF(sec, PCI_DEVFN(0, 0));
> +
> +if (pci_is_express(dev) &&
> +pcie_cap_get_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE) {
> +dev_id_b = dev_id_a;
> +} else {
> +dev_id_b = PCI_BUILD_BDF(pci_bus_num(bus), dev->devfn);
> +}
> +
> +/* "Alias Start of Range" IVHD entry, type 0x43, 8 bytes */
> +build_append_int_noprefix(table_data, dev_id_a << 8 | 0x43, 4);
> +build_append_int_noprefix(table_data, dev_id_b << 8 | 0x0, 4);
> +
> +/* "End of Range" IVHD entry, type 0x4 */
> +entry = PCI_BUILD_BDF(sub, PCI_DEVFN(31, 7)) << 8 | 0x4;
> +build_append_int_noprefix(table_data, entry, 4);
> +}

We've implmented the similar logic for multiple times:

  - When we want to do DMA (pci_requester_id)
  - When we want to fetch the DMA address space (the previous patch)
  - When we fill in the AMD ACPI table (this patch)

Do you think we can generalize them somehow?  I'm thinking how about
we directly fetch RID in the 2nd/3rd use case using pci_requester_id()
(which existed already) and simply use it?

[...]

> +/*
> + * A PCI bus walk, for each PCI host bridge, is necessary to create a
> + * complete set of IVHD entries.  Do this into a separate blob so that we
> + * can calculate the total IVRS table length here and then append the new
> + * blob further below.  Fall back to an entry covering all devices, which
> + * is sufficient when no aliases are present.
> + */
> +object_child_foreach_recursive(object_get_root(),
> +

Re: [Qemu-devel] [PATCH 3/3] memory-device: break the loop if tmp exceed the hinted range

[Wei Yang]

On Mon, Jul 29, 2019 at 09:49:37AM +0200, David Hildenbrand wrote:
>On 28.07.19 15:13, Wei Yang wrote:
>> The memory-device list built by memory_device_build_list is ordered by
>> its address, this means if the tmp range exceed the hinted range, all
>> the following range will not overlap with it.
>> 
>> Signed-off-by: Wei Yang 
>> ---
>>  hw/mem/memory-device.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index 413b514586..aea47ab3e8 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -180,7 +180,7 @@ static uint64_t memory_device_get_free_addr(MachineState 
>> *ms,
>>  range_make_empty(&new);
>>  break;
>>  }
>> -} else if (!hint) {
>> +} else if (!hint || range_lob(&tmp) > range_upb(&new)) {
>>  break;
>>  }
>>  }
>> 
>
>Lower bound is inclusive, upper bound is exclusive. Shouldn't this be
>
>range_lob(&tmp) >= range_upb(&new)
>

Per my understanding, a range with start = 0, size = 0x1, is represented

[0, 0x]

So if I have another range [0x, 0x1], they seems to overlap. The range
[0x1, 0x1] doesn't overlap with [0, 0x].

My original comparison looks right. Do I miss some point?

>Also, I wonder if patch #2 is now really needed?
>

Hmm... I think you are right.

I am afraid without Patch #2, the condition check is not that intuitive. Would
this bring some confusion for audience and maintenance?

I am not sure the percentage of occurrence when hint is provided, while the
generated code for check NULL is less than compare two values.

>-- 
>
>Thanks,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 3/3] memory-device: break the loop if tmp exceed the hinted range

[Igor Mammedov]

On Mon, 29 Jul 2019 09:49:37 +0200
David Hildenbrand  wrote:

> On 28.07.19 15:13, Wei Yang wrote:
> > The memory-device list built by memory_device_build_list is ordered by
> > its address, this means if the tmp range exceed the hinted range, all
> > the following range will not overlap with it.
> > 
> > Signed-off-by: Wei Yang 
> > ---
> >  hw/mem/memory-device.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> > index 413b514586..aea47ab3e8 100644
> > --- a/hw/mem/memory-device.c
> > +++ b/hw/mem/memory-device.c
> > @@ -180,7 +180,7 @@ static uint64_t 
> > memory_device_get_free_addr(MachineState *ms,
> >  range_make_empty(&new);
> >  break;
> >  }
> > -} else if (!hint) {
> > +} else if (!hint || range_lob(&tmp) > range_upb(&new)) {
> >  break;
> >  }
> >  }
> >   
> 
> Lower bound is inclusive, upper bound is exclusive. Shouldn't this be
> 
> range_lob(&tmp) >= range_upb(&new)
> 
> Also, I wonder if patch #2 is now really needed?
Indeed, it looks like 3/3 will break early in both hinted and
non-hinted cases so 2/3 looks not necessary (in case 2/3 is dropped
this commit message needs to be amended). 

Re: [Qemu-devel] [PATCH v8 00/11] Build ACPI Heterogeneous Memory Attribute Table (HMAT)

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190729063127.2801-1-tao3...@intel.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==13824==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==13869==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==13869==WARNING: ASan is ignoring requested __asan_handle_no_return: stack 
top: 0x7ffe5d5d2000; bottom 0x7fae1b5f8000; size: 0x005041fda000 (344704524288)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
==13888==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 26 test-aio /aio-gsource/event/flush
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
==13897==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 28 test-aio /aio-gsource/timer/schedule
==13903==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==13910==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 2 ide-test /x86_64/ide/flush
==13924==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==13935==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==13941==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
==13952==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
==13958==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 7 ide-test /x86_64/ide/bmdma/long_prdt
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
==13964==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==13964==WARNING: ASan is ignoring requested __asan_handle_no_return: stack 
top: 0x7fffb65ab000; bottom 0x7f1eb49fe000; size: 0x00e101bad000 (966396661760)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
---
PASS 9 ide-test /x86_64/ide/flush/nodev
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==13988==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 13 test-throttle /thrott

[Qemu-devel] [PULL 1/5] e1000: don't raise interrupt in pre_save()

[Jason Wang]

We should not raise any interrupt after VM has been stopped but this
is what e1000 currently did when mit timer is active in
pre_save(). Fixing this by scheduling a timer in post_load() which can
make sure the interrupt was raised when VM is running.

Reported-and-tested-by: Longpeng 
Signed-off-by: Jason Wang 
---
 hw/net/e1000.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 1dc1466..a023ceb 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1381,11 +1381,6 @@ static int e1000_pre_save(void *opaque)
 E1000State *s = opaque;
 NetClientState *nc = qemu_get_queue(s->nic);
 
-/* If the mitigation timer is active, emulate a timeout now. */
-if (s->mit_timer_on) {
-e1000_mit_timer(s);
-}
-
 /*
  * If link is down and auto-negotiation is supported and ongoing,
  * complete auto-negotiation immediately. This allows us to look
@@ -1423,7 +1418,8 @@ static int e1000_post_load(void *opaque, int version_id)
 s->mit_irq_level = false;
 }
 s->mit_ide = 0;
-s->mit_timer_on = false;
+s->mit_timer_on = true;
+timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
 
 /* nc.link_down can't be migrated, so infer link_down according
  * to link status bit in mac_reg[STATUS].
-- 
2.5.0

[Qemu-devel] [PULL 0/5] Net patches

[Jason Wang]

The following changes since commit fff3159900d2b95613a9cb75fc3703e67a674729:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190726' 
into staging (2019-07-26 16:23:07 +0100)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to f77bed14f01557596727c4eea042e9818c242049:

  net/colo-compare.c: Fix memory leak and code style issue. (2019-07-29 
16:29:30 +0800)




Jason Wang (1):
  e1000: don't raise interrupt in pre_save()

Prasad J Pandit (3):
  qemu-bridge-helper: restrict interface name to IFNAMSIZ
  qemu-bridge-helper: move repeating code in parse_acl_file
  net: tap: replace snprintf with g_strdup_printf calls

Zhang Chen (1):
  net/colo-compare.c: Fix memory leak and code style issue.

 hw/net/e1000.c   |  8 ++--
 net/colo-compare.c   | 27 ---
 net/tap.c| 19 +++
 qemu-bridge-helper.c | 24 +---
 4 files changed, 50 insertions(+), 28 deletions(-)

[Qemu-devel] [PULL 5/5] net/colo-compare.c: Fix memory leak and code style issue.

[Jason Wang]

From: Zhang Chen 

This patch to fix the origin "char *data" memory leak, code style issue
and add necessary check here.
Reported-by: Coverity (CID 1402785)

Signed-off-by: Zhang Chen 
Reviewed-by: Peter Maydell 
Signed-off-by: Jason Wang 
---
 net/colo-compare.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 909dd6c..7489840 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -127,6 +127,17 @@ static int compare_chr_send(CompareState *s,
 uint32_t vnet_hdr_len,
 bool notify_remote_frame);
 
+static bool packet_matches_str(const char *str,
+   const uint8_t *buf,
+   uint32_t packet_len)
+{
+if (packet_len != strlen(str)) {
+return false;
+}
+
+return !memcmp(str, buf, strlen(str));
+}
+
 static void notify_remote_frame(CompareState *s)
 {
 char msg[] = "DO_CHECKPOINT";
@@ -1008,21 +1019,23 @@ static void compare_notify_rs_finalize(SocketReadState 
*notify_rs)
 {
 CompareState *s = container_of(notify_rs, CompareState, notify_rs);
 
-/* Get Xen colo-frame's notify and handle the message */
-char *data = g_memdup(notify_rs->buf, notify_rs->packet_len);
-char msg[] = "COLO_COMPARE_GET_XEN_INIT";
+const char msg[] = "COLO_COMPARE_GET_XEN_INIT";
 int ret;
 
-if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) {
+if (packet_matches_str("COLO_USERSPACE_PROXY_INIT",
+   notify_rs->buf,
+   notify_rs->packet_len)) {
 ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
 if (ret < 0) {
 error_report("Notify Xen COLO-frame INIT failed");
 }
-}
-
-if (!strcmp(data, "COLO_CHECKPOINT")) {
+} else if (packet_matches_str("COLO_CHECKPOINT",
+  notify_rs->buf,
+  notify_rs->packet_len)) {
 /* colo-compare do checkpoint, flush pri packet and remove sec packet 
*/
 g_queue_foreach(&s->conn_list, colo_flush_packets, s);
+} else {
+error_report("COLO compare got unsupported instruction");
 }
 }
 
-- 
2.5.0

[Qemu-devel] [PULL 2/5] qemu-bridge-helper: restrict interface name to IFNAMSIZ

[Jason Wang]

From: Prasad J Pandit 

The network interface name in Linux is defined to be of size
IFNAMSIZ(=16), including the terminating null('\0') byte.
The same is applied to interface names read from 'bridge.conf'
file to form ACL rules. If user supplied '--br=bridge' name
is not restricted to the same length, it could lead to ACL bypass
issue. Restrict interface name to IFNAMSIZ, including null byte.

Reported-by: Riccardo Schirone 
Signed-off-by: Prasad J Pandit 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Li Qiang 
Signed-off-by: Jason Wang 
---
 qemu-bridge-helper.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 95624bc..2058e10 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -119,6 +119,13 @@ static int parse_acl_file(const char *filename, ACLList 
*acl_list)
 }
 *argend = 0;
 
+if (!g_str_equal(cmd, "include") && strlen(arg) >= IFNAMSIZ) {
+fprintf(stderr, "name `%s' too long: %zu\n", arg, strlen(arg));
+fclose(f);
+errno = EINVAL;
+return -1;
+}
+
 if (strcmp(cmd, "deny") == 0) {
 acl_rule = g_malloc(sizeof(*acl_rule));
 if (strcmp(arg, "all") == 0) {
@@ -269,6 +276,10 @@ int main(int argc, char **argv)
 usage();
 return EXIT_FAILURE;
 }
+if (strlen(bridge) >= IFNAMSIZ) {
+fprintf(stderr, "name `%s' too long: %zu\n", bridge, strlen(bridge));
+return EXIT_FAILURE;
+}
 
 /* parse default acl file */
 QSIMPLEQ_INIT(&acl_list);
-- 
2.5.0

[Qemu-devel] [PULL 3/5] qemu-bridge-helper: move repeating code in parse_acl_file

[Jason Wang]

From: Prasad J Pandit 

Move repeating error handling sequence in parse_acl_file routine
to an 'err' label.

Signed-off-by: Prasad J Pandit 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Li Qiang 
Signed-off-by: Jason Wang 
---
 qemu-bridge-helper.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 2058e10..3d50ec0 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -102,9 +102,7 @@ static int parse_acl_file(const char *filename, ACLList 
*acl_list)
 
 if (arg == NULL) {
 fprintf(stderr, "Invalid config line:\n  %s\n", line);
-fclose(f);
-errno = EINVAL;
-return -1;
+goto err;
 }
 
 *arg = 0;
@@ -121,9 +119,7 @@ static int parse_acl_file(const char *filename, ACLList 
*acl_list)
 
 if (!g_str_equal(cmd, "include") && strlen(arg) >= IFNAMSIZ) {
 fprintf(stderr, "name `%s' too long: %zu\n", arg, strlen(arg));
-fclose(f);
-errno = EINVAL;
-return -1;
+goto err;
 }
 
 if (strcmp(cmd, "deny") == 0) {
@@ -149,15 +145,18 @@ static int parse_acl_file(const char *filename, ACLList 
*acl_list)
 parse_acl_file(arg, acl_list);
 } else {
 fprintf(stderr, "Unknown command `%s'\n", cmd);
-fclose(f);
-errno = EINVAL;
-return -1;
+goto err;
 }
 }
 
 fclose(f);
-
 return 0;
+
+err:
+fclose(f);
+errno = EINVAL;
+return -1;
+
 }
 
 static bool has_vnet_hdr(int fd)
-- 
2.5.0

[Qemu-devel] [PULL 4/5] net: tap: replace snprintf with g_strdup_printf calls

[Jason Wang]

From: Prasad J Pandit 

When invoking qemu-bridge-helper in 'net_bridge_run_helper',
instead of using fixed sized buffers, use dynamically allocated
ones initialised and returned by g_strdup_printf().

Signed-off-by: Prasad J Pandit 
Reviewed-by: Li Qiang 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Jason Wang 
---
 net/tap.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index e8aadd8..fc38029 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -498,9 +498,9 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge,
 }
 if (pid == 0) {
 int open_max = sysconf(_SC_OPEN_MAX), i;
-char fd_buf[6+10];
-char br_buf[6+IFNAMSIZ] = {0};
-char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15];
+char *fd_buf = NULL;
+char *br_buf = NULL;
+char *helper_cmd = NULL;
 
 for (i = 3; i < open_max; i++) {
 if (i != sv[1]) {
@@ -508,17 +508,17 @@ static int net_bridge_run_helper(const char *helper, 
const char *bridge,
 }
 }
 
-snprintf(fd_buf, sizeof(fd_buf), "%s%d", "--fd=", sv[1]);
+fd_buf = g_strdup_printf("%s%d", "--fd=", sv[1]);
 
 if (strrchr(helper, ' ') || strrchr(helper, '\t')) {
 /* assume helper is a command */
 
 if (strstr(helper, "--br=") == NULL) {
-snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge);
+br_buf = g_strdup_printf("%s%s", "--br=", bridge);
 }
 
-snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s",
- helper, "--use-vnet", fd_buf, br_buf);
+helper_cmd = g_strdup_printf("%s %s %s %s", helper,
+"--use-vnet", fd_buf, br_buf ? br_buf : "");
 
 parg = args;
 *parg++ = (char *)"sh";
@@ -527,10 +527,11 @@ static int net_bridge_run_helper(const char *helper, 
const char *bridge,
 *parg++ = NULL;
 
 execv("/bin/sh", args);
+g_free(helper_cmd);
 } else {
 /* assume helper is just the executable path name */
 
-snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge);
+br_buf = g_strdup_printf("%s%s", "--br=", bridge);
 
 parg = args;
 *parg++ = (char *)helper;
@@ -541,6 +542,8 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge,
 
 execv(helper, args);
 }
+g_free(fd_buf);
+g_free(br_buf);
 _exit(1);
 
 } else {
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/3] memory-device: break the loop if tmp exceed the hinted range

[David Hildenbrand]

On 29.07.19 10:30, Wei Yang wrote:
> On Mon, Jul 29, 2019 at 09:49:37AM +0200, David Hildenbrand wrote:
>> On 28.07.19 15:13, Wei Yang wrote:
>>> The memory-device list built by memory_device_build_list is ordered by
>>> its address, this means if the tmp range exceed the hinted range, all
>>> the following range will not overlap with it.
>>>
>>> Signed-off-by: Wei Yang 
>>> ---
>>>  hw/mem/memory-device.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>>> index 413b514586..aea47ab3e8 100644
>>> --- a/hw/mem/memory-device.c
>>> +++ b/hw/mem/memory-device.c
>>> @@ -180,7 +180,7 @@ static uint64_t 
>>> memory_device_get_free_addr(MachineState *ms,
>>>  range_make_empty(&new);
>>>  break;
>>>  }
>>> -} else if (!hint) {
>>> +} else if (!hint || range_lob(&tmp) > range_upb(&new)) {
>>>  break;
>>>  }
>>>  }
>>>
>>
>> Lower bound is inclusive, upper bound is exclusive. Shouldn't this be
>>
>> range_lob(&tmp) >= range_upb(&new)
>>
> 
> Per my understanding, a range with start = 0, size = 0x1, is represented
> 
> [0, 0x]
> 
> So if I have another range [0x, 0x1], they seems to overlap. The range
> [0x1, 0x1] doesn't overlap with [0, 0x].
> 
> My original comparison looks right. Do I miss some point?

I guess you saw my other reply by now. :)

> 
>> Also, I wonder if patch #2 is now really needed?
>>
> 
> Hmm... I think you are right.
> 
> I am afraid without Patch #2, the condition check is not that intuitive. Would
> this bring some confusion for audience and maintenance?

Less checks, less confusion :)

> 
> I am not sure the percentage of occurrence when hint is provided, while the
> generated code for check NULL is less than compare two values.

Nobody should care about that performance difference here.

I guess it is fine to just drop patch #2.

Thanks!


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Paolo Bonzini]

On 29/07/19 09:09, Markus Armbruster wrote:
> Peter Maydell  writes:
> 
>> On Sat, 27 Jul 2019 at 13:24, Paolo Bonzini  wrote:
>>>
>>> On 27/07/19 09:16, Markus Armbruster wrote:
 We started with a single trace-events.  That wasn't good, so we split it
 up into one per directory.  That isn't good, so what about splitting it
 up into one per source file?  Pass -DTRACE_HEADER='"trace-DIR-FOO.h"
 instead of -DTRACE_HEADER='"trace-DIR.h"' when compiling DIR/FOO.c.
>>>
>>> For Make this would all work great, however not for Meson because it
>>> doesn't allow per-file compile flags.
>>
>> Apologies for randomly parachuting into this email thread, but if
>> Meson doesn't support per-file compile flags then what's the plan
>> for handling the cases where we currently need per-file compile flags ?
>> (This is one of the things that I thought was quite a nice move
>> forward in our make infrastructure -- we now have clean syntax
>> for saying "these files need to be built with these warnings disabled
>> or these extra include paths or whatever" and also "these files
>> imply we're going to need to link against library X".)
> 
> I vaguely remember from my review that Meson lets us express "these
> files imply we're going to need to link against library X" even more
> clearly.  I can't point you to an example, though.

Yes, you can do just

   common_ss.add(when: curl, if_true: files('curl.c'))

as a replacement for

   common-obj-$(CONFIG_CURL) += curl.c
   curl.c-cflags = $(CURL_CFLAGS)
   curl.c-libs = $(CURL_LIBS)


If conditional compilation is handled inside the file, i.e the same but
with common-obj-y, you can instead do

   common_ss.add(files('vl.c'), sdl)


In both cases, the cflags from the dependency are applied to the whole
target, rather than just to repectively curl.c and vl.c.  So you do lose
the possibility of specifying cflags only for a file.

There is no case where we're using per-.o file CFLAGS for anything other
than dependencies.  I was using per-file -f... options in the
(not-yet-applied) CET series though.  I would have to check whether
pragmas work in that case (if they do, I feel they are superior to
specifying CFLAGS in the Makefile).

>>> Meson maintainers suggest building a static library for each special set
>>> of compile flags; we could do that automatically per-directory(*) but it
>>> would be harder to scale that to per-file.
> 
> This is clearly not "minimal fuss", not even per directory, let alone
> per file.
> 
> It's pretty lame even for the large sets we have (per target,
> target-independent): I guess we'd either throw away the .a unused, or
> link with --wholearchive.

> For me, forwarding headers are just fine for a PoC, quite tolerable
> while a conversion is in progress, and perhaps even tolerable as a
> permanent work-around.  My *actual* question is how we could do per-file
> rather than per-directory trace.h with Meson.  Quoting myself:
> 
> We have trace-events with hundreds of tracepoints for tens of source
> files.  The generated trace.h clock in at hundreds of KiB for me.
> Messing with tracepoints in one source file recompiles most of the
> directory.  This is so much better than it used to be, but clearly not
> as good as it could be.
> 
> The worst of the lot is trace-root.h, which gets included for >1300 .o
> in my "build everything" tree, mostly because it contains tracepoints
> for static inline functions in widely included headers.  See also
> "[PATCH 07/28] trace: Do not include qom/cpu.h into generated trace.h",
> Message-Id: <20190726120542.9894-8-arm...@redhat.com>.
> 
> We started with a single trace-events.  That wasn't good, so we split it
> up into one per directory.  That isn't good, so what about splitting it
> up into one per source file?
> 
> Any ideas?

Per-file (really per-device) would really be easier than per-directory,
I think, because with fine-grained trace-events there is a point in
including a specific header like

   #include "trace-mptsas.h"

or even

   #include "trace/trace-mptsas.h"

(possibly generated from trace-events-mptsas).  The only constraint
would be that the subsystem names would have to be global across all of
QEMU.  If it's per-directory as it is now, instead, you'd have something
like

   #include "trace/trace-hw_scsi.h"

which is really ugly and then forwarding headers look better.

Paolo

Re: [Qemu-devel] [PULL 0/1] Linux user for 4.1 patches

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:35, Laurent Vivier  wrote:
>
> The following changes since commit fff3159900d2b95613a9cb75fc3703e67a674729:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20190726' into staging (2019-07-26 
> 16:23:07 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-4.1-pull-request
>
> for you to fetch changes up to 5bfce0b74fbd5d53089bb866919d685c47edad9e:
>
>   linux-user: Make sigaltstack stacks per-thread (2019-07-26 19:24:33 +0200)
>
> 
> Fix multi-threaded go runtime crash
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Peter Maydell]

On Mon, 29 Jul 2019 at 09:21, Daniel P. Berrangé  wrote:
> You can disable warnings selectively per file using a Pragma in the
> source.

In at least one of these cases (libvixl) the point of using
the per-file flags is that the source files are third
party upstream ones which we don't want to carry local
modifications to. Otherwise we'd just change the source
to suppress the warnings the way we do for other files.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Peter Maydell]

On Mon, 29 Jul 2019 at 09:51, Paolo Bonzini  wrote:
> There is no case where we're using per-.o file CFLAGS for anything other
> than dependencies.

disas/libvixl is a counterexample -- we use per-.o-file CFLAGS for:
 * suppressing warnings in third-party code we don't want to
   carry local modifications to
 * dealing with a NetBSD weirdness about header inclusion order

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] virtiofsd: add FUSE_INIT map_alignment field

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefa...@redhat.com) wrote:
> The client must know the server's alignment constraints for FUSE_SETUPMAPPING
> and FUSE_REMOVEMAPPING.  This is necessary because mmap(2)/munmap(2) have
> alignment constraints and the guest may have a different page size from the
> host.  The new FUSE_INIT map_alignment field communicates this information to
> the client.

OK, merged into my local world.

DAve

> Stefan Hajnoczi (3):
>   virtiofsd: sync up fuse.h Linux 5.1 header
>   virtiofsd: add map_alignment to fuse_kernel.h
>   virtiofsd: implement FUSE_INIT map_alignment field
> 
>  contrib/virtiofsd/fuse_kernel.h   | 56 +--
>  contrib/virtiofsd/fuse_lowlevel.c |  8 +
>  2 files changed, 47 insertions(+), 17 deletions(-)
> 
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Paolo Bonzini]

On 29/07/19 11:21, Peter Maydell wrote:
> On Mon, 29 Jul 2019 at 09:51, Paolo Bonzini  wrote:
>> There is no case where we're using per-.o file CFLAGS for anything other
>> than dependencies.
> 
> disas/libvixl is a counterexample -- we use per-.o-file CFLAGS for:
>  * suppressing warnings in third-party code we don't want to
>carry local modifications to
>  * dealing with a NetBSD weirdness about header inclusion order

The NetBSD thing could be worked around with a static library but
instead those -D options could be added as global C++ flags:

__STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS are a workaround to
allow C++ programs to use stdint.h macros specified in the C99
standard that aren't in the C++ standard.

Likewise, the -Wno-sign-compare probably should be added to all files
for GCC <=4.6, but in fact we don't support anymore GCC 4.6 so it can go
away.

Paolo

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Peter Maydell]

On Mon, 29 Jul 2019 at 10:29, Paolo Bonzini  wrote:
>
> On 29/07/19 11:21, Peter Maydell wrote:
> > On Mon, 29 Jul 2019 at 09:51, Paolo Bonzini  wrote:
> >> There is no case where we're using per-.o file CFLAGS for anything other
> >> than dependencies.
> >
> > disas/libvixl is a counterexample -- we use per-.o-file CFLAGS for:
> >  * suppressing warnings in third-party code we don't want to
> >carry local modifications to
> >  * dealing with a NetBSD weirdness about header inclusion order
>
> The NetBSD thing could be worked around with a static library but
> instead those -D options could be added as global C++ flags:
>
> __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS are a workaround to
> allow C++ programs to use stdint.h macros specified in the C99
> standard that aren't in the C++ standard.
>
> Likewise, the -Wno-sign-compare probably should be added to all files
> for GCC <=4.6, but in fact we don't support anymore GCC 4.6 so it can go
> away.

I think it's an indication that the mechanism in general is
useful. Switching to a new build system worries me if we
already find that it is lacking flexibility we're using with
our current build system -- it suggests that there's likely
to be missing stuff we're going to run into in future as well...

thanks
-- PMM

[Qemu-devel] Reminder: rc3 tomorrow, please list any remaining release-critical issues on wiki

[Peter Maydell]

Hi; are there any remaining release-critical issues for 4.1?
If you know of any, please list them on the wiki page:
https://wiki.qemu.org/Planning/4.1#Known_issues
(and/or just reply to this email)

rc3 will be tomorrow, and ideally that will be the final
rc before our release.

thanks
-- PMM

[Qemu-devel] [Bug 1696773] Re: golang calls to exec crash user emulation

[Peter Maydell]

The sigaltstack fix is now in master (commit 5bfce0b74fbd5d530) and at
least in my test environment this also fixes the "can't build hello.go
reliably" example. So I'm marking this as 'fix committed'. If there are
still problems with running Go binaries, these are likely to be
independent bugs, so please open fresh LP issues for them.


** Changed in: qemu
   Status: In Progress => Fix Committed

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

Title:
  golang calls to exec crash user emulation

Status in QEMU:
  Fix Committed

Bug description:
  An example program can be found here:

  https://github.com/willnewton/qemucrash

  This code starts a goroutine (thread) and calls exec repeatedly. This
  works ok natively but when run under ARM user emulation it segfaults
  (usually, there are occasionally other failures).

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

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Paolo Bonzini]

On 29/07/19 11:32, Peter Maydell wrote:
>> The NetBSD thing could be worked around with a static library but
>> instead those -D options could be added as global C++ flags:
>>
>> __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS are a workaround to
>> allow C++ programs to use stdint.h macros specified in the C99
>> standard that aren't in the C++ standard.
>>
>> Likewise, the -Wno-sign-compare probably should be added to all files
>> for GCC <=4.6, but in fact we don't support anymore GCC 4.6 so it can go
>> away.
>
> I think it's an indication that the mechanism in general is
> useful. Switching to a new build system worries me if we
> already find that it is lacking flexibility we're using with
> our current build system -- it suggests that there's likely
> to be missing stuff we're going to run into in future as well...

I agree that the mechanism in general is useful and it's worth thinking
twice about the consequences of not having it (see the CET example).
However, in both of these cases it seems to me that the per-file CFLAGS
were used when they should have not.

Paolo

Re: [Qemu-devel] [PATCH v3] block/rbd: add preallocation support

[Stefano Garzarella]

On Fri, Jul 26, 2019 at 08:46:56AM -0400, Jason Dillaman wrote:
> On Fri, Jul 26, 2019 at 4:48 AM Stefano Garzarella  
> wrote:
> >
> > On Thu, Jul 25, 2019 at 09:30:30AM -0400, Jason Dillaman wrote:
> > > On Thu, Jul 25, 2019 at 4:13 AM Stefano Garzarella  
> > > wrote:
> > > >
> > > > On Wed, Jul 24, 2019 at 01:48:42PM -0400, Jason Dillaman wrote:
> > > > > On Tue, Jul 23, 2019 at 3:13 AM Stefano Garzarella 
> > > > >  wrote:
> > > > > >
> > > > > > This patch adds the support of preallocation (off/full) for the RBD
> > > > > > block driver.
> > > > > > If rbd_writesame() is available and supports zeroed buffers, we use
> > > > > > it to quickly fill the image when full preallocation is required.
> > > > > >
> > > > > > Signed-off-by: Stefano Garzarella 
> > > > > > ---
> > > > > > v3:
> > > > > >  - rebased on master
> > > > > >  - filled with zeroed buffer [Max]
> > > > > >  - used rbd_writesame() only when we can disable the discard of 
> > > > > > zeroed
> > > > > >buffers
> > > > > >  - added 'since: 4.2' in qapi/block-core.json [Max]
> > > > > >  - used buffer as large as the "stripe unit"
> > > > > > ---
> > > > > >  block/rbd.c  | 202 
> > > > > > ---
> > > > > >  qapi/block-core.json |   5 +-
> > > > > >  2 files changed, 192 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/block/rbd.c b/block/rbd.c
> > > > > > index 59757b3120..d923a5a26c 100644
> > > > > > --- a/block/rbd.c
> > > > > > +++ b/block/rbd.c
> > > > > > @@ -64,6 +64,7 @@
> > > > > >  #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER)
> > > > > >
> > > > > >  #define RBD_MAX_SNAPS 100
> > > > > > +#define RBD_DEFAULT_CONCURRENT_OPS 10
> > > > > >
> > > > > >  /* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */
> > > > > >  #ifdef LIBRBD_SUPPORTS_IOVEC
> > > > > > @@ -104,6 +105,7 @@ typedef struct BDRVRBDState {
> > > > > >  char *image_name;
> > > > > >  char *snap;
> > > > > >  uint64_t image_size;
> > > > > > +bool ws_zero_supported; /* rbd_writesame() supports zeroed 
> > > > > > buffers */
> > > > > >  } BDRVRBDState;
> > > > > >
> > > > > >  static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t 
> > > > > > *io_ctx,
> > > > > > @@ -333,6 +335,155 @@ static void qemu_rbd_memset(RADOSCB *rcb, 
> > > > > > int64_t offs)
> > > > > >  }
> > > > > >  }
> > > > > >
> > > > > > +static int qemu_rbd_get_max_concurrent_ops(rados_t cluster)
> > > > > > +{
> > > > > > +char buf[16];
> > > > > > +int ret, max_concurrent_ops;
> > > > > > +
> > > > > > +ret = rados_conf_get(cluster, "rbd_concurrent_management_ops", 
> > > > > > buf,
> > > > > > + sizeof(buf));
> > > > > > +if (ret < 0) {
> > > > > > +return RBD_DEFAULT_CONCURRENT_OPS;
> > > > > > +}
> > > > > > +
> > > > > > +ret = qemu_strtoi(buf, NULL, 10, &max_concurrent_ops);
> > > > > > +if (ret < 0) {
> > > > > > +return RBD_DEFAULT_CONCURRENT_OPS;
> > > > > > +}
> > > > > > +
> > > > > > +return max_concurrent_ops;
> > > > > > +}
> > > > > > +
> > > > > > +static int qemu_rbd_do_truncate(rados_t cluster, rbd_image_t image,
> > > > > > +int64_t offset, PreallocMode 
> > > > > > prealloc,
> > > > > > +bool ws_zero_supported, Error 
> > > > > > **errp)
> > > > > > +{
> > > > > > +uint64_t current_length;
> > > > > > +char *buf = NULL;
> > > > > > +int ret;
> > > > > > +
> > > > > > +ret = rbd_get_size(image, ¤t_length);
> > > > > > +if (ret < 0) {
> > > > > > +error_setg_errno(errp, -ret, "Failed to get file length");
> > > > > > +goto out;
> > > > > > +}
> > > > > > +
> > > > > > +if (current_length > offset && prealloc != PREALLOC_MODE_OFF) {
> > > > > > +error_setg(errp, "Cannot use preallocation for shrinking 
> > > > > > files");
> > > > > > +ret = -ENOTSUP;
> > > > > > +goto out;
> > > > > > +}
> > > > > > +
> > > > > > +switch (prealloc) {
> > > > > > +case PREALLOC_MODE_FULL: {
> > > > > > +uint64_t buf_size, current_offset = current_length;
> > > > > > +ssize_t bytes;
> > > > > > +
> > > > > > +ret = rbd_get_stripe_unit(image, &buf_size);
> > > > > > +if (ret < 0) {
> > > > > > +error_setg_errno(errp, -ret, "Failed to get stripe 
> > > > > > unit");
> > > > > > +goto out;
> > > > > > +}
> > > > > > +
> > > > > > +ret = rbd_resize(image, offset);
> > > > > > +if (ret < 0) {
> > > > > > +error_setg_errno(errp, -ret, "Failed to resize file");
> > > > > > +goto out;
> > > > > > +}
> > > > > > +
> > > > > > +buf = g_malloc0(buf_size);
> > > > > > +
> > > > > > +#ifdef LIBRBD_SUPPORTS_WRITESAME
> > > > > > +if (ws_zero_supported) {
> > > > > > +uint64_t writesame_max_size;
> > > > > > +int max_concurrent_ops;
> > > > > >

[Qemu-devel] [Bug 1809453] Re: Windows qemu download Big file bug in net user mode

[elmarco]

What is your version of qemu? I understand you are running qemu on
ubuntu.

The VM is windows? which version? Which URL are you downloading? What is
the program being used?

thanks


** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Windows  qemu download Big file bug in net user mode

Status in QEMU:
  Incomplete

Bug description:
  hi

  Windows qemu with -net user downloading big files has a bug, -net tap
  is good!

  I suspect that the Slirp protocol has a bug on the Windows pc, which
  is normal on ubuntu.

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

[Qemu-devel] [Bug 1010484] Re: slirp to accept non-local dns server

[elmarco]

ping

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  slirp to accept non-local dns server

Status in QEMU:
  Incomplete

Bug description:
  current version of slirp doesn't allow feeded dns address to be outside of 
given network.
  in many scenarios you need to provide dns server that isn't local.

  this simple patch removes checking for if dns server isn't in local
  subnet.

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

Re: [Qemu-devel] [PATCH] Fix Guest VM crash due to iSCSI Sense Key error

[Stefan Hajnoczi]

On Fri, Jul 26, 2019 at 04:18:46PM -0400, John Snow wrote:
> Paolo, Stefan and Kevin: can I loop you in here? I'm quite uncertain
> about this and I'd like to clear this up quickly if it's possible:
> 
> On 7/25/19 8:58 PM, John Snow wrote:
> > 
> > 
> > On 7/7/19 10:55 PM, shaju.abra...@nutanix.com wrote:
> >> From: Shaju Abraham 
> >>
> >> During the  IDE DMA transfer for a ISCSI target,when libiscsi encounters
> >> a SENSE KEY error, it sets the task->sense to  the value "COMMAND ABORTED".
> >> The function iscsi_translate_sense() later translaters this error to 
> >> -ECANCELED
> >> and this value is passed to the callback function. In the case of  IDE DMA 
> >> read
> >> or write, the callback function returns immediately if the value of the ret
> >> argument is -ECANCELED.
> >> Later when ide_cancel_dma_sync() function is invoked  the assertion
> >> "s->bus->dma->aiocb == ((void *)0)" fails and the qemu process gets 
> >> terminated.
> >> Fix the issue by making the value of s->bus->dma->aiocb = NULL when
> >> -ECANCELED is passed to the callback.
> >>
> >> Signed-off-by: Shaju Abraham 
> >> ---
> >>  hw/ide/core.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/hw/ide/core.c b/hw/ide/core.c
> >> index 6afadf8..78ea357 100644
> >> --- a/hw/ide/core.c
> >> +++ b/hw/ide/core.c
> >> @@ -841,6 +841,7 @@ static void ide_dma_cb(void *opaque, int ret)
> >>  bool stay_active = false;
> >>  
> >>  if (ret == -ECANCELED) {
> >> +s->bus->dma->aiocb = NULL;
> >>  return;
> >>  }
> >>  
> >>
> > 
> > The part that makes me nervous here is that I can't remember why we do
> > NO cleanup whatsoever for the ECANCELED case.
> > 
> > commit 0d910cfeaf2076b116b4517166d5deb0fea76394
> > Author: Fam Zheng 
> > Date:   Thu Sep 11 13:41:07 2014 +0800
> > 
> > ide/ahci: Check for -ECANCELED in aio callbacks
> > 
> > 
> > ... This looks like we never expected the aio callbacks to ever get
> > called with ECANCELED, so we treat this as a QEMU-internal signal.
> > 
> > It looks like we expect these callbacks to do NOTHING in this case; but
> > I'm not sure where the IDE state machine does its cleanup otherwise.
> > (The DMA might have been canceled, but the DMA and IDE state machines
> > still need to exit their loop.)
> > 
> > If you take a look at this patch from 2014 though, there are many other
> > spots where we have littered ECANCELED checks that might also cause
> > problems if we're receiving error codes we thought we couldn't get normally.
> > 
> > I am worried this patch papers over something worse.
> > 
> I'm not clear why Fam's patch adds a do-nothing return to the ide_dma_cb
> if it's invoked with ECANCELED: shouldn't it be the case that the IDE
> state machine needs to know that a transfer it was relying on to service
> an ATA command was canceled and treat it like an error?
> 
> Why was it ever correct to ignore these? Is it because we only ever
> canceled DMA during reset/shutdown/etc?
> 
> It appears as if iscsi requests can actually genuinely return an
> ECANCELED errno, so there are likely several places in the IDE code that
> need to accommodate this from happening.
> 
> The easiest fix LOOKS like just deleting the special-casing of ECANCELED
> altogether and letting the error pathways handle things as normal.
> 
> Am I mistaken?

I think your instincts are right that there are deeper issues.  The
first step would be test cases, then you can be sure various scenarios
have been handled correctly.

I noticed that ide_sector_read_cb(), ide_sector_write_cb(), and
ide_flush_cb() all differ in whether they reset s->pio_aiocb and
s->status before returning early due to -ECANCELED.  That must be a bug.

I didn't look at the ide_dma_cb() code path.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL] RISC-V Patch for 4.1-rc3

[Peter Maydell]

On Sat, 27 Jul 2019 at 00:27, Palmer Dabbelt  wrote:
>
> The following changes since commit bf8b024372bf8abf5a9f40bfa65eeefad23ff988:
>
>   Update version for v4.1.0-rc2 release (2019-07-23 18:28:08 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/palmer-dabbelt/qemu.git tags/riscv-for-master-4.1-rc3
>
> for you to fetch changes up to 75ea2529cf09aa4630c5357f9814f04b6697e8b7:
>
>   riscv/boot: Fixup the RISC-V firmware warning (2019-07-26 16:03:48 -0700)
>
> 
> RISC-V Patch for 4.1-rc3
>
> This contains a single patch that fixes the warning introduced as part
> of the OpenSBI integration.
>
> 
> Alistair Francis (1):
>   riscv/boot: Fixup the RISC-V firmware warning
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [QUESTION] SDL 1.2 support

[Philippe Mathieu-Daudé]

Hi Aleksandar,

On 7/18/19 8:20 AM, Philippe Mathieu-Daudé wrote:
> On 7/16/19 8:20 PM, Philippe Mathieu-Daudé wrote:
>> Hi Aleksandar,
>>
>> On 7/16/19 7:09 PM, Aleksandar Markovic wrote:
>>> On Tue, Jul 16, 2019 at 1:54 PM Thomas Huth  wrote:

 On 16/07/2019 13.17, Aleksandar Markovic wrote:
> Hello, Gerd, Daniel, and others involved.
>
> I have multiple reports from end users that say that transition from
> SDL 1.2 to SDL 2.0 was difficult, or even impossible for their hosts.
> In that light, they don't appreciate removing SDL 1.2 support from
> QEMU. The most notable example is Ubutnu 16.04, where it looks there
> is no way of installing SDL 2.0 that does not involve complete OS
> upgrade, which, for various reasons, many are not willing to do.

 What's the problem here? According to
 https://packages.ubuntu.com/xenial/libsdl2-2.0-0 the library should be
 available there.

>>>
>>> Yes, we, as developers, are good at upgrading, we like flexibility in
>>> our development systems, and naturally want to try latest and greatest
>>> tools and libraries.
>>>
>>> However, in QA / build / test environments, the things seem to look
>>> different. Their main concern is stability and repeatibility of their
>>> systems. They don't like updates and upgrades. If a new of library
>>> is available for an OS, this does not mean it will be installed, or it
>>> will be desired to be installed.
>>>
>>> It appears that Ubuntu 16.04 came originally with SDL 1.2, and
>>> SDL 2.0 was made available later on.
>>
>> I am a bit confused, I checked the older Xenial image I can find is a
>> pre-release:
>>
>> 16.04.20151218.1-xenial-baseline
>>
[...]
> $ make
> [...]
>   GEN util/trace.c
> CHK version_gen.h
>  LEX convert-dtsv0-lexer.lex.c
> make[1]: flex: Command not found
>  BISON dtc-parser.tab.c
> make[1]: bison: Command not found
>  LEX dtc-lexer.lex.c
> make[1]: flex: Command not found
>   AR  libcapstone.a
> ar: creating /tmp/qemu-4.1.0-rc0/capstone/libcapstone.a
> [...]
>   CC  mipsel-softmmu/trace/generated-helpers.o
>   LINKmipsel-softmmu/qemu-system-mipsel
> [...]
>   CC  mips64el-softmmu/target/mips/cp0_timer.o
>   GEN trace/generated-helpers.c
>   CC  mips64el-softmmu/trace/control-target.o
>   CC  mips64el-softmmu/trace/generated-helpers.o
>   LINKmips64el-softmmu/qemu-system-mips64el
> [...]
> 
> $ wget
> https://mipsdistros.mips.com/LinuxDistro/nanomips/kernels/v4.15.18-432-gb2eb9a8b07a1-20180627102142/generic_nano32r6el_page16k_up.xz
> 
> $ unxz generic_nano32r6el_page16k_up.xz
> 
> $ ./mipsel-softmmu/qemu-system-mipsel -M malta -cpu I7200 -kernel
> generic_nano32r6el_page16k_up -append console=ttyS0 -nographic
> [0.00] Linux version 4.15.18 (emubuild@mipscs567) (gcc version
> 6.3.0 (Codescape GNU Tools 2018.04-02 for nanoMIPS Linux)) #1 Wed Jun 27
> 11:13:09 PDT 2018
> [0.00] GCRs appear to have been moved (expected them at 0x1fbf8000)!
> [0.00] GCRs appear to have been moved (expected them at 0x1fbf8000)!
> [0.00] CPU0 revision is: 0001 (MIPS GENERIC QEMU)
> [0.00] MIPS: machine is mti,malta
> [0.00] Determined physical RAM map:
> [0.00]  memory: 0800 @  (usable)
> [0.00] earlycon: ns16550a0 at I/O port 0x3f8 (options '38400n8')
> [...]
> 
> $ make check-tcg
> [...]
>   BUILD   TCG tests for mips-softmmu
>   BUILD   mips guest-tests SKIPPED
>   RUN TCG tests for mips-softmmu
>   RUN tests for mips SKIPPED
>   BUILD   TCG tests for mips64-softmmu
>   BUILD   mips64 guest-tests SKIPPED
>   RUN TCG tests for mips64-softmmu
>   RUN tests for mips64 SKIPPED
>   BUILD   TCG tests for mips64el-softmmu
>   BUILD   mips64el guest-tests SKIPPED
>   RUN TCG tests for mips64el-softmmu
>   RUN tests for mips64el SKIPPED
>   BUILD   TCG tests for mipsel-softmmu
>   BUILD   mipsel guest-tests SKIPPED
>   RUN TCG tests for mipsel-softmmu
>   RUN tests for mipsel SKIPPED
> [...]
>   BUILD   x86_64 guest-tests with cc
>   RUN tests for x86_64
>   TESTtest-mmap (default) on x86_64
>   TESTsha1 on x86_64
>   TESTlinux-test on x86_64
>   TESTtestthread on x86_64
>   TESTtest-x86_64 on x86_64
>   TESTtest-mmap (4096 byte pages) on x86_64
> [...]
> $
> 
> Ah, cross-target tests are skipped because I don't have Docker for
> cross-building tests.
> 
> Let's see how you use them:
> 
> $ cat tests/tcg/mips/user/ase/msa/README
> The tests in subdirectories of this directory are supposed to be
> compiled for
> mips64el MSA-enabled CPU (I6400, I6500), using an appropriate MIPS
> toolchain.
> For example:
> 
> /opt/img/bin/mips-img-linux-gnu-gcc   \
> -EL -static -mabi=64 -march=mips64r6 -mmsa  -o 
> 
> They are to be executed using QEMU user mode, using command line:
> 
> mips64el-linux-user/qemu-mips64el -cpu I6400 
> [...]
> 
> Googling I find this link:
> https

[Qemu-devel] [PATCH for-4.1] block/copy-on-read: Fix permissions for inactive node

[Kevin Wolf]

The copy-on-read drive must not request the WRITE_UNCHANGED permission
for its child if the node is inactive, otherwise starting a migration
destination with -incoming will fail because the child cannot provide
write access yet:

  qemu-system-x86_64: -blockdev copy-on-read,file=img,node-name=cor: Block node 
is read-only

Earlier QEMU versions additionally ran into an abort() on the migration
source side: bdrv_inactivate_recurse() failed to update permissions.
This is silently ignored today because it was only supposed to loosen
restrictions. This is the symptom that was originally reported here:

  https://bugzilla.redhat.com/show_bug.cgi?id=1733022

Signed-off-by: Kevin Wolf 
---
 block/copy-on-read.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 22f24fd0db..6631f30205 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -56,16 +56,14 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild 
*c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
 {
-if (c == NULL) {
-*nperm = (perm & PERM_PASSTHROUGH) | BLK_PERM_WRITE_UNCHANGED;
-*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
-return;
-}
+*nperm = perm & PERM_PASSTHROUGH;
+*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
 
-*nperm = (perm & PERM_PASSTHROUGH) |
- (c->perm & PERM_UNCHANGED);
-*nshared = (shared & PERM_PASSTHROUGH) |
-   (c->shared_perm & PERM_UNCHANGED);
+/* We must not request write permissions for an inactive node, the child
+ * cannot provide it. */
+if (!(bs->open_flags & BDRV_O_INACTIVE)) {
+*nperm |= BLK_PERM_WRITE_UNCHANGED;
+}
 }
 
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH] Support load kernel(vmlinux)/dtb/initrd separately for Boston in QEMU.

[Aleksandar Markovic]

ping

On Mon, Apr 1, 2019 at 11:26 AM Archer Yan  wrote:

> Currently boston in QEMU only supports boot with FIT format. Since ELF file
> can provide symbol infomation in debug, this patch enables Boston boot from
> vmlinux&dtb.
>
> Signed-off-by: Archer Yan 
> ---
>  hw/mips/boston.c | 224 ++-
>  1 file changed, 201 insertions(+), 23 deletions(-)
>
>
Hi, Paul,

Note that we now have 2 proposals dealing with this topic: this one and

https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05108.html

I think it would be cool to integrate this support in 4.2.

Yours,
Aleksandar





> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index e5bab3cadc..5910ffdc8a 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -39,6 +39,10 @@
>  #include "sysemu/device_tree.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/qtest.h"
> +#include "elf.h"
> +#include "sysemu/kvm.h"
> +#include "hw/mips/mips.h"
> +#include "qemu/option.h"
>
>  #include 
>
> @@ -58,6 +62,11 @@ typedef struct {
>
>  hwaddr kernel_entry;
>  hwaddr fdt_base;
> +long kernel_size;
> +
> +uint64_t initrd_size;
> +uint64_t initrd_start;
> +uint64_t initrd_offset;
>  } BostonState;
>
>  enum boston_plat_reg {
> @@ -328,31 +337,59 @@ static void gen_firmware(uint32_t *p, hwaddr
> kernel_entry, hwaddr fdt_addr,
>  stl_p(p++, 0x0329); /* jr   $25 */
>  }
>
> -static const void *boston_fdt_filter(void *opaque, const void *fdt_orig,
> - const void *match_data, hwaddr
> *load_addr)
> +typedef uint64_t (*xlate_to_kseg0)(void *, uint64_t);
> +static xlate_to_kseg0 get_xlate_to_kseg0_fn(BostonState *s)
> +{
> +/* Check where the kernel has been linked */
> +if (s->kernel_entry & 0x8000ll) {
> +if (kvm_enabled()) {
> +error_report("KVM guest kernels must be linked in useg. "
> + "Did you forget to enable CONFIG_KVM_GUEST?");
> +return NULL;
> +}
> +return cpu_mips_phys_to_kseg0;
> +} else {
> +/* if kernel entry is in useg it is probably a KVM T&E kernel */
> +mips_um_ksegs_enable();
> +return cpu_mips_kvm_um_phys_to_kseg0;
> +}
> +}
> +
> +/*Adapt fdt to insert initrd parameters*/
> +static int boston_initrd_fdt(BostonState *s, void *fdt)
>  {
> -BostonState *s = BOSTON(opaque);
> -MachineState *machine = s->mach;
>  const char *cmdline;
> +char *args_str = NULL;
> +MachineState *machine = s->mach;
> +int initrd_args_len = 64;
>  int err;
> -void *fdt;
> -size_t fdt_sz, ram_low_sz, ram_high_sz;
> +size_t ram_low_sz, ram_high_sz;
> +uint64_t (*xlate_to_kseg0_fn) (void *opaque, uint64_t addr);
>
> -fdt_sz = fdt_totalsize(fdt_orig) * 2;
> -fdt = g_malloc0(fdt_sz);
> +cmdline = (machine->kernel_cmdline && machine->kernel_cmdline[0])
> +? machine->kernel_cmdline : " ";
> +xlate_to_kseg0_fn = get_xlate_to_kseg0_fn(s);
> +if (NULL == xlate_to_kseg0_fn) {
> +fprintf(stderr, "couldn't get xlate_to_kseg0\n");
> +return -1;
> +}
>
> -err = fdt_open_into(fdt_orig, fdt, fdt_sz);
> -if (err) {
> -fprintf(stderr, "unable to open FDT\n");
> -return NULL;
> +s->initrd_start = xlate_to_kseg0_fn(NULL, s->initrd_offset);
> +
> +if (s->initrd_size) {
> +args_str = g_malloc(strlen(cmdline) + initrd_args_len);
> +if (args_str != NULL) {
> +sprintf((char *)args_str, "rd_start=0x%lx rd_size=0x%lx %s",
> +s->initrd_start, s->initrd_size, cmdline);
> +cmdline = args_str;
> +}
>  }
>
> -cmdline = (machine->kernel_cmdline && machine->kernel_cmdline[0])
> -? machine->kernel_cmdline : " ";
>  err = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
>  if (err < 0) {
>  fprintf(stderr, "couldn't set /chosen/bootargs\n");
> -return NULL;
> +g_free(args_str);
> +return -1;
>  }
>
>  ram_low_sz = MIN(256 * MiB, machine->ram_size);
> @@ -360,10 +397,41 @@ static const void *boston_fdt_filter(void *opaque,
> const void *fdt_orig,
>  qemu_fdt_setprop_sized_cells(fdt, "/memory@0", "reg",
>   1, 0x, 1, ram_low_sz,
>   1, 0x9000, 1, ram_high_sz);
> +g_free(args_str);
> +return 0;
> +}
>
> -fdt = g_realloc(fdt, fdt_totalsize(fdt));
> -qemu_fdt_dumpdtb(fdt, fdt_sz);
> +static const void *boston_fdt_filter(void *opaque, const void *fdt_orig,
> + const void *match_data, hwaddr
> *load_addr)
> +{
> +BostonState *s = BOSTON(opaque);
> +MachineState *machine = s->mach;
> +int err;
> +void *fdt;
> +int fdt_sz;
> +if (machine->dtb) {
> +/*Use QEMU cmd specified dtb*/
> +fdt = load_device_tree(machine->dtb, &fdt_s

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Markus Armbruster]

Pino Toscano  writes:

> On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
>> On 7/26/19 9:09 AM, Pino Toscano wrote:
>> > Add a 'private-key' option which represents the path of a private key
>> > to use for authentication, and 'private-key-secret' as the name of an
>> > object with its passphrase.
>> > 
>> > Signed-off-by: Pino Toscano 
>> 
>> > +++ b/qapi/block-core.json
>> > @@ -3226,6 +3226,11 @@
>> >  # @password-secret: ID of a QCryptoSecret object providing a password
>> >  #   for authentication (since 4.2)
>> >  #
>> > +# @private-key: path to the private key (since 4.2)
>> > +#
>> > +# @private-key-secret:  ID of a QCryptoSecret object providing the 
>> > passphrase
>> > +#   for 'private-key' (since 4.2)
>> 
>> Is password-secret intended to be mutually-exclusive with
>> private-key/private-key-secret?
>
> My initial thought was to allow users to specify data for all the
> authentication methods possible.  Either ways (all of them, or a single
> one) are fine for me.

How does this work at the libssh level?  Can you configure multiple
authentication methods, and let negotiation pick the one to be used?

Re: [Qemu-devel] [PATCH v4 7/7] monitor: adding info tbs, tb, and coverset

[Alex Bennée]

vandersonmr  writes:

> Adding info [tbs|tb|coverset] commands to HMP.
> These commands allow the exploration of TBs
> generated by the TCG. Understand which one
> hotter, with more guest/host instructions...
> and examine their guest, host and IR code.
>
> The goal of this command is to allow the dynamic exploration
> of TCG behavior and code quality. Therefore, for now, a
> corresponding QMP command is not worthwhile.
>
> Signed-off-by: vandersonmr 
> ---
>  accel/tcg/tb-stats.c | 275 +++
>  hmp-commands-info.hx |  23 +++
>  include/exec/tb-stats.h  |  37 +
>  include/qemu/log-for-trace.h |   2 +
>  include/qemu/log.h   |   1 +
>  linux-user/exit.c|   4 +
>  monitor/misc.c   |  71 +
>  util/log.c   |  26 +++-
>  8 files changed, 431 insertions(+), 8 deletions(-)
>
> diff --git a/accel/tcg/tb-stats.c b/accel/tcg/tb-stats.c
> index 6c330e1b02..85a16cd563 100644
> --- a/accel/tcg/tb-stats.c
> +++ b/accel/tcg/tb-stats.c
> @@ -212,3 +212,278 @@ void do_hmp_tbstats_safe(CPUState *cpu, run_on_cpu_data 
> icmd)
>  g_free(cmdinfo);
>  }
>
> +static void collect_tb_stats(void *p, uint32_t hash, void *userp)
> +{
> +last_search = g_list_prepend(last_search, p);
> +}
> +
> +static void dump_tb_header(TBStatistics *tbs)
> +{
> +unsigned g = tbs->translations.total ?
> +tbs->code.num_guest_inst / tbs->translations.total : 0;
> +unsigned ops = tbs->translations.total ?
> +tbs->code.num_tcg_ops / tbs->translations.total : 0;
> +unsigned ops_opt = tbs->translations.total ?
> +tbs->code.num_tcg_ops_opt / tbs->translations.total : 0;
> +unsigned h = tbs->translations.total ?
> +tbs->code.num_host_inst / tbs->translations.total : 0;
> +unsigned spills = tbs->translations.total ?
> +tbs->code.spills / tbs->translations.total : 0;
> +
> +float guest_host_prop = g ? ((float) h / g) : 0;
> +
> +qemu_log("TB%d: phys:0x"TB_PAGE_ADDR_FMT" virt:0x"TARGET_FMT_lx
> + " flags:%#08x (trans:%lu uncached:%lu exec:%lu ints: g:%u op:%u 
> op_opt:%u h:%u h/g:%.2f spills:%d)\n",
> + tbs->display_id,
> + tbs->phys_pc, tbs->pc, tbs->flags,
> + tbs->translations.total, tbs->translations.uncached,
> + tbs->executions.total, g, ops, ops_opt, h, guest_host_prop,
> + spills);
> +}
> +
> +static gint
> +inverse_sort_tbs(gconstpointer p1, gconstpointer p2, gpointer psort_by)
> +{
> +const TBStatistics *tbs1 = (TBStatistics *) p1;
> +const TBStatistics *tbs2 = (TBStatistics *) p2;
> +int sort_by = *((int *) psort_by);
> +unsigned long c1 = 0;
> +unsigned long c2 = 0;
> +
> +if (likely(sort_by == SORT_BY_SPILLS)) {
> +c1 = tbs1->code.spills;
> +c2 = tbs2->code.spills;
> +} else if (likely(sort_by == SORT_BY_HOTNESS)) {
> +c1 = tbs1->executions.total;
> +c2 = tbs2->executions.total;
> +} else if (likely(sort_by == SORT_BY_HG)) {
> +if (tbs1->code.num_guest_inst == 0) {
> +return -1;
> +}
> +if (tbs2->code.num_guest_inst == 0) {
> +return 1;
> +}
> +
> +float a = (float) tbs1->code.num_host_inst / 
> tbs1->code.num_guest_inst;
> +float b = (float) tbs2->code.num_host_inst / 
> tbs2->code.num_guest_inst;
> +c1 = a <= b ? 0 : 1;
> +c2 = a <= b ? 1 : 0;
> +}
> +
> +return c1 < c2 ? 1 : c1 == c2 ? 0 : -1;
> +}
> +
> +static void do_dump_coverset_info(int percentage)
> +{
> +uint64_t total_exec_count = 0;
> +uint64_t covered_exec_count = 0;
> +unsigned coverset_size = 0;
> +int id = 1;
> +GList *i;
> +
> +g_list_free(last_search);
> +last_search = NULL;
> +
> +qht_iter(&tb_ctx.tb_stats, collect_tb_stats, NULL);
> +
> +int sort_by = SORT_BY_HOTNESS;
> +last_search = g_list_sort_with_data(last_search, inverse_sort_tbs, 
> &sort_by);
> +
> +if (!last_search) {
> +qemu_log("No data collected yet\n");
> +return;
> +}
> +
> +/* Compute total execution count for all tbs */
> +for (i = last_search; i; i = i->next) {
> +TBStatistics *tbs = (TBStatistics *) i->data;
> +total_exec_count += tbs->executions.total * tbs->code.num_guest_inst;
> +}
> +
> +for (i = last_search; i; i = i->next) {
> +TBStatistics *tbs = (TBStatistics *) i->data;
> +covered_exec_count += tbs->executions.total * 
> tbs->code.num_guest_inst;
> +tbs->display_id = id++;
> +coverset_size++;
> +dump_tb_header(tbs);
> +
> +/* Iterate and display tbs until reach the percentage count cover */
> +if (((double) covered_exec_count / total_exec_count) >
> +((double) percentage / 100)) {
> +break;
> +}
> +}
> +
> +qemu_log("\n--\n");
> +qemu_log("# of TBs to reach %d%% of

Re: [Qemu-devel] [PULL 0/2] ppc-for-4.1 queue 20190728

[Peter Maydell]

On Sun, 28 Jul 2019 at 07:44, David Gibson  wrote:
>
> The following changes since commit fff3159900d2b95613a9cb75fc3703e67a674729:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20190726' into staging (2019-07-26 
> 16:23:07 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-4.1-20190728
>
> for you to fetch changes up to 8d216d8c5370495fc416bb8ac573299779867aad:
>
>   xics/kvm: Fix fallback to emulated XICS (2019-07-28 11:50:26 +1000)
>
> 
> ppc patch queue (for 4.1) 2019-07-28
>
> Here's a pull request for qemu-4.1, which I hope will be the last from
> the ppc tree.  This applies a couple of last minute fixes for the XIVE
> code.
>
> 
> Greg Kurz (2):
>   spapr/irq: Inform the user when falling back to emulated IC
>   xics/kvm: Fix fallback to emulated XICS



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Kevin Wolf]

Am 26.07.2019 um 16:24 hat Eric Blake geschrieben:
> On 7/26/19 9:09 AM, Pino Toscano wrote:
> > Add a 'private-key' option which represents the path of a private key
> > to use for authentication, and 'private-key-secret' as the name of an
> > object with its passphrase.
> > 
> > Signed-off-by: Pino Toscano 
> 
> > +++ b/qapi/block-core.json
> > @@ -3226,6 +3226,11 @@
> >  # @password-secret: ID of a QCryptoSecret object providing a password
> >  #   for authentication (since 4.2)
> >  #
> > +# @private-key: path to the private key (since 4.2)
> > +#
> > +# @private-key-secret:  ID of a QCryptoSecret object providing the 
> > passphrase
> > +#   for 'private-key' (since 4.2)
> 
> Is password-secret intended to be mutually-exclusive with
> private-key/private-key-secret?  If so, this should probably utilize an
> enum for a discriminator
> { 'enum': 'SshAuth', 'data': ['ssh-agent', 'password', 'private'key'] }
> 
> then update BlockdevOptionsSsh to be a union type with an optional
> discriminator (defaulting to ssh-agent) for back-compat, where
> 'auth':'ssh-agent' needs no further fields, 'auth':'password' adds in a
> 'secret' field for use as password, or where 'auth':'private-key' adds
> in both 'key-file' and 'secret' for use as the two pieces needed for
> private key use.

Can we actually support optional discriminators when we don't have
defaults in the QAPI schema yet?

> On a different topic, how much of this work overlaps with the nbdkit ssh
> plugin? Should we be duplicating efforts with both projects supporting
> ssh natively, or is it worth considering getting qemu out of the ssh
> business and instead connecting to an nbd device provided by nbdkit
> connecting to ssh?

ssh behaves essentially like a filesystem whereas NBD behaves like a
block device. This is especially relevant for everything related to the
file size. As far as I know, using an image format like qcow2 that wants
to grow the image file isn't possible over NBD, whereas I expect it to
work with the ssh block driver.

Kevin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Markus Armbruster]

Paolo Bonzini  writes:

> On 29/07/19 11:32, Peter Maydell wrote:
>>> The NetBSD thing could be worked around with a static library but
>>> instead those -D options could be added as global C++ flags:
>>>
>>> __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS are a workaround to
>>> allow C++ programs to use stdint.h macros specified in the C99
>>> standard that aren't in the C++ standard.
>>>
>>> Likewise, the -Wno-sign-compare probably should be added to all files
>>> for GCC <=4.6, but in fact we don't support anymore GCC 4.6 so it can go
>>> away.
>>
>> I think it's an indication that the mechanism in general is
>> useful. Switching to a new build system worries me if we
>> already find that it is lacking flexibility we're using with
>> our current build system -- it suggests that there's likely
>> to be missing stuff we're going to run into in future as well...
>
> I agree that the mechanism in general is useful and it's worth thinking
> twice about the consequences of not having it (see the CET example).
> However, in both of these cases it seems to me that the per-file CFLAGS
> were used when they should have not.

We have uses of per-file flags that could and maybe even should be
per-some-other-thing (static library, program, global).  Perhaps a
pre-conversion sweep to clean that up would help see us whatever remains
more clearly.

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Pino Toscano]

On Monday, 29 July 2019 12:57:40 CEST Markus Armbruster wrote:
> Pino Toscano  writes:
> 
> > On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
> >> On 7/26/19 9:09 AM, Pino Toscano wrote:
> >> > Add a 'private-key' option which represents the path of a private key
> >> > to use for authentication, and 'private-key-secret' as the name of an
> >> > object with its passphrase.
> >> > 
> >> > Signed-off-by: Pino Toscano 
> >> 
> >> > +++ b/qapi/block-core.json
> >> > @@ -3226,6 +3226,11 @@
> >> >  # @password-secret: ID of a QCryptoSecret object providing a 
> >> > password
> >> >  #   for authentication (since 4.2)
> >> >  #
> >> > +# @private-key: path to the private key (since 4.2)
> >> > +#
> >> > +# @private-key-secret:  ID of a QCryptoSecret object providing the 
> >> > passphrase
> >> > +#   for 'private-key' (since 4.2)
> >> 
> >> Is password-secret intended to be mutually-exclusive with
> >> private-key/private-key-secret?
> >
> > My initial thought was to allow users to specify data for all the
> > authentication methods possible.  Either ways (all of them, or a single
> > one) are fine for me.
> 
> How does this work at the libssh level?  Can you configure multiple
> authentication methods, and let negotiation pick the one to be used?

The remote servers sends all the authentication methods supported, and
the user of libssh can decide which one(s) to attempt.  See for example
the small tutorial in the libssh documentation:
http://api.libssh.org/stable/libssh_tutor_authentication.html

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.

Re: [Qemu-devel] [QUESTION] SDL 1.2 support

[Aleksandar Markovic]

>
>
>
> >
> > I already spent 2h on this today, I have to continue other tasks
> > meanwhile, I might continue later.
>
> I spent 1 more hour installing the CodeScape MIPS SDK,...


Philippe, thank you so much for your 3 hours!

Regards,
Aleksandar

Re: [Qemu-devel] [PULL 0/5] Net patches

[Peter Maydell]

On Mon, 29 Jul 2019 at 09:33, Jason Wang  wrote:
>
> The following changes since commit fff3159900d2b95613a9cb75fc3703e67a674729:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20190726' into staging (2019-07-26 
> 16:23:07 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to f77bed14f01557596727c4eea042e9818c242049:
>
>   net/colo-compare.c: Fix memory leak and code style issue. (2019-07-29 
> 16:29:30 +0800)
>
> 
>
> 
> Jason Wang (1):
>   e1000: don't raise interrupt in pre_save()
>
> Prasad J Pandit (3):
>   qemu-bridge-helper: restrict interface name to IFNAMSIZ
>   qemu-bridge-helper: move repeating code in parse_acl_file
>   net: tap: replace snprintf with g_strdup_printf calls
>
> Zhang Chen (1):
>   net/colo-compare.c: Fix memory leak and code style issue.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH v3] block/rbd: add preallocation support

[Jason Dillaman]

On Mon, Jul 29, 2019 at 5:40 AM Stefano Garzarella  wrote:
>
> On Fri, Jul 26, 2019 at 08:46:56AM -0400, Jason Dillaman wrote:
> > On Fri, Jul 26, 2019 at 4:48 AM Stefano Garzarella  
> > wrote:
> > >
> > > On Thu, Jul 25, 2019 at 09:30:30AM -0400, Jason Dillaman wrote:
> > > > On Thu, Jul 25, 2019 at 4:13 AM Stefano Garzarella 
> > > >  wrote:
> > > > >
> > > > > On Wed, Jul 24, 2019 at 01:48:42PM -0400, Jason Dillaman wrote:
> > > > > > On Tue, Jul 23, 2019 at 3:13 AM Stefano Garzarella 
> > > > > >  wrote:
> > > > > > >
> > > > > > > This patch adds the support of preallocation (off/full) for the 
> > > > > > > RBD
> > > > > > > block driver.
> > > > > > > If rbd_writesame() is available and supports zeroed buffers, we 
> > > > > > > use
> > > > > > > it to quickly fill the image when full preallocation is required.
> > > > > > >
> > > > > > > Signed-off-by: Stefano Garzarella 
> > > > > > > ---
> > > > > > > v3:
> > > > > > >  - rebased on master
> > > > > > >  - filled with zeroed buffer [Max]
> > > > > > >  - used rbd_writesame() only when we can disable the discard of 
> > > > > > > zeroed
> > > > > > >buffers
> > > > > > >  - added 'since: 4.2' in qapi/block-core.json [Max]
> > > > > > >  - used buffer as large as the "stripe unit"
> > > > > > > ---
> > > > > > >  block/rbd.c  | 202 
> > > > > > > ---
> > > > > > >  qapi/block-core.json |   5 +-
> > > > > > >  2 files changed, 192 insertions(+), 15 deletions(-)
> > > > > > >
> > > > > > > diff --git a/block/rbd.c b/block/rbd.c
> > > > > > > index 59757b3120..d923a5a26c 100644
> > > > > > > --- a/block/rbd.c
> > > > > > > +++ b/block/rbd.c
> > > > > > > @@ -64,6 +64,7 @@
> > > > > > >  #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER)
> > > > > > >
> > > > > > >  #define RBD_MAX_SNAPS 100
> > > > > > > +#define RBD_DEFAULT_CONCURRENT_OPS 10
> > > > > > >
> > > > > > >  /* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */
> > > > > > >  #ifdef LIBRBD_SUPPORTS_IOVEC
> > > > > > > @@ -104,6 +105,7 @@ typedef struct BDRVRBDState {
> > > > > > >  char *image_name;
> > > > > > >  char *snap;
> > > > > > >  uint64_t image_size;
> > > > > > > +bool ws_zero_supported; /* rbd_writesame() supports zeroed 
> > > > > > > buffers */
> > > > > > >  } BDRVRBDState;
> > > > > > >
> > > > > > >  static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t 
> > > > > > > *io_ctx,
> > > > > > > @@ -333,6 +335,155 @@ static void qemu_rbd_memset(RADOSCB *rcb, 
> > > > > > > int64_t offs)
> > > > > > >  }
> > > > > > >  }
> > > > > > >
> > > > > > > +static int qemu_rbd_get_max_concurrent_ops(rados_t cluster)
> > > > > > > +{
> > > > > > > +char buf[16];
> > > > > > > +int ret, max_concurrent_ops;
> > > > > > > +
> > > > > > > +ret = rados_conf_get(cluster, 
> > > > > > > "rbd_concurrent_management_ops", buf,
> > > > > > > + sizeof(buf));
> > > > > > > +if (ret < 0) {
> > > > > > > +return RBD_DEFAULT_CONCURRENT_OPS;
> > > > > > > +}
> > > > > > > +
> > > > > > > +ret = qemu_strtoi(buf, NULL, 10, &max_concurrent_ops);
> > > > > > > +if (ret < 0) {
> > > > > > > +return RBD_DEFAULT_CONCURRENT_OPS;
> > > > > > > +}
> > > > > > > +
> > > > > > > +return max_concurrent_ops;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int qemu_rbd_do_truncate(rados_t cluster, rbd_image_t 
> > > > > > > image,
> > > > > > > +int64_t offset, PreallocMode 
> > > > > > > prealloc,
> > > > > > > +bool ws_zero_supported, Error 
> > > > > > > **errp)
> > > > > > > +{
> > > > > > > +uint64_t current_length;
> > > > > > > +char *buf = NULL;
> > > > > > > +int ret;
> > > > > > > +
> > > > > > > +ret = rbd_get_size(image, ¤t_length);
> > > > > > > +if (ret < 0) {
> > > > > > > +error_setg_errno(errp, -ret, "Failed to get file 
> > > > > > > length");
> > > > > > > +goto out;
> > > > > > > +}
> > > > > > > +
> > > > > > > +if (current_length > offset && prealloc != 
> > > > > > > PREALLOC_MODE_OFF) {
> > > > > > > +error_setg(errp, "Cannot use preallocation for shrinking 
> > > > > > > files");
> > > > > > > +ret = -ENOTSUP;
> > > > > > > +goto out;
> > > > > > > +}
> > > > > > > +
> > > > > > > +switch (prealloc) {
> > > > > > > +case PREALLOC_MODE_FULL: {
> > > > > > > +uint64_t buf_size, current_offset = current_length;
> > > > > > > +ssize_t bytes;
> > > > > > > +
> > > > > > > +ret = rbd_get_stripe_unit(image, &buf_size);
> > > > > > > +if (ret < 0) {
> > > > > > > +error_setg_errno(errp, -ret, "Failed to get stripe 
> > > > > > > unit");
> > > > > > > +goto out;
> > > > > > > +}
> > > > > > > +
> > > > > > > +ret = rbd_resize(image, offset);
> > > > > > > +if (ret < 0) {
> > > > > > > +error_setg_errno(errp, 

Re: [Qemu-devel] [Virtio-fs] [PATCH 2/5] virtiofsd: prevent lo_lookup() NULL pointer dereference

[piaojun]

Hi Stefan,

On 2019/7/26 17:11, Stefan Hajnoczi wrote:
> Most lo_do_lookup() have already checked that the parent inode exists.
> lo_lookup() hasn't and can therefore hit a NULL pointer dereference when
> lo_inode(req, parent) returns NULL.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  contrib/virtiofsd/passthrough_ll.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/contrib/virtiofsd/passthrough_ll.c 
> b/contrib/virtiofsd/passthrough_ll.c
> index 9ae1381618..277a17fc03 100644
> --- a/contrib/virtiofsd/passthrough_ll.c
> +++ b/contrib/virtiofsd/passthrough_ll.c
> @@ -766,6 +766,10 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t 
> parent, const char *name,
>   struct lo_data *lo = lo_data(req);
>   struct lo_inode *inode, *dir = lo_inode(req, parent);
>  
> + if (!dir) {
> + return EBADF;
> + }
> +

I worry about that dir will be released or set NULL just after NULL
checking. Or could we use some lock to prevent the simultaneity?

Thanks,
Jun

>   memset(e, 0, sizeof(*e));
>   e->attr_timeout = lo->timeout;
>   e->entry_timeout = lo->timeout;
> 

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Alex Bennée]

Daniel P. Berrangé  writes:

> On Sat, Jul 27, 2019 at 07:20:15PM +0100, Peter Maydell wrote:
>> On Sat, 27 Jul 2019 at 13:24, Paolo Bonzini  wrote:
>> >
>> > On 27/07/19 09:16, Markus Armbruster wrote:
>> > > We started with a single trace-events.  That wasn't good, so we split it
>> > > up into one per directory.  That isn't good, so what about splitting it
>> > > up into one per source file?  Pass -DTRACE_HEADER='"trace-DIR-FOO.h"
>> > > instead of -DTRACE_HEADER='"trace-DIR.h"' when compiling DIR/FOO.c.
>> >
>> > For Make this would all work great, however not for Meson because it
>> > doesn't allow per-file compile flags.
>>
>> Apologies for randomly parachuting into this email thread, but if
>> Meson doesn't support per-file compile flags then what's the plan
>> for handling the cases where we currently need per-file compile flags ?
>
> I'd suggest we don't actually /need/ per-file compiler flags in most
> cases. eg when we add  $foo.o-libs += $(FOO_LIBS) that's not really
> a per-file setting when it gets expanded onto the final linker line.
> Its just a "-lfoo" that gets used for the library as a while.

We do for tests, often to select a specific processor type or feature we
need to build that particular instance of the test.

--
Alex Bennée

Re: [Qemu-devel] [PATCH v5] target/arm: generate a custom MIDR for -cpu max

[Peter Maydell]

On Fri, 26 Jul 2019 at 12:39, Alex Bennée  wrote:
>
> While most features are now detected by probing the ID_* registers
> kernels can (and do) use MIDR_EL1 for working out of they have to
> apply errata. This can trip up warnings in the kernel as it tries to
> work out if it should apply workarounds to features that don't
> actually exist in the reported CPU type.
>
> Avoid this problem by synthesising our own MIDR value.
>
> Signed-off-by: Alex Bennée 
> Reviewed-by: Peter Maydell 
>
> ---
> v2
>   - don't leak QEMU version into ID reg
> v3
>   - move comment into one block
>   - explicit setting of more fields
> v4
>   - minor reword of comment
> v5
>   - VARIANT->PARTNUM and extra words

Applied to target-arm.next for 4.2, thanks.

-- PMM

[Qemu-devel] [PATCH] tests: Set read-zeroes on for null-co driver

[Andrey Shinkevich]

This patch is to reduce the number of Valgrind report messages about
using uninitialized memory with the null-co driver. It helps to filter
real memory issues and is the same work done for the iotests with the
commit ID a6862418fec4072.

Suggested-by: Kevin Wolf 
Signed-off-by: Andrey Shinkevich 
---
 tests/drive_del-test.c | 3 ++-
 tests/libqos/virtio-scsi.c | 3 ++-
 tests/megasas-test.c   | 3 ++-
 tests/nvme-test.c  | 3 ++-
 tests/qmp-test.c   | 2 +-
 tests/test-blockjob-txn.c  | 5 -
 tests/test-blockjob.c  | 5 -
 tests/usb-hcd-uhci-test.c  | 3 ++-
 tests/usb-hcd-xhci-test.c  | 3 ++-
 tests/virtio-blk-test.c| 6 --
 tests/virtio-ccw-test.c| 9 ++---
 tests/virtio-scsi-test.c   | 6 --
 12 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index b56b223..5f8839b 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -121,7 +121,8 @@ static void test_drive_del_device_del(void)
 QTestState *qts;
 
 /* Start with a drive used by a device that unplugs instantaneously */
-qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,format=raw"
+qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
+  "file.read-zeroes=on,format=raw"
   " -device virtio-scsi-%s"
   " -device scsi-hd,drive=drive0,id=dev0",
   qvirtio_get_dev_type());
diff --git a/tests/libqos/virtio-scsi.c b/tests/libqos/virtio-scsi.c
index 94842ec..de739be 100644
--- a/tests/libqos/virtio-scsi.c
+++ b/tests/libqos/virtio-scsi.c
@@ -95,7 +95,8 @@ static void virtio_scsi_register_nodes(void)
 };
 
 QOSGraphEdgeOptions opts = {
-.before_cmd_line = "-drive id=drv0,if=none,file=null-co://,format=raw",
+.before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
+   "file.read-zeroes=on,format=raw",
 .after_cmd_line = "-device scsi-hd,bus=vs0.0,drive=drv0",
 };
 
diff --git a/tests/megasas-test.c b/tests/megasas-test.c
index c3e4ab6..d6796b9 100644
--- a/tests/megasas-test.c
+++ b/tests/megasas-test.c
@@ -75,7 +75,8 @@ static void megasas_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
 .extra_device_opts = "addr=04.0,id=scsi0",
-.before_cmd_line = "-drive id=drv0,if=none,file=null-co://,format=raw",
+.before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
+   "file.read-zeroes=on,format=raw",
 .after_cmd_line = "-device scsi-hd,bus=scsi0.0,drive=drv0",
 };
 
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index 5052993..ff04421 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -70,7 +70,8 @@ static void nvme_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
 .extra_device_opts = "addr=04.0,drive=drv0,serial=foo",
-.before_cmd_line = "-drive id=drv0,if=none,file=null-co://,format=raw",
+.before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
+   "file.read-zeroes=on,format=raw",
 };
 
 add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 48a4fa7..1b0eb69 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -187,7 +187,7 @@ static void send_cmd_that_blocks(QTestState *s, const char 
*id)
" 'arguments': {"
" 'driver': 'blkdebug', 'node-name': %s,"
" 'config': %s,"
-   " 'image': { 'driver': 'null-co' } } }",
+   " 'image': { 'driver': 'null-co', 'read-zeroes': true } } 
}",
id, id, fifo_name);
 }
 
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index 86606f9..7da9216 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -15,6 +15,7 @@
 #include "qemu/main-loop.h"
 #include "block/blockjob_int.h"
 #include "sysemu/block-backend.h"
+#include "qapi/qmp/qdict.h"
 
 typedef struct {
 BlockJob common;
@@ -96,7 +97,9 @@ static BlockJob *test_block_job_start(unsigned int iterations,
 
 data = g_new0(TestBlockJobCBData, 1);
 
-bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort);
+QDict *opt = qdict_new();
+qdict_put_str(opt, "file.read-zeroes", "on");
+bs = bdrv_open("null-co://", NULL, opt, 0, &error_abort);
 g_assert_nonnull(bs);
 
 snprintf(job_id, sizeof(job_id), "job%u", counter++);
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index b33f899..68a0819 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -15,6 +15,7 @@
 #include "qemu/main-loop.h"
 #include "block/blockjob_int.h"
 #include "sysemu/block-backend.h"
+#include "qapi/qmp/qdict.h"
 
 static const BlockJobDriver test_block_job_driver = {
 .job_driver = {
@@ -71,7 +72,9 @@ static BlockBackend *create_blk(const char *name)
 BlockBackend *blk = blk_new(qemu_get_aio_cont

Re: [Qemu-devel] [PULL 0/5] Net patches

[Jason Wang]

On 2019/7/29 下午7:47, Peter Maydell wrote:

On Mon, 29 Jul 2019 at 09:33, Jason Wang  wrote:

The following changes since commit fff3159900d2b95613a9cb75fc3703e67a674729:

   Merge remote-tracking branch 
'remotes/pmaydell/tags/pull-target-arm-20190726' into staging (2019-07-26 
16:23:07 +0100)

are available in the git repository at:

   https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to f77bed14f01557596727c4eea042e9818c242049:

   net/colo-compare.c: Fix memory leak and code style issue. (2019-07-29 
16:29:30 +0800)




Jason Wang (1):
   e1000: don't raise interrupt in pre_save()

Prasad J Pandit (3):
   qemu-bridge-helper: restrict interface name to IFNAMSIZ
   qemu-bridge-helper: move repeating code in parse_acl_file
   net: tap: replace snprintf with g_strdup_printf calls

Zhang Chen (1):
   net/colo-compare.c: Fix memory leak and code style issue.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM



Thanks for the reminding.

There's no user-visible changes.

Re: [Qemu-devel] [PATCH 3/3] memory-device: break the loop if tmp exceed the hinted range

[Wei Yang]

On Mon, Jul 29, 2019 at 10:30:56AM +0200, Igor Mammedov wrote:
>On Mon, 29 Jul 2019 09:49:37 +0200
>David Hildenbrand  wrote:
>
>> On 28.07.19 15:13, Wei Yang wrote:
>> > The memory-device list built by memory_device_build_list is ordered by
>> > its address, this means if the tmp range exceed the hinted range, all
>> > the following range will not overlap with it.
>> > 
>> > Signed-off-by: Wei Yang 
>> > ---
>> >  hw/mem/memory-device.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> > index 413b514586..aea47ab3e8 100644
>> > --- a/hw/mem/memory-device.c
>> > +++ b/hw/mem/memory-device.c
>> > @@ -180,7 +180,7 @@ static uint64_t 
>> > memory_device_get_free_addr(MachineState *ms,
>> >  range_make_empty(&new);
>> >  break;
>> >  }
>> > -} else if (!hint) {
>> > +} else if (!hint || range_lob(&tmp) > range_upb(&new)) {
>> >  break;
>> >  }
>> >  }
>> >   
>> 
>> Lower bound is inclusive, upper bound is exclusive. Shouldn't this be
>> 
>> range_lob(&tmp) >= range_upb(&new)
>> 
>> Also, I wonder if patch #2 is now really needed?
>Indeed, it looks like 3/3 will break early in both hinted and
>non-hinted cases so 2/3 looks not necessary (in case 2/3 is dropped
>this commit message needs to be amended). 
>

ok, let me drop #2

-- 
Wei Yang
Help you, Help me

[Qemu-devel] [RFC] virtio-mmio: implement modern (v2) personality (virtio-1)

[Sergio Lopez]

Implement the modern (v2) personality, according to the VirtIO 1.0
specification.

Support for v2 among guests is not as widespread as it'd be
desirable. While the Linux driver has had it for a while, support is
missing, at least, from Tianocore EDK II, NetBSD and FreeBSD.

For this reason, the v2 personality is disabled, keeping the legacy
behavior as default. Machine types willing to use v2, can enable it
using MachineClass's compat_props.

Signed-off-by: Sergio Lopez 
---
 hw/virtio/virtio-mmio.c | 264 ++--
 1 file changed, 254 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 97b7f35496..1da841336f 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -47,14 +47,24 @@
 OBJECT_CHECK(VirtIOMMIOProxy, (obj), TYPE_VIRTIO_MMIO)
 
 #define VIRT_MAGIC 0x74726976 /* 'virt' */
-#define VIRT_VERSION 1
+#define VIRT_VERSION_LEGACY 1
+#define VIRT_VERSION_MODERN 2
 #define VIRT_VENDOR 0x554D4551 /* 'QEMU' */
 
+typedef struct VirtIOMMIOQueue {
+uint16_t num;
+bool enabled;
+uint32_t desc[2];
+uint32_t avail[2];
+uint32_t used[2];
+} VirtIOMMIOQueue;
+
 typedef struct {
 /* Generic */
 SysBusDevice parent_obj;
 MemoryRegion iomem;
 qemu_irq irq;
+bool modern;
 /* Guest accessible state needing migration and reset */
 uint32_t host_features_sel;
 uint32_t guest_features_sel;
@@ -62,6 +72,9 @@ typedef struct {
 /* virtio-bus */
 VirtioBusState bus;
 bool format_transport_address;
+/* Fields only used for v2 (modern) devices */
+uint32_t guest_features[2];
+VirtIOMMIOQueue vqs[VIRTIO_QUEUE_MAX];
 } VirtIOMMIOProxy;
 
 static bool virtio_mmio_ioeventfd_enabled(DeviceState *d)
@@ -115,7 +128,11 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
offset, unsigned size)
 case VIRTIO_MMIO_MAGIC_VALUE:
 return VIRT_MAGIC;
 case VIRTIO_MMIO_VERSION:
-return VIRT_VERSION;
+if (proxy->modern) {
+return VIRT_VERSION_MODERN;
+} else {
+return VIRT_VERSION_LEGACY;
+}
 case VIRTIO_MMIO_VENDOR_ID:
 return VIRT_VENDOR;
 default:
@@ -146,14 +163,18 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
offset, unsigned size)
 case VIRTIO_MMIO_MAGIC_VALUE:
 return VIRT_MAGIC;
 case VIRTIO_MMIO_VERSION:
-return VIRT_VERSION;
+if (proxy->modern) {
+return VIRT_VERSION_MODERN;
+} else {
+return VIRT_VERSION_LEGACY;
+}
 case VIRTIO_MMIO_DEVICE_ID:
 return vdev->device_id;
 case VIRTIO_MMIO_VENDOR_ID:
 return VIRT_VENDOR;
 case VIRTIO_MMIO_DEVICE_FEATURES:
 if (proxy->host_features_sel) {
-return 0;
+return vdev->host_features >> 32;
 }
 return vdev->host_features;
 case VIRTIO_MMIO_QUEUE_NUM_MAX:
@@ -162,12 +183,34 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
offset, unsigned size)
 }
 return VIRTQUEUE_MAX_SIZE;
 case VIRTIO_MMIO_QUEUE_PFN:
+if (proxy->modern) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: read from legacy register in modern mode\n",
+  __func__);
+return 0;
+}
 return virtio_queue_get_addr(vdev, vdev->queue_sel)
 >> proxy->guest_page_shift;
+case VIRTIO_MMIO_QUEUE_READY:
+if (!proxy->modern) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: read from modern register in legacy mode\n",
+  __func__);
+return 0;
+}
+return proxy->vqs[vdev->queue_sel].enabled;
 case VIRTIO_MMIO_INTERRUPT_STATUS:
 return atomic_read(&vdev->isr);
 case VIRTIO_MMIO_STATUS:
 return vdev->status;
+case VIRTIO_MMIO_CONFIG_GENERATION:
+if (!proxy->modern) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: read from modern register in legacy mode\n",
+  __func__);
+return 0;
+}
+return vdev->generation;
 case VIRTIO_MMIO_DEVICE_FEATURES_SEL:
 case VIRTIO_MMIO_DRIVER_FEATURES:
 case VIRTIO_MMIO_DRIVER_FEATURES_SEL:
@@ -177,6 +220,12 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
offset, unsigned size)
 case VIRTIO_MMIO_QUEUE_ALIGN:
 case VIRTIO_MMIO_QUEUE_NOTIFY:
 case VIRTIO_MMIO_INTERRUPT_ACK:
+case VIRTIO_MMIO_QUEUE_DESC_LOW:
+case VIRTIO_MMIO_QUEUE_DESC_HIGH:
+case VIRTIO_MMIO_QUEUE_AVAIL_LOW:
+case VIRTIO_MMIO_QUEUE_AVAIL_HIGH:
+case VIRTIO_MMIO_QUEUE_USED_LOW:
+case VIRTIO_MMIO_QUEUE_USED_HIGH:
 qemu_log_mask(LOG_GUEST_ERROR,
   "%s: read of write-only register\n",
   __func__);
@@ -232,14 +281,26 @@ static void virtio_mmio_writ

Re: [Qemu-devel] [PATCH 3/6] hw/arm: Use sysbus_init_child_obj for correct reference counting

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé  wrote:
>
> As explained in commit aff39be0ed97:
>
>   Both functions, object_initialize() and object_property_add_child()
>   increase the reference counter of the new object, so one of the
>   references has to be dropped afterwards to get the reference
>   counting right. Otherwise the child object will not be properly
>   cleaned up when the parent gets destroyed.
>   Thus let's use now object_initialize_child() instead to get the
>   reference counting here right.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/exynos4_boards.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index ac0b0dc2a9..5dd53d2a23 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -129,8 +129,8 @@ exynos4_boards_init_common(MachineState *machine,
>  exynos4_boards_init_ram(s, get_system_memory(),
>  exynos4_board_ram_size[board_type]);
>
> -object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
> -qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
> +sysbus_init_child_obj(OBJECT(machine), "soc",
> +  &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>  object_property_set_bool(OBJECT(&s->soc), true, "realized",
>   &error_fatal);

I suspect the code change here is correct but it doesn't seem
to match the commit message -- the old code is not calling
object_property_add_child() at all, and the new code does not
call object_initialize_child()...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/6] hw/arm/fsl-imx: Add the cpu as child of the SoC object

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé  wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/fsl-imx25.c | 4 +++-
>  hw/arm/fsl-imx31.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
> index 869ee89b15..a237e967e4 100644
> --- a/hw/arm/fsl-imx25.c
> +++ b/hw/arm/fsl-imx25.c
> @@ -36,7 +36,9 @@ static void fsl_imx25_init(Object *obj)
>  FslIMX25State *s = FSL_IMX25(obj);
>  int i;
>
> -object_initialize(&s->cpu, sizeof(s->cpu), "arm926-" TYPE_ARM_CPU);
> +object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +ARM_CPU_TYPE_NAME("arm926"),
> +&error_abort, NULL);
>
>  sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic),
>TYPE_IMX_AVIC);
> diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
> index 662fe78f1b..423d9ef076 100644
> --- a/hw/arm/fsl-imx31.c
> +++ b/hw/arm/fsl-imx31.c
> @@ -33,7 +33,9 @@ static void fsl_imx31_init(Object *obj)
>  FslIMX31State *s = FSL_IMX31(obj);
>  int i;
>
> -object_initialize(&s->cpu, sizeof(s->cpu), "arm1136-" TYPE_ARM_CPU);
> +object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +ARM_CPU_TYPE_NAME("arm1136"),
> +&error_abort, NULL);
>
>  sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic),
>TYPE_IMX_AVIC);
> --
> 2.20.1

Really the ARM_CPU_TYPE_NAME part of this change should be
in patch 1 I think...

Could you expand the commit message a little to explain why
we want to make the CPU a child of the SoC object?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/6] hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé  wrote:
>
> Commit ba1ba5cca introduce the ARM_CPU_TYPE_NAME() macro.
> Unify the code base by use it in all places.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---


> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 05505bac56..1cbd8674bf 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -35,7 +35,8 @@ static void fsl_imx6ul_init(Object *obj)
>  for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>  snprintf(name, NAME_SIZE, "cpu%d", i);
>  object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
> -"cortex-a7-" TYPE_ARM_CPU, &error_abort, 
> NULL);
> +ARM_CPU_TYPE_NAME("cortex-a7"),
> +&error_abort, NULL);
>  }
>
>  /*

You'll find this part needs a minor fix to apply to master
now we've got rid of this loop-over-CPUs in fsl-imx6ul.c.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/6] hw/arm: Use object_initialize_child for correct reference counting

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé  wrote:
>
> As explained in commit aff39be0ed97:
>
>   Both functions, object_initialize() and object_property_add_child()
>   increase the reference counter of the new object, so one of the
>   references has to be dropped afterwards to get the reference
>   counting right. Otherwise the child object will not be properly
>   cleaned up when the parent gets destroyed.
>   Thus let's use now object_initialize_child() instead to get the
>   reference counting here right.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [RFC] virtio-mmio: implement modern (v2) personality (virtio-1)

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190729125755.45008-1-...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [RFC] virtio-mmio: implement modern (v2) personality 
(virtio-1)
Message-id: 20190729125755.45008-1-...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20190729125755.45008-1-...@redhat.com -> 
patchew/20190729125755.45008-1-...@redhat.com
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for 
path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) 
registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 
'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 
'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 
'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered 
for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) 
registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for 
path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) 
registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for 
path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) 
registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for 
path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for 
path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for 
path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) 
registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 
'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' 
(https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 
'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' 
(https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 
'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) 
registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out 
'22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 
'90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 
'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out 
'20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) 
registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' 
(https://github.com/openssl/openssl) registered for path 
'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': 
checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out 
'50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered 
for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) 
registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': 
checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked 
out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 
'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out 
'09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...
Submodule path '

Re: [Qemu-devel] [PATCH v8 02/11] numa: move numa global variable nb_numa_nodes into MachineState

[Igor Mammedov]

On Mon, 29 Jul 2019 14:31:18 +0800
Tao Xu  wrote:

> Add struct NumaState in MachineState and move existing numa global
> nb_numa_nodes(renamed as "num_nodes") into NumaState. And add variable
> numa_support into MachineClass to decide which submachines support NUMA.
> 
> Suggested-by: Igor Mammedov 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Tao Xu 
> ---
> 
> Changes in v8:
> - Add check if numa->numa_state is NULL in pxb_dev_realize_common
> - Use nb_nodes in spapr_populate_memory() (Igor)
> ---
[...]
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 821f0d4a49..1c7c12c415 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -331,7 +331,7 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> SpaprMachineState *spapr)
>  return ret;
>  }
>  
> -if (nb_numa_nodes > 1) {
> +if (ms->numa_state->num_nodes > 1) {
>  ret = spapr_fixup_cpu_numa_dt(fdt, offset, cpu);
>  if (ret < 0) {
>  return ret;
> @@ -351,9 +351,9 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> SpaprMachineState *spapr)
>  
>  static hwaddr spapr_node0_size(MachineState *machine)
>  {
> -if (nb_numa_nodes) {
> +if (machine->numa_state->num_nodes) {
>  int i;
> -for (i = 0; i < nb_numa_nodes; ++i) {
> +for (i = 0; i < machine->numa_state->num_nodes; ++i) {
>  if (numa_info[i].node_mem) {
>  return MIN(pow2floor(numa_info[i].node_mem),
> machine->ram_size);
> @@ -398,13 +398,14 @@ static int spapr_populate_memory(SpaprMachineState 
> *spapr, void *fdt)
>  {
>  MachineState *machine = MACHINE(spapr);
>  hwaddr mem_start, node_size;
> -int i, nb_nodes = nb_numa_nodes;
> +int i, nb_nodes = machine->numa_state->num_nodes;
>  NodeInfo *nodes = numa_info;
>  NodeInfo ramnode;
>  
>  /* No NUMA nodes, assume there is just one node with whole RAM */
> -if (!nb_numa_nodes) {
> +if (!nb_nodes) {
>  nb_nodes = 1;
> +machine->numa_state->num_nodes = nb_nodes;
You've not addressed a v7 comment
On Tue, 23 Jul 2019 16:56:41 +0200
Igor Mammedov  wrote:

> I don't like user fixing up generic machine data that came from CLI
> (or luck of such)
[...]
> I'd keep fixup local (i.e. using nb_nodes)

i.e. do not modify machine->numa_state->num_nodes and just use value local
like current code does.

>  ramnode.node_mem = machine->ram_size;
>  nodes = &ramnode;
>  }
[...]

[Qemu-devel] [PATCH for 4.1 v2 7/7] linux-user: Add support for semtimedop() syscall

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Add support for semtimedop() emulation. It is based on invocation
of safe_semtimedop().

Conversion is left out of safe_semtimedop(), since other safe_xxx()
usually don't contain similar conversions.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/syscall.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee80175..c7b08f5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6650,7 +6650,39 @@ static inline abi_long host_to_target_statx(struct 
target_statx *host_stx,
 return 0;
 }
 #endif
+#ifdef TARGET_NR_semtimedop
+static inline abi_long do_semtimedop(int semid, abi_long ptr, unsigned nsops,
+ abi_long timeout)
+{
+struct sembuf *sops;
+struct timespec ts, *pts;
+abi_long ret;
+
+if (timeout) {
+pts = &ts;
+if (target_to_host_timespec(pts, timeout)) {
+return -TARGET_EFAULT;
+}
+} else {
+pts = NULL;
+}
 
+sops = g_malloc(sizeof(struct sembuf) * nsops);
+if (sops == NULL) {
+return -TARGET_EFAULT;
+}
+
+if (target_to_host_sembuf(sops, ptr, nsops)) {
+g_free(sops);
+return -TARGET_EFAULT;
+}
+
+ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
+g_free(sops);
+
+return ret;
+}
+#endif
 
 /* ??? Using host futex calls even when target atomic operations
are not really atomic probably breaks things.  However implementing
@@ -9194,6 +9226,10 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 case TARGET_NR_semop:
 return do_semop(arg1, arg2, arg3);
 #endif
+#ifdef TARGET_NR_semtimedop
+case TARGET_NR_semtimedop:
+return do_semtimedop(arg1, arg2, arg3, arg4);
+#endif
 #ifdef TARGET_NR_semctl
 case TARGET_NR_semctl:
 return do_semctl(arg1, arg2, arg3, arg4);
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 0/7] linux-user: Misc patches for 4.2

[Aleksandar Markovic]

From: Aleksandar Markovic 

A set of misc linux user patches for 4.2.


v2->v3:

  - minor code formatting improvements
  - added a patch on semtimedop()

v1->v2:

  - updated commit messages
  - minor improvements of code formatting
  - added three patches containing support for ten additional
ioctls

Aleksandar Markovic (5):
  linux-user: Add support for FDMSGON and FDMSGOFF ioctls
  linux-user: Add support for FDRESET, FDRAWCMD, FDTWADDLE, and FDEJECT
ioctls
  linux-user: Add support for FDFMTBEG, FDFMTTRK, and FDFMTEND ioctls
  linux-user: Add support for FDSETEMSGTRESH, FDSETMAXERRS, and
FDGETMAXERRS ioctls
  linux-user: Add support for RNDRESEEDCRNG ioctl

Aleksandar Rikalo (1):
  linux-user: Add support for semtimedop() syscall

Yunqiang Su (1):
  linux user: Add support for FDFLUSH ioctl

 linux-user/ioctls.h| 16 
 linux-user/syscall.c   | 37 +
 linux-user/syscall_defs.h  | 33 +
 linux-user/syscall_types.h | 12 
 4 files changed, 98 insertions(+)

-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 6/7] linux-user: Add support for RNDRESEEDCRNG ioctl

[Aleksandar Markovic]

From: Aleksandar Markovic 

RNDRESEEDCRNG is a newer ioctl (added in kernel 4.17), and an
"ifdef" guard is used for that reason in this patch.

Signed-off-by: Aleksandar Markovic 
Reviewed-by: Laurent Vivier 
---
 linux-user/ioctls.h   | 3 +++
 linux-user/syscall_defs.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 6551938..c1fcf7b 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -259,6 +259,9 @@
   IOCTL(RNDADDTOENTCNT, IOC_W, MK_PTR(TYPE_INT))
   IOCTL(RNDZAPENTCNT, 0, TYPE_NULL)
   IOCTL(RNDCLEARPOOL, 0, TYPE_NULL)
+#ifdef RNDRESEEDCRNG
+  IOCTL(RNDRESEEDCRNG, 0, TYPE_NULL)
+#endif
 
   IOCTL(CDROMPAUSE, 0, TYPE_NULL)
   IOCTL(CDROMSTART, 0, TYPE_NULL)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 36256b0..85b732b 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -850,6 +850,7 @@ struct target_pollfd {
 #define TARGET_RNDADDTOENTCNT  TARGET_IOW('R', 0x01, int)
 #define TARGET_RNDZAPENTCNTTARGET_IO('R', 0x04)
 #define TARGET_RNDCLEARPOOLTARGET_IO('R', 0x06)
+#define TARGET_RNDRESEEDCRNG   TARGET_IO('R', 0x07)
 
 /* From  */
 
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 1/7] linux user: Add support for FDFLUSH ioctl

[Aleksandar Markovic]

From: Yunqiang Su 

FDFLUSH is used for flushing buffers of floppy drives. Support in
QEMU is needed because some of Debian packages use this ioctl while
running post-build tests. One such example is 'tar' package.

Signed-off-by: Yunqiang Su 
Signed-off-by: Aleksandar Markovic 
Reviewed-by: Laurent Vivier 
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall.c  | 1 +
 linux-user/syscall_defs.h | 4 
 3 files changed, 7 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 3281c97..fb7b014 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -112,6 +112,8 @@
  IOCTL(BLKZEROOUT, IOC_W, MK_PTR(MK_ARRAY(TYPE_ULONGLONG, 2)))
 #endif
 
+ IOCTL(FDFLUSH, 0, TYPE_NULL)
+
 #ifdef FIBMAP
  IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8367cb1..ee80175 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -87,6 +87,7 @@
 #include 
 #include 
 #include 
+#include 
 #if defined(CONFIG_FIEMAP)
 #include 
 #endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 0662270..fb30bce 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -883,6 +883,10 @@ struct target_pollfd {
 #define TARGET_BLKROTATIONAL TARGET_IO(0x12, 126)
 #define TARGET_BLKZEROOUT TARGET_IO(0x12, 127)
 
+/* From  */
+
+#define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
+
 #define TARGET_FIBMAP TARGET_IO(0x00,1)  /* bmap access */
 #define TARGET_FIGETBSZ   TARGET_IO(0x00,2)  /* get the block size used for 
bmap */
 
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 5/7] linux-user: Add support for FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls

[Aleksandar Markovic]

From: Aleksandar Markovic 

FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
for controlling error reporting of a floppy drive.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h|  2 ++
 linux-user/syscall_defs.h  | 19 +++
 linux-user/syscall_types.h |  7 +++
 3 files changed, 28 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index e393ad6..6551938 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -118,6 +118,8 @@
  IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
  IOCTL(FDFMTEND, 0, TYPE_NULL)
  IOCTL(FDFLUSH, 0, TYPE_NULL)
+ IOCTL(FDSETMAXERRS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
+ IOCTL(FDGETMAXERRS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
  IOCTL(FDRESET, 0, TYPE_NULL)
  IOCTL(FDRAWCMD, 0, TYPE_NULL)
  IOCTL(FDTWADDLE, 0, TYPE_NULL)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 1ca115d..36256b0 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -885,12 +885,31 @@ struct target_pollfd {
 
 /* From  */
 
+struct target_floppy_max_errors {
+abi_uintabort;
+abi_uintread_track;
+abi_uintreset;
+abi_uintrecal;
+abi_uintreporting;
+};
+
+struct target_format_descr {
+abi_uintdevice;
+abi_uinthead;
+abi_uinttrack;
+};
+
 #define TARGET_FDMSGONTARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
 #define TARGET_FDFMTBEG   TARGET_IO(2, 0x47)
 #define TARGET_FDFMTTRK  TARGET_IOW(2, 0x48, struct target_format_descr)
 #define TARGET_FDFMTEND   TARGET_IO(2, 0x49)
+#define TARGET_FDSETEMSGTRESH TARGET_IO(2, 0x4a)
 #define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
+#define TARGET_FDSETMAXERRS  TARGET_IOW(2, 0x4c,   
\
+struct target_floppy_max_errors)
+#define TARGET_FDGETMAXERRS  TARGET_IOR(2, 0x0e,   
\
+struct target_floppy_max_errors)
 #define TARGET_FDRESETTARGET_IO(2, 0x54)
 #define TARGET_FDRAWCMD   TARGET_IO(2, 0x58)
 #define TARGET_FDTWADDLE  TARGET_IO(2, 0x59)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index d82d1a5..5ba7c34 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -261,6 +261,13 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* datalen */
TYPE_PTRVOID) /* data */
 
+STRUCT(floppy_max_errors,
+   TYPE_INT, /* abort */
+   TYPE_INT, /* read_track */
+   TYPE_INT, /* reset */
+   TYPE_INT, /* recal */
+   TYPE_INT) /* reporting */
+
 STRUCT(format_descr,
TYPE_INT, /* device */
TYPE_INT, /* head */
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 4/7] linux-user: Add support for FDFMTBEG, FDFMTTRK, and FDFMTEND ioctls

[Aleksandar Markovic]

From: Aleksandar Markovic 

FDFMTBEG, FDFMTTRK, and FDFMTEND ioctls provide means for controlling
formatting of a floppy drive.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h| 3 +++
 linux-user/syscall_defs.h  | 3 +++
 linux-user/syscall_types.h | 5 +
 3 files changed, 11 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index ab4ef2e..e393ad6 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -114,6 +114,9 @@
 
  IOCTL(FDMSGON, 0, TYPE_NULL)
  IOCTL(FDMSGOFF, 0, TYPE_NULL)
+ IOCTL(FDFMTBEG, 0, TYPE_NULL)
+ IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
+ IOCTL(FDFMTEND, 0, TYPE_NULL)
  IOCTL(FDFLUSH, 0, TYPE_NULL)
  IOCTL(FDRESET, 0, TYPE_NULL)
  IOCTL(FDRAWCMD, 0, TYPE_NULL)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 4185391..1ca115d 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -887,6 +887,9 @@ struct target_pollfd {
 
 #define TARGET_FDMSGONTARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
+#define TARGET_FDFMTBEG   TARGET_IO(2, 0x47)
+#define TARGET_FDFMTTRK  TARGET_IOW(2, 0x48, struct target_format_descr)
+#define TARGET_FDFMTEND   TARGET_IO(2, 0x49)
 #define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
 #define TARGET_FDRESETTARGET_IO(2, 0x54)
 #define TARGET_FDRAWCMD   TARGET_IO(2, 0x58)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 4e36983..d82d1a5 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -261,6 +261,11 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* datalen */
TYPE_PTRVOID) /* data */
 
+STRUCT(format_descr,
+   TYPE_INT, /* device */
+   TYPE_INT, /* head */
+   TYPE_INT) /* track */
+
 #if defined(CONFIG_USBFS)
 /* usb device ioctls */
 STRUCT(usbdevfs_ctrltransfer,
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 2/7] linux-user: Add support for FDMSGON and FDMSGOFF ioctls

[Aleksandar Markovic]

From: Aleksandar Markovic 

FDMSGON and FDMSGOFF switch informational messages of floppy drives
on and off.

Signed-off-by: Aleksandar Markovic 
Reviewed-by: Laurent Vivier 
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall_defs.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index fb7b014..9978163 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -112,6 +112,8 @@
  IOCTL(BLKZEROOUT, IOC_W, MK_PTR(MK_ARRAY(TYPE_ULONGLONG, 2)))
 #endif
 
+ IOCTL(FDMSGON, 0, TYPE_NULL)
+ IOCTL(FDMSGOFF, 0, TYPE_NULL)
  IOCTL(FDFLUSH, 0, TYPE_NULL)
 
 #ifdef FIBMAP
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index fb30bce..cd97e9b 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -885,6 +885,8 @@ struct target_pollfd {
 
 /* From  */
 
+#define TARGET_FDMSGONTARGET_IO(2, 0x45)
+#define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
 #define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
 
 #define TARGET_FIBMAP TARGET_IO(0x00,1)  /* bmap access */
-- 
2.7.4

[Qemu-devel] [PATCH for 4.1 v2 3/7] linux-user: Add support for FDRESET, FDRAWCMD, FDTWADDLE, and FDEJECT ioctls

[Aleksandar Markovic]

From: Aleksandar Markovic 

FDRESET, FDRAWCMD, FDTWADDLE, and FDEJECT ioctls are misc commands
for controlling a floppy drive.

Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 4 
 linux-user/syscall_defs.h | 4 
 2 files changed, 8 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 9978163..ab4ef2e 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -115,6 +115,10 @@
  IOCTL(FDMSGON, 0, TYPE_NULL)
  IOCTL(FDMSGOFF, 0, TYPE_NULL)
  IOCTL(FDFLUSH, 0, TYPE_NULL)
+ IOCTL(FDRESET, 0, TYPE_NULL)
+ IOCTL(FDRAWCMD, 0, TYPE_NULL)
+ IOCTL(FDTWADDLE, 0, TYPE_NULL)
+ IOCTL(FDEJECT, 0, TYPE_NULL)
 
 #ifdef FIBMAP
  IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index cd97e9b..4185391 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -888,6 +888,10 @@ struct target_pollfd {
 #define TARGET_FDMSGONTARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF   TARGET_IO(2, 0x46)
 #define TARGET_FDFLUSHTARGET_IO(2, 0x4b)
+#define TARGET_FDRESETTARGET_IO(2, 0x54)
+#define TARGET_FDRAWCMD   TARGET_IO(2, 0x58)
+#define TARGET_FDTWADDLE  TARGET_IO(2, 0x59)
+#define TARGET_FDEJECTTARGET_IO(2, 0x5a)
 
 #define TARGET_FIBMAP TARGET_IO(0x00,1)  /* bmap access */
 #define TARGET_FIGETBSZ   TARGET_IO(0x00,2)  /* get the block size used for 
bmap */
-- 
2.7.4

Re: [Qemu-devel] [PULL 2/5] block/nvme: support larger that 512 bytes sector devices

[Peter Maydell]

On Mon, 22 Jul 2019 at 18:26, Max Reitz  wrote:
>
> From: Maxim Levitsky 
>
> Currently the driver hardcodes the sector size to 512,
> and doesn't check the underlying device. Fix that.
>
> Also fail if underlying nvme device is formatted with metadata
> as this needs special support.
>
> Signed-off-by: Maxim Levitsky 
> Message-id: 20190716163020.13383-3-mlevi...@redhat.com
> Signed-off-by: Max Reitz 

> +static int64_t nvme_get_blocksize(BlockDriverState *bs)
> +{
> +BDRVNVMeState *s = bs->opaque;
> +assert(s->blkshift >= BDRV_SECTOR_BITS);
> +return 1 << s->blkshift;
> +}

Hi -- Coverity points out here that we calculate the
"1 << s->blkshift" as a 32-bit shift, but then return an
int64_t type (CID 1403771).

Can the blkshift ever really be 31 or more ?

The types here seem weird anyway -- we return an int64_t,
but the only user of this is nvme_probe_blocksizes(),
which uses the value only to set BlockSizes::phys and ::log,
both of which are of type "uint32_t". That leads me to think
that the right return type for the function is uint32_t.

PS: this is the only Coverity issue currently outstanding so
if it's a trivial fix it might be nice to put it into rc3.

thanks
-- PMM

Re: [Qemu-devel] [PULL 2/5] block/nvme: support larger that 512 bytes sector devices

[Max Reitz]

On 29.07.19 15:16, Peter Maydell wrote:
> On Mon, 22 Jul 2019 at 18:26, Max Reitz  wrote:
>>
>> From: Maxim Levitsky 
>>
>> Currently the driver hardcodes the sector size to 512,
>> and doesn't check the underlying device. Fix that.
>>
>> Also fail if underlying nvme device is formatted with metadata
>> as this needs special support.
>>
>> Signed-off-by: Maxim Levitsky 
>> Message-id: 20190716163020.13383-3-mlevi...@redhat.com
>> Signed-off-by: Max Reitz 
> 
>> +static int64_t nvme_get_blocksize(BlockDriverState *bs)
>> +{
>> +BDRVNVMeState *s = bs->opaque;
>> +assert(s->blkshift >= BDRV_SECTOR_BITS);
>> +return 1 << s->blkshift;
>> +}
> 
> Hi -- Coverity points out here that we calculate the
> "1 << s->blkshift" as a 32-bit shift, but then return an
> int64_t type (CID 1403771).
> 
> Can the blkshift ever really be 31 or more ?
> 
> The types here seem weird anyway -- we return an int64_t,
> but the only user of this is nvme_probe_blocksizes(),
> which uses the value only to set BlockSizes::phys and ::log,
> both of which are of type "uint32_t". That leads me to think
> that the right return type for the function is uint32_t.
> 
> PS: this is the only Coverity issue currently outstanding so
> if it's a trivial fix it might be nice to put it into rc3.

Maxim, what do you think?

How about we let nvme_identify() limit blkshift to something sane and
then return a uint32_t here?

In theory it would be limited by page_size, and that has a maximum value
of 2^27.  In practice, though, that limit is checked by another 32-bit
shift...

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for 4.1 v2 7/7] linux-user: Add support for semtimedop() syscall

[Aleksandar Markovic]

On Mon, Jul 29, 2019 at 3:11 PM Aleksandar Markovic <
aleksandar.marko...@rt-rk.com> wrote:

> From: Aleksandar Rikalo 
>
> Add support for semtimedop() emulation. It is based on invocation
> of safe_semtimedop().
>
> Conversion is left out of safe_semtimedop(), since other safe_xxx()
> usually don't contain similar conversions.
>
> Signed-off-by: Aleksandar Rikalo 
> Signed-off-by: Aleksandar Markovic 
> ---
>

Aleksandar R., Laurent,

Please note that I just rebased the patch compared to its last incarnation
- no code change.

Laurent's hints were as follows last time:

"To avoid duplicate code (and cleanup the stack allocation), you should

remove do_semop()  and call do_semtimedop(..., NULL) from IPCOP_semop
and TARGET_NR_semop.

Thanks, Laurent"

 I guess they are still valid.

Thanks,
Aleksandar



>  linux-user/syscall.c | 36 
>  1 file changed, 36 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee80175..c7b08f5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6650,7 +6650,39 @@ static inline abi_long host_to_target_statx(struct
> target_statx *host_stx,
>  return 0;
>  }
>  #endif
> +#ifdef TARGET_NR_semtimedop
> +static inline abi_long do_semtimedop(int semid, abi_long ptr, unsigned
> nsops,
> + abi_long timeout)
> +{
> +struct sembuf *sops;
> +struct timespec ts, *pts;
> +abi_long ret;
> +
> +if (timeout) {
> +pts = &ts;
> +if (target_to_host_timespec(pts, timeout)) {
> +return -TARGET_EFAULT;
> +}
> +} else {
> +pts = NULL;
> +}
>
> +sops = g_malloc(sizeof(struct sembuf) * nsops);
> +if (sops == NULL) {
> +return -TARGET_EFAULT;
> +}
> +
> +if (target_to_host_sembuf(sops, ptr, nsops)) {
> +g_free(sops);
> +return -TARGET_EFAULT;
> +}
> +
> +ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
> +g_free(sops);
> +
> +return ret;
> +}
> +#endif
>
>  /* ??? Using host futex calls even when target atomic operations
> are not really atomic probably breaks things.  However implementing
> @@ -9194,6 +9226,10 @@ static abi_long do_syscall1(void *cpu_env, int num,
> abi_long arg1,
>  case TARGET_NR_semop:
>  return do_semop(arg1, arg2, arg3);
>  #endif
> +#ifdef TARGET_NR_semtimedop
> +case TARGET_NR_semtimedop:
> +return do_semtimedop(arg1, arg2, arg3, arg4);
> +#endif
>  #ifdef TARGET_NR_semctl
>  case TARGET_NR_semctl:
>  return do_semctl(arg1, arg2, arg3, arg4);
> --
> 2.7.4
>
>
>

Re: [Qemu-devel] high-level view of packet processing for virtio NIC?

[Stefan Hajnoczi]

On Tue, Jul 23, 2019 at 10:18:01AM -0600, Chris Friesen wrote:
> I'm looking for information on what the qemu architecture looks like for
> processing virtio network packets in a two-vCPU guest.
> 
> It looks like there's an IO thread doing a decent fraction of the work,
> separate from the vCPU threads--is that correct?  There's no disk involved
> in this case, purely network packet processing.

Most production x86 KVM guests use vhost_net.ko to perform virtio-net
rx/tx virtqueue processing in the host kernel.  That means the QEMU code
isn't used and the code path is totally different.

Before spending too much time on this, check which code path you are
interested in.

If you are using QEMU's virtio-net without vhost then the main loop
thread processes rx/tx virtqueue kicks and packet rx/tx events.  The
vcpu threads are not directly involved because the ioeventfd feature is
used to direct virtqueue kicks to the main loop thread instead of
blocking vcpu threads.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for 4.1 v2 7/7] linux-user: Add support for semtimedop() syscall

[Laurent Vivier]

Le 29/07/2019 à 15:09, Aleksandar Markovic a écrit :
> From: Aleksandar Rikalo 
> 
> Add support for semtimedop() emulation. It is based on invocation
> of safe_semtimedop().
> 
> Conversion is left out of safe_semtimedop(), since other safe_xxx()
> usually don't contain similar conversions.
> 
> Signed-off-by: Aleksandar Rikalo 
> Signed-off-by: Aleksandar Markovic 
> ---
>  linux-user/syscall.c | 36 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ee80175..c7b08f5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6650,7 +6650,39 @@ static inline abi_long host_to_target_statx(struct 
> target_statx *host_stx,
>  return 0;
>  }
>  #endif
> +#ifdef TARGET_NR_semtimedop
> +static inline abi_long do_semtimedop(int semid, abi_long ptr, unsigned nsops,
> + abi_long timeout)
> +{
> +struct sembuf *sops;
> +struct timespec ts, *pts;
> +abi_long ret;
> +
> +if (timeout) {
> +pts = &ts;
> +if (target_to_host_timespec(pts, timeout)) {
> +return -TARGET_EFAULT;
> +}
> +} else {
> +pts = NULL;
> +}
>  
> +sops = g_malloc(sizeof(struct sembuf) * nsops);
> +if (sops == NULL) {
> +return -TARGET_EFAULT;
> +}
> +
> +if (target_to_host_sembuf(sops, ptr, nsops)) {
> +g_free(sops);
> +return -TARGET_EFAULT;
> +}
> +
> +ret = get_errno(safe_semtimedop(semid, sops, nsops, pts));
> +g_free(sops);
> +
> +return ret;
> +}
> +#endif

As we have a "#ifdef__NR_semtimedop" around the definition of
safe_semtimedop() you need the same #ifdef around its use:
perhaps you can update the existing do_semop() to implement do_semtimedop().

>  
>  /* ??? Using host futex calls even when target atomic operations
> are not really atomic probably breaks things.  However implementing
> @@ -9194,6 +9226,10 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>  case TARGET_NR_semop:
>  return do_semop(arg1, arg2, arg3);
>  #endif
> +#ifdef TARGET_NR_semtimedop
> +case TARGET_NR_semtimedop:
> +return do_semtimedop(arg1, arg2, arg3, arg4);
> +#endif
>  #ifdef TARGET_NR_semctl
>  case TARGET_NR_semctl:
>  return do_semctl(arg1, arg2, arg3, arg4);
> 

You should also implement in do_ipc() the IPCOP_semtimedop operation.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH for 4.1 v2 7/7] linux-user: Add support for semtimedop() syscall

[Laurent Vivier]

Le 29/07/2019 à 15:26, Aleksandar Markovic a écrit :
> 
> 
> On Mon, Jul 29, 2019 at 3:11 PM Aleksandar Markovic
> mailto:aleksandar.marko...@rt-rk.com>>
> wrote:
> 
> From: Aleksandar Rikalo  >
> 
> Add support for semtimedop() emulation. It is based on invocation
> of safe_semtimedop().
> 
> Conversion is left out of safe_semtimedop(), since other safe_xxx()
> usually don't contain similar conversions.
> 
> Signed-off-by: Aleksandar Rikalo  >
> Signed-off-by: Aleksandar Markovic  >
> ---
> 
> 
> Aleksandar R., Laurent,
> 
> Please note that I just rebased the patch compared to its last
> incarnation - no code change.
> 
> Laurent's hints were as follows last time:
> 
> "To avoid duplicate code (and cleanup the stack allocation), you should
> 
> remove do_semop()  and call do_semtimedop(..., NULL) from IPCOP_semop
> and TARGET_NR_semop.
> 
> Thanks, Laurent"
> 
>  I guess they are still valid.

Yes, I didn't remember my comment and do the same again...

Thanks,
Laurent

Re: [Qemu-devel] [PATCH for-4.1] block/copy-on-read: Fix permissions for inactive node

[Eric Blake]

On 7/29/19 5:53 AM, Kevin Wolf wrote:
> The copy-on-read drive must not request the WRITE_UNCHANGED permission
> for its child if the node is inactive, otherwise starting a migration
> destination with -incoming will fail because the child cannot provide
> write access yet:
> 
>   qemu-system-x86_64: -blockdev copy-on-read,file=img,node-name=cor: Block 
> node is read-only
> 
> Earlier QEMU versions additionally ran into an abort() on the migration
> source side: bdrv_inactivate_recurse() failed to update permissions.
> This is silently ignored today because it was only supposed to loosen
> restrictions. This is the symptom that was originally reported here:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1733022
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/copy-on-read.c | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)

Do any of the iotests cover this?  Should they, especially if you are
trying to get this in for -rc3 tomorrow?

> 
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 22f24fd0db..6631f30205 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -56,16 +56,14 @@ static void cor_child_perm(BlockDriverState *bs, 
> BdrvChild *c,
> uint64_t perm, uint64_t shared,
> uint64_t *nperm, uint64_t *nshared)
>  {
> -if (c == NULL) {
> -*nperm = (perm & PERM_PASSTHROUGH) | BLK_PERM_WRITE_UNCHANGED;
> -*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
> -return;
> -}
> +*nperm = perm & PERM_PASSTHROUGH;
> +*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
>  
> -*nperm = (perm & PERM_PASSTHROUGH) |
> - (c->perm & PERM_UNCHANGED);
> -*nshared = (shared & PERM_PASSTHROUGH) |
> -   (c->shared_perm & PERM_UNCHANGED);

The old code unconditionally returned one set of permissions when c ==
NULL, or made a choice based on c's existing permissions on whether to
pass in those two bits.

> +/* We must not request write permissions for an inactive node, the child
> + * cannot provide it. */
> +if (!(bs->open_flags & BDRV_O_INACTIVE)) {
> +*nperm |= BLK_PERM_WRITE_UNCHANGED;
> +}

The new code changes the condition for or'ing in WRITE_UNCHANGED to
*nperm (it is no longer dependent on whether c == NULL, but whether the
drive is inactive), which matches your commit message.

But the new code also changes to always pass in the PERM_UNCHANGED to
*nshared; that used to be skipped if c was non-NULL and did not already
have the permission.  I don't follow that change from the commit
message, am I missing something?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 01/67] decodetree: Allow !function with no input bits

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> With this, we can have the function return a value from the DisasContext.
>
> Signed-off-by: Richard Henderson 
> ---
>  scripts/decodetree.py | 5 -
>  tests/decode/succ_function.decode | 2 ++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>  create mode 100644 tests/decode/succ_function.decode
>
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index d7a59d63ac..4259d87a95 100755
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -195,7 +195,10 @@ class MultiField:
>  """Class representing a compound instruction field"""
>  def __init__(self, subs, mask):
>  self.subs = subs
> -self.sign = subs[0].sign
> +if len(subs):
> +self.sign = subs[0].sign
> +else:
> +self.sign = 0
>  self.mask = mask
>
>  def __str__(self):
> diff --git a/tests/decode/succ_function.decode 
> b/tests/decode/succ_function.decode
> new file mode 100644
> index 00..632a9de252
> --- /dev/null
> +++ b/tests/decode/succ_function.decode
> @@ -0,0 +1,2 @@
> +%foo  !function=foo
> +foo    %foo
> --
> 2.17.1

Could you also update the documentation in docs/devel/decodetree.rst ?

This code change looks like it also now allows definitions
of fields that specify nothing at all (ie there's no check
that a field definition with no "unnamed_field" parts has
a !function specifier) -- what do they do, or should they
be made syntax errors ?

Is one of these functions which just returns a constant
from no input bits still a "static int func(DisasContext *s, int x)"
taking a pointless input argument, or is it now a
"static int func(DisasContext *s)" ? I guess from the fact
this code doesn't change the way a call is output that it
is the former, but would the latter be cleaner ?
(This would probably be implemented something like allowing
FunctionField to be passed a base == None instead of
allowing MultiFields with len(subs) == 0.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 02/67] target/arm: Remove offset argument to gen_exception_insn

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> The address of the current insn is still available in s->base.pc_next.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-vfp.inc.c |  6 +++---
>  target/arm/translate.c | 32 
>  2 files changed, 19 insertions(+), 19 deletions(-)

This works because all our uses of gen_exception_insn() are
for generating an exception at the PC of the current insn;
things like SVC/SMC/HVC which want to generate an exception
where the PC is that of the following insn are already handled
via a different function.

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 03/67] target/arm: Remove offset argument to gen_exception_bkpt_insn

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> The address of the current insn is still available in s->base.pc_next.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 33f78296eb..19b126d4f3 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -1256,12 +1256,12 @@ static void gen_exception_insn(DisasContext *s, int 
> excp, int syn,
>  s->base.is_jmp = DISAS_NORETURN;
>  }
>
> -static void gen_exception_bkpt_insn(DisasContext *s, int offset, uint32_t 
> syn)
> +static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)
>  {
>  TCGv_i32 tcg_syn;
>
>  gen_set_condexec(s);
> -gen_set_pc_im(s, s->pc - offset);
> +gen_set_pc_im(s, s->base.pc_next);
>  tcg_syn = tcg_const_i32(syn);
>  gen_helper_exception_bkpt_insn(cpu_env, tcg_syn);
>  tcg_temp_free_i32(tcg_syn);
> @@ -8139,7 +8139,7 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  case 1:
>  /* bkpt */
>  ARCH(5);
> -gen_exception_bkpt_insn(s, 4, syn_aa32_bkpt(imm16, false));
> +gen_exception_bkpt_insn(s, syn_aa32_bkpt(imm16, false));
>  break;
>  case 2:
>  /* Hypervisor call (v7) */
> @@ -11611,7 +11611,7 @@ static void disas_thumb_insn(DisasContext *s, 
> uint32_t insn)
>  {
>  int imm8 = extract32(insn, 0, 8);
>  ARCH(5);
> -gen_exception_bkpt_insn(s, 2, syn_aa32_bkpt(imm8, true));
> +gen_exception_bkpt_insn(s, syn_aa32_bkpt(imm8, true));
>  break;
>  }

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 04/67] target/arm: Remove offset argument to gen_exception_internal_insn

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> The actual argument is 0 for all callers.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 19b126d4f3..0848fb933a 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -1239,10 +1239,10 @@ static inline void gen_smc(DisasContext *s)
>  s->base.is_jmp = DISAS_SMC;
>  }
>
> -static void gen_exception_internal_insn(DisasContext *s, int offset, int 
> excp)
> +static void gen_exception_internal_insn(DisasContext *s, int excp)
>  {
>  gen_set_condexec(s);
> -gen_set_pc_im(s, s->pc - offset);
> +gen_set_pc_im(s, s->pc);
>  gen_exception_internal(excp);
>  s->base.is_jmp = DISAS_NORETURN;
>  }
> @@ -1294,7 +1294,7 @@ static inline void gen_hlt(DisasContext *s, int imm)
>  s->current_el != 0 &&
>  #endif
>  (imm == (s->thumb ? 0x3c : 0xf000))) {
> -gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
> +gen_exception_internal_insn(s, EXCP_SEMIHOST);
>  return;
>  }
>
> @@ -11984,7 +11984,7 @@ static bool arm_tr_breakpoint_check(DisasContextBase 
> *dcbase, CPUState *cpu,
>  /* End the TB early; it's likely not going to be executed */
>  dc->base.is_jmp = DISAS_TOO_MANY;
>  } else {
> -gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
> +gen_exception_internal_insn(dc, EXCP_DEBUG);
>  /* The address covered by the breakpoint must be
> included in [tb->pc, tb->pc + tb->size) in order
> to for it to be properly cleared -- thus we
> --
> 2.17.1


I'm not so convinced about this one -- gen_exception_insn()
and gen_exception_internal_insn() shouldn't have the
same pattern of function prototype but different semantics
like this, it's confusing. It happens that both the cases
of wanting to generate an "internal" exception happen to want
it to be taken with the PC being for the following insn,
not the current one, but that seems more coincidence to
me than anything else.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 05/67] target/arm: Use the saved value of the insn address

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> The address of the current insn is still available in s->base.pc_next,
> and need not be recomputed from s->pc - 4.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)


Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH] docs/nvdimm: add example on persistent backend setup

[Stefan Hajnoczi]

On Wed, Jul 24, 2019 at 03:03:07PM +0800, Wei Yang wrote:
> Persistent backend setup requires some knowledge about nvdimm and ndctl
> tool. Some users report they may struggle to gather these knowledge and
> have difficulty to setup it properly.
> 
> Here we provide two examples for persistent backend and gives the link
> to ndctl. By doing so, user could try it directly and do more
> investigation on persistent backend setup with ndctl.
> 
> Signed-off-by: Wei Yang 
> ---
>  docs/nvdimm.txt | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
> index b531cacd35..baba7a940d 100644
> --- a/docs/nvdimm.txt
> +++ b/docs/nvdimm.txt
> @@ -171,6 +171,32 @@ guest software that this vNVDIMM device contains a 
> region that cannot
>  accept persistent writes. In result, for example, the guest Linux
>  NVDIMM driver, marks such vNVDIMM device as read-only.
>  
> +Backend File Setup Example
> +..

For consistency with the rest of the document please use '-' instead of
'.'.

> +
> +Here is two examples for how to setup these persistent backend on
> +linux, which leverages the tool ndctl [3].

Small grammar tweaks:

  Here are two examples showing how to set up persistent backends on
  Linux using the tool ndctl [3].

> +
> +It is easy to setup DAX device backend file.

Please move this into the "A. DAX device" section and use it as an
introduction to explain what this section is about:

  Use the following command to set up /dev/dax0.0 so that the entirety
  of namespace0.0 can be exposed as an emulated NVDIMM to the guest:

> +
> +A. DAX device
> +
> +ndctl create-namespace -f -e namespace0.0 -m devdax
> +
> +The /dev/dax0.0 could be used directly in "mem-path" option.
> +
> +For DAX file, it is more than creating the proper namespace. The
> +block device should be partitioned and mounted (with dax option).

Please move this into "B. DAX file":

  Individual files on a DAX host file system can be exposed as emulated
  NVDIMMS.  First an fsdax block device is created, partitioned, and
  then mounted with the "dax" mount option:

> +
> +B. DAX file
> +
> +ndctl create-namespace -f -e namespace0.0 -m fsdax
> +(partition /dev/pmem0 with name pmem0p1)
> +mount -o dax /dev/pmem0p1 /mnt
> +(dd a file with proper size in /mnt)

"dd a file" could be "create or copy a disk image file with qemu-img(1),
cp(1), or dd(1) in /mnt".


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 06/67] target/arm: Introduce pc_read

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> We currently have 3 different ways of computing the architectural
> value of "PC" as seen in the ARM ARM.
>
> The value of s->pc has been incremented past the current insn,
> but that is all.  Thus for a32, PC = s->pc + 4; for t32, PC = s->pc;
> for t16, PC = s->pc + 2.  These differing computations make it
> impossible at present to unify the various code paths.
>
> Let s->pc_read hold the architectural value of PC for all cases.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.h | 10 
>  target/arm/translate.c | 53 ++
>  2 files changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index a20f6e2056..2dfdd8ca66 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -9,7 +9,17 @@ typedef struct DisasContext {
>  DisasContextBase base;
>  const ARMISARegisters *isar;
>
> +/*
> + * Summary of the various values for "PC":
> + * base.pc_next -- the start of the current insn
> + * pc   -- the start of the next insn

These are confusingly named -- logically "pc_next" ought to
be the PC of the next instruction and "pc" ought to be
that of the current one...

> + * pc_read  -- the value for "PC" in the ARM ARM;

nit: this line should end with a colon, rather than a semicolon

> + * in arm mode, the current insn + 8;
> + * in thumb mode, the current insn + 4;
> + * in aa64 mode, unused.
> + */
>  target_ulong pc;
> +target_ulong pc_read;
>  target_ulong page_start;
>  uint32_t insn;

Why target_ulong rather than uint32_t, given this is
aarch32 only?

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Paolo Bonzini]

On 29/07/19 14:41, Alex Bennée wrote:
>> I'd suggest we don't actually /need/ per-file compiler flags in most
>> cases. eg when we add  $foo.o-libs += $(FOO_LIBS) that's not really
>> a per-file setting when it gets expanded onto the final linker line.
>> Its just a "-lfoo" that gets used for the library as a while.
> We do for tests, often to select a specific processor type or feature we
> need to build that particular instance of the test.

Tests are also adding cflags per-executable, not per-.o file:

tests/tcg/aarch64/Makefile.softmmu-target:memory: CFLAGS+=-DCHECK_UNALIGNED=1
tests/tcg/alpha/Makefile.softmmu-target:memory: CFLAGS+=-DCHECK_UNALIGNED=0
tests/tcg/alpha/Makefile.target:test-cmov: EXTRA_CFLAGS=-DTEST_CMOV
tests/tcg/arm/Makefile.softmmu-target:test-armv6m-undef: 
EXTRA_CFLAGS+=-mcpu=cortex-m0
tests/tcg/arm/Makefile.target:hello-arm: CFLAGS+=-marm -ffreestanding
tests/tcg/arm/Makefile.target:test-arm-iwmmxt: CFLAGS+=-marm -march=iwmmxt 
-mabi=aapcs -mfpu=fpv4-sp-d16
tests/tcg/arm/Makefile.target:# fcvt: CFLAGS+=-march=armv8.2-a+fp16 
-mfpu=neon-fp-armv8
tests/tcg/i386/Makefile.softmmu-target:memory: CFLAGS+=-DCHECK_UNALIGNED=1
tests/tcg/i386/Makefile.target:hello-i386: CFLAGS+=-ffreestanding
tests/tcg/mips/Makefile.target:hello-mips: CFLAGS+=-mno-abicalls -fno-PIC 
-mabi=32

(They also do not use unnesting).

Paolo

Re: [Qemu-devel] [PATCH 07/67] target/arm: Introduce add_reg_for_lit

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> Used only on the thumb side so far, but will be more obvious
> once we start unifying the implementation of A32+T32.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-vfp.inc.c |  34 +--
>  target/arm/translate.c | 163 +++--
>  2 files changed, 76 insertions(+), 121 deletions(-)
>
> diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
> index e7389bc057..4066b2febf 100644
> --- a/target/arm/translate-vfp.inc.c
> +++ b/target/arm/translate-vfp.inc.c
> @@ -941,14 +941,7 @@ static bool trans_VLDR_VSTR_sp(DisasContext *s, 
> arg_VLDR_VSTR_sp *a)
>  offset = -offset;
>  }
>
> -if (s->thumb && a->rn == 15) {
> -/* This is actually UNPREDICTABLE */
> -addr = tcg_temp_new_i32();
> -tcg_gen_movi_i32(addr, s->pc & ~2);
> -} else {
> -addr = load_reg(s, a->rn);
> -}
> -tcg_gen_addi_i32(addr, addr, offset);
> +addr = add_reg_for_lit(s, a->rn, offset);
>  tmp = tcg_temp_new_i32();
>  if (a->l) {
>  gen_aa32_ld32u(s, tmp, addr, get_mem_index(s));
> @@ -983,14 +976,7 @@ static bool trans_VLDR_VSTR_dp(DisasContext *s, 
> arg_VLDR_VSTR_dp *a)
>  offset = -offset;
>  }
>
> -if (s->thumb && a->rn == 15) {
> -/* This is actually UNPREDICTABLE */
> -addr = tcg_temp_new_i32();
> -tcg_gen_movi_i32(addr, s->pc & ~2);
> -} else {
> -addr = load_reg(s, a->rn);
> -}
> -tcg_gen_addi_i32(addr, addr, offset);
> +addr = add_reg_for_lit(s, a->rn, offset);
>  tmp = tcg_temp_new_i64();
>  if (a->l) {
>  gen_aa32_ld64(s, tmp, addr, get_mem_index(s));
> @@ -1029,13 +1015,7 @@ static bool trans_VLDM_VSTM_sp(DisasContext *s, 
> arg_VLDM_VSTM_sp *a)
>  return true;
>  }
>
> -if (s->thumb && a->rn == 15) {
> -/* This is actually UNPREDICTABLE */
> -addr = tcg_temp_new_i32();
> -tcg_gen_movi_i32(addr, s->pc & ~2);
> -} else {
> -addr = load_reg(s, a->rn);
> -}
> +addr = add_reg_for_lit(s, a->rn, 0);
>  if (a->p) {
>  /* pre-decrement */
>  tcg_gen_addi_i32(addr, addr, -(a->imm << 2));
> @@ -1112,13 +1092,7 @@ static bool trans_VLDM_VSTM_dp(DisasContext *s, 
> arg_VLDM_VSTM_dp *a)
>  return true;
>  }
>
> -if (s->thumb && a->rn == 15) {
> -/* This is actually UNPREDICTABLE */
> -addr = tcg_temp_new_i32();
> -tcg_gen_movi_i32(addr, s->pc & ~2);
> -} else {
> -addr = load_reg(s, a->rn);
> -}
> +addr = add_reg_for_lit(s, a->rn, 0);
>  if (a->p) {
>  /* pre-decrement */
>  tcg_gen_addi_i32(addr, addr, -(a->imm << 2));
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index a48e9a90f8..5e2dd8bb16 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -214,6 +214,23 @@ static inline TCGv_i32 load_reg(DisasContext *s, int reg)
>  return tmp;
>  }
>
> +/*
> + * Create a new temp, incremented by OFS, except PC is aligned but not
> + * incremented for thumb.  This is used for load/store for which use of
> + * PC implies (literal), or ADD that implies ADR.
> + */
> +static TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs)
> +{
> +TCGv_i32 tmp = tcg_temp_new_i32();
> +
> +if (reg == 15) {
> +tcg_gen_movi_i32(tmp, (s->pc_read & ~3) + ofs);
> +} else {
> +tcg_gen_addi_i32(tmp, cpu_R[reg], ofs);
> +}
> +return tmp;
> +}

This is losing the information in the comments about the UNPREDICTABLE
cases. Are there callsites where the new function is called where
"thumb and reg == 15" is not UNPREDICTABLE, or are they all
that way?

thanks
-- PMM

[Qemu-devel] [Bug 1696773] Re: golang calls to exec crash user emulation

[Peter Maydell]

Note that this fix will be in the upcoming 4.1 release.

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

Title:
  golang calls to exec crash user emulation

Status in QEMU:
  Fix Committed

Bug description:
  An example program can be found here:

  https://github.com/willnewton/qemucrash

  This code starts a goroutine (thread) and calls exec repeatedly. This
  works ok natively but when run under ARM user emulation it segfaults
  (usually, there are occasionally other failures).

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

Re: [Qemu-devel] [PATCH 08/67] target/arm: Use store_reg_from_load in thumb2 code

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> There is an extra always-true ARMv5 test, but this will
> become more obvious once we start unifying the
> implementation of A32+T32.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.c | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 5e2dd8bb16..e316eeb312 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9773,13 +9773,11 @@ static void disas_thumb2_insn(DisasContext *s, 
> uint32_t insn)
>  /* Load.  */
>  tmp = tcg_temp_new_i32();
>  gen_aa32_ld32u(s, tmp, addr, get_mem_index(s));
> -if (i == 15) {
> -gen_bx_excret(s, tmp);
> -} else if (i == rn) {
> +if (i == rn) {
>  loaded_var = tmp;
>  loaded_base = 1;
>  } else {
> -store_reg(s, i, tmp);
> +store_reg_from_load(s, i, tmp);
>  }
>  } else {
>  /* Store.  */

I thought at first this would change behaviour if rn == 15
but in practice it doesn't because 15 is the end of the loop
anyway. (This is an UNPREDICTABLE case, but if they get in our
way it might be better to explicitly make them UNDEF rather than
just behaving like whatever falls out from the implementation.)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 09/67] target/arm: Fold a pc load into load_reg

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index e316eeb312..53c46fcdc4 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9161,11 +9161,7 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  }
>  } else {
>  /* store */
> -if (i == 15) {
> -/* special case: r15 = PC + 8 */
> -tmp = tcg_temp_new_i32();
> -tcg_gen_movi_i32(tmp, s->pc_read);
> -} else if (user) {
> +if (user && i != 15) {
>  tmp = tcg_temp_new_i32();
>  tmp2 = tcg_const_i32(i);
>  gen_helper_get_user_reg(tmp, cpu_env, tmp2);
> --
> 2.17.1

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH-4.2 v1 3/6] riscv: plic: Remove unused interrupt functions

[Philippe Mathieu-Daudé]

On 7/25/19 8:52 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis 
> ---
>  hw/riscv/sifive_plic.c | 12 
>  include/hw/riscv/sifive_plic.h |  3 ---
>  2 files changed, 15 deletions(-)
> 
> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
> index 0950e89e15..864a1bed42 100644
> --- a/hw/riscv/sifive_plic.c
> +++ b/hw/riscv/sifive_plic.c
> @@ -161,18 +161,6 @@ static void sifive_plic_update(SiFivePLICState *plic)
>  }
>  }
>  
> -void sifive_plic_raise_irq(SiFivePLICState *plic, uint32_t irq)
> -{
> -sifive_plic_set_pending(plic, irq, true);
> -sifive_plic_update(plic);
> -}
> -
> -void sifive_plic_lower_irq(SiFivePLICState *plic, uint32_t irq)
> -{
> -sifive_plic_set_pending(plic, irq, false);
> -sifive_plic_update(plic);
> -}
> -
>  static uint32_t sifive_plic_claim(SiFivePLICState *plic, uint32_t addrid)
>  {
>  int i, j;
> diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h
> index ce8907f6aa..3b8a623919 100644
> --- a/include/hw/riscv/sifive_plic.h
> +++ b/include/hw/riscv/sifive_plic.h
> @@ -69,9 +69,6 @@ typedef struct SiFivePLICState {
>  uint32_t aperture_size;
>  } SiFivePLICState;
>  
> -void sifive_plic_raise_irq(SiFivePLICState *plic, uint32_t irq);
> -void sifive_plic_lower_irq(SiFivePLICState *plic, uint32_t irq);
> -
>  DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
>  uint32_t num_sources, uint32_t num_priorities,
>  uint32_t priority_base, uint32_t pending_base,
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH-4.2 v1 4/6] target/riscv: Create function to test if FP is enabled

[Philippe Mathieu-Daudé]

On 7/25/19 8:52 PM, Alistair Francis wrote:
> Let's creaate a function that tests if floating point support is

"create"

> enabled. We can then protect all floating point operations based on if
> they are enabled.
> 
> This patch so far doesn't change anything, it's just preparing for the
> Hypervisor support for floating point operations.
> 
> Signed-off-by: Alistair Francis 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/riscv/cpu.h|  6 +-
>  target/riscv/cpu_helper.c | 10 ++
>  target/riscv/csr.c| 19 ++-
>  3 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0adb307f32..2dc9b17678 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -255,6 +255,7 @@ void riscv_cpu_do_interrupt(CPUState *cpu);
>  int riscv_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
> +bool riscv_cpu_fp_enabled(CPURISCVState *env);
>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
>  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> @@ -298,7 +299,10 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
> *env, target_ulong *pc,
>  #ifdef CONFIG_USER_ONLY
>  *flags = TB_FLAGS_MSTATUS_FS;
>  #else
> -*flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
> +*flags = cpu_mmu_index(env, 0);
> +if (riscv_cpu_fp_enabled(env)) {
> +*flags |= env->mstatus & MSTATUS_FS;
> +}
>  #endif
>  }
>  
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f027be7f16..225e407cff 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -71,6 +71,16 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
> interrupt_request)
>  
>  #if !defined(CONFIG_USER_ONLY)
>  
> +/* Return true is floating point support is currently enabled */
> +bool riscv_cpu_fp_enabled(CPURISCVState *env)
> +{
> +if (env->mstatus & MSTATUS_FS) {
> +return true;
> +}
> +
> +return false;
> +}
> +
>  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
>  {
>  CPURISCVState *env = &cpu->env;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index af3b762c8b..7b73b73cf7 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -46,7 +46,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
>  static int fs(CPURISCVState *env, int csrno)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>  return -1;
>  }
>  #endif
> @@ -108,7 +108,7 @@ static int pmp(CPURISCVState *env, int csrno)
>  static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>  return -1;
>  }
>  #endif
> @@ -119,7 +119,7 @@ static int read_fflags(CPURISCVState *env, int csrno, 
> target_ulong *val)
>  static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>  return -1;
>  }
>  env->mstatus |= MSTATUS_FS;
> @@ -131,7 +131,7 @@ static int write_fflags(CPURISCVState *env, int csrno, 
> target_ulong val)
>  static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>  return -1;
>  }
>  #endif
> @@ -142,7 +142,7 @@ static int read_frm(CPURISCVState *env, int csrno, 
> target_ulong *val)
>  static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>  return -1;
>  }
>  env->mstatus |= MSTATUS_FS;
> @@ -154,7 +154,7 @@ static int write_frm(CPURISCVState *env, int csrno, 
> target_ulong val)
>  static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>  return -1;
>  }
>  #endif
> @@ -166,7 +166,7 @@ static int read_fcsr(CPURISCVState *env, int csrno, 
> target_ulong *val)
>  static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +if (!env->debugger

Re: [Qemu-devel] [PATCH 10/67] target/arm: Move test for AL into arm_skip_unless

[Peter Maydell]

On Fri, 26 Jul 2019 at 18:50, Richard Henderson
 wrote:
>
> We will shortly be calling this function much more often.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate.c | 28 
>  1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 53c46fcdc4..36419025db 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -7705,8 +7705,14 @@ static void arm_gen_condlabel(DisasContext *s)
>  /* Skip this instruction if the ARM condition is false */
>  static void arm_skip_unless(DisasContext *s, uint32_t cond)
>  {
> -arm_gen_condlabel(s);
> -arm_gen_test_cc(cond ^ 1, s->condlabel);
> +/*
> + * Conditionally skip the insn. Note that both 0xe and 0xf mean
> + * "always"; 0xf is not "never".
> + */
> +if (cond < 0xe) {
> +arm_gen_condlabel(s);
> +arm_gen_test_cc(cond ^ 1, s->condlabel);
> +}
>  }
>
>  static void disas_arm_insn(DisasContext *s, unsigned int insn)
> @@ -7944,11 +7950,9 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  }
>  goto illegal_op;
>  }
> -if (cond != 0xe) {
> -/* if not always execute, we generate a conditional jump to
> -   next instruction */
> -arm_skip_unless(s, cond);
> -}
> +
> +arm_skip_unless(s, cond);
> +
>  if ((insn & 0x0f90) == 0x0300) {
>  if ((insn & (1 << 21)) == 0) {
>  ARCH(6T2);
> @@ -12095,15 +12099,7 @@ static void thumb_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cpu)
>  dc->insn = insn;
>
>  if (dc->condexec_mask && !thumb_insn_is_unconditional(dc, insn)) {
> -uint32_t cond = dc->condexec_cond;
> -
> -/*
> - * Conditionally skip the insn. Note that both 0xe and 0xf mean
> - * "always"; 0xf is not "never".
> - */
> -if (cond < 0x0e) {
> -arm_skip_unless(dc, cond);
> -}
> +arm_skip_unless(dc, dc->condexec_cond);
>  }

In the other callsites for arm_skip_unless() the cond argument
can never be 0xe or 0xf.

Reviewed-by: Peter Maydell 

thanks
-- PMM