[Qemu-devel] [Bug 1838703] Re: Makefile BUG in edk2 firmware install 4.1.0-rc3

[Laszlo Ersek (Red Hat)]

The same issue was reported and patched on qemu-devel by Olaf Hering two
months ago. The patch received three Reviewed-by tags, but nobody
bothered to queue it.

[Qemu-devel] [PATCH v1] Makefile: remove DESTDIR from firmware file cont

The thread is split over two months, hence two links below, into the mailing 
list archive:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg07093.html
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00690.html

Patchew link:
https://patchew.org/QEMU/20190530192812.17637-1-o...@aepfle.de/

There shouldn't be a need to post a new patch, just for someone to pick
up what's been posted already.

Phil: "get_maintainer.pl" reports no official owners for the root
Makefile, but you come out on top as commit-signer (26/70). Can you pick
this one up, please? Thanks!

** Changed in: qemu
   Status: New => In Progress

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

Title:
  Makefile BUG in edk2 firmware install 4.1.0-rc3

Status in QEMU:
  In Progress

Bug description:
  DESTDIR installs end up with wrong paths in JSON files installed to
  $prefix/share/qemu/firmware. For example, the file:

50-edk2-x86_64-secure.json

  ends up incorrectly with:

"filename": "/build/qemu/pkg/qemu/usr/share/qemu/edk2-x86_64-secure-
  code.fd",

  instead of the correct:

"filename": "/usr/share/qemu/edk2-x86_64-secure-code.fd",

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

Re: [Qemu-devel] [PATCH v3 03/15] target/arm/monitor: Introduce qmp_query_cpu_model_expansion

[Richard Henderson]

On 8/2/19 9:27 AM, Richard Henderson wrote:
> On 8/2/19 5:25 AM, Andrew Jones wrote:
>> Note, certainly more features may be added to the list of
>> advertised features, e.g. 'vfp' and 'neon'. The only requirement
>> is that their property set accessors fail when invalid
>> configurations are detected. For vfp we would need something like
>>
>>  set_vfp()
>>  {
>>if (arm_feature(env, ARM_FEATURE_AARCH64) &&
>>cpu->has_vfp != cpu->has_neon)
>>error("AArch64 CPUs must have both VFP and Neon or neither")
>>
>> in its set accessor, and the same for neon, rather than doing that
>> check at realize time, which isn't executed at qmp query time.
> 
> How could this succeed?  Either set_vfp or set_neon would be called first, at
> which point the two features are temporarily out of sync, but the error would
> trigger anyway.
> 
> This would seem to require some separate "qmp validate" step that is processed
> after a collection of properties are set.
> 
> I was about to say something about this being moot until someone actually 
> wants
> to be able to disable vfp+neon on aarch64, but then...
> 
>> +A note about CPU feature dependencies
>> +-
>> +
>> +It's possible for features to have dependencies on other features. I.e.
>> +it may be possible to change one feature at a time without error, but
>> +when attempting to change all features at once an error could occur
>> +depending on the order they are processed.  It's also possible changing
>> +all at once doesn't generate an error, because a feature's dependencies
>> +are satisfied with other features, but the same feature cannot be changed
>> +independently without error.  For these reasons callers should always
>> +attempt to make their desired changes all at once in order to ensure the
>> +collection is valid.
> 
> ... this language makes me think that you've already encountered an ordering
> problem that might be better solved with a separate validate step?

It appears to me that we can handle both use cases with a single function to
handle validation of the cross-dependent properties.

It would need to be called at the beginning of arm_cpu_realizefn, for the case
in which we are building a cpu that we wish to instantiate, and

> +if (!err) {
> +visit_check_struct(visitor, );
> +}

here, inside qmp_query_cpu_model_expansion for the query case.

Looking at the validation code scattered across multiple functions, across 4
patches, convinces me that the code will be smaller and more readable if we
consolidate them into a single validation function.


r~

[Qemu-devel] [PATCH] riscv: sifive_e: Correct various SoC IP block sizes

[Bin Meng]

Some of the SoC IP block sizes are wrong. Correct them according
to the FE310 manual.

Signed-off-by: Bin Meng 
---

 hw/riscv/sifive_e.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 2a499d8..9655847 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -53,13 +53,13 @@ static const struct MemmapEntry {
 hwaddr base;
 hwaddr size;
 } sifive_e_memmap[] = {
-[SIFIVE_E_DEBUG] ={0x0,  0x100 },
+[SIFIVE_E_DEBUG] ={0x0, 0x1000 },
 [SIFIVE_E_MROM] = { 0x1000, 0x2000 },
 [SIFIVE_E_OTP] =  {0x2, 0x2000 },
 [SIFIVE_E_CLINT] ={  0x200,0x1 },
 [SIFIVE_E_PLIC] = {  0xc00,  0x400 },
-[SIFIVE_E_AON] =  { 0x1000, 0x8000 },
-[SIFIVE_E_PRCI] = { 0x10008000, 0x8000 },
+[SIFIVE_E_AON] =  { 0x1000, 0x1000 },
+[SIFIVE_E_PRCI] = { 0x10008000, 0x1000 },
 [SIFIVE_E_OTP_CTRL] = { 0x1001, 0x1000 },
 [SIFIVE_E_GPIO0] ={ 0x10012000, 0x1000 },
 [SIFIVE_E_UART0] ={ 0x10013000, 0x1000 },
-- 
2.7.4

[Qemu-devel] [Bug 1838703] Re: Makefile BUG in edk2 firmware install 4.1.0-rc3

[Toolybird]

I'm on Arch, but that shouldn't matter. It's a clear bug in the
Makefile.

I note that Fedora doesn't ship these blobs as they're provide by
separate edk2 package.

Attached patch fixes it for me.

** Patch added: "edk2 Makefile fix"
   
https://bugs.launchpad.net/qemu/+bug/1838703/+attachment/5280623/+files/qemu-makefile.patch

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

Title:
  Makefile BUG in edk2 firmware install 4.1.0-rc3

Status in QEMU:
  New

Bug description:
  DESTDIR installs end up with wrong paths in JSON files installed to
  $prefix/share/qemu/firmware. For example, the file:

50-edk2-x86_64-secure.json

  ends up incorrectly with:

"filename": "/build/qemu/pkg/qemu/usr/share/qemu/edk2-x86_64-secure-
  code.fd",

  instead of the correct:

"filename": "/usr/share/qemu/edk2-x86_64-secure-code.fd",

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

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

[Laszlo Ersek]

On 08/02/19 11:20, Peter Maydell wrote:
> On Fri, 2 Aug 2019 at 01:26, Laszlo Ersek  wrote:
>> But it's extra work, not entirely risk-free (regressions), and I can't
>> tell if someone out there still uses virtio-mmio (despite me thinking
>> that would be unreasonable). I wouldn't like to see more work sunk into
>> it either way :)
> 
> The main reasons I still see people using virtio-mmio for
> the 'virt' board are:
>  * still using old but working command lines
>  * newer setups that were put together working from older tutorials
>that recommended virtio-mmio because they predated virtio-pci
>support being widespread
>  * using older (eg distro) kernels -- for 32-bit kernels in
>particular it was a while before the virtio-pci support
>got built in the default configs I think, and then longer
>again until those got into stable distro releases

There was one time when edk2 core modules gained a feature for marking
the DXE phase stack non-executable. We happily enabled it in OVMF. Then
people reported that UEFI grub in their old Debian guests had broken --
on their hosts carrying OVMF binaries built from upstream. :) We had to
flip the default off in OVMF, and we've stuck with that ever since...

  https://github.com/tianocore/edk2/commit/d20b06a3afdf

> I wouldn't be surprised if some of those applied also to
> via-OVMF boot setups as well as direct kernel boot. So it
> depends a bit what your tolerance for breaking existing
> user setups is.

Near zero. :)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH-for-3.1.1 1/1] Fix heap overflow in ip_reass on big packet input

[Marc-André Lureau]

On Fri, Aug 2, 2019 at 10:09 PM Philippe Mathieu-Daudé
 wrote:
>
> From: Samuel Thibault 
>
> When the first fragment does not fit in the preallocated buffer, q will
> already be pointing to the ext buffer, so we mustn't try to update it.
>
> Signed-off-by: Samuel Thibault 
> (cherry picked from libslirp commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210)
> Fixes: CVE-2019-14378
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Marc-André Lureau 


thanks

> ---
>  slirp/ip_input.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/ip_input.c b/slirp/ip_input.c
> index 348e1dca5a..07d8808671 100644
> --- a/slirp/ip_input.c
> +++ b/slirp/ip_input.c
> @@ -334,6 +334,8 @@ insert:
>  q = fp->frag_link.next;
> m = dtom(slirp, q);
>
> +   int was_ext = m->m_flags & M_EXT;
> +
> q = (struct ipasfrag *) q->ipf_next;
> while (q != (struct ipasfrag*)>frag_link) {
>   struct mbuf *t = dtom(slirp, q);
> @@ -356,7 +358,7 @@ insert:
>  * the old buffer (in the mbuf), so we must point ip
>  * into the new buffer.
>  */
> -   if (m->m_flags & M_EXT) {
> +   if (!was_ext && m->m_flags & M_EXT) {
>   int delta = (char *)q - m->m_dat;
>   q = (struct ipasfrag *)(m->m_ext + delta);
> }
> --
> 2.20.1
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] hmp/info_migration: formatting migration capability output

[Wei Yang]

On Fri, Aug 02, 2019 at 11:02:20AM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> On Fri, Aug 02, 2019 at 07:24:34AM +0200, Markus Armbruster wrote:
>> >Subject doesn't quite conform to conventions.  Suggest
>> >
>> >hmp: Improve how "info migrate" formats capabilities
>> 
>> Thanks
>> 
>> >
>> >Wei Yang  writes:
>> >
>> >> Current we put all migration capability in one line, which make it hard
>> >> to read them and someone them are missed due to terminal width.
>> >>
>> >> This patch formats it to print 4 in one line, which looks like this now:
>> >>
>> >> capabilities:
>> >>   xbzrle: off rdma-pin-all: offauto-converge: 
>> >> off  zero-blocks: off
>> >> compress:  on   events: off postcopy-ram: 
>> >> off   x-colo: off
>> >>  release-ram: offblock: off  return-path: 
>> >> off pause-before-switchover: off
>> >>  multifd: offdirty-bitmaps: off   postcopy-blocktime: 
>> >> off  late-block-activate: off
>> >>  x-ignore-shared: off
>> >>
>> >> Signed-off-by: Wei Yang 
>> >> ---
>> >>  monitor/hmp-cmds.c | 8 ++--
>> >>  1 file changed, 6 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> >> index 5ca3ebe942..29ce5b73e4 100644
>> >> --- a/monitor/hmp-cmds.c
>> >> +++ b/monitor/hmp-cmds.c
>> >> @@ -229,9 +229,13 @@ void hmp_info_migrate(Monitor *mon, const QDict 
>> >> *qdict)
>> >>  
>> >>  /* do not display parameters during setup */
>> >>  if (info->has_status && caps) {
>> >> +int index = 0;
>> >>  monitor_printf(mon, "capabilities: ");
>> >> -for (cap = caps; cap; cap = cap->next) {
>> >> -monitor_printf(mon, "%s: %s ",
>> >> +for (cap = caps; cap; cap = cap->next, index++) {
>> >> +if (!(index % 4)) {
>> >> +monitor_printf(mon, "\n");
>> >> +}
>> >> +monitor_printf(mon, "%20s: %3s ",
>> >> 
>> >> MigrationCapability_str(cap->value->capability),
>> >> cap->value->state ? "on" : "off");
>> >>  }
>> >
>> >This assumes migration capability names are at most 20 characters long.
>> >late-block-activate is pushing it already: 19 characters.
>> >
>> >It adds up to 104 characters per line, which is rather wide.
>> >
>> >What about putting each capability on its own line, just like globals,
>> >and just like "info migrate_capabilities"?
>> >
>> >(qemu) info migrate
>> >globals:
>> >store-global-state: on
>> >only-migratable: off
>> >send-configuration: on
>> >send-section-footer: on
>> >decompress-error-check: on
>> >clear-bitmap-shift: 18
>> >capabilities:
>> >xbzrle: off
>> >rdma-pin-all: off
>> >auto-converge: off
>> >[...]
>> 
>> This looks good, while would reader may feel difficult to clarify from which
>> one the capabilities stops. 
>> 
>> How about add 4 space at the beginning of each capability?
>> 
>> Would the output be too long? How about wrap two capability into one line?
>
>IMHO the simpler way is just not to display the capabilities on
>'info migrate' at all.  If people want the list of capabilities there's
>already 'info migrate_capabiltiies' they can use.
>
>It was nice displaying them at the start of 'info migrate'
>when we only had a few, but nowe we've got so many it's perhaps
>easier just to omit them.
>

Got it.

>Dave
>
>> -- 
>> Wei Yang
>> Help you, Help me
>--
>Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH] util/hbitmap: fix unaligned reset

[Max Reitz]

On 02.08.19 20:58, Vladimir Sementsov-Ogievskiy wrote:
> hbitmap_reset is broken: it rounds up the requested region. It leads to
> the following bug, which is shown by fixed test:
> 
> assume granularity = 2
> set(0, 3) # count becomes 4
> reset(0, 1) # count becomes 2
> 
> But user of the interface assume that virtual bit 1 should be still
> dirty, so hbitmap should report count to be 4!
> 
> In other words, because of granularity, when we set one "virtual" bit,
> yes, we make all "virtual" bits in same chunk to be dirty. But this
> should not be so for reset.
> 
> Fix this, aligning bound correctly.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> Hi all!
> 
> Hmm, is it a bug or feature? :)
> I don't have a test for mirror yet, but I think that sync mirror may be broken
> because of this, as do_sync_target_write() seems to be using unaligned reset.

Crap.


Yes, you’re right.  This would fix it, and it wouldn’t fix it in the
worst way.

But I don’t know whether this patch is the best way forward still.  I
think call hbitmap_reset() with unaligned boundaries generally calls for
trouble, as John has laid out.  If mirror’s do_sync_target_write() is
the only offender right now, I’d prefer for hbitmap_reset() to assert
that the boundaries are aligned (for 4.2), and for
do_sync_target_write() to be fixed (for 4.1? :-/).

(A practical problem with this patch is that do_sync_target_write() will
still do the write, but it won’t change anything in the bitmap, so the
copy operation was effectively useless.)

I don’t know how to fix mirror exactly, though.  I have four ideas:

(A) Quick fix 1: do_sync_target_write() should shrink [offset, offset +
bytes) such that it is aligned.  This would make it skip writes that
don’t fill one whole chunk.

+: Simple fix.  Could go into 4.1.
-: Makes copy-mode=write-blocking equal to copy-mode=background unless
   you set the granularity to like 512. (Still beats just being
   completely broken.)

(B) Quick fix 2: Setting the request_alignment block limit to the job’s
granularity when in write-blocking mode.

+: Very simple fix.  Could go into 4.1.
+: Every write will trigger a RMW cycle, which copies the whole chunk to
   the target, so write-blocking will do what it’s supposed to do.
-: request_alignment forces everything to have the same granularity, so
   this slows down reads needlessly.  (But only for write-blocking.)

(C) Maybe the right fix 1: Let do_sync_target_write() expand [offset,
offset + bytes) such that it is aligned and read head and tail from the
source node.  (So it would do the RMW itself.)

+ Doesn’t slow reads down.
+ Writes to dirty areas will make them clean – which is what
  write-blocking is for.
- Probably more complicated.  Nothing for 4.1.

(D) Maybe the right fix 2: Split BlockLimits.request_alignment into
read_alignment and write_alignment.  Then do (B).

In effect, this is more or less the same as (C), but probably in a
simpler way.  Still not simple enough for 4.1, though.


So...  I tend to do either (A) or (B) now, and then probably (D) for
4.2?  (And because (D) is an extension to (B), it would make sense to do
(B) now, unless you’d prefer (A).)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 27/28] sysemu: Move the VMChangeStateEntry typedef to qemu/typedefs.h

[Markus Armbruster]

Markus Armbruster  writes:

> Paolo Bonzini  writes:
>
>> On 26/07/19 14:05, Markus Armbruster wrote:
>>> +typedef struct VMChangeStateEntry VMChangeStateEntry;
>>>  typedef struct VMStateDescription VMStateDescription;
>>>  
>>
>> This is a bit borderline; I'd rather split sysemu/sysemu.h, e.g. adding
>> sysemu/runstate.h that would have VMChangeStateEntry functions.  If
>> there aren't many conflicts, perhaps you can drop this patch?
>
> Without it, the next one will be ineffective.

Actually, "ineffective" is an exaggeration, it's not *that* bad.  Two
headers are affected:

Before 27  After 27+28  Just 28
qapi/qapi-types-run-state.h  5500 4400 5000
sysemu/vmstate-notify.h 0  180 1000

The numbers are .o depending on the header in my "build everything"
tree, out of 6600 total (not counting tests and objects that don't
depend on qemu/osdep.h).

Mainly because hw/virtio/virtio.h (760 .o) and hw/scsi/scsi.h (180 .o)
need to include sysemu/vmstate-notify.h to get the VMChangeStateEntry
typedef.  Four more headers need it as well, but they are small
potatoes.

Splitting sysemu/runstate.h off sysemu/sysemu.h makes sense anyway, but
won't improve these numbers.

I'd keep PATCH 27.

[...]

Re: [Qemu-devel] [ANNOUNCE] QEMU 3.1.1 Stable released

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2019-08-02 12:59:08)
> On 8/2/19 7:50 PM, Michael Roth wrote:
> > Hi everyone,
> > 
> > I am pleased to announce that the QEMU v3.1.1 stable release is now
> > available:
> > 
> > You can grab the tarball from our download page here:
> > 
> >   https://www.qemu.org/download/#source
> > 
> > v3.1.1 is now tagged in the official qemu.git repository,
> > and the stable-3.1 branch has been updated accordingly:
> > 
> >   https://git.qemu.org/?p=qemu.git;a=shortlog;h=refs/heads/stable-3.1
> > 
> > This update contains a fix for CVE-2018-16872 (usb-mtp) and bug/security
> > fixes for a number of areas.
> > 
> > Please see the changelog for additional details and update accordingly.
> > 
> > Thank you to everyone involved!
> > 
> > CHANGELOG:
> > 
> > 71049d2a74: Update version for 3.1.1 release (Michael Roth)
> > 03d7712b4b: qemu-bridge-helper: restrict interface name to IFNAMSIZ (Prasad 
> > J Pandit)
> > 4482258130: block: Fix hangs in synchronous APIs with iothreads (Kevin Wolf)
> > 41dd30ff63: pvrdma: release ring object in case of an error (Prasad J 
> > Pandit)
> > a1001760ab: pvrdma: check return value from pvrdma_idx_ring_has_ routines 
> > (Prasad J Pandit)
> > 2a0e6f1369: pvrdma: check number of pages when creating rings (Prasad J 
> > Pandit)
> > 017f271f7a: device_tree: Fix integer overflowing in load_device_tree() 
> > (Markus Armbruster)
> > 5149630fed: device_tree.c: Don't use load_image() (Peter Maydell)
> > 59a823017a: sun4u: add power_mem_read routine (Prasad J Pandit)
> > 3be7eb2f47: qxl: check release info object (Prasad J Pandit)
> > 576964bf2a: seccomp: don't kill process for resource control syscalls 
> > (Daniel P. Berrangé)
> > 4c7f4c4bbb: i2c-ddc: fix oob read (Gerd Hoffmann)
> > 4e74e7a867: slirp: check data length while emulating ident function (Prasad 
> > J Pandit)
> > 375667af78: scsi-generic: avoid possible out-of-bounds access to r->buf 
> > (Paolo Bonzini)
> > bceff528ba: pvrdma: add uar_read routine (Prasad J Pandit)
> > 1549e3a54a: pvrdma: release device resources in case of an error (Prasad J 
> > Pandit)
> > 86d4f40141: gluster: the glfs_io_cbk callback function pointer adds 
> > pre/post stat args (Niels de Vos)
> > 37867211d9: gluster: Handle changed glfs_ftruncate signature (Prasanna 
> > Kumar Kalever)
> > 43a3a1b694: hw/block/pflash_cfi01: Add missing DeviceReset() handler 
> > (Philippe Mathieu-Daudé)
> > ddacb784b3: hw: Use PFLASH_CFI0{1,2} and TYPE_PFLASH_CFI0{1,2} (Markus 
> > Armbruster)
> > 03f130c682: pflash: Rename *CFI_PFLASH* to *PFLASH_CFI* (Markus Armbruster)
> > 10b1d6070a: pflash_cfi01: Log use of flawed "write to buffer" (Markus 
> > Armbruster)
> > e634054c9d: pflash_cfi01: Do not exit() on guest aborting "write to buffer" 
> > (Markus Armbruster)
> > 13cb31ce08: pflash: Rename pflash_t to PFlashCFI01, PFlashCFI02 (Markus 
> > Armbruster)
> > d0cb440280: block/pflash_cfi02: Fix memory leak and potential 
> > use-after-free (Stephen Checkoway)
> > 21e5c69b85: hw/display/xlnx_dp: Avoid crash when reading empty RX FIFO 
> > (Philippe Mathieu-Daudé)
> > e2ec206ea9: hw/ssi/mss-spi: Avoid crash when reading empty RX FIFO 
> > (Philippe Mathieu-Daudé)
> > 1f30e35861: hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[] 
> > (Philippe Mathieu-Daudé)
> > 19f55e7ab5: target/m68k: Fix a tcg_temp leak (Philippe Mathieu-Daudé)
> > c6b77a64b4: virtio-balloon: free pbp more aggressively (Michael S. Tsirkin)
> > 0965d5583e: virtio-balloon: don't track subpages for the PBP (David 
> > Hildenbrand)
> > 14d9028a7d: virtio-balloon: Use temporary PBP only (David Hildenbrand)
> > 2aa5009412: virtio-balloon: Rework pbp tracking data (David Hildenbrand)
> > 46275f9091: virtio-balloon: Better names for offset variables in 
> > inflate/deflate code (David Hildenbrand)
> > b0d6feca99: virtio-balloon: Simplify deflate with pbp (David Hildenbrand)
> > 07026c30c3: virtio-balloon: Fix QEMU crashes on pagesize > 
> > BALLOON_PAGE_SIZE (David Hildenbrand)
> > 11dd808dc0: virtio-balloon: Fix wrong sign extension of PFNs (David 
> > Hildenbrand)
> > f8364784f0: virtio-balloon: Restore MADV_WILLNEED hint on balloon deflate 
> > (David Gibson)
> > 38e8e9007d: virtio-balloon: Fix possible guest memory corruption with 
> > inflates & deflates (David Gibson)
> > 80c96a7b60: virtio-balloon: Don't mismatch g_malloc()/free (CID 1399146) 
> > (David Gibson)
> > 118112024d: virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page 
> > size (David Gibson)
> > 83cddbaadf: virtio-balloon: Use ram_block_discard_range() instead of raw 
> > madvise() (David Gibson)
> > 89b0e359bc: virtio-balloon: Rework ballon_page() interface (David Gibson)
> > b260cdec21: virtio-balloon: Corrections to address verification (David 
> > Gibson)
> > 7a31a0af31: virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate 
> > (David Gibson)
> > f0a334345b: hw/virtio/virtio-balloon: zero-initialize the 
> > virtio_balloon_config struct (Peter Maydell)
> > fc6c2bce38: i386/acpi: 

[Qemu-devel] [Bug 1838658] Re: qemu 4.0.0 broken by glib update

[Andreas Gustafsson]

> So looks like there's some further variable involved beyond just the 
> glib update - perhaps something about the host OS is combining with
> the glib update to trigger it.

Agreed - I just retested using a Fedora 30 instance on EC2, with
glib2-2.60.1-2.fc30.x86_64, and was also unable to reproduce the
issue there.

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

Title:
  qemu 4.0.0 broken by glib update

Status in QEMU:
  New

Bug description:
  In brief, an install CD will successfully boot with qemu 4.0.0 built with 
glib 2.58.3, but freeze during boot with qemu 4.0.0 built with glib 2.60.0. I 
tracked it down to glib's GHashTable improvements. qemu is happy with a glib 
built from
  ```
   git checkout -f 2.60.4
   git revert --no-edit 86c6f7e2b..3bed8a13b
   git revert --no-edit 75f8ec1df9b48b0c3a13a9125f2c7d7c5adf5159
   git revert --no-edit 603fb5958..d3074a748
   git revert --no-edit 0b45ddc55..0600dd322
  ```
  When the GHashTable improvements were committed, there was already a 
preemptive note about any breakage being due to using private implementation 
details, hence mentioning it here rather than with glib.

  For the full saga, see: http://gnats.netbsd.org/54310

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

Re: [Qemu-devel] [PATCH] util/hbitmap: fix unaligned reset

[John Snow]

On 8/2/19 2:58 PM, Vladimir Sementsov-Ogievskiy wrote:
> hbitmap_reset is broken: it rounds up the requested region. It leads to
> the following bug, which is shown by fixed test:
> 
> assume granularity = 2
> set(0, 3) # count becomes 4
> reset(0, 1) # count becomes 2
> 
> But user of the interface assume that virtual bit 1 should be still
> dirty, so hbitmap should report count to be 4!
> 
> In other words, because of granularity, when we set one "virtual" bit,
> yes, we make all "virtual" bits in same chunk to be dirty. But this
> should not be so for reset.
> 
> Fix this, aligning bound correctly.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> Hi all!
> 
> Hmm, is it a bug or feature? :)

Very, very good question.

> I don't have a test for mirror yet, but I think that sync mirror may be broken
> because of this, as do_sync_target_write() seems to be using unaligned reset.
> 

Honestly I was worried about this -- if you take a look at my patches
where I add new bitmap sync modes, I bent over backwards to align
requests for the sync=top bitmap initialization methods because I was
worried about this possibly being the case.


I'm not sure what the "right" behavior ought to be.

Let's say you have a granularity of 8 bytes:

if you reset 0-3 in one call, and then 4-7 in the next, what happens? If
the caller naively thinks there's a 1:1 relationship, it might actually
expect that to reflect a cleared bit. With alignment protection, we'll
just fail to clear it both times and it remains set.

On the other hand, if you do allow partial clears, the first reset for
0-3 will toggle off 4-7 too, where we might rely on the fact that it's
actually still dirty.

Whether or not that's dangerous depends on the context, and only the
caller knows the context. I think we need to make the semantic effect of
the reset "obvious" to the caller.


I envision this:

- hbitmap_reset(bitmap, start, length)
  returns -EINVAL if the range is not properly aligned

- hbitmap_reset_flags(bitmap, flags, start, length)
  if (flags & HBITMAP_ALIGN_DOWN) align request to only full bits
  if (flags & HBITMAP_ALIGN_UP) align request to cover any bit even
partially touched by the specified range
  otherwise, pass range through as-is to hbitmap_reset (and possibly get
-EINVAL if caller did not align the request.)


That way the semantics are always clear to the caller.

--js

