Re: [Qemu-devel] [PATCH for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU

[Philippe Mathieu-Daudé]

Le ven. 22 mars 2019 00:33, Laszlo Ersek  a écrit :

> On 03/21/19 23:32, Laszlo Ersek wrote:
>
> > Cool, so let me try this. I'm going to download the xz.old file
> > manually. Rename it to just xz. It will then match the built-in
> > checksum, and will be used as a cached copy. Then I will try building my
> > series in *that* ("old") VM.
>
> Summary:
>
> (1) The image file at
>  has been recently
> uploaded ("Last-Modified: Wed, 27 Feb 2019 11:48:18 GMT") by someone
> unknown to me, and its sha256sum doesn't match the sha256sum in the
> "tests/vm/openbsd" test script.
>
> This is why my earlier attempts at the OpenBSD build test have failed.
>

Can someone include Fam/Paolo/Brad in this thread please? (I don't have
their emails in my cellphone). Thanks.

And in fact I don't understand how it could work for anyone else -- the
> compiler that the "tests/vm/openbsd" script specifies is neither
> installed, nor available with "pkg_add", in this image.
>
>
> (2) Against the "old" image
> , which indeed
> has the expected
> sha256sum=8c6cedc483e602cfee5e04f0406c64eb99138495e8ca580bc0293bcf0640c1bf,
> the build test *does* succeed.
>
> (
>
> In order to make use of the old image, it has to be downloaded manually,
> then moved/renamed to:
>
>   $HOME/.cache/qemu-vm/download/bc4733f6c6e76931702528a515a1bf70eb8baecd
>
> because the last filename component must be the sha1sum of the URL
> itself, for the caching mechanism to recognize the compressed image:
>
> > $ echo -n 'http://download.patchew.org/openbsd-6.1-amd64.img.xz' \
> >   | sha1sum
> > bc4733f6c6e76931702528a515a1bf70eb8baecd  -
>
> )
>
> I'm attaching the log of the successful OpenBSD build test, which I
> captured with "screen" (see the "BUNZIP2" lines in it, in particular).
>
> Thanks,
> Laszlo
>

Re: [Qemu-devel] [PATCH v3 14/15] spapr/irq: initialize the IRQ device only once

[Cédric Le Goater]

On 3/22/19 3:15 AM, David Gibson wrote:
> On Thu, Mar 21, 2019 at 03:49:13PM +0100, Cédric Le Goater wrote:
>> Add a check to make sure that the routine initializing the emulated
>> IRQ device is called once. We don't have much to test on the XICS
>> side, so we introduce a 'static bool'. Not very elegant but works well
>> enough.
> 
> What's the rationale for this?

The rationale for initializing the IRQ device only once is that reset
will do the initialization when KVM support is added for the dual 
interrupt mode. When using the emulated device, some inits can only be
done once. I should have probably added this patch at the end of the 
patchset. 

The rationale for the 'static bool' is pragmatism. Others solutions
could be to  :

  1. add a new 'init' attribute under ICSState
  2. modify spapr_register_hypercall() and spapr_rtas_register()
 to allow multiple definitions
  3. add a spapr_is_registered_hypercall() helper
  4. add a device fini routine (but it doesn't work for XIVE)

I picked the most obvious and less intrusive one. solution 1 was
my next favored one.

C.
   

>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive.c |  9 +
>>  hw/intc/xics_spapr.c | 10 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index f889c89dc2e9..15d41d9cd36d 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -337,6 +337,15 @@ void spapr_xive_init(SpaprXive *xive, Error **errp)
>>  XiveSource *xsrc = &xive->source;
>>  XiveENDSource *end_xsrc = &xive->end_source;
>>  
>> +/*
>> + * The emulated XIVE device can only be initialized once. If the
>> + * ESB memory region has been already mapped, it means we have been
>> + * through there.
>> + */
>> +if (memory_region_is_mapped(&xsrc->esb_mmio)) {
>> +return;
>> +}
>> +
>>  /* TIMA initialization */
>>  memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops, xive,
>>"xive.tima", 4ull << TM_SHIFT);
>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
>> index 9d2b8adef7c5..67e82f3720b0 100644
>> --- a/hw/intc/xics_spapr.c
>> +++ b/hw/intc/xics_spapr.c
>> @@ -239,6 +239,16 @@ static void rtas_int_on(PowerPCCPU *cpu, 
>> SpaprMachineState *spapr,
>>  
>>  void xics_spapr_init(SpaprMachineState *spapr)
>>  {
>> +static bool init_done;
>> +
>> +/*
>> + * Emulated mode can only be initialized once.
>> + */
>> +if (init_done) {
>> +return;
>> +}
>> +init_done = true;
>> +
>>  /* Registration of global state belongs into realize */
>>  spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
>>  spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
> 

Re: [Qemu-devel] [PATCH v3 03/15] spapr/xive: add hcall support when under KVM

[Cédric Le Goater]

On 3/22/19 1:51 AM, David Gibson wrote:
> On Thu, Mar 21, 2019 at 03:49:02PM +0100, Cédric Le Goater wrote:
>> XIVE hcalls are all redirected to QEMU as none are on a fast path.
>> When necessary, QEMU invokes KVM through specific ioctls to perform
>> host operations. QEMU should have done the necessary checks before
>> calling KVM and, in case of failure, H_HARDWARE is simply returned.
>>
>> H_INT_ESB is a special case that could have been handled under KVM
>> but the impact on performance was low when under QEMU. Here are some
>> figures :
>>
>> kernel irqchip  OFF  ON
>> H_INT_ESBKVM   QEMU
>>
>> rtl8139 (LSI )  1.19 1.24  1.23  Gbits/sec
>> virtio 31.8042.30   --   Gbits/sec
>>
>> Signed-off-by: Cédric Le Goater 
> 
> Reviewed-by: David Gibson 
> 
>> ---
>>
>>  What has not changed :
>>
>>  - Memory barriers have not been included. Only loads are performed on
>>the ESB management page (no stores for now) and stores are done
>>only on the trigger page. We should be fine as for the load
>>ordering on the management page.
> 
> I'm not entirely sure I'm convinced by this reasoning, but I'll take
> your word for it.
> 

I had a word with our HW designer and we can only have one load outstanding 
per physical thread, so multiple CI loads will go out as ordered.

C. 


Re: [Qemu-devel] [RFC for-4.1 00/25] Many style fixes for target/ppc

[Markus Armbruster]

David Gibson  writes:

> target/ppc has a lot of old code that doesn't stick to the modern
> style guidelines.  That means we keep getting checkpatch warnings from
> code motions in there, or from people copying the local style rather
> than the global style.
>
> I'm sick of it, so here's a big series to fix many of the style
> problems in target/ppc.
>
> It doesn't cover every checkpatch warning: outright false positives
> are ignored of course, but there are also some things that it's not
> obvious how to fix (often involving hairy nested macros).  Still, it's
> a good start.

I reply because you marked this RFC, which indicates some uncertainty.
I'd like to encourage you.

The more bad examples exist in the code, the more time we'll waste on
correcting them in patch submission.  You're sick of it.  You're right.

We've demanded compliance to the QEMU coding style since 2009 (commit
e68b98dc723).  If we had started to clean up existing code back then,
we'd be long done by now, and style cleanup's annoying impact on
git-blame would've long decayed to irrelevant levels.

I expect the only regrets you'll have about this series is not to have
done it earlier.

Diffstat without the patches you included by mistake:

 26 files changed, 1210 insertions(+), 802 deletions(-)

This is actually a rather small fraction of the PPC code:

$ git-ls-files hw/ppc/ include/hw/ppc target/ppc/ | xargs wc | tail -n 1
  90193  284261 3087364 total
$ git-ls-files hw/ppc/ include/hw/ppc target/ppc/ | wc -l
128

[Qemu-devel] [Bug 1819649] Re: Win10 Preview Build 18351 - no input

[Mira Limbeck]

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

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

Title:
  Win10 Preview Build 18351 - no input

Status in QEMU:
  Invalid

Bug description:
  The issue exists in both 2.12.1 and current master (built 8.3.2019).
  Neither keyboard nor mouse input seems to work and there's no cursor
  available (VNC, SPICE, SDL tested). If all the 'hv_*' flags are
  removed input works again.

  In the attachments you can find a simple script to reproduce it
  (requires the ISO path to be changed).

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

Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node

[Vladimir Sementsov-Ogievskiy]

On 21.03.2019 20:05, Andrey Shinkevich wrote:
> 
> 
> On 21/03/2019 13:53, Kevin Wolf wrote:
>> Am 20.03.2019 um 18:02 hat Alberto Garcia geschrieben:
>>> On Wed 20 Mar 2019 10:16:10 AM CET, Kevin Wolf wrote:
> Oh, I see. Let's use a shorter chain for simplicity:
>
>  A <- B <- C <- D <- E

 Written from right to left, i.e. E being the base and A the top layer?
 We usually write things the other write round, I hope this doesn't get
 too confusing later.
>>>
>>> Oh my... yes, of course you're right, I should have written it the other
>>> way around:
>>>
>>>  E <- D <- C <- B <- A
>>>
> 1) If we stream first from E to C we add a filter F:
>
>  A <- B <- F <- C <- D <- E
>>>
>>> ( which should have been written   E <- D <- C <- F <- B <- A )
>>>
>  Now we can't stream from C to A because F is on the way, and the F-C
>  link is frozen.

 Why is a frozen link a problem? The streaming operation isn't going to
 change this link, it just copies data from the subchain (including F
 and C) to A. This is not something that a frozen link should prevent.
>>>
>>> Not the operation itself, but the first thing that block-stream does is
>>> freeze the chain from top (A) to base (C), so this would fail if there's
>>> already a frozen link on the way (C <- F on this case?).
>>
>> Oh, I see. I think this is why I suggested a counter originally instead
>> of a bool.
>>
 So it seems frozen links allow the wrong case, but block the correct
 one? :-(
>>>
>>> Yes, we probably need to rethink this scenario a bit.
>>
>> But yes, even with a counter, the other problem would still remain (that
>> the first job can't remove the filter on completion because the second
>> one has frozen its link to the filter).
> 
> With this example E <- D <- C <- F <- B <- A,
> 
> In the current implementation of the copy-on-read filter,
> its bs->backing is not initialized (while it is not true for the filter
> in block-commit job). So, bdrv_freeze_backing_chain() doesn't go beyond
> the cor-filter node. With the two parallel block-stream jobs, we get the
> following sub-chains frozen:
> F <- B <- A
> E <- D <- C
> as C <- F backing BdrvChild link doesn't exist.
> If the cor-filter is inserted with the bdrv_append(), we get two
> BdrvChild links (file and backing) pointed to the same BlockDriverState
> 'C' and additionally some child-permissions issues that I have not
> resolved yet...
> Due to the fact mentioned above, freezing the backing chain works with
> the filter inserted. But, with the one BdrvChild *file link only in the
> BlockDriverState of the cor-filter, we encounter a broken chain each
> time we iterate through it with the backing_bs(F) (=NULL) in many other
> pieces of the code. In turn, it breaks the existing model.
> That's the point! (((
> What can we do with that?
> 
> In my patch Stream-block-job-involves-copy-on-read-filter.patch :
> 
> static BlockDriverState *child_file_bs(BlockDriverState *bs)
> {
>   return bs->file ? bs->file->bs : NULL;
> }
> 
> static BlockDriverState *skip_filter(BlockDriverState *bs)
> {
>   BlockDriverState *ret_bs = bs;
> 
>   while (ret_bs && ret_bs->drv && ret_bs->drv->is_filter) {
>   ret_bs = child_file_bs(ret_bs);
>   }
> 
>   return ret_bs;
> }
> 
> But the solution above looks clumsy to me.
> I would appreciate to hear any other ideas from you.
> 

As I understand, we can't drop .file child from copy-on-read and use 
backing instead as it'll break backward compatibility. So I propose to
support backing child in COR, so that it may operate through .file or
through .backing, depending on what it has. (But forbid creating both
children)

[Qemu-devel] [PATCH] device_tree: check device tree blob file size

[P J P]

From: Prasad J Pandit 

Device tree blob(dtb) file can not be larger than 2MB in size.[*]
Add check to avoid loading large dtb files in load_device_tree(),
and potential integer(dt_size) overflow.

[*] linux.git/tree/Documentation/arm64/booting.txt

Reported-by: Kurtis Miller 
Signed-off-by: Prasad J Pandit 
---
 device_tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 296278e12a..9059ee5545 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -79,9 +79,9 @@ void *load_device_tree(const char *filename_path, int *sizep)
 
 *sizep = 0;
 dt_size = get_image_size(filename_path);
-if (dt_size < 0) {
-error_report("Unable to get size of device tree file '%s'",
- filename_path);
+if (dt_size < 0 || dt_size > FDT_MAX_SIZE) {
+error_report("Invalid size of device tree file: %s: %d",
+ filename_path, dt_size);
 goto fail;
 }
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH v3 04/15] spapr/xive: add state synchronization with KVM

[Cédric Le Goater]

On 3/22/19 2:00 AM, David Gibson wrote:
> On Thu, Mar 21, 2019 at 03:49:03PM +0100, Cédric Le Goater wrote:
>> This extends the KVM XIVE device backend with 'synchronize_state'
>> methods used to retrieve the state from KVM. The HW state of the
>> sources, the KVM device and the thread interrupt contexts are
>> collected for the monitor usage and also migration.
>>
>> These get operations rely on their KVM counterpart in the host kernel
>> which acts as a proxy for OPAL, the host firmware. The set operations
>> will be added for migration support later.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>
>>  Changes since v2:
>>
>>  - removed the capture of the OS CAM line value from KVM
>>  - added xive_end_is_valid() check
>>
>>  include/hw/ppc/spapr_xive.h |  8 
>>  include/hw/ppc/xive.h   |  1 +
>>  hw/intc/spapr_xive.c| 17 +---
>>  hw/intc/spapr_xive_kvm.c| 87 +
>>  hw/intc/xive.c  | 10 +
>>  5 files changed, 116 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index 03685910e76b..7e49badd8c9a 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -44,6 +44,13 @@ typedef struct SpaprXive {
>>  void  *tm_mmap;
>>  } SpaprXive;
>>  
>> +/*
>> + * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
>> + * to the controller block id value. It can nevertheless be changed
>> + * for testing purpose.
>> + */
>> +#define SPAPR_XIVE_BLOCK_ID 0x0
>> +
>>  bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi);
>>  bool spapr_xive_irq_free(SpaprXive *xive, uint32_t lisn);
>>  void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
>> @@ -74,5 +81,6 @@ void kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t 
>> end_blk,
>>  void kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk,
>>   uint32_t end_idx, XiveEND *end,
>>   Error **errp);
>> +void kvmppc_xive_synchronize_state(SpaprXive *xive, Error **errp);
>>  
>>  #endif /* PPC_SPAPR_XIVE_H */
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index dd115da30ebc..78c919c4a590 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -435,5 +435,6 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, int 
>> srcno, Error **errp);
>>  void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp);
>>  void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val);
>>  void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp);
>> +void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp);
>>  
>>  #endif /* PPC_XIVE_H */
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index cde2fd7c8997..4d07140f1078 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -40,13 +40,6 @@
>>  
>>  #define SPAPR_XIVE_NVT_BASE 0x400
>>  
>> -/*
>> - * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
>> - * to the controller block id value. It can nevertheless be changed
>> - * for testing purpose.
>> - */
>> -#define SPAPR_XIVE_BLOCK_ID 0x0
>> -
>>  /*
>>   * sPAPR NVT and END indexing helpers
>>   */
>> @@ -156,6 +149,16 @@ void spapr_xive_pic_print_info(SpaprXive *xive, Monitor 
>> *mon)
>>  XiveSource *xsrc = &xive->source;
>>  int i;
>>  
>> +if (kvm_irqchip_in_kernel()) {
>> +Error *local_err = NULL;
>> +
>> +kvmppc_xive_synchronize_state(xive, &local_err);
>> +if (local_err) {
>> +error_report_err(local_err);
>> +return;
>> +}
>> +}
>> +
>>  monitor_printf(mon, "  LSIN PQEISN CPU/PRIO EQ\n");
>>  
>>  for (i = 0; i < xive->nr_irqs; i++) {
>> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
>> index 2714f8e4702e..2e46661cb3ad 100644
>> --- a/hw/intc/spapr_xive_kvm.c
>> +++ b/hw/intc/spapr_xive_kvm.c
>> @@ -60,6 +60,51 @@ static void kvm_cpu_enable(CPUState *cs)
>>  /*
>>   * XIVE Thread Interrupt Management context (KVM)
>>   */
>> +static void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp)
>> +{
>> +uint64_t state[2] = { 0 };
>> +int ret;
>> +
>> +ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_VP_STATE, state);
>> +if (ret != 0) {
>> +error_setg_errno(errp, errno,
>> + "XIVE: could not capture KVM state of CPU %ld",
>> + kvm_arch_vcpu_id(tctx->cs));
>> +return;
>> +}
>> +
>> +/* word0 and word1 of the OS ring. */
>> +*((uint64_t *) &tctx->regs[TM_QW1_OS]) = state[0];
>> +}
>> +
>> +typedef struct {
>> +XiveTCTX *tctx;
>> +Error *err;
>> +} XiveCpuGetState;
>> +
>> +static void kvmppc_xive_cpu_do_synchronize_state(CPUState *cpu,
>> + run_on_cpu_data arg)
>> +{
>> +XiveCpuGetState *s = arg.host_ptr;
>> +
>> +kvmppc_xive_cpu_get_sta

Re: [Qemu-devel] [PATCH v1 0/3 for 4.0] reduce timeouts on Travis

[Stefano Garzarella]

On Tue, Mar 19, 2019 at 12:47:57PM +, Alex Bennée wrote:
> 
> Hi,
> 
> This is a fixup patch for 4.0 to help reduce the number of timeouts we
> are seeing on Travis. We introduce a new way to slice the target list
> and then use that to split up a few of the builds that are getting
> close to the time limit.
> 
> Alex Bennée (3):
>   configure: add --target-list-exclude
>   .travis.yml: split some more system builds
>   .travis.yml: --disable-user for --without-default-devices
> 
>  .travis.yml | 25 ++---
>  configure   | 34 +++---
>  2 files changed, 49 insertions(+), 10 deletions(-)

Yeah, with this series plus your "[PATCH] .travis.yml: reduce number of
targets built while disabling things" all jobs are able to complete in
time :)


Acked-by: Stefano Garzarella 
Tested-by: Stefano Garzarella 

Thanks,
Stefano

Re: [Qemu-devel] [PATCH] migration: avoid copying ignore-shared ramblock when in incoming migration

[Peter Maydell]

On Thu, 21 Mar 2019 at 15:32, Catherine Ho  wrote:
> So in igore-shared case, dtb is not required here ?

Can you explain what the "ignore-shared" case is?

In general, when we reset the system, we want to bring it
back to exactly the state that it was in when QEMU was
first started. That means we need to reload all the rom blob
data into memory (because the guest might have modified
that memory while it was running).

If I understand correctly from other threads, the idea is
that some of the RAM is shared between source and destination
so it does not need to be manually copied during migration.
If that is correct, then perhaps the right thing is that
in the rom_reset code:
 * if this rom blob is being loaded into a "shared" ram block
 * and this reset is happening specifically before an
   inbound migration
 * then skip loading the rom blob data

?

I don't know the right way to determine either the "is this
a shared ram area" or "is this the reset prior to inbound
migration", but perhaps you can fill that in.

> Maybe I could add a flag in struct Rom to set it when the rom is created by
> rom_add_blob_fixed_as()? And in rom_reset, just bypass this rom when
> in_incoming_migration && ignore_shared

No, I think this is wrong -- the particular function used
to create the rom blob data should not affect how we are
treating it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] migration: avoid copying ignore-shared ramblock when in incoming migration

[Peter Maydell]

On Thu, 21 Mar 2019 at 06:20, Peter Xu  wrote:
> I tried to dig on how x86 implemented the "-kernel" parameter and I
> noticed that it's dumping the kernel image into fw_cfg and probably
> that's also the reason why this should not be a problem for x86
> because rom reset on x86 won't overwrite the image.  Meanwhile, it
> seems totally different comparing to what has been done by ARM, which
> should probably be using rom_add_elf_program() to load the image if my
> reading is correct, so the kernel image is a ROM on ARM even if it can
> be written... Is my understanding correct?

The "usual" case for x86 passes the kernel through the fw_cfg
device, whereas the "usual" case for arm has rom-blobs that are
written directly to the backing RAMBlock for the flash device
or for real RAM. But it's possible also to have x86 configurations
and command lines which write to the real RAM. For instance if
you use the 'generic loader' device (documented in
docs/generic-loader.txt) to load auxiliary data files into RAM
on x86 you will hit the same problem.

> With that, I still feel that hacking into the memory write functions
> are tricky and I feel like it could bring hard to debug issues.  Would
> it be possible that we identify somehow that this ROM is a fake one
> (since it can actually be written) and we ignore the reset of it when
> proper (e.g., the initial reset on destination)?

I don't know what you mean by 'fake ROM' and anyway
this all applies to RAM as well...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] device_tree: check device tree blob file size

[Peter Maydell]

On Fri, 22 Mar 2019 at 07:38, P J P  wrote:
>
> From: Prasad J Pandit 
>
> Device tree blob(dtb) file can not be larger than 2MB in size.[*]
> Add check to avoid loading large dtb files in load_device_tree(),
> and potential integer(dt_size) overflow.
>
> [*] linux.git/tree/Documentation/arm64/booting.txt

This document is specific to aarch64, but the part of
QEMU's device tree code being modified here is
architecture independent.

Cc'ing David Gibson who will probably know if there is
an architecture-independent limit on DTB size we should
be enforcing, or whether we are better just to have a check
that avoids the overflow.

It's also worth noting in the commit message that this is
not a security problem -- even if the "add 1 and double"
calculation overflows, the load_image_size() function will
not load more data into the buffer than will fit, so the
behaviour will be to truncate the DTB.

> Reported-by: Kurtis Miller 
> Signed-off-by: Prasad J Pandit 
> ---
>  device_tree.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index 296278e12a..9059ee5545 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -79,9 +79,9 @@ void *load_device_tree(const char *filename_path, int 
> *sizep)
>
>  *sizep = 0;
>  dt_size = get_image_size(filename_path);
> -if (dt_size < 0) {
> -error_report("Unable to get size of device tree file '%s'",
> - filename_path);
> +if (dt_size < 0 || dt_size > FDT_MAX_SIZE) {
> +error_report("Invalid size of device tree file: %s: %d",
> + filename_path, dt_size);
>  goto fail;
>  }

thanks
-- PMM

[Qemu-devel] [RFT 1/4] virtio-bus: introduce a new method for querying the queue status

[Jason Wang]

Sometime we need to check whether a queue is enabled, e.g for vhost,
we don't want to start the backend for the virtqueues that is not
enabled. So introduce a new method to do this.

Signed-off-by: Jason Wang 
---
 include/hw/virtio/virtio-bus.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 7fec9dc929..db20fecf13 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -84,6 +84,10 @@ typedef struct VirtioBusClass {
  */
 int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
 int n, bool assign);
+/*
+ * is the queue enabled?
+ */
+bool (*queue_enabled)(DeviceState *d, int n);
 /*
  * Does the transport have variable vring alignment?
  * (ie can it ever call virtio_queue_set_align()?)
-- 
2.19.1

[Qemu-devel] [RFT 4/4] vhost_net: don't start vhost for the virtqueue that is not enabled

[Jason Wang]

According to the spec, device should not use the virtqueues that is
not enabled. So this patch just try to obey the spec by checking
whether queue is enabled before starting it.

Signed-off-by: Jason Wang 
---
 hw/virtio/vhost.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7f61018f2a..b61d659a35 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1580,6 +1580,9 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, 
uint16_t queue_size,
 /* Host notifiers must be enabled at this point. */
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
+BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+VirtioBusState *vbus = VIRTIO_BUS(qbus);
+VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
 int i, r;
 
 /* should only be called after backend is connected */
@@ -1604,6 +1607,10 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 goto fail_mem;
 }
 for (i = 0; i < hdev->nvqs; ++i) {
+if (k->queue_enabled &&
+!k->queue_enabled(qbus->parent, hdev->vq_index + i)) {
+continue;
+}
 r = vhost_virtqueue_start(hdev,
   vdev,
   hdev->vqs + i,
@@ -1645,6 +1652,10 @@ fail_log:
 vhost_log_put(hdev, false);
 fail_vq:
 while (--i >= 0) {
+if (k->queue_enabled &&
+!k->queue_enabled(qbus->parent, hdev->vq_index + i)) {
+continue;
+}
 vhost_virtqueue_stop(hdev,
  vdev,
  hdev->vqs + i,
-- 
2.19.1

Re: [Qemu-devel] [PATCH for-4.1 v3 00/12] bundle edk2 platform firmware with QEMU

[Laszlo Ersek]

On 03/22/19 08:02, Philippe Mathieu-Daudé wrote:
> Le ven. 22 mars 2019 00:33, Laszlo Ersek  a écrit :
> 
>> On 03/21/19 23:32, Laszlo Ersek wrote:
>>
>>> Cool, so let me try this. I'm going to download the xz.old file
>>> manually. Rename it to just xz. It will then match the built-in
>>> checksum, and will be used as a cached copy. Then I will try building my
>>> series in *that* ("old") VM.
>>
>> Summary:
>>
>> (1) The image file at
>>  has been recently
>> uploaded ("Last-Modified: Wed, 27 Feb 2019 11:48:18 GMT") by someone
>> unknown to me, and its sha256sum doesn't match the sha256sum in the
>> "tests/vm/openbsd" test script.
>>
>> This is why my earlier attempts at the OpenBSD build test have failed.
>>
> 
> Can someone include Fam/Paolo/Brad in this thread please? (I don't have
> their emails in my cellphone). Thanks.

Done.

Thanks
Laszlo

>> And in fact I don't understand how it could work for anyone else -- the
>> compiler that the "tests/vm/openbsd" script specifies is neither
>> installed, nor available with "pkg_add", in this image.
>>
>>
>> (2) Against the "old" image
>> , which indeed
>> has the expected
>> sha256sum=8c6cedc483e602cfee5e04f0406c64eb99138495e8ca580bc0293bcf0640c1bf,
>> the build test *does* succeed.
>>
>> (
>>
>> In order to make use of the old image, it has to be downloaded manually,
>> then moved/renamed to:
>>
>>   $HOME/.cache/qemu-vm/download/bc4733f6c6e76931702528a515a1bf70eb8baecd
>>
>> because the last filename component must be the sha1sum of the URL
>> itself, for the caching mechanism to recognize the compressed image:
>>
>>> $ echo -n 'http://download.patchew.org/openbsd-6.1-amd64.img.xz' \
>>>   | sha1sum
>>> bc4733f6c6e76931702528a515a1bf70eb8baecd  -
>>
>> )
>>
>> I'm attaching the log of the successful OpenBSD build test, which I
>> captured with "screen" (see the "BUNZIP2" lines in it, in particular).
>>
>> Thanks,
>> Laszlo
>>
> 

[Qemu-devel] [RFT 2/4] virtio-pci: set enabled for legacy device

[Jason Wang]

We don't set enabled for legacy device, this turns out to be an issue
if we want to check it through vhost. So just simply set it when
setting pa of a virtqueue.

Signed-off-by: Jason Wang 
---
 hw/virtio/virtio-pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index cb44e19b67..887ee2783c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -306,8 +306,10 @@ static void virtio_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
 if (pa == 0) {
 virtio_pci_reset(DEVICE(proxy));
 }
-else
+else {
 virtio_queue_set_addr(vdev, vdev->queue_sel, pa);
+proxy->vqs[vdev->queue_sel].enabled = 1;
+}
 break;
 case VIRTIO_PCI_QUEUE_SEL:
 if (val < VIRTIO_QUEUE_MAX)
-- 
2.19.1

[Qemu-devel] [RFT 3/4] virtio-pci: implement queue_enabled

[Jason Wang]

Signed-off-by: Jason Wang 
---
 hw/virtio/virtio-pci.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 887ee2783c..8bce42ff6f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1094,6 +1094,13 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState 
*d)
 return pci_get_address_space(dev);
 }
 
+static bool virtio_pci_queue_enabled(DeviceState *d, int n)
+{
+VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
+
+return proxy->vqs[n].enabled;
+}
+
 static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
struct virtio_pci_cap *cap)
 {
@@ -2037,6 +2044,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, 
void *data)
 k->ioeventfd_enabled = virtio_pci_ioeventfd_enabled;
 k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
 k->get_dma_as = virtio_pci_get_dma_as;
+k->queue_enabled = virtio_pci_queue_enabled;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
-- 
2.19.1

Re: [Qemu-devel] [PATCH] virtio-net: do not start queues that are not enabled by the guest

[Jason Wang]

On 2019/3/21 下午4:44, Michael S. Tsirkin wrote:

On Thu, Mar 14, 2019 at 10:45:25PM +0800, Jason Wang wrote:

On 2019/2/25 下午8:33, Michael S. Tsirkin wrote:

On Mon, Feb 25, 2019 at 03:47:48PM +0800, Jason Wang wrote:

On 2019/2/22 下午12:22, Michael S. Tsirkin wrote:

On Thu, Feb 21, 2019 at 10:10:08PM -0500, Michael S. Tsirkin wrote:

On Fri, Feb 22, 2019 at 11:04:05AM +0800, Jason Wang wrote:

On 2019/2/22 上午9:35, Michael S. Tsirkin wrote:

On Thu, Feb 21, 2019 at 05:40:22PM +0800, Jason Wang wrote:

On 2019/2/21 下午4:18, Yuri Benditovich wrote:

For 1.0 device, we can fix the queue_enable, but for 0.9x device 
how do
you enable one specific queue in this case? (setting status?)


Do I understand correctly that for 0.9 device in some cases the device 
will
receive feature _MQ set, but will not receive 
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET?
Or the problem is different?


Let me clarify, VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET is used to control the the
number of queue pairs used by device for doing transmission and reception. It
was not used to enable or disable a virtqueue.

For 1.0 device, we should use queue_enable in pci cfg to enable and disable
queue:


We could do:

1) allocate memory and set queue_enable for vq0

