Re: [Bug 1881004] [NEW] fpu/softfloat.c: error: bitwise negation of a boolean expression
On 27/05/2020 23.54, Eric Blake wrote:
> On 5/27/20 4:40 PM, Peter Maydell wrote:
>> On Wed, 27 May 2020 at 20:21, Philippe Mathieu-Daudé
>> <1881...@bugs.launchpad.net> wrote:
>>>
>>> Public bug reported:
>>>
>>> Last time I built QEMU was on commit
>>> d5c75ec500d96f1d93447f990cd5a4ef5ba27fae,
>>> I just pulled to fea8f3ed739536fca027cf56af7f5576f37ef9cd and now get:
>>>
>>> CC lm32-softmmu/fpu/softfloat.o
>>> fpu/softfloat.c:3365:13: error: bitwise negation of a boolean
>>> expression; did you mean logical negation? [-Werror,-Wbool-operation]
>>> absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
>>> ^~
>>> !
>>
>>
>> "(x & y)" is not a boolean expression, so we should report this to clang
>> as a bug (I assume what they actually are trying to complain about is
>> bitwise AND with a boolean expression).
>
> We have:
>
> uint64_t &= ~ ( ( ( int8_t ^ int ) == int ) & bool )
>
> which is
>
> uint64_t &= ~ ( bool & bool )
>
> which is then
>
> uint64_t &= ~ ( int )
>
> resulting in one of:
>
> uint64_t &= 0x
> uint64_t &= 0xfffe
>
> It is a very odd way of stating that 'if this condition is true, mask
> out the least-significant-bit'. In general, 'bool & bool' is used where
> the side-effect-skipping 'bool && bool' is inappropriate; I'm a bit
> surprised that clang is not questioning whether we meant '&&' instead of
> '&' (the two operators give the same effect in this case).
>
> You are right that clang is fishy for calling it logical negation of a
> bool, when it is really logical negation of an int, but we are also
> fishy in that we are using bitwise AND of two bools as an int in the
> first place.
>
> Regardless of whether clang changes, would it be better to write the
> code as:
>
> if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
> absZ &= ~1;
> }
I agree, that's also much better to read.
And FWIW, WinUAE fixed a similar problem in the same way recently:
https://github.com/tonioni/WinUAE/commit/51f5e7bfc39cf37daf7283
So I think this is the right way to go. Could you send your suggestion
as a proper patch?
Thanks,
Thomas
Re: [PATCH v6 1/3] memory: drop guest writes to read-only ram device regions
On Thu, May 28, 2020 at 07:10:46AM +0200, Paolo Bonzini wrote: > On 28/05/20 06:35, Yan Zhao wrote: > > On Tue, May 26, 2020 at 10:26:35AM +0100, Peter Maydell wrote: > >> On Mon, 25 May 2020 at 11:20, Paolo Bonzini wrote: > >>> Not all of them, only those that need to return MEMTX_ERROR. I would > >>> like some guidance from Peter as to whether (or when) reads from ROMs > >>> should return MEMTX_ERROR. This way, we can use that information to > >>> device what the read-only ram-device regions should do. > >> > >> In general I think writes to ROMs (and indeed reads from ROMs) should > >> not return MEMTX_ERROR. I think that in real hardware you could have > >> a ROM that behaved either way; so our default behaviour should probably > >> be to do what we've always done and not report a MEMTX_ERROR. (If we > >> needed to I suppose we should implement a MEMTX_ERROR-reporting ROM, > >> but to be honest there aren't really many real ROMs in systems these > >> days: it's more often flash, whose response to writes is defined > >> by the spec and is I think to ignore writes which aren't the > >> magic "shift to program-the-flash-mode" sequence.) > >> > > then should I just drop the writes to read-only ram-device regions and > > vfio regions without returning MEMTX_ERROR? > > do you think it's good? > > I am not really sure, I have to think more about it. I think read-only > RAMD regions are slightly different because the guest can expect "magic" > behavior from RAMD regions (e.g. registers that trigger I/O on writes) > that are simply not there for ROM. So I'm still inclined to queue your > v6 patch series. > ok. thank you Paolo. :)
Re: [PATCH v5 0/7] coroutines: generate wrapper code
28.05.2020 00:57, Eric Blake wrote: On 5/27/20 4:46 PM, no-re...@patchew.org wrote: Patchew URL: https://patchew.org/QEMU/20200527203733.16129-1-vsement...@virtuozzo.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 === TypeError: __init__() got an unexpected keyword argument 'capture_output' CC /tmp/qemu-test/build/slirp/src/bootp.o GEN ui/input-keymap-usb-to-qcode.c make: *** [block/block-gen.c] Error 1 make: *** Deleting file `block/block-gen.c' make: *** Waiting for unfinished jobs GEN ui/input-keymap-win32-to-qcode.c The more interesting part of the failure: File "/tmp/qemu-test/src/scripts/coroutine-wrapper.py", line 173, in print(gen_wrappers_file(sys.stdin.read())) File "/tmp/qemu-test/src/scripts/coroutine-wrapper.py", line 169, in gen_wrappers_file return prettify(res) # prettify to wrap long lines File "/tmp/qemu-test/src/scripts/coroutine-wrapper.py", line 40, in prettify encoding='utf-8', input=code, capture_output=True) File "/usr/lib64/python3.6/subprocess.py", line 423, in run with Popen(*popenargs, **kwargs) as process: TypeError: __init__() got an unexpected keyword argument 'capture_output' which indeed looks like a bug in the patch. Ah, yes, capture_output is since python 3.7. So, s/capture_output=True/stdout=subprocess.PIPE/ . -- Best regards, Vladimir
Re: USB pass-through problems
On Wed, May 27, 2020 at 09:44:54PM +0200, BALATON Zoltan wrote: > Hello, > > I've seen a case when QEMU hangs with a passed through USB device. This is > with -device usb-ehci and pass through with usb-host. This works until the > attached USB device reboots (so likely it disconnects and reconnects) at > which point QEMU hangs and need to be SIGKILL-ed to end (that's a bit hard > to do with mouse and keyboard grabbed). I've got this stack trace: > > #0 0x7f23e7bd4949 in poll () at /lib64/libc.so.6 > #1 0x7f23e8bfa9a5 in () at /lib64/libusb-1.0.so.0 > #2 0x7f23e8bfbb13 in libusb_handle_events_timeout_completed () at > /lib64/libusb-1.0.so.0 > #3 0x55e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at > hw/usb/host-libusb.c:963 > #4 0x55e09854b87a in usb_host_close (s=0x55e09b036dd0) at > hw/usb/host-libusb.c:977 > #5 0x55e09854b92e in usb_host_nodev_bh (opaque=0x55e09b036dd0) at > hw/usb/host-libusb.c:998 > #6 0x55e098757500 in aio_bh_call (bh=0x55e099ad9cc0) at util/async.c:136 > #7 0x55e09875760a in aio_bh_poll (ctx=0x55e0996c2620) at util/async.c:164 > #8 0x55e09875cb2a in aio_dispatch (ctx=0x55e0996c2620) at > util/aio-posix.c:380 > #9 0x55e098757a3d in aio_ctx_dispatch (source=0x55e0996c2620, > callback=0x0, user_data=0x0) at util/async.c:306 > #10 0x7f23e8c59665 in g_main_context_dispatch () at > /lib64/libglib-2.0.so.0 > #11 0x55e09875b0a9 in glib_pollfds_poll () at util/main-loop.c:219 > #12 0x55e09875b123 in os_host_main_loop_wait (timeout=0) at > util/main-loop.c:242 > #13 0x55e09875b228 in main_loop_wait (nonblocking=0) at > util/main-loop.c:518 > #14 0x55e0982c91f8 in qemu_main_loop () at softmmu/vl.c:1664 > #15 0x55e098162e7e in main (argc=, argv=, > envp=) at softmmu/main.c:49 > > so the problem may be in libusb but QEMU should not hang completely. The > host is Linux with libusb 1.0.22. Hmm, does reverting 76d0a9362c6a6a7d88aa18c84c4186c9107ecaef change behavior? cheers, Gerd
Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS
On 27/05/2020 18.36, Alex Bennée wrote:
>
> Thomas Huth writes:
>
>> On 27/05/2020 16.44, Laurent Vivier wrote:
>>> Le 25/05/2020 à 15:18, Thomas Huth a écrit :
From: Alex Bennée
Newer clangs rightly spot that you can never exceed the full address
space of 64 bit hosts with:
linux-user/elfload.c:2076:41: error: result of comparison 'unsigned
long' > 18446744073709551615 is always false
[-Werror,-Wtautological-type-limit-compare]
4685 if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
4686 ~~~ ^ ~
4687 1 error generated.
So lets limit the check to 32 bit hosts only.
Fixes: ee94743034bf
Reported-by: Thomas Huth
Signed-off-by: Alex Bennée
[thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS ==
32]
Signed-off-by: Thomas Huth
---
linux-user/elfload.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 01a9323a63..ebc663ea0b 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char
*image_name, abi_ulong guest_loaddr,
exit(EXIT_FAILURE);
}
} else {
+#if HOST_LONG_BITS < TARGET_ABI_BITS
if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
error_report("%s: requires more virtual address space "
"than the host can provide (0x%" PRIx64 ")",
image_name, (uint64_t)guest_hiaddr - guest_base);
exit(EXIT_FAILURE);
}
+#endif
}
/*
>>>
>>> Philippe sent the same patch:
>>>
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html
>>
>> Indeed, but looking more closely, he's using slightly different
>> locations for the #if and #endif ... not sure what's better though...?
>
> Richard was more inclined to suppress the warning:
>
> Subject: Re: [PATCH v2] linux-user: limit check to HOST_LONG_BITS == 32
> From: Richard Henderson
> Message-ID: <3069bc1b-115d-f361-8271-c775bf695...@linaro.org>
> Date: Thu, 21 May 2020 20:15:51 -0700
>
> One reason I dropped the f32 patch from my last PR was because this
> wasn't the only warning the latest clang picks up.
... but this is currently the only spot that is required to get the
gitlab CI going again, so I think we should include this patch until we
have a final decision whether to disable the warning or not (and we can
still revert this patch after we disabled the warning). Ok?
Thomas
Re: [PATCH v2 0/3] account for NVDIMM nodes during SRAT generation
Patchew URL: https://patchew.org/QEMU/20200528054807.21278-1-vishal.l.ve...@intel.com/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200528054807.21278-1-vishal.l.ve...@intel.com Subject: [PATCH v2 0/3] account for NVDIMM nodes during SRAT generation Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Switched to a new branch 'test' 92f6e3c tests/acpi: update expected SRAT files 325a20a hw/acpi-build: account for NVDIMM numa nodes in SRAT ff8f589 diffs-allowed: add the SRAT AML to diffs-allowed === OUTPUT BEGIN === 1/3 Checking commit ff8f5897d948 (diffs-allowed: add the SRAT AML to diffs-allowed) 2/3 Checking commit 325a20aae003 (hw/acpi-build: account for NVDIMM numa nodes in SRAT) 3/3 Checking commit 92f6e3cdac4c (tests/acpi: update expected SRAT files) ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/SRAT.dimmpxm and tests/qtest/bios-tables-test-allowed-diff.h found ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/data/acpi/q35/SRAT.dimmpxm and tests/qtest/bios-tables-test-allowed-diff.h found total: 2 errors, 0 warnings, 1 lines checked Patch 3/3 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/20200528054807.21278-1-vishal.l.ve...@intel.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Bug 1877706] Re: [Feature request] qemu does not support for Octeon MIPS64 on X86
** Changed in: qemu Importance: Undecided => Wishlist -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1877706 Title: [Feature request] qemu does not support for Octeon MIPS64 on X86 Status in QEMU: New Bug description: Description of problem: I use mips64-octeon-linux-gnu-gcc cross toolchain on X86,and generate binary file. > mips64-octeon-linux-gnu-gcc hello.c -static > file a.out > a.out: ELF 32-bit MSB executable, MIPS, N32 MIPS64 rel2 version 1 (SYSV), statically linked, for GNU/Linux 2.4.0, not stripped I execute it with mips64-linux-user mode in qemu, it is invalid. > ./qemu-5.0.0/mips64-linux-user/qemu-mips64 a.out > a.out: Invalid ELF image for this architecture when I choose mips-linux-user mode, it regards as illegal instruction. > ./qemu-5.0.0/mips-linux-user/qemu-mips a.out > qemu: uncaught target signal 4 (Illegal instruction) - core dumped > Illegal instruction (core dumped) I would like to know, is this due to my problem or does qemu not support Octeon MIPS64 on X86? if qemu has supported Octeon MIPS64 on X86, how can I emulate it. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1877706/+subscriptions
Re: [PATCH v3 14/22] microvm: use 2G split unconditionally
Hi, > > (1) we loose a bit of memory. > it's probably not a big enough to care about, we do similar ovarlay > mapping on pc/q35 > at the beginning of RAM Yes, shouldn't be too much. > > (2) we loose a gigabyte page. > I'm not sure waht exactly we loose in this case. The 1G page for 0xc000 -> 0x (as explained by paolo). > > (3) we wouldn't have guard pages (unused address space) between > > between ram and mmio space. >if it's holes' mmio,then do we really need them (access is going > to be terminated >either in always valid RAM or in valid mmio hole)? Not required, but more robust. Less likely that the guest touches mmio by accident. I'd expect it also requires some e820 hacks. cheers, Gerd
Re: [PATCH v3] arm/aspeed: Rework NIC attachment
Cédric Le Goater writes: > On 5/27/20 3:36 PM, Markus Armbruster wrote: >> Cédric Le Goater writes: >> >>> The number of MACs supported by an Aspeed SoC is defined by "macs_num" >>> under the SoC model, that is two for the AST2400 and AST2500 and four >>> for the AST2600. The model initializes the maximum number of supported >>> MACs but the number of realized devices is capped by the number of >>> network device back-ends defined on the command line. This can leave >>> unrealized devices hanging around in the QOM composition tree. >>> >>> Modify the machine initialization to define which MACs are attached to >>> a network device back-end using a bit-field property "macs-mask" and >>> let the SoC realize all network devices. >>> >>> The default setting of "macs-mask" is "use MAC0" only, which works for >>> all our AST2400 and AST2500 machines. The AST2600 machines have >>> different configurations. The AST2600 EVB machine activates MAC1, MAC2 >>> and MAC3 and the Tacoma BMC machine activates MAC2. >> >> Let's be more clear on what this means, and "This is actually a device >> modelling fix for these two machines." Okay? > > Well, I guess so. It's a fix in the way we attach network backends to > the MACs of the machines. Yes, it's that too. > On the tacoma-bmc, we had to use '-nic -nic -nic ' > to configure the MAC2 in use by the machine. Which was dubious. I think you had to use something like -nic none -nic none -nic GOOD-ONE -nic none to get virtual hardware that matches the physical hardware, which I understand has all four MACs on the die, but only MAC2 connected to the outside. In particular, the default configuration (no -nic, -nodefaults, etc.) got you only MAC0. With just -nodefaults, you got none at all. > Now, a single -nic is enough. Now you get all four MACs regardless of configuration, but only MAC2 can be connected to a backend, e.g. with a single -nic. The default configuration (no -nic, -nodefaults, etc.) just works: MAC2 connected to the default network backend. With just -nodefaults, MAC2 remains unconnected. This matches how other machines work. >>> Inactive MACs will have no peer and QEMU may warn the user with : >>> >>> qemu-system-arm: warning: nic ftgmac100.0 has no peer >>> qemu-system-arm: warning: nic ftgmac100.1 has no peer >>> qemu-system-arm: warning: nic ftgmac100.3 has no peer >>> >>> Signed-off-by: Cédric Le Goater >>> Reviewed-by: Markus Armbruster >> >> Here's the "info qom-tree" change for tacoma-bmc: >> >> /machine (tacoma-bmc-machine) >>/peripheral (container) >>/peripheral-anon (container) >>/soc (ast2600-a1) >> [...] >> /ftgmac100[0] (ftgmac100) >>/ftgmac100[0] (qemu:memory-region) >> /ftgmac100[1] (ftgmac100) >> + /ftgmac100[0] (qemu:memory-region) >> /ftgmac100[2] (ftgmac100) >> + /ftgmac100[0] (qemu:memory-region) >> /ftgmac100[3] (ftgmac100) >> + /ftgmac100[0] (qemu:memory-region) > > Yes. All are realized now. > >> [...] >> /mii[0] (aspeed-mmi) >>/aspeed-mmi[0] (qemu:memory-region) >> /mii[1] (aspeed-mmi) >> + /aspeed-mmi[0] (qemu:memory-region) >> /mii[2] (aspeed-mmi) >> + /aspeed-mmi[0] (qemu:memory-region) >> /mii[3] (aspeed-mmi) >> + /aspeed-mmi[0] (qemu:memory-region) > > Same for the MMI interfaces on AST2600. > >> These changes are due to realizing MAC1, MAC2, MAC3. Looks good. >> >> Here's "info qtree": >> >>dev: ftgmac100, id "" >> gpio-out "sysbus-irq" 1 >> aspeed = true >> -mac = "52:54:00:12:34:56" >> -netdev = "hub0port0" >> +mac = "52:54:00:12:34:57" >> +netdev = "" >> mmio 1e66/2000 >>dev: ftgmac100, id "" >> -aspeed = false >> -mac = "00:00:00:00:00:00" >> +gpio-out "sysbus-irq" 1 >> +aspeed = true >> +mac = "52:54:00:12:34:58" >> netdev = "" >> +mmio 1e68/2000 >>dev: ftgmac100, id "" >> -aspeed = false >> -mac = "00:00:00:00:00:00" >> -netdev = "" >> +gpio-out "sysbus-irq" 1 >> +aspeed = true >> +mac = "52:54:00:12:34:56" >> +netdev = "hub0port0" >> +mmio 1e67/2000 >>dev: ftgmac100, id "" >> -aspeed = false >> -mac = "00:00:00:00:00:00" >> +gpio-out "sysbus-irq" 1 >> +aspeed = true >> +mac = "52:54:00:12:34:59" >> netdev = "" >> +mmio 1e69/2000 >>[...] >>dev: aspeed-mmi, id "" >> mmio 1e65/0008 >>dev: aspeed-mmi, id "" >> +mmio 1e650008/0008 >>dev: aspeed-mmi, id "" >> +mmio 1e650010/0008 >>dev: aspeed-mmi, id "" >> +
Re: [PATCH v2 04/11] tests/acceptance: add kernel record/replay test for x86_64
On 27.05.2020 17:53, Philippe Mathieu-Daudé wrote:
On 5/27/20 12:31 PM, Pavel Dovgalyuk wrote:
This patch adds a test for record/replay an execution of x86_64 machine.
Execution scenario includes simple kernel boot, which allows testing
basic hardware interaction in RR mode.
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/tests/acceptance/replay_kernel.py
b/tests/acceptance/replay_kernel.py
index b8b277ad2f..c7526f1aba 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -55,3 +55,19 @@ class ReplayKernel(LinuxKernelUtils):
True, shift, args)
self.run_vm(kernel_path, kernel_command_line, console_pattern,
False, shift, args)
+
+def test_x86_64_pc(self):
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:pc
+"""
+kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+ '/linux/releases/29/Everything/x86_64/os/images/pxeboot'
+ '/vmlinuz')
+kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
+console_pattern = 'Kernel command line: %s' % kernel_command_line
+
+self.run_rr(kernel_path, kernel_command_line, console_pattern)
This one timeouted (I build with --enable-debug):
I've got the strange behavior for the couple of times.
Console output was correct (I saw 'Kernel command line' in logs), but
_console_interation function didn't notice it.
Therefore the test finished with timeout.
How this could be possible?
(1/1) tests/acceptance/replay_kernel.py:ReplayKernel.test_x86_64_pc:
replay: recording...
replay: replaying...
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'1-tests/acceptance/replay_kernel.py:ReplayKernel.test_x86_64_pc',
'logdir':
'avocado/job-results/job-2020-05-27T16.48-71d7bf4/test-results/1-tes...
(90.68 s)
Re: [PATCH v2 0/7] Misc display/sm501 clean ups and fixes
On Thu, May 21, 2020 at 09:39:44PM +0200, BALATON Zoltan wrote: > Second try of clean ups and changes to hopefully improve 2D engine > performance and fix a security bug in it. This one actually works with > AmigaOS handling overlapping blits, fixes the overflow checks and adds > Reviewed-by tags. Unless some problems are found this should be OK to > merge. Added to vga queue. thanks, Gerd
RE: [PATCH v6 2/5] block: consolidate blocksize properties consistency checks
> -Original Message- > From: Roman Kagan > Sent: 27 May 2020 13:45 > To: qemu-devel@nongnu.org > Cc: Kevin Wolf ; xen-de...@lists.xenproject.org; Gerd > Hoffmann ; > Daniel P. Berrangé ; Paolo Bonzini > ; Anthony Perard > ; Laurent Vivier ; Max Reitz > ; qemu- > bl...@nongnu.org; Philippe Mathieu-Daudé ; Eric Blake > ; Paul > Durrant ; Fam Zheng ; John Snow > ; Michael S. Tsirkin > ; Eduardo Habkost ; Keith Busch > ; Stefano > Stabellini ; Stefan Hajnoczi > Subject: [PATCH v6 2/5] block: consolidate blocksize properties consistency > checks > > Several block device properties related to blocksize configuration must > be in certain relationship WRT each other: physical block must be no > smaller than logical block; min_io_size, opt_io_size, and > discard_granularity must be a multiple of a logical block. > > To ensure these requirements are met, add corresponding consistency > checks to blkconf_blocksizes, adjusting its signature to communicate > possible error to the caller. Also remove the now redundant consistency > checks from the specific devices. > > Signed-off-by: Roman Kagan Reviewed-by: Paul Durrant
Re: [PATCH v2 11/11] tests/acceptance: Linux boot test for record/replay
On 27.05.2020 19:44, Alex Bennée wrote:
Pavel Dovgalyuk writes:
This patch adds a test for record/replay, which boots Linux
image from the disk and interacts with the network.
The idea and code of this test is borrowed from boot_linux.py
However, currently record/replay works only for x86_64,
therefore other tests were excluded.
Each test consists of the following phases:
- downloading the disk image
- recording the execution
- replaying the execution
Replay does not validates the output, but waits until QEMU
finishes the execution. This is reasonable, because
QEMU usually hangs when replay goes wrong.
Two things:
- We need to tag these tests as slow so they aren't run by default
Which flag is responsible for this?
- 1800s is a long timeout to wait for to know it's a problem
Right, I just doubled boot_linux timeout. I think, that it could be reduced.
Looking at the log shows my test is still running? Maybe we can check
the output as we go?
How this could look like?
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/MAINTAINERS b/MAINTAINERS
index e9a9ce4f66..97f066a9b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2498,6 +2498,7 @@ F: include/sysemu/replay.h
F: docs/replay.txt
F: stubs/replay.c
F: tests/acceptance/replay_kernel.py
+F: tests/acceptance/replay_linux.py
IOVA Tree
M: Peter Xu
diff --git a/tests/acceptance/replay_linux.py b/tests/acceptance/replay_linux.py
new file mode 100644
index 00..7c5971f156
--- /dev/null
+++ b/tests/acceptance/replay_linux.py
@@ -0,0 +1,97 @@
+# Record/replay test that boots a complete Linux system via a cloud image
+#
+# Copyright (c) 2020 ISP RAS
+#
+# Author:
+# Pavel Dovgalyuk
+#
+# 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.utils import cloudinit
+from avocado.utils import network
+from avocado.utils import vmimage
+from avocado.utils import datadrainer
+from avocado.utils.path import find_command
+from boot_linux import BootLinuxBase
+
+class ReplayLinux(BootLinuxBase):
+"""
+Boots a Linux system, checking for a successful initialization
+"""
+
+timeout = 1800
+chksum = None
+hdd = 'ide-hd'
+cd = 'ide-cd'
+bus = 'ide'
+
+def setUp(self):
+super(ReplayLinux, self).setUp()
+self.boot_path = self.download_boot()
+self.cloudinit_path = self.download_cloudinit()
+
+def vm_add_disk(self, vm, path, id, device):
+bus_string = ''
+if self.bus:
+bus_string = ',bus=%s.%d' % (self.bus, id,)
+vm.add_args('-drive', 'file=%s,snapshot,id=disk%s,if=none' % (path,
id))
+vm.add_args('-drive',
'driver=blkreplay,id=disk%s-rr,if=none,image=disk%s' % (id, id))
+vm.add_args('-device', '%s,drive=disk%s-rr%s' % (device, id,
bus_string))
+
+def launch_and_wait(self, record, args, shift):
+vm = self.get_vm()
+vm.add_args('-smp', '1')
+vm.add_args('-m', '1024')
+vm.add_args('-object', 'filter-replay,id=replay,netdev=hub0port0')
+if args:
+vm.add_args(*args)
+self.vm_add_disk(vm, self.boot_path, 0, self.hdd)
+self.vm_add_disk(vm, self.cloudinit_path, 1, self.cd)
+if record:
+mode = 'record'
+else:
+mode = 'replay'
+vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s' %
+(shift, mode, os.path.join(self.workdir, 'replay.bin')))
+
+vm.set_console()
+vm.launch()
+console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(),
+
logger=self.log.getChild('console'),
+ stop_check=(lambda : not
vm.is_running()))
+console_drainer.start()
+if record:
+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)
+vm.shutdown()
+else:
+self.log.info('VM launched, waiting the recorded execution to be
replayed')
+vm.wait()
+
+def run_rr(self, args=None, shift=7):
+self.launch_and_wait(True, args, shift)
+self.launch_and_wait(False, args, shift)
+
+class ReplayLinuxX8664(ReplayLinux):
+"""
+:avocado: tags=arch:x86_64
+"""
+
+chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
+
+def test_pc_i440fx(self):
+"""
+:avocado: tags=machine:pc
+:avocado: tags=accel:tcg
+"""
+self.run_rr(shift=1)
+
+def test_pc_q35(self):
+"""
+:avocado: tags=machine:q35
+:avocado: tags=accel:tcg
+"""
+self.run_rr(shift=3)
Re: [PATCH v2 04/11] tests/acceptance: add kernel record/replay test for x86_64
On 27.05.2020 18:41, Alex Bennée wrote:
Pavel Dovgalyuk writes:
This patch adds a test for record/replay an execution of x86_64 machine.
Execution scenario includes simple kernel boot, which allows testing
basic hardware interaction in RR mode.
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/tests/acceptance/replay_kernel.py
b/tests/acceptance/replay_kernel.py
index b8b277ad2f..c7526f1aba 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -55,3 +55,19 @@ class ReplayKernel(LinuxKernelUtils):
True, shift, args)
self.run_vm(kernel_path, kernel_command_line, console_pattern,
False, shift, args)
+
+def test_x86_64_pc(self):
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:pc
+"""
+kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+ '/linux/releases/29/Everything/x86_64/os/images/pxeboot'
+ '/vmlinuz')
+kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0'
+console_pattern = 'Kernel command line: %s' % kernel_command_line
+
+self.run_rr(kernel_path, kernel_command_line, console_pattern)
This test fails for me on the replay:
Have you applied latest RR patches?
2020-05-27 16:22:21,658 machine L0326 DEBUG| VM launch command:
'x86_64-softmmu/qemu-system-x86_64 -display none -vga none -chardev
socket,id=mon,path=/var/tmp/tmp4n_geosi/qemu-9516-monitor.sock -mon
chardev=mon,mode=control -machine pc -chardev
socket,id=console,path=/var/tmp/tmp4n_geosi/qemu-9516-console.sock,server,nowait
-serial chardev:console -icount
shift=7,rr=replay,rrfile=/var/tmp/avocado_b85h3ycg/avocado_job_8xrxksgj/1-._tests_acceptance_replay_kernel.py_ReplayKernel.test_x86_64_pc/replay.bin
-kernel
/home/alex/avocado/data/cache/by_location/df533120a0e0ffda2626bed6e8a975d3b07e3f05/vmlinuz
-append printk.time=0 console=ttyS0 -net none'
2020-05-27 16:22:21,725 qmp L0194 DEBUG| >>> {'execute':
'qmp_capabilities'}
2020-05-27 16:22:21,736 qmp L0202 DEBUG| <<< {'return': {}}
2020-05-27 16:23:49,372 stacktrace L0039 ERROR|
2020-05-27 16:23:49,372 stacktrace L0042 ERROR| Reproduced traceback
from:
/home/alex/lsrc/qemu.git/builds/all/tests/venv/lib/python3.7/site-packages/avocado/core/test.py:860
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| Traceback (most recent
call last):
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/acceptance/replay_kernel.py", line
73, in test_x86_64_pc
2020-05-27 16:23:49,373 stacktrace L0045 ERROR|
self.run_rr(kernel_path, kernel_command_line, console_pattern)
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/acceptance/replay_kernel.py", line
57, in run_rr
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| False, shift, args)
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/acceptance/replay_kernel.py", line
46, in run_vm
2020-05-27 16:23:49,373 stacktrace L0045 ERROR|
self.wait_for_console_pattern(console_pattern, vm)
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/acceptance/boot_linux_console.py",
line 37, in wait_for_console_pattern
2020-05-27 16:23:49,373 stacktrace L0045 ERROR| vm=vm)
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/acceptance/avocado_qemu/__init__.py",
line 131, in wait_for_console_pattern
2020-05-27 16:23:49,374 stacktrace L0045 ERROR|
_console_interaction(test, success_message, failure_message, None, vm=vm)
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/acceptance/avocado_qemu/__init__.py",
line 83, in _console_interaction
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| msg =
console.readline().strip()
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| File
"/usr/lib/python3.7/socket.py", line 589, in readinto
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| return
self._sock.recv_into(b)
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| File
"/home/alex/lsrc/qemu.git/builds/all/tests/venv/lib/python3.7/site-packages/avocado/plugins/runner.py",
line 89, in sigterm_handler
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| raise
RuntimeError("Test interrupted by SIGTERM")
2020-05-27 16:23:49,374 stacktrace L0045 ERROR| RuntimeError: Test
interrupted by SIGTERM
Re: [PATCH v2 03/11] tests/acceptance: add base class record/replay kernel tests
On 27.05.2020 18:20, Alex Bennée wrote:
Pavel Dovgalyuk writes:
This patch adds a base for testing kernel boot recording and replaying.
Each test has the phase of recording and phase of replaying.
Virtual machines just boot the kernel and do not interact with
the network.
Structure and image links for the tests are borrowed from boot_linux_console.py
Testing controls the message pattern at the end of the kernel
boot for both record and replay modes. In replay mode QEMU is also
intended to finish the execution automatically.
Signed-off-by: Pavel Dovgalyuk
diff --git a/MAINTAINERS b/MAINTAINERS
index 47ef3139e6..e9a9ce4f66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2497,6 +2497,7 @@ F: net/filter-replay.c
F: include/sysemu/replay.h
F: docs/replay.txt
F: stubs/replay.c
+F: tests/acceptance/replay_kernel.py
IOVA Tree
M: Peter Xu
diff --git a/tests/acceptance/replay_kernel.py
b/tests/acceptance/replay_kernel.py
new file mode 100644
index 00..b8b277ad2f
--- /dev/null
+++ b/tests/acceptance/replay_kernel.py
@@ -0,0 +1,57 @@
+# Record/replay test that boots a Linux kernel
+#
+# Copyright (c) 2020 ISP RAS
+#
+# Author:
+# Pavel Dovgalyuk
+#
+# 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
+import gzip
Do we actually use gzip in this test?
Removed that, thanks.
+
+from avocado_qemu import wait_for_console_pattern
+from avocado.utils import process
+from avocado.utils import archive
+from boot_linux_console import LinuxKernelUtils
+
+class ReplayKernel(LinuxKernelUtils):
+"""
+Boots a Linux kernel in record mode and checks that the console
+is operational and the kernel command line is properly passed
+from QEMU to the kernel.
+Then replays the same scenario and verifies, that QEMU correctly
+terminates.
Shouldn't we be doing more to verify the replay behaved the same as the
recorded session? What happens if things go wrong? Does QEMU barf out or
just deviate from the previous run?
We hardly can compare vCPU states during record and replay.
But in the most cases it is not needed. When control flow goes in the
wrong direction, it affects the interrupts and exceptions.
And interrupts and exceptions are the synchronization points in the
replay log. Therefore when the executions differ, QEMU replay just hangs.
+"""
+
+timeout = 90
+
+def run_vm(self, kernel_path, kernel_command_line, console_pattern,
+ record, shift, args):
+vm = self.get_vm()
+vm.set_console()
+if record:
+mode = 'record'
+else:
+mode = 'replay'
+vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s' %
+(shift, mode, os.path.join(self.workdir, 'replay.bin')),
+'-kernel', kernel_path,
+'-append', kernel_command_line,
+'-net', 'none')
+if args:
+vm.add_args(*args)
+vm.launch()
+self.wait_for_console_pattern(console_pattern, vm)
+if record:
+vm.shutdown()
+else:
+vm.wait()
+
+def run_rr(self, kernel_path, kernel_command_line, console_pattern,
+shift=7, args=None):
+self.run_vm(kernel_path, kernel_command_line, console_pattern,
+True, shift, args)
+self.run_vm(kernel_path, kernel_command_line, console_pattern,
+False, shift, args)
Re: [PATCH v3 14/22] microvm: use 2G split unconditionally
Hi, > But why use 2G split instead of 3G? There's only very little MMIO and > no PCI hole (including no huge MMCONFIG BAR) on microvm. Yes, we can go for 3G, we are indeed not short on address space ;) take care, Gerd
Re: [PATCH 09/24] macio: Fix to realize "mos6522-cuda" and "mos6522-pmu" devices
Paolo Bonzini writes: > On 27/05/20 17:05, Peter Maydell wrote: >> I disagree with these. We're in a realize function, the API >> says "on errors, report them via the Error* you got passed", >> so we should do that, not blow up. _abort only makes >> sense if (a) we have no better way to report errors than >> to abort (which isn't the case here) or (b) if we can guarantee >> that in fact the thing we're doing won't ever fail >> (which we can't here without knowing more about the internal >> implementation details of the MOS6522 device than we >> really ought to). > > Note however that before replacing _abort with error propagation > you need to make sure that you are "un-realizing" yourself properly. So > it may be better to have inferior (but clearly visible) error > propagation behavior, than untested (and perhaps untestable) buggy code > that looks great on the surface. This is exactly why I have to stop at _abort in this series. It's 24 patches of fixes to enable 50+ patches of refactoring, with more in the pipeline. If I stray even deeper into the weeds, my pipeline is going to explode %-}
[PATCH] fpu/softfloat: Silent 'bitwise negation of a boolean expression' warning
When building with clang version 10.0.0-4ubuntu1, we get:
CC lm32-softmmu/fpu/softfloat.o
fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
^~
!
fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
^
!
fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
^
!
fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
^~
!
fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
^~~
!
fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
^
!
fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
^
!
fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
^~~
!
Fix by rewriting the fishy bitwise AND of two bools as an int.
Cc: Toni Wilen
Suggested-by: Eric Blake
Buglink: https://bugs.launchpad.net/bugs/1881004
Signed-off-by: Philippe Mathieu-Daudé
---
fpu/softfloat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6c8f2d597a..0dd57eddd7 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -3362,7 +3362,9 @@ static int32_t roundAndPackInt32(bool zSign, uint64_t
absZ,
}
roundBits = absZ & 0x7F;
absZ = ( absZ + roundIncrement )>>7;
-absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
+if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
+absZ &= ~1;
+}
z = absZ;
if ( zSign ) z = - z;
if ( ( absZ>>32 ) || ( z && ( ( z < 0 ) ^ zSign ) ) ) {
@@ -3420,7 +3422,9 @@ static int64_t roundAndPackInt64(bool zSign, uint64_t
absZ0, uint64_t absZ1,
if ( increment ) {
++absZ0;
if ( absZ0 == 0 ) goto overflow;
-absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
+if (((absZ1 << 1) == 0) && roundNearestEven) {
+absZ0 &= ~1;
+}
}
z = absZ0;
if ( zSign ) z = - z;
@@ -3480,7 +3484,9 @@ static int64_t roundAndPackUint64(bool zSign, uint64_t
absZ0,
float_raise(float_flag_invalid, status);
return UINT64_MAX;
}
-absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
+if (((absZ1 << 1) == 0) && roundNearestEven) {
+absZ0 &= ~1;
+}
}
if (zSign && absZ0) {
@@ -3603,7 +3609,9 @@ static float32 roundAndPackFloat32(bool zSign, int zExp,
uint32_t zSig,
status->float_exception_flags |= float_flag_inexact;
}
zSig = ( zSig + roundIncrement )>>7;
-zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
+if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
+zSig &= ~1;
+}
if ( zSig == 0 ) zExp = 0;
return packFloat32( zSign, zExp, zSig );
@@ -3757,7 +3765,9 @@ static float64 roundAndPackFloat64(bool zSign, int zExp,
uint64_t zSig,
status->float_exception_flags |= float_flag_inexact;
}
zSig = ( zSig + roundIncrement )>>10;
-zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
+if (((roundBits ^ 0x200) == 0) && roundNearestEven) {
+zSig &= ~1;
+}
if ( zSig == 0 ) zExp = 0;
return packFloat64( zSign, zExp, zSig );
@@ -3983,8 +3993,9 @@ floatx80
Re: [PATCH 6/7] gitlab-ci: Determine the number of jobs dynamically
Thomas Huth writes:
> Some people might want to run the gitlab CI pipelines in an environment
> where multiple CPUs are available to the runners, so let's rather get
> the number for "-j" from the "nproc" program (increased by 1 to compensate
> for jobs that wait for I/O) instead of hard-coding it.
>
> Signed-off-by: Thomas Huth
> @@ -25,8 +27,8 @@ build-system1:
> - ../configure --enable-werror --target-list="aarch64-softmmu alpha-softmmu
>cris-softmmu hppa-softmmu lm32-softmmu moxie-softmmu
> microblazeel-softmmu
>mips64el-softmmu m68k-softmmu ppc-softmmu riscv64-softmmu
> sparc-softmmu"
> - - make -j2
> - - make -j2 check
> + - make -j"$JOBS"
> + - make -j"$JOBS" check
>
> build-system2:
> image: fedora:latest
> @@ -40,8 +42,8 @@ build-system2:
> - ../configure --enable-werror --target-list="tricore-softmmu
> unicore32-softmmu
>microblaze-softmmu mips-softmmu riscv32-softmmu s390x-softmmu
> sh4-softmmu
>sparc64-softmmu x86_64-softmmu xtensa-softmmu nios2-softmmu
> or1k-softmmu"
> - - make -j2
> - - make -j2 check
> + - make -j"$JOBS"
> + - make -j"$JOBS" check
>
> build-disabled:
> image: fedora:latest
> @@ -56,8 +58,8 @@ build-disabled:
>--disable-qom-cast-debug --disable-spice --disable-vhost-vsock
>--disable-vhost-net --disable-vhost-crypto --disable-vhost-user
>--target-list="i386-softmmu ppc64-softmmu mips64-softmmu
> i386-linux-user"
> - - make -j2
> - - make -j2 check-qtest SPEED=slow
> + - make -j"$JOBS"
> + - make -j"$JOBS" check-qtest SPEED=slow
I would make all the check jobs use a single core as it otherwise gets
hard to figure out exactly where something broke/hung.
> @@ -100,8 +102,8 @@ build-clang:
> - ../configure --cc=clang --cxx=clang++ --enable-werror
>--target-list="alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
> ppc-softmmu s390x-softmmu x86_64-softmmu arm-linux-user"
> - - make -j2
> - - make -j2 check
> + - make -j"$JOBS"
> + - make -j"$JOBS" check
Ditto for this check
>
> build-tci:
> image: centos:8
> @@ -112,7 +114,7 @@ build-tci:
> - cd build
> - ../configure --enable-tcg-interpreter
>--target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu ';
> done)"
> - - make -j2
> + - make -j"$JOBS"
> - make run-tcg-tests-x86_64-softmmu
> - make tests/qtest/boot-serial-test tests/qtest/cdrom-test
> tests/qtest/pxe-test
> - for tg in $TARGETS ; do
Otherwise:
Reviewed-by: Alex Bennée
--
Alex Bennée
Re: Fully operable build post
Chris Hoy writes: > Hello all, > > I am proud to see that I have my barebones implementation of qemu finally > working. After starting earlier this year, I have slowly made progress to > fully integrate my kernel hardware into a gpu-passthrough vm. I went > through many settings and templates from various sources and finally have > something that works for me for a start and would be really interested in > lining out my experiences. It is my observation that the documentation may > be a bit unevenly distributed on the official page as far as templates go > and so if I could help I would like to send my template through the > official forum. Any help or advice with this would be greatly > appreciated. We have a top level page on contribution here: https://www.qemu.org/contribute/ specifically the page: https://wiki.qemu.org/Contribute/SubmitAPatch goes into some detail on how to prepare, format and submit your patches. Specifically it has notes on basing on master, breaking up a long series for easy review and how to submit to the mailing list using git send-email. If there is something you don't understand in those pages or something that could be made clearer then please ask for clarification here or on the IRC channel. > > Best Regards -- Alex Bennée
Re: [PATCH v4 3/5] virtio-scsi: default num_queues to -smp N
On Wed, May 27, 2020 at 11:38:17AM +0100, Daniel P. Berrangé wrote: > On Wed, May 27, 2020 at 11:29:23AM +0100, Stefan Hajnoczi wrote: > > Automatically size the number of virtio-scsi-pci, vhost-scsi-pci, and > > vhost-user-scsi-pci request virtqueues to match the number of vCPUs. > > Other transports continue to default to 1 request virtqueue. > > IIRC this was raised on earlier versions of the series, but i don't > recall the outcome and no caveats are mentioned here... > > Is this default still valid for very large $vCPUs. eg if I run QEMU > with "-smp 512" or even larger (I've seen people discussing 1000's of > CPUs), is this going to cause problems with the virtio-scsi default > queue counts ? Is there such a thing as "too large" for the num > of queues setting ? num vectors defaults to a value derived from > num queues, so is there concept of "too large" for num of vectors > setting ? > > Ideally the commit message would answer these questions for future > reference. Same for the next patch to virtio-blk Good point. Actually this patch and the virtio-blk ones no longer contain the queue number policy. The new virtio_pci_optimal_num_queues() function encapsulates the policy to avoid duplication. I'll resend and update that patch with the full rationale. Thanks! signature.asc Description: PGP signature
[PATCH v4 1/2] char-socket: return -1 in case of disconnect during tcp_chr_write
During testing of the vhost-user-blk reconnect functionality the qemu
SIGSEGV was triggered:
start qemu as:
x86_64-softmmu/qemu-system-x86_64 -m 1024M -M q35 \
-object
memory-backend-file,id=ram-node0,size=1024M,mem-path=/dev/shm/qemu,share=on \
-numa node,cpus=0,memdev=ram-node0 \
-chardev socket,id=chardev0,path=./vhost.sock,noserver,reconnect=1 \
-device vhost-user-blk-pci,chardev=chardev0,num-queues=4 --enable-kvm
start vhost-user-blk daemon:
./vhost-user-blk -s ./vhost.sock -b test-img.raw
If vhost-user-blk will be killed during the vhost initialization
process, for instance after getting VHOST_SET_VRING_CALL command, then
QEMU will fail with the following backtrace:
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x559272bb in vhost_user_read (dev=0x7fffef2d53e0, msg=0x7fffd5b0)
at ./hw/virtio/vhost-user.c:260
260 CharBackend *chr = u->user->chr;
#0 0x559272bb in vhost_user_read (dev=0x7fffef2d53e0,
msg=0x7fffd5b0)
at ./hw/virtio/vhost-user.c:260
#1 0x5592acb8 in vhost_user_get_config (dev=0x7fffef2d53e0,
config=0x7fffef2d5394 "", config_len=60)
at ./hw/virtio/vhost-user.c:1645
#2 0x55925525 in vhost_dev_get_config (hdev=0x7fffef2d53e0,
config=0x7fffef2d5394 "", config_len=60)
at ./hw/virtio/vhost.c:1490
#3 0x558cc46b in vhost_user_blk_device_realize (dev=0x7fffef2d51a0,
errp=0x7fffd8f0)
at ./hw/block/vhost-user-blk.c:429
#4 0x55920090 in virtio_device_realize (dev=0x7fffef2d51a0,
errp=0x7fffd948)
at ./hw/virtio/virtio.c:3615
#5 0x55a9779c in device_set_realized (obj=0x7fffef2d51a0, value=true,
errp=0x7fffdb88)
at ./hw/core/qdev.c:891
...
The problem is that vhost_user_write doesn't get an error after
disconnect and try to call vhost_user_read(). The tcp_chr_write()
routine should return -1 in case of disconnect. Indicate the EIO error
if this routine is called in the disconnected state.
Signed-off-by: Dima Stepanov
Reviewed-by: Marc-André Lureau
---
chardev/char-socket.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 232e0a8..c2462e0 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -174,15 +174,16 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
*buf, int len)
if (ret < 0 && errno != EAGAIN) {
if (tcp_chr_read_poll(chr) <= 0) {
+/* Perform disconnect and return error. */
tcp_chr_disconnect_locked(chr);
-return len;
} /* else let the read handler finish it properly */
}
return ret;
} else {
-/* XXX: indicate an error ? */
-return len;
+/* Indicate an error. */
+errno = EIO;
+return -1;
}
}
--
2.7.4
[PATCH v4 2/2] vhost-user-blk: delay vhost_user_blk_disconnect
A socket write during vhost-user communication may trigger a disconnect
event, calling vhost_user_blk_disconnect() and clearing all the
vhost_dev structures holding data that vhost-user functions expect to
remain valid to roll back initialization correctly. Delay the cleanup to
keep vhost_dev structure valid.
There are two possible states to handle:
1. RUN_STATE_PRELAUNCH: skip bh oneshot call and perform disconnect in
the caller routine.
2. RUN_STATE_RUNNING: delay by using bh
BH changes are based on the similar changes for the vhost-user-net
device:
commit e7c83a885f865128ae3cf1946f8cb538b63cbfba
"vhost-user: delay vhost_user_stop"
Signed-off-by: Dima Stepanov
---
hw/block/vhost-user-blk.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9d8c0b3..76838e7 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -349,6 +349,19 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
vhost_dev_cleanup(>dev);
}
+static void vhost_user_blk_event(void *opaque, QEMUChrEvent event);
+
+static void vhost_user_blk_chr_closed_bh(void *opaque)
+{
+DeviceState *dev = opaque;
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+VHostUserBlk *s = VHOST_USER_BLK(vdev);
+
+vhost_user_blk_disconnect(dev);
+qemu_chr_fe_set_handlers(>chardev, NULL, NULL, vhost_user_blk_event,
+NULL, opaque, NULL, true);
+}
+
static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
{
DeviceState *dev = opaque;
@@ -363,7 +376,30 @@ static void vhost_user_blk_event(void *opaque,
QEMUChrEvent event)
}
break;
case CHR_EVENT_CLOSED:
-vhost_user_blk_disconnect(dev);
+/*
+ * A close event may happen during a read/write, but vhost
+ * code assumes the vhost_dev remains setup, so delay the
+ * stop & clear. There are two possible paths to hit this
+ * disconnect event:
+ * 1. When VM is in the RUN_STATE_PRELAUNCH state. The
+ * vhost_user_blk_device_realize() is a caller.
+ * 2. In tha main loop phase after VM start.
+ *
+ * For p2 the disconnect event will be delayed. We can't
+ * do the same for p1, because we are not running the loop
+ * at this moment. So just skip this step and perform
+ * disconnect in the caller function.
+ *
+ * TODO: maybe it is a good idea to make the same fix
+ * for other vhost-user devices.
+ */
+if (runstate_is_running()) {
+AioContext *ctx = qemu_get_current_aio_context();
+
+qemu_chr_fe_set_handlers(>chardev, NULL, NULL, NULL, NULL,
+NULL, NULL, false);
+aio_bh_schedule_oneshot(ctx, vhost_user_blk_chr_closed_bh, opaque);
+}
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
--
2.7.4
[PATCH v4 0/2] vhost-user reconnect issues during vhost initialization
Changes is v4: - Update the "[PATCH v4 2/2] vhost-user-blk: delay vhost_user_blk_disconnect" patch based on Raphael's comment and Li Feng previous commit: https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg02255.html Don't change the vhost_user_blk_device_realize() function. Update the comment for the CHR_EVENT_CLOSED event. Changes in v3: - "[PATCH v3 1/2] char-socket: return -1 in case of disconnect during tcp_chr_write" made a small cleanup suggested by Li Feng. Added "Reviewed-by: Marc-André Lureau" - Rework the vhost_user_blk_disconnect call logic to delay it. - Remove the migration patch from the patch set, since we are still having some discussion about it. In general the current idea is good, but need to make some more investigation of how to handle reconnect during migration properly Changes in v2: - Add to CC list: Li Feng , since it looks like that we are working on pretty similar issues - Remove [RFC PATCH v1 1/7] contrib/vhost-user-blk: add option to simulate disconnect on init. Going to send this functionality in the separate patch, with the LIBVHOST_USER_DEBUG rework. Need to think how to reuse this option and silence the messages first. - Remove [RFC PATCH v1 3/7] char-socket: initialize reconnect timer only if close is emitted. This will be handled in the separate patchset: [PATCH 3/4] char-socket: avoid double call tcp_chr_free_connection by Li Feng v1: During vhost-user reconnect functionality we hit several issues, if vhost-user-blk daemon is "crashed" or made disconnect during vhost initialization. The general scenario is as follows: - vhost start routine is called - vhost write failed due to SIGPIPE - this call the disconnect routine and vhost_dev_cleanup routine which set to 0 all the field of the vhost_dev structure - return back to vhost start routine with the error - on the fail path vhost start routine tries to rollback the changes by using vhost_dev struct fields which were already reset - sometimes this leads to SIGSEGV, sometimes to SIGABRT Before revising the vhost-user initialization code, we suggest adding the sanity checks to be aware of the possible disconnect event and that the vhost_dev structure can be in "uninitialized" state. The vhost-user-blk daemon is updated with the additional "--simulate-disconnect-stage=CASENUM" argument to simulate disconnect during VHOST device initialization. For instance: 1. $ ./vhost-user-blk -s ./vhost.sock -b test-img.raw --simulate-disconnect-stage=1 This command will simulate disconnect in the SET_VRING_CALL handler. In this case the vhost device in QEMU is not set the started field to true. 2. $ ./vhost-user-blk -s ./vhost.sock -b test-img.raw --simulate-disconnect-stage=2 This command will simulate disconnect in the SET_VRING_NUM handler. In this case the started field is set to true. These two cases test different QEMU parts. Also to trigger different code paths disconnect should be simulated in two ways: - before any successful initialization - make successful initialization once and try to simulate disconnects Also we catch SIGABRT on the migration start if vhost-user daemon disconnected during vhost-user set log commands communication. Dima Stepanov (2): char-socket: return -1 in case of disconnect during tcp_chr_write vhost-user-blk: delay vhost_user_blk_disconnect chardev/char-socket.c | 7 --- hw/block/vhost-user-blk.c | 38 +- 2 files changed, 41 insertions(+), 4 deletions(-) -- 2.7.4
How to fuzz devices that use timers?
Hi, Some devices use timers (QEMUTimer) to emulate hardware precise timings. i.e. with a flash device, the guest orders erasing it, the physical erasure takes some time - let say 200ms - and upon completion a bit is set in a register, and an interruption is eventually raised. When not multi-tasking, a guest usually poll the register until bit set. While fuzzing, a payload schedule triggers an erase, then (if we don't reset the device) thousands of payloads are tested in vain until the device is ready to process further requests. It is then hard to understand the patterns used (in 200ms libFuzzer tried ~5000 I/O other accesses). Even if we can reproduce the pattern with proper timings, it is also hard to reproduce. Since we run the fuzzer with the QTest accelerator, my first idea was to check for 'if (qtest_enabled())' in the timer code, and directly expire a timer instead of scheduling it. This way we can test reproducers. However various tests require/verify precise timing, so this would break various qtests. Second idea was to add a fuzzer_enabled() method, and check it. But then I noticed some devices use timers the other way, they start a timer and wait an event to happen in a correct amount of time, else the timer kick and error bit is set (this is the watchdog style). If I modify the timers to directly expire checking fuzzer_enabled(), then these devices enter failure mode directly without processing further requests. So I guess I have to go thru each device and check for fuzzer_enabled() where appropriate... Any clever ideas? Thanks Phil.
[Bug 1881004] Re: fpu/softfloat.c: error: bitwise negation of a boolean expression
Patch sent: https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg07861.html -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1881004 Title: fpu/softfloat.c: error: bitwise negation of a boolean expression Status in QEMU: New Bug description: Last time I built QEMU was on commit d5c75ec500d96f1d93447f990cd5a4ef5ba27fae, I just pulled to fea8f3ed739536fca027cf56af7f5576f37ef9cd and now get: CC lm32-softmmu/fpu/softfloat.o fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven ); ^~ ! fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven ); ^ ! fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven); ^ ! fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven ); ^~ ! fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven ); ^~~ ! fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven ); ^ ! fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven ); ^ ! fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation] zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven ); ^~~ ! 8 errors generated. $ clang -v clang version 10.0.0-4ubuntu1 Target: aarch64-unknown-linux-gnu $ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description:Ubuntu 20.04 LTS Release:20.04 Codename: focal To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1881004/+subscriptions
Re: [PATCH] linux-user/mmap.c: fix integer underflow in target_mremap
Le 02/05/2020 à 18:12, Jonathan Marler a écrit :
> Fixes: https://bugs.launchpad.net/bugs/1876373
>
> This code path in mmap occurs when a page size is decreased with mremap.
> When a section of pages is shrunk, qemu calls mmap_reserve on the pages that
> were released. However, it has the diff operation reversed, subtracting the
> larger old_size from the smaller new_size. Instead, it should be subtracting
> the smaller new_size from the larger old_size. You can also see in the
> previous line of the change that this mmap_reserve call only occurs when
> old_size > new_size.
>
> Signed-off-by: Jonathan Marler
> ---
> linux-user/mmap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index e378033797..caab62909e 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -708,7 +708,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> old_size,
> if (prot == 0) {
> host_addr = mremap(g2h(old_addr), old_size, new_size, flags);
> if (host_addr != MAP_FAILED && reserved_va && old_size >
> new_size) {
> -mmap_reserve(old_addr + old_size, new_size - old_size);
> +mmap_reserve(old_addr + old_size, old_size - new_size);
> }
> } else {
> errno = ENOMEM;
>
Reviewed-by: Laurent Vivier
[PATCH v3 03/10] block/vdi: return ZERO block-status when appropriate
In case of !VDI_IS_ALLOCATED[], we do zero out the corresponding chunk
of qiov. So, this should be reported as ZERO.
Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.
There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).
BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".
We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi at this patch and
vpc with the following) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/vdi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 2f506a01ba..c4527a9d8c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -334,7 +334,6 @@ static int vdi_get_info(BlockDriverState *bs,
BlockDriverInfo *bdi)
logout("\n");
bdi->cluster_size = s->block_size;
bdi->vm_state_offset = 0;
-bdi->unallocated_blocks_are_zero = true;
return 0;
}
@@ -536,7 +535,7 @@ static int coroutine_fn
vdi_co_block_status(BlockDriverState *bs,
*pnum = MIN(s->block_size - index_in_block, bytes);
result = VDI_IS_ALLOCATED(bmap_entry);
if (!result) {
-return 0;
+return BDRV_BLOCK_ZERO;
}
*map = s->header.offset_data + (uint64_t)bmap_entry * s->block_size +
--
2.18.0
[PATCH v3 07/10] block/file-posix: drop unallocated_blocks_are_zero
raw_co_block_status() in block/file-posix.c never returns 0, so
unallocated_blocks_are_zero is useless (it doesn't affect the only user
of the field: bdrv_co_block_status()). Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/file-posix.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 3ab8f5a0fa..d86ea57769 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2878,9 +2878,6 @@ static int coroutine_fn raw_co_pwrite_zeroes(
static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
-BDRVRawState *s = bs->opaque;
-
-bdi->unallocated_blocks_are_zero = s->discard_zeroes;
return 0;
}
--
2.18.0
[PATCH v3 08/10] block/vhdx: drop unallocated_blocks_are_zero
vhdx doesn't have .bdrv_co_block_status handler, so DATA|ALLOCATED is always assumed for it in bdrv_co_block_status(). unallocated_blocks_are_zero is useless (it doesn't affect the only user of the field: bdrv_co_block_status()), drop it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake --- block/vhdx.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/block/vhdx.c b/block/vhdx.c index fa9e544a5e..645dc4b4f4 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1164,9 +1164,6 @@ static int vhdx_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) bdi->cluster_size = s->block_size; -bdi->unallocated_blocks_are_zero = -(s->params.data_bits & VHDX_PARAMS_HAS_PARENT) == 0; - return 0; } -- 2.18.0
Re: How to fuzz devices that use timers?
> On 28 May 2020, at 11:26, Philippe Mathieu-Daudé wrote: > > Hi, > > Some devices use timers (QEMUTimer) to emulate hardware precise timings. > i.e. with a flash device, the guest orders erasing it, the physical > erasure takes some time - let say 200ms - and upon completion a bit is > set in a register, and an interruption is eventually raised. > When not multi-tasking, a guest usually poll the register until bit set. > > While fuzzing, a payload schedule triggers an erase, then (if we don't > reset the device) thousands of payloads are tested in vain until the > device is ready to process further requests. It is then hard to > understand the patterns used (in 200ms libFuzzer tried ~5000 I/O > other accesses). Even if we can reproduce the pattern with proper > timings, it is also hard to reproduce. > > Since we run the fuzzer with the QTest accelerator, my first idea was to > check for 'if (qtest_enabled())' in the timer code, and directly expire > a timer instead of scheduling it. This way we can test reproducers. > However various tests require/verify precise timing, so this would break > various qtests. > > Second idea was to add a fuzzer_enabled() method, and check it. But then > I noticed some devices use timers the other way, they start a timer and > wait an event to happen in a correct amount of time, else the timer kick > and error bit is set (this is the watchdog style). If I modify the > timers to directly expire checking fuzzer_enabled(), then these devices > enter failure mode directly without processing further requests. > > So I guess I have to go thru each device and check for fuzzer_enabled() > where appropriate... > Any clever ideas? Would it make sense to introduce the idea of device reservation / exclusivity in the tests? In other words, if you have an “erase”, then you add some “qtest_reserve_device(device, timeout, …)” and then modify the payload scheduling to avoid reserved devices. If there are less of these cases than general watchdogs, this may be a win in the end, and also clarify the intent. > > Thanks > > Phil. >
Re: [PATCH 0/2] linux-user: Load a vdso for x86_64
Le 19/05/2020 à 21:44, Richard Henderson a écrit : > The subject of AT_SYSINFO came up on launchpad recently. > > There is definite room for improvement in all of this: > > (1) We could build the vdso binary into qemu instead of really > loading it from the file system. This would obviate the > several problems of locating the .so file. It would also > mean that --static builds continue to create a standalone > qemu binary. > > (2) We could use our cross-build system to build the vdso. > Though we'd still likely want to keep the image in git > along side the other rom images for when cross-build is > not available. > > (3) There are some ??? comments where some decisions could be made, > and other ??? that are merely commenting on weirdness. > > (4) It shouldn't take too much effort to create vdsos for the > other architectures. But we should get this one as clean > as we can first. > > Amusingly, this patch set has just turned 10 years old. > First posted April 4, 2010. I don't recall ever seeing > any review on the several postings over the years. > > > r~ > > > Richard Henderson (2): > linux-user: Build vdso for x64. > linux-user: Load a VDSO for x86-64. > > Makefile | 4 +- > linux-user/elfload.c | 203 +- > pc-bios/Makefile | 5 + > pc-bios/vdso-linux-x64.S | 115 + > pc-bios/vdso-linux-x64.ld | 81 +++ > pc-bios/vdso-linux-x64.so | Bin 0 -> 7500 bytes > 6 files changed, 401 insertions(+), 7 deletions(-) > create mode 100644 pc-bios/vdso-linux-x64.S > create mode 100644 pc-bios/vdso-linux-x64.ld > create mode 100755 pc-bios/vdso-linux-x64.so > Applied to my linux-user branch. Thanks, Laurent
Re: [PATCH Kernel v22 0/8] Add UAPIs to support migration for VFIO devices
> > > This is my understanding of the protocol as well, when the device is
> > > running, pending_bytes might drop to zero if no internal state has
> > > changed and may be non-zero on the next iteration due to device
> > > activity. When the device is not running, pending_bytes reporting zero
> > > indicates the device is done, there is no further state to transmit.
> > > Does that meet your need/expectation?
> > >
> > (1) on one side, as in vfio_save_pending(),
> > vfio_save_pending()
> > {
> > ...
> > ret = vfio_update_pending(vbasedev);
> > ...
> > *res_precopy_only += migration->pending_bytes;
> > ...
> > }
> > the pending_bytes tells migration thread how much data is still hold in
> > device side.
> > the device data includes
> > device internal data + running device dirty data + device state.
> >
> > so the pending_bytes should include device state as well, right?
> > if so, the pending_bytes should never reach 0 if there's any device
> > state to be sent after device is stopped.
>
> I hadn't expected the pending-bytes to include a fixed offset for device
> state (If you mean a few registers etc) - I'd expect pending to drop
> possibly to zero; the heuristic as to when to switch from iteration to
> stop, is based on the total pending across all iterated devices; so it's
> got to be allowed to drop otherwise you'll never transition to stop.
>
ok. got it.
> > (2) on the other side,
> > along side we updated the pending_bytes in vfio_save_pending() and
> > enter into the vfio_save_iterate(), if we repeatedly update
> > pending_bytes in vfio_save_iterate(), it would enter into a scenario
> > like
> >
> > initially pending_bytes=500M.
> > vfio_save_iterate() -->
> > round 1: transmitted 500M.
> > round 2: update pending bytes, pending_bytes=50M (50M dirty data).
> > round 3: update pending bytes, pending_bytes=50M.
> > ...
> > round N: update pending bytes, pending_bytes=50M.
> >
> > If there're two vfio devices, the vfio_save_iterate() for the second device
> > may never get chance to be called because there's always pending_bytes
> > produced by the first device, even the size if small.
>
> And between RAM and the vfio devices?
yes, is that right?
Thanks
Yan
Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS
Thomas Huth writes:
> On 27/05/2020 18.36, Alex Bennée wrote:
>>
>> Thomas Huth writes:
>>
>>> On 27/05/2020 16.44, Laurent Vivier wrote:
Le 25/05/2020 à 15:18, Thomas Huth a écrit :
> From: Alex Bennée
>
> Newer clangs rightly spot that you can never exceed the full address
> space of 64 bit hosts with:
>
> linux-user/elfload.c:2076:41: error: result of comparison 'unsigned
> long' > 18446744073709551615 is always false
> [-Werror,-Wtautological-type-limit-compare]
> 4685 if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
> 4686 ~~~ ^ ~
> 4687 1 error generated.
>
> So lets limit the check to 32 bit hosts only.
>
> Fixes: ee94743034bf
> Reported-by: Thomas Huth
> Signed-off-by: Alex Bennée
> [thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS ==
> 32]
> Signed-off-by: Thomas Huth
> ---
> linux-user/elfload.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 01a9323a63..ebc663ea0b 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char
> *image_name, abi_ulong guest_loaddr,
> exit(EXIT_FAILURE);
> }
> } else {
> +#if HOST_LONG_BITS < TARGET_ABI_BITS
> if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
> error_report("%s: requires more virtual address space "
> "than the host can provide (0x%" PRIx64 ")",
> image_name, (uint64_t)guest_hiaddr -
> guest_base);
> exit(EXIT_FAILURE);
> }
> +#endif
> }
>
> /*
>
Philippe sent the same patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html
>>>
>>> Indeed, but looking more closely, he's using slightly different
>>> locations for the #if and #endif ... not sure what's better though...?
>>
>> Richard was more inclined to suppress the warning:
>>
>> Subject: Re: [PATCH v2] linux-user: limit check to HOST_LONG_BITS == 32
>> From: Richard Henderson
>> Message-ID: <3069bc1b-115d-f361-8271-c775bf695...@linaro.org>
>> Date: Thu, 21 May 2020 20:15:51 -0700
>>
>> One reason I dropped the f32 patch from my last PR was because this
>> wasn't the only warning the latest clang picks up.
>
> ... but this is currently the only spot that is required to get the
> gitlab CI going again, so I think we should include this patch until we
> have a final decision whether to disable the warning or not (and we can
> still revert this patch after we disabled the warning). Ok?
I'm certainly happy with that if it gets gitlab working.
My experience with make docker-test-vlang@fedora (with 32) was there
where more things to fix. I guess gitlab didn't trigger them.
--
Alex Bennée
Re: [PATCH v2] fpu/softfloat: Silent 'bitwise negation of a boolean expression' warning
On 28/05/2020 10.48, Philippe Mathieu-Daudé wrote:
> When building with clang version 10.0.0-4ubuntu1, we get:
>
> CC lm32-softmmu/fpu/softfloat.o
> fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
> ^~
>
> fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
>^
>
> fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
>^
>
> fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
> ^~
>
> fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
> ^~~
>
> fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven
> );
>
> ^
>
> fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) &
> roundNearestEven );
>
> ^
>
> fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression;
> did you mean logical negation? [-Werror,-Wbool-operation]
> zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
>^~~
>
> Fix by rewriting the fishy bitwise AND of two bools as an int.
>
> Suggested-by: Eric Blake
> Buglink: https://bugs.launchpad.net/bugs/1881004
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> v2: Resend without the Cc: "Toni Wilen " tag
> ---
> fpu/softfloat.c | 33 -
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 6c8f2d597a..0dd57eddd7 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -3362,7 +3362,9 @@ static int32_t roundAndPackInt32(bool zSign, uint64_t
> absZ,
> }
> roundBits = absZ & 0x7F;
> absZ = ( absZ + roundIncrement )>>7;
> -absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
> +if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
> +absZ &= ~1;
> +}
You could get rid of some more parentheses now:
if ((roundBits ^ 0x40) == 0 && roundNearestEven)
... also in the other hunks.
Anyway:
Reviewed-by: Thomas Huth
Re: USB pass-through problems
On Thu, 28 May 2020, Gerd Hoffmann wrote: On Wed, May 27, 2020 at 09:44:54PM +0200, BALATON Zoltan wrote: Hello, I've seen a case when QEMU hangs with a passed through USB device. This is with -device usb-ehci and pass through with usb-host. This works until the attached USB device reboots (so likely it disconnects and reconnects) at which point QEMU hangs and need to be SIGKILL-ed to end (that's a bit hard to do with mouse and keyboard grabbed). I've got this stack trace: #0 0x7f23e7bd4949 in poll () at /lib64/libc.so.6 #1 0x7f23e8bfa9a5 in () at /lib64/libusb-1.0.so.0 #2 0x7f23e8bfbb13 in libusb_handle_events_timeout_completed () at /lib64/libusb-1.0.so.0 #3 0x55e09854b7da in usb_host_abort_xfers (s=0x55e09b036dd0) at hw/usb/host-libusb.c:963 #4 0x55e09854b87a in usb_host_close (s=0x55e09b036dd0) at hw/usb/host-libusb.c:977 #5 0x55e09854b92e in usb_host_nodev_bh (opaque=0x55e09b036dd0) at hw/usb/host-libusb.c:998 #6 0x55e098757500 in aio_bh_call (bh=0x55e099ad9cc0) at util/async.c:136 #7 0x55e09875760a in aio_bh_poll (ctx=0x55e0996c2620) at util/async.c:164 #8 0x55e09875cb2a in aio_dispatch (ctx=0x55e0996c2620) at util/aio-posix.c:380 #9 0x55e098757a3d in aio_ctx_dispatch (source=0x55e0996c2620, callback=0x0, user_data=0x0) at util/async.c:306 #10 0x7f23e8c59665 in g_main_context_dispatch () at /lib64/libglib-2.0.so.0 #11 0x55e09875b0a9 in glib_pollfds_poll () at util/main-loop.c:219 #12 0x55e09875b123 in os_host_main_loop_wait (timeout=0) at util/main-loop.c:242 #13 0x55e09875b228 in main_loop_wait (nonblocking=0) at util/main-loop.c:518 #14 0x55e0982c91f8 in qemu_main_loop () at softmmu/vl.c:1664 #15 0x55e098162e7e in main (argc=, argv=, envp=) at softmmu/main.c:49 so the problem may be in libusb but QEMU should not hang completely. The host is Linux with libusb 1.0.22. Hmm, does reverting 76d0a9362c6a6a7d88aa18c84c4186c9107ecaef change behavior? Yes it does. Reverting that patch fixes the problem, no hang and device reconnects without problem. Thanks, BALATON Zoltan
Re: [PATCH] linux-user/mmap.c: fix integer underflow in target_mremap
Le 02/05/2020 à 18:12, Jonathan Marler a écrit :
> Fixes: https://bugs.launchpad.net/bugs/1876373
>
> This code path in mmap occurs when a page size is decreased with mremap.
> When a section of pages is shrunk, qemu calls mmap_reserve on the pages that
> were released. However, it has the diff operation reversed, subtracting the
> larger old_size from the smaller new_size. Instead, it should be subtracting
> the smaller new_size from the larger old_size. You can also see in the
> previous line of the change that this mmap_reserve call only occurs when
> old_size > new_size.
>
> Signed-off-by: Jonathan Marler
> ---
> linux-user/mmap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index e378033797..caab62909e 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -708,7 +708,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> old_size,
> if (prot == 0) {
> host_addr = mremap(g2h(old_addr), old_size, new_size, flags);
> if (host_addr != MAP_FAILED && reserved_va && old_size >
> new_size) {
> -mmap_reserve(old_addr + old_size, new_size - old_size);
> +mmap_reserve(old_addr + old_size, old_size - new_size);
> }
> } else {
> errno = ENOMEM;
>
Applied to my linux-user branch.
Thanks,
Laurent
[PATCH v3 02/10] block: inline bdrv_unallocated_blocks_are_zero()
The function has only one user: bdrv_co_block_status(). Inline it to
simplify reviewing of the following patches, which will finally drop
unallocated_blocks_are_zero field too.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
include/block/block.h | 1 -
block.c | 15 ---
block/io.c| 11 ---
3 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 25e299605e..68699ebbfa 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -488,7 +488,6 @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t
bytes);
int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
int bdrv_has_zero_init_1(BlockDriverState *bs);
int bdrv_has_zero_init(BlockDriverState *bs);
-bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs);
bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
int bdrv_block_status(BlockDriverState *bs, int64_t offset,
int64_t bytes, int64_t *pnum, int64_t *map,
diff --git a/block.c b/block.c
index 8416376c9b..34adf1298f 100644
--- a/block.c
+++ b/block.c
@@ -5408,21 +5408,6 @@ int bdrv_has_zero_init(BlockDriverState *bs)
return 0;
}
-bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs)
-{
-BlockDriverInfo bdi;
-
-if (bs->backing) {
-return false;
-}
-
-if (bdrv_get_info(bs, ) == 0) {
-return bdi.unallocated_blocks_are_zero;
-}
-
-return false;
-}
-
bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs)
{
if (!(bs->open_flags & BDRV_O_UNMAP)) {
diff --git a/block/io.c b/block/io.c
index 121ce17a49..6f9507f718 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2387,15 +2387,20 @@ static int coroutine_fn
bdrv_co_block_status(BlockDriverState *bs,
if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
ret |= BDRV_BLOCK_ALLOCATED;
} else if (want_zero) {
-if (bdrv_unallocated_blocks_are_zero(bs)) {
-ret |= BDRV_BLOCK_ZERO;
-} else if (bs->backing) {
+if (bs->backing) {
BlockDriverState *bs2 = bs->backing->bs;
int64_t size2 = bdrv_getlength(bs2);
if (size2 >= 0 && offset >= size2) {
ret |= BDRV_BLOCK_ZERO;
}
+} else {
+BlockDriverInfo bdi;
+int ret2 = bdrv_get_info(bs, );
+
+if (ret2 == 0 && bdi.unallocated_blocks_are_zero) {
+ret |= BDRV_BLOCK_ZERO;
+}
}
}
--
2.18.0
[PATCH v3 01/10] qemu-img: convert: don't use unallocated_blocks_are_zero
qemu-img convert wants to distinguish ZERO which comes from short
backing files. unallocated_blocks_are_zero field of bdi is unrelated:
space after EOF is always considered to be zero anyway. So, just make
post_backing_zero true in case of short backing file.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
qemu-img.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 2d30682f12..9fcfafe470 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1660,7 +1660,6 @@ typedef struct ImgConvertState {
BlockBackend *target;
bool has_zero_init;
bool compressed;
-bool unallocated_blocks_are_zero;
bool target_is_new;
bool target_has_backing;
int64_t target_backing_sectors; /* negative if unknown */
@@ -1705,7 +1704,7 @@ static int convert_iteration_sectors(ImgConvertState *s,
int64_t sector_num)
if (s->target_backing_sectors >= 0) {
if (sector_num >= s->target_backing_sectors) {
-post_backing_zero = s->unallocated_blocks_are_zero;
+post_backing_zero = true;
} else if (sector_num + n > s->target_backing_sectors) {
/* Split requests around target_backing_sectors (because
* starting from there, zeros are handled differently) */
@@ -2610,7 +2609,6 @@ static int img_convert(int argc, char **argv)
} else {
s.compressed = s.compressed || bdi.needs_compressed_writes;
s.cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
-s.unallocated_blocks_are_zero = bdi.unallocated_blocks_are_zero;
}
ret = convert_do_copy();
--
2.18.0
[RFC] some semihosting interrogation
Hi all,
Just wonderring if there is any reason not to be able to defer
qemu_semihosting_connect_chardevs a little more to be able to specify
chardev=serial0?
Like:
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 6390cf0..9fa1553 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4333,8 +4333,6 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_opts_foreach(qemu_find_opts("chardev"),
chardev_init_func, NULL, _fatal);
-/* now chardevs have been created we may have semihosting to connect */
-qemu_semihosting_connect_chardevs();
#ifdef CONFIG_VIRTFS
qemu_opts_foreach(qemu_find_opts("fsdev"),
@@ -4484,6 +4482,9 @@ void qemu_init(int argc, char **argv, char **envp)
if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
exit(1);
+/* now chardevs have been created we may have semihosting to connect */
+qemu_semihosting_connect_chardevs();
+
/* If no default VGA is requested, the default is "none". */
if (default_vga) {
vga_model = get_default_vga_model(machine_class);
Also I found out that the trailing \0 is sent to the chardev
(console.c:copy_user_string) is that expected in case of semihosting?
static GString *copy_user_string(CPUArchState *env, target_ulong addr)
{
CPUState *cpu = env_cpu(env);
GString *s = g_string_sized_new(128);
uint8_t c;
do {
if (cpu_memory_rw_debug(cpu, addr++, , 1, 0) == 0) {
if (c) {
s = g_string_append_c(s, c);
}
} else {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: passed inaccessible address " TARGET_FMT_lx,
__func__, addr);
break;
}
} while (c!=0);
return s;
}
I can roll out two patches if needed..
Cheers,
Fred
[PATCH v3 05/10] block/crypto: drop unallocated_blocks_are_zero
It's false by default, no needs to set it. We are going to drop this variable at all, so drop it now here, it doesn't hurt. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake --- block/crypto.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/crypto.c b/block/crypto.c index b216e12c31..6d930e2d54 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -710,7 +710,6 @@ static int block_crypto_get_info_luks(BlockDriverState *bs, return ret; } -bdi->unallocated_blocks_are_zero = false; bdi->cluster_size = subbdi.cluster_size; return 0; -- 2.18.0
[PATCH v3 10/10] qed: Simplify backing reads
From: Eric Blake
The other four drivers that support backing files (qcow, qcow2,
parallels, vmdk) all rely on the block layer to populate zeroes when
reading beyond EOF of a short backing file. We can simplify the qed
code by doing likewise.
Signed-off-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/qed.h | 1 -
block/qed.c | 64 +
2 files changed, 6 insertions(+), 59 deletions(-)
diff --git a/block/qed.h b/block/qed.h
index 42c115d822..3d12bf78d4 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -140,7 +140,6 @@ typedef struct QEDAIOCB {
/* Current cluster scatter-gather list */
QEMUIOVector cur_qiov;
-QEMUIOVector *backing_qiov;
uint64_t cur_pos; /* position on block device, in bytes */
uint64_t cur_cluster; /* cluster offset in image file */
unsigned int cur_nclusters; /* number of clusters being accessed */
diff --git a/block/qed.c b/block/qed.c
index a2dd952699..ece8b9bb60 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -849,56 +849,18 @@ static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
* @s: QED state
* @pos:Byte position in device
* @qiov: Destination I/O vector
- * @backing_qiov: Possibly shortened copy of qiov, to be allocated here
- * @cb: Completion function
- * @opaque: User data for completion function
*
* This function reads qiov->size bytes starting at pos from the backing file.
* If there is no backing file then zeroes are read.
*/
static int coroutine_fn qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
- QEMUIOVector *qiov,
- QEMUIOVector **backing_qiov)
+ QEMUIOVector *qiov)
{
-uint64_t backing_length = 0;
-size_t size;
-int ret;
-
-/* If there is a backing file, get its length. Treat the absence of a
- * backing file like a zero length backing file.
- */
if (s->bs->backing) {
-int64_t l = bdrv_getlength(s->bs->backing->bs);
-if (l < 0) {
-return l;
-}
-backing_length = l;
-}
-
-/* Zero all sectors if reading beyond the end of the backing file */
-if (pos >= backing_length ||
-pos + qiov->size > backing_length) {
-qemu_iovec_memset(qiov, 0, 0, qiov->size);
-}
-
-/* Complete now if there are no backing file sectors to read */
-if (pos >= backing_length) {
-return 0;
-}
-
-/* If the read straddles the end of the backing file, shorten it */
-size = MIN((uint64_t)backing_length - pos, qiov->size);
-
-assert(*backing_qiov == NULL);
-*backing_qiov = g_new(QEMUIOVector, 1);
-qemu_iovec_init(*backing_qiov, qiov->niov);
-qemu_iovec_concat(*backing_qiov, qiov, 0, size);
-
-BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO);
-ret = bdrv_co_preadv(s->bs->backing, pos, size, *backing_qiov, 0);
-if (ret < 0) {
-return ret;
+BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO);
+return bdrv_co_preadv(s->bs->backing, pos, qiov->size, qiov, 0);
}
+qemu_iovec_memset(qiov, 0, 0, qiov->size);
return 0;
}
@@ -915,7 +877,6 @@ static int coroutine_fn
qed_copy_from_backing_file(BDRVQEDState *s,
uint64_t offset)
{
QEMUIOVector qiov;
-QEMUIOVector *backing_qiov = NULL;
int ret;
/* Skip copy entirely if there is no work to do */
@@ -925,13 +886,7 @@ static int coroutine_fn
qed_copy_from_backing_file(BDRVQEDState *s,
qemu_iovec_init_buf(, qemu_blockalign(s->bs, len), len);
-ret = qed_read_backing_file(s, pos, , _qiov);
-
-if (backing_qiov) {
-qemu_iovec_destroy(backing_qiov);
-g_free(backing_qiov);
-backing_qiov = NULL;
-}
+ret = qed_read_backing_file(s, pos, );
if (ret) {
goto out;
@@ -1339,8 +1294,7 @@ static int coroutine_fn qed_aio_read_data(void *opaque,
int ret,
qemu_iovec_memset(>cur_qiov, 0, 0, acb->cur_qiov.size);
r = 0;
} else if (ret != QED_CLUSTER_FOUND) {
-r = qed_read_backing_file(s, acb->cur_pos, >cur_qiov,
- >backing_qiov);
+r = qed_read_backing_file(s, acb->cur_pos, >cur_qiov);
} else {
BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
r = bdrv_co_preadv(bs->file, offset, acb->cur_qiov.size,
@@ -1365,12 +1319,6 @@ static int coroutine_fn qed_aio_next_io(QEDAIOCB *acb)
while (1) {
trace_qed_aio_next_io(s, acb, 0, acb->cur_pos + acb->cur_qiov.size);
-if (acb->backing_qiov) {
-qemu_iovec_destroy(acb->backing_qiov);
-g_free(acb->backing_qiov);
-acb->backing_qiov = NULL;
-}
-
acb->qiov_offset += acb->cur_qiov.size;
acb->cur_pos += acb->cur_qiov.size;
Re: [PATCH v2 04/11] tests/acceptance: add kernel record/replay test for x86_64
On 27.05.2020 19:20, Alex Bennée wrote:
Alex Bennée writes:
Pavel Dovgalyuk writes:
This patch adds a test for record/replay an execution of x86_64 machine.
Execution scenario includes simple kernel boot, which allows testing
basic hardware interaction in RR mode.
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/tests/acceptance/replay_kernel.py
b/tests/acceptance/replay_kernel.py
index b8b277ad2f..c7526f1aba 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -55,3 +55,19 @@ class ReplayKernel(LinuxKernelUtils):
True, shift, args)
self.run_vm(kernel_path, kernel_command_line, console_pattern,
False, shift, args)
+
+def test_x86_64_pc(self):
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:pc
+"""
+kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+ '/linux/releases/29/Everything/x86_64/os/images/pxeboot'
+ '/vmlinuz')
+kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE +
'console=ttyS0'
I note that:
KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
and given we are looking for repeatability here maybe we should use our
own command line so we can compare the recorded and replayed boot?
To build on that I think a command line like:
KERNEL_COMMON_COMMAND_LINE = 'printk.time=1 panic=-1 '
called with --no-reboot and a pattern:
console_pattern = 'VFS: Cannot open root device'
You will run more of the kernel (importantly with timestamps > 0.000) so
we can have a better compare between the recorded and replayed run.
This is reasonable, thank you.
Re: [PATCH v2 03/11] tests/acceptance: add base class record/replay kernel tests
Pavel Dovgalyuk writes: > On 27.05.2020 18:20, Alex Bennée wrote: >> Pavel Dovgalyuk writes: >> >>> This patch adds a base for testing kernel boot recording and replaying. >>> Each test has the phase of recording and phase of replaying. >>> Virtual machines just boot the kernel and do not interact with >>> the network. >>> Structure and image links for the tests are borrowed from >>> boot_linux_console.py >>> Testing controls the message pattern at the end of the kernel >>> boot for both record and replay modes. In replay mode QEMU is also >>> intended to finish the execution automatically. >>> >>> Signed-off-by: Pavel Dovgalyuk >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 47ef3139e6..e9a9ce4f66 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -2497,6 +2497,7 @@ F: net/filter-replay.c >> F: include/sysemu/replay.h >> F: docs/replay.txt >> F: stubs/replay.c >> +F: tests/acceptance/replay_kernel.py >> IOVA Tree >> M: Peter Xu >> diff --git a/tests/acceptance/replay_kernel.py >> b/tests/acceptance/replay_kernel.py >> new file mode 100644 >> index 00..b8b277ad2f >> --- /dev/null >> +++ b/tests/acceptance/replay_kernel.py >> @@ -0,0 +1,57 @@ >> +# Record/replay test that boots a Linux kernel >> +# >> +# Copyright (c) 2020 ISP RAS >> +# >> +# Author: >> +# Pavel Dovgalyuk >> +# >> +# 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 >> +import gzip >> >> Do we actually use gzip in this test? > > Removed that, thanks. > >> >> + >> +from avocado_qemu import wait_for_console_pattern >> +from avocado.utils import process >> +from avocado.utils import archive >> +from boot_linux_console import LinuxKernelUtils >> + >> +class ReplayKernel(LinuxKernelUtils): >> +""" >> +Boots a Linux kernel in record mode and checks that the console >> +is operational and the kernel command line is properly passed >> +from QEMU to the kernel. >> +Then replays the same scenario and verifies, that QEMU correctly >> +terminates. >> >> Shouldn't we be doing more to verify the replay behaved the same as the >> recorded session? What happens if things go wrong? Does QEMU barf out or >> just deviate from the previous run? > > We hardly can compare vCPU states during record and replay. > > But in the most cases it is not needed. When control flow goes in the > wrong direction, it affects the interrupts and exceptions. > > And interrupts and exceptions are the synchronization points in the > replay log. Therefore when the executions differ, QEMU replay just > hangs. Maybe we should fix that and exit with a more definitive error? Hangs are just plain ugly to debug because your first step has to be to start poking around with a debugger. -- Alex Bennée
Re: [PATCH v2 04/11] tests/acceptance: add kernel record/replay test for x86_64
On 5/28/20 9:12 AM, Pavel Dovgalyuk wrote:
>
> On 27.05.2020 17:53, Philippe Mathieu-Daudé wrote:
>> On 5/27/20 12:31 PM, Pavel Dovgalyuk wrote:
>>> This patch adds a test for record/replay an execution of x86_64 machine.
>>> Execution scenario includes simple kernel boot, which allows testing
>>> basic hardware interaction in RR mode.
>>>
>>> Signed-off-by: Pavel Dovgalyuk
>>> ---
>>> 0 files changed
>>>
>>> diff --git a/tests/acceptance/replay_kernel.py
>>> b/tests/acceptance/replay_kernel.py
>>> index b8b277ad2f..c7526f1aba 100644
>>> --- a/tests/acceptance/replay_kernel.py
>>> +++ b/tests/acceptance/replay_kernel.py
>>> @@ -55,3 +55,19 @@ class ReplayKernel(LinuxKernelUtils):
>>> True, shift, args)
>>> self.run_vm(kernel_path, kernel_command_line, console_pattern,
>>> False, shift, args)
>>> +
>>> + def test_x86_64_pc(self):
>>> + """
>>> + :avocado: tags=arch:x86_64
>>> + :avocado: tags=machine:pc
>>> + """
>>> + kernel_url =
>>> ('https://archives.fedoraproject.org/pub/archive/fedora'
>>> +
>>> '/linux/releases/29/Everything/x86_64/os/images/pxeboot'
>>> + '/vmlinuz')
>>> + kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
>>> + kernel_path = self.fetch_asset(kernel_url,
>>> asset_hash=kernel_hash)
>>> +
>>> + kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE +
>>> 'console=ttyS0'
>>> + console_pattern = 'Kernel command line: %s' %
>>> kernel_command_line
>>> +
>>> + self.run_rr(kernel_path, kernel_command_line, console_pattern)
>>>
>> This one timeouted (I build with --enable-debug):
>
> I've got the strange behavior for the couple of times.
>
> Console output was correct (I saw 'Kernel command line' in logs), but
> _console_interation function didn't notice it.
>
> Therefore the test finished with timeout.
>
> How this could be possible?
IIRC there is a problem in how Avocado consume the chardev output.
Cleber has been working on some PoC / kludge.
Cleber/Eduardo do you remember the problem?
>
>> (1/1) tests/acceptance/replay_kernel.py:ReplayKernel.test_x86_64_pc:
>> replay: recording...
>> replay: replaying...
>> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
>> reached\nOriginal status: ERROR\n{'name':
>> '1-tests/acceptance/replay_kernel.py:ReplayKernel.test_x86_64_pc',
>> 'logdir':
>> 'avocado/job-results/job-2020-05-27T16.48-71d7bf4/test-results/1-tes...
>> (90.68 s)
>>
>
Re: [PATCH 5/7] gitlab-ci: Do not use the standard container images from gitlab
Thomas Huth writes: > Currently all pipelines of the gitlab CI are failing, except for the > "build-user" pipeline. There is an issue with the default container > image (likely Debian stable) where they imported something bad in one > of the system headers: > > /usr/include/linux/swab.h: In function '__swab': > /builds/huth/qemu/include/qemu/bitops.h:20:34: error: "sizeof" is not > defined, evaluates to 0 [-Werror=undef] > #define BITS_PER_LONG (sizeof (unsigned long) * BITS_PER_BYTE) > > We could maybe work-around this issue or wait for the default containers > to get fixed, but considering that we use Ubuntu (and thus Debian-style) > CI in Travis already to a very large extent, we should consider to use > some RPM-based distros in our gitlab CI instead. Thus let's change the > failing pipelines to use Fedora and CentOS (and also one Ubuntu 19.10, > since 20.04 is broken, too) now. > > Signed-off-by: Thomas Huth Acked-by: Alex Bennée I will say Fedora/CentOS won't be immune to this failure if they update the kernel headers to somewhere between the breakage and the fix. > --- > .gitlab-ci.yml | 37 + > 1 file changed, 25 insertions(+), 12 deletions(-) > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index 5208d93ff8..559ec2ab4d 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -5,9 +5,17 @@ include: > .update_apt_template: _script_apt > before_script: >- apt-get update -qq > - - apt-get install -y -qq libglib2.0-dev libpixman-1-dev genisoimage > + - apt-get install -y -qq git gcc libglib2.0-dev libpixman-1-dev make > +genisoimage > + > +.update_dnf_template: _script_dnf > + before_script: > + - dnf update -y > + - dnf install -y bzip2 diffutils gcc git genisoimage findutils glib2-devel > +make python3 perl-podlators perl-Test-Harness pixman-devel zlib-devel > > build-system1: > + image: ubuntu:19.10 > <<: *before_script_apt > script: > - apt-get install -y -qq libgtk-3-dev libvte-dev nettle-dev libcacard-dev > @@ -21,11 +29,12 @@ build-system1: > - make -j2 check > > build-system2: > - <<: *before_script_apt > + image: fedora:latest > + <<: *before_script_dnf > script: > - - apt-get install -y -qq libsdl2-dev libgcrypt-dev libbrlapi-dev libaio-dev > - libfdt-dev liblzo2-dev librdmacm-dev libibverbs-dev libibumad-dev > - libzstd-dev > + - yum install -y SDL2-devel libgcrypt-devel brlapi-devel libaio-devel > + libfdt-devel lzo-devel librdmacm-devel libibverbs-devel > libibumad-devel > + libzstd-devel > - mkdir build > - cd build > - ../configure --enable-werror --target-list="tricore-softmmu > unicore32-softmmu > @@ -35,7 +44,8 @@ build-system2: > - make -j2 check > > build-disabled: > - <<: *before_script_apt > + image: fedora:latest > + <<: *before_script_dnf > script: > - mkdir build > - cd build > @@ -50,9 +60,10 @@ build-disabled: > - make -j2 check-qtest SPEED=slow > > build-tcg-disabled: > - <<: *before_script_apt > + image: centos:8 > + <<: *before_script_dnf > script: > - - apt-get install -y -qq clang libgtk-3-dev libusb-dev > + - dnf install -y clang gtk3-devel libusbx-devel libgcrypt-devel > - mkdir build > - cd build > - ../configure --cc=clang --enable-werror --disable-tcg --audio-drv-list="" > @@ -79,10 +90,11 @@ build-user: > - make run-tcg-tests-i386-linux-user run-tcg-tests-x86_64-linux-user > > build-clang: > - <<: *before_script_apt > + image: fedora:latest > + <<: *before_script_dnf > script: > - - apt-get install -y -qq clang libsdl2-dev libattr1-dev libcap-ng-dev > - xfslibs-dev libiscsi-dev libnfs-dev libseccomp-dev gnutls-dev > librbd-dev > + - yum install -y clang SDL2-devel libattr-devel libcap-ng-devel > xfsprogs-devel > + libiscsi-devel libnfs-devel libseccomp-devel gnutls-devel librbd-devel > - mkdir build > - cd build > - ../configure --cc=clang --cxx=clang++ --enable-werror > @@ -92,7 +104,8 @@ build-clang: > - make -j2 check > > build-tci: > - <<: *before_script_apt > + image: centos:8 > + <<: *before_script_dnf > script: > - TARGETS="aarch64 alpha arm hppa m68k microblaze moxie ppc64 s390x x86_64" > - mkdir build -- Alex Bennée
[PATCH v2] fpu/softfloat: Silent 'bitwise negation of a boolean expression' warning
When building with clang version 10.0.0-4ubuntu1, we get:
CC lm32-softmmu/fpu/softfloat.o
fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
^~
fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
^
fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
^
fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
^~
fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
^~~
fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
^
fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven
);
^
fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did
you mean logical negation? [-Werror,-Wbool-operation]
zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
^~~
Fix by rewriting the fishy bitwise AND of two bools as an int.
Suggested-by: Eric Blake
Buglink: https://bugs.launchpad.net/bugs/1881004
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Resend without the Cc: "Toni Wilen " tag
---
fpu/softfloat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6c8f2d597a..0dd57eddd7 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -3362,7 +3362,9 @@ static int32_t roundAndPackInt32(bool zSign, uint64_t
absZ,
}
roundBits = absZ & 0x7F;
absZ = ( absZ + roundIncrement )>>7;
-absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
+if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
+absZ &= ~1;
+}
z = absZ;
if ( zSign ) z = - z;
if ( ( absZ>>32 ) || ( z && ( ( z < 0 ) ^ zSign ) ) ) {
@@ -3420,7 +3422,9 @@ static int64_t roundAndPackInt64(bool zSign, uint64_t
absZ0, uint64_t absZ1,
if ( increment ) {
++absZ0;
if ( absZ0 == 0 ) goto overflow;
-absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
+if (((absZ1 << 1) == 0) && roundNearestEven) {
+absZ0 &= ~1;
+}
}
z = absZ0;
if ( zSign ) z = - z;
@@ -3480,7 +3484,9 @@ static int64_t roundAndPackUint64(bool zSign, uint64_t
absZ0,
float_raise(float_flag_invalid, status);
return UINT64_MAX;
}
-absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
+if (((absZ1 << 1) == 0) && roundNearestEven) {
+absZ0 &= ~1;
+}
}
if (zSign && absZ0) {
@@ -3603,7 +3609,9 @@ static float32 roundAndPackFloat32(bool zSign, int zExp,
uint32_t zSig,
status->float_exception_flags |= float_flag_inexact;
}
zSig = ( zSig + roundIncrement )>>7;
-zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
+if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
+zSig &= ~1;
+}
if ( zSig == 0 ) zExp = 0;
return packFloat32( zSign, zExp, zSig );
@@ -3757,7 +3765,9 @@ static float64 roundAndPackFloat64(bool zSign, int zExp,
uint64_t zSig,
status->float_exception_flags |= float_flag_inexact;
}
zSig = ( zSig + roundIncrement )>>10;
-zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
+if (((roundBits ^ 0x200) == 0) && roundNearestEven) {
+zSig &= ~1;
+}
if ( zSig == 0 ) zExp = 0;
return packFloat64( zSign, zExp, zSig );
@@ -3983,8 +3993,9 @@ floatx80 roundAndPackFloatx80(int8_t roundingPrecision,
bool
Re: [PATCH 6/7] gitlab-ci: Determine the number of jobs dynamically
On 28/05/2020 10.41, Alex Bennée wrote: > > Thomas Huth writes: > >> Some people might want to run the gitlab CI pipelines in an environment >> where multiple CPUs are available to the runners, so let's rather get >> the number for "-j" from the "nproc" program (increased by 1 to compensate >> for jobs that wait for I/O) instead of hard-coding it. >> >> Signed-off-by: Thomas Huth > >> @@ -25,8 +27,8 @@ build-system1: >> - ../configure --enable-werror --target-list="aarch64-softmmu alpha-softmmu >>cris-softmmu hppa-softmmu lm32-softmmu moxie-softmmu >> microblazeel-softmmu >>mips64el-softmmu m68k-softmmu ppc-softmmu riscv64-softmmu >> sparc-softmmu" >> - - make -j2 >> - - make -j2 check >> + - make -j"$JOBS" >> + - make -j"$JOBS" check >> >> build-system2: >> image: fedora:latest >> @@ -40,8 +42,8 @@ build-system2: >> - ../configure --enable-werror --target-list="tricore-softmmu >> unicore32-softmmu >>microblaze-softmmu mips-softmmu riscv32-softmmu s390x-softmmu >> sh4-softmmu >>sparc64-softmmu x86_64-softmmu xtensa-softmmu nios2-softmmu >> or1k-softmmu" >> - - make -j2 >> - - make -j2 check >> + - make -j"$JOBS" >> + - make -j"$JOBS" check >> >> build-disabled: >> image: fedora:latest >> @@ -56,8 +58,8 @@ build-disabled: >>--disable-qom-cast-debug --disable-spice --disable-vhost-vsock >>--disable-vhost-net --disable-vhost-crypto --disable-vhost-user >>--target-list="i386-softmmu ppc64-softmmu mips64-softmmu >> i386-linux-user" >> - - make -j2 >> - - make -j2 check-qtest SPEED=slow >> + - make -j"$JOBS" >> + - make -j"$JOBS" check-qtest SPEED=slow > > I would make all the check jobs use a single core as it otherwise gets > hard to figure out exactly where something broke/hung. It's a somewhat double-edged sword ... either faster CI test times, or more deterministic output ... so far I didn't suffer the problem with the deterministic output in the gitlab-CI yet (unlike with Travis), so I'd rather keep the -j here for now. We can still remove it later if we hit a bug that is hard to debug otherwise. Thomas
[PATCH v3 09/10] block: drop unallocated_blocks_are_zero
Currently this field only set by qed and qcow2. But in fact, all
backing-supporting formats (parallels, qcow, qcow2, qed, vmdk) share
these semantics: on unallocated blocks, if there is no backing file they
just memset the buffer with zeroes.
So, document this behavior for .supports_backing and drop
.unallocated_blocks_are_zero
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
include/block/block.h | 5 -
include/block/block_int.h | 12 +++-
block/io.c| 9 ++---
block/qcow2.c | 1 -
block/qed.c | 1 -
5 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 68699ebbfa..ceb9defb58 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -21,11 +21,6 @@ typedef struct BlockDriverInfo {
/* offset at which the VM state can be saved (0 if not possible) */
int64_t vm_state_offset;
bool is_dirty;
-/*
- * True if unallocated blocks read back as zeroes. This is equivalent
- * to the LBPRZ flag in the SCSI logical block provisioning page.
- */
-bool unallocated_blocks_are_zero;
/*
* True if this block driver only supports compressed writes
*/
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 791de6a59c..faeeef1706 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -123,7 +123,17 @@ struct BlockDriver {
*/
bool bdrv_needs_filename;
-/* Set if a driver can support backing files */
+/*
+ * Set if a driver can support backing files. This also implies the
+ * following semantics:
+ *
+ * - Return status 0 of .bdrv_co_block_status means that corresponding
+ *blocks are not allocated in this layer of backing-chain
+ * - For such (unallocated) blocks, read will:
+ *- fill buffer with zeros if there is no backing file
+ *- read from the backing file otherwise, where the block layer
+ * takes care of reading zeros beyond EOF if backing file is short
+ */
bool supports_backing;
/* For handling image reopen for split or non-split files */
diff --git a/block/io.c b/block/io.c
index 6f9507f718..ddcd533c5d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2386,7 +2386,7 @@ static int coroutine_fn
bdrv_co_block_status(BlockDriverState *bs,
if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
ret |= BDRV_BLOCK_ALLOCATED;
-} else if (want_zero) {
+} else if (want_zero && bs->drv->supports_backing) {
if (bs->backing) {
BlockDriverState *bs2 = bs->backing->bs;
int64_t size2 = bdrv_getlength(bs2);
@@ -2395,12 +2395,7 @@ static int coroutine_fn
bdrv_co_block_status(BlockDriverState *bs,
ret |= BDRV_BLOCK_ZERO;
}
} else {
-BlockDriverInfo bdi;
-int ret2 = bdrv_get_info(bs, );
-
-if (ret2 == 0 && bdi.unallocated_blocks_are_zero) {
-ret |= BDRV_BLOCK_ZERO;
-}
+ret |= BDRV_BLOCK_ZERO;
}
}
diff --git a/block/qcow2.c b/block/qcow2.c
index dfab8d2f6c..72ed3bf305 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4973,7 +4973,6 @@ err:
static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVQcow2State *s = bs->opaque;
-bdi->unallocated_blocks_are_zero = true;
bdi->cluster_size = s->cluster_size;
bdi->vm_state_offset = qcow2_vm_state_offset(s);
return 0;
diff --git a/block/qed.c b/block/qed.c
index c0c65015c7..a2dd952699 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1514,7 +1514,6 @@ static int bdrv_qed_get_info(BlockDriverState *bs,
BlockDriverInfo *bdi)
memset(bdi, 0, sizeof(*bdi));
bdi->cluster_size = s->header.cluster_size;
bdi->is_dirty = s->header.features & QED_F_NEED_CHECK;
-bdi->unallocated_blocks_are_zero = true;
return 0;
}
--
2.18.0
[PATCH v3 06/10] block/iscsi: drop unallocated_blocks_are_zero
We set bdi->unallocated_blocks_are_zero = iscsilun->lbprz, but
iscsi_co_block_status doesn't return 0 in case of iscsilun->lbprz, it
returns ZERO when appropriate. So actually unallocated_blocks_are_zero
is useless (it doesn't affect the only user of the field:
bdrv_co_block_status()). Drop it now.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/iscsi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index a8b76979d8..767e3e75fd 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2163,7 +2163,6 @@ static int coroutine_fn
iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
IscsiLun *iscsilun = bs->opaque;
-bdi->unallocated_blocks_are_zero = iscsilun->lbprz;
bdi->cluster_size = iscsilun->cluster_size;
return 0;
}
--
2.18.0
[PATCH v3 00/10] drop unallocated_blocks_are_zero
This is first step to block-status refactoring, and solves most simple problem mentioned in my investigation of block-status described in the thread "backing chain & block status & filters": https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04706.html The whole series is reviewed, let's proceed with it please! v3: rebase on master 02: grammar fix in commit msg, add Eric's r-b 03,04,09: add Eric's r-b 10: add my r-b Eric Blake (1): qed: Simplify backing reads Vladimir Sementsov-Ogievskiy (9): qemu-img: convert: don't use unallocated_blocks_are_zero block: inline bdrv_unallocated_blocks_are_zero() block/vdi: return ZERO block-status when appropriate block/vpc: return ZERO block-status when appropriate block/crypto: drop unallocated_blocks_are_zero block/iscsi: drop unallocated_blocks_are_zero block/file-posix: drop unallocated_blocks_are_zero block/vhdx: drop unallocated_blocks_are_zero block: drop unallocated_blocks_are_zero block/qed.h | 1 - include/block/block.h | 6 include/block/block_int.h | 12 +++- block.c | 15 - block/crypto.c| 1 - block/file-posix.c| 3 -- block/io.c| 8 ++--- block/iscsi.c | 1 - block/qcow2.c | 1 - block/qed.c | 65 --- block/vdi.c | 3 +- block/vhdx.c | 3 -- block/vpc.c | 3 +- qemu-img.c| 4 +-- 14 files changed, 24 insertions(+), 102 deletions(-) -- 2.18.0
[PATCH v3 04/10] block/vpc: return ZERO block-status when appropriate
In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.
Note that this changes visible output of "qemu-img map --output=json"
and "qemu-io -c map" commands. For qemu-img map, the change is obvious:
we just mark as zero what is really zero. For qemu-io it's less
obvious: what was unallocated now is allocated.
There is an inconsistency in understanding of unallocated regions in
Qemu: backing-supporting format-drivers return 0 block-status to report
go-to-backing logic for this area. Some protocol-drivers (iscsi) return
0 to report fs-unallocated-non-zero status (i.e., don't occupy space on
disk, read result is undefined).
BDRV_BLOCK_ALLOCATED is defined as something more close to
go-to-backing logic. Still it is calculated as ZERO | DATA, so 0 from
iscsi is treated as unallocated. It doesn't influence backing-chain
behavior, as iscsi can't have backing file. But it does influence
"qemu-io -c map".
We should solve this inconsistency at some future point. Now, let's
just make backing-not-supporting format drivers (vdi in the previous
patch and vpc now) to behave more like backing-supporting drivers
and not report 0 block-status. More over, returning ZERO status is
absolutely valid thing, and again, corresponds to how the other
format-drivers (backing-supporting) work.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense (as the only user of it
is bdrv_co_block_status and it checks unallocated_blocks_are_zero only
for unallocated areas). Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
block/vpc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index c055591641..01fcd37e3c 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -606,7 +606,6 @@ static int vpc_get_info(BlockDriverState *bs,
BlockDriverInfo *bdi)
bdi->cluster_size = s->block_size;
}
-bdi->unallocated_blocks_are_zero = true;
return 0;
}
@@ -745,7 +744,7 @@ static int coroutine_fn
vpc_co_block_status(BlockDriverState *bs,
image_offset = get_image_offset(bs, offset, false, NULL);
allocated = (image_offset != -1);
*pnum = 0;
-ret = 0;
+ret = BDRV_BLOCK_ZERO;
do {
/* All sectors in a block are contiguous (without using the bitmap) */
--
2.18.0
Re: [PATCH v3] arm/aspeed: Rework NIC attachment
On Wed, 27 May 2020 at 12:44, Cédric Le Goater wrote: > > The number of MACs supported by an Aspeed SoC is defined by "macs_num" > under the SoC model, that is two for the AST2400 and AST2500 and four > for the AST2600. The model initializes the maximum number of supported > MACs but the number of realized devices is capped by the number of > network device back-ends defined on the command line. This can leave > unrealized devices hanging around in the QOM composition tree. > > Modify the machine initialization to define which MACs are attached to > a network device back-end using a bit-field property "macs-mask" and > let the SoC realize all network devices. > > The default setting of "macs-mask" is "use MAC0" only, which works for > all our AST2400 and AST2500 machines. The AST2600 machines have > different configurations. The AST2600 EVB machine activates MAC1, MAC2 > and MAC3 and the Tacoma BMC machine activates MAC2. > > Inactive MACs will have no peer and QEMU may warn the user with : > > qemu-system-arm: warning: nic ftgmac100.0 has no peer > qemu-system-arm: warning: nic ftgmac100.1 has no peer > qemu-system-arm: warning: nic ftgmac100.3 has no peer > > Signed-off-by: Cédric Le Goater > Reviewed-by: Markus Armbruster Reviewed-by: Joel Stanley This is a good usability improvement, thanks Cédric. > @@ -680,6 +691,7 @@ static void > aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data) > amc->fmc_model = "w25q512jv"; > amc->spi_model = "mx66u51235f"; > amc->num_cs= 1; > +amc->macs_mask = ASPEED_MAC1_ON | ASPEED_MAC2_ON | ASPEED_MAC3_ON; In the future we could enable all four. The A0 silicon had a broken MAC4 but it is working on A1.
Re: GDB get wrong debug infos on TI DSP architecture extension
casmac <1482995...@qq.com> writes:
> Hi,
> Thank you for forwarding my question to developers and sharing
> the C6x implementation.
> Perhaps I should follow up with another problem I encountered.
> The senerio is the emulator keeps running eventhough the program it
> emulates has already exited. And it keeps retrieving instructions which are
> all zero "instruction"(0x).
>
> It looks to me that in function cpu_exec(CPUState *cpu), the
> following loop never terminate:
> while (!cpu_handle_exception(cpu, ret)) {
> TranslationBlock *last_tb = NULL;
> int tb_exit = 0;
> while (!cpu_handle_interrupt(cpu,
> last_tb)) { ... }
> Is it because cpu-exit_request remains 0 ?
>
> At what point should we make cpu-exit_request=1 ?
cpu->exit_request is set for asynchronus conditions (e.g. timer IRQs or
IO events). A number of helpers will "kick" the cpu by calling
cpu_exit().
> Thanks again!!
>
>
> regards
> xiaolei
>
>
>
> --Original--
> From:"Philippe Mathieu-Daudé" Date:Wed, May 27, 2020 03:19 PM
> To:"casmac"<1482995...@qq.com;"qemu-devel" Cc:"Taylor Simpson" Bennée" Subject:Re: GDB get wrong debug infos on TI DSP architecture extension
>
>
>
> Hi Xiaolei,
>
> Cc'ing more developers who might answer you.
>
> On 5/27/20 8:48 AM, casmac wrote:
> Hi all,
> I am working on a TI DSP architecture extension for QEMU.
>
> FYI you can find the TI TMS320C6x target implemented here:
> https://github.com/philmd/qemu/releases/tag/target-c6x-2.4
>
> I started rebasing it to QEMU 4.2 but then got distracted.
>
> Now, we are
> adding GDB debugging features.
> We have done the following, but not sure we are on the
> right track :
> - add a xml description file in gdb-xml, without
> understanding the
> purpose of the file, why some architectures don't provide such xml file?
> - add ***_cpu_gdb_read_register(),
> ***_cpu_gdb_write_register();
> - added dsp_cpu_get_phys_page_attrs_debug(), but
> uncertain about
> what to return
>dsp_cpu_get_phys_page_attrs_debug(CPUState *cs,
> vaddr addr,
> MemTxAttrs *attrs)
>{
> return addr
> TARGET_PAGE_MASK;
>}
>
> We run QEMU with the these arguments
> qemu-system-dsp ... -kernel filename.out -S -s
>
> It turns out that gdb reads incorrect register values, and
> complains
> : "warning: Target-supplied registers are not supported by the current
> architecture".
>
> Something is missing here, or we do it in a wrong
> way. Any advise
> would be helpful to us.
>
> Thanks.
>
> xiaolei
>
> - ti_dsp.xml -
>
>
--
Alex Bennée
Re: [PATCH v2] fpu/softfloat: Silent 'bitwise negation of a boolean expression' warning
On 5/28/20 10:57 AM, Thomas Huth wrote:
> On 28/05/2020 10.48, Philippe Mathieu-Daudé wrote:
>> When building with clang version 10.0.0-4ubuntu1, we get:
>>
>> CC lm32-softmmu/fpu/softfloat.o
>> fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
>> ^~
>>
>> fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
>>^
>>
>> fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
>>^
>>
>> fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
>> ^~
>>
>> fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
>> ^~~
>>
>> fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) &
>> roundNearestEven );
>>
>> ^
>>
>> fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) &
>> roundNearestEven );
>>
>> ^
>>
>> fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression;
>> did you mean logical negation? [-Werror,-Wbool-operation]
>> zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
>>^~~
>>
>> Fix by rewriting the fishy bitwise AND of two bools as an int.
>>
>> Suggested-by: Eric Blake
>> Buglink: https://bugs.launchpad.net/bugs/1881004
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> v2: Resend without the Cc: "Toni Wilen " tag
>> ---
>> fpu/softfloat.c | 33 -
>> 1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 6c8f2d597a..0dd57eddd7 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -3362,7 +3362,9 @@ static int32_t roundAndPackInt32(bool zSign, uint64_t
>> absZ,
>> }
>> roundBits = absZ & 0x7F;
>> absZ = ( absZ + roundIncrement )>>7;
>> -absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
>> +if (((roundBits ^ 0x40) == 0) && roundNearestEven) {
>> +absZ &= ~1;
>> +}
>
> You could get rid of some more parentheses now:
>
>if ((roundBits ^ 0x40) == 0 && roundNearestEven)
>
> ... also in the other hunks.
I first wrote
if (!(roundBits ^ 0x40) && roundNearestEven)
But then thought this would diverge from Eric suggestion, so I kept what
he wrote (which is a bit closer to the style of rest of this file).
>
> Anyway:
> Reviewed-by: Thomas Huth
>
Thanks!
Re: [PATCH v2 2/9] block: inline bdrv_unallocated_blocks_are_zero()
07.05.2020 17:08, Eric Blake wrote:
On 5/7/20 3:47 AM, Vladimir Sementsov-Ogievskiy wrote:
The function has the only user: bdrv_co_block_status(). Inline it to
s/the only/only one/
simplify reviewing of the following patches, which will finally drop
unallocated_blocks_are_zero field too.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
include/block/block.h | 1 -
block.c | 15 ---
block/io.c | 11 ---
3 files changed, 8 insertions(+), 19 deletions(-)
+++ b/block/io.c
@@ -2386,15 +2386,20 @@ static int coroutine_fn
bdrv_co_block_status(BlockDriverState *bs,
if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
ret |= BDRV_BLOCK_ALLOCATED;
} else if (want_zero) {
- if (bdrv_unallocated_blocks_are_zero(bs)) {
- ret |= BDRV_BLOCK_ZERO;
- } else if (bs->backing) {
+ if (bs->backing) {
BlockDriverState *bs2 = bs->backing->bs;
int64_t size2 = bdrv_getlength(bs2);
if (size2 >= 0 && offset >= size2) {
ret |= BDRV_BLOCK_ZERO;
}
+ } else {
+ BlockDriverInfo bdi;
+ int ret2 = bdrv_get_info(bs, );
+
+ if (ret2 == 0 && bdi.unallocated_blocks_are_zero) {
Could perhaps condense to:
else {
BlockDriverInfo bdi;
if (bdrv_get_info(bs, ) == 0 &&
bdi.unallocated_blocks_are_zero) {
but that's cosmetic.
I'll keep it as is, as it is to be removed in 09 anyway.
Reviewed-by: Eric Blake
--
Best regards,
Vladimir
?????? GDB get wrong debug infos on TI DSP architecture extension
Hi ,
Thanks for the hint.
I have been looking for how QEMU determine the target program terminates by
checking the host implementation (eg i386). I thought the target program
termination is connected to the initial FP pushed to stack, I am not sure this
is how QEMU does. Could you be more specific on the termination check ? is it
target architecture dependent( an added DSP target architecture should deal
with it explicitly)?
Thanks again
xiaolei
-- --
??: "Alex Benn??e";
: 2020??5??28??(??) 4:34
??: "casmac"<1482995...@qq.com>;
: "Philippe
Mathieu-Daud??";"qemu-devel";
: Re: GDB get wrong debug infos on TI DSP architecture extension
casmac <1482995...@qq.com> writes:
> Hi,
Thank you for forwarding my question to developers and sharing the C6x
implementation.
> Perhaps I should follow up with another problem I encountered.
> The senerio is the emulator keeps running eventhough the program it
> emulates has already exited. And it keeps retrieving instructions which are
> all zero "instruction"(0x).
>
> It looks to me that in function cpu_exec(CPUState *cpu), the following loop
> never terminate:
> while (!cpu_handle_exception(cpu, ret)) {
> TranslationBlock *last_tb = NULL;
> int tb_exit = 0;
> while (!cpu_handle_interrupt(cpu, last_tb)) { ... }
> Is it because cpu-exit_request remains 0 ?
>
> At what point should we make cpu-exit_request=1 ?
cpu->exit_request is set for asynchronus conditions (e.g. timer IRQs or
IO events). A number of helpers will "kick" the cpu by calling
cpu_exit().
> Thanks again!!
>
>
> regards
> xiaolei
>
>
>
Re: [RFC] some semihosting interrogation
Hi Fred,
On 5/28/20 11:44 AM, Fred Konrad wrote:
> Hi all,
>
> Just wonderring if there is any reason not to be able to defer
> qemu_semihosting_connect_chardevs a little more to be able to specify
> chardev=serial0?
>
> Like:
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 6390cf0..9fa1553 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -4333,8 +4333,6 @@ void qemu_init(int argc, char **argv, char **envp)
>
> qemu_opts_foreach(qemu_find_opts("chardev"),
> chardev_init_func, NULL, _fatal);
> - /* now chardevs have been created we may have semihosting to
> connect */
> - qemu_semihosting_connect_chardevs();
>
> #ifdef CONFIG_VIRTFS
> qemu_opts_foreach(qemu_find_opts("fsdev"),
> @@ -4484,6 +4482,9 @@ void qemu_init(int argc, char **argv, char **envp)
> if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
> exit(1);
>
> + /* now chardevs have been created we may have semihosting to
> connect */
> + qemu_semihosting_connect_chardevs();
Cc'ing Markus for this part because he had headaches recently moving
things around there; but the change looks.
> +
> /* If no default VGA is requested, the default is "none". */
> if (default_vga) {
> vga_model = get_default_vga_model(machine_class);
>
> Also I found out that the trailing \0 is sent to the chardev
> (console.c:copy_user_string) is that expected in case of semihosting?
No, your patch fixes a bug.
>
> static GString *copy_user_string(CPUArchState *env, target_ulong addr)
> {
> CPUState *cpu = env_cpu(env);
> GString *s = g_string_sized_new(128);
> uint8_t c;
>
> do {
> if (cpu_memory_rw_debug(cpu, addr++, , 1, 0) == 0) {
> if (c) {
> s = g_string_append_c(s, c);
> }
> } else {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: passed inaccessible address " TARGET_FMT_lx,
> __func__, addr);
> break;
> }
> } while (c!=0);
>
> return s;
> }
>
> I can roll out two patches if needed..
>
> Cheers,
> Fred
>
[Bug 1877052] Re: KVM Win 10 guest pauses after kernel upgrade
The warnings in the report like "MSR(48FH).vmx-exit-load-perf-global-ctrl" are unrelated (in regard to guest hang). Those happen on a) too old kernels that don't support the feature b) mismatch of expectations of a chips vs its actual capabilities E.g. if libvirt thinks a feature should be supported by a chip, but isn't. There are toomany SKUs out there to be perfect - so these are red-herrings at best. I have not seen similar reports recently nor anyone else chiming in on this one. After loosing what e thought could be a track to the bgu I'm puzzled what to do now on this? @Andreas - did you in the meantime find any new insight on this? ** Changed in: qemu (Ubuntu) Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1877052 Title: KVM Win 10 guest pauses after kernel upgrade Status in QEMU: New Status in qemu package in Ubuntu: Incomplete Bug description: Hello! Unfortunately the bug has apparently reappeared. I have a Windows 10 running in a VM, which after my today's "apt upgrade" goes into pause mode after a few seconds of running time. Until yesterday it used to work and I was able to boot the VM. During the kernel update (from 5.4.0-28.33 to 5.4.0-29.34) the VM was active and then went into pause mode. Even after a reboot of my host system the problem still persists: the VM boots for a few seconds and then switches to pause mode. Current Kernel: Linux andreas-laptop 5.4.0-29-generic #33-Ubuntu SMP Wed Apr 29 14:32:27 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux Maybe relevant logfile lines: 2020-05-06T07:46:42.857574Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-06T07:46:42.857718Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-06T07:46:42.860567Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-06T07:46:42.860582Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-06T07:47:22.901057Z qemu-system-x86_64: terminating on signal 15 from pid 1593 (/usr/sbin/libvirtd) 2020-05-06 07:47:23.101+: shutting down, reason=destroyed Kind regards, Andreas To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1877052/+subscriptions
[Bug 1877052] Re: KVM Win 10 guest pauses after kernel upgrade
@Andreas - If we find nothing else to try I'll ping you when I have a newer qemu build for Ubuntu 20.10 for you to try. ** Tags added: qemu-20.10 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1877052 Title: KVM Win 10 guest pauses after kernel upgrade Status in QEMU: New Status in qemu package in Ubuntu: Incomplete Bug description: Hello! Unfortunately the bug has apparently reappeared. I have a Windows 10 running in a VM, which after my today's "apt upgrade" goes into pause mode after a few seconds of running time. Until yesterday it used to work and I was able to boot the VM. During the kernel update (from 5.4.0-28.33 to 5.4.0-29.34) the VM was active and then went into pause mode. Even after a reboot of my host system the problem still persists: the VM boots for a few seconds and then switches to pause mode. Current Kernel: Linux andreas-laptop 5.4.0-29-generic #33-Ubuntu SMP Wed Apr 29 14:32:27 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux Maybe relevant logfile lines: 2020-05-06T07:46:42.857574Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-06T07:46:42.857718Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-06T07:46:42.860567Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-06T07:46:42.860582Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-06T07:47:22.901057Z qemu-system-x86_64: terminating on signal 15 from pid 1593 (/usr/sbin/libvirtd) 2020-05-06 07:47:23.101+: shutting down, reason=destroyed Kind regards, Andreas To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1877052/+subscriptions
Re: [PATCH v6 2/5] softmmu/vl: Let -fw_cfg option take a 'blob_id' argument
On 5/20/20 12:34 AM, Laszlo Ersek wrote:
> On 05/19/20 20:20, Philippe Mathieu-Daudé wrote:
>> The 'blob_id' argument refers to a QOM object able to produce
>> data consumable by the fw_cfg device. The producer object must
>> implement the FW_CFG_DATA_GENERATOR interface.
>
> OK, this answers my OBJECT_CHECK() question under patch #1 (in the
> negative -- an assert would be wrong).
>
>>
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> softmmu/vl.c | 17 +
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index ae5451bc23..f76c53ad2e 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -489,6 +489,10 @@ static QemuOptsList qemu_fw_cfg_opts = {
>> .name = "string",
>> .type = QEMU_OPT_STRING,
>> .help = "Sets content of the blob to be inserted from a string",
>> +}, {
>> +.name = "blob_id",
>> +.type = QEMU_OPT_STRING,
>> +.help = "Sets id of the object generating fw_cfg blob to be
>> used",
>> },
>> { /* end of list */ }
>> },
>> @@ -2020,7 +2024,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
>> Error **errp)
>> {
>> gchar *buf;
>> size_t size;
>> -const char *name, *file, *str;
>> +const char *name, *file, *str, *blob_id;
>> FWCfgState *fw_cfg = (FWCfgState *) opaque;
>>
>> if (fw_cfg == NULL) {
>> @@ -2030,14 +2034,17 @@ static int parse_fw_cfg(void *opaque, QemuOpts
>> *opts, Error **errp)
>> name = qemu_opt_get(opts, "name");
>> file = qemu_opt_get(opts, "file");
>> str = qemu_opt_get(opts, "string");
>> +blob_id = qemu_opt_get(opts, "blob_id");
>>
>> /* we need name and either a file or the content string */
>
> (1) Please update this comment. If the option is given, we need the
> name, and exactly one of: file, content string, blob_id.
>
>> -if (!(nonempty_str(name) && (nonempty_str(file) || nonempty_str(str
>> {
>> +if (!(nonempty_str(name)
>> + && (nonempty_str(file) || nonempty_str(str) ||
>> nonempty_str(blob_id)))
>> + ) {
>> error_setg(errp, "invalid argument(s)");
>> return -1;
>> }
>
> (2) Coding style: does QEMU keep operators on the left or on the right
> when breaking subconditions to new lines? (I vaguely recall "to the
> right", but I could be wrong... Well, "hw/nvram/fw_cfg.c" has at least 7
> examples of the operator being on the right.)
You are right, I have been confused with a recommendation from Eric
Blake, but it was about shell script, not C.
>
>> -if (nonempty_str(file) && nonempty_str(str)) {
>> -error_setg(errp, "file and string are mutually exclusive");
>> +if (nonempty_str(file) && nonempty_str(str) && nonempty_str(blob_id)) {
>> +error_setg(errp, "file, string and blob_id are mutually exclusive");
>> return -1;
>> }
>
> (3) I believe this catches only when all three of name/string/blob_id
> are given. But we should continue catching "two given".
>
> How about reworking both "if"s, *and* the comment at (1) at the same
> time, into:
>
> if (!nonempty_str(name) ||
> nonempty_str(file) + nonempty_str(str) + nonempty_str(blob_id) != 1) {
> error_setg(errp, "name, plus exactly one of file, string and blob_id,
> "
>"are needed");
> return -1;
> }
>
> (Regarding the addition, nonempty_str() returns a "bool", which is a
> macro to _Bool, which is promoted to "int" or "unsigned int".)
>
>> if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
>> @@ -2052,6 +2059,8 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
>> Error **errp)
>> if (nonempty_str(str)) {
>> size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
>> buf = g_memdup(str, size);
>> +} else if (nonempty_str(blob_id)) {
>> +return fw_cfg_add_from_generator(fw_cfg, name, blob_id, errp);
>> } else {
>> GError *err = NULL;
>> if (!g_file_get_contents(file, , , )) {
>>
>
> (4) The "-fw_cfg" command line option is documented in both the qemu(1)
> manual, and the "docs/specs/fw_cfg.txt" file.
>
> I think we may have to update those. In particular I mean *where* the
> option is documented (in both texts).
Oh I forgot this part, thanks.
>
> In the manual, "-fw_cfg" is currently under "Debug/Expert options", but
> that will no longer apply (I think?) after this series.
Well I'm not sure, the intent is the same, targeting mostly libvirt as
management interface; other uses are for "experts".
>
> Similarly, in "docs/specs/fw_cfg.txt", the section is called "Externally
> Provided Items" -- but that might not be strictly true any more either.
>
> Maybe leave the current "-fw_cfg" mentions in peace, and document
> "-fw_cfg blob_id=..." separately (in different docs sections)? The
> "fw_cfg generators" concept could deserve dedicated sections.
>
>
Re: [PATCH 0/2] linux-user: Load a vdso for x86_64
On Tue, 19 May 2020 at 20:45, Richard Henderson wrote: > Makefile | 4 +- > linux-user/elfload.c | 203 +- > pc-bios/Makefile | 5 + > pc-bios/vdso-linux-x64.S | 115 + > pc-bios/vdso-linux-x64.ld | 81 +++ > pc-bios/vdso-linux-x64.so | Bin 0 -> 7500 bytes I'm not really a fan of binaries in source control :-( thanks -- PMM
Re: [PULL 00/15] ppc-for-5.1 queue 20200527
On Wed, 27 May 2020 at 06:38, David Gibson wrote: > > The following changes since commit ddc760832fa8cf5e93b9d9e6e854a5114ac63510: > > Merge remote-tracking branch 'remotes/gkurz/tags/9p-next-2020-05-26' into > staging (2020-05-26 14:05:53 +0100) > > are available in the Git repository at: > > git://github.com/dgibson/qemu.git tags/ppc-for-5.1-20200527 > > for you to fetch changes up to 9c7c0407028355ca83349b8a60fddfad46f2ebd8: > > vfio/nvlink: Remove exec permission to avoid SELinux AVCs (2020-05-27 > 15:29:36 +1000) > > > ppc patch queue 2020-05-27 > > Here's the next pull request for qemu-5.1. It includes: > * Support for the scv and rfscv POWER9 instructions in TCG > * Support for the new SPAPR_LMB_FLAGS_HOTREMOVABLE flag, which >provides a way for guests to know memory which should be removable >(so the guest can avoid putting immovable allocations there). > * Some fixes for the recently added partition scope radix translation >in softmmu > * Assorted minor fixes and cleanups > > It includes one patch to avoid a clash with SELinux when using NVLink > VFIO devices. That's not technically within the files under my > maintainership, but it is in a section of the VFIO quirks code that's > specific to the POWER-only NVLink devices, and has an ack from Alex > Williamson. Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
[PATCH v4 3/5] block/io: bdrv_common_block_status_above: support bs == base
We are going to reuse bdrv_common_block_status_above in
bdrv_is_allocated_above. bdrv_is_allocated_above may be called with
include_base == false and still bs == base (for ex. from img_rebase()).
So, support this corner case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Kevin Wolf
Reviewed-by: Eric Blake
---
block/io.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/block/io.c b/block/io.c
index ae1c806720..6f90b322f4 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2458,7 +2458,11 @@ static int coroutine_fn
bdrv_co_block_status_above(BlockDriverState *bs,
int ret = 0;
bool first = true;
-assert(include_base || bs != base);
+if (!include_base && bs == base) {
+*pnum = bytes;
+return 0;
+}
+
for (p = bs; include_base || p != base; p = backing_bs(p)) {
ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
file);
--
2.18.0
[PATCH v4 5/5] iotests: add commit top->base cases to 274
These cases are fixed by previous patches around block_status and
is_allocated.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
---
tests/qemu-iotests/274 | 20
tests/qemu-iotests/274.out | 65 ++
2 files changed, 85 insertions(+)
diff --git a/tests/qemu-iotests/274 b/tests/qemu-iotests/274
index 5d1bf34dff..e910455f13 100755
--- a/tests/qemu-iotests/274
+++ b/tests/qemu-iotests/274
@@ -115,6 +115,26 @@ with iotests.FilePath('base') as base, \
iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid)
iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff), mid)
+iotests.log('=== Testing qemu-img commit (top -> base) ===')
+
+create_chain()
+iotests.qemu_img_log('commit', '-b', base, top)
+iotests.img_info_log(base)
+iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, base)
+iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff),
base)
+
+iotests.log('=== Testing QMP active commit (top -> base) ===')
+
+create_chain()
+with create_vm() as vm:
+vm.launch()
+vm.qmp_log('block-commit', device='top', base_node='base',
+ job_id='job0', auto_dismiss=False)
+vm.run_job('job0', wait=5)
+
+iotests.img_info_log(mid)
+iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, base)
+iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff),
base)
iotests.log('== Resize tests ==')
diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out
index d24ff681af..9806dea8b6 100644
--- a/tests/qemu-iotests/274.out
+++ b/tests/qemu-iotests/274.out
@@ -129,6 +129,71 @@ read 1048576/1048576 bytes at offset 0
read 1048576/1048576 bytes at offset 1048576
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+=== Testing qemu-img commit (top -> base) ===
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536
lazy_refcounts=off refcount_bits=16 compression_type=zlib
+
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576
backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off
refcount_bits=16 compression_type=zlib
+
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152
backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off
refcount_bits=16 compression_type=zlib
+
+wrote 2097152/2097152 bytes at offset 0
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Image committed.
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 2 MiB (2097152 bytes)
+cluster_size: 65536
+Format specific information:
+compat: 1.1
+compression type: zlib
+lazy refcounts: false
+refcount bits: 16
+corrupt: false
+
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing QMP active commit (top -> base) ===
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=2097152 cluster_size=65536
lazy_refcounts=off refcount_bits=16 compression_type=zlib
+
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 size=1048576
backing_file=TEST_DIR/PID-base cluster_size=65536 lazy_refcounts=off
refcount_bits=16 compression_type=zlib
+
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 size=2097152
backing_file=TEST_DIR/PID-mid cluster_size=65536 lazy_refcounts=off
refcount_bits=16 compression_type=zlib
+
+wrote 2097152/2097152 bytes at offset 0
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+{"execute": "block-commit", "arguments": {"auto-dismiss": false, "base-node":
"base", "device": "top", "job-id": "job0"}}
+{"return": {}}
+{"execute": "job-complete", "arguments": {"id": "job0"}}
+{"return": {}}
+{"data": {"device": "job0", "len": 1048576, "offset": 1048576, "speed": 0,
"type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds":
"USECS", "seconds": "SECS"}}
+{"data": {"device": "job0", "len": 1048576, "offset": 1048576, "speed": 0,
"type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp":
{"microseconds": "USECS", "seconds": "SECS"}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1 MiB (1048576 bytes)
+cluster_size: 65536
+backing file: TEST_DIR/PID-base
+Format specific information:
+compat: 1.1
+compression type: zlib
+lazy refcounts: false
+refcount bits: 16
+corrupt: false
+
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
== Resize tests ==
=== preallocation=off ===
Formatting 'TEST_DIR/PID-base', fmt=qcow2 size=6442450944 cluster_size=65536
lazy_refcounts=off refcount_bits=16 compression_type=zlib
--
2.18.0
Re: [PATCH 0/2] linux-user: Load a vdso for x86_64
Le 28/05/2020 à 12:08, Peter Maydell a écrit : > On Tue, 19 May 2020 at 20:45, Richard Henderson > wrote: >> Makefile | 4 +- >> linux-user/elfload.c | 203 +- >> pc-bios/Makefile | 5 + >> pc-bios/vdso-linux-x64.S | 115 + >> pc-bios/vdso-linux-x64.ld | 81 +++ >> pc-bios/vdso-linux-x64.so | Bin 0 -> 7500 bytes > > I'm not really a fan of binaries in source control :-( Can't we see that as a firmware or a ROM? It's only 7,4 KB and needs a cross-compilation env to be rebuilt. Do you have another solution? If you don't like this I can remove the series. Let me know. Thanks, Laurent
Re: [PATCH v4 5/6] i386: Hyper-V VMBus ACPI DSDT entry
On Thu, 28 May 2020 08:26:42 +0300 Jon Doron wrote: > On 22/05/2020, Igor Mammedow wrote: > >On Thu, 21 May 2020 18:02:07 +0200 > >Paolo Bonzini wrote: > > > >> On 13/05/20 17:34, Igor Mammedov wrote: > >> > I'd rather avoid using random IRQ numbers (considering we are > >> > dealing with black-box here). So if it's really necessary to have > >> > IRQ described here, I'd suggest to implement them in device model > >> > so they would be reserved and QEMU would error out in a sane way if > >> > IRQ conflict is detected. > >> > >> We don't generally detect ISA IRQ conflicts though, do we? > > > >that I don't know that's why I'm not suggesting how to do it. > >The point is hard-coding in AML random IRQs is not right thing to do, > >(especially with the lack of 'any' spec), as minimum AML should pull > >it from device model and that probably should be configurable and set > >by board. > > > >Other thing is: > >I haven't looked at VMBus device model in detail, but DSDT part aren't > >matching device though (device model is not ISA device hence AML part > >shouldn't be on in ISA scope), where to put it is open question. > >There were other issues with AML code, I've commented on, so I was > >waiting on respin with comments addressed. > >I don't think that this patch is good enough for merging. > > > > > > But it seems like the current patch does match what's Microsoft HyperV > is publishing in it's APCI tables. > > I dont think it's correct for us to "fix" Microsoft emulation even if > it's wrong, since that's what Windows probably expects to see... > > I tried looking where Microsoft uses the ACPI tables to identify the > VMBus but without much luck in order to understand how flexible a change > would be for the OS to still detect the VMBus device, but in general > I think "correcting" something that is emulated 1:1 because there is no > spec is the right way. I'd agree, if removing nonsense would break VMBus detection (does it?). if something is that doesn't make sense but has to stay because it is need to make windows happy, that's fine , just add annotate is with comment, so it won't confuse anyone why that code exists there later on. I suggest to: 1. try dropping _PS* & _STA as it doesn't actually does anything and _PS3 is plain wrong 2. drop one IRQ, newer hyper-v seems to be doing fine with only one 3. it's not ISA device, I'd suggest to move into _SB scope 4. I don't know much about IRQs but git grep DEFINE_PROP_ | grep -i iqr yields nothing so I'm not sure if it's acceptable. Typically it's board that assigns IRQ and not device, for Sysbus devices (see: sysbus_init_irq/sysbus_connect_irq). So I'd leave it upto Paolo or someone else to decide/comment on. > > >> > >> Paolo > >> > > >
Re: How to fuzz devices that use timers?
On 28/05/20 11:52, Christophe de Dinechin wrote: > > Since we run the fuzzer with the QTest accelerator, my first idea was to > check for 'if (qtest_enabled())' in the timer code, and directly expire > a timer instead of scheduling it. This way we can test reproducers. > However various tests require/verify precise timing, so this would break > various qtests. There is a clock_step command that advance the QEMU_CLOCK_VIRTUAL clock to the next deadline. You just have to insert it into the fuzzing input. Paolo
Re: [PATCH 2/3] hw/acpi-build: account for NVDIMM numa nodes in SRAT
On Thu, 28 May 2020 01:24:42 +
"Verma, Vishal L" wrote:
> On Thu, 2020-05-21 at 17:16 +0200, Igor Mammedov wrote:
>
> Hi Igor, Thanks for the review.
>
> [..]
> > >
> > > @@ -2429,6 +2430,25 @@ build_srat(GArray *table_data, BIOSLinker *linker,
> > > MachineState *machine)
> > >MEM_AFFINITY_ENABLED);
> > > }
> > > }
> > > +
> > > +if (machine->nvdimms_state->is_enabled) {
> > > +GSList *device_list = nvdimm_get_device_list();
> > > +
> > > +for (; device_list; device_list = device_list->next) {
> > > +DeviceState *dev = device_list->data;
> > > +int node = object_property_get_int(OBJECT(dev),
> > > PC_DIMM_NODE_PROP,
> > > + NULL);
> > > +uint64_t addr = object_property_get_uint(OBJECT(dev),
> > > + PC_DIMM_ADDR_PROP,
> > > NULL);
> > > +uint64_t size = object_property_get_uint(OBJECT(dev),
> > > + PC_DIMM_SIZE_PROP,
> > > NULL);
> > > +
> > suggest to use error_abort in getters
>
> Yep, fixed in v2.
>
> >
> > > +numamem = acpi_data_push(table_data, sizeof *numamem);
> > > +build_srat_memory(numamem, addr, size, node,
> > > + MEM_AFFINITY_ENABLED |
> > > MEM_AFFINITY_NON_VOLATILE);
> > > +}
> > who is in charge of freeing device_list ?
>
> Thanks, also fixed in v2.
>
> >
> > > +}
> >
> > There is ARM version of build_srat(),
> > I suggest to put this NVDIMM specific part in helper function within
> > hw/acpi/nvdimm.c
> > and use it from both build_srat() functions.
>
> Splitting the work out into a helper function in nvdimm.c does make
> sense, and I've done that. However, looking at the arm version of
> build_srat and generally in virt-acpi-build.c, I don't see any NVDIMM
> support, so unless I'm mistaken, it wouldn't make sense to actually call
> this from the arm version of build_srat.
perhaps you are lookin into old version on QEMU
current HEAD has followin snippet:
virt-acpi-build.c:
if (ms->nvdimms_state->is_enabled) {
nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
ms->nvdimms_state, ms->ram_slots);
}
>
> I'll send a v2 with the above fixes.
>
> >
> > > +
> > > slots = (table_data->len - numa_start) / sizeof *numamem;
> > > for (; slots < pcms->numa_nodes + 2; slots++) {
> > > numamem = acpi_data_push(table_data, sizeof *numamem);
Re: [PATCH] linux-user: implement OFD locks
Le 25/05/2020 à 09:59, Andreas Schwab a écrit :
> Signed-off-by: Andreas Schwab
> ---
> linux-user/generic/fcntl.h | 4
> linux-user/syscall.c | 6 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/linux-user/generic/fcntl.h b/linux-user/generic/fcntl.h
> index 9f727d4df2..c85c5b9fed 100644
> --- a/linux-user/generic/fcntl.h
> +++ b/linux-user/generic/fcntl.h
> @@ -99,6 +99,10 @@
> #define TARGET_F_SETLKW64 14
> #endif
>
> +#define TARGET_F_OFD_GETLK 36
> +#define TARGET_F_OFD_SETLK 37
> +#define TARGET_F_OFD_SETLKW38
> +
> #ifndef TARGET_F_SETOWN_EX
> #define TARGET_F_SETOWN_EX 15
> #define TARGET_F_GETOWN_EX 16
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7eac6b7d47..492450e77d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6097,6 +6097,9 @@ static int target_to_host_fcntl_cmd(int cmd)
> case TARGET_F_SETFD:
> case TARGET_F_GETFL:
> case TARGET_F_SETFL:
> +case TARGET_F_OFD_GETLK:
> +case TARGET_F_OFD_SETLK:
> +case TARGET_F_OFD_SETLKW:
> ret = cmd;
> break;
> case TARGET_F_GETLK:
> @@ -6382,6 +6385,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> break;
>
> case TARGET_F_GETLK64:
> +case TARGET_F_OFD_GETLK:
> ret = copy_from_user_flock64(, arg);
> if (ret) {
> return ret;
> @@ -6393,6 +6397,8 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> break;
> case TARGET_F_SETLK64:
> case TARGET_F_SETLKW64:
> +case TARGET_F_OFD_SETLK:
> +case TARGET_F_OFD_SETLKW:
> ret = copy_from_user_flock64(, arg);
> if (ret) {
> return ret;
>
Reviewed-by: Laurent Vivier
[PATCH v2 06/24] armv7m: Delete unused "ARM,bitband-memory" devices
These devices are optional, and enabled by property "enable-bitband".
armv7m_instance_init() creates them unconditionally, because the
property has not been set then. armv7m_realize() realizes them only
when the property is true. Works, although it leaves unrealized
devices hanging around in the QOM composition tree. Affects machines
microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.
Delete the unused devices by making armv7m_realize() unparent them.
Visible in "info qom-tree"; here's the change for microbit:
/machine (microbit-machine)
/microbit.twi (microbit.i2c)
/microbit.twi[0] (qemu:memory-region)
/nrf51 (nrf51-soc)
/armv6m (armv7m)
/armv7m-container[0] (qemu:memory-region)
- /bitband[0] (ARM,bitband-memory)
-/bitband[0] (qemu:memory-region)
- /bitband[1] (ARM,bitband-memory)
-/bitband[0] (qemu:memory-region)
/cpu (cortex-m0-arm-cpu)
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster
---
hw/arm/armv7m.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 7da57f56d3..f930619f53 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -245,8 +245,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(>container, 0xe000e000,
sysbus_mmio_get_region(sbd, 0));
-if (s->enable_bitband) {
-for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+if (s->enable_bitband) {
Object *obj = OBJECT(>bitband[i]);
SysBusDevice *sbd = SYS_BUS_DEVICE(>bitband[i]);
@@ -265,6 +265,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(>container, bitband_output_addr[i],
sysbus_mmio_get_region(sbd, 0));
+} else {
+object_unparent(OBJECT(>bitband[i]));
}
}
}
--
2.21.3
[PATCH v2 13/24] ppc4xx: Drop redundant device realization
object_property_set_bool(OBJECT(dev), true, "realized", ...) right after qdev_init_nofail(dev) does nothing, because qdev_init_nofail() already realizes. Drop. Cc: BALATON Zoltan Signed-off-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: BALATON Zoltan Reviewed-by: Thomas Huth --- hw/ppc/ppc440_uc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c index b30e093cbb..dc318c7aa7 100644 --- a/hw/ppc/ppc440_uc.c +++ b/hw/ppc/ppc440_uc.c @@ -1370,12 +1370,10 @@ void ppc460ex_pcie_init(CPUPPCState *env) dev = qdev_create(NULL, TYPE_PPC460EX_PCIE_HOST); qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE0_BASE); qdev_init_nofail(dev); -object_property_set_bool(OBJECT(dev), true, "realized", NULL); ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), env); dev = qdev_create(NULL, TYPE_PPC460EX_PCIE_HOST); qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE1_BASE); qdev_init_nofail(dev); -object_property_set_bool(OBJECT(dev), true, "realized", NULL); ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), env); } -- 2.21.3
[PATCH v2 03/24] sd/pxa2xx_mmci: Fix to realize "pxa2xx-mmci" device
pxa2xx_mmci_init() creates a "pxa2xx-mmci" device, but neglects to realize it. Affects machines akita, borzoi, connex, mainstone, spitz, terrier, tosa, verdex, and z2. In theory, a device becomes real only on realize. In practice, the transition from unreal to real is a fuzzy one. The work to make a device real can be spread between realize methods (fine), instance_init methods (wrong), and board code wiring up the device (fine as long as it effectively happens on realize). Depending on what exactly is done where, a device can work even when we neglect to realize it. This one appears to work. Nevertheless, it's a clear misuse of the interface. Even when it works today (more or less by chance), it can break tomorrow. Fix by realizing it right away. Visible in "info qom-tree"; here's the change for akita: /machine (akita-machine) [...] /unattached (container) [...] +/device[5] (pxa2xx-mmci) + /pxa2xx-mmci[0] (qemu:memory-region) + /sd-bus (pxa2xx-mmci-bus) [rest of device[*] renumbered...] Fixes: 7a9468c92517e19037bfe2272f64f5dadaf9db15 Cc: Andrzej Zaborowski Cc: Peter Maydell Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell --- hw/sd/pxa2xx_mmci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index f9c50ddda5..c32df1b8f9 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -492,6 +492,7 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, sysbus_connect_irq(sbd, 0, irq); qdev_connect_gpio_out_named(dev, "rx-dma", 0, rx_dma); qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma); +qdev_init_nofail(dev); /* Create and plug in the sd card */ carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD); -- 2.21.3
[PATCH v2 24/24] qdev: Assert onboard devices all get realized properly
This would have caught some of the bugs I just fixed.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
hw/core/qdev.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b5b42b2616..a68ba674db 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -427,6 +427,19 @@ void qdev_init_nofail(DeviceState *dev)
object_unref(OBJECT(dev));
}
+static int qdev_assert_realized_properly(Object *obj, void *opaque)
+{
+DeviceState *dev = DEVICE(object_dynamic_cast(obj, TYPE_DEVICE));
+DeviceClass *dc;
+
+if (dev) {
+dc = DEVICE_GET_CLASS(dev);
+assert(dev->realized);
+assert(dev->parent_bus || !dc->bus_type);
+}
+return 0;
+}
+
void qdev_machine_creation_done(void)
{
/*
@@ -434,6 +447,9 @@ void qdev_machine_creation_done(void)
* only create hotpluggable devices
*/
qdev_hotplug = true;
+
+object_child_foreach_recursive(object_get_root(),
+ qdev_assert_realized_properly, NULL);
}
bool qdev_machine_modified(void)
--
2.21.3
[PATCH v2 05/24] arm/aspeed: Rework NIC attachment
From: Cédric Le Goater
The number of MACs supported by an Aspeed SoC is defined by "macs_num"
under the SoC model, that is two for the AST2400 and AST2500 and four
for the AST2600. The model initializes the maximum number of supported
MACs but the number of realized devices is capped by the number of
network device back-ends defined on the command line. This can leave
unrealized devices hanging around in the QOM composition tree.
To get virtual hardware that matches the physical hardware, you have
to pass exactly as many -nic options as there are MACs, and some of
them must be -nic none:
* Machines ast2500-evb, palmetto-bmc, romulus-bmc, sonorapass-bmc,
swift-bmc, and witherspoon-bmc: two -nic, and the second one must be
-nic none.
* Machine ast2600-evb: four -nic, the first one must be -nic none.
* Machine tacoma-bmc: four nic, the first two and the last one must be
-nic none.
Modify the machine initialization to define which MACs are attached to
a network device back-end using a bit-field property "macs-mask" and
let the SoC realize all network devices.
The default setting of "macs-mask" is "use MAC0" only, which works for
all our AST2400 and AST2500 machines. The AST2600 machines have
different configurations. The AST2600 EVB machine activates MAC1, MAC2
and MAC3 and the Tacoma BMC machine activates MAC2.
Incompatible CLI change: -nic options now apply to *active* MACs:
MAC1, MAC2, MAC3 for ast2600-evb, MAC2 for tacoma-bmc, and MAC0 for
all the others.
The machines now always get all MACs as they should. Visible in "info
qom-tree", here's the change for tacoma-bmc:
/machine (tacoma-bmc-machine)
/peripheral (container)
/peripheral-anon (container)
/soc (ast2600-a1)
[...]
/ftgmac100[0] (ftgmac100)
/ftgmac100[0] (qemu:memory-region)
/ftgmac100[1] (ftgmac100)
+ /ftgmac100[0] (qemu:memory-region)
/ftgmac100[2] (ftgmac100)
+ /ftgmac100[0] (qemu:memory-region)
/ftgmac100[3] (ftgmac100)
+ /ftgmac100[0] (qemu:memory-region)
[...]
/mii[0] (aspeed-mmi)
/aspeed-mmi[0] (qemu:memory-region)
/mii[1] (aspeed-mmi)
+ /aspeed-mmi[0] (qemu:memory-region)
/mii[2] (aspeed-mmi)
+ /aspeed-mmi[0] (qemu:memory-region)
/mii[3] (aspeed-mmi)
+ /aspeed-mmi[0] (qemu:memory-region)
Also visible in "info qtree"; here's the change for tacoma-bmc:
dev: ftgmac100, id ""
gpio-out "sysbus-irq" 1
aspeed = true
-mac = "52:54:00:12:34:56"
-netdev = "hub0port0"
+mac = "52:54:00:12:34:57"
+netdev = ""
mmio 1e66/2000
dev: ftgmac100, id ""
-aspeed = false
-mac = "00:00:00:00:00:00"
+gpio-out "sysbus-irq" 1
+aspeed = true
+mac = "52:54:00:12:34:58"
netdev = ""
+mmio 1e68/2000
dev: ftgmac100, id ""
-aspeed = false
-mac = "00:00:00:00:00:00"
-netdev = ""
+gpio-out "sysbus-irq" 1
+aspeed = true
+mac = "52:54:00:12:34:56"
+netdev = "hub0port0"
+mmio 1e67/2000
dev: ftgmac100, id ""
-aspeed = false
-mac = "00:00:00:00:00:00"
+gpio-out "sysbus-irq" 1
+aspeed = true
+mac = "52:54:00:12:34:59"
netdev = ""
+mmio 1e69/2000
[...]
dev: aspeed-mmi, id ""
mmio 1e65/0008
dev: aspeed-mmi, id ""
+mmio 1e650008/0008
dev: aspeed-mmi, id ""
+mmio 1e650010/0008
dev: aspeed-mmi, id ""
+mmio 1e650018/0008
Inactive MACs will have no peer and QEMU may warn the user with :
qemu-system-arm: warning: nic ftgmac100.0 has no peer
qemu-system-arm: warning: nic ftgmac100.1 has no peer
qemu-system-arm: warning: nic ftgmac100.3 has no peer
Signed-off-by: Cédric Le Goater
Reviewed-by: Markus Armbruster
Message-Id: <20200527124406.329503-1-...@kaod.org>
Reviewed-by: Joel Stanley
[Commit message expanded]
Signed-off-by: Markus Armbruster
---
include/hw/arm/aspeed.h | 6 ++
hw/arm/aspeed.c | 13 +
hw/arm/aspeed_ast2600.c | 3 +--
hw/arm/aspeed_soc.c | 3 +--
4 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 18521484b9..95b4daece8 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -23,6 +23,11 @@ typedef struct AspeedMachine {
bool mmio_exec;
} AspeedMachine;
+#define ASPEED_MAC0_ON (1 << 0)
+#define ASPEED_MAC1_ON (1 << 1)
+#define ASPEED_MAC2_ON (1 << 2)
+#define ASPEED_MAC3_ON (1 << 3)
+
#define ASPEED_MACHINE_CLASS(klass) \
OBJECT_CLASS_CHECK(AspeedMachineClass, (klass), TYPE_ASPEED_MACHINE)
#define
[PATCH v2 15/24] macio: Fix macio-bus to be a subtype of System bus
The devices we plug into the macio-bus are all sysbus devices
(DeviceClass member bus_type is TYPE_SYSTEM_BUS), but macio-bus does
not derive from TYPE_SYSTEM_BUS. Fix that.
"info qtree" now shows the devices' mmio ranges, as it should
Cc: Mark Cave-Ayland
Cc: David Gibson
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
hw/misc/macio/macio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index ebc96cc8f6..53a9fd5696 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -492,7 +492,7 @@ static void macio_class_init(ObjectClass *klass, void *data)
static const TypeInfo macio_bus_info = {
.name = TYPE_MACIO_BUS,
-.parent = TYPE_BUS,
+.parent = TYPE_SYSTEM_BUS,
.instance_size = sizeof(MacIOBusState),
};
--
2.21.3
[PATCH v2 14/24] macio: Put "macio-nvram" device on the macio bus
macio_oldworld_init() creates a "macio-nvram", sysbus device, but neglects to but it on a bus. Put it on the macio bus. Affects machine g3beige. Visible in "info qtree": bus: macio.0 type macio-bus [...] + dev: macio-nvram, id "" +size = 8192 (0x2000) +it_shift = 4 (0x4) This also makes it a QOM child of macio-oldworld. Visible in "info qom-tree": /machine (g3beige-machine) [...] /unattached (container) [...] /device[6] (macio-oldworld) [...] -/device[7] (macio-nvram) - /macio-nvram[0] (qemu:memory-region) + /nvram (macio-nvram) +/macio-nvram[0] (qemu:memory-region) [rest of device[*] renumbered...] Cc: Mark Cave-Ayland Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Mark Cave-Ayland --- hw/misc/macio/macio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index b3dddf8be7..ebc96cc8f6 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -245,7 +245,8 @@ static void macio_oldworld_init(Object *obj) macio_init_child_obj(s, "cuda", >cuda, sizeof(s->cuda), TYPE_CUDA); -object_initialize(>nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM); +macio_init_child_obj(s, "nvram", >nvram, sizeof(os->nvram), + TYPE_MACIO_NVRAM); dev = DEVICE(>nvram); qdev_prop_set_uint32(dev, "size", 0x2000); qdev_prop_set_uint32(dev, "it_shift", 4); -- 2.21.3
Re: [PATCH v2 04/11] tests/acceptance: add kernel record/replay test for x86_64
Philippe Mathieu-Daudé writes:
> On 5/28/20 9:12 AM, Pavel Dovgalyuk wrote:
>>
>> On 27.05.2020 17:53, Philippe Mathieu-Daudé wrote:
>>> On 5/27/20 12:31 PM, Pavel Dovgalyuk wrote:
This patch adds a test for record/replay an execution of x86_64 machine.
Execution scenario includes simple kernel boot, which allows testing
basic hardware interaction in RR mode.
Signed-off-by: Pavel Dovgalyuk
---
0 files changed
diff --git a/tests/acceptance/replay_kernel.py
b/tests/acceptance/replay_kernel.py
index b8b277ad2f..c7526f1aba 100644
--- a/tests/acceptance/replay_kernel.py
+++ b/tests/acceptance/replay_kernel.py
@@ -55,3 +55,19 @@ class ReplayKernel(LinuxKernelUtils):
True, shift, args)
self.run_vm(kernel_path, kernel_command_line, console_pattern,
False, shift, args)
+
+def test_x86_64_pc(self):
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:pc
+"""
+kernel_url =
('https://archives.fedoraproject.org/pub/archive/fedora'
+
'/linux/releases/29/Everything/x86_64/os/images/pxeboot'
+ '/vmlinuz')
+kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
+kernel_path = self.fetch_asset(kernel_url,
asset_hash=kernel_hash)
+
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE +
'console=ttyS0'
+console_pattern = 'Kernel command line: %s' %
kernel_command_line
+
+self.run_rr(kernel_path, kernel_command_line, console_pattern)
>>> This one timeouted (I build with --enable-debug):
>>
>> I've got the strange behavior for the couple of times.
>>
>> Console output was correct (I saw 'Kernel command line' in logs), but
>> _console_interation function didn't notice it.
>>
>> Therefore the test finished with timeout.
>>
>> How this could be possible?
>
> IIRC there is a problem in how Avocado consume the chardev output.
Is this the same problem Robert has tried to work around in tests/vm?
From: Robert Foley
Subject: [PATCH v7 12/12] tests/vm: Add workaround to consume console
Date: Tue, 19 May 2020 09:22:59 -0400
Message-Id: <20200519132259.405-13-robert.fo...@linaro.org>
>
> Cleber has been working on some PoC / kludge.
>
> Cleber/Eduardo do you remember the problem?
>
>>
>>> (1/1) tests/acceptance/replay_kernel.py:ReplayKernel.test_x86_64_pc:
>>> replay: recording...
>>> replay: replaying...
>>> INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
>>> reached\nOriginal status: ERROR\n{'name':
>>> '1-tests/acceptance/replay_kernel.py:ReplayKernel.test_x86_64_pc',
>>> 'logdir':
>>> 'avocado/job-results/job-2020-05-27T16.48-71d7bf4/test-results/1-tes...
>>> (90.68 s)
>>>
>>
--
Alex Bennée
[PATCH v2 20/24] riscv: Fix type of SiFive[EU]SocState, member parent_obj
Device "riscv.sifive.e.soc" is a direct subtype of TYPE_DEVICE, but
its instance struct SiFiveESoCState's member @parent_obj is
SysBusDevice instead of DeviceState. Correct that.
Same for "riscv.sifive.u.soc"'s instance struct SiFiveUSoCState.
Cc: Palmer Dabbelt
Cc: Alistair Francis
Cc: Sagar Karandikar
Cc: Bastian Koppelmann
Cc: qemu-ri...@nongnu.org
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
---
include/hw/riscv/sifive_e.h | 2 +-
include/hw/riscv/sifive_u.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 25ce7aa9d5..f05644df7c 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -29,7 +29,7 @@
typedef struct SiFiveESoCState {
/*< private >*/
-SysBusDevice parent_obj;
+DeviceState parent_obj;
/*< public >*/
RISCVHartArrayState cpus;
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 16c297ec5f..5f62cf5f85 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -31,7 +31,7 @@
typedef struct SiFiveUSoCState {
/*< private >*/
-SysBusDevice parent_obj;
+DeviceState parent_obj;
/*< public >*/
CPUClusterState e_cluster;
--
2.21.3
Re: [PATCH v2 18/24] display/sm501 display/ati: Fix to realize "i2c-ddc"
Just adding Huacai, the original author and the new maintainer for Fuloong 2E machine. 1:04 PM Čet, 28.05.2020. Markus Armbruster је написао/ла: > > sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but > neglect to realize it. Affects machines sam460ex, shix, r2d, and > fulong2e. > > In theory, a device becomes real only on realize. In practice, the > transition from unreal to real is a fuzzy one. The work to make a > device real can be spread between realize methods (fine), > instance_init methods (wrong), and board code wiring up the device > (fine as long as it effectively happens on realize). Depending on > what exactly is done where, a device can work even when we neglect > to realize it. > > This one appears to work. Nevertheless, it's a clear misuse of the > interface. Even when it works today (more or less by chance), it can > break tomorrow. > > Fix by realizing it right away. Visible in "info qom-tree"; here's > the change for sam460ex: > > /machine (sam460ex-machine) >[...] >/unattached (container) > [...] > -/device[14] (sii3112) > +/device[14] (i2c-ddc) > +/device[15] (sii3112) > [rest of device[*] renumbered...] > > Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a > Fixes: c82c7336de58876862e6b4dccbda29e9240fd388 > Cc: BALATON Zoltan > Cc: qemu-...@nongnu.org > Cc: Magnus Damm > Cc: Philippe Mathieu-Daudé > Cc: Aleksandar Markovic > Signed-off-by: Markus Armbruster > Tested-by: BALATON Zoltan > --- > hw/display/ati.c | 2 ++ > hw/display/sm501.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/hw/display/ati.c b/hw/display/ati.c > index 065f197678..5c71e5f295 100644 > --- a/hw/display/ati.c > +++ b/hw/display/ati.c > @@ -929,6 +929,8 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp) > bitbang_i2c_init(>bbi2c, i2cbus); > I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC)); > i2c_set_slave_address(i2cddc, 0x50); > +object_property_set_bool(OBJECT(i2cddc), true, "realized", > + _abort); > > /* mmio register space */ > memory_region_init_io(>mm, OBJECT(s), _mm_ops, s, > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index acc692531a..fbedc56715 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -1816,6 +1816,8 @@ static void sm501_init(SM501State *s, DeviceState *dev, > /* ddc */ > I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC)); > i2c_set_slave_address(I2C_SLAVE(ddc), 0x50); > +object_property_set_bool(OBJECT(ddc), true, "realized", > + _abort); > > /* mmio */ > memory_region_init(>mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE); > -- > 2.21.3 >
[PATCH v2 04/24] arm/aspeed: Compute the number of CPUs from the SoC definition
From: Cédric Le Goater
Commit ece09beec457 ("aspeed: introduce a configurable number of CPU
per machine") was a convient change during bringup but the Aspeed SoCs
have a fixed number of CPUs : one for the AST2400 and AST2500, and two
for the AST2600.
When the number of CPUs configured with -smp is less than the SoC's
fixed number, the "unconfigured" CPUs are left unrealized. This can
happen for machines ast2600-evb and tacoma-bmc, where the SoC's fixed
number is 2. To get virtual hardware that matches the physical
hardware, you have to pass -smp cpus=2 (or its sugared form -smp 2).
We normally reject -smp cpus=N when N exceeds the machine's limit.
Except we ignore cpus=2 (and only cpus=2) with a warning for machines
ast2500-evb, palmetto-bmc, romulus-bmc, sonorapass-bmc, swift-bmc, and
witherspoon-bmc.
Remove the "num-cpu" property from the SoC state and use the fixed
number of CPUs defined in the SoC class instead. Compute the default,
min, max number of CPUs of the machine directly from the SoC class
definition.
Machines ast2600-evb and tacoma-bmc now always get their second CPU as
they should. Visible in "info qom-tree"; here's the change for
ast2600-evb:
/machine (ast2600-evb-machine)
/peripheral (container)
/peripheral-anon (container)
/soc (ast2600-a1)
/a7mpcore (a15mpcore_priv)
/a15mp-priv-container[0] (qemu:memory-region)
/gic (arm_gic)
/gic_cpu[0] (qemu:memory-region)
/gic_cpu[1] (qemu:memory-region)
+/gic_cpu[2] (qemu:memory-region)
/gic_dist[0] (qemu:memory-region)
/gic_vcpu[0] (qemu:memory-region)
/gic_viface[0] (qemu:memory-region)
/gic_viface[1] (qemu:memory-region)
+/gic_viface[2] (qemu:memory-region)
/unnamed-gpio-in[0] (irq)
[...]
+/unnamed-gpio-in[160] (irq)
[same for 161 to 190...]
+/unnamed-gpio-in[191] (irq)
Also visible in "info qtree"; here's the change for ast2600-evb:
bus: main-system-bus
type System
dev: a15mpcore_priv, id ""
gpio-in "" 128
-gpio-out "sysbus-irq" 5
-num-cpu = 1 (0x1)
+gpio-out "sysbus-irq" 10
+num-cpu = 2 (0x2)
num-irq = 160 (0xa0)
mmio 4046/8000
dev: arm_gic, id ""
-gpio-in "" 160
-num-cpu = 1 (0x1)
+gpio-in "" 192
+num-cpu = 2 (0x2)
num-irq = 160 (0xa0)
revision = 2 (0x2)
has-security-extensions = true
has-virtualization-extensions = true
num-priority-bits = 8 (0x8)
mmio /1000
mmio /2000
mmio /1000
mmio /2000
mmio /0100
+mmio /0100
+mmio /0200
mmio /0200
The other machines now reject -smp cpus=2 just like -smp cpus=3 and up.
Signed-off-by: Cédric Le Goater
Message-Id: <20200519091631.1006073-1-...@kaod.org>
Reviewed-by: Markus Armbruster
[Commit message expanded]
Signed-off-by: Markus Armbruster
---
include/hw/arm/aspeed_soc.h | 1 -
hw/arm/aspeed.c | 29 -
hw/arm/aspeed_ast2600.c | 20 +++-
hw/arm/aspeed_soc.c | 9 +
4 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 78b9f6ae53..914115f3ef 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -40,7 +40,6 @@ typedef struct AspeedSoCState {
/*< public >*/
ARMCPU cpu[ASPEED_CPUS_NUM];
-uint32_t num_cpus;
A15MPPrivState a7mpcore;
MemoryRegion *dram_mr;
MemoryRegion sram;
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 2c23297edf..fd5cc542a5 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -283,8 +283,6 @@ static void aspeed_machine_init(MachineState *machine)
_abort);
object_property_set_int(OBJECT(>soc), amc->num_cs, "num-cs",
_abort);
-object_property_set_int(OBJECT(>soc), machine->smp.cpus, "num-cpus",
-_abort);
object_property_set_link(OBJECT(>soc), OBJECT(>ram_container),
"dram", _abort);
if (machine->kernel_filename) {
@@ -337,7 +335,7 @@ static void aspeed_machine_init(MachineState *machine)
}
}
-if (machine->kernel_filename && bmc->soc.num_cpus > 1) {
+if (machine->kernel_filename && sc->num_cpus > 1) {
/* With no u-boot we must set up a boot stub for the secondary CPU */
MemoryRegion *smpboot = g_new(MemoryRegion, 1);
memory_region_init_ram(smpboot, OBJECT(bmc), "aspeed.smpboot",
@@ -352,7 +350,7 @@
Re: [PULL 00/10] Error reporting patches for 2020-05-27
On Wed, 27 May 2020 at 07:03, Markus Armbruster wrote: > > The following changes since commit ddc760832fa8cf5e93b9d9e6e854a5114ac63510: > > Merge remote-tracking branch 'remotes/gkurz/tags/9p-next-2020-05-26' into > staging (2020-05-26 14:05:53 +0100) > > are available in the Git repository at: > > git://repo.or.cz/qemu/armbru.git tags/pull-error-2020-05-27 > > for you to fetch changes up to 49e2fa85ff04a9be89ed15f922c7d8dae2be9e74: > > i386: Fix x86_cpu_load_model() error API violation (2020-05-27 07:45:45 > +0200) > > > Error reporting patches for 2020-05-27 > > > Markus Armbruster (10): > nvdimm: Plug memory leak in uuid property setter > xen: Fix and improve handling of device_add usb-host errors > s390x/cpumodel: Fix harmless misuse of visit_check_struct() > tests/migration: Tighten error checking > error: Use error_reportf_err() where appropriate > mips/malta: Fix create_cps() error handling > mips/boston: Fix boston_mach_init() error handling > mips/boston: Plug memory leak in boston_mach_init() > arm/sabrelite: Consistently use _fatal in sabrelite_init() > i386: Fix x86_cpu_load_model() error API violation Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
[PATCH v2 19/24] riscv: Fix to put "riscv.hart_array" devices on sysbus
riscv_sifive_e_soc_init(), riscv_sifive_u_soc_init(), spike_board_init(), spike_v1_10_0_board_init(), spike_v1_09_1_board_init(), and riscv_virt_board_init() create "riscv-hart_array" sysbus devices in a way that leaves them unplugged. Create them the common way that puts them into the main system bus. Affects machines sifive_e, sifive_u, spike, spike_v1.10, spike_v1.9.1, and virt. Visible in "info qtree", here's the change for sifive_e: bus: main-system-bus type System + dev: riscv.hart_array, id "" +num-harts = 1 (0x1) +hartid-base = 0 (0x0) +cpu-type = "sifive-e31-riscv-cpu" dev: sifive_soc.gpio, id "" Cc: Palmer Dabbelt Cc: Alistair Francis Cc: Sagar Karandikar Cc: Bastian Koppelmann Cc: qemu-ri...@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Alistair Francis --- hw/riscv/sifive_e.c | 5 ++--- hw/riscv/sifive_u.c | 14 ++ hw/riscv/spike.c| 12 ++-- hw/riscv/virt.c | 4 ++-- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index b53109521e..8831e6728e 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -120,9 +120,8 @@ static void riscv_sifive_e_soc_init(Object *obj) MachineState *ms = MACHINE(qdev_get_machine()); SiFiveESoCState *s = RISCV_E_SOC(obj); -object_initialize_child(obj, "cpus", >cpus, -sizeof(s->cpus), TYPE_RISCV_HART_ARRAY, -_abort, NULL); +sysbus_init_child_obj(obj, "cpus", >cpus, + sizeof(s->cpus), TYPE_RISCV_HART_ARRAY); object_property_set_int(OBJECT(>cpus), ms->smp.cpus, "num-harts", _abort); sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0", diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 4299bdf480..bb69fd8e48 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -491,10 +491,9 @@ static void riscv_sifive_u_soc_init(Object *obj) _abort, NULL); qdev_prop_set_uint32(DEVICE(>e_cluster), "cluster-id", 0); -object_initialize_child(OBJECT(>e_cluster), "e-cpus", ->e_cpus, sizeof(s->e_cpus), -TYPE_RISCV_HART_ARRAY, _abort, -NULL); +sysbus_init_child_obj(OBJECT(>e_cluster), "e-cpus", + >e_cpus, sizeof(s->e_cpus), + TYPE_RISCV_HART_ARRAY); qdev_prop_set_uint32(DEVICE(>e_cpus), "num-harts", 1); qdev_prop_set_uint32(DEVICE(>e_cpus), "hartid-base", 0); qdev_prop_set_string(DEVICE(>e_cpus), "cpu-type", SIFIVE_E_CPU); @@ -504,10 +503,9 @@ static void riscv_sifive_u_soc_init(Object *obj) _abort, NULL); qdev_prop_set_uint32(DEVICE(>u_cluster), "cluster-id", 1); -object_initialize_child(OBJECT(>u_cluster), "u-cpus", ->u_cpus, sizeof(s->u_cpus), -TYPE_RISCV_HART_ARRAY, _abort, -NULL); +sysbus_init_child_obj(OBJECT(>u_cluster), "u-cpus", + >u_cpus, sizeof(s->u_cpus), + TYPE_RISCV_HART_ARRAY); qdev_prop_set_uint32(DEVICE(>u_cpus), "num-harts", ms->smp.cpus - 1); qdev_prop_set_uint32(DEVICE(>u_cpus), "hartid-base", 1); qdev_prop_set_string(DEVICE(>u_cpus), "cpu-type", SIFIVE_U_CPU); diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index d0c4843712..01d52e758e 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -169,8 +169,8 @@ static void spike_board_init(MachineState *machine) unsigned int smp_cpus = machine->smp.cpus; /* Initialize SOC */ -object_initialize_child(OBJECT(machine), "soc", >soc, sizeof(s->soc), -TYPE_RISCV_HART_ARRAY, _abort, NULL); +sysbus_init_child_obj(OBJECT(machine), "soc", >soc, sizeof(s->soc), + TYPE_RISCV_HART_ARRAY); object_property_set_str(OBJECT(>soc), machine->cpu_type, "cpu-type", _abort); object_property_set_int(OBJECT(>soc), smp_cpus, "num-harts", @@ -275,8 +275,8 @@ static void spike_v1_10_0_board_init(MachineState *machine) } /* Initialize SOC */ -object_initialize_child(OBJECT(machine), "soc", >soc, sizeof(s->soc), -TYPE_RISCV_HART_ARRAY, _abort, NULL); +sysbus_init_child_obj(OBJECT(machine), "soc", >soc, sizeof(s->soc), + TYPE_RISCV_HART_ARRAY); object_property_set_str(OBJECT(>soc), SPIKE_V1_10_0_CPU, "cpu-type", _abort); object_property_set_int(OBJECT(>soc), smp_cpus, "num-harts", @@ -365,8 +365,8 @@ static void spike_v1_09_1_board_init(MachineState *machine) } /* Initialize SOC */ -object_initialize_child(OBJECT(machine), "soc", >soc, sizeof(s->soc), -TYPE_RISCV_HART_ARRAY, _abort, NULL); +
[PATCH v2 21/24] sparc/leon3: Fix to put grlib,* devices on sysbus
leon3_generic_hw_init() creates a "grlib,ahbpnp" and a "grlib,apbpnp" sysbus device in a way that leaves them unplugged. Create them the common way that puts them into the main system bus. Affects machine leon3_generic. Visible in "info qtree": bus: main-system-bus type System + dev: grlib,ahbpnp, id "" +mmio f000/1000 + dev: grlib,apbpnp, id "" +mmio 800ff000/1000 dev: grlib,irqmp, id "" Cc: Fabien Chouteau Cc: KONRAD Frederic Cc: Mark Cave-Ayland Cc: Artyom Tarasenko Signed-off-by: Markus Armbruster Reviewed-by: KONRAD Frederic Reviewed-by: Philippe Mathieu-Daudé --- hw/sparc/leon3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c index 8f024dab7b..3facb8c2ae 100644 --- a/hw/sparc/leon3.c +++ b/hw/sparc/leon3.c @@ -213,14 +213,14 @@ static void leon3_generic_hw_init(MachineState *machine) reset_info->sp= LEON3_RAM_OFFSET + ram_size; qemu_register_reset(main_cpu_reset, reset_info); -ahb_pnp = GRLIB_AHB_PNP(object_new(TYPE_GRLIB_AHB_PNP)); +ahb_pnp = GRLIB_AHB_PNP(qdev_create(NULL, TYPE_GRLIB_AHB_PNP)); object_property_set_bool(OBJECT(ahb_pnp), true, "realized", _fatal); sysbus_mmio_map(SYS_BUS_DEVICE(ahb_pnp), 0, LEON3_AHB_PNP_OFFSET); grlib_ahb_pnp_add_entry(ahb_pnp, 0, 0, GRLIB_VENDOR_GAISLER, GRLIB_LEON3_DEV, GRLIB_AHB_MASTER, GRLIB_CPU_AREA); -apb_pnp = GRLIB_APB_PNP(object_new(TYPE_GRLIB_APB_PNP)); +apb_pnp = GRLIB_APB_PNP(qdev_create(NULL, TYPE_GRLIB_APB_PNP)); object_property_set_bool(OBJECT(apb_pnp), true, "realized", _fatal); sysbus_mmio_map(SYS_BUS_DEVICE(apb_pnp), 0, LEON3_APB_PNP_OFFSET); grlib_ahb_pnp_add_entry(ahb_pnp, LEON3_APB_PNP_OFFSET, 0xFFF, -- 2.21.3
[PATCH] tests/acceptance/migration.py: Wait for both sides
From: "Dr. David Alan Gilbert"
When the source finishes migration the destination will still be
receiving the data sent by the source, so it might not have quite
finished yet, so won't quite have reached 'completed'.
This lead to occasional asserts in the next few checks.
After the source has finished, check the destination as well.
(We can't just switch to checking the destination, because it doesn't
give a status until it has started receiving the migration).
Reported-by: Alex Bennée
Signed-off-by: Dr. David Alan Gilbert
Tested-by: Alex Bennée
---
tests/acceptance/migration.py | 4
1 file changed, 4 insertions(+)
diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 0365289cda..792639cb69 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -35,6 +35,10 @@ class Migration(Test):
timeout=self.timeout,
step=0.1,
args=(src_vm,))
+wait.wait_for(self.migration_finished,
+ timeout=self.timeout,
+ step=0.1,
+ args=(dst_vm,))
self.assertEqual(src_vm.command('query-migrate')['status'],
'completed')
self.assertEqual(dst_vm.command('query-migrate')['status'],
'completed')
self.assertEqual(dst_vm.command('query-status')['status'], 'running')
--
2.26.2
Re: [PATCH] tests/acceptance/migration.py: Wait for both sides
On 5/28/20 1:24 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> When the source finishes migration the destination will still be
> receiving the data sent by the source, so it might not have quite
> finished yet, so won't quite have reached 'completed'.
> This lead to occasional asserts in the next few checks.
>
> After the source has finished, check the destination as well.
> (We can't just switch to checking the destination, because it doesn't
> give a status until it has started receiving the migration).
>
Fixes: a7abb53765 ?
> Reported-by: Alex Bennée
> Signed-off-by: Dr. David Alan Gilbert
> Tested-by: Alex Bennée
> ---
> tests/acceptance/migration.py | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 0365289cda..792639cb69 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -35,6 +35,10 @@ class Migration(Test):
>timeout=self.timeout,
>step=0.1,
>args=(src_vm,))
> +wait.wait_for(self.migration_finished,
> + timeout=self.timeout,
I'm not sure the Test.timeout is well used (it represents the maximum
total time the framework will wait this test can take). Anyway this is
incorrectly used before your patch, so I wouldn't bother...
> + step=0.1,
> + args=(dst_vm,))
Reviewed-by: Philippe Mathieu-Daudé
> self.assertEqual(src_vm.command('query-migrate')['status'],
> 'completed')
> self.assertEqual(dst_vm.command('query-migrate')['status'],
> 'completed')
> self.assertEqual(dst_vm.command('query-status')['status'], 'running')
>
Re: [PATCH v2 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices
On 5/28/20 1:04 PM, Markus Armbruster wrote:
> stm32f405_soc_initfn() creates six such devices, but
> stm32f405_soc_realize() realizes only one. Affects machine
> netduinoplus2.
>
> In theory, a device becomes real only on realize. In practice, the
> transition from unreal to real is a fuzzy one. The work to make a
> device real can be spread between realize methods (fine),
> instance_init methods (wrong), and board code wiring up the device
> (fine as long as it effectively happens on realize). Depending on
> what exactly is done where, a device can work even when we neglect
> to realize it.
>
> The five unrealized devices appear to stay unreal: neither MMIO nor
> IRQ get wired up.
>
> Fix stm32f405_soc_realize() to realize and wire up all six. Visible
> in "info qtree":
>
> bus: main-system-bus
>type System
>dev: stm32f405-soc, id ""
> cpu-type = "cortex-m4-arm-cpu"
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012000/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012100/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012200/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012300/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio 40012000/00ff
> +mmio 40012400/00ff
>dev: stm32f2xx-adc, id ""
> gpio-out "sysbus-irq" 1
> -mmio /00ff
> +mmio 40012500/00ff
>dev: armv7m, id ""
>
> Fixes: 529fc5fd3e18ace8f739afd02dc0953354f39442
> Cc: Alistair Francis
> Cc: Peter Maydell
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster
> ---
> hw/arm/stm32f405_soc.c | 23 +--
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index 4f10ce6176..c9a530eecf 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -37,7 +37,8 @@ static const uint32_t usart_addr[] = { 0x40011000,
> 0x40004400, 0x40004800,
> /* At the moment only Timer 2 to 5 are modelled */
> static const uint32_t timer_addr[] = { 0x4000, 0x4400,
> 0x4800, 0x4C00 };
> -#define ADC_ADDR 0x40012000
> +static const uint32_t adc_addr[] = { 0x40012000, 0x40012100, 0x40012200,
> + 0x40012300, 0x40012400, 0x40012500 };
> static const uint32_t spi_addr[] = { 0x40013000, 0x40003800, 0x40003C00,
> 0x40013400, 0x40015000, 0x40015400 };
> #define EXTI_ADDR 0x40013C00
> @@ -185,16 +186,18 @@ static void stm32f405_soc_realize(DeviceState *dev_soc,
> Error **errp)
> qdev_connect_gpio_out(DEVICE(>adc_irqs), 0,
>qdev_get_gpio_in(armv7m, ADC_IRQ));
>
> -dev = DEVICE(&(s->adc[i]));
> -object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
> -if (err != NULL) {
> -error_propagate(errp, err);
> -return;
> +for (i = 0; i < STM_NUM_ADCS; i++) {
Correct fix.
Problem will come back when we'll want to implement a STM SoC with 8
ADCs, modifying the definition... We'll need to remember to unref() again.
Reviewed-by: Philippe Mathieu-Daudé
> +dev = DEVICE(&(s->adc[i]));
> +object_property_set_bool(OBJECT(>adc[i]), true, "realized", );
> +if (err != NULL) {
> +error_propagate(errp, err);
> +return;
> +}
> +busdev = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(busdev, 0, adc_addr[i]);
> +sysbus_connect_irq(busdev, 0,
> + qdev_get_gpio_in(DEVICE(>adc_irqs), i));
> }
> -busdev = SYS_BUS_DEVICE(dev);
> -sysbus_mmio_map(busdev, 0, ADC_ADDR);
> -sysbus_connect_irq(busdev, 0,
> - qdev_get_gpio_in(DEVICE(>adc_irqs), i));
>
> /* SPI devices */
> for (i = 0; i < STM_NUM_SPIS; i++) {
>
[PULL 7/7] gitlab-ci: Determine the number of jobs dynamically
Some people might want to run the gitlab CI pipelines in an environment
where multiple CPUs are available to the runners, so let's rather get
the number for "-j" from the "nproc" program (increased by 1 to compensate
for jobs that wait for I/O) instead of hard-coding it.
Message-Id: <20200525131823.715-7-th...@redhat.com>
Reviewed-by: Alex Bennée
Signed-off-by: Thomas Huth
---
.gitlab-ci.yml | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 559ec2ab4d..349c77aa58 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -7,12 +7,14 @@ include:
- apt-get update -qq
- apt-get install -y -qq git gcc libglib2.0-dev libpixman-1-dev make
genisoimage
+ - JOBS=$(expr $(nproc) + 1)
.update_dnf_template: _script_dnf
before_script:
- dnf update -y
- dnf install -y bzip2 diffutils gcc git genisoimage findutils glib2-devel
make python3 perl-podlators perl-Test-Harness pixman-devel zlib-devel
+ - JOBS=$(expr $(nproc) + 1)
build-system1:
image: ubuntu:19.10
@@ -25,8 +27,8 @@ build-system1:
- ../configure --enable-werror --target-list="aarch64-softmmu alpha-softmmu
cris-softmmu hppa-softmmu lm32-softmmu moxie-softmmu microblazeel-softmmu
mips64el-softmmu m68k-softmmu ppc-softmmu riscv64-softmmu sparc-softmmu"
- - make -j2
- - make -j2 check
+ - make -j"$JOBS"
+ - make -j"$JOBS" check
build-system2:
image: fedora:latest
@@ -40,8 +42,8 @@ build-system2:
- ../configure --enable-werror --target-list="tricore-softmmu
unicore32-softmmu
microblaze-softmmu mips-softmmu riscv32-softmmu s390x-softmmu sh4-softmmu
sparc64-softmmu x86_64-softmmu xtensa-softmmu nios2-softmmu or1k-softmmu"
- - make -j2
- - make -j2 check
+ - make -j"$JOBS"
+ - make -j"$JOBS" check
build-disabled:
image: fedora:latest
@@ -56,8 +58,8 @@ build-disabled:
--disable-qom-cast-debug --disable-spice --disable-vhost-vsock
--disable-vhost-net --disable-vhost-crypto --disable-vhost-user
--target-list="i386-softmmu ppc64-softmmu mips64-softmmu i386-linux-user"
- - make -j2
- - make -j2 check-qtest SPEED=slow
+ - make -j"$JOBS"
+ - make -j"$JOBS" check-qtest SPEED=slow
build-tcg-disabled:
image: centos:8
@@ -67,7 +69,7 @@ build-tcg-disabled:
- mkdir build
- cd build
- ../configure --cc=clang --enable-werror --disable-tcg --audio-drv-list=""
- - make -j2
+ - make -j"$JOBS"
- make check-unit
- make check-qapi-schema
- cd tests/qemu-iotests/
@@ -86,7 +88,7 @@ build-user:
- cd build
- ../configure --enable-werror --disable-system --disable-guest-agent
--disable-capstone --disable-slirp --disable-fdt
- - make -j2
+ - make -j"$JOBS"
- make run-tcg-tests-i386-linux-user run-tcg-tests-x86_64-linux-user
build-clang:
@@ -100,8 +102,8 @@ build-clang:
- ../configure --cc=clang --cxx=clang++ --enable-werror
--target-list="alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
ppc-softmmu s390x-softmmu x86_64-softmmu arm-linux-user"
- - make -j2
- - make -j2 check
+ - make -j"$JOBS"
+ - make -j"$JOBS" check
build-tci:
image: centos:8
@@ -112,7 +114,7 @@ build-tci:
- cd build
- ../configure --enable-tcg-interpreter
--target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu '; done)"
- - make -j2
+ - make -j"$JOBS"
- make run-tcg-tests-x86_64-softmmu
- make tests/qtest/boot-serial-test tests/qtest/cdrom-test
tests/qtest/pxe-test
- for tg in $TARGETS ; do
--
2.18.1
[PULL 2/7] MAINTAINERS: Add Philippe, Alex and Wainer to the Gitlab-CI section
Initially, I was the only one who was using Gitlab while most developers had their git trees still on other systems, but that has changed nowadays. There is now much more interest in the Gitlab-CI today, so it would be good to have more than only one maintainer / reviewer for the gitlab-ci.yml file. Alex, Wainer and Philippe kindly offered their help here, so let's add them to the corresponding section in the MAINTAINERS file now. Message-Id: <20200210155115.9371-1-th...@redhat.com> Reviewed-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daude Reviewed-by: Wainer dos Santos Moschetta Signed-off-by: Thomas Huth --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a209b5d8ce..71a0438843 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2901,6 +2901,9 @@ W: https://cirrus-ci.com/github/qemu/qemu GitLab Continuous Integration M: Thomas Huth +M: Philippe Mathieu-Daudé +M: Alex Bennée +R: Wainer dos Santos Moschetta S: Maintained F: .gitlab-ci.yml -- 2.18.1
[PULL 4/7] GitLab CI: avoid calling before_scripts on unintended jobs
From: Cleber Rosa At this point it seems that all jobs depend on those steps, with maybe the EDK2 jobs as exceptions. The jobs that will be added later will not want those scripts to be run, so let's move these steps to the appropriate jobs, while still trying to avoid repetition. Signed-off-by: Cleber Rosa Message-Id: <20200525131823.715-4-th...@redhat.com> Reviewed-by: Alex Bennée [thuth: Rebased to current master branch, use separate template] Signed-off-by: Thomas Huth --- .gitlab-ci.yml | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 994774250f..bc6aee6aba 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2,11 +2,13 @@ include: - local: '/.gitlab-ci-edk2.yml' - local: '/.gitlab-ci-opensbi.yml' -before_script: - - apt-get update -qq - - apt-get install -y -qq libglib2.0-dev libpixman-1-dev genisoimage +.update_apt_template: _script_apt + before_script: + - apt-get update -qq + - apt-get install -y -qq libglib2.0-dev libpixman-1-dev genisoimage build-system1: + <<: *before_script_apt script: - apt-get install -y -qq libgtk-3-dev libvte-dev nettle-dev libcacard-dev libusb-dev libvde-dev libspice-protocol-dev libgl1-mesa-dev libvdeplug-dev @@ -19,6 +21,7 @@ build-system1: - make -j2 check build-system2: + <<: *before_script_apt script: - apt-get install -y -qq libsdl2-dev libgcrypt-dev libbrlapi-dev libaio-dev libfdt-dev liblzo2-dev librdmacm-dev libibverbs-dev libibumad-dev @@ -32,6 +35,7 @@ build-system2: - make -j2 check build-disabled: + <<: *before_script_apt script: - mkdir build - cd build @@ -46,6 +50,7 @@ build-disabled: - make -j2 check-qtest SPEED=slow build-tcg-disabled: + <<: *before_script_apt script: - apt-get install -y -qq clang libgtk-3-dev libusb-dev - mkdir build @@ -64,6 +69,7 @@ build-tcg-disabled: 260 261 262 263 264 270 272 273 277 279 build-user: + <<: *before_script_apt script: - mkdir build - cd build @@ -73,6 +79,7 @@ build-user: - make run-tcg-tests-i386-linux-user run-tcg-tests-x86_64-linux-user build-clang: + <<: *before_script_apt script: - apt-get install -y -qq clang libsdl2-dev libattr1-dev libcap-ng-dev xfslibs-dev libiscsi-dev libnfs-dev libseccomp-dev gnutls-dev librbd-dev @@ -85,6 +92,7 @@ build-clang: - make -j2 check build-tci: + <<: *before_script_apt script: - TARGETS="aarch64 alpha arm hppa m68k microblaze moxie ppc64 s390x x86_64" - mkdir build -- 2.18.1
[PATCH v4 2/5] block/io: bdrv_common_block_status_above: support include_base
In order to reuse bdrv_common_block_status_above in
bdrv_is_allocated_above, let's support include_base parameter.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Kevin Wolf
Reviewed-by: Eric Blake
---
block/io.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index 412b91b08f..ae1c806720 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2223,6 +2223,7 @@ int bdrv_flush_all(void)
typedef struct BdrvCoBlockStatusData {
BlockDriverState *bs;
BlockDriverState *base;
+bool include_base;
bool want_zero;
int64_t offset;
int64_t bytes;
@@ -2445,6 +2446,7 @@ early_out:
static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
+ bool include_base,
bool want_zero,
int64_t offset,
int64_t bytes,
@@ -2456,8 +2458,8 @@ static int coroutine_fn
bdrv_co_block_status_above(BlockDriverState *bs,
int ret = 0;
bool first = true;
-assert(bs != base);
-for (p = bs; p != base; p = backing_bs(p)) {
+assert(include_base || bs != base);
+for (p = bs; include_base || p != base; p = backing_bs(p)) {
ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
file);
if (ret < 0) {
@@ -2496,6 +2498,11 @@ static int coroutine_fn
bdrv_co_block_status_above(BlockDriverState *bs,
/* [offset, pnum] unallocated on this layer, which could be only
* the first part of [offset, bytes]. */
+
+if (p == base) {
+break;
+}
+
assert(*pnum <= bytes);
bytes = *pnum;
first = false;
@@ -2510,7 +2517,7 @@ static void coroutine_fn
bdrv_block_status_above_co_entry(void *opaque)
BdrvCoBlockStatusData *data = opaque;
data->ret = bdrv_co_block_status_above(data->bs, data->base,
- data->want_zero,
+ data->include_base, data->want_zero,
data->offset, data->bytes,
data->pnum, data->map, data->file);
data->done = true;
@@ -2524,6 +2531,7 @@ static void coroutine_fn
bdrv_block_status_above_co_entry(void *opaque)
*/
static int bdrv_common_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
+ bool include_base,
bool want_zero, int64_t offset,
int64_t bytes, int64_t *pnum,
int64_t *map,
@@ -2533,6 +2541,7 @@ static int
bdrv_common_block_status_above(BlockDriverState *bs,
BdrvCoBlockStatusData data = {
.bs = bs,
.base = base,
+.include_base = include_base,
.want_zero = want_zero,
.offset = offset,
.bytes = bytes,
@@ -2557,7 +2566,7 @@ int bdrv_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
int64_t offset, int64_t bytes, int64_t *pnum,
int64_t *map, BlockDriverState **file)
{
-return bdrv_common_block_status_above(bs, base, true, offset, bytes,
+return bdrv_common_block_status_above(bs, base, false, true, offset, bytes,
pnum, map, file);
}
@@ -2574,7 +2583,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs,
int64_t offset,
int ret;
int64_t dummy;
-ret = bdrv_common_block_status_above(bs, backing_bs(bs), false, offset,
+ret = bdrv_common_block_status_above(bs, bs, true, false, offset,
bytes, pnum ? pnum : , NULL,
NULL);
if (ret < 0) {
--
2.18.0
Re: [PATCH v6 4/5] crypto: Add tls-cipher-suites object
On 5/27/20 1:36 PM, Daniel P. Berrangé wrote:
> On Tue, May 19, 2020 at 08:20:23PM +0200, Philippe Mathieu-Daudé wrote:
>> Example of use to dump:
>>
>> $ qemu-system-x86_64 -S \
>> -object tls-cipher-suites,id=mysuite,priority=@SYSTEM,verbose=yes
>> Cipher suites for @SYSTEM:
>> - TLS_AES_256_GCM_SHA3840x13, 0x02
>> TLS1.3
>> - TLS_CHACHA20_POLY1305_SHA256 0x13, 0x03
>> TLS1.3
>> - TLS_AES_128_GCM_SHA2560x13, 0x01
>> TLS1.3
>> - TLS_AES_128_CCM_SHA2560x13, 0x04
>> TLS1.3
>> - TLS_ECDHE_RSA_AES_256_GCM_SHA384 0xc0, 0x30
>> TLS1.2
>> - TLS_ECDHE_RSA_CHACHA20_POLY1305 0xcc, 0xa8
>> TLS1.2
>> - TLS_ECDHE_RSA_AES_256_CBC_SHA10xc0, 0x14
>> TLS1.0
>> - TLS_ECDHE_RSA_AES_128_GCM_SHA256 0xc0, 0x2f
>> TLS1.2
>> - TLS_ECDHE_RSA_AES_128_CBC_SHA10xc0, 0x13
>> TLS1.0
>> - TLS_ECDHE_ECDSA_AES_256_GCM_SHA3840xc0, 0x2c
>> TLS1.2
>> - TLS_ECDHE_ECDSA_CHACHA20_POLY1305 0xcc, 0xa9
>> TLS1.2
>> - TLS_ECDHE_ECDSA_AES_256_CCM 0xc0, 0xad
>> TLS1.2
>> - TLS_ECDHE_ECDSA_AES_256_CBC_SHA1 0xc0, 0x0a
>> TLS1.0
>> - TLS_ECDHE_ECDSA_AES_128_GCM_SHA2560xc0, 0x2b
>> TLS1.2
>> - TLS_ECDHE_ECDSA_AES_128_CCM 0xc0, 0xac
>> TLS1.2
>> - TLS_ECDHE_ECDSA_AES_128_CBC_SHA1 0xc0, 0x09
>> TLS1.0
>> - TLS_RSA_AES_256_GCM_SHA3840x00, 0x9d
>> TLS1.2
>> - TLS_RSA_AES_256_CCM 0xc0, 0x9d
>> TLS1.2
>> - TLS_RSA_AES_256_CBC_SHA1 0x00, 0x35
>> TLS1.0
>> - TLS_RSA_AES_128_GCM_SHA2560x00, 0x9c
>> TLS1.2
>> - TLS_RSA_AES_128_CCM 0xc0, 0x9c
>> TLS1.2
>> - TLS_RSA_AES_128_CBC_SHA1 0x00, 0x2f
>> TLS1.0
>> - TLS_DHE_RSA_AES_256_GCM_SHA3840x00, 0x9f
>> TLS1.2
>> - TLS_DHE_RSA_CHACHA20_POLY1305 0xcc, 0xaa
>> TLS1.2
>> - TLS_DHE_RSA_AES_256_CCM 0xc0, 0x9f
>> TLS1.2
>> - TLS_DHE_RSA_AES_256_CBC_SHA1 0x00, 0x39
>> TLS1.0
>> - TLS_DHE_RSA_AES_128_GCM_SHA2560x00, 0x9e
>> TLS1.2
>> - TLS_DHE_RSA_AES_128_CCM 0xc0, 0x9e
>> TLS1.2
>> - TLS_DHE_RSA_AES_128_CBC_SHA1 0x00, 0x33
>> TLS1.0
>> total: 29 ciphers
>
> IMHO this "verbose" option shouldn't exist. Instead we should be
> using the QEMU trace infrastructure to log this information. This
> will make it possible to trace the info at runtime in production
> deployments too
OK, clever.
>
>> +static void parse_cipher_suites(QCryptoTLSCipherSuites *s,
>> +const char *priority_name, Error **errp)
>> +{
>> +#ifdef CONFIG_GNUTLS
>
> Instead of doing this..
>
>
>> diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
>> index c2a371b0b4..ce706d322a 100644
>> --- a/crypto/Makefile.objs
>> +++ b/crypto/Makefile.objs
>> @@ -13,6 +13,7 @@ crypto-obj-y += cipher.o
>> crypto-obj-$(CONFIG_AF_ALG) += afalg.o
>> crypto-obj-$(CONFIG_AF_ALG) += cipher-afalg.o
>> crypto-obj-$(CONFIG_AF_ALG) += hash-afalg.o
>> +crypto-obj-y += tls-cipher-suites.o
>
> Use crypto-obj-$(CONFIG_GNUTLS) += tls-cipher-suites.o
>
> This lets the mgmt appliction introspect QEMU to discover whether the
> TLS cipher suits object is present & usable.
OK, thanks!
>
>> crypto-obj-y += tlscreds.o
>> crypto-obj-y += tlscredsanon.o
>> crypto-obj-y += tlscredspsk.o
>> --
>> 2.21.3
>>
>
> Regards,
> Daniel
>
Re: [PATCH v4 5/6] i386: Hyper-V VMBus ACPI DSDT entry
On 28/05/2020, Igor Mammedov wrote: On Thu, 28 May 2020 08:26:42 +0300 Jon Doron wrote: On 22/05/2020, Igor Mammedow wrote: >On Thu, 21 May 2020 18:02:07 +0200 >Paolo Bonzini wrote: > >> On 13/05/20 17:34, Igor Mammedov wrote: >> > I'd rather avoid using random IRQ numbers (considering we are >> > dealing with black-box here). So if it's really necessary to have >> > IRQ described here, I'd suggest to implement them in device model >> > so they would be reserved and QEMU would error out in a sane way if >> > IRQ conflict is detected. >> >> We don't generally detect ISA IRQ conflicts though, do we? > >that I don't know that's why I'm not suggesting how to do it. >The point is hard-coding in AML random IRQs is not right thing to do, >(especially with the lack of 'any' spec), as minimum AML should pull >it from device model and that probably should be configurable and set >by board. > >Other thing is: >I haven't looked at VMBus device model in detail, but DSDT part aren't >matching device though (device model is not ISA device hence AML part >shouldn't be on in ISA scope), where to put it is open question. >There were other issues with AML code, I've commented on, so I was >waiting on respin with comments addressed. >I don't think that this patch is good enough for merging. > > But it seems like the current patch does match what's Microsoft HyperV is publishing in it's APCI tables. I dont think it's correct for us to "fix" Microsoft emulation even if it's wrong, since that's what Windows probably expects to see... I tried looking where Microsoft uses the ACPI tables to identify the VMBus but without much luck in order to understand how flexible a change would be for the OS to still detect the VMBus device, but in general I think "correcting" something that is emulated 1:1 because there is no spec is the right way. I'd agree, if removing nonsense would break VMBus detection (does it?). if something is that doesn't make sense but has to stay because it is need to make windows happy, that's fine , just add annotate is with comment, so it won't confuse anyone why that code exists there later on. I suggest to: 1. try dropping _PS* & _STA as it doesn't actually does anything and _PS3 is plain wrong 2. drop one IRQ, newer hyper-v seems to be doing fine with only one 3. it's not ISA device, I'd suggest to move into _SB scope 4. I don't know much about IRQs but git grep DEFINE_PROP_ | grep -i iqr yields nothing so I'm not sure if it's acceptable. Typically it's board that assigns IRQ and not device, for Sysbus devices (see: sysbus_init_irq/sysbus_connect_irq). So I'd leave it upto Paolo or someone else to decide/comment on. Sounds like a plan, I'll try to come up with the test results (at least for Windows 10 guest which is what I have setup) and update this thread with the results. -- Jon. >> >> Paolo >> >
[PATCH v2 16/24] ppc/pnv: Put "*-pnv-chip" and "pnv-xive" on the main system bus
pnv_init() creates "power10_v1.0-pnv-chip", "power8_v2.0-pnv-chip",
"power8e_v2.1-pnv-chip", "power8nvl_v1.0-pnv-chip", or
"power9_v2.0-pnv-chip" sysbus devices in a way that leaves them
unplugged.
pnv_chip_power9_instance_init() creates a "pnv-xive" sysbus device in
a way that leaves it unplugged.
Create them the common way that puts them into the main system bus.
Affects machines powernv8, powernv9, and powernv10. Visible in "info
qtree". Here's the change for powernv9:
bus: main-system-bus
type System
+ dev: power9_v2.0-pnv-chip, id ""
+chip-id = 0 (0x0)
+ram-start = 0 (0x0)
+ram-size = 1879048192 (0x7000)
+nr-cores = 1 (0x1)
+cores-mask = 72057594037927935 (0xff)
+nr-threads = 1 (0x1)
+num-phbs = 6 (0x6)
+mmio 000603fc/0004
[...]
+ dev: pnv-xive, id ""
+ic-bar = 1692157036462080 (0x603020310)
+vc-bar = 1689949371891712 (0x60100)
+pc-bar = 1690499127705600 (0x60180)
+tm-bar = 1692157036986368 (0x603020318)
Cc: "Cédric Le Goater"
Cc: David Gibson
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster
Reviewed-by: Cédric Le Goater
---
hw/ppc/pnv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index da637822f9..8d4fc8109a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -818,7 +818,7 @@ static void pnv_init(MachineState *machine)
pnv->chips = g_new0(PnvChip *, pnv->num_chips);
for (i = 0; i < pnv->num_chips; i++) {
char chip_name[32];
-Object *chip = object_new(chip_typename);
+Object *chip = OBJECT(qdev_create(NULL, chip_typename));
pnv->chips[i] = PNV_CHIP(chip);
@@ -1317,8 +1317,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
int i;
-object_initialize_child(obj, "xive", >xive, sizeof(chip9->xive),
-TYPE_PNV_XIVE, _abort, NULL);
+sysbus_init_child_obj(obj, "xive", >xive, sizeof(chip9->xive),
+ TYPE_PNV_XIVE);
object_property_add_alias(obj, "xive-fabric", OBJECT(>xive),
"xive-fabric");
--
2.21.3
[PATCH v2 08/24] mac_via: Fix to realize "mos6522-q800-via*" devices
mac_via_realize() creates a "mos6522-q800-via1" and a "mos6522-q800-via2" device, but neglects to realize them. Affects machine q800. In theory, a device becomes real only on realize. In practice, the transition from unreal to real is a fuzzy one. The work to make a device real can be spread between realize methods (fine), instance_init methods (wrong), and board code wiring up the device (fine as long as it effectively happens on realize). Depending on what exactly is done where, a device can work even when we neglect to realize it. These two appear to work. Nevertheless, it's a clear misuse of the interface. Even when it works today (more or less by chance), it can break tomorrow. Fix by realizing them right away. Fixes: 6dca62af95e0b7020aa00d0ca9b2c421f341 Cc: Laurent Vivier Signed-off-by: Markus Armbruster Reviewed-by: Mark Cave-Ayland --- hw/misc/mac_via.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index e05623d730..82614c155a 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -890,6 +890,11 @@ static void mac_via_realize(DeviceState *dev, Error **errp) object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms), SYSBUS_DEVICE_GPIO_IRQ "[0]"); +object_property_set_bool(OBJECT(>mos6522_via1), true, "realized", + _abort); +object_property_set_bool(OBJECT(>mos6522_via2), true, "realized", + _abort); + /* Pass through mos6522 input IRQs */ qdev_pass_gpios(DEVICE(>mos6522_via1), dev, "via1-irq"); qdev_pass_gpios(DEVICE(>mos6522_via2), dev, "via2-irq"); -- 2.21.3
[PATCH v2 18/24] display/sm501 display/ati: Fix to realize "i2c-ddc"
sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but neglect to realize it. Affects machines sam460ex, shix, r2d, and fulong2e. In theory, a device becomes real only on realize. In practice, the transition from unreal to real is a fuzzy one. The work to make a device real can be spread between realize methods (fine), instance_init methods (wrong), and board code wiring up the device (fine as long as it effectively happens on realize). Depending on what exactly is done where, a device can work even when we neglect to realize it. This one appears to work. Nevertheless, it's a clear misuse of the interface. Even when it works today (more or less by chance), it can break tomorrow. Fix by realizing it right away. Visible in "info qom-tree"; here's the change for sam460ex: /machine (sam460ex-machine) [...] /unattached (container) [...] -/device[14] (sii3112) +/device[14] (i2c-ddc) +/device[15] (sii3112) [rest of device[*] renumbered...] Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a Fixes: c82c7336de58876862e6b4dccbda29e9240fd388 Cc: BALATON Zoltan Cc: qemu-...@nongnu.org Cc: Magnus Damm Cc: Philippe Mathieu-Daudé Cc: Aleksandar Markovic Signed-off-by: Markus Armbruster Tested-by: BALATON Zoltan --- hw/display/ati.c | 2 ++ hw/display/sm501.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/hw/display/ati.c b/hw/display/ati.c index 065f197678..5c71e5f295 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -929,6 +929,8 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp) bitbang_i2c_init(>bbi2c, i2cbus); I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC)); i2c_set_slave_address(i2cddc, 0x50); +object_property_set_bool(OBJECT(i2cddc), true, "realized", + _abort); /* mmio register space */ memory_region_init_io(>mm, OBJECT(s), _mm_ops, s, diff --git a/hw/display/sm501.c b/hw/display/sm501.c index acc692531a..fbedc56715 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -1816,6 +1816,8 @@ static void sm501_init(SM501State *s, DeviceState *dev, /* ddc */ I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC)); i2c_set_slave_address(I2C_SLAVE(ddc), 0x50); +object_property_set_bool(OBJECT(ddc), true, "realized", + _abort); /* mmio */ memory_region_init(>mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE); -- 2.21.3
[PATCH v2 11/24] pnv/phb4: Delete unused "pnv-phb4-pec-stack" devices
The number of stacks is controlled by property "num-stacks".
pnv_pec_instance_init() creates the maximum supported number, because
the property has not been set then. pnv_pec_realize() realizes only
the wanted number. Works, although it can leave unrealized devices
hanging around in the QOM composition tree. Affects machine powernv9.
Delete the unused devices by making pnv_pec_realize() unparent them.
Visible in "info qom-tree":
/machine (powernv9-machine)
/chip[0] (power9_v2.0-pnv-chip)
[...]
/pec[0] (pnv-phb4-pec)
/stack[0] (pnv-phb4-pec-stack)
[...]
- /stack[1] (pnv-phb4-pec-stack)
-/phb (pnv-phb4)
- /pcie-mmcfg-mmio[0] (qemu:memory-region)
- /root (pnv-phb4-root-port)
- /source (xive-source)
- /stack[2] (pnv-phb4-pec-stack)
-/phb (pnv-phb4)
- /pcie-mmcfg-mmio[0] (qemu:memory-region)
- /root (pnv-phb4-root-port)
- /source (xive-source)
/xscom-pec-0.0-nest[0] (qemu:memory-region)
/xscom-pec-0.0-pci[0] (qemu:memory-region)
/pec[1] (pnv-phb4-pec)
/stack[0] (pnv-phb4-pec-stack)
[...]
/stack[1] (pnv-phb4-pec-stack)
[...]
- /stack[2] (pnv-phb4-pec-stack)
-/phb (pnv-phb4)
- /pcie-mmcfg-mmio[0] (qemu:memory-region)
- /root (pnv-phb4-root-port)
- /source (xive-source)
/xscom-pec-0.1-nest[0] (qemu:memory-region)
/xscom-pec-0.1-pci[0] (qemu:memory-region)
/pec[2] (pnv-phb4-pec)
/stack[0] (pnv-phb4-pec-stack)
[...]
/stack[1] (pnv-phb4-pec-stack)
[...]
/stack[2] (pnv-phb4-pec-stack)
[...]
Cc: Cédric Le Goater
Cc: David Gibson
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster
Reviewed-by: Greg Kurz
Reviewed-by: Cédric Le Goater
---
hw/pci-host/pnv_phb4_pec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 911d147ffd..565345a018 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -397,6 +397,9 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
return;
}
}
+for (; i < PHB4_PEC_MAX_STACKS; i++) {
+object_unparent(OBJECT(>stacks[i]));
+}
/* Initialize the XSCOM regions for the PEC registers */
snprintf(name, sizeof(name), "xscom-pec-%d.%d-nest", pec->chip_id,
--
2.21.3
[PATCH v2 12/24] MAINTAINERS: Make section PowerNV cover pci-host/pnv* as well
Cc: Cédric Le Goater Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster Acked-by: David Gibson --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3690f313c3..3bd5c613f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1213,7 +1213,9 @@ S: Maintained F: hw/ppc/pnv* F: hw/intc/pnv* F: hw/intc/xics_pnv.c +F: hw/pci-host/pnv* F: include/hw/ppc/pnv* +F: include/hw/pci-host/pnv* F: pc-bios/skiboot.lid F: tests/qtest/pnv* -- 2.21.3
[PATCH v2 09/24] macio: Fix to realize "mos6522-cuda" and "mos6522-pmu" devices
cuda_init() creates a "mos6522-cuda" device, but it's never realized.
Affects machines mac99 with via=cuda (default) and g3beige.
pmu_init() creates a "mos6522-pmu" device, but it's never realized.
Affects machine mac99 with via=pmu and via=pmu-adb,
In theory, a device becomes real only on realize. In practice, the
transition from unreal to real is a fuzzy one. The work to make a
device real can be spread between realize methods (fine),
instance_init methods (wrong), and board code wiring up the device
(fine as long as it effectively happens on realize). Depending on
what exactly is done where, a device can work even when we neglect
to realize it.
These onetwo appear to work. Nevertheless, it's a clear misuse of the
interface. Even when it works today (more or less by chance), it can
break tomorrow.
Fix by realizing them in cuda_realize() and pmu_realize(),
respectively.
Fixes: 6dca62af95e0b7020aa00d0ca9b2c421f341
Cc: Laurent Vivier
Signed-off-by: Markus Armbruster
---
hw/misc/macio/cuda.c | 10 +-
hw/misc/macio/pmu.c | 10 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index e0cc0aac5d..763a785f1a 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -33,6 +33,7 @@
#include "hw/misc/macio/cuda.h"
#include "qemu/timer.h"
#include "sysemu/runstate.h"
+#include "qapi/error.h"
#include "qemu/cutils.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -523,15 +524,14 @@ static void cuda_realize(DeviceState *dev, Error **errp)
{
CUDAState *s = CUDA(dev);
SysBusDevice *sbd;
-MOS6522State *ms;
-DeviceState *d;
struct tm tm;
+object_property_set_bool(OBJECT(>mos6522_cuda), true, "realized",
+ _abort);
+
/* Pass IRQ from 6522 */
-d = DEVICE(>mos6522_cuda);
-ms = MOS6522(d);
sbd = SYS_BUS_DEVICE(s);
-sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms));
+sysbus_pass_irq(sbd, SYS_BUS_DEVICE(>mos6522_cuda));
qemu_get_timedate(, 0);
s->tick_offset = (uint32_t)mktimegm() + RTC_OFFSET;
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 9a9cd427e1..4264779396 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -40,6 +40,7 @@
#include "hw/misc/macio/pmu.h"
#include "qemu/timer.h"
#include "sysemu/runstate.h"
+#include "qapi/error.h"
#include "qemu/cutils.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -740,15 +741,14 @@ static void pmu_realize(DeviceState *dev, Error **errp)
{
PMUState *s = VIA_PMU(dev);
SysBusDevice *sbd;
-MOS6522State *ms;
-DeviceState *d;
struct tm tm;
+object_property_set_bool(OBJECT(>mos6522_pmu), true, "realized",
+ _abort);
+
/* Pass IRQ from 6522 */
-d = DEVICE(>mos6522_pmu);
-ms = MOS6522(d);
sbd = SYS_BUS_DEVICE(s);
-sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms));
+sysbus_pass_irq(sbd, SYS_BUS_DEVICE(>mos6522_pmu));
qemu_get_timedate(, 0);
s->tick_offset = (uint32_t)mktimegm() + RTC_OFFSET;
--
2.21.3
[PATCH v2 17/24] pnv/psi: Correct the pnv-psi* devices not to be sysbus devices
pnv_chip_power8_instance_init() creates a "pnv-psi-POWER8" sysbus
device in a way that leaves it unplugged.
pnv_chip_power9_instance_init() and pnv_chip_power10_instance_init()
do the same for "pnv-psi-POWER9" and "pnv-psi-POWER10", respectively.
These devices aren't actually sysbus devices. Correct that.
Cc: "Cédric Le Goater"
Cc: David Gibson
Cc: qemu-...@nongnu.org
Signed-off-by: Markus Armbruster
Reviewed-by: Cédric Le Goater
---
include/hw/ppc/pnv_psi.h | 2 +-
hw/ppc/pnv_psi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
index f0f5b55197..979fc59f33 100644
--- a/include/hw/ppc/pnv_psi.h
+++ b/include/hw/ppc/pnv_psi.h
@@ -31,7 +31,7 @@
#define PSIHB_XSCOM_MAX 0x20
typedef struct PnvPsi {
-SysBusDevice parent;
+DeviceState parent;
MemoryRegion regs_mr;
uint64_t bar;
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index cfd5b7bc25..82f0769465 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -943,7 +943,7 @@ static void pnv_psi_class_init(ObjectClass *klass, void
*data)
static const TypeInfo pnv_psi_info = {
.name = TYPE_PNV_PSI,
-.parent= TYPE_SYS_BUS_DEVICE,
+.parent= TYPE_DEVICE,
.instance_size = sizeof(PnvPsi),
.class_init= pnv_psi_class_init,
.class_size= sizeof(PnvPsiClass),
--
2.21.3
