Re: [PATCH] linux-user/syscall.c: add target-to-host mapping for epoll_create1()

[Laurent Vivier]

Le 16/04/2020 à 00:05, Sergei Trofimovich a écrit :
> Noticed by Barnabás Virágh as a python-3.7 failue on qemu-alpha.
> 
> The bug shows up on alpha as it's one of the targets where
> EPOLL_CLOEXEC differs from other targets:
> sysdeps/unix/sysv/linux/alpha/bits/epoll.h: EPOLL_CLOEXEC  = 0100
> sysdeps/unix/sysv/linux/bits/epoll.h:EPOLL_CLOEXEC = 0200
> 
> Bug: https://bugs.gentoo.org/717548
> Reported-by: Barnabás Virágh
> Signed-off-by: Sergei Trofimovich 
> CC: Riku Voipio 
> CC: Laurent Vivier 
> ---
>  linux-user/syscall.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 674f70e70a..05f03919ff 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -12012,7 +12012,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>  #endif
>  #if defined(TARGET_NR_epoll_create1) && defined(CONFIG_EPOLL_CREATE1)
>  case TARGET_NR_epoll_create1:
> -return get_errno(epoll_create1(arg1));
> +return get_errno(epoll_create1(target_to_host_bitmask(arg1, 
> fcntl_flags_tbl)));
>  #endif
>  #if defined(TARGET_NR_epoll_ctl)
>  case TARGET_NR_epoll_ctl:
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH] linux-user/syscall.c: add target-to-host mapping for epoll_create1()

[Laurent Vivier]

Le 16/04/2020 à 00:05, Sergei Trofimovich a écrit :
> Noticed by Barnabás Virágh as a python-3.7 failue on qemu-alpha.
> 
> The bug shows up on alpha as it's one of the targets where
> EPOLL_CLOEXEC differs from other targets:
> sysdeps/unix/sysv/linux/alpha/bits/epoll.h: EPOLL_CLOEXEC  = 0100
> sysdeps/unix/sysv/linux/bits/epoll.h:EPOLL_CLOEXEC = 0200
> 
> Bug: https://bugs.gentoo.org/717548
> Reported-by: Barnabás Virágh
> Signed-off-by: Sergei Trofimovich 
> CC: Riku Voipio 
> CC: Laurent Vivier 
> ---
>  linux-user/syscall.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 674f70e70a..05f03919ff 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -12012,7 +12012,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>  #endif
>  #if defined(TARGET_NR_epoll_create1) && defined(CONFIG_EPOLL_CREATE1)
>  case TARGET_NR_epoll_create1:
> -return get_errno(epoll_create1(arg1));
> +return get_errno(epoll_create1(target_to_host_bitmask(arg1, 
> fcntl_flags_tbl)));
>  #endif
>  #if defined(TARGET_NR_epoll_ctl)
>  case TARGET_NR_epoll_ctl:
> 

Applied to my linux-user-for-5.0 branch.

Thanks,
Laurent

Re: Integration of qemu-img

[Markus Armbruster]

Cc: qemu-block

 writes:

> Dear Sir or Madam,
>
>  
>
> I am a PhD student at the Friedrich-Alexander-University Erlangen-Nürnberg
> in Bavaria Germany and I am currently working on an open-source forensic
> analysis tool. I would like to use qemu-img for converting virtual discs to
> raw files and to get virtual disc information. By now I tried to create a
> qemu-img DLL with the qemu source code you provide on your website, but I am
> unable to compile it properly. Therefore, I would like to ask you if there
> is a simple solution to integrate qemu-img to other C++ projects? Or is
> there a precompiled qemu-img DLL which I could use? Thank you very much for
> your support.
>
>  
>
> Best,
>
> Janine Schneider
>
>  

Re: [EXTERNAL] [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of April 15, 2020 4:49 pm:
> On 4/14/20 1:11 PM, Nicholas Piggin wrote:
>> 
>> The confusion arises from L=0 being "context synchronizing" whereas L=1
>> is "execution synchronizing", which is a weaker semantic. However this
>> is not a relaxation of the requirement that these exceptions cause
>> interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as
>> TCG is doing here), rather it specifies how a pipelined processor can
>> have multiple instructions in flight where one may influence how another
>> behaves.
> 
> I was expecting more changes but this looks fine. 

It _seems_ to just be these, from what I could see, but could quite 
easily be other issues I missed.

There is at least one other "funny" thing with this synchronization, 
which is the TLB flushing. I don't think it has a bug, but comments
are a bit suspect. tlbie/l doesn't have anything to do with context
/ execution synchronization, so it's a bit interesting to check them
in isync and rfi etc.

ptesync is required because the page table walkers are not necessarily 
coherent with the main CPU's memory pipeline, so you store a new value 
to a PTE then do a tlbiel, you can't have the MMU reload the TLB with 
the old PTE because the store was sitting in the store queue that 
doesn't forward to the table walker. This condition can persist after
the store instruction itself has completed so no amount of this
context synchronization would help.

It does kind of make sense to check the tlb flush in rfi, so you catch 
stray ones that didn't have the right ptesync/tlbsync, but it would 
almost be a condition you could catch and add a warn for. isync doesn't
make a lot of sense though, as far as I can see.

Thanks,
Nick

> Reviewed-by: Cédric Le Goater 

Sorry I always get your email wrong, phantom address book entry.

qemu_coroutine_yield switches thread?

[Stefan Reiter]

Hi list,

quick question: Can a resume from a qemu_coroutine_yield happen in a 
different thread?


Well, it can, since I'm seeing it happen, but is that okay or a bug?

I.e. in a backup-job the following can sporadically trip:

  unsigned long tid = pthread_self();
  qemu_get_current_aio_context(); // returns main context
  qemu_coroutine_yield();
  qemu_get_current_aio_context(); // still returns main context, but:
  assert(tid == pthread_self()); // this fails

It seems to be called from a vCPU thread when it happens. VM uses no 
iothreads.


~

Re: [PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature

[David Hildenbrand]

>>
>> Postcopy is a very good point, bought!
>>
>> But (what you wrote above) it sounds like that this is really what we *have 
>> to* do, not an optimization. I‘ll double check the spec tomorrow (hopefully 
>> it was documented). We should rephrase the comment then.
> 
> Do you have a link to the spec that I could look at? I am not hopeful
> that this will be listed there since the poison side of QEMU was never
> implemented. The flag is only here because it was copied over in the
> kernel header.

Introducing a feature without

a) specification what it does
b) implementation (of both sides) showing what has to be done
c) any kind of documentation of what needs to be done

just horrible.

The latest-greatest spec lives in

https://github.com/oasis-tcs/virtio-spec.git

I can't spot any signs of free page hinting/page poisioning. :(

We should document our result of page poisoning, free page hinting, and
free page reporting there as well. I hope you'll have time for the latter.

-
Semantics of VIRTIO_BALLOON_F_PAGE_POISON
-

"The VIRTIO_BALLOON_F_PAGE_POISON feature bit is used to indicate if the
guest is using page poisoning. Guest writes to the poison_val config
field to tell host about the page poisoning value that is in use."
-> Very little information, no signs about what has to be done.

"Let the hypervisor know that we are expecting a specific value to be
written back in balloon pages."
-> Okay, that talks about "balloon pages", which would include right now
-- pages "inflated" and then "deflated" using free page hinting
-- pages "inflated" and then "deflated" using oridnary inflate/deflate
   queue
-- pages "inflated" using free page reporting and automatically
   "deflated" on access

So VIRTIO_BALLOON_F_PAGE_POISON really means "whenever the guest
deflates a page (either explicitly, or implicitly with free page
reporting), it is filled with "poison_val".

And I would add

"However, if the inflated page was not filled with "poison_val" when
inflating, it's not predictable if the original page or a page filled
with "poison_val" is returned."

Which would cover the "we did not discard the page in the hypervisor, so
the original page is still there".


We should also document what is expected to happen if "poison_val" is
suddenly changed by the guest at one point in time again. (e.g., not
supported, unexpected things can happen, etc.) Also, we might have to
document that a device reset resets the poison_val to 0. (not sure if
that's really necessary)

-
What we have to do in the guest/Linux:
-

A guest which relies on this (esp., free page reporting in Linux only,
right?), has to not use/advertise VIRTIO_BALLOON_F_REPORTING *in case
VIRTIO_BALLOON_F_PAGE_POISON is not provided* by the host. AFAIKS,
ordinary inflation/deflation and free page hinting does currently not
care, as we go via free_page(), so that is currently fine AFAIKs.

-
What we have to do in the hypervisor/QEMU:
-

With VIRTIO_BALLOON_F_PAGE_POISON, we could provide free page reporting
easily IFF "page_val"==0. However, as you said, it will currently be
expensive in case of postcopy, as the guest still zeroes out all pages.
Document that properly.

With VIRTIO_BALLOON_F_PAGE_POISON, don't inflate any pages right now
(not discarding anything), OR fill the pages with poison_val when
deflating. I guess the latter would be better - even if current Linux
does not need it, it's what we are expected to do AFAIKS.

With VIRTIO_BALLOON_F_PAGE_POISON and page_val != 0, discard all free
page reporting attempts. (== what your patch #3 does). Document that
properly.


Makes sense? See below for guest migration thingies.

> 
>> I could imagine that we also have to care about ordinary page 
>> inflation/deflation when handling page poisoning. (Iow, don‘t 
>> inflate/deflate if set for now).
> 
> The problem is this was broken from the start for that. The issue is
> that the poison feature was wrapped up within the page hinting
> feature. So as a result enabling it for a VM that doesn't recognize
> the feature independently would likely leave the poison value
> uninitialized and flagging as though a value of 0 is used. In addition
> the original poison checking wasn't making sure that the page wasn't
> init_on_free which has the same effect as page poisoning but isn't
> page poisoning.

If the guest agrees to have VIRTIO_BALLOON_F_PAGE_POISON but does not
initialize poison_val, then it's a guest bug, I wouldn't worry about
that for now.

> 
>>>
>>> The worst case scenario would be one in which the VM was suspended for
>>> migrat

Re: qemu_coroutine_yield switches thread?

[Kevin Wolf]

Am 16.04.2020 um 10:06 hat Stefan Reiter geschrieben:
> Hi list,
> 
> quick question: Can a resume from a qemu_coroutine_yield happen in a
> different thread?
> 
> Well, it can, since I'm seeing it happen, but is that okay or a bug?

Yes, it can happen. At least for devices like IDE where a request is
started during a vmexit (MMIO or I/O port write), the coroutine will
usually begin its life in the vcpu thread and then move to the main loop
thread.

This is not a problem because the vcpu thread holds the BQL while
running the request coroutine.

> I.e. in a backup-job the following can sporadically trip:
> 
>   unsigned long tid = pthread_self();
>   qemu_get_current_aio_context(); // returns main context
>   qemu_coroutine_yield();
>   qemu_get_current_aio_context(); // still returns main context, but:
>   assert(tid == pthread_self()); // this fails
> 
> It seems to be called from a vCPU thread when it happens. VM uses no
> iothreads.

This must be a guest request that was intercepted by the backup job. I
think it wouldn't happen for the background copy, these requests already
start in the main loop.

Kevin

Re: [PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature

[David Hildenbrand]

On 16.04.20 10:18, David Hildenbrand wrote:
>>>
>>> Postcopy is a very good point, bought!
>>>
>>> But (what you wrote above) it sounds like that this is really what we *have 
>>> to* do, not an optimization. I‘ll double check the spec tomorrow (hopefully 
>>> it was documented). We should rephrase the comment then.
>>
>> Do you have a link to the spec that I could look at? I am not hopeful
>> that this will be listed there since the poison side of QEMU was never
>> implemented. The flag is only here because it was copied over in the
>> kernel header.
> 
> Introducing a feature without
> 
> a) specification what it does
> b) implementation (of both sides) showing what has to be done
> c) any kind of documentation of what needs to be done
> 
> just horrible.
> 
> The latest-greatest spec lives in
> 
> https://github.com/oasis-tcs/virtio-spec.git
> 
> I can't spot any signs of free page hinting/page poisioning. :(
> 
> We should document our result of page poisoning, free page hinting, and
> free page reporting there as well. I hope you'll have time for the latter.
> 
> -
> Semantics of VIRTIO_BALLOON_F_PAGE_POISON
> -
> 
> "The VIRTIO_BALLOON_F_PAGE_POISON feature bit is used to indicate if the
> guest is using page poisoning. Guest writes to the poison_val config
> field to tell host about the page poisoning value that is in use."
> -> Very little information, no signs about what has to be done.
> 
> "Let the hypervisor know that we are expecting a specific value to be
> written back in balloon pages."
> -> Okay, that talks about "balloon pages", which would include right now
> -- pages "inflated" and then "deflated" using free page hinting
> -- pages "inflated" and then "deflated" using oridnary inflate/deflate
>queue
> -- pages "inflated" using free page reporting and automatically
>"deflated" on access
> 
> So VIRTIO_BALLOON_F_PAGE_POISON really means "whenever the guest
> deflates a page (either explicitly, or implicitly with free page
> reporting), it is filled with "poison_val".
> 
> And I would add
> 
> "However, if the inflated page was not filled with "poison_val" when
> inflating, it's not predictable if the original page or a page filled
> with "poison_val" is returned."
> 
> Which would cover the "we did not discard the page in the hypervisor, so
> the original page is still there".
> 
> 
> We should also document what is expected to happen if "poison_val" is
> suddenly changed by the guest at one point in time again. (e.g., not
> supported, unexpected things can happen, etc.) Also, we might have to
> document that a device reset resets the poison_val to 0. (not sure if
> that's really necessary)

Looking at the spec, we will only have to care about
"VIRTIO_BALLOON_F_MUST_TELL_HOST". Especially:

"If the VIRTIO_BALLOON_F_MUST_TELL_HOST feature is negotiated, the
driver MUST NOT use pages from the balloon until the device has
acknowledged the deflate request. Otherwise, if the
VIRTIO_BALLOON_F_MUST_TELL_HOST feature is not negotiated, the driver
MAY begin to re-use pages previously given to the balloon before the
device has acknowledged the deflate request."

So I guess, in QEMU, if
- VIRTIO_BALLOON_F_MUST_TELL_HOST is not set (== currently always)
- VIRTIO_BALLOON_F_PAGE_POISON is set
- poison_val is != 0
then, don't discard any pages, because we cannot fill the page reliably
during a deflation request (== guest could already be reusing the page
and expecting a certain value).

In QEMU, we should always set VIRTIO_BALLOON_F_MUST_TELL_HOST when
setting VIRTIO_BALLOON_F_PAGE_POISON.

In the spec, we should document that VIRTIO_BALLOON_F_PAGE_POISON should
only be used with VIRTIO_BALLOON_F_MUST_TELL_HOST or sth like that ...

confusing stuff. Let me know what you think.


-- 
Thanks,

David / dhildenb

[PATCH] Cadence: gem: fix wraparound in 64bit descriptors

[Ramon Fried]

Wraparound of TX descriptor cyclic buffer only updated
the low 32 bits of the descriptor.
Fix that by checking if we're working with 64bit descriptors.

Signed-off-by: Ramon Fried 
---
 hw/net/cadence_gem.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 51ec5a072d..b8ae21cc0d 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1238,7 +1238,14 @@ static void gem_transmit(CadenceGEMState *s)
 /* read next descriptor */
 if (tx_desc_get_wrap(desc)) {
 tx_desc_set_last(desc);
-packet_desc_addr = s->regs[GEM_TXQBASE];
+
+if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+packet_desc_addr = s->regs[GEM_TBQPH];
+packet_desc_addr <<= 32;
+} else {
+packet_desc_addr = 0;
+}
+packet_desc_addr |= s->regs[GEM_TXQBASE];
 } else {
 packet_desc_addr += 4 * gem_get_desc_len(s, false);
 }
-- 
2.26.0

Re: [PATCH] Cadence: gem: fix wraparound in 64bit descriptors

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200416090247.353414-1-rfried@gmail.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
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 6 fdc-test /x86_64/fdc/relative_seek
PASS 7 fdc-test /x86_64/fdc/read_id
PASS 8 fdc-test /x86_64/fdc/verify
==6189==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 9 fdc-test /x86_64/fdc/media_insert
PASS 13 test-opts-visitor /visitor/opts/i64/range/max/pos/b
PASS 14 test-opts-visitor /visitor/opts/i64/range/max/neg/a
---
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" 
==6208==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
==6208==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffdf93df000; bottom 0x7faa93ac9000; size: 0x005365916000 (358186311680)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 3 test-coroutine /basic/yield
---
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
PASS 14 test-aio /aio/timer/schedule
==6223==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
==6234==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==6231==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 2 test-aio-multithread /aio/multi/schedule
==6251==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
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==6262==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/simple_rw
==6273==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
==6279==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
---
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==6296==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
PASS 11 test-throttle /throttle/config/conflicting
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==6302==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
==6300==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
---
PASS 3 test-hbitmap /hbitmap/size/unaligned
PASS 4 test-hbitmap /hbitmap/iter/empty
PASS 5 test-hbitmap /hbitmap/iter/partial
==6377==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-hbitmap /hbitmap/iter/granularity
PASS 7 test-hbitmap /hbitmap/iter/iter_and_reset
PASS 8 test-hbitmap /hbitmap/get/all
---
PASS

[PATCH 0/1] LUKS: Fix error message when underlying fs don't support large enough files

[Maxim Levitsky]

This is a repost of the same trivial patch I already, which fell through the 
cracks.
Could someone queue it up so I close the bugzilla I have for this?

Best regards,
Maxim Levitsky

Maxim Levitsky (1):
  block/crypto: better error message when creating too large files

 block/crypto.c | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

-- 
2.17.2

[PATCH 1/1] block/crypto: better error message when creating too large files

[Maxim Levitsky]

Currently if you attampt to create too large file with luks you
get the following error message:

Formatting 'test.luks', fmt=luks size=17592186044416 key-secret=sec0
qemu-img: test.luks: Could not resize file: File too large

While for raw format the error message is
qemu-img: test.img: The image size is too large for file format 'raw'


The reason for this is that qemu-img checks for errno of the failure,
and presents the later error when it is -EFBIG

However crypto generic code 'swallows' the errno and replaces it
with -EIO.

As an attempt to make it better, we can make luks driver,
detect -EFBIG and in this case present a better error message,
which is what this patch does

The new error message is:

qemu-img: error creating test.luks: The requested file size is too large

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898
Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block/crypto.c | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index d577f89659..1b604beb30 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -104,18 +104,35 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
   Error **errp)
 {
 struct BlockCryptoCreateData *data = opaque;
+Error *local_error = NULL;
+int ret;
 
 if (data->size > INT64_MAX || headerlen > INT64_MAX - data->size) {
-error_setg(errp, "The requested file size is too large");
-return -EFBIG;
+ret = -EFBIG;
+goto error;
 }
 
 /* User provided size should reflect amount of space made
  * available to the guest, so we must take account of that
  * which will be used by the crypto header
  */
-return blk_truncate(data->blk, data->size + headerlen, false,
+ret = blk_truncate(data->blk, data->size + headerlen, false,
 data->prealloc, errp);
+
+if (ret >= 0) {
+return ret;
+}
+
+error:
+if (ret == -EFBIG) {
+/* Replace the error message with a better one */
+error_free(local_error);
+error_setg(errp, "The requested file size is too large");
+} else {
+error_propagate(errp, local_error);
+}
+
+return ret;
 }
 
 
-- 
2.17.2

Re: [PATCH 0/1] LUKS: Fix error message when underlying fs don't support large enough files

[Maxim Levitsky]

On Thu, 2020-04-16 at 12:50 +0300, Maxim Levitsky wrote:
> This is a repost of the same trivial patch I already, which fell through the 
> cracks.
> Could someone queue it up so I close the bugzilla I have for this?

I a word - need more coffee :-)
I mean I already posted this patch few months ago but it was forgotten,
due to merge window/etc.

Best regards,
Maxim Levitsky

> 
> Best regards,
>   Maxim Levitsky
> 
> Maxim Levitsky (1):
>   block/crypto: better error message when creating too large files
> 
>  block/crypto.c | 23 ---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 

Re: qemu_coroutine_yield switches thread?

[Dietmar Maurer]

> > quick question: Can a resume from a qemu_coroutine_yield happen in a
> > different thread?
> > 
> > Well, it can, since I'm seeing it happen, but is that okay or a bug?
> 
> Yes, it can happen. At least for devices like IDE where a request is
> started during a vmexit (MMIO or I/O port write), the coroutine will
> usually begin its life in the vcpu thread and then move to the main loop
> thread.
> 
> This is not a problem because the vcpu thread holds the BQL while
> running the request coroutine.

Isn't that a problem when using QemuRecMutex, for example:

qemu_rec_mutex_lock(lock)
...
qemu_coroutine_yield() // wait for something
// we are now inside a different thread
qemu_rec_mutex_unlock(lock) // Crash - wrong thread!!

?

Re: qemu_coroutine_yield switches thread?

[Kevin Wolf]

Am 16.04.2020 um 12:09 hat Dietmar Maurer geschrieben:
> > > quick question: Can a resume from a qemu_coroutine_yield happen in a
> > > different thread?
> > > 
> > > Well, it can, since I'm seeing it happen, but is that okay or a bug?
> > 
> > Yes, it can happen. At least for devices like IDE where a request is
> > started during a vmexit (MMIO or I/O port write), the coroutine will
> > usually begin its life in the vcpu thread and then move to the main loop
> > thread.
> > 
> > This is not a problem because the vcpu thread holds the BQL while
> > running the request coroutine.
> 
> Isn't that a problem when using QemuRecMutex, for example:
> 
> qemu_rec_mutex_lock(lock)
> ...
> qemu_coroutine_yield() // wait for something
> // we are now inside a different thread
> qemu_rec_mutex_unlock(lock) // Crash - wrong thread!!

Acquiring a lock (other than CoMutex etc.) in a coroutine would be wrong
even if the coroutine stays in the same thread because acquiring the
lock can block, and the coroutine must yield in that case instead of
blocking. Coroutines must use the the coroutine locking primitives from
qemu/coroutine.h.

Kevin

[Bug 1871267] Re: Multiple (Repeating) Keystrokes in macOS

[Alex]

Issues with time emulation. MacOS runs on qemu with a specific cpu option: -cpu 
Penryn,vendor=GenuineIntel,+invtsc,vmware-cpuid-freq=on.
The code of cpu_x86_cpuid has no handler for 0x4010, so vmware-cpuid-freq 
is ignored.
Another solution is to modify tsc_increment_by_tick value in 
MSR_IA32_PERF_STATUS returned from helper_rdmsr. Currently it is val = 1000ULL. 
Try to set it to 2000ULL, and see what happens.

The solution for hardware emulation is to return real hardware values to
the guest. I think for tcg it can be passed from command line, so the
user can adjust it's value.

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

Title:
  Multiple (Repeating) Keystrokes in macOS

Status in QEMU:
  New

Bug description:
  Hi,

  I am finding this issue with v4.2.0, or the latest master - on a
  Windows host, with macOS guest. It happens using gtk (SPICE?) or VNC.
  When I get to a place to enter a keystroke, I quite reliably get
  multiple of the same key (i.e. press A, get ).

  Thinking there may be a basic setting to address this? I did try it in
  Linux (kvm), no issue there.

  Thanks!

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

[Bug 1829242] Re: qemu on windows host exits after savevm command

[Alex]

** Changed in: qemu
   Status: Fix Committed => Fix Released

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

Title:
  qemu on windows host exits after savevm command

Status in QEMU:
  Fix Released

Bug description:
  I'm running qemu-system-i386.exe 3.1.0 with this command line:
  "C:\Program Files\qemu\qemu-system-i386.exe"  -L C:\user\qemu\pc-bios\ -name 
win7 -m 4G -uuid 564db62e-e031-b5cf-5f34-a75f8cefa98e -rtc base=localtime 
-accel hax -hdd C:\VirtualMachines\Dev\Win10x64_VS17\swap.qcow 
"C:\VirtualMachines\qemu\qemu_win7.qcow"
  Host OS Windows 10 x64, guest OS Wondows 7 x86.

  Wait till the OS loads, go to compat_monitor0 tab and enter command:
  savevm loaded_win
  After a few seconds qemu exits, running it another time and entering command:
  info snapshots
  says "There is no snapshot available". I've tried rinning it with -accel tcg, 
with same results. I've tried less memory (1G), same results.

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

Re: [PATCH] Cadence: gem: fix wraparound in 64bit descriptors

[Philippe Mathieu-Daudé]

On 4/16/20 11:39 AM, no-re...@patchew.org wrote:

Patchew URL: 
https://patchew.org/QEMU/20200416090247.353414-1-rfried@gmail.com/


[...]

ERROR:/tmp/qemu-test/src/tests/qtest/migration-test.c:1229:test_migrate_auto_converge:
 'got_stop' should be FALSE
ERROR - Bail out! 
ERROR:/tmp/qemu-test/src/tests/qtest/migration-test.c:1229:test_migrate_auto_converge:
 'got_stop' should be FALSE
make: *** [/tmp/qemu-test/src/tests/Makefile.include:636: check-qtest-x86_64] 
Error 1


Doesn't seem related to the patch content...

Re: m68k: gdbstub crashing setting float register on cfv4e cpu

[Laurent Vivier]

