Re: [PATCH] Semihost SYS_READC implementation (v4)

[Keith Packard]

Peter Maydell  writes:

> I'm going to push for somebody actually writing out a
> document and putting it somewhere that we can point to
> and say "that's the authoritative spec", please...
> it doesn't have to be a big formal thing, but I do
> think you want it written down, because the whole point
> is for multiple implementations and users to interoperate.

That happened in June -- I was just looking at the wrong version of the
spec. In the current version, which can be found here:

https://riscv.org/specifications/

   The RISC-V Instruction Set Manual
   Volume I: Unprivileged ISA
Document Version 20190608-Base-Ratified

Section 2.8 says:

Another use of EBREAK is to support “semihosting”, where the
execution environment includes a debugger that can provide
services over an alternate system call interface built around
the EBREAK instruction.  Because the RISC-V base ISA does not
provide more than one EBREAK instruction, RISC-V semihosting
uses a special sequence of instructions to distinguish a
semihosting EBREAK from a debugger inserted EBREAK.

slli x0, x0, 0x1f   # Entry NOP
ebreak  # Break to debugger
srai x0, x0, 7  # NOP encoding the semihosting call number 7

Note that these three instructions must be 32-bit-wide
instructions, i.e., they mustn’t be among the compressed 16-bit
instructions described in Chapter 16.

The shift NOP instructions are still considered available for
use as HINTS.

Semihosting is a form of service call and would be more
naturally encoded as an ECALL using an existing ABI, but this
would require the debugger to be able to intercept ECALLs, which
is a newer addition to the debug standard.  We intend to move
over to using ECALLs with a standard ABI, in which case,
semihosting can share a service ABI with an existing standard.

We note that ARM processors have also moved to using SVC instead
of BKPT for semihosting calls in newer designs.

I'd like to get the READC patch landed and then post the RISC-V patch
afterwards as the RISC-V patch currently includes READC support.

-- 
-keith


signature.asc
Description: PGP signature

Live migration primary vncviewer blocked.

[Chang Howard]

Hi, I know after live migration, primary would be blocked and vnc is also
blocked.
Could I let primary vnc still work after live migration?
I do vm start after live migration and in text mode primary's vnc can still
work, but graphic mode it would blocked.
How can I do to make in graphic mode vnc can still work too?





thanks a lot!

Re: [Qemu-discuss] Cross-posted : Odd QXL/KVM performance issue with a Windows 7 Guest

[Brad Campbell]

On 6/9/19 4:49 pm, Brad Campbell wrote:

On 2/9/19 6:23 pm, Brad Campbell wrote:



Here is the holdup :

11725@1567416625.003504:qxl_ring_command_check 0 native
11725@1567416625.102653:qxl_io_write 0 native addr=0 
(QXL_IO_NOTIFY_CMD) val=0 size=1 async=0


~100ms delay prior to each logged QXL_IO_NOTIFY_CMD on the AMD box 
which explains the performance difference. Now I just need to figure 
out if that lies in the guest, the guest QXL driver, QEMU or SPICE and 
why it exhibits on the AMD box and not the i7.


To get to this point, I recompiled the kernel on the i7 box with both 
AMD and Intel KVM modules. Once that was running I cloned the drive 
and put it in the AMD box, so the OS, software stack and all 
dependencies are identical.


Reacp :

I have a machine with a Windows 7 VM which is running on an i7-3770. 
This works perfectly.


Clone the disk and put it in a new(ish) AMD Ryzen 1500x machine and the 
display output using qxl/spice is now limited to ~5-7fps.


I originally cloned the entire machine to keep the software versions 
identical.


To simplify debugging and reproduction I'm now using :
- An identical SPICE version to that on the i7.
- A fresh 64 bit Windows 7 VM.
- The D2D benchmark from Crystalmark 2004R7.

The machine is booted with :

qemu -enable-kvm \
  -m 8192\
  -rtc base=localtime\
  -vga qxl\
  -device qxl\
  -global qxl-vga.guestdebug=3\
  -global qxl-vga.cmdlog=1\
  -global qxl-vga.vram_size=65536\
  -global qxl.vram_size=65536\
  -global qxl-vga.ram_size=65536\
  -global qxl.ram_size=65536\
  -net nic,model=virtio\
  -net tap,ifname=tap0,script=/etc/qemu-ifup,vhost=on\
  -usbdevice tablet\
  -spice port=5930,disable-ticketing\
  -device virtio-serial\
  -chardev spicevmc,id=vdagent,name=vdagent\
  -device virtserialport,chardev=vdagent,name=com.redhat.spice.0\
  -smp 3,maxcpus=3,cores=3,threads=1,sockets=1\
  -cpu qemu64,hv_relaxed,hv_spinlocks=0x1fff,hv_vapic,hv_time \
  -drive 
file=/server/VM/Cadbox.raw,if=virtio,aio=threads,format=raw,cache=unsafe 
-boot c \
  -drive 
file=/server/VM/Cadbox_swap.raw,if=virtio,aio=threads,format=raw,cache=unsafe 
\



The D2D benchmark runs through a series of Sprites (stars) and it just 
shuffles them around the screen.


With KVM enabled I get :

Sprite    10 - 8.66fps
Sprite   100 - 8.47fps
Sprite   500 - 8.45fps
Sprite  1000 - 8.18fps
Sprite  5000 - 7.64fps
Sprite 1 - 7.26fps

With the identical system, with KVM disabled I get :

Sprite    10 - 28.97fps
Sprite   100 - 27.24fps
Sprite   500 - 23.85fps
Sprite  1000 - 22.00fps
Sprite  5000 - 11.11fps
Sprite 1 -  4.50fps

On the i7 with the same software version and kvm enabled  :

Sprite    10 - 88.58fps
Sprite   100 - 88.35fps
Sprite   500 - 85.64fps
Sprite  1000 - 83.33fps
Sprite  5000 - 58.08fps
Sprite 1 - 45.29fps



Just to round this out, I spent quite a bit of time trying to profile 
the guest and qxl driver, and then gave up.


I tried installing Windows 8.1 and the new WDDM driver and it didn't 
exhibit the fault, so I resigned myself to rebuild all the guests.


Regards,
Brad
--
An expert is a person who has found out by his own painful
experience all the mistakes that one can make in a very
narrow field. - Niels Bohr

Adding New, Unsupported ISA to Qemu

[Hanson, Seth]

Hello,


I'm looking for in-depth documentation pertaining to how an unsupported 16 bit 
RISC ISA can be emulated in Qemu.


I've referenced this:


https://wiki.qemu.org/Documentation/TCG


and have been hoping there's additional, related documentation that I've 
overlooked.


Please advise.



Respectfully,

Seth

Re: Questions about the VFIO BAR region

[Li Qiang]

Alex Williamson  于2019年11月5日周二 上午2:49写道：

> On Tue, 5 Nov 2019 00:40:39 +0800
> Li Qiang  wrote:
>
> > Hello Alex, Auger and all,
> >
> > I have a question about the VFIO virtual device BAR.
> >
> > In vfio_region_setup, it initialize a ‘region->mem’ MR and set its ops
> to ‘vfio_regions_ops’.
> > In ‘vfio_region_mmap’, it maps the physical device’s MMIO to QEMU’s
> virtual address space
> > as a raw MR ‘region->mmaps[i].mem’.
> > And also it set the latter MR as a subregion of the first one.
> >
> > So when the guest accesses the BAR, it will direct go to the physical
> device’s BAR.
> > My question is here:
> > When the qemu will use the ‘vfio_regions_ops’ to read/write the BAR?
> > Also whey in the last of ‘vfio_region_write/read’ we need to call
> ‘vbasedev->ops->vfio_eoi(vbasedev);’?
>
> We support:
>
>  a) sparse mmaps where the entire BAR is not covered by an mmap
>

Got.



>  b) quirks, which layer on top of the mmaps to provide virtualized
> access
>

Do you mean like in 'vfio_probe_ati_bar4_quirk', register a high priority
subregion of VFIORegion.mem.
So when the guest write the BAR, vfio_regions_ops will be used. Here
'quirks' do you mean such things?

static void vfio_probe_ati_bar4_quirk(VFIOPCIDevice *vdev, int nr)
{
VFIOQuirk *quirk;
VFIOConfigWindowQuirk *window;

...
memory_region_init_io(window->addr_mem, OBJECT(vdev),
  _generic_window_address_quirk, window,
  "vfio-ati-bar4-window-address-quirk", 4);
memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
window->address_offset,
window->addr_mem, 1);
   ...
}



>  c) INTx emulation which disables mmaps MRs in order to detect device
> access as a generic mechanism for inferring interrupt
> acknowledgment.
>

In the above two cases, in 'vfio_region_write/read' we always access the
physical device's BAR.
So as far as I can understand, the physical device(sometimes) will trigger
interrupts. And the responsible of clear it
will be by the 'guest'. So I can't understand why there calls
'vbasedev->ops->vfio_eoi'. Could you please give me an
example.


Thanks,
Li Qiang



>
> The latter being the reason we call vfio_eoi.  Thanks,
>
> Alex
>
>

Re: QEMU for Qualcomm Hexagon - KVM Forum talk and code available

[Aleksandar Markovic]

On Friday, October 25, 2019, Taylor Simpson  wrote:

> We would like inform the you that we will be doing a talk at the KVM Forum
> next week on QEMU for Qualcomm Hexagon.  Alessandro Di Federico, Niccolo
> Izzo, and I have been working independently on implementations of the
> Hexagon target.  We plan to merge the implementations, have a community
> review, and ultimately have Hexagon be an official target in QEMU.  Our
> code is available at the links below.
>
> *https://github.com/revng/qemu-hexagon
> *
>
> *https://github.com/quic/qemu *
>
> If anyone has any feedback on the code as it stands today or guidance on
> how best to prepare it for review, please let us know.
>
>
>

Hi, Taylor, Niccolo (and Alessandro too).

I didn't have a chance to take a look at neither the code nor the docs, but
I did attend you presentation at KVM Forum, and I found it superb and
attractive, one of the best on the conference, if not the very best.

I just have a couple of general questions:

- Regarding the code you plan to upstream, are all SIMD instructions
implemented via tcg API, or perhaps some of them remain being implemented
using helpers?

- Most of SIMD instructions can be viewed simply as several paralel
elementary operations. However, for a given SIMD instruction set, usually
not all of them fit into this pattern. For example, "horizontal add"
(addind data elements from the same SIMD register), various
"pack/unpack/interleave/merge" operations, and more general
"shuffle/permute" operations as well (here I am not sure which of these are
included in Hexagon SIMD set, but there must be some). How did you deal
with them?

- What were the most challenging Hexagon SIMD instructions you came accross
while developing your solution?

Sincerely,
Aleksandar





> Thanks,
>
> Taylor
>

[Bug 1851095] Re: [feature request] awareness of instructions that are well emulated

[Aleksandar Markovic]

Can you provide a clearer repro example of what doesn't wirk on mipsel
platform?

In last two QEMU releases mips (Wave) developers went to great lenghts
making sure both mips SIMD and mips FP instructions (in both scalar and
vector variants) are emulated properly. Some of the unit tests were
published, but also many were left internal, and there are many
integration tests devised and run as well. We in mips (Wave) consider
these two areas well tested. Still, we'll consider seriuosly fixing your
example, if you prove experimentally that this is a mips-related bug,
but just provides us with a reasonably convenient repro procedure.

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

Title:
  [feature request] awareness of instructions that are well emulated

Status in QEMU:
  Invalid

Bug description:
  While qemu's scalar emulation tends to be excellent, qemu's SIMD
  emulation tends to be incorrect (except for arm64 from x86_64)--i have
  found this both for mipsel and arm32. Until these code paths are
  audited, which is probably a large job, it would be nice if qemu knew
  its emulation of this class of instructions was not very good, and
  thus it would give up on finding these instructions if a "careful"
  operation is passed.

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

Re: [PATCH] global: Squash 'the the'

[no-reply]

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



Hi,

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

Subject: [PATCH] global: Squash 'the the'
Type: series
Message-id: 20191104185202.102504-1-dgilb...@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
Switched to a new branch 'test'
42a6850 global: Squash 'the the'

=== OUTPUT BEGIN ===
ERROR: do not use C99 // comments
#24: FILE: disas/libvixl/vixl/invalset.h:105:
+  // Note that this does not mean the backing storage is empty: it can still

total: 1 errors, 0 warnings, 56 lines checked

Commit 42a68503c11b (global: Squash 'the the') has style problems, please 
review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


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

SIGSEGV question (Hexagon)

[Taylor Simpson]

Philippe suggested that I run the TCG tests for Hexagon.  Thanks Philippe!!

I discovered that I'm not handling SIGSEGV properly.  We pass other signal 
tests, but not this one.  I'm hoping someone can help.  The first thing that I 
realized is that I hadn't provided a tlb_fill function for CPUClass.  What is 
this function supposed to do?  I looked at other targets and found they are 
setting cs->exception_index to something and then call cpu_loop_exit_restore.  
I tried various values for exception_index, but the best I seem to get is 
re-executing the offending instruction forever.

Any insight would be greatly appreciated.

Thanks,
Taylor


PS  The only other bug that these tests uncovered was that truncate isn't 
implemented properly.  This was straightforward to fix.

Re: [PATCH] net/cadence_gem: Set PHY autonegotiation restart status

[Alistair Francis]

On Mon, Nov 4, 2019 at 2:02 PM  wrote:
>
> From: Linus Ziegert 
>
> The Linux kernel PHY driver sets AN_RESTART in the BMCR of the
> PHY when autonegotiation is started.
> Recently the kernel started to read back the PHY's AN_RESTART
> bit and now checks whether the autonegotiation is complete and
> the bit was cleared [1]. Otherwise the link status is down.
>
> The emulated PHY needs to clear AN_RESTART immediately to inform
> the kernel driver about the completion of autonegotiation phase.
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c36757eb9dee
>
> Signed-off-by: Linus Ziegert 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/net/cadence_gem.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 7f9cb5ab95..b8be73dc55 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -271,9 +271,10 @@
>  #define PHY_REG_EXT_PHYSPCFC_ST   27
>  #define PHY_REG_CABLE_DIAG   28
>
> -#define PHY_REG_CONTROL_RST  0x8000
> -#define PHY_REG_CONTROL_LOOP 0x4000
> -#define PHY_REG_CONTROL_ANEG 0x1000
> +#define PHY_REG_CONTROL_RST   0x8000
> +#define PHY_REG_CONTROL_LOOP  0x4000
> +#define PHY_REG_CONTROL_ANEG  0x1000
> +#define PHY_REG_CONTROL_ANRESTART 0x0200
>
>  #define PHY_REG_STATUS_LINK 0x0004
>  #define PHY_REG_STATUS_ANEGCMPL 0x0020
> @@ -1345,7 +1346,7 @@ static void gem_phy_write(CadenceGEMState *s, unsigned 
> reg_num, uint16_t val)
>  }
>  if (val & PHY_REG_CONTROL_ANEG) {
>  /* Complete autonegotiation immediately */
> -val &= ~PHY_REG_CONTROL_ANEG;
> +val &= ~(PHY_REG_CONTROL_ANEG | PHY_REG_CONTROL_ANRESTART);
>  s->phy_regs[PHY_REG_STATUS] |= PHY_REG_STATUS_ANEGCMPL;
>  }
>  if (val & PHY_REG_CONTROL_LOOP) {
> --
> 2.21.0
>
>

[Bug 1851095] Re: [feature request] awareness of instructions that are well emulated

[Alex Bennée]

I can confirm bench_simple gives the same result on both qemu-arm and my
aarch32 hardware.

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

Title:
  [feature request] awareness of instructions that are well emulated

Status in QEMU:
  Invalid

Bug description:
  While qemu's scalar emulation tends to be excellent, qemu's SIMD
  emulation tends to be incorrect (except for arm64 from x86_64)--i have
  found this both for mipsel and arm32. Until these code paths are
  audited, which is probably a large job, it would be nice if qemu knew
  its emulation of this class of instructions was not very good, and
  thus it would give up on finding these instructions if a "careful"
  operation is passed.

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

Re: [PATCH v5 0/2] RTC support for QEMU RISC-V virt machine

[Alistair Francis]

On Sat, Nov 2, 2019 at 3:37 AM Peter Maydell  wrote:
>
> On Tue, 29 Oct 2019 at 13:25, Alistair Francis  wrote:
> >
> > On Fri, Oct 25, 2019 at 6:28 AM Anup Patel  wrote:
> > >
> > > This series adds RTC device to QEMU RISC-V virt machine. We have
> > > selected Goldfish RTC device model for this. It's a pretty simple
> > > synthetic device with few MMIO registers and no dependency external
> > > clock. The driver for Goldfish RTC is already available in Linux so
> > > we just need to enable it in Kconfig for RISCV and also update Linux
> > > defconfigs.
> > >
> > > We have tested this series with Linux-5.4-rc4 plus defconfig changes
> > > available in 'goldfish_rtc_v2' branch of:
> > > https://github.com/avpatel/linux.git
> >
> > @Peter Maydell this has been reviewed, do you mind taking this in you
> > next PR? I don't see a maintainer for hw/rtc.
>
> Generally devices used by a single architecture should
> go via the tree of the maintainer who handles that
> architecture -- in this case that's riscv. (The reason
> is that it's only riscv folks who can test whether the
> device works in the machine model, and only riscv
> folks who can reasonably review whether the device
> is the right way of implementing the functionality for
> them.)

Makes sense

Alistair

>
> thanks
> -- PMM

Re: [PATCH v5 0/2] RTC support for QEMU RISC-V virt machine

[Alistair Francis]

On Sun, Nov 3, 2019 at 12:12 AM Anup Patel  wrote:
>
> On Sat, Nov 2, 2019 at 4:44 AM Palmer Dabbelt  wrote:
> >
> > On Fri, 01 Nov 2019 08:40:24 PDT (-0700), a...@brainfault.org wrote:
> > > On Tue, Oct 29, 2019 at 6:55 PM Alistair Francis  
> > > wrote:
> > >>
> > >> On Fri, Oct 25, 2019 at 6:28 AM Anup Patel  wrote:
> > >> >
> > >> > This series adds RTC device to QEMU RISC-V virt machine. We have
> > >> > selected Goldfish RTC device model for this. It's a pretty simple
> > >> > synthetic device with few MMIO registers and no dependency external
> > >> > clock. The driver for Goldfish RTC is already available in Linux so
> > >> > we just need to enable it in Kconfig for RISCV and also update Linux
> > >> > defconfigs.
> > >> >
> > >> > We have tested this series with Linux-5.4-rc4 plus defconfig changes
> > >> > available in 'goldfish_rtc_v2' branch of:
> > >> > https://github.com/avpatel/linux.git
> > >>
> > >> @Peter Maydell this has been reviewed, do you mind taking this in you
> > >> next PR? I don't see a maintainer for hw/rtc.
> > >
> > > It would be great if this series can be taken for QEMU-4.2
> >
> > It doesn't look like there's anyone who maintains hw/rtc, so maybe that's 
> > why
> > this has been going slowly?  I'd happy to PR it, but I don't really have the
> > bandwidth to sign up to maintain more stuff right now.

The PR is reviewed so it should be ok to merge now (and it made it
into soft freeze).

>
> No problem, I will maintain Goldfish RTC emulation until someone else
> is willing to maintain it.

You can add me as a maintainer as well if you want.

Alistair

>
> Regards,
> Anup
>
> >
> > >
> > > Regards,
> > > Anup
> > >
> > >>
> > >> Alistair
> > >>
> > >> >
> > >> > Changes since v4:
> > >> >  - Fixed typo in trace event usage
> > >> >  - Moved goldfish_rtc.h to correct location
> > >> >
> > >> > Changes since v3:
> > >> >  - Address all nit comments from Alistair
> > >> >
> > >> > Changes since v2:
> > >> >  - Rebased on RTC code refactoring
> > >> >
> > >> > Changes since v1:
> > >> >  - Implemented VMState save/restore callbacks
> > >> >
> > >> > Anup Patel (2):
> > >> >   hw: rtc: Add Goldfish RTC device
> > >> >   riscv: virt: Use Goldfish RTC device
> > >> >
> > >> >  hw/riscv/Kconfig  |   1 +
> > >> >  hw/riscv/virt.c   |  15 ++
> > >> >  hw/rtc/Kconfig|   3 +
> > >> >  hw/rtc/Makefile.objs  |   1 +
> > >> >  hw/rtc/goldfish_rtc.c | 288 ++
> > >> >  hw/rtc/trace-events   |   4 +
> > >> >  include/hw/riscv/virt.h   |   2 +
> > >> >  include/hw/rtc/goldfish_rtc.h |  46 ++
> > >> >  8 files changed, 360 insertions(+)
> > >> >  create mode 100644 hw/rtc/goldfish_rtc.c
> > >> >  create mode 100644 include/hw/rtc/goldfish_rtc.h
> > >> >
> > >> > --
> > >> > 2.17.1
> > >> >

Re: [PATCH v6 3/3] MAINTAINERS: Add maintainer entry for Goldfish RTC

[Alistair Francis]

On Sun, Nov 3, 2019 at 12:55 AM Anup Patel  wrote:
>
> Add myself as Goldfish RTC maintainer until someone else is
> willing to maintain it.
>
> Signed-off-by: Anup Patel 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  MAINTAINERS | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c2a68555ae..f200e985da 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -558,6 +558,13 @@ F: include/hw/arm/digic.h
>  F: hw/*/digic*
>  F: include/hw/*/digic*
>
> +Goldfish RTC
> +M: Anup Patel 
> +L: qemu-ri...@nongnu.org
> +S: Maintained
> +F: hw/rtc/goldfish_rtc.c
> +F: include/hw/rtc/goldfish_rtc.h
> +
>  Gumstix
>  M: Peter Maydell 
>  R: Philippe Mathieu-Daud?? 
> --
> 2.17.1
>
>

Re: [PATCH v7 0/11] add failover feature for assigned network devices

[Laine Stump]

On 11/4/19 4:21 PM, Parav Pandit wrote:




Patches are great to see failover capability enabled on the netdevice.
However it's very difficult to test it without having libvirt support.
Can we please have the necessary libvirt enhancements?


I'm working on libvirt support, and will make sure to Cc you with the 
patches.

Re: [RESEND PATCH 0/2] PCI DMA alias support

[Alex Williamson]

On Wed, 23 Oct 2019 16:47:02 -0600
Alex Williamson  wrote:

> Previous posting:
> https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg06182.html
> 
> Rebased (no change) and added Peter's R-b.  Please apply for QEMU 4.2.

Anyone?  I suppose this has missed another release despite pings from
me, Peter, and a resend :-(

Alex
 
> Previous cover letter:
> 
> Please see patch 1/ for the motivation and utility of this series.
> This v1 submission improves on the previous RFC with revised commit
> logs, comments, and more testing, and the missing IVRS support for DMA
> alias ranges is now included.  Testing has been done with Linux guests
> with both SeaBIOS and OVMF with configurations of intel-iommu and
> amd-iommu.  Intel-iommu testing includes device assignment, amd-iommu
> is necessarily limited to emulated devices with interrupt remapping
> disabled and iommu=pt in the guest (enabling interrupt remapping or
> disabling guest passthrough mode fails to work regardless of this
> series).  This series is NOT intended for QEMU v4.1.  Thanks,
> 
> Alex
> 
> ---
> 
> Alex Williamson (2):
>   pci: Use PCI aliases when determining device IOMMU address space
>   hw/i386: AMD-Vi IVRS DMA alias support
> 
> 
>  hw/i386/acpi-build.c |  127 
> +++---
>  hw/pci/pci.c |   43 -
>  2 files changed, 160 insertions(+), 10 deletions(-)

[PATCH] net/cadence_gem: Set PHY autonegotiation restart status

[linus . ziegert]

From: Linus Ziegert 

The Linux kernel PHY driver sets AN_RESTART in the BMCR of the
PHY when autonegotiation is started.
Recently the kernel started to read back the PHY's AN_RESTART
bit and now checks whether the autonegotiation is complete and
the bit was cleared [1]. Otherwise the link status is down.

The emulated PHY needs to clear AN_RESTART immediately to inform
the kernel driver about the completion of autonegotiation phase.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c36757eb9dee

Signed-off-by: Linus Ziegert 
---
 hw/net/cadence_gem.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7f9cb5ab95..b8be73dc55 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -271,9 +271,10 @@
 #define PHY_REG_EXT_PHYSPCFC_ST   27
 #define PHY_REG_CABLE_DIAG   28
 
-#define PHY_REG_CONTROL_RST  0x8000
-#define PHY_REG_CONTROL_LOOP 0x4000
-#define PHY_REG_CONTROL_ANEG 0x1000
+#define PHY_REG_CONTROL_RST   0x8000
+#define PHY_REG_CONTROL_LOOP  0x4000
+#define PHY_REG_CONTROL_ANEG  0x1000
+#define PHY_REG_CONTROL_ANRESTART 0x0200
 
 #define PHY_REG_STATUS_LINK 0x0004
 #define PHY_REG_STATUS_ANEGCMPL 0x0020
@@ -1345,7 +1346,7 @@ static void gem_phy_write(CadenceGEMState *s, unsigned 
reg_num, uint16_t val)
 }
 if (val & PHY_REG_CONTROL_ANEG) {
 /* Complete autonegotiation immediately */
-val &= ~PHY_REG_CONTROL_ANEG;
+val &= ~(PHY_REG_CONTROL_ANEG | PHY_REG_CONTROL_ANRESTART);
 s->phy_regs[PHY_REG_STATUS] |= PHY_REG_STATUS_ANEGCMPL;
 }
 if (val & PHY_REG_CONTROL_LOOP) {
-- 
2.21.0

Re: [PATCH for-4.2 v10 06/15] virtio-iommu: Endpoint and domains structs and helpers

[Jean-Philippe Brucker]

Hi Eric,

On Tue, Jul 30, 2019 at 07:21:28PM +0200, Eric Auger wrote:
>  static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
>  {
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
> +
> +g_tree_destroy(s->domains);
> +g_tree_destroy(s->endpoints);

virtio_iommu_device_reset() must completely clear the internal state as
well (noticed while testing modprobe/rmmod).

Thanks,
Jean

Re: [PATCH] global: Squash 'the the'

[Philippe Mathieu-Daudé]

On 11/4/19 10:40 PM, no-re...@patchew.org wrote:

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

Hi,

This series failed the docker-quick@centos7 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-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

   TESTiotest-qcow2: 009
   TESTiotest-qcow2: 010
**
ERROR:/tmp/qemu-test/src/tests/migration-test.c:903:wait_for_migration_fail: assertion failed: (!strcmp(status, 
"setup") || !strcmp(status, "failed") || (allow_active && !strcmp(status, 
"active")))
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/migration-test.c:903:wait_for_migration_fail: assertion failed: 
(!strcmp(status, "setup") || !strcmp(status, "failed") || (allow_active && 
!strcmp(status, "active")))
make: *** [check-qtest-aarch64] Error 1
make: *** Waiting for unfinished jobs


=)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] global: Squash 'the the'