2) allocate memory and set queue_enable for vq1

3) Set vq paris to 1

4) allocate memory and set queue_enable for vq2

5) allocate memory and set queue_enable for vq3

6) set vq pairs to 2

I do not think spec allows this.


The driver MUST follow this sequence to initialize a device:
1. Reset the device.
2. Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
3. Set the DRIVER status bit: the guest OS knows how to drive the device.
4. Read device feature bits, and write the subset of feature bits understood by 
the OS and driver to the
device. During this step the driver MAY read (but MUST NOT write) the 
device-specific configuration
fields to check that it can support the device before accepting it.
5. Set the FEATURES_OK status bit. The driver MUST NOT accept new feature bits 
after this step.
6. Re-read device status to ensure the FEATURES_OK bit is still set: otherwise, 
the device does not
support our subset of features and the device is unusable.
7. Perform device-specific setup, including discovery of virtqueues for the 
device, optional per-bus setup,
reading and possibly writing the device’s virtio configuration space, and 
population of virtqueues.
8. Set the DRIVER_OK status bit. At this point the device is “live”.


Thus vqs are setup at step 7.

# of vq pairs are set up through a command which is a special
buffer, and spec says:

The driver MUST NOT send any buffer available notifications to the device 
before setting DRIVER_OK.

So you meant write to queue_enable is forbidden after DRIVER_OK (though it's
not very clear to me from the  spec). And if a driver want to enable new
queues, it must reset the device?

That's my reading.  What do you think?

Looks like I can infer this from the spec, maybe it's better to clarify.



Btw some legacy drivers might violate this by addig buffers
before driver_ok.

Yes, but it's probably too late to fix them.

Thanks.

Right. As a work around virtio net also checks rings
when it detects driver ok. We can disable that when VIRTIO_1
has been negotiated.


So, it looks to me the proper fix is probably start the vhost virtqueues
selectively only when the vring address is not zero (for 0.9x) or
queue_enable is set (for 1.0).

Thanks

Any update here? Looks like a spec violation in qemu
so I think we should fix this before we put out the
release. Jason do you have the time to work on a fix?



Will post patches for Yuri to test soon.

Thanks

Re: [Qemu-devel] [PATCH] MAINTAINERS: DT: Remove myself and degrade

[Peter Maydell]

Alex and I just noticed that we didn't pick up this
MAINTAINERS patch for the "Device Tree" section.
In the interim Peter Crosthwaite has also removed himself
from maintainership, so by default this will move into
the "Orphan" state. Is anybody interested in taking up
the maintainership role here? In practice it doesn't really
need much care as mostly it's dealt with by the people
who maintain the boards that use the dtb utility code
(currently arm, ppc, riscv) and there's not much change
in this area.

On Tue, 30 Oct 2018 at 09:40, Alexander Graf  wrote:
>
> I haven't really maintained the device tree infrastructure in QEMU for a long
> time by now. I also haven't seen Peter work on the code for the last 2 years,
> so let's put it into Odd Fixes to make room for others to step up :).
>
> Signed-off-by: Alexander Graf 
> ---
>  MAINTAINERS | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8bc9a594da..08ea12172e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1548,8 +1548,7 @@ F: include/qom/cpu.h
>
>  Device Tree
>  M: Peter Crosthwaite 
> -M: Alexander Graf 
> -S: Maintained
> +S: Odd Fixes
>  F: device_tree.c
>  F: include/sysemu/device_tree.h

thanks
-- PMM

Re: [Qemu-devel] [multiprocess RFC PATCH 36/37] multi-process: add the concept description to docs/devel/qemu-multiprocess

[Daniel P . Berrangé]

On Thu, Mar 21, 2019 at 08:26:47PM -0700, John G Johnson wrote:
> 
>  
> >  On Thu, Mar 07, 2019 at 02:51:20PM +, Daniel P. Berrangé wrote:
> > 
> >> On Mar 7, 2019, at 11:27 AM, Stefan Hajnoczi  wrote:
> >>> 
> >>> On Thu, Mar 07, 2019 at 02:51:20PM +, Daniel P. Berrangé wrote:
>  I guess one obvious answer is that the existing security mechanisms like
>  SELinux/ApArmor/DAC can be made to work in a more fine grained manner if
>  there are distinct processes. This would allow for a more useful seccomp
>  filter to better protect against secondary kernel exploits should QEMU
>  itself be exploited, if we can protect individual components.
> >>> 
> >>> Fine-grained sandboxing is possible in theory but tedious in practice.
> >>> From what I can tell this patch series doesn't implement any sandboxing
> >>> for child processes.
> >>> 
> >> 
> >> The policies aren’t in QEMU, but in the selinux config files.
> >> They would say, for example, that when the QEMU process exec()s the
> >> disk emulation process, the process security context type transitions
> >> to a new type.  This type would have permission to access the VM image
> >> objects, whereas the QEMU process type (and any other device emulation
> >> process types) cannot access them.
> > 
> > 
> > Note that currently all QEMU instances run by libvirt have seccomp
> > policy applied that explicitly forbids any use of fork+exec as a way
> > to reduce avenues of attack for an exploited QEMU.
> > 
> > Even in a modularized QEMU I'd be loathe to allow QEMU to have the
> > fork+exec privileged, unless "QEMU" in this case was just a stub
> > process that does nothing more than fork+exec the other binaries,
> > while having zero attack exposed to the untrusted guest OS.
> > 
> 
>   We’re looking at a couple ways to address your concerns.
> One is a stub process, as you mentioned above, but if we need to
> create programming to fork() and exec() the required emulation
> programs before exec()ing QEMU, then it may make sense to just put
> that programming into libvirt itself.
> 
>   Both paths would need similar changes to QEMU, such as the
> ability to receive descriptions of the emulation processes the parent
> process has created, and file descriptors that it has setup to
> communicate with them.  Each remote device would then be matched with
> its corresponding external process.
> 
>   The difference would be whether to create a new stub program
> to create the emulation processes, or delegate that task to libvirt’s
> QEMU driver.
> 
>   Do you have an opinion on a stub program vs libvirt integration?

Libvirt preference would be to retain full control over what programs
are spawned. This allows us to control their resource usage / placement
/ security policies. Having a stub that hides this from libvirt will
make this control harder, as we'll then need to interogate the stub
to find out what it did & applying controls appropriately. Also if more
external processes need to be spawned when hotplugging a device, then
libvirt would definitely want to have full control, as once QEMU vCPUS
have been started we don't trust it.

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 :|

[Qemu-devel] [RFT 0/4] Don't start virtqueues that are not enabled for vhost

[Jason Wang]

Hi:

This series try to avoid starting virtqueue that is not enabled. This
is done through querying it through a bus specific way and skip the
virtqueues if not enabled when starting vhost virtqueues.

Only PCI is implemented, maybe it's better to move the enable flag to
virito genenic virtqueue structure.

Yuri, Could you please to test this series to see if it solves the
issues when using windows driver?

Thanks

Jason Wang (4):
  virtio-bus: introduce a new method for querying the queue status
  virtio-pci: set enabled for legacy device
  virtio-pci: implement queue_enabled
  vhost_net: don't start vhost for the virtqueue that is not enabled

 hw/virtio/vhost.c  | 11 +++
 hw/virtio/virtio-pci.c | 12 +++-
 include/hw/virtio/virtio-bus.h |  4 
 3 files changed, 26 insertions(+), 1 deletion(-)

-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node

[Kevin Wolf]

Am 21.03.2019 um 18:05 hat Andrey Shinkevich geschrieben:
> 
> 
> On 21/03/2019 13:53, Kevin Wolf wrote:
> > Am 20.03.2019 um 18:02 hat Alberto Garcia geschrieben:
> >> On Wed 20 Mar 2019 10:16:10 AM CET, Kevin Wolf wrote:
>  Oh, I see. Let's use a shorter chain for simplicity:
> 
>  A <- B <- C <- D <- E
> >>>
> >>> Written from right to left, i.e. E being the base and A the top layer?
> >>> We usually write things the other write round, I hope this doesn't get
> >>> too confusing later.
> >>
> >> Oh my... yes, of course you're right, I should have written it the other
> >> way around:
> >>
> >> E <- D <- C <- B <- A
> >>
>  1) If we stream first from E to C we add a filter F:
> 
>  A <- B <- F <- C <- D <- E
> >>
> >> ( which should have been written   E <- D <- C <- F <- B <- A )
> >>
>  Now we can't stream from C to A because F is on the way, and the F-C
>  link is frozen.
> >>>
> >>> Why is a frozen link a problem? The streaming operation isn't going to
> >>> change this link, it just copies data from the subchain (including F
> >>> and C) to A. This is not something that a frozen link should prevent.
> >>
> >> Not the operation itself, but the first thing that block-stream does is
> >> freeze the chain from top (A) to base (C), so this would fail if there's
> >> already a frozen link on the way (C <- F on this case?).
> > 
> > Oh, I see. I think this is why I suggested a counter originally instead
> > of a bool.
> > 
> >>> So it seems frozen links allow the wrong case, but block the correct
> >>> one? :-(
> >>
> >> Yes, we probably need to rethink this scenario a bit.
> > 
> > But yes, even with a counter, the other problem would still remain (that
> > the first job can't remove the filter on completion because the second
> > one has frozen its link to the filter).
> 
> With this example E <- D <- C <- F <- B <- A,
> 
> In the current implementation of the copy-on-read filter,
> its bs->backing is not initialized (while it is not true for the filter
> in block-commit job). So, bdrv_freeze_backing_chain() doesn't go beyond 
> the cor-filter node. With the two parallel block-stream jobs, we get the
> following sub-chains frozen:
> F <- B <- A
> E <- D <- C
> as C <- F backing BdrvChild link doesn't exist.

Hm, yes, but that's more by accident than a proper design. Of course,
freezing the chain shouldn't be stopped by a filter.

Currently, bdrv_freeze_backing_chain() simply stops if we reach a point
where the backing chain ends, even if we haven't reached the base. I
think this should be an assertion failure because the function should
never be called with a base that isn't even in the backing chain.

> If the cor-filter is inserted with the bdrv_append(), we get two
> BdrvChild links (file and backing) pointed to the same BlockDriverState
> 'C' and additionally some child-permissions issues that I have not 
> resolved yet...
> Due to the fact mentioned above, freezing the backing chain works with
> the filter inserted. But, with the one BdrvChild *file link only in the
> BlockDriverState of the cor-filter, we encounter a broken chain each
> time we iterate through it with the backing_bs(F) (=NULL) in many other
> pieces of the code. In turn, it breaks the existing model.
> That's the point! (((
> What can we do with that?

I think Max's series "Deal with filters" might be very helpful for this
kind of thing. With it, bdrv_freeze_backing_chain() can easily be made
work even across the cor filter.

Basically, the solution in this series is that it replaces backing_bs()
with different other functions depending on what is really wanted.
Amonst others, it introduces a function to get to the next non-filter in
the backing file chain, no matter whether it has to go through bs->file
or bs->backing, which is probably the right replacement in your case.

Kevin

Re: [Qemu-devel] [PATCH v4] hw/acpi: extract acpi_add_rom_blob()

[Igor Mammedov]

On Fri, 22 Mar 2019 08:20:27 +0800
Wei Yang  wrote:

> On Thu, Mar 21, 2019 at 02:50:42PM +0100, Igor Mammedov wrote:
> >On Thu, 21 Mar 2019 08:21:53 +0800
> >Wei Yang  wrote:
> >  
> >> arm and i386 has almost the same function acpi_add_rom_blob(), except
> >> giving different FWCfgCallback function.
> >> 
> >> This patch moves acpi_add_rom_blob() to utils.c by passing
> >> FWCfgCallback to it.
> >> 
> >> Signed-off-by: Wei Yang 
> >> 
> >> ---
> >> v4:
> >>   * extract -> moves
> >>   * adjust comment in source to make checkpatch happy
> >> v3:
> >>   * put acpi_add_rom_blob() to hw/acpi/utils.c
> >> v2:
> >>   * remove unused header in original source file
> >> ---
> >>  hw/acpi/Makefile.objs|  2 +-
> >>  hw/acpi/utils.c  | 36 
> >>  hw/arm/virt-acpi-build.c | 26 ++
> >>  hw/i386/acpi-build.c | 26 ++
> >>  include/hw/acpi/utils.h  |  9 +
> >>  5 files changed, 66 insertions(+), 33 deletions(-)
> >>  create mode 100644 hw/acpi/utils.c
> >>  create mode 100644 include/hw/acpi/utils.h
> >> 
> >> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> >> index 2d46e3789a..ba93c5b64a 100644
> >> --- a/hw/acpi/Makefile.objs
> >> +++ b/hw/acpi/Makefile.objs
> >> @@ -10,7 +10,7 @@ common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
> >>  
> >>  common-obj-y += acpi_interface.o
> >>  common-obj-y += bios-linker-loader.o
> >> -common-obj-y += aml-build.o
> >> +common-obj-y += aml-build.o utils.o
> >>  common-obj-$(CONFIG_TPM) += tpm.o
> >>  
> >>  common-obj-$(CONFIG_IPMI) += ipmi.o
> >> diff --git a/hw/acpi/utils.c b/hw/acpi/utils.c
> >> new file mode 100644
> >> index 00..9747996c8c
> >> --- /dev/null
> >> +++ b/hw/acpi/utils.c
> >> @@ -0,0 +1,36 @@
> >> +/*
> >> + * Utilities for generating ACPI tables and passing them to Guests
> >> + *
> >> + * Copyright (C) 2019 Intel Corporation
> >> + * Copyright (C) 2019 Red Hat Inc
> >> + *
> >> + * Author: Wei Yang 
> >> + * Author: Igor Mammedov 
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> +
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> +
> >> + * You should have received a copy of the GNU General Public License along
> >> + * with this program; if not, see .
> >> + */
> >> +
> >> +#include "qemu/osdep.h"  
> >  
> >> +#include   
> >is it necessary?
> >  
> 
> Will remove this.
> 
> >> +#include "hw/acpi/aml-build.h"
> >> +#include "hw/acpi/utils.h"  
> >
> >  
> >> +
> >> +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
> >> +GArray *blob, const char *name,
> >> +uint64_t max_size)
> >> +{
> >> +return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, 
> >> -1,
> >> +name, update, opaque, NULL, true);
> >> +}
> >> +
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index 57679a89bf..a846f74a14 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -37,9 +37,9 @@
> >>  #include "hw/acpi/acpi.h"
> >>  #include "hw/nvram/fw_cfg.h"
> >>  #include "hw/acpi/bios-linker-loader.h"
> >> -#include "hw/loader.h"
> >>  #include "hw/hw.h"
> >>  #include "hw/acpi/aml-build.h"
> >> +#include "hw/acpi/utils.h"
> >>  #include "hw/pci/pcie_host.h"
> >>  #include "hw/pci/pci.h"
> >>  #include "hw/arm/virt.h"
> >> @@ -881,14 +881,6 @@ static void virt_acpi_build_reset(void *build_opaque)
> >>  build_state->patched = false;
> >>  }
> >>  
> >> -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
> >> -   GArray *blob, const char *name,
> >> -   uint64_t max_size)
> >> -{
> >> -return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, 
> >> -1,
> >> -name, virt_acpi_build_update, build_state, NULL, 
> >> true);
> >> -}
> >> -
> >>  static const VMStateDescription vmstate_virt_acpi_build = {
> >>  .name = "virt_acpi_build",
> >>  .version_id = 1,
> >> @@ -920,20 +912,22 @@ void virt_acpi_setup(VirtMachineState *vms)
> >>  virt_acpi_build(vms, &tables);
> >>  
> >>  /* Now expose it all to Guest */
> >> -build_state->table_mr = acpi_add_rom_blob(build_state, 
> >> tables.table_data,
> >> -   ACPI_BUILD_TABLE_FILE,
> >> -   ACPI_BUILD_TABLE_MAX_SIZE);
> >> + 

Re: [Qemu-devel] [PATCH] device_tree: check device tree blob file size

[P J P]

+-- On Fri, 22 Mar 2019, Peter Maydell wrote --+
| This document is specific to aarch64, but the part of
| QEMU's device tree code being modified here is
| architecture independent.
| 
| Cc'ing David Gibson who will probably know if there is
| an architecture-independent limit on DTB size we should
| be enforcing, or whether we are better just to have a check
| that avoids the overflow.

Thank you for CC'ing David. It seems Agraf did not receive email @suse.de.

Current limit defined by FDT_MAX_SIZE is ~1MB.

device_tree.c:
#define FDT_MAX_SIZE  0x10
 
| It's also worth noting in the commit message that this is
| not a security problem -- even if the "add 1 and double"
| calculation overflows, the load_image_size() function will
| not load more data into the buffer than will fit, so the
| behaviour will be to truncate the DTB.

True, load_image_size() helps to avoid buffer overflow issue.

Proposed check (dt_size > FDT_MAX_SIZE) in this patch is to enforce same size 
limit as used in create_device_tree() and avoid loading large files and the 
said integer overflow.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [RFT 0/4] Don't start virtqueues that are not enabled for vhost

[no-reply]

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



Hi,

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

Message-id: 20190322092806.21838-1-jasow...@redhat.com
Subject: [Qemu-devel] [RFT 0/4] Don't start virtqueues that are not enabled for 
vhost
Type: series

=== 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
 t [tag update]patchew/20190319124800.7454-1-alex.ben...@linaro.org 
-> patchew/20190319124800.7454-1-alex.ben...@linaro.org
 * [new tag]   patchew/20190322092806.21838-1-jasow...@redhat.com 
-> patchew/20190322092806.21838-1-jasow...@redhat.com
Switched to a new branch 'test'
227660b1c2 vhost_net: don't start vhost for the virtqueue that is not enabled
3e5639f199 virtio-pci: implement queue_enabled
c82bc06348 virtio-pci: set enabled for legacy device
7d9319ada4 virtio-bus: introduce a new method for querying the queue status

=== OUTPUT BEGIN ===
1/4 Checking commit 7d9319ada4d5 (virtio-bus: introduce a new method for 
querying the queue status)
2/4 Checking commit c82bc06348ef (virtio-pci: set enabled for legacy device)
ERROR: else should follow close brace '}'
#23: FILE: hw/virtio/virtio-pci.c:309:
 }
+else {

total: 1 errors, 0 warnings, 11 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/4 Checking commit 3e5639f199b0 (virtio-pci: implement queue_enabled)
4/4 Checking commit 227660b1c26c (vhost_net: don't start vhost for the 
virtqueue that is not enabled)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190322092806.21838-1-jasow...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH] migration: avoid copying ignore-shared ramblock when in incoming migration

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Thu, 21 Mar 2019 at 15:32, Catherine Ho  wrote:
> > So in igore-shared case, dtb is not required here ?
> 
> Can you explain what the "ignore-shared" case is?

It's a migration capability added recently by Yury Kotov,
which as you say below, skips migration of shared ram blocks.
The definition of 'shared' is that the RAMBlock rb->flags has
RAM_SHARED set, and that normally corresponds to when we call
mmap with MAP_SHARED.  The user visible version of that is
normally if you set up memory with a hostmem backend it has a 
'share' property.  So 'ignore-shared' corresponds to 'share=on'
on the memory backend.

> In general, when we reset the system, we want to bring it
> back to exactly the state that it was in when QEMU was
> first started. That means we need to reload all the rom blob
> data into memory (because the guest might have modified
> that memory while it was running).
> 
> If I understand correctly from other threads, the idea is
> that some of the RAM is shared between source and destination
> so it does not need to be manually copied during migration.
> If that is correct, then perhaps the right thing is that
> in the rom_reset code:
>  * if this rom blob is being loaded into a "shared" ram block
>  * and this reset is happening specifically before an
>inbound migration
>  * then skip loading the rom blob data
> 
> ?
> 
> I don't know the right way to determine either the "is this
> a shared ram area" or "is this the reset prior to inbound
> migration", but perhaps you can fill that in.

Right, so in Catherine's patch there's a simple in_incoming_migration
and checking ramblock_is_ignored;  it might be better to use the
qemu_ram_is_shared() call and I wonder if we can just check for being in
RUN_STATE_INMIGRATE - if that's early enough.

Dave

> > Maybe I could add a flag in struct Rom to set it when the rom is created by
> > rom_add_blob_fixed_as()? And in rom_reset, just bypass this rom when
> > in_incoming_migration && ignore_shared
> 
> No, I think this is wrong -- the particular function used
> to create the rom blob data should not affect how we are
> treating it.
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH V5 3/4] Migration/colo.c: Add the necessary checks for colo_do_failover

[Zhang Chen]

From: Zhang Chen 

Signed-off-by: Zhang Chen 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/colo.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index dbe2b88807..d1ae2e6d11 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -197,10 +197,16 @@ void colo_do_failover(MigrationState *s)
 vm_stop_force_state(RUN_STATE_COLO);
 }
 
-if (get_colo_mode() == COLO_MODE_PRIMARY) {
+switch (get_colo_mode()) {
+case COLO_MODE_PRIMARY:
 primary_vm_do_failover();
-} else {
+break;
+case COLO_MODE_SECONDARY:
 secondary_vm_do_failover();
+break;
+default:
+error_report("colo_do_failover failed because the colo mode"
+ " could not be obtained");
 }
 }
 
-- 
2.17.GIT

[Qemu-devel] [PATCH V5 0/4] Migration/colo: Fix upstream bugs when occur failover

[Zhang Chen]

From: Zhang Chen 

This series focus on COLO failover bug fix and optimization.

V5:
 - Rewrite patch 4/4 to add new feild in query_colo_status.

V4:
 - Remove merged patch.
 - Fix grammer issues in patch 2/4 to address markus's comments.
 - Rebased on upstream code.

V3:
 - Fix grammer issues in patch 4/7.
 - Add more information in commit log of patch 5/7.

V2:
 - Add patch 4/7 to handle failover state.
 - Add new patches to optimize failover status.

V1:
 - Init patch.


Zhang Chen (4):
  Migration/colo.c: Fix COLO failover status error
  Migration/colo.c: Add new COLOExitReason to handle all failover state
  Migration/colo.c: Add the necessary checks for colo_do_failover
  Migration/colo.c: Make user obtain the last COLO mode info after
failover

 migration/colo.c| 57 -
 qapi/migration.json | 22 +++--
 2 files changed, 56 insertions(+), 23 deletions(-)

-- 
2.17.GIT

[Qemu-devel] [PATCH V5 2/4] Migration/colo.c: Add new COLOExitReason to handle all failover state

[Zhang Chen]

From: Zhang Chen 

In this patch we add the processing state for COLOExitReason,
because we have to identify COLO in the failover processing state or
failover error state. In the way, we can handle all the failover state.
We have improved the description of the COLOExitReason by the way.

Signed-off-by: Zhang Chen 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/colo.c| 24 +---
 qapi/migration.json | 15 +--
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 89325952c7..dbe2b88807 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -267,7 +267,11 @@ COLOStatus *qmp_query_colo_status(Error **errp)
 s->reason = COLO_EXIT_REASON_REQUEST;
 break;
 default:
-s->reason = COLO_EXIT_REASON_ERROR;
+if (migration_in_colo_state()) {
+s->reason = COLO_EXIT_REASON_PROCESSING;
+} else {
+s->reason = COLO_EXIT_REASON_ERROR;
+}
 }
 
 return s;
@@ -579,16 +583,13 @@ out:
  * or the user triggered failover.
  */
 switch (failover_get_state()) {
-case FAILOVER_STATUS_NONE:
-qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
-  COLO_EXIT_REASON_ERROR);
-break;
 case FAILOVER_STATUS_COMPLETED:
 qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
   COLO_EXIT_REASON_REQUEST);
 break;
 default:
-abort();
+qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
+  COLO_EXIT_REASON_ERROR);
 }
 
 /* Hope this not to be too long to wait here */
@@ -850,17 +851,18 @@ out:
 error_report_err(local_err);
 }
 
+/*
+ * There are only two reasons we can get here, some error happened
+ * or the user triggered failover.
+ */
 switch (failover_get_state()) {
-case FAILOVER_STATUS_NONE:
-qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
-  COLO_EXIT_REASON_ERROR);
-break;
 case FAILOVER_STATUS_COMPLETED:
 qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
   COLO_EXIT_REASON_REQUEST);
 break;
 default:
-abort();
+qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
+  COLO_EXIT_REASON_ERROR);
 }
 
 if (fb) {
diff --git a/qapi/migration.json b/qapi/migration.json
index 5684733754..0bd044512f 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1037,19 +1037,22 @@
 ##
 # @COLOExitReason:
 #
-# The reason for a COLO exit
+# The reason for a COLO exit.
 #
-# @none: no failover has ever happened. This can't occur in the
-# COLO_EXIT event, only in the result of query-colo-status.
+# @none: failover has never happened. This state does not occur
+# in the COLO_EXIT event, and is only visible in the result of
+# query-colo-status.
 #
-# @request: COLO exit is due to an external request
+# @request: COLO exit is due to an external request.
 #
-# @error: COLO exit is due to an internal error
+# @error: COLO exit is due to an internal error.
+#
+# @processing: COLO is currently handling a failover (since 4.0).
 #
 # Since: 3.1
 ##
 { 'enum': 'COLOExitReason',
-  'data': [ 'none', 'request', 'error' ] }
+  'data': [ 'none', 'request', 'error' , 'processing' ] }
 
 ##
 # @x-colo-lost-heartbeat:
-- 
2.17.GIT

[Qemu-devel] [PATCH V5 1/4] Migration/colo.c: Fix COLO failover status error

[Zhang Chen]

From: Zhang Chen 

When finished COLO failover, the status is FAILOVER_STATUS_COMPLETED.
The origin codes misunderstand the FAILOVER_STATUS_REQUIRE.

Signed-off-by: Zhang Chen 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/colo.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 5ba610dc01..89325952c7 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -121,6 +121,7 @@ static void secondary_vm_do_failover(void)
 }
 /* Notify COLO incoming thread that failover work is finished */
 qemu_sem_post(&mis->colo_incoming_sem);
+
 /* For Secondary VM, jump to incoming co */
 if (mis->migration_incoming_co) {
 qemu_coroutine_enter(mis->migration_incoming_co);
@@ -262,7 +263,7 @@ COLOStatus *qmp_query_colo_status(Error **errp)
 case FAILOVER_STATUS_NONE:
 s->reason = COLO_EXIT_REASON_NONE;
 break;
-case FAILOVER_STATUS_REQUIRE:
+case FAILOVER_STATUS_COMPLETED:
 s->reason = COLO_EXIT_REASON_REQUEST;
 break;
 default:
@@ -582,7 +583,7 @@ out:
 qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
   COLO_EXIT_REASON_ERROR);
 break;
-case FAILOVER_STATUS_REQUIRE:
+case FAILOVER_STATUS_COMPLETED:
 qapi_event_send_colo_exit(COLO_MODE_PRIMARY,
   COLO_EXIT_REASON_REQUEST);
 break;
@@ -854,7 +855,7 @@ out:
 qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
   COLO_EXIT_REASON_ERROR);
 break;
-case FAILOVER_STATUS_REQUIRE:
+case FAILOVER_STATUS_COMPLETED:
 qapi_event_send_colo_exit(COLO_MODE_SECONDARY,
   COLO_EXIT_REASON_REQUEST);
 break;
-- 
2.17.GIT

[Qemu-devel] [PATCH V5 4/4] Migration/colo.c: Make user obtain the last COLO mode info after failover

[Zhang Chen]

From: Zhang Chen 

Add the last_colo_mode to save the status after failover.
This patch can solve the issue that user want to get last colo mode
use query_colo_status after failover.

Signed-off-by: Zhang Chen 
---
 migration/colo.c| 16 
 qapi/migration.json |  7 ++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/migration/colo.c b/migration/colo.c
index d1ae2e6d11..238a6d62c7 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -38,6 +38,9 @@
 static bool vmstate_loading;
 static Notifier packets_compare_notifier;
 
+/* User need to know colo mode after COLO failover */
+static COLOMode last_colo_mode;
+
 #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
 
 bool migration_in_colo_state(void)
@@ -264,6 +267,7 @@ COLOStatus *qmp_query_colo_status(Error **errp)
 COLOStatus *s = g_new0(COLOStatus, 1);
 
 s->mode = get_colo_mode();
+s->last_mode = last_colo_mode;
 
 switch (failover_get_state()) {
 case FAILOVER_STATUS_NONE:
@@ -515,6 +519,12 @@ static void colo_process_checkpoint(MigrationState *s)
 Error *local_err = NULL;
 int ret;
 
+last_colo_mode = get_colo_mode();
+if (last_colo_mode != COLO_MODE_PRIMARY) {
+error_report("COLO mode must be COLO_MODE_PRIMARY");
+return;
+}
+
 failover_init_state();
 
 s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file);
@@ -688,6 +698,12 @@ void *colo_process_incoming_thread(void *opaque)
 migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
   MIGRATION_STATUS_COLO);
 
+last_colo_mode = get_colo_mode();
+if (last_colo_mode != COLO_MODE_SECONDARY) {
+error_report("COLO mode must be COLO_MODE_SECONDARY");
+return NULL;
+}
+
 failover_init_state();
 
 mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
diff --git a/qapi/migration.json b/qapi/migration.json
index 0bd044512f..415eca7d4f 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1379,12 +1379,17 @@
 # @mode: COLO running mode. If COLO is running, this field will return
 #'primary' or 'secondary'.
 #
+# @last_mode: COLO last running mode. If COLO is running, this field
+# will return same like mode field, after failover we can
+# use this field to get last colo mode. (since 4.1)
+#
 # @reason: describes the reason for the COLO exit.
 #
 # Since: 3.1
 ##
 { 'struct': 'COLOStatus',
-  'data': { 'mode': 'COLOMode', 'reason': 'COLOExitReason' } }
+  'data': { 'mode': 'COLOMode', 'last_mode': 'COLOMode',
+'reason': 'COLOExitReason' } }
 
 ##
 # @query-colo-status:
-- 
2.17.GIT

Re: [Qemu-devel] [PATCH] device_tree: check device tree blob file size

[Peter Maydell]

On Fri, 22 Mar 2019 at 10:11, P J P  wrote:
>
> +-- On Fri, 22 Mar 2019, Peter Maydell wrote --+
> | This document is specific to aarch64, but the part of
> | QEMU's device tree code being modified here is
> | architecture independent.
> |
> | Cc'ing David Gibson who will probably know if there is
> | an architecture-independent limit on DTB size we should
> | be enforcing, or whether we are better just to have a check
> | that avoids the overflow.
>
> Thank you for CC'ing David. It seems Agraf did not receive email @suse.de.

Yes, Alex's email has changed (I've updated the cc list).

> Current limit defined by FDT_MAX_SIZE is ~1MB.

But currently this is only used when creating a DT from scratch.

> Proposed check (dt_size > FDT_MAX_SIZE) in this patch is to enforce same size
> limit as used in create_device_tree() and avoid loading large files and the
> said integer overflow.

My worry is that this might possibly break existing
working use cases which load a device tree that is larger
than 1MB. Unless there's a cross-architecture justification
for the 1MB limit it seems quite a low one to be enforcing
(especially since the one limit we've found so far for
aarch64 is 2MB, not 1MB).

thanks
-- PMM

Re: [Qemu-devel] [RFT 0/4] Don't start virtqueues that are not enabled for vhost

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190322092806.21838-1-jasow...@redhat.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
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 3 check-qstring /public/append_chr
PASS 4 check-qstring /public/from_substr
PASS 5 check-qstring /public/to_qstring
==7666==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/check-qlist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="check-qlist" 
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
---
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" 
==7749==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-coroutine /basic/no-dangling-access
PASS 2 test-coroutine /basic/lifecycle
==7749==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffe5bbfb000; bottom 0x7fe91aaf8000; size: 0x001541103000 (91285893120)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 3 test-coroutine /basic/yield
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 14 test-aio /aio/timer/schedule
==7764==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
PASS 17 test-aio /aio-gsource/bh/schedule
---
PASS 28 test-aio /aio-gsource/timer/schedule
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" 
PASS 1 test-aio-multithread /aio/multi/lifecycle
==7770==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
PASS 3 test-aio-multithread /aio/multi/mutex/contended
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" 
==7798==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
==7804==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==7810==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/setup
PASS 4 ide-test /x86_64/ide/bmdma/simple_rw
PASS 5 ide-test /x86_64/ide/bmdma/trim
PASS 6 ide-test /x86_64/ide/bmdma/short_prdt
PASS 7 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
PASS 8 ide-test /x86_64/ide/bmdma/long_prdt
==7810==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffeba2b1000; bottom 0x7f33bdef9000; size: 0x00cafc3b8000 (871815151616)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ide-test /x86_64/ide/bmdma/no_busmaster
---
PASS 10 ide-test /x86_64/ide/bmdma/teardown
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 11 ide-test /x86_64/ide/flush/nodev
==7831==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 12 ide-test /x86_64/ide/flush/empty_drive
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" 
==7836==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 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==7843==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 9 test-thrott

Re: [Qemu-devel] [PATCH] migration: avoid copying ignore-shared ramblock when in incoming migration

[Peter Maydell]

On Fri, 22 Mar 2019 at 10:12, Dr. David Alan Gilbert
 wrote:
> Right, so in Catherine's patch there's a simple in_incoming_migration
> and checking ramblock_is_ignored

Mmm, but I think it is in the wrong place. It is being checked
in address_space_write_rom_internal(). Either we want to
suppress any and all writes to these RAM blocks, in which
case I don't think that function covers all the ways that
code can get hold of a RAM block and write to it; or we are
confident that only the ROM blobs are an issue, in which
case it is too low in the call stack and we should do the
check in rom_reset().

Are there any other cases where we might write to RAM
during reset/migration ? I thought of "user write via
the debug stub or monitor", but perhaps those either
can't happen or we define them as user error. But I
there might be some other obscure cases, which perhaps
argues for doing this at the lowest level possible.

thanks
-- PMM

[Qemu-devel] [PATCH] RISC-V: fix single stepping over ret and other branching instructions

[Fabien Chouteau]

This patch introduces wrappers around the tcg_gen_exit_tb() and
tcg_gen_lookup_and_goto_ptr() functions that handle single stepping,
i.e. call gen_exception_debug() when single stepping is enabled.

Theses functions are then used instead of the originals, bringing single
stepping handling in places where it was previously ignored such as jalr
and system branch instructions (ecall, mret, sret, etc.).