Le 14/04/2020 à 18:56, Alex Bennée a écrit :
> 
> Philippe Mathieu-Daudé  writes:
> 
>> gdbstub/m68k seems broken with floats, previous to refactor commit
>> a010bdbe719 ("extend GByteArray to read register helpers").
>>
>> HEAD at 6fb1603aa2:
>>
>> $ qemu-system-m68k -s -S -cpu cfv4e
>>
>> ---[GUEST]---
>>
>> (gdb) set architecture m68k:cfv4e
>> The target architecture is assumed to be m68k:cfv4e
>> (gdb) target remote 172.17.0.1:1234
>> Remote debugging using 172.17.0.1:1234
>> (gdb) info float
>> fp0-nan(0xfff7f) (raw 0xff7f)
>> fp1-nan(0xfff7f) (raw 0xff7f)
>> fp2-nan(0xfff7f) (raw 0xff7f)
>> fp3-nan(0xfff7f) (raw 0xff7f)
>> fp4-nan(0xfff7f) (raw 0xff7f)
>> fp5-nan(0xfff7f) (raw 0xff7f)
>> fp6-nan(0xfff7f) (raw 0xff7f)
>> fp7-nan(0xfff7f) (raw 0xff7f)
>> fpcontrol  0x0 0
>> fpstatus   0x0 0
>> fpiaddr0x0 0x0
>> (gdb) set $fp0=1
>> Remote communication error.  Target disconnected.: Connection reset by
>> peer.
> 
> With my sha1 debugging test case I get different results depending on
> the cpu type:
> 
>   /home/alex/lsrc/qemu.git/tests/guest-debug/run-test.py --gdb 
> /home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb --qemu 
> /home/alex/lsrc/qemu.git/builds/user.static/m68k-linux-user/qemu-m68k --qargs 
> "" --bin tests/tcg/m68k-linux-user/sha1 --test 
> /home/alex/lsrc/qemu.git/tests/tcg/multiarch/gdbstub/sha1.py
>   GNU gdb (GDB) 10.0.50.20200414-git
>   Copyright (C) 2020 Free Software Foundation, Inc.
>   License GPLv3+: GNU GPL version 3 or later 
> 
>   This is free software: you are free to change and redistribute it.
>   There is NO WARRANTY, to the extent permitted by law.
>   Type "show copying" and "show warranty" for details.
>   This GDB was configured as "x86_64-pc-linux-gnu".
>   Type "show configuration" for configuration details.
>   For bug reporting instructions, please see:
>   .
>   Find the GDB manual and other documentation resources online at:
>   .
> 
>   For help, type "help".
>   Type "apropos word" to search for commands related to "word"...
>   Executed .gdbinit
>   Reading symbols from tests/tcg/m68k-linux-user/sha1...
>   Remote debugging using localhost:1234
>   warning: Register "fp0" has an unsupported size (96 bits)
>   warning: Register "fp1" has an unsupported size (96 bits)
>   warning: Register "fp2" has an unsupported size (96 bits)
>   warning: Register "fp3" has an unsupported size (96 bits)
>   warning: Register "fp4" has an unsupported size (96 bits)
>   warning: Register "fp5" has an unsupported size (96 bits)
>   warning: Register "fp6" has an unsupported size (96 bits)
>   warning: Register "fp7" has an unsupported size (96 bits)
>   Remote 'g' packet reply is too long (expected 148 bytes, got 180 bytes): 
> 408009f083407fff7fff7fff7fff7fff7fff7fff7fff

This is a bug in GDB that doesn't support 96bit float registers of 680x0
but only 64bit registers of coldfire.

There was a rework of GDB in the past that has broken that and no one
noticed. I bisected and found the commit but it was really too complex
and difficult to fix.

To be able to debug remotely m68k I use gdb from etch-m68k in a chroot
(or from real hardware).

Thanks,
Laurent

Re: [PATCH 01/16] accel/tcg: Add block comment for probe_access

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 


Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 02/16] accel/tcg: Add probe_access_flags

[Peter Maydell]

On Thu, 12 Mar 2020 at 04:34, Richard Henderson
 wrote:
>
> This new interface will allow targets to probe for a page
> and then handle watchpoints themselves.  This will be most
> useful for vector predicated memory operations, where one
> page lookup can be used for many operations, and one test
> can avoid many watchpoint checks.
>
> Signed-off-by: Richard Henderson 
> ---
> v2: Fix return of host pointer in softmmu probe_access_flags.
> ---

> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e3b5750c3b..bbe265ce28 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1231,86 +1231,16 @@ static void notdirty_write(CPUState *cpu, vaddr 
> mem_vaddr, unsigned size,
>  }
>  }
>
> -/*
> - * Probe for whether the specified guest access is permitted. If it is not
> - * permitted then an exception will be taken in the same way as if this
> - * were a real access (and we will not return).
> - * If the size is 0 or the page requires I/O access, returns NULL; otherwise,
> - * returns the address of the host page similar to tlb_vaddr_to_host().
> - */
> -void *probe_access(CPUArchState *env, target_ulong addr, int size,
> -   MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +static int probe_access_internal(CPUArchState *env, target_ulong addr,
> + int fault_size, MMUAccessType access_type,
> + int mmu_idx, bool nonfault,
> + void **phost, uintptr_t retaddr)
>  {
>  uintptr_t index = tlb_index(env, mmu_idx, addr);
>  CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> -target_ulong tlb_addr;
> -size_t elt_ofs;
> -int wp_access;
> -
> -g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> -
> -switch (access_type) {
> -case MMU_DATA_LOAD:
> -elt_ofs = offsetof(CPUTLBEntry, addr_read);
> -wp_access = BP_MEM_READ;
> -break;
> -case MMU_DATA_STORE:
> -elt_ofs = offsetof(CPUTLBEntry, addr_write);
> -wp_access = BP_MEM_WRITE;
> -break;
> -case MMU_INST_FETCH:
> -elt_ofs = offsetof(CPUTLBEntry, addr_code);
> -wp_access = BP_MEM_READ;
> -break;
> -default:
> -g_assert_not_reached();
> -}
> -tlb_addr = tlb_read_ofs(entry, elt_ofs);
> -
> -if (unlikely(!tlb_hit(tlb_addr, addr))) {
> -if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
> -addr & TARGET_PAGE_MASK)) {
> -tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, 
> retaddr);
> -/* TLB resize via tlb_fill may have moved the entry. */
> -index = tlb_index(env, mmu_idx, addr);
> -entry = tlb_entry(env, mmu_idx, addr);
> -}
> -tlb_addr = tlb_read_ofs(entry, elt_ofs);
> -}

All of the code above seems to have disappeared in this
refactoring -- it's not in probe_access_internal()
but it hasn't moved to the new probe_access().


> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 4be78eb9b3..c52dd8a95a 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -190,13 +190,12 @@ static inline int handle_cpu_signal(uintptr_t pc, 
> siginfo_t *info,
>  g_assert_not_reached();
>  }
>
> -void *probe_access(CPUArchState *env, target_ulong addr, int size,
> -   MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +int probe_access_flags(CPUArchState *env, target_ulong addr,
> +   MMUAccessType access_type, int mmu_idx,
> +   bool nonfault, void **phost, uintptr_t retaddr)
>  {
>  int flags;
>
> -g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> -
>  switch (access_type) {
>  case MMU_DATA_STORE:
>  flags = PAGE_WRITE;
> @@ -211,15 +210,30 @@ void *probe_access(CPUArchState *env, target_ulong 
> addr, int size,
>  g_assert_not_reached();
>  }
>
> -if (!guest_addr_valid(addr) || page_check_range(addr, size, flags) < 0) {
> -CPUState *cpu = env_cpu(env);
> -CPUClass *cc = CPU_GET_CLASS(cpu);
> -cc->tlb_fill(cpu, addr, size, access_type, MMU_USER_IDX, false,
> - retaddr);
> -g_assert_not_reached();
> +if (!guest_addr_valid(addr) || page_check_range(addr, 1, flags) < 0) {
> +if (nonfault) {
> +*phost = NULL;
> +return TLB_INVALID_MASK;
> +} else {
> +CPUState *cpu = env_cpu(env);
> +CPUClass *cc = CPU_GET_CLASS(cpu);
> +cc->tlb_fill(cpu, addr, 0, access_type, MMU_USER_IDX, false, 
> retaddr);
> +g_assert_not_reached();
> +}
>  }
>
> -return size ? g2h(addr) : NULL;
> +*phost = g2h(addr);
> +return 0;
> +}
> +
> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
> +   MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +{
> +void *host;
> +
> +g_assert(-

Re: [PATCH 03/16] exec: Add cpu_probe_watchpoint

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Allow probing of a watchpoint *without* raising an exception.
> This is of most use for no-fault loads, which should indicate
> via some architectural means that the load did not occur.
>
> Signed-off-by: Richard Henderson 
> ---
>  include/hw/core/cpu.h |  7 +++
>  exec.c| 19 +++
>  2 files changed, 26 insertions(+)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 73e9a869a4..8ec44267a4 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1090,6 +1090,12 @@ static inline void cpu_check_watchpoint(CPUState *cpu, 
> vaddr addr, vaddr len,
>  {
>  }
>
> +static inline bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
> +int flags)
> +{
> +return false;
> +}
> +
>  static inline int cpu_watchpoint_address_matches(CPUState *cpu,
>   vaddr addr, vaddr len)
>  {
> @@ -1104,6 +1110,7 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, 
> CPUWatchpoint *watchpoint);
>  void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
>  void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
>MemTxAttrs attrs, int flags, uintptr_t ra);
> +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags);

Could we have a doc comment for the new function?

>  int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
>  #endif
>
> diff --git a/exec.c b/exec.c
> index 0cc500d53a..2b8f601b9e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2735,6 +2735,25 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, 
> vaddr len,
>  }
>  }
>
> +bool cpu_probe_watchpoint(CPUState *cpu, vaddr addr, vaddr len, int flags)
> +{
> +CPUClass *cc = CPU_GET_CLASS(cpu);
> +CPUWatchpoint *wp;
> +
> +assert(tcg_enabled());
> +
> +addr = cc->adjust_watchpoint_address(cpu, addr, len);
> +QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
> +if (watchpoint_address_matches(wp, addr, len) &&
> +(wp->flags & flags) &&
> +(!(wp->flags & BP_CPU) ||
> + !cc->debug_check_watchpoint(cpu, wp))) {
> +return true;
> +}
> +}
> +return false;
> +}

Clearly the insn emulation needs to do the right thing for
guest architectural watchpoints, but should a gdb watchpoint
also affect no-fault-load behaviour? I suppose making them
both behave the same way is probably the least-surprising choice.

thanks
-- PMM

Re: [PATCH 04/16] target/arm: Use cpu_*_data_ra for sve_ldst_tlb_fn

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Use the "normal" memory access functions, rather than the
> softmmu internal helper functions directly.
>
> Since fb901c905dc3, cpu_mem_index is now a simple extract
> from env->hflags and not a large computation.  Which means
> that it's now more work to pass around this value than it
> is to recompute it.
>
> This only adjusts the primitives, and does not clean up
> all of the uses within sve_helper.c.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve_helper.c | 241 ++--
>  1 file changed, 107 insertions(+), 134 deletions(-)

> +#ifdef TARGET_WORDS_BIGENDIAN
> +# define be_bswap16(x)  ((uint16_t)(x))
> +# define be_bswap32(x)  ((uint32_t)(x))
> +# define be_bswap64(x)  ((uint64_t)(x))
> +# define le_bswap16(x)  bswap16(x)
> +# define le_bswap32(x)  bswap32(x)
> +# define le_bswap64(x)  bswap64(x)
> +#else
> +# define be_bswap16(x)  bswap16(x)
> +# define be_bswap32(x)  bswap32(x)
> +# define be_bswap64(x)  bswap64(x)
> +# define le_bswap16(x)  ((uint16_t)(x))
> +# define le_bswap32(x)  ((uint32_t)(x))
> +# define le_bswap64(x)  ((uint64_t)(x))
> +#endif

Am I confused, or are these just reimplementing
cpu_to_be16()/cpu_to_le16()/le16_to_cpu()/be16_to_cpu() etc from bswap.h ?

(It seems a pity to have to lose the memory subsystem handling
endianness for us.)

thanks
-- PMM

Re: [PATCH 05/16] target/arm: Drop manual handling of set/clear_helper_retaddr

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Since we converted back to cpu_*_data_ra, we do not need to
> do this ourselves.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [EXTERNAL] [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts

[Alex Bennée]

Cédric Le Goater  writes:

> On 4/14/20 1:11 PM, Nicholas Piggin wrote:
>> If mtmsr L=1 sets MSR[EE] while there is a maskable exception pending,
>> it does not cause an interrupt. This causes the test case to hang:
>> 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Dppc_2019-2D10_msg00826.html&d=DwIDAg&c=jf_iaSHvJObTbx-siA1ZOg&r=XHJsZhhuWSw9713Fp0ciew&m=TQfi_v-8XYgz7MiMDAZ_CjkyalSh9-EXhQ3oDesUm74&s=pFoesXbioVBh5wCuzEnzwgfze6X7e-a9unkfUgsRwiw&e=
>>  
>> 

> I was expecting more changes but this looks fine. 
>
> Reviewed-by: Cédric Le Goater 
>
>> Cc: qemu-sta...@nongnu.org
>> Reported-by: Anton Blanchard 
>> Reported-by: Nathan Chancellor 
>> Tested-by: Nathan Chancellor 
>> Signed-off-by: Nicholas Piggin 
>
> I gave it a try on PowerNV, pseries and mac99. All good.
>
> Tested-by: Cédric Le Goater 
>
> I don't know how we could include tests in QEMU such as the one Anton 
> sent. These are good exercisers for our exception model.

It certainly looks possible. The ideal would be a fresh boot.S for ppc64
that supported:

 a) some sort of serial output
 b) a way to exit the test case with a result

For ARM we use semihosting, for x86 we use plain ioport and an ACPI
exit. See the tests in: tests/tcg/[x86_64/aarch64]/system/boot.S and the
Makefile.softmmu-target files under tests/tcg.

>
> Thanks,
>
> C. 
>
>> ---
>> Thanks very much to Nathan for reporting and testing it, I added his
>> Tested-by tag despite a more polished patch, as the the basics are 
>> still the same (and still fixes his test case here).
>> 
>> This bug possibly goes back to early v2.04 / mtmsrd L=1 support around
>> 2007, and the code has been changed several times since then so may
>> require some backporting.
>> 
>> 32-bit / mtmsr untested at the moment, I don't have an environment
>> handy.
>>
>> 
>>  target/ppc/translate.c | 46 +-
>>  1 file changed, 27 insertions(+), 19 deletions(-)
>> 
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index b207fb5386..9959259dba 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -4361,30 +4361,34 @@ static void gen_mtmsrd(DisasContext *ctx)
>>  CHK_SV;
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
>> +gen_io_start();
>> +}
>>  if (ctx->opcode & 0x0001) {
>> -/* Special form that does not need any synchronisation */
>> +/* L=1 form only updates EE and RI */
>>  TCGv t0 = tcg_temp_new();
>> +TCGv t1 = tcg_temp_new();
>>  tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
>>  (1 << MSR_RI) | (1 << MSR_EE));
>> -tcg_gen_andi_tl(cpu_msr, cpu_msr,
>> +tcg_gen_andi_tl(t1, cpu_msr,
>>  ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
>> -tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
>> +tcg_gen_or_tl(t1, t1, t0);
>> +
>> +gen_helper_store_msr(cpu_env, t1);
>>  tcg_temp_free(t0);
>> +tcg_temp_free(t1);
>> +
>>  } else {
>>  /*
>>   * XXX: we need to update nip before the store if we enter
>>   *  power saving mode, we will exit the loop directly from
>>   *  ppc_store_msr
>>   */
>> -if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
>> -gen_io_start();
>> -}
>>  gen_update_nip(ctx, ctx->base.pc_next);
>>  gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
>> -/* Must stop the translation as machine state (may have) changed */
>> -/* Note that mtmsr is not always defined as context-synchronizing */
>> -gen_stop_exception(ctx);
>>  }
>> +/* Must stop the translation as machine state (may have) changed */
>> +gen_stop_exception(ctx);
>>  #endif /* !defined(CONFIG_USER_ONLY) */
>>  }
>>  #endif /* defined(TARGET_PPC64) */
>> @@ -4394,15 +4398,23 @@ static void gen_mtmsr(DisasContext *ctx)
>>  CHK_SV;
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>> -   if (ctx->opcode & 0x0001) {
>> -/* Special form that does not need any synchronisation */
>> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
>> +gen_io_start();
>> +}
>> +if (ctx->opcode & 0x0001) {
>> +/* L=1 form only updates EE and RI */
>>  TCGv t0 = tcg_temp_new();
>> +TCGv t1 = tcg_temp_new();
>>  tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
>>  (1 << MSR_RI) | (1 << MSR_EE));
>> -tcg_gen_andi_tl(cpu_msr, cpu_msr,
>> +tcg_gen_andi_tl(t1, cpu_msr,
>>  ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
>> -tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
>> +tcg_gen_or_tl(t1, t1, t0);
>> +
>> +gen_helper_store_msr(cpu_env, t1);
>>  tcg_temp_free(t0);
>> +tcg_temp_free(t1);
>> +
>>  } else {
>>  TCGv msr = tcg_temp_new();
>>  
>> @@ -4411

Re: [PATCH RFC] configure: prefer sphinx-build to sphinx-build-3

[Alex Bennée]

John Snow  writes:

> On 4/15/20 1:55 PM, Peter Maydell wrote:
>> On Wed, 15 Apr 2020 at 18:33, John Snow  wrote:
>>>
>>> sphinx-build is the name of the script entry point from the sphinx
>>> package itself. sphinx-build-3 is a pacakging convention by Linux
>>> distributions. Prefer, where possible, the canonical package name.
>> 
>> This was Markus's code originally; cc'ing him.
>> 
>> (Incidentally I think when we say "Linux distributions" we
>> really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)
>> 
>
> I'll take your word for it :)
>
>> thanks
>> -- PMM
>> (rest of email untrimmed for context)
>> 
>
> My only goal here is that if you are using a virtual environment with
> sphinx installed that it prefers that, so non-standard names need to
> come last.
>
> There's probably 10,000,000 ways to do that, hence the RFC.

What's wrong with just passing --sphinx-build=sphinx-build in your
configure string? It will override whatever we auto-detect AFAICT.

-- 
Alex Bennée

Re: [PATCH v18 3/4] qcow2: add zstd cluster compression

[Alberto Garcia]

On Thu 02 Apr 2020 08:36:44 AM CEST, Denis Plotnikov wrote:
> +static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
> +   const void *src, size_t src_size)
> +{
> +ssize_t ret;
> +ZSTD_outBuffer output = { dest, dest_size, 0 };
> +ZSTD_inBuffer input = { src, src_size, 0 };
> +ZSTD_CCtx *cctx = ZSTD_createCCtx();
> +
> +if (!cctx) {
> +return -EIO;
> +}
> +/*
> + * Use the zstd streamed interface for symmetry with decompression,
> + * where streaming is essential since we don't record the exact
> + * compressed size.
> + *
> + * In the loop, we try to compress all the data into one zstd frame.
> + * ZSTD_compressStream2 potentially can finish a frame earlier
> + * than the full input data is consumed. That's why we are looping
> + * until all the input data is consumed.
> + */
> +while (input.pos < input.size) {
> +size_t zstd_ret;
> +/*
> + * ZSTD spec: "You must continue calling ZSTD_compressStream2()
> + * with ZSTD_e_end until it returns 0, at which point you are
> + * free to start a new frame". We assume that "start a new frame"
> + * means call ZSTD_compressStream2 in the very beginning or when
> + * ZSTD_compressStream2 has returned with 0.
> + */
> +do {
> +zstd_ret = ZSTD_compressStream2(cctx, &output, &input, 
> ZSTD_e_end);
> +
> +if (ZSTD_isError(zstd_ret)) {
> +ret = -EIO;
> +goto out;
> +}
> +/* Dest buffer isn't big enough to store compressed content */
> +if (zstd_ret > output.size - output.pos) {
> +ret = -ENOMEM;
> +goto out;
> +}
> +} while (zstd_ret);
> +}
> +/* make sure we can safely return compressed buffer size with ssize_t */
> +assert(output.pos <= SSIZE_MAX);

The patch looks good to me, but why don't you assert this at the
beginning of the function? You already know the buffer sizes...

Either way,

Reviewed-by: Alberto Garcia 

Berto

Re: [PATCH 06/16] target/arm: Add sve infrastructure for page lookup

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> For contiguous predicated memory operations, we want to
> minimize the number of tlb lookups performed.  We have
> open-coded this for sve_ld1_r, but for correctness with
> MTE we will need this for all of the memory operations.
>
> Create a structure that holds the bounds of active elements,
> and metadata for two pages.  Add routines to find those
> active elements, lookup the pages, and run watchpoints
> for those pages.
>
> Temporarily mark the functions unused to avoid Werror.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve_helper.c | 240 
>  1 file changed, 240 insertions(+)
>
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index 8b470991db..3f653e46a0 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -4155,6 +4155,246 @@ static intptr_t max_for_page(target_ulong base, 
> intptr_t mem_off,
>  return MIN(split, mem_max - mem_off) + mem_off;
>  }
>
> +/*
> + * Resolve the guest virtual address to info->host and info->flags.
> + * If @nofault, return false if the page is invalid, otherwise
> + * exit via page fault exception.
> + */
> +
> +typedef struct {
> +void *host;
> +int flags;
> +MemTxAttrs attrs;
> +} SVEHostPage;
> +
> +static bool sve_probe_page(SVEHostPage *info, bool nofault,
> +   CPUARMState *env, target_ulong addr,
> +   int mem_off, MMUAccessType access_type,
> +   int mmu_idx, uintptr_t retaddr)
> +{
> +int flags;
> +
> +addr += mem_off;
> +flags = probe_access_flags(env, addr, access_type, mmu_idx, nofault,
> +   &info->host, retaddr);
> +info->flags = flags;
> +
> +if (flags & TLB_INVALID_MASK) {
> +g_assert(nofault);
> +return false;
> +}
> +
> +/* Ensure that info->host[] is relative to addr, not addr + mem_off. */
> +info->host -= mem_off;
> +
> +#ifdef CONFIG_USER_ONLY
> +memset(&info->attrs, 0, sizeof(info->attrs));

Could just write "info->attrs = {};" ?

> +#else
> +/*
> + * Find the iotlbentry for addr and return the transaction attributes.
> + * This *must* be present in the TLB because we just found the mapping.
> + */
> +{
> +uintptr_t index = tlb_index(env, mmu_idx, addr);
> +
> +# ifdef CONFIG_DEBUG_TCG
> +CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> +target_ulong comparator = (access_type == MMU_DATA_LOAD
> +   ? entry->addr_read
> +   : tlb_addr_write(entry));
> +g_assert(tlb_hit(comparator, addr));
> +# endif
> +
> +CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
> +info->attrs = iotlbentry->attrs;
> +}
> +#endif
> +
> +return true;
> +}
> +
> +
> +/*
> + * Analyse contiguous data, protected by a governing predicate.
> + */
> +
> +typedef enum {
> +FAULT_NO,
> +FAULT_FIRST,
> +FAULT_ALL,
> +} SVEContFault;
> +
> +typedef struct {
> +/* First and last element wholy contained within the two pages. */

"wholly"


> +int16_t mem_off_first[2];
> +int16_t reg_off_first[2];
> +int16_t reg_off_last[2];

It would be helpful to document what these actually are,
and in particular what the behaviour is if the whole thing
fits in a single page. (Judging by the code, the elements
at index 1 for the 2nd page are set to -1 ?)

> +
> +/* One element that is misaligned and spans both pages. */
> +int16_t mem_off_split;
> +int16_t reg_off_split;
> +int16_t page_split;
> +
> +/* TLB data for the two pages. */
> +SVEHostPage page[2];
> +} SVEContLdSt;
> +
> +/*
> + * Find first active element on each page, and a loose bound for the
> + * final element on each page.  Identify any single element that spans
> + * the page boundary.  Return true if there are any active elements.
> + */
> +static bool __attribute__((unused))
> +sve_cont_ldst_elements(SVEContLdSt *info, target_ulong addr, uint64_t *vg,
> +   intptr_t reg_max, int esz, int msize)
> +{
> +const int esize = 1 << esz;
> +const uint64_t pg_mask = pred_esz_masks[esz];
> +intptr_t reg_off_first = -1, reg_off_last = -1, reg_off_split;
> +intptr_t mem_off_last, mem_off_split;
> +intptr_t page_split, elt_split;
> +intptr_t i;

intptr_t seems like a funny type to be using here, since these
aren't actually related to pointers as far as I can tell.
It's also odd that the type is not the same one used in the SVEContLdSt
struct for the corresponding fields.

> +
> +/* Set all of the element indicies to -1, and the TLB data to 0. */

"indices"

> +memset(info, -1, offsetof(SVEContLdSt, page));

I guess this isn't conceptually much different from zeroing
out integer struct fields, but it feels a bit less safe somehow.

> +memset(info->page, 0, sizeof(i

Re: [PATCH v18 3/4] qcow2: add zstd cluster compression

[Denis Plotnikov]

On 16.04.2020 15:55, Alberto Garcia wrote:

On Thu 02 Apr 2020 08:36:44 AM CEST, Denis Plotnikov wrote:

+static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
+   const void *src, size_t src_size)
+{
+ssize_t ret;
+ZSTD_outBuffer output = { dest, dest_size, 0 };
+ZSTD_inBuffer input = { src, src_size, 0 };
+ZSTD_CCtx *cctx = ZSTD_createCCtx();
+
+if (!cctx) {
+return -EIO;
+}
+/*
+ * Use the zstd streamed interface for symmetry with decompression,
+ * where streaming is essential since we don't record the exact
+ * compressed size.
+ *
+ * In the loop, we try to compress all the data into one zstd frame.
+ * ZSTD_compressStream2 potentially can finish a frame earlier
+ * than the full input data is consumed. That's why we are looping
+ * until all the input data is consumed.
+ */
+while (input.pos < input.size) {
+size_t zstd_ret;
+/*
+ * ZSTD spec: "You must continue calling ZSTD_compressStream2()
+ * with ZSTD_e_end until it returns 0, at which point you are
+ * free to start a new frame". We assume that "start a new frame"
+ * means call ZSTD_compressStream2 in the very beginning or when
+ * ZSTD_compressStream2 has returned with 0.
+ */
+do {
+zstd_ret = ZSTD_compressStream2(cctx, &output, &input, ZSTD_e_end);
+
+if (ZSTD_isError(zstd_ret)) {
+ret = -EIO;
+goto out;
+}
+/* Dest buffer isn't big enough to store compressed content */
+if (zstd_ret > output.size - output.pos) {
+ret = -ENOMEM;
+goto out;
+}
+} while (zstd_ret);
+}
+/* make sure we can safely return compressed buffer size with ssize_t */
+assert(output.pos <= SSIZE_MAX);

The patch looks good to me, but why don't you assert this at the
beginning of the function? You already know the buffer sizes...
Yes, that's true. But I thought that it's reasonable to check what is 
returned.
"output.pos" could be less then or equal to ssize_max when output.size > 
ssize_max.
Anyway, this case most probably won't exist, and this is just an 
overflow inssurance.


Either way

Reviewed-by: Alberto Garcia 

Berto

Thanks for reviewing!

Denis

Re: [PATCH 07/16] target/arm: Adjust interface of sve_ld1_host_fn

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> The current interface includes a loop; change it to load a
> single element.  We will then be able to use the function
> for ld{2,3,4} where individual vector elements are not adjacent.
>
> Replace each call with the simplest possible loop over active
> elements.
>
> Signed-off-by: Richard Henderson 
> ---

Reviewed-by: Peter Maydell 
(I haven't attempted to verify the details of all the
arithmetic, but I assume you've tested it all...)

thanks
-- PMM

Re: [PATCH 08/16] target/arm: Use SVEContLdSt in sve_ld1_r

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> First use of the new helper functions, so we can remove the
> unused markup.  No longer need a scratch for user-only, as
> we completely probe the page set before reading; system mode
> still requires a scratch for MMIO.
>
> Signed-off-by: Richard Henderson 

> +/* The entire operation is in RAM, on valid pages. */
> +
> +memset(vd, 0, reg_max);
> +mem_off = info.mem_off_first[0];
> +reg_off = info.reg_off_first[0];
> +reg_last = info.reg_off_last[0];
> +host = info.page[0].host;
> +
> +while (reg_off <= reg_last) {
> +uint64_t pg = vg[reg_off >> 6];
> +do {
> +if ((pg >> (reg_off & 63)) & 1) {
> +host_fn(vd, reg_off, host + mem_off);
> +}
> +reg_off += 1 << esz;
> +mem_off += 1 << msz;
> +} while (reg_off <= reg_last && (reg_off & 63));
> +}
> +
> +/*
> + * Use the slow path to manage the cross-page misalignment.
> + * But we know this is RAM and cannot trap.
> + */
> +mem_off = info.mem_off_split;
> +if (unlikely(mem_off >= 0)) {
> +tlb_fn(env, vd, info.reg_off_split, addr + mem_off, retaddr);
> +}
> +
> +mem_off = info.mem_off_first[1];
> +if (unlikely(mem_off >= 0)) {
> +reg_off = info.reg_off_first[1];
> +reg_last = info.reg_off_last[1];
> +host = info.page[1].host;
> +
> +do {
> +uint64_t pg = vg[reg_off >> 6];
> +do {
> +if ((pg >> (reg_off & 63)) & 1) {
> +host_fn(vd, reg_off, host + mem_off);
> +}
> +reg_off += 1 << esz;
> +mem_off += 1 << msz;
> +} while (reg_off & 63);
> +} while (reg_off <= reg_last);

Does this loop for the second page need to be phrased
differently than the loop for the first page was? I was
expecting the two chunks of code to be identical, and they
almost are, but not quite...

Either way
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 09/16] target/arm: Handle watchpoints in sve_ld1_r

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Handle all of the watchpoints for active elements all at once,
> before we've modified the vector register.  This removes the
> TLB_WATCHPOINT bit from page[].flags, which means that we can
> use the normal fast path via RAM.
>
> Signed-off-by: Richard Henderson 

> +if (flags0 & TLB_WATCHPOINT) {
> +mem_off = info->mem_off_first[0];
> +reg_off = info->reg_off_first[0];
> +reg_last = info->reg_off_last[0];
> +
> +while (reg_off <= reg_last) {
> +uint64_t pg = vg[reg_off >> 6];
> +do {
> +if ((pg >> (reg_off & 63)) & 1) {
> +cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
> + info->page[0].attrs, wp_access, 
> retaddr);
> +}
> +reg_off += esize;
> +mem_off += msize;
> +} while (reg_off <= reg_last && (reg_off & 63));
> +}
> +}
> +
> +mem_off = info->mem_off_split;
> +if (mem_off >= 0) {
> +cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
> + info->page[0].attrs, wp_access, retaddr);
> +}
> +
> +mem_off = info->mem_off_first[1];
> +if ((flags1 & TLB_WATCHPOINT) && mem_off >= 0) {
> +reg_off = info->reg_off_first[1];
> +reg_last = info->reg_off_last[1];
> +
> +do {
> +uint64_t pg = vg[reg_off >> 6];
> +do {
> +if ((pg >> (reg_off & 63)) & 1) {
> +cpu_check_watchpoint(env_cpu(env), addr + mem_off, msize,
> + info->page[1].attrs, wp_access, 
> retaddr);
> +}
> +reg_off += esize;
> +mem_off += msize;
> +} while (reg_off & 63);
> +} while (reg_off <= reg_last);
> +}

Another pair of almost-but-not-quite-the-same loops.

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH RFC v2] target/arm: Implement SVE2 HISTCNT, HISTSEG

[Stephen Long]

Signed-off-by: Stephen Long 
---
Whoops, I was mistaken on what HISTSEG was doing.

 target/arm/helper-sve.h|  7 +++
 target/arm/sve.decode  |  6 +++
 target/arm/sve_helper.c| 90 ++
 target/arm/translate-sve.c | 29 
 4 files changed, 132 insertions(+)

diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index 4733614614..958ad623f6 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2526,6 +2526,13 @@ DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_b, TCG_CALL_NO_RWG,
 DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_h, TCG_CALL_NO_RWG,
i32, ptr, ptr, ptr, ptr, i32)
 
+DEF_HELPER_FLAGS_5(sve2_histcnt_s, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_histcnt_d, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(sve2_histseg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_h, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_s, TCG_CALL_NO_RWG,
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 26690d4208..9dd20eb6ec 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -147,6 +147,7 @@
 &rprrr_esz rn=%reg_movprfx
 @rdn_pg_rm_ra    esz:2 . ra:5  ... pg:3 rm:5 rd:5 \
 &rprrr_esz rn=%reg_movprfx
+@rd_pg_rn_rm    esz:2 . rm:5 ... pg:3 rn:5 rd:5   &rprr_esz
 
 # One register operand, with governing predicate, vector element size
 @rd_pg_rn    esz:2 ... ... ... pg:3 rn:5 rd:5   &rpr_esz
@@ -1325,6 +1326,11 @@ UQRSHRNT01000101 .. 1 . 00  . .  
@rd_rn_tszimm_shr
 MATCH   01000101 .. 1 . 100 ... . 0  @pd_pg_rn_rm
 NMATCH  01000101 .. 1 . 100 ... . 1  @pd_pg_rn_rm
 
+### SVE2 Histogram Computation
+
+HISTCNT 01000101 .. 1 . 110 ... . .  @rd_pg_rn_rm
+HISTSEG 01000101 .. 1 . 101 000 . .  @rd_rn_rm
+
 ## SVE2 floating-point pairwise operations
 
 FADDP   01100100 .. 010 00 0 100 ... . . @rdn_pg_rm
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 7c65009bb8..1489141792 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7016,3 +7016,93 @@ DO_PPZZ_MATCH(sve2_nmatch_ppzz_b, MO_8, true)
 DO_PPZZ_MATCH(sve2_nmatch_ppzz_h, MO_16, true)
 
 #undef DO_PPZZ_MATCH
+
+void HELPER(sve2_histcnt_s)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+uint32_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; i += 4) {
+uint64_t count = 0;
+uint8_t pred = pg[H1(i >> 3)] >> (i & 7);
+if (pred & 1) {
+uint32_t nn = n[H4(i >> 2)];
+for (j = 0; j <= i; j += 4) {
+uint8_t pred = pg[H1(j >> 3)] >> (j & 7);
+if (pred & 1 && nn == m[H4(j >> 2)]) {
+++count;
+}
+}
+}
+d[H4(i >> 2)] = count;
+}
+}
+
+void HELPER(sve2_histcnt_d)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc) / 8;
+uint64_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; ++i) {
+uint64_t count = 0;
+if (pg[H1(i)] & 1) {
+uint64_t nn = n[i];
+for (j = 0; j <= i; ++j) {
+if (pg[H1(j)] & 1 && nn == m[j]) {
+++count;
+}
+}
+}
+d[i] = count;
+}
+}
+
+/*
+ * Returns the number of bytes in m0 and m1 that match n.
+ * See comment for do_match2().
+ * */
+static inline uint8_t do_histseg_cnt(uint8_t n, uint64_t m0, uint64_t m1)
+{
+int esz = 0;
+int bits = 8 << esz;
+uint64_t ones = dup_const(esz, 1);
+uint64_t signs = ones << (bits - 1);
+uint64_t cmp0, cmp1;
+
+cmp1 = dup_const(1, n);
+cmp0 = cmp1 ^ m0;
+cmp1 = cmp1 ^ m1;
+cmp0 = (cmp0 - ones) & ~cmp0;
+cmp1 = (cmp1 - ones) & ~cmp1;
+return ctpop64((cmp0 | cmp1) & signs);
+}
+
+void HELPER(sve2_histseg)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+uint64_t m0, m1;
+
+for (i = 0; i < opr_sz; i += 8) {
+uint64_t n = *(uint64_t *)(vn + i);
+uint64_t out = 0;
+
+if ((i & 1) == 0) {
+m0 = *(uint64_t *)(vm + i);
+m1 = *(uint64_t *)(vm + i + 8);
+}
+
+for (j = 0; j < 64; j += 8) {
+uint8_t count = do_histseg_cnt(n >> j, m0, m1);
+out |= count << j;
+}
+
+*(uint64_t *)(vd + i) = out;
+}
+}
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 5403ceb1d1..b2b6456a1c 100