[no-reply]

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



Hi,

This series failed the docker-quick@centos7 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-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTiotest-qcow2: 009
  TESTiotest-qcow2: 010
**
ERROR:/tmp/qemu-test/src/tests/migration-test.c:903:wait_for_migration_fail: 
assertion failed: (!strcmp(status, "setup") || !strcmp(status, "failed") || 
(allow_active && !strcmp(status, "active")))
ERROR - Bail out! 
ERROR:/tmp/qemu-test/src/tests/migration-test.c:903:wait_for_migration_fail: 
assertion failed: (!strcmp(status, "setup") || !strcmp(status, "failed") || 
(allow_active && !strcmp(status, "active")))
make: *** [check-qtest-aarch64] Error 1
make: *** Waiting for unfinished jobs
  TESTcheck-unit: tests/test-bdrv-drain
  TESTiotest-qcow2: 011
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=acf3f0f780e741e6a5c367dda157d023', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-_ad8j8yg/src/docker-src.2019-11-04-16.29.40.7445:/var/tmp/qemu:z,ro',
 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=acf3f0f780e741e6a5c367dda157d023
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-_ad8j8yg/src'
make: *** [docker-run-test-quick@centos7] Error 2

real10m19.416s
user0m7.957s


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

RE: [PATCH v7 0/11] add failover feature for assigned network devices

[Parav Pandit]

Hi Jens,


> -Original Message-
> From: Jens Freimann 
> Sent: Tuesday, October 29, 2019 6:49 AM
> To: qemu-devel@nongnu.org
> Cc: ehabk...@redhat.com; m...@redhat.com; berra...@redhat.com;
> la...@redhat.com; aa...@redhat.com; ai...@redhat.com; Parav Pandit
> ; dgilb...@redhat.com; alex.william...@redhat.com;
> arm...@redhat.com; ebl...@redhat.com; jasow...@redhat.com;
> quint...@redhat.com; pbonz...@redhat.com
> Subject: [PATCH v7 0/11] add failover feature for assigned network devices
> 
> This is implementing the host side of the net_failover concept
> (https://www.kernel.org/doc/html/latest/networking/net_failover.html)
> 
> Changes since v6:
> * reword patch description of 06/11 (Markus)
> * have qemu_savevm_state_guest_unplug_pending() return true when at
>   least one device is still not unplugged (Dave)
> 
> 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 adds checks to PCIDevice for only allowing ethernet devices as
>   failover primary and only PCIExpress capable devices
> 
> * Patch 3 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 4 sets the pending_deleted_event before triggering the guest
>   unplug request
> 
> * 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 make sure that we can unplug the vfio-device before
>   migration starts
> 
> * Patch 8 adds a new migration state that is entered while we wait for
>   devices to be unplugged by guest OS
> 
> * Patch 9 just adds the new migration state to a check in libqos code
> 
> * Patch 10 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 11 allows migration for failover vfio-pci devices
> 

Patches are great to see failover capability enabled on the netdevice.
However it's very difficult to test it without having libvirt support.
Can we please have the necessary libvirt enhancements?
I will be able to test it with Mellanox ConnectX5 device + qemu + libvirt.

[..]

[Bug 1851095] Re: [feature request] awareness of instructions that are well emulated

[Shawn Landden]

appears the random number generator produces different results on 32-bit
arches, while my code seems to work fine in qemu

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

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

Title:
  [feature request] awareness of instructions that are well emulated

Status in QEMU:
  Invalid

Bug description:
  While qemu's scalar emulation tends to be excellent, qemu's SIMD
  emulation tends to be incorrect (except for arm64 from x86_64)--i have
  found this both for mipsel and arm32. Until these code paths are
  audited, which is probably a large job, it would be nice if qemu knew
  its emulation of this class of instructions was not very good, and
  thus it would give up on finding these instructions if a "careful"
  operation is passed.

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

[PATCH] Semihost SYS_READC implementation (v6)

[Keith Packard]

Provides a blocking call to read a character from the console using
semihosting.chardev, if specified. This takes some careful command
line options to use stdio successfully as the serial ports, monitor
and semihost all want to use stdio. Here's a sample set of command
line options which share stdio betwen semihost, monitor and serial
ports:

qemu \
-chardev stdio,mux=on,id=stdio0 \
-serial chardev:stdio0 \
-semihosting-config enable=on,chardev=stdio0 \
-mon chardev=stdio0,mode=readline

This creates a chardev hooked to stdio and then connects all of the
subsystems to it. A shorter mechanism would be good to hear about.

Signed-off-by: Keith Packard 

---

v2:
Add implementation in linux-user/arm/semihost.c

v3:  (thanks to Paolo Bonzini )
Replace hand-rolled fifo with fifo8
Avoid mixing code and declarations
Remove spurious (void) cast of function parameters
Define qemu_semihosting_console_init when CONFIG_USER_ONLY

v4:
Add qemu_semihosting_console_init to stubs/semihost.c for
hosts that don't support semihosting

v5:
Move #include statements to the top of the file.
Actually include the stubs/semihost.c patch that was
supposed to be in v4
v6:
Move call to qemu_semihosting_console_init earlier in
main so that the mux starts connected to the serial device
---
 hw/semihosting/console.c  | 72 +++
 include/hw/semihosting/console.h  | 12 ++
 include/hw/semihosting/semihost.h |  4 ++
 linux-user/arm/semihost.c | 23 ++
 stubs/semihost.c  |  4 ++
 target/arm/arm-semi.c |  3 +-
 vl.c  |  3 ++
 7 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/hw/semihosting/console.c b/hw/semihosting/console.c
index b4b17c8afb..4db68d6227 100644
--- a/hw/semihosting/console.c
+++ b/hw/semihosting/console.c
@@ -22,6 +22,12 @@
 #include "exec/gdbstub.h"
 #include "qemu/log.h"
 #include "chardev/char.h"
+#include 
+#include "chardev/char-fe.h"
+#include "sysemu/sysemu.h"
+#include "qemu/main-loop.h"
+#include "qapi/error.h"
+#include "qemu/fifo8.h"
 
 int qemu_semihosting_log_out(const char *s, int len)
 {
@@ -98,3 +104,69 @@ void qemu_semihosting_console_outc(CPUArchState *env, 
target_ulong addr)
   __func__, addr);
 }
 }
+
+#define FIFO_SIZE   1024
+
+typedef struct SemihostingConsole {
+CharBackend backend;
+pthread_mutex_t mutex;
+pthread_cond_t  cond;
+boolgot;
+Fifo8   fifo;
+} SemihostingConsole;
+
+static SemihostingConsole console = {
+.mutex = PTHREAD_MUTEX_INITIALIZER,
+.cond = PTHREAD_COND_INITIALIZER
+};
+
+static int console_can_read(void *opaque)
+{
+SemihostingConsole *c = opaque;
+int ret;
+pthread_mutex_lock(>mutex);
+ret = (int) fifo8_num_free(>fifo);
+pthread_mutex_unlock(>mutex);
+return ret;
+}
+
+static void console_read(void *opaque, const uint8_t *buf, int size)
+{
+SemihostingConsole *c = opaque;
+pthread_mutex_lock(>mutex);
+while (size-- && !fifo8_is_full(>fifo)) {
+fifo8_push(>fifo, *buf++);
+}
+pthread_cond_broadcast(>cond);
+pthread_mutex_unlock(>mutex);
+}
+
+target_ulong qemu_semihosting_console_inc(CPUArchState *env)
+{
+uint8_t ch;
+SemihostingConsole *c = 
+qemu_mutex_unlock_iothread();
+pthread_mutex_lock(>mutex);
+while (fifo8_is_empty(>fifo)) {
+pthread_cond_wait(>cond, >mutex);
+}
+ch = fifo8_pop(>fifo);
+pthread_mutex_unlock(>mutex);
+qemu_mutex_lock_iothread();
+return (target_ulong) ch;
+}
+
+void qemu_semihosting_console_init(void)
+{
+Chardev *chr = semihosting_get_chardev();
+
+if  (chr) {
+fifo8_create(, FIFO_SIZE);
+qemu_chr_fe_init(, chr, _abort);
+qemu_chr_fe_set_handlers(,
+ console_can_read,
+ console_read,
+ NULL, NULL, ,
+ NULL, true);
+}
+}
diff --git a/include/hw/semihosting/console.h b/include/hw/semihosting/console.h
index 9be9754bcd..f7d5905b41 100644
--- a/include/hw/semihosting/console.h
+++ b/include/hw/semihosting/console.h
@@ -37,6 +37,18 @@ int qemu_semihosting_console_outs(CPUArchState *env, 
target_ulong s);
  */
 void qemu_semihosting_console_outc(CPUArchState *env, target_ulong c);
 
+/**
+ * qemu_semihosting_console_inc:
+ * @env: CPUArchState
+ *
+ * Receive single character from debug console. This
+ * may be the remote gdb session if a softmmu guest is currently being
+ * debugged.
+ *
+ * Returns: character read or -1 on error
+ */
+target_ulong qemu_semihosting_console_inc(CPUArchState *env);
+
 /**
  * qemu_semihosting_log_out:
  * @s: pointer to string
diff --git a/include/hw/semihosting/semihost.h 
b/include/hw/semihosting/semihost.h
index 

[Bug 1851095] Re: [feature request] awareness of instructions that are well emulated

[Shawn Landden]

Here is the same thing compiled with optimizations on

** Attachment added: "bench_simple"
   
https://bugs.launchpad.net/qemu/+bug/1851095/+attachment/5302825/+files/bench_simple

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

Title:
  [feature request] awareness of instructions that are well emulated

Status in QEMU:
  New

Bug description:
  While qemu's scalar emulation tends to be excellent, qemu's SIMD
  emulation tends to be incorrect (except for arm64 from x86_64)--i have
  found this both for mipsel and arm32. Until these code paths are
  audited, which is probably a large job, it would be nice if qemu knew
  its emulation of this class of instructions was not very good, and
  thus it would give up on finding these instructions if a "careful"
  operation is passed.

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

Re: [PATCH] global: Squash 'the the'

[Laurent Vivier]

Le 04/11/2019 à 19:52, Dr. David Alan Gilbert (git) a écrit :
> From: "Dr. David Alan Gilbert" 
> 
> 'the' has a tendency to double up; squash them back down.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  disas/libvixl/vixl/invalset.h   | 2 +-
>  docs/interop/pr-helper.rst  | 2 +-
>  docs/specs/ppc-spapr-hotplug.txt| 2 +-
>  docs/specs/ppc-xive.rst | 2 +-
>  docs/specs/tpm.txt  | 2 +-
>  include/hw/xen/interface/io/blkif.h | 2 +-
>  scripts/dump-guest-memory.py| 2 +-
>  7 files changed, 7 insertions(+), 7 deletions(-)



Reviewed-by: Laurent Vivier 

Re: [RFC v2 15/22] intel_iommu: bind/unbind guest page table to host

[Peter Xu]

On Thu, Oct 24, 2019 at 08:34:36AM -0400, Liu Yi L wrote:
> This patch captures the guest PASID table entry modifications and
> propagates the changes to host to setup nested translation. The
> guest page table is configured as 1st level page table (GVA->GPA)
> whose translation result would further go through host VT-d 2nd
> level page table(GPA->HPA) under nested translation mode. This is
> a key part of vSVA support.
> 
> Cc: Kevin Tian 
> Cc: Jacob Pan 
> Cc: Peter Xu 
> Cc: Yi Sun 
> Signed-off-by: Liu Yi L 
> ---
>  hw/i386/intel_iommu.c  | 81 
> ++
>  hw/i386/intel_iommu_internal.h | 20 +++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index d8827c9..793b0de 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -41,6 +41,7 @@
>  #include "migration/vmstate.h"
>  #include "trace.h"
>  #include "qemu/jhash.h"
> +#include 
>  
>  /* context entry operations */
>  #define VTD_CE_GET_RID2PASID(ce) \
> @@ -695,6 +696,16 @@ static inline uint16_t 
> vtd_pe_get_domain_id(VTDPASIDEntry *pe)
>  return VTD_SM_PASID_ENTRY_DID((pe)->val[1]);
>  }
>  
> +static inline uint32_t vtd_pe_get_fl_aw(VTDPASIDEntry *pe)
> +{
> +return 48 + ((pe->val[2] >> 2) & VTD_SM_PASID_ENTRY_FLPM) * 9;
> +}
> +
> +static inline dma_addr_t vtd_pe_get_flpt_base(VTDPASIDEntry *pe)
> +{
> +return pe->val[2] & VTD_SM_PASID_ENTRY_FLPTPTR;
> +}
> +
>  static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>  {
>  return pdire->val & 1;
> @@ -1850,6 +1861,67 @@ static void 
> vtd_context_global_invalidate(IntelIOMMUState *s)
>  vtd_iommu_replay_all(s);
>  }
>  
> +static void vtd_bind_guest_pasid(IntelIOMMUState *s, VTDBus *vtd_bus,
> +int devfn, int pasid, VTDPASIDEntry *pe, VTDPASIDOp op)
> +{
> +#ifdef __linux__
> +VTDIOMMUContext *vtd_ic;
> +IOMMUCTXEventData event_data;
> +IOMMUCTXPASIDBindData bind;
> +struct iommu_gpasid_bind_data *g_bind_data;
> +
> +vtd_ic = vtd_bus->dev_ic[devfn];
> +if (!vtd_ic) {
> +return;
> +}
> +
> +g_bind_data = g_malloc0(sizeof(*g_bind_data));
> +bind.flag = 0;
> +g_bind_data->flags = 0;
> +g_bind_data->vtd.flags = 0;
> +switch (op) {
> +case VTD_PASID_BIND:
> +case VTD_PASID_UPDATE:
> +g_bind_data->version = IOMMU_GPASID_BIND_VERSION_1;
> +g_bind_data->format = IOMMU_PASID_FORMAT_INTEL_VTD;
> +g_bind_data->gpgd = vtd_pe_get_flpt_base(pe);
> +g_bind_data->addr_width = vtd_pe_get_fl_aw(pe);
> +g_bind_data->hpasid = pasid;
> +g_bind_data->gpasid = pasid;
> +g_bind_data->flags |= IOMMU_SVA_GPASID_VAL;
> +g_bind_data->vtd.flags =
> + (VTD_SM_PASID_ENTRY_SRE_BIT(pe->val[2]) ? 1 : 0)
> +   | (VTD_SM_PASID_ENTRY_EAFE_BIT(pe->val[2]) ? 1 : 
> 0)
> +   | (VTD_SM_PASID_ENTRY_PCD_BIT(pe->val[1]) ? 1 : 0)
> +   | (VTD_SM_PASID_ENTRY_PWT_BIT(pe->val[1]) ? 1 : 0)
> +   | (VTD_SM_PASID_ENTRY_EMTE_BIT(pe->val[1]) ? 1 : 
> 0)
> +   | (VTD_SM_PASID_ENTRY_CD_BIT(pe->val[1]) ? 1 : 0);
> +g_bind_data->vtd.pat = VTD_SM_PASID_ENTRY_PAT(pe->val[1]);
> +g_bind_data->vtd.emt = VTD_SM_PASID_ENTRY_EMT(pe->val[1]);
> +bind.flag |= IOMMU_CTX_BIND_PASID;
> +break;
> +
> +case VTD_PASID_UNBIND:
> +g_bind_data->gpgd = 0;
> +g_bind_data->addr_width = 0;
> +g_bind_data->hpasid = pasid;
> +bind.flag |= IOMMU_CTX_UNBIND_PASID;
> +break;
> +
> +default:
> +printf("Unknown VTDPASIDOp!!\n");

Please don't use printf()..  Here assert() suits.

> +break;
> +}
> +if (bind.flag) {

Will this be untrue?  If not, assert() works too.

> +event_data.event = IOMMU_CTX_EVENT_PASID_BIND;
> +bind.data = g_bind_data;
> +event_data.data = 
> +iommu_ctx_event_notify(_ic->iommu_context, _data);
> +}
> +g_free(g_bind_data);
> +#endif
> +}
> +
>  /* Do a context-cache device-selective invalidation.
>   * @func_mask: FM field after shifting
>   */
> @@ -2528,12 +2600,17 @@ static gboolean vtd_flush_pasid(gpointer key, 
> gpointer value,
>  pc_entry->pasid_cache_gen = s->pasid_cache_gen;
>  if (!vtd_pasid_entry_compare(, _entry->pasid_entry)) {
>  pc_entry->pasid_entry = pe;
> +vtd_bind_guest_pasid(s, vtd_bus, devfn,
> + pasid, , VTD_PASID_UPDATE);
>  /*
>   * TODO: when pasid-base-iotlb(piotlb) infrastructure is
>   * ready, should invalidate QEMU piotlb togehter with 
> this
>   * change.
>   */
>  }
> +} else {
> +

[Bug 1851095] Re: [feature request] awareness of instructions that are well emulated

[Shawn Landden]

example binary doing double-precision exponent on 16 megs

expected output:

checksum: f181b401cd42aa7b

actual output:

checksum: 4004022b0ba624fb


** Attachment added: "bench_simple"
   
https://bugs.launchpad.net/qemu/+bug/1851095/+attachment/5302817/+files/bench_simple

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

Title:
  [feature request] awareness of instructions that are well emulated

Status in QEMU:
  New

Bug description:
  While qemu's scalar emulation tends to be excellent, qemu's SIMD
  emulation tends to be incorrect (except for arm64 from x86_64)--i have
  found this both for mipsel and arm32. Until these code paths are
  audited, which is probably a large job, it would be nice if qemu knew
  its emulation of this class of instructions was not very good, and
  thus it would give up on finding these instructions if a "careful"
  operation is passed.

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

Re: [RFC v2 13/22] intel_iommu: add PASID cache management infrastructure

[Peter Xu]

On Thu, Oct 24, 2019 at 08:34:34AM -0400, Liu Yi L wrote:
> This patch adds a PASID cache management infrastructure based on
> new added structure VTDPASIDAddressSpace, which is used to track
> the PASID usage and future PASID tagged DMA address translation
> support in vIOMMU.
> 
> struct VTDPASIDAddressSpace {
> VTDBus *vtd_bus;
> uint8_t devfn;
> AddressSpace as;
> uint32_t pasid;
> IntelIOMMUState *iommu_state;
> VTDContextCacheEntry context_cache_entry;
> QLIST_ENTRY(VTDPASIDAddressSpace) next;
> VTDPASIDCacheEntry pasid_cache_entry;
> };
> 
> Ideally, a VTDPASIDAddressSpace instance is created when a PASID
> is bound with a DMA AddressSpace. Intel VT-d spec requires guest
> software to issue pasid cache invalidation when bind or unbind a
> pasid with an address space under caching-mode. However, as
> VTDPASIDAddressSpace instances also act as pasid cache in this
> implementation, its creation also happens during vIOMMU PASID
> tagged DMA translation. The creation in this path will not be
> added in this patch since no PASID-capable emulated devices for
> now.
> 
> The implementation in this patch manages VTDPASIDAddressSpace
> instances per PASID+BDF (lookup and insert will use PASID and
> BDF) since Intel VT-d spec allows per-BDF PASID Table. When a
> guest bind a PASID with an AddressSpace, QEMU will capture the
> guest pasid selective pasid cache invalidation, and allocate
> remove a VTDPASIDAddressSpace instance per the invalidation
> reasons:
> 
> *) a present pasid entry moved to non-present
> *) a present pasid entry to be a present entry
> *) a non-present pasid entry moved to present
> 
> vIOMMU emulator could figure out the reason by fetching latest
> guest pasid entry.
> 
> Cc: Kevin Tian 
> Cc: Jacob Pan 
> Cc: Peter Xu 
> Cc: Yi Sun 
> Signed-off-by: Liu Yi L 

Ok feel free to ignore my previous reply... I didn't notice it's
actually the pasid entry cache layer rather than the whole pasid
layer (including piotlb).  Comments below.