>  tests/test-hbitmap.c |  2 +-
>  util/hbitmap.c   | 24 +++-
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
> index 592d8219db..0008025a9f 100644
> --- a/tests/test-hbitmap.c
> +++ b/tests/test-hbitmap.c
> @@ -424,7 +424,7 @@ static void test_hbitmap_granularity(TestHBitmapData 
> *data,
>  hbitmap_test_set(data, 0, 3);
>  g_assert_cmpint(hbitmap_count(data->hb), ==, 4);
>  hbitmap_test_reset(data, 0, 1);
> -g_assert_cmpint(hbitmap_count(data->hb), ==, 2);
> +g_assert_cmpint(hbitmap_count(data->hb), ==, 4);
>  }
>  
>  static void test_hbitmap_iter_granularity(TestHBitmapData *data,
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 7905212a8b..61a813994a 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -473,15 +473,29 @@ void hbitmap_reset(HBitmap *hb, uint64_t start, 
> uint64_t count)
>  {
>  /* Compute range in the last layer.  */
>  uint64_t first;
> -uint64_t last = start + count - 1;
> +uint64_t last;
> +uint64_t end = start + count;
> +uint64_t gran = UINT64_C(1) << hb->granularity;
>  
> -trace_hbitmap_reset(hb, start, count,
> -start >> hb->granularity, last >> hb->granularity);
> +/*
> + * We should clear only bits, fully covered by requested region. 
> Otherwise
> + * we may clear something that is actually still dirty.
> + */
> +first = DIV_ROUND_UP(start, gran);
>  
> -first = start >> hb->granularity;
> -last >>= hb->granularity;
> +if (end == hb->orig_size) {
> +end = DIV_ROUND_UP(end, gran);
> +} else {
> +end = end >> hb->granularity;
> +}
> +if (end <= first) {
> +return;
> +}
> +last = end - 1;
>  assert(last < hb->size);
>  
> +trace_hbitmap_reset(hb, start, count, first, last);
> +
>  hb->count -= hb_count_between(hb, first, last);
>  if (hb_reset_between(hb, HBITMAP_LEVELS - 1, first, last) &&
>  hb->meta) {
> 

[Qemu-devel] [PATCH] util/hbitmap: fix unaligned reset

[Vladimir Sementsov-Ogievskiy]

hbitmap_reset is broken: it rounds up the requested region. It leads to
the following bug, which is shown by fixed test:

assume granularity = 2
set(0, 3) # count becomes 4
reset(0, 1) # count becomes 2

But user of the interface assume that virtual bit 1 should be still
dirty, so hbitmap should report count to be 4!

In other words, because of granularity, when we set one "virtual" bit,
yes, we make all "virtual" bits in same chunk to be dirty. But this
should not be so for reset.

Fix this, aligning bound correctly.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi all!

Hmm, is it a bug or feature? :)
I don't have a test for mirror yet, but I think that sync mirror may be broken
because of this, as do_sync_target_write() seems to be using unaligned reset.

 tests/test-hbitmap.c |  2 +-
 util/hbitmap.c   | 24 +++-
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 592d8219db..0008025a9f 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -424,7 +424,7 @@ static void test_hbitmap_granularity(TestHBitmapData *data,
 hbitmap_test_set(data, 0, 3);
 g_assert_cmpint(hbitmap_count(data->hb), ==, 4);
 hbitmap_test_reset(data, 0, 1);
-g_assert_cmpint(hbitmap_count(data->hb), ==, 2);
+g_assert_cmpint(hbitmap_count(data->hb), ==, 4);
 }
 
 static void test_hbitmap_iter_granularity(TestHBitmapData *data,
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 7905212a8b..61a813994a 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -473,15 +473,29 @@ void hbitmap_reset(HBitmap *hb, uint64_t start, uint64_t 
count)
 {
 /* Compute range in the last layer.  */
 uint64_t first;
-uint64_t last = start + count - 1;
+uint64_t last;
+uint64_t end = start + count;
+uint64_t gran = UINT64_C(1) << hb->granularity;
 
-trace_hbitmap_reset(hb, start, count,
-start >> hb->granularity, last >> hb->granularity);
+/*
+ * We should clear only bits, fully covered by requested region. Otherwise
+ * we may clear something that is actually still dirty.
+ */
+first = DIV_ROUND_UP(start, gran);
 
-first = start >> hb->granularity;
-last >>= hb->granularity;
+if (end == hb->orig_size) {
+end = DIV_ROUND_UP(end, gran);
+} else {
+end = end >> hb->granularity;
+}
+if (end <= first) {
+return;
+}
+last = end - 1;
 assert(last < hb->size);
 
+trace_hbitmap_reset(hb, start, count, first, last);
+
 hb->count -= hb_count_between(hb, first, last);
 if (hb_reset_between(hb, HBITMAP_LEVELS - 1, first, last) &&
 hb->meta) {
-- 
2.18.0

Re: [Qemu-devel] [PATCH] riscv: hmp: Add a command to show virtual memory mappings

[Dr. David Alan Gilbert]

* Bin Meng (bmeng...@gmail.com) wrote:
> This adds 'info mem' command for RISC-V, to show virtual memory
> mappings that aids debugging.
> 
> Rather than showing every valid PTE, the command compacts the
> output by merging all contiguous physical address mappings into
> one block and only shows the merged block mapping details.
> 
> Signed-off-by: Bin Meng 

I'm fine from the HMP side; I'll leave it to RiscV people tolook
ath the bit wrangling.


Acked-by: Dr. David Alan Gilbert 

> ---
> 
>  hmp-commands-info.hx   |   2 +-
>  target/riscv/Makefile.objs |   4 +
>  target/riscv/monitor.c | 227 
> +
>  3 files changed, 232 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/monitor.c
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index c59444c..257ee7d 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -249,7 +249,7 @@ STEXI
>  Show virtual to physical memory mappings.
>  ETEXI
>  
> -#if defined(TARGET_I386)
> +#if defined(TARGET_I386) || defined(TARGET_RISCV)
>  {
>  .name   = "mem",
>  .args_type  = "",
> diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
> index b1c79bc..a8ceccd 100644
> --- a/target/riscv/Makefile.objs
> +++ b/target/riscv/Makefile.objs
> @@ -1,5 +1,9 @@
>  obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o 
> gdbstub.o pmp.o
>  
> +ifeq ($(CONFIG_SOFTMMU),y)
> +obj-y += monitor.o
> +endif
> +
>  DECODETREE = $(SRC_PATH)/scripts/decodetree.py
>  
>  decode32-y = $(SRC_PATH)/target/riscv/insn32.decode
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> new file mode 100644
> index 000..30560ff
> --- /dev/null
> +++ b/target/riscv/monitor.c
> @@ -0,0 +1,227 @@
> +/*
> + * QEMU monitor for RISC-V
> + *
> + * Copyright (c) 2019 Bin Meng 
> + *
> + * RISC-V specific monitor commands implementation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "cpu_bits.h"
> +#include "monitor/monitor.h"
> +#include "monitor/hmp-target.h"
> +
> +#ifdef TARGET_RISCV64
> +#define PTE_HEADER_FIELDS   "vaddrpaddr"\
> +"size attr\n"
> +#define PTE_HEADER_DELIMITER"  "\
> +" ---\n"
> +#else
> +#define PTE_HEADER_FIELDS   "vaddrpaddrsize attr\n"
> +#define PTE_HEADER_DELIMITER"   
> ---\n"
> +#endif
> +
> +/* Perform linear address sign extension */
> +static target_ulong addr_canonical(int va_bits, target_ulong addr)
> +{
> +#ifdef TARGET_RISCV64
> +if (addr & (1UL << (va_bits - 1))) {
> +addr |= (hwaddr)-(1L << va_bits);
> +}
> +#endif
> +
> +return addr;
> +}
> +
> +static void print_pte_header(Monitor *mon)
> +{
> +monitor_printf(mon, PTE_HEADER_FIELDS);
> +monitor_printf(mon, PTE_HEADER_DELIMITER);
> +}
> +
> +static void print_pte(Monitor *mon, int va_bits, target_ulong vaddr,
> +  hwaddr paddr, target_ulong size, int attr)
> +{
> +/* santity check on vaddr */
> +if (vaddr >= (1UL << va_bits)) {
> +return;
> +}
> +
> +if (!size) {
> +return;
> +}
> +
> +monitor_printf(mon, TARGET_FMT_lx " " TARGET_FMT_plx " " TARGET_FMT_lx
> +   " %c%c%c%c%c%c%c\n",
> +   addr_canonical(va_bits, vaddr),
> +   paddr, size,
> +   attr & PTE_R ? 'r' : '-',
> +   attr & PTE_W ? 'w' : '-',
> +   attr & PTE_X ? 'x' : '-',
> +   attr & PTE_U ? 'u' : '-',
> +   attr & PTE_G ? 'g' : '-',
> +   attr & PTE_A ? 'a' : '-',
> +   attr & PTE_D ? 'd' : '-');
> +}
> +
> +static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
> + int level, int ptidxbits, int ptesize, int va_bits,
> + hwaddr *vbase, hwaddr *pbase, hwaddr *last_paddr,
> + target_ulong *last_size, int *last_attr)
> +{
> +hwaddr pte_addr;
> +hwaddr paddr;
> +target_ulong pgsize;
> +target_ulong pte;
> +int ptshift;
> +int attr;
> +int idx;
> +
> +if (level < 0) {
> +return;

Re: [Qemu-devel] [PATCH v4 13/54] plugin: add user-facing API

[Aaron Lindsay OS via Qemu-devel]

One thing I would find useful is the ability to access register values
during an execution-time callback. I think the easiest way to do that
generically would be to expose them via the gdb functionality (like
Pavel's earlier patchset did [1]), though that (currently) limits you to
the general-purpose registers. Ideally it would be nice be able to
access other registers (i.e. floating-point, or maybe even system
registers), though those are more difficult to do generically.

Perhaps if we added some sort of architectural-support checking for
individual plugins like I mentioned in another response to this
patchset, we could allow some limited architecture-specific
functionality in this vein? I confess I haven't thought through all the
ramifications of that yet, though. 

-Aaron

[1] - See qemulib_read_register() at
  https://patchwork.ozlabs.org/patch/925393/

Re: [Qemu-devel] [PATCH v3 12/15] target/arm/kvm64: max cpu: Enable SVE when available

[Richard Henderson]

On 8/2/19 5:25 AM, Andrew Jones wrote:
> Enable SVE in the KVM guest when the 'max' cpu type is configured
> and KVM supports it. KVM SVE requires use of the new finalize
> vcpu ioctl, so we add that now too. For starters SVE can only be
> turned on or off, getting all vector lengths the host CPU supports
> when on. We'll add the other SVE CPU properties in later patches.
> 
> Signed-off-by: Andrew Jones 
> ---
>  target/arm/cpu64.c   | 17 ++---
>  target/arm/kvm.c |  5 +
>  target/arm/kvm64.c   | 20 +++-
>  target/arm/kvm_arm.h | 27 +++
>  tests/arm-cpu-features.c |  1 +
>  5 files changed, 66 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 11/15] target/arm/kvm64: Add kvm_arch_get/put_sve

[Richard Henderson]

On 8/2/19 5:25 AM, Andrew Jones wrote:
> +/*
> + * SVE registers are encoded in KVM's memory in an endianness-invariant 
> format.
> + * The byte at offset i from the start of the in-memory representation 
> contains
> + * the bits [(7 + 8 * i) : (8 * i)] of the register value. As this means the
> + * lowest offsets are stored in the lowest memory addresses, then that nearly
> + * matches QEMU's representation, which is to use an array of host-endian
> + * uint64_t's, where the lower offsets are at the lower indices. To complete
> + * the translation we just need to byte swap the uint64_t's on big-endian 
> hosts.
> + */
> +#ifdef HOST_WORDS_BIGENDIAN
> +static uint64_t *sve_bswap64(uint64_t *dst, uint64_t *src, int nr)
> +{
> +int i;
> +
> +for (i = 0; i < nr; ++i) {
> +dst[i] = bswap64(src[i]);
> +}
> +
> +return dst;
> +}
> +#endif

Maybe better as

static uint64_t *sve_bswap64(uint64_t *tmp, uint64_t *src, int nr)
{
#ifdef HOST_WORDS_BIGENDIAN
int i;

for (i = 0; i < nr; ++i) {
tmp[i] = bswap64(src[i]);
}

return tmp;
#else
return src;
#endif
}

and then the rest of the ifdefs can be removed.

Otherwise,
Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [Bug 1790460] Re: -icount, sleep=off mode is broken (target slows down or hangs)

[Peter Maydell]

I think we fixed this bug in commit 013aabdc665e4256b38d which would
have been in the 3.1.0 release (this is why we closed #1774677, which as
you say is the same issue).


** Changed in: qemu
   Status: New => 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/1790460

Title:
  -icount,sleep=off mode is broken (target slows down or hangs)

Status in QEMU:
  Fix Released

Bug description:
  QEMU running with options "-icount,sleep=off -rtc clock=vm" doesn't execute 
emulation at maximum possible speed.
  Target virtual clock may run faster or slower than realtime clock by N times, 
where N value depends on various unrelated conditions (i.e. random from the 
user point of view). The worst case is when target hangs (hopefully, early in 
booting stage).
  Example scenarios I've described here: 
http://lists.nongnu.org/archive/html/qemu-discuss/2018-08/msg00102.html

  QEMU process just sleeps most of the time (polling, waiting some
  condition, etc.). I've tried to debug issue and came to 99% conclusion
  that there are racing somewhere in qemu internals.

  The feature is broken since v2.6.0 release.
  Bad commit is 281b2201e4e18d5b9a26e1e8d81b62b5581a13be by Pavel Dovgalyuk, 
03/10/2016 05:56 PM:

icount: remove obsolete warp call

qemu_clock_warp call in qemu_tcg_wait_io_event function is not needed
anymore, because it is called in every iteration of main_loop_wait.

Reviewed-by: Paolo Bonzini 

Signed-off-by: Pavel Dovgalyuk 
Message-Id: <20160310115603.4812.67559.stgit@PASHA-ISP>
Signed-off-by: Paolo Bonzini 

  I've reverted commit to all major releases and latest git master
  branch. Issue was fixed for all of them. My adaptation is trivial:
  just restoring removed function call before "qemu_cond_wait(...)"
  line.

  I'm sure following bugs are just particular cases of the issue:
  #1774677, #1653063 .

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

[Qemu-devel] [Bug 1653063] Re: qemu-system-arm hangs with -icount and -nodefaults

[Peter Maydell]

I think we fixed this bug in commit 013aabdc665e4256b38d which would
have been in the 3.1.0 release (this is why we closed #1774677, which is
the same issue).


** Changed in: qemu
   Status: Confirmed => 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/1653063

Title:
  qemu-system-arm hangs with -icount and -nodefaults

Status in QEMU:
  Fix Released

Bug description:
  I tested with release 2.8.0 and the latest git repo, (commit:
  dbe2b65566e76d3c3a0c3358285c0336ac61e757).

  My configure options when building QEMU:
  '../configure' '--prefix=$HOME/local/qemu.git' 
'--target-list=aarch64-softmmu,arm-softmmu' '--cpu=x86_64' '--cc=gcc' 
'--disable-user' '--disable-sdl' '--disable-stack-protector' '--disable-attr' 
'--disable-pie' '--disable-linux-aio' '--disable-tpm' '--without-system-pixman' 
'--disable-docs' '--disable-guest-agent' '--disable-guest-agent-msi' 
'--disable-modules' '--disable-sparse' '--disable-gnutls' '--disable-nettle' 
'--disable-gcrypt' '--disable-gtk' '--disable-vte' '--disable-curses' 
'--disable-vnc' '--disable-cocoa' '--disable-virtfs' '--disable-xen' 
'--disable-brlapi' '--disable-curl' '--disable-bluez' '--disable-rdma' 
'--disable-uuid' '--disable-vde' '--disable-netmap' '--disable-cap-ng' 
'--disable-attr' '--disable-vhost-net' '--disable-spice' '--disable-rbd' 
'--disable-libiscsi' '--disable-libnfs' '--disable-smartcard' 
'--disable-libusb' '--disable-usb-redir' '--disable-lzo' '--disable-snappy' 
'--disable-bzip2' '--disable-seccomp' '--disable-glusterfs' 
'--disable-archipelago' '--disable-libssh2' '--disable-vhdx' '--disable-numa' 
'--disable-werror' '--disable-blobs' '--disable-vhost-scsi' '--enable-debug' 
'--disable-strip' '--enable-debug-tcg' '--enable-debug-info' 
'--extra-cflags=-fPIC'

  My host OS is Redhat RHEL-6.5. uname command gives:
  Linux rslpc1 2.6.32-431.el6.x86_64 #1 SMP Sun Nov 10 22:19:54 EST 2013 x86_64 
x86_64 x86_64 GNU/Linux

  The test image is downloaded from http://wiki.qemu.org/download/arm-
  test-0.2.tar.gz

  The command to re-produce the problem:
  qemu-system-arm -M integratorcp -kernel arm-test/zImage.integrator -initrd 
arm-test/arm_root.img -nographic -icount 1 -nodefaults -chardev 
stdio,mux=on,id=char0 -serial chardev:char0 --append "console=ttyAMA0"

  After console prints the message below:
  "Uncompressing 
Linux.. 
done, booting the kernel."
  there's no further action noticed.

  If "-icount" is not set, namely run as:
  qemu-system-arm -M integratorcp -kernel arm-test/zImage.integrator -initrd 
arm-test/arm_root.img -nographic -nodefaults -chardev stdio,mux=on,id=char0 
-serial chardev:char0 --append "console=ttyAMA0"

  or if "-nodefaults" is not set, namely run as:
  qemu-system-arm -M integratorcp -kernel arm-test/zImage.integrator -initrd 
arm-test/arm_root.img -nographic -icount 1 --append "console=ttyAMA0"

  The Linux boot procedure can finish successfully.

  Thanks.
  Hansni

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

[Qemu-devel] [PATCH-for-3.1.1 1/1] Fix heap overflow in ip_reass on big packet input

[Philippe Mathieu-Daudé]

From: Samuel Thibault 

When the first fragment does not fit in the preallocated buffer, q will
already be pointing to the ext buffer, so we mustn't try to update it.

Signed-off-by: Samuel Thibault 
(cherry picked from libslirp commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210)
Fixes: CVE-2019-14378
Signed-off-by: Philippe Mathieu-Daudé 
---
 slirp/ip_input.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/slirp/ip_input.c b/slirp/ip_input.c
index 348e1dca5a..07d8808671 100644
--- a/slirp/ip_input.c
+++ b/slirp/ip_input.c
@@ -334,6 +334,8 @@ insert:
 q = fp->frag_link.next;
m = dtom(slirp, q);
 
+   int was_ext = m->m_flags & M_EXT;
+
q = (struct ipasfrag *) q->ipf_next;
while (q != (struct ipasfrag*)>frag_link) {
  struct mbuf *t = dtom(slirp, q);
@@ -356,7 +358,7 @@ insert:
 * the old buffer (in the mbuf), so we must point ip
 * into the new buffer.
 */
-   if (m->m_flags & M_EXT) {
+   if (!was_ext && m->m_flags & M_EXT) {
  int delta = (char *)q - m->m_dat;
  q = (struct ipasfrag *)(m->m_ext + delta);
}
-- 
2.20.1

[Qemu-devel] [PATCH-for-3.1.1 0/1] Backport of CVE-2019-14378 fix

[Philippe Mathieu-Daudé]

Trivial backport from the libslirp repository.

Samuel Thibault (1):
  Fix heap overflow in ip_reass on big packet input

 slirp/ip_input.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.20.1

[Qemu-devel] [Bug 1830864] Re: Assertion `no_aa32 || ({ ARMCPU *cpu_ = (cpu); isar_feature_arm_div(_->isar); })' failed

[Peter Maydell]

Fix for this is in git and will be in 4.1.0.


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

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

Title:
  Assertion `no_aa32 || ({ ARMCPU *cpu_ = (cpu);
  isar_feature_arm_div(_->isar); })' failed

Status in QEMU:
  Fix Committed

Bug description:
  The following assertion:

  assert(no_aa32 || cpu_isar_feature(arm_div, cpu));

  introduced in commit 0f8d06f16c9d ("target/arm: Conditionalize some
  asserts on aarch32 support", 2018-11-02), fails for me. I intended to
  launch a 32-bit ARM guest (with KVM acceleration) on my AArch64 host
  (APM Mustang A3).

  Libvirt generated the following QEMU command line:

  > LC_ALL=C \
  > PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin \
  > QEMU_AUDIO_DRV=none \
  > /opt/qemu-installed-optimized/bin/qemu-system-aarch64 \
  >   -name guest=f28.32bit,debug-threads=on \
  >   -S \
  >   -object 
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-2-f28.32bit/master-key.aes
 \
  >   -machine virt-4.1,accel=kvm,usb=off,dump-guest-core=off,gic-version=2 \
  >   -cpu host,aarch64=off \
  >   -drive 
file=/root/QEMU_EFI.fd.padded,if=pflash,format=raw,unit=0,readonly=on \
  >   -drive 
file=/var/lib/libvirt/qemu/nvram/f28.32bit_VARS.fd,if=pflash,format=raw,unit=1 \
  >   -m 8192 \
  >   -realtime mlock=off \
  >   -smp 8,sockets=8,cores=1,threads=1 \
  >   -uuid d525042e-1b37-4058-86ca-c6a2086e8485 \
  >   -no-user-config \
  >   -nodefaults \
  >   -chardev socket,id=charmonitor,fd=27,server,nowait \
  >   -mon chardev=charmonitor,id=monitor,mode=control \
  >   -rtc base=utc \
  >   -no-shutdown \
  >   -boot strict=on \
  >   -device 
pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 
\
  >   -device 
pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
  >   -device 
pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
  >   -device 
pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
  >   -device 
pcie-root-port,port=0xc,chassis=5,id=pci.5,bus=pcie.0,addr=0x1.0x4 \
  >   -device 
pcie-root-port,port=0xd,chassis=6,id=pci.6,bus=pcie.0,addr=0x1.0x5 \
  >   -device qemu-xhci,id=usb,bus=pci.1,addr=0x0 \
  >   -device virtio-scsi-pci,id=scsi0,bus=pci.2,addr=0x0 \
  >   -device virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 \
  >   -drive 
file=/var/lib/libvirt/images/f28.32bit.root.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0,werror=enospc,cache=writeback,discard=unmap
 \
  >   -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1,write-cache=on
 \
  >   -drive 
file=/var/lib/libvirt/images/f28.32bit.home.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-1,werror=enospc,cache=writeback,discard=unmap
 \
  >   -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,id=scsi0-0-0-1,write-cache=on
 \
  >   -netdev tap,fd=29,id=hostnet0,vhost=on,vhostfd=30 \
  >   -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:6f:d1:c8,bus=pci.4,addr=0x0,romfile=
 \
  >   -chardev pty,id=charserial0 \
  >   -serial chardev:charserial0 \
  >   -chardev socket,id=charchannel0,fd=31,server,nowait \
  >   -device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
 \
  >   -device usb-tablet,id=input0,bus=usb.0,port=1 \
  >   -device usb-kbd,id=input1,bus=usb.0,port=2 \
  >   -vnc 127.0.0.1:0 \
  >   -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.5,addr=0x0 \
  >   -object rng-random,id=objrng0,filename=/dev/urandom \
  >   -device 
virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=1048576,period=1000,bus=pci.6,addr=0x0
 \
  >   -msg timestamp=on

  and then I got:

  > qemu-system-aarch64: /root/src/upstream/qemu/target/arm/cpu.c:986:
  > arm_cpu_realizefn: Assertion `no_aa32 || ({ ARMCPU *cpu_ = (cpu);
  > isar_feature_arm_div(_->isar); })' failed.

  QEMU was built at commit 8dc7fd56dd4f ("Merge remote-tracking branch
  'remotes/philmd-gitlab/tags/fw_cfg-20190523-pull-request' into staging",
  2019-05-23).

  (Originally reported on the mailing list in the following thread:
  
.)

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

[Qemu-devel] [Bug 1838277] Re: qemu-system-aarch64: regression in 3.1: breakpoint instructions always routed to EL_D even when current EL is higher

[Peter Maydell]

The fix for this is now in git and will be in the 4.1.0 release.


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

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

Title:
  qemu-system-aarch64: regression in 3.1: breakpoint instructions always
  routed to EL_D even when current EL is higher

Status in QEMU:
  Fix Committed

Bug description:
  Affects 3.1.0 (latest stable release) and latest commit
  (893dc8300c80e3dc32f31e968cf7aa0904da50c3) but did *not* affect 2.11
  (qemu from bionic ubuntu LTS).

  With the following code and shell commands:

  test.s:

  .text
  mov x0, #0x6000
  msr vbar_el2, x0
  dsb sy
  isb sy

  $ aarch64-none-elf-as test.s -o test.o
  $ aarch64-none-elf-objcopy -S -O binary test.o test.bin
  $ qemu-system-aarch64 -nographic -machine virt,virtualization=on -cpu 
cortex-a57 -kernel test.bin -s -S

  vbar_el2 is still 0 after the code, instead of being the expected
  0x6000. (see screenshot).

  This regression doesn't seem to happen for vbar_el1 &
  virtualization=off.

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

[Qemu-devel] [Bug 1817345] Re: configure script breaks when $source_path contains white spaces

[Peter Maydell]

Antonio's patches are in git and will be in the upcoming 4.1.0 release.


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

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

Title:
  configure script breaks when $source_path contains white spaces

Status in QEMU:
  Fix Committed

Bug description:
  Hi,

  I noticed that the configure script breaks when the qemu source
  directory is in a path containing white spaces, in particular the list
  of targets is not correctly generated when calling "./configure
  --help".

  Steps to reproduce the problem:

  $ mkdir "dir with spaces"
  $ cd dir\ with\ spaces/
  $ git clone https://git.qemu.org/git/qemu.git
  $ cd qemu/
  $ ./configure --help | grep -A3 target-list

  
  Actual result:

--target-list=LIST   set target list (default: build everything)
 Available targets: dir with *-softmmu dir with 
 *-linux-user

  
  Expected result:

--target-list=LIST   set target list (default: build everything)
 Available targets: aarch64-softmmu alpha-softmmu 
 arm-softmmu cris-softmmu hppa-softmmu i386-softmmu 
 lm32-softmmu m68k-softmmu microblaze-softmmu 

  
  This happens because the $mak_wilds variable uses spaces to separate 
different paths, maybe newlines may be used, which are less likely to be in 
directory names.

  BTW "shellcheck" may help finding some other problems.

  Qemu version:

  $ git describe 
  v3.1.0-1960-ga05838cb2a

  Thanks,
 Antonio

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

[Qemu-devel] [Bug 1838763] Re: Bugs in SSH module (ssh.c)

[Philippe Mathieu-Daudé]

$ uname -smrv
Linux 5.1.17 #7 SMP Wed Jul 10 08:35:08 UTC 2019 aarch64

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:Ubuntu 18.04.2 LTS
Release:18.04
Codename:   bionic

$ dpkg -l|fgrep libssh
ii  libssh-4:arm64   0.8.0~20170825.94fa1e38-1ubuntu0.2 
arm64tiny C SSH library (OpenSSL flavor)
ii  libssh-dev   0.8.0~20170825.94fa1e38-1ubuntu0.2 
arm64tiny C SSH library. Development files (OpenSSL flavor)

$ ./configure
...
libssh supportyes
...

$ make
...
  CC  block/ssh.o
block/ssh.c: In function 'check_host_key_knownhosts':
block/ssh.c:281:28: error: storage size of 'state' isn't known
 enum ssh_known_hosts_e state;
^
block/ssh.c:289:13: error: implicit declaration of function 
'ssh_session_is_known_server' [-Werror=implicit-function-declaration]
 state = ssh_session_is_known_server(s->session);
 ^~~
block/ssh.c:289:13: error: nested extern declaration of 
'ssh_session_is_known_server' [-Werror=nested-externs]
block/ssh.c:293:10: error: 'SSH_KNOWN_HOSTS_OK' undeclared (first use in this 
function); did you mean 'SSH_OPTIONS_HOSTKEYS'?
 case SSH_KNOWN_HOSTS_OK:
  ^~
  SSH_OPTIONS_HOSTKEYS
block/ssh.c:293:10: note: each undeclared identifier is reported only once for 
each function it appears in
block/ssh.c:297:10: error: 'SSH_KNOWN_HOSTS_CHANGED' undeclared (first use in 
this function); did you mean 'SSH_KNOWN_HOSTS_OK'?
 case SSH_KNOWN_HOSTS_CHANGED:
  ^~~
  SSH_KNOWN_HOSTS_OK
block/ssh.c:301:48: error: 'SSH_PUBLICKEY_HASH_SHA256' undeclared (first use in 
this function); did you mean 'SSH_PUBLICKEY_HASH_SHA1'?
 r = ssh_get_publickey_hash(pubkey, SSH_PUBLICKEY_HASH_SHA256,
^
SSH_PUBLICKEY_HASH_SHA1
block/ssh.c:307:27: error: implicit declaration of function 
'ssh_get_fingerprint_hash'; did you mean 'ssh_get_pubkey_hash'? 
[-Werror=implicit-function-declaration]
 fingerprint = ssh_get_fingerprint_hash(SSH_PUBLICKEY_HASH_SHA256,
   ^~~~
   ssh_get_pubkey_hash
block/ssh.c:307:27: error: nested extern declaration of 
'ssh_get_fingerprint_hash' [-Werror=nested-externs]
block/ssh.c:324:10: error: 'SSH_KNOWN_HOSTS_OTHER' undeclared (first use in 
this function); did you mean 'SSH_KNOWN_HOSTS_OK'?
 case SSH_KNOWN_HOSTS_OTHER:
  ^
  SSH_KNOWN_HOSTS_OK
block/ssh.c:329:10: error: 'SSH_KNOWN_HOSTS_UNKNOWN' undeclared (first use in 
this function); did you mean 'SSH_KNOWN_HOSTS_CHANGED'?
 case SSH_KNOWN_HOSTS_UNKNOWN:
  ^~~
  SSH_KNOWN_HOSTS_CHANGED
block/ssh.c:333:10: error: 'SSH_KNOWN_HOSTS_NOT_FOUND' undeclared (first use in 
this function); did you mean 'SSH_KNOWN_HOSTS_UNKNOWN'?
 case SSH_KNOWN_HOSTS_NOT_FOUND:
  ^
  SSH_KNOWN_HOSTS_UNKNOWN
block/ssh.c:337:10: error: 'SSH_KNOWN_HOSTS_ERROR' undeclared (first use in 
this function); did you mean 'SSH_KNOWN_HOSTS_OTHER'?
 case SSH_KNOWN_HOSTS_ERROR:
  ^
  SSH_KNOWN_HOSTS_OTHER
block/ssh.c:281:28: error: unused variable 'state' [-Werror=unused-variable]
 enum ssh_known_hosts_e state;
^
cc1: all warnings being treated as errors
rules.mak:69: recipe for target 'block/ssh.o' failed
make: *** [block/ssh.o] Error 1

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

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

Title:
  Bugs in SSH module (ssh.c)

Status in QEMU:
  Confirmed

Bug description:
  I installed gcc-8* on my Ubuntu 18.04 arm64.When I was
  compiling any version of qemu like 3.1.0 4.0.0or 4.1.0 with SSH
  support,the GCC went wrong.It said some vars undeclared
  like'SSH_KNOWN_HOSTS_OTHER','SSH_KNOWN_HOST_UNKNOWN',etc.

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

Re: [Qemu-devel] [ANNOUNCE] QEMU 3.1.1 Stable released

[Philippe Mathieu-Daudé]

On 8/2/19 7:50 PM, Michael Roth wrote:
> Hi everyone,
> 
> I am pleased to announce that the QEMU v3.1.1 stable release is now
> available:
> 
> You can grab the tarball from our download page here:
> 
>   https://www.qemu.org/download/#source
> 
> v3.1.1 is now tagged in the official qemu.git repository,
> and the stable-3.1 branch has been updated accordingly:
> 
>   https://git.qemu.org/?p=qemu.git;a=shortlog;h=refs/heads/stable-3.1
> 
> This update contains a fix for CVE-2018-16872 (usb-mtp) and bug/security
> fixes for a number of areas.
> 
> Please see the changelog for additional details and update accordingly.
> 
> Thank you to everyone involved!
> 
> CHANGELOG:
> 
> 71049d2a74: Update version for 3.1.1 release (Michael Roth)
> 03d7712b4b: qemu-bridge-helper: restrict interface name to IFNAMSIZ (Prasad J 
> Pandit)
> 4482258130: block: Fix hangs in synchronous APIs with iothreads (Kevin Wolf)
> 41dd30ff63: pvrdma: release ring object in case of an error (Prasad J Pandit)
> a1001760ab: pvrdma: check return value from pvrdma_idx_ring_has_ routines 
> (Prasad J Pandit)
> 2a0e6f1369: pvrdma: check number of pages when creating rings (Prasad J 
> Pandit)
> 017f271f7a: device_tree: Fix integer overflowing in load_device_tree() 
> (Markus Armbruster)
> 5149630fed: device_tree.c: Don't use load_image() (Peter Maydell)
> 59a823017a: sun4u: add power_mem_read routine (Prasad J Pandit)
> 3be7eb2f47: qxl: check release info object (Prasad J Pandit)
> 576964bf2a: seccomp: don't kill process for resource control syscalls (Daniel 
> P. Berrangé)
> 4c7f4c4bbb: i2c-ddc: fix oob read (Gerd Hoffmann)
> 4e74e7a867: slirp: check data length while emulating ident function (Prasad J 
> Pandit)
> 375667af78: scsi-generic: avoid possible out-of-bounds access to r->buf 
> (Paolo Bonzini)
> bceff528ba: pvrdma: add uar_read routine (Prasad J Pandit)
> 1549e3a54a: pvrdma: release device resources in case of an error (Prasad J 
> Pandit)
> 86d4f40141: gluster: the glfs_io_cbk callback function pointer adds pre/post 
> stat args (Niels de Vos)
> 37867211d9: gluster: Handle changed glfs_ftruncate signature (Prasanna Kumar 
> Kalever)
> 43a3a1b694: hw/block/pflash_cfi01: Add missing DeviceReset() handler 
> (Philippe Mathieu-Daudé)
> ddacb784b3: hw: Use PFLASH_CFI0{1,2} and TYPE_PFLASH_CFI0{1,2} (Markus 
> Armbruster)
> 03f130c682: pflash: Rename *CFI_PFLASH* to *PFLASH_CFI* (Markus Armbruster)
> 10b1d6070a: pflash_cfi01: Log use of flawed "write to buffer" (Markus 
> Armbruster)
> e634054c9d: pflash_cfi01: Do not exit() on guest aborting "write to buffer" 
> (Markus Armbruster)
> 13cb31ce08: pflash: Rename pflash_t to PFlashCFI01, PFlashCFI02 (Markus 
> Armbruster)
> d0cb440280: block/pflash_cfi02: Fix memory leak and potential use-after-free 
> (Stephen Checkoway)
> 21e5c69b85: hw/display/xlnx_dp: Avoid crash when reading empty RX FIFO 
> (Philippe Mathieu-Daudé)
> e2ec206ea9: hw/ssi/mss-spi: Avoid crash when reading empty RX FIFO (Philippe 
> Mathieu-Daudé)
> 1f30e35861: hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[] 
> (Philippe Mathieu-Daudé)
> 19f55e7ab5: target/m68k: Fix a tcg_temp leak (Philippe Mathieu-Daudé)
> c6b77a64b4: virtio-balloon: free pbp more aggressively (Michael S. Tsirkin)
> 0965d5583e: virtio-balloon: don't track subpages for the PBP (David 
> Hildenbrand)
> 14d9028a7d: virtio-balloon: Use temporary PBP only (David Hildenbrand)
> 2aa5009412: virtio-balloon: Rework pbp tracking data (David Hildenbrand)
> 46275f9091: virtio-balloon: Better names for offset variables in 
> inflate/deflate code (David Hildenbrand)
> b0d6feca99: virtio-balloon: Simplify deflate with pbp (David Hildenbrand)
> 07026c30c3: virtio-balloon: Fix QEMU crashes on pagesize > BALLOON_PAGE_SIZE 
> (David Hildenbrand)
> 11dd808dc0: virtio-balloon: Fix wrong sign extension of PFNs (David 
> Hildenbrand)
> f8364784f0: virtio-balloon: Restore MADV_WILLNEED hint on balloon deflate 
> (David Gibson)
> 38e8e9007d: virtio-balloon: Fix possible guest memory corruption with 
> inflates & deflates (David Gibson)
> 80c96a7b60: virtio-balloon: Don't mismatch g_malloc()/free (CID 1399146) 
> (David Gibson)
> 118112024d: virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size 
> (David Gibson)
> 83cddbaadf: virtio-balloon: Use ram_block_discard_range() instead of raw 
> madvise() (David Gibson)
> 89b0e359bc: virtio-balloon: Rework ballon_page() interface (David Gibson)
> b260cdec21: virtio-balloon: Corrections to address verification (David Gibson)
> 7a31a0af31: virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate 
> (David Gibson)
> f0a334345b: hw/virtio/virtio-balloon: zero-initialize the 
> virtio_balloon_config struct (Peter Maydell)
> fc6c2bce38: i386/acpi: show PCI Express bus on pxb-pcie expanders (Evgeny 
> Yakovlev)
> 11665ca918: i386/acpi: fix gint overflow in crs_range_compare (Evgeny 
> Yakovlev)
> df42bc4897: ioapic: kvm: Skip route updates for masked pins (Jan Kiszka)
> c00635946f: tpm_emulator: Translate TPM 

Re: [Qemu-devel] [Qemu-stable] [PATCH 00/36] Patch Round-up for stable 3.1.1, freeze on 2019-07-29

[Philippe Mathieu-Daudé]

On 7/23/19 8:50 PM, Michael Roth wrote:
> Quoting Michael Roth (2019-07-23 12:00:28)
>> Hi everyone, 
>>  
>>
>> The following new patches are queued for QEMU stable v3.1.1:
>>
>>   https://github.com/mdroth/qemu/commits/stable-3.1-staging
>>
>> The release is planned for 2019-08-01:
>>
>>   https://wiki.qemu.org/Planning/3.1
>>
>> Please respond here or CC qemu-sta...@nongnu.org on any patches you
>> think should be included in the release.
>>
>> Note that this update falls outside the normal stable release support
>> window (~1 development cycle), but is being released now since it was
>> delayed from its intended release date.
>>
>> Thanks!
>>
> 
> Forgot to include the usual patch list:
> 
> 
> BALATON Zoltan (1):
>   i2c: Move typedef of bitbang_i2c_interface to i2c.h
> 
> Christian Borntraeger (2):
>   iotests: make 235 work on s390 (and others)
>   s390x/cpumodel: ignore csske for expansion
> 
> Christophe Fergeau (1):
>   json: Fix % handling when not interpolating
> 
> Corey Minyard (1):
>   pc:piix4: Update smbus I/O space after a migration
> 
> Cornelia Huck (1):
>   vfio-ap: flag as compatible with balloon
> 
> Dan Streetman (1):
>   do not call vhost_net_cleanup() on running net from char user event
> 
> Daniel Henrique Barboza (1):
>   qga: update docs with systemd suspend support info
> 
> Daniel P. Berrangé (1):
>   qemu-img: fix error reporting for -object
> 
> Eric Blake (1):
>   cutils: Fix size_to_str() on 32-bit platforms
> 
> Gerd Hoffmann (1):
>   usb-mtp: use O_NOFOLLOW and O_CLOEXEC.
> 
> Janosch Frank (1):
>   s390x: Return specification exception for unimplemented diag 308 
> subcodes
> 
> Kevin Wolf (3):
>   block: Fix invalidate_cache error path for parent activation
>   qcow2: Avoid COW during metadata preallocation
>   block: Fix AioContext switch for bs->drv == NULL
> 
> Li Hangjing (1):
>   vhost: fix vhost_log size overflow during migration
> 
> Liam Merwick (1):
>   tpm_tis: fix loop that cancels any seizure by a lower locality
> 
> Marcel Apfelbaum (1):
>   hw/rdma: another clang compilation fix
> 
> Mark Cave-Ayland (2):
>   mac_oldworld: use node name instead of alias name for hd device in 
> FWPathProvider
>   mac_newworld: use node name instead of alias name for hd device in 
> FWPathProvider
> 
> Max Reitz (3):
>   iotests: Filter second BLOCK_JOB_ERROR from 229
>   block/file-posix: Unaligned O_DIRECT block-status
>   iotests: Test unaligned raw images with O_DIRECT
> 
> Michael Roth (1):
>   qga-win: include glib when building VSS DLL
> 
> Paolo Bonzini (1):
>   i386: remove the 'INTEL_PT' CPUID bit from named CPU models
> 
> Paul A. Clarke (1):
>   Changes requirement for "vsubsbs" instruction
> 
> Peter Lieven (1):
>   megasas: fix mapped frame size
> 
> Peter Maydell (2):
>   linux-user: make pwrite64/pread64(fd, NULL, 0, offset) return 0
>   exec.c: Don't reallocate IOMMUNotifiers that are in use
> 
> Robert Hoo (1):
>   i386: remove the new CPUID 'PCONFIG' from Icelake-Server CPU model
> 
> Stefan Berger (2):
>   tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid
>   tpm: Make sure the locality received from backend is valid
> 
> Thomas Huth (1):
>   hw/s390x: Fix bad mask in time2tod()
> 
> Thomas Petazzoni (1):
>   configure: improve usbfs check
> 
> William Bowling (1):
>   slirp: check sscanf result when emulating ident
> 
> Zheng Xiang (1):
>   pcie: set link state inactive/active after hot unplug/plug

I am having a spice build failure fixed by this commit:

commit be812c0ab7d5ab741d0d87387a75a0e8bb6461e7
Author: Lukáš Hrázký 
Date:   Fri Feb 15 16:09:19 2019 +0100

 spice: set device address and device display ID in QXL interface

 Calls the new SPICE QXL interface function spice_qxl_set_device_info to
 set the hardware address of the graphics device represented by the QXL
 interface (e.g. a PCI path) and the device display IDs (the IDs of the
 device's monitors that belong to this QXL interface).

 Also stops using the deprecated spice_qxl_set_max_monitors, the new
 interface function replaces it.

[Qemu-devel] [ANNOUNCE] QEMU 3.1.1 Stable released

[Michael Roth]

Hi everyone,

I am pleased to announce that the QEMU v3.1.1 stable release is now
available:

You can grab the tarball from our download page here:

  https://www.qemu.org/download/#source

v3.1.1 is now tagged in the official qemu.git repository,
and the stable-3.1 branch has been updated accordingly:

  https://git.qemu.org/?p=qemu.git;a=shortlog;h=refs/heads/stable-3.1

This update contains a fix for CVE-2018-16872 (usb-mtp) and bug/security
fixes for a number of areas.

Please see the changelog for additional details and update accordingly.

Thank you to everyone involved!

CHANGELOG:

71049d2a74: Update version for 3.1.1 release (Michael Roth)
03d7712b4b: qemu-bridge-helper: restrict interface name to IFNAMSIZ (Prasad J 
Pandit)
4482258130: block: Fix hangs in synchronous APIs with iothreads (Kevin Wolf)
41dd30ff63: pvrdma: release ring object in case of an error (Prasad J Pandit)
a1001760ab: pvrdma: check return value from pvrdma_idx_ring_has_ routines 
(Prasad J Pandit)
2a0e6f1369: pvrdma: check number of pages when creating rings (Prasad J Pandit)
017f271f7a: device_tree: Fix integer overflowing in load_device_tree() (Markus 
Armbruster)
5149630fed: device_tree.c: Don't use load_image() (Peter Maydell)
59a823017a: sun4u: add power_mem_read routine (Prasad J Pandit)
3be7eb2f47: qxl: check release info object (Prasad J Pandit)
576964bf2a: seccomp: don't kill process for resource control syscalls (Daniel 
P. Berrangé)
4c7f4c4bbb: i2c-ddc: fix oob read (Gerd Hoffmann)
4e74e7a867: slirp: check data length while emulating ident function (Prasad J 
Pandit)
375667af78: scsi-generic: avoid possible out-of-bounds access to r->buf (Paolo 
Bonzini)
bceff528ba: pvrdma: add uar_read routine (Prasad J Pandit)
1549e3a54a: pvrdma: release device resources in case of an error (Prasad J 
Pandit)
86d4f40141: gluster: the glfs_io_cbk callback function pointer adds pre/post 
stat args (Niels de Vos)
37867211d9: gluster: Handle changed glfs_ftruncate signature (Prasanna Kumar 
Kalever)
43a3a1b694: hw/block/pflash_cfi01: Add missing DeviceReset() handler (Philippe 
Mathieu-Daudé)
ddacb784b3: hw: Use PFLASH_CFI0{1,2} and TYPE_PFLASH_CFI0{1,2} (Markus 
Armbruster)
03f130c682: pflash: Rename *CFI_PFLASH* to *PFLASH_CFI* (Markus Armbruster)
10b1d6070a: pflash_cfi01: Log use of flawed "write to buffer" (Markus 
Armbruster)
e634054c9d: pflash_cfi01: Do not exit() on guest aborting "write to buffer" 
(Markus Armbruster)
13cb31ce08: pflash: Rename pflash_t to PFlashCFI01, PFlashCFI02 (Markus 
Armbruster)
d0cb440280: block/pflash_cfi02: Fix memory leak and potential use-after-free 
(Stephen Checkoway)
21e5c69b85: hw/display/xlnx_dp: Avoid crash when reading empty RX FIFO 
(Philippe Mathieu-Daudé)
e2ec206ea9: hw/ssi/mss-spi: Avoid crash when reading empty RX FIFO (Philippe 
Mathieu-Daudé)
1f30e35861: hw/ssi/xilinx_spips: Avoid out-of-bound access to lqspi_buf[] 
(Philippe Mathieu-Daudé)
19f55e7ab5: target/m68k: Fix a tcg_temp leak (Philippe Mathieu-Daudé)
c6b77a64b4: virtio-balloon: free pbp more aggressively (Michael S. Tsirkin)
0965d5583e: virtio-balloon: don't track subpages for the PBP (David Hildenbrand)
14d9028a7d: virtio-balloon: Use temporary PBP only (David Hildenbrand)
2aa5009412: virtio-balloon: Rework pbp tracking data (David Hildenbrand)
46275f9091: virtio-balloon: Better names for offset variables in 
inflate/deflate code (David Hildenbrand)
b0d6feca99: virtio-balloon: Simplify deflate with pbp (David Hildenbrand)
07026c30c3: virtio-balloon: Fix QEMU crashes on pagesize > BALLOON_PAGE_SIZE 
(David Hildenbrand)
11dd808dc0: virtio-balloon: Fix wrong sign extension of PFNs (David Hildenbrand)
f8364784f0: virtio-balloon: Restore MADV_WILLNEED hint on balloon deflate 
(David Gibson)
38e8e9007d: virtio-balloon: Fix possible guest memory corruption with inflates 
& deflates (David Gibson)
80c96a7b60: virtio-balloon: Don't mismatch g_malloc()/free (CID 1399146) (David 
Gibson)
118112024d: virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size 
(David Gibson)
83cddbaadf: virtio-balloon: Use ram_block_discard_range() instead of raw 
madvise() (David Gibson)
89b0e359bc: virtio-balloon: Rework ballon_page() interface (David Gibson)
b260cdec21: virtio-balloon: Corrections to address verification (David Gibson)
7a31a0af31: virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate (David 
Gibson)
f0a334345b: hw/virtio/virtio-balloon: zero-initialize the virtio_balloon_config 
struct (Peter Maydell)
fc6c2bce38: i386/acpi: show PCI Express bus on pxb-pcie expanders (Evgeny 
Yakovlev)
11665ca918: i386/acpi: fix gint overflow in crs_range_compare (Evgeny Yakovlev)
df42bc4897: ioapic: kvm: Skip route updates for masked pins (Jan Kiszka)
c00635946f: tpm_emulator: Translate TPM error codes to strings (Stefan Berger)
51ce84e119: tpm: Exit in reset when backend indicates failure (Stefan Berger)
0318166a9f: vhost: fix vhost_log size overflow during migration (Li Hangjing)
261d7f653a: iotests: Test unaligned raw images with 

[Qemu-devel] [Bug 1763536] Re: go build fails under qemu-ppc64le-static (qemu-user)

[Peter Maydell]

We recently fixed bug #1696773 which was a cause of various crashes or
other problems when trying to run go binaries under linux-user,
including "go build hello.go". So I strongly suspect this is a duplicate
of that bug. Could you test with the QEMU v4.1.0 rc3 or later, please?

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

Title:
  go build fails under qemu-ppc64le-static (qemu-user)

Status in QEMU:
  New

Bug description:
  I am using qemu-user (built static) in a docker container environment.
  When running multi-threaded go commands in the container (go build for
  example) the process may hang, report segfaults or other errors.  I
  built qemu-ppc64le from the upstream git (master).

  I see the problem running on a multi core system with Intel i7 processors.
  # cat /proc/cpuinfo | grep "model name"
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz
  model name: Intel(R) Core(TM) i7-2760QM CPU @ 2.40GHz

  Steps to reproduce:
  1) Build qemu-ppc64le as static and copy into docker build directory named it 
qemu-ppc64le-static.

  2) Add hello.go to docker build dir.

  package main
  import "fmt"
  func main() {
fmt.Println("hello world")
  }

  3) Create the Dockerfile from below:

  FROM ppc64le/golang:1.10.1-alpine3.
  COPY qemu-ppc64le-static /usr/bin/
  COPY hello.go /go

  4) Build container
  $ docker build -t qemutest -f Dockerfile ./go 

  5) Run test
  $ docker run -it qemutest

  /go # /usr/bin/qemu-ppc64le-static --version
  qemu-ppc64le version 2.11.93 (v2.12.0-rc3-dirty)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  /go # go version
  go version go1.10.1 linux/ppc64le

  /go # go build hello.go
  fatal error: fatal error: stopm holding locksunexpected signal during runtime 
execution

  panic during panic
  [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1003528c]

  runtime stack:
  runtime: unexpected return pc for syscall.Syscall6 called from 0xc42007f500
  stack: frame={sp:0xc4203be840, fp:0xc4203be860} 
stack=[0x4000b7ecf0,0x4000b928f0)

  syscall.Syscall6(0x100744e8, 0x3d, 0xc42050c140, 0x20, 0x18, 0x10422b80, 
0xc4203be968[signal , 0x10012d88SIGSEGV: segmentation violation, 0xc420594000 
code=, 0x00x1 addr=0x0 pc=0x1003528c)
  ]

  runtime stack:

/usr/local/go/src/syscall/asm_linux_ppc64x.s:61runtime.throw(0x10472d19, 0x13)
   +/usr/local/go/src/runtime/panic.go:0x6c616 +0x68

  
  runtime.stopm()
/usr/local/go/src/runtime/proc.go:1939goroutine  +10x158
   [runtime.exitsyscall0semacquire(0xc42007f500)
/usr/local/go/src/runtime/proc.go:3129 +]:
  0x130
  runtime.mcall(0xc42007f500)
/usr/local/go/src/runtime/asm_ppc64x.s:183 +0x58sync.runtime_Semacquire
  (0xc4201fab1c)
/usr/local/go/src/runtime/sema.go:56 +0x38

  
  Note the results may differ between attempts,  hangs and other faults 
sometimes happen.
  
  If I run "go: single threaded I don't see the problem, for example:

  /go # GOMAXPROCS=1 go build -p 1 hello.go 
  /go # ./hello
  hello world

  I see the same issue with arm64.  I don't think this is a go issue,
  but don't have a real evidence to prove that.  This problem looks
  similar to other problem I have seen reported against qemu running
  multi-threaded applications.

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

Re: [Qemu-devel] [PATCH] ehci: Ensure that device is not NULL before calling usb_ep_get

[Guenter Roeck]

On Fri, Aug 02, 2019 at 09:46:26AM -0700, Guenter Roeck wrote:
> On Fri, Aug 02, 2019 at 04:11:49PM +0200, Gerd Hoffmann wrote:
> > On Wed, Jul 31, 2019 at 01:08:50PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 7/30/19 7:45 PM, Guenter Roeck wrote:
> > > > The following assert is seen once in a while while resetting the
> > > > Linux kernel.
> > > > 
> > > > qemu-system-x86_64: hw/usb/core.c:734: usb_ep_get:
> > > > Assertion `dev != NULL' failed.
> > > > 
> > > > The call to usb_ep_get() originates from ehci_execute().
> > > > Analysis and debugging shows that p->queue->dev can indeed be NULL
> > > > in this function. Add check for this condition and return an error
> > > > if it is seen.
> > > 
> > > Your patch is not wrong as it corrects your case, but I wonder why we
> > > get there. This assert seems to have catched a bug.
> > 
> > https://bugzilla.redhat.com//show_bug.cgi?id=1715801 maybe.
> > 
> > > Gerd, shouldn't we call usb_packet_cleanup() in ehci_reset() rather than
> > > ehci_finalize()? Then we shouldn't need this patch.
> > 
> > The two ehci_queues_rip_all() calls in ehci_reset() should clean up 
> > everything
> > properly.
> > 
> > Can you try the patch below to see whenever a ehci_find_device failure is 
> > the
> > root cause?
> > 
> > thanks,
> >   Gerd
> > 
> > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> > index 62dab0592fa2..2b0a57772ed5 100644
> > --- a/hw/usb/hcd-ehci.c
> > +++ b/hw/usb/hcd-ehci.c
> > @@ -1644,6 +1644,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState 
> > *ehci, int async)
> >  q->dev = ehci_find_device(q->ehci,
> >get_field(q->qh.epchar, 
> > QH_EPCHAR_DEVADDR));
> >  }
> > +if (q->dev == NULL) {
> > +fprintf(stderr, "%s: device %d not found\n", __func__,
> > +get_field(q->qh.epchar, QH_EPCHAR_DEVADDR));
> > +}
> 
> I had tried that, but this does happen as standard behavior for some
> architectures (I didn't write down where exactly since I thought it
> must be normal). But, sure, I'll add a log message.
> 

With the log message added, I see it a lot when booting riscv64 images
from usb-ehci.

ehci_state_fetchqh: device 0 not found

It looks like this happens for each usb access (a whopping 800+ times
for a simple boot test). So it is definitely a very common condition.
The relevant qemu command line is something like

-usb -device usb-ehci,id=ehci -device usb-storage,bus=ehci.0,drive=d0 \
-drive file=rootfs.ext2,if=none,id=d0,format=raw

The image works fine otherwise, so I thought that the condition is normal.

Guenter

[Qemu-devel] [Bug 1838658] Re: qemu 4.0.0 broken by glib update

[Daniel Berrange]

Doh,  sorry for my comment earlier where I mistakenly used sparc instead
of sparc64.

I've now tested QEMU git master with that sparc64 ISO and qemu-system-
sparc64.

I still can't reproduce it though - it boots past the disk probing, and
into the installer, where it asks for the terminal type.

So looks like there's some further variable involved beyond just the
glib update - perhaps something about the host OS is combining with the
glib update to trigger it.

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

Title:
  qemu 4.0.0 broken by glib update

Status in QEMU:
  New

Bug description:
  In brief, an install CD will successfully boot with qemu 4.0.0 built with 
glib 2.58.3, but freeze during boot with qemu 4.0.0 built with glib 2.60.0. I 
tracked it down to glib's GHashTable improvements. qemu is happy with a glib 
built from
  ```
   git checkout -f 2.60.4
   git revert --no-edit 86c6f7e2b..3bed8a13b
   git revert --no-edit 75f8ec1df9b48b0c3a13a9125f2c7d7c5adf5159
   git revert --no-edit 603fb5958..d3074a748
   git revert --no-edit 0b45ddc55..0600dd322
  ```
  When the GHashTable improvements were committed, there was already a 
preemptive note about any breakage being due to using private implementation 
details, hence mentioning it here rather than with glib.

  For the full saga, see: http://gnats.netbsd.org/54310

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

Re: [Qemu-devel] [PATCH] ehci: Ensure that device is not NULL before calling usb_ep_get

[Guenter Roeck]

On Fri, Aug 02, 2019 at 04:11:49PM +0200, Gerd Hoffmann wrote:
> On Wed, Jul 31, 2019 at 01:08:50PM +0200, Philippe Mathieu-Daudé wrote:
> > On 7/30/19 7:45 PM, Guenter Roeck wrote:
> > > The following assert is seen once in a while while resetting the
> > > Linux kernel.
> > > 
> > > qemu-system-x86_64: hw/usb/core.c:734: usb_ep_get:
> > >   Assertion `dev != NULL' failed.
> > > 
> > > The call to usb_ep_get() originates from ehci_execute().
> > > Analysis and debugging shows that p->queue->dev can indeed be NULL
> > > in this function. Add check for this condition and return an error
> > > if it is seen.
> > 
> > Your patch is not wrong as it corrects your case, but I wonder why we
> > get there. This assert seems to have catched a bug.
> 
> https://bugzilla.redhat.com//show_bug.cgi?id=1715801 maybe.
> 
> > Gerd, shouldn't we call usb_packet_cleanup() in ehci_reset() rather than
> > ehci_finalize()? Then we shouldn't need this patch.
> 
> The two ehci_queues_rip_all() calls in ehci_reset() should clean up everything
> properly.
> 
> Can you try the patch below to see whenever a ehci_find_device failure is the
> root cause?
> 
> thanks,
>   Gerd
> 
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 62dab0592fa2..2b0a57772ed5 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -1644,6 +1644,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, 
> int async)
>  q->dev = ehci_find_device(q->ehci,
>get_field(q->qh.epchar, 
> QH_EPCHAR_DEVADDR));
>  }
> +if (q->dev == NULL) {
> +fprintf(stderr, "%s: device %d not found\n", __func__,
> +get_field(q->qh.epchar, QH_EPCHAR_DEVADDR));
> +}

I had tried that, but this does happen as standard behavior for some
architectures (I didn't write down where exactly since I thought it
must be normal). But, sure, I'll add a log message.

Guenter

>  
>  if (async && (q->qh.epchar & QH_EPCHAR_H)) {
>  

[Qemu-devel] [Bug 1838475] Re: qemu-system-arm exits when cortex-m4 floating point used and irq occurs

[Peter Maydell]

Now fixed in git master; will be in the imminent 4.1 release.


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

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

Title:
  qemu-system-arm exits when cortex-m4 floating point used and irq
  occurs

Status in QEMU:
  Fix Committed

Bug description:
  qemu-system-arm exits with

  "...Secure UsageFault with CFSR.NOCP because NSACR.CP10 prevents stacking FP 
regs
  ...taking pending nonsecure exception 3
  Taking exception 7 [Breakpoint]
  qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)" 

  when emulating Cortex-m4, executing at least 1 floating point
  instruction, and then an irq (e.g. sys tick) occurring.

  CPACR.CP10 and CPACR.CP11 are set to 0x3 respectively prior to
  executing the fp instructions.

  NOTE: NSACR does not appear to be a cortex m4 register.

  Attached is a simplified elf to repro the issue.

  The qemu command line is: "qemu-system-arm --gdb tcp::1234 -cpu
  cortex-m4 -machine lm3s6965evb -nographic -semihosting-config
  enable=on,target=native -kernel QemuExitWhenUsingFPAndIRQOccurs.elf -d
  int"

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

Re: [Qemu-devel] [PATCH v3 1/9] block: add .bdrv_need_rw_file_child_during_reopen_rw handler

[Vladimir Sementsov-Ogievskiy]

02.08.2019 19:23, Vladimir Sementsov-Ogievskiy wrote:
> 02.08.2019 18:42, Kevin Wolf wrote:
>> Am 31.07.2019 um 14:09 hat Max Reitz geschrieben:
>>> On 25.07.19 11:18, Vladimir Sementsov-Ogievskiy wrote:
 On reopen to rw parent may need rw access to child in .prepare, for
 example qcow2 needs to write IN_USE flags into stored bitmaps
 (currently it is done in a hacky way after commit and don't work).
 So, let's introduce such logic.

 The drawback is that in worst case bdrv_reopen_set_read_only may finish
 with error and in some intermediate state: some nodes reopened RW and
 some are not. But this is a way to fix bug around reopening qcow2
 bitmaps in the following commits.
>>>
>>> This commit message doesn’t really explain what this patch does.
>>>
 Signed-off-by: Vladimir Sementsov-Ogievskiy 
 ---
   include/block/block_int.h |   2 +
   block.c   | 144 ++
   2 files changed, 133 insertions(+), 13 deletions(-)

 diff --git a/include/block/block_int.h b/include/block/block_int.h
 index 3aa1e832a8..7bd6fd68dd 100644
 --- a/include/block/block_int.h
 +++ b/include/block/block_int.h
 @@ -531,6 +531,8 @@ struct BlockDriver {
    uint64_t parent_perm, uint64_t 
 parent_shared,
    uint64_t *nperm, uint64_t *nshared);
 + bool (*bdrv_need_rw_file_child_during_reopen_rw)(BlockDriverState 
 *bs);
 +
   /**
    * Bitmaps should be marked as 'IN_USE' in the image on reopening 
 image
    * as rw. This handler should realize it. It also should unset 
 readonly
 diff --git a/block.c b/block.c
 index cbd8da5f3b..3c8e1c59b4 100644
 --- a/block.c
 +++ b/block.c
 @@ -1715,10 +1715,12 @@ static void 
 bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
    uint64_t *shared_perm);
   typedef struct BlockReopenQueueEntry {
 - bool prepared;
 - bool perms_checked;
 - BDRVReopenState state;
 - QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
 +    bool reopened_file_child_rw;
 +    bool changed_file_child_perm_rw;
 +    bool prepared;
 +    bool perms_checked;
 +    BDRVReopenState state;
 +    QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
   } BlockReopenQueueEntry;
   /*
 @@ -3421,6 +3423,105 @@ BlockReopenQueue 
 *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
  keep_old_opts);
   }
 +static int bdrv_reopen_set_read_only_drained(BlockDriverState *bs,
 + bool read_only,
 + Error **errp)
 +{
 +    BlockReopenQueue *queue;
 +    QDict *opts = qdict_new();
 +
 +    qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only);
 +
 +    queue = bdrv_reopen_queue(NULL, bs, opts, true);
 +
 +    return bdrv_reopen_multiple(queue, errp);
 +}
 +
 +/*
 + * handle_recursive_reopens
 + *
 + * On fail it needs rollback_recursive_reopens to be called.
>>>
>>> It would be nice if this description actually said anything about what
>>> the function is supposed to do.
>>>
 + */
 +static int handle_recursive_reopens(BlockReopenQueueEntry *current,
 +    Error **errp)
 +{
 +    int ret;
 +    BlockDriverState *bs = current->state.bs;
 +
 +    /*
 + * We use the fact that in reopen-queue children are always following
 + * parents.
 + * TODO: Switch BlockReopenQueue to be QTAILQ and use
 + *   QTAILQ_FOREACH_REVERSE.
>>>
>>> Why don’t you do that first?  It would make the code more obvious at
>>> least to me.
>>>
 + */
 +    if (QSIMPLEQ_NEXT(current, entry)) {
 +    ret = handle_recursive_reopens(QSIMPLEQ_NEXT(current, entry), 
 errp);
 +    if (ret < 0) {
 +    return ret;
 +    }
 +    }
 +
 +    if ((current->state.flags & BDRV_O_RDWR) && bs->file && bs->drv &&
 +    bs->drv->bdrv_need_rw_file_child_during_reopen_rw &&
 +    bs->drv->bdrv_need_rw_file_child_during_reopen_rw(bs))
 +    {
 +    if (!bdrv_is_writable(bs->file->bs)) {
 +    ret = bdrv_reopen_set_read_only_drained(bs->file->bs, false, 
 errp);
>>>
>>> Hm.  Sorry, I find this all a bit hard to understand.  (No comments and
>>> all.)
>>>
>>> I understand that this is for an RO -> RW transition?  Everything is
>>> still RO, but the parent will need an RW child before it transitions to
>>> RW itself.
>>>
>>>
>>> I’m going to be honest up front, I don’t like this very much.  But I
>>> think it may be a reasonable solution for now.
>>>
>>> As I remember, the problem was that when reopening a qcow2 node 

Re: [Qemu-devel] [PATCH for-4.1] target/arm: Avoid bogus NSACR traps on M-profile without Security Extension

[Peter Maydell]

On Fri, 2 Aug 2019 at 08:51, Damien Hedde  wrote:
>
>
> On 8/1/19 4:38 PM, Peter Maydell wrote:
> > On Thu, 1 Aug 2019 at 15:20, Damien Hedde  
> > wrote:
> >>
> >>
> >> On 8/1/19 12:57 PM, Peter Maydell wrote:
> >>> In Arm v8.0 M-profile CPUs without the Security Extension and also in
> >>> v7M CPUs, there is no NSACR register. However, the code we have to handle
> >>> the FPU does not always check whether the ARM_FEATURE_M_SECURITY bit
> >>> is set before testing whether env->v7m.nsacr permits access to the
> >>> FPU. This means that for a CPU with an FPU but without the Security
> >>> Extension we would always take a bogus fault when trying to stack
> >>> the FPU registers on an exception entry.
> >>>
> >>> We could fix this by adding extra feature bit checks for all uses,
> >>> but it is simpler to just make the internal value of nsacr 0x3ff
> >>
> >> s/0x3ff/0xcff/ I think, given you put 0xcff after and in the code
> >
> > Yes, 0xcff is correct and the commit message is wrong. (Bits 8 and 9
> > of the NSACR are RES0 in all situations.)
> >
>
> Reviewed-by: Damien Hedde 

Thanks; since we need an rc4 anyway I have pushed this to master,
with the commit message typo fixed.

-- PMM

Re: [Qemu-devel] [PATCH v3 07/15] target/arm: Allow SVE to be disabled via a CPU property

[Richard Henderson]

On 8/2/19 5:25 AM, Andrew Jones wrote:
> Since 97a28b0eeac14 ("target/arm: Allow VFP and Neon to be disabled via
> a CPU property") we can disable the 'max' cpu model's VFP and neon
> features, but there's no way to disable SVE. Add the 'sve=on|off'
> property to give it that flexibility. We also rename
> cpu_max_get/set_sve_vq to cpu_max_get/set_sve_max_vq in order for them
> to follow the typical *_get/set_ pattern.
> 
> Signed-off-by: Andrew Jones 
> ---
>  target/arm/cpu.c |  3 ++-
>  target/arm/cpu64.c   | 42 ++--
>  target/arm/monitor.c |  2 +-
>  tests/arm-cpu-features.c |  1 +
>  4 files changed, 40 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 06/15] target/arm/cpu: Use div-round-up to determine predicate register array size

[Richard Henderson]

On 8/2/19 5:25 AM, Andrew Jones wrote:
> Unless we're guaranteed to always increase ARM_MAX_VQ by a multiple of
> four, then we should use DIV_ROUND_UP to ensure we get an appropriate
> array size.
> 
> Signed-off-by: Andrew Jones 
> ---
>  target/arm/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 3/3] hw/mips/mips_jazz: Remove no-longer-necessary override of do_unassigned_access

[Philippe Mathieu-Daudé]

On 8/2/19 6:04 PM, Peter Maydell wrote:
> Now that the MIPS CPU implementation uses the new
> do_transaction_failed hook, we can remove the old code that handled
> the do_unassigned_access hook.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/mips/mips_jazz.c | 27 ---
>  1 file changed, 4 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index c64b4c78809..540943932fd 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -111,18 +111,6 @@ static const MemoryRegionOps dma_dummy_ops = {
>  #define MAGNUM_BIOS_SIZE_MAX 0x7e000
>  #define MAGNUM_BIOS_SIZE (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : 
> MAGNUM_BIOS_SIZE_MAX)
>  
> -static CPUUnassignedAccess real_do_unassigned_access;
> -static void mips_jazz_do_unassigned_access(CPUState *cpu, hwaddr addr,
> -   bool is_write, bool is_exec,
> -   int opaque, unsigned size)
> -{
> -if (!is_exec) {
> -/* ignore invalid access (ie do not raise exception) */
> -return;
> -}
> -(*real_do_unassigned_access)(cpu, addr, is_write, is_exec, opaque, size);
> -}
> -
>  static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr,
>vaddr addr, unsigned size,
>MMUAccessType access_type,
> @@ -184,9 +172,8 @@ static void mips_jazz_init(MachineState *machine,
>   * However, we can't simply add a global memory region to catch
>   * everything, as this would make all accesses including instruction
>   * accesses be ignored and not raise exceptions.
> - * So instead we hijack either the do_unassigned_access method or
> - * the do_transaction_failed method on the CPU, and do not raise 
> exceptions
> - * for data access.
> + * So instead we hijack the do_transaction_failed method on the CPU, and
> + * do not raise exceptions for data access.
>   *
>   * NOTE: this behaviour of raising exceptions for bad instruction
>   * fetches but not bad data accesses was added in commit 54e755588cf1e9
> @@ -197,14 +184,8 @@ static void mips_jazz_init(MachineState *machine,
>   * memory region that catches all memory accesses, as we do on Malta.
>   */
>  cc = CPU_GET_CLASS(cpu);
> -if (cc->do_unassigned_access) {
> -real_do_unassigned_access = cc->do_unassigned_access;
> -cc->do_unassigned_access = mips_jazz_do_unassigned_access;
> -}
> -if (cc->do_transaction_failed) {
> -real_do_transaction_failed = cc->do_transaction_failed;
> -cc->do_transaction_failed = mips_jazz_do_transaction_failed;
> -}
> +real_do_transaction_failed = cc->do_transaction_failed;
> +cc->do_transaction_failed = mips_jazz_do_transaction_failed;
>  
>  /* allocate RAM */
>  memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH 2/3] target/mips: Switch to do_transaction_failed() hook

[Philippe Mathieu-Daudé]

Cc'ing James Hogan.

On 8/2/19 6:04 PM, Peter Maydell wrote:
> Switch the MIPS target from the old unassigned_access hook to the new
> do_transaction_failed hook.
> 
> Unlike the old hook, do_transaction_failed is only ever called from
> the TCG memory access paths, so there is no need for the "ignore this
> if we're using KVM" hack that we were previously using to work around
> the way unassigned_access was called for all kinds of memory accesses
> to unassigned physical addresses.
> 
> The MIPS target does not ever do direct memory reads by physical
> address (via either ldl_phys etc or address_space_ldl etc), so the
> only memory accesses this affects are the 'normal' guest loads and
> stores, which will be handled by the new hook; their behaviour is
> unchanged.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/mips/internal.h  |  8 +---
>  target/mips/cpu.c   |  2 +-
>  target/mips/op_helper.c | 24 
>  3 files changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index b2b41a51ab4..26533bb937d 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -138,9 +138,11 @@ void r4k_helper_tlbinv(CPUMIPSState *env);
>  void r4k_helper_tlbinvf(CPUMIPSState *env);
>  void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);
>  
> -void mips_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> -bool is_write, bool is_exec, int unused,
> -unsigned size);
> +void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> +vaddr addr, unsigned size,
> +MMUAccessType access_type,
> +int mmu_idx, MemTxAttrs attrs,
> +MemTxResult response, uintptr_t retaddr);
>  hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,
>int rw);
>  #endif
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 39eafafc5cd..a79badcb1a6 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -197,7 +197,7 @@ static void mips_cpu_class_init(ObjectClass *c, void 
> *data)
>  cc->gdb_read_register = mips_cpu_gdb_read_register;
>  cc->gdb_write_register = mips_cpu_gdb_write_register;
>  #ifndef CONFIG_USER_ONLY
> -cc->do_unassigned_access = mips_cpu_unassigned_access;
> +cc->do_transaction_failed = mips_cpu_do_transaction_failed;
>  cc->do_unaligned_access = mips_cpu_do_unaligned_access;
>  cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
>  cc->vmsd = _mips_cpu;
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 9e2e02f8586..65ff9f5b935 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -2666,27 +2666,19 @@ void mips_cpu_do_unaligned_access(CPUState *cs, vaddr 
> addr,
>  do_raise_exception_err(env, excp, error_code, retaddr);
>  }
>  
> -void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> -bool is_write, bool is_exec, int unused,
> -unsigned size)
> +void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> +vaddr addr, unsigned size,
> +MMUAccessType access_type,
> +int mmu_idx, MemTxAttrs attrs,
> +MemTxResult response, uintptr_t retaddr)
>  {
>  MIPSCPU *cpu = MIPS_CPU(cs);
>  CPUMIPSState *env = >env;
>  
> -/*
> - * Raising an exception with KVM enabled will crash because it won't be 
> from
> - * the main execution loop so the longjmp won't have a matching setjmp.
> - * Until we can trigger a bus error exception through KVM lets just 
> ignore
> - * the access.
> - */
> -if (kvm_enabled()) {
> -return;

Reviewed-by: Philippe Mathieu-Daudé 

> -}
> -
> -if (is_exec) {
> -raise_exception(env, EXCP_IBE);
> +if (access_type == MMU_INST_FETCH) {
> +do_raise_exception(env, EXCP_IBE, retaddr);
>  } else {
> -raise_exception(env, EXCP_DBE);
> +do_raise_exception(env, EXCP_DBE, retaddr);
>  }
>  }
>  #endif /* !CONFIG_USER_ONLY */
> 

Re: [Qemu-devel] [PATCH 0/3] target/mips: Convert to do_transaction_failed hook

[Philippe Mathieu-Daudé]

Cc'ing broader MIPS audience.

On 8/2/19 6:04 PM, Peter Maydell wrote:
> This patchset converts the MIPS target away from the
> old broken do_unassigned_access hook to the new (added in
> 2017...) do_transaction_failed hook.
> 
> The motivation here is:
>  * do_unassigned_access is broken because:
> + it will be called for any kind of access to physical addresses
>   where there is no assigned device, whether that access is by the
>   CPU or by something else (like a DMA controller!), so it can
>   result in spurious guest CPU exceptions.
> + It will also get called even when using KVM, when there's nothing
>   useful it can do.
> + It isn't passed in the return-address within the TCG generated
>   code, so it isn't able to correctly restore the CPU state
>   before generating the exception, and so the exception will
>   often be generated with the wrong faulting guest PC value
>  * there are now only a few targets still using the old hook,
>so if we can convert them we can delete all the old code
>and complete this API transation. (Patches for SPARC are on
>the list; the other user is RISCV, which accidentally
>implemented the old hook rather than the new one recently.)
> 
> The general approach to the conversion is to check the target for
> load/store-by-physical-address operations which were previously
> implicitly causing exceptions, to see if they now need to explicitly
> check for and handle memory access failures. (The 'git grep' regexes
> in docs/devel/loads-stores.rst are useful here: the API families to
> look for are ld*_phys/st*_phys, address_space_ld/st*, and
> cpu_physical_memory*.)
> 
> For MIPS, there are none of these (the usual place where targets do
> this is hardware page table walks where the page table entries are
> loaded by physical address, and MIPS doesn't seem to have those).
> 
> Code audit out of the way, the actual hook changeover is pretty
> simple.
> 
> The complication here is the MIPS Jazz board, which has some rather
> dubious code that intercepts the do_unassigned_access hook to suppress
> generation of exceptions for invalid accesses due to data accesses,
> while leaving exceptions for invalid instruction fetches in place. I'm
> a bit dubious about whether the behaviour we have implemented here is
> really what the hardware does -- it seems pretty odd to me to not
> generate exceptions for d-side accesses but to generate them for
> i-side accesses, and looking back through git and mailing list history
> this code is here mainly as "put back the behaviour we had before a
> previous commit broke it", and that older behaviour in turn I think is
> more historical-accident than because anybody deliberately checked the
> hardware behaviour and made QEMU work that way. However, I don't have
> any real hardware to do comparative tests on, so this series retains
> the same behaviour we had before on this board, by making it intercept
> the new hook in the same way it did the old one. I've beefed up the
> comment somewhat to indicate what we're doing, why, and why it might
> not be right.
> 
> The patch series is structured in three parts:
>  * make the Jazz board code support CPUs regardless of which
>of the two hooks they implement
>  * switch the MIPS CPUs over to implementing the new hook
>  * remove the no-longer-needed Jazz board code for the old
>hook
> (This seemed cleaner to me than squashing the whole thing into
> a single patch that touched core mips code and the jazz board
> at the same time.)
> 
> I have tested this with:
>  * the ARC Multiboot BIOS linked to from the bug
>https://bugs.launchpad.net/qemu/+bug/1245924 (and which
>was the test case for needing the hook intercept)
>  * a Linux kernel for the 'mips' mips r4k machine
>  * 'make check'
> Obviously more extensive testing would be useful, but I
> don't have any other test images. I also don't have
> a KVM MIPS host, which would be worth testing to confirm
> that it also still works.
> 
> If anybody happens by some chance to still have a working
> real-hardware Magnum or PICA61 board, we could perhaps test
> how it handles accesses to invalid memory, but I suspect that
> nobody does any more :-)
> 
> thanks
> -- PMM
> 
> 
> Peter Maydell (3):
>   hw/mips/mips_jazz: Override do_transaction_failed hook
>   target/mips: Switch to do_transaction_failed() hook
>   hw/mips/mips_jazz: Remove no-longer-necessary override of
> do_unassigned_access
> 
>  target/mips/internal.h  |  8 ---
>  hw/mips/mips_jazz.c | 47 +
>  target/mips/cpu.c   |  2 +-
>  target/mips/op_helper.c | 24 +++--
>  4 files changed, 47 insertions(+), 34 deletions(-)
> 

Re: [Qemu-devel] [PATCH v3 03/15] target/arm/monitor: Introduce qmp_query_cpu_model_expansion

[Richard Henderson]

On 8/2/19 5:25 AM, Andrew Jones wrote:
> Note, certainly more features may be added to the list of
> advertised features, e.g. 'vfp' and 'neon'. The only requirement
> is that their property set accessors fail when invalid
> configurations are detected. For vfp we would need something like
> 
>  set_vfp()
>  {
>if (arm_feature(env, ARM_FEATURE_AARCH64) &&
>cpu->has_vfp != cpu->has_neon)
>error("AArch64 CPUs must have both VFP and Neon or neither")
> 
> in its set accessor, and the same for neon, rather than doing that
> check at realize time, which isn't executed at qmp query time.

How could this succeed?  Either set_vfp or set_neon would be called first, at
which point the two features are temporarily out of sync, but the error would
trigger anyway.

This would seem to require some separate "qmp validate" step that is processed
after a collection of properties are set.

I was about to say something about this being moot until someone actually wants
to be able to disable vfp+neon on aarch64, but then...

> +A note about CPU feature dependencies
> +-
> +
> +It's possible for features to have dependencies on other features. I.e.
> +it may be possible to change one feature at a time without error, but
> +when attempting to change all features at once an error could occur
> +depending on the order they are processed.  It's also possible changing
> +all at once doesn't generate an error, because a feature's dependencies
> +are satisfied with other features, but the same feature cannot be changed
> +independently without error.  For these reasons callers should always
> +attempt to make their desired changes all at once in order to ensure the
> +collection is valid.

... this language makes me think that you've already encountered an ordering
problem that might be better solved with a separate validate step?

The actual code looks good.
Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [Bug 1838658] Re: qemu 4.0.0 broken by glib update

[Andreas Gustafsson]

> The test image that the netbsd bug points to no longer exists.

Please try this one instead:

  https://www.gson.org/bugs/qemu/NetBSD-8.99.47-sparc64.iso

I just verified that this image works for reproducing the bug.

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

Title:
  qemu 4.0.0 broken by glib update

Status in QEMU:
  New

Bug description:
  In brief, an install CD will successfully boot with qemu 4.0.0 built with 
glib 2.58.3, but freeze during boot with qemu 4.0.0 built with glib 2.60.0. I 
tracked it down to glib's GHashTable improvements. qemu is happy with a glib 
built from
  ```
   git checkout -f 2.60.4
   git revert --no-edit 86c6f7e2b..3bed8a13b
   git revert --no-edit 75f8ec1df9b48b0c3a13a9125f2c7d7c5adf5159
   git revert --no-edit 603fb5958..d3074a748
   git revert --no-edit 0b45ddc55..0600dd322
  ```
  When the GHashTable improvements were committed, there was already a 
preemptive note about any breakage being due to using private implementation 
details, hence mentioning it here rather than with glib.

  For the full saga, see: http://gnats.netbsd.org/54310

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

Re: [Qemu-devel] [PATCH v3 1/9] block: add .bdrv_need_rw_file_child_during_reopen_rw handler

[Vladimir Sementsov-Ogievskiy]

02.08.2019 18:42, Kevin Wolf wrote:
> Am 31.07.2019 um 14:09 hat Max Reitz geschrieben:
>> On 25.07.19 11:18, Vladimir Sementsov-Ogievskiy wrote:
>>> On reopen to rw parent may need rw access to child in .prepare, for
>>> example qcow2 needs to write IN_USE flags into stored bitmaps
>>> (currently it is done in a hacky way after commit and don't work).
>>> So, let's introduce such logic.
>>>
>>> The drawback is that in worst case bdrv_reopen_set_read_only may finish
>>> with error and in some intermediate state: some nodes reopened RW and
>>> some are not. But this is a way to fix bug around reopening qcow2
>>> bitmaps in the following commits.
>>
>> This commit message doesn’t really explain what this patch does.
>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>   include/block/block_int.h |   2 +
>>>   block.c   | 144 ++
>>>   2 files changed, 133 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>>> index 3aa1e832a8..7bd6fd68dd 100644
>>> --- a/include/block/block_int.h
>>> +++ b/include/block/block_int.h
>>> @@ -531,6 +531,8 @@ struct BlockDriver {
>>>uint64_t parent_perm, uint64_t parent_shared,
>>>uint64_t *nperm, uint64_t *nshared);
>>>   
>>> + bool (*bdrv_need_rw_file_child_during_reopen_rw)(BlockDriverState 
>>> *bs);
>>> +
>>>   /**
>>>* Bitmaps should be marked as 'IN_USE' in the image on reopening 
>>> image
>>>* as rw. This handler should realize it. It also should unset 
>>> readonly
>>> diff --git a/block.c b/block.c
>>> index cbd8da5f3b..3c8e1c59b4 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -1715,10 +1715,12 @@ static void 
>>> bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
>>>uint64_t *shared_perm);
>>>   
>>>   typedef struct BlockReopenQueueEntry {
>>> - bool prepared;
>>> - bool perms_checked;
>>> - BDRVReopenState state;
>>> - QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
>>> +bool reopened_file_child_rw;
>>> +bool changed_file_child_perm_rw;
>>> +bool prepared;
>>> +bool perms_checked;
>>> +BDRVReopenState state;
>>> +QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
>>>   } BlockReopenQueueEntry;
>>>   
>>>   /*
>>> @@ -3421,6 +3423,105 @@ BlockReopenQueue 
>>> *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
>>>  keep_old_opts);
>>>   }
>>>   
>>> +static int bdrv_reopen_set_read_only_drained(BlockDriverState *bs,
>>> + bool read_only,
>>> + Error **errp)
>>> +{
>>> +BlockReopenQueue *queue;
>>> +QDict *opts = qdict_new();
>>> +
>>> +qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only);
>>> +
>>> +queue = bdrv_reopen_queue(NULL, bs, opts, true);
>>> +
>>> +return bdrv_reopen_multiple(queue, errp);
>>> +}
>>> +
>>> +/*
>>> + * handle_recursive_reopens
>>> + *
>>> + * On fail it needs rollback_recursive_reopens to be called.
>>
>> It would be nice if this description actually said anything about what
>> the function is supposed to do.
>>
>>> + */
>>> +static int handle_recursive_reopens(BlockReopenQueueEntry *current,
>>> +Error **errp)
>>> +{
>>> +int ret;
>>> +BlockDriverState *bs = current->state.bs;
>>> +
>>> +/*
>>> + * We use the fact that in reopen-queue children are always following
>>> + * parents.
>>> + * TODO: Switch BlockReopenQueue to be QTAILQ and use
>>> + *   QTAILQ_FOREACH_REVERSE.
>>
>> Why don’t you do that first?  It would make the code more obvious at
>> least to me.
>>
>>> + */
>>> +if (QSIMPLEQ_NEXT(current, entry)) {
>>> +ret = handle_recursive_reopens(QSIMPLEQ_NEXT(current, entry), 
>>> errp);
>>> +if (ret < 0) {
>>> +return ret;
>>> +}
>>> +}
>>> +
>>> +if ((current->state.flags & BDRV_O_RDWR) && bs->file && bs->drv &&
>>> +bs->drv->bdrv_need_rw_file_child_during_reopen_rw &&
>>> +bs->drv->bdrv_need_rw_file_child_during_reopen_rw(bs))
>>> +{
>>> +if (!bdrv_is_writable(bs->file->bs)) {
>>> +ret = bdrv_reopen_set_read_only_drained(bs->file->bs, false, 
>>> errp);
>>
>> Hm.  Sorry, I find this all a bit hard to understand.  (No comments and
>> all.)
>>
>> I understand that this is for an RO -> RW transition?  Everything is
>> still RO, but the parent will need an RW child before it transitions to
>> RW itself.
>>
>>
>> I’m going to be honest up front, I don’t like this very much.  But I
>> think it may be a reasonable solution for now.
>>
>> As I remember, the problem was that when reopening a qcow2 node from RO
>> to RW, we need to write something in .prepare() (because it can fail),
>> but naturally no .prepare() is called after any .commit(), so no matter
>> 

Re: [Qemu-devel] [PATCH 1/3] hw/mips/mips_jazz: Override do_transaction_failed hook

[Philippe Mathieu-Daudé]

On 8/2/19 6:04 PM, Peter Maydell wrote:
> The MIPS Jazz ('magnum' and 'pica61') boards have some code which
> overrides the CPU's do_unassigned_access hook, so they can intercept
> it and not raise exceptions on data accesses to invalid addresses,
> only for instruction fetches.
> 
> We want to switch MIPS over to using the do_transaction_failed
> hook instead, so add an intercept for that as well, and make
> the board code install whichever hook the CPU is actually using.
> Once we've changed the CPU implementation we can remove the
> redundant code for the old hook.
> 
> Note: I am suspicious that the behaviour as implemented here may not
> be what the hardware really does.  It was added in commit
> 54e755588cf1e90f0b14 to restore the behaviour that was broken by
> commit c658b94f6e8c206c59d.  But prior to commit c658b94f6e8c206c59d
> every MIPS board generated exceptions for instruction access to
> invalid addresses but not for data accesses; and other boards,
> notably Malta, were fixed by making all invalid accesses behave as
> reads-as-zero (see the call to empty_slot_init() in
> mips_malta_init()).  Hardware that raises exceptions for instruction
> access and not data access seems to me to be an unlikely design, and
> it's possible that the right way to emulate this is to make the Jazz
> boards do what we did with Malta (or some variation of that).
> Nonetheless, since I don't have access to real hardware to test
> against I have taken the approach of "make QEMU continue to behave
> the same way it did before this commit".  I have updated the comment
> to correct the parts that are no longer accurate and note that
> the hardware might behave differently.
> 
> The test case for the need for the hook-hijacking is in
> https://bugs.launchpad.net/qemu/+bug/1245924 That BIOS will boot OK
> either with this overriding of both hooks, or with a simple "global
> memory region to ignore bad accesses of all types", so it doesn't
> provide evidence either way, unfortunately.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/mips/mips_jazz.c | 54 ++---
>  1 file changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index fa8775d4284..c64b4c78809 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -123,6 +123,28 @@ static void mips_jazz_do_unassigned_access(CPUState 
> *cpu, hwaddr addr,
>  (*real_do_unassigned_access)(cpu, addr, is_write, is_exec, opaque, size);
>  }
>  
> +static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr,
> +  vaddr addr, unsigned size,
> +  MMUAccessType access_type,
> +  int mmu_idx, MemTxAttrs attrs,
> +  MemTxResult response,
> +  uintptr_t retaddr);
> +
> +static void mips_jazz_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> +vaddr addr, unsigned size,
> +MMUAccessType access_type,
> +int mmu_idx, MemTxAttrs attrs,
> +MemTxResult response,
> +uintptr_t retaddr)
> +{
> +if (access_type != MMU_INST_FETCH) {
> +/* ignore invalid access (ie do not raise exception) */
> +return;
> +}
> +(*real_do_transaction_failed)(cs, physaddr, addr, size, access_type,
> +  mmu_idx, attrs, response, retaddr);
> +}
> +
>  static void mips_jazz_init(MachineState *machine,
> enum jazz_model_e jazz_model)
>  {
> @@ -157,16 +179,32 @@ static void mips_jazz_init(MachineState *machine,
>  env = >env;
>  qemu_register_reset(main_cpu_reset, cpu);
>  
> -/* Chipset returns 0 in invalid reads and do not raise data exceptions.
> +/*
> + * Chipset returns 0 in invalid reads and do not raise data exceptions.
>   * However, we can't simply add a global memory region to catch
> - * everything, as memory core directly call unassigned_mem_read/write
> - * on some invalid accesses, which call do_unassigned_access on the
> - * CPU, which raise an exception.
> - * Handle that case by hijacking the do_unassigned_access method on
> - * the CPU, and do not raise exceptions for data access. */
> + * everything, as this would make all accesses including instruction
> + * accesses be ignored and not raise exceptions.
> + * So instead we hijack either the do_unassigned_access method or
> + * the do_transaction_failed method on the CPU, and do not raise 
> exceptions
> + * for data access.
> + *
> + * NOTE: this behaviour of raising exceptions for bad instruction
> + * fetches but not bad data accesses was added in commit 

Re: [Qemu-devel] [PATCH v2 0/9] add failover feature for assigned network devices

[no-reply]

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



Hi,

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

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

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==10754==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==10817==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==10817==WARNING: ASan is ignoring requested __asan_handle_no_return: stack 
top: 0x7fffbb748000; bottom 0x7ff8aa6f8000; size: 0x00071105 (30350311424)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
==10836==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 25 test-aio /aio-gsource/event/wait
PASS 26 test-aio /aio-gsource/event/flush
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
==10845==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
==10851==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 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" 
==10860==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==10857==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
PASS 2 test-aio-multithread /aio/multi/schedule
==10878==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==10889==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
==10900==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
==10906==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 7 ide-test /x86_64/ide/bmdma/long_prdt
==10912==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
==10912==WARNING: ASan is ignoring requested __asan_handle_no_return: stack 
top: 0x7ffc98368000; bottom 0x7fbd2f3fe000; size: 0x003f68f6a000 (272343932928)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 9 ide-test /x86_64/ide/flush/nodev
==10933==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
PASS 10 ide-test /x86_64/ide/flush/empty_drive
==10940==WARNING: ASan doesn't fully support 

Re: [Qemu-devel] [PATCH for 4.2 v5 10/12] target/mips: Style improvements in machine.c

[Philippe Mathieu-Daudé]

On 8/1/19 6:45 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Fixes mostly errors and warings reported by 'checkpatch.pl -f'.

"warnings"

> 
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/machine.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/machine.c b/target/mips/machine.c
> index e23b767..06887d8 100644
> --- a/target/mips/machine.c
> +++ b/target/mips/machine.c
> @@ -25,7 +25,7 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size,
>  int i;
>  fpr_t *v = pv;
>  /* Restore entire MSA vector register */
> -for (i = 0; i < MSA_WRLEN/64; i++) {
> +for (i = 0; i < MSA_WRLEN / 64; i++) {
>  qemu_get_sbe64s(f, >wr.d[i]);
>  }
>  return 0;
> @@ -37,7 +37,7 @@ static int put_fpr(QEMUFile *f, void *pv, size_t size,
>  int i;
>  fpr_t *v = pv;
>  /* Save entire MSA vector register */
> -for (i = 0; i < MSA_WRLEN/64; i++) {
> +for (i = 0; i < MSA_WRLEN / 64; i++) {
>  qemu_put_sbe64s(f, >wr.d[i]);
>  }
>  
> 

Re: [Qemu-devel] [PATCH for 4.2 v5 09/12] target/mips: Style improvements in internal.h

[Philippe Mathieu-Daudé]

On 8/1/19 6:45 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Fixes mostly errors and warings reported by 'checkpatch.pl -f'.

"warnings"

> 
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/internal.h | 57 
> +++---
>  1 file changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index d9216fb..836de7a 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -1,4 +1,5 @@
> -/* mips internal definitions and helpers
> +/*
> + * MIPS internal definitions and helpers
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.
> @@ -8,8 +9,10 @@
>  #define MIPS_INTERNAL_H
>  
>  
> -/* MMU types, the first four entries have the same layout as the
> -   CP0C0_MT field.  */
> +/*
> + * MMU types, the first four entries have the same layout as the
> + * CP0C0_MT field.
> + */
>  enum mips_mmu_types {
>  MMU_TYPE_NONE,
>  MMU_TYPE_R4000,
> @@ -159,9 +162,11 @@ static inline bool 
> cpu_mips_hw_interrupts_enabled(CPUMIPSState *env)
>  !(env->CP0_Status & (1 << CP0St_EXL)) &&
>  !(env->CP0_Status & (1 << CP0St_ERL)) &&
>  !(env->hflags & MIPS_HFLAG_DM) &&
> -/* Note that the TCStatus IXMT field is initialized to zero,
> -   and only MT capable cores can set it to one. So we don't
> -   need to check for MT capabilities here.  */
> +/*
> + * Note that the TCStatus IXMT field is initialized to zero,
> + * and only MT capable cores can set it to one. So we don't
> + * need to check for MT capabilities here.
> + */
>  !(env->active_tc.CP0_TCStatus & (1 << CP0TCSt_IXMT));
>  }
>  
> @@ -176,14 +181,18 @@ static inline bool 
> cpu_mips_hw_interrupts_pending(CPUMIPSState *env)
>  status = env->CP0_Status & CP0Ca_IP_mask;
>  
>  if (env->CP0_Config3 & (1 << CP0C3_VEIC)) {
> -/* A MIPS configured with a vectorizing external interrupt controller
> -   will feed a vector into the Cause pending lines. The core treats
> -   the status lines as a vector level, not as indiviual masks.  */
> +/*
> + * A MIPS configured with a vectorizing external interrupt controller
> + * will feed a vector into the Cause pending lines. The core treats
> + * the status lines as a vector level, not as indiviual masks.
> + */
>  r = pending > status;
>  } else {
> -/* A MIPS configured with compatibility or VInt (Vectored Interrupts)
> -   treats the pending lines as individual interrupt lines, the status
> -   lines are individual masks.  */
> +/*
> + * A MIPS configured with compatibility or VInt (Vectored Interrupts)
> + * treats the pending lines as individual interrupt lines, the status
> + * lines are individual masks.
> + */
>  r = (pending & status) != 0;
>  }
>  return r;
> @@ -268,12 +277,14 @@ static inline int mips_vpe_active(CPUMIPSState *env)
>  active = 0;
>  }
>  
> -/* Now verify that there are active thread contexts in the VPE.
> -
> -   This assumes the CPU model will internally reschedule threads
> -   if the active one goes to sleep. If there are no threads available
> -   the active one will be in a sleeping state, and we can turn off
> -   the entire VPE.  */
> +/*
> + * Now verify that there are active thread contexts in the VPE.
> + *
> + * This assumes the CPU model will internally reschedule threads
> + * if the active one goes to sleep. If there are no threads available
> + * the active one will be in a sleeping state, and we can turn off
> + * the entire VPE.
> + */
>  if (!(env->active_tc.CP0_TCStatus & (1 << CP0TCSt_A))) {
>  /* TC is not activated.  */
>  active = 0;
> @@ -396,10 +407,12 @@ static inline void compute_hflags(CPUMIPSState *env)
>  env->hflags |= MIPS_HFLAG_COP1X;
>  }
>  } else if (env->insn_flags & ISA_MIPS4) {
> -/* All supported MIPS IV CPUs use the XX (CU3) to enable
> -   and disable the MIPS IV extensions to the MIPS III ISA.
> -   Some other MIPS IV CPUs ignore the bit, so the check here
> -   would be too restrictive for them.  */
> +/*
> + * All supported MIPS IV CPUs use the XX (CU3) to enable
> + * and disable the MIPS IV extensions to the MIPS III ISA.
> + * Some other MIPS IV CPUs ignore the bit, so the check here
> + * would be too restrictive for them.
> + */
>  if (env->CP0_Status & (1U << CP0St_CU3)) {
>  env->hflags |= MIPS_HFLAG_COP1X;
>  }
> 

Re: [Qemu-devel] [PATCH for 4.2 v5 07/12] target/mips: Style improvements in cpu.c

[Philippe Mathieu-Daudé]

On 8/1/19 6:45 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Fixes mostly errors and warings reported by 'checkpatch.pl -f'.

"warnings"

> 
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/cpu.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 39eafaf..3ffa342 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -57,9 +57,11 @@ static bool mips_cpu_has_work(CPUState *cs)
>  CPUMIPSState *env = >env;
>  bool has_work = false;
>  
> -/* Prior to MIPS Release 6 it is implementation dependent if non-enabled
> -   interrupts wake-up the CPU, however most of the implementations only
> -   check for interrupts that can be taken. */
> +/*
> + * Prior to MIPS Release 6 it is implementation dependent if non-enabled
> + * interrupts wake-up the CPU, however most of the implementations only
> + * check for interrupts that can be taken.
> + */
>  if ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
>  cpu_mips_hw_interrupts_pending(env)) {
>  if (cpu_mips_hw_interrupts_enabled(env) ||
> @@ -70,8 +72,10 @@ static bool mips_cpu_has_work(CPUState *cs)
>  
>  /* MIPS-MT has the ability to halt the CPU.  */
>  if (env->CP0_Config3 & (1 << CP0C3_MT)) {
> -/* The QEMU model will issue an _WAKE request whenever the CPUs
> -   should be woken up.  */
> +/*
> + * The QEMU model will issue an _WAKE request whenever the CPUs
> + * should be woken up.
> + */
>  if (cs->interrupt_request & CPU_INTERRUPT_WAKE) {
>  has_work = true;
>  }
> @@ -112,7 +116,8 @@ static void mips_cpu_reset(CPUState *s)
>  #endif
>  }
>  
> -static void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info) {
> +static void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info)
> +{
>  MIPSCPU *cpu = MIPS_CPU(s);
>  CPUMIPSState *env = >env;
>  
> 

Re: [Qemu-devel] [PATCH for 4.2 v5 06/12] target/mips: Style improvements in cp0_timer.c

[Philippe Mathieu-Daudé]

On 8/1/19 6:45 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Fixes mostly errors and warings reported by 'checkpatch.pl -f'.

"warnings"

> 
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/cp0_timer.c | 42 +++---
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
> index f471639..b5f3560 100644
> --- a/target/mips/cp0_timer.c
> +++ b/target/mips/cp0_timer.c
> @@ -29,7 +29,7 @@
>  #define TIMER_PERIOD 10 /* 10 ns period for 100 Mhz frequency */
>  
>  /* XXX: do not use a global */
> -uint32_t cpu_mips_get_random (CPUMIPSState *env)
> +uint32_t cpu_mips_get_random(CPUMIPSState *env)
>  {
>  static uint32_t seed = 1;
>  static uint32_t prev_idx = 0;
> @@ -42,8 +42,10 @@ uint32_t cpu_mips_get_random (CPUMIPSState *env)
>  
>  /* Don't return same value twice, so get another value */
>  do {
> -/* Use a simple algorithm of Linear Congruential Generator
> - * from ISO/IEC 9899 standard. */
> +/*
> + * Use a simple algorithm of Linear Congruential Generator
> + * from ISO/IEC 9899 standard.
> + */
>  seed = 1103515245 * seed + 12345;
>  idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
>  } while (idx == prev_idx);
> @@ -73,7 +75,7 @@ static void cpu_mips_timer_expire(CPUMIPSState *env)
>  qemu_irq_raise(env->irq[(env->CP0_IntCtl >> CP0IntCtl_IPTI) & 0x7]);
>  }
>  
> -uint32_t cpu_mips_get_count (CPUMIPSState *env)
> +uint32_t cpu_mips_get_count(CPUMIPSState *env)
>  {
>  if (env->CP0_Cause & (1 << CP0Ca_DC)) {
>  return env->CP0_Count;
> @@ -91,16 +93,16 @@ uint32_t cpu_mips_get_count (CPUMIPSState *env)
>  }
>  }
>  
> -void cpu_mips_store_count (CPUMIPSState *env, uint32_t count)
> +void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
>  {
>  /*
>   * This gets called from cpu_state_reset(), potentially before timer 
> init.
>   * So env->timer may be NULL, which is also the case with KVM enabled so
>   * treat timer as disabled in that case.
>   */
> -if (env->CP0_Cause & (1 << CP0Ca_DC) || !env->timer)
> +if (env->CP0_Cause & (1 << CP0Ca_DC) || !env->timer) {
>  env->CP0_Count = count;
> -else {
> +} else {
>  /* Store new count register */
>  env->CP0_Count = count -
> (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / 
> TIMER_PERIOD);
> @@ -109,13 +111,15 @@ void cpu_mips_store_count (CPUMIPSState *env, uint32_t 
> count)
>  }
>  }
>  
> -void cpu_mips_store_compare (CPUMIPSState *env, uint32_t value)
> +void cpu_mips_store_compare(CPUMIPSState *env, uint32_t value)
>  {
>  env->CP0_Compare = value;
> -if (!(env->CP0_Cause & (1 << CP0Ca_DC)))
> +if (!(env->CP0_Cause & (1 << CP0Ca_DC))) {
>  cpu_mips_timer_update(env);
> -if (env->insn_flags & ISA_MIPS32R2)
> +}
> +if (env->insn_flags & ISA_MIPS32R2) {
>  env->CP0_Cause &= ~(1 << CP0Ca_TI);
> +}
>  qemu_irq_lower(env->irq[(env->CP0_IntCtl >> CP0IntCtl_IPTI) & 0x7]);
>  }
>  
> @@ -131,27 +135,27 @@ void cpu_mips_stop_count(CPUMIPSState *env)
>   TIMER_PERIOD);
>  }
>  
> -static void mips_timer_cb (void *opaque)
> +static void mips_timer_cb(void *opaque)
>  {
>  CPUMIPSState *env;
>  
>  env = opaque;
> -#if 0
> -qemu_log("%s\n", __func__);
> -#endif
>  
> -if (env->CP0_Cause & (1 << CP0Ca_DC))
> +if (env->CP0_Cause & (1 << CP0Ca_DC)) {
>  return;
> +}
>  
> -/* ??? This callback should occur when the counter is exactly equal to
> -   the comparator value.  Offset the count by one to avoid immediately
> -   retriggering the callback before any virtual time has passed.  */
> +/*
> + * ??? This callback should occur when the counter is exactly equal to
> + * the comparator value.  Offset the count by one to avoid immediately
> + * retriggering the callback before any virtual time has passed.
> + */
>  env->CP0_Count++;
>  cpu_mips_timer_expire(env);
>  env->CP0_Count--;
>  }
>  
> -void cpu_mips_clock_init (MIPSCPU *cpu)
> +void cpu_mips_clock_init(MIPSCPU *cpu)
>  {
>  CPUMIPSState *env = >env;
>  
> 

Re: [Qemu-devel] [PATCH v4 1/4] block: introducing 'bdrv_co_delete_file' interface

[Kevin Wolf]

Am 28.06.2019 um 21:45 hat Daniel Henrique Barboza geschrieben:
> Adding to Block Drivers the capability of being able to clean up
> its created files can be useful in certain situations. For the
> LUKS driver, for instance, a failure in one of its authentication
> steps can leave files in the host that weren't there before.
> 
> This patch adds the 'bdrv_co_delete_file' interface to block
> drivers and add it to the 'file' driver in file-posix.c. The
> implementation is given by 'raw_co_delete_file'. The helper
> 'bdrv_path_is_regular_file' is being used only in raw_co_delete_file
> at this moment, but it will be used inside LUKS in a later patch.
> Foreseeing this future use, let's put it in block.c and make it
> public.
> 
> Suggested-by: Daniel P. Berrangé 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  block.c   | 11 +++
>  block/file-posix.c| 28 
>  include/block/block.h |  1 +
>  include/block/block_int.h |  6 ++
>  4 files changed, 46 insertions(+)
> 
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -309,6 +309,12 @@ struct BlockDriver {
>   */
>  int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
>  
> +/*
> + * Delete a local created file.
> + */
> +int coroutine_fn (*bdrv_co_delete_file)(const char *filename,
> +Error **errp);

I wonder if it wouldn't make more sense to pass a BlockDriverState
instead of a filename. In the create options we usually have a BDS
around, so it should be possible to use.

The only case where it wouldn't work is if creating the image file
worked, but bdrv_open() fails. I think this is unlikely, and it's even
more unlikely that unlinking such a file would then work, so I wouldn't
see it as a problem.

The reason why I'm suggesting this is that we've spent a lot of time in
the past years to change open and create to an interface that doesn't
use only filenames, because filenames cover only part of the possibe
block devices.

So I'm kind of hesitant to add a new interface that can only use
filenames, while we know that filenames just don't quite cut it in the
general case - especially if using a BDS, which already has all the
information needed, is possible as an alternative.

>  /*
>   * Flushes all data that was already written to the OS all the way down 
> to
>   * the disk (for example file-posix.c calls fsync()).
> -- 
> 2.20.1
> 
> diff --git a/block.c b/block.c
> index c139540f2b..6e2b0f528d 100644
> --- a/block.c
> +++ b/block.c
> @@ -621,6 +621,17 @@ int get_tmp_filename(char *filename, int size)
>  #endif
>  }
>  
> +/**
> + * Helper that checks if a given string represents a regular
> + * local file.
> + */
> +bool bdrv_path_is_regular_file(const char *path)
> +{
> +struct stat st;
> +
> +return (stat(path, ) == 0) && S_ISREG(st.st_mode);
> +}
> +
>  /*
>   * Detect host devices. By convention, /dev/cdrom[N] is always
>   * recognized as a host CDROM.

This hunk isn't generic, it belong in file-posix.c

Kevin

Re: [Qemu-devel] [PATCH v7] qemu-io: add pattern file for write command

[Max Reitz]

On 05.07.19 12:21, Denis Plotnikov wrote:
> The patch allows to provide a pattern file for write
> command. There was no similar ability before.
> 
> Signed-off-by: Denis Plotnikov 
> ---
> v7:
>   * fix variable naming
>   * make code more readable
>   * extend help for write command
> 
> v6:
>   * the pattern file is read once to reduce io
> 
> v5:
>   * file name initiated with null to make compilers happy
> 
> v4:
>   * missing signed-off clause added
> 
> v3:
>   * missing file closing added
>   * exclusive flags processing changed
>   * buffer void* converted to char* to fix pointer arithmetics
>   * file reading error processing added
> ---
>  qemu-io-cmds.c | 86 ++
>  1 file changed, 80 insertions(+), 6 deletions(-)
> 
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 09750a23ce..495170380a 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -343,6 +343,66 @@ static void *qemu_io_alloc(BlockBackend *blk, size_t 
> len, int pattern)
>  return buf;
>  }
>  
> +static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
> + char *file_name)

Wouldn’t a const char * suffice?

> +{
> +char *buf, *buf_origin;
> +FILE *f = fopen(file_name, "r");
> +int pattern_len;
> +
> +if (!f) {
> +printf("'%s': %s\n", file_name, strerror(errno));

(1) Why stdout and not stderr?

(2) Why not just perror()?

> +return NULL;
> +}
> +
> +if (qemuio_misalign) {
> +len += MISALIGN_OFFSET;
> +}
> +
> +buf_origin = buf = blk_blockalign(blk, len);
> +
> +pattern_len = fread(buf, sizeof(char), len, f);

I know I pointed out already that the C standard defines sizeof(char) to
be 1, but I just want to add that “sizeof(char)” does not appear
anywhere in qemu so far.

That doesn’t make it wrong, but here it looks a bit weird...

> +
> +if (ferror(f)) {
> +printf("'%s': %s\n", file_name, strerror(errno));
> +goto error;
> +}
> +
> +if (pattern_len == 0) {
> +printf("'%s' is empty\n", file_name);
> +goto error;
> +}
> +
> +fclose(f);
> +
> +if (len > pattern_len) {
> +char *file_buf = g_malloc(sizeof(char) * pattern_len);

...and here you’d actually have to check that the multiplication doesn’t
overflow if you actually consider it a possibility that
sizeof(char) != 1.

But actually, this allocation is just unnecessary altogether, as
Vladimir has said for v6.  You can drop this line,...

> +memcpy(file_buf, buf, pattern_len);

...this line,...

> +len -= pattern_len;
> +buf += pattern_len;
> +
> +while (len > 0) {
> +size_t len_to_copy = MIN(pattern_len, len);
> +
> +memcpy(buf, file_buf, len_to_copy);

...make this a memcpy(buf, buf_origin, len_to_copy);,...

> +
> +len -= len_to_copy;
> +buf += len_to_copy;
> +}
> +qemu_vfree(file_buf);

...and drop this line, too.

> +}
> +
> +if (qemuio_misalign) {
> +buf_origin += MISALIGN_OFFSET;

This will change the pattern, though, because now you’re skipping the
first MISALIGN_OFFSET bytes.  Should the fread() above read to
buf_origin + MISALIGN_OFFSET in that case?

[...]

> @@ -983,8 +1044,9 @@ static int write_f(BlockBackend *blk, int argc, char 
> **argv)
>  /* Some compilers get confused and warn if this is not initialized.  */
>  int64_t total = 0;
>  int pattern = 0xcd;
> +char *file_name = NULL;
>  
> -while ((c = getopt(argc, argv, "bcCfnpP:quz")) != -1) {
> +while ((c = getopt(argc, argv, "bcCfnpP:quzs:")) != -1) {
>  switch (c) {
>  case 'b':
>  bflag = true;
> @@ -1020,6 +1082,10 @@ static int write_f(BlockBackend *blk, int argc, char 
> **argv)
>  case 'z':
>  zflag = true;
>  break;
> +case 's':
> +sflag = true;
> +file_name = g_strdup(optarg);

Why does file_name need to be a char * and duplicated here?  (optarg
always points into argv, so as long as that doesn’t change, the optarg
pointer stays valid.)

Max

> +break;
>  default:
>  qemuio_command_usage(_cmd);
>  return -EINVAL;



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1838658] Re: qemu 4.0.0 broken by glib update

[Andreas Gustafsson]

> From the netbsd bug report it looks like the reproducer was demoed
> using the sparc emulator - is that the only QEMU arch that is affected ?

Only one arch is affected, but it's sparc64, not sparc.

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

Title:
  qemu 4.0.0 broken by glib update

Status in QEMU:
  New

Bug description:
  In brief, an install CD will successfully boot with qemu 4.0.0 built with 
glib 2.58.3, but freeze during boot with qemu 4.0.0 built with glib 2.60.0. I 
tracked it down to glib's GHashTable improvements. qemu is happy with a glib 
built from
  ```
   git checkout -f 2.60.4
   git revert --no-edit 86c6f7e2b..3bed8a13b
   git revert --no-edit 75f8ec1df9b48b0c3a13a9125f2c7d7c5adf5159
   git revert --no-edit 603fb5958..d3074a748
   git revert --no-edit 0b45ddc55..0600dd322
  ```
  When the GHashTable improvements were committed, there was already a 
preemptive note about any breakage being due to using private implementation 
details, hence mentioning it here rather than with glib.

  For the full saga, see: http://gnats.netbsd.org/54310

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

[Qemu-devel] [PATCH 2/3] target/mips: Switch to do_transaction_failed() hook

[Peter Maydell]

Switch the MIPS target from the old unassigned_access hook to the new
do_transaction_failed hook.

Unlike the old hook, do_transaction_failed is only ever called from
the TCG memory access paths, so there is no need for the "ignore this
if we're using KVM" hack that we were previously using to work around
the way unassigned_access was called for all kinds of memory accesses
to unassigned physical addresses.

The MIPS target does not ever do direct memory reads by physical
address (via either ldl_phys etc or address_space_ldl etc), so the
only memory accesses this affects are the 'normal' guest loads and
stores, which will be handled by the new hook; their behaviour is
unchanged.

Signed-off-by: Peter Maydell 
---
 target/mips/internal.h  |  8 +---
 target/mips/cpu.c   |  2 +-
 target/mips/op_helper.c | 24 
 3 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index b2b41a51ab4..26533bb937d 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -138,9 +138,11 @@ void r4k_helper_tlbinv(CPUMIPSState *env);
 void r4k_helper_tlbinvf(CPUMIPSState *env);
 void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);
 
-void mips_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
-bool is_write, bool is_exec, int unused,
-unsigned size);
+void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
+vaddr addr, unsigned size,
+MMUAccessType access_type,
+int mmu_idx, MemTxAttrs attrs,
+MemTxResult response, uintptr_t retaddr);
 hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,
   int rw);
 #endif
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 39eafafc5cd..a79badcb1a6 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -197,7 +197,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
 cc->gdb_read_register = mips_cpu_gdb_read_register;
 cc->gdb_write_register = mips_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
-cc->do_unassigned_access = mips_cpu_unassigned_access;
+cc->do_transaction_failed = mips_cpu_do_transaction_failed;
 cc->do_unaligned_access = mips_cpu_do_unaligned_access;
 cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
 cc->vmsd = _mips_cpu;
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 9e2e02f8586..65ff9f5b935 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2666,27 +2666,19 @@ void mips_cpu_do_unaligned_access(CPUState *cs, vaddr 
addr,
 do_raise_exception_err(env, excp, error_code, retaddr);
 }
 
-void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
-bool is_write, bool is_exec, int unused,
-unsigned size)
+void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
+vaddr addr, unsigned size,
+MMUAccessType access_type,
+int mmu_idx, MemTxAttrs attrs,
+MemTxResult response, uintptr_t retaddr)
 {
 MIPSCPU *cpu = MIPS_CPU(cs);
 CPUMIPSState *env = >env;
 
-/*
- * Raising an exception with KVM enabled will crash because it won't be 
from
- * the main execution loop so the longjmp won't have a matching setjmp.
- * Until we can trigger a bus error exception through KVM lets just ignore
- * the access.
- */
-if (kvm_enabled()) {
-return;
-}
-
-if (is_exec) {
-raise_exception(env, EXCP_IBE);
+if (access_type == MMU_INST_FETCH) {
+do_raise_exception(env, EXCP_IBE, retaddr);
 } else {
-raise_exception(env, EXCP_DBE);
+do_raise_exception(env, EXCP_DBE, retaddr);
 }
 }
 #endif /* !CONFIG_USER_ONLY */
-- 
2.20.1

[Qemu-devel] [PATCH 3/3] hw/mips/mips_jazz: Remove no-longer-necessary override of do_unassigned_access

[Peter Maydell]

Now that the MIPS CPU implementation uses the new
do_transaction_failed hook, we can remove the old code that handled
the do_unassigned_access hook.

Signed-off-by: Peter Maydell 
---
 hw/mips/mips_jazz.c | 27 ---
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index c64b4c78809..540943932fd 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -111,18 +111,6 @@ static const MemoryRegionOps dma_dummy_ops = {
 #define MAGNUM_BIOS_SIZE_MAX 0x7e000
 #define MAGNUM_BIOS_SIZE (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : 
MAGNUM_BIOS_SIZE_MAX)
 
-static CPUUnassignedAccess real_do_unassigned_access;
-static void mips_jazz_do_unassigned_access(CPUState *cpu, hwaddr addr,
-   bool is_write, bool is_exec,
-   int opaque, unsigned size)
-{
-if (!is_exec) {
-/* ignore invalid access (ie do not raise exception) */
-return;
-}
-(*real_do_unassigned_access)(cpu, addr, is_write, is_exec, opaque, size);
-}
-
 static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr,
   vaddr addr, unsigned size,
   MMUAccessType access_type,
@@ -184,9 +172,8 @@ static void mips_jazz_init(MachineState *machine,
  * However, we can't simply add a global memory region to catch
  * everything, as this would make all accesses including instruction
  * accesses be ignored and not raise exceptions.
- * So instead we hijack either the do_unassigned_access method or
- * the do_transaction_failed method on the CPU, and do not raise exceptions
- * for data access.
+ * So instead we hijack the do_transaction_failed method on the CPU, and
+ * do not raise exceptions for data access.
  *
  * NOTE: this behaviour of raising exceptions for bad instruction
  * fetches but not bad data accesses was added in commit 54e755588cf1e9
@@ -197,14 +184,8 @@ static void mips_jazz_init(MachineState *machine,
  * memory region that catches all memory accesses, as we do on Malta.
  */
 cc = CPU_GET_CLASS(cpu);
-if (cc->do_unassigned_access) {
-real_do_unassigned_access = cc->do_unassigned_access;
-cc->do_unassigned_access = mips_jazz_do_unassigned_access;
-}
-if (cc->do_transaction_failed) {
-real_do_transaction_failed = cc->do_transaction_failed;
-cc->do_transaction_failed = mips_jazz_do_transaction_failed;
-}
+real_do_transaction_failed = cc->do_transaction_failed;
+cc->do_transaction_failed = mips_jazz_do_transaction_failed;
 
 /* allocate RAM */
 memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
-- 
2.20.1

[Qemu-devel] [PATCH 1/3] hw/mips/mips_jazz: Override do_transaction_failed hook

[Peter Maydell]

The MIPS Jazz ('magnum' and 'pica61') boards have some code which
overrides the CPU's do_unassigned_access hook, so they can intercept
it and not raise exceptions on data accesses to invalid addresses,
only for instruction fetches.

We want to switch MIPS over to using the do_transaction_failed
hook instead, so add an intercept for that as well, and make
the board code install whichever hook the CPU is actually using.
Once we've changed the CPU implementation we can remove the
redundant code for the old hook.

Note: I am suspicious that the behaviour as implemented here may not
be what the hardware really does.  It was added in commit
54e755588cf1e90f0b14 to restore the behaviour that was broken by
commit c658b94f6e8c206c59d.  But prior to commit c658b94f6e8c206c59d
every MIPS board generated exceptions for instruction access to
invalid addresses but not for data accesses; and other boards,
notably Malta, were fixed by making all invalid accesses behave as
reads-as-zero (see the call to empty_slot_init() in
mips_malta_init()).  Hardware that raises exceptions for instruction
access and not data access seems to me to be an unlikely design, and
it's possible that the right way to emulate this is to make the Jazz
boards do what we did with Malta (or some variation of that).
Nonetheless, since I don't have access to real hardware to test
against I have taken the approach of "make QEMU continue to behave
the same way it did before this commit".  I have updated the comment
to correct the parts that are no longer accurate and note that
the hardware might behave differently.

The test case for the need for the hook-hijacking is in
https://bugs.launchpad.net/qemu/+bug/1245924 That BIOS will boot OK
either with this overriding of both hooks, or with a simple "global
memory region to ignore bad accesses of all types", so it doesn't
provide evidence either way, unfortunately.

Signed-off-by: Peter Maydell 
---
 hw/mips/mips_jazz.c | 54 ++---
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index fa8775d4284..c64b4c78809 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -123,6 +123,28 @@ static void mips_jazz_do_unassigned_access(CPUState *cpu, 
hwaddr addr,
 (*real_do_unassigned_access)(cpu, addr, is_write, is_exec, opaque, size);
 }
 
+static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr,
+  vaddr addr, unsigned size,
+  MMUAccessType access_type,
+  int mmu_idx, MemTxAttrs attrs,
+  MemTxResult response,
+  uintptr_t retaddr);
+
+static void mips_jazz_do_transaction_failed(CPUState *cs, hwaddr physaddr,
+vaddr addr, unsigned size,
+MMUAccessType access_type,
+int mmu_idx, MemTxAttrs attrs,
+MemTxResult response,
+uintptr_t retaddr)
+{
+if (access_type != MMU_INST_FETCH) {
+/* ignore invalid access (ie do not raise exception) */
+return;
+}
+(*real_do_transaction_failed)(cs, physaddr, addr, size, access_type,
+  mmu_idx, attrs, response, retaddr);
+}
+
 static void mips_jazz_init(MachineState *machine,
enum jazz_model_e jazz_model)
 {
@@ -157,16 +179,32 @@ static void mips_jazz_init(MachineState *machine,
 env = >env;
 qemu_register_reset(main_cpu_reset, cpu);
 
-/* Chipset returns 0 in invalid reads and do not raise data exceptions.
+/*
+ * Chipset returns 0 in invalid reads and do not raise data exceptions.
  * However, we can't simply add a global memory region to catch
- * everything, as memory core directly call unassigned_mem_read/write
- * on some invalid accesses, which call do_unassigned_access on the
- * CPU, which raise an exception.
- * Handle that case by hijacking the do_unassigned_access method on
- * the CPU, and do not raise exceptions for data access. */
+ * everything, as this would make all accesses including instruction
+ * accesses be ignored and not raise exceptions.
+ * So instead we hijack either the do_unassigned_access method or
+ * the do_transaction_failed method on the CPU, and do not raise exceptions
+ * for data access.
+ *
+ * NOTE: this behaviour of raising exceptions for bad instruction
+ * fetches but not bad data accesses was added in commit 54e755588cf1e9
+ * to restore behaviour broken by c658b94f6e8c206, but it is not clear
+ * whether the real hardware behaves this way. It is possible that
+ * real hardware ignores bad instruction fetches as well -- if so then

[Qemu-devel] [PATCH 0/3] target/mips: Convert to do_transaction_failed hook

[Peter Maydell]

This patchset converts the MIPS target away from the
old broken do_unassigned_access hook to the new (added in
2017...) do_transaction_failed hook.

The motivation here is:
 * do_unassigned_access is broken because:
+ it will be called for any kind of access to physical addresses
  where there is no assigned device, whether that access is by the
  CPU or by something else (like a DMA controller!), so it can
  result in spurious guest CPU exceptions.
+ It will also get called even when using KVM, when there's nothing
  useful it can do.
+ It isn't passed in the return-address within the TCG generated
  code, so it isn't able to correctly restore the CPU state
  before generating the exception, and so the exception will
  often be generated with the wrong faulting guest PC value
 * there are now only a few targets still using the old hook,
   so if we can convert them we can delete all the old code
   and complete this API transation. (Patches for SPARC are on
   the list; the other user is RISCV, which accidentally
   implemented the old hook rather than the new one recently.)

The general approach to the conversion is to check the target for
load/store-by-physical-address operations which were previously
implicitly causing exceptions, to see if they now need to explicitly
check for and handle memory access failures. (The 'git grep' regexes
in docs/devel/loads-stores.rst are useful here: the API families to
look for are ld*_phys/st*_phys, address_space_ld/st*, and
cpu_physical_memory*.)

For MIPS, there are none of these (the usual place where targets do
this is hardware page table walks where the page table entries are
loaded by physical address, and MIPS doesn't seem to have those).

Code audit out of the way, the actual hook changeover is pretty
simple.

The complication here is the MIPS Jazz board, which has some rather
dubious code that intercepts the do_unassigned_access hook to suppress
generation of exceptions for invalid accesses due to data accesses,
while leaving exceptions for invalid instruction fetches in place. I'm
a bit dubious about whether the behaviour we have implemented here is
really what the hardware does -- it seems pretty odd to me to not
generate exceptions for d-side accesses but to generate them for
i-side accesses, and looking back through git and mailing list history
this code is here mainly as "put back the behaviour we had before a
previous commit broke it", and that older behaviour in turn I think is
more historical-accident than because anybody deliberately checked the
hardware behaviour and made QEMU work that way. However, I don't have
any real hardware to do comparative tests on, so this series retains
the same behaviour we had before on this board, by making it intercept
the new hook in the same way it did the old one. I've beefed up the
comment somewhat to indicate what we're doing, why, and why it might
not be right.

The patch series is structured in three parts:
 * make the Jazz board code support CPUs regardless of which
   of the two hooks they implement
 * switch the MIPS CPUs over to implementing the new hook
 * remove the no-longer-needed Jazz board code for the old
   hook
(This seemed cleaner to me than squashing the whole thing into
a single patch that touched core mips code and the jazz board
at the same time.)

I have tested this with:
 * the ARC Multiboot BIOS linked to from the bug
   https://bugs.launchpad.net/qemu/+bug/1245924 (and which
   was the test case for needing the hook intercept)
 * a Linux kernel for the 'mips' mips r4k machine
 * 'make check'
Obviously more extensive testing would be useful, but I
don't have any other test images. I also don't have
a KVM MIPS host, which would be worth testing to confirm
that it also still works.

If anybody happens by some chance to still have a working
real-hardware Magnum or PICA61 board, we could perhaps test
how it handles accesses to invalid memory, but I suspect that
nobody does any more :-)

thanks
-- PMM


Peter Maydell (3):
  hw/mips/mips_jazz: Override do_transaction_failed hook
  target/mips: Switch to do_transaction_failed() hook
  hw/mips/mips_jazz: Remove no-longer-necessary override of
do_unassigned_access

 target/mips/internal.h  |  8 ---
 hw/mips/mips_jazz.c | 47 +
 target/mips/cpu.c   |  2 +-
 target/mips/op_helper.c | 24 +++--
 4 files changed, 47 insertions(+), 34 deletions(-)

-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/2] tests/acceptance: Refactor and improve reporting in linux_ssh_mips_malta.py

[Philippe Mathieu-Daudé]

On 8/2/19 5:35 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> This patch restructures code organization around the test case
> executions. At the same time, rather than outputing a cryptic message:
> 
> FAIL: True not found in [False],
> 
> the following will be reported too, if the command output does not meet
> specified expectations:
> 
> 'lspci -d 11ab:4620' output doesn't contain the word 'GT-64120'
> 
> Signed-off-by: Aleksandar Markovic 
> ---
>  tests/acceptance/linux_ssh_mips_malta.py | 36 
> +++-
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
> b/tests/acceptance/linux_ssh_mips_malta.py
> index aafb0c3..8368e1f 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -145,27 +145,33 @@ class LinuxSSH(Test):
>  self.ssh_disconnect_vm()
>  self.wait_for_console_pattern('Power down')
>  
> -def run_common_commands(self):
> -stdout, stderr = self.ssh_command('lspci -d 11ab:4620')
> -self.assertIn(True, ["GT-64120" in line for line in stdout])
> -
> -stdout, stderr = self.ssh_command('cat 
> /sys/bus/i2c/devices/i2c-0/name')
> -self.assertIn(True, ["SMBus PIIX4 adapter" in line
> - for line in stdout])
> -
> -stdout, stderr = self.ssh_command('cat /proc/mtd')
> -self.assertIn(True, ["YAMON" in line
> - for line in stdout])
> +def ssh_command_output_contains(self, cmd, exp):
> +stdout, _ = self.ssh_command(cmd)
> +for line in stdout:
> +if exp in line:
> +break
> +else:
> +self.fail('"%s" output does not contain "%s"' % (cmd, exp))

Or easier using 'return':

 def ssh_command_output_contains(self, cmd, exp):
stdout, _ = self.ssh_command(cmd)
for line in stdout:
if exp in line:
return
self.fail('"%s" output does not contain "%s"' % (cmd, exp))

Regardless, thanks for the cleanup!

Reviewed-by: Philippe Mathieu-Daudé 

>  
> +def run_common_commands(self):
> +self.ssh_command_output_contains(
> +'lspci -d 11ab:4620',
> +'GT-64120')
> +self.ssh_command_output_contains(
> +'cat /sys/bus/i2c/devices/i2c-0/name',
> +'SMBus PIIX4 adapter')
> +self.ssh_command_output_contains(
> +'cat /proc/mtd',
> +'YAMON')
>  # Empty 'Board Config'
> -stdout, stderr = self.ssh_command('md5sum /dev/mtd2ro')
> -self.assertIn(True, ["0dfbe8aa4c20b52e1b8bf3cb6cbdf193" in line
> - for line in stdout])
> +self.ssh_command_output_contains(
> +'md5sum /dev/mtd2ro',
> +'0dfbe8aa4c20b52e1b8bf3cb6cbdf193')
>  
>  def do_test_mips_malta(self, endianess, kernel_path, uname_m):
>  self.boot_debian_wheezy_image_and_ssh_login(endianess, kernel_path)
>  
> -stdout, stderr = self.ssh_command('uname -a')
> +stdout, _ = self.ssh_command('uname -a')
>  self.assertIn(True, [uname_m + " GNU/Linux" in line for line in 
> stdout])
>  
>  self.run_common_commands()
> 

Re: [Qemu-devel] [PATCH v3 1/9] block: add .bdrv_need_rw_file_child_during_reopen_rw handler

[Kevin Wolf]

Am 31.07.2019 um 14:09 hat Max Reitz geschrieben:
> On 25.07.19 11:18, Vladimir Sementsov-Ogievskiy wrote:
> > On reopen to rw parent may need rw access to child in .prepare, for
> > example qcow2 needs to write IN_USE flags into stored bitmaps
> > (currently it is done in a hacky way after commit and don't work).
> > So, let's introduce such logic.
> > 
> > The drawback is that in worst case bdrv_reopen_set_read_only may finish
> > with error and in some intermediate state: some nodes reopened RW and
> > some are not. But this is a way to fix bug around reopening qcow2
> > bitmaps in the following commits.
> 
> This commit message doesn’t really explain what this patch does.
> 
> > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > ---
> >  include/block/block_int.h |   2 +
> >  block.c   | 144 ++
> >  2 files changed, 133 insertions(+), 13 deletions(-)
> > 
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 3aa1e832a8..7bd6fd68dd 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -531,6 +531,8 @@ struct BlockDriver {
> >   uint64_t parent_perm, uint64_t parent_shared,
> >   uint64_t *nperm, uint64_t *nshared);
> >  
> > + bool (*bdrv_need_rw_file_child_during_reopen_rw)(BlockDriverState 
> > *bs);
> > +
> >  /**
> >   * Bitmaps should be marked as 'IN_USE' in the image on reopening image
> >   * as rw. This handler should realize it. It also should unset readonly
> > diff --git a/block.c b/block.c
> > index cbd8da5f3b..3c8e1c59b4 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1715,10 +1715,12 @@ static void 
> > bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
> >   uint64_t *shared_perm);
> >  
> >  typedef struct BlockReopenQueueEntry {
> > - bool prepared;
> > - bool perms_checked;
> > - BDRVReopenState state;
> > - QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
> > +bool reopened_file_child_rw;
> > +bool changed_file_child_perm_rw;
> > +bool prepared;
> > +bool perms_checked;
> > +BDRVReopenState state;
> > +QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
> >  } BlockReopenQueueEntry;
> >  
> >  /*
> > @@ -3421,6 +3423,105 @@ BlockReopenQueue 
> > *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
> > keep_old_opts);
> >  }
> >  
> > +static int bdrv_reopen_set_read_only_drained(BlockDriverState *bs,
> > + bool read_only,
> > + Error **errp)
> > +{
> > +BlockReopenQueue *queue;
> > +QDict *opts = qdict_new();
> > +
> > +qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only);
> > +
> > +queue = bdrv_reopen_queue(NULL, bs, opts, true);
> > +
> > +return bdrv_reopen_multiple(queue, errp);
> > +}
> > +
> > +/*
> > + * handle_recursive_reopens
> > + *
> > + * On fail it needs rollback_recursive_reopens to be called.
> 
> It would be nice if this description actually said anything about what
> the function is supposed to do.
> 
> > + */
> > +static int handle_recursive_reopens(BlockReopenQueueEntry *current,
> > +Error **errp)
> > +{
> > +int ret;
> > +BlockDriverState *bs = current->state.bs;
> > +
> > +/*
> > + * We use the fact that in reopen-queue children are always following
> > + * parents.
> > + * TODO: Switch BlockReopenQueue to be QTAILQ and use
> > + *   QTAILQ_FOREACH_REVERSE.
> 
> Why don’t you do that first?  It would make the code more obvious at
> least to me.
> 
> > + */
> > +if (QSIMPLEQ_NEXT(current, entry)) {
> > +ret = handle_recursive_reopens(QSIMPLEQ_NEXT(current, entry), 
> > errp);
> > +if (ret < 0) {
> > +return ret;
> > +}
> > +}
> > +
> > +if ((current->state.flags & BDRV_O_RDWR) && bs->file && bs->drv &&
> > +bs->drv->bdrv_need_rw_file_child_during_reopen_rw &&
> > +bs->drv->bdrv_need_rw_file_child_during_reopen_rw(bs))
> > +{
> > +if (!bdrv_is_writable(bs->file->bs)) {
> > +ret = bdrv_reopen_set_read_only_drained(bs->file->bs, false, 
> > errp);
> 
> Hm.  Sorry, I find this all a bit hard to understand.  (No comments and
> all.)
> 
> I understand that this is for an RO -> RW transition?  Everything is
> still RO, but the parent will need an RW child before it transitions to
> RW itself.
> 
> 
> I’m going to be honest up front, I don’t like this very much.  But I
> think it may be a reasonable solution for now.
> 
> As I remember, the problem was that when reopening a qcow2 node from RO
> to RW, we need to write something in .prepare() (because it can fail),
> but naturally no .prepare() is called after any .commit(), so no matter
> the order of nodes in the ReopenQueue, the child node will never be RW
> by this 

[Qemu-devel] [PATCH 2/2] tests/acceptance: Add new test cases in linux_ssh_mips_malta.py

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add 15 new tests cases. They all rely on simple commands used for
detecting hardware configuration information.

Signed-off-by: Aleksandar Markovic 
---
 tests/acceptance/linux_ssh_mips_malta.py | 45 
 1 file changed, 45 insertions(+)

diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
b/tests/acceptance/linux_ssh_mips_malta.py
index 8368e1f..c153c41 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -155,6 +155,51 @@ class LinuxSSH(Test):
 
 def run_common_commands(self):
 self.ssh_command_output_contains(
+'cat /proc/cpuinfo',
+'24Kc')
+self.ssh_command_output_contains(
+'uname -m',
+'mips')
+self.ssh_command_output_contains(
+'uname -r',
+'3.2.0-4-4kc-malta')
+self.ssh_command_output_contains(
+'cat /proc/interrupts',
+'timer')
+self.ssh_command_output_contains(
+'cat /proc/interrupts',
+'i8042')
+self.ssh_command_output_contains(
+'cat /proc/interrupts',
+'serial')
+self.ssh_command_output_contains(
+'cat /proc/interrupts',
+'ata_piix')
+self.ssh_command_output_contains(
+'cat /proc/interrupts',
+'eth0')
+self.ssh_command_output_contains(
+'cat /proc/interrupts',
+'eth0')
+self.ssh_command_output_contains(
+'cat /proc/devices',
+'input')
+self.ssh_command_output_contains(
+'cat /proc/devices',
+'usb')
+self.ssh_command_output_contains(
+'cat /proc/devices',
+'fb')
+self.ssh_command_output_contains(
+'cat /proc/ioports',
+'serial')
+self.ssh_command_output_contains(
+'cat /proc/ioports',
+'ata_piix')
+self.ssh_command_output_contains(
+'cat /proc/ioports',
+'piix4_smbus')
+self.ssh_command_output_contains(
 'lspci -d 11ab:4620',
 'GT-64120')
 self.ssh_command_output_contains(
-- 
2.7.4

[Qemu-devel] [PATCH 0/2] tests/acceptance: Update MIPS Malta ssh test

[Aleksandar Markovic]

From: Aleksandar Markovic 

This little series improves linux_ssh_mips_malta.py, both in the sense
of code organization and in the sense of quantity of executed tests.

Aleksandar Markovic (2):
  tests/acceptance: Refactor and improve reporting in
linux_ssh_mips_malta.py
  tests/acceptance: Add new test cases in linux_ssh_mips_malta.py

 tests/acceptance/linux_ssh_mips_malta.py | 81 ++--
 1 file changed, 66 insertions(+), 15 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/2] tests/acceptance: Refactor and improve reporting in linux_ssh_mips_malta.py

[Aleksandar Markovic]

From: Aleksandar Markovic 

This patch restructures code organization around the test case
executions. At the same time, rather than outputing a cryptic message:

FAIL: True not found in [False],

the following will be reported too, if the command output does not meet
specified expectations:

'lspci -d 11ab:4620' output doesn't contain the word 'GT-64120'

Signed-off-by: Aleksandar Markovic 
---
 tests/acceptance/linux_ssh_mips_malta.py | 36 +++-
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
b/tests/acceptance/linux_ssh_mips_malta.py
index aafb0c3..8368e1f 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -145,27 +145,33 @@ class LinuxSSH(Test):
 self.ssh_disconnect_vm()
 self.wait_for_console_pattern('Power down')
 
-def run_common_commands(self):
-stdout, stderr = self.ssh_command('lspci -d 11ab:4620')
-self.assertIn(True, ["GT-64120" in line for line in stdout])
-
-stdout, stderr = self.ssh_command('cat 
/sys/bus/i2c/devices/i2c-0/name')
-self.assertIn(True, ["SMBus PIIX4 adapter" in line
- for line in stdout])
-
-stdout, stderr = self.ssh_command('cat /proc/mtd')
-self.assertIn(True, ["YAMON" in line
- for line in stdout])
+def ssh_command_output_contains(self, cmd, exp):
+stdout, _ = self.ssh_command(cmd)
+for line in stdout:
+if exp in line:
+break
+else:
+self.fail('"%s" output does not contain "%s"' % (cmd, exp))
 
+def run_common_commands(self):
+self.ssh_command_output_contains(
+'lspci -d 11ab:4620',
+'GT-64120')
+self.ssh_command_output_contains(
+'cat /sys/bus/i2c/devices/i2c-0/name',
+'SMBus PIIX4 adapter')
+self.ssh_command_output_contains(
+'cat /proc/mtd',
+'YAMON')
 # Empty 'Board Config'
-stdout, stderr = self.ssh_command('md5sum /dev/mtd2ro')
-self.assertIn(True, ["0dfbe8aa4c20b52e1b8bf3cb6cbdf193" in line
- for line in stdout])
+self.ssh_command_output_contains(
+'md5sum /dev/mtd2ro',
+'0dfbe8aa4c20b52e1b8bf3cb6cbdf193')
 
 def do_test_mips_malta(self, endianess, kernel_path, uname_m):
 self.boot_debian_wheezy_image_and_ssh_login(endianess, kernel_path)
 
-stdout, stderr = self.ssh_command('uname -a')
+stdout, _ = self.ssh_command('uname -a')
 self.assertIn(True, [uname_m + " GNU/Linux" in line for line in 
stdout])
 
 self.run_common_commands()
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 0/9] add failover feature for assigned network devices

[no-reply]

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



Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH v2 0/9] add failover feature for assigned network 
devices
Message-id: 20190802150605.5880-1-jfreim...@redhat.com

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

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

Re: [Qemu-devel] [PATCH v2 0/9] add failover feature for assigned network devices

[Michael S. Tsirkin]

On Fri, Aug 02, 2019 at 05:05:56PM +0200, Jens Freimann wrote:
> This is implementing the host side of the net_failover concept
> (https://www.kernel.org/doc/html/latest/networking/net_failover.html)
> 
> Changes since v1:
> - add new QMP events,
>- one is send when the primary device is unplugged
>- one is send when VIRTIO_NET_F_STANDBY is not negotiated.
>  This is needed because we hide the primary device until
>  the feature bit is negotiated and then add it with
>  qdev_add_device(). The event is for libvirt to be aware of that.
> - patch 7/9: a new migration state, called wait-unplug.
>   It is entered after SETUP and before ACTIVE. In this phase we check
>   devices for pending guest unplug complete event.
>   This patch still has a problem. It checks in a loop if there are still
>   devices that are not unplugged by the guest. If the guest never
>   returns it will run forever. How to terminate this loop? I thought
>   about a timed wait semaphore or just spinning for a certain amount of time,
>   but nothing seems good. Any ideas here?
> - patch 2/9: When unplugging the primary devices, only do the guest part i.e.
>   call hotplug_handler_unplug_request() which calls the pcie attention
>   button code. The unrealize part is not done so the ressources of the
>   device are not freed.  In case of migration failure we can re-plug the 
> device to
>   the guest with hotplug_handler_hotplug(). I tested migration failure and
>   a following second attempt to migrate that doesn't fail.
> - add the primary device on the target VM, done in runstate change
>   handler.
> - fix error reporting (dgilbert)
> - get rid of timer to add device after feature negotiation
> 
> The general idea is that we have a pair of devices, a vfio-pci and a
> virtio-net device. Before migration the vfio device is unplugged and data
> flows to the virtio-net device, on the target side another vfio-pci device
> is plugged in to take over the data-path. In the guest the net_failover
> module will pair net devices with the same MAC address.
> 
> * Patch 1 adds the infrastructure to hide the device for the qbus and qdev 
> APIs
> 
> * Patch 2 In the second patch the virtio-net uses the API to defer adding the 
> vfio
>   device until the VIRTIO_NET_F_STANDBY feature is acked. It also
>   implements the migration handler to unplug the device from the guest and
>   re-plug in case of migration failure
> 
> * Patch 3 and 4 make sure that we can unplug the vfio-device before
>   migration starts
> 
> * Patch 5 and 6 add new qmp events, one sends the device id of a device
>   that was just requested to be unplugged from the guest and another one
>   to let libvirt know if VIRTIO_NET_F_STANDBY was negotiated
> 
> * Patch 7 adds a new migration state that is entered while we wait for
>   devices to be unplugged by guest OS
> 
> * Patch 8 sets a new flag for PCIDevice 'partially_hotplugged' which we
>   use to skip the unrealize code path when doing a unplug of the primary
>   device
> 
> * Patch 9 sets the pending_deleted_event before triggering the guest
>   unplug request
> 
> Previous discussion:
>   RFC v1 https://patchwork.ozlabs.org/cover/989098/
>   RFC v2 https://www.mail-archive.com/qemu-devel@nongnu.org/msg606906.html
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03968.html
> 
> To summarize concerns/feedback from previous discussion:
> 1.- guest OS can reject or worse _delay_ unplug by any amount of time.
>   Migration might get stuck for unpredictable time with unclear reason.
>   This approach combines two tricky things, hot/unplug and migration.
>   -> We need to let libvirt know what's happening. Add new qmp events
>  and a new migration state. When a primary device is (partially)
>  unplugged (only from guest) we send a qmp event with the device id. When
>  it is unplugged from the guest the DEVICE_DELETED event is sent.
>  Migration will enter the wait-unplug state while waiting for the guest
>  os to unplug all primary devices and then move on with migration.
> 2. PCI devices are a precious ressource. The primary device should never
>   be added to QEMU if it won't be used by guest instead of hiding it in
>   QEMU.
>   -> We only hotplug the device when the standby feature bit was
>  negotiated. We save the device cmdline options until we need it for
>  qdev_device_add()
>  Hiding a device can be a useful concept to model. For example a
>  pci device in a powered-off slot could be marked as hidden until the 
> slot is
>  powered on (mst).
> 3. Management layer software should handle this. Open Stack already has
>   components/code to handle unplug/replug VFIO devices and metadata to
>   provide to the guest for detecting which devices should be paired.
>   -> An approach that includes all software from firmware to
>  higher-level management software wasn't tried in the last years. This is
>  an attempt to keep it simple and contained in 

[Qemu-devel] [PATCH 7/9] migration: Add new migration state wait-unplug

[Jens Freimann]

This patch is not ready for inclusion yet, I'm looking for
feedback/ideas on a particular problem. See below.

This patch adds a new migration state called wait-unplug.  It is
entered after the SETUP state and will transition into ACTIVE once all
devices were succesfully unplugged from the guest.

So if a guest doesn't respond or takes long to honor the unplug
request the user will see the migration state 'wait-unplug'.

It adds a new function callback to VMStateDescription which is
called for every device that implements it and reports its
device_pending status.

Now this loop in the migration thread:

while (qemu_savevm_state_guest_unplug_pending()) { continue; }

clearly needs a condition to terminate after a while/or a certain
number of iterations. But I'm not sure what is a good solution. How much
waiting time is acceptable for a migration?

Signed-off-by: Jens Freimann 
---
 include/migration/vmstate.h |  2 ++
 migration/migration.c   | 14 ++
 migration/savevm.c  | 18 ++
 migration/savevm.h  |  1 +
 qapi/migration.json |  5 -
 5 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index c2bfa7a7f0..8b2a125c4c 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -187,6 +187,8 @@ struct VMStateDescription {
 int (*pre_save)(void *opaque);
 int (*post_save)(void *opaque);
 bool (*needed)(void *opaque);
+bool (*dev_unplug_pending)(void *opaque);
+
 const VMStateField *fields;
 const VMStateDescription **subsections;
 };
diff --git a/migration/migration.c b/migration/migration.c
index 8a607fe1e2..a7d21b73fe 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -946,6 +946,9 @@ static void fill_source_migration_info(MigrationInfo *info)
 case MIGRATION_STATUS_CANCELLED:
 info->has_status = true;
 break;
+case MIGRATION_STATUS_WAIT_UNPLUG:
+info->has_status = true;
+break;
 }
 info->status = s->state;
 }
@@ -1680,6 +1683,7 @@ bool migration_is_idle(void)
 case MIGRATION_STATUS_COLO:
 case MIGRATION_STATUS_PRE_SWITCHOVER:
 case MIGRATION_STATUS_DEVICE:
+case MIGRATION_STATUS_WAIT_UNPLUG:
 return false;
 case MIGRATION_STATUS__MAX:
 g_assert_not_reached();
@@ -1712,6 +1716,7 @@ void migrate_init(MigrationState *s)
 error_free(s->error);
 s->error = NULL;
 
+/* go to WAIT_UNPLUG first? */
 migrate_set_state(>state, MIGRATION_STATUS_NONE, 
MIGRATION_STATUS_SETUP);
 
 s->start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
@@ -3218,6 +3223,15 @@ static void *migration_thread(void *opaque)
 
 qemu_savevm_state_setup(s->to_dst_file);
 
+migrate_set_state(>state, MIGRATION_STATUS_SETUP,
+  MIGRATION_STATUS_WAIT_UNPLUG);
+while (qemu_savevm_state_guest_unplug_pending()) {
+continue;
+}
+migrate_set_state(>state, MIGRATION_STATUS_WAIT_UNPLUG,
+  MIGRATION_STATUS_ACTIVE); 
+
+
 s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
 migrate_set_state(>state, MIGRATION_STATUS_SETUP,
   MIGRATION_STATUS_ACTIVE);
diff --git a/migration/savevm.c b/migration/savevm.c
index 79ed44d475..2bb54b3a8a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1085,6 +1085,24 @@ void qemu_savevm_state_header(QEMUFile *f)
 }
 }
 
+bool qemu_savevm_state_guest_unplug_pending(void)
+{
+SaveStateEntry *se;
+bool ret = false;
+
+QTAILQ_FOREACH(se, _state.handlers, entry) {
+if (!se->vmsd || !se->vmsd->dev_unplug_pending) {
+continue;
+}
+ret = se->vmsd->dev_unplug_pending(se->opaque);
+if (ret) {
+break;
+}
+}
+
+return ret;
+}
+
 void qemu_savevm_state_setup(QEMUFile *f)
 {
 SaveStateEntry *se;
diff --git a/migration/savevm.h b/migration/savevm.h
index 51a4b9caa8..ba64a7e271 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -31,6 +31,7 @@
 
 bool qemu_savevm_state_blocked(Error **errp);
 void qemu_savevm_state_setup(QEMUFile *f);
+bool qemu_savevm_state_guest_unplug_pending(void);
 int qemu_savevm_state_resume_prepare(MigrationState *s);
 void qemu_savevm_state_header(QEMUFile *f);
 int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
diff --git a/qapi/migration.json b/qapi/migration.json
index d567ac9fc3..c42381a85f 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -133,6 +133,9 @@
 # @device: During device serialisation when pause-before-switchover is enabled
 #(since 2.11)
 #
+# @wait-unplug: wait for device unplug request by guest OS to be completed.
+#   (since 4.2) 
+#
 # Since: 2.3
 #
 ##
@@ -140,7 +143,7 @@
   'data': [ 'none', 'setup', 'cancelling', 'cancelled',
 'active', 'postcopy-active', 'postcopy-paused',
 'postcopy-recover', 'completed', 'failed', 'colo',
- 

[Qemu-devel] [PATCH 4/9] migration: allow unplug during migration for failover devices

[Jens Freimann]

In "b06424de62 migration: Disable hotplug/unplug during migration" we
added a check to disable unplug for all devices until we have figured
out what works. For failover primary devices qdev_unplug() is called
from the migration handler, i.e. during migration.

This patch adds a flag to DeviceState which is set to false for all
devices and makes an exception for vfio-pci devices that are also
primary devices in a failover pair.

Signed-off-by: Jens Freimann 
---
 hw/core/qdev.c | 1 +
 include/hw/qdev-core.h | 1 +
 qdev-monitor.c | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 31e90ebaa2..f43f744133 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -973,6 +973,7 @@ static void device_initfn(Object *obj)
 
 dev->instance_id_alias = -1;
 dev->realized = false;
+dev->allow_unplug_during_migration = false;
 
 object_property_add_bool(obj, "realized",
  device_get_realized, device_set_realized, NULL);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 6174538b01..af4857d391 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -147,6 +147,7 @@ struct DeviceState {
 bool pending_deleted_event;
 QemuOpts *opts;
 int hotplugged;
+bool allow_unplug_during_migration;
 BusState *parent_bus;
 QLIST_HEAD(, NamedGPIOList) gpios;
 QLIST_HEAD(, BusState) child_bus;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index c69e283419..e7ab5cacb3 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -845,7 +845,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 return;
 }
 
-if (!migration_is_idle() && !migration_in_setup(migrate_get_current())) {
+if (!migration_is_idle() && !dev->allow_unplug_during_migration) {
 error_setg(errp, "device_del not allowed while migrating");
 return;
 }
-- 
2.21.0

[Qemu-devel] [PATCH 6/9] qapi: Add failover negotiated event

[Jens Freimann]

This event is sent to let libvirt know that VIRTIO_NET_F_STANDBY
feature was not negotiated during virtio feature negotiation. If this
event is received it means any primary devices hotplugged before
this were were never really added to QEMU devices.

Signed-off-by: Jens Freimann 
---
 qapi/net.json | 16 
 1 file changed, 16 insertions(+)

diff --git a/qapi/net.json b/qapi/net.json
index 728990f4fb..8c5f3f1fb2 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -737,3 +737,19 @@
 ##
 { 'command': 'announce-self', 'boxed': true,
   'data' : 'AnnounceParameters'}
+
+##
+# @FAILOVER_NEGOTIATED:
+#
+# Emitted when VIRTIO_NET_F_STANDBY was negotiated during feature negotiation
+#
+# Since: 4.2
+#
+# Example:
+#
+# <- { "event": "FAILOVER_NEGOTIATED",
+#  "data": {} }
+#
+##
+{ 'event': 'FAILOVER_NEGOTIATED',
+  'data': {} }
-- 
2.21.0

[Qemu-devel] [PATCH 8/9] pci: mark devices partially unplugged

[Jens Freimann]

Only the guest unplug request was triggered. This is needed for
the failover feature. In case of a failed migration we need to
plug the device back to the guest.

Signed-off-by: Jens Freimann 
---
 hw/pci/pci.c | 2 ++
 hw/pci/pcie.c| 3 +++
 include/hw/pci/pci.h | 1 +
 3 files changed, 6 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8076a80ab3..935127a98f 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2074,6 +2074,8 @@ static void pci_qdev_realize(DeviceState *qdev, Error 
**errp)
 Error *local_err = NULL;
 bool is_default_rom;
 
+pci_dev->partially_hotplugged = false;
+
 /* initialize cap_present for pci_is_express() and pci_config_size(),
  * Note that hybrid PCIs are not set automatically and need to manage
  * QEMU_PCI_CAP_EXPRESS manually */
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index a6beb567bd..19363ff8ce 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -456,6 +456,9 @@ static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, 
void *opaque)
 {
 HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(dev));
 
+if (dev->partially_hotplugged) {
+return;
+}
 hotplug_handler_unplug(hotplug_ctrl, DEVICE(dev), _abort);
 object_unparent(OBJECT(dev));
 }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index aaf1b9f70d..e0b82d3ad4 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -265,6 +265,7 @@ typedef struct PCIReqIDCache PCIReqIDCache;
 
 struct PCIDevice {
 DeviceState qdev;
+bool partially_hotplugged;
 
 /* PCI config space */
 uint8_t *config;
-- 
2.21.0

[Qemu-devel] [PATCH 3/9] vfio: unplug failover primary device before migration

[Jens Freimann]

As usual block all vfio-pci devices from being migrated, but make an
exception for failover primary devices. This is achieved by setting
unmigratable to 0 but also add a migration blocker for all vfio-pci
devices except failover primary devices. These will be unplugged before
migration happens by the migration handler of the corresponding
virtio-net standby device.

Signed-off-by: Jens Freimann 
---
 hw/vfio/pci.c | 24 +++-
 hw/vfio/pci.h |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d6ae9bd4ac..398d26669b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -35,6 +35,9 @@
 #include "pci.h"
 #include "trace.h"
 #include "qapi/error.h"
+#include "migration/blocker.h"
+#include "qemu/option.h"
+#include "qemu/option_int.h"
 
 #define TYPE_VFIO_PCI "vfio-pci"
 #define PCI_VFIO(obj)OBJECT_CHECK(VFIOPCIDevice, obj, TYPE_VFIO_PCI)
@@ -2693,6 +2696,12 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice 
*vdev)
 vdev->req_enabled = false;
 }
 
+static int has_standby_arg(void *opaque, const char *name,
+   const char *value, Error **errp)
+{
+return strcmp(name, "standby") == 0;
+}
+
 static void vfio_realize(PCIDevice *pdev, Error **errp)
 {
 VFIOPCIDevice *vdev = PCI_VFIO(pdev);
@@ -2706,6 +2715,19 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 int i, ret;
 bool is_mdev;
 
+if (qemu_opt_foreach(pdev->qdev.opts, has_standby_arg,
+ (void *) pdev->qdev.opts, ) == 0) {
+error_setg(>migration_blocker,
+"VFIO device doesn't support migration");
+ret = migrate_add_blocker(vdev->migration_blocker, );
+if (err) {
+error_propagate(errp, err);
+error_free(vdev->migration_blocker);
+}
+} else {
+pdev->qdev.allow_unplug_during_migration = true;
+}
+
 if (!vdev->vbasedev.sysfsdev) {
 if (!(~vdev->host.domain || ~vdev->host.bus ||
   ~vdev->host.slot || ~vdev->host.function)) {
@@ -3148,7 +3170,7 @@ static Property vfio_pci_dev_properties[] = {
 
 static const VMStateDescription vfio_pci_vmstate = {
 .name = "vfio-pci",
-.unmigratable = 1,
+.unmigratable = 0,
 };
 
 static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 27d58fc55b..0f6f8cb395 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -169,6 +169,7 @@ typedef struct VFIOPCIDevice {
 bool no_vfio_ioeventfd;
 bool enable_ramfb;
 VFIODisplay *dpy;
+Error *migration_blocker;
 } VFIOPCIDevice;
 
 uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
-- 
2.21.0

[Qemu-devel] [PATCH 9/9] pci: mark device having guest unplug request pending

[Jens Freimann]

Set pending_deleted_event before triggering the unplug request and reset
it in the unplug handler when the device is marked as partially
hotplugged. This is required to be able to plug it back in in case of a
migration failure.

Signed-off-by: Jens Freimann 
---
 hw/pci/pcie.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 19363ff8ce..08718188bb 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -457,6 +457,7 @@ static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, 
void *opaque)
 HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(dev));
 
 if (dev->partially_hotplugged) {
+dev->qdev.pending_deleted_event = false;
 return;
 }
 hotplug_handler_unplug(hotplug_ctrl, DEVICE(dev), _abort);
@@ -476,6 +477,8 @@ void pcie_cap_slot_unplug_request_cb(HotplugHandler 
*hotplug_dev,
 return;
 }
 
+dev->pending_deleted_event = true;
+
 /* In case user cancel the operation of multi-function hot-add,
  * remove the function that is unexposed to guest individually,
  * without interaction with guest.
-- 
2.21.0

[Qemu-devel] [PATCH 5/9] qapi: add unplug primary event

[Jens Freimann]

Add new qmp event to send the device id of a device that was
requested to be unplugged by the guest OS.
 
Signed-off-by: Jens Freimann 
---
 qapi/migration.json | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/qapi/migration.json b/qapi/migration.json
index 9cfbaf8c6c..d567ac9fc3 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1445,3 +1445,22 @@
 # Since: 3.0
 ##
 { 'command': 'migrate-pause', 'allow-oob': true }
+
+##
+# @UNPLUG_PRIMARY:
+#
+# Emitted from source side of a migration when migration state is
+# WAIT_UNPLUG. Device was unplugged by guest operating system.
+# Device resources in QEMU are kept on standby to be able to re-plug it in case
+# of migration failure.
+#
+# @device_id: QEMU device id of the unplugged device
+#
+# Since: 4.2
+#
+# Example:
+#   {"event": "UNPLUG_PRIMARY", "data": {"device_id": "hostdev0"} }
+#
+##
+{ 'event': 'UNPLUG_PRIMARY',
+  'data': { 'device_id': 'str' } }
-- 
2.21.0

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

[Jens Freimann]

This patch adds support to handle failover device pairs of a virtio-net
device and a vfio-pci device, where the virtio-net acts as the standby
device and the vfio-pci device as the primary.

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

To achieve this we need:

1. Provide a callback function for the should_be_hidden DeviceListener.
   It is called when the primary device is plugged in. Evaluate the QOpt
   passed in to check if it is the matching primary device. It returns
   two values:
 - one to signal if the device to be added is the matching
   primary device
 - another one to signal to qdev if it should actually
   continue with adding the device or skip it.

   In the latter case it stores the device options in the VirtioNet
   struct and the device is added once the VIRTIO_NET_F_STANDBY feature is
   negotiated during virtio feature negotiation.

2. Register a callback for migration status notifier. When called it
   will unplug its primary device before the migration happens.

Signed-off-by: Jens Freimann 
---
 hw/net/virtio-net.c| 180 +
 include/hw/virtio/virtio-net.h |  13 +++
 include/hw/virtio/virtio.h |   1 +
 3 files changed, 194 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b9e1cd71cf..2f29155d9f 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/atomic.h"
 #include "qemu/iov.h"
 #include "qemu/module.h"
 #include "hw/virtio/virtio.h"
@@ -20,16 +21,24 @@
 #include "net/tap.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
+#include "qemu/option.h"
+#include "qemu/option_int.h"
+#include "qemu/config-file.h"
+#include "qapi/qmp/qdict.h"
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
 #include "net/announce.h"
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-net.h"
+#include "qapi/qapi-types-migration.h"
+#include "qapi/qapi-events-migration.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/misc.h"
 #include "standard-headers/linux/ethtool.h"
 #include "trace.h"
+#include "monitor/qdev.h"
+#include "hw/pci/pci.h"
 
 #define VIRTIO_NET_VM_VERSION11
 
@@ -743,6 +752,24 @@ static inline uint64_t 
virtio_net_supported_guest_offloads(VirtIONet *n)
 return virtio_net_guest_offloads_by_features(vdev->guest_features);
 }
 
+static void failover_add_primary(VirtIONet *n)
+{
+Error *err = NULL;
+
+n->primary_device_opts = qemu_opts_find(qemu_find_opts("device"),
+n->primary_device_id);
+if (n->primary_device_opts) {
+n->primary_dev = qdev_device_add(n->primary_device_opts, );
+if (n->primary_dev) {
+n->primary_bus = n->primary_dev->parent_bus;
+}
+}
+if (err) {
+error_report_err(err);
+}
+}
+
+
 static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
 {
 VirtIONet *n = VIRTIO_NET(vdev);
@@ -787,6 +814,11 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
uint64_t features)
 } else {
 memset(n->vlans, 0xff, MAX_VLAN >> 3);
 }
+
+if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
+atomic_set(>primary_should_be_hidden, false);
+failover_add_primary(n);
+}
 }
 
 static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
@@ -2627,6 +2659,119 @@ void virtio_net_set_netclient_name(VirtIONet *n, const 
char *name,
 n->netclient_type = g_strdup(type);
 }
 
+static bool failover_unplug_primary(VirtIONet *n)
+{
+HotplugHandler *hotplug_ctrl;
+PCIDevice *pci_dev;
+Error *err = NULL;
+
+hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
+if (hotplug_ctrl) {
+pci_dev = PCI_DEVICE(n->primary_dev);
+pci_dev->partially_hotplugged = true;
+hotplug_handler_unplug_request(hotplug_ctrl, n->primary_dev, );
+if (err) {
+error_report_err(err);
+return false;
+}
+} else {
+return false;
+}
+return true;
+}
+
+static bool failover_replug_primary(VirtIONet *n, Error *err)
+{
+HotplugHandler *hotplug_ctrl;
+
+if (!n->primary_device_opts) {
+n->primary_device_opts = qemu_opts_from_qdict(
+qemu_find_opts("device"),
+n->primary_device_dict, );
+}
+if (n->primary_device_opts) {
+qdev_set_parent_bus(n->primary_dev, n->primary_bus);
+n->primary_should_be_hidden = false;
+qemu_opt_set_bool(n->primary_device_opts,
+"partially_hotplugged", true, );
+hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
+ 

[Qemu-devel] [PATCH v2 0/9] add failover feature for assigned network devices

[Jens Freimann]

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

Changes since v1:
- add new QMP events,
   - one is send when the primary device is unplugged
   - one is send when VIRTIO_NET_F_STANDBY is not negotiated.
 This is needed because we hide the primary device until
 the feature bit is negotiated and then add it with
 qdev_add_device(). The event is for libvirt to be aware of that.
- patch 7/9: a new migration state, called wait-unplug.
  It is entered after SETUP and before ACTIVE. In this phase we check
  devices for pending guest unplug complete event.
  This patch still has a problem. It checks in a loop if there are still
  devices that are not unplugged by the guest. If the guest never
  returns it will run forever. How to terminate this loop? I thought
  about a timed wait semaphore or just spinning for a certain amount of time,
  but nothing seems good. Any ideas here?
- patch 2/9: When unplugging the primary devices, only do the guest part i.e.
  call hotplug_handler_unplug_request() which calls the pcie attention
  button code. The unrealize part is not done so the ressources of the
  device are not freed.  In case of migration failure we can re-plug the device 
to
  the guest with hotplug_handler_hotplug(). I tested migration failure and
  a following second attempt to migrate that doesn't fail.
- add the primary device on the target VM, done in runstate change
  handler.
- fix error reporting (dgilbert)
- get rid of timer to add device after feature negotiation

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

* Patch 1 adds the infrastructure to hide the device for the qbus and qdev APIs

* Patch 2 In the second patch the virtio-net uses the API to defer adding the 
vfio
  device until the VIRTIO_NET_F_STANDBY feature is acked. It also
  implements the migration handler to unplug the device from the guest and
  re-plug in case of migration failure

* Patch 3 and 4 make sure that we can unplug the vfio-device before
  migration starts

* Patch 5 and 6 add new qmp events, one sends the device id of a device
  that was just requested to be unplugged from the guest and another one
  to let libvirt know if VIRTIO_NET_F_STANDBY was negotiated

* Patch 7 adds a new migration state that is entered while we wait for
  devices to be unplugged by guest OS

* Patch 8 sets a new flag for PCIDevice 'partially_hotplugged' which we
  use to skip the unrealize code path when doing a unplug of the primary
  device

* Patch 9 sets the pending_deleted_event before triggering the guest
  unplug request

Previous discussion:
  RFC v1 https://patchwork.ozlabs.org/cover/989098/
  RFC v2 https://www.mail-archive.com/qemu-devel@nongnu.org/msg606906.html
  v1: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03968.html

To summarize concerns/feedback from previous discussion:
1.- guest OS can reject or worse _delay_ unplug by any amount of time.
  Migration might get stuck for unpredictable time with unclear reason.
  This approach combines two tricky things, hot/unplug and migration.
  -> We need to let libvirt know what's happening. Add new qmp events
 and a new migration state. When a primary device is (partially)
 unplugged (only from guest) we send a qmp event with the device id. When
 it is unplugged from the guest the DEVICE_DELETED event is sent.
 Migration will enter the wait-unplug state while waiting for the guest
 os to unplug all primary devices and then move on with migration.
2. PCI devices are a precious ressource. The primary device should never
  be added to QEMU if it won't be used by guest instead of hiding it in
  QEMU.
  -> We only hotplug the device when the standby feature bit was
 negotiated. We save the device cmdline options until we need it for
 qdev_device_add()
 Hiding a device can be a useful concept to model. For example a
 pci device in a powered-off slot could be marked as hidden until the slot 
is
 powered on (mst).
3. Management layer software should handle this. Open Stack already has
  components/code to handle unplug/replug VFIO devices and metadata to
  provide to the guest for detecting which devices should be paired.
  -> An approach that includes all software from firmware to
 higher-level management software wasn't tried in the last years. This is
 an attempt to keep it simple and contained in QEMU as much as possible.
 One of the problems that stopped management software and libvirt from
 implementing this idea is that it can't be sure that it's possible to
 re-plug the primary device. By not freeing the devices resources in 

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

[Jens Freimann]

This adds support for hiding a device to the qbus and qdev APIs.
qdev_device_add() is modified to check for a standby argument in the
option string. A DeviceListener callback should_be_hidden() is added. It
can be used by a standby device to inform qdev that this device should
not be added now. The standby device handler can store the device
options to plug the device in at a later point in time.

Signed-off-by: Jens Freimann 
---
 hw/core/qdev.c | 19 +++
 hw/vfio/pci.c  |  1 +
 hw/vfio/pci.h  |  1 +
 include/hw/qdev-core.h |  8 
 qdev-monitor.c | 43 ++
 vl.c   |  6 --
 6 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 94ebc0a4a1..31e90ebaa2 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -211,6 +211,25 @@ void device_listener_unregister(DeviceListener *listener)
 QTAILQ_REMOVE(_listeners, listener, link);
 }
 
+bool qdev_should_hide_device(QemuOpts *opts, Error **errp)
+{
+bool res = false;
+bool match_found = false;
+
+DeviceListener *listener;
+
+QTAILQ_FOREACH(listener, _listeners, link) {
+   if (listener->should_be_hidden) {
+listener->should_be_hidden(listener, opts, _found, );
+}
+
+if (match_found) {
+break;
+}
+}
+return res;
+}
+
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
  int required_for_version)
 {
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d7a4e1875c..d6ae9bd4ac 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3105,6 +3105,7 @@ static Property vfio_pci_dev_properties[] = {
 display, ON_OFF_AUTO_OFF),
 DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
 DEFINE_PROP_UINT32("yres", VFIOPCIDevice, display_yres, 0),
+DEFINE_PROP_STRING("standby", VFIOPCIDevice, standby),
 DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice,
intx.mmap_timeout, 1100),
 DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 834a90d646..27d58fc55b 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -134,6 +134,7 @@ typedef struct VFIOPCIDevice {
 PCIHostDeviceAddress host;
 EventNotifier err_notifier;
 EventNotifier req_notifier;
+char *standby;
 int (*resetfn)(struct VFIOPCIDevice *);
 uint32_t vendor_id;
 uint32_t device_id;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 136df7774c..6174538b01 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -158,6 +158,13 @@ struct DeviceState {
 struct DeviceListener {
 void (*realize)(DeviceListener *listener, DeviceState *dev);
 void (*unrealize)(DeviceListener *listener, DeviceState *dev);
+/*
+ * This callback is called just upon init of the DeviceState
+ * and can be used by a standby device for informing qdev if this
+ * device should be hidden by checking the device opts
+ */
+void (*should_be_hidden)(DeviceListener *listener, QemuOpts *device_opts,
+bool *match_found, bool *res);
 QTAILQ_ENTRY(DeviceListener) link;
 };
 
@@ -457,5 +464,6 @@ void device_listener_unregister(DeviceListener *listener);
 VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
  VMChangeStateHandler *cb,
  void *opaque);
+bool qdev_should_hide_device(QemuOpts *opts, Error **errp);
 
 #endif
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 58222c2211..c69e283419 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -32,8 +32,10 @@
 #include "qemu/help_option.h"
 #include "qemu/option.h"
 #include "qemu/qemu-print.h"
+#include "qemu/option_int.h"
 #include "sysemu/block-backend.h"
 #include "migration/misc.h"
+#include "migration/migration.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -561,14 +563,45 @@ void qdev_set_id(DeviceState *dev, const char *id)
 }
 }
 
+static int is_failover_device(void *opaque, const char *name, const char 
*value,
+Error **errp)
+{
+if (strcmp(name, "standby") == 0) {
+QemuOpts *opts = (QemuOpts *)opaque;
+
+if (qdev_should_hide_device(opts, errp) && errp && !*errp) {
+return 1;
+} else if (errp && *errp) {
+return -1;
+}
+}
+
+return 0;
+}
+
+static bool should_hide_device(QemuOpts *opts, Error **err)
+{
+if (qemu_opt_foreach(opts, is_failover_device, opts, err) == 0) {
+return false;
+}
+return true;
+}
+
 DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
 {
 DeviceClass *dc;
 const char *driver, *path;
-DeviceState *dev;
+DeviceState *dev = NULL;
 BusState *bus = NULL;
 Error *err = NULL;
 
+if (opts && 

Re: [Qemu-devel] [PATCH for-4.2 v3 0/2] s390: stop abusing memory_region_allocate_system_memory()

[Christian Borntraeger]

On 02.08.19 16:59, Christian Borntraeger wrote:
> 
> 
> On 02.08.19 16:42, Christian Borntraeger wrote:
>> On 02.08.19 15:32, Igor Mammedov wrote:
>>> Changelog:
>>>   since v2:
>>> - break migration from old QEMU (since 2.12-4.1) for guest with >8TB RAM
>>>   and drop migratable aliases patch as was agreed during v2 review
>>> - drop 4.2 machines patch as it's not prerequisite anymore
>>>   since v1:
>>> - include 4.2 machines patch for adding compat RAM layout on top
>>> - 2/4 add missing in v1 patch for splitting too big MemorySection on
>>>   several memslots
>>> - 3/4 amend code path on alias destruction to ensure that RAMBlock is
>>>   cleaned properly
>>> - 4/4 add compat machine code to keep old layout (migration-wise) for
>>>   4.1 and older machines 
>>>
>>>
>>> While looking into unifying guest RAM allocation to use hostmem backends
>>> for initial RAM (especially when -mempath is used) and retiring
>>> memory_region_allocate_system_memory() API, leaving only single hostmem 
>>> backend,
>>> I was inspecting how currently it is used by boards and it turns out several
>>> boards abuse it by calling the function several times (despite documented 
>>> contract
>>> forbiding it).
>>>
>>> s390 is one of such boards where KVM limitation on memslot size got 
>>> propagated
>>> to board design and memory_region_allocate_system_memory() was abused to 
>>> satisfy
>>> KVM requirement for max RAM chunk where memory region alias would suffice.
>>>
>>> Unfortunately, memory_region_allocate_system_memory() usage created 
>>> migration
>>> dependency where guest RAM is transferred in migration stream as several 
>>> RAMBlocks
>>> if it's more than KVM_SLOT_MAX_BYTES. During v2 review it was agreed to 
>>> ignore
>>> migration breakage (documenting it in release notes) and leaving only KVM 
>>> fix.
>>>
>>> In order to replace these several RAM chunks with a single memdev and keep 
>>> it
>>> working with KVM memslot size limit, following was done:
>>>* [1/2] split too big RAM chunk inside of KVM code on several memory 
>>> slots
>>>if necessary
>>>* [2/2] drop manual ram splitting in s390 code
>>>
>>>
>>> CC: pbonz...@redhat.com
>>> CC: qemu-s3...@nongnu.org
>>> CC: borntrae...@de.ibm.com
>>> CC: th...@redhat.com
>>> CC: da...@redhat.com
>>> CC: coh...@redhat.com
>>
>> With the fixup this patch set seems to work on s390. I can start 9TB guests 
>> and
>> I can migrate smaller guests between 4.1+patch and 4.0 and 3.1. I currently 
>> can
>> not test migration for the 9TB guest due to lack of a 2nd system. 
> 
> I have to correct myself. The 9TB guest started up but it does not seem to do
> anything useful (it hangs).

Seems that the userspace addr is wrong (its the same). 
[pid 258234] ioctl(10, KVM_SET_USER_MEMORY_REGION, {slot=0, flags=0, 
guest_phys_addr=0, memory_size=8796091973632, userspace_addr=0x3fff7d0}) = 0
[pid 258234] ioctl(10, KVM_SET_USER_MEMORY_REGION, {slot=1, flags=0, 
guest_phys_addr=0x7f0, memory_size=1099512676352, 
userspace_addr=0x3fff7d0}) = 0

Re: [Qemu-devel] [PATCH for-4.2 v3 0/2] s390: stop abusing memory_region_allocate_system_memory()

[Christian Borntraeger]

On 02.08.19 16:42, Christian Borntraeger wrote:
> On 02.08.19 15:32, Igor Mammedov wrote:
>> Changelog:
>>   since v2:
>> - break migration from old QEMU (since 2.12-4.1) for guest with >8TB RAM
>>   and drop migratable aliases patch as was agreed during v2 review
>> - drop 4.2 machines patch as it's not prerequisite anymore
>>   since v1:
>> - include 4.2 machines patch for adding compat RAM layout on top
>> - 2/4 add missing in v1 patch for splitting too big MemorySection on
>>   several memslots
>> - 3/4 amend code path on alias destruction to ensure that RAMBlock is
>>   cleaned properly
>> - 4/4 add compat machine code to keep old layout (migration-wise) for
>>   4.1 and older machines 
>>
>>
>> While looking into unifying guest RAM allocation to use hostmem backends
>> for initial RAM (especially when -mempath is used) and retiring
>> memory_region_allocate_system_memory() API, leaving only single hostmem 
>> backend,
>> I was inspecting how currently it is used by boards and it turns out several
>> boards abuse it by calling the function several times (despite documented 
>> contract
>> forbiding it).
>>
>> s390 is one of such boards where KVM limitation on memslot size got 
>> propagated
>> to board design and memory_region_allocate_system_memory() was abused to 
>> satisfy
>> KVM requirement for max RAM chunk where memory region alias would suffice.
>>
>> Unfortunately, memory_region_allocate_system_memory() usage created migration
>> dependency where guest RAM is transferred in migration stream as several 
>> RAMBlocks
>> if it's more than KVM_SLOT_MAX_BYTES. During v2 review it was agreed to 
>> ignore
>> migration breakage (documenting it in release notes) and leaving only KVM 
>> fix.
>>
>> In order to replace these several RAM chunks with a single memdev and keep it
>> working with KVM memslot size limit, following was done:
>>* [1/2] split too big RAM chunk inside of KVM code on several memory slots
>>if necessary
>>* [2/2] drop manual ram splitting in s390 code
>>
>>
>> CC: pbonz...@redhat.com
>> CC: qemu-s3...@nongnu.org
>> CC: borntrae...@de.ibm.com
>> CC: th...@redhat.com
>> CC: da...@redhat.com
>> CC: coh...@redhat.com
> 
> With the fixup this patch set seems to work on s390. I can start 9TB guests 
> and
> I can migrate smaller guests between 4.1+patch and 4.0 and 3.1. I currently 
> can
> not test migration for the 9TB guest due to lack of a 2nd system. 

I have to correct myself. The 9TB guest started up but it does not seem to do
anything useful (it hangs).

I will try to investigate. 


>>
>> Igor Mammedov (2):
>>   kvm: s390: split too big memory section on several memslots
>>   s390: do not call memory_region_allocate_system_memory() multiple
>> times
>>
>>  include/hw/s390x/s390-virtio-ccw.h | 10 
>>  include/sysemu/kvm_int.h   |  1 +
>>  accel/kvm/kvm-all.c| 79 ++
>>  hw/s390x/s390-virtio-ccw.c | 30 ++--
>>  target/s390x/kvm.c |  1 +
>>  5 files changed, 63 insertions(+), 58 deletions(-)
>>

Re: [Qemu-devel] make vm-build-openbsd (was: Re: [PATCH v2 22/23] tests: Run the iotests during "make check" again)

[Thomas Huth]

On 8/2/19 4:28 PM, Gerd Hoffmann wrote:
> On Tue, Jul 23, 2019 at 09:20:43AM +0200, Thomas Huth wrote:
>> On 22/07/2019 21.53, Philippe Mathieu-Daudé wrote:
>> [...]
>>> Since Gerd updated the OpenBSD image, do you know if we can run vm-test
>>> again?
>>
>> I just tried it, but the OpenBSD build seems to be completely broken right 
>> now:
>>
>> $ nice make vm-build-openbsd 
> 
> Works fine here.
> Can you try again with "V=1" ?

Meanwhile, I've updated my system from RHEL7 to RHEL8, and now when I
tried again, it's also working fine here. Must have been some oddity
with RHEL7, I guess...

Anyway, thanks for checking,
 Thomas

Re: [Qemu-devel] [PATCH for-4.2 v3 0/2] s390: stop abusing memory_region_allocate_system_memory()

[Christian Borntraeger]

On 02.08.19 15:32, Igor Mammedov wrote:
> Changelog:
>   since v2:
> - break migration from old QEMU (since 2.12-4.1) for guest with >8TB RAM
>   and drop migratable aliases patch as was agreed during v2 review
> - drop 4.2 machines patch as it's not prerequisite anymore
>   since v1:
> - include 4.2 machines patch for adding compat RAM layout on top
> - 2/4 add missing in v1 patch for splitting too big MemorySection on
>   several memslots
> - 3/4 amend code path on alias destruction to ensure that RAMBlock is
>   cleaned properly
> - 4/4 add compat machine code to keep old layout (migration-wise) for
>   4.1 and older machines 
> 
> 
> While looking into unifying guest RAM allocation to use hostmem backends
> for initial RAM (especially when -mempath is used) and retiring
> memory_region_allocate_system_memory() API, leaving only single hostmem 
> backend,
> I was inspecting how currently it is used by boards and it turns out several
> boards abuse it by calling the function several times (despite documented 
> contract
> forbiding it).
> 
> s390 is one of such boards where KVM limitation on memslot size got propagated
> to board design and memory_region_allocate_system_memory() was abused to 
> satisfy
> KVM requirement for max RAM chunk where memory region alias would suffice.
> 
> Unfortunately, memory_region_allocate_system_memory() usage created migration
> dependency where guest RAM is transferred in migration stream as several 
> RAMBlocks
> if it's more than KVM_SLOT_MAX_BYTES. During v2 review it was agreed to ignore
> migration breakage (documenting it in release notes) and leaving only KVM fix.
> 
> In order to replace these several RAM chunks with a single memdev and keep it
> working with KVM memslot size limit, following was done:
>* [1/2] split too big RAM chunk inside of KVM code on several memory slots
>if necessary
>* [2/2] drop manual ram splitting in s390 code
> 
> 
> CC: pbonz...@redhat.com
> CC: qemu-s3...@nongnu.org
> CC: borntrae...@de.ibm.com
> CC: th...@redhat.com
> CC: da...@redhat.com
> CC: coh...@redhat.com

With the fixup this patch set seems to work on s390. I can start 9TB guests and
I can migrate smaller guests between 4.1+patch and 4.0 and 3.1. I currently can
not test migration for the 9TB guest due to lack of a 2nd system. 
> 
> Igor Mammedov (2):
>   kvm: s390: split too big memory section on several memslots
>   s390: do not call memory_region_allocate_system_memory() multiple
> times
> 
>  include/hw/s390x/s390-virtio-ccw.h | 10 
>  include/sysemu/kvm_int.h   |  1 +
>  accel/kvm/kvm-all.c| 79 ++
>  hw/s390x/s390-virtio-ccw.c | 30 ++--
>  target/s390x/kvm.c |  1 +
>  5 files changed, 63 insertions(+), 58 deletions(-)
> 

Re: [Qemu-devel] [PATCH 22/28] Include hw/boards.h a bit less

[Markus Armbruster]

Markus Armbruster  writes:

> Eduardo Habkost  writes:
>
>> On Fri, Jul 26, 2019 at 02:05:36PM +0200, Markus Armbruster wrote:
>>> hw/boards.h pulls in almost 60 headers.  The less we include it into
>>> headers, the better.  As a first step, drop superfluous inclusions,
>>> and downgrade some more to what's actually needed.  Gets rid of just
>>> one inclusion into a header.
>>> 
>>> Cc: Eduardo Habkost 
>>> Cc: Marcel Apfelbaum 
>>> Signed-off-by: Markus Armbruster 
>>> ---
>>
>> The following files use the MACHINE macro and require
>> hw/boards.h, but are touched by this patch:
>>
>> hw/acpi/cpu.c:MachineState *machine = MACHINE(qdev_get_machine());
>> hw/acpi/memory_hotplug.c:MachineState *machine = 
>> MACHINE(qdev_get_machine());
>> hw/i386/intel_iommu.c:MachineState *ms = MACHINE(qdev_get_machine());
>> hw/i386/x86-iommu.c:MachineState *ms = MACHINE(qdev_get_machine());
>> hw/ppc/spapr_rtas.c:MachineState *ms = MACHINE(qdev_get_machine());
>> hw/s390x/s390-stattrib-kvm.c:MachineState *machine = 
>> MACHINE(qdev_get_machine());
>> hw/s390x/s390-stattrib-kvm.c:MachineState *machine = 
>> MACHINE(qdev_get_machine());
>
> Since they still compile, they obviously still get it indirectly.  I'll
> drop these hunks.
>
>> Maybe there are other files touched by this patch that require
>> struct MachineClass or struct MachineState contents to be
>> defined, but this is a bit trickier to verify.
>
> I tried to exclude those, but I might have screwed it up, just like I
> screwed up for MACHINE().  I'll double-check.

None contain matches for /\bMachine(Class|State) *[^ *]/.

Re: [Qemu-devel] [PATCH] block: Simplify bdrv_filter_default_perms()

[Eric Blake]

On 8/2/19 9:03 AM, Kevin Wolf wrote:
> The same change as commit 2b23f28639 ('block/copy-on-read: Fix
> permissions for inactive node') made for the copy-on-read driver can be
> made for bdrv_filter_default_perms(): Retaining the old permissions from
> the BdrvChild if it is given complicates things unnecessary when in the

unnecessarily

> end this only means that the options set in the c == NULL case (i.e.
> during child creation) are retained.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c | 12 ++--
>  1 file changed, 2 insertions(+), 10 deletions(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] make vm-build-openbsd (was: Re: [PATCH v2 22/23] tests: Run the iotests during "make check" again)

[Gerd Hoffmann]

On Tue, Jul 23, 2019 at 09:20:43AM +0200, Thomas Huth wrote:
> On 22/07/2019 21.53, Philippe Mathieu-Daudé wrote:
> [...]
> > Since Gerd updated the OpenBSD image, do you know if we can run vm-test
> > again?
> 
> I just tried it, but the OpenBSD build seems to be completely broken right 
> now:
> 
> $ nice make vm-build-openbsd 

Works fine here.
Can you try again with "V=1" ?

thanks,
  Gerd

Re: [Qemu-devel] [RFC PATCH v1 22/22] target/i386: reimplement (V)P(EQ, CMP)(B, W, D)

[Aleksandar Markovic]

>
>
>   Functions whose address are passed as a callback, as these are, are
> always forced out of line.
>
>
OK, Richard. However, on a much higher level than this single patch, I am
really
curious about this: what would be the rationale beyond the use of callbacks
in TCG
vector support interface? What is, in fact, achieved with such interface
design that
could not be achieved with callback-less approach?

Thanks,
Aleksandar

Re: [Qemu-devel] [PATCH for-4.2 v3 0/2] s390: stop abusing memory_region_allocate_system_memory()

[no-reply]

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



Hi,

This series failed build test on s390x host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e

echo
echo "=== ENV ==="
env

echo
echo "=== PACKAGES ==="
rpm -qa

echo
echo "=== UNAME ==="
uname -a

CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

  CC  s390x-softmmu/target/s390x/sigp.o
  CC  s390x-softmmu/target/s390x/kvm.o
/var/tmp/patchew-tester-tmp-bcfxbkeg/src/target/s390x/kvm.c: In function 
‘kvm_arch_init’:
/var/tmp/patchew-tester-tmp-bcfxbkeg/src/target/s390x/kvm.c:350:5: error: 
implicit declaration of function ‘kvm_set_max_memslot_size’; did you mean 
‘kvm_get_max_memslots’? [-Werror=implicit-function-declaration]
  350 | kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES);
  | ^~~~
  | kvm_get_max_memslots
/var/tmp/patchew-tester-tmp-bcfxbkeg/src/target/s390x/kvm.c:350:5: error: 
nested extern declaration of ‘kvm_set_max_memslot_size’ [-Werror=nested-externs]
cc1: all warnings being treated as errors
make[1]: *** [/var/tmp/patchew-tester-tmp-bcfxbkeg/src/rules.mak:69: 
target/s390x/kvm.o] Error 1
make: *** [Makefile:472: s390x-softmmu/all] Error 2


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

Re: [Qemu-devel] [PATCH] ehci: Ensure that device is not NULL before calling usb_ep_get

[Gerd Hoffmann]

On Wed, Jul 31, 2019 at 01:08:50PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/30/19 7:45 PM, Guenter Roeck wrote:
> > The following assert is seen once in a while while resetting the
> > Linux kernel.
> > 
> > qemu-system-x86_64: hw/usb/core.c:734: usb_ep_get:
> > Assertion `dev != NULL' failed.
> > 
> > The call to usb_ep_get() originates from ehci_execute().
> > Analysis and debugging shows that p->queue->dev can indeed be NULL
> > in this function. Add check for this condition and return an error
> > if it is seen.
> 
> Your patch is not wrong as it corrects your case, but I wonder why we
> get there. This assert seems to have catched a bug.

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

> Gerd, shouldn't we call usb_packet_cleanup() in ehci_reset() rather than
> ehci_finalize()? Then we shouldn't need this patch.

The two ehci_queues_rip_all() calls in ehci_reset() should clean up everything
properly.

Can you try the patch below to see whenever a ehci_find_device failure is the
root cause?

thanks,
  Gerd

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 62dab0592fa2..2b0a57772ed5 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1644,6 +1644,10 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, 
int async)
 q->dev = ehci_find_device(q->ehci,
   get_field(q->qh.epchar, QH_EPCHAR_DEVADDR));
 }
+if (q->dev == NULL) {
+fprintf(stderr, "%s: device %d not found\n", __func__,
+get_field(q->qh.epchar, QH_EPCHAR_DEVADDR));
+}
 
 if (async && (q->qh.epchar & QH_EPCHAR_H)) {
 

Re: [Qemu-devel] [PULL 0/1] Slirp cve 2019 14378 patches

[Peter Maydell]

On Fri, 2 Aug 2019 at 12:18, Marc-André Lureau
 wrote:
>
> The following changes since commit 3bd6cbbb181b6ae60a1d1f33ccd325b45f71aa2a:
>
>   Update version for v4.1.0-rc3 release (2019-07-30 22:02:05 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/elmarco/qemu.git tags/slirp-CVE-2019-14378-pull-request
>
> for you to fetch changes up to e1a4a24d262ba5ac74ea1795adb3ab1cd574c7fb:
>
>   slirp: update with CVE-2019-14378 fix (2019-08-02 15:14:56 +0400)
>
> 
> Slirp CVE-2019-14378 pull request
>
> 
>
> Marc-André Lureau (1):
>   slirp: update with CVE-2019-14378 fix
>
>  slirp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)



Applied, thanks.

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

-- PMM

Re: [Qemu-devel] [PATCH 2/3] iotests: Enable -d for Python non-unittest tests

[Kevin Wolf]

Am 01.08.2019 um 19:57 hat Max Reitz geschrieben:
> On 01.08.19 17:17, Kevin Wolf wrote:
> > The part of iotests.main() that is related to the implementation of the
> > debug option -d and enables QEMU and QMP logging is not only useful in
> > tests that use the Python unittest framework, but also in tests that
> > work by comparing with a reference output.
> > 
> > Factor these parts out into iotests.init() and call it from the test
> > cases that currently lack support for debug output.
> 
> How does this relate to
> https://lists.nongnu.org/archive/html/qemu-block/2019-07/msg01212.html ?

Hm, no idea? :-)

Looks like John's patch depends on some other patches which would then
conflict with mine, too, so maybe I'll just drop my patch and wait what
happens?

John, any opinion?

Kevin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 0/2] pci: bugfix

[Peter Maydell]

On Tue, 30 Jul 2019 at 20:45, Michael S. Tsirkin  wrote:
>
> The following changes since commit 22235bb609c18547cf6b215bad1f9d2ec56ad371:
>
>   pc-dimm: fix crash when invalid slot number is used (2019-07-29 16:57:27 
> -0400)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to c8557f1b4873549fc231496f3a9e5389c8dd:
>
>   pcie_root_port: Disable ACS on older machines (2019-07-30 12:07:07 -0400)
>
> 
> pci: bugfix
>
> A last minute fix to cross-version migration.
> Better late than never.
>
> Signed-off-by: Michael S. Tsirkin 
>
> 
> Dr. David Alan Gilbert (2):
>   pcie_root_port: Allow ACS to be disabled
>   pcie_root_port: Disable ACS on older machines


Applied, thanks (before rc3 was tagged but I forgot to send this email).

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

-- PMM

Re: [Qemu-devel] [RFC PATCH v1 22/22] target/i386: reimplement (V)P(EQ, CMP)(B, W, D)

[Jan Bobek]

On 7/31/19 5:31 PM, Richard Henderson wrote:
> On 7/31/19 1:09 PM, Aleksandar Markovic wrote:
>>
>>
>> On Wed, Jul 31, 2019 at 9:51 PM Richard Henderson 
>> > > wrote:
>>
>> On 7/31/19 10:57 AM, Jan Bobek wrote:
>> > +static inline void gen_gvec_cmpeq(unsigned vece, uint32_t dofs,
>> > +                                  uint32_t aofs, uint32_t bofs,
>> > +                                  uint32_t oprsz, uint32_t maxsz)
>> > +{
>> > +    tcg_gen_gvec_cmp(TCG_COND_EQ, vece, dofs, aofs, bofs, oprsz, 
>> maxsz);
>> > +}
>> ...
>> > +static inline void gen_gvec_cmpgt(unsigned vece, uint32_t dofs,
>> > +                                  uint32_t aofs, uint32_t bofs,
>> > +                                  uint32_t oprsz, uint32_t maxsz)
>> > +{
>> > +    tcg_gen_gvec_cmp(TCG_COND_GT, vece, dofs, aofs, bofs, oprsz, 
>> maxsz);
>> > +}
>>
>> Drop the inlines.
>>
>>
>> Why? The compiler will decide at the end of the day, but at least "inline" 
>> here
>> says that the code author thinks that inlining is desirable, logical, and 
>> expected
>> in these cases, which is in turn a valuable information for the code reader.
> 
> In this case it is in fact a lie that will only confuse the reader, as it did
> you.  Functions whose address are passed as a callback, as these are, are
> always forced out of line.
> 
> But beyond that, clang diagnoses unused static inline within *.c while gcc 
> does
> not (I'm not sure I agree with clang, but it is what it is).  By leaving off
> the inline, but compilers will diagnose when code rearrangement leaves a
> function unused.

Dang, I completely forgot about the function-address vs. inlining rule. I 
thought
of these as macros, really; they are only functions because I needed to pass
them to the gen_gvec_ld_modrm_* helpers.

I'll drop the inline, compilers ignore it anyway.

-Jan



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] block: Simplify bdrv_filter_default_perms()

[Kevin Wolf]

The same change as commit 2b23f28639 ('block/copy-on-read: Fix
permissions for inactive node') made for the copy-on-read driver can be
made for bdrv_filter_default_perms(): Retaining the old permissions from
the BdrvChild if it is given complicates things unnecessary when in the
end this only means that the options set in the c == NULL case (i.e.
during child creation) are retained.

Signed-off-by: Kevin Wolf 
---
 block.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index cbd8da5f3b..6db8ecd62b 100644
--- a/block.c
+++ b/block.c
@@ -2168,16 +2168,8 @@ void bdrv_filter_default_perms(BlockDriverState *bs, 
BdrvChild *c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
 {
-if (c == NULL) {
-*nperm = perm & DEFAULT_PERM_PASSTHROUGH;
-*nshared = (shared & DEFAULT_PERM_PASSTHROUGH) | 
DEFAULT_PERM_UNCHANGED;
-return;
-}
-
-*nperm = (perm & DEFAULT_PERM_PASSTHROUGH) |
- (c->perm & DEFAULT_PERM_UNCHANGED);
-*nshared = (shared & DEFAULT_PERM_PASSTHROUGH) |
-   (c->shared_perm & DEFAULT_PERM_UNCHANGED);
+*nperm = perm & DEFAULT_PERM_PASSTHROUGH;
+*nshared = (shared & DEFAULT_PERM_PASSTHROUGH) | DEFAULT_PERM_UNCHANGED;
 }
 
 void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
-- 
2.20.1

Re: [Qemu-devel] [RFC PATCH v1 08/22] target/i386: reimplement (V)PAND, (V)ANDPS, (V)ANDPD

[Jan Bobek]

On 7/31/19 3:35 PM, Richard Henderson wrote:
> On 7/31/19 10:56 AM, Jan Bobek wrote:
>> +#define gen_pand_mm(env, s, modrm)   gen_gvec_ld_modrm_mm  ((env), (s), 
>> (modrm), MO_64, tcg_gen_gvec_and, 0112)
>> +#define gen_pand_xmm(env, s, modrm)  gen_gvec_ld_modrm_xmm ((env), (s), 
>> (modrm), MO_64, tcg_gen_gvec_and, 0112)
>> +#define gen_vpand_xmm(env, s, modrm) gen_gvec_ld_modrm_vxmm((env), (s), 
>> (modrm), MO_64, tcg_gen_gvec_and, 0123)
>> +#define gen_vpand_ymm(env, s, modrm) gen_gvec_ld_modrm_vymm((env), (s), 
>> (modrm), MO_64, tcg_gen_gvec_and, 0123)
>> +#define gen_andps_xmm  gen_pand_xmm
>> +#define gen_vandps_xmm gen_vpand_xmm
>> +#define gen_vandps_ymm gen_vpand_ymm
>> +#define gen_andpd_xmm  gen_pand_xmm
>> +#define gen_vandpd_xmm gen_vpand_xmm
>> +#define gen_vandpd_ymm gen_vpand_ymm
> 
> 
> Why all of these extra defines?
> 
>> +enum {
>> +M_0F= 0x01 << 8,
>> +M_0F38  = 0x02 << 8,
>> +M_0F3A  = 0x04 << 8,
>> +P_66= 0x08 << 8,
>> +P_F3= 0x10 << 8,
>> +P_F2= 0x20 << 8,
>> +VEX_128 = 0x40 << 8,
>> +VEX_256 = 0x80 << 8,
>> +};
>> +
>> +switch(b | M_0F
>> +   | (s->prefix & PREFIX_DATA ? P_66 : 0)
>> +   | (s->prefix & PREFIX_REPZ ? P_F3 : 0)
>> +   | (s->prefix & PREFIX_REPNZ ? P_F2 : 0)
>> +   | (s->prefix & PREFIX_VEX ? (s->vex_l ? VEX_256 : VEX_128) : 0)) 
>> {
> 
> I think you can move this above almost everything in this function, so that 
> all
> of the legacy bits follow this switch.
> 
>> +case 0xdb | M_0F:  gen_pand_mm(env, s, modrm); return;
> 
> You'll want to put these on the next lines -- checkpatch.pl again.
> 
>> +case 0xdb | M_0F | P_66:   gen_pand_xmm(env, s, modrm); return;
>> +case 0xdb | M_0F | P_66 | VEX_128: gen_vpand_xmm(env, s, modrm); return;
>> +case 0xdb | M_0F | P_66 | VEX_256: gen_vpand_ymm(env, s, modrm); return;
>> +case 0x54 | M_0F:  gen_andps_xmm(env, s, modrm); return;
>> +case 0x54 | M_0F | VEX_128:gen_vandps_xmm(env, s, modrm); 
>> return;
>> +case 0x54 | M_0F | VEX_256:gen_vandps_ymm(env, s, modrm); 
>> return;
>> +case 0x54 | M_0F | P_66:   gen_andpd_xmm(env, s, modrm); return;
>> +case 0x54 | M_0F | P_66 | VEX_128: gen_vandpd_xmm(env, s, modrm); 
>> return;
>> +case 0x54 | M_0F | P_66 | VEX_256: gen_vandpd_ymm(env, s, modrm); 
>> return;
>> +default: break;
>> +}
> 
> Perhaps group cases together?
> 
> case 0xdb | M_0F | P_66:  /* PAND */
> case 0x54 | M_0F: /* ANDPS */
> case 0x54 | M_0F | P_66:  /* ANDPD */
>gen_gvec_ld_modrm_xmm(env, s, modrm, MO_64, tcg_gen_gvec_and, 0112);
>return;

As Aleksandar pointed out in his email, the general intuition was to
have self-documenting code. Seeing

case 0x54 | M_0F | VEX_256: gen_vandps_ymm(env, s, modrm); return;

clearly states that this particular case is a VANDPS, and if one wants
to see what we do with it, they can go look gen_vandps_ymm up.

That being said, I have to the conclusion in the meantime that keeping
all the extra macros is just too much code and not worth it, so I'll
do it like you suggest above.

> How are you planning to handle CPUID checks?  I know the currently handling is
> quite spotty, but with a reorg we might as well fix that too.

Good question. CPUID checks are not handled in this patch at all, I
will need to come up with a workable approach.

-Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-4.2 v3 1/2] kvm: s390: split too big memory section on several memslots

[Christian Borntraeger]

On 02.08.19 15:32, Igor Mammedov wrote:
> Max memslot size supported by kvm on s390 is 8Tb,
> move logic of splitting RAM in chunks upto 8T to KVM code.
> 
> This way it will hide KVM specific restrictions in KVM code
> and won't affect baord level design decisions. Which would allow
> us to avoid misusing memory_region_allocate_system_memory() API
> and eventually use a single hostmem backend for guest RAM.
> 
> Signed-off-by: Igor Mammedov 
> ---
> patch prepares only KVM side for switching to single RAM memory region
> another patch will take care of  dropping manual RAM partitioning in
> s390 code.
> ---


/home/cborntra/REPOS/qemu/target/s390x/kvm.c: In function ‘kvm_arch_init’:
/home/cborntra/REPOS/qemu/target/s390x/kvm.c:350:5: error: implicit declaration 
of function ‘kvm_set_max_memslot_size’; did you mean ‘kvm_get_max_memslots’? 
[-Werror=implicit-function-declaration]
  350 | kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES);
  | ^~~~
  | kvm_get_max_memslots
/home/cborntra/REPOS/qemu/target/s390x/kvm.c:350:5: error: nested extern 
declaration of ‘kvm_set_max_memslot_size’ [-Werror=nested-externs]
cc1: all warnings being treated as errors
make[1]: *** [/home/cborntra/REPOS/qemu/rules.mak:69: target/s390x/kvm.o] Error 
1
make: *** [Makefile:472: s390x-softmmu/all] Error 2


We probably need an include of sysemu/kvm_int.h in target/s390x/kvm.c
I will test with that fixup

>  include/hw/s390x/s390-virtio-ccw.h | 10 
>  include/sysemu/kvm_int.h   |  1 +
>  accel/kvm/kvm-all.c| 79 ++
>  hw/s390x/s390-virtio-ccw.c |  9 
>  target/s390x/kvm.c |  1 +
>  5 files changed, 60 insertions(+), 40 deletions(-)
> 
> diff --git a/include/hw/s390x/s390-virtio-ccw.h 
> b/include/hw/s390x/s390-virtio-ccw.h
> index 8aa27199c9..00632f94b4 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -21,6 +21,16 @@
>  #define S390_MACHINE_CLASS(klass) \
>  OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
>  
> +/*
> + * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
> + * as the dirty bitmap must be managed by bitops that take an int as
> + * position indicator. If we have a guest beyond that we will split off
> + * new subregions. The split must happen on a segment boundary (1MB).
> + */
> +#define KVM_MEM_MAX_NR_PAGES ((1ULL << 31) - 1)
> +#define SEG_MSK (~0xfULL)
> +#define KVM_SLOT_MAX_BYTES ((KVM_MEM_MAX_NR_PAGES * TARGET_PAGE_SIZE) & 
> SEG_MSK)
> +
>  typedef struct S390CcwMachineState {
>  /*< private >*/
>  MachineState parent_obj;
> diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
> index 31df465fdc..7f7520bce2 100644
> --- a/include/sysemu/kvm_int.h
> +++ b/include/sysemu/kvm_int.h
> @@ -41,4 +41,5 @@ typedef struct KVMMemoryListener {
>  void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
>AddressSpace *as, int as_id);
>  
> +void kvm_set_max_memslot_size(hwaddr max_slot_size);
>  #endif
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index f450f25295..7a6efb9612 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -138,6 +138,7 @@ bool kvm_direct_msi_allowed;
>  bool kvm_ioeventfd_any_length_allowed;
>  bool kvm_msi_use_devid;
>  static bool kvm_immediate_exit;
> +static hwaddr kvm_max_slot_size = ~0;
>  
>  static const KVMCapabilityInfo kvm_required_capabilites[] = {
>  KVM_CAP_INFO(USER_MEMORY),
> @@ -951,6 +952,14 @@ kvm_check_extension_list(KVMState *s, const 
> KVMCapabilityInfo *list)
>  return NULL;
>  }
>  
> +void kvm_set_max_memslot_size(hwaddr max_slot_size)
> +{
> +g_assert(
> +ROUND_UP(max_slot_size, qemu_real_host_page_size) == max_slot_size
> +);
> +kvm_max_slot_size = max_slot_size;
> +}
> +
>  static void kvm_set_phys_mem(KVMMemoryListener *kml,
>   MemoryRegionSection *section, bool add)
>  {
> @@ -958,7 +967,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
>  int err;
>  MemoryRegion *mr = section->mr;
>  bool writeable = !mr->readonly && !mr->rom_device;
> -hwaddr start_addr, size;
> +hwaddr start_addr, size, slot_size;
>  void *ram;
>  
>  if (!memory_region_is_ram(mr)) {
> @@ -983,41 +992,49 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
>  kvm_slots_lock(kml);
>  
>  if (!add) {
> -mem = kvm_lookup_matching_slot(kml, start_addr, size);
> -if (!mem) {
> -goto out;
> -}
> -if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
> -kvm_physical_sync_dirty_bitmap(kml, section);
> -}
> +do {
> +slot_size = kvm_max_slot_size < size ? kvm_max_slot_size : size;
> +mem = kvm_lookup_matching_slot(kml, start_addr, slot_size);
> +if (!mem) {
> +goto out;
> +   

Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] backup: Copy only dirty areas

[Max Reitz]

On 02.08.19 15:34, Kevin Wolf wrote:
> Am 01.08.2019 um 19:38 hat Max Reitz geschrieben:
>> Hi,
>>
>> In a discussion with Vladimir today, we noticed that the backup job
>> currently is pretty broken when using copy offloading.  I don’t know
>> about you, but my local filesystem (XFS) supports copy offloading, so
>> the job uses it automatically.  That means, that backup is broken and
>> has been broken for a year on my local FS.
>>
>> The last working version was 2.12, so this isn’t a regression in 4.1.
>> We could thus justify moving it to 4.2.  But I think this should really
>> go into 4.1, because this is not really an edge case and as far as I
>> know users cannot do anything to prevent the backup job from performing
>> copy offloading if the system and all involved block drivers support it.
>> I just wonder why nobody has noticed...
> 
> This sounds bad indeed. But are we already going to have an -rc4 for
> other reasons, or would this mean to have one only for the backup fix?

Looks like we are going to have an rc4 in any case because of the slirp
submodule.

> Also, if you say this was broken in 4.0, Cc: qemu-stable

Oh, right.  I have been forgetting that for quite a while now.

Max




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-4.2 v3 2/2] s390: do not call memory_region_allocate_system_memory() multiple times

[David Hildenbrand]

On 02.08.19 15:41, Christian Borntraeger wrote:
> 
> 
> On 02.08.19 15:36, David Hildenbrand wrote:
>> On 02.08.19 15:32, Igor Mammedov wrote:
>>> s390 was trying to solve limited KVM memslot size issue by abusing
>>> memory_region_allocate_system_memory(), which breaks API contract
>>> where the function might be called only once.
>>>
>>> Beside an invalid use of API, the approach also introduced migration
>>> issue, since RAM chunks for each KVM_SLOT_MAX_BYTES are transferred in
>>> migration stream as separate RAMBlocks.
>>>
>>> After discussion [1], it was agreed to break migration from older
>>> QEMU for guest with RAM >8Tb (as it was relatively new (since 2.12)
>>> and considered to be not actually used downstream).
>>> Migration should keep working for guests with less than 8TB and for
>>> more than 8TB with QEMU 4.2 and newer binary.
>>> In case user tries to migrate more than 8TB guest, between incompatible
>>> QEMU versions, migration should fail gracefully due to non-exiting
>>> RAMBlock ID or RAMBlock size mismatch.
>>>
>>> Taking in account above and that now KVM code is able to split too
>>> big MemorySection into several memslots, stop abusing
>>>   memory_region_allocate_system_memory()
>>> and use only one memory region for RAM.
>>>
>>> 1) [PATCH RFC v2 4/4] s390: do not call  
>>> memory_region_allocate_system_memory() multiple times
>>>
>>> Signed-off-by: Igor Mammedov 
>>> ---
>>> v3:
>>>   - drop migration compat code
>>>
>>> PS:
>>> I don't have access to a suitable system to test it.
>>> ---
>>>  hw/s390x/s390-virtio-ccw.c | 21 +++--
>>>  1 file changed, 3 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 0c03ffb7c7..e30058df38 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -154,27 +154,12 @@ static void virtio_ccw_register_hcalls(void)
>>>  static void s390_memory_init(ram_addr_t mem_size)
>>>  {
>>>  MemoryRegion *sysmem = get_system_memory();
>>> -ram_addr_t chunk, offset = 0;
>>> -unsigned int number = 0;
>>> +MemoryRegion *ram = g_new(MemoryRegion, 1);
>>>  Error *local_err = NULL;
>>> -gchar *name;
>>>  
>>>  /* allocate RAM for core */
>>> -name = g_strdup_printf("s390.ram");
>>> -while (mem_size) {
>>> -MemoryRegion *ram = g_new(MemoryRegion, 1);
>>> -uint64_t size = mem_size;
>>> -
>>> -/* KVM does not allow memslots >= 8 TB */
>>> -chunk = MIN(size, KVM_SLOT_MAX_BYTES);
>>> -memory_region_allocate_system_memory(ram, NULL, name, chunk);
>>> -memory_region_add_subregion(sysmem, offset, ram);
>>> -mem_size -= chunk;
>>> -offset += chunk;
>>> -g_free(name);
>>> -name = g_strdup_printf("s390.ram.%u", ++number);
>>> -}
>>> -g_free(name);
>>> +memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
>>> +memory_region_add_subregion(sysmem, 0, ram);
>>>  
>>>  /*
>>>   * Configure the maximum page size. As no memory devices were created
>>>
>>
>> Reviewed-by: David Hildenbrand 
>>
>> We have to remember putting it into the next release notes. (or do we
>> even want to get this into v4.1 ?)
> 
> This is too invasive for 4.1
> 

Makes sense

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH for-4.2 v3 2/2] s390: do not call memory_region_allocate_system_memory() multiple times

[Christian Borntraeger]

On 02.08.19 15:36, David Hildenbrand wrote:
> On 02.08.19 15:32, Igor Mammedov wrote:
>> s390 was trying to solve limited KVM memslot size issue by abusing
>> memory_region_allocate_system_memory(), which breaks API contract
>> where the function might be called only once.
>>
>> Beside an invalid use of API, the approach also introduced migration
>> issue, since RAM chunks for each KVM_SLOT_MAX_BYTES are transferred in
>> migration stream as separate RAMBlocks.
>>
>> After discussion [1], it was agreed to break migration from older
>> QEMU for guest with RAM >8Tb (as it was relatively new (since 2.12)
>> and considered to be not actually used downstream).
>> Migration should keep working for guests with less than 8TB and for
>> more than 8TB with QEMU 4.2 and newer binary.
>> In case user tries to migrate more than 8TB guest, between incompatible
>> QEMU versions, migration should fail gracefully due to non-exiting
>> RAMBlock ID or RAMBlock size mismatch.
>>
>> Taking in account above and that now KVM code is able to split too
>> big MemorySection into several memslots, stop abusing
>>   memory_region_allocate_system_memory()
>> and use only one memory region for RAM.
>>
>> 1) [PATCH RFC v2 4/4] s390: do not call  
>> memory_region_allocate_system_memory() multiple times
>>
>> Signed-off-by: Igor Mammedov 
>> ---
>> v3:
>>   - drop migration compat code
>>
>> PS:
>> I don't have access to a suitable system to test it.
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 21 +++--
>>  1 file changed, 3 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 0c03ffb7c7..e30058df38 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -154,27 +154,12 @@ static void virtio_ccw_register_hcalls(void)
>>  static void s390_memory_init(ram_addr_t mem_size)
>>  {
>>  MemoryRegion *sysmem = get_system_memory();
>> -ram_addr_t chunk, offset = 0;
>> -unsigned int number = 0;
>> +MemoryRegion *ram = g_new(MemoryRegion, 1);
>>  Error *local_err = NULL;
>> -gchar *name;
>>  
>>  /* allocate RAM for core */
>> -name = g_strdup_printf("s390.ram");
>> -while (mem_size) {
>> -MemoryRegion *ram = g_new(MemoryRegion, 1);
>> -uint64_t size = mem_size;
>> -
>> -/* KVM does not allow memslots >= 8 TB */
>> -chunk = MIN(size, KVM_SLOT_MAX_BYTES);
>> -memory_region_allocate_system_memory(ram, NULL, name, chunk);
>> -memory_region_add_subregion(sysmem, offset, ram);
>> -mem_size -= chunk;
>> -offset += chunk;
>> -g_free(name);
>> -name = g_strdup_printf("s390.ram.%u", ++number);
>> -}
>> -g_free(name);
>> +memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
>> +memory_region_add_subregion(sysmem, 0, ram);
>>  
>>  /*
>>   * Configure the maximum page size. As no memory devices were created
>>
> 
> Reviewed-by: David Hildenbrand 
> 
> We have to remember putting it into the next release notes. (or do we
> even want to get this into v4.1 ?)

This is too invasive for 4.1

Re: [Qemu-devel] [PATCH for-4.2 v3 2/2] s390: do not call memory_region_allocate_system_memory() multiple times

[David Hildenbrand]

On 02.08.19 15:32, Igor Mammedov wrote:
> s390 was trying to solve limited KVM memslot size issue by abusing
> memory_region_allocate_system_memory(), which breaks API contract
> where the function might be called only once.
> 
> Beside an invalid use of API, the approach also introduced migration
> issue, since RAM chunks for each KVM_SLOT_MAX_BYTES are transferred in
> migration stream as separate RAMBlocks.
> 
> After discussion [1], it was agreed to break migration from older
> QEMU for guest with RAM >8Tb (as it was relatively new (since 2.12)
> and considered to be not actually used downstream).
> Migration should keep working for guests with less than 8TB and for
> more than 8TB with QEMU 4.2 and newer binary.
> In case user tries to migrate more than 8TB guest, between incompatible
> QEMU versions, migration should fail gracefully due to non-exiting
> RAMBlock ID or RAMBlock size mismatch.
> 
> Taking in account above and that now KVM code is able to split too
> big MemorySection into several memslots, stop abusing
>   memory_region_allocate_system_memory()
> and use only one memory region for RAM.
> 
> 1) [PATCH RFC v2 4/4] s390: do not call  
> memory_region_allocate_system_memory() multiple times
> 
> Signed-off-by: Igor Mammedov 
> ---
> v3:
>   - drop migration compat code
> 
> PS:
> I don't have access to a suitable system to test it.
> ---
>  hw/s390x/s390-virtio-ccw.c | 21 +++--
>  1 file changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 0c03ffb7c7..e30058df38 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -154,27 +154,12 @@ static void virtio_ccw_register_hcalls(void)
>  static void s390_memory_init(ram_addr_t mem_size)
>  {
>  MemoryRegion *sysmem = get_system_memory();
> -ram_addr_t chunk, offset = 0;
> -unsigned int number = 0;
> +MemoryRegion *ram = g_new(MemoryRegion, 1);
>  Error *local_err = NULL;
> -gchar *name;
>  
>  /* allocate RAM for core */
> -name = g_strdup_printf("s390.ram");
> -while (mem_size) {
> -MemoryRegion *ram = g_new(MemoryRegion, 1);
> -uint64_t size = mem_size;
> -
> -/* KVM does not allow memslots >= 8 TB */
> -chunk = MIN(size, KVM_SLOT_MAX_BYTES);
> -memory_region_allocate_system_memory(ram, NULL, name, chunk);
> -memory_region_add_subregion(sysmem, offset, ram);
> -mem_size -= chunk;
> -offset += chunk;
> -g_free(name);
> -name = g_strdup_printf("s390.ram.%u", ++number);
> -}
> -g_free(name);
> +memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
> +memory_region_add_subregion(sysmem, 0, ram);
>  
>  /*
>   * Configure the maximum page size. As no memory devices were created
> 

Reviewed-by: David Hildenbrand 

We have to remember putting it into the next release notes. (or do we
even want to get this into v4.1 ?)


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] backup: Copy only dirty areas

[Kevin Wolf]

Am 01.08.2019 um 19:38 hat Max Reitz geschrieben:
> Hi,
> 
> In a discussion with Vladimir today, we noticed that the backup job
> currently is pretty broken when using copy offloading.  I don’t know
> about you, but my local filesystem (XFS) supports copy offloading, so
> the job uses it automatically.  That means, that backup is broken and
> has been broken for a year on my local FS.
> 
> The last working version was 2.12, so this isn’t a regression in 4.1.
> We could thus justify moving it to 4.2.  But I think this should really
> go into 4.1, because this is not really an edge case and as far as I
> know users cannot do anything to prevent the backup job from performing
> copy offloading if the system and all involved block drivers support it.
> I just wonder why nobody has noticed...

This sounds bad indeed. But are we already going to have an -rc4 for
other reasons, or would this mean to have one only for the backup fix?

Also, if you say this was broken in 4.0, Cc: qemu-stable

Kevin

Re: [Qemu-devel] [RFC PATCH v1 06/22] target/i386: introduce gen_gvec_ld_modrm_* helpers

[Jan Bobek]

On 7/31/19 6:47 PM, Richard Henderson wrote:
> I suppose there aren't so many different combinations, but did you consider
> separate callbacks per operand?  If you have
> 
> typedef unsigned (*gen_offset)(CPUX86State *, DisasContext *, int);
> 
> static unsigned offset_Pq(CPUX86State *env, DisasContext *s, int modrm)
> {
> int reg = (modrm >> 3) & 7; /* Ignore REX_R */
> return offsetof(CPUX86State, fpregs[reg].mmx);
> }
> 
> static unsigned offset_Qq(CPUX86State *env, DisasContext *s, int modrm)
> {
> int mod = (modrm >> 6) & 3;
> unsigned ret;
> 
> if (mod == 3) {
> int rm = modrm & 7; /* Ignore REX_B */
> ret = offsetof(CPUX86State, fpregs[rm].mmx);
> } else {
> ret = offsetof(CPUX86State, mmx_t0);
> gen_lea_modrm(env, s, modrm);
> gen_ldq_env_A0(s, ret);
> }
> return ret;
> }
> 
> static unsigned offset_Vx(CPUX86State *env, DisasContext *s, int modrm)
> {
> int reg = ((modrm >> 3) & 7) | REX_R(s);
> return offsetof(CPUX86State, xmm_regs[reg]);
> }
> 
> static unsigned offset_Wx(CPUX86State *env, DisasContext *s, int modrm)
> {
> int mod = (modrm >> 6) & 3;
> unsigned ret;
> 
> if (mod == 3) {
> int rm = (modrm & 7) | REX_B(s);
> ret = offsetof(CPUX86State, xmm_regs[rm]);
> } else {
> ret = offsetof(CPUX86State, xmm_t0);
> gen_lea_modrm(env, s, modrm);
> gen_ldo_env_A0(s, ret);
> }
> return ret;
> }
> 
> static unsigned offset_Hx(CPUX86State *env, DisasContext *s, int modrm)
> {
> return offsetof(CPUX86State, xmm_regs[s->vex_v]);
> }
> 
> Then you can have
> 
> #define GEN_GVEC_3(OP0, OP1, OP2, OPRSZ, MAXSZ)
> static void gen_gvec_ld_modrm_##OP0##OP1##OP2(CPUX86State *env,  \
> DisasContext *s, int modrm, unsigned vece,  gen_gvec_2_fp_t gen) \
> {   \
> int ofd = offset_##OP0(env, s, modrm);  \
> int of1 = offset_##OP1(env, s, modrm);  \
> int of2 = offset_##OP2(env, s, modrm);  \
> gen(vece, opd, opa, opb, OPRSZ, MAXSZ); \
> }
> 
> GEN_GVEC_3(Pq, Pq, Qq, sizeof(MMXReg), sizeof(MMXReg))
> GEN_GVEC_3(Vx, Vx, Wx, sizeof(XMMReg), max_vec_size(s))
> GEN_GVEC_3(Vx, Hx, Wx, sizeof(XMMReg), max_vec_size(s))
> 
> The PqPqQq and VxVxWx sub-strings aren't quite canonical, but imo a better fit
> to the actual format of the instruction, with 2 inputs and 1 output.

Funny, I had a similar idea and converged to almost identical
solution. This will be part of v2.

> You can also do
> 
> GEN_GVEC_3(Pq, Qq, Pq, sizeof(MMXReg), sizeof(MMXReg))
> 
> for those rare "reversed" operations like PANDN.  Now you don't need to carry
> around the OPCTL argument, which I initially found non-obvious.

Yup, solves the problem nicely and more clearly.

> I initially thought you'd be able to infer maxsz from the set of arguments, 
> but
> since there are vex encoded operations that do not use vex. that is not
> always the case.  Thus I suggest
> 
> static size_t max_vec_size(DisasContext *s)
> {
> if (s->prefixes & PREFIX_VEX) {
> /*
>  * TODO: When avx512 is supported and enabled, sizeof(ZMMReg).
>  * In the meantime don't waste time zeroing data that is not
>  * architecturally present.
>  */
> return sizeof(YMMReg);
> } else {
> /* Without vex encoding, only the low 128 bits are modified. */
> return sizeof(XMMReg);
> }
> }

Looks good.

-Jan



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH for-4.2 v3 2/2] s390: do not call memory_region_allocate_system_memory() multiple times