Re: [PATCH 10/16] target/arm: Use SVEContLdSt for multi-register contiguous loads

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve_helper.c | 223 ++--
>  1 file changed, 79 insertions(+), 144 deletions(-)


>  #define DO_LDN_1(N) \
> -void QEMU_FLATTEN HELPER(sve_ld##N##bb_r) \
> -(CPUARMState *env, void *vg, target_ulong addr, uint32_t desc)  \
> -{   \
> -sve_ld##N##_r(env, vg, addr, desc, 1, GETPC(), sve_ld1bb_tlb);  \
> +void HELPER(sve_ld##N##bb_r)(CPUARMState *env, void *vg,\
> + target_ulong addr, uint32_t desc)  \
> +{   \
> +sve_ldN_r(env, vg, addr, desc, GETPC(), MO_8, MO_8, N,  \
> +  sve_ld1bb_host, sve_ld1bb_tlb);   \
>  }

Deliberately losing the QEMU_FLATTEN here?

Otherwise

Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH RFC v3] target/arm: Implement SVE2 HISTCNT, HISTSEG

[Stephen Long]

Signed-off-by: Stephen Long 
---
Disregard my previous patch. There was a mistake in translate_HISTCNT().

 target/arm/helper-sve.h|  7 +++
 target/arm/sve.decode  |  6 +++
 target/arm/sve_helper.c| 90 ++
 target/arm/translate-sve.c | 29 
 4 files changed, 132 insertions(+)

diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index 4733614614..958ad623f6 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2526,6 +2526,13 @@ DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_b, TCG_CALL_NO_RWG,
 DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_h, TCG_CALL_NO_RWG,
i32, ptr, ptr, ptr, ptr, i32)
 
+DEF_HELPER_FLAGS_5(sve2_histcnt_s, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_histcnt_d, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(sve2_histseg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_h, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_s, TCG_CALL_NO_RWG,
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 26690d4208..9dd20eb6ec 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -147,6 +147,7 @@
 &rprrr_esz rn=%reg_movprfx
 @rdn_pg_rm_ra    esz:2 . ra:5  ... pg:3 rm:5 rd:5 \
 &rprrr_esz rn=%reg_movprfx
+@rd_pg_rn_rm    esz:2 . rm:5 ... pg:3 rn:5 rd:5   &rprr_esz
 
 # One register operand, with governing predicate, vector element size
 @rd_pg_rn    esz:2 ... ... ... pg:3 rn:5 rd:5   &rpr_esz
@@ -1325,6 +1326,11 @@ UQRSHRNT01000101 .. 1 . 00  . .  
@rd_rn_tszimm_shr
 MATCH   01000101 .. 1 . 100 ... . 0  @pd_pg_rn_rm
 NMATCH  01000101 .. 1 . 100 ... . 1  @pd_pg_rn_rm
 
+### SVE2 Histogram Computation
+
+HISTCNT 01000101 .. 1 . 110 ... . .  @rd_pg_rn_rm
+HISTSEG 01000101 .. 1 . 101 000 . .  @rd_rn_rm
+
 ## SVE2 floating-point pairwise operations
 
 FADDP   01100100 .. 010 00 0 100 ... . . @rdn_pg_rm
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 7c65009bb8..1489141792 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7016,3 +7016,93 @@ DO_PPZZ_MATCH(sve2_nmatch_ppzz_b, MO_8, true)
 DO_PPZZ_MATCH(sve2_nmatch_ppzz_h, MO_16, true)
 
 #undef DO_PPZZ_MATCH
+
+void HELPER(sve2_histcnt_s)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+uint32_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; i += 4) {
+uint64_t count = 0;
+uint8_t pred = pg[H1(i >> 3)] >> (i & 7);
+if (pred & 1) {
+uint32_t nn = n[H4(i >> 2)];
+for (j = 0; j <= i; j += 4) {
+uint8_t pred = pg[H1(j >> 3)] >> (j & 7);
+if (pred & 1 && nn == m[H4(j >> 2)]) {
+++count;
+}
+}
+}
+d[H4(i >> 2)] = count;
+}
+}
+
+void HELPER(sve2_histcnt_d)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc) / 8;
+uint64_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; ++i) {
+uint64_t count = 0;
+if (pg[H1(i)] & 1) {
+uint64_t nn = n[i];
+for (j = 0; j <= i; ++j) {
+if (pg[H1(j)] & 1 && nn == m[j]) {
+++count;
+}
+}
+}
+d[i] = count;
+}
+}
+
+/*
+ * Returns the number of bytes in m0 and m1 that match n.
+ * See comment for do_match2().
+ * */
+static inline uint8_t do_histseg_cnt(uint8_t n, uint64_t m0, uint64_t m1)
+{
+int esz = 0;
+int bits = 8 << esz;
+uint64_t ones = dup_const(esz, 1);
+uint64_t signs = ones << (bits - 1);
+uint64_t cmp0, cmp1;
+
+cmp1 = dup_const(1, n);
+cmp0 = cmp1 ^ m0;
+cmp1 = cmp1 ^ m1;
+cmp0 = (cmp0 - ones) & ~cmp0;
+cmp1 = (cmp1 - ones) & ~cmp1;
+return ctpop64((cmp0 | cmp1) & signs);
+}
+
+void HELPER(sve2_histseg)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+uint64_t m0, m1;
+
+for (i = 0; i < opr_sz; i += 8) {
+uint64_t n = *(uint64_t *)(vn + i);
+uint64_t out = 0;
+
+if ((i & 1) == 0) {
+m0 = *(uint64_t *)(vm + i);
+m1 = *(uint64_t *)(vm + i + 8);
+}
+
+for (j = 0; j < 64; j += 8) {
+uint8_t count = do_histseg_cnt(n >> j, m0, m1);
+out |= count << j;
+}
+
+*(uint64_t *)(vd + i) = out;
+}
+}
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 540

RE: [PATCH v25 00/10] Add ARMv8 RAS virtualization support in QEMU

[gengdongjiu]

ping

发件人：gengdongjiu 
收件人：imammedo ;mst ;xiaoguangrong.eric 
;Peter Maydell 
;shannon.zhaosl ;fam 
;rth ;Eduardo Habkost 
;mtosatti ;qemu-devel 
;kvm ;qemu-arm 
;pbonzini 
抄 送：gengdongjiu ;zhengxiang (A) 
;Jonathan Cameron 
;Linuxarm 
时 间：2020-04-10 19:45:28
主 题：[PATCH v25 00/10] Add ARMv8 RAS virtualization support in QEMU

In the ARMv8 platform, the CPU error types includes synchronous external 
abort(SEA)
and SError Interrupt (SEI). If exception happens in guest, host does not know 
the detailed
information of guest, so it is expected that guest can do the recovery. For 
example, if an
exception happens in a guest user-space application, host does not know which 
application
encounters errors, only guest knows it.

For the ARMv8 SEA/SEI, KVM or host kernel delivers SIGBUS to notify userspace.
After user space gets the notification, it will record the CPER into guest GHES
buffer and inject an exception or IRQ to guest.

In the current implementation, if the type of SIGBUS is BUS_MCEERR_AR, we will
treat it as a synchronous exception, and notify guest with ARMv8 SEA
notification type after recording CPER into guest.

A) This series of patches are based on Qemu 4.2, which include two parts:
1. Generate APEI/GHES table.
2. Handle the SIGBUS signal, record the CPER in runtime and fill it into guest
   memory, then notify guest according to the type of SIGBUS.

B) The solution was suggested by James(james.mo...@arm.com); The APEI part 
solution was suggested by Laszlo(ler...@redhat.com). Show some discussions in 
[1].

C) This series of patches have already been tested on ARM64 platform with RAS
feature enabled:
1. Show the APEI part verification result in [2].
2. Show the SIGBUS of BUS_MCEERR_AR handling verification result in [3].

D) Add 'ras' option in command Line to enable guest RAS error recovery feature, 
For example:
KVM model: ./qemu-system-aarch64 --enable-kvm -cpu host --bios QEMU_EFI.fd_new  
-machine virt,gic-version=3,ras,kernel-irqchip=on
-smp 4 -nographic -kernel Image  -append "rdinit=/init console=ttyAMA0 mem=512M 
root=/dev/ram0" -initrd guestfs_new.cpio.gz
TCG model: ./qemu-system-aarch64 -cpu cortex-a57 --bios QEMU_EFI.fd_new  
-machine virt,gic-version=3,ras,kernel-irqchip=on  -smp 4
-nographic -kernel Image  -append "rdinit=/init console=ttyAMA0 mem=512M 
root=/dev/ram0" -initrd guestfs_new.cpio.gz
---
Change since v23:
1. fix a warning for uuid

Change since v22:
1. Using 1 * KiB instead of 0x400 to define max size of one error block
2. Make the alignment to 8 bytes in bios_linker_loader_alloc()
3. Change "Copyright (c) 2019" to "Copyright (c) 2020" in file header
4. Fix some code style warnings/errors and add some comments in code
5. Address Jonathan's comments to easily support CCIX error injection
6. Add vmstate_ghes_state .subsections in vmstate_acpi_ged

Change since v21:
1. Make the user-facing 'ras' option description more clearly to address 
Peter's comments.
2. Update the doc description in "docs/specs/acpi_hest_ghes.rst"
3. Split HEST/GHES patches to more patches to make the review easily
4. Using source_id to index the location to save the CPER.
5. Optimize and simplify the logic to build HEST/GHES table to address 
Igor/Michael/Beata comments.
6. make ghes_addr_le a part of GED device.

Change since v20:
1. Move some implementation details from acpi_ghes.h to acpi_ghes.c
2. Add the reviewers for the ACPI/APEI/GHES part

Change since v19:
1. Fix clang compile error
2. Fix sphinx build error

Change since v18:
1. Fix some code-style and typo/grammar problems.
2. Remove no_ras in the VirtMachineClass struct.
3. Convert documentation to rst format.
4. Simplize the code and add comments for some magic value.
5. Move kvm_inject_arm_sea() function into the patch where it's used.
6. Register the reset handler(kvm_unpoison_all()) in the kvm_init() function.

Change since v17:
1. Improve some commit messages and comments.
2. Fix some code-style problems.
3. Add a *ras* machine option.
4. Move HEST/GHES related structures and macros into "hw/acpi/acpi_ghes.*".
5. Move HWPoison page functions into "include/sysemu/kvm_int.h".
6. Fix some bugs.
7. Improve the design document.

Change since v16:
1. check whether ACPI table is enabled when handling the memory error in the 
SIGBUS handler.

Change since v15:
1. Add a doc-comment in the proper format for 'include/exec/ram_addr.h'
2. Remove write_part_cpustate_to_list() because there is another bug fix patch
   has been merged "arm: Allow system registers for KVM guests to be changed by 
QEMU code"
3. Add some comments for kvm_inject_arm_sea() in 'target/arm/kvm64.c'
4. Compare the arm_current_el() return value to 0,1,2,3, not to PSTATE_MODE_* 
constants.
5. Change the RAS support wasn't introduced before 4.1 QEMU version.
6. Move the no_ras flag  patch to begin in this series

Change since v14:
1. Remove the BUS_MCEERR_AO handling logic because this asynchronous signal was 
masked by main thread
2. Address some Igor Mammedov's comments

Re: [PATCH v25 00/10] Add ARMv8 RAS virtualization support in QEMU

[Peter Maydell]

On Thu, 16 Apr 2020 at 14:54, gengdongjiu  wrote:
>
> ping

Hi; this is on my to-review queue, but so are 25 other patchsets.
(I've built up a bit of a backlog due to concentrating on work
for the 5.0 release while we're in the freeze period.) I will
get to it eventually if nobody else does first...

thanks
-- PMM

RE: [PATCH v25 00/10] Add ARMv8 RAS virtualization support in QEMU

[gengdongjiu]

ok，thanks very much for peter's time and reply

发件人：Peter Maydell 
收件人：gengdongjiu 
抄 送：imammedo ;mst ;xiaoguangrong.eric 
;shannon.zhaosl ;fam 
;rth ;Eduardo Habkost 
;mtosatti ;qemu-devel 
;kvm ;qemu-arm 
;pbonzini ;zhengxiang (A) 
;Linuxarm ;Jonathan Cameron 

时 间：2020-04-16 22:02:34
主 题：Re: [PATCH v25 00/10] Add ARMv8 RAS virtualization support in QEMU

On Thu, 16 Apr 2020 at 14:54, gengdongjiu  wrote:
>
> ping

Hi; this is on my to-review queue, but so are 25 other patchsets.
(I've built up a bit of a backlog due to concentrating on work
for the 5.0 release while we're in the freeze period.) I will
get to it eventually if nobody else does first...

thanks
-- PMM

Re: [edk2-discuss] Load Option passing. Either bugs or my confusion.

[Laszlo Ersek]

On 04/16/20 06:38, Hou Qiming wrote:
> Very good point, I did neglect ramfb resolution changes... But there is one
> important thing: it *can* cause a QEMU crash, a potentially exploitable
> one, not always a guest crash. That's what motivated my heavy-handed
> approach since allowing resolution change would have necessitated a good
> deal of security checks. It has crashed my host *kernel* quite a few times.
> 
> The point is, while the QemuRamfbDxe driver may behave properly, nothing
> prevents the guest from writing garbage or *malicious* values to the ramfb
> config space. Then the values are sent to the display component without any
> sanity check. For some GUI frontends, this means allocating an OpenGL
> texture with guest-supplied dimensions and uploading guest memory content
> to it, which means that guest memory content goes straight into a *kernel
> driver*, *completely unchecked*. Some integer overflow and a lenient GPU
> driver later, and the guest escapes straight to kernel.
> 
> The proper way to enable ramfb resolution change again is adding sanity
> checks for ramfb resolution / pointer / etc. on the QEMU side. We have to
> make sure it doesn't exceed what the host GPU driver supports. Maybe clamp
> both width and height to between 1 and 2048? We also need to validate that
> OpenGL texture dimension update succeeds. Note that OpenGL is not obliged
> to validate anything and everything has to be checked on the QEMU side.

I agree that QEMU should sanity check the resolution requested by the
guest. I also agree that "arbitrary" limits are acceptable, for
preventing integer overflows and -- hopefully -- memory allocation
failures too.

But I don't see the host kernel / OpenGL / physical GPU angle, at least
not directly. That angle seems to be specific to your particular use
case (particular choice of display backend).

For example, if you nest QEMU/TCG in QEMU/TCG, with no KVM and no device
assignment in the picture anywhere, and OVMF drives ramfb in L2, and the
display *backend* (such as GTK or SDL GUI window) for the QEMU process
running in L1 sits on top of a virtual device (such as bochs-display)
provided by QEMU running in L0, then the ramfb stuff (including the
resolution changes and the range checks) should work just the same,
between L2 and L1.

I kinda feel like ramfb has been hijacked for providing a boot time
display crutch for kvmgt. (I might not be using the correct terminology
here; sorry about that). That's *not* what ramfb was originally intended
for, as far as I recall. Compare:

- 59926de9987c ("Merge remote-tracking branch
'remotes/kraxel/tags/vga-20180618-pull-request' into staging", 2018-06-19)

- dddb37495b84 ("Merge remote-tracking branch
'remotes/awilliam/tags/vfio-updates-20181015.0' into staging", 2018-10-15)

IIRC, Gerd originally invented ramfb for giving AARCH64 Windows the
linear framebuffer that the latter so badly wants, in particular so that
the framebuffer exist in guest RAM (not in guest MMIO), in order to
avoid the annoying S1/S2 caching behavior of AARCH64/KVM when the guest
maps an area as MMIO that is mapped as RAM on the host [1]. See:

- https://bugzilla.tianocore.org/show_bug.cgi?id=785#c4
- https://bugzilla.tianocore.org/show_bug.cgi?id=785#c7
- https://bugzilla.tianocore.org/show_bug.cgi?id=785#c8

and the further references given in those bugzilla comments.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1679680#c0

Component reuse is obviously *hugely* important, and it would be silly
for me to argue against reusing ramfb wherever it applies. Just please
don't break the original use case.

Should I file a bug report in LaunchPad, or is this thread enough for
tracking the QEMU regression?

Thanks
Laszlo

> 
> Qiming
> 
> 
> On Wed, Apr 15, 2020 at 11:05 PM Laszlo Ersek  wrote:
> 
>> (CC Gerd, Qiming, Marcel, qemu-devel for ramfb:)
>>
>> On 04/14/20 23:20, valerij zaporogeci wrote:
>>
>> [snip]
>>
>>> There is a Boot Manager UI display problem, I don't know if this is
>>> qemu problem, but with the ARM (both 32 and 64 bits, the qemu version
>>> is 4.2.0, the OVMF is fresh), and using "ramfb" device, the Boot
>>> Manager has troubles with drawing - it's interfase looks entirely
>>> broken, like this (I'll try to attach the screenshot). UEFI shell
>>> doesn't have this problem. switching to "serial" (which is -serial vc)
>>> doesn't produce it too. Only when ramfb is chosen, the Boot Manager UI
>>> gets smeared. But it takes input and presumable works properly, except
>>> displaying things. qemu writes these messages in the command prompt:
>>> ramfb_fw_cfg_write: 640x480 @ 0x4bd0
>>> ramfb_fw_cfg_write: resolution locked, change rejected
>>> ramfb_fw_cfg_write: 800x600 @ 0x4bd0
>>> ramfb_fw_cfg_write: resolution locked, change rejected
>>
>> Gerd contributed the OVMF QemuRamfbDxe driver in edk2 commit
>> 1d25ff51af5c ("OvmfPkg: add QemuRamfbDxe", 2018-06-14). Note the date:
>> June 2018.
>>
>> The then-latest (released) QEMU version was v2.12.

Re: [PATCH 11/16] target/arm: Update contiguous first-fault and no-fault loads

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> With sve_cont_ldst_pages, the differences between first-fault and no-fault
> are minimal, so unify the routines.  With cpu_probe_watchpoint, we are able
> to make progress through pages with TLB_WATCHPOINT set when the watchpoint
> does not actually fire.
>
> Signed-off-by: Richard Henderson 

>  /*
> - * Perform one normal read, which will fault or not.
> - * But it is likely to bring the page into the tlb.
> + * From this point on, all memory operations are MemSingleNF.
> + *
> + * Per the MemSingleNF pseudocode, a no-fault load from Device memory
> + * must not actually hit the bus -- it returns (UNKNOWN, FAULT) instead.
> + * If you map non-RAM with Normal memory attributes and do a NF
> + * load then it should access the bus -- but doing so is illegal.
> + *
> + * While we do not have access to the memory attributes from the PTE
> + * to tell Device memory from Normal memory, we can validly assume that
> + * non-RAM has been mapped as Device memory.  Thus we indicate fault
> + * on all MMIO.

I don't think you can assume this; for instance a QEMU 'romd'
device might reasonably be mapped as Normal memory but currently
be in "send all accesses to my read/write functions" mode.

> + *
> + * Similarly, CPU_BP breakpoints would raise exceptions, and so
> + * return (UNKNOWN, FAULT).  For simplicity, we consider gdb and
> + * architectural breakpoints the same.
>   */

thanks
-- PMM

Re: [PATCH 12/16] target/arm: Use SVEContLdSt for contiguous stores

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Follow the model set up for contiguous loads.  This handles
> watchpoints correctly for contiguous stores, recognizing the
> exception before any changes to memory.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve_helper.c | 288 ++--
>  1 file changed, 162 insertions(+), 126 deletions(-)
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 15/16] target/arm: Reuse sve_probe_page for gather loads

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve_helper.c | 208 +---
>  1 file changed, 109 insertions(+), 99 deletions(-)
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 13/16] target/arm: Reuse sve_probe_page for gather first-fault loads

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> This avoids the need for a separate set of helpers to implement
> no-fault semantics, and will enable MTE in the future.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 14/16] target/arm: Reuse sve_probe_page for scatter stores

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve_helper.c | 182 
>  1 file changed, 111 insertions(+), 71 deletions(-)
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 16/16] target/arm: Remove sve_memopidx

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> None of the sve helpers use TCGMemOpIdx any longer, so we can
> stop passing it.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH 00/16] target/arm: sve load/store improvements