> ---
>  hw/i386/intel_iommu.c  | 356 
> +
>  hw/i386/intel_iommu_internal.h |  10 ++
>  hw/i386/trace-events   |   1 +
>  include/hw/i386/intel_iommu.h  |  36 -
>  4 files changed, 402 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 90b8f6c..d8827c9 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -40,6 +40,7 @@
>  #include "kvm_i386.h"
>  #include "migration/vmstate.h"
>  #include "trace.h"
> +#include "qemu/jhash.h"
>  
>  /* context entry operations */
>  #define VTD_CE_GET_RID2PASID(ce) \
> @@ -65,6 +66,8 @@
>  static void vtd_address_space_refresh_all(IntelIOMMUState *s);
>  static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n);
>  
> +static void vtd_pasid_cache_reset(IntelIOMMUState *s);
> +
>  static void vtd_panic_require_caching_mode(void)
>  {
>  error_report("We need to set caching-mode=on for intel-iommu to enable "
> @@ -276,6 +279,7 @@ static void vtd_reset_caches(IntelIOMMUState *s)
>  vtd_iommu_lock(s);
>  vtd_reset_iotlb_locked(s);
>  vtd_reset_context_cache_locked(s);
> +vtd_pasid_cache_reset(s);
>  vtd_iommu_unlock(s);
>  }
>  
> @@ -686,6 +690,11 @@ static inline bool vtd_pe_type_check(X86IOMMUState 
> *x86_iommu,
>  return true;
>  }
>  
> +static inline uint16_t vtd_pe_get_domain_id(VTDPASIDEntry *pe)
> +{
> +return VTD_SM_PASID_ENTRY_DID((pe)->val[1]);
> +}
> +
>  static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>  {
>  return pdire->val & 1;
> @@ -2389,19 +2398,361 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState 
> *s, VTDInvDesc *inv_desc)
>  return true;
>  }
>  
> +static inline struct pasid_key *vtd_get_pasid_key(uint32_t pasid,
> +  uint16_t sid)
> +{
> +struct pasid_key *key = g_malloc0(sizeof(*key));

I think you can simply return the pasid_key directly maybe otherwise
should be careful on mem leak.  Actually I think it's leaked below...

> +key->pasid = pasid;
> +key->sid = sid;
> +return key;
> +}
> +
> +static guint vtd_pasid_as_key_hash(gconstpointer v)
> +{
> +struct pasid_key *key = (struct pasid_key *)v;
> +uint32_t a, b, c;
> +
> +/* Jenkins hash */
> +a = b = c = JHASH_INITVAL + sizeof(*key);
> +a += key->sid;
> +b += extract32(key->pasid, 0, 16);
> +c += extract32(key->pasid, 16, 16);
> +
> +__jhash_mix(a, b, c);
> +__jhash_final(a, b, c);

I'm totally not good at hash, but I'm curious why no one wants to
introduce at least a jhash() so we don't need to call these internals
(I believe that's how kernel did it).  At the meantime I don't see how
it would be better than things like g_str_hash() too so I'd be glad if
anyone could help explain a bit...

> +
> +return c;
> +}
> +
> +static gboolean 

Re: [PATCH] global: Squash 'the the'

[Alex Bennée]

Dr. David Alan Gilbert (git)  writes:

> From: "Dr. David Alan Gilbert" 
>
> 'the' has a tendency to double up; squash them back down.
>
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Alex Bennée 

> ---
>  disas/libvixl/vixl/invalset.h   | 2 +-
>  docs/interop/pr-helper.rst  | 2 +-
>  docs/specs/ppc-spapr-hotplug.txt| 2 +-
>  docs/specs/ppc-xive.rst | 2 +-
>  docs/specs/tpm.txt  | 2 +-
>  include/hw/xen/interface/io/blkif.h | 2 +-
>  scripts/dump-guest-memory.py| 2 +-
>  7 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/disas/libvixl/vixl/invalset.h b/disas/libvixl/vixl/invalset.h
> index ffdc0237b4..ef5e49d6fe 100644
> --- a/disas/libvixl/vixl/invalset.h
> +++ b/disas/libvixl/vixl/invalset.h
> @@ -102,7 +102,7 @@ template class InvalSet {
>size_t size() const;
>
>// Returns true if no elements are stored in the set.
> -  // Note that this does not mean the the backing storage is empty: it can 
> still
> +  // Note that this does not mean the backing storage is empty: it can still
>// contain invalid elements.
>bool empty() const;
>
> diff --git a/docs/interop/pr-helper.rst b/docs/interop/pr-helper.rst
> index 9f76d5bcf9..e926f0a6c9 100644
> --- a/docs/interop/pr-helper.rst
> +++ b/docs/interop/pr-helper.rst
> @@ -10,7 +10,7 @@ can delegate implementation of persistent reservations to 
> an external
>  restricting access to block devices to specific initiators in a shared
>  storage setup.
>
> -For a more detailed reference please refer the the SCSI Primary
> +For a more detailed reference please refer to the SCSI Primary
>  Commands standard, specifically the section on Reservations and the
>  "PERSISTENT RESERVE IN" and "PERSISTENT RESERVE OUT" commands.
>
> diff --git a/docs/specs/ppc-spapr-hotplug.txt 
> b/docs/specs/ppc-spapr-hotplug.txt
> index cc7833108e..859d52cce6 100644
> --- a/docs/specs/ppc-spapr-hotplug.txt
> +++ b/docs/specs/ppc-spapr-hotplug.txt
> @@ -385,7 +385,7 @@ Each LMB list entry consists of the following elements:
>is used to retrieve the right associativity list to be used for this
>LMB.
>  - A 32bit flags word. The bit at bit position 0x0008 defines whether
> -  the LMB is assigned to the the partition as of boot time.
> +  the LMB is assigned to the partition as of boot time.
>
>  ibm,dynamic-memory-v2
>
> diff --git a/docs/specs/ppc-xive.rst b/docs/specs/ppc-xive.rst
> index 148d57eb6a..83d43f658b 100644
> --- a/docs/specs/ppc-xive.rst
> +++ b/docs/specs/ppc-xive.rst
> @@ -163,7 +163,7 @@ Interrupt Priority Register (PIPR) is also updated using 
> the IPB. This
>  register represent the priority of the most favored pending
>  notification.
>
> -The PIPR is then compared to the the Current Processor Priority
> +The PIPR is then compared to the Current Processor Priority
>  Register (CPPR). If it is more favored (numerically less than), the
>  CPU interrupt line is raised and the EO bit of the Notification Source
>  Register (NSR) is updated to notify the presence of an exception for
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 5d8c26b1ad..9c8cca042d 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -89,7 +89,7 @@ TPM upon reboot. The PPI specification defines the 
> operation requests and the
>  actions the firmware has to take. The system administrator passes the 
> operation
>  request number to the firmware through an ACPI interface which writes this
>  number to a memory location that the firmware knows. Upon reboot, the 
> firmware
> -finds the number and sends commands to the the TPM. The firmware writes the 
> TPM
> +finds the number and sends commands to the TPM. The firmware writes the TPM
>  result code and the operation request number to a memory location that ACPI 
> can
>  read from and pass the result on to the administrator.
>
> diff --git a/include/hw/xen/interface/io/blkif.h 
> b/include/hw/xen/interface/io/blkif.h
> index 8b1be50ce8..d07fa1e078 100644
> --- a/include/hw/xen/interface/io/blkif.h
> +++ b/include/hw/xen/interface/io/blkif.h
> @@ -341,7 +341,7 @@
>   *  access (even when it should be read-only). If the frontend hits the
>   *  maximum number of allowed persistently mapped grants, it can fallback
>   *  to non persistent mode. This will cause a performance degradation,
> - *  since the the backend driver will still try to map those grants
> + *  since the backend driver will still try to map those grants
>   *  persistently. Since the persistent grants protocol is compatible with
>   *  the previous protocol, a frontend driver can choose to work in
>   *  persistent mode even when the backend doesn't support it.
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 2c587cbefc..9371e45813 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -170,7 +170,7 @@ class ELF(object):
>  self.ehdr.e_phnum += 1
>
>  

Re: [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test

[no-reply]

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



Hi,

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

Subject: [PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test
Type: series
Message-id: 20191104151323.9883-1-cr...@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
Switched to a new branch 'test'
8675afa Acceptance test: add "boot_linux" tests
409affc Acceptance tests: depend on qemu-img
58c0f47 Acceptance tests: add the build directory to the system PATH
236ecfa Acceptance tests: keep a stable reference to the QEMU build dir
6a147b4 Acceptance tests: use relative location for tests
bcd92a3 Acceptance tests: use avocado tags for machine type
0f52135 Acceptance tests: introduce utility method for tags unique vals
050f1d1 Acceptance test x86_cpu_model_versions: use default vm

=== OUTPUT BEGIN ===
1/8 Checking commit 050f1d1ea660 (Acceptance test x86_cpu_model_versions: use 
default vm)
ERROR: line over 90 characters
#47: FILE: tests/acceptance/x86_cpu_model_versions.py:248:
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#61: FILE: tests/acceptance/x86_cpu_model_versions.py:256:
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#76: FILE: tests/acceptance/x86_cpu_model_versions.py:265:
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')

ERROR: line over 90 characters
#90: FILE: tests/acceptance/x86_cpu_model_versions.py:273:
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')

ERROR: line over 90 characters
#105: FILE: tests/acceptance/x86_cpu_model_versions.py:282:
+self.vm.add_args('-cpu', 
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#120: FILE: tests/acceptance/x86_cpu_model_versions.py:290:
+self.vm.add_args('-cpu', 
'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#123: FILE: tests/acceptance/x86_cpu_model_versions.py:293:
+'pc-i440fx-4.0 + Cascadelake-Server-v2 should have 
arch-capabilities')

ERROR: line over 90 characters
#136: FILE: tests/acceptance/x86_cpu_model_versions.py:299:
+self.vm.add_args('-cpu', 
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off,+arch-capabilities')

ERROR: line over 90 characters
#139: FILE: tests/acceptance/x86_cpu_model_versions.py:302:
+'pc-i440fx-4.0 + 
Cascadelake-Server-v1,+arch-capabilities should have arch-capabilities')

ERROR: line over 90 characters
#150: FILE: tests/acceptance/x86_cpu_model_versions.py:307:
+self.vm.add_args('-cpu', 
'Cascadelake-Server-v2,x-force-features=on,check=off,enforce=off,-arch-capabilities')

total: 10 errors, 0 warnings, 134 lines checked

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

2/8 Checking commit 0f5213583c78 (Acceptance tests: introduce utility method 
for tags unique vals)
3/8 Checking commit bcd92a334692 (Acceptance tests: use avocado tags for 
machine type)
WARNING: line over 80 characters
#59: FILE: tests/acceptance/avocado_qemu/__init__.py:119:
+   
default=self._get_unique_tag_val('machine'))

total: 0 errors, 1 warnings, 450 lines checked

Patch 3/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/8 Checking commit 6a147b4b2eb1 (Acceptance tests: use relative location for 
tests)
5/8 Checking commit 236ecfa68ee0 (Acceptance tests: keep a stable reference to 
the QEMU build dir)
ERROR: line over 90 characters
#48: FILE: tests/acceptance/avocado_qemu/__init__.py:19:
+SRC_ROOT_DIR = 
os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__

total: 1 errors, 0 warnings, 8 lines checked

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

6/8 Checking commit 58c0f475d5ec (Acceptance tests: add the build directory to 
the system PATH)
7/8 Checking commit 409affc034ce (Acceptance tests: depend on qemu-img)
8/8 Checking commit 8675afa0dead (Acceptance test: add "boot_linux" tests)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

WARNING: line over 80 characters
#122: FILE: 

Re: [Qemu-devel] [Fail] tests/test-util-filemonitor fails

[Miguel Arruga Vivas]

Hi Daniel,

I've been trying to open a bug in launchpad about exactly this, but it
always raises an error when trying to log in.  Then I found this
thread diving into the archives, so I'll try to kindly ask here about
it.

Daniel P. Berrangé  wrote:
> On Fri, Aug 09, 2019 at 08:06:09AM +0800, Wei Yang wrote:
> > On Thu, Aug 08, 2019 at 10:22:13AM +0100, Daniel P. Berrangé
> > wrote:  
> > >On Thu, Aug 08, 2019 at 04:46:53PM +0800, Wei Yang wrote:  
> > >> On Thu, Aug 08, 2019 at 09:02:29AM +0100, Daniel P. Berrangé
> > >> wrote:  
> > >> >On Thu, Aug 08, 2019 at 10:07:23AM +0800, Wei Yang wrote:  
> > >> >> Current qemu fails tests/test-util-filemonitor.  
> > >> >
> > >> >You'll need to provide more info. The test works for me and
> > >> >passes in all the QEMU CI environments.
> > >> >  
> > >> 
> > >> The error message from my side is:
> > >> 
> > >> /util/filemonitor: Expected watch id 2 but got 1
> > >> **
> > >> ERROR:tests/test-util-filemonitor.c:665:test_file_monitor_events:
> > >> assertion failed: (err == 0)
> > >> 
> > >> What else you'd prefer to have?  
> > >
> > >Can you set the  "FILEMONITOR_DEBUG=1" env variable before running
> > >the test - it will print out lots more info
> > >  
> > 
> > Here is the output with more info.
> > 
> > $ FILEMONITOR_DEBUG=1
> > QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> > tests/test-util-filemonitor  
> 
> > Rmdir /tmp/test-util-filemonitor-151B6Z/fish
> > Event id=2 event=4 file=
> > Expected watch id 2 but got 1
> > **  
> 
> Ok, so the kernel is sending the events in an unexpected order

I've been reading about the issue and as far as I understand the inotify
man-page[http://man7.org/linux/man-pages/man7/inotify.7.html], section
"Dealing with rename() events", points out that the order nor the
atomicity of the concurrent events is something that should be relied
on.
 
> > ERROR:tests/test-util-filemonitor.c:665:test_file_monitor_events:
> > assertion failed: (err == 0) Aborted (core dumped)
> > 
> >   
> > >Also what operating system are you using, and what kernel version

We have hit this error on GNU Guix master
[http://issues.guix.gnu.org/issue/37860]. I'm using linux-libre 5.3.4
on x86_64.  It does not seem to be something deterministic, but I just
commented out the test the fifth time I've hit the same error.

Is there any way to change the test to not rely on the ordering of the
events from different views of the same fs action?

Best regards,
Miguel

Re: [PATCH v1 3/6] tests/vm: use console_consume for netbsd

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> Hi Alex,
>
> On 11/4/19 6:36 PM, Alex Bennée wrote:
>> From: Gerd Hoffmann 
>> Use new helper to read all pending console output,
>> not just a single char.  Unblocks installer boot.
>
> Again, why not use this by default for everything?

I thought that has already got merged via other updates. Will double
check.

>
> Anyway,
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
>
>> Signed-off-by: Gerd Hoffmann 
>> Message-Id: <20191031085306.2-4-kra...@redhat.com>
>> Signed-off-by: Alex Bennée 
>> ---
>>   tests/vm/netbsd | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/tests/vm/netbsd b/tests/vm/netbsd
>> index 5e04dcd9b16..d1bfd01 100755
>> --- a/tests/vm/netbsd
>> +++ b/tests/vm/netbsd
>> @@ -93,7 +93,7 @@ class NetBSDVM(basevm.BaseVM):
>>   for char in list("5consdev com0\n"):
>>   time.sleep(0.2)
>>   self.console_send(char)
>> -self.console_wait("")
>> +self.console_consume()
>>   self.console_wait_send("> ", "boot\n")
>> self.console_wait_send("Terminal type",
>> "xterm\n")
>>


--
Alex Bennée

Re: [PATCH v1 5/6] tests: only run ipmi-bt-test if CONFIG_LINUX

[Alex Bennée]

Corey Minyard  writes:

> On Mon, Nov 04, 2019 at 05:36:53PM +, Alex Bennée wrote:
>> This test has been unstable on NetBSD for awhile. It seems the
>> mechanism used to listen to a random port is a Linux-ism (although a
>> received wisdom Linux-ism rather than a well documented one). As
>
> Hmm.  I got reports of this issue and tried to reproduce, but I was
> never able to.  I thought I had fixed it via other means, but I had no
> idea this was the cause of the issue.
>
> It's not a Linux-ism, I don't think, Richard Steven's "Unix Network
> Programming" mentions that is how this works (at least on UDP), and that
> is pre-Linux.  That's probably where I got this technique.  I've saw
> some web pages mention that Solaris and Windows do it this way.
>
> However, it's not specified, so it's probably a bad idea.  The only way
> I can think to do it another way is to remove the bind() call, then it
> should randomly assign the port (per the spec, I think).  The trouble
> with that is the address will be INADDR_ANY, so it will be bound on
> other interfaces besides the loopback, which make me slightly worried
> from a security point of view.

The other option is to keep the bind but seed it ourselves with a random
port. However then you need to loop until you succeed and avoid locking
up if something fatal has gone wrong.

> I'm ok with this being linux-only, but I'd like to fix it so it works
> everywhere.

One option could be to use the glib networking stuff to set this up.
That has options for creating dynamic ports for incoming connections
although the docs are a bit light on details.
g_socket_listener_add_any_inet_port certainly seems to do the retry
approach (for a weirdly arbitrary 37 times). It leaves the fancy stuff
to a hopefully well tested library although I note QEMU's own use of
g_io APIs seems to be pretty sporadic.


>
> -corey
>
>> working around would add more hard to test complexity to the test I've
>> gone for the easier option of making it CONFIG_LINUX only.
>>
>> Signed-off-by: Alex Bennée 
>> Cc: Corey Minyard 
>> Cc: Kamil Rytarowski 
>> ---
>>  tests/Makefile.include | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 534ee487436..8566f5f119d 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -177,7 +177,9 @@ check-qtest-i386-$(CONFIG_SGA) += 
>> tests/boot-serial-test$(EXESUF)
>>  check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF)
>>  check-qtest-i386-y += tests/rtc-test$(EXESUF)
>>  check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF)
>> +ifdef CONFIG_LINUX
>>  check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF)
>> +endif
>>  check-qtest-i386-y += tests/i440fx-test$(EXESUF)
>>  check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
>>  check-qtest-i386-y += tests/device-plug-test$(EXESUF)
>> --
>> 2.20.1
>>


--
Alex Bennée

Re: [PATCH v1 1/6] tests/vm: netbsd autoinstall, using serial console

[Philippe Mathieu-Daudé]

On 11/4/19 6:36 PM, Alex Bennée wrote:

From: Gerd Hoffmann 

Instead of fetching the prebuilt image from patchew download the install
iso and prepare the image locally.  Install to disk, using the serial
console.  Create qemu user, configure ssh login.  Install packages
needed for qemu builds.


It would be nice to be able to mount some host directory as target 
ccachedir and use ccache within the VM.



Signed-off-by: Gerd Hoffmann 
Reviewed-by: Kamil Rytarowski 
Tested-by: Thomas Huth 
[ehabkost: rebased to latest qemu.git master]
Signed-off-by: Eduardo Habkost 
Message-Id: <20191031085306.2-2-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
  tests/vm/netbsd | 189 +---
  1 file changed, 179 insertions(+), 10 deletions(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 18aa56ae826..5e04dcd9b16 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -2,10 +2,11 @@
  #
  # NetBSD VM image
  #
-# Copyright 2017 Red Hat Inc.
+# Copyright 2017-2019 Red Hat Inc.
  #
  # Authors:
  #  Fam Zheng 
+#  Gerd Hoffmann 
  #
  # This code is licensed under the GPL version 2 or later.  See
  # the COPYING file in the top-level directory.
@@ -13,20 +14,53 @@
  
  import os

  import sys
+import time
  import subprocess
  import basevm
  
  class NetBSDVM(basevm.BaseVM):

  name = "netbsd"
  arch = "x86_64"
+
+link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.0/images/NetBSD-8.0-amd64.iso;
+size = "20G"
+pkgs = [
+# tools
+"git-base",
+"pkgconf",
+"xz",
+"python37",
+
+# gnu tools
+"bash",
+"gmake",
+"gsed",
+"flex", "bison",
+
+# libs: crypto
+"gnutls",
+
+# libs: images
+"jpeg",
+"png",
+
+   # libs: ui
+"SDL2",
+"gtk3+",
+"libxkbcommon",
+]

[...]

[PATCH] global: Squash 'the the'

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

'the' has a tendency to double up; squash them back down.

Signed-off-by: Dr. David Alan Gilbert 
---
 disas/libvixl/vixl/invalset.h   | 2 +-
 docs/interop/pr-helper.rst  | 2 +-
 docs/specs/ppc-spapr-hotplug.txt| 2 +-
 docs/specs/ppc-xive.rst | 2 +-
 docs/specs/tpm.txt  | 2 +-
 include/hw/xen/interface/io/blkif.h | 2 +-
 scripts/dump-guest-memory.py| 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/disas/libvixl/vixl/invalset.h b/disas/libvixl/vixl/invalset.h
index ffdc0237b4..ef5e49d6fe 100644
--- a/disas/libvixl/vixl/invalset.h
+++ b/disas/libvixl/vixl/invalset.h
@@ -102,7 +102,7 @@ template class InvalSet {
   size_t size() const;
 
   // Returns true if no elements are stored in the set.
-  // Note that this does not mean the the backing storage is empty: it can 
still
+  // Note that this does not mean the backing storage is empty: it can still
   // contain invalid elements.
   bool empty() const;
 
diff --git a/docs/interop/pr-helper.rst b/docs/interop/pr-helper.rst
index 9f76d5bcf9..e926f0a6c9 100644
--- a/docs/interop/pr-helper.rst
+++ b/docs/interop/pr-helper.rst
@@ -10,7 +10,7 @@ can delegate implementation of persistent reservations to an 
external
 restricting access to block devices to specific initiators in a shared
 storage setup.
 
-For a more detailed reference please refer the the SCSI Primary
+For a more detailed reference please refer to the SCSI Primary
 Commands standard, specifically the section on Reservations and the
 "PERSISTENT RESERVE IN" and "PERSISTENT RESERVE OUT" commands.
 
diff --git a/docs/specs/ppc-spapr-hotplug.txt b/docs/specs/ppc-spapr-hotplug.txt
index cc7833108e..859d52cce6 100644
--- a/docs/specs/ppc-spapr-hotplug.txt
+++ b/docs/specs/ppc-spapr-hotplug.txt
@@ -385,7 +385,7 @@ Each LMB list entry consists of the following elements:
   is used to retrieve the right associativity list to be used for this
   LMB.
 - A 32bit flags word. The bit at bit position 0x0008 defines whether
-  the LMB is assigned to the the partition as of boot time.
+  the LMB is assigned to the partition as of boot time.
 
 ibm,dynamic-memory-v2
 
diff --git a/docs/specs/ppc-xive.rst b/docs/specs/ppc-xive.rst
index 148d57eb6a..83d43f658b 100644
--- a/docs/specs/ppc-xive.rst
+++ b/docs/specs/ppc-xive.rst
@@ -163,7 +163,7 @@ Interrupt Priority Register (PIPR) is also updated using 
the IPB. This
 register represent the priority of the most favored pending
 notification.
 
-The PIPR is then compared to the the Current Processor Priority
+The PIPR is then compared to the Current Processor Priority
 Register (CPPR). If it is more favored (numerically less than), the
 CPU interrupt line is raised and the EO bit of the Notification Source
 Register (NSR) is updated to notify the presence of an exception for
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 5d8c26b1ad..9c8cca042d 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -89,7 +89,7 @@ TPM upon reboot. The PPI specification defines the operation 
requests and the
 actions the firmware has to take. The system administrator passes the operation
 request number to the firmware through an ACPI interface which writes this
 number to a memory location that the firmware knows. Upon reboot, the firmware
-finds the number and sends commands to the the TPM. The firmware writes the TPM
+finds the number and sends commands to the TPM. The firmware writes the TPM
 result code and the operation request number to a memory location that ACPI can
 read from and pass the result on to the administrator.
 
diff --git a/include/hw/xen/interface/io/blkif.h 
b/include/hw/xen/interface/io/blkif.h
index 8b1be50ce8..d07fa1e078 100644
--- a/include/hw/xen/interface/io/blkif.h
+++ b/include/hw/xen/interface/io/blkif.h
@@ -341,7 +341,7 @@
  *  access (even when it should be read-only). If the frontend hits the
  *  maximum number of allowed persistently mapped grants, it can fallback
  *  to non persistent mode. This will cause a performance degradation,
- *  since the the backend driver will still try to map those grants
+ *  since the backend driver will still try to map those grants
  *  persistently. Since the persistent grants protocol is compatible with
  *  the previous protocol, a frontend driver can choose to work in
  *  persistent mode even when the backend doesn't support it.
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 2c587cbefc..9371e45813 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -170,7 +170,7 @@ class ELF(object):
 self.ehdr.e_phnum += 1
 
 def to_file(self, elf_file):
-"""Writes all ELF structures to the the passed file.
+"""Writes all ELF structures to the passed file.
 
 Structure:
 Ehdr
-- 
2.23.0

Re: [PATCH v1 6/6] tests/vm: support sites with sha512 checksums

[Philippe Mathieu-Daudé]

On 11/4/19 6:36 PM, Alex Bennée wrote:

The NetBSD project uses SHA512 for its checksums so lets support that
in the download helper.

Signed-off-by: Alex Bennée 
---
  tests/vm/basevm.py | 10 --
  tests/vm/netbsd|  3 ++-
  2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 086bfb2c66d..91a9226026d 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -95,19 +95,25 @@ class BaseVM(object):
  logging.info("KVM not available, not using -enable-kvm")
  self._data_args = []
  
-def _download_with_cache(self, url, sha256sum=None):

+def _download_with_cache(self, url, sha256sum=None, sha512sum=None):
  def check_sha256sum(fname):
  if not sha256sum:
  return True
  checksum = subprocess.check_output(["sha256sum", 
fname]).split()[0]
  return sha256sum == checksum.decode("utf-8")
  
+def check_sha512sum(fname):

+if not sha512sum:
+return True
+checksum = subprocess.check_output(["sha512sum", fname]).split()[0]
+return sha512sum == checksum.decode("utf-8")
+
  cache_dir = os.path.expanduser("~/.cache/qemu-vm/download")
  if not os.path.exists(cache_dir):
  os.makedirs(cache_dir)
  fname = os.path.join(cache_dir,
   hashlib.sha1(url.encode("utf-8")).hexdigest())
-if os.path.exists(fname) and check_sha256sum(fname):
+if os.path.exists(fname) and check_sha256sum(fname) and 
check_sha512sum(fname):
  return fname
  logging.debug("Downloading %s to %s...", url, fname)
  subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"],
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 33779402dd1..89390e99fdd 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -23,6 +23,7 @@ class NetBSDVM(basevm.BaseVM):
  arch = "x86_64"
  
  link = "https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.1/images/NetBSD-8.1-amd64.iso;

+csum = 
"718f275b7e0879599bdac95630c5e3f2184700032fdb6cdebf3bdd63687898c48ff3f08f57b89f4437a86cdd8ea07c01a39d432dbb37e1e4b008f4985f98da3f"
  size = "20G"
  pkgs = [
  # tools
@@ -70,7 +71,7 @@ class NetBSDVM(basevm.BaseVM):
  ipv6 = False
  
  def build_image(self, img):

-cimg = self._download_with_cache(self.link)
+cimg = self._download_with_cache(self.link, sha512sum=self.csum)
  img_tmp = img + ".tmp"
  iso = img + ".install.iso"
  



Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: Questions about the VFIO BAR region

[Alex Williamson]

On Tue, 5 Nov 2019 00:40:39 +0800
Li Qiang  wrote:

> Hello Alex, Auger and all,
> 
> I have a question about the VFIO virtual device BAR.
> 
> In vfio_region_setup, it initialize a ‘region->mem’ MR and set its ops to 
> ‘vfio_regions_ops’. 
> In ‘vfio_region_mmap’, it maps the physical device’s MMIO to QEMU’s virtual 
> address space 
> as a raw MR ‘region->mmaps[i].mem’. 
> And also it set the latter MR as a subregion of the first one.
> 
> So when the guest accesses the BAR, it will direct go to the physical 
> device’s BAR.
> My question is here:
> When the qemu will use the ‘vfio_regions_ops’ to read/write the BAR?
> Also whey in the last of ‘vfio_region_write/read’ we need to call 
> ‘vbasedev->ops->vfio_eoi(vbasedev);’?

We support:

 a) sparse mmaps where the entire BAR is not covered by an mmap
 b) quirks, which layer on top of the mmaps to provide virtualized
access
 c) INTx emulation which disables mmaps MRs in order to detect device
access as a generic mechanism for inferring interrupt
acknowledgment.

The latter being the reason we call vfio_eoi.  Thanks,

Alex

Re: [PATCH v1 3/6] tests/vm: use console_consume for netbsd

[Philippe Mathieu-Daudé]

Hi Alex,

On 11/4/19 6:36 PM, Alex Bennée wrote:

From: Gerd Hoffmann 

Use new helper to read all pending console output,
not just a single char.  Unblocks installer boot.


Again, why not use this by default for everything?

Anyway,
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


Signed-off-by: Gerd Hoffmann 
Message-Id: <20191031085306.2-4-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
  tests/vm/netbsd | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 5e04dcd9b16..d1bfd01 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -93,7 +93,7 @@ class NetBSDVM(basevm.BaseVM):
  for char in list("5consdev com0\n"):
  time.sleep(0.2)
  self.console_send(char)
-self.console_wait("")
+self.console_consume()
  self.console_wait_send("> ", "boot\n")
  
  self.console_wait_send("Terminal type","xterm\n")

Re: [PATCH v1 4/6] tests/vm: update netbsd to version 8.1

[Philippe Mathieu-Daudé]

On 11/4/19 6:36 PM, Alex Bennée wrote:

From: Gerd Hoffmann 

Signed-off-by: Gerd Hoffmann 


Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


Message-Id: <20191031085306.2-5-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
  tests/vm/netbsd | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index d1bfd01..33779402dd1 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -22,7 +22,7 @@ class NetBSDVM(basevm.BaseVM):
  name = "netbsd"
  arch = "x86_64"
  
-link = "https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.0/images/NetBSD-8.0-amd64.iso;

+link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.1/images/NetBSD-8.1-amd64.iso;
  size = "20G"
  pkgs = [
  # tools

Re: [PATCH v1 2/6] tests/vm: add console_consume helper

[Philippe Mathieu-Daudé]

On 11/4/19 6:36 PM, Alex Bennée wrote:

From: Gerd Hoffmann 

Helper function to read all console output.

Signed-off-by: Gerd Hoffmann 


Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


Message-Id: <20191031085306.2-3-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
  tests/vm/basevm.py | 19 +++
  1 file changed, 19 insertions(+)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 2929de23aa7..086bfb2c66d 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -242,6 +242,25 @@ class BaseVM(object):
  return False
  return True
  
+def console_consume(self):

+vm = self._guest
+output = ""
+vm.console_socket.setblocking(0)
+while True:
+try:
+chars = vm.console_socket.recv(1)
+except:
+break
+output += chars.decode("latin1")
+if "\r" in output or "\n" in output:
+lines = re.split("[\r\n]", output)
+output = lines.pop()
+if self.debug:
+self.console_log("\n".join(lines))
+if self.debug:
+self.console_log(output)
+vm.console_socket.setblocking(1)
+
  def console_send(self, command):
  vm = self._guest
  if self.debug:

Re: [PATCH v1 1/6] tests/vm: netbsd autoinstall, using serial console

[Philippe Mathieu-Daudé]

On 11/4/19 6:36 PM, Alex Bennée wrote:

From: Gerd Hoffmann 

Instead of fetching the prebuilt image from patchew download the install
iso and prepare the image locally.  Install to disk, using the serial
console.  Create qemu user, configure ssh login.  Install packages
needed for qemu builds.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Kamil Rytarowski 
Tested-by: Thomas Huth 
[ehabkost: rebased to latest qemu.git master]
Signed-off-by: Eduardo Habkost 
Message-Id: <20191031085306.2-2-kra...@redhat.com>


Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


Signed-off-by: Alex Bennée 
---
  tests/vm/netbsd | 189 +---
  1 file changed, 179 insertions(+), 10 deletions(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 18aa56ae826..5e04dcd9b16 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -2,10 +2,11 @@
  #
  # NetBSD VM image
  #
-# Copyright 2017 Red Hat Inc.
+# Copyright 2017-2019 Red Hat Inc.
  #
  # Authors:
  #  Fam Zheng 
+#  Gerd Hoffmann 
  #
  # This code is licensed under the GPL version 2 or later.  See
  # the COPYING file in the top-level directory.
@@ -13,20 +14,53 @@
  
  import os

  import sys
+import time
  import subprocess
  import basevm
  
  class NetBSDVM(basevm.BaseVM):

  name = "netbsd"
  arch = "x86_64"
+
+link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.0/images/NetBSD-8.0-amd64.iso;
+size = "20G"
+pkgs = [
+# tools
+"git-base",
+"pkgconf",
+"xz",
+"python37",
+
+# gnu tools
+"bash",
+"gmake",
+"gsed",
+"flex", "bison",
+
+# libs: crypto
+"gnutls",
+
+# libs: images
+"jpeg",
+"png",
+
+   # libs: ui
+"SDL2",
+"gtk3+",
+"libxkbcommon",
+]
+
  BUILD_SCRIPT = """
  set -e;
-rm -rf /var/tmp/qemu-test.*
-cd $(mktemp -d /var/tmp/qemu-test.XX);
+rm -rf /home/qemu/qemu-test.*
+cd $(mktemp -d /home/qemu/qemu-test.XX);
+mkdir src build; cd src;
  tar -xf /dev/rld1a;
-./configure --python=python2.7 {configure_opts};
+cd ../build
+../src/configure --python=python3.7 --disable-opengl {configure_opts};
  gmake --output-sync -j{jobs} {target} {verbose};
  """
+poweroff = "/sbin/poweroff"
  
  # Workaround for NetBSD + IPv6 + slirp issues.

  # NetBSD seems to ignore the ICMPv6 Destination Unreachable
@@ -36,14 +70,149 @@ class NetBSDVM(basevm.BaseVM):
  ipv6 = False
  
  def build_image(self, img):

-cimg = 
self._download_with_cache("http://download.patchew.org/netbsd-7.1-amd64.img.xz;,
- 
sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
-img_tmp_xz = img + ".tmp.xz"
+cimg = self._download_with_cache(self.link)
  img_tmp = img + ".tmp"
-sys.stderr.write("Extracting the image...\n")
-subprocess.check_call(["ln", "-f", cimg, img_tmp_xz])
-subprocess.check_call(["xz", "--keep", "-dvf", img_tmp_xz])
+iso = img + ".install.iso"
+
+self.print_step("Preparing iso and disk image")
+subprocess.check_call(["ln", "-f", cimg, iso])
+subprocess.check_call(["qemu-img", "create", "-f", "qcow2",
+   img_tmp, self.size])
+
+self.print_step("Booting installer")
+self.boot(img_tmp, extra_args = [
+"-bios", "pc-bios/bios-256k.bin",
+"-machine", "graphics=off",
+"-cdrom", iso
+])
+self.console_init()
+self.console_wait("Primary Bootstrap")
+
+# serial console boot menu output doesn't work for some
+# reason, so we have to fly blind ...
+for char in list("5consdev com0\n"):
+time.sleep(0.2)
+self.console_send(char)
+self.console_wait("")
+self.console_wait_send("> ", "boot\n")
+
+self.console_wait_send("Terminal type","xterm\n")
+self.console_wait_send("a: Installation messages", "a\n")
+self.console_wait_send("b: US-English","b\n")
+self.console_wait_send("a: Install NetBSD","a\n")
+self.console_wait("Shall we continue?")
+self.console_wait_send("b: Yes",   "b\n")
+
+self.console_wait_send("a: ld0",   "a\n")
+self.console_wait_send("a: This is the correct",   "a\n")
+self.console_wait_send("b: Use the entire disk",   "b\n")
+self.console_wait("NetBSD bootcode")
+self.console_wait_send("a: Yes",   "a\n")
+self.console_wait_send("b: Use existing part", "b\n")
+self.console_wait_send("x: Partition sizes ok","x\n")
+self.console_wait_send("for your NetBSD disk", "\n")
+self.console_wait("Shall we continue?")
+ 

Re: [PATCH v26 00/21] Add RX archtecture support

[Philippe Mathieu-Daudé]

Hi Yoshinori,

On 10/14/19 1:57 PM, Yoshinori Sato wrote:

Hello.
This patch series is added Renesas RX target emulation.


Your series is (very) ready to get merged.
IIUC it won't be merged for the next release (4.2) but will be
merged first thing once 5.0 opens.
You don't need to do anything (except eventually send a ping
in 3 weeks).
Sorry it took so long, but introducing a new target is not an
easy task.

Regards,

Phil.

Re: [PATCH v1 5/6] tests: only run ipmi-bt-test if CONFIG_LINUX

[Corey Minyard]

On Mon, Nov 04, 2019 at 05:36:53PM +, Alex Bennée wrote:
> This test has been unstable on NetBSD for awhile. It seems the
> mechanism used to listen to a random port is a Linux-ism (although a
> received wisdom Linux-ism rather than a well documented one). As

Hmm.  I got reports of this issue and tried to reproduce, but I was
never able to.  I thought I had fixed it via other means, but I had no
idea this was the cause of the issue.

It's not a Linux-ism, I don't think, Richard Steven's "Unix Network
Programming" mentions that is how this works (at least on UDP), and that
is pre-Linux.  That's probably where I got this technique.  I've saw
some web pages mention that Solaris and Windows do it this way.

However, it's not specified, so it's probably a bad idea.  The only way
I can think to do it another way is to remove the bind() call, then it
should randomly assign the port (per the spec, I think).  The trouble
with that is the address will be INADDR_ANY, so it will be bound on
other interfaces besides the loopback, which make me slightly worried
from a security point of view.

I'm ok with this being linux-only, but I'd like to fix it so it works
everywhere.

-corey

> working around would add more hard to test complexity to the test I've
> gone for the easier option of making it CONFIG_LINUX only.
> 
> Signed-off-by: Alex Bennée 
> Cc: Corey Minyard 
> Cc: Kamil Rytarowski 
> ---
>  tests/Makefile.include | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 534ee487436..8566f5f119d 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -177,7 +177,9 @@ check-qtest-i386-$(CONFIG_SGA) += 
> tests/boot-serial-test$(EXESUF)
>  check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF)
>  check-qtest-i386-y += tests/rtc-test$(EXESUF)
>  check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF)
> +ifdef CONFIG_LINUX
>  check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF)
> +endif
>  check-qtest-i386-y += tests/i440fx-test$(EXESUF)
>  check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
>  check-qtest-i386-y += tests/device-plug-test$(EXESUF)
> -- 
> 2.20.1
> 

Re: The problems about COLO

[Lukas Straub]

On Thu, 31 Oct 2019 17:05:20 +0800
Daniel Cho  wrote:

> Hello all,
> I have some questions about the COLO.
> 1)  Could we dynamic set fault tolerance feature on running VM?
> In your document, the primary VM could not  start first (if you start
> primary VM, the secondary VM will need to start), it means to if I
> want this VM with fault-tolerance feature, it needs to be set while
> we boot it.

Hi Daniel,
Yes, this is possible as long you have a quorum block node. The rest
can be added while running.

> 2)  If primary VM or secondary VM broke, could we start the third VM
> to keep fault tolerance feature?

I'm currently working on this, see my latest PATCH series here:
https://lore.kernel.org/qemu-devel/cover.1571925699.git.lukasstra...@web.de/

>
> Best regard,
> Daniel Cho.

Re: [PATCH v7 4/8] Acceptance tests: use relative location for tests

[Philippe Mathieu-Daudé]

On 11/4/19 4:13 PM, Cleber Rosa wrote:

An Avocado Test ID[1] is composed by a number of components, but it
starts with the Test Name, usually a file system location that was
given to the loader.

Because the source directory is being given as a prefix to the
"tests/acceptance" directory containing the acceptance tests, the test
names will needlessly include the directory the user is using to host
the QEMU sources (and/or build tree).

Let's remove the source dir (or a build dir) from the path given to
the test loader.  This should give more constant names, and when using
result servers and databases, it should give the same test names
across executions from different people or from different directories.


Can we strip the full path to directory and only keep the filename in 
the database? (Thinking about out-of-tree tests).




[1] - 
https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id

Signed-off-by: Cleber Rosa 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/Makefile.include | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 56f73b46e2..65e85f5275 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1180,7 +1180,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
  --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) 
\
  --filter-by-tags-include-empty --filter-by-tags-include-empty-key 
\
  $(AVOCADO_TAGS) \
---failfast=on $(SRC_PATH)/tests/acceptance, \
+--failfast=on tests/acceptance, \
  "AVOCADO", "tests/acceptance")
  
  # Consolidated targets