Signed-off-by: Fabien Chouteau 
---
 .../riscv/insn_trans/trans_privileged.inc.c   |  8 +++---
 target/riscv/insn_trans/trans_rvi.inc.c   |  6 ++--
 target/riscv/translate.c  | 28 +++
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/target/riscv/insn_trans/trans_privileged.inc.c 
b/target/riscv/insn_trans/trans_privileged.inc.c
index acb605923e..fe660ab3dd 100644
--- a/target/riscv/insn_trans/trans_privileged.inc.c
+++ b/target/riscv/insn_trans/trans_privileged.inc.c
@@ -22,7 +22,7 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 {
 /* always generates U-level ECALL, fixed in do_interrupt handler */
 generate_exception(ctx, RISCV_EXCP_U_ECALL);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -30,7 +30,7 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
 {
 generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -47,7 +47,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 
 if (has_ext(ctx, RVS)) {
 gen_helper_sret(cpu_pc, cpu_env, cpu_pc);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 } else {
 return false;
@@ -68,7 +68,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 #ifndef CONFIG_USER_ONLY
 tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
 gen_helper_mret(cpu_pc, cpu_env, cpu_pc);
-tcg_gen_exit_tb(NULL, 0); /* no chaining */
+exit_tb(ctx, NULL, 0); /* no chaining */
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 #else
diff --git a/target/riscv/insn_trans/trans_rvi.inc.c 
b/target/riscv/insn_trans/trans_rvi.inc.c
index d420a4d8b2..44ae573768 100644
--- a/target/riscv/insn_trans/trans_rvi.inc.c
+++ b/target/riscv/insn_trans/trans_rvi.inc.c
@@ -60,7 +60,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
 if (a->rd != 0) {
 tcg_gen_movi_tl(cpu_gpr[a->rd], ctx->pc_succ_insn);
 }
-tcg_gen_lookup_and_goto_ptr();
+lookup_and_goto_ptr(ctx);
 
 if (misaligned) {
 gen_set_label(misaligned);
@@ -483,7 +483,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
  * however we need to end the translation block
  */
 tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
-tcg_gen_exit_tb(NULL, 0);
+exit_tb(ctx, NULL, 0);
 ctx->base.is_jmp = DISAS_NORETURN;
 return true;
 }
@@ -504,7 +504,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 gen_io_end(); \
 gen_set_gpr(a->rd, dest); \
 tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); \
-tcg_gen_exit_tb(NULL, 0); \
+exit_tb(ctx, NULL, 0); \
 ctx->base.is_jmp = DISAS_NORETURN; \
 tcg_temp_free(source1); \
 tcg_temp_free(csr_store); \
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 049fa65c66..c438400b19 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -109,6 +109,26 @@ static void gen_exception_debug(void)
 tcg_temp_free_i32(helper_tmp);
 }
 
+/* Wrapper around tcg_gen_exit_tb that handles single stepping */
+static void exit_tb(DisasContext *ctx, TranslationBlock *tb, unsigned idx)
+{
+if (ctx->base.singlestep_enabled) {
+gen_exception_debug();
+} else {
+tcg_gen_exit_tb(tb, idx);
+}
+}
+
+/* Wrapper around tcg_gen_lookup_and_goto_ptr that handles single stepping */
+static void lookup_and_goto_ptr(DisasContext *ctx)
+{
+if (ctx->base.singlestep_enabled) {
+gen_exception_debug();
+} else {
+tcg_gen_lookup_and_goto_ptr();
+}
+}
+
 static void gen_exception_illegal(DisasContext *ctx)
 {
 generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
@@ -138,14 +158,10 @@ static void gen_goto_tb(DisasContext *ctx, int n, 
target_ulong dest)
 /* chaining is only allowed when the jump is to the same page */
 tcg_gen_goto_tb(n);
 tcg_gen_movi_tl(cpu_pc, dest);
-tcg_gen_exit_tb(ctx->base.tb, n);
+exit_tb(ctx, ctx->base.tb, n);
 } else {
 tcg_gen_movi_tl(cpu_pc, dest);
-if (ctx->base.singlestep_enabled) {
-gen_exception_debug();
-} else {
-tcg_gen_lookup_and_goto_ptr();
-}
+lookup_and_goto_ptr(c

[Qemu-devel] [PATCH 1/4] Clean up wrong usage of FALSE and TRUE in places that use bool from stdbool.h

[JafarAbdi]

Signed-off-by: JafarAbdi 
---
 authz/listfile.c | 2 +-
 qemu.config  | 2 +
 qemu.creator | 1 +
 qemu.files   | 28238 +
 qemu.includes|  2568 +
 5 files changed, 30810 insertions(+), 1 deletion(-)
 create mode 100644 qemu.config
 create mode 100644 qemu.creator
 create mode 100644 qemu.files
 create mode 100644 qemu.includes

diff --git a/authz/listfile.c b/authz/listfile.c
index d457976..03eaf46 100644
--- a/authz/listfile.c
+++ b/authz/listfile.c
@@ -238,7 +238,7 @@ qauthz_list_file_init(Object *obj)
 
 authz->file_watch = -1;
 #ifdef CONFIG_INOTIFY1
-authz->refresh = TRUE;
+authz->refresh = true;
 #endif
 }
 

-- 
2.7.4

[Qemu-devel] [PATCH 2/4] Clean up wrong usage of FALSE and TRUE in places that use bool from stdbool.h

[JafarAbdi]

Signed-off-by: JafarAbdi 
---
 hw/tpm/tpm_tis.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index fd183e8..c1eb094 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -611,7 +611,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
 while ((TPM_TIS_IS_VALID_LOCTY(s->active_locty) &&
 locty > s->active_locty) ||
 !TPM_TIS_IS_VALID_LOCTY(s->active_locty)) {
-bool higher_seize = FALSE;
+bool higher_seize = false;
 
 /* already a pending SEIZE ? */
 if ((s->loc[locty].access & TPM_TIS_ACCESS_SEIZE)) {
@@ -621,7 +621,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
 /* check for ongoing seize by a higher locality */
 for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES; l++) {
 if ((s->loc[l].access & TPM_TIS_ACCESS_SEIZE)) {
-higher_seize = TRUE;
+higher_seize = true;
 break;
 }
 }
-- 
2.7.4

[Qemu-devel] [PATCH 3/4] Clean up wrong usage of FALSE and TRUE in places that use bool from stdbool.h

[JafarAbdi]

Signed-off-by: JafarAbdi 
---
 tests/libqos/pci-pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index 4ab16fa..407d8af 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -125,7 +125,7 @@ void qpci_init_pc(QPCIBusPC *qpci, QTestState *qts, 
QGuestAllocator *alloc)
 assert(qts);
 
 /* tests can use pci-bus */
-qpci->bus.has_buggy_msi = FALSE;
+qpci->bus.has_buggy_msi = false;
 
 qpci->bus.pio_readb = qpci_pc_pio_readb;
 qpci->bus.pio_readw = qpci_pc_pio_readw;
-- 
2.7.4

[Qemu-devel] [PATCH 4/4] Clean up wrong usage of FALSE and TRUE in places that use bool from stdbool.h

[JafarAbdi]

Signed-off-by: JafarAbdi 
---
 tests/libqos/pci-spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
index 6925925..58ba27a 100644
--- a/tests/libqos/pci-spapr.c
+++ b/tests/libqos/pci-spapr.c
@@ -156,7 +156,7 @@ void qpci_init_spapr(QPCIBusSPAPR *qpci, QTestState *qts,
 assert(qts);
 
 /* tests cannot use spapr, needs to be fixed first */
-qpci->bus.has_buggy_msi = TRUE;
+qpci->bus.has_buggy_msi = true;
 
 qpci->alloc = alloc;
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] acpi: verify file entries in bios_linker_loader_add_pointer()

[Igor Mammedov]

On Thu, 21 Mar 2019 20:13:49 +
Liam Merwick  wrote:

> The callers to bios_linker_find_file() assert that the file entry returned
> is not NULL, except for those in bios_linker_loader_add_pointer().  Add two
> asserts in that case for completeness and to facilitate static code analysis.
> 
> Signed-off-by: Liam Merwick 

Reviewed-by: Igor Mammedov 

> ---
>  hw/acpi/bios-linker-loader.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
> index d16b8bbcb187..626c04a39f92 100644
> --- a/hw/acpi/bios-linker-loader.c
> +++ b/hw/acpi/bios-linker-loader.c
> @@ -283,6 +283,8 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker,
>  const BiosLinkerFileEntry *source_file =
>  bios_linker_find_file(linker, src_file);
>  
> +assert(dst_file);
> +assert(source_file);
>  assert(dst_patched_offset < dst_file->blob->len);
>  assert(dst_patched_offset + dst_patched_size <= dst_file->blob->len);
>  assert(src_offset < source_file->blob->len);

Re: [Qemu-devel] [BUG] [low severity] a strange appearance of message involving slirp while doing "empty" make

[Peter Maydell]

On Fri, 22 Mar 2019 at 04:59, Thomas Huth  wrote:
> On 20/03/2019 22.08, Aleksandar Markovic wrote:
> > $ make
> > make[1]: Entering directory '/home/build/malta-mips64r6/qemu-4.0/slirp'
> > make[1]: Nothing to be done for 'all'.
> > make[1]: Leaving directory '/home/build/malta-mips64r6/qemu-4.0/slirp'
> >   CHK version_gen.h
> > $
> >
> > Not sure how significant is that, but I report it just in case.
>
> It's likely because slirp is currently being reworked to become a
> separate project, so the makefiles have been changed a little bit. I
> guess the message will go away again once slirp has become a stand-alone
> library.

Well, we'll still need to ship slirp for the foreseeable future...

I think the cause of this is that the rule in Makefile for
calling the slirp Makefile is not passing it $(SUBDIR_MAKEFLAGS)
like all the other recursive make invocations. If we do that
then we'll suppress the entering/leaving messages for
non-verbose builds. (Some tweaking will be needed as
it looks like the slirp makefile has picked an incompatible
meaning for $BUILD_DIR, which the SUBDIR_MAKEFLAGS will
also be passing to it.)

thanks
-- PMM

[Qemu-devel] question about 9pfs fsdriver types

[П , Алексей]

Hi there. Cannot find any info and examples about "proxy" and "handle"
types of fsdriver of virtfs. Can anyone advise me description of?
Witrh regards/

[Qemu-devel] [PATCH v1] exec: check the range in the address_space_unmap routine

[Dima Stepanov]

In case of the virtio-blk communication, can get the following assertion
for the specifically crafted virtio packet:
  qemu-system-x86_64: exec.c:3725: address_space_unmap: Assertion `mr !=
  NULL' failed.
This assertion is triggered if the length of the first descriptor in the
block request chain (block command descriptor) is more than block command
size. In this case the hw/block/virtio-blk.c:virtio_blk_handle_request()
routine calls the iov_discard_front() function and the iov base and size
are changed. As a result the address can not be found during the
address_space_unmap() call.

The fix is to check the whole address range in the address_space_unmap
function.

Signed-off-by: Dima Stepanov 
---
 exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 86a38d3..0eeb018 100644
--- a/exec.c
+++ b/exec.c
@@ -3717,7 +3717,7 @@ void *address_space_map(AddressSpace *as,
 void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
  int is_write, hwaddr access_len)
 {
-if (buffer != bounce.buffer) {
+if ((buffer < bounce.buffer) || (buffer + access_len > bounce.buffer + 
bounce.len)) {
 MemoryRegion *mr;
 ram_addr_t addr1;
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v8 05/16] gdbstub: add multiprocess support to vCont packets

[Jan Kiszka]

On 07.12.18 10:01, Luc Michel wrote:
> Add the gdb_first_attached_cpu() and gdb_next_attached_cpu() to iterate
> over all the CPUs in currently attached processes.
> 
> Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
> iterate over CPUs of a given process.
> 
> Use them to add multiprocess extension support to vCont packets.
> 
> Signed-off-by: Luc Michel 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Edgar E. Iglesias 
> Acked-by: Alistair Francis 
> ---
>   gdbstub.c | 117 +++---
>   1 file changed, 102 insertions(+), 15 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 911faa225a..77b3dbb2c8 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -721,10 +721,40 @@ static CPUState *find_cpu(uint32_t thread_id)
>   }
>   
>   return NULL;
>   }
>   
> +static CPUState *get_first_cpu_in_process(const GDBState *s,
> +  GDBProcess *process)
> +{
> +CPUState *cpu;
> +
> +CPU_FOREACH(cpu) {
> +if (gdb_get_cpu_pid(s, cpu) == process->pid) {
> +return cpu;
> +}
> +}
> +
> +return NULL;
> +}
> +
> +static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
> +{
> +uint32_t pid = gdb_get_cpu_pid(s, cpu);
> +cpu = CPU_NEXT(cpu);
> +
> +while (cpu) {
> +if (gdb_get_cpu_pid(s, cpu) == pid) {
> +break;
> +}
> +
> +cpu = CPU_NEXT(cpu);
> +}
> +
> +return cpu;
> +}
> +
>   static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
>   {
>   GDBProcess *process;
>   CPUState *cpu;
>   
> @@ -750,10 +780,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, 
> uint32_t pid, uint32_t tid)
>   }
>   
>   return cpu;
>   }
>   
> +/* Return the cpu following @cpu, while ignoring
> + * unattached processes.
> + */
> +static CPUState *gdb_next_attached_cpu(const GDBState *s, CPUState *cpu)
> +{
> +cpu = CPU_NEXT(cpu);
> +
> +while (cpu) {
> +if (gdb_get_cpu_process(s, cpu)->attached) {
> +break;
> +}
> +
> +cpu = CPU_NEXT(cpu);
> +}
> +
> +return cpu;
> +}
> +
> +/* Return the first attached cpu */
> +static CPUState *gdb_first_attached_cpu(const GDBState *s)
> +{
> +CPUState *cpu = first_cpu;
> +GDBProcess *process = gdb_get_cpu_process(s, cpu);
> +
> +if (!process->attached) {
> +return gdb_next_attached_cpu(s, cpu);
> +}
> +
> +return cpu;
> +}
> +
>   static const char *get_feature_xml(const char *p, const char **newp,
>  CPUClass *cc)
>   {
>   size_t len;
>   int i;
> @@ -1088,14 +1149,16 @@ static int is_query_packet(const char *p, const char 
> *query, char separator)
>* returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if 
> there is
>* a format error, 0 on success.
>*/
>   static int gdb_handle_vcont(GDBState *s, const char *p)
>   {
> -int res, idx, signal = 0;
> +int res, signal = 0;
>   char cur_action;
>   char *newstates;
>   unsigned long tmp;
> +uint32_t pid, tid;
> +GDBProcess *process;
>   CPUState *cpu;
>   #ifdef CONFIG_USER_ONLY
>   int max_cpus = 1; /* global variable max_cpus exists only in system 
> mode */
>   
>   CPU_FOREACH(cpu) {
> @@ -1134,29 +1197,52 @@ static int gdb_handle_vcont(GDBState *s, const char 
> *p)
>   } else if (cur_action != 'c' && cur_action != 's') {
>   /* unknown/invalid/unsupported command */
>   res = -ENOTSUP;
>   goto out;
>   }
> -/* thread specification. special values: (none), -1 = all; 0 = any */
> -if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
> -if (*p == ':') {
> -p += 3;
> -}
> -for (idx = 0; idx < max_cpus; idx++) {
> -if (newstates[idx] == 1) {
> -newstates[idx] = cur_action;
> +
> +if (*p++ != ':') {
> +res = -ENOTSUP;
> +goto out;
> +}
> +
> +switch (read_thread_id(p, &p, &pid, &tid)) {
> +case GDB_READ_THREAD_ERR:
> +res = -EINVAL;
> +goto out;
> +
> +case GDB_ALL_PROCESSES:
> +cpu = gdb_first_attached_cpu(s);
> +while (cpu) {
> +if (newstates[cpu->cpu_index] == 1) {
> +newstates[cpu->cpu_index] = cur_action;
>   }
> +
> +cpu = gdb_next_attached_cpu(s, cpu);
>   }
> -} else if (*p == ':') {
> -p++;
> -res = qemu_strtoul(p, &p, 16, &tmp);
> -if (res) {
> +break;
> +
> +case GDB_ALL_THREADS:
> +process = gdb_get_process(s, pid);
> +
> +if (!process->attached) {
> +res = -EINVAL;
>   goto out;
>   }
>   

[Qemu-devel] [PATCH 4/6] file-posix: Support BDRV_REQ_NO_FALLBACK for zero writes

[Kevin Wolf]

We know that the kernel implements a slow fallback code path for
BLKZEROOUT, so if BDRV_REQ_NO_FALLBACK is given, we shouldn't call it.
The other operations we call in the context of .bdrv_co_pwrite_zeroes
should usually be quick, so no modification should be needed for them.
If we ever notice that there are additional problematic cases, we can
still make these conditional as well.

Signed-off-by: Kevin Wolf 
---
 include/block/raw-aio.h |  1 +
 block/file-posix.c  | 24 
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index 6799614e56..ba223dd1f1 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -40,6 +40,7 @@
 /* AIO flags */
 #define QEMU_AIO_MISALIGNED   0x1000
 #define QEMU_AIO_BLKDEV   0x2000
+#define QEMU_AIO_NO_FALLBACK  0x4000
 
 
 /* linux-aio.c - Linux native implementation */
diff --git a/block/file-posix.c b/block/file-posix.c
index d102f3b222..db4cccbe51 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -652,7 +652,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 #endif
 
-bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
+bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK;
 ret = 0;
 fail:
 if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) {
@@ -1500,14 +1500,19 @@ static ssize_t 
handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb)
 }
 
 #ifdef BLKZEROOUT
-do {
-uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes };
-if (ioctl(aiocb->aio_fildes, BLKZEROOUT, range) == 0) {
-return 0;
-}
-} while (errno == EINTR);
+/* The BLKZEROOUT implementation in the kernel doesn't set
+ * BLKDEV_ZERO_NOFALLBACK, so we can't call this if we have to avoid slow
+ * fallbacks. */
+if (!(aiocb->aio_type & QEMU_AIO_NO_FALLBACK)) {
+do {
+uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes };
+if (ioctl(aiocb->aio_fildes, BLKZEROOUT, range) == 0) {
+return 0;
+}
+} while (errno == EINTR);
 
-ret = translate_err(-errno);
+ret = translate_err(-errno);
+}
 #endif
 
 if (ret == -ENOTSUP) {
@@ -2659,6 +2664,9 @@ raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t 
offset, int bytes,
 if (blkdev) {
 acb.aio_type |= QEMU_AIO_BLKDEV;
 }
+if (flags & BDRV_REQ_NO_FALLBACK) {
+acb.aio_type |= QEMU_AIO_NO_FALLBACK;
+}
 
 if (flags & BDRV_REQ_MAY_UNMAP) {
 acb.aio_type |= QEMU_AIO_DISCARD;
-- 
2.20.1

[Qemu-devel] [PATCH 0/6] block: Fix slow pre-zeroing in qemu-img convert

[Kevin Wolf]

If qemu-img convert sees that the target image isn't zero-initialised
yet, it tries to do an efficient zero write for the whole image first
to save the overhead of repeated explicit zero writes during the
conversion. Obviously, this provides only an advantage if the
pre-zeroing is actually efficient. Otherwise, we can end up writing
zeroes slowly while zeroing out the whole image, and then overwrite the
same blocks again with real data, potentially doubling the written data.

Additionally, commit 9776f0db changed NBD to advertise for all NBD
devices that zero writes with unmap is supported for all NBD devices, no
matter whether the storage of NBD server actually can do this or whether
we would fall back to explicit zero writes.

This series adds a new BDRV_REQ_NO_FALLBACK flag for writing zeroes,
which makes the request return an error if it can't guarantee that we
don't end up running a slow fallback path.

For NBD, this means that we still support zero writes from guests, but
qemu-img convert will not try to use it to zero out the whole image.
This is potentially suboptimal if the server does actually support
efficient zero writes. We'd need an NBD protocol extension to transfer
this flag so we can re-enable the qemu-img convert feature for the NBD
driver without regressing on storage that can't provide efficient zero
writes.

Kevin Wolf (6):
  block: Remove error messages in bdrv_make_zero()
  block: Add BDRV_REQ_NO_FALLBACK
  block: Advertise BDRV_REQ_NO_FALLBACK in filter drivers
  file-posix: Support BDRV_REQ_NO_FALLBACK for zero writes
  qemu-img: Use BDRV_REQ_NO_FALLBACK for pre-zeroing
  qemu-io: Add write -n for BDRV_REQ_NO_FALLBACK

 include/block/block.h   |  7 ++-
 include/block/raw-aio.h |  1 +
 block/blkdebug.c|  2 +-
 block/copy-on-read.c|  7 +++
 block/file-posix.c  | 24 
 block/io.c  | 16 +++-
 block/mirror.c  |  3 ++-
 block/raw-format.c  |  2 +-
 qemu-img.c  |  2 +-
 qemu-io-cmds.c  | 13 +++--
 10 files changed, 53 insertions(+), 24 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 6/6] qemu-io: Add write -n for BDRV_REQ_NO_FALLBACK

[Kevin Wolf]

This makes the new BDRV_REQ_NO_FALLBACK flag available in the qemu-io
write command.

Signed-off-by: Kevin Wolf 
---
 qemu-io-cmds.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 35dcdcf413..09750a23ce 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -946,6 +946,7 @@ static void write_help(void)
 " -b, -- write to the VM state rather than the virtual disk\n"
 " -c, -- write compressed data with blk_write_compressed\n"
 " -f, -- use Force Unit Access semantics\n"
+" -n, -- with -z, don't allow slow fallback\n"
 " -p, -- ignored for backwards compatibility\n"
 " -P, -- use different pattern to fill file\n"
 " -C, -- report statistics in a machine parsable format\n"
@@ -964,7 +965,7 @@ static const cmdinfo_t write_cmd = {
 .perm   = BLK_PERM_WRITE,
 .argmin = 2,
 .argmax = -1,
-.args   = "[-bcCfquz] [-P pattern] off len",
+.args   = "[-bcCfnquz] [-P pattern] off len",
 .oneline= "writes a number of bytes at a specified offset",
 .help   = write_help,
 };
@@ -983,7 +984,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
 int64_t total = 0;
 int pattern = 0xcd;
 
-while ((c = getopt(argc, argv, "bcCfpP:quz")) != -1) {
+while ((c = getopt(argc, argv, "bcCfnpP:quz")) != -1) {
 switch (c) {
 case 'b':
 bflag = true;
@@ -997,6 +998,9 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
 case 'f':
 flags |= BDRV_REQ_FUA;
 break;
+case 'n':
+flags |= BDRV_REQ_NO_FALLBACK;
+break;
 case 'p':
 /* Ignored for backwards compatibility */
 break;
@@ -1037,6 +1041,11 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 return -EINVAL;
 }
 
+if ((flags & BDRV_REQ_NO_FALLBACK) && !zflag) {
+printf("-n requires -z to be specified\n");
+return -EINVAL;
+}
+
 if ((flags & BDRV_REQ_MAY_UNMAP) && !zflag) {
 printf("-u requires -z to be specified\n");
 return -EINVAL;
-- 
2.20.1

Re: [Qemu-devel] [PATCH v1] exec: check the range in the address_space_unmap routine

[Peter Maydell]

On Fri, 22 Mar 2019 at 13:19, Dima Stepanov  wrote:
>
> In case of the virtio-blk communication, can get the following assertion
> for the specifically crafted virtio packet:
>   qemu-system-x86_64: exec.c:3725: address_space_unmap: Assertion `mr !=
>   NULL' failed.
> This assertion is triggered if the length of the first descriptor in the
> block request chain (block command descriptor) is more than block command
> size. In this case the hw/block/virtio-blk.c:virtio_blk_handle_request()
> routine calls the iov_discard_front() function and the iov base and size
> are changed. As a result the address can not be found during the
> address_space_unmap() call.
>
> The fix is to check the whole address range in the address_space_unmap
> function.
>
> Signed-off-by: Dima Stepanov 
> ---
>  exec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/exec.c b/exec.c
> index 86a38d3..0eeb018 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3717,7 +3717,7 @@ void *address_space_map(AddressSpace *as,
>  void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
>   int is_write, hwaddr access_len)
>  {
> -if (buffer != bounce.buffer) {
> +if ((buffer < bounce.buffer) || (buffer + access_len > bounce.buffer + 
> bounce.len)) {
>  MemoryRegion *mr;
>  ram_addr_t addr1;

A quick look at the xen_invalidate_map_cache_entry() implementation
suggests that it also assumes that the address passed to
address_space_unmap() must be the same address that was
originally handed out via address_space_map().

So I think we either need to also change the Xen code, or
we need to fix this at the virtio level by having it keep
track of the buffer it was handed so it can unmap it.

thanks
-- PMM

[Qemu-devel] [PATCH 5/6] qemu-img: Use BDRV_REQ_NO_FALLBACK for pre-zeroing

[Kevin Wolf]

If qemu-img convert sees that the target image isn't zero-initialised
yet, it tries to do an efficient zero write for the whole image first
to save the overhead of repeated explicit zero writes during the
conversion. Obviously, this provides only an advantage if the
pre-zeroing is actually efficient. Otherwise, we can end up writing
zeroes slowly while zeroing out the whole image, and then overwrite the
same blocks again with real data, potentially doubling the written data.

Pass BDRV_REQ_NO_FALLBACK to blk_make_zero() to avoid this case. If we
can't efficiently zero out, we'll instead write explicit zeroes only if
there is no data to be written to a block.

Signed-off-by: Kevin Wolf 
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 5fac840742..8ee63daeae 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1932,7 +1932,7 @@ static int convert_do_copy(ImgConvertState *s)
 if (!s->has_zero_init && !s->target_has_backing &&
 bdrv_can_write_zeroes_with_unmap(blk_bs(s->target)))
 {
-ret = blk_make_zero(s->target, BDRV_REQ_MAY_UNMAP);
+ret = blk_make_zero(s->target, BDRV_REQ_MAY_UNMAP | 
BDRV_REQ_NO_FALLBACK);
 if (ret == 0) {
 s->has_zero_init = true;
 }
-- 
2.20.1

[Qemu-devel] [RFC PATCH 0/2] implement the failover feature for assigned network devices

[Jens Freimann]

This is another attempt at implementing the host side of the
net_failover concept
(https://www.kernel.org/doc/html/latest/networking/net_failover.html)

The general idea is that we have a pair of devices, a vfio-pci and a
emulated device. Before migration the vfio device is unplugged and data
flows to the emulated device, on the target side another vfio-pci device
is plugged in to take over the data-path. In the guest the net_failover
module will pair net devices with the same MAC address.

* In the first patch the infrastructure for hiding the device is added
  for the qbus and qdev APIs. A "hidden" boolean is added to the device
  state and it is set based on a callback to the standby device which
  registers itself for handling the assessment: "should the primary device
  be hidden?" by cross validating the ids of the devices.

* In the second patch the virtio-net uses the API to hide the vfio
  device and unhides it when the feature is acked.

Previous discussion: https://patchwork.ozlabs.org/cover/989098/

To summarize concerns/feedback from previous discussion:
1.- guest OS can reject or worse _delay_ unplug by any amount of time.
  Migration might get stuck for unpredictable time with unclear reason.
  This approach combines two tricky things, hot/unplug and migration. 
  -> We can surprise-remove the PCI device and in QEMU we can do all
 necessary rollbacks transparent to management software. Will it be
 easy, probably not.
2. PCI devices are a precious ressource. The primary device should never
  be added to QEMU if it won't be used by guest instead of hiding it in
  QEMU. 
  -> We only hotplug the device when the standby feature bit was
 negotiated. We save the device cmdline options until we need it for
 qdev_device_add()
 Hiding a device can be a useful concept to model. For example a
 pci device in a powered-off slot could be marked as hidden until the slot 
is
 powered on (mst).
3. Management layer software should handle this. Open Stack already has
  components/code to handle unplug/replug VFIO devices and metadata to
  provide to the guest for detecting which devices should be paired.
  -> An approach that includes all software from firmware to
 higher-level management software wasn't tried in the last years. This is
 an attempt to keep it simple and contained in QEMU as much as possible.
4. Hotplugging a device and then making it part of a failover setup is
   not possible
  -> addressed by extending qdev hotplug functions to check for hidden
 attribute, so e.g. device_add can be used to plug a device.

There are still some open issues:

Migration: I'm looking for something like a pre-migration hook that I
could use to unplug the vfio-pci device. I tried with a migration
notifier but it is called to late, i.e. after migration is aborted due
to vfio-pci marked unmigrateable. I worked around this by setting it
to migrateable and used a migration notifier on the virtio-net device.

Commandline: There is a dependency between vfio-pci and virtio-net
devices. One points to the other via new parameters
primar= and standby=''. This means
that the primary device needs to be specified after standby device on 
the qemu command line. Not sure how to solve this. 

Error handling: Patches don't cover all possible error scenarios yet.

I have tested this with a mlx5 NIC and was able to migrate the VM with
above mentioned workarounds for open problems.

Command line example:

qemu-system-x86_64 -enable-kvm -m 3072 -smp 3 \
-machine q35,kernel-irqchip=split -cpu host   \
-k fr   \
-serial stdio   \
-net none \
-qmp unix:/tmp/qmp.socket,server,nowait \
-monitor telnet:127.0.0.1:,server,nowait \
-device pcie-root-port,id=root0,multifunction=on,chassis=0,addr=0xa \
-device pcie-root-port,id=root1,bus=pcie.0,chassis=1 \
-device pcie-root-port,id=root2,bus=pcie.0,chassis=2 \
-netdev 
tap,script=/root/bin/bridge.sh,downscript=no,id=hostnet1,vhost=on \
-device 
virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:6f:55:cc,bus=root2,primary=hostdev0
 \  
   
-device vfio-pci,host=5e:00.2,id=hostdev0,bus=root1,standby=net1 \
/root/rhel-guest-image-8.0-1781.x86_64.qcow2

I'm grateful for any remarks or ideas!

Thanks!

regards,
Jens 

Sameeh Jubran (2):
  qdev/qbus: Add hidden device support
  net/virtio: add failover support

 hw/core/qdev.c | 27 ++
 hw/net/virtio-net.c| 95 ++
 hw/pci/pci.c   |  1 +
 include/hw/pci/pci.h   |  2 +
 include/hw/qdev-core.h |  8 +++
 include/hw/virtio/virtio-net.h |  7 +++
 qdev-monitor.c | 48 +++--
 vl.c   |  7 ++-
 8 files changed, 189 insertions(+), 6 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 2/6] block: Add BDRV_REQ_NO_FALLBACK

[Kevin Wolf]

For qemu-img convert, we want an operation that zeroes out the whole
image if this can be done efficiently, but that returns an error
otherwise so we don't write explicit zeroes and immediately overwrite
them with the real data, potentially doubling the amount of data to be
written.

Signed-off-by: Kevin Wolf 
---
 include/block/block.h |  7 ++-
 block/io.c| 12 +++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index e452988b66..c7a26199aa 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -83,8 +83,13 @@ typedef enum {
  */
 BDRV_REQ_SERIALISING= 0x80,
 
+/* Execute the request only if the operation can be offloaded or otherwise
+ * be executed efficiently, but return an error instead of using a slow
+ * fallback. */
+BDRV_REQ_NO_FALLBACK= 0x100,
+
 /* Mask of valid flags */
-BDRV_REQ_MASK   = 0xff,
+BDRV_REQ_MASK   = 0x1ff,
 } BdrvRequestFlags;
 
 typedef struct BlockSizes {
diff --git a/block/io.c b/block/io.c
index 952372c2bb..dfc153b8d8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1015,6 +1015,7 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 unsigned int nb_sectors;
 
 assert(!(flags & ~BDRV_REQ_MASK));
+assert(!(flags & BDRV_REQ_NO_FALLBACK));
 
 if (!drv) {
 return -ENOMEDIUM;
@@ -1061,6 +1062,7 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 int ret;
 
 assert(!(flags & ~BDRV_REQ_MASK));
+assert(!(flags & BDRV_REQ_NO_FALLBACK));
 
 if (!drv) {
 return -ENOMEDIUM;
@@ -1467,6 +1469,10 @@ static int coroutine_fn 
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
 return -ENOMEDIUM;
 }
 
+if ((flags & ~bs->supported_zero_flags) & BDRV_REQ_NO_FALLBACK) {
+return -ENOTSUP;
+}
+
 assert(alignment % bs->bl.request_alignment == 0);
 head = offset % alignment;
 tail = (offset + bytes) % alignment;
@@ -1510,7 +1516,7 @@ static int coroutine_fn 
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
 assert(!bs->supported_zero_flags);
 }
 
-if (ret == -ENOTSUP) {
+if (ret == -ENOTSUP && !(flags & BDRV_REQ_NO_FALLBACK)) {
 /* Fall back to bounce buffer if write zeroes is unsupported */
 BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE;
 
@@ -2949,6 +2955,10 @@ static int coroutine_fn bdrv_co_copy_range_internal(
 BdrvTrackedRequest req;
 int ret;
 
+/* TODO We can support BDRV_REQ_NO_FALLBACK here */
+assert(!(read_flags & BDRV_REQ_NO_FALLBACK));
+assert(!(write_flags & BDRV_REQ_NO_FALLBACK));
+
 if (!dst || !dst->bs) {
 return -ENOMEDIUM;
 }
-- 
2.20.1

[Qemu-devel] [PATCH 3/6] block: Advertise BDRV_REQ_NO_FALLBACK in filter drivers

[Kevin Wolf]

Filter drivers that support .bdrv_co_pwrite_zeroes can safely advertise
BDRV_REQ_NO_FALLBACK because they just forward the request flags to
their child node.

Signed-off-by: Kevin Wolf 
---
 block/blkdebug.c | 2 +-
 block/copy-on-read.c | 7 +++
 block/mirror.c   | 3 ++-
 block/raw-format.c   | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 1ea835c2b9..efd9441625 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -401,7 +401,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
 (BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
 bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
-((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
 bs->file->bs->supported_zero_flags);
 ret = -EINVAL;
 
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index d670fec42b..53972b1da3 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -34,12 +34,11 @@ static int cor_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 
 bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
-(BDRV_REQ_FUA &
-bs->file->bs->supported_write_flags);
+(BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
 
 bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
-   ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
-bs->file->bs->supported_zero_flags);
+((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
+bs->file->bs->supported_zero_flags);
 
 return 0;
 }
diff --git a/block/mirror.c b/block/mirror.c
index eb9a4cdf56..ff15cfb197 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1548,7 +1548,8 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
 }
 mirror_top_bs->total_sectors = bs->total_sectors;
 mirror_top_bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED;
-mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED;
+mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
+  BDRV_REQ_NO_FALLBACK;
 bs_opaque = g_new0(MirrorBDSOpaque, 1);
 mirror_top_bs->opaque = bs_opaque;
 bdrv_set_aio_context(mirror_top_bs, bdrv_get_aio_context(bs));
diff --git a/block/raw-format.c b/block/raw-format.c
index cec29986cc..385cdc2490 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -434,7 +434,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
 (BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
 bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
-((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
 bs->file->bs->supported_zero_flags);
 
 if (bs->probed && !bdrv_is_read_only(bs)) {
-- 
2.20.1

[Qemu-devel] [RFC PATCH 1/2] qdev/qbus: Add hidden device support

[Jens Freimann]

From: Sameeh Jubran 

Signed-off-by: Jens Freimann 
---
 hw/core/qdev.c | 27 
 hw/pci/pci.c   |  1 +
 include/hw/pci/pci.h   |  2 ++
 include/hw/qdev-core.h |  8 +++
 qdev-monitor.c | 48 ++
 vl.c   |  7 --
 6 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f9b6efe509..5e1c8f0120 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -211,6 +211,33 @@ void device_listener_unregister(DeviceListener *listener)
 QTAILQ_REMOVE(&device_listeners, listener, link);
 }
 
+bool qdev_should_hide_device(QemuOpts *opts, Error **errp)
+{
+bool res = false;
+bool match_found = false;
+
+DeviceListener *listener;
+
+QTAILQ_FOREACH(listener, &device_listeners, link) {
+   if (listener->should_be_hidden) {
+listener->should_be_hidden(listener, opts, &match_found, &res);
+}
+
+if (match_found)
+{
+break;
+}
+}
+/* No suitable pair device was found */
+if (!match_found)
+{
+error_setg(errp, "An error occurred: Couln't attach to standby device"
+" Please note that the primary device should be"
+" must be placed after the standby device in the command line");
+}
+return res;
+}
+
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
  int required_for_version)
 {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 35451c1e99..67b1b7c500 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -70,6 +70,7 @@ static Property pci_props[] = {
 QEMU_PCIE_LNKSTA_DLLLA_BITNR, true),
 DEFINE_PROP_BIT("x-pcie-extcap-init", PCIDevice, cap_present,
 QEMU_PCIE_EXTCAP_INIT_BITNR, true),
+DEFINE_PROP_STRING("standby", PCIDevice, standby_id_str),
 DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d87f5f93e9..f75dace1ec 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -351,6 +351,8 @@ struct PCIDevice {
 MSIVectorUseNotifier msix_vector_use_notifier;
 MSIVectorReleaseNotifier msix_vector_release_notifier;
 MSIVectorPollNotifier msix_vector_poll_notifier;
+
+char *standby_id_str;
 };
 
 void pci_register_bar(PCIDevice *pci_dev, int region_num,
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 33ed3b8dde..e3b11f5503 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -144,6 +144,7 @@ struct DeviceState {
 char *canonical_path;
 bool realized;
 bool pending_deleted_event;
+bool hidden;
 QemuOpts *opts;
 int hotplugged;
 BusState *parent_bus;
@@ -157,6 +158,11 @@ struct DeviceState {
 struct DeviceListener {
 void (*realize)(DeviceListener *listener, DeviceState *dev);
 void (*unrealize)(DeviceListener *listener, DeviceState *dev);
+/* This callback is called just upon init of the DeviceState
+ * and can be used by a standby device for informing qdev if this
+ * device should be hidden by checking the device opts
+ */
+void (*should_be_hidden)(DeviceListener *listener, QemuOpts 
*device_opts,bool *match_found, bool *res);
 QTAILQ_ENTRY(DeviceListener) link;
 };
 
@@ -453,4 +459,6 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
 void device_listener_register(DeviceListener *listener);
 void device_listener_unregister(DeviceListener *listener);
 
+bool qdev_should_hide_device(QemuOpts *opts, Error **errp);
+
 #endif
diff --git a/qdev-monitor.c b/qdev-monitor.c
index d4320986a2..f4cfc8d4a1 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -33,6 +33,7 @@
 #include "qemu/option.h"
 #include "sysemu/block-backend.h"
 #include "migration/misc.h"
+#include "migration/migration.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -569,13 +570,49 @@ void qdev_set_id(DeviceState *dev, const char *id)
 }
 }
 
+static int has_standby_device(void *opaque, const char *name, const char 
*value,
+Error **errp)
+{
+if (strcmp(name, "standby") == 0)
+{
+QemuOpts *opts = (QemuOpts *)opaque;
+if (qdev_should_hide_device(opts, errp) && errp && !*errp)
+{
+return 1;
+}
+else if (errp && *errp)
+{
+return -1;
+}
+}
+return 0;
+}
+
+static bool should_hide_device(QemuOpts *opts, Error **err)
+{
+if (qemu_opt_foreach(opts, has_standby_device, opts, err) == 0)
+{
+return false;
+}
+return true;
+}
+
 DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
 {
 DeviceClass *dc;
 const char *driver, *path;
-DeviceState *dev;
+DeviceState *dev = NULL;
 BusState *bus = NULL;
 Error *err = NULL;
+
+if (opts && should_hide_device(opts, &err))
+{
+if(err)
+{
+goto err_del_dev;
+}
+return NU

[Qemu-devel] [RFC PATCH 2/2] net/virtio: add failover support

[Jens Freimann]

From: Sameeh Jubran 

---
 hw/net/virtio-net.c| 95 ++
 include/hw/virtio/virtio-net.h |  7 +++
 2 files changed, 102 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7e2c2a6f6a..880420a673 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/atomic.h"
 #include "qemu/iov.h"
 #include "hw/virtio/virtio.h"
 #include "net/net.h"
@@ -19,6 +20,7 @@
 #include "net/tap.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
+#include "qemu/option.h"
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
 #include "net/announce.h"
@@ -29,6 +31,8 @@
 #include "migration/misc.h"
 #include "standard-headers/linux/ethtool.h"
 #include "trace.h"
+#include "monitor/qdev.h"
+#include "hw/pci/pci.h"
 
 #define VIRTIO_NET_VM_VERSION11
 
@@ -364,6 +368,9 @@ static void virtio_net_set_status(struct VirtIODevice 
*vdev, uint8_t status)
 }
 }
 
+
+static void virtio_net_primary_plug_timer(void *opaque);
+
 static void virtio_net_set_link_status(NetClientState *nc)
 {
 VirtIONet *n = qemu_get_nic_opaque(nc);
@@ -786,6 +793,14 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
uint64_t features)
 } else {
 memset(n->vlans, 0xff, MAX_VLAN >> 3);
 }
+
+if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
+atomic_set(&n->primary_should_be_hidden, false);
+if (n->primary_device_timer)
+timer_mod(n->primary_device_timer,
+qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+4000);
+}
 }
 
 static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
@@ -2626,6 +2641,74 @@ void virtio_net_set_netclient_name(VirtIONet *n, const 
char *name,
 n->netclient_type = g_strdup(type);
 }
 
+static void virtio_net_primary_plug_timer(void *opaque)
+{
+VirtIONet *n = opaque;
+Error *err = NULL;
+
+n->primary_dev = qdev_device_add(n->primary_device_opts, &err);
+if (!n->primary_dev && err)
+{
+if (n->primary_device_timer)
+timer_mod(n->primary_device_timer,
+qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+100);
+}
+}
+
+static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState * 
s)
+{
+Error *err = NULL;
+bool should_be_hidden = atomic_read(&n->primary_should_be_hidden);
+
+if (migration_in_setup(s) && !should_be_hidden && n->primary_dev) {
+/* Request unplug
+ *
+ *
+*/
+qdev_unplug(n->primary_dev, &err);
+if (!err)
+{
+atomic_set(&n->primary_should_be_hidden, true);
+n->primary_dev = NULL;
+}
+} else if (migration_has_failed(s)) {
+if (should_be_hidden && !n->primary_dev)
+{
+/* We already unplugged the device let's plugged it back */
+n->primary_dev = qdev_device_add(n->primary_device_opts, &err);
+}
+else
+{
+}
+}
+}
+
+static void migration_state_notifier(Notifier *notifier, void *data)
+{
+MigrationState *s = data;
+VirtIONet *n = container_of(notifier, VirtIONet, migration_state);
+virtio_net_handle_migration_primary(n,s);
+}
+
+static void virtio_net_primary_should_be_hidden(DeviceListener *listener, 
QemuOpts *device_opts,
+bool *match_found, bool *res)
+{
+   VirtIONet *n = container_of(listener, VirtIONet, primary_listener);
+   const char * dev_id = qemu_opts_id(device_opts);
+
+*match_found = !strcmp(n->net_conf.primary_id_str ,dev_id);
+if (atomic_read(&n->primary_should_be_hidden) && 
!strcmp(qemu_opt_get(device_opts, "driver"), "vfio-pci")
+&& *match_found)
+{
+n->primary_device_opts = device_opts;
+*res = true;
+return;
+}
+
+*res = false;
+}
+
 static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -2656,6 +2739,17 @@ static void virtio_net_device_realize(DeviceState *dev, 
Error **errp)
 n->host_features |= (1ULL << VIRTIO_NET_F_SPEED_DUPLEX);
 }
 
+if (n->net_conf.primary_id_str) {
+n->primary_listener.should_be_hidden = 
virtio_net_primary_should_be_hidden;
+atomic_set(&n->primary_should_be_hidden, true);
+device_listener_register(&n->primary_listener);
+n->migration_state.notify = migration_state_notifier;
+add_migration_state_change_notifier(&n->migration_state);
+n->host_features |= (1ULL << VIRTIO_NET_F_STANDBY);
+n->primary_device_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+ virtio_net_primary_plug_timer, n);
+}
+
 virtio_net_set_config_size(n, n->host_features);
 virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
@@ -2885,6 +2979,7 @@ static Property virtio_net_properties[] = {
  true),
 DEFINE_PROP_INT32("speed

[Qemu-devel] [PATCH 1/6] block: Remove error messages in bdrv_make_zero()

[Kevin Wolf]

There is only a single caller of bdrv_make_zero(), which is qemu-img
convert. If the function fails, we just fall back to a different method
of zeroing out blocks on the target image. There is no good reason to
print error messages on stderr when the higher level operation will
actually succeed.

Signed-off-by: Kevin Wolf 
---
 block/io.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index 2ba603c7bc..952372c2bb 100644
--- a/block/io.c
+++ b/block/io.c
@@ -909,8 +909,6 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
 }
 ret = bdrv_block_status(bs, offset, bytes, &bytes, NULL, NULL);
 if (ret < 0) {
-error_report("error getting block status at offset %" PRId64 ": 
%s",
- offset, strerror(-ret));
 return ret;
 }
 if (ret & BDRV_BLOCK_ZERO) {
@@ -919,8 +917,6 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
 }
 ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
 if (ret < 0) {
-error_report("error writing zeroes at offset %" PRId64 ": %s",
- offset, strerror(-ret));
 return ret;
 }
 offset += bytes;
-- 
2.20.1

Re: [Qemu-devel] [RFC PATCH 0/2] implement the failover feature for assigned network devices

[no-reply]

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



Hi,

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

Type: series
Message-id: 20190322134447.14831-1-jfreim...@redhat.com
Subject: [Qemu-devel] [RFC PATCH 0/2] implement the failover feature for 
assigned network devices

=== 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/20190322134447.14831-1-jfreim...@redhat.com -> 
patchew/20190322134447.14831-1-jfreim...@redhat.com
Switched to a new branch 'test'
4aa7f06 net/virtio: add failover support
5d2911f qdev/qbus: Add hidden device support

=== OUTPUT BEGIN ===
1/2 Checking commit 5d2911f6d6d4 (qdev/qbus: Add hidden device support)
ERROR: trailing whitespace
#22: FILE: hw/core/qdev.c:218:
+$

ERROR: that open brace { should be on the previous line
#30: FILE: hw/core/qdev.c:226:
+if (match_found)
+{

ERROR: that open brace { should be on the previous line
#36: FILE: hw/core/qdev.c:232:
+if (!match_found)
+{

WARNING: Block comments use a leading /* on a separate line
#89: FILE: include/hw/qdev-core.h:161:
+/* This callback is called just upon init of the DeviceState

ERROR: line over 90 characters
#93: FILE: include/hw/qdev-core.h:165:
+void (*should_be_hidden)(DeviceListener *listener, QemuOpts 
*device_opts,bool *match_found, bool *res);

ERROR: space required after that ',' (ctx:VxV)
#93: FILE: include/hw/qdev-core.h:165:
+void (*should_be_hidden)(DeviceListener *listener, QemuOpts 
*device_opts,bool *match_found, bool *res);
 ^

ERROR: that open brace { should be on the previous line
#123: FILE: qdev-monitor.c:576:
+if (strcmp(name, "standby") == 0)
+{

ERROR: that open brace { should be on the previous line
#126: FILE: qdev-monitor.c:579:
+if (qdev_should_hide_device(opts, errp) && errp && !*errp)
+{

ERROR: that open brace { should be on the previous line
#130: FILE: qdev-monitor.c:583:
+else if (errp && *errp)
+{

ERROR: else should follow close brace '}'
#130: FILE: qdev-monitor.c:583:
+}
+else if (errp && *errp)

ERROR: that open brace { should be on the previous line
#140: FILE: qdev-monitor.c:593:
+if (qemu_opt_foreach(opts, has_standby_device, opts, err) == 0)
+{

ERROR: trailing whitespace
#155: FILE: qdev-monitor.c:607:
+$

ERROR: that open brace { should be on the previous line
#156: FILE: qdev-monitor.c:608:
+if (opts && should_hide_device(opts, &err))
+{

ERROR: that open brace { should be on the previous line
#158: FILE: qdev-monitor.c:610:
+if(err)
+{

ERROR: space required before the open parenthesis '('
#158: FILE: qdev-monitor.c:610:
+if(err)

ERROR: that open brace { should be on the previous line
#173: FILE: qdev-monitor.c:688:
+if (dev)
+{

ERROR: that open brace { should be on the previous line
#202: FILE: vl.c:2343:
+} else if (dev)
+{

total: 16 errors, 1 warnings, 165 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/2 Checking commit 4aa7f06bed23 (net/virtio: add failover support)
ERROR: that open brace { should be on the previous line
#73: FILE: hw/net/virtio-net.c:2650:
+if (!n->primary_dev && err)
+{

WARNING: line over 80 characters
#82: FILE: hw/net/virtio-net.c:2659:
+static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState * 
s)

WARNING: Block comments use a leading /* on a separate line
#88: FILE: hw/net/virtio-net.c:2665:
+/* Request unplug

WARNING: Block comments should align the * on each line
#91: FILE: hw/net/virtio-net.c:2668:
+ *
+*/

ERROR: that open brace { should be on the previous line
#93: FILE: hw/net/virtio-net.c:2670:
+if (!err)
+{

ERROR: that open brace { should be on the previous line
#99: FILE: hw/net/virtio-net.c:2676:
+if (should_be_hidden && !n->primary_dev)
+{

ERROR: that open brace { should be on the previous line
#104: FILE: hw/net/virtio-net.c:2681:
+else
+{

ERROR: else should follow close brace '}'
#104: FILE: hw/net/virtio-net.c:2681:
+}
+else

ERROR: space required after that ',' (ctx:VxV)
#114: FILE: hw/net/virtio-net.c:2691:
+virtio_net_handle_migration_primary(n,s);
  ^

ERROR: line over 90 characters
#117: FILE: hw/net/virtio-net.c:2694:
+static void virtio_net_primary_should_be_hidden(DeviceListener *listener, 
QemuOpts *de

Re: [Qemu-devel] [PATCH 0/6] block: Fix slow pre-zeroing in qemu-img convert

[Eric Blake]

On 3/22/19 8:21 AM, Kevin Wolf wrote:
> If qemu-img convert sees that the target image isn't zero-initialised
> yet, it tries to do an efficient zero write for the whole image first
> to save the overhead of repeated explicit zero writes during the
> conversion. Obviously, this provides only an advantage if the
> pre-zeroing is actually efficient. Otherwise, we can end up writing
> zeroes slowly while zeroing out the whole image, and then overwrite the
> same blocks again with real data, potentially doubling the written data.
> 
> Additionally, commit 9776f0db changed NBD to advertise for all NBD
> devices that zero writes with unmap is supported for all NBD devices, no
> matter whether the storage of NBD server actually can do this or whether
> we would fall back to explicit zero writes.
> 
> This series adds a new BDRV_REQ_NO_FALLBACK flag for writing zeroes,
> which makes the request return an error if it can't guarantee that we
> don't end up running a slow fallback path.
> 
> For NBD, this means that we still support zero writes from guests, but
> qemu-img convert will not try to use it to zero out the whole image.
> This is potentially suboptimal if the server does actually support
> efficient zero writes. We'd need an NBD protocol extension to transfer
> this flag so we can re-enable the qemu-img convert feature for the NBD
> driver without regressing on storage that can't provide efficient zero
> writes.

I haven't reviewed closely yet, but the summary is sane, so:
Acked-by: Eric Blake 

and I'll be cross-posting another mail soon with the desired NBD
protocol extension.

-- 
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] question about 9pfs fsdriver types

[Greg Kurz]

On Fri, 22 Mar 2019 11:57:04 +0200
"П, Алексей"  wrote:

> Hi there. Cannot find any info and examples about "proxy" and "handle"
> types of fsdriver of virtfs. Can anyone advise me description of?
> Witrh regards/

Hi,

The "handle" backend was deprecated in QEMU 2.12, for various reasons...
requires elevated privileges, broken symlinks, no known users. The code
got removed for upcoming QEMU 4.0.

About "proxy", maybe I'll try to find some time to update the wiki
at https://wiki.qemu.org/Documentation/9psetup .

In the meantime, here's an example:

1) start the helper as root in some shell

$ sudo virtfs-proxy-helper -n -p /var/tmp/test \
-s /var/tmp/virtfs-proxy-sock \
-u $(id -u) -g $(id -g)
Started

2) start QEMU in some other shell

$ qemu-system-x864_64 ${YOUR_FAVORITE_CMDLINE_ARGUMENTS} \
-fsdev proxy,id=fs0,socket=/var/tmp/virtfs-proxy-sock \
-device virtio-9p-pci,fsdev=fs0,mount_tag=${SOME_TAG}

Cheers,

--
Greg

Re: [Qemu-devel] [PATCH v8 05/16] gdbstub: add multiprocess support to vCont packets

[Jan Kiszka]

On 22.03.19 15:01, Luc Michel wrote:

On 3/22/19 2:29 PM, Jan Kiszka wrote:

On 07.12.18 10:01, Luc Michel wrote:

Add the gdb_first_attached_cpu() and gdb_next_attached_cpu() to iterate
over all the CPUs in currently attached processes.

Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
iterate over CPUs of a given process.

Use them to add multiprocess extension support to vCont packets.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Edgar E. Iglesias 
Acked-by: Alistair Francis 
---
   gdbstub.c | 117 +++---
   1 file changed, 102 insertions(+), 15 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 911faa225a..77b3dbb2c8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -721,10 +721,40 @@ static CPUState *find_cpu(uint32_t thread_id)
   }
   
   return NULL;

   }
   
+static CPUState *get_first_cpu_in_process(const GDBState *s,

+  GDBProcess *process)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (gdb_get_cpu_pid(s, cpu) == process->pid) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
+{
+uint32_t pid = gdb_get_cpu_pid(s, cpu);
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_pid(s, cpu) == pid) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
   static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
   {
   GDBProcess *process;
   CPUState *cpu;
   
@@ -750,10 +780,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)

   }
   
   return cpu;

   }
   
+/* Return the cpu following @cpu, while ignoring

+ * unattached processes.
+ */
+static CPUState *gdb_next_attached_cpu(const GDBState *s, CPUState *cpu)
+{
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_process(s, cpu)->attached) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
+/* Return the first attached cpu */
+static CPUState *gdb_first_attached_cpu(const GDBState *s)
+{
+CPUState *cpu = first_cpu;
+GDBProcess *process = gdb_get_cpu_process(s, cpu);
+
+if (!process->attached) {
+return gdb_next_attached_cpu(s, cpu);
+}
+
+return cpu;
+}
+
   static const char *get_feature_xml(const char *p, const char **newp,
  CPUClass *cc)
   {
   size_t len;
   int i;
@@ -1088,14 +1149,16 @@ static int is_query_packet(const char *p, const char 
*query, char separator)
* returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there 
is
* a format error, 0 on success.
*/
   static int gdb_handle_vcont(GDBState *s, const char *p)
   {
-int res, idx, signal = 0;
+int res, signal = 0;
   char cur_action;
   char *newstates;
   unsigned long tmp;
+uint32_t pid, tid;
+GDBProcess *process;
   CPUState *cpu;
   #ifdef CONFIG_USER_ONLY
   int max_cpus = 1; /* global variable max_cpus exists only in system mode 
*/
   
   CPU_FOREACH(cpu) {

@@ -1134,29 +1197,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
   } else if (cur_action != 'c' && cur_action != 's') {
   /* unknown/invalid/unsupported command */
   res = -ENOTSUP;
   goto out;
   }
-/* thread specification. special values: (none), -1 = all; 0 = any */
-if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
-if (*p == ':') {
-p += 3;
-}
-for (idx = 0; idx < max_cpus; idx++) {
-if (newstates[idx] == 1) {
-newstates[idx] = cur_action;
+
+if (*p++ != ':') {
+res = -ENOTSUP;
+goto out;
+}
+
+switch (read_thread_id(p, &p, &pid, &tid)) {
+case GDB_READ_THREAD_ERR:
+res = -EINVAL;
+goto out;
+
+case GDB_ALL_PROCESSES:
+cpu = gdb_first_attached_cpu(s);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
   }
+
+cpu = gdb_next_attached_cpu(s, cpu);
   }
-} else if (*p == ':') {
-p++;
-res = qemu_strtoul(p, &p, 16, &tmp);
-if (res) {
+break;
+
+case GDB_ALL_THREADS:
+process = gdb_get_process(s, pid);
+
+if (!process->attached) {
+res = -EINVAL;
   goto out;
   }
   
-/* 0 means any thread, so we pick the first valid CPU */

-cpu = tmp ? find_cpu(tmp) : first_cpu;
+cpu = get_first_cpu_in_process(s, process);
+while (cpu) {
+   

[Qemu-devel] [PATCH 1/3] tests: refactor file monitor test to make it more understandable

[Daniel P . Berrangé]

The current file monitor unit tests are too clever for their own good
making it hard to understand the desired output.

Instead of trying to infer the expected events, explicitly list the
events we expect in the operation sequence.

Instead of dynamically building a matrix of tests, just have one giant
operation sequence that validates all scenarios in a single test.

Reviewed-by: Marc-André Lureau 
Signed-off-by: Daniel P. Berrangé 
---
 tests/test-util-filemonitor.c | 534 +-
 1 file changed, 208 insertions(+), 326 deletions(-)

diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 5d95cea5ee..ea3715a8f4 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -26,6 +26,9 @@
 #include 
 
 enum {
+QFILE_MONITOR_TEST_OP_ADD_WATCH,
+QFILE_MONITOR_TEST_OP_DEL_WATCH,
+QFILE_MONITOR_TEST_OP_EVENT,
 QFILE_MONITOR_TEST_OP_CREATE,
 QFILE_MONITOR_TEST_OP_APPEND,
 QFILE_MONITOR_TEST_OP_TRUNC,
@@ -38,20 +41,10 @@ typedef struct {
 int type;
 const char *filesrc;
 const char *filedst;
+int watchid;
+int eventid;
 } QFileMonitorTestOp;
 
-typedef struct {
-const char *file;
-} QFileMonitorTestWatch;
-
-typedef struct {
-gsize nwatches;
-const QFileMonitorTestWatch *watches;
-
-gsize nops;
-const QFileMonitorTestOp *ops;
-} QFileMonitorTestPlan;
-
 typedef struct {
 int id;
 QFileMonitorEvent event;
@@ -67,6 +60,7 @@ typedef struct {
 static QemuMutex evlock;
 static bool evstopping;
 static bool evrunning;
+static bool debug;
 
 /*
  * Main function for a background thread that is
@@ -200,9 +194,125 @@ qemu_file_monitor_test_expect(QFileMonitorTestData *data,
 
 
 static void
-test_file_monitor_events(const void *opaque)
+test_file_monitor_events(void)
 {
-const QFileMonitorTestPlan *plan = opaque;
+QFileMonitorTestOp ops[] = {
+{ .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+  .filesrc = NULL, .watchid = 0 },
+{ .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+  .filesrc = "one.txt", .watchid = 1 },
+{ .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+  .filesrc = "two.txt", .watchid = 2 },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_CREATE,
+  .filesrc = "one.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 1,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_CREATE,
+  .filesrc = "two.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 2,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_CREATE,
+  .filesrc = "three.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "three.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_UNLINK,
+  .filesrc = "three.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "three.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_RENAME,
+  .filesrc = "one.txt", .filedst = "two.txt" },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 1,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 2,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_APPEND,
+  .filesrc = "two.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_MODIFIED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 2,
+  .eventid = QFILE_MONITOR_EVENT_MODIFIED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_TOUCH,
+  .filesrc = "two.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_ATTRIBUTES },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 2,
+  .eventid = QFILE_MONITOR_EVENT_ATTRIBUTES },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_DEL_WATCH,
+

Re: [Qemu-devel] [PATCH RFC v3 00/11] Add RX archtecture support

[Yoshinori Sato]

On Thu, 21 Mar 2019 10:35:07 +0900,
Richard Henderson wrote:
> 
> On 3/20/19 7:05 AM, Yoshinori Sato wrote:
> > OK. fixed another way.
> > But RX big-endian mode only data access.
> > So operand value always little-endian order.
> 
> Oh that is convenient.
> Therefore the operand can always be put together by pieces.  E.g.
> 
> -%b4_dsp_16 0:16 !function=dsp16
> -%b4_bdsp   0:24 !function=bdsp_a
> +%b4_dsp16  0:s8 8:8
> +%b4_dsp24  0:s8 8:8 16:8
> 
> Also note the 's' qualifier that defines signed fields.
> 
> -%b2_bdsp   16:8 !function=bdsp_b
> ...
> -@b2_bcnd_b  cd:4   &bcnd dsp=%b2_bdsp sz=2
> -@b2_bra_b      &jdsp dsp=%b2_bdsp sz=2
> +@b2_bcnd_b  cd:4 dsp:s8&bcnd sz=2
> +@b2_bra_b    dsp:s8&jdsp sz=2
>

OK.

> 
> >>> +/* push rs */
> >>> +static bool trans_PUSH_r(DisasContext *ctx, arg_PUSH_r *a)
> >>> +{
> >>> +if (a->rs != 0) {
> >>> +tcg_gen_subi_i32(cpu_regs[0], cpu_regs[0], 4);
> >>> +rx_gen_ldst(a->sz, RX_MEMORY_ST, cpu_regs[a->rs], cpu_regs[0]);
> >>> +} else {
> >>> +tcg_gen_mov_i32(ctx->src, cpu_regs[a->rs]);
> >>> +tcg_gen_subi_i32(cpu_regs[0], cpu_regs[0], 4);
> >>> +rx_gen_ldst(a->sz, RX_MEMORY_ST, ctx->src, cpu_regs[0]);
> >>> +}
> >>> +return true;
> >>
> >> As far as I can see the THEN and ELSE cases have identical operation.
> > 
> > It little different.
> > In the case of r0, the value before decrementing is stored in memory.
> > I added comment.
> 
> What I mean is that the sequence that you use for r0 could also be used for 
> all
> other rN.
> 
> I understand that RX does not have an mmu, but the normal way we handle this 
> is
> 
>   tcg_gen_subi_i32(addr, cpu_regs[0], 4);
>   rx_gen_st(a->sz, cpu_regs[a->rs], addr);
>   tcg_gen_mov_i32(cpu_regs[0], addr);
> 
> so that the stack pointer is not modified if the store raises an exception.
>

r0 is stack pointer.
The push / pop instructions read and write the address indicated by r0.

This part is complicated, so let's fix it a little more.
It should be able to expand into transfer instruction of
pre-decrement and post-increment.
> 
> r~
> 

-- 
Yosinori Sato

[Qemu-devel] [PATCH 3/3] filemon: fix watch IDs to avoid potential wraparound issues

[Daniel P . Berrangé]

Watch IDs are allocated from incrementing a int counter against
the QFileMonitor object. In very long life QEMU processes with
a huge amount of USB MTP activity creating & deleting directories
it is just about conceivable that the int counter can wrap
around. This would result in incorrect behaviour of the file
monitor watch APIs due to clashing watch IDs.

Instead of trying to detect this situation, this patch changes
the way watch IDs are allocated. It is turned into an int64_t
variable where the high 32 bits are set from the underlying
inotify "int" ID. This gives an ID that is guaranteed unique
for the directory as a whole, and we can rely on the kernel
to enforce this. QFileMonitor then sets the low 32 bits from
a per-directory counter.

The USB MTP device only sets watches on the directory as a
whole, not files within, so there is no risk of guest
triggered wrap around on the low 32 bits.

Signed-off-by: Daniel P. Berrangé 
---
 authz/listfile.c  |   2 +-
 hw/usb/dev-mtp.c  |  10 +--
 include/authz/listfile.h  |   2 +-
 include/qemu/filemonitor.h|  16 ++---
 tests/test-util-filemonitor.c | 128 +++---
 util/filemonitor-inotify.c|  25 +++
 util/filemonitor-stub.c   |   4 +-
 util/trace-events |   6 +-
 8 files changed, 103 insertions(+), 90 deletions(-)

diff --git a/authz/listfile.c b/authz/listfile.c
index d4579767e7..bc2b58ef6d 100644
--- a/authz/listfile.c
+++ b/authz/listfile.c
@@ -93,7 +93,7 @@ qauthz_list_file_load(QAuthZListFile *fauthz, Error **errp)
 
 
 static void
-qauthz_list_file_event(int wd G_GNUC_UNUSED,
+qauthz_list_file_event(int64_t wd G_GNUC_UNUSED,
QFileMonitorEvent ev G_GNUC_UNUSED,
const char *name G_GNUC_UNUSED,
void *opaque)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 06e376bcd2..0cce3f53fe 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -170,7 +170,7 @@ struct MTPObject {
 char *path;
 struct stat  stat;
 /* file monitor watch id */
-int  watchid;
+int64_t  watchid;
 MTPObject*parent;
 uint32_t nchildren;
 QLIST_HEAD(, MTPObject) children;
@@ -498,7 +498,7 @@ static MTPObject *usb_mtp_object_lookup_name(MTPObject 
*parent,
 return NULL;
 }
 
-static MTPObject *usb_mtp_object_lookup_id(MTPState *s, int id)
+static MTPObject *usb_mtp_object_lookup_id(MTPState *s, int64_t id)
 {
 MTPObject *iter;
 
@@ -511,7 +511,7 @@ static MTPObject *usb_mtp_object_lookup_id(MTPState *s, int 
id)
 return NULL;
 }
 
-static void file_monitor_event(int id,
+static void file_monitor_event(int64_t id,
QFileMonitorEvent ev,
const char *name,
void *opaque)
@@ -625,8 +625,8 @@ static void usb_mtp_object_readdir(MTPState *s, MTPObject 
*o)
 }
 
 if (s->file_monitor) {
-int id = qemu_file_monitor_add_watch(s->file_monitor, o->path, NULL,
- file_monitor_event, s, &err);
+int64_t id = qemu_file_monitor_add_watch(s->file_monitor, o->path, 
NULL,
+ file_monitor_event, s, &err);
 if (id == -1) {
 error_report("usb-mtp: failed to add watch for %s: %s", o->path,
  error_get_pretty(err));
diff --git a/include/authz/listfile.h b/include/authz/listfile.h
index ebbd5a4288..0d618c903c 100644
--- a/include/authz/listfile.h
+++ b/include/authz/listfile.h
@@ -92,7 +92,7 @@ struct QAuthZListFile {
 char *filename;
 bool refresh;
 QFileMonitor *file_monitor;
-int file_watch;
+int64_t file_watch;
 };
 
 
diff --git a/include/qemu/filemonitor.h b/include/qemu/filemonitor.h
index cd031832ed..64267d09b2 100644
--- a/include/qemu/filemonitor.h
+++ b/include/qemu/filemonitor.h
@@ -52,7 +52,7 @@ typedef enum {
  * empty.
  *
  */
-typedef void (*QFileMonitorHandler)(int id,
+typedef void (*QFileMonitorHandler)(int64_t id,
 QFileMonitorEvent event,
 const char *filename,
 void *opaque);
@@ -103,12 +103,12 @@ void qemu_file_monitor_free(QFileMonitor *mon);
  *
  * Returns: a positive integer watch ID, or -1 on error
  */
-int qemu_file_monitor_add_watch(QFileMonitor *mon,
-const char *dirpath,
-const char *filename,
-QFileMonitorHandler cb,
-void *opaque,
-Error **errp);
+int64_t qemu_file_monitor_add_watch(QFileMonitor *mon,
+const char *dirpath,
+const char *filename,
+QFileMonitorHandler cb,
+void *opaque,
+

Re: [Qemu-devel] [RFC PATCH] scripts/decodetree.py: Fix variable-length ISA

[Yoshinori Sato]

On Thu, 21 Mar 2019 10:43:37 +0900,
Richard Henderson wrote:
> 
> On 3/20/19 7:27 AM, Yoshinori Sato wrote:
> > Hi.
> > I found some problem in tested RX instructions.
> > It is usable in RX instructions, but I think that there
> > is a better fix because I am not familiar with Python.
> 
> The patch itself look ok, but needs some changes.
> 
> > I fixed three point.
> > - Added ctx to !function args.
> 
> This seems fine.  But it needs either
> 
> (1) A command-line option, or
> (2) Changes to all existing users in the tree.
> 
> Otherwise the build breaks for the other targets.
> 
> > - Fixed group operaiton. varinsns required width field.
> > - Fixed symbol in decode_load_bytes args.
> 
> Thanks.  I can fold these fixes into v2 of the varinsns patches.
>

OK. Thanks.

> 
> r~
> 

-- 
Yosinori Sato

Re: [Qemu-devel] [PATCH v8 05/16] gdbstub: add multiprocess support to vCont packets

[Luc Michel]

On 3/22/19 2:29 PM, Jan Kiszka wrote:
> On 07.12.18 10:01, Luc Michel wrote:
>> Add the gdb_first_attached_cpu() and gdb_next_attached_cpu() to iterate
>> over all the CPUs in currently attached processes.
>>
>> Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
>> iterate over CPUs of a given process.
>>
>> Use them to add multiprocess extension support to vCont packets.
>>
>> Signed-off-by: Luc Michel 
>> Reviewed-by: Philippe Mathieu-Daudé 
>> Reviewed-by: Edgar E. Iglesias 
>> Acked-by: Alistair Francis 
>> ---
>>   gdbstub.c | 117 +++---
>>   1 file changed, 102 insertions(+), 15 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 911faa225a..77b3dbb2c8 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -721,10 +721,40 @@ static CPUState *find_cpu(uint32_t thread_id)
>>   }
>>   
>>   return NULL;
>>   }
>>   
>> +static CPUState *get_first_cpu_in_process(const GDBState *s,
>> +  GDBProcess *process)
>> +{
>> +CPUState *cpu;
>> +
>> +CPU_FOREACH(cpu) {
>> +if (gdb_get_cpu_pid(s, cpu) == process->pid) {
>> +return cpu;
>> +}
>> +}
>> +
>> +return NULL;
>> +}
>> +
>> +static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
>> +{
>> +uint32_t pid = gdb_get_cpu_pid(s, cpu);
>> +cpu = CPU_NEXT(cpu);
>> +
>> +while (cpu) {
>> +if (gdb_get_cpu_pid(s, cpu) == pid) {
>> +break;
>> +}
>> +
>> +cpu = CPU_NEXT(cpu);
>> +}
>> +
>> +return cpu;
>> +}
>> +
>>   static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
>>   {
>>   GDBProcess *process;
>>   CPUState *cpu;
>>   
>> @@ -750,10 +780,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, 
>> uint32_t pid, uint32_t tid)
>>   }
>>   
>>   return cpu;
>>   }
>>   
>> +/* Return the cpu following @cpu, while ignoring
>> + * unattached processes.
>> + */
>> +static CPUState *gdb_next_attached_cpu(const GDBState *s, CPUState *cpu)
>> +{
>> +cpu = CPU_NEXT(cpu);
>> +
>> +while (cpu) {
>> +if (gdb_get_cpu_process(s, cpu)->attached) {
>> +break;
>> +}
>> +
>> +cpu = CPU_NEXT(cpu);
>> +}
>> +
>> +return cpu;
>> +}
>> +
>> +/* Return the first attached cpu */
>> +static CPUState *gdb_first_attached_cpu(const GDBState *s)
>> +{
>> +CPUState *cpu = first_cpu;
>> +GDBProcess *process = gdb_get_cpu_process(s, cpu);
>> +
>> +if (!process->attached) {
>> +return gdb_next_attached_cpu(s, cpu);
>> +}
>> +
>> +return cpu;
>> +}
>> +
>>   static const char *get_feature_xml(const char *p, const char **newp,
>>  CPUClass *cc)
>>   {
>>   size_t len;
>>   int i;
>> @@ -1088,14 +1149,16 @@ static int is_query_packet(const char *p, const char 
>> *query, char separator)
>>* returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if 
>> there is
>>* a format error, 0 on success.
>>*/
>>   static int gdb_handle_vcont(GDBState *s, const char *p)
>>   {
>> -int res, idx, signal = 0;
>> +int res, signal = 0;
>>   char cur_action;
>>   char *newstates;
>>   unsigned long tmp;
>> +uint32_t pid, tid;
>> +GDBProcess *process;
>>   CPUState *cpu;
>>   #ifdef CONFIG_USER_ONLY
>>   int max_cpus = 1; /* global variable max_cpus exists only in system 
>> mode */
>>   
>>   CPU_FOREACH(cpu) {
>> @@ -1134,29 +1197,52 @@ static int gdb_handle_vcont(GDBState *s, const char 
>> *p)
>>   } else if (cur_action != 'c' && cur_action != 's') {
>>   /* unknown/invalid/unsupported command */
>>   res = -ENOTSUP;
>>   goto out;
>>   }
>> -/* thread specification. special values: (none), -1 = all; 0 = any 
>> */
>> -if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
>> -if (*p == ':') {
>> -p += 3;
>> -}
>> -for (idx = 0; idx < max_cpus; idx++) {
>> -if (newstates[idx] == 1) {
>> -newstates[idx] = cur_action;
>> +
>> +if (*p++ != ':') {
>> +res = -ENOTSUP;
>> +goto out;
>> +}
>> +
>> +switch (read_thread_id(p, &p, &pid, &tid)) {
>> +case GDB_READ_THREAD_ERR:
>> +res = -EINVAL;
>> +goto out;
>> +
>> +case GDB_ALL_PROCESSES:
>> +cpu = gdb_first_attached_cpu(s);
>> +while (cpu) {
>> +if (newstates[cpu->cpu_index] == 1) {
>> +newstates[cpu->cpu_index] = cur_action;
>>   }
>> +
>> +cpu = gdb_next_attached_cpu(s, cpu);
>>   }
>> -} else if (*p == ':') {
>> -p++;
>> -res = qemu_strtoul(p, &p, 16, &tmp);
>> -if (res) {
>> +break;
>> +
>> +

[Qemu-devel] [PATCH 0/3] filemon: various fixes / improvements to file monitor for USB MTP

[Daniel P . Berrangé]

Previously posted in 2 separate pieces at

  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04885.html
  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05833.html

And a failed pull request

  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06035.html

The last patch had bigger changes in the test suite than anticipated,
so needs a explicit new review before I can justify re-trying the PULL
request

Daniel P. Berrangé (3):
  tests: refactor file monitor test to make it more understandable
  filemon: ensure watch IDs are unique to QFileMonitor scope
  filemon: fix watch IDs to avoid potential wraparound issues

 authz/listfile.c  |   2 +-
 hw/usb/dev-mtp.c  |  10 +-
 include/authz/listfile.h  |   2 +-
 include/qemu/filemonitor.h|  16 +-
 tests/test-util-filemonitor.c | 648 +-
 util/filemonitor-inotify.c|  26 +-
 util/filemonitor-stub.c   |   4 +-
 util/trace-events |   6 +-
 8 files changed, 354 insertions(+), 360 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 2/3] filemon: ensure watch IDs are unique to QFileMonitor scope

[Daniel P . Berrangé]

The watch IDs are mistakenly only unique within the scope of the
directory being monitored. This is not useful for clients which are
monitoring multiple directories. They require watch IDs to be unique
globally within the QFileMonitor scope.

Reviewed-by: Marc-André Lureau 
Tested-by: Bandan Das 
Reviewed-by: Bandan Das 
Signed-off-by: Daniel P. Berrangé 
---
 tests/test-util-filemonitor.c | 116 +++---
 util/filemonitor-inotify.c|   5 +-
 2 files changed, 110 insertions(+), 11 deletions(-)

diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index ea3715a8f4..71a7cf5de0 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -35,6 +35,8 @@ enum {
 QFILE_MONITOR_TEST_OP_RENAME,
 QFILE_MONITOR_TEST_OP_TOUCH,
 QFILE_MONITOR_TEST_OP_UNLINK,
+QFILE_MONITOR_TEST_OP_MKDIR,
+QFILE_MONITOR_TEST_OP_RMDIR,
 };
 
 typedef struct {
@@ -298,6 +300,54 @@ test_file_monitor_events(void)
   .eventid = QFILE_MONITOR_EVENT_DELETED },
 
 
+{ .type = QFILE_MONITOR_TEST_OP_MKDIR,
+  .filesrc = "fish", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "fish", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+  .filesrc = "fish/", .watchid = 4 },
+{ .type = QFILE_MONITOR_TEST_OP_ADD_WATCH,
+  .filesrc = "fish/one.txt", .watchid = 5 },
+{ .type = QFILE_MONITOR_TEST_OP_CREATE,
+  .filesrc = "fish/one.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 4,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 5,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_DEL_WATCH,
+  .filesrc = "fish/one.txt", .watchid = 5 },
+{ .type = QFILE_MONITOR_TEST_OP_RENAME,
+  .filesrc = "fish/one.txt", .filedst = "two.txt", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "one.txt", .watchid = 4,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = 2,
+  .eventid = QFILE_MONITOR_EVENT_CREATED },
+
+
+{ .type = QFILE_MONITOR_TEST_OP_RMDIR,
+  .filesrc = "fish", },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "", .watchid = 4,
+  .eventid = QFILE_MONITOR_EVENT_IGNORED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "fish", .watchid = 0,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+{ .type = QFILE_MONITOR_TEST_OP_DEL_WATCH,
+  .filesrc = "fish", .watchid = 4 },
+
+
 { .type = QFILE_MONITOR_TEST_OP_UNLINK,
   .filesrc = "two.txt", },
 { .type = QFILE_MONITOR_TEST_OP_EVENT,
@@ -366,6 +416,8 @@ test_file_monitor_events(void)
 int fd;
 int watchid;
 struct utimbuf ubuf;
+char *watchdir;
+const char *watchfile;
 
 pathsrc = g_strdup_printf("%s/%s", dir, op->filesrc);
 if (op->filedst) {
@@ -378,13 +430,26 @@ test_file_monitor_events(void)
 g_printerr("Add watch %s %s %d\n",
dir, op->filesrc, op->watchid);
 }
+if (op->filesrc && strchr(op->filesrc, '/')) {
+watchdir = g_strdup_printf("%s/%s", dir, op->filesrc);
+watchfile = strrchr(watchdir, '/');
+*(char *)watchfile = '\0';
+watchfile++;
+if (*watchfile == '\0') {
+watchfile = NULL;
+}
+} else {
+watchdir = g_strdup(dir);
+watchfile = op->filesrc;
+}
 watchid =
 qemu_file_monitor_add_watch(mon,
-dir,
-op->filesrc,
+watchdir,
+watchfile,
 qemu_file_monitor_test_handler,
 &data,
 &local_err);
+g_free(watchdir);
 if (watchid < 0) {
 g_printerr("Unable to add watch %s",
error_get_pretty(local_err));
@@ -400,9 +465,17 @@ test_file_monitor_events(void)
 if (debug) {
 g_printerr("Del watch %s %d\n", dir, op->watchid);
 }
+if (op->filesrc && strchr(op->filesrc, '/')) {
+watchdir = g_strdup_printf("%s/%s", dir, 

Re: [Qemu-devel] [PATCH for-4.0? 0/6] block: Fix slow pre-zeroing in qemu-img convert

[Eric Blake]

On 3/22/19 9:21 AM, Eric Blake wrote:
> On 3/22/19 8:21 AM, Kevin Wolf wrote:
>> If qemu-img convert sees that the target image isn't zero-initialised
>> yet, it tries to do an efficient zero write for the whole image first
>> to save the overhead of repeated explicit zero writes during the
>> conversion. Obviously, this provides only an advantage if the
>> pre-zeroing is actually efficient. Otherwise, we can end up writing
>> zeroes slowly while zeroing out the whole image, and then overwrite the
>> same blocks again with real data, potentially doubling the written data.
>>
>> Additionally, commit 9776f0db changed NBD to advertise for all NBD
>> devices that zero writes with unmap is supported for all NBD devices, no
>> matter whether the storage of NBD server actually can do this or whether
>> we would fall back to explicit zero writes.
>>
>> This series adds a new BDRV_REQ_NO_FALLBACK flag for writing zeroes,
>> which makes the request return an error if it can't guarantee that we
>> don't end up running a slow fallback path.
>>
>> For NBD, this means that we still support zero writes from guests, but
>> qemu-img convert will not try to use it to zero out the whole image.
>> This is potentially suboptimal if the server does actually support
>> efficient zero writes. We'd need an NBD protocol extension to transfer
>> this flag so we can re-enable the qemu-img convert feature for the NBD
>> driver without regressing on storage that can't provide efficient zero
>> writes.
> 
> I haven't reviewed closely yet, but the summary is sane, so:
> Acked-by: Eric Blake 
> 
> and I'll be cross-posting another mail soon with the desired NBD
> protocol extension.

I also meant to add - since the performance slowdown is so noticeable,
and we are still at -rc0, I think this series is reasonable as 4.0
material as a bug fix (performance bugs rank lower than correctness
bugs, but they are still bugs)

-- 
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] migration: avoid copying ignore-shared ramblock when in incoming migration

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Fri, 22 Mar 2019 at 10:12, Dr. David Alan Gilbert
>  wrote:
> > Right, so in Catherine's patch there's a simple in_incoming_migration
> > and checking ramblock_is_ignored
> 
> Mmm, but I think it is in the wrong place. It is being checked
> in address_space_write_rom_internal(). Either we want to
> suppress any and all writes to these RAM blocks, in which
> case I don't think that function covers all the ways that
> code can get hold of a RAM block and write to it; or we are
> confident that only the ROM blobs are an issue, in which
> case it is too low in the call stack and we should do the
> check in rom_reset().
> 
> Are there any other cases where we might write to RAM
> during reset/migration ? I thought of "user write via
> the debug stub or monitor", but perhaps those either
> can't happen or we define them as user error. But I
> there might be some other obscure cases, which perhaps
> argues for doing this at the lowest level possible.

Right, the thought of the 'might be other obscure cases'
is why in Yury's 'QEMU may write to system_memory before guest starts'
patch he marks all shared regions as read-only to see what
hits it.

I'm not sure; tbh inserting this type of check at the lowest
level seems a bit invasive so I'd prefer doing it at the ROM blocks
level; but we are bound to hit those obscure cases and then
the failure is a real pain to debug when you find something
has overwritten some of the RAM.

Dave

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

Re: [Qemu-devel] [PULL 0/5] x86 queue for -rc1

[Peter Maydell]

On Thu, 21 Mar 2019 at 19:36, Eduardo Habkost  wrote:
>
> The following changes since commit 62a172e6a77d9072bb1a18f295ce0fcf4b90a4f2:
>
>   Update version for v4.0.0-rc0 release (2019-03-19 17:17:22 +)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-next-pull-request
>
> for you to fetch changes up to 21ee4787e53367590f284915bf4c30c684e65bdf:
>
>   docs: add note about stibp CPU feature for spectre v2 (2019-03-20 12:18:15 
> -0300)
>
> 
> x86 queue for -rc1
>
> A few fixes that missed -rc0:
> * CPU model documentation updates (Daniel P. Berrangé)
> * Fix bogus OSPKE warnings (Eduardo Habkost)
> * Work around KVM bugs when handing arch_capabilities
>   (Eduardo Habkost)
>

Applied, thanks.

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

-- PMM

Re: [Qemu-devel] [qemu-s390x] [PATCH] hw/s390x: fix clang compilation on 32bit machines

[Halil Pasic]

On Mon, 18 Mar 2019 22:08:50 +0100
Philippe Mathieu-Daudé  wrote:

> Le lun. 18 mars 2019 11:34, Marcel Apfelbaum  a
> écrit :
> 
> > Hi Christian,
> >
> > On 3/18/19 11:27 AM, Christian Borntraeger wrote:
> > >
> > > On 16.03.19 12:09, Philippe Mathieu-Daudé wrote:
> > >> Hi Marcel,
> > >>
> > >> On 3/16/19 10:50 AM, Marcel Apfelbaum wrote:
> > >>> Configuring QEMU with:
> > >>>  configure --cc=clang --target-list=s390x-softmmu
> > >>> And compiling it using a 32 bit machine leads to:
> > >> Because there sizeof(ram_addr_t) = sizeof(uintptr_t) = 32.
> > >>
> > >>>  hw/s390x/s390-virtio-ccw.c:175:27: error: implicit conversion from
> > >>>'unsigned long long' to 'ram_addr_t' (aka 'unsigned int')
> > changes value
> > >>>from 8796091973632 to 4293918720 [-Werror,-Wconstant-conversion]
> > >>>  chunk = MIN(size, KVM_SLOT_MAX_BYTES);
> > >>>~ ~~^~~
> > >> The comment 1 line earlier is:
> > >>
> > >>  /* KVM does not allow memslots >= 8 TB */
> > >>
> > >> Clang is correct, this KVM_SLOT_MAX_BYTES is incorrect on a 32bit s390,
> > >> you need a 64bit system.

Sorry guys for the long wait. We are decimated by flue at the moment.

IMHO Clang is wrong about this. The value put in chunk is guaranteed to
fit unsigned int.

Namely


static void s390_memory_init(ram_addr_t mem_size)   
{   
MemoryRegion *sysmem = get_system_memory(); 
ram_addr_t chunk, offset = 0;   
unsigned int number = 0;
gchar *name;

/* allocate RAM for core */ 
name = g_strdup_printf("s390.ram"); 
while (mem_size) {  
MemoryRegion *ram = g_new(MemoryRegion, 1); 
uint64_t size = mem_size;

The most significant 32 bits of size are zeros because mem_size
is effectively uint. 

/* KVM does not allow memslots >= 8 TB */   
chunk = MIN(size, KVM_SLOT_MAX_BYTES);

Thus the result of MIN() is guaranteed to fit into chunk despite of its
type being wider.

> > > KVM is only supported on 64bit s390.
> > >
> >
> > So maybe the fix I proposed is enough.
> >
> 
> Enough to silent a warning due to a bug, as confirmed Christian KVM code
> should be reachable on 32 bit hosts.
> Safer would it be to fix the bug.



IMHO there is no bug! Thus I think Marcel's fix is sufficient. A simple
cast to ram_addr_t could be even simpler, but I did not check if that
silences Clang. @Marcel would you like to try that out?

> 
> >
> > >
> > >> Per Hacking:
> > >>
> > >>Use hwaddr for guest physical addresses except pcibus_t
> > >>for PCI addresses.  In addition, ram_addr_t is a QEMU internal
> > address
> > >>space that maps guest RAM physical addresses into an intermediate
> > >>address space that can map to host virtual address spaces.  Generally
> > >>speaking, the size of guest memory can always fit into ram_addr_t but
> > >>it would not be correct to store an actual guest physical address in
> > a
> > >>ram_addr_t.
> > >>
> > >> My understanding is we should not use ram_addr_t with KVM but rather
> > >> hwaddr, but I'm not sure.

I tend to agree with you. The usage of the types is IMHO messy in the
function under discussion. But I'm not a memory guy, and I would hate to
make calls on this.

> > >>
> > >> I don't know about s390, if 32bit host is supported or supports KVM.
> > >> If it is, maybe this could work:
> > >>
> > >> I don't think the following is clean:
> > >>
> > >> #if TARGET_LONG_BITS == 32
> > >> # define KVM_SLOT_MAX_BYTES RAM_ADDR_MAX
> > >> #else
> > >> # define KVM_SLOT_MAX_BYTES \
> > >>   ((KVM_MEM_MAX_NR_PAGES * TARGET_PAGE_SIZE) & SEG_MSK)
> > >> #endif
> > >>
> > >> But checking ifdef CONFIG_KVM might be clever:
> > >>
> > >> -- >8 --
> > >> @@ -161,7 +161,7 @@ static void virtio_ccw_register_hcalls(void)
> > >>   static void s390_memory_init(ram_addr_t mem_size)
> > >>   {
> > >>   MemoryRegion *sysmem = get_system_memory();
> > >> -ram_addr_t chunk, offset = 0;
> > >> +hwaddr offset = 0;
> > >>   unsigned int number = 0;
> > >>   gchar *name;
> > >>
> > >> @@ -169,14 +169,16 @@ static void s390_memory_init(ram_addr_t mem_size)
> > >>   name = g_strdup_printf("s390.ram");
> > >>   while (mem_size) {
> > >>   MemoryRegion *ram = g_new(MemoryRegion, 1);
> > >> -  

Re: [Qemu-devel] [PATCH 1/2] iotests: 030 TestParallelOps non-shared base node

[Alberto Garcia]

On Thu 21 Mar 2019 03:51:12 PM CET, Alberto Garcia  wrote:

> I was checking the tests that run commit and stream in parallel in
> 030, but they do commit on the upper images and stream on the lower
> ones, so that's safe. I'll try to run them the other way around
> because we might have a problem there.

I considered these scenarios with the following backing chain:

   E <- D <- C <- B <- A

1) stream from C to A, then commit from C to E

   This fails because qmp_block_commit() checks for op blockers in C's
   overlay (B), which is blocked by the stream block job.
   ("Node 'B' is busy: block device is in use by block job: stream")

2) commit from C to E, then stream from C to A

   This fails because the commit job inserts a filter between C and B
   and the bdrv_freeze_backing_chain(bs, base) call in stream_start()
   fails.

   However! I found this crash in a couple of occasions, I believe that
   it happens if the commit job finishes before block_stream, but I need
   to debug it further to see why the previous error didn't happen.

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x559aca6e745d in stream_prepare (job=0x559acdafad70) at 
block/stream.c:80
80  base_fmt = base->drv->format_name;
(gdb) print base
$1 = (BlockDriverState *) 0x559acd070240
(gdb) print base->drv
$2 = (BlockDriver *) 0xb5b5b5b5b5b5b5b5
(gdb) bt
#0  0x559aca6e745d in stream_prepare (job=0x559acdafad70) at 
block/stream.c:80
#1  0x559aca973a40 in job_prepare (job=0x559acdafad70) at job.c:771
#2  0x559aca9722fd in job_txn_apply (txn=0x559acd01e6d0, fn=0x559aca973a03 
) at job.c:146
#3  0x559aca973ad2 in job_do_finalize (job=0x559acdafad70) at job.c:788
#4  0x559aca973ca0 in job_completed_txn_success (job=0x559acdafad70) at 
job.c:842
#5  0x559aca973d3d in job_completed (job=0x559acdafad70) at job.c:855
#6  0x559aca973d8c in job_exit (opaque=0x559acdafad70) at job.c:874
#7  0x559acaa99c55 in aio_bh_call (bh=0x559acd3247f0) at util/async.c:90
#8  0x559acaa99ced in aio_bh_poll (ctx=0x559accfb9a30) at util/async.c:118
#9  0x559acaa9ebc0 in aio_dispatch (ctx=0x559accfb9a30) at 
util/aio-posix.c:460
#10 0x559acaa9a088 in aio_ctx_dispatch (source=0x559accfb9a30, 
callback=0x0, user_data=0x0) at util/async.c:261
#11 0x7f7d8e7787f7 in g_main_context_dispatch () from 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x559acaa9d4bf in glib_pollfds_poll () at util/main-loop.c:222
#13 0x559acaa9d539 in os_host_main_loop_wait (timeout=0) at 
util/main-loop.c:245
#14 0x559acaa9d63e in main_loop_wait (nonblocking=0) at util/main-loop.c:521
#15 0x559aca6c0ace in main_loop () at vl.c:1969
#16 0x559aca6c7db3 in main (argc=18, argv=0x7ffe11ee6d58, 
envp=0x7ffe11ee6df0) at vl.c:4589

So we need to look into this :( but I'd say that it seems that stream
should not need 'base' at all, just the node on top of it.

Berto

[Qemu-devel] [PATCH 1/4] target/mips: Fix . MSA instructions for MIPS big endian host

[Mateja Marjanovic]

From: Mateja Marjanovic 

Load and store MSA instructions when executed on a MIPS
big endian CPU, didn't change the endianness, and were
behaving like on little endian.

Signed-off-by: Mateja Marjanovic 
---
 target/mips/op_helper.c | 79 ++---
 1 file changed, 55 insertions(+), 24 deletions(-)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 0f272a5..5441ab2 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -4371,18 +4371,37 @@ FOP_CONDN_S(sne,  (float32_lt(fst1, fst0, 
&env->active_fpu.fp_status)
 #define MEMOP_IDX(DF)
 #endif
 
-#define MSA_LD_DF(DF, TYPE, LD_INSN, ...)   \
-void helper_msa_ld_ ## TYPE(CPUMIPSState *env, uint32_t wd, \
-target_ulong addr)  \
-{   \
-wr_t *pwd = &(env->active_fpu.fpr[wd].wr);  \
-wr_t wx;\
-int i;  \
-MEMOP_IDX(DF)   \
-for (i = 0; i < DF_ELEMENTS(DF); i++) { \
-wx.TYPE[i] = LD_INSN(env, addr + (i << DF), ##__VA_ARGS__); \
-}   \
-memcpy(pwd, &wx, sizeof(wr_t)); \
+#if defined(HOST_WORDS_BIGENDIAN)
+bool big_endian = 1;
+#else
+bool big_endian = 0;
+#endif
+
+#define MSA_LD_DF(DF, TYPE, LD_INSN, ...)   \
+void helper_msa_ld_ ## TYPE(CPUMIPSState *env, uint32_t wd, \
+target_ulong addr)  \
+{   \
+wr_t *pwd = &(env->active_fpu.fpr[wd].wr);  \
+wr_t wx;\
+int i, k;   \
+MEMOP_IDX(DF)   \
+if (!big_endian) {  \
+for (i = 0; i < DF_ELEMENTS(DF); i++) { \
+wx.TYPE[i] = LD_INSN(env, addr + (i << DF), ##__VA_ARGS__); \
+}   \
+} else {\
+for (i = 0; i < DF_ELEMENTS(DF); i++) { \
+if (i < DF_ELEMENTS(DF) / 2) {  \
+k = DF_ELEMENTS(DF) / 2 - i - 1;\
+wx.TYPE[i] = LD_INSN(env, addr + (k << DF), ##__VA_ARGS__); \
+} else {\
+k = 3 * DF_ELEMENTS(DF) / 2 - i - 1;\
+wx.TYPE[i] = LD_INSN(env, addr + (k << DF), ##__VA_ARGS__); \
+}   \
+}   \
+}   \
+\
+memcpy(pwd, &wx, sizeof(wr_t)); \
 }
 
 #if !defined(CONFIG_USER_ONLY)
@@ -4417,18 +4436,30 @@ static inline void ensure_writable_pages(CPUMIPSState 
*env,
 #endif
 }
 
-#define MSA_ST_DF(DF, TYPE, ST_INSN, ...)   \
-void helper_msa_st_ ## TYPE(CPUMIPSState *env, uint32_t wd, \
-target_ulong addr)  \
-{   \
-wr_t *pwd = &(env->active_fpu.fpr[wd].wr);  \
-int mmu_idx = cpu_mmu_index(env, false);   \
-int i;  \
-MEMOP_IDX(DF)   \
-ensure_writable_pages(env, addr, mmu_idx, GETPC()); \
-for (i = 0; i < DF_ELEMENTS(DF); i++) { \
-ST_INSN(env, addr + (i << DF), pwd->TYPE[i], ##__VA_ARGS__);\
-}   \
+#define MSA_ST_DF(DF, TYPE, ST_INSN, ...)\
+void helper_msa_st_ ## TYPE(CPUMIPSState *env, uint32_t wd,  \
+target_ulong addr)   \
+{\
+wr_t *pwd = &(env->active_fpu.fpr[wd].wr

[Qemu-devel] [PATCH 0/4] target/mips: Add support for MSA instructions on a big endian host

[Mateja Marjanovic]

From: Mateja Marjanovic 

Change endianness when the host machine has a big endian CPU, for
MSA instructions ., copy_., insert..

Mateja Marjanovic (4):
  target/mips: Fix . MSA instructions for MIPS big
endian host
  target/mips: Fix copy_s. for MIPS big endian host
  target/mips: Fix copy_u. for MIPS big endian host
  target/mips: Fix insert. for MIPS big endian host

 target/mips/msa_helper.c | 23 ++
 target/mips/op_helper.c  | 79 +---
 2 files changed, 78 insertions(+), 24 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 2/4] target/mips: Fix copy_s. for MIPS big endian host

[Mateja Marjanovic]

From: Mateja Marjanovic 

Signed element copy from MSA registers to GPR when
executed on a MIPS big endian CPU, didn't pick the
right element, and was behaving like on little endian.

Signed-off-by: Mateja Marjanovic 
---
 target/mips/msa_helper.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 421dced..012f373 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -1435,6 +1435,13 @@ void helper_msa_copy_s_df(CPUMIPSState *env, uint32_t 
df, uint32_t rd,
   uint32_t ws, uint32_t n)
 {
 n %= DF_ELEMENTS(df);
+#if defined(HOST_WORDS_BIGENDIAN)
+if (n < DF_ELEMENTS(df) / 2) {
+n = DF_ELEMENTS(df) / 2 - n - 1;
+} else {
+n = 3 * DF_ELEMENTS(df) / 2 - n - 1;
+}
+#endif
 
 switch (df) {
 case DF_BYTE:
-- 
2.7.4

Re: [Qemu-devel] [PATCH] RISC-V: fix single stepping over ret and other branching instructions

[Richard Henderson]

On 3/22/19 4:22 AM, Fabien Chouteau wrote:
> +/* Wrapper around tcg_gen_exit_tb that handles single stepping */
> +static void exit_tb(DisasContext *ctx, TranslationBlock *tb, unsigned idx)
> +{
> +if (ctx->base.singlestep_enabled) {
> +gen_exception_debug();
> +} else {
> +tcg_gen_exit_tb(tb, idx);
> +}
> +}

You should remove the TB and idx parameters here and pass NULL, 0 to
tcg_gen_exit_tb.

> @@ -138,14 +158,10 @@ static void gen_goto_tb(DisasContext *ctx, int n, 
> target_ulong dest)
>  /* chaining is only allowed when the jump is to the same page */
>  tcg_gen_goto_tb(n);
>  tcg_gen_movi_tl(cpu_pc, dest);
> -tcg_gen_exit_tb(ctx->base.tb, n);
> +exit_tb(ctx, ctx->base.tb, n);

Because this is the only non-zero use, and it is already protected by
use_goto_tb, which includes the single-step check.

Because goto_tb(n) must be paired with exit_tb(tb, n), and vice-versa.


r~

[Qemu-devel] [PATCH 4/4] target/mips: Fix insert. for MIPS big endian host

[Mateja Marjanovic]

From: Mateja Marjanovic 

Inserting from GPR to an element in a MSA register when
executed on a MIPS big endian CPU, didn't pick the
right element, and was behaving like on little endian.

Signed-off-by: Mateja Marjanovic 
---
 target/mips/msa_helper.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 8caf186..0049191 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -1500,6 +1500,13 @@ void helper_msa_insert_df(CPUMIPSState *env, uint32_t 
df, uint32_t wd,
 {
 wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
 target_ulong rs = env->active_tc.gpr[rs_num];
+#if defined(HOST_WORDS_BIGENDIAN)
+if (n < DF_ELEMENTS(df) / 2) {
+n = DF_ELEMENTS(df) / 2 - n - 1;
+} else {
+n = 3 * DF_ELEMENTS(df) / 2 - n - 1;
+}
+#endif
 
 switch (df) {
 case DF_BYTE:
@@ -1511,9 +1518,11 @@ void helper_msa_insert_df(CPUMIPSState *env, uint32_t 
df, uint32_t wd,
 case DF_WORD:
 pwd->w[n] = (int32_t)rs;
 break;
+#ifdef TARGET_MIPS64
 case DF_DOUBLE:
 pwd->d[n] = (int64_t)rs;
 break;
+#endif
 default:
 assert(0);
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH 0/2] trace: fix compilation issues for QEMU 4.0.0

[Stefan Hajnoczi]

On Thu, Mar 21, 2019 at 05:08:29PM +, Stefan Hajnoczi wrote:
> These patches fix compilation issues spotted by Markus Armbruster
> .  See the patches for details.
> 
> Stefan Hajnoczi (2):
>   trace: handle tracefs path truncation
>   trace: avoid SystemTap dtrace(1) warnings on empty files
> 
>  trace/ftrace.c| 12 ++--
>  scripts/tracetool/format/d.py |  5 +
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> -- 
> 2.20.1
> 

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH 3/4] target/mips: Fix copy_u. for MIPS big endian host

[Mateja Marjanovic]

From: Mateja Marjanovic 

Unsigned element copy from MSA registers to GPR when
executed on a MIPS big endian CPU, didn't pick the
right element, and was behaving like on little endian.

Signed-off-by: Mateja Marjanovic 
---
 target/mips/msa_helper.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 012f373..8caf186 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -1467,6 +1467,13 @@ void helper_msa_copy_u_df(CPUMIPSState *env, uint32_t 
df, uint32_t rd,
   uint32_t ws, uint32_t n)
 {
 n %= DF_ELEMENTS(df);
+#if defined(HOST_WORDS_BIGENDIAN)
+if (n < DF_ELEMENTS(df) / 2) {
+n = DF_ELEMENTS(df) / 2 - n - 1;
+} else {
+n = 3 * DF_ELEMENTS(df) / 2 - n - 1;
+}
+#endif
 
 switch (df) {
 case DF_BYTE:
-- 
2.7.4

[Qemu-devel] [PATCH 1/3] target/arm: fix crash on pmu register access

[Andrew Jones]

Fix a QEMU NULL derefence that occurs when the guest attempts to
enable PMU counters with a non-v8 cpu model or a v8 cpu model
which has not configured a PMU.

Fixes: 4e7beb0cc0f3 ("target/arm: Add a timer to predict PMU counter overflow")
Signed-off-by: Andrew Jones 
---
 target/arm/helper.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index c8d3c213b6b7..fc73488f6cc0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1259,6 +1259,10 @@ static bool pmu_counter_enabled(CPUARMState *env, 
uint8_t counter)
 int el = arm_current_el(env);
 uint8_t hpmn = env->cp15.mdcr_el2 & MDCR_HPMN;
 
+if (!arm_feature(env, ARM_FEATURE_PMU)) {
+return false;
+}
+
 if (!arm_feature(env, ARM_FEATURE_EL2) ||
 (counter < hpmn || counter == 31)) {
 e = env->cp15.c9_pmcr & PMCRE;
-- 
2.17.2

Re: [Qemu-devel] [PATCH 0/5] trace-events: Clean up

[Stefan Hajnoczi]

On Thu, Mar 14, 2019 at 07:09:24PM +0100, Markus Armbruster wrote:
> Markus Armbruster (5):
>   trace-events: Consistently point to docs/devel/tracing.txt
>   trace-events: Shorten file names in comments
>   scripts/cleanup-trace-events: Update for current practice
>   trace-events: Delete unused trace points
>   trace-events: Fix attribution of trace points to source
> 
>  accel/kvm/trace-events  |  2 +-
>  accel/tcg/trace-events  |  2 +-
>  audio/trace-events  |  6 +--
>  authz/trace-events  | 10 ++---
>  block/trace-events  | 49 +++
>  chardev/trace-events|  4 +-
>  crypto/trace-events | 10 ++---
>  hw/9pfs/trace-events|  2 +-
>  hw/acpi/trace-events|  6 +--
>  hw/alpha/trace-events   |  2 +-
>  hw/arm/trace-events | 17 +++-
>  hw/audio/trace-events   |  6 +--
>  hw/block/dataplane/trace-events |  2 +-
>  hw/block/trace-events   | 15 ---
>  hw/char/trace-events| 24 +--
>  hw/display/trace-events | 28 ++---
>  hw/dma/trace-events |  6 +--
>  hw/gpio/trace-events|  2 +-
>  hw/hppa/trace-events|  2 +-
>  hw/i2c/trace-events |  2 +-
>  hw/i386/trace-events| 10 ++---
>  hw/i386/xen/trace-events|  6 ++-
>  hw/ide/trace-events | 23 +--
>  hw/input/trace-events   | 16 
>  hw/intc/trace-events| 35 -
>  hw/isa/trace-events |  4 +-
>  hw/mem/trace-events |  4 +-
>  hw/misc/macio/trace-events  |  9 ++---
>  hw/misc/trace-events| 40 +--
>  hw/net/trace-events | 42 ++--
>  hw/nvram/trace-events   |  4 +-
>  hw/pci-host/trace-events|  6 +--
>  hw/pci/trace-events |  6 +--
>  hw/ppc/trace-events | 40 +--
>  hw/rdma/trace-events|  4 +-
>  hw/rdma/vmw/trace-events|  4 +-
>  hw/s390x/trace-events   |  4 +-
>  hw/scsi/trace-events| 22 +--
>  hw/sd/trace-events  | 13 +++---
>  hw/sparc/trace-events   |  6 +--
>  hw/sparc64/trace-events |  6 +--
>  hw/timer/trace-events   | 24 +--
>  hw/tpm/trace-events | 12 +++---
>  hw/usb/trace-events | 22 +--
>  hw/vfio/trace-events| 15 ---
>  hw/virtio/trace-events  | 10 ++---
>  hw/watchdog/trace-events|  2 +-
>  hw/xen/trace-events |  6 +--
>  io/trace-events | 12 +++---
>  linux-user/trace-events |  3 +-
>  migration/trace-events  | 70 ++---
>  nbd/trace-events| 10 ++---
>  net/trace-events| 10 ++---
>  qapi/trace-events   |  4 +-
>  qom/trace-events|  2 +-
>  scripts/cleanup-trace-events.pl | 19 ++---
>  scsi/trace-events   |  4 +-
>  target/arm/trace-events |  4 +-
>  target/hppa/trace-events|  4 +-
>  target/i386/trace-events|  4 +-
>  target/mips/trace-events|  2 +-
>  target/ppc/trace-events |  2 +-
>  target/s390x/trace-events   | 10 ++---
>  target/sparc/trace-events   |  8 ++--
>  trace-events| 13 +-
>  ui/trace-events | 19 +
>  util/trace-events   | 28 ++---
>  67 files changed, 420 insertions(+), 400 deletions(-)

I have folded in your Patch 1 commit description clarification.

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] qcow2: avoid lseek on block_status if possible

[Vladimir Sementsov-Ogievskiy]

25.01.2019 17:21, Vladimir Sementsov-Ogievskiy wrote:
> drv_co_block_status digs bs->file for additional, more accurate search
> for hole inside region, reported as DATA by bs since 5daa74a6ebc.
> 
> This accuracy is not free: assume we have qcow2 disk. Actually, qcow2
> knows, where are holes and where is data. But every block_status
> request calls lseek additionally. Assume a big disk, full of
> data, in any iterative copying block job (or img convert) we'll call
> lseek(HOLE) on every iteration, and each of these lseeks will have to
> iterate through all metadata up to the end of file. It's obviously
> ineffective behavior. And for many scenarios we don't need this lseek
> at all.
> 
> However, lseek is needed when we have metadata-preallocated image.
> 
> So, let's detect metadata-preallocation case and don't dig qcow2's
> protocol file in other cases.
> 
> The idea is to compare allocation size in POV of filesystem with
> allocations size in POV of Qcow2 (by refcounts). If allocation in fs is
> significantly lower, consider it as metadata-preallocation case.
> 
> Suggested-by: Denis V. Lunev 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> Hi!
> 
> So, to continue talk about lseek/no lseek when qcow2 block_status reports
> DATA.
> 
> Results on tmpfs:
> cached is lseek cache by Kevin
> detect is this patch
> no lseek is just remove block_status query on bs->file->bs in
>   bdrv_co_block_status
> 
>  +-++++--+
>  | | master | cached | detect | no lseek |
>  +-++++--+
>  | test.qcow2  | 80 | 40 | 0.169  | 0.162|
>  +-++++--+
>  | test_forward.qcow2  | 79 | 0.171  | 0.169  | 0.163|
>  +-++++--+
>  | test_prealloc.qcow2 | 0.054  | 0.053  | 0.055  | 0.263|
>  +-++++--+
> 
>   block/qcow2.h |  1 +
>   include/block/block_int.h |  7 +++
>   block/io.c|  3 ++-
>   block/qcow2-refcount.c| 36 
>   block/qcow2.c |  7 +++
>   5 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 438a1dee9e..d7113ed44c 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -610,6 +610,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int 
> refcount_order,
>   void *cb_opaque, Error **errp);
>   int qcow2_shrink_reftable(BlockDriverState *bs);
>   int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size);
> +int qcow2_detect_metadata_preallocation(BlockDriverState *bs);
>   
>   /* qcow2-cluster.c functions */
>   int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index f605622216..c895ca7169 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -59,6 +59,12 @@
>   
>   #define BLOCK_PROBE_BUF_SIZE512
>   
> +typedef enum BdrvYesNoUnknown {
> +BDRV_UNKNOWN = 0,
> +BDRV_NO,
> +BDRV_YES,
> +} BdrvYesNoUnknown;
> +
>   enum BdrvTrackedRequestType {
>   BDRV_TRACKED_READ,
>   BDRV_TRACKED_WRITE,
> @@ -682,6 +688,7 @@ struct BlockDriverState {
>   bool probed;/* if true, format was probed rather than specified */
>   bool force_share; /* if true, always allow all shared permissions */
>   bool implicit;  /* if true, this filter node was automatically inserted 
> */
> +BdrvYesNoUnknown metadata_preallocation;
>   
>   BlockDriver *drv; /* NULL means no media */
>   void *opaque;
> diff --git a/block/io.c b/block/io.c
> index bd9d688f8b..815661750a 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2186,7 +2186,8 @@ static int coroutine_fn 
> bdrv_co_block_status(BlockDriverState *bs,
>   }
>   }
>   
> -if (want_zero && local_file && local_file != bs &&
> +if (want_zero && bs->metadata_preallocation != BDRV_NO &&
> +local_file && local_file != bs &&
>   (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>   (ret & BDRV_BLOCK_OFFSET_VALID)) {
>   int64_t file_pnum;
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 1c63ac244a..008196d849 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -3379,3 +3379,39 @@ int64_t qcow2_get_last_cluster(BlockDriverState *bs, 
> int64_t size)
>   "There are no references in the refcount 
> table.");
>   return -EIO;
>   }
> +
> +int qcow2_detect_metadata_preallocation(BlockDriverState *bs)
> +{
> +BDRVQcow2State *s = bs->opaque;
> +int64_t i, end_cluster, cluster_count = 0;
> +int64_t file_length, real_allocation, metadata_allocation, file_tail;
> +u

[Qemu-devel] [PATCH 2/3] target/arm: cortex-a7 and cortex-a15 have pmus

[Andrew Jones]

cortex-a7 and cortex-a15 have pmus (PMUv2) and they advertise
them in ID_DFR0. Let's allow them to function. This also enables
the pmu cpu property to work with these cpu types, i.e. we can
now do '-cpu cortex-a15,pmu=off' to remove the pmu.

Signed-off-by: Andrew Jones 
---
 target/arm/cpu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 96f0ff0ec727..504a4771fbd3 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1109,6 +1109,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 #endif
 } else {
 cpu->id_aa64dfr0 &= ~0xf00;
+cpu->id_dfr0 &= ~(0xf << 24);
 cpu->pmceid0 = 0;
 cpu->pmceid1 = 0;
 }
@@ -1744,6 +1745,7 @@ static void cortex_a7_initfn(Object *obj)
 set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
 set_feature(&cpu->env, ARM_FEATURE_EL2);
 set_feature(&cpu->env, ARM_FEATURE_EL3);
+set_feature(&cpu->env, ARM_FEATURE_PMU);
 cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7;
 cpu->midr = 0x410fc075;
 cpu->reset_fpsid = 0x41023075;
@@ -1789,6 +1791,7 @@ static void cortex_a15_initfn(Object *obj)
 set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
 set_feature(&cpu->env, ARM_FEATURE_EL2);
 set_feature(&cpu->env, ARM_FEATURE_EL3);
+set_feature(&cpu->env, ARM_FEATURE_PMU);
 cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
 cpu->midr = 0x412fc0f1;
 cpu->reset_fpsid = 0x410430f0;
-- 
2.17.2

[Qemu-devel] [PATCH 0/3] target/arm: pmu fixes

[Andrew Jones]

The first two patches fix a regression I found when running
the arm (as opposed to arm64) pmu kvm-unit-tests test on tcg,

  $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
  $ cd kvm-unit-tests
  $ ./configure --arch=arm --cross-prefix=arm-linux-gnu-
  $ make -j
  $ export QEMU=/path/to/qemu-system-arm or qemu-system-aarch64
  $ arm/run arm/pmu.flat

After checking the QEMU code I found it's also reproducible with
the arm64 test if the PMU is removed,

  $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
  $ make -j
  $ export QEMU=/path/to/qemu-system-aarch64
  $ arm/run arm/pmu.flat -cpu cortex-a57,pmu=off

Those tests used to pass, but now QEMU was crashing. I've broken
the fix into two patches because the second patch is a bit of
an RFC since I don't know if it's safe to enable all
ARM_FEATURE_PMU code for PMUv2. Maybe that feature is only for
PMUv3? This patch also enables the pmu cpu property to work with
those cpu types, i.e. we can now do '-cpu cortex-a15,pmu=off' to
remove the pmu. Although it wasn't clear to me if the PMU is
optional (permitted to be removed) on those cpu types from the
manuals.

The last patch is just a trivial cleanup.

Andrew Jones (3):
  target/arm: fix crash on pmu register access
  target/arm: cortex-a7 and cortex-a15 have pmus
  target/arm: make pmccntr_op_start/finish static

 target/arm/cpu.c|  3 +++
 target/arm/cpu.h| 11 ---
 target/arm/helper.c |  8 ++--
 3 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH v4 2/8] slirp: relicense GPL files to BSD-3

[Marc-André Lureau]

In order to make slirp a standalone project, the project must have a
clear license, and be compatible with the GPL or LGPL.

Since commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
advertising clause from the slirp license"), slirp is BSD-3. But new
files have been added under slirp/ with QEMU GPL license since then.

The copyright holders have been asked to relicense files to BSD-3 and
gave their permission:

- slirp/dhcpv6.{c,h}

Subject: Re: Clearing slirp/ license
To: "Marc-André Lureau" , QEMU 
, Thomas Huth 
Cc: Peter Maydell , Samuel Thibault 

References: 
From: "Cédric Le Goater" 
Message-ID: 
Date: Mon, 11 Mar 2019 16:23:25 +0100

> Could you reply that you have no objection in relicensing those files
> are 3-Clause BSD?

Fine for me. You can change the license of slirp/ncsi.c and
slirp/ncsi-pkt.hto a 3-Clause BSD.

Thanks,

C.

Subject: Re: [Qemu-devel] Clearing slirp/ license
To: Peter Maydell , Shan Gavin 
Cc: Alexey Kardashevskiy , "Marc-André Lureau" 
, Gavin Shan , Thomas 
Huth , QEMU , Samuel Thibault 

References: 
 
 
 
 
 

From: "Cédric Le Goater" 
Message-ID: <4ddf6031-0df1-b3b5-965e-a181266e4...@kaod.org>
Date: Tue, 12 Mar 2019 11:49:21 +0100

> Is the code in question copyright you personally, or copyright
> IBM as your employer at the time ? If the latter, it is IBM that
> would need to approve the relicensing.

That was done. I had our legal team approve the change of license.

Thanks,

C.

From: Shan Gavin 
Date: Tue, 12 Mar 2019 15:04:54 +0800
Message-ID: 
Subject: Re: [Qemu-devel] Clearing slirp/ license
To: Alexey Kardashevskiy 
Cc: "Marc-André Lureau" , "Cédric Le Goater" 
, gws...@linux.vnet.ibm.com, Peter Maydell 
, Thomas Huth , QEMU 
, Samuel Thibault 

> Gavin, could you reply that you have no objection in relicensing
> ncsi-pkt.h as 3-Clause BSD?

No objection. Please go ahead with the relicensing.

Cheers,
Gavin

- ncsi.c, ncsi-pkt.h

Subject: Re: Clearing slirp/ license
To: "Marc-André Lureau" , QEMU 
, "Cédric Le Goater" 
Cc: Peter Maydell , Samuel Thibault 

References: 
From: Thomas Huth 
Message-ID: 
Date: Wed, 13 Feb 2019 12:30:32 +0100

> Could you reply that you have no objection in relicensing those files
> are 3-Clause BSD?

Ok, for the records: I'm fine if you change the license of dhcpv6.[ch]
to either 3-Clause BSD or 2-Clause BSD.

 Thomas

- vmstate.{c,h}

From: Juan Quintela 
To: "Marc-André Lureau" 
Cc: QEMU , Peter Maydell , 
Samuel Thibault 
Subject: Re: Clearing slirp/ license
Date: Tue, 12 Mar 2019 12:43:17 +0100
Message-ID: <87k1h4qpwq@trasno.org>

> Juan, Could you reply that you have no objection in relicensing the
> vmstate files as 3-Clause BSD?

No problem at all on my side.

Later, Juan.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
[ for the NC-SI files ]
Reviewed-by: Cédric Le Goater 
Acked-by: Thomas Huth 
---
 slirp/src/dhcpv6.h   | 31 +--
 slirp/src/ncsi-pkt.h | 33 +
 slirp/src/vmstate.h  | 42 +++---
 slirp/src/dhcpv6.c   | 37 +++--
 slirp/src/ncsi.c | 31 +--
 slirp/src/vmstate.c  | 31 +--
 6 files changed, 170 insertions(+), 35 deletions(-)

diff --git a/slirp/src/dhcpv6.h b/slirp/src/dhcpv6.h
index 3373f6cb89..af0e193b06 100644
--- a/slirp/src/dhcpv6.h
+++ b/slirp/src/dhcpv6.h
@@ -3,8 +3,35 @@
  *
  * Copyright 2016 Thomas Huth, Red Hat Inc.
  *
- * This work is licensed under the terms of the GNU GPL, version 2
- * or later. See the COPYING file in the top-level directory.
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT L

[Qemu-devel] [PATCH v4 8/8] slirp: is not maintained by Kelly Price for a long time

[Marc-André Lureau]

slirp has been maintained by the QEMU maintainers and will be
maintained under an independent project soon.

Reviewed-by: Eric Blake 
Signed-off-by: Kelly Price 
Signed-off-by: Marc-André Lureau 
---
 slirp/COPYRIGHT | 2 --
 1 file changed, 2 deletions(-)

diff --git a/slirp/COPYRIGHT b/slirp/COPYRIGHT
index 9863ea31cb..ed49512dbc 100644
--- a/slirp/COPYRIGHT
+++ b/slirp/COPYRIGHT
@@ -1,8 +1,6 @@
 Slirp was written by Danny Gasparovski.
 Copyright (c), 1995,1996 All Rights Reserved.
 
-Slirp is maintained by Kelly Price 
-
 Slirp is free software; "free" as in you don't have to pay for it, and you
 are free to do whatever you want with it.  I do not accept any donations,
 monetary or otherwise, for Slirp.  Instead, I would ask you to pass this
-- 
2.21.0.4.g36eb1cb9cf

[Qemu-devel] [PATCH v4 0/8] slirp: clarify license of slirp as BSD-3

[Marc-André Lureau]

Hi,

In order to make slirp a standalone project, the project must have a
clear license, and be compatible with the GPL or LGPL.

Since commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
advertising clause from the slirp license"), slirp is BSD-3. But new
files have been added under slirp/ with QEMU GPL license since then.

v4:
 - update SPDX-patches commit titles to differentiate them, as
   suggested by Eric
 - add a-b & r-b tags

v3:
 - add preliminary "slirp: update COPYRIGHT to use full 3-Clause BSD
   License"
 - split "slirp: clarify license of slirp files using SPDX" patch,
 - fix SPDX for files with MIT license header
 - add r-b tags, wording/spelling changes

v2:
 - split the initial patch to add BSD-3 header & then SPDX lines
 - do not modify existing copyright headers without copyright holder
   authorization
 - drop the weak/ambiguous notice to the COPYRIGHT file
 - added a RFC patch to remove Kelly Price from the maintainer duties

Marc-André Lureau (8):
  slirp: update COPYRIGHT to use full 3-Clause BSD License
  slirp: relicense GPL files to BSD-3
  slirp: clarify license of slirp files using SPDX: explicit BSD
  slirp: clarify license of slirp files using SPDX: explicit MIT
  slirp: clarify license of slirp files using SPDX: implicit via
COPYRIGHT
  slirp: clarify license of slirp files using SPDX: implicit via
unstated
  slirp: remove reference to COPYRIGHT file
  slirp: is not maintained by Kelly Price for a long time

 slirp/src/bootp.h  |  1 +
 slirp/src/debug.h  |  4 +---
 slirp/src/dhcpv6.h | 32 +--
 slirp/src/if.h |  4 +---
 slirp/src/ip.h |  1 +
 slirp/src/ip6.h|  1 +
 slirp/src/ip6_icmp.h   |  1 +
 slirp/src/ip_icmp.h|  1 +
 slirp/src/libslirp.h   |  1 +
 slirp/src/main.h   |  4 +---
 slirp/src/mbuf.h   |  1 +
 slirp/src/misc.h   |  4 +---
 slirp/src/ncsi-pkt.h   | 34 +
 slirp/src/qtailq.h |  1 +
 slirp/src/sbuf.h   |  4 +---
 slirp/src/slirp.h  |  1 +
 slirp/src/socket.h |  4 +---
 slirp/src/stream.h |  1 +
 slirp/src/tcp.h|  1 +
 slirp/src/tcp_timer.h  |  1 +
 slirp/src/tcp_var.h|  1 +
 slirp/src/tcpip.h  |  1 +
 slirp/src/tftp.h   |  1 +
 slirp/src/udp.h|  1 +
 slirp/src/util.h   |  1 +
 slirp/src/vmstate.h| 43 +++---
 slirp/src/arp_table.c  |  1 +
 slirp/src/bootp.c  |  1 +
 slirp/src/cksum.c  |  1 +
 slirp/src/dhcpv6.c | 38 +++--
 slirp/src/dnssearch.c  |  1 +
 slirp/src/if.c |  4 +---
 slirp/src/ip6_icmp.c   |  1 +
 slirp/src/ip6_input.c  |  1 +
 slirp/src/ip6_output.c |  1 +
 slirp/src/ip_icmp.c|  1 +
 slirp/src/ip_input.c   |  4 +---
 slirp/src/ip_output.c  |  4 +---
 slirp/src/mbuf.c   |  4 +---
 slirp/src/misc.c   |  4 +---
 slirp/src/ncsi.c   | 32 +--
 slirp/src/ndp_table.c  |  1 +
 slirp/src/sbuf.c   |  4 +---
 slirp/src/slirp.c  |  1 +
 slirp/src/socket.c |  4 +---
 slirp/src/state.c  |  1 +
 slirp/src/stream.c |  1 +
 slirp/src/tcp_input.c  |  4 +---
 slirp/src/tcp_output.c |  4 +---
 slirp/src/tcp_subr.c   |  4 +---
 slirp/src/tcp_timer.c  |  1 +
 slirp/src/tftp.c   |  1 +
 slirp/src/udp.c|  1 +
 slirp/src/udp6.c   |  1 +
 slirp/src/util.c   |  1 +
 slirp/src/vmstate.c| 32 +--
 slirp/COPYRIGHT|  5 +++--
 57 files changed, 229 insertions(+), 85 deletions(-)

-- 
2.21.0.4.g36eb1cb9cf

[Qemu-devel] [PATCH v4 4/8] slirp: clarify license of slirp files using SPDX: explicit MIT

[Marc-André Lureau]

Add SPDX license identifier to clarify the license of files with
explicit MIT license header.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Thomas Huth 
---
 slirp/src/util.h  | 1 +
 slirp/src/arp_table.c | 1 +
 slirp/src/bootp.c | 1 +
 slirp/src/dnssearch.c | 1 +
 slirp/src/slirp.c | 1 +
 slirp/src/state.c | 1 +
 slirp/src/stream.c| 1 +
 slirp/src/tftp.c  | 1 +
 slirp/src/util.c  | 1 +
 9 files changed, 9 insertions(+)

diff --git a/slirp/src/util.h b/slirp/src/util.h
index e94ee4e7f1..01f1e0e068 100644
--- a/slirp/src/util.h
+++ b/slirp/src/util.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * Copyright (c) 2003-2008 Fabrice Bellard
  * Copyright (c) 2010-2019 Red Hat, Inc.
diff --git a/slirp/src/arp_table.c b/slirp/src/arp_table.c
index 58eafdcfd8..9d7a59eb2c 100644
--- a/slirp/src/arp_table.c
+++ b/slirp/src/arp_table.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * ARP table
  *
diff --git a/slirp/src/bootp.c b/slirp/src/bootp.c
index d396849a05..b208e3b216 100644
--- a/slirp/src/bootp.c
+++ b/slirp/src/bootp.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * QEMU BOOTP/DHCP server
  *
diff --git a/slirp/src/dnssearch.c b/slirp/src/dnssearch.c
index c459cece8d..12c488971e 100644
--- a/slirp/src/dnssearch.c
+++ b/slirp/src/dnssearch.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * Domain search option for DHCP (RFC 3397)
  *
diff --git a/slirp/src/slirp.c b/slirp/src/slirp.c
index 18af670a0a..169c85b906 100644
--- a/slirp/src/slirp.c
+++ b/slirp/src/slirp.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * libslirp glue
  *
diff --git a/slirp/src/state.c b/slirp/src/state.c
index c3e3f0b671..09cea3590e 100644
--- a/slirp/src/state.c
+++ b/slirp/src/state.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * libslirp
  *
diff --git a/slirp/src/stream.c b/slirp/src/stream.c
index d114dde334..9c1764c0b7 100644
--- a/slirp/src/stream.c
+++ b/slirp/src/stream.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * libslirp io streams
  *
diff --git a/slirp/src/tftp.c b/slirp/src/tftp.c
index 2d8f978786..2071dca2a6 100644
--- a/slirp/src/tftp.c
+++ b/slirp/src/tftp.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * tftp.c - a simple, read-only tftp server for qemu
  *
diff --git a/slirp/src/util.c b/slirp/src/util.c
index 5ec2fa87ab..60bb200801 100644
--- a/slirp/src/util.c
+++ b/slirp/src/util.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: MIT */
 /*
  * util.c (mostly based on QEMU os-win32.c)
  *
-- 
2.21.0.4.g36eb1cb9cf

[Qemu-devel] [PATCH v4 7/8] slirp: remove reference to COPYRIGHT file

[Marc-André Lureau]

The slirp COPYRIGHT file is a BSD-3 license. Instead of referring to
another project file, the SPDX license notice present in all source
files states that unequivocally.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
---
 slirp/src/debug.h  | 3 ---
 slirp/src/if.h | 3 ---
 slirp/src/main.h   | 3 ---
 slirp/src/misc.h   | 3 ---
 slirp/src/sbuf.h   | 3 ---
 slirp/src/socket.h | 3 ---
 slirp/src/if.c | 3 ---
 slirp/src/ip_input.c   | 3 ---
 slirp/src/ip_output.c  | 3 ---
 slirp/src/mbuf.c   | 3 ---
 slirp/src/misc.c   | 3 ---
 slirp/src/sbuf.c   | 3 ---
 slirp/src/socket.c | 3 ---
 slirp/src/tcp_input.c  | 3 ---
 slirp/src/tcp_output.c | 3 ---
 slirp/src/tcp_subr.c   | 3 ---
 16 files changed, 48 deletions(-)

diff --git a/slirp/src/debug.h b/slirp/src/debug.h
index 2e503ad7fa..c95fd8ffd2 100644
--- a/slirp/src/debug.h
+++ b/slirp/src/debug.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef DEBUG_H_
diff --git a/slirp/src/if.h b/slirp/src/if.h
index 8a60c4e052..b71c37d6ea 100644
--- a/slirp/src/if.h
+++ b/slirp/src/if.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef IF_H
diff --git a/slirp/src/main.h b/slirp/src/main.h
index a88774215f..3b3f883703 100644
--- a/slirp/src/main.h
+++ b/slirp/src/main.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef SLIRP_MAIN_H
diff --git a/slirp/src/misc.h b/slirp/src/misc.h
index 4eaa2466d7..23b7490448 100644
--- a/slirp/src/misc.h
+++ b/slirp/src/misc.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef MISC_H
diff --git a/slirp/src/sbuf.h b/slirp/src/sbuf.h
index ece616e317..337af1bbde 100644
--- a/slirp/src/sbuf.h
+++ b/slirp/src/sbuf.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef SBUF_H
diff --git a/slirp/src/socket.h b/slirp/src/socket.h
index 10a0c78a26..25403898cd 100644
--- a/slirp/src/socket.h
+++ b/slirp/src/socket.h
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #ifndef SLIRP_SOCKET_H
diff --git a/slirp/src/if.c b/slirp/src/if.c
index b8cddebf66..6eaac7292a 100644
--- a/slirp/src/if.c
+++ b/slirp/src/if.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/ip_input.c b/slirp/src/ip_input.c
index 6ad6765938..a714fecd58 100644
--- a/slirp/src/ip_input.c
+++ b/slirp/src/ip_input.c
@@ -34,9 +34,6 @@
 /*
  * Changes and additions relating to SLiRP are
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/ip_output.c b/slirp/src/ip_output.c
index 927efb..8560197cf6 100644
--- a/slirp/src/ip_output.c
+++ b/slirp/src/ip_output.c
@@ -34,9 +34,6 @@
 /*
  * Changes and additions relating to SLiRP are
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/mbuf.c b/slirp/src/mbuf.c
index f079a86d78..800406ca9e 100644
--- a/slirp/src/mbuf.c
+++ b/slirp/src/mbuf.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 /*
diff --git a/slirp/src/misc.c b/slirp/src/misc.c
index da41f3bb5f..7c5db0e0aa 100644
--- a/slirp/src/misc.c
+++ b/slirp/src/misc.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms and conditions of the copyright.
  */
 
 #include "slirp.h"
diff --git a/slirp/src/sbuf.c b/slirp/src/sbuf.c
index 815823ffbe..9c0b31b513 100644
--- a/slirp/src/sbuf.c
+++ b/slirp/src/sbuf.c
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
- *
- * Please read the file COPYRIGHT for the
- * terms an

Re: [Qemu-devel] [PATCH v4 0/8] slirp: clarify license of slirp as BSD-3

[Samuel Thibault]

Marc-André Lureau, le ven. 22 mars 2019 17:46:12 +0100, a ecrit:
> On Fri, Mar 22, 2019 at 5:43 PM Marc-André Lureau
>  wrote:
> >
> > Hi,
> >
> > In order to make slirp a standalone project, the project must have a
> > clear license, and be compatible with the GPL or LGPL.
> >
> > Since commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
> > advertising clause from the slirp license"), slirp is BSD-3. But new
> > files have been added under slirp/ with QEMU GPL license since then.
> >
> > v4:
> >  - update SPDX-patches commit titles to differentiate them, as
> >suggested by Eric
> >  - add a-b & r-b tags
> >
> 
> This version should be queue-able, hopefully. Samuel, I am not sure if
> you are willing to do it.
> 
> If no objections, I can also send a pullreq to Peter?

I don't see a use for going through my tree :)

Samuel

[Qemu-devel] [PATCH v4 3/8] slirp: clarify license of slirp files using SPDX: explicit BSD

[Marc-André Lureau]

Add SPDX license identifier to clarify the license of files with
explicit 3-clause BSD license header.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Thomas Huth 
---
 slirp/src/dhcpv6.h | 1 +
 slirp/src/ip.h | 1 +
 slirp/src/ip_icmp.h| 1 +
 slirp/src/mbuf.h   | 1 +
 slirp/src/ncsi-pkt.h   | 1 +
 slirp/src/qtailq.h | 1 +
 slirp/src/tcp.h| 1 +
 slirp/src/tcp_timer.h  | 1 +
 slirp/src/tcp_var.h| 1 +
 slirp/src/tcpip.h  | 1 +
 slirp/src/udp.h| 1 +
 slirp/src/vmstate.h| 1 +
 slirp/src/cksum.c  | 1 +
 slirp/src/dhcpv6.c | 1 +
 slirp/src/ip_icmp.c| 1 +
 slirp/src/ip_input.c   | 1 +
 slirp/src/ip_output.c  | 1 +
 slirp/src/ncsi.c   | 1 +
 slirp/src/tcp_input.c  | 1 +
 slirp/src/tcp_output.c | 1 +
 slirp/src/tcp_subr.c   | 1 +
 slirp/src/tcp_timer.c  | 1 +
 slirp/src/udp.c| 1 +
 slirp/src/vmstate.c| 1 +
 24 files changed, 24 insertions(+)

diff --git a/slirp/src/dhcpv6.h b/slirp/src/dhcpv6.h
index af0e193b06..dc26a93cff 100644
--- a/slirp/src/dhcpv6.h
+++ b/slirp/src/dhcpv6.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Definitions and prototypes for SLIRP stateless DHCPv6
  *
diff --git a/slirp/src/ip.h b/slirp/src/ip.h
index 73a4d2a3d2..1484de1176 100644
--- a/slirp/src/ip.h
+++ b/slirp/src/ip.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/ip_icmp.h b/slirp/src/ip_icmp.h
index a4e5b8b265..05d85c59dd 100644
--- a/slirp/src/ip_icmp.h
+++ b/slirp/src/ip_icmp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/mbuf.h b/slirp/src/mbuf.h
index e2d443418a..732c85c63c 100644
--- a/slirp/src/mbuf.h
+++ b/slirp/src/mbuf.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1988, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/ncsi-pkt.h b/slirp/src/ncsi-pkt.h
index 3867feb1d3..4c0be39f6e 100644
--- a/slirp/src/ncsi-pkt.h
+++ b/slirp/src/ncsi-pkt.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright Gavin Shan, IBM Corporation 2016.
  *
diff --git a/slirp/src/qtailq.h b/slirp/src/qtailq.h
index a89b0c439a..d8aa0e19a4 100644
--- a/slirp/src/qtailq.h
+++ b/slirp/src/qtailq.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*  $NetBSD: queue.h,v 1.52 2009/04/20 09:56:08 mschuett Exp $ */
 
 /*
diff --git a/slirp/src/tcp.h b/slirp/src/tcp.h
index 47aaea6c5b..79d3251bb5 100644
--- a/slirp/src/tcp.h
+++ b/slirp/src/tcp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/tcp_timer.h b/slirp/src/tcp_timer.h
index b25b3911d7..709f63987a 100644
--- a/slirp/src/tcp_timer.h
+++ b/slirp/src/tcp_timer.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/tcp_var.h b/slirp/src/tcp_var.h
index 27ef1a51cb..162be6e95e 100644
--- a/slirp/src/tcp_var.h
+++ b/slirp/src/tcp_var.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993, 1994
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/tcpip.h b/slirp/src/tcpip.h
index 07dbf2c432..560a86417c 100644
--- a/slirp/src/tcpip.h
+++ b/slirp/src/tcpip.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/udp.h b/slirp/src/udp.h
index 3d29504caa..29c0297179 100644
--- a/slirp/src/udp.h
+++ b/slirp/src/udp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1982, 1986, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/vmstate.h b/slirp/src/vmstate.h
index 21157b5ec2..44efea7b50 100644
--- a/slirp/src/vmstate.h
+++ b/slirp/src/vmstate.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * QEMU migration/snapshot declarations
  *
diff --git a/slirp/src/cksum.c b/slirp/src/cksum.c
index 25bfa67348..9599f6a280 100644
--- a/slirp/src/cksum.c
+++ b/slirp/src/cksum.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1988, 1992, 1993
  * The Regents of the University of California.  All rights reserved.
diff --git a/slirp/src/dhcpv6.c b/slirp/src/dhcpv6.c
index df350e9f26..3c8f420912 100644
--- a/slirp/src/dhcpv6.c
+++ b/slirp/src/dhcpv6.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * SLIR

[Qemu-devel] [PATCH v4 1/8] slirp: update COPYRIGHT to use full 3-Clause BSD License

[Marc-André Lureau]

According to commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove
the advertising clause from the slirp license"), Danny Gasparovski
gave permission to license slirp code under 3-clause BSD license:

Subject: RE: Slirp license
Date: Thu, 8 Jan 2009 10:51:00 +1100
From: "Gasparovski, Daniel" 
To: "Richard Fontana" 

I have no objection to having Slirp code in QEMU be licensed under
the 3-clause BSD license.

slirp/COPYRIGHT's initial version in 2004 (commit 5fafdf24) listed
only 3 clauses BUT used the poisonous advertising clause for clause 3
which is the controversial clause of non-free 4-clause (that is, it
appears that the BSD-4 license was copied, and then the WRONG clause
was deleted, when creating COPYRIGHT.  Perhaps explained as an easy
mistake to make since 3-clause was created by removing clause 3 of the
4-clause, where you sometimes see the three-clause version with
clauses 1, 2, 4; but more commonly see a renumbered version with
clauses 1, 2, 3 to close the gap. If you pay attention only to clause
numbers instead of content, it can be easy to confuse which clause to
delete to go from 4-clause to 3-clause).

Commit 2f5f89963 removed the poisonous wrong clause on
the grounds of moving from 4-clause to 3-clause; but did not add the
missing clause, which makes it LOOK like the 2-clause version.  But I
think we have a decent enough trail showing the intent for 3-clause.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Reviewed-by: Thomas Huth 
---
 slirp/COPYRIGHT | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/slirp/COPYRIGHT b/slirp/COPYRIGHT
index 1bc83d497e..9863ea31cb 100644
--- a/slirp/COPYRIGHT
+++ b/slirp/COPYRIGHT
@@ -25,6 +25,9 @@ The copyright terms and conditions:
  2. Redistributions in binary form must reproduce the above copyright
 notice, this list of conditions and the following disclaimer in the
 documentation and/or other materials provided with the distribution.
+ 3. Neither the name of the copyright holder nor the names of its
+contributors may be used to endorse or promote products derived
+from this software without specific prior written permission.
 
  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
  INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
-- 
2.21.0.4.g36eb1cb9cf

[Qemu-devel] [PATCH v4 6/8] slirp: clarify license of slirp files using SPDX: implicit via unstated

[Marc-André Lureau]

Add SPDX license identifier to clarify the license of files without
explicit license header.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Thomas Huth 
---
 slirp/src/bootp.h  | 1 +
 slirp/src/ip6.h| 1 +
 slirp/src/ip6_icmp.h   | 1 +
 slirp/src/libslirp.h   | 1 +
 slirp/src/slirp.h  | 1 +
 slirp/src/stream.h | 1 +
 slirp/src/tftp.h   | 1 +
 slirp/src/ip6_icmp.c   | 1 +
 slirp/src/ip6_input.c  | 1 +
 slirp/src/ip6_output.c | 1 +
 slirp/src/ndp_table.c  | 1 +
 slirp/src/udp6.c   | 1 +
 12 files changed, 12 insertions(+)

diff --git a/slirp/src/bootp.h b/slirp/src/bootp.h
index 4043489835..d881ad620a 100644
--- a/slirp/src/bootp.h
+++ b/slirp/src/bootp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /* bootp/dhcp defines */
 
 #ifndef SLIRP_BOOTP_H
diff --git a/slirp/src/ip6.h b/slirp/src/ip6.h
index 1b3364f960..33683c8e20 100644
--- a/slirp/src/ip6.h
+++ b/slirp/src/ip6.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ip6_icmp.h b/slirp/src/ip6_icmp.h
index e8ed753db5..d8d13e30fc 100644
--- a/slirp/src/ip6_icmp.h
+++ b/slirp/src/ip6_icmp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/libslirp.h b/slirp/src/libslirp.h
index 2d13950065..3b28764bec 100644
--- a/slirp/src/libslirp.h
+++ b/slirp/src/libslirp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 #ifndef LIBSLIRP_H
 #define LIBSLIRP_H
 
diff --git a/slirp/src/slirp.h b/slirp/src/slirp.h
index 8068ba1d1e..39580934f3 100644
--- a/slirp/src/slirp.h
+++ b/slirp/src/slirp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 #ifndef SLIRP_H
 #define SLIRP_H
 
diff --git a/slirp/src/stream.h b/slirp/src/stream.h
index 985334c043..08bb5b6610 100644
--- a/slirp/src/stream.h
+++ b/slirp/src/stream.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 #ifndef STREAM_H_
 #define STREAM_H_
 
diff --git a/slirp/src/tftp.h b/slirp/src/tftp.h
index a4c4a64e64..3fe3b70205 100644
--- a/slirp/src/tftp.h
+++ b/slirp/src/tftp.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /* tftp defines */
 
 #ifndef SLIRP_TFTP_H
diff --git a/slirp/src/ip6_icmp.c b/slirp/src/ip6_icmp.c
index c1e3d30470..5642457fdd 100644
--- a/slirp/src/ip6_icmp.c
+++ b/slirp/src/ip6_icmp.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ip6_input.c b/slirp/src/ip6_input.c
index 1b8c003c66..d9d2b7e9cd 100644
--- a/slirp/src/ip6_input.c
+++ b/slirp/src/ip6_input.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ip6_output.c b/slirp/src/ip6_output.c
index 19d1ae7748..b86110662c 100644
--- a/slirp/src/ip6_output.c
+++ b/slirp/src/ip6_output.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/ndp_table.c b/slirp/src/ndp_table.c
index 34ea4fdf1f..78324877e2 100644
--- a/slirp/src/ndp_table.c
+++ b/slirp/src/ndp_table.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne.
diff --git a/slirp/src/udp6.c b/slirp/src/udp6.c
index be5cba1f54..bfcc7ec6fa 100644
--- a/slirp/src/udp6.c
+++ b/slirp/src/udp6.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 2013
  * Guillaume Subiron
-- 
2.21.0.4.g36eb1cb9cf

Re: [Qemu-devel] [PATCH 0/3] target/arm: pmu fixes

[Andrew Jones]

On Fri, Mar 22, 2019 at 05:23:30PM +0100, Andrew Jones wrote:
> The first two patches fix a regression I found when running
> the arm (as opposed to arm64) pmu kvm-unit-tests test on tcg,
> 
>   $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
>   $ cd kvm-unit-tests
>   $ ./configure --arch=arm --cross-prefix=arm-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-arm or qemu-system-aarch64
>   $ arm/run arm/pmu.flat
> 
> After checking the QEMU code I found it's also reproducible with
> the arm64 test if the PMU is removed,
> 
>   $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-aarch64
>   $ arm/run arm/pmu.flat -cpu cortex-a57,pmu=off

Small correction here: I had to also hack the unit test to not
bail out when it didn't see a PMU advertised in the ID register.

diff --git a/arm/pmu.c b/arm/pmu.c
index 1de7d77d184f..185ff77244ed 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -291,8 +291,9 @@ int main(int argc, char *argv[])
cpi = atol(argv[1]);
 
if (!pmu_probe()) {
-  printf("No PMU found, test skipped...\n");
-  return report_summary();
+//  printf("No PMU found, test skipped...\n");
+  printf("No PMU found, let's crash!\n");
+//  return report_summary();
}
 
report_prefix_push("pmu");

> 
> Those tests used to pass, but now QEMU was crashing. I've broken
> the fix into two patches because the second patch is a bit of
> an RFC since I don't know if it's safe to enable all
> ARM_FEATURE_PMU code for PMUv2. Maybe that feature is only for
> PMUv3? This patch also enables the pmu cpu property to work with
> those cpu types, i.e. we can now do '-cpu cortex-a15,pmu=off' to
> remove the pmu. Although it wasn't clear to me if the PMU is
> optional (permitted to be removed) on those cpu types from the
> manuals.
> 
> The last patch is just a trivial cleanup.
> 
> Andrew Jones (3):
>   target/arm: fix crash on pmu register access
>   target/arm: cortex-a7 and cortex-a15 have pmus
>   target/arm: make pmccntr_op_start/finish static
> 
>  target/arm/cpu.c|  3 +++
>  target/arm/cpu.h| 11 ---
>  target/arm/helper.c |  8 ++--
>  3 files changed, 9 insertions(+), 13 deletions(-)
> 
> -- 
> 2.17.2
> 

[Qemu-devel] [PATCH v4 5/8] slirp: clarify license of slirp files using SPDX: implicit via COPYRIGHT

[Marc-André Lureau]

Add SPDX license identifier to clarify the license of files with
reference to BSD license from slirp COPYRIGHT file.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Thomas Huth 
---
 slirp/src/debug.h  | 1 +
 slirp/src/if.h | 1 +
 slirp/src/main.h   | 1 +
 slirp/src/misc.h   | 1 +
 slirp/src/sbuf.h   | 1 +
 slirp/src/socket.h | 1 +
 slirp/src/if.c | 1 +
 slirp/src/mbuf.c   | 1 +
 slirp/src/misc.c   | 1 +
 slirp/src/sbuf.c   | 1 +
 slirp/src/socket.c | 1 +
 11 files changed, 11 insertions(+)

diff --git a/slirp/src/debug.h b/slirp/src/debug.h
index 44d922df37..2e503ad7fa 100644
--- a/slirp/src/debug.h
+++ b/slirp/src/debug.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/if.h b/slirp/src/if.h
index 69569c10df..8a60c4e052 100644
--- a/slirp/src/if.h
+++ b/slirp/src/if.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/main.h b/slirp/src/main.h
index f11d4572b7..a88774215f 100644
--- a/slirp/src/main.h
+++ b/slirp/src/main.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/misc.h b/slirp/src/misc.h
index c2ceadb591..4eaa2466d7 100644
--- a/slirp/src/misc.h
+++ b/slirp/src/misc.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/sbuf.h b/slirp/src/sbuf.h
index 1cb9a42834..ece616e317 100644
--- a/slirp/src/sbuf.h
+++ b/slirp/src/sbuf.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/socket.h b/slirp/src/socket.h
index e4d12cd591..10a0c78a26 100644
--- a/slirp/src/socket.h
+++ b/slirp/src/socket.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/if.c b/slirp/src/if.c
index 1830cc396c..b8cddebf66 100644
--- a/slirp/src/if.c
+++ b/slirp/src/if.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/mbuf.c b/slirp/src/mbuf.c
index 521c02c967..f079a86d78 100644
--- a/slirp/src/mbuf.c
+++ b/slirp/src/mbuf.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski
  *
diff --git a/slirp/src/misc.c b/slirp/src/misc.c
index 937a418d4e..da41f3bb5f 100644
--- a/slirp/src/misc.c
+++ b/slirp/src/misc.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/sbuf.c b/slirp/src/sbuf.c
index 51a9f0cc7d..815823ffbe 100644
--- a/slirp/src/sbuf.c
+++ b/slirp/src/sbuf.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
diff --git a/slirp/src/socket.c b/slirp/src/socket.c
index f2428a3ae8..ad58262a06 100644
--- a/slirp/src/socket.c
+++ b/slirp/src/socket.c
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1995 Danny Gasparovski.
  *
-- 
2.21.0.4.g36eb1cb9cf

Re: [Qemu-devel] [PATCH v4 0/8] slirp: clarify license of slirp as BSD-3

[Marc-André Lureau]

Hi
On Fri, Mar 22, 2019 at 5:43 PM Marc-André Lureau
 wrote:
>
> Hi,
>
> In order to make slirp a standalone project, the project must have a
> clear license, and be compatible with the GPL or LGPL.
>
> Since commit 2f5f89963186d42a7ded253bc6cf5b32abb45cec ("Remove the
> advertising clause from the slirp license"), slirp is BSD-3. But new
> files have been added under slirp/ with QEMU GPL license since then.
>
> v4:
>  - update SPDX-patches commit titles to differentiate them, as
>suggested by Eric
>  - add a-b & r-b tags
>

This version should be queue-able, hopefully. Samuel, I am not sure if
you are willing to do it.

If no objections, I can also send a pullreq to Peter?

cheers

Re: [Qemu-devel] target/arm: kvm-unit-tests gicv2 test failures on tcg

[Peter Maydell]

On Fri, 22 Mar 2019 at 16:40, Andrew Jones  wrote:
> Hi TCG GIC developers,
>
> There are a few gicv2 test failures when running over TCG that we don't
> see when running over KVM. I don't believe these are regressions - I'm
> pretty sure they've been failing since Andre first introduced the tests.
> I'm just pointing them out now in case anybody would like to look into
> them:
>
>   $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
>   $ cd kvm-unit-tests
>   $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
>   $ make -j
>   $ export QEMU=/path/to/qemu-system-aarch64
>   $ ./run_tests.sh -g gic
>   $ grep FAIL logs/gic*
>   logs/gicv2-mmio-3p.log:FAIL: gicv2: mmio: ITARGETSR: bits for 5 
> non-existent CPUs masked
>   logs/gicv2-mmio-3p.log:FAIL: gicv2: mmio: ITARGETSR: register content 
> preserved (01030207 => 0103020f)
>   logs/gicv2-mmio-3p.log:FAIL: gicv2: mmio: ITARGETSR: byte writes successful 
> (0x1f => 0x011f020f)
>   logs/gicv2-mmio.log:FAIL: gicv2: mmio: ITARGETSR: bits for 4 non-existent 
> CPUs masked
>   logs/gicv2-mmio.log:FAIL: gicv2: mmio: ITARGETSR: byte writes successful 
> (0x1f => 0x011f020f)
>   logs/gicv2-mmio-up.log:FAIL: gicv2: mmio: ITARGETSR: register content 
> preserved (01010001 => )
>   logs/gicv2-mmio-up.log:FAIL: gicv2: mmio: ITARGETSR: byte writes successful 
> (0x1f => 0x)

Haven't looked at the tests or the GIC code, but I'm guessing from
the logs that these are the usual tendency of and our device models
to implement "bits that should be reserved" in registers as
reads-as-written, relying on the guest code not to set them.
This is strictly incorrect (the GICv2 spec says implementations
must make RAZ/WI bits really read-as-zero) but it seems unlikely
to trip up real world code.

I'm happy to review a patch if somebody wants to write one.

thanks
-- PMM

[Qemu-devel] [PATCH] target/arm: default config

[Andrew Jones]

In the kconfig shuffle arm lost pci-testdev which is used by
kvm-unit-tests. Let's add it back.

Signed-off-by: Andrew Jones 
---
 default-configs/arm-softmmu.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 2a7efc11674a..613d19a06db2 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -2,6 +2,7 @@
 
 CONFIG_PCI=y
 CONFIG_PCI_DEVICES=y
+CONFIG_PCI_TESTDEV=y
 CONFIG_VGA=y
 CONFIG_NAND=y
 CONFIG_ECC=y
-- 
2.17.2

[Qemu-devel] target/arm: kvm-unit-tests gicv2 test failures on tcg

[Andrew Jones]

Hi TCG GIC developers,

There are a few gicv2 test failures when running over TCG that we don't
see when running over KVM. I don't believe these are regressions - I'm
pretty sure they've been failing since Andre first introduced the tests.
I'm just pointing them out now in case anybody would like to look into
them:

  $ git clone git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
  $ cd kvm-unit-tests
  $ ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
  $ make -j
  $ export QEMU=/path/to/qemu-system-aarch64
  $ ./run_tests.sh -g gic
  $ grep FAIL logs/gic*
  logs/gicv2-mmio-3p.log:FAIL: gicv2: mmio: ITARGETSR: bits for 5 non-existent 
CPUs masked
  logs/gicv2-mmio-3p.log:FAIL: gicv2: mmio: ITARGETSR: register content 
preserved (01030207 => 0103020f)
  logs/gicv2-mmio-3p.log:FAIL: gicv2: mmio: ITARGETSR: byte writes successful 
(0x1f => 0x011f020f)
  logs/gicv2-mmio.log:FAIL: gicv2: mmio: ITARGETSR: bits for 4 non-existent 
CPUs masked
  logs/gicv2-mmio.log:FAIL: gicv2: mmio: ITARGETSR: byte writes successful 
(0x1f => 0x011f020f)
  logs/gicv2-mmio-up.log:FAIL: gicv2: mmio: ITARGETSR: register content 
preserved (01010001 => )
  logs/gicv2-mmio-up.log:FAIL: gicv2: mmio: ITARGETSR: byte writes successful 
(0x1f => 0x)

Thanks,
drew

[Qemu-devel] [PATCH 3/3] target/arm: make pmccntr_op_start/finish static

[Andrew Jones]

These functions are not used outside helper.c

Signed-off-by: Andrew Jones 
---
 target/arm/cpu.h| 11 ---
 target/arm/helper.c |  4 ++--
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5f23c621325c..d4d2836923df 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -992,17 +992,6 @@ static inline bool is_a64(CPUARMState *env)
 int cpu_arm_signal_handler(int host_signum, void *pinfo,
void *puc);
 
-/**
- * pmccntr_op_start/finish
- * @env: CPUARMState
- *
- * Convert the counter in the PMCCNTR between its delta form (the typical mode
- * when it's enabled) and the guest-visible value. These two calls must always
- * surround any action which might affect the counter.
- */
-void pmccntr_op_start(CPUARMState *env);
-void pmccntr_op_finish(CPUARMState *env);
-
 /**
  * pmu_op_start/finish
  * @env: CPUARMState
diff --git a/target/arm/helper.c b/target/arm/helper.c
index fc73488f6cc0..a36f4b3d6997 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1337,7 +1337,7 @@ static void pmu_update_irq(CPUARMState *env)
  * etc. can be done logically. This is essentially a no-op if the counter is
  * not enabled at the time of the call.
  */
-void pmccntr_op_start(CPUARMState *env)
+static void pmccntr_op_start(CPUARMState *env)
 {
 uint64_t cycles = cycles_get_count(env);
 
@@ -1367,7 +1367,7 @@ void pmccntr_op_start(CPUARMState *env)
  * guest-visible count. A call to pmccntr_op_finish should follow every call to
  * pmccntr_op_start.
  */
-void pmccntr_op_finish(CPUARMState *env)
+static void pmccntr_op_finish(CPUARMState *env)
 {
 if (pmu_counter_enabled(env, 31)) {
 #ifndef CONFIG_USER_ONLY
-- 
2.17.2

Re: [Qemu-devel] [PATCH 4/4] target/mips: Fix insert. for MIPS big endian host

[Aleksandar Markovic]

> From: Mateja Marjanovic 
> Subject: [PATCH 4/4] target/mips: Fix insert. for MIPS big endian host
> 
> From: Mateja Marjanovic 
> 
> Inserting from GPR to an element in a MSA register when
> executed on a MIPS big endian CPU, didn't pick the
> right element, and was behaving like on little endian.
> 
> Signed-off-by: Mateja Marjanovic 
> ---

...

> @@ -1511,9 +1518,11 @@ void helper_msa_insert_df(CPUMIPSState *env, uint32_t 
> df, uint32_t wd,
>  case DF_WORD:
>  pwd->w[n] = (int32_t)rs;
>  break;
> +#ifdef TARGET_MIPS64
>  case DF_DOUBLE:
>  pwd->d[n] = (int64_t)rs;
>  break;
> +#endif
>  default:
>  assert(0);
>  }

You are right that this case should be under ifdef the way you did.

In fact, this code should be impossible to reach, since there is a check
for MIPS32/64 in translate.c before invoking this helper, so technically
there is no bug. However, it is a latent bag, and also an instance of
"dead code" (for MIPS32). So, you are rightfully removing this case
for MIPS32.

May I just ask you to put this in a separate patch, since this has nothing
to do with endianess etc. (with the title, let's say:

"target/mips: Remove handling of nonexistent flavor of INSERT for MIPS32",

and the commit message

"INSERT.D is present in MIPS64 MSA only. [1] page 

[1] "

)?

Thanks,
Aleksandar