[Peter Maydell]

On Wed, 11 Mar 2020 at 06:44, Richard Henderson
 wrote:
>
> The goal here is to support MTE, but there's some cleanup to do.
>
> Technically, we have sufficient interfaces in cputlb.c now, but it
> requires multiple tlb lookups on different interfaces to do so.
>
> Adding probe_access_flags() allows probing the tlb and getting out
> some of the flags buried in the tlb comparator, such as TLB_MMIO
> and TLB_WATCHPOINT.  In addition, we get no-fault semantics,
> which we don't have via probe_acccess().
>
> Adding cpu_probe_watchpoint() allows to *not* stop a first-fault
> or no-fault load when the page contains a watchpoint, but the actual
> access does not hit.
>
> Having these available means that we can handle all of the watchpoints
> for a given set of loads/stores all at once, before we begin doing any
> actual memory operations.  Further, the actual memory operation on a
> page of ram that has a watchpoint can still use the fast path.
>
> Looking forward to MTE, we can examine the Tagged bit on a per-page
> basis and avoid dozens of mte_check calls that must be Unchecked.
> That comes later, in a new version of the MTE patch set, but I do
> add comments for where the checks should be added.

Series reviewed; I didn't bother to flag up the checkpatch
complaints, but I think all but one of them are legit, so
please fix those too.

thanks
-- PMM

Re: [PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature

[Michael S. Tsirkin]

On Thu, Apr 16, 2020 at 10:18:49AM +0200, David Hildenbrand wrote:
> >>
> >> Postcopy is a very good point, bought!
> >>
> >> But (what you wrote above) it sounds like that this is really what we 
> >> *have to* do, not an optimization. I‘ll double check the spec tomorrow 
> >> (hopefully it was documented). We should rephrase the comment then.
> > 
> > Do you have a link to the spec that I could look at? I am not hopeful
> > that this will be listed there since the poison side of QEMU was never
> > implemented. The flag is only here because it was copied over in the
> > kernel header.
> 
> Introducing a feature without
> 
> a) specification what it does
> b) implementation (of both sides) showing what has to be done
> c) any kind of documentation of what needs to be done
> 
> just horrible.
> 
> The latest-greatest spec lives in
> 
> https://github.com/oasis-tcs/virtio-spec.git
> 
> I can't spot any signs of free page hinting/page poisioning. :(

Right. I merged the hinting support in Linux on the hope that
spec and qemu upstream will surface later. It seemed so
since IIRC there were some drafts (even though I don't
have any links to hand). Unfortunately neither happened.



> We should document our result of page poisoning, free page hinting, and
> free page reporting there as well. I hope you'll have time for the latter.
> 
> -
> Semantics of VIRTIO_BALLOON_F_PAGE_POISON
> -
> 
> "The VIRTIO_BALLOON_F_PAGE_POISON feature bit is used to indicate if the
> guest is using page poisoning. Guest writes to the poison_val config
> field to tell host about the page poisoning value that is in use."
> -> Very little information, no signs about what has to be done.

I think it's an informational field. Knowing that free pages
are full of a specific pattern can be handy for the hypervisor
for a variety of reasons. E.g. compression/deduplication?


> "Let the hypervisor know that we are expecting a specific value to be
> written back in balloon pages."



> -> Okay, that talks about "balloon pages", which would include right now
> -- pages "inflated" and then "deflated" using free page hinting
> -- pages "inflated" and then "deflated" using oridnary inflate/deflate
>queue

ATM, in this case driver calls "free" and that fills page with the
poison value.

> -- pages "inflated" using free page reporting and automatically
>"deflated" on access
> 
> So VIRTIO_BALLOON_F_PAGE_POISON really means "whenever the guest
> deflates a page (either explicitly, or implicitly with free page
> reporting), it is filled with "poison_val".

It might be a valid optimization to allow driver to skip
poisoning of freed pages in this case.

> And I would add
> 
> "However, if the inflated page was not filled with "poison_val" when
> inflating, it's not predictable if the original page or a page filled
> with "poison_val" is returned."
> 
> Which would cover the "we did not discard the page in the hypervisor, so
> the original page is still there".
> 
> 
> We should also document what is expected to happen if "poison_val" is
> suddenly changed by the guest at one point in time again. (e.g., not
> supported, unexpected things can happen, etc.)

Right. I think we should require that this can only be changed
before features have been negotiated.
That is the only point where hypervisor can still fail
gracefully (i.e. fail FEATURES_OK).


> Also, we might have to
> document that a device reset resets the poison_val to 0. (not sure if
> that's really necessary)

Probably yes, for things like kexec.

> -
> What we have to do in the guest/Linux:
> -
> 
> A guest which relies on this (esp., free page reporting in Linux only,
> right?), has to not use/advertise VIRTIO_BALLOON_F_REPORTING *in case
> VIRTIO_BALLOON_F_PAGE_POISON is not provided* by the host. AFAIKS,
> ordinary inflation/deflation and free page hinting does currently not
> care, as we go via free_page(), so that is currently fine AFAIKs.
> 
> -
> What we have to do in the hypervisor/QEMU:
> -
> 
> With VIRTIO_BALLOON_F_PAGE_POISON, we could provide free page reporting
> easily IFF "page_val"==0. However, as you said, it will currently be
> expensive in case of postcopy, as the guest still zeroes out all pages.
> Document that properly.
> 
> With VIRTIO_BALLOON_F_PAGE_POISON, don't inflate any pages right now
> (not discarding anything), OR fill the pages with poison_val when
> deflating. I guess the latter would be better - even if current Linux
> does not need it, it's what we are expected to do AFAIKS.
> 
> With VIRTIO_BALLOON_F_PAGE_POISON and page_val !=

[PATCH RFC v4] target/arm: Implement SVE2 HISTCNT, HISTSEG

[Stephen Long]

Signed-off-by: Stephen Long 
---
Fix error in the helper function for HISTSEG

 target/arm/helper-sve.h|  7 +++
 target/arm/sve.decode  |  6 +++
 target/arm/sve_helper.c| 94 ++
 target/arm/translate-sve.c | 29 
 4 files changed, 136 insertions(+)

diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index 4733614614..958ad623f6 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2526,6 +2526,13 @@ DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_b, TCG_CALL_NO_RWG,
 DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_h, TCG_CALL_NO_RWG,
i32, ptr, ptr, ptr, ptr, i32)
 
+DEF_HELPER_FLAGS_5(sve2_histcnt_s, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_histcnt_d, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(sve2_histseg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_h, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_s, TCG_CALL_NO_RWG,
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 26690d4208..9dd20eb6ec 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -147,6 +147,7 @@
 &rprrr_esz rn=%reg_movprfx
 @rdn_pg_rm_ra    esz:2 . ra:5  ... pg:3 rm:5 rd:5 \
 &rprrr_esz rn=%reg_movprfx
+@rd_pg_rn_rm    esz:2 . rm:5 ... pg:3 rn:5 rd:5   &rprr_esz
 
 # One register operand, with governing predicate, vector element size
 @rd_pg_rn    esz:2 ... ... ... pg:3 rn:5 rd:5   &rpr_esz
@@ -1325,6 +1326,11 @@ UQRSHRNT01000101 .. 1 . 00  . .  
@rd_rn_tszimm_shr
 MATCH   01000101 .. 1 . 100 ... . 0  @pd_pg_rn_rm
 NMATCH  01000101 .. 1 . 100 ... . 1  @pd_pg_rn_rm
 
+### SVE2 Histogram Computation
+
+HISTCNT 01000101 .. 1 . 110 ... . .  @rd_pg_rn_rm
+HISTSEG 01000101 .. 1 . 101 000 . .  @rd_rn_rm
+
 ## SVE2 floating-point pairwise operations
 
 FADDP   01100100 .. 010 00 0 100 ... . . @rdn_pg_rm
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 7c65009bb8..793ce581a8 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7016,3 +7016,97 @@ DO_PPZZ_MATCH(sve2_nmatch_ppzz_b, MO_8, true)
 DO_PPZZ_MATCH(sve2_nmatch_ppzz_h, MO_16, true)
 
 #undef DO_PPZZ_MATCH
+
+void HELPER(sve2_histcnt_s)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+uint32_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; i += 4) {
+uint64_t count = 0;
+uint8_t pred = pg[H1(i >> 3)] >> (i & 7);
+if (pred & 1) {
+uint32_t nn = n[H4(i >> 2)];
+for (j = 0; j <= i; j += 4) {
+uint8_t pred = pg[H1(j >> 3)] >> (j & 7);
+if (pred & 1 && nn == m[H4(j >> 2)]) {
+++count;
+}
+}
+}
+d[H4(i >> 2)] = count;
+}
+}
+
+void HELPER(sve2_histcnt_d)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc) / 8;
+uint64_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; ++i) {
+uint64_t count = 0;
+if (pg[H1(i)] & 1) {
+uint64_t nn = n[i];
+for (j = 0; j <= i; ++j) {
+if (pg[H1(j)] & 1 && nn == m[j]) {
+++count;
+}
+}
+}
+d[i] = count;
+}
+}
+
+/*
+ * Returns the number of bytes in m0 and m1 that match n.
+ * See comment for do_match2().
+ * */
+static inline uint8_t do_histseg_cnt(uint8_t n, uint64_t m0, uint64_t m1)
+{
+int esz = 0;
+int bits = 8 << esz;
+uint64_t ones = dup_const(esz, 1);
+uint64_t signs = ones << (bits - 1);
+uint64_t cmp0, cmp1;
+
+cmp1 = dup_const(1, n);
+cmp0 = cmp1 ^ m0;
+cmp1 = cmp1 ^ m1;
+cmp0 = (cmp0 - ones) & ~cmp0;
+cmp1 = (cmp1 - ones) & ~cmp1;
+return ctpop64((cmp0 | cmp1) & signs);
+}
+
+void HELPER(sve2_histseg)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+
+for (i = 0; i < opr_sz; i += 16) {
+uint64_t n0 = *(uint64_t *)(vn + i);
+uint64_t n1 = *(uint64_t *)(vn + i + 8);
+
+uint64_t m0 = *(uint64_t *)(vm + i);
+uint64_t m1 = *(uint64_t *)(vm + i + 8);
+
+uint64_t out0 = 0;
+uint64_t out1 = 0;
+
+for (j = 0; j < 64; j += 8) {
+uint8_t count0 = do_histseg_cnt(n0 >> j, m0, m1);
+out0 |= count0 << j;
+
+uint8_t count1 = do_histseg_cnt(n1 >> j, m0, m1);
+out1 |= count1 << j;
+}
+
+*(uint64_t 

[PATCH 0/3] qemu-img: Add convert --bitmaps

[Eric Blake]

Without this series, the process for copying one qcow2 image to
another including all of its bitmaps involves running qemu and doing
the copying by hand with a series of QMP commands.  This makes the
process a bit more convenient.

I still think that someday we will need a 'qemu-img bitmap' with
various subcommands for manipulating bitmaps within an offline image,
but in the meantime, this seems like a useful addition on its own.

Series can also be downloaded at:
https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/qemu-img-bitmaps-v1

Eric Blake (3):
  blockdev: Split off basic bitmap operations for qemu-img
  qemu-img: Add convert --bitmaps option
  iotests: Add test 291 to for qemu-img convert --bitmaps

 docs/tools/qemu-img.rst|   6 +-
 Makefile.objs  |   2 +-
 include/sysemu/blockdev.h  |  10 ++
 blockbitmaps.c | 217 +
 blockdev.c | 184 ---
 qemu-img.c |  81 +-
 MAINTAINERS|   1 +
 qemu-img-cmds.hx   |   4 +-
 tests/qemu-iotests/291 | 143 
 tests/qemu-iotests/291.out |  56 ++
 tests/qemu-iotests/group   |   1 +
 11 files changed, 514 insertions(+), 191 deletions(-)
 create mode 100644 blockbitmaps.c
 create mode 100755 tests/qemu-iotests/291
 create mode 100644 tests/qemu-iotests/291.out

-- 
2.26.0

[PATCH 1/3] blockdev: Split off basic bitmap operations for qemu-img

[Eric Blake]

The next patch wants to teach qemu how to copy a bitmap from one qcow2
file to another.  But blockdev.o is too heavyweight to link into
qemu-img, so it's time to split off the bare bones of what we will
need into a new file blockbitmaps.o.  Transactions are not needed in
qemu-img (if things fail while creating the new image, the fix is to
delete the botched copy, rather than worrying about atomic rollback).

For now, I stuck to just the minimum code motion (add and merge); we
could instead decide to move everything bitmap-related that does not
also pull in transactions (delete, enable, disable).

Signed-off-by: Eric Blake 
---
 Makefile.objs |   2 +-
 include/sysemu/blockdev.h |  10 ++
 blockbitmaps.c| 217 ++
 blockdev.c| 184 
 MAINTAINERS   |   1 +
 5 files changed, 229 insertions(+), 185 deletions(-)
 create mode 100644 blockbitmaps.c

diff --git a/Makefile.objs b/Makefile.objs
index a7c967633acf..44e30fa9a6e3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ chardev-obj-y = chardev/
 authz-obj-y = authz/

 block-obj-y = nbd/
-block-obj-y += block.o blockjob.o job.o
+block-obj-y += block.o blockbitmaps.o blockjob.o job.o
 block-obj-y += block/ scsi/
 block-obj-y += qemu-io-cmds.o
 block-obj-$(CONFIG_REPLICATION) += replication.o
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index a86d99b3d875..95cfeb29bc0a 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -57,4 +57,14 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, 
const char *file,
 DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type,
  Error **errp);

+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+   const char *name,
+   BlockDriverState **pbs,
+   Error **errp);
+BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
+ const char *target,
+ BlockDirtyBitmapMergeSourceList 
*bitmaps,
+ HBitmap **backup, Error **errp);
+
+
 #endif
diff --git a/blockbitmaps.c b/blockbitmaps.c
new file mode 100644
index ..0d334d82006d
--- /dev/null
+++ b/blockbitmaps.c
@@ -0,0 +1,217 @@
+/*
+ * QEMU host block device bitmaps
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ *
+ * This file incorporates work covered by the following copyright and
+ * permission notice:
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "sysemu/blockdev.h"
+#include "block/block.h"
+#include "qapi/qapi-commands-block.h"
+#include "qapi/error.h"
+
+/**
+ * block_dirty_bitmap_lookup:
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names.
+ *
+ * @node: The name of the BDS node to search for bitmaps
+ * @name: The name of the bitmap to search for
+ * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
+ * @errp: Output pointer for error information. Can be NULL.
+ *
+ * @return: A bitmap object on success, or NULL on failure.
+ */
+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+   const char *name,
+   BlockDriverState **pbs,
+   Error **errp)
+{
+BlockDriverState *bs;
+BdrvDirtyBitmap *bitmap;
+
+if (!node) {
+error_setg(errp, "Node cannot be NULL");
+return NULL;
+}
+if (!name) {
+error_setg(errp, "Bitma

[PATCH 3/3] iotests: Add test 291 to for qemu-img convert --bitmaps

[Eric Blake]

Add a new test covering the feature added in the previous patch.

Signed-off-by: Eric Blake 
---
 tests/qemu-iotests/291 | 143 +
 tests/qemu-iotests/291.out |  56 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 200 insertions(+)
 create mode 100755 tests/qemu-iotests/291
 create mode 100644 tests/qemu-iotests/291.out

diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
new file mode 100755
index ..dfdcc8e352c8
--- /dev/null
+++ b/tests/qemu-iotests/291
@@ -0,0 +1,143 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img convert --bitmaps
+#
+# Copyright (C) 2018-2020 Red Hat, Inc.
+#
+# 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 .
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+nbd_server_stop
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.nbd
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+_require_command QEMU_NBD
+
+do_run_qemu()
+{
+echo Testing: "$@"
+$QEMU -nographic -qmp stdio -serial none "$@"
+echo
+}
+
+run_qemu()
+{
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+  | _filter_qemu | _filter_imgfmt \
+  | _filter_actual_image_size | _filter_qemu_io
+}
+
+# Create initial image and populate two bitmaps: one active, one inactive
+_make_test_img 10M
+run_qemu <

[PATCH 2/3] qemu-img: Add convert --bitmaps option

[Eric Blake]

Make it easier to copy all the persistent bitmaps of a source image
along with the contents, by adding a boolean flag for use with
qemu-img convert.

See also https://bugzilla.redhat.com/show_bug.cgi?id=1779893

Signed-off-by: Eric Blake 
---
 docs/tools/qemu-img.rst |  6 ++-
 qemu-img.c  | 81 +++--
 qemu-img-cmds.hx|  4 +-
 3 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76c9..8c4d85e0b835 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -186,6 +186,10 @@ Parameters to convert subcommand:

 .. program:: qemu-img-convert

+.. option:: --bitmaps
+
+  Additionally copy all bitmaps
+
 .. option:: -n

   Skip the creation of the target volume
@@ -373,7 +377,7 @@ Command description:
   4
 Error on reading data

-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] 
[--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T 
SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] 
[-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] 
OUTPUT_FILENAME
+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] 
[--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t 
CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l 
SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 
[...]] OUTPUT_FILENAME

   Convert the disk image *FILENAME* or a snapshot *SNAPSHOT_PARAM*
   to disk image *OUTPUT_FILENAME* using format *OUTPUT_FMT*. It can
diff --git a/qemu-img.c b/qemu-img.c
index 821cbf610e5f..6541357179c2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -28,6 +28,7 @@
 #include "qemu-common.h"
 #include "qemu-version.h"
 #include "qapi/error.h"
+#include "qapi/qapi-commands-block-core.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/qjson.h"
@@ -71,6 +72,7 @@ enum {
 OPTION_SHRINK = 266,
 OPTION_SALVAGE = 267,
 OPTION_TARGET_IS_ZERO = 268,
+OPTION_BITMAPS = 269,
 };

 typedef enum OutputFormat {
@@ -176,6 +178,7 @@ static void QEMU_NORETURN help(void)
"   hiding corruption that has already occurred.\n"
"\n"
"Parameters to convert subcommand:\n"
+   "  '--bitmaps' copies all persistent bitmaps to destination\n"
"  '-m' specifies how many coroutines work in parallel during the 
convert\n"
"   process (defaults to 8)\n"
"  '-W' allow to write to the target out of order rather than 
sequential\n"
@@ -2054,6 +2057,47 @@ static int convert_do_copy(ImgConvertState *s)
 return s->ret;
 }

+static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)
+{
+BdrvDirtyBitmap *bm;
+Error *err = NULL;
+BlockDirtyBitmapMergeSource *merge;
+BlockDirtyBitmapMergeSourceList *list;
+
+FOR_EACH_DIRTY_BITMAP(src, bm) {
+const char *name;
+
+if (!bdrv_dirty_bitmap_get_persistence(bm)) {
+continue;
+}
+name = bdrv_dirty_bitmap_name(bm);
+qmp_block_dirty_bitmap_add(dst->node_name, name,
+   true, bdrv_dirty_bitmap_granularity(bm),
+   true, true,
+   true, !bdrv_dirty_bitmap_enabled(bm),
+   &err);
+if (err) {
+error_reportf_err(err, "Failed to create bitmap %s: ", name);
+return -1;
+}
+
+merge = g_new0(BlockDirtyBitmapMergeSource, 1);
+merge->type = QTYPE_QDICT;
+merge->u.external.node = g_strdup(src->node_name);
+merge->u.external.name = g_strdup(name);
+list = g_new0(BlockDirtyBitmapMergeSourceList, 1);
+list->value = merge;
+qmp_block_dirty_bitmap_merge(dst->node_name, name, list, &err);
+qapi_free_BlockDirtyBitmapMergeSourceList(list);
+if (err) {
+error_reportf_err(err, "Failed to populate bitmap %s: ", name);
+return -1;
+}
+}
+
+return 0;
+}
+
 #define MAX_BUF_SECTORS 32768

 static int img_convert(int argc, char **argv)
@@ -2075,6 +2119,8 @@ static int img_convert(int argc, char **argv)
 int64_t ret = -EINVAL;
 bool force_share = false;
 bool explict_min_sparse = false;