Re: [PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm

[Philippe Mathieu-Daudé]

On 11/4/19 4:13 PM, Cleber Rosa wrote:

The default vm provided by the test, available as self.vm, serves the
same purpose of the one obtained by self.get_vm(), but saves a line
and matches the style of other tests.

Signed-off-by: Cleber Rosa 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/acceptance/x86_cpu_model_versions.py | 100 ++---
  1 file changed, 46 insertions(+), 54 deletions(-)

diff --git a/tests/acceptance/x86_cpu_model_versions.py 
b/tests/acceptance/x86_cpu_model_versions.py
index 5fc9ca4bc6..6eb977954d 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -25,10 +25,6 @@
  import avocado_qemu
  import re
  
-def get_cpu_prop(vm, prop):

-cpu_path = vm.command('query-cpus')[0].get('qom_path')
-return vm.command('qom-get', path=cpu_path, property=prop)
-
  class X86CPUModelAliases(avocado_qemu.Test):
  """
  Validation of PC CPU model versions and CPU model aliases
@@ -241,78 +237,74 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
  
  :avocado: tags=arch:x86_64

  """
+def get_cpu_prop(self, prop):
+cpu_path = self.vm.command('query-cpus')[0].get('qom_path')
+return self.vm.command('qom-get', path=cpu_path, property=prop)
+
  def test_4_1(self):
  # machine-type only:
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.1')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.1')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
   'pc-i440fx-4.1 + Cascadelake-Server should not have 
arch-capabilities')
  
  def test_4_0(self):

-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.0')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
   'pc-i440fx-4.0 + Cascadelake-Server should not have 
arch-capabilities')
  
  def test_set_4_0(self):

  # command line must override machine-type if CPU model is not 
versioned:
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
-vm.launch()
-self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.0')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+self.vm.launch()
+self.assertTrue(self.get_cpu_prop('arch-capabilities'),
  'pc-i440fx-4.0 + 
Cascadelake-Server,+arch-capabilities should have arch-capabilities')
  
  def test_unset_4_1(self):

-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.1')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.1')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
   'pc-i440fx-4.1 + 
Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
  
  def test_v1_4_0(self):

  # versioned CPU model overrides machine-type:
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.0')
+self.vm.add_args('-cpu', 
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
   'pc-i440fx-4.0 + Cascadelake-Server-v1 should not 
have arch-capabilities')
  
  def test_v2_4_0(self):

-  

Questions about the VFIO BAR region

[Li Qiang]

Hello Alex, Auger and all,

I have a question about the VFIO virtual device BAR.

In vfio_region_setup, it initialize a ‘region->mem’ MR and set its ops to 
‘vfio_regions_ops’. 
In ‘vfio_region_mmap’, it maps the physical device’s MMIO to QEMU’s virtual 
address space 
as a raw MR ‘region->mmaps[i].mem’. 
And also it set the latter MR as a subregion of the first one.

So when the guest accesses the BAR, it will direct go to the physical device’s 
BAR.
My question is here:
When the qemu will use the ‘vfio_regions_ops’ to read/write the BAR?
Also whey in the last of ‘vfio_region_write/read’ we need to call 
‘vbasedev->ops->vfio_eoi(vbasedev);’?


Thanks,
Li Qiang

Re: [PATCH v2 0/2] block/nvme: add support for write zeros and discard

[Maxim Levitsky]

On Tue, 2019-10-29 at 09:33 -0400, John Snow wrote:
> 
> On 10/28/19 6:35 AM, Max Reitz wrote:
> > On 13.09.19 15:36, Maxim Levitsky wrote:
> > > This is the second part of the patches I prepared
> > > for this driver back when I worked on mdev-nvme.
> > > 
> > > V2: addressed review feedback, no major changes
> > > 
> > > Best regards,
> > >   Maxim Levitsky
> > > 
> > > Maxim Levitsky (2):
> > >   block/nvme: add support for write zeros
> > >   block/nvme: add support for discard
> > > 
> > >  block/nvme.c | 155 ++-
> > >  block/trace-events   |   3 +
> > >  include/block/nvme.h |  19 +-
> > >  3 files changed, 175 insertions(+), 2 deletions(-)
> > 
> > Thanks, fixed the indentation in nvme.h in patch 1, and applied to my
> > block branch:
> > 
> > https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> > 
> > For the record, I don’t think !!x has benefits over x != 0 and I
> > personally prefer bool y = x over any of it. O:-)
> > 
> 
> Well, that's even better :) For me, it's about making booleans obvious
> as booleans and that's all.
> 
> --js
Thanks to all of you!!
Best regards,
Maxim Levitsky

Re: [PATCH v2 2/2] nbd: Allow description when creating NBD blockdev

[Maxim Levitsky]

On Thu, 2019-10-10 at 16:00 -0500, Eric Blake wrote:
> Allow blockdevs to match the feature already present in qemu-nbd -D.
> Enhance iotest 223 to cover it.
> 
> Signed-off-by: Eric Blake 
> ---
>  qapi/block.json| 8 +---
>  blockdev-nbd.c | 9 -
>  monitor/hmp-cmds.c | 4 ++--
>  tests/qemu-iotests/223 | 2 +-
>  tests/qemu-iotests/223.out | 1 +
>  5 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/qapi/block.json b/qapi/block.json
> index 145c268bb646..a6617b5bd03a 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -250,9 +250,11 @@
>  # @name: Export name. If unspecified, the @device parameter is used as the
>  #export name. (Since 2.12)
>  #
> +# @description: Free-form description of the export. (Since 4.2)
> +#
>  # @writable: Whether clients should be able to write to the device via the
>  # NBD connection (default false).
> -
> +#
>  # @bitmap: Also export the dirty bitmap reachable from @device, so the
>  #  NBD client can use NBD_OPT_SET_META_CONTEXT with
>  #  "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
> @@ -263,8 +265,8 @@
>  # Since: 1.3.0
>  ##
>  { 'command': 'nbd-server-add',
> -  'data': {'device': 'str', '*name': 'str', '*writable': 'bool',
> -   '*bitmap': 'str' } }
> +  'data': {'device': 'str', '*name': 'str', '*description': 'str',
> +   '*writable': 'bool', '*bitmap': 'str' } }
> 
>  ##
>  # @NbdServerRemoveMode:
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 8c20baa4a4b9..de2f2ff71320 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -144,6 +144,7 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
>  }
> 
>  void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
> +bool has_description, const char *description,
>  bool has_writable, bool writable,
>  bool has_bitmap, const char *bitmap, Error **errp)
>  {
> @@ -167,6 +168,11 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>  return;
>  }
> 
> +if (has_description && strlen(description) > NBD_MAX_STRING_SIZE) {
> +error_setg(errp, "description '%s' too long", description);
> +return;
> +}
> +
>  if (nbd_export_find(name)) {
>  error_setg(errp, "NBD server already has export named '%s'", name);
>  return;
> @@ -195,7 +201,8 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>  writable = false;
>  }
> 
> -exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, 
> !writable,
> +exp = nbd_export_new(bs, 0, len, name, description, bitmap,
> + !writable, !writable,
>   NULL, false, on_eject_blk, errp);
>  if (!exp) {
>  goto out;
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index b2551c16d129..574c6321c9d0 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -2352,7 +2352,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict 
> *qdict)
>  continue;
>  }
> 
> -qmp_nbd_server_add(info->value->device, false, NULL,
> +qmp_nbd_server_add(info->value->device, false, NULL, false, NULL,
> true, writable, false, NULL, _err);
> 
>  if (local_err != NULL) {
> @@ -2374,7 +2374,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict 
> *qdict)
>  bool writable = qdict_get_try_bool(qdict, "writable", false);
>  Error *local_err = NULL;
> 
> -qmp_nbd_server_add(device, !!name, name, true, writable,
> +qmp_nbd_server_add(device, !!name, name, false, NULL, true, writable,
> false, NULL, _err);
>  hmp_handle_error(mon, _err);
>  }
> diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
> index 2ba3d8124b4f..06bdc96be48f 100755
> --- a/tests/qemu-iotests/223
> +++ b/tests/qemu-iotests/223
> @@ -144,7 +144,7 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
>"bitmap":"b3"}}' "error" # Missing bitmap
>  _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
>"arguments":{"device":"n", "name":"n2", "writable":true,
> -  "bitmap":"b2"}}' "return"
> +  "description":"some text", "bitmap":"b2"}}' "return"
>  $QEMU_NBD_PROG -L -k "$TEST_DIR/nbd"
> 
>  echo
> diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
> index 23b34fcd202e..16d597585b4f 100644
> --- a/tests/qemu-iotests/223.out
> +++ b/tests/qemu-iotests/223.out
> @@ -49,6 +49,7 @@ exports available: 2
> base:allocation
> qemu:dirty-bitmap:b
>   export: 'n2'
> +  description: some text
>size:  4194304
>flags: 0xced ( flush fua trim zeroes df cache fast-zero )
>min block: 1

Reviewed-by: Maxim Levitsky 

Best regards,
Maxim Levitsky

[PATCH v1 6/6] tests/vm: support sites with sha512 checksums

[Alex Bennée]

The NetBSD project uses SHA512 for its checksums so lets support that
in the download helper.

Signed-off-by: Alex Bennée 
---
 tests/vm/basevm.py | 10 --
 tests/vm/netbsd|  3 ++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 086bfb2c66d..91a9226026d 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -95,19 +95,25 @@ class BaseVM(object):
 logging.info("KVM not available, not using -enable-kvm")
 self._data_args = []
 
-def _download_with_cache(self, url, sha256sum=None):
+def _download_with_cache(self, url, sha256sum=None, sha512sum=None):
 def check_sha256sum(fname):
 if not sha256sum:
 return True
 checksum = subprocess.check_output(["sha256sum", fname]).split()[0]
 return sha256sum == checksum.decode("utf-8")
 
+def check_sha512sum(fname):
+if not sha512sum:
+return True
+checksum = subprocess.check_output(["sha512sum", fname]).split()[0]
+return sha512sum == checksum.decode("utf-8")
+
 cache_dir = os.path.expanduser("~/.cache/qemu-vm/download")
 if not os.path.exists(cache_dir):
 os.makedirs(cache_dir)
 fname = os.path.join(cache_dir,
  hashlib.sha1(url.encode("utf-8")).hexdigest())
-if os.path.exists(fname) and check_sha256sum(fname):
+if os.path.exists(fname) and check_sha256sum(fname) and 
check_sha512sum(fname):
 return fname
 logging.debug("Downloading %s to %s...", url, fname)
 subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"],
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 33779402dd1..89390e99fdd 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -23,6 +23,7 @@ class NetBSDVM(basevm.BaseVM):
 arch = "x86_64"
 
 link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.1/images/NetBSD-8.1-amd64.iso;