[Igor Mammedov]

s390 was trying to solve limited KVM memslot size issue by abusing
memory_region_allocate_system_memory(), which breaks API contract
where the function might be called only once.

Beside an invalid use of API, the approach also introduced migration
issue, since RAM chunks for each KVM_SLOT_MAX_BYTES are transferred in
migration stream as separate RAMBlocks.

After discussion [1], it was agreed to break migration from older
QEMU for guest with RAM >8Tb (as it was relatively new (since 2.12)
and considered to be not actually used downstream).
Migration should keep working for guests with less than 8TB and for
more than 8TB with QEMU 4.2 and newer binary.
In case user tries to migrate more than 8TB guest, between incompatible
QEMU versions, migration should fail gracefully due to non-exiting
RAMBlock ID or RAMBlock size mismatch.

Taking in account above and that now KVM code is able to split too
big MemorySection into several memslots, stop abusing
  memory_region_allocate_system_memory()
and use only one memory region for RAM.

1) [PATCH RFC v2 4/4] s390: do not call  memory_region_allocate_system_memory() 
multiple times

Signed-off-by: Igor Mammedov 
---
v3:
  - drop migration compat code

PS:
I don't have access to a suitable system to test it.
---
 hw/s390x/s390-virtio-ccw.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 0c03ffb7c7..e30058df38 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -154,27 +154,12 @@ static void virtio_ccw_register_hcalls(void)
 static void s390_memory_init(ram_addr_t mem_size)
 {
 MemoryRegion *sysmem = get_system_memory();
-ram_addr_t chunk, offset = 0;
-unsigned int number = 0;
+MemoryRegion *ram = g_new(MemoryRegion, 1);
 Error *local_err = NULL;
-gchar *name;
 
 /* allocate RAM for core */
-name = g_strdup_printf("s390.ram");
-while (mem_size) {
-MemoryRegion *ram = g_new(MemoryRegion, 1);
-uint64_t size = mem_size;
-
-/* KVM does not allow memslots >= 8 TB */
-chunk = MIN(size, KVM_SLOT_MAX_BYTES);
-memory_region_allocate_system_memory(ram, NULL, name, chunk);
-memory_region_add_subregion(sysmem, offset, ram);
-mem_size -= chunk;
-offset += chunk;
-g_free(name);
-name = g_strdup_printf("s390.ram.%u", ++number);
-}
-g_free(name);
+memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
+memory_region_add_subregion(sysmem, 0, ram);
 
 /*
  * Configure the maximum page size. As no memory devices were created
-- 
2.18.1

[Qemu-devel] [PATCH for-4.2 v3 1/2] kvm: s390: split too big memory section on several memslots

[Igor Mammedov]

Max memslot size supported by kvm on s390 is 8Tb,
move logic of splitting RAM in chunks upto 8T to KVM code.

This way it will hide KVM specific restrictions in KVM code
and won't affect baord level design decisions. Which would allow
us to avoid misusing memory_region_allocate_system_memory() API
and eventually use a single hostmem backend for guest RAM.

Signed-off-by: Igor Mammedov 
---
patch prepares only KVM side for switching to single RAM memory region
another patch will take care of  dropping manual RAM partitioning in
s390 code.
---
 include/hw/s390x/s390-virtio-ccw.h | 10 
 include/sysemu/kvm_int.h   |  1 +
 accel/kvm/kvm-all.c| 79 ++
 hw/s390x/s390-virtio-ccw.c |  9 
 target/s390x/kvm.c |  1 +
 5 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index 8aa27199c9..00632f94b4 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -21,6 +21,16 @@
 #define S390_MACHINE_CLASS(klass) \
 OBJECT_CLASS_CHECK(S390CcwMachineClass, (klass), TYPE_S390_CCW_MACHINE)
 
+/*
+ * KVM does only support memory slots up to KVM_MEM_MAX_NR_PAGES pages
+ * as the dirty bitmap must be managed by bitops that take an int as
+ * position indicator. If we have a guest beyond that we will split off
+ * new subregions. The split must happen on a segment boundary (1MB).
+ */
+#define KVM_MEM_MAX_NR_PAGES ((1ULL << 31) - 1)
+#define SEG_MSK (~0xfULL)
+#define KVM_SLOT_MAX_BYTES ((KVM_MEM_MAX_NR_PAGES * TARGET_PAGE_SIZE) & 
SEG_MSK)
+
 typedef struct S390CcwMachineState {
 /*< private >*/
 MachineState parent_obj;
diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
index 31df465fdc..7f7520bce2 100644
--- a/include/sysemu/kvm_int.h
+++ b/include/sysemu/kvm_int.h
@@ -41,4 +41,5 @@ typedef struct KVMMemoryListener {
 void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
   AddressSpace *as, int as_id);
 
+void kvm_set_max_memslot_size(hwaddr max_slot_size);
 #endif
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f450f25295..7a6efb9612 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -138,6 +138,7 @@ bool kvm_direct_msi_allowed;
 bool kvm_ioeventfd_any_length_allowed;
 bool kvm_msi_use_devid;
 static bool kvm_immediate_exit;
+static hwaddr kvm_max_slot_size = ~0;
 
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
 KVM_CAP_INFO(USER_MEMORY),
@@ -951,6 +952,14 @@ kvm_check_extension_list(KVMState *s, const 
KVMCapabilityInfo *list)
 return NULL;
 }
 