+bool bitmaps = false;
+size_t nbitmaps = 0;

 ImgConvertState s = (ImgConvertState) {
 /* Need at least 4k of zeros for sparse detection */
@@ -2094,6 +2140,7 @@ static int img_convert(int argc, char **argv)
 {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
 {"salvage", no_argument, 0, OPTION_SALVAGE},
 {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
+{"bitmaps", no_argument, 0, OPTION_BITMAPS},
 {0, 0, 0, 0}
 

[Qemu devel PATCH v6 2/3] msf2: Add EMAC block to SmartFusion2 SoC

[sundeep . lkml]

From: Subbaraya Sundeep 

With SmartFusion2 Ethernet MAC model in
place this patch adds the same to SoC.

Signed-off-by: Subbaraya Sundeep 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
---
 hw/arm/msf2-soc.c | 26 --
 include/hw/arm/msf2-soc.h |  2 ++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index 588d643..a455b88 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -1,7 +1,7 @@
 /*
  * SmartFusion2 SoC emulation.
  *
- * Copyright (c) 2017 Subbaraya Sundeep 
+ * Copyright (c) 2017-2020 Subbaraya Sundeep 
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
@@ -35,11 +35,14 @@
 
 #define MSF2_TIMER_BASE   0x40004000
 #define MSF2_SYSREG_BASE  0x40038000
+#define MSF2_EMAC_BASE0x40041000
 
 #define ENVM_BASE_ADDRESS 0x6000
 
 #define SRAM_BASE_ADDRESS 0x2000
 
+#define MSF2_EMAC_IRQ 12
+
 #define MSF2_ENVM_MAX_SIZE(512 * KiB)
 
 /*
@@ -81,6 +84,13 @@ static void m2sxxx_soc_initfn(Object *obj)
 sysbus_init_child_obj(obj, "spi[*]", &s->spi[i], sizeof(s->spi[i]),
   TYPE_MSS_SPI);
 }
+
+sysbus_init_child_obj(obj, "emac", &s->emac, sizeof(s->emac),
+  TYPE_MSS_EMAC);
+if (nd_table[0].used) {
+qemu_check_nic_model(&nd_table[0], TYPE_MSS_EMAC);
+qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
+}
 }
 
 static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -192,6 +202,19 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error 
**errp)
 g_free(bus_name);
 }
 
+dev = DEVICE(&s->emac);
+object_property_set_link(OBJECT(&s->emac), OBJECT(get_system_memory()),
+ "ahb-bus", &error_abort);
+object_property_set_bool(OBJECT(&s->emac), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, MSF2_EMAC_BASE);
+sysbus_connect_irq(busdev, 0,
+   qdev_get_gpio_in(armv7m, MSF2_EMAC_IRQ));
+
 /* Below devices are not modelled yet. */
 create_unimplemented_device("i2c_0", 0x40002000, 0x1000);
 create_unimplemented_device("dma", 0x40003000, 0x1000);
@@ -202,7 +225,6 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error 
**errp)
 create_unimplemented_device("can", 0x40015000, 0x1000);
 create_unimplemented_device("rtc", 0x40017000, 0x1000);
 create_unimplemented_device("apb_config", 0x4002, 0x1);
-create_unimplemented_device("emac", 0x40041000, 0x1000);
 create_unimplemented_device("usb", 0x40043000, 0x1000);
 }
 
diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
index 3cfe5c7..c9cb214 100644
--- a/include/hw/arm/msf2-soc.h
+++ b/include/hw/arm/msf2-soc.h
@@ -29,6 +29,7 @@
 #include "hw/timer/mss-timer.h"
 #include "hw/misc/msf2-sysreg.h"
 #include "hw/ssi/mss-spi.h"
+#include "hw/net/msf2-emac.h"
 
 #define TYPE_MSF2_SOC "msf2-soc"
 #define MSF2_SOC(obj) OBJECT_CHECK(MSF2State, (obj), TYPE_MSF2_SOC)
@@ -62,6 +63,7 @@ typedef struct MSF2State {
 MSF2SysregState sysreg;
 MSSTimerState timer;
 MSSSpiState spi[MSF2_NUM_SPIS];
+MSF2EmacState emac;
 } MSF2State;
 
 #endif
-- 
2.7.4

[Qemu devel PATCH v6 0/3] Add SmartFusion2 EMAC block

[sundeep . lkml]

From: Subbaraya Sundeep 

This patch set emulates Ethernet MAC block
present in Microsemi SmartFusion2 SoC.

v6:
 Fixed destination address matching logic
 Added missing break in emac_write
v5:
 As per Philippe comments:
Returned size in receive function
Added link property to pass DMA memory
Used FIELD() APIs
Added mac_addr in emac state
Used FIELD_EX32 and FIELD_DP32 APIs
Simplified if else logics in emac_write/read
v4:
  Added loop back as per Jason's comment 
v3:
  Added SmartFusion2 ethernet test to tests/acceptance
v2:
  No changes. Fixed Signed-off mail id in patch 2/2

Testing:

1. Download u-boot.bin, uImage and msf2-devkit.dtb from
   https://github.com/Subbaraya-Sundeep/qemu-test-binaries.git
2. Copy uImage and msf2-devkit.dtb to suitable Qemu tftp directory
3. Launch Qemu by
   ./arm-softmmu/qemu-system-arm -M emcraft-sf2 -serial mon:stdio -kernel \
   u-boot.bin -display none -nic user,tftp=

Example:
./arm-softmmu/qemu-system-arm -M emcraft-sf2 -serial mon:stdio -kernel u-boot 
-display none -nic user,tftp=/home/hyd1358/qemu_tftp

U-Boot 2010.03-0-ga7695d6 (Apr 04 2020 - 15:07:27)

CPU  : SmartFusion2 SoC (Cortex-M3 Hard IP)
Freqs: CORTEX-M3=142MHz,PCLK0=71MHz,PCLK1=71MHz
Board: M2S-FG484-SOM Rev 1A, www.emcraft.com
DRAM:  64 MB
*** Warning - bad CRC, using default environment

In:serial
Out:   serial
Err:   serial
Net:   M2S_MAC

Hit any key to stop autoboot:  3  0 

M2S-FG484-SOM> run netboot
Using M2S_MAC device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename 'uImage'.
Load address: 0xa0007fc0
Loading: *#
 #
 #
 ###
done
Bytes transferred = 3681568 (382d20 hex)
Using M2S_MAC device
TFTP from server 10.0.2.2; our IP address is 10.0.2.15
Filename 'msf2-devkit.dtb'.
Load address: 0xa200
Loading: *#
done
Bytes transferred = 1712 (6b0 hex)
## Booting kernel from Legacy Image at a0007fc0 ...
   Image Name:   
   Image Type:   ARM Linux Kernel Image (uncompressed)
   Data Size:3681504 Bytes =  3.5 MB
   Load Address: a0008000
   Entry Point:  a0008001
   Verifying Checksum ... OK
   Loading Kernel Image ... OK
OK

Starting kernel ...

[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 4.5.0-gb0e5502-dirty (hyd1358@hyd1358) (gcc 
version 4.4.1 (Sourcery G++ Lite 2010q1-189) ) #85 PREEMPT Sat Apr 4 23:26:40 
IST 2020
[0.00] CPU: ARMv7-M [410fc231] revision 1 (ARMv7M), cr=
[0.00] CPU: unknown data cache, unknown instruction cache
[0.00] Machine model: Microsemi SmartFusion 2 development board
[0.00] bootconsole [earlycon0] enabled
[0.00] Built 1 zonelists in Zone order, mobility grouping on.  Total 
pages: 16256
[0.00] Kernel command line: console=ttyS0,115200n8 panic=10 
mem=64M@0xa000 earlyprintk
[0.00] PID hash table entries: 256 (order: -2, 1024 bytes)
[0.00] Dentry cache hash table entries: 8192 (order: 3, 32768 bytes)
[0.00] Inode-cache hash table entries: 4096 (order: 2, 16384 bytes)
[0.00] Memory: 61212K/65536K available (1612K kernel code, 75K rwdata, 
680K rodata, 1224K init, 120K bss, 4324K reserved, 0K cma-reserved)
[0.00] Virtual kernel memory layout:
[0.00] vector  : 0x - 0x1000   (   4 kB)
[0.00] fixmap  : 0xffc0 - 0xfff0   (3072 kB)
[0.00] vmalloc : 0x - 0x   (4095 MB)
[0.00] lowmem  : 0xa000 - 0xa400   (  64 MB)
[0.00] modules : 0xa000 - 0xa080   (   8 MB)
[0.00]   .text : 0xa0008000 - 0xa02453e8   (2293 kB)
[0.00]   .init : 0xa0246000 - 0xa0378000   (1224 kB)
[0.00]   .data : 0xa0378000 - 0xa038ace0   (  76 kB)
[0.00].bss : 0xa038ace0 - 0xa03a8ea0   ( 121 kB)
[0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[0.00] Preemptible hierarchical RCU implementation.
.
.
.
[0.445184] Found M2S MAC at 0x40041000, irq 18
[0.448810] libphy: msf2 MII bus: probed
[0.527047] ipip: IPv4 over IPv4 tunneling driver
[0.532367] NET: Registered protocol family 10
[0.542307] sit: IPv6 over IPv4 tunneling driver
[0.544655] NET: Registered protocol family 17
[0.565395] Freeing unused kernel memory: 1224K (a0246000 - a0378000)
init started: BusyBox v1.31.1 (2020-01-25 20:01:06 IST)
starting pid 26, tty '': '/etc/rc'
starting pid 31, tty '/dev/ttyS0': '/bin/hush -i'


BusyBox v1.31.1 (2020-01-25 20:01:06 IST) hush - the humble shell
Enter 'help' for a list of built-in commands.

/ # ifconfig eth0 10.0.2.15
[   11.116091] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
/ # [   11.653634] eth0: link up (100/full)
[   11.655246] IPv6: ADDRCONF(NETDEV_C

[Qemu devel PATCH v6 3/3] tests/boot_linux_console: Add ethernet test to SmartFusion2

[sundeep . lkml]

From: Subbaraya Sundeep 

In addition to simple serial test this patch uses ping
to test the ethernet block modelled in SmartFusion2 SoC.

Signed-off-by: Subbaraya Sundeep 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/boot_linux_console.py | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index f825cd9..c6b06a1 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -336,13 +336,13 @@ class BootLinuxConsole(Test):
 """
 uboot_url = ('https://raw.githubusercontent.com/'
  'Subbaraya-Sundeep/qemu-test-binaries/'
- 'fa030bd77a014a0b8e360d3b7011df89283a2f0b/u-boot')
-uboot_hash = 'abba5d9c24cdd2d49cdc2a8aa92976cf20737eff'
+ 'fe371d32e50ca682391e1e70ab98c2942aeffb01/u-boot')
+uboot_hash = 'cbb8cbab970f594bf6523b9855be209c08374ae2'
 uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
 spi_url = ('https://raw.githubusercontent.com/'
'Subbaraya-Sundeep/qemu-test-binaries/'
-   'fa030bd77a014a0b8e360d3b7011df89283a2f0b/spi.bin')
-spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
+   'fe371d32e50ca682391e1e70ab98c2942aeffb01/spi.bin')
+spi_hash = '65523a1835949b6f4553be96dec1b6a38fb05501'
 spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
 
 self.vm.set_console()
@@ -352,7 +352,12 @@ class BootLinuxConsole(Test):
  '-drive', 'file=' + spi_path + ',if=mtd,format=raw',
  '-no-reboot')
 self.vm.launch()
-self.wait_for_console_pattern('init started: BusyBox')
+self.wait_for_console_pattern('Enter \'help\' for a list')
+
+exec_command_and_wait_for_pattern(self, 'ifconfig eth0 10.0.2.15',
+ 'eth0: link becomes ready')
+exec_command_and_wait_for_pattern(self, 'ping -c 3 10.0.2.2',
+'3 packets transmitted, 3 packets received, 0% packet loss')
 
 def do_test_arm_raspi2(self, uart_id):
 """
-- 
2.7.4

[Qemu devel PATCH v6 1/3] hw/net: Add Smartfusion2 emac block

[sundeep . lkml]

From: Subbaraya Sundeep 

Modelled Ethernet MAC of Smartfusion2 SoC.
Micrel KSZ8051 PHY is present on Emcraft's
SOM kit hence same PHY is emulated.

Signed-off-by: Subbaraya Sundeep 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS|   2 +
 hw/net/Makefile.objs   |   1 +
 hw/net/msf2-emac.c | 589 +
 include/hw/net/msf2-emac.h |  53 
 4 files changed, 645 insertions(+)
 create mode 100644 hw/net/msf2-emac.c
 create mode 100644 include/hw/net/msf2-emac.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8cbc1fa..cea5733 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -919,6 +919,8 @@ F: include/hw/arm/msf2-soc.h
 F: include/hw/misc/msf2-sysreg.h
 F: include/hw/timer/mss-timer.h
 F: include/hw/ssi/mss-spi.h
+F: hw/net/msf2-emac.c
+F: include/hw/net/msf2-emac.h
 
 Emcraft M2S-FG484
 M: Subbaraya Sundeep 
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
index af4d194..f2b7398 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -55,3 +55,4 @@ common-obj-$(CONFIG_ROCKER) += rocker/rocker.o 
rocker/rocker_fp.o \
 obj-$(call lnot,$(CONFIG_ROCKER)) += rocker/qmp-norocker.o
 
 common-obj-$(CONFIG_CAN_BUS) += can/
+common-obj-$(CONFIG_MSF2) += msf2-emac.o
diff --git a/hw/net/msf2-emac.c b/hw/net/msf2-emac.c
new file mode 100644
index 000..32ba9e8
--- /dev/null
+++ b/hw/net/msf2-emac.c
@@ -0,0 +1,589 @@
+/*
+ * QEMU model of the Smartfusion2 Ethernet MAC.
+ *
+ * Copyright (c) 2020 Subbaraya Sundeep .
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Refer to section Ethernet MAC in the document:
+ * UG0331: SmartFusion2 Microcontroller Subsystem User Guide
+ * Datasheet URL:
+ * https://www.microsemi.com/document-portal/cat_view/56661-internal-documents/
+ * 56758-soc?lang=en&limit=20&limitstart=220
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+#include "hw/registerfields.h"
+#include "hw/net/msf2-emac.h"
+#include "hw/net/mii.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+
+REG32(CFG1, 0x0)
+FIELD(CFG1, RESET, 31, 1)
+FIELD(CFG1, RX_EN, 2, 1)
+FIELD(CFG1, TX_EN, 0, 1)
+FIELD(CFG1, LB_EN, 8, 1)
+REG32(CFG2, 0x4)
+REG32(IFG, 0x8)
+REG32(HALF_DUPLEX, 0xc)
+REG32(MAX_FRAME_LENGTH, 0x10)
+REG32(MII_CMD, 0x24)
+FIELD(MII_CMD, READ, 0, 1)
+REG32(MII_ADDR, 0x28)
+FIELD(MII_ADDR, REGADDR, 0, 5)
+FIELD(MII_ADDR, PHYADDR, 8, 5)
+REG32(MII_CTL, 0x2c)
+REG32(MII_STS, 0x30)
+REG32(STA1, 0x40)
+REG32(STA2, 0x44)
+REG32(FIFO_CFG0, 0x48)
+REG32(FIFO_CFG4, 0x58)
+FIELD(FIFO_CFG4, BCAST, 9, 1)
+FIELD(FIFO_CFG4, MCAST, 8, 1)
+REG32(FIFO_CFG5, 0x5C)
+FIELD(FIFO_CFG5, BCAST, 9, 1)
+FIELD(FIFO_CFG5, MCAST, 8, 1)
+REG32(DMA_TX_CTL, 0x180)
+FIELD(DMA_TX_CTL, EN, 0, 1)
+REG32(DMA_TX_DESC, 0x184)
+REG32(DMA_TX_STATUS, 0x188)
+FIELD(DMA_TX_STATUS, PKTCNT, 16, 8)
+FIELD(DMA_TX_STATUS, UNDERRUN, 1, 1)
+FIELD(DMA_TX_STATUS, PKT_SENT, 0, 1)
+REG32(DMA_RX_CTL, 0x18c)
+FIELD(DMA_RX_CTL, EN, 0, 1)
+REG32(DMA_RX_DESC, 0x190)
+REG32(DMA_RX_STATUS, 0x194)
+FIELD(DMA_RX_STATUS, PKTCNT, 16, 8)
+FIELD(DMA_RX_STATUS, OVERFLOW, 2, 1)
+FIELD(DMA_RX_STATUS, PKT_RCVD, 0, 1)
+REG32(DMA_IRQ_MASK, 0x198)
+REG32(DMA_IRQ, 0x19c)
+
+#define EMPTY_MASK  (1 << 31)
+#define PKT_SIZE0x7FF
+#define PHYADDR 0x1
+#define MAX_PKT_SIZE2048
+
+typedef struct {
+uint32_t pktaddr;
+uint32_t pktsize;
+uint32_t next;
+} EmacDesc;
+
+static uint32_t emac_get_isr(MSF2EmacState *s)
+{
+uint32_t ier = s->regs[R_DMA_IRQ_MASK];
+uint32_t tx = s->regs[R_DMA_TX_STATUS] & 0xF;
+uint32_t rx = s->regs[R_DMA_RX_STATUS] & 0xF;
+uint32_t isr = (rx << 4) | tx;
+
+s->regs[R_DMA_IRQ] = ier & isr;
+return s->regs[R_DMA_IRQ];
+}
+
+stati

Re: [PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature

[David Hildenbrand]

>> We should document our result of page poisoning, free page hinting, and
>> free page reporting there as well. I hope you'll have time for the latter.
>>
>> -
>> Semantics of VIRTIO_BALLOON_F_PAGE_POISON
>> -
>>
>> "The VIRTIO_BALLOON_F_PAGE_POISON feature bit is used to indicate if the
>> guest is using page poisoning. Guest writes to the poison_val config
>> field to tell host about the page poisoning value that is in use."
>> -> Very little information, no signs about what has to be done.
> 
> I think it's an informational field. Knowing that free pages
> are full of a specific pattern can be handy for the hypervisor
> for a variety of reasons. E.g. compression/deduplication?

I was referring to the documentation of the feature and what we
(hypervisor) are expected to do (in regards to inflation/deflation).

Yes, it might be valuable to know that the guest is using poisoning. I
assume compression/deduplication (IOW KSM) will figure out themselves
that such pages are equal.

>> "Let the hypervisor know that we are expecting a specific value to be
>> written back in balloon pages."
> 
> 
> 
>> -> Okay, that talks about "balloon pages", which would include right now
>> -- pages "inflated" and then "deflated" using free page hinting
>> -- pages "inflated" and then "deflated" using oridnary inflate/deflate
>>queue
> 
> ATM, in this case driver calls "free" and that fills page with the
> poison value.

Yes, that's what I mentioned somehwere, it's currently done by Linux and ...

> 
> It might be a valid optimization to allow driver to skip
> poisoning of freed pages in this case.

... we should prepare for that :)

> 
>> And I would add
>>
>> "However, if the inflated page was not filled with "poison_val" when
>> inflating, it's not predictable if the original page or a page filled
>> with "poison_val" is returned."
>>
>> Which would cover the "we did not discard the page in the hypervisor, so
>> the original page is still there".
>>
>>
>> We should also document what is expected to happen if "poison_val" is
>> suddenly changed by the guest at one point in time again. (e.g., not
>> supported, unexpected things can happen, etc.)
> 
> Right. I think we should require that this can only be changed
> before features have been negotiated.
> That is the only point where hypervisor can still fail
> gracefully (i.e. fail FEATURES_OK).

Agreed.

I can totally understand if Alex would want to stop working on
VIRTIO_BALLOON_F_PAGE_POISON at this point and only fix the guest to not
enable free page reporting in case we don't have
VIRTIO_BALLOON_F_PAGE_POISON (unless that's already done), lol. :)

-- 
Thanks,

David / dhildenb

Re: [PATCH] 9pfs: local: ignore O_NOATIME if we don't have permissions

[Christian Schoenebeck]

On Donnerstag, 16. April 2020 02:44:33 CEST Omar Sandoval wrote:
> From: Omar Sandoval 
> 
> QEMU's local 9pfs server passes through O_NOATIME from the client. If
> the QEMU process doesn't have permissions to use O_NOATIME (namely, it
> does not own the file nor have the CAP_FOWNER capability), the open will
> fail. This causes issues when from the client's point of view, it
> believes it has permissions to use O_NOATIME (e.g., a process running as
> root in the virtual machine). Additionally, overlayfs on Linux opens
> files on the lower layer using O_NOATIME, so in this case a 9pfs mount
> can't be used as a lower layer for overlayfs (cf.
> https://github.com/osandov/drgn/blob/dabfe1971951701da13863dbe6d8a1d172ad965
> 0/vmtest/onoatimehack.c and https://github.com/NixOS/nixpkgs/issues/54509).
> 
> Luckily, O_NOATIME is effectively a hint, and is often ignored by, e.g.,
> network filesystems. open(2) notes that O_NOATIME "may not be effective
> on all filesystems. One example is NFS, where the server maintains the
> access time." This means that we can honor it when possible but fall
> back to ignoring it.

I am not sure whether NFS would simply silently ignore O_NOATIME i.e. without 
returning EPERM. I don't read it that way. Fact is on Linux the expected 
behaviour is returning EPERM if O_NOATIME cannot be satisfied, consistent 
since its introduction 22 years ago:
http://lkml.iu.edu/hypermail/linux/kernel/9811.2/0118.html

> Signed-off-by: Omar Sandoval 
> ---
>  hw/9pfs/9p-util.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 79ed6b233e..50842d540f 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -37,9 +37,14 @@ static inline int openat_file(int dirfd, const char
> *name, int flags, {
>  int fd, serrno, ret;
> 
> +again:
>  fd = openat(dirfd, name, flags | O_NOFOLLOW | O_NOCTTY | O_NONBLOCK,
>  mode);
>  if (fd == -1) {
> +if (errno == EPERM && (flags & O_NOATIME)) {
> +flags &= ~O_NOATIME;
> +goto again;
> +}
>  return -1;
>  }

It would certainly fix the problem in your use case. But it would also unmask 
O_NOATIME for all other ones (i.e. regular users on guest).

I mean I understand your point, but I also have to take other use cases into 
account which might expect to receive EPERM if O_NOATIME cannot be granted.

May I ask how come that file/dir in question does not share the same uid in 
your specific use case? Are the file(s) created outside of QEMU, i.e. directly 
by some app on host?

Best regards,
Christian Schoenebeck

Re: [PATCH v2 2/8] hw/watchdog: Implement full i.MX watchdog support

[Peter Maydell]

On Sun, 22 Mar 2020 at 21:19, Guenter Roeck  wrote:
>
> Implement full support for the watchdog in i.MX systems.
> Pretimeout support is optional because the watchdog hardware on i.MX31
> does not support pretimeouts.
>
> Signed-off-by: Guenter Roeck 
> ---
> v2: Fixup of CONFIG_WDT_IMX -> CONFIG_WDT_IMX2 moved to patch 1/8

Sorry for not getting to this earlier, I've been focusing on
work for the 5.0 release. Some comments below, but overall
this looks pretty good.

>
>  hw/watchdog/wdt_imx2.c | 196 +++--
>  include/hw/watchdog/wdt_imx2.h |  49 -
>  2 files changed, 231 insertions(+), 14 deletions(-)
>
> diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c
> index ad1ef02e9e..f5339f3590 100644
> --- a/hw/watchdog/wdt_imx2.c
> +++ b/hw/watchdog/wdt_imx2.c
> @@ -13,24 +13,157 @@
>  #include "qemu/bitops.h"
>  #include "qemu/module.h"
>  #include "sysemu/watchdog.h"
> +#include "migration/vmstate.h"
> +#include "hw/qdev-properties.h"
>
>  #include "hw/watchdog/wdt_imx2.h"
>
> -#define IMX2_WDT_WCR_WDABIT(5)  /* -> External Reset WDOG_B */
> -#define IMX2_WDT_WCR_SRSBIT(4)  /* -> Software Reset Signal */
> +static void imx2_wdt_interrupt(void *opaque)
> +{
> +IMX2WdtState *s = IMX2_WDT(opaque);
> +
> +s->wicr |= IMX2_WDT_WICR_WTIS;
> +qemu_set_irq(s->irq, 1);
> +}
>
> -static uint64_t imx2_wdt_read(void *opaque, hwaddr addr,
> -  unsigned int size)
> +static void imx2_wdt_expired(void *opaque)
>  {
> +IMX2WdtState *s = IMX2_WDT(opaque);
> +
> +s->wrsr = IMX2_WDT_WRSR_TOUT;
> +
> +/* Perform watchdog action if watchdog is enabled */
> +if (s->wcr & IMX2_WDT_WCR_WDE) {
> +watchdog_perform_action();
> +}
> +}
> +
> +static void imx2_wdt_reset(DeviceState *dev)
> +{
> +IMX2WdtState *s = IMX2_WDT(dev);
> +
> +s->wcr = IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS;
> +s->wsr = 0;
> +s->wrsr &= ~(IMX2_WDT_WRSR_TOUT | IMX2_WDT_WRSR_SFTW);
> +s->wicr = 4;
> +s->wmcr = IMX2_WDT_WMCR_PDE;

Your reset function probably also needs to ptimer_stop()
the timers or otherwise put them into whatever is the
correct state for the device-as-reset.

> +}

> +
>  static void imx2_wdt_write(void *opaque, hwaddr addr,
> uint64_t value, unsigned int size)
>  {
> -if (addr == IMX2_WDT_WCR &&
> -(~value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS))) {
> -watchdog_perform_action();
> +IMX2WdtState *s = IMX2_WDT(opaque);
> +
> +switch (addr) {
> +case IMX2_WDT_WCR:
> +s->wcr = value;
> +if (!(value & IMX2_WDT_WCR_SRS)) {
> +s->wrsr = IMX2_WDT_WRSR_SFTW;
> +}
> +if (!(value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS)) ||
> +(!(value & IMX2_WDT_WCR_WT) && (value & IMX2_WDT_WCR_WDE))) {
> +watchdog_perform_action();
> +}
> +s->wcr |= IMX2_WDT_WCR_SRS;
> +imx_wdt2_update_timer(s, true);
> +break;
> +case IMX2_WDT_WSR:
> +if (s->wsr == IMX2_WDT_SEQ1 && value == IMX2_WDT_SEQ2) {
> +imx_wdt2_update_timer(s, false);
> +}
> +s->wsr = value;
> +break;
> +case IMX2_WDT_WRSR:
> +break;
> +case IMX2_WDT_WICR:
> +if (!s->pretimeout_support) {
> +return;
> +}
> +/* The pretimeout value is write-once */

My imx6 manual says that the WICR WIE bit is also write-once,
so I think that changes to it should also be guarded under
!pretimeout_locked, like the WICT bits.

(In fact quite a lot of registers seem to have write-once bits.)

> +if (s->pretimeout_locked) {
> +value &= ~IMX2_WDT_WICR_WICT;
> +s->wicr &= (IMX2_WDT_WICR_WTIS | IMX2_WDT_WICR_WICT);
> +} else {
> +s->wicr &= IMX2_WDT_WICR_WTIS;
> +}
> +s->wicr |= value & (IMX2_WDT_WICR_WIE | IMX2_WDT_WICR_WICT);
> +if (value & IMX2_WDT_WICR_WTIS) {
> +s->wicr &= ~IMX2_WDT_WICR_WTIS;
> +qemu_set_irq(s->irq, 0);
> +}
> +imx_wdt2_update_itimer(s, true);
> +s->pretimeout_locked = true;
> +break;
> +case IMX2_WDT_WMCR:
> +s->wmcr = value & IMX2_WDT_WMCR_PDE;
> +break;
>  }
>  }

>  static void imx2_wdt_realize(DeviceState *dev, Error **errp)
>  {
>  IMX2WdtState *s = IMX2_WDT(dev);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>
>  memory_region_init_io(&s->mmio, OBJECT(dev),
>&imx2_wdt_ops, s,
> -  TYPE_IMX2_WDT".mmio",
> -  IMX2_WDT_REG_NUM * sizeof(uint16_t));
> -sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
> +  TYPE_IMX2_WDT,
> +  IMX2_WDT_MMIO_SIZE);
> +sysbus_init_mmio(sbd, &s->mmio);
> +sysbus_init_irq(sbd, &s->irq);
> +
> +s->timer = ptimer_init(imx2_wdt_expired, s, PTIMER_POLICY_DEFAULT);

PTIMER_POLICY_DEF

Re: [PATCH v2 7/8] hw/arm/fsl-imx7: Instantiate various unimplemented devices

[Peter Maydell]

On Sun, 22 Mar 2020 at 21:19, Guenter Roeck  wrote:
>
> Instantiating PWM, CAN, CAAM, and OCOTP devices is necessary to avoid
> crashes when booting mainline Linux.
>
> Signed-off-by: Guenter Roeck 

Reviewed-by: Peter Maydell 

thanks
-- PMM

[PULL 0/1] Linux user for 5.0 patches

[Laurent Vivier]

The following changes since commit 20038cd7a8412feeb49c01f6ede89e36c8995472:

  Update version for v5.0.0-rc3 release (2020-04-15 20:51:54 +0100)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-5.0-pull-request

for you to fetch changes up to 386d38656889a40d29b514ee6f34997ca18f741e:

  linux-user/syscall.c: add target-to-host mapping for epoll_create1() 
(2020-04-16 09:24:22 +0200)


Fix epoll_create1() for qemu-alpha



Sergei Trofimovich (1):
  linux-user/syscall.c: add target-to-host mapping for epoll_create1()

 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.2

[PULL 1/1] linux-user/syscall.c: add target-to-host mapping for epoll_create1()

[Laurent Vivier]

From: Sergei Trofimovich 

Noticed by Barnabás Virágh as a python-3.7 failue on qemu-alpha.

The bug shows up on alpha as it's one of the targets where
EPOLL_CLOEXEC differs from other targets:
sysdeps/unix/sysv/linux/alpha/bits/epoll.h: EPOLL_CLOEXEC  = 0100
sysdeps/unix/sysv/linux/bits/epoll.h:EPOLL_CLOEXEC = 0200

Bug: https://bugs.gentoo.org/717548
Reported-by: Barnabás Virágh
Signed-off-by: Sergei Trofimovich 
CC: Riku Voipio 
CC: Laurent Vivier 
Reviewed-by: Laurent Vivier 
Message-Id: <20200415220508.5044-1-sly...@gentoo.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 674f70e70a56..05f03919ff07 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12012,7 +12012,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 #endif
 #if defined(TARGET_NR_epoll_create1) && defined(CONFIG_EPOLL_CREATE1)
 case TARGET_NR_epoll_create1:
-return get_errno(epoll_create1(arg1));
+return get_errno(epoll_create1(target_to_host_bitmask(arg1, 
fcntl_flags_tbl)));
 #endif
 #if defined(TARGET_NR_epoll_ctl)
 case TARGET_NR_epoll_ctl:
-- 
2.25.2

Re: [PATCH v2 8/8] hw/arm/fsl-imx7: Connect watchdog interrupts

[Peter Maydell]

On Sun, 22 Mar 2020 at 21:19, Guenter Roeck  wrote:
>
> i.MX7 supports watchdog pretimeout interupts. With this commit,
> the watchdog in mcimx7d-sabre is fully operational, including
> pretimeout support.
>
> Signed-off-by: Guenter Roeck 

> diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
> index 47826da2b7..da977f9ffb 100644
> --- a/include/hw/arm/fsl-imx7.h
> +++ b/include/hw/arm/fsl-imx7.h
> @@ -228,6 +228,11 @@ enum FslIMX7IRQs {
>  FSL_IMX7_USB2_IRQ = 42,
>  FSL_IMX7_USB3_IRQ = 40,
>
> +FSL_IMX7_WDOG1_IRQ= 78,
> +FSL_IMX7_WDOG2_IRQ= 79,
> +FSL_IMX7_WDOG3_IRQ= 10,
> +FSL_IMX7_WDOG4_IRQ= 109,

irq 10 for wdog3 seems to match the kernel's dts, but it's
a bit weird that it's way out of the range of the others.
Did you sanity check it against the imx7 data sheet and/or
real h/w behaviour that it's not a typo for
one-hundred-and-something? (108 would be the obvious guess...)

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH 2/2] qemu-storage-daemon: Fix non-string --object properties

[Kevin Wolf]

After processing the option string with the keyval parser, we get a
QDict that contains only strings. This QDict must be fed to a keyval
visitor which converts the strings into the right data types.

qmp_object_add(), however, uses the normal QObject input visitor, which
expects a QDict where all properties already have the QType that matches
the data type required by the QOM object type.

Change the --object implementation in qemu-storage-daemon so that it
doesn't call qmp_object_add(), but calls user_creatable_add_dict()
directly instead and pass it a new keyval boolean that decides which
visitor must be used.

Reported-by: Coiby Xu 
Signed-off-by: Kevin Wolf 
---
 include/qom/object_interfaces.h | 6 +-
 qemu-storage-daemon.c   | 4 +---
 qom/object_interfaces.c | 8 ++--
 qom/qom-qmp-cmds.c  | 2 +-
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index a0037968a4..65172120fa 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -90,6 +90,10 @@ Object *user_creatable_add_type(const char *type, const char 
*id,
 /**
  * user_creatable_add_dict:
  * @qdict: the object definition
+ * @keyval: if true, use a keyval visitor for processing @qdict (i.e.
+ *  assume that all @qdict values are strings); otherwise, use
+ *  the normal QObject visitor (i.e. assume all @qdict values
+ *  have the QType expected by the QOM object type)
  * @errp: if an error occurs, a pointer to an area to store the error
  *
  * Create an instance of the user creatable object that is defined by
@@ -97,7 +101,7 @@ Object *user_creatable_add_type(const char *type, const char 
*id,
  * ID from the key 'id'. The remaining entries in @qdict are used to
  * initialize the object properties.
  */
-void user_creatable_add_dict(QDict *qdict, Error **errp);
+void user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp);
 
 /**
  * user_creatable_add_opts:
diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
index dd128978cc..9e7adfe3a6 100644
--- a/qemu-storage-daemon.c
+++ b/qemu-storage-daemon.c
@@ -278,7 +278,6 @@ static void process_options(int argc, char *argv[])
 QemuOpts *opts;
 const char *type;
 QDict *args;
-QObject *ret_data = NULL;
 
 /* FIXME The keyval parser rejects 'help' arguments, so we must
  * unconditionall try QemuOpts first. */
@@ -291,9 +290,8 @@ static void process_options(int argc, char *argv[])
 qemu_opts_del(opts);
 
 args = keyval_parse(optarg, "qom-type", &error_fatal);
-qmp_object_add(args, &ret_data, &error_fatal);
+user_creatable_add_dict(args, true, &error_fatal);
 qobject_unref(args);
-qobject_unref(ret_data);
 break;
 }
 default:
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 739e3e5172..bc36f96e47 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -106,7 +106,7 @@ out:
 return obj;
 }
 
-void user_creatable_add_dict(QDict *qdict, Error **errp)
+void user_creatable_add_dict(QDict *qdict, bool keyval, Error **errp)
 {
 Visitor *v;
 Object *obj;
@@ -127,7 +127,11 @@ void user_creatable_add_dict(QDict *qdict, Error **errp)
 }
 qdict_del(qdict, "id");
 
-v = qobject_input_visitor_new(QOBJECT(qdict));
+if (keyval) {
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+} else {
+v = qobject_input_visitor_new(QOBJECT(qdict));
+}
 obj = user_creatable_add_type(type, id, qdict, v, errp);
 visit_free(v);
 object_unref(obj);
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 35db44b50e..c5249e44d0 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -263,7 +263,7 @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error 
**errp)
 qobject_unref(pdict);
 }
 