+csum = 
"718f275b7e0879599bdac95630c5e3f2184700032fdb6cdebf3bdd63687898c48ff3f08f57b89f4437a86cdd8ea07c01a39d432dbb37e1e4b008f4985f98da3f"
 size = "20G"
 pkgs = [
 # tools
@@ -70,7 +71,7 @@ class NetBSDVM(basevm.BaseVM):
 ipv6 = False
 
 def build_image(self, img):
-cimg = self._download_with_cache(self.link)
+cimg = self._download_with_cache(self.link, sha512sum=self.csum)
 img_tmp = img + ".tmp"
 iso = img + ".install.iso"
 
-- 
2.20.1

Re: [PATCH v2 1/2] nbd: Don't send oversize strings

[Maxim Levitsky]

On Tue, 2019-10-15 at 16:16 +, Vladimir Sementsov-Ogievskiy wrote:
> 15.10.2019 18:07, Eric Blake wrote:
> > On 10/11/19 2:32 AM, Vladimir Sementsov-Ogievskiy wrote:
> > > 11.10.2019 0:00, Eric Blake wrote:
> > > > Qemu as server currently won't accept export names larger than 256
> > > > bytes, nor create dirty bitmap names longer than 1023 bytes, so most
> > > > uses of qemu as client or server have no reason to get anywhere near
> > > > the NBD spec maximum of a 4k limit per string.
> > > > 
> > > > However, we weren't actually enforcing things, ignoring when the
> > > > remote side violates the protocol on input, and also having several
> > > > code paths where we send oversize strings on output (for example,
> > > > qemu-nbd --description could easily send more than 4k).  Tighten
> > > > things up as follows:
> > > > 
> > > > client:
> > > > - Perform bounds check on export name and dirty bitmap request prior
> > > > to handing it to server
> > > > - Validate that copied server replies are not too long (ignoring
> > > > NBD_INFO_* replies that are not copied is not too bad)
> > > > server:
> > > > - Perform bounds check on export name and description prior to
> > > > advertising it to client
> > > > - Reject client name or metadata query that is too long
> > > > 
> > > > Signed-off-by: Eric Blake 
> > > > ---
> > > > +++ b/include/block/nbd.h
> > > > @@ -232,6 +232,7 @@ enum {
> > > > * going larger would require an audit of more code to make sure we
> > > > * aren't overflowing some other buffer. */
> > > 
> > > This comment says, that we restrict export name to 256...
> > 
> > Yes, because we still stack-allocate the name in places, but 4k is too 
> > large for stack allocation.  But we're inconsistent on where we use the 
> > smaller 256-limit; the server won't serve an image
> > that large, but doesn't prevent a client from requesting a 4k name export 
> > (even though that export will not be present).
> > 
> > 
> > > > +++ b/blockdev-nbd.c
> > > > @@ -162,6 +162,11 @@ void qmp_nbd_server_add(const char *device, bool 
> > > > has_name, const char *name,
> > > >name = device;
> > > >}
> > > > 
> > > > +if (strlen(name) > NBD_MAX_STRING_SIZE) {
> > > > +error_setg(errp, "export name '%s' too long", name);
> > > > +return;
> > > > +}
> > > 
> > > Hmmm, no, so here we restrict to 4096, but, we will not allow client to 
> > > request more than
> > > 256. Seems, to correctly update server-part, we should drop 
> > > NBD_MAX_NAME_SIZE and do the
> > > audit mentioned in the comment above its definition.
> > 
> > Yeah, I guess it's time to just get rid of NBD_MAX_NAME_SIZE, and move away 
> > from stack allocations.  Should I do that as a followup to this patch, or 
> > spin a v3?
> 
> Hmm. It's OK too.
> 
> With
>   - fixed mem-leak in nbd_process_options
>   - s/x_dirty_bitmap/x-dirty-bitmap in nbd_process_options in error message
>   - following yours new wordings
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> 
> However, this patch introduces possible crash point, asserting on bitmap name 
> below, so it would better
> be fixed before this patch or immediately after it.. Still, it's unlikely to 
> have a bitmap with name
> longer than 4k..
> 
> > 
> > > > +++ b/nbd/client.c
> > > > @@ -289,8 +289,8 @@ static int nbd_receive_list(QIOChannel *ioc, char 
> > > > **name, char **description,
> > > >return -1;
> > > >}
> > > >len -= sizeof(namelen);
> > > > -if (len < namelen) {
> > > > -error_setg(errp, "incorrect option name length");
> > > > +if (len < namelen || namelen > NBD_MAX_STRING_SIZE) {
> > > > +error_setg(errp, "incorrect list name length");
> > > 
> > > New wording made me go above and read the comment, what functions does. 
> > > Comment is good, but without
> > > it, it sounds like name of the list for me...
> > 
> > Maybe:
> > 
> > incorrect name length in server's list response
> 
> Yes, this is better, thanks
> 
> > 
> > > 
> > > >nbd_send_opt_abort(ioc);
> > > >return -1;
> > > >}
> > > > @@ -303,6 +303,11 @@ static int nbd_receive_list(QIOChannel *ioc, char 
> > > > **name, char **description,
> > > >local_name[namelen] = '\0';
> > > >len -= namelen;
> > > >if (len) {
> > > > +if (len > NBD_MAX_STRING_SIZE) {
> > > > +error_setg(errp, "incorrect list description length");
> > 
> > and
> > 
> > incorrect description length in server's list response
> > 
> > 
> > > > @@ -648,6 +657,7 @@ static int nbd_send_meta_query(QIOChannel *ioc, 
> > > > uint32_t opt,
> > > >if (query) {
> > > >query_len = strlen(query);
> > > >data_len += sizeof(query_len) + query_len;
> > > > +assert(query_len <= NBD_MAX_STRING_SIZE);
> > > >} else {
> > > >assert(opt == NBD_OPT_LIST_META_CONTEXT);
> > > >}
> > > 
> > > you may assert export_len as 

[PATCH v1 1/6] tests/vm: netbsd autoinstall, using serial console

[Alex Bennée]

From: Gerd Hoffmann 

Instead of fetching the prebuilt image from patchew download the install
iso and prepare the image locally.  Install to disk, using the serial
console.  Create qemu user, configure ssh login.  Install packages
needed for qemu builds.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Kamil Rytarowski 
Tested-by: Thomas Huth 
[ehabkost: rebased to latest qemu.git master]
Signed-off-by: Eduardo Habkost 
Message-Id: <20191031085306.2-2-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
 tests/vm/netbsd | 189 +---
 1 file changed, 179 insertions(+), 10 deletions(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 18aa56ae826..5e04dcd9b16 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -2,10 +2,11 @@
 #
 # NetBSD VM image
 #
-# Copyright 2017 Red Hat Inc.
+# Copyright 2017-2019 Red Hat Inc.
 #
 # Authors:
 #  Fam Zheng 
+#  Gerd Hoffmann 
 #
 # This code is licensed under the GPL version 2 or later.  See
 # the COPYING file in the top-level directory.
@@ -13,20 +14,53 @@
 
 import os
 import sys
+import time
 import subprocess
 import basevm
 
 class NetBSDVM(basevm.BaseVM):
 name = "netbsd"
 arch = "x86_64"
+
+link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.0/images/NetBSD-8.0-amd64.iso;
+size = "20G"
+pkgs = [
+# tools
+"git-base",
+"pkgconf",
+"xz",
+"python37",
+
+# gnu tools
+"bash",
+"gmake",
+"gsed",
+"flex", "bison",
+
+# libs: crypto
+"gnutls",
+
+# libs: images
+"jpeg",
+"png",
+
+   # libs: ui
+"SDL2",
+"gtk3+",
+"libxkbcommon",
+]
+
 BUILD_SCRIPT = """
 set -e;
-rm -rf /var/tmp/qemu-test.*
-cd $(mktemp -d /var/tmp/qemu-test.XX);
+rm -rf /home/qemu/qemu-test.*
+cd $(mktemp -d /home/qemu/qemu-test.XX);
+mkdir src build; cd src;
 tar -xf /dev/rld1a;
-./configure --python=python2.7 {configure_opts};
+cd ../build
+../src/configure --python=python3.7 --disable-opengl {configure_opts};
 gmake --output-sync -j{jobs} {target} {verbose};
 """
+poweroff = "/sbin/poweroff"
 
 # Workaround for NetBSD + IPv6 + slirp issues.
 # NetBSD seems to ignore the ICMPv6 Destination Unreachable
@@ -36,14 +70,149 @@ class NetBSDVM(basevm.BaseVM):
 ipv6 = False
 
 def build_image(self, img):
-cimg = 
self._download_with_cache("http://download.patchew.org/netbsd-7.1-amd64.img.xz;,
- 
sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
-img_tmp_xz = img + ".tmp.xz"
+cimg = self._download_with_cache(self.link)
 img_tmp = img + ".tmp"
-sys.stderr.write("Extracting the image...\n")
-subprocess.check_call(["ln", "-f", cimg, img_tmp_xz])
-subprocess.check_call(["xz", "--keep", "-dvf", img_tmp_xz])
+iso = img + ".install.iso"
+
+self.print_step("Preparing iso and disk image")
+subprocess.check_call(["ln", "-f", cimg, iso])
+subprocess.check_call(["qemu-img", "create", "-f", "qcow2",
+   img_tmp, self.size])
+
+self.print_step("Booting installer")
+self.boot(img_tmp, extra_args = [
+"-bios", "pc-bios/bios-256k.bin",
+"-machine", "graphics=off",
+"-cdrom", iso
+])
+self.console_init()
+self.console_wait("Primary Bootstrap")
+
+# serial console boot menu output doesn't work for some
+# reason, so we have to fly blind ...
+for char in list("5consdev com0\n"):
+time.sleep(0.2)
+self.console_send(char)
+self.console_wait("")
+self.console_wait_send("> ", "boot\n")
+
+self.console_wait_send("Terminal type","xterm\n")
+self.console_wait_send("a: Installation messages", "a\n")
+self.console_wait_send("b: US-English","b\n")
+self.console_wait_send("a: Install NetBSD","a\n")
+self.console_wait("Shall we continue?")
+self.console_wait_send("b: Yes",   "b\n")
+
+self.console_wait_send("a: ld0",   "a\n")
+self.console_wait_send("a: This is the correct",   "a\n")
+self.console_wait_send("b: Use the entire disk",   "b\n")
+self.console_wait("NetBSD bootcode")
+self.console_wait_send("a: Yes",   "a\n")
+self.console_wait_send("b: Use existing part", "b\n")
+self.console_wait_send("x: Partition sizes ok","x\n")
+self.console_wait_send("for your NetBSD disk", "\n")
+self.console_wait("Shall we continue?")
+self.console_wait_send("b: Yes",   "b\n")
+
+self.console_wait_send("b: Use serial port com0",  "b\n")
+

[PATCH v1 4/6] tests/vm: update netbsd to version 8.1

[Alex Bennée]

From: Gerd Hoffmann 

Signed-off-by: Gerd Hoffmann 
Message-Id: <20191031085306.2-5-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
 tests/vm/netbsd | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index d1bfd01..33779402dd1 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -22,7 +22,7 @@ class NetBSDVM(basevm.BaseVM):
 name = "netbsd"
 arch = "x86_64"
 
-link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.0/images/NetBSD-8.0-amd64.iso;
+link = 
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.1/images/NetBSD-8.1-amd64.iso;
 size = "20G"
 pkgs = [
 # tools
-- 
2.20.1

[PATCH v1 5/6] tests: only run ipmi-bt-test if CONFIG_LINUX

[Alex Bennée]

This test has been unstable on NetBSD for awhile. It seems the
mechanism used to listen to a random port is a Linux-ism (although a
received wisdom Linux-ism rather than a well documented one). As
working around would add more hard to test complexity to the test I've
gone for the easier option of making it CONFIG_LINUX only.

Signed-off-by: Alex Bennée 
Cc: Corey Minyard 
Cc: Kamil Rytarowski 
---
 tests/Makefile.include | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 534ee487436..8566f5f119d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -177,7 +177,9 @@ check-qtest-i386-$(CONFIG_SGA) += 
tests/boot-serial-test$(EXESUF)
 check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF)
+ifdef CONFIG_LINUX
 check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF)
+endif
 check-qtest-i386-y += tests/i440fx-test$(EXESUF)
 check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
 check-qtest-i386-y += tests/device-plug-test$(EXESUF)
-- 
2.20.1

[PATCH v1 0/6] testing/next (netbsd stuff)

[Alex Bennée]

Hi,

As we approach hard-freeze I'm trying to temper what comes in through
the testing/next tree. However it would be nice to get the NetBSD upto
speed with the other NetBSDs. Although the serial install is working
well for me this has had a rocky road so if others could also give it
a good testing that would be great. I've also disabled one of the
regular failing tests for non-Linux targets. There are other tests
that still fail however including the tests/test-aio-multithread which
asserts in the async utils around about 20% of the time:

  assertion "QSLIST_EMPTY(>scheduled_coroutines)" failed: file
"/home/qemu/qemu-test.nS1czd/src/util/async.c", line 279, function
"aio_ctx_finalize"

You can run:

  make vm-build-netbsd TARGET_LIST=x86_64-softmmu BUILD_TARGET=check-unit

on repeat to trigger it. If anyone has cycles to work out why please
have a look.

The following patches need review:
   05 - tests only run ipmi bt test if CONFIG_LINUX
   06 - tests vm support sites with sha512 checksums

Alex Bennée (2):
  tests: only run ipmi-bt-test if CONFIG_LINUX
  tests/vm: support sites with sha512 checksums

Gerd Hoffmann (4):
  tests/vm: netbsd autoinstall, using serial console
  tests/vm: add console_consume helper
  tests/vm: use console_consume for netbsd
  tests/vm: update netbsd to version 8.1

 tests/Makefile.include |   2 +
 tests/vm/basevm.py |  29 ++-
 tests/vm/netbsd| 190 ++---
 3 files changed, 209 insertions(+), 12 deletions(-)

-- 
2.20.1

[PATCH v1 2/6] tests/vm: add console_consume helper

[Alex Bennée]

From: Gerd Hoffmann 

Helper function to read all console output.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20191031085306.2-3-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
 tests/vm/basevm.py | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 2929de23aa7..086bfb2c66d 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -242,6 +242,25 @@ class BaseVM(object):
 return False
 return True
 
+def console_consume(self):
+vm = self._guest
+output = ""
+vm.console_socket.setblocking(0)
+while True:
+try:
+chars = vm.console_socket.recv(1)
+except:
+break
+output += chars.decode("latin1")
+if "\r" in output or "\n" in output:
+lines = re.split("[\r\n]", output)
+output = lines.pop()
+if self.debug:
+self.console_log("\n".join(lines))
+if self.debug:
+self.console_log(output)
+vm.console_socket.setblocking(1)
+
 def console_send(self, command):
 vm = self._guest
 if self.debug:
-- 
2.20.1

[PATCH v1 3/6] tests/vm: use console_consume for netbsd

[Alex Bennée]

From: Gerd Hoffmann 

Use new helper to read all pending console output,
not just a single char.  Unblocks installer boot.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20191031085306.2-4-kra...@redhat.com>
Signed-off-by: Alex Bennée 
---
 tests/vm/netbsd | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 5e04dcd9b16..d1bfd01 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -93,7 +93,7 @@ class NetBSDVM(basevm.BaseVM):
 for char in list("5consdev com0\n"):
 time.sleep(0.2)
 self.console_send(char)
-self.console_wait("")
+self.console_consume()
 self.console_wait_send("> ", "boot\n")
 
 self.console_wait_send("Terminal type","xterm\n")
-- 
2.20.1

[PULL 1/2] qga-win: network-get-interfaces command name field bug fix

[Michael Roth]

From: Bishara AbuHattoum 

Network interface name is fetched as an encoded WCHAR array, (wide
character), then it is decoded using the guest's CP_ACP Windows code
page, which is the default code page as configure in the guest's
Windows, then it is returned as a byte array, (char array).

As stated in the BZ#1733165, when renaming a network interface to a
Chinese name and invoking this command, the returned name field has
the (\ufffd) value for each Chinese character the name had, this
value is an indication that the code page does not have the decoding
information for the given character.

This bug is a result of using the CP_ACP code page for decoding which
is an interchangeable code page, instead CP_UTF8 code page should be
used for decoding the network interface's name.

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

Signed-off-by: Bishara AbuHattoum 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Roth 
---
 qga/commands-win32.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 6b67f16faf..64b1c754b0 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1387,12 +1387,12 @@ static IP_ADAPTER_ADDRESSES 
*guest_get_adapters_addresses(Error **errp)
 static char *guest_wctomb_dup(WCHAR *wstr)
 {
 char *str;
-size_t i;
+size_t str_size;
 
-i = wcslen(wstr) + 1;
-str = g_malloc(i);
-WideCharToMultiByte(CP_ACP, WC_COMPOSITECHECK,
-wstr, -1, str, i, NULL, NULL);
+str_size = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL, NULL);
+/* add 1 to str_size for NULL terminator */
+str = g_malloc(str_size + 1);
+WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, str_size, NULL, NULL);
 return str;
 }
 
-- 
2.17.1

[PULL 0/2] qemu-ga patch queue for 4.2-rc0

[Michael Roth]

The following changes since commit 36609b4fa36f0ac934874371874416f7533a5408:

  Merge remote-tracking branch 'remotes/palmer/tags/palmer-for-master-4.2-sf1' 
into staging (2019-11-02 17:59:03 +)

are available in the Git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2019-11-04-tag

for you to fetch changes up to 28d8dd355be98da6239bd5569721980c833df6a1:

  qga: Add "guest-get-memory-block-info" to blacklist (2019-11-04 08:50:54 
-0600)


qemu-ga patch queue for hard-freeze

* fix handling of Chinese network device names in
  guest-network-get-interfaces
* add missing blacklist entries for guest-get-memory-block-info for
  w32/non-linux builds


Basil Salman (1):
  qga: Add "guest-get-memory-block-info" to blacklist

Bishara AbuHattoum (1):
  qga-win: network-get-interfaces command name field bug fix

 qga/commands-posix.c |  3 ++-
 qga/commands-win32.c | 12 ++--
 2 files changed, 8 insertions(+), 7 deletions(-)

[PULL 2/2] qga: Add "guest-get-memory-block-info" to blacklist

[Michael Roth]

From: Basil Salman 

Memory block commands are only supported for linux with sysfs,
"guest-get-memory-block-info" was not in blacklist for other
cases.

Reported on:
https://bugzilla.redhat.com/show_bug.cgi?id=1751431

Signed-off-by: Basil Salman 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Michael Roth 
---
 qga/commands-posix.c | 3 ++-
 qga/commands-win32.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index dfc05f5b8a..1c1a165dae 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2730,7 +2730,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-suspend-hybrid", "guest-network-get-interfaces",
 "guest-get-vcpus", "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
-"guest-get-memory-block-size", NULL};
+"guest-get-memory-block-size", "guest-get-memory-block-info",
+NULL};
 char **p = (char **)list;
 
 while (*p) {
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 64b1c754b0..55ba5b263a 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1894,7 +1894,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-suspend-hybrid",
 "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
-"guest-get-memory-block-size",
+"guest-get-memory-block-size", "guest-get-memory-block-info",
 NULL};
 char **p = (char **)list_unsupported;
 
-- 
2.17.1

Re: [RFC v2 00/22] intel_iommu: expose Shared Virtual Addressing to VM

[Peter Xu]

On Thu, Oct 24, 2019 at 08:34:21AM -0400, Liu Yi L wrote:
> Shared virtual address (SVA), a.k.a, Shared virtual memory (SVM) on Intel
> platforms allow address space sharing between device DMA and applications.
> SVA can reduce programming complexity and enhance security.
> This series is intended to expose SVA capability to VMs. i.e. shared guest
> application address space with passthru devices. The whole SVA virtualization
> requires QEMU/VFIO/IOMMU changes. This series includes the QEMU changes, for
> VFIO and IOMMU changes, they are in separate series (listed in the "Related
> series").
> 
> The high-level architecture for SVA virtualization is as below:
> 
> .-.  .---.
> |   vIOMMU|  | Guest process CR3, FL only|
> | |  '---'
> ./
> | PASID Entry |--- PASID cache flush -
> '-'   |
> | |   V
> | |CR3 in GPA
> '-'
> Guest
> --| Shadow |--|
>   vv  v
> Host
> .-.  .--.
> |   pIOMMU|  | Bind FL for GVA-GPA  |
> | |  '--'
> ./  |
> | PASID Entry | V (Nested xlate)
> '\.--.
> | |   |SL for GPA-HPA, default domain|
> | |   '--'
> '-'
> Where:
>  - FL = First level/stage one page tables
>  - SL = Second level/stage two page tables

Yi,

Would you mind to always mention what tests you have been done with
the patchset in the cover letter?  It'll be fine to say that you're
running this against FPGAs so no one could really retest it, but still
it would be good to know that as well.  It'll even be better to
mention that which part of the series is totally untested if you are
aware of.

Thanks,

-- 
Peter Xu

Re: [RFC v2 13/22] intel_iommu: add PASID cache management infrastructure

[Peter Xu]

On Thu, Oct 24, 2019 at 08:34:34AM -0400, Liu Yi L wrote:
> This patch adds a PASID cache management infrastructure based on
> new added structure VTDPASIDAddressSpace, which is used to track
> the PASID usage and future PASID tagged DMA address translation
> support in vIOMMU.
> 
> struct VTDPASIDAddressSpace {
> VTDBus *vtd_bus;
> uint8_t devfn;
> AddressSpace as;
> uint32_t pasid;
> IntelIOMMUState *iommu_state;
> VTDContextCacheEntry context_cache_entry;
> QLIST_ENTRY(VTDPASIDAddressSpace) next;
> VTDPASIDCacheEntry pasid_cache_entry;
> };
> 
> Ideally, a VTDPASIDAddressSpace instance is created when a PASID
> is bound with a DMA AddressSpace. Intel VT-d spec requires guest
> software to issue pasid cache invalidation when bind or unbind a
> pasid with an address space under caching-mode. However, as
> VTDPASIDAddressSpace instances also act as pasid cache in this
> implementation, its creation also happens during vIOMMU PASID
> tagged DMA translation. The creation in this path will not be
> added in this patch since no PASID-capable emulated devices for
> now.

So is this patch an incomplete version even for the pasid caching
layer for emulated device?

IMHO it would be considered as acceptable to merge something that is
even not ready from hardware pov but at least from software pov it is
complete (so when the hardware is ready we should logically run the
binary directly on them, bugs can happen but that's another story).
However if for this case:

  - it's not even complete as is (in translation functions it seems
that we don't ever use this cache layer at all),

  - we don't have emulated device supported for pasid yet at all, so
even further to have this code start to make any sense, and,

  - this is a 400 line patch as standalone :)  Which means that we
need to start maintain these 400 LOC starting from the day when it
gets merged, while it's far from even being tested.  Then I don't
see how to maintain...

With above, I would suggest you put this patch into the future
patchset where you would like to have the first emulated device for
pasid and then you can even test this patch with those ones.  What do
you think?

Thanks,

-- 
Peter Xu

Re: [PATCH 1/1] target/arm: Add support for cortex-m7 CPU

[Alex Bennée]

Christophe Lyon  writes:

> This is derived from cortex-m4 description, adding DP support and FPv5
> instructions with the corresponding flags in isar and mvfr2.
>
> Checked that it could successfully execute
> vrinta.f32 s15, s15
> while cortex-m4 emulation rejects it with "illegal instruction".

I couldn't verify the cpu->midr values as most of the sections seem to
be IMPDEF but the rest of the feature bits look OK to me:

Reviewed-by: Alex Bennée 

>
> Signed-off-by: Christophe Lyon 
> ---
>  target/arm/cpu.c | 33 +
>  1 file changed, 33 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 13813fb..ccae849 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1954,6 +1954,37 @@ static void cortex_m4_initfn(Object *obj)
>  cpu->isar.id_isar6 = 0x;
>  }
>
> +static void cortex_m7_initfn(Object *obj)
> +{
> +ARMCPU *cpu = ARM_CPU(obj);
> +
> +set_feature(>env, ARM_FEATURE_V7);
> +set_feature(>env, ARM_FEATURE_M);
> +set_feature(>env, ARM_FEATURE_M_MAIN);
> +set_feature(>env, ARM_FEATURE_THUMB_DSP);
> +set_feature(>env, ARM_FEATURE_VFP4);
> +cpu->midr = 0x411fc272; /* r1p2 */
> +cpu->pmsav7_dregion = 8;
> +cpu->isar.mvfr0 = 0x10110221;
> +cpu->isar.mvfr1 = 0x1211;
> +cpu->isar.mvfr2 = 0x0040;
> +cpu->id_pfr0 = 0x0030;
> +cpu->id_pfr1 = 0x0200;
> +cpu->id_dfr0 = 0x0010;
> +cpu->id_afr0 = 0x;
> +cpu->id_mmfr0 = 0x00100030;
> +cpu->id_mmfr1 = 0x;
> +cpu->id_mmfr2 = 0x0100;
> +cpu->id_mmfr3 = 0x;
> +cpu->isar.id_isar0 = 0x01101110;
> +cpu->isar.id_isar1 = 0x02112000;
> +cpu->isar.id_isar2 = 0x20232231;
> +cpu->isar.id_isar3 = 0x0131;
> +cpu->isar.id_isar4 = 0x01310132;
> +cpu->isar.id_isar5 = 0x;
> +cpu->isar.id_isar6 = 0x;
> +}
> +
>  static void cortex_m33_initfn(Object *obj)
>  {
>  ARMCPU *cpu = ARM_CPU(obj);
> @@ -2538,6 +2569,8 @@ static const ARMCPUInfo arm_cpus[] = {
>   .class_init = arm_v7m_class_init },
>  { .name = "cortex-m4",   .initfn = cortex_m4_initfn,
>   .class_init = arm_v7m_class_init },
> +{ .name = "cortex-m7",   .initfn = cortex_m7_initfn,
> + .class_init = arm_v7m_class_init },
>  { .name = "cortex-m33",  .initfn = cortex_m33_initfn,
>   .class_init = arm_v7m_class_init },
>  { .name = "cortex-r5",   .initfn = cortex_r5_initfn },


--
Alex Bennée

Re: [RFC v2 14/22] vfio/pci: add iommu_context notifier for pasid bind/unbind

[David Gibson]

On Thu, Oct 24, 2019 at 08:34:35AM -0400, Liu Yi L wrote:
> This patch adds notifier for pasid bind/unbind. VFIO registers this
> notifier to listen to the dual-stage translation (a.k.a. nested
> translation) configuration changes and propagate to host. Thus vIOMMU
> is able to set its translation structures to host.
> 
> Cc: Kevin Tian 
> Cc: Jacob Pan 
> Cc: Peter Xu 
> Cc: Eric Auger 
> Cc: Yi Sun 
> Cc: David Gibson 
> Signed-off-by: Liu Yi L 
> ---
>  hw/vfio/pci.c| 39 +++
>  include/hw/iommu/iommu.h | 11 +++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 8721ff6..012b8ed 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2767,6 +2767,41 @@ static void 
> vfio_iommu_pasid_free_notify(IOMMUCTXNotifier *n,
>  pasid_req->free_result = ret;
>  }
>  
> +static void vfio_iommu_pasid_bind_notify(IOMMUCTXNotifier *n,
> + IOMMUCTXEventData *event_data)
> +{
> +#ifdef __linux__

Is hw/vfio/pci.c even built on non-linux hosts?

> +VFIOIOMMUContext *giommu_ctx = container_of(n, VFIOIOMMUContext, n);
> +VFIOContainer *container = giommu_ctx->container;
> +IOMMUCTXPASIDBindData *pasid_bind =
> +  (IOMMUCTXPASIDBindData *) event_data->data;
> +struct vfio_iommu_type1_bind *bind;
> +struct iommu_gpasid_bind_data *bind_data;
> +unsigned long argsz;
> +
> +argsz = sizeof(*bind) + sizeof(*bind_data);
> +bind = g_malloc0(argsz);
> +bind->argsz = argsz;
> +bind->bind_type = VFIO_IOMMU_BIND_GUEST_PASID;
> +bind_data = (struct iommu_gpasid_bind_data *) >data;
> +*bind_data = *pasid_bind->data;
> +
> +if (pasid_bind->flag & IOMMU_CTX_BIND_PASID) {
> +if (ioctl(container->fd, VFIO_IOMMU_BIND, bind) != 0) {
> +error_report("%s: pasid (%llu:%llu) bind failed: %d", __func__,
> + bind_data->gpasid, bind_data->hpasid, -errno);
> +}
> +} else if (pasid_bind->flag & IOMMU_CTX_UNBIND_PASID) {
> +if (ioctl(container->fd, VFIO_IOMMU_UNBIND, bind) != 0) {
> +error_report("%s: pasid (%llu:%llu) unbind failed: %d", __func__,
> + bind_data->gpasid, bind_data->hpasid, -errno);
> +}
> +}
> +
> +g_free(bind);
> +#endif
> +}
> +
>  static void vfio_realize(PCIDevice *pdev, Error **errp)
>  {
>  VFIOPCIDevice *vdev = PCI_VFIO(pdev);
> @@ -3079,6 +3114,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>   iommu_context,
>   vfio_iommu_pasid_free_notify,
>   IOMMU_CTX_EVENT_PASID_FREE);
> +vfio_register_iommu_ctx_notifier(vdev,
> + iommu_context,
> + vfio_iommu_pasid_bind_notify,
> + IOMMU_CTX_EVENT_PASID_BIND);
>  }
>  
>  return;
> diff --git a/include/hw/iommu/iommu.h b/include/hw/iommu/iommu.h
> index 4352afd..4f21aa1 100644
> --- a/include/hw/iommu/iommu.h
> +++ b/include/hw/iommu/iommu.h
> @@ -33,6 +33,7 @@ typedef struct IOMMUContext IOMMUContext;
>  enum IOMMUCTXEvent {
>  IOMMU_CTX_EVENT_PASID_ALLOC,
>  IOMMU_CTX_EVENT_PASID_FREE,
> +IOMMU_CTX_EVENT_PASID_BIND,
>  IOMMU_CTX_EVENT_NUM,
>  };
>  typedef enum IOMMUCTXEvent IOMMUCTXEvent;
> @@ -50,6 +51,16 @@ union IOMMUCTXPASIDReqDesc {
>  };
>  typedef union IOMMUCTXPASIDReqDesc IOMMUCTXPASIDReqDesc;
>  
> +struct IOMMUCTXPASIDBindData {
> +#define IOMMU_CTX_BIND_PASID   (1 << 0)
> +#define IOMMU_CTX_UNBIND_PASID (1 << 1)
> +uint32_t flag;
> +#ifdef __linux__
> +struct iommu_gpasid_bind_data *data;

Embedding a linux specific structure in the notification message seems
dubious to me.

> +#endif
> +};
> +typedef struct IOMMUCTXPASIDBindData IOMMUCTXPASIDBindData;
> +
>  struct IOMMUCTXEventData {
>  IOMMUCTXEvent event;
>  uint64_t length;

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PING] [PATCH 1/1] target/arm: Add support for cortex-m7 CPU

[Christophe Lyon]

ping?

http://patchwork.ozlabs.org/patch/1183934/

On Fri, 25 Oct 2019 at 11:08, Christophe Lyon
 wrote:
>
> This is derived from cortex-m4 description, adding DP support and FPv5
> instructions with the corresponding flags in isar and mvfr2.
>
> Checked that it could successfully execute
> vrinta.f32 s15, s15
> while cortex-m4 emulation rejects it with "illegal instruction".
>
> Signed-off-by: Christophe Lyon 
> ---
>  target/arm/cpu.c | 33 +
>  1 file changed, 33 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 13813fb..ccae849 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1954,6 +1954,37 @@ static void cortex_m4_initfn(Object *obj)
>  cpu->isar.id_isar6 = 0x;
>  }
>
> +static void cortex_m7_initfn(Object *obj)
> +{
> +ARMCPU *cpu = ARM_CPU(obj);
> +
> +set_feature(>env, ARM_FEATURE_V7);
> +set_feature(>env, ARM_FEATURE_M);
> +set_feature(>env, ARM_FEATURE_M_MAIN);
> +set_feature(>env, ARM_FEATURE_THUMB_DSP);
> +set_feature(>env, ARM_FEATURE_VFP4);
> +cpu->midr = 0x411fc272; /* r1p2 */
> +cpu->pmsav7_dregion = 8;
> +cpu->isar.mvfr0 = 0x10110221;
> +cpu->isar.mvfr1 = 0x1211;
> +cpu->isar.mvfr2 = 0x0040;
> +cpu->id_pfr0 = 0x0030;
> +cpu->id_pfr1 = 0x0200;
> +cpu->id_dfr0 = 0x0010;
> +cpu->id_afr0 = 0x;
> +cpu->id_mmfr0 = 0x00100030;
> +cpu->id_mmfr1 = 0x;
> +cpu->id_mmfr2 = 0x0100;
> +cpu->id_mmfr3 = 0x;
> +cpu->isar.id_isar0 = 0x01101110;
> +cpu->isar.id_isar1 = 0x02112000;
> +cpu->isar.id_isar2 = 0x20232231;
> +cpu->isar.id_isar3 = 0x0131;
> +cpu->isar.id_isar4 = 0x01310132;
> +cpu->isar.id_isar5 = 0x;
> +cpu->isar.id_isar6 = 0x;
> +}
> +
>  static void cortex_m33_initfn(Object *obj)
>  {
>  ARMCPU *cpu = ARM_CPU(obj);
> @@ -2538,6 +2569,8 @@ static const ARMCPUInfo arm_cpus[] = {
>   .class_init = arm_v7m_class_init },
>  { .name = "cortex-m4",   .initfn = cortex_m4_initfn,
>   .class_init = arm_v7m_class_init },
> +{ .name = "cortex-m7",   .initfn = cortex_m7_initfn,
> + .class_init = arm_v7m_class_init },
>  { .name = "cortex-m33",  .initfn = cortex_m33_initfn,
>   .class_init = arm_v7m_class_init },
>  { .name = "cortex-r5",   .initfn = cortex_r5_initfn },
> --
> 2.7.4
>

Re: [PATCH v17 4/7] target/ppc: Build rtas error log upon an MCE

[David Gibson]

On Thu, Oct 24, 2019 at 01:13:04PM +0530, Ganesh Goudar wrote:
> From: Aravinda Prasad 
> 
> Upon a machine check exception (MCE) in a guest address space,
> KVM causes a guest exit to enable QEMU to build and pass the
> error to the guest in the PAPR defined rtas error log format.
> 
> This patch builds the rtas error log, copies it to the rtas_addr
> and then invokes the guest registered machine check handler. The
> handler in the guest takes suitable action(s) depending on the type
> and criticality of the error. For example, if an error is
> unrecoverable memory corruption in an application inside the
> guest, then the guest kernel sends a SIGBUS to the application.
> For recoverable errors, the guest performs recovery actions and
> logs the error.
> 
> [Assume SLOF has allocated enough room for rtas error log]

Is that correct with the SLOF image currently included in qemu?

Apart from that detail,

Reviewed-by: David Gibson 

> Signed-off-by: Ganesh Goudar 
> Signed-off-by: Aravinda Prasad 



> ---
>  hw/ppc/spapr_events.c  | 220 -
>  hw/ppc/spapr_rtas.c|  26 +
>  include/hw/ppc/spapr.h |   6 +-
>  target/ppc/kvm.c   |   4 +-
>  4 files changed, 253 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 0ce96b86be..db44e09154 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -214,6 +214,104 @@ struct hp_extended_log {
>  struct rtas_event_log_v6_hp hp;
>  } QEMU_PACKED;
>  
> +struct rtas_event_log_v6_mc {
> +#define RTAS_LOG_V6_SECTION_ID_MC   0x4D43 /* MC */
> +struct rtas_event_log_v6_section_header hdr;
> +uint32_t fru_id;
> +uint32_t proc_id;
> +uint8_t error_type;
> +#define RTAS_LOG_V6_MC_TYPE_UE   0
> +#define RTAS_LOG_V6_MC_TYPE_SLB  1
> +#define RTAS_LOG_V6_MC_TYPE_ERAT 2
> +#define RTAS_LOG_V6_MC_TYPE_TLB  4
> +#define RTAS_LOG_V6_MC_TYPE_D_CACHE  5
> +#define RTAS_LOG_V6_MC_TYPE_I_CACHE  7
> +uint8_t sub_err_type;
> +#define RTAS_LOG_V6_MC_UE_INDETERMINATE  0
> +#define RTAS_LOG_V6_MC_UE_IFETCH 1
> +#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH 2
> +#define RTAS_LOG_V6_MC_UE_LOAD_STORE 3
> +#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE 4
> +#define RTAS_LOG_V6_MC_SLB_PARITY0
> +#define RTAS_LOG_V6_MC_SLB_MULTIHIT  1
> +#define RTAS_LOG_V6_MC_SLB_INDETERMINATE 2
> +#define RTAS_LOG_V6_MC_ERAT_PARITY   1
> +#define RTAS_LOG_V6_MC_ERAT_MULTIHIT 2
> +#define RTAS_LOG_V6_MC_ERAT_INDETERMINATE3
> +#define RTAS_LOG_V6_MC_TLB_PARITY1
> +#define RTAS_LOG_V6_MC_TLB_MULTIHIT  2
> +#define RTAS_LOG_V6_MC_TLB_INDETERMINATE 3
> +uint8_t reserved_1[6];
> +uint64_t effective_address;
> +uint64_t logical_address;
> +} QEMU_PACKED;
> +
> +struct mc_extended_log {
> +struct rtas_event_log_v6 v6hdr;
> +struct rtas_event_log_v6_mc mc;
> +} QEMU_PACKED;
> +
> +struct MC_ierror_table {
> +unsigned long srr1_mask;
> +unsigned long srr1_value;
> +bool nip_valid; /* nip is a valid indicator of faulting address */
> +uint8_t error_type;
> +uint8_t error_subtype;
> +unsigned int initiator;
> +unsigned int severity;
> +};
> +
> +static const struct MC_ierror_table mc_ierror_table[] = {
> +{ 0x081c, 0x0004, true,
> +  RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_IFETCH,
> +  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x081c, 0x0008, true,
> +  RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
> +  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x081c, 0x000c, true,
> +  RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT,
> +  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x081c, 0x0010, true,
> +  RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
> +  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x081c, 0x0014, true,
> +  RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
> +  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
> +{ 0x081c, 0x0018, true,
> +  RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH,
> +  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, } };
> +
> +struct MC_derror_table {
> +unsigned long dsisr_value;
> +bool dar_valid; /* dar is a valid indicator of faulting address */
> +uint8_t error_type;
> +uint8_t error_subtype;
> +unsigned int initiator;
> +unsigned int severity;
> +};
> +
> +static const struct MC_derror_table 

Re: RFC: New device for zero-copy VM memory access

[Dr. David Alan Gilbert]

* ge...@hostfission.com (ge...@hostfission.com) wrote:
> 
> 
> On 2019-11-04 22:55, Dr. David Alan Gilbert wrote:
> > * ge...@hostfission.com (ge...@hostfission.com) wrote:
> > > 
> > > 
> > > On 2019-11-03 21:10, ge...@hostfission.com wrote:
> > > > On 2019-11-01 02:52, Dr. David Alan Gilbert wrote:
> > > > > * ge...@hostfission.com (ge...@hostfission.com) wrote:
> > > > > >
> > > > > >
> > > > > > On 2019-11-01 01:52, Peter Maydell wrote:
> > > > > > > On Thu, 31 Oct 2019 at 14:26,  wrote:
> > > > > > > > As the author of Looking Glass, I also have to consider the
> > > > > > > > maintenance
> > > > > > > > and the complexity of implementing the vhost protocol into the
> > > > > > > > project.
> > > > > > > > At this time a complete Porthole client can be implemented in 
> > > > > > > > 150
> > > > > > > > lines
> > > > > > > > of C without external dependencies, and most of that is 
> > > > > > > > boilerplate
> > > > > > > > socket code. This IMO is a major factor in deciding to avoid
> > > > > > > > vhost-user.
> > > > > > >
> > > > > > > This is essentially a proposal that we should make our project and
> > > > > > > code more complicated so that your project and code can be 
> > > > > > > simpler.
> > > > > > > I hope you can see why this isn't necessarily an argument that 
> > > > > > > will hold
> > > > > > > very much weight for us :-)
> > > > > >
> > > > > > Certainly, I do which is why I am still going to see about using
> > > > > > vhost,
> > > > > > however, a device that uses vhost is likely more complex then
> > > > > > the device
> > > > > > as it stands right now and as such more maintenance would be
> > > > > > involved on
> > > > > > your end also. Or have I missed something in that vhost-user can
> > > > > > be used
> > > > > > directly as a device?
> > > > >
> > > > > The basic vhost-user stuff isn't actually that hard;  if you aren't
> > > > > actually shuffling commands over the queues you should find it pretty
> > > > > simple - so I think your assumption about it being simpler if you
> > > > > avoid
> > > > > it might be wrong.  It might be easier if you use it!
> > > >
> > > > I have been looking into this and I am yet to find some decent
> > > > documentation or a simple device example I can use to understand how to
> > > > create such a device. Do you know of any reading or examples I can
> > > > obtain
> > > > on how to get an initial do nothing device up and running?
> > > >
> > > > -Geoff
> > > 
> > > Scratch that, the design just solidified for me and I am now making
> > > progress, however it seems that vhost-user can't do what we need here:
> > > 
> > > 1) I dont see any way to recieve notification of socket
> > > disconnection, in
> > > our use case the client app needs to be able to be (re)connected
> > > dynamically. It might be possible to get this event by registering
> > > it on
> > > the chardev manually but this seems like it would be a kludge.
> > 
> > My understanding was that someone added support for reconnection of
> > vhost-user;  I'm not sure of the detail - cc'ing in Maxime and
> > Marc-Andre.
> > 
> > > 2) I don't see any method of notifying the vhost-user client of the
> > > removal of a shared memory mapping. Again, these may not be
> > > persistently
> > > mapped in the guest as we have no control over the buffer
> > > allocation, and
> > > as such, we need a method to notify the client that the mapping has
> > > become
> > > invalid.
> > > 
> > > 3) VHOST_USER_SET_MEM_TABLE is a one time request, again this breaks
> > > our
> > > usage as we need to change this dynamically at runtime.
> > 
> > I've seen (3) being sent multiple times (It's messy but it happens); so
> > I think that fixes (2) as well for you.
> 
> Yes, but it's ignored.
> 
> /*
>  * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
>  * we just need send it once in the first time. For later such
>  * request, we just ignore it.
>  */
> if (vhost_user_one_time_request(msg->hdr.request) && dev->vq_index != 0)
> {
>  msg->hdr.flags &= ~VHOST_USER_NEED_REPLY_MASK;
>  return 0;
> }