+void kvm_set_max_memslot_size(hwaddr max_slot_size)
+{
+g_assert(
+ROUND_UP(max_slot_size, qemu_real_host_page_size) == max_slot_size
+);
+kvm_max_slot_size = max_slot_size;
+}
+
 static void kvm_set_phys_mem(KVMMemoryListener *kml,
  MemoryRegionSection *section, bool add)
 {
@@ -958,7 +967,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
 int err;
 MemoryRegion *mr = section->mr;
 bool writeable = !mr->readonly && !mr->rom_device;
-hwaddr start_addr, size;
+hwaddr start_addr, size, slot_size;
 void *ram;
 
 if (!memory_region_is_ram(mr)) {
@@ -983,41 +992,49 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
 kvm_slots_lock(kml);
 
 if (!add) {
-mem = kvm_lookup_matching_slot(kml, start_addr, size);
-if (!mem) {
-goto out;
-}
-if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
-kvm_physical_sync_dirty_bitmap(kml, section);
-}
+do {
+slot_size = kvm_max_slot_size < size ? kvm_max_slot_size : size;
+mem = kvm_lookup_matching_slot(kml, start_addr, slot_size);
+if (!mem) {
+goto out;
+}
+if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
+kvm_physical_sync_dirty_bitmap(kml, section);
+}
 
-/* unregister the slot */
-g_free(mem->dirty_bmap);
-mem->dirty_bmap = NULL;
-mem->memory_size = 0;
-mem->flags = 0;
-err = kvm_set_user_memory_region(kml, mem, false);
-if (err) {
-fprintf(stderr, "%s: error unregistering slot: %s\n",
-__func__, strerror(-err));
-abort();
-}
+/* unregister the slot */
+g_free(mem->dirty_bmap);
+mem->dirty_bmap = NULL;
+mem->memory_size = 0;
+mem->flags = 0;
+err = kvm_set_user_memory_region(kml, mem, false);
+if (err) {
+fprintf(stderr, "%s: error unregistering slot: %s\n",
+__func__, strerror(-err));
+abort();
+}
+start_addr += slot_size;
+} while ((size -= slot_size));
 goto out;
 }
 
 /* 

[Qemu-devel] [PATCH for-4.2 v3 0/2] s390: stop abusing memory_region_allocate_system_memory()

[Igor Mammedov]

Changelog:
  since v2:
- break migration from old QEMU (since 2.12-4.1) for guest with >8TB RAM
  and drop migratable aliases patch as was agreed during v2 review
- drop 4.2 machines patch as it's not prerequisite anymore
  since v1:
- include 4.2 machines patch for adding compat RAM layout on top
- 2/4 add missing in v1 patch for splitting too big MemorySection on
  several memslots
- 3/4 amend code path on alias destruction to ensure that RAMBlock is
  cleaned properly
- 4/4 add compat machine code to keep old layout (migration-wise) for
  4.1 and older machines 


While looking into unifying guest RAM allocation to use hostmem backends
for initial RAM (especially when -mempath is used) and retiring
memory_region_allocate_system_memory() API, leaving only single hostmem backend,
I was inspecting how currently it is used by boards and it turns out several
boards abuse it by calling the function several times (despite documented 
contract
forbiding it).

s390 is one of such boards where KVM limitation on memslot size got propagated
to board design and memory_region_allocate_system_memory() was abused to satisfy
KVM requirement for max RAM chunk where memory region alias would suffice.

Unfortunately, memory_region_allocate_system_memory() usage created migration
dependency where guest RAM is transferred in migration stream as several 
RAMBlocks
if it's more than KVM_SLOT_MAX_BYTES. During v2 review it was agreed to ignore
migration breakage (documenting it in release notes) and leaving only KVM fix.

In order to replace these several RAM chunks with a single memdev and keep it
working with KVM memslot size limit, following was done:
   * [1/2] split too big RAM chunk inside of KVM code on several memory slots
   if necessary
   * [2/2] drop manual ram splitting in s390 code


CC: pbonz...@redhat.com
CC: qemu-s3...@nongnu.org
CC: borntrae...@de.ibm.com
CC: th...@redhat.com
CC: da...@redhat.com
CC: coh...@redhat.com

Igor Mammedov (2):
  kvm: s390: split too big memory section on several memslots
  s390: do not call memory_region_allocate_system_memory() multiple
times

 include/hw/s390x/s390-virtio-ccw.h | 10 
 include/sysemu/kvm_int.h   |  1 +
 accel/kvm/kvm-all.c| 79 ++
 hw/s390x/s390-virtio-ccw.c | 30 ++--
 target/s390x/kvm.c |  1 +
 5 files changed, 63 insertions(+), 58 deletions(-)

-- 
2.18.1

Re: [Qemu-devel] [RFC PATCH v1 05/22] target/i386: introduce gen_ld_modrm_* helpers

[Jan Bobek]

On 7/31/19 3:08 PM, Richard Henderson wrote:
> On 7/31/19 10:56 AM, Jan Bobek wrote:
>> These help with decoding/loading ModR/M vector operands; the operand's
>> register offset is returned, which is suitable for use with gvec
>> infrastructure.
>>
>> Signed-off-by: Jan Bobek 
>> ---
>>  target/i386/translate.c | 47 +
>>  1 file changed, 47 insertions(+)
>>
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 9e22eca2dc..7548677e1f 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
>> @@ -3040,6 +3040,53 @@ static const struct SSEOpHelper_eppi 
>> sse_op_table7[256] = {
>>  [0xdf] = AESNI_OP(aeskeygenassist),
>>  };
>>  
>> +static inline void gen_ld_modrm_PqQq(CPUX86State *env, DisasContext *s, int 
>> modrm,
>> + uint32_t* dofs, uint32_t* aofs)
> 
> s/uint32_t* /uint32_t */
> 
> Drop the inlines; let the compiler choose.
> 
> 
>> +{
>> +const int mod = (modrm >> 6) & 3;
>> +const int reg = (modrm >> 3) & 7; /* no REX_R */
>> +*dofs = offsetof(CPUX86State, fpregs[reg].mmx);
>> +
>> +if(mod == 3) {
> 
> s/if(/if (/
> 
> Both of these errors should be caught by ./scripts/checkpatch.pl.

I have the script set up; I disabled it temporarily (or so I thought)
some time ago when it was preventing me from git stash'ing some
experimental hacks, and never got around to enabling it again.

Anyway, I'll make sure not to forget to run it prior to submission
next time.

>> +gen_ldo_env_A0(s, *aofs); /* FIXME this needs to load 32 bytes for 
>> YMM 
> 
> Better as "TODO", since this isn't broken and in need of fixing, since we do
> not yet support AVX.
> 
> Otherwise,
> Reviewed-by: Richard Henderson 
> 
> 
> r~
> 



signature.asc
Description: OpenPGP digital signature