-user_creatable_add_dict(qdict, errp);
+user_creatable_add_dict(qdict, false, errp);
 }
 
 void qmp_object_del(const char *id, Error **errp)
-- 
2.20.1

[PATCH 0/2] qemu-storage-daemon: Fix non-string --object properties

[Kevin Wolf]

Kevin Wolf (2):
  qom: Factor out user_creatable_add_dict()
  qemu-storage-daemon: Fix non-string --object properties

 include/qom/object_interfaces.h | 16 
 qemu-storage-daemon.c   |  4 +---
 qom/object_interfaces.c | 31 +++
 qom/qom-qmp-cmds.c  | 24 +---
 4 files changed, 49 insertions(+), 26 deletions(-)

-- 
2.20.1

[PATCH 1/2] qom: Factor out user_creatable_add_dict()

[Kevin Wolf]

The QMP handler qmp_object_add() and the implementation of --object in
qemu-storage-daemon can share most of the code. Currently,
qemu-storage-daemon calls qmp_object_add(), but this is not correct
because different visitors need to be used.

As a first step towards a fix, make qmp_object_add() a wrapper around a
new function user_creatable_add_dict() that can get an additional
parameter. The handling of "props" is only required for compatibility
and not required for the qemu-storage-daemon command line, so it stays
in qmp_object_add().

Signed-off-by: Kevin Wolf 
---
 include/qom/object_interfaces.h | 12 
 qom/object_interfaces.c | 27 +++
 qom/qom-qmp-cmds.c  | 24 +---
 3 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 6f92f3cebb..a0037968a4 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -87,6 +87,18 @@ Object *user_creatable_add_type(const char *type, const char 
*id,
 const QDict *qdict,
 Visitor *v, Error **errp);
 
+/**
+ * user_creatable_add_dict:
+ * @qdict: the object definition
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object that is defined by
+ * @qdict.  The object type is taken from the QDict key 'qom-type', its
+ * ID from the key 'id'. The remaining entries in @qdict are used to
+ * initialize the object properties.
+ */
+void user_creatable_add_dict(QDict *qdict, Error **errp);
+
 /**
  * user_creatable_add_opts:
  * @opts: the object definition
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 72cb9e32a9..739e3e5172 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -6,6 +6,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/qobject-input-visitor.h"
 #include "qom/object_interfaces.h"
 #include "qemu/help_option.h"
 #include "qemu/module.h"
@@ -105,6 +106,32 @@ out:
 return obj;
 }
 
+void user_creatable_add_dict(QDict *qdict, Error **errp)
+{
+Visitor *v;
+Object *obj;
+g_autofree char *type = NULL;
+g_autofree char *id = NULL;
+
+type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
+if (!type) {
+error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
+return;
+}
+qdict_del(qdict, "qom-type");
+
+id = g_strdup(qdict_get_try_str(qdict, "id"));
+if (!id) {
+error_setg(errp, QERR_MISSING_PARAMETER, "id");
+return;
+}
+qdict_del(qdict, "id");
+
+v = qobject_input_visitor_new(QOBJECT(qdict));
+obj = user_creatable_add_type(type, id, qdict, v, errp);
+visit_free(v);
+object_unref(obj);
+}
 
 Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
 {
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index e47ebe8ed1..35db44b50e 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -21,7 +21,6 @@
 #include "qapi/qapi-commands-qom.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
-#include "qapi/qobject-input-visitor.h"
 #include "qemu/cutils.h"
 #include "qom/object_interfaces.h"
 #include "qom/qom-qobject.h"
@@ -245,24 +244,6 @@ void qmp_object_add(QDict *qdict, QObject **ret_data, 
Error **errp)
 {
 QObject *props;
 QDict *pdict;
-Visitor *v;
-Object *obj;
-g_autofree char *type = NULL;
-g_autofree char *id = NULL;
-
-type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
-if (!type) {
-error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
-return;
-}
-qdict_del(qdict, "qom-type");
-
-id = g_strdup(qdict_get_try_str(qdict, "id"));
-if (!id) {
-error_setg(errp, QERR_MISSING_PARAMETER, "id");
-return;
-}
-qdict_del(qdict, "id");
 
 props = qdict_get(qdict, "props");
 if (props) {
@@ -282,10 +263,7 @@ void qmp_object_add(QDict *qdict, QObject **ret_data, 
Error **errp)
 qobject_unref(pdict);
 }
 
-v = qobject_input_visitor_new(QOBJECT(qdict));
-obj = user_creatable_add_type(type, id, qdict, v, errp);
-visit_free(v);
-object_unref(obj);
+user_creatable_add_dict(qdict, errp);
 }
 
 void qmp_object_del(const char *id, Error **errp)
-- 
2.20.1

Re: [PATCH v2 4/6] dwc-hsotg USB host controller emulation

[Peter Maydell]

On Sun, 29 Mar 2020 at 00:18, Paul Zimmerman  wrote:
>
> Add the dwc-hsotg (dwc2) USB host controller emulation code.
> Based on hw/usb/hcd-ehci.c and hw/usb/hcd-ohci.c.
>
> Note that to use this with the dwc-otg driver in the Raspbian
> kernel, you must pass the option "dwc_otg.fiq_fsm_enable=0" on
> the kernel command line.
>
> Emulation of slave mode and of descriptor-DMA mode has not been
> implemented yet. These modes are seldom used.
>
> I have used some on-line sources of information while developing
> this emulation, including:
>
> http://www.capital-micro.com/PDF/CME-M7_Family_User_Guide_EN.pdf
> has a pretty complete description of the controller starting on
> page 370.
>
> https://sourceforge.net/p/wive-ng/wive-ng-mt/ci/master/tree/docs/DataSheets/RT3050_5x_V2.0_081408_0902.pdf
> has a description of the controller registers starting on page
> 130.

Ooh, these reference URLs are very helpful. Could you put
them in a comment at the top of the C file as well as in the
commit message, please?

> Signed-off-by: Paul Zimmerman 
> ---
>  hw/usb/hcd-dwc2.c   | 1301 +++
>  hw/usb/trace-events |   47 ++
>  2 files changed, 1348 insertions(+)
>  create mode 100644 hw/usb/hcd-dwc2.c
>
> diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c
> new file mode 100644
> index 00..fd85543f4d
> --- /dev/null
> +++ b/hw/usb/hcd-dwc2.c
> @@ -0,0 +1,1301 @@
> +/*
> + * dwc-hsotg (dwc2) USB host controller emulation
> + *
> + * Based on hw/usb/hcd-ehci.c and hw/usb/hcd-ohci.c
> + *
> + * Copyright (c) 2020 Paul Zimmerman 
> + *
> + * 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.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/usb/dwc2-regs.h"
> +#include "hw/usb/hcd-dwc2.h"
> +#include "trace.h"
> +#include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
> +
> +#define USB_HZ_FS   1200
> +#define USB_HZ_HS   9600
> +
> +/* nifty macros from Arnon's EHCI version  */
> +#define get_field(data, field) \
> +(((data) & field##_MASK) >> field##_SHIFT)
> +
> +#define set_field(data, newval, field) do { \
> +uint32_t val = *data; \
> +val &= ~field##_MASK; \
> +val |= ((newval) << field##_SHIFT) & field##_MASK; \
> +*data = val; \
> +} while (0)
> +
> +#define get_bit(data, bitmask) \
> +(!!((data) & bitmask))

Could you use the standard field definition, extract, and deposit
macros from include/hw/registerfields.h, please?

> +static void dwc2_sysbus_realize(DeviceState *dev, Error **errp)
> +{
> +SysBusDevice *d = SYS_BUS_DEVICE(dev);
> +DWC2State *s = DWC2_USB(dev);
> +
> +s->glbregbase = 0;
> +s->fszregbase = 0x0100;
> +s->hreg0base = 0x0400;
> +s->hreg1base = 0x0500;
> +s->pcgregbase = 0x0e00;
> +s->hreg2base = 0x1000;
> +s->portnr = NB_PORTS;
> +s->as = &address_space_memory;

Ideally this should be a device property. (hw/dma/pl080.c
has an example of how to declare a TYPE_MEMORY_REGION
property and then create an AddressSpace from it in
the realize method. hw/arm/versatilepb.c and hw/arm/mps2-tz.c
show the other end, using object_property_set_link() to pass
the appropriate MemoryRegion to the device before realizing it.)

> +
> +dwc2_realize(s, dev, errp);

Why have you divided the realize function up into
dwc2_sysbus_realize() and dwc2_realize() and
dwc2_init()? The usual expectation would be that
there is (if you need it) an instance_init called
dwc2_init() and a realize called dwc2_realize(),
so using these names for functions that are just
called from the realize method is a bit confusing.
object_property_set_link(OBJECT(dev), OBJECT(sysmem), "downstream",
 &error_fatal);

> +dwc2_init(s, dev);
> +sysbus_init_irq(d, &s->irq);
> +sysbus_init_mmio(d, &s->mem);
> +}
> +
> +static void dwc2_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +dc->realize = dwc2_sysbus_realize;
> +dc->reset = dwc2_sysbus_reset;
> +set_bit(DEVICE_CATEGORY_USB, dc->categories);

Could you provide a VMStateDescription for dc->vmsd, please?

> +}
> +
> +static const TypeInfo dwc2_usb_type_info = {
> +.name  = TYPE_DWC2_USB,
> +.parent= TYPE_SYS_BUS_DEVICE,
> +.instance_size = sizeof(DWC2State),
> +.class_init= dwc2_class_init,
> +};
> +
> +static void dwc2_usb_register_types(void)
> +{
> +type_register_static(&dwc2_usb_type_info);
> +}

thanks
-- PMM

Re: [PATCH v2 4/6] dwc-hsotg USB host controller emulation

[Peter Maydell]

On Thu, 16 Apr 2020 at 16:45, Peter Maydell  wrote:
>
> On Sun, 29 Mar 2020 at 00:18, Paul Zimmerman  wrote:

> > +s->as = &address_space_memory;
>
> Ideally this should be a device property. (hw/dma/pl080.c
> has an example of how to declare a TYPE_MEMORY_REGION
> property and then create an AddressSpace from it in
> the realize method. hw/arm/versatilepb.c and hw/arm/mps2-tz.c
> show the other end, using object_property_set_link() to pass
> the appropriate MemoryRegion to the device before realizing it.)

On closer inspection you're already doing that with the dma_as/
dma_mr. What's this AddressSpace for if it's different?

thanks
-- PMM

Re: [PULL 0/1] Linux user for 5.0 patches

[Peter Maydell]

On Thu, 16 Apr 2020 at 16:29, Laurent Vivier  wrote:
>
> The following changes since commit 20038cd7a8412feeb49c01f6ede89e36c8995472:
>
>   Update version for v5.0.0-rc3 release (2020-04-15 20:51:54 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-5.0-pull-request
>
> for you to fetch changes up to 386d38656889a40d29b514ee6f34997ca18f741e:
>
>   linux-user/syscall.c: add target-to-host mapping for epoll_create1() 
> (2020-04-16 09:24:22 +0200)
>
> 
> Fix epoll_create1() for qemu-alpha
>
> 

How critical is this bug fix? After rc3, I really don't want
to have to create an rc4 unless it's unavoidable...

thanks
-- PMM

Re: [PATCH RFC v4] target/arm: Implement SVE2 HISTCNT, HISTSEG

[Richard Henderson]

On 4/16/20 7:42 AM, Stephen Long wrote:
> +static inline uint8_t do_histseg_cnt(uint8_t n, uint64_t m0, uint64_t m1)
> +{
> +int esz = 0;

Clearer to use MO_8.

> +int bits = 8 << esz;
> +uint64_t ones = dup_const(esz, 1);
> +uint64_t signs = ones << (bits - 1);
> +uint64_t cmp0, cmp1;
> +
> +cmp1 = dup_const(1, n);

Error in the esz argument here.

> +cmp0 = cmp1 ^ m0;
> +cmp1 = cmp1 ^ m1;
> +cmp0 = (cmp0 - ones) & ~cmp0;
> +cmp1 = (cmp1 - ones) & ~cmp1;
> +return ctpop64((cmp0 | cmp1) & signs);
> +}

Ah, well, I may have been too brief in my suggestion before.  I encourage you
to have a look at the bithacks patch and understand the algorithm here -- it's
quite clever.

We cannot simply OR the two halves together, since 8 | 8 == 8 loses one from
the count of bits.  So:

  cmp0 = (cmp0 - ones) & ~cmp0 & signs;
  cmp1 = (cmp1 - ones) & ~cmp1 & signs;

  /*
   * Combine the two compares in a way that the bits do
   * not overlap, and so preserves the count of set bits.
   * If the host has a efficient instruction for ctpop,
   * then ctpop(x) + ctpop(y) has the same number of
   * operations as ctpop(x | (y >> 1)).  If the host does
   * not have an efficient ctpop, then we only want to
   * use it once.
   */
  return ctpop64(cmp0 | (cmp1 >> 1));

> +for (j = 0; j < 64; j += 8) {
> +uint8_t count0 = do_histseg_cnt(n0 >> j, m0, m1);
> +out0 |= count0 << j;
> +
> +uint8_t count1 = do_histseg_cnt(n1 >> j, m0, m1);
> +out1 |= count1 << j;
> +}

Wrong type for count0/count1 for shifting by e.g. 56.

You might as well just use uint64_t as the return value from do_histseg_cnt()
so that we don't get unnecessary zero-extensions from the compiler.


r~

Re: [PATCH v2 4/6] dwc-hsotg USB host controller emulation

[Philippe Mathieu-Daudé]

On 4/16/20 5:47 PM, Peter Maydell wrote:

On Thu, 16 Apr 2020 at 16:45, Peter Maydell  wrote:


On Sun, 29 Mar 2020 at 00:18, Paul Zimmerman  wrote:



+s->as = &address_space_memory;


Ideally this should be a device property. (hw/dma/pl080.c
has an example of how to declare a TYPE_MEMORY_REGION
property and then create an AddressSpace from it in
the realize method. hw/arm/versatilepb.c and hw/arm/mps2-tz.c
show the other end, using object_property_set_link() to pass
the appropriate MemoryRegion to the device before realizing it.)


On closer inspection you're already doing that with the dma_as/
dma_mr. What's this AddressSpace for if it's different?


s->as is not used, probably a leftover (s->dma_as is used).



thanks
-- PMM

Re: [PATCH v1 0/2] dma/xlnx-zdma: Fix descriptor loading wrt host endianness

[Peter Maydell]

On Sat, 4 Apr 2020 at 13:26, Edgar E. Iglesias  wrote:
>
> From: "Edgar E. Iglesias" 
>
> Hi,
>
> This fixes the endinannes related bugs with descriptor loading
> that Peter pointed out.
>
> Cheers,
> Edgar
>
> Edgar E. Iglesias (2):
>   dma/xlnx-zdma: Fix descriptor loading (MEM) wrt endianness
>   dma/xlnx-zdma: Fix descriptor loading (REG) wrt endianness



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

-- PMM

Re: [PATCH v2] aspeed: Add boot stub for smp booting

[Peter Maydell]

On Thu, 9 Apr 2020 at 07:31, Joel Stanley  wrote:
>
> This is a boot stub that is similar to the code u-boot runs, allowing
> the kernel to boot the secondary CPU.

> +static void aspeed_write_smpboot(ARMCPU *cpu,
> + const struct arm_boot_info *info)
> +{
> +static const uint32_t poll_mailbox_ready[] = {
> +/*
> + * r2 = per-cpu go sign value
> + * r1 = AST_SMP_MBOX_FIELD_ENTRY
> + * r0 = AST_SMP_MBOX_FIELD_GOSIGN
> + */
> +0xee100fb0,  /* mrc p15, 0, r0, c0, c0, 5 */
> +0xe21000ff,  /* andsr0, r0, #255  */
> +0xe59f201c,  /* ldr r2, [pc, #28] */
> +0xe1822000,  /* orr r2, r2, r0*/
> +
> +0xe59f1018,  /* ldr r1, [pc, #24] */
> +0xe59f0018,  /* ldr r0, [pc, #24] */
> +
> +0xe320f002,  /* wfe   */
> +0xe5904000,  /* ldr r4, [r0]  */
> +0xe1520004,  /* cmp r2, r4*/
> +0x1afb,  /* bne  */

Note that unlike "wfi", QEMU's "wfe" implementation is merely
a 'yield', so a secondary-CPU boot loop that has wfe in it
will basically be a busy-loop of those vcpu threads.
(This is why the smpboot code in hw/arm/boot.c uses wfi.)

I don't suppose the secondary boot protocol on these boards
is such that a wfi loop will work ? (Depends on what the
primary code in the kernel does to prod the secondary after
writing the magic value.)


> +0xe591f000,  /* ldr pc, [r1]  */
> +AST_SMP_MBOX_GOSIGN,
> +AST_SMP_MBOX_FIELD_ENTRY,
> +AST_SMP_MBOX_FIELD_GOSIGN,
> +};

thanks
-- PMM

[PATCH 0/2] virtiofsd: drop Linux capabilities(7)

[Stefan Hajnoczi]

virtiofsd doesn't need of all Linux capabilities(7) available to root.  Keep a
whitelisted set of capabilities that we require.  This improves security in
case virtiofsd is compromised by making it hard for an attacker to gain further
access to the system.

Stefan Hajnoczi (2):
  virtiofsd: only retain file system capabilities
  virtiofsd: drop all capabilities in the wait parent process

 tools/virtiofsd/passthrough_ll.c | 51 
 1 file changed, 51 insertions(+)

-- 
2.25.1

[PATCH 1/2] virtiofsd: only retain file system capabilities

[Stefan Hajnoczi]

virtiofsd runs as root but only needs a subset of root's Linux
capabilities(7).  As a file server its purpose is to create and access
files on behalf of a client.  It needs to be able to access files with
arbitrary uid/gid owners.  It also needs to be create device nodes.

Introduce a Linux capabilities(7) whitelist and drop all capabilities
that we don't need, making the virtiofsd process less powerful than a
regular uid root process.

  # cat /proc/PID/status
  ...
  Before   After
  CapInh:  
  CapPrm: 003f 88df
  CapEff: 003f 88df
  CapBnd: 003f 
  CapAmb:  

Note that file capabilities cannot be used to achieve the same effect on
the virtiofsd executable because mount is used during sandbox setup.
Therefore we drop capabilities programmatically at the right point
during startup.

This patch only affects the sandboxed child process.  The parent process
that sits in waitpid(2) still has full root capabilities and will be
addressed in the next patch.

Signed-off-by: Stefan Hajnoczi 
---
 tools/virtiofsd/passthrough_ll.c | 38 
 1 file changed, 38 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 4c35c95b25..af97ba1c41 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2695,6 +2695,43 @@ static void setup_mounts(const char *source)
 close(oldroot);
 }
 
+/*
+ * Only keep whitelisted capabilities that are needed for file system operation
+ */
+static void setup_capabilities(void)
+{
+pthread_mutex_lock(&cap.mutex);
+capng_restore_state(&cap.saved);
+
+/*
+ * Whitelist file system-related capabilities that are needed for a file
+ * server to act like root.  Drop everything else like networking and
+ * sysadmin capabilities.
+ *
+ * Exclusions:
+ * 1. CAP_LINUX_IMMUTABLE is not included because it's only used via ioctl
+ *and we don't support that.
+ * 2. CAP_MAC_OVERRIDE is not included because it only seems to be
+ *used by the Smack LSM.  Omit it until there is demand for it.
+ */
+capng_setpid(syscall(SYS_gettid));
+capng_clear(CAPNG_SELECT_BOTH);
+capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE,
+CAP_CHOWN,
+CAP_DAC_OVERRIDE,
+CAP_DAC_READ_SEARCH,
+CAP_FOWNER,
+CAP_FSETID,
+CAP_SETGID,
+CAP_SETUID,
+CAP_MKNOD,
+CAP_SETFCAP);
+capng_apply(CAPNG_SELECT_BOTH);
+
+cap.saved = capng_save_state();
+pthread_mutex_unlock(&cap.mutex);
+}
+
 /*
  * Lock down this process to prevent access to other processes or files outside
  * source directory.  This reduces the impact of arbitrary code execution bugs.
@@ -2705,6 +2742,7 @@ static void setup_sandbox(struct lo_data *lo, struct 
fuse_session *se,
 setup_namespaces(lo, se);
 setup_mounts(lo->source);
 setup_seccomp(enable_syslog);
+setup_capabilities();
 }
 
 /* Raise the maximum number of open file descriptors */
-- 
2.25.1

[PATCH 2/2] virtiofsd: drop all capabilities in the wait parent process

[Stefan Hajnoczi]

All this process does is wait for its child.  No capabilities are
needed.

Signed-off-by: Stefan Hajnoczi 
---
 tools/virtiofsd/passthrough_ll.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index af97ba1c41..0c3f33b074 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2530,6 +2530,17 @@ static void print_capabilities(void)
 printf("}\n");
 }
 