Curious.  I could swear I'd already dealt with multiple copies of this
message coming over vhost-user and having to deal with it when it did.
But now I'm confused, isn't vq_index a unique number per queue, so is
this really stopping it happening multiple times, or just making sure it
only happens for the first queue?

Dave

> > 
> > Dave
> > 
> > > Unless there are viable solutions to these problems there is no way
> > > that
> > > vhost-user can be used for this kind of a device.
> > > 
> > > -Geoff
> > > 
> > > >
> > > > >
> > > > > Dave
> > > > >
> > > > > > >
> > > > > > > thanks
> > > > > > > -- PMM
> > > > > --
> > > > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH] tcg plugins: expose an API version concept

[Philippe Mathieu-Daudé]

On 11/4/19 2:18 PM, Alex Bennée wrote:

This is a very simple versioning API which allows the plugin
infrastructure to check the API a plugin was built against. We also
expose a min/cur API version to the plugin via the info block in case
it wants to avoid using old deprecated APIs in the future.

Signed-off-by: Alex Bennée 
---
  include/qemu/qemu-plugin.h | 19 +++
  plugins/loader.c   | 15 +++
  plugins/plugin.h   |  2 ++
  tests/plugin/bb.c  |  2 ++
  tests/plugin/empty.c   |  2 ++
  tests/plugin/hotpages.c|  2 ++
  tests/plugin/howvec.c  |  2 ++
  tests/plugin/insn.c|  2 ++
  tests/plugin/mem.c |  2 ++
  9 files changed, 48 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index a00a7deb461..5502e112c81 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -38,9 +38,28 @@
  
  typedef uint64_t qemu_plugin_id_t;
  