+/*
+ * Drop all Linux capabilities because the wait parent process only needs to
+ * sit in waitpid(2) and terminate.
+ */
+static void setup_wait_parent_capabilities(void)
+{
+capng_setpid(syscall(SYS_gettid));
+capng_clear(CAPNG_SELECT_BOTH);
+capng_apply(CAPNG_SELECT_BOTH);
+}
+
 /*
  * Move to a new mount, net, and pid namespaces to isolate this process.
  */
@@ -2561,6 +2572,8 @@ static void setup_namespaces(struct lo_data *lo, struct 
fuse_session *se)
 pid_t waited;
 int wstatus;
 
+setup_wait_parent_capabilities();
+
 /* The parent waits for the child */
 do {
 waited = waitpid(child, &wstatus, 0);
-- 
2.25.1

Re: [PATCH v2] nrf51: Fix last GPIO CNF address

[Peter Maydell]

On Wed, 15 Apr 2020 at 05:37, Cameron Esfahani  wrote:
>
> NRF51_GPIO_REG_CNF_END doesn't actually refer to the start of the last
> valid CNF register: it's referring to the last byte of the last valid
> CNF register.
>
> This hasn't been a problem up to now, as current implementation in
> memory.c turns an unaligned 4-byte read from 0x77f to a single byte read
> and the qtest only looks at the least-significant byte of the register.
>
> But when running with patches which fix unaligned accesses in memory.c,
> the qtest breaks.
>
> Considering NRF51 doesn't support unaligned accesses, the simplest fix
> is to actually set NRF51_GPIO_REG_CNF_END to the start of the last valid
> CNF register: 0x77c.
>
> Now, qtests work with or without the unaligned access patches.
>
> Reviewed-by: Cédric Le Goater 
> Tested-by: Cédric Le Goater 
> Reviewed-by: Joel Stanley 
> Signed-off-by: Cameron Esfahani 



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

-- PMM

Re: [PULL 0/1] Linux user for 5.0 patches

[Laurent Vivier]

Le 16/04/2020 à 18:03, Peter Maydell a écrit :
> On Thu, 16 Apr 2020 at 16:29, Laurent Vivier  wrote:
>>
>> The following changes since commit 20038cd7a8412feeb49c01f6ede89e36c8995472:
>>
>>   Update version for v5.0.0-rc3 release (2020-04-15 20:51:54 +0100)
>>
>> are available in the Git repository at:
>>
>>   git://github.com/vivier/qemu.git tags/linux-user-for-5.0-pull-request
>>
>> for you to fetch changes up to 386d38656889a40d29b514ee6f34997ca18f741e:
>>
>>   linux-user/syscall.c: add target-to-host mapping for epoll_create1() 
>> (2020-04-16 09:24:22 +0200)
>>
>> 
>> Fix epoll_create1() for qemu-alpha
>>
>> 
> 
> How critical is this bug fix? After rc3, I really don't want
> to have to create an rc4 unless it's unavoidable...

See the launchpad bug (https://bugs.gentoo.org/717548): on alpha, it
prevents the use of python3 in gentoo chroot, and thus we can't use
emerge to install packages. It also impacts cmake on debian (see
https://bugs.launchpad.net/bugs/1860553).

But it's not a regression, so up to you to reject it. It appears now
because most of the distro have switched from python2 to python3.

It's a low risk change, only in linux-user and for archs that have a
different EPOLL_CLOEXEC value.

Thanks,
Laurent

[PATCH RFC v5] target/arm: Implement SVE2 HISTCNT, HISTSEG

[Stephen Long]

Signed-off-by: Stephen Long 
---
Made the fixes Richard noted.

 target/arm/helper-sve.h|   7 +++
 target/arm/sve.decode  |   6 +++
 target/arm/sve_helper.c| 104 +
 target/arm/translate-sve.c |  29 +++
 4 files changed, 146 insertions(+)

diff --git a/target/arm/helper-sve.h b/target/arm/helper-sve.h
index 4733614614..958ad623f6 100644
--- a/target/arm/helper-sve.h
+++ b/target/arm/helper-sve.h
@@ -2526,6 +2526,13 @@ DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_b, TCG_CALL_NO_RWG,
 DEF_HELPER_FLAGS_5(sve2_nmatch_ppzz_h, TCG_CALL_NO_RWG,
i32, ptr, ptr, ptr, ptr, i32)
 
+DEF_HELPER_FLAGS_5(sve2_histcnt_s, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(sve2_histcnt_d, TCG_CALL_NO_RWG,
+   void, ptr, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(sve2_histseg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_h, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_6(sve2_faddp_zpzz_s, TCG_CALL_NO_RWG,
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 26690d4208..9dd20eb6ec 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -147,6 +147,7 @@
 &rprrr_esz rn=%reg_movprfx
 @rdn_pg_rm_ra    esz:2 . ra:5  ... pg:3 rm:5 rd:5 \
 &rprrr_esz rn=%reg_movprfx
+@rd_pg_rn_rm    esz:2 . rm:5 ... pg:3 rn:5 rd:5   &rprr_esz
 
 # One register operand, with governing predicate, vector element size
 @rd_pg_rn    esz:2 ... ... ... pg:3 rn:5 rd:5   &rpr_esz
@@ -1325,6 +1326,11 @@ UQRSHRNT01000101 .. 1 . 00  . .  
@rd_rn_tszimm_shr
 MATCH   01000101 .. 1 . 100 ... . 0  @pd_pg_rn_rm
 NMATCH  01000101 .. 1 . 100 ... . 1  @pd_pg_rn_rm
 
+### SVE2 Histogram Computation
+
+HISTCNT 01000101 .. 1 . 110 ... . .  @rd_pg_rn_rm
+HISTSEG 01000101 .. 1 . 101 000 . .  @rd_rn_rm
+
 ## SVE2 floating-point pairwise operations
 
 FADDP   01100100 .. 010 00 0 100 ... . . @rdn_pg_rm
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 7c65009bb8..65857e27b4 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -7016,3 +7016,107 @@ DO_PPZZ_MATCH(sve2_nmatch_ppzz_b, MO_8, true)
 DO_PPZZ_MATCH(sve2_nmatch_ppzz_h, MO_16, true)
 
 #undef DO_PPZZ_MATCH
+
+void HELPER(sve2_histcnt_s)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+uint32_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; i += 4) {
+uint64_t count = 0;
+uint8_t pred = pg[H1(i >> 3)] >> (i & 7);
+if (pred & 1) {
+uint32_t nn = n[H4(i >> 2)];
+for (j = 0; j <= i; j += 4) {
+uint8_t pred = pg[H1(j >> 3)] >> (j & 7);
+if (pred & 1 && nn == m[H4(j >> 2)]) {
+++count;
+}
+}
+}
+d[H4(i >> 2)] = count;
+}
+}
+
+void HELPER(sve2_histcnt_d)(void *vd, void *vn, void *vm, void *vg,
+uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc) / 8;
+uint64_t *d = vd, *n = vn, *m = vm;
+uint8_t *pg = vg;
+
+for (i = 0; i < opr_sz; ++i) {
+uint64_t count = 0;
+if (pg[H1(i)] & 1) {
+uint64_t nn = n[i];
+for (j = 0; j <= i; ++j) {
+if (pg[H1(j)] & 1 && nn == m[j]) {
+++count;
+}
+}
+}
+d[i] = count;
+}
+}
+
+/*
+ * Returns the number of bytes in m0 and m1 that match n.
+ * See comment for do_match2().
+ * */
+static inline uint64_t do_histseg_cnt(uint8_t n, uint64_t m0, uint64_t m1)
+{
+int esz = MO_8;
+int bits = 8 << esz;
+uint64_t ones = dup_const(esz, 1);
+uint64_t signs = ones << (bits - 1);
+uint64_t cmp0, cmp1;
+
+cmp1 = dup_const(esz, n);
+cmp0 = cmp1 ^ m0;
+cmp1 = cmp1 ^ m1;
+cmp0 = (cmp0 - ones) & ~cmp0 & signs;
+cmp1 = (cmp1 - ones) & ~cmp1 & signs;
+
+/*
+ * Combine the two compares in a way that the bits do
+ * not overlap, and so preserves the count of set bits.
+ * If the host has a efficient instruction for ctpop,
+ * then ctpop(x) + ctpop(y) has the same number of
+ * operations as ctpop(x | (y >> 1)).  If the host does
+ * not have an efficient ctpop, then we only want to
+ * use it once.
+ */
+return ctpop64(cmp0 | (cmp1 >> 1));
+}
+
+void HELPER(sve2_histseg)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, j;
+intptr_t opr_sz = simd_oprsz(desc);
+
+for (i = 0; i < opr_sz; i += 16) {
+uint64_t n0 = *(uint64_t *)(vn + i);
+uint64_t n1 = *(uint64_t *)(vn + i + 8);
+
+uint64_t m0 = 

Re: [PATCH 2/2] virtiofsd: drop all capabilities in the wait parent process

[Philippe Mathieu-Daudé]

On 4/16/20 6:49 PM, Stefan Hajnoczi wrote:

All this process does is wait for its child.  No capabilities are
needed.

Signed-off-by: Stefan Hajnoczi 
---
  tools/virtiofsd/passthrough_ll.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index af97ba1c41..0c3f33b074 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2530,6 +2530,17 @@ static void print_capabilities(void)
  printf("}\n");
  }
  
+/*

+ * Drop all Linux capabilities because the wait parent process only needs to
+ * sit in waitpid(2) and terminate.
+ */
+static void setup_wait_parent_capabilities(void)
+{
+capng_setpid(syscall(SYS_gettid));


Maybe worth a /* Drop all capabilities */ comment here.

Reviewed-by: Philippe Mathieu-Daudé 


+capng_clear(CAPNG_SELECT_BOTH);
+capng_apply(CAPNG_SELECT_BOTH);
+}
+
  /*
   * Move to a new mount, net, and pid namespaces to isolate this process.
   */
@@ -2561,6 +2572,8 @@ static void setup_namespaces(struct lo_data *lo, struct 
fuse_session *se)
  pid_t waited;
  int wstatus;
  
+setup_wait_parent_capabilities();

+
  /* The parent waits for the child */
  do {
  waited = waitpid(child, &wstatus, 0);

[PATCH] linux-user/strace.list: fix epoll_create{,1} -strace output

[Sergei Trofimovich]

Fix syscall name and parameters priinter.

Before the change:

```
$ alpha-linux-user/qemu-alpha -strace -L /usr/alpha-unknown-linux-gnu/ /tmp/a
...
1274697 
%s(%d)(2097152,274903156744,274903156760,274905840712,274877908880,274903235616)
 = 3
1274697 exit_group(0)
```

After the change:

```
$ alpha-linux-user/qemu-alpha -strace -L /usr/alpha-unknown-linux-gnu/ /tmp/a
...
1273719 epoll_create1(2097152) = 3
1273719 exit_group(0)
```

Signed-off-by: Sergei Trofimovich 
CC: Riku Voipio 
CC: Laurent Vivier 
---
 linux-user/strace.list | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.list b/linux-user/strace.list
index d49a1e92a8..9281c0a758 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -125,10 +125,10 @@
 { TARGET_NR_dup3, "dup3" , "%s(%d,%d,%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_epoll_create
-{ TARGET_NR_epoll_create, "%s(%d)", NULL, NULL, NULL },
+{ TARGET_NR_epoll_create, "epoll_create", "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_epoll_create1
-{ TARGET_NR_epoll_create1, "%s(%d)", NULL, NULL, NULL },
+{ TARGET_NR_epoll_create1, "epoll_create1", "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_epoll_ctl
 { TARGET_NR_epoll_ctl, "epoll_ctl" , NULL, NULL, NULL },
-- 
2.26.1

Re: [PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature

[Alexander Duyck]

On Thu, Apr 16, 2020 at 7:55 AM David Hildenbrand  wrote:
>
> >> We should document our result of page poisoning, free page hinting, and
> >> free page reporting there as well. I hope you'll have time for the latter.
> >>
> >> -
> >> Semantics of VIRTIO_BALLOON_F_PAGE_POISON
> >> -
> >>
> >> "The VIRTIO_BALLOON_F_PAGE_POISON feature bit is used to indicate if the
> >> guest is using page poisoning. Guest writes to the poison_val config
> >> field to tell host about the page poisoning value that is in use."
> >> -> Very little information, no signs about what has to be done.
> >
> > I think it's an informational field. Knowing that free pages
> > are full of a specific pattern can be handy for the hypervisor
> > for a variety of reasons. E.g. compression/deduplication?
>
> I was referring to the documentation of the feature and what we
> (hypervisor) are expected to do (in regards to inflation/deflation).
>
> Yes, it might be valuable to know that the guest is using poisoning. I
> assume compression/deduplication (IOW KSM) will figure out themselves
> that such pages are equal.

The other thing to keep in mind is that the poison value only really
comes into play with hinting/reporting. In the case of the standard
balloon the pages are considered allocated from the guest's
perspective until the balloon is deflated. Then any poison/init will
occur over again anyway so I don't think the standard balloon should
really care.

For hinting it somewhat depends. Currently the implementation is
inflating a balloon so having poisoning or init_on_free means it is
written to immediately after it is freed so it defeats the purpose of
the hinting. However that is a Linux implementation issue, not
necessarily an issue with the QEMU implementation. As such may be I
should fix that in the Linux driver since that has been ignored in
QEMU up until now anyway. The more interesting bit is what should the
behavior be from the hypervisor when a page is marked as being hinted.
I think right now the behavior is to just not migrate the page. I
wonder though if we shouldn't instead just consider the page a zero
page, and then maybe modify the zero page behavior for the case where
we know page poisoning is enabled.

For reporting it is a matter of tracking the contents. We don't want
to modify the contents in any way as we are attempting to essentially
do in-place tracking of the page. So if it is poisoned or initialized
it needs to stay in that state so we cannot invalidate the page if
doing so will cause it to lose state information.

> >> "Let the hypervisor know that we are expecting a specific value to be
> >> written back in balloon pages."
> >
> >
> >
> >> -> Okay, that talks about "balloon pages", which would include right now
> >> -- pages "inflated" and then "deflated" using free page hinting
> >> -- pages "inflated" and then "deflated" using oridnary inflate/deflate
> >>queue
> >
> > ATM, in this case driver calls "free" and that fills page with the
> > poison value.
>
> Yes, that's what I mentioned somehwere, it's currently done by Linux and ...
>
> >
> > It might be a valid optimization to allow driver to skip
> > poisoning of freed pages in this case.
>
> ... we should prepare for that :)

Agreed.

> >
> >> And I would add
> >>
> >> "However, if the inflated page was not filled with "poison_val" when
> >> inflating, it's not predictable if the original page or a page filled
> >> with "poison_val" is returned."
> >>
> >> Which would cover the "we did not discard the page in the hypervisor, so
> >> the original page is still there".
> >>
> >>
> >> We should also document what is expected to happen if "poison_val" is
> >> suddenly changed by the guest at one point in time again. (e.g., not
> >> supported, unexpected things can happen, etc.)
> >
> > Right. I think we should require that this can only be changed
> > before features have been negotiated.
> > That is the only point where hypervisor can still fail
> > gracefully (i.e. fail FEATURES_OK).
>
> Agreed.

I believe that is the current behavior. Essentially if poisoning
enabled then the feature flag is left set. I think the one change I
will make in the driver is that if poisoning is enabled in the kernel,
but PAGE_POISON is not available as a feature, I am going to disable
both the reporting and hinting features in virtballoon_validate.

> I can totally understand if Alex would want to stop working on
> VIRTIO_BALLOON_F_PAGE_POISON at this point and only fix the guest to not
> enable free page reporting in case we don't have
> VIRTIO_BALLOON_F_PAGE_POISON (unless that's already done), lol. :)

I already have a patch for that.

The bigger issue is how to deal with the PAGE_POISON being enabled
with FREE_PAGE_HINTING. The legacy code at this point is just broken
and is plowing through with FREE_PAGE_HINTING while it is enable

Re: [PATCH 0/3] qemu-img: Add convert --bitmaps

[Nir Soffer]

On Thu, Apr 16, 2020 at 5:51 PM Eric Blake  wrote:
>
> Without this series, the process for copying one qcow2 image to
> another including all of its bitmaps involves running qemu and doing
> the copying by hand with a series of QMP commands.  This makes the
> process a bit more convenient.

This seems good for copying an image chain from one storage to another,
but I think we need a similar --bitmaps option to qemu-img measure to make
this really useful.

Here is example use case showing how qemu-img measure is related:

Source chain:
/dev/vg1/base
/dev/vg1/top

Destination chain:
/dev/vg2/base
/dev/vg2/top

We create empty lvs with the same name on destination storage (/dev/vg2).

We measure the base lv using qemu-img measure for creating the target lv:

qemu-img measure -f qcow2 -O qcow2 /dev/vg1/base
lvcreate -L required_size /dev/vg2/base
qemu-img create -f qcow2 /dev/vg2/base 10g

For the top lv we use the current size of the source lv - I think we
should measure it instead but
I'm not sure if qemu-img measure supports measuring a single image in a chain
(maybe -o backing_file?).

lvcreate -L current_size /dev/vg2/top
qemu-img create -f qcow2 -b /dev/vg2/base -F qcow2 /dev/vg2/top 10g

And then convert the lvs one by one:

qemu-img convert -f qcow2 -O qcow2 -n --bitmaps /dev/vg1/base /dev/vg2/base
qemu-img convert -f qcow2 -O qcow2 -n --bitmaps -B /dev/vg2/base
/dev/vg1/top /dev/vg2/top

The first copy may fail with ENOSPC since qemu-img measure of the base
does not consider the
bitmaps in the required size.

So I think we need to add a similar --bitmaps option to qemu-img
measure, hopefully reusing the
same code to find and estimate the size of the bitmaps.

Maybe we can estimate the size using qemu-img info --bitmaps, but I
think the right way to
do this is in qemu-img measure.

We have also another use case when we collapsed an image chain to single image:

Source chain:
/dev/vg1/base
/dev/vg1/top

Destination:
/dev/vg2/collapsed

In this case we measure the size of the entire chain (/dev/vg1/base <-
/dev/vg1/top) and create
/dev/vg2/collapsed in the correct size, and then we convert the chain using:

   qemu-img convert /dev/vg1/top /dev/vg2/collapsed

Currently we use this for exporting images, for example when creating
templates, or as a simple
backup. In this case we don't need to copy the bitmaps in the target
image - this is a new image
not used by any VM. Copying the bitmaps may also be non-trivial since
we may have the bitmaps
with the same names in several layers (e.g. result of live snapshot).

So I think using --bitmaps should be disabled when doing this kind of
convert. We can handle this
on our side easily, but I think this should fail or log a warning on
qemu-img, or require merging of
bitmaps with same names during the copy. I did not check if you
already handle this.

Finally we also have a use case when we copy the chain as is to new or
same storage, but
we create a new vm. In this case I don't think the backup history
makes sense for the new
vm, so we don't need to copy the bitmaps.

I will review the rest of the patches next week and can maybe give
this some testing.

Nir

> I still think that someday we will need a 'qemu-img bitmap' with
> various subcommands for manipulating bitmaps within an offline image,
> but in the meantime, this seems like a useful addition on its own.
>
> Series can also be downloaded at:
> https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/qemu-img-bitmaps-v1
>
> Eric Blake (3):
>   blockdev: Split off basic bitmap operations for qemu-img
>   qemu-img: Add convert --bitmaps option
>   iotests: Add test 291 to for qemu-img convert --bitmaps
>
>  docs/tools/qemu-img.rst|   6 +-
>  Makefile.objs  |   2 +-
>  include/sysemu/blockdev.h  |  10 ++
>  blockbitmaps.c | 217 +
>  blockdev.c | 184 ---
>  qemu-img.c |  81 +-
>  MAINTAINERS|   1 +
>  qemu-img-cmds.hx   |   4 +-
>  tests/qemu-iotests/291 | 143 
>  tests/qemu-iotests/291.out |  56 ++
>  tests/qemu-iotests/group   |   1 +
>  11 files changed, 514 insertions(+), 191 deletions(-)
>  create mode 100644 blockbitmaps.c
>  create mode 100755 tests/qemu-iotests/291
>  create mode 100644 tests/qemu-iotests/291.out
>
> --
> 2.26.0
>

Re: [PATCH RFC] configure: prefer sphinx-build to sphinx-build-3

[John Snow]

On 4/16/20 8:31 AM, Alex Bennée wrote:
> 
> John Snow  writes:
> 
>> On 4/15/20 1:55 PM, Peter Maydell wrote:
>>> On Wed, 15 Apr 2020 at 18:33, John Snow  wrote:

 sphinx-build is the name of the script entry point from the sphinx
 package itself. sphinx-build-3 is a pacakging convention by Linux
 distributions. Prefer, where possible, the canonical package name.
>>>
>>> This was Markus's code originally; cc'ing him.
>>>
>>> (Incidentally I think when we say "Linux distributions" we
>>> really mean "Red Hat"; Debian/Ubuntu don't use the "sphinx-build-3" name.)
>>>
>>
>> I'll take your word for it :)
>>
>>> thanks
>>> -- PMM
>>> (rest of email untrimmed for context)
>>>
>>
>> My only goal here is that if you are using a virtual environment with
>> sphinx installed that it prefers that, so non-standard names need to
>> come last.
>>
>> There's probably 10,000,000 ways to do that, hence the RFC.
> 
> What's wrong with just passing --sphinx-build=sphinx-build in your
> configure string? It will override whatever we auto-detect AFAICT.
> 

My goal is to make virtual environments work out of the box.

I.e., if you run ./configure from inside a VENV, it should "just work."

--js

Re: [PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature

[David Hildenbrand]

> The other thing to keep in mind is that the poison value only really
> comes into play with hinting/reporting. In the case of the standard
> balloon the pages are considered allocated from the guest's

Currently just as free page hinting IMHO. They are temporarily
considered allocated.

> perspective until the balloon is deflated. Then any poison/init will
> occur over again anyway so I don't think the standard balloon should
> really care.

I think we should make this consistent. And as we discuss below, this
allows for a nice optimization in the guest even for ordinary
inflation/deflation (no need to zero out/poison again when giving the
pages back to the buddy).

> 
> For hinting it somewhat depends. Currently the implementation is
> inflating a balloon so having poisoning or init_on_free means it is
> written to immediately after it is freed so it defeats the purpose of
> the hinting. However that is a Linux implementation issue, not

Yeah, and as we discuss below, we can optimize that later in Linux. It's
sub-optimal, I agree.

> necessarily an issue with the QEMU implementation. As such may be I
> should fix that in the Linux driver since that has been ignored in
> QEMU up until now anyway. The more interesting bit is what should the
> behavior be from the hypervisor when a page is marked as being hinted.
> I think right now the behavior is to just not migrate the page. I
> wonder though if we shouldn't instead just consider the page a zero
> page, and then maybe modify the zero page behavior for the case where
> we know page poisoning is enabled.

I consider that maybe future work. Let's keep it simple for now (iow,
try to get page poisoning handling right first). The optimize the guest
handling on balloon deflation / end of free page hinting.

[...]

>> I can totally understand if Alex would want to stop working on
>> VIRTIO_BALLOON_F_PAGE_POISON at this point and only fix the guest to not
>> enable free page reporting in case we don't have
>> VIRTIO_BALLOON_F_PAGE_POISON (unless that's already done), lol. :)
> 
> I already have a patch for that.
> 
> The bigger issue is how to deal with the PAGE_POISON being enabled
> with FREE_PAGE_HINTING. The legacy code at this point is just broken
> and is plowing through with FREE_PAGE_HINTING while it is enabled.
> That is safe for now because it is using a balloon, the side effect is
> that it is going to defer migration. If it switches to a page
> reporting type setup at some point in the future we would need to make
> sure something is written to the other end to identify the poison/zero
> pages.


I think we don't have to worry about that for now. Might be sub-optimal,
but then, I don't think actual page poisoning isn't all that frequently
used in production setups.


-- 
Thanks,

David / dhildenb

Re: [PATCH] 9pfs: local: ignore O_NOATIME if we don't have permissions

[Omar Sandoval]

On Thu, Apr 16, 2020 at 04:58:31PM +0200, Christian Schoenebeck wrote:
> On Donnerstag, 16. April 2020 02:44:33 CEST Omar Sandoval wrote:
> > From: Omar Sandoval 
> > 
> > QEMU's local 9pfs server passes through O_NOATIME from the client. If
> > the QEMU process doesn't have permissions to use O_NOATIME (namely, it
> > does not own the file nor have the CAP_FOWNER capability), the open will
> > fail. This causes issues when from the client's point of view, it
> > believes it has permissions to use O_NOATIME (e.g., a process running as
> > root in the virtual machine). Additionally, overlayfs on Linux opens
> > files on the lower layer using O_NOATIME, so in this case a 9pfs mount
> > can't be used as a lower layer for overlayfs (cf.
> > https://github.com/osandov/drgn/blob/dabfe1971951701da13863dbe6d8a1d172ad965
> > 0/vmtest/onoatimehack.c and https://github.com/NixOS/nixpkgs/issues/54509).
> > 
> > Luckily, O_NOATIME is effectively a hint, and is often ignored by, e.g.,
> > network filesystems. open(2) notes that O_NOATIME "may not be effective
> > on all filesystems. One example is NFS, where the server maintains the
> > access time." This means that we can honor it when possible but fall
> > back to ignoring it.
> 
> I am not sure whether NFS would simply silently ignore O_NOATIME i.e. without 
> returning EPERM. I don't read it that way.

As far as I can tell, the NFS protocol has nothing equivalent to
O_NOATIME and thus can't honor it. Feel free to test it:

  # mount -t nfs -o vers=4,rw 10.0.2.2:/ /mnt
  # echo foo > /mnt/foo
  # touch -d "1 hour ago" /mnt/foo
  # stat /mnt/foo | grep 'Access: [0-9]'
  Access: 2020-04-16 10:43:36.838952593 -0700
  # # Drop caches so we have to go to the NFS server.
  # echo 3 > /proc/sys/vm/drop_caches
  # strace -e openat dd if=/mnt/foo of=/dev/null iflag=noatime |& grep /mnt/foo
  openat(AT_FDCWD, "/mnt/foo", O_RDONLY|O_NOATIME) = 3
  # stat /mnt/foo | grep 'Access: [0-9]'
  Access: 2020-04-16 11:43:36.906462928 -0700

> Fact is on Linux the expected 
> behaviour is returning EPERM if O_NOATIME cannot be satisfied, consistent 
> since its introduction 22 years ago:
> http://lkml.iu.edu/hypermail/linux/kernel/9811.2/0118.html

The exact phrasing in the man-page is: "EPERM  The O_NOATIME flag was
specified, but the effective user ID of the caller did not match the
owner of the file and the caller was not privileged." IMO, it's about
whether the (guest) process has permission from the (guest) kernel's
point of view, not whether the filesystem could satisfy it.

> > Signed-off-by: Omar Sandoval 
> > ---
> >  hw/9pfs/9p-util.h | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > index 79ed6b233e..50842d540f 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -37,9 +37,14 @@ static inline int openat_file(int dirfd, const char
> > *name, int flags, {
> >  int fd, serrno, ret;
> > 
> > +again:
> >  fd = openat(dirfd, name, flags | O_NOFOLLOW | O_NOCTTY | O_NONBLOCK,
> >  mode);
> >  if (fd == -1) {
> > +if (errno == EPERM && (flags & O_NOATIME)) {
> > +flags &= ~O_NOATIME;
> > +goto again;
> > +}
> >  return -1;
> >  }
> 
> It would certainly fix the problem in your use case. But it would also unmask 
> O_NOATIME for all other ones (i.e. regular users on guest).

The guest kernel will still check whether processes on the guest have
permission to use O_NOATIME. This only changes the behavior when the
guest kernel believes that the process has permission even though the
host QEMU process doesn't.

> I mean I understand your point, but I also have to take other use cases into 
> account which might expect to receive EPERM if O_NOATIME cannot be granted.

If you'd still like to preserve this behavior, would it be acceptable to
make this a QEMU option? Maybe something like "-virtfs
honor_noatime=no": the default would be "yes", which is the current
behavior, and "no" would always mask out NOATIME.

> May I ask how come that file/dir in question does not share the same uid in 
> your specific use case? Are the file(s) created outside of QEMU, i.e. 
> directly 
> by some app on host?

My use case is running tests on different versions of the Linux kernel
while reusing the host's userspace environment. I export the host's root
filesystem read-only to the guest via 9pfs, and the guest sets up
overlayfs on top of it (to allow certain modifications) and chroots into
that. Without a workaround like the LD_PRELOAD one I mentioned in the
commit message, any (read) accesses to files owned by root on the host
(like /bin/sh) will fail.

Re: [PULL 0/1] Linux user for 5.0 patches

[Peter Maydell]

On Thu, 16 Apr 2020 at 18:16, Laurent Vivier  wrote:
>
> Le 16/04/2020 à 18:03, Peter Maydell a écrit :
> > On Thu, 16 Apr 2020 at 16:29, Laurent Vivier  wrote:
> >>
> >> The following changes since commit 
> >> 20038cd7a8412feeb49c01f6ede89e36c8995472:
> >>
> >>   Update version for v5.0.0-rc3 release (2020-04-15 20:51:54 +0100)
> >>
> >> are available in the Git repository at:
> >>
> >>   git://github.com/vivier/qemu.git tags/linux-user-for-5.0-pull-request
> >>
> >> for you to fetch changes up to 386d38656889a40d29b514ee6f34997ca18f741e:
> >>
> >>   linux-user/syscall.c: add target-to-host mapping for epoll_create1() 
> >> (2020-04-16 09:24:22 +0200)
> >>
> >> 
> >> Fix epoll_create1() for qemu-alpha
> >>
> >> 
> >
> > How critical is this bug fix? After rc3, I really don't want
> > to have to create an rc4 unless it's unavoidable...
>
> See the launchpad bug (https://bugs.gentoo.org/717548): on alpha, it
> prevents the use of python3 in gentoo chroot, and thus we can't use
> emerge to install packages. It also impacts cmake on debian (see
> https://bugs.launchpad.net/bugs/1860553).
>
> But it's not a regression, so up to you to reject it. It appears now
> because most of the distro have switched from python2 to python3.
>
> It's a low risk change, only in linux-user and for archs that have a
> different EPOLL_CLOEXEC value.

Thanks for the explanation. I think that I'll put it to one
side and if we need an rc4 for some other reason it can go in,
but it's not sufficiently major to merit an rc4 by itself.

-- PMM

Re: [PULL 0/1] Linux user for 5.0 patches

[Laurent Vivier]

Le 16/04/2020 à 21:08, Peter Maydell a écrit :
> On Thu, 16 Apr 2020 at 18:16, Laurent Vivier  wrote:
>>
>> Le 16/04/2020 à 18:03, Peter Maydell a écrit :
>>> On Thu, 16 Apr 2020 at 16:29, Laurent Vivier  wrote:

 The following changes since commit 
 20038cd7a8412feeb49c01f6ede89e36c8995472:

   Update version for v5.0.0-rc3 release (2020-04-15 20:51:54 +0100)

 are available in the Git repository at:

   git://github.com/vivier/qemu.git tags/linux-user-for-5.0-pull-request

 for you to fetch changes up to 386d38656889a40d29b514ee6f34997ca18f741e:

   linux-user/syscall.c: add target-to-host mapping for epoll_create1() 
 (2020-04-16 09:24:22 +0200)

 
 Fix epoll_create1() for qemu-alpha

 
>>>
>>> How critical is this bug fix? After rc3, I really don't want
>>> to have to create an rc4 unless it's unavoidable...
>>
>> See the launchpad bug (https://bugs.gentoo.org/717548): on alpha, it
>> prevents the use of python3 in gentoo chroot, and thus we can't use
>> emerge to install packages. It also impacts cmake on debian (see
>> https://bugs.launchpad.net/bugs/1860553).
>>
>> But it's not a regression, so up to you to reject it. It appears now
>> because most of the distro have switched from python2 to python3.
>>
>> It's a low risk change, only in linux-user and for archs that have a
>> different EPOLL_CLOEXEC value.
> 
> Thanks for the explanation. I think that I'll put it to one
> side and if we need an rc4 for some other reason it can go in,
> but it's not sufficiently major to merit an rc4 by itself.
> 

Thank you, I agree.

Laurent

Re: [PATCH RFC] configure: prefer sphinx-build to sphinx-build-3

[Peter Maydell]

On Thu, 16 Apr 2020 at 19:22, John Snow  wrote:
> My goal is to make virtual environments work out of the box.
>
> I.e., if you run ./configure from inside a VENV, it should "just work."

Yeah, this seems reasonable to me. If I understand your
patch correctly it ought to work without breaking
the setup Markus describes, because in that case
'sphinx-build' exists but will fail the test_sphinx_build
step (because it's a Python 2 sphinx-build) and we'll
then move on and use sphinx-build-3.

Patch looks good to me, but you'll need to rebase and update it
to take account of commits 516e8b7d4a and 988ae6c3a7
now in master.

thanks
-- PMM

Re: [PATCH 0/3] qemu-img: Add convert --bitmaps

[Eric Blake]

(adding Markus for a CLI question, look for [*])

On 4/16/20 1:20 PM, Nir Soffer wrote:

On Thu, Apr 16, 2020 at 5:51 PM Eric Blake  wrote:


Without this series, the process for copying one qcow2 image to
another including all of its bitmaps involves running qemu and doing
the copying by hand with a series of QMP commands.  This makes the
process a bit more convenient.


This seems good for copying an image chain from one storage to another,
but I think we need a similar --bitmaps option to qemu-img measure to make
this really useful.

Here is example use case showing how qemu-img measure is related:

Source chain:
/dev/vg1/base
/dev/vg1/top

Destination chain:
/dev/vg2/base
/dev/vg2/top

We create empty lvs with the same name on destination storage (/dev/vg2).

We measure the base lv using qemu-img measure for creating the target lv:

 qemu-img measure -f qcow2 -O qcow2 /dev/vg1/base
 lvcreate -L required_size /dev/vg2/base
 qemu-img create -f qcow2 /dev/vg2/base 10g

For the top lv we use the current size of the source lv - I think we
should measure it instead but
I'm not sure if qemu-img measure supports measuring a single image in a chain
(maybe -o backing_file?).


qemu-measure --image-opts should be able to measure a single image by 
specifying image opts that purposefully treat the image as standalone 
rather than with its normal backing file included.  Let's see if I can 
whip up an example:


$ qemu-img create -f qcow2 img.base 100M
Formatting 'img.base', fmt=qcow2 size=104857600 cluster_size=65536 
lazy_refcounts=off refcount_bits=16

$ qemu-io -f qcow2 -c 'w 0 25m' img.base
wrote 26214400/26214400 bytes at offset 0
25 MiB, 1 ops; 00.24 sec (103.405 MiB/sec and 4.1362 ops/sec)
$ qemu-img create -f qcow2 -F qcow2 -b img.base img.top
Formatting 'img.top', fmt=qcow2 size=104857600 backing_file=img.base 
backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16

$ qemu-io -f qcow2 -c 'w 25m 25m' img.top
wrote 26214400/26214400 bytes at offset 26214400
25 MiB, 1 ops; 00.24 sec (103.116 MiB/sec and 4.1247 ops/sec)
$ qemu-img measure -f qcow2 -O qcow2 img.base
required size: 26542080
fully allocated size: 105185280
required size: 52756480
fully allocated size: 105185280

Okay, I can reproduce what you are seeing - measuring the top image 
defaults to measuring the full allocation of the entire chain, rather 
than the allocation of just the top image.  And now with --image-opts to 
the rescue:


$ qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing=,file.driver=file,file.filename=img.top
qemu-img: warning: Use of "backing": "" is deprecated; use "backing": 
null instead

required size: 26542080
fully allocated size: 105185280

There you go - by forcing qemu to treat the overlay image as though it 
had no backing, you can then measure that image in isolation.


(*) Hmm - that warning about backing="" being deprecated is annoying, 
but I don't know any other way to use dotted command line syntax and 
still express that we want a QMP null.  I tried to see if I could inject 
an alternative backing driver, such as null-co, but was met with errors:


$ ./qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing.driver=null-co,file.driver=file,file.filename=img.top
qemu-img: Could not open 
'driver=qcow2,backing.driver=null-co,file.driver=file,file.filename=img.top': 
Could not open backing file: The only allowed filename for this driver 
is 'null-co://'
$ ./qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing.driver=null-co,backing.file=null-co://,file.driver=file,file.filename=img.top
qemu-img: Could not open 
'driver=qcow2,backing.driver=null-co,backing.file=null-co://,file.driver=file,file.filename=img.top': 
Could not open backing file: The only allowed filename for this driver 
is 'null-co://'
$ ./qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing.driver=null-co,backing.file.filename=null-co://,file.driver=file,file.filename=img.top
qemu-img: Could not open 
'driver=qcow2,backing.driver=null-co,backing.file.filename=null-co://,file.driver=file,file.filename=img.top': 
Could not open backing file: Block protocol 'null-co' doesn't support 
the option 'file.filename'


We don't want to support "" in the QMP syntax forever, but if the CLI 
syntax has to handle the empty string specially in order to get null 
passed to the QMP code, then so be it.


I also tried, but failed, to use JSON syntax.  I don't know why we 
haven't wired up --image-opts to use JSON syntax yet.


$ qemu-img measure --image-opts -O qcow2 '{"driver":"qcow2", "backing":null,
  "file":{"driver":"file", "filename":"img.top"}}'
qemu-img: Could not open '{"driver":"qcow2", "backing":null,
  "file":{"driver":"file", "filename":"img.top"}}': Cannot find 
device={"driver":"qcow2" nor node_name={"driver":"qcow2"


I guess there's always the pseudo-json protocol:

$ qemu-img measure -O qcow2 'json:{"driver":"qcow2", "backing":null,
  "file":{"driver":"file", "filename":"img.top"}}'
require

[PATCH v1 PATCH v1 1/1] tests: machine-none-test: Enable MicroBlaze testing

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable MicroBlaze testing.

Signed-off-by: Edgar E. Iglesias 
---
 tests/qtest/machine-none-test.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/machine-none-test.c b/tests/qtest/machine-none-test.c
index 8bb54a6360..209d86eb57 100644
--- a/tests/qtest/machine-none-test.c
+++ b/tests/qtest/machine-none-test.c
@@ -33,8 +33,8 @@ static struct arch2cpu cpus_map[] = {
 { "cris", "crisv32" },
 { "lm32", "lm32-full" },
 { "m68k", "m5206" },
-/* FIXME: { "microblaze", "any" }, doesn't work with -M none -cpu any */
-/* FIXME: { "microblazeel", "any" }, doesn't work with -M none -cpu any */
+{ "microblaze", "any" },
+{ "microblazeel", "any" },
 { "mips", "4Kc" },
 { "mipsel", "I7200" },
 { "mips64", "20Kc" },
@@ -79,10 +79,8 @@ static void test_machine_cpu_cli(void)
 QTestState *qts;
 
 if (!cpu_model) {
-if (!(!strcmp(arch, "microblaze") || !strcmp(arch, "microblazeel"))) {
-fprintf(stderr, "WARNING: cpu name for target '%s' isn't defined,"
-" add it to cpus_map\n", arch);
-}
+fprintf(stderr, "WARNING: cpu name for target '%s' isn't defined,"
+" add it to cpus_map\n", arch);
 return; /* TODO: die here to force all targets have a test */
 }
 qts = qtest_initf("-machine none -cpu '%s'", cpu_model);
-- 
2.20.1

[PATCH v1 PATCH v1 0/1] tests: machine-none-test: Enable MicroBlaze testing

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This is to re-enable machine-none MicroBlaze testing.

Cheers,
Edgar

Edgar E. Iglesias (1):
  tests: machine-none-test: Enable MicroBlaze testing

 tests/qtest/machine-none-test.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.20.1

[Bug 1873335] [NEW] Dos Keypad is not working for numbers - numlock is not working

[ruthan]

Public bug reported:

Hello,
i tried to use Qemu 4.2 for Dos, but there is problem what in Dos is not 
possible turn on Numlock for input numbers, so games need it.. Numlock only 
working as arrow keys.
  I tested bough Windows and Linux builds.

With same setting, when i use Windows 98 or later os, numlock is working
fine.

** Affects: qemu
 Importance: Undecided
 Status: New

** Description changed:

  Hello,
  i tried to use Qemu 4.2 for Dos, but there is problem what in Dos is not 
possible turn on Numlock for input numbers, so games need it.. Numlock only 
working as arrow keys.
-   I tested bough Windows and Linux builds.
+   I tested bough Windows and Linux builds.
+ 
+ With same setting, when i use Windows 98 or later os, numlock is working
+ fine.

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

Title:
  Dos Keypad is not working for numbers - numlock is not working

Status in QEMU:
  New

Bug description:
  Hello,
  i tried to use Qemu 4.2 for Dos, but there is problem what in Dos is not 
possible turn on Numlock for input numbers, so games need it.. Numlock only 
working as arrow keys.
    I tested bough Windows and Linux builds.

  With same setting, when i use Windows 98 or later os, numlock is
  working fine.

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

[Bug 1873339] [NEW] Qemu DOS Quake - 640x480 and above resolutions - Unable to load VESA palette in dos prompt and game crashing are not working

[ruthan]

Public bug reported:

I have problem make Quake Demo working with 640x480+, with 320x200 working fine.
I tried 3 virtual videocards settings: -vga cirrus 640x480 is not available, 
probably emulated GPU has not enough VRAM or some Vesa2 utility is needed. For 
-vga std and -vga vmware // 640x480 is available in game menu, but when i tried 
to set it, im getting: Unable to load VESA palette in dos prompt and game 
crashing.
With vmware svgaII other Q2DOS 640x480 and 1024x768 its working fine, so it not 
working only with some games.

  Qemu 4.2, its same on Linux and Windows.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Qemu DOS Quake - 640x480 and above resolutions - Unable to load VESA
  palette in dos prompt and game crashing are not working

Status in QEMU:
  New

Bug description:
  I have problem make Quake Demo working with 640x480+, with 320x200 working 
fine.
  I tried 3 virtual videocards settings: -vga cirrus 640x480 is not available, 
probably emulated GPU has not enough VRAM or some Vesa2 utility is needed. For 
-vga std and -vga vmware // 640x480 is available in game menu, but when i tried 
to set it, im getting: Unable to load VESA palette in dos prompt and game 
crashing.
  With vmware svgaII other Q2DOS 640x480 and 1024x768 its working fine, so it 
not working only with some games.

Qemu 4.2, its same on Linux and Windows.

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

[Bug 1873338] [NEW] Dos on the fly CD image replacement is not Working with DOS

[ruthan]

Public bug reported:

Im not able to exchange CD image on the fly (needed for some games). I
messed with command like - in console(ATL+CRTL+2) eject ide1-cd0 and
change ide-cd0 D:/Games/!Emulators/Dos-QEMU/ISOs/TestChangeISO.iso , but
system so never able to find new CD data.. simply drive so empty.. but
when i reboot virtual machine, new change image is now working.

  Qemu 4.2.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Dos on the fly CD image replacement is not Working with DOS

Status in QEMU:
  New

Bug description:
  Im not able to exchange CD image on the fly (needed for some games). I
  messed with command like - in console(ATL+CRTL+2) eject ide1-cd0 and
  change ide-cd0 D:/Games/!Emulators/Dos-QEMU/ISOs/TestChangeISO.iso ,
  but system so never able to find new CD data.. simply drive so empty..
  but when i reboot virtual machine, new change image is now working.

Qemu 4.2.

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

[Bug 1873337] [NEW] Arrow keys press is double in some programs in Dos

[ruthan]

Public bug reported:

Hello,
im trying to use Qemu for Dos machines.

 But there is problem with some programs that arrow key press is double
in some problems. As advanced Filemanagers - Dos Navigator or File
Wizard, same Scandisk.

There is gif:
https://www.vogons.org/download/file.php?id=77141&mode=view

 Its blocking to use such problem, unless you use Numlock key for it,
but im used 25+ years to arrow keys and its bug.. I guess that it would
mess with some games too.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Arrow keys press is double in some programs in Dos

Status in QEMU:
  New

Bug description:
  Hello,
  im trying to use Qemu for Dos machines.

   But there is problem with some programs that arrow key press is
  double in some problems. As advanced Filemanagers - Dos Navigator or
  File Wizard, same Scandisk.

  There is gif:
  https://www.vogons.org/download/file.php?id=77141&mode=view

   Its blocking to use such problem, unless you use Numlock key for it,
  but im used 25+ years to arrow keys and its bug.. I guess that it
  would mess with some games too.

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

[Bug 1873340] [NEW] KVM Old ATI(pre) AMD card passthrough is not working

[ruthan]

Public bug reported:

Hello,
tried to passthroug old ATI pre AMD PCI / PCI-E cards, on machine where 
anything else is working - Nvidia /Matrox / 3dfx cards..

Here are results:
ATI Mach 64 PCI - videocard - machine start segfault
ATI Rage XL PCI - videocard - machine start segfault
ATI Radeon 7000 PCI - Segmentation fault
ATI X600 Giabyte GV-RX60P128D - Segmentation fault
ATI X700 PCI-E Legend - videocard - completely broken picture from boot
ATI X800 XL PCI-E Gigabyte - videocard - completely broken picture from boot
  All cards have last bioses.

ATI X600 - HP one professional with DMS-59 connector, im unable to make
passthrough, but im not able to set in Windows 98/WinXP machine..
anything less than 16 bit colors.. Im getting VM crashes or boot
freezes, when i try to boot with more colors.

 Qemu 2.11 and 4.2, is the same, Mint Linux 19.3. Giabyte Z170 MB.

** Affects: qemu
 Importance: Undecided
 Status: New

** Description changed:

  Hello,
  tried to passthroug old ATI pre AMD PCI / PCI-E cards, on machine where 
anything else is working - Nvidia /Matrox / 3dfx cards..
  
  Here are results:
  ATI Mach 64 PCI - videocard - machine start segfault
  ATI Rage XL PCI - videocard - machine start segfault
  ATI Radeon 7000 PCI - Segmentation fault
  ATI X600 Giabyte GV-RX60P128D - Segmentation fault
  ATI X700 PCI-E Legend - videocard - completely broken picture from boot
  ATI X800 XL PCI-E Gigabyte - videocard - completely broken picture from boot
-   All cards has last bioses.
+   All cards have last bioses.
  
  ATI X600 - HP one professional with DMS-59 connector, im unable to make
  passthrough, but im not able to set in Windows 98/WinXP machine..
  anything less than 16 bit colors.. Im getting VM crashes or boot
  freezes, when i try to boot with more colors.
  
-  Qemu 2.11 and 4.2, is the same, Mint Linux 19.3.
+  Qemu 2.11 and 4.2, is the same, Mint Linux 19.3. Giabyte Z170 MB.

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

Title:
  KVM Old ATI(pre) AMD card passthrough is not working

Status in QEMU:
  New

Bug description:
  Hello,
  tried to passthroug old ATI pre AMD PCI / PCI-E cards, on machine where 
anything else is working - Nvidia /Matrox / 3dfx cards..

  Here are results:
  ATI Mach 64 PCI - videocard - machine start segfault
  ATI Rage XL PCI - videocard - machine start segfault
  ATI Radeon 7000 PCI - Segmentation fault
  ATI X600 Giabyte GV-RX60P128D - Segmentation fault
  ATI X700 PCI-E Legend - videocard - completely broken picture from boot
  ATI X800 XL PCI-E Gigabyte - videocard - completely broken picture from boot
    All cards have last bioses.

  ATI X600 - HP one professional with DMS-59 connector, im unable to
  make passthrough, but im not able to set in Windows 98/WinXP machine..
  anything less than 16 bit colors.. Im getting VM crashes or boot
  freezes, when i try to boot with more colors.

   Qemu 2.11 and 4.2, is the same, Mint Linux 19.3. Giabyte Z170 MB.

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

Re: [PATCH v2 4/6] dwc-hsotg USB host controller emulation

[Paul Zimmerman]

On 4/16/20 9:30 AM, Philippe Mathieu-Daudé wrote:

On 4/16/20 5:47 PM, Peter Maydell wrote:

On Thu, 16 Apr 2020 at 16:45, Peter Maydell  wrote:


On Sun, 29 Mar 2020 at 00:18, Paul Zimmerman  wrote:



+    s->as = &address_space_memory;


Ideally this should be a device property. (hw/dma/pl080.c
has an example of how to declare a TYPE_MEMORY_REGION
property and then create an AddressSpace from it in
the realize method. hw/arm/versatilepb.c and hw/arm/mps2-tz.c
show the other end, using object_property_set_link() to pass
the appropriate MemoryRegion to the device before realizing it.)


On closer inspection you're already doing that with the dma_as/
dma_mr. What's this AddressSpace for if it's different?


s->as is not used, probably a leftover (s->dma_as is used).



thanks
-- PMM


Thanks for the reviews guys, I will take all your suggestions into
account before I post the next series.

Thanks,
Paul

[Bug 1873341] [NEW] Qemu Win98 VM with KVM videocard passthrough DOS mode video is not working for most of games..

[ruthan]

Public bug reported:

Hello,
im using Win98 machine with KVM videocards passthrough which is working fine, 
but when i try Windows 98 - Dosbox mode, there is something work with all 
videocards which i tried PCI-E/PCI - Nvidia, 3Dfx, Matrox.

 Often is framerate is very slow, as slideshow:
Doom 2, Blood, even for Fdisk start - i can see how its slowly rendering 
individual lines, or its not working at all - freeze / black screen only - 
Warcraft 2 demo (vesa 640x480). 

 There is something wrong with it.

 Qemu 2.11 + 4.2, Linux Mint 19.3. Gigabyte Z170 MB.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Qemu Win98 VM with KVM videocard passthrough DOS mode video is not
  working for most of games..

Status in QEMU:
  New

Bug description:
  Hello,
  im using Win98 machine with KVM videocards passthrough which is working fine, 
but when i try Windows 98 - Dosbox mode, there is something work with all 
videocards which i tried PCI-E/PCI - Nvidia, 3Dfx, Matrox.

   Often is framerate is very slow, as slideshow:
  Doom 2, Blood, even for Fdisk start - i can see how its slowly rendering 
individual lines, or its not working at all - freeze / black screen only - 
Warcraft 2 demo (vesa 640x480). 

   There is something wrong with it.

   Qemu 2.11 + 4.2, Linux Mint 19.3. Gigabyte Z170 MB.

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

[PATCH v20 QEMU 1/5] linux-headers: Update to allow renaming of free_page_report_cmd_id

[Alexander Duyck]

From: Alexander Duyck 

Sync to the latest upstream changes for free page hinting. To be
replaced by a full linux header sync.

Signed-off-by: Alexander Duyck 
---
 include/standard-headers/linux/virtio_balloon.h |   11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/standard-headers/linux/virtio_balloon.h 
b/include/standard-headers/linux/virtio_balloon.h
index 9375ca2a70de..af0a6b59dab2 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -47,8 +47,15 @@ struct virtio_balloon_config {
uint32_t num_pages;
/* Number of pages we've actually got in balloon. */
uint32_t actual;
-   /* Free page report command id, readonly by guest */
-   uint32_t free_page_report_cmd_id;
+   /*
+* Free page hint command id, readonly by guest.
+* Was previously name free_page_report_cmd_id so we
+* need to carry that name for legacy support.
+*/
+   union {
+   uint32_t free_page_hint_cmd_id;
+   uint32_t free_page_report_cmd_id;   /* deprecated */
+   };
/* Stores PAGE_POISON if page poisoning is in use */
uint32_t poison_val;
 };