+/*

+ * Versioning plugins:
+ *
+ * The plugin API will pass a minimum and current API version that
+ * QEMU currently supports. The minimum API will be incremented if an
+ * API needs to be deprecated.
+ *
+ * The plugins export the API they were built against by exposing the
+ * symbol qemu_plugin_version which can be checked.
+ */
+
+extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
+
+#define QEMU_PLUGIN_VERSION 0
+
  typedef struct {
  /* string describing architecture */
  const char *target_name;
+struct {
+int min;
+int cur;
+} version;
  /* is this a full system emulation? */
  bool system_emulation;
  union {
diff --git a/plugins/loader.c b/plugins/loader.c
index ce724ed5839..1bcca909691 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -178,6 +178,19 @@ static int plugin_load(struct qemu_plugin_desc *desc, 
const qemu_info_t *info)
  goto err_symbol;
  }
  
+if (!g_module_symbol(ctx->handle, "qemu_plugin_version", )) {

+warn_report("%s: missing version %s", __func__, g_module_error());
+} else {
+int version = *(int *)sym;
+if (version < QEMU_PLUGIN_MIN_VERSION ||
+version > QEMU_PLUGIN_VERSION) {
+error_report("%s: bad plugin version %d vs %d/%d",
+ __func__, version, QEMU_PLUGIN_MIN_VERSION,
+ QEMU_PLUGIN_VERSION);
+goto err_symbol;
+}
+}
+
  qemu_rec_mutex_lock();
  
  /* find an unused random id with  as the seed */

@@ -248,6 +261,8 @@ int qemu_plugin_load_list(QemuPluginList *head)
  g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
  
  info->target_name = TARGET_NAME;

+info->version.min = QEMU_PLUGIN_MIN_VERSION;
+info->version.cur = QEMU_PLUGIN_VERSION;
  #ifndef CONFIG_USER_ONLY
  MachineState *ms = MACHINE(qdev_get_machine());
  info->system_emulation = true;
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 5482168d797..1aa29dcaddf 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -14,6 +14,8 @@
  
  #include 
  
+#define QEMU_PLUGIN_MIN_VERSION 0

+
  /* global state */
  struct qemu_plugin_state {
  QTAILQ_HEAD(, qemu_plugin_ctx) ctxs;
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index 45e1de5bd68..f30bea08dcc 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -14,6 +14,8 @@
  
  #include 
  
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

+
  static uint64_t bb_count;
  static uint64_t insn_count;
  static bool do_inline;
diff --git a/tests/plugin/empty.c b/tests/plugin/empty.c
index 3f60f690278..8fa6bacd93d 100644
--- a/tests/plugin/empty.c
+++ b/tests/plugin/empty.c
@@ -13,6 +13,8 @@
  
  #include 
  
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

+
  /*
   * Empty TB translation callback.
   * This allows us to measure the overhead of injecting and then
diff --git a/tests/plugin/hotpages.c b/tests/plugin/hotpages.c
index 77df07a3ccf..ecd6c187327 100644
--- a/tests/plugin/hotpages.c
+++ b/tests/plugin/hotpages.c
@@ -18,6 +18,8 @@
  
  #include 
  
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

+
  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
  
  static uint64_t page_size = 4096;

diff --git a/tests/plugin/howvec.c b/tests/plugin/howvec.c
index 58fa675e348..4ca555e1239 100644
--- a/tests/plugin/howvec.c
+++ b/tests/plugin/howvec.c
@@ -20,6 +20,8 @@
  
  #include 
  
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

+
  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
  
  typedef enum {

diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index e5fd07fb64b..0a8f5ae 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -14,6 +14,8 @@
  
  #include 
  
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

+
  static uint64_t insn_count;
  static bool do_inline;
  
diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c

index d9673889896..878abf09d19 100644
--- 

Re: [RFC 0/3] block/file-posix: Work around XFS bug

[Max Reitz]

On 04.11.19 16:49, Alberto Garcia wrote:
> On Mon 04 Nov 2019 04:14:56 PM CET, Max Reitz wrote:
> 
 No, what I meant was that the original problem that led to
 c8bb23cbdbe would go away.
>>>
>>> Ah, right. Not quite, according to my numbers:
>>>
>>> |--++-+-|
>>> | Cluster size | subclusters=on | subclusters=off | fallocate() |
>>> |--++-+-|
>>> |   256 KB | 10182 IOPS |966 IOPS |  14007 IOPS |
>>> |   512 KB |  7919 IOPS |563 IOPS |  13442 IOPS |
>>> |  1024 KB |  5050 IOPS |465 IOPS |  13887 IOPS |
>>> |  2048 KB |  2465 IOPS |271 IOPS |  13885 IOPS |
>>> |--++-+-|
>>>
>>> There's obviously no backing image, and only the last column uses
>>> handle_alloc_space() / fallocate().
>>
>> Thanks for providing some numbers!
>>
>> It was my impression, too, that subclusters wouldn’t solve it.  But it
>> didn’t seem like that big of a difference to me.  Did you run this
>> with aio=native?  (Because that’s where we have the XFS problem)
> 
> Here's with aio=native
> 
> |--++-+-|
> | Cluster size | subclusters=on | subclusters=off | fallocate() |
> |--++-+-|
> |   256 KB | 19897 IOPS |891 IOPS |  32194 IOPS |
> |   512 KB | 17881 IOPS |436 IOPS |  33092 IOPS |
> |  1024 KB | 17050 IOPS |341 IOPS |  32768 IOPS |
> |  2048 KB |  7854 IOPS |207 IOPS |  30944 IOPS |
> |--++-+-|

OK, thanks again. :-)

So it seems right not to revert the commit and just work around the XFS
bug in file-posix.

Max



signature.asc
Description: OpenPGP digital signature

Re: [RFC 0/3] block/file-posix: Work around XFS bug

[Alberto Garcia]

On Mon 04 Nov 2019 04:14:56 PM CET, Max Reitz wrote:

>>> No, what I meant was that the original problem that led to
>>> c8bb23cbdbe would go away.
>> 
>> Ah, right. Not quite, according to my numbers:
>> 
>> |--++-+-|
>> | Cluster size | subclusters=on | subclusters=off | fallocate() |
>> |--++-+-|
>> |   256 KB | 10182 IOPS |966 IOPS |  14007 IOPS |
>> |   512 KB |  7919 IOPS |563 IOPS |  13442 IOPS |
>> |  1024 KB |  5050 IOPS |465 IOPS |  13887 IOPS |
>> |  2048 KB |  2465 IOPS |271 IOPS |  13885 IOPS |
>> |--++-+-|
>> 
>> There's obviously no backing image, and only the last column uses
>> handle_alloc_space() / fallocate().
>
> Thanks for providing some numbers!
>
> It was my impression, too, that subclusters wouldn’t solve it.  But it
> didn’t seem like that big of a difference to me.  Did you run this
> with aio=native?  (Because that’s where we have the XFS problem)

Here's with aio=native

|--++-+-|
| Cluster size | subclusters=on | subclusters=off | fallocate() |
|--++-+-|
|   256 KB | 19897 IOPS |891 IOPS |  32194 IOPS |
|   512 KB | 17881 IOPS |436 IOPS |  33092 IOPS |
|  1024 KB | 17050 IOPS |341 IOPS |  32768 IOPS |
|  2048 KB |  7854 IOPS |207 IOPS |  30944 IOPS |
|--++-+-|

Berto

Re: [PATCH v5 08/13] hw/core: deprecate old reset functions and introduce new ones

[Philippe Mathieu-Daudé]

On 11/4/19 1:01 PM, Damien Hedde wrote:

On 11/1/19 12:35 AM, Philippe Mathieu-Daudé wrote:

On 10/18/19 5:06 PM, Damien Hedde wrote:

Deprecate device_legacy_reset(), qdev_reset_all() and
qbus_reset_all() to be replaced by new functions
device_cold_reset() and bus_cold_reset() which uses resettable API.

Also introduce resettable_cold_reset_fn() which may be used as a
replacement for qdev_reset_all_fn and qbus_reset_all_fn().

Following patches will be needed to look at legacy reset call sites
and switch to resettable api. The legacy functions will be removed
when unused.

Signed-off-by: Damien Hedde 
---
[...]>>   +void resettable_cold_reset_fn(void *opaque)
+{
+    resettable_reset((Object *) opaque, RESET_TYPE_COLD);


Why not take a Object* argument?


This function is used to register a reset callback with
qemu_register_reset() (path 10 and 11), so we need void* to match the
prototype.


Alright.
Reviewed-by: Philippe Mathieu-Daudé 

Re: [RFC 0/3] block/file-posix: Work around XFS bug

[Max Reitz]

On 04.11.19 16:12, Alberto Garcia wrote:
> On Mon 04 Nov 2019 03:25:12 PM CET, Max Reitz wrote:
>> So, it's obvious that c8bb23cbdbe32f5c326 is significant for 1M
>> cluster-size, even on rotational disk, which means that previous
>> assumption about calling handle_alloc_space() only for ssd is wrong,
>> we need smarter heuristics..
>>
>> So, I'd prefer (1) or (2).

 OK.  I wonder whether that problem would go away with Berto’s subcluster
 series, though.
>>>
>>> Catching up with this now. I was told about this last week at the KVM
>>> Forum, but if the problems comes with the use of fallocate() and XFS,
>>> the I don't think subclusters will solve it.
>>>
>>> handle_alloc_space() is used to fill a cluster with zeroes when there's
>>> COW, and that happens the same with subclusters, just at the subcluster
>>> level instead of course.
>>>
>>> What can happen, if the subcluster size matches the filesystem block
>>> size, is that there's no need for any COW and therefore the bug is never
>>> triggered. But that's not quite the same as a fix :-)
>>
>> No, what I meant was that the original problem that led to c8bb23cbdbe
>> would go away.
> 
> Ah, right. Not quite, according to my numbers:
> 
> |--++-+-|
> | Cluster size | subclusters=on | subclusters=off | fallocate() |
> |--++-+-|
> |   256 KB | 10182 IOPS |966 IOPS |  14007 IOPS |
> |   512 KB |  7919 IOPS |563 IOPS |  13442 IOPS |
> |  1024 KB |  5050 IOPS |465 IOPS |  13887 IOPS |
> |  2048 KB |  2465 IOPS |271 IOPS |  13885 IOPS |
> |--++-+-|
> 
> There's obviously no backing image, and only the last column uses
> handle_alloc_space() / fallocate().

Thanks for providing some numbers!

It was my impression, too, that subclusters wouldn’t solve it.  But it
didn’t seem like that big of a difference to me.  Did you run this with
aio=native?  (Because that’s where we have the XFS problem)

Max



signature.asc
Description: OpenPGP digital signature

[PATCH v7 7/8] Acceptance tests: depend on qemu-img

[Cleber Rosa]

Tests using the avocado.utils.vmimage library make use of qemu-img,
and because it makes sense to use the version matching the rest of the
source code, let's make sure it gets built.

Its selection, instead of a possible qemu-img binary installed system
wide, is already dealt with by the change that adds the build dir to
the PATH during the test execution.

This is based on the same work for qemu-iotests, and suggested by its
author:

  - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html

CC: Philippe Mathieu-Daudé 
Signed-off-by: Cleber Rosa 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 65e85f5275..559c3e6375 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1174,7 +1174,7 @@ $(TESTS_RESULTS_DIR):
 
 check-venv: $(TESTS_VENV_DIR)
 
-check-acceptance: check-venv $(TESTS_RESULTS_DIR)
+check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
$(call quiet-command, \
 $(TESTS_VENV_DIR)/bin/python -m avocado \
 --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
-- 
2.21.0

[PATCH v7 8/8] Acceptance test: add "boot_linux" tests

[Cleber Rosa]

This acceptance test, validates that a full blown Linux guest can
successfully boot in QEMU.  In this specific case, the guest chosen is
Fedora version 31.

 * x86_64, pc and q35 machine types, with and without kvm as an
   accellerator

 * aarch64 and virt machine type, with and without kvm as an
   accellerator

 * ppc64 and pseries machine type

 * s390x and s390-ccw-virtio machine type

The method for checking the successful boot is based on "cloudinit"
and its "phone home" feature.  The guest is given an ISO image
with the location of the phone home server, and the information to
post (the instance ID).  Upon receiving the correct information,
from the guest, the test is considered to have PASSed.

This test is currently limited to user mode networking only, and
instructs the guest to connect to the "router" address that is hard
coded in QEMU.

To create the cloudinit ISO image that will be used to configure the
guest, the pycdlib library is also required and has been added as
requirement to the virtual environment created by "check-venv".

The console output is read by a separate thread, by means of the
Avocado datadrainer utility module.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/boot_linux.py | 175 +
 tests/requirements.txt |   1 +
 2 files changed, 176 insertions(+)
 create mode 100644 tests/acceptance/boot_linux.py

diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
new file mode 100644
index 00..882f7dc5df
--- /dev/null
+++ b/tests/acceptance/boot_linux.py
@@ -0,0 +1,175 @@
+# Functional test that boots a complete Linux system via a cloud image
+#
+# Copyright (c) 2018-2019 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import os
+
+from avocado_qemu import Test, SRC_ROOT_DIR
+
+from qemu import kvm_available
+
+from avocado.utils import cloudinit
+from avocado.utils import network
+from avocado.utils import vmimage
+from avocado.utils import datadrainer
+
+
+KVM_NOT_AVAILABLE = "KVM accelerator does not seem to be available"
+
+
+class BootLinux(Test):
+"""
+Boots a Linux system, checking for a successful initialization
+"""
+
+timeout = 600
+chksum = None
+
+def setUp(self):
+super(BootLinux, self).setUp()
+self.prepare_boot()
+self.vm.add_args('-smp', '2')
+self.vm.add_args('-m', '2048')
+self.vm.add_args('-drive', 'file=%s' % self.boot.path)
+self.prepare_cloudinit()
+
+def prepare_boot(self):
+self.log.info('Downloading/preparing boot image')
+# Fedora 31 only provides ppc64le images
+image_arch = self.arch
+if image_arch == 'ppc64':
+image_arch = 'ppc64le'
+try:
+self.boot = vmimage.get(
+'fedora', arch=image_arch, version='31',
+checksum=self.chksum,
+algorithm='sha256',
+cache_dir=self.cache_dirs[0],
+snapshot_dir=self.workdir)
+except:
+self.cancel('Failed to download/prepare boot image')
+
+def prepare_cloudinit(self):
+self.log.info('Preparing cloudinit image')
+try:
+cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
+self.phone_home_port = network.find_free_port()
+cloudinit.iso(cloudinit_iso, self.name,
+  username='root',
+  password='password',
+  # QEMU's hard coded usermode router address
+  phone_home_host='10.0.2.2',
+  phone_home_port=self.phone_home_port)
+self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
+except Exception:
+self.cancel('Failed to prepared cloudinit image')
+
+def launch_and_wait(self):
+self.vm.set_console()
+self.vm.launch()
+console_drainer = 
datadrainer.LineLogger(self.vm.console_socket.fileno(),
+ 
logger=self.log.getChild('console'))
+console_drainer.start()
+self.log.info('VM launched, waiting for boot confirmation from guest')
+cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port), 
self.name)
+
+
+class BootLinuxX8664(BootLinux):
+"""
+:avocado: tags=arch:x86_64
+"""
+
+chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
+
+def test_pc(self):
+"""
+:avocado: tags=machine:pc
+"""
+self.launch_and_wait()
+
+def test_kvm_pc(self):
+"""
+:avocado: tags=machine:pc
+:avocado: tags=accel:kvm
+"""
+if not kvm_available(self.arch):
+self.cancel(KVM_NOT_AVAILABLE)
+self.vm.add_args("-accel", "kvm")
+self.launch_and_wait()
+
+

[PATCH v7 6/8] Acceptance tests: add the build directory to the system PATH

[Cleber Rosa]

So that when binaries such as qemu-img are searched for, those in the
build tree will be favored.  As a clarification, SRC_ROOT_DIR is
dependent on the location from where tests are executed, so they are
equal to the build directory if one is being used.

The original motivation is that Avocado libraries such as
avocado.utils.vmimage.get() may use the matching binaries, but it may
also apply to any other binary that test code may eventually attempt
to execute.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 17ce583c87..a4bb796a47 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -110,6 +110,12 @@ class Test(avocado.Test):
 return None
 
 def setUp(self):
+# Some utility code uses binaries from the system's PATH.  For
+# instance, avocado.utils.vmimage.get() uses qemu-img, to
+# create a snapshot image.  This is a transparent way of
+# making sure those utilities find and use binaries on the
+# build tree by default.
+os.environ['PATH'] = '%s:%s' % (SRC_ROOT_DIR, os.environ['PATH'])
 self._vms = {}
 
 self.arch = self.params.get('arch',
-- 
2.21.0

[PATCH v7 4/8] Acceptance tests: use relative location for tests

[Cleber Rosa]

An Avocado Test ID[1] is composed by a number of components, but it
starts with the Test Name, usually a file system location that was
given to the loader.

Because the source directory is being given as a prefix to the
"tests/acceptance" directory containing the acceptance tests, the test
names will needlessly include the directory the user is using to host
the QEMU sources (and/or build tree).

Let's remove the source dir (or a build dir) from the path given to
the test loader.  This should give more constant names, and when using
result servers and databases, it should give the same test names
across executions from different people or from different directories.

[1] - 
https://avocado-framework.readthedocs.io/en/69.0/ReferenceGuide.html#test-id

Signed-off-by: Cleber Rosa 
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 56f73b46e2..65e85f5275 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1180,7 +1180,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
 --filter-by-tags-include-empty --filter-by-tags-include-empty-key \
 $(AVOCADO_TAGS) \
---failfast=on $(SRC_PATH)/tests/acceptance, \
+--failfast=on tests/acceptance, \
 "AVOCADO", "tests/acceptance")
 
 # Consolidated targets
-- 
2.21.0

[PATCH v7 5/8] Acceptance tests: keep a stable reference to the QEMU build dir

[Cleber Rosa]

This is related to the the differences in in-tree and out-of-tree
builds in QEMU.  For simplification,  means my build directory.

Currently, by running a `make check-acceptance` one gets (in
tests/acceptance/avocado_qemu/__init__.py):

   SRC_ROOT_DIR: /tests/acceptance/avocado_qemu/../../..

This in itself is problematic, because after the parent directories
are applied, one may be left not with a pointer to the build directory
as intended, but with the location of the source tree (assuming they
differ). Built binaries, such as qemu-img, are of course not there and
can't be found.

Given that a Python '__file__' will contain the absolute path to the
file backing the module, say:

   __file__: /tests/acceptance/avocado_qemu/__init__.py

  |  4  | 3|  2 | 1 |

A solution is to not "evaluate" the third parent dir (marked as 4
here) because that ends up following the "tests" directory symlink to
the source tree.  In fact, there's no need to keep or evaluate any of
the parent directories, we can just drop the rightmost 4 components,
and we'll keep a stable reference to the build directory (with no
symlink being followed).  This works for either a dedicated build
directory or also a combined source and build tree.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 6618ea67c1..17ce583c87 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -16,7 +16,7 @@ import tempfile
 
 import avocado
 
-SRC_ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', '..')
+SRC_ROOT_DIR = 
os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(__file__
 sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
 
 from qemu.machine import QEMUMachine
-- 
2.21.0

[PATCH v7 2/8] Acceptance tests: introduce utility method for tags unique vals

[Cleber Rosa]

Currently a test can describe the target architecture binary that it
should primarily be run with, be setting a single tag value.

The same approach is expected to be done with other QEMU aspects to be
tested, for instance, the machine type and accelerator, so let's
generalize the logic into a utility method.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 9a57c020d8..e676d9c4e7 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -100,14 +100,21 @@ def exec_command_and_wait_for_pattern(test, command,
 
 
 class Test(avocado.Test):
+def _get_unique_tag_val(self, tag_name):
+"""
+Gets a tag value, if unique for a key
+"""
+vals = self.tags.get(tag_name, [])
+if len(vals) == 1:
+return vals.pop()
+return None
+
 def setUp(self):
 self._vms = {}
-arches = self.tags.get('arch', [])
-if len(arches) == 1:
-arch = arches.pop()
-else:
-arch = None
-self.arch = self.params.get('arch', default=arch)
+
+self.arch = self.params.get('arch',
+default=self._get_unique_tag_val('arch'))
+
 default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
 self.qemu_bin = self.params.get('qemu_bin',
 default=default_qemu_bin)
-- 
2.21.0

[PATCH v7 3/8] Acceptance tests: use avocado tags for machine type

[Cleber Rosa]

The same way the arch tag is being used as a fallback for the arch
parameter, let's do the same for QEMU's machine and avoid some boiler
plate code.

Signed-off-by: Cleber Rosa 
---
 docs/devel/testing.rst | 18 
 tests/acceptance/avocado_qemu/__init__.py  |  5 ++
 tests/acceptance/boot_linux_console.py | 19 +---
 tests/acceptance/cpu_queries.py|  2 +-
 tests/acceptance/linux_initrd.py   |  2 +-
 tests/acceptance/linux_ssh_mips_malta.py   |  5 --
 tests/acceptance/machine_m68k_nextcube.py  | 21 ++---
 tests/acceptance/machine_sparc_leon3.py|  3 +-
 tests/acceptance/ppc_prep_40p.py   |  3 --
 tests/acceptance/x86_cpu_model_versions.py | 53 --
 10 files changed, 72 insertions(+), 59 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 8e981e062d..27f286858a 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -746,6 +746,17 @@ name.  If one is not given explicitly, it will either be 
set to
 ``None``, or, if the test is tagged with one (and only one)
 ``:avocado: tags=arch:VALUE`` tag, it will be set to ``VALUE``.
 
+machine
+~~~
+
+The machine type that will be set to all QEMUMachine instances created
+by the test.
+
+The ``machine`` attribute will be set to the test parameter of the same
+name.  If one is not given explicitly, it will either be set to
+``None``, or, if the test is tagged with one (and only one)
+``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
+
 qemu_bin
 
 
@@ -781,6 +792,13 @@ architecture of a kernel or disk image to boot a VM with.
 This parameter has a direct relation with the ``arch`` attribute.  If
 not given, it will default to None.
 
+machine
+~~~
+
+The machine type that will be set to all QEMUMachine instances created
+by the test.
+
+
 qemu_bin
 
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index e676d9c4e7..6618ea67c1 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -115,6 +115,9 @@ class Test(avocado.Test):
 self.arch = self.params.get('arch',
 default=self._get_unique_tag_val('arch'))
 
+self.machine = self.params.get('machine',
+   
default=self._get_unique_tag_val('machine'))
+
 default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
 self.qemu_bin = self.params.get('qemu_bin',
 default=default_qemu_bin)
@@ -136,6 +139,8 @@ class Test(avocado.Test):
 name = str(uuid.uuid4())
 if self._vms.get(name) is None:
 self._vms[name] = self._new_vm(*args)
+if self.machine is not None:
+self._vms[name].set_machine(self.machine)
 return self._vms[name]
 
 def tearDown(self):
diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 7e41cebd47..6732cc59ca 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -62,7 +62,6 @@ class BootLinuxConsole(Test):
 kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
 kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-self.vm.set_machine('pc')
 self.vm.set_console()
 kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
 self.vm.add_args('-kernel', kernel_path,
@@ -85,7 +84,6 @@ class BootLinuxConsole(Test):
 kernel_path = self.extract_from_deb(deb_path,
 '/boot/vmlinux-2.6.32-5-4kc-malta')
 
-self.vm.set_machine('malta')
 self.vm.set_console()
 kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
 self.vm.add_args('-kernel', kernel_path,
@@ -118,7 +116,6 @@ class BootLinuxConsole(Test):
 kernel_path = self.extract_from_deb(deb_path,
 '/boot/vmlinux-2.6.32-5-5kc-malta')
 
-self.vm.set_machine('malta')
 self.vm.set_console()
 kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
 self.vm.add_args('-kernel', kernel_path,
@@ -148,7 +145,6 @@ class BootLinuxConsole(Test):
 initrd_path = self.workdir + "rootfs.cpio"
 archive.gzip_uncompress(initrd_path_gz, initrd_path)
 
-self.vm.set_machine('malta')
 self.vm.set_console()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
+ 'console=ttyS0 console=tty '
@@ -188,7 +184,6 @@ class BootLinuxConsole(Test):
 initrd_path = self.workdir + "rootfs.cpio"
 archive.gzip_uncompress(initrd_path_gz, initrd_path)
 
-self.vm.set_machine('malta')
 self.vm.set_console()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE
  

[PATCH v7 1/8] Acceptance test x86_cpu_model_versions: use default vm

[Cleber Rosa]

The default vm provided by the test, available as self.vm, serves the
same purpose of the one obtained by self.get_vm(), but saves a line
and matches the style of other tests.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/x86_cpu_model_versions.py | 100 ++---
 1 file changed, 46 insertions(+), 54 deletions(-)

diff --git a/tests/acceptance/x86_cpu_model_versions.py 
b/tests/acceptance/x86_cpu_model_versions.py
index 5fc9ca4bc6..6eb977954d 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -25,10 +25,6 @@
 import avocado_qemu
 import re
 
-def get_cpu_prop(vm, prop):
-cpu_path = vm.command('query-cpus')[0].get('qom_path')
-return vm.command('qom-get', path=cpu_path, property=prop)
-
 class X86CPUModelAliases(avocado_qemu.Test):
 """
 Validation of PC CPU model versions and CPU model aliases
@@ -241,78 +237,74 @@ class CascadelakeArchCapabilities(avocado_qemu.Test):
 
 :avocado: tags=arch:x86_64
 """
+def get_cpu_prop(self, prop):
+cpu_path = self.vm.command('query-cpus')[0].get('qom_path')
+return self.vm.command('qom-get', path=cpu_path, property=prop)
+
 def test_4_1(self):
 # machine-type only:
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.1')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.1')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
  'pc-i440fx-4.1 + Cascadelake-Server should not have 
arch-capabilities')
 
 def test_4_0(self):
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.0')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
  'pc-i440fx-4.0 + Cascadelake-Server should not have 
arch-capabilities')
 
 def test_set_4_0(self):
 # command line must override machine-type if CPU model is not 
versioned:
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
-vm.launch()
-self.assertTrue(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.0')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')
+self.vm.launch()
+self.assertTrue(self.get_cpu_prop('arch-capabilities'),
 'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities 
should have arch-capabilities')
 
 def test_unset_4_1(self):
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.1')
-vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.1')
+self.vm.add_args('-cpu', 
'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
  'pc-i440fx-4.1 + 
Cascadelake-Server,-arch-capabilities should not have arch-capabilities')
 
 def test_v1_4_0(self):
 # versioned CPU model overrides machine-type:
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
-vm.launch()
-self.assertFalse(get_cpu_prop(vm, 'arch-capabilities'),
+self.vm.add_args('-S')
+self.vm.set_machine('pc-i440fx-4.0')
+self.vm.add_args('-cpu', 
'Cascadelake-Server-v1,x-force-features=on,check=off,enforce=off')
+self.vm.launch()
+self.assertFalse(self.get_cpu_prop('arch-capabilities'),
  'pc-i440fx-4.0 + Cascadelake-Server-v1 should not 
have arch-capabilities')
 
 def test_v2_4_0(self):
-vm = self.get_vm()
-vm.add_args('-S')
-vm.set_machine('pc-i440fx-4.0')
-vm.add_args('-cpu', 

[PATCH v7 0/8] Acceptance test: Add "boot_linux" acceptance test

[Cleber Rosa]

This acceptance test, validates that a full blown Linux guest can
successfully boot in QEMU.  In this specific case, the guest chosen is
Fedora version 31.

 * x86_64, pc and q35 machine types, with and without kvm as an
   accellerator

 * aarch64 and virt machine type, with and without kvm as an
   accellerator

 * ppc64 and pseries machine type

 * s390x and s390-ccw-virtio machine type

This has been tested on x86_64 and ppc64le hosts and has been running
reliably (in my experience) on Travis CI.

Some hopefully useful pointers for reviewers:
=

Git Info:
  - URI: https://github.com/clebergnu/qemu/tree/test_boot_linux_v7
  - Remote: https://github.com/clebergnu/qemu
  - Branch: test_boot_linux_v7

Travis CI Info:
  - Build: https://travis-ci.org/clebergnu/qemu/builds/606191094
  - Job: https://travis-ci.org/clebergnu/qemu/jobs/606191120

Previous version:
  - v6: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01202.html
  - v5: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04652.html
  - v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg02032.html
  - v3: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01677.html
  - v2: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04318.html
  - v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02530.html

Changes from v6:


 * Bumped Fedora to most recently released version (31).

 * Included new architectures (ppc64 and s390x), consolidating all
   tests into the same commit.

 * New commit: "Acceptance tests: use avocado tags for machine type"

 * New commit: "Acceptance tests: introduce utility method for tags
   unique vals"

 * New commit: "Acceptance test x86_cpu_model_versions: use default
   vm", needed to normalize the use of the machine type tags

 * Added a lot of leniency to the test setup (and reliability to the
   test/job), canceling the test if there are any failures while
   downloading/preparing the boot images.

 * Made use of Avocado's data drainer a regular feature (dropped the
   commit with RFC) and squashed it.

 * Bumped pycdlib version to 1.8.0

 * Dropped explicit "--enable-slirp=git" (added on v5) to Travis CI
   configure line, as the default configuration on Travis CI now
   results in user networking capabilities.

Changes from v5:


 * Added explicit "--enable-slirp=git" to Travis CI configure line, as
   these tests depend on "-netdev user" like networking.

 * Bumped Fedora to most recently released version (30).

 * Changed "checksum" parameter to 'sha256' and use the same hashes as
   provided by the Fedora project (instead of using Avocado's default
   sha1 and compute and use a different hash value).

 * New commit: Add "boot_linux" test for aarch64 and virt machine type

 * New commit: [RFC]: use Avocado data drainer for console logging

Changes from v4:


 * New commit "Acceptance tests: use relative location for tests"

 * New commit "Acceptance tests: keep a stable reference to the QEMU build dir"

 * Pinned the Fedora 29 image by adding a checksum.  The goal is to
   never allow more than one component to change at a time (the one
   allowed to change is QEMU itself).  Updates to the image should be
   manual. (Based on comments from Cornelia)

 * Moved the downloading of the Fedora 29 cloud image to the test
   setUp() method, canceling the test if the image can not be
   downloaded.

 * Removed the ":avocado: enable" tag, given that Avocado versions
   68.0 and later operate on a "recursive by default" manner, that
   is able to correctly identify this as an Avocado test.

Changes from v3:


 * New patch "Acceptance tests: depend on qemu-img"

Known Issues on v3 (no longer applicable):
==

 * A recent TCG performance regression[1] affects this test in a
   number of ways:
   - The test execution may timeout by itself
   - The generation of SSH host keys in the guest's first boot is also
 affected (possibly also a timeout)
   - The cloud-init "phone home" feature attempts to read the host keys
 and fails, causing the test to timeout and fail

   These are not observed anymore once the fix[2] is applied.

[1] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00338.html
[2] - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01129.html

Changes from v2:


 * Updated the tag to include the "arch:" key, in a similar fashion as to
   the tests in the "Acceptance Tests: target architecture support":
   - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00369.html

 * Renamed the test method name to test_x86_64_pc, again, similarly to the
   boot_linux_console.py tests in the series mentioned before.

 * Set the machine type explicitly, again similarly to the
   boot_linux_console.py tests in the series mentioned before.

 * Added messages after the launch of the VM, to 

Re: [RFC 0/3] block/file-posix: Work around XFS bug

[Alberto Garcia]

On Mon 04 Nov 2019 03:25:12 PM CET, Max Reitz wrote:
> So, it's obvious that c8bb23cbdbe32f5c326 is significant for 1M
> cluster-size, even on rotational disk, which means that previous
> assumption about calling handle_alloc_space() only for ssd is wrong,
> we need smarter heuristics..
>
> So, I'd prefer (1) or (2).
>>>
>>> OK.  I wonder whether that problem would go away with Berto’s subcluster
>>> series, though.
>> 
>> Catching up with this now. I was told about this last week at the KVM
>> Forum, but if the problems comes with the use of fallocate() and XFS,
>> the I don't think subclusters will solve it.
>> 
>> handle_alloc_space() is used to fill a cluster with zeroes when there's
>> COW, and that happens the same with subclusters, just at the subcluster
>> level instead of course.
>> 
>> What can happen, if the subcluster size matches the filesystem block
>> size, is that there's no need for any COW and therefore the bug is never
>> triggered. But that's not quite the same as a fix :-)
>
> No, what I meant was that the original problem that led to c8bb23cbdbe
> would go away.

Ah, right. Not quite, according to my numbers:

|--++-+-|
| Cluster size | subclusters=on | subclusters=off | fallocate() |
|--++-+-|
|   256 KB | 10182 IOPS |966 IOPS |  14007 IOPS |
|   512 KB |  7919 IOPS |563 IOPS |  13442 IOPS |
|  1024 KB |  5050 IOPS |465 IOPS |  13887 IOPS |
|  2048 KB |  2465 IOPS |271 IOPS |  13885 IOPS |
|--++-+-|

There's obviously no backing image, and only the last column uses
handle_alloc_space() / fallocate().

Berto

[PATCH] hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine

[Clement Deschamps]

Set the NSACR CP11 and CP10 bits, to allow FPU access in Non-Secure state
when using dummy SMC setup routine. Otherwise an AArch32 kernel will UNDEF as
soon as it tries to use the FPU.

This fixes kernel panic when booting raspbian on raspi2.

Successfully tested with:
  2017-01-11-raspbian-jessie-lite.img
  2018-11-13-raspbian-stretch-lite.img
  2019-07-10-raspbian-buster-lite.img

See also commit ece628fcf6 that fixes the issue when *not* using the
dummy SMC setup routine.

Fixes: fc1120a7f5
Signed-off-by: Clement Deschamps 
---
 hw/arm/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ef6724960c..8fb4a63606 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -240,6 +240,9 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
 };
 uint32_t board_setup_blob[] = {
 /* board setup addr */
+0xee110f51, /* mrc p15, 0, r0, c1, c1, 2  ;read NSACR */
+0xe3800b03, /* orr r0, #0xc00 ;set CP11, CP10 */
+0xee010f51, /* mcr p15, 0, r0, c1, c1, 2  ;write NSACR */
 0xe3a00e00 + (mvbar_addr >> 4), /* mov r0, #mvbar_addr */
 0xee0c0f30, /* mcr p15, 0, r0, c12, c0, 1 ;set MVBAR */
 0xee110f11, /* mrc p15, 0, r0, c1 , c1, 0 ;read SCR */
-- 
2.23.0

Re: [RFC PATCH v2 15/26] qcow2: Add subcluster support to zero_in_l2_slice()

[Max Reitz]

On 04.11.19 16:04, Max Reitz wrote:
> On 26.10.19 23:25, Alberto Garcia wrote:
>> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
>> image has subclusters. Instead, the individual 'all zeroes' bits must
>> be used.
>>
>> Signed-off-by: Alberto Garcia 
>> ---
>>  block/qcow2-cluster.c | 14 ++
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>> index e67559152f..3e4ba8d448 100644
>> --- a/block/qcow2-cluster.c
>> +++ b/block/qcow2-cluster.c
>> @@ -1852,7 +1852,7 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
>> uint64_t offset,
>>  assert(nb_clusters <= INT_MAX);
>>  
>>  for (i = 0; i < nb_clusters; i++) {
>> -uint64_t old_offset;
>> +uint64_t old_offset, l2_entry = 0;
>>  QCow2ClusterType cluster_type;
>>  
>>  old_offset = get_l2_entry(s, l2_slice, l2_index + i);
>> @@ -1869,12 +1869,18 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
>> uint64_t offset,
>>  
>>  qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
>>  if (cluster_type == QCOW2_CLUSTER_COMPRESSED || unmap) {
>> -set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
>>  qcow2_free_any_clusters(bs, old_offset, 1, 
>> QCOW2_DISCARD_REQUEST);
> 
> It feels wrong to me to free the cluster before updating the L2 entry.

(Although it’s pre-existing, as set_l2_entry() is just an in-cache
operation anyway :-/)

Max



signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v2 16/26] qcow2: Add subcluster support to discard_in_l2_slice()

[Max Reitz]

On 26.10.19 23:25, Alberto Garcia wrote:
> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
> image has subclusters. Instead, the individual 'all zeroes' bits must
> be used.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-cluster.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 3e4ba8d448..aa3eb727a5 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1772,7 +1772,11 @@ static int discard_in_l2_slice(BlockDriverState *bs, 
> uint64_t offset,
>  
>  /* First remove L2 entries */
>  qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
> -if (!full_discard && s->qcow_version >= 3) {
> +if (has_subclusters(s)) {
> +set_l2_entry(s, l2_slice, l2_index + i, 0);
> +set_l2_bitmap(s, l2_slice, l2_index + i,
> +  full_discard ? 0 : QCOW_L2_BITMAP_ALL_ZEROES);

But only for !full_discard, right?

Max

> +} else if (!full_discard && s->qcow_version >= 3) {
>  set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
>  } else {
>  set_l2_entry(s, l2_slice, l2_index + i, 0);
> 




signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v2 15/26] qcow2: Add subcluster support to zero_in_l2_slice()

[Max Reitz]

On 26.10.19 23:25, Alberto Garcia wrote:
> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
> image has subclusters. Instead, the individual 'all zeroes' bits must
> be used.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-cluster.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index e67559152f..3e4ba8d448 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1852,7 +1852,7 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
> uint64_t offset,
>  assert(nb_clusters <= INT_MAX);
>  
>  for (i = 0; i < nb_clusters; i++) {
> -uint64_t old_offset;
> +uint64_t old_offset, l2_entry = 0;
>  QCow2ClusterType cluster_type;
>  
>  old_offset = get_l2_entry(s, l2_slice, l2_index + i);
> @@ -1869,12 +1869,18 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
> uint64_t offset,
>  
>  qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
>  if (cluster_type == QCOW2_CLUSTER_COMPRESSED || unmap) {
> -set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
>  qcow2_free_any_clusters(bs, old_offset, 1, 
> QCOW2_DISCARD_REQUEST);

It feels wrong to me to free the cluster before updating the L2 entry.

Max

>  } else {
> -uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
> -set_l2_entry(s, l2_slice, l2_index + i, entry | QCOW_OFLAG_ZERO);
> +l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
>  }
> +
> +if (has_subclusters(s)) {
> +set_l2_bitmap(s, l2_slice, l2_index + i, 
> QCOW_L2_BITMAP_ALL_ZEROES);
> +} else {
> +l2_entry |= QCOW_OFLAG_ZERO;
> +}
> +
> +set_l2_entry(s, l2_slice, l2_index + i, l2_entry);
>  }
>  
>  qcow2_cache_put(s->l2_table_cache, (void **) _slice);
> 




signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v2 11/26] qcow2: Add qcow2_get_subcluster_type()

[Max Reitz]

On 04.11.19 13:35, Max Reitz wrote:
> On 26.10.19 23:25, Alberto Garcia wrote:
>> This function returns the type of an individual subcluster. If an
>> image does not have subclusters then this returns the exact same value
>> as qcow2_get_cluster_type().
>>
>> The information in standard and extended L2 entries is encoded in a
>> slightly different way, but all existing QCow2ClusterType values are
>> also valid for subclusters and have the same meanings (although they
>> typically only apply to the requested subcluster).
>>
>> There are two important exceptions to this:
>>
>>   a) QCOW2_CLUSTER_COMPRESSED means that the whole cluster is
>>  compressed. We do not support compression at the subcluster
>>  level.
>>
>>   b) QCOW2_CLUSTER_UNALLOCATED means that the cluster is unallocated,
>>  that is, the offset field of the L2 entry does not point to a
>>  host cluster. All subclusters are obviously unallocated too but
>>  any of them could be of type QCOW2_CLUSTER_ZERO_PLAIN.
>>
>> In addition to that, extended L2 entries allow one new scenario where
>> the cluster is normally allocated but an individual subcluster is not.
>> This is very different from (b) and because of that this patch adds a
>> new value called QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER.
>>
>> As a last thing, this patch adds QCOW2_CLUSTER_INVALID to detect the
>> cases where an L2 entry has a value that violates the spec. The caller
>> is responsible for handling these situations.
>>
>> To prevent compatibility problems with images that have invalid values
>> but are currently being read by QEMU without causing side effects,
>> QCOW2_CLUSTER_INVALID is only returned for images with extended L2
>> entries.
>>
>> Signed-off-by: Alberto Garcia 
>> ---
>>  block/qcow2.h | 62 +++
>>  1 file changed, 62 insertions(+)
> 
> [...]
> 
>> +case QCOW2_CLUSTER_ZERO_PLAIN:
>> +case QCOW2_CLUSTER_ZERO_ALLOC:
>> +return QCOW2_CLUSTER_INVALID;
> 
> The spec doesn’t say anything about this, though.

It does, I just forgot. O:-)

Max



signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v2 14/26] qcow2: Add subcluster support to qcow2_get_cluster_offset()

[Max Reitz]

On 26.10.19 23:25, Alberto Garcia wrote:
> The logic of this function remains pretty much the same, except that
> it uses count_contiguous_subclusters(), which combines the logic of
> count_contiguous_clusters() / count_contiguous_clusters_unallocated()
> and checks individual subclusters.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-cluster.c | 111 --
>  1 file changed, 52 insertions(+), 59 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 990bc070af..e67559152f 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -372,66 +372,51 @@ fail:
>  }
>  
>  /*
> - * Checks how many clusters in a given L2 slice are contiguous in the image
> - * file. As soon as one of the flags in the bitmask stop_flags changes 
> compared
> - * to the first cluster, the search is stopped and the cluster is not counted
> - * as contiguous. (This allows it, for example, to stop at the first 
> compressed
> - * cluster which may require a different handling)
> + * Return the number of contiguous subclusters of the exact same type
> + * in a given L2 slice, starting from cluster @l2_index, subcluster
> + * @sc_index. At most @nb_clusters are checked. Allocated clusters are

I’d add a note that reassures that @nb_clusters really is a number of
clusters, not subclusters.  (Because if the variable were named “x”
(i.e., it’d be “At most @x are checked”), you’d assume it refers to a
number of subclusters.)

OTOH, what I don’t like so far about this series is that the “cluster
logic” is still everywhere when I think it should just be about
subclusters now.  (Except in few places where it must be about clusters
as in something that can have a distinct host offset and/or has an own
L2 entry.)  So maybe the parameter should really be @nb_subclusters.

But I’m not sure.  For how this function is written right now, it makes
sense for it to be @nb_clusters, but I think it could be changed so it
would work with @nb_subclusters, too.  (Just a single loop over the
subclusters and then loading l2_entry+l2_bitmap and checking the offset
at every cluster boundary.)

> + * also required to be contiguous in the image file.
>   */
> -static int count_contiguous_clusters(BlockDriverState *bs, int nb_clusters,
> -int cluster_size, uint64_t *l2_slice, int l2_index, uint64_t 
> stop_flags)
> +static int count_contiguous_subclusters(BlockDriverState *bs, int 
> nb_clusters,
> +unsigned sc_index, uint64_t 
> *l2_slice,
> +int l2_index)
>  {
>  BDRVQcow2State *s = bs->opaque;
> -int i;
> -QCow2ClusterType first_cluster_type;
> -uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW_OFLAG_COMPRESSED;
> -uint64_t first_entry = get_l2_entry(s, l2_slice, l2_index);
> -uint64_t offset = first_entry & mask;
> -
> -first_cluster_type = qcow2_get_cluster_type(bs, first_entry);
> -if (first_cluster_type == QCOW2_CLUSTER_UNALLOCATED) {
> -return 0;
> +int i, j, count = 0;
> +uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index);
> +uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
> +uint64_t expected_offset = l2_entry & L2E_OFFSET_MASK;
> +bool check_offset = true;
> +QCow2ClusterType type =
> +qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc_index);
> +
> +assert(type != QCOW2_CLUSTER_INVALID); /* The caller should check this */
> +
> +if (type == QCOW2_CLUSTER_COMPRESSED) {
> +return 1; /* Compressed clusters are always counted one by one */

Hm, yes, but cluster by cluster, not subcluster by subcluster, so this
should be s->subclusters_per_cluster, and perhaps sc_index should be
asserted to be 0.  (Or it should be s->subclusters_per_cluster - sc_index.)

[...]

> @@ -514,8 +499,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,

I suppose this is get_subcluster_offset now.

[...]

> @@ -587,6 +574,13 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>  goto fail;
>  }
>  switch (type) {
> +case QCOW2_CLUSTER_INVALID:
> +qcow2_signal_corruption(bs, true, -1, -1, "Invalid cluster entry 
> found "
> +" (L2 offset: %#" PRIx64 ", L2 index: %#x)",
> +l2_offset, l2_index);
> +ret = -EIO;
> +goto fail;
> +break;

No need to break here.

>  case QCOW2_CLUSTER_COMPRESSED:
>  if (has_data_file(bs)) {
>  qcow2_signal_corruption(bs, true, -1, -1, "Compressed cluster "
> @@ -602,16 +596,15 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>  break;

Here (QCOW2_CLUSTER_COMPRESSED), c is 1, even though it should count
subclusters.

Max

>  case QCOW2_CLUSTER_ZERO_PLAIN:
>  case QCOW2_CLUSTER_UNALLOCATED:
> -/* how many empty clusters ? */
> -c = 

Re: [PATCH v5 02/13] hw/core/qdev: add trace events to help with resettable transition

[Philippe Mathieu-Daudé]

On 11/4/19 1:16 PM, Damien Hedde wrote:

On 11/1/19 12:23 AM, Philippe Mathieu-Daudé wrote:

On 10/18/19 5:06 PM, Damien Hedde wrote:

Adds trace events to reset procedure and when updating the parent
bus of a device.

Signed-off-by: Damien Hedde 
---
   hw/core/qdev.c   | 27 ---
   hw/core/trace-events |  9 +
   2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 60be2f2fef..f230063189 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -38,6 +38,7 @@
   #include "hw/boards.h"
   #include "hw/sysbus.h"
   #include "migration/vmstate.h"
+#include "trace.h"
     bool qdev_hotplug = false;
   static bool qdev_hot_added = false;
@@ -98,7 +99,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState
*bus)
   bool replugging = dev->parent_bus != NULL;
     if (replugging) {
-    /* Keep a reference to the device while it's not plugged into
+    trace_qdev_update_parent_bus(dev, dev->parent_bus, bus);


Nitpicking, if you respin, can you add object_get_typename(OBJECT(dev)))
and object_get_typename(OBJECT(bus)))?


sure. I was wondering if having some kind of qom object support to trace
would be feasible. Because it's a bit tedious to add this each time. And
IMO it would be more useful to have the path, but we can't reasonably
compute it as a trace_..() arguments.


Meanwhile you can use:

  if (trace_event_get_state_backends(TRACE_QDEV_UPDATE_PARENT_BUS)) {
  ...



With/without it:
Reviewed-by: Philippe Mathieu-Daudé 


+    /*
+ * Keep a reference to the device while it's not plugged into
    * any bus, to avoid it potentially evaporating when it is
    * dereffed in bus_remove_child().
    */
@@ -272,6 +275,18 @@ HotplugHandler
*qdev_get_hotplug_handler(DeviceState *dev)
   return hotplug_ctrl;
   }
   +static int qdev_prereset(DeviceState *dev, void *opaque)
+{
+    trace_qdev_reset_tree(dev, object_get_typename(OBJECT(dev)));
+    return 0;
+}
+
+static int qbus_prereset(BusState *bus, void *opaque)
+{
+    trace_qbus_reset_tree(bus, object_get_typename(OBJECT(bus)));
+    return 0;
+}
+
   static int qdev_reset_one(DeviceState *dev, void *opaque)
   {
   device_legacy_reset(dev);
@@ -282,6 +297,7 @@ static int qdev_reset_one(DeviceState *dev, void
*opaque)
   static int qbus_reset_one(BusState *bus, void *opaque)
   {
   BusClass *bc = BUS_GET_CLASS(bus);
+    trace_qbus_reset(bus, object_get_typename(OBJECT(bus)));
   if (bc->reset) {
   bc->reset(bus);
   }
@@ -290,7 +306,9 @@ static int qbus_reset_one(BusState *bus, void
*opaque)
     void qdev_reset_all(DeviceState *dev)
   {
-    qdev_walk_children(dev, NULL, NULL, qdev_reset_one,
qbus_reset_one, NULL);
+    trace_qdev_reset_all(dev, object_get_typename(OBJECT(dev)));
+    qdev_walk_children(dev, qdev_prereset, qbus_prereset,
+   qdev_reset_one, qbus_reset_one, NULL);
   }
     void qdev_reset_all_fn(void *opaque)
@@ -300,7 +318,9 @@ void qdev_reset_all_fn(void *opaque)
     void qbus_reset_all(BusState *bus)
   {
-    qbus_walk_children(bus, NULL, NULL, qdev_reset_one,
qbus_reset_one, NULL);
+    trace_qbus_reset_all(bus, object_get_typename(OBJECT(bus)));
+    qbus_walk_children(bus, qdev_prereset, qbus_prereset,
+   qdev_reset_one, qbus_reset_one, NULL);
   }
     void qbus_reset_all_fn(void *opaque)
@@ -1108,6 +1128,7 @@ void device_legacy_reset(DeviceState *dev)
   {
   DeviceClass *klass = DEVICE_GET_CLASS(dev);
   +    trace_qdev_reset(dev, object_get_typename(OBJECT(dev)));
   if (klass->reset) {
   klass->reset(dev);
   }
diff --git a/hw/core/trace-events b/hw/core/trace-events
index fe47a9c8cb..1a669be0ea 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -1,2 +1,11 @@
   # loader.c
   loader_write_rom(const char *name, uint64_t gpa, uint64_t size, bool
isrom) "%s: @0x%"PRIx64" size=0x%"PRIx64" ROM=%d"
+
+# qdev.c
+qdev_reset(void *obj, const char *objtype) "obj=%p(%s)"
+qdev_reset_all(void *obj, const char *objtype) "obj=%p(%s)"
+qdev_reset_tree(void *obj, const char *objtype) "obj=%p(%s)"
+qbus_reset(void *obj, const char *objtype) "obj=%p(%s)"
+qbus_reset_all(void *obj, const char *objtype) "obj=%p(%s)"
+qbus_reset_tree(void *obj, const char *objtype) "obj=%p(%s)"
+qdev_update_parent_bus(void *obj, void *oldp, void *newp) "obj=%p
old_parent=%p new_parent=%p"

Re: [RFC 0/3] block/file-posix: Work around XFS bug

[Max Reitz]

On 04.11.19 15:03, Alberto Garcia wrote:
> On Fri 25 Oct 2019 04:19:30 PM CEST, Max Reitz wrote:
 So, it's obvious that c8bb23cbdbe32f5c326 is significant for 1M
 cluster-size, even on rotational disk, which means that previous
 assumption about calling handle_alloc_space() only for ssd is wrong,
 we need smarter heuristics..

 So, I'd prefer (1) or (2).
>>
>> OK.  I wonder whether that problem would go away with Berto’s subcluster
>> series, though.
> 
> Catching up with this now. I was told about this last week at the KVM
> Forum, but if the problems comes with the use of fallocate() and XFS,
> the I don't think subclusters will solve it.
> 
> handle_alloc_space() is used to fill a cluster with zeroes when there's
> COW, and that happens the same with subclusters, just at the subcluster
> level instead of course.
> 
> What can happen, if the subcluster size matches the filesystem block
> size, is that there's no need for any COW and therefore the bug is never
> triggered. But that's not quite the same as a fix :-)

No, what I meant was that the original problem that led to c8bb23cbdbe
would go away.

c8bb23cbdbe was added because small writes to new clusters are slow when
the clusters are large (because you need to do a 2 MB write on the host
for a 4 kB write from the guest).  So handle_alloc_space() was added to
alleviate the problem with a zero-write instead of actually writing zeroes.

The question is whether there is no need for handle_alloc_space() with
subclusters because a normal write with explicit zeroes being written in
the COW areas would be sufficiently quick.  (Because the subclusters for
2 MB clusters are just 64 kB in size.)

If that were so (right now it doesn’t look like it), we could revert
c8bb23cbdbe and wouldn’t see the bug anymore.

Max

>> Maybe make a decision based both on the ratio of data size to COW area
>> length (only invoke handle_alloc_space() under a certain threshold),
>> and the absolute COW area length (always invoke it above a certain
>> threshold, unless the ratio doesn’t allow it)?
> 
> Maybe combining that with the smaller clusters/subclusters can work
> around the problem. The maximum subcluster size is 64KB (for a 2MB
> cluster).
> 
> Berto
> 




signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH v2 13/26] qcow2: Add subcluster support to calculate_l2_meta()

[Max Reitz]

On 26.10.19 23:25, Alberto Garcia wrote:
> If an image has subclusters then there are more copy-on-write
> scenarios that we need to consider. Let's say we have a write request
> from the middle of subcluster #3 until the end of the cluster:
> 
>- If the cluster is new, then subclusters #0 to #3 from the old
>  cluster must be copied into the new one.

You mean for snapshots?

(That isn’t quite clear, and I only guess this based on the next bullet
point which differentiates based on “the old cluster was unallocated”.
That’s weird, too, because what does that mean, old cluster and new
cluster?  I suppose it’s abstract and it just means “There was no old
cluster and now we’ve allocated something”.  I can only understand the
concept of old and new clusters for COW inside of an image, i.e. for
snapshots and compressed clusters (theoretically).)

>- If the cluster is new but the old cluster was unallocated, then
>  only subcluster #3 needs copy-on-write. #0 to #2 are marked as
>  unallocated in the bitmap of the new L2 entry.
> 
>- If we are overwriting an old cluster and subcluster #3 is
>  unallocated or has the all-zeroes bit set then we need
>  copy-on-write on subcluster #3.
> 
>- If we are overwriting an old cluster and subcluster #3 was
>  allocated then there is no need to copy-on-write.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-cluster.c | 136 +-
>  1 file changed, 108 insertions(+), 28 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 1f509bda15..990bc070af 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1034,14 +1034,16 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
> QCowL2Meta *m)
>   * If @keep_old is true it means that the clusters were already
>   * allocated and will be overwritten. If false then the clusters are
>   * new and we have to decrease the reference count of the old ones.
> + *
> + * Returns 1 on success, -errno on failure.

I think there should be a note here on why this doesn’t follow the
general 0/-errno schema (i.e., “, because that is what callers generally
expect”).

>   */

[...]

> +if (!keep_old) {
> +switch (type) {
> +case QCOW2_CLUSTER_NORMAL:
> +case QCOW2_CLUSTER_COMPRESSED:
> +case QCOW2_CLUSTER_ZERO_ALLOC:
> +case QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER:
> +cow_start_from = 0;

Somehow (I don’t know why) I find this a bit tough to understand.

Wouldn’t it work to let cow_start start from the first subcluster for
ZERO_ALLOC and UNALLOCATED_SUBCLUSTER?  We don’t need to COW those, it
should be sufficient to just make the subclusters before that zero or
unallocated, respectively.

(Same for cow_end)

Max

> +break;
> +case QCOW2_CLUSTER_ZERO_PLAIN:
> +case QCOW2_CLUSTER_UNALLOCATED:
> +cow_start_from = sc_index << s->subcluster_bits;
> +break;
> +default:
> +g_assert_not_reached();
> +}



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] qom: Fix error message in object_class_property_add()

[Laurent Vivier]

Le 04/11/2019 à 14:23, Greg Kurz a écrit :
> The error message in object_class_property_add() was copied from
> object_property_add() in commit 16bf7f522a2ff. Clarify that it is
> about a class, not an object.
> 
> While here, have the format string in both functions to fit in a
> single line for better grep-ability, despite the checkpatch warning.
> 
> Signed-off-by: Greg Kurz 
> ---
>  qom/object.c |   10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 6fa9c619fac4..d51b57fba11e 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1106,9 +1106,8 @@ object_property_add(Object *obj, const char *name, 
> const char *type,
>  }
>  
>  if (object_property_find(obj, name, NULL) != NULL) {
> -error_setg(errp, "attempt to add duplicate property '%s'"
> -   " to object (type '%s')", name,
> -   object_get_typename(obj));
> +error_setg(errp, "attempt to add duplicate property '%s' to object 
> (type '%s')",
> +   name, object_get_typename(obj));
>  return NULL;
>  }
>  
> @@ -1139,9 +1138,8 @@ object_class_property_add(ObjectClass *klass,
>  ObjectProperty *prop;
>  
>  if (object_class_property_find(klass, name, NULL) != NULL) {
> -error_setg(errp, "attempt to add duplicate property '%s'"
> -   " to object (type '%s')", name,
> -   object_class_get_name(klass));
> +error_setg(errp, "attempt to add duplicate property '%s' to class 
> (type '%s')",
> +   name, object_class_get_name(klass));
>  return NULL;
>  }
>  
> 
> 

Reviewed-by: Laurent Vivier 

Re: [RFC 0/3] block/file-posix: Work around XFS bug

[Alberto Garcia]

On Fri 25 Oct 2019 04:19:30 PM CEST, Max Reitz wrote:
>>> So, it's obvious that c8bb23cbdbe32f5c326 is significant for 1M
>>> cluster-size, even on rotational disk, which means that previous
>>> assumption about calling handle_alloc_space() only for ssd is wrong,
>>> we need smarter heuristics..
>>>
>>> So, I'd prefer (1) or (2).
>
> OK.  I wonder whether that problem would go away with Berto’s subcluster
> series, though.

Catching up with this now. I was told about this last week at the KVM
Forum, but if the problems comes with the use of fallocate() and XFS,
the I don't think subclusters will solve it.

handle_alloc_space() is used to fill a cluster with zeroes when there's
COW, and that happens the same with subclusters, just at the subcluster
level instead of course.

What can happen, if the subcluster size matches the filesystem block
size, is that there's no need for any COW and therefore the bug is never
triggered. But that's not quite the same as a fix :-)

> Maybe make a decision based both on the ratio of data size to COW area
> length (only invoke handle_alloc_space() under a certain threshold),
> and the absolute COW area length (always invoke it above a certain
> threshold, unless the ratio doesn’t allow it)?

Maybe combining that with the smaller clusters/subclusters can work
around the problem. The maximum subcluster size is 64KB (for a 2MB
cluster).

Berto

[PATCH] pl031: Expose RTCICR as proper WC register

[Alexander Graf]

The current pl031 RTCICR register implementation always clears the IRQ
pending status on a register write, regardless of the value it writes.

To justify that behavior, it references the arm926e documentation
(DDI0287B) and indicates that said document states that any write clears
the internal IRQ state. I could however not find any text in that document
backing the statement. In fact, it explicitly says:

  "Writing 1 to bit 0 of RTCICR clears the RTCINTR flag."

which describes it as much as a write-to-clear register as the PL031 spec
(DDI0224) does:

  "Writing 1 to bit position 0 clears the corresponding interrupt.
   Writing 0 has no effect."

Let's remove the bogus comment and instead follow both specs to what they
say.

Reported-by: Hendrik Borghorst 
Signed-off-by: Alexander Graf 
---
 hw/rtc/pl031.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c
index 3a982752a2..c57cf83165 100644
--- a/hw/rtc/pl031.c
+++ b/hw/rtc/pl031.c
@@ -149,11 +149,7 @@ static void pl031_write(void * opaque, hwaddr offset,
 pl031_update(s);
 break;
 case RTC_ICR:
-/* The PL031 documentation (DDI0224B) states that the interrupt is
-   cleared when bit 0 of the written value is set.  However the
-   arm926e documentation (DDI0287B) states that the interrupt is
-   cleared when any value is written.  */
-s->is = 0;
+s->is &= ~value;
 pl031_update(s);
 break;
 case RTC_CR:
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

[PATCH] qom: Fix error message in object_class_property_add()

[Greg Kurz]

The error message in object_class_property_add() was copied from
object_property_add() in commit 16bf7f522a2ff. Clarify that it is
about a class, not an object.

While here, have the format string in both functions to fit in a
single line for better grep-ability, despite the checkpatch warning.

Signed-off-by: Greg Kurz 
---
 qom/object.c |   10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 6fa9c619fac4..d51b57fba11e 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1106,9 +1106,8 @@ object_property_add(Object *obj, const char *name, const 
char *type,
 }
 
 if (object_property_find(obj, name, NULL) != NULL) {
-error_setg(errp, "attempt to add duplicate property '%s'"
-   " to object (type '%s')", name,
-   object_get_typename(obj));
+error_setg(errp, "attempt to add duplicate property '%s' to object 
(type '%s')",
+   name, object_get_typename(obj));
 return NULL;
 }
 
@@ -1139,9 +1138,8 @@ object_class_property_add(ObjectClass *klass,
 ObjectProperty *prop;
 
 if (object_class_property_find(klass, name, NULL) != NULL) {
-error_setg(errp, "attempt to add duplicate property '%s'"
-   " to object (type '%s')", name,
-   object_class_get_name(klass));
+error_setg(errp, "attempt to add duplicate property '%s' to class 
(type '%s')",
+   name, object_class_get_name(klass));
 return NULL;
 }
 

[PATCH] tcg plugins: expose an API version concept

[Alex Bennée]

This is a very simple versioning API which allows the plugin
infrastructure to check the API a plugin was built against. We also
expose a min/cur API version to the plugin via the info block in case
it wants to avoid using old deprecated APIs in the future.

Signed-off-by: Alex Bennée 
---
 include/qemu/qemu-plugin.h | 19 +++
 plugins/loader.c   | 15 +++
 plugins/plugin.h   |  2 ++
 tests/plugin/bb.c  |  2 ++
 tests/plugin/empty.c   |  2 ++
 tests/plugin/hotpages.c|  2 ++
 tests/plugin/howvec.c  |  2 ++
 tests/plugin/insn.c|  2 ++
 tests/plugin/mem.c |  2 ++
 9 files changed, 48 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index a00a7deb461..5502e112c81 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -38,9 +38,28 @@
 
 typedef uint64_t qemu_plugin_id_t;
 
+/*
+ * Versioning plugins:
+ *
+ * The plugin API will pass a minimum and current API version that
+ * QEMU currently supports. The minimum API will be incremented if an
+ * API needs to be deprecated.
+ *
+ * The plugins export the API they were built against by exposing the
+ * symbol qemu_plugin_version which can be checked.
+ */
+
+extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
+
+#define QEMU_PLUGIN_VERSION 0
+
 typedef struct {
 /* string describing architecture */
 const char *target_name;
+struct {
+int min;
+int cur;
+} version;
 /* is this a full system emulation? */
 bool system_emulation;
 union {
diff --git a/plugins/loader.c b/plugins/loader.c
index ce724ed5839..1bcca909691 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -178,6 +178,19 @@ static int plugin_load(struct qemu_plugin_desc *desc, 
const qemu_info_t *info)
 goto err_symbol;
 }
 
+if (!g_module_symbol(ctx->handle, "qemu_plugin_version", )) {
+warn_report("%s: missing version %s", __func__, g_module_error());
+} else {
+int version = *(int *)sym;
+if (version < QEMU_PLUGIN_MIN_VERSION ||
+version > QEMU_PLUGIN_VERSION) {
+error_report("%s: bad plugin version %d vs %d/%d",
+ __func__, version, QEMU_PLUGIN_MIN_VERSION,
+ QEMU_PLUGIN_VERSION);
+goto err_symbol;
+}
+}
+
 qemu_rec_mutex_lock();
 
 /* find an unused random id with  as the seed */
@@ -248,6 +261,8 @@ int qemu_plugin_load_list(QemuPluginList *head)
 g_autofree qemu_info_t *info = g_new0(qemu_info_t, 1);
 
 info->target_name = TARGET_NAME;
+info->version.min = QEMU_PLUGIN_MIN_VERSION;
+info->version.cur = QEMU_PLUGIN_VERSION;
 #ifndef CONFIG_USER_ONLY
 MachineState *ms = MACHINE(qdev_get_machine());
 info->system_emulation = true;
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 5482168d797..1aa29dcaddf 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -14,6 +14,8 @@
 
 #include 
 
+#define QEMU_PLUGIN_MIN_VERSION 0
+
 /* global state */
 struct qemu_plugin_state {
 QTAILQ_HEAD(, qemu_plugin_ctx) ctxs;
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index 45e1de5bd68..f30bea08dcc 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -14,6 +14,8 @@
 
 #include 
 
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
 static uint64_t bb_count;
 static uint64_t insn_count;
 static bool do_inline;
diff --git a/tests/plugin/empty.c b/tests/plugin/empty.c
index 3f60f690278..8fa6bacd93d 100644
--- a/tests/plugin/empty.c
+++ b/tests/plugin/empty.c
@@ -13,6 +13,8 @@
 
 #include 
 
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
 /*
  * Empty TB translation callback.
  * This allows us to measure the overhead of injecting and then
diff --git a/tests/plugin/hotpages.c b/tests/plugin/hotpages.c
index 77df07a3ccf..ecd6c187327 100644
--- a/tests/plugin/hotpages.c
+++ b/tests/plugin/hotpages.c
@@ -18,6 +18,8 @@
 
 #include 
 
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 static uint64_t page_size = 4096;
diff --git a/tests/plugin/howvec.c b/tests/plugin/howvec.c
index 58fa675e348..4ca555e1239 100644
--- a/tests/plugin/howvec.c
+++ b/tests/plugin/howvec.c
@@ -20,6 +20,8 @@
 
 #include 
 
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 typedef enum {
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index e5fd07fb64b..0a8f5ae 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -14,6 +14,8 @@
 
 #include 
 
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
 static uint64_t insn_count;
 static bool do_inline;
 
diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index d9673889896..878abf09d19 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -14,6 +14,8 @@
 
 #include 
 
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = 

[Bug 1851095] Re: [feature request] awareness of instructions that are well emulated

[Laurent Desnogues]

Can you please provide a binary (preferably statically built or with
required shared libraries attached)?

Thanks,

Laurent

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

Title:
  [feature request] awareness of instructions that are well emulated

Status in QEMU:
  New

Bug description:
  While qemu's scalar emulation tends to be excellent, qemu's SIMD
  emulation tends to be incorrect (except for arm64 from x86_64)--i have
  found this both for mipsel and arm32. Until these code paths are
  audited, which is probably a large job, it would be nice if qemu knew
  its emulation of this class of instructions was not very good, and
  thus it would give up on finding these instructions if a "careful"
  operation is passed.

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

Re: [RFC PATCH v2 12/26] qcow2: Handle QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER

[Max Reitz]

On 04.11.19 14:03, Alberto Garcia wrote:
> On Mon 04 Nov 2019 01:57:42 PM CET, Max Reitz wrote:
>> On 26.10.19 23:25, Alberto Garcia wrote:
>>> In the previous patch we added a new QCow2ClusterType named
>>> QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER. There is a couple of places
>>> where this new value needs to be handled, and that is what this patch
>>> does.
>>>
>>> Signed-off-by: Alberto Garcia 
>>> ---
>>>  block/qcow2.c | 13 +
>>>  1 file changed, 9 insertions(+), 4 deletions(-)
>> This patch deals with everything in qcow2.c.  There are more places that
>> reference QCOW2_CLUSTER_* constants elsewhere, and I suppose most of
>> them are handled by the following patches.
>>
>> But I wonder what the criterion is on where it needs to be handled and
>> where it’s OK not to.  Right now it looks to me like it’s a bit
>> arbitrary maybe?  But I suppose I’ll just have to wait until after the
>> next patches.
> 
> This is the part of the series that I'm the least happy about, because
> the existing qcow2_get_cluster_type() can never return this new value, I
> only updated the cases where this can actually happen.
> 
> I'm still considering a different approach for this.
I still don’t know what you’re doing in the later patches, but to me it
looks a bit like you don’t dare breaking up the existing structure that
just deals with clusters.

If that is so, I think it will help to make a clear cut between what
concerns subclusters and what concerns clusters at a whole.
QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER shouldn’t be in QCow2ClusterType;
there should be a separate QCow2SubclusterType.

OTOH, that would require more modifications, but (naïvely) I believe
that would make for the cleaner interface in the end.

Max



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Makefile: install bios-microvm like other binary blobs

[Sergio Lopez]

Philippe Mathieu-Daudé  writes:

> From: Bruce Rogers 
>
> Commit 0d5fae3e52e introduced bios-microvm.bin but forgot to add
> it to the list of blobs being installed.
> Add it to the list of BLOBS that get installed.
>
> Fixes: 0d5fae3e52e "roms: add microvm-bios (qboot) as binary"
> Signed-off-by: Bruce Rogers 
> [PMD: Reworded description]
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index bd6376d295..755aa6c5f5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -770,7 +770,7 @@ de-ch  es fo  fr-ca  hu ja  mk  pt  sl tr \
>  bepocz
>  
>  ifdef INSTALL_BLOBS
> -BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
> +BLOBS=bios.bin bios-256k.bin bios-microvm.bin sgabios.bin vgabios.bin 
> vgabios-cirrus.bin \
>  vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
>  vgabios-ramfb.bin vgabios-bochs-display.bin vgabios-ati.bin \
>  ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin 
> QEMU,cgthree.bin \

Thanks.

Reviewed-by: Sergio Lopez 

Re: [RFC PATCH v2 12/26] qcow2: Handle QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER

[Alberto Garcia]

On Mon 04 Nov 2019 01:57:42 PM CET, Max Reitz wrote:
> On 26.10.19 23:25, Alberto Garcia wrote:
>> In the previous patch we added a new QCow2ClusterType named
>> QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER. There is a couple of places
>> where this new value needs to be handled, and that is what this patch
>> does.
>> 
>> Signed-off-by: Alberto Garcia 
>> ---
>>  block/qcow2.c | 13 +
>>  1 file changed, 9 insertions(+), 4 deletions(-)
> This patch deals with everything in qcow2.c.  There are more places that
> reference QCOW2_CLUSTER_* constants elsewhere, and I suppose most of
> them are handled by the following patches.
>
> But I wonder what the criterion is on where it needs to be handled and
> where it’s OK not to.  Right now it looks to me like it’s a bit
> arbitrary maybe?  But I suppose I’ll just have to wait until after the
> next patches.

This is the part of the series that I'm the least happy about, because
the existing qcow2_get_cluster_type() can never return this new value, I
only updated the cases where this can actually happen.

I'm still considering a different approach for this.

Berto