This commit was produced with the Coccinelle script
scripts/coccinelle/memory-region-housekeeping.cocci.
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This commit was produced with the Coccinelle script
scripts/coccinelle/memory-region-housekeeping.cocci.
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This commit was produced with the Coccinelle script
scripts/coccinelle/memory-region-housekeeping.cocci.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This commit was produced with the Coccinelle script
scripts/coccinelle/memory-region-housekeeping.cocci.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This commit was produced with the Coccinelle script
scripts/coccinelle/memory-region-housekeeping.cocci.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This commit was produced with the Coccinelle script
scripts/coccinelle/memory-region-housekeeping.cocci.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Add a semantic patch to replace memory_region_init_ram(readonly)
by memory_region_init_rom().
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
As we are going to add various semantic changes related to the memory
region API, rename this script to be more generic.
Add a 'usage' header, and an entry in MAINTAINERS to avoid checkpatch
warning.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
memory_region_init_rom_nomigrate() has the same content than
memory_region_init_ram_shared_nomigrate(), with setting the
readonly mode. The code is easier to review as creating a
readonly ram/shared/nomigrate region.
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Since memory region aliases are neither rom nor ram, they are
described as i/o, which is often incorrect. Return instead the
type of the original region we are aliasing.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
The polymorphic locking macros don't support QemuRecMutex yet. Add it
so that lock guards can be used with QemuRecMutex.
Convert TCG plugins functions that benefit from these macros. Manual
qemu_rec_mutex_lock/unlock() callers are left unmodified in cases where
clarity would not improve by switching to the macros.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This patch introduces two lock guard macros that automatically unlock a
lock object (QemuMutex and others):
void f(void) {
QEMU_LOCK_GUARD(&mutex);
if (!may_fail()) {
return; /* automatically unlocks mutex */
}
...
}
and:
WITH_QEMU_LOCK_GUARD(&mutex) {
if (!may_fail()) {
return; /* automatically unlocks mutex */
}
}
/* automatically unlocks mutex here */
...
Convert qemu-timer.c functions that benefit from these macros as an
example. Manual qemu_mutex_lock/unlock() callers are left unmodified in
cases where clarity would not improve by switching to the macros.
Many other QemuMutex users remain in the codebase that might benefit
from lock guards. Over time they can be converted, if that is
desirable.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Use QEMU_MAKE_LOCKABLE_NONNULL. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This will be needed for lock guards, because if the lock is NULL the
dummy for loop of the lock guard never runs. This can cause confusion
and dummy warnings in the compiler, but even if it did not, aborting
with a NULL pointer dereference is a less surprising behavior.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We found an issue when repeat reboot in guest during migration, it cause the
migration thread never be waken up again.
<main loop> |<migration_thread>
|
LOCK BQL |
... |
main_loop_should_exit |
pause_all_vcpus |
1. set all cpus ->stop=true |
and then kick |
2. return if all cpus is paused |
(by '->stopped == true'), else|
3. qemu_cond_wait [BQL UNLOCK] |
|LOCK BQL
|...
|do_vm_stop
| pause_all_vcpus
| (A)set all cpus ->stop=true
| and then kick
| (B)return if all cpus is paused
| (by '->stopped == true'), else
| (C)qemu_cond_wait [BQL UNLOCK]
4. be waken up and LOCK BQL | (D)be waken up BUT wait for BQL
5. goto 2. |
(BQL is still LOCKed) |
resume_all_vcpus |
1. set all cpus ->stop=false |
and ->stopped=false |
... |
BQL UNLOCK | (E)LOCK BQL
| (F)goto B. [but stopped is false now!]
|Finally, sleep at step 3 forever.
resume_all_vcpus should notice this race, so we need to move the change
of runstate before pause_all_vcpus in do_vm_stop() and ignore the resume
request if runstate is not running.
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Longpeng <longpeng2@huawei.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200316083732.2010-1-longpeng2@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Commit 355477f8c7 skips rom reset when we're an incoming migration
so as not to overwrite shared ram in the ignore-shared migration
optimisation.
However, it's got an unexpected side effect that because it skips
freeing the ROM data, when rom_reset gets called later on, after
migration (e.g. during a reboot), the ROM does get reset to the original
file contents. Because of seabios/x86's weird reboot process
this confuses a reboot into hanging after a migration.
Fixes: 355477f8c7 ("migration: do not rom_reset() during incoming migration")
https://bugzilla.redhat.com/show_bug.cgi?id=1809380
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
glibc and Linux-provided headers are known to generate macro
redefinition warnings when used together. For example:
<linux/mman.h> and <sys/mman.h> duplicate some macro definitions.
We normally never see those warnings because GCC suppresses
warnings generated by system headers. We carry our own copy of
Linux header files, though, and this makes those warnings not be
suppressed when glibc headers are included before Linux headers
(e.g. if <sys/mman.h> is included before <linux/mman.h>).
Use -isystem instead of -I for linux-headers. This makes the
compiler treat our linux-headers directory the same way it treats
system-provided Linux headers, and suppress warnings generated by
them.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Currently if you try to ask for the list of CPUs for a target
architecture which does not specify a default machine type
you just get an error:
$ qemu-system-arm -cpu help
qemu-system-arm: No machine specified, and there is no default
Use -machine help to list supported machines
Since the list of CPUs doesn't depend on the machine, this is
unnecessarily unhelpful. "-device help" has a similar problem.
Move the checks for "did the user ask for -cpu help or -device help"
up so they precede the select_machine() call which checks that the
user specified a valid machine type.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Add the '-noTSX' variants for CascadeLake and SkyLake.
- Document the three MSR bits: 'mds-no', 'taa-no', and 'tsx-ctrl'
Two confusing things about 'mds-no' (and the first point applies to
the other two MSRs too):
(1) The 'mds-no' bit will _not_ show up in the guest's /proc/cpuinfo.
Rather it is used to fill in the guest's sysfs:
/sys/devices/system/cpu/vulnerabilities/mds:Not affected
Paolo confirmed on IRC as such.
(2) There are _three_ variants[+] of CascadeLake CPUs, with different
stepping levels: 5, 6, and 7. To quote wikichip.org[*]:
"note that while steppings 6 & 7 are fully mitigated, earlier
stepping 5 is not protected against MSBDS, MLPDS, nor MDSUM"
The above is also indicated in the Intel's document[+], as
indicated by "No" under the three columns of MFBDS, MSBDS, and
MLPDS.
I've expressed this in the docs without belabouring the details.
[+] https://software.intel.com/security-software-guidance/insights/processors-affected-microarchitectural-data-sampling
[*] https://en.wikichip.org/wiki/intel/microarchitectures/cascade_lake#Key_changes_from_Skylake
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
Message-Id: <20200225165618.6571-3-kchamart@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We only need these arrays when using the curses display.
Move them from the .bss to the .heap (sizes reported on
x86_64 host: screen[] is 64KiB, vga_to_curses 7KiB).
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
As we only use this array as input, make it const.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The USB descriptor sizes are specified as 16-bit for idVendor /
idProduct, and 8-bit for bInterfaceClass / bInterfaceSubClass /
bInterfaceProtocol. Doing so we reduce the usbredir_raw_serial_ids[]
and usbredir_ftdi_serial_ids[] arrays from 16KiB to 6KiB (size
reported on x86_64 host, building with --extra-cflags=-Os).
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The intel-hda model uses an array of register indexed by the
register address. This array also contains a pair of aliased
registers at offset 0x2000. This creates a huge hole in the
array, which ends up eating 4.6MiB of .rodata (size reported
on x86_64 host, building with --extra-cflags=-Os).
By using a memory region alias, we reduce this array to 132kB.
Before:
(qemu) info mtree
00000000febd4000-00000000febd7fff (prio 1, i/o): intel-hda
After:
(qemu) info mtree
00000000febd4000-00000000febd7fff (prio 1, i/o): intel-hda
00000000febd4000-00000000febd7fff (prio 1, i/o): intel-hda-container
00000000febd4000-00000000febd5fff (prio 0, i/o): intel-hda
00000000febd6000-00000000febd7fff (prio 0, i/o): alias intel-hda-alias @intel-hda 0000000000000000-0000000000001fff
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This buffer is only used by the adlib audio device. Move it to
the .heap to release 32KiB of .bss (size reported on x86_64 host).
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The 'help' target is displayed unaligned. Add a print-help
function and use it. Now if someone want to change the
indentation, there is a single place to modify.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
WHPX wasn't using the proper synchronization primitives while
processing async events, which can cause issues with SMP.
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When HAX is enabled (--enable-hax), GCC 9.2.1 reports issues with
snprintf(). Replacing old snprintf() by g_strdup_printf() fixes the
problem with boundary checks of vm_id and vcpu_id and finally the
warnings produced by GCC.
For more details, one example of warning:
CC i386-softmmu/target/i386/hax-posix.o
qemu/target/i386/hax-posix.c: In function ‘hax_host_open_vm’:
qemu/target/i386/hax-posix.c:124:56: error: ‘%02d’ directive output may be
truncated writing between 2 and 11 bytes into a region of size 3
[-Werror=format-truncation=]
124 | snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
| ^~~~
qemu/target/i386/hax-posix.c:124:41: note: directive argument in the range
[-2147483648, 64]
124 | snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
| ^~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:867,
from qemu/include/qemu/osdep.h:99,
from qemu/target/i386/hax-posix.c:14:
/usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output
between 17 and 26 bytes into a destination of size 17
67 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
68 | __bos (__s), __fmt, __va_arg_pack ());
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Several objects implemented their own uint property getters and setters,
despite them being straightforward (without any checks/validations on
the values themselves) and identical across objects. This makes use of
an enhanced API for object_property_add_uintXX_ptr() which offers
default setters.
Some of these setters used to update the value even if the type visit
failed (eg. because the value being set overflowed over the given type).
The new setter introduces a check for these errors, not updating the
value if an error occurred. The error is propagated.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Currently, ich9_lpc_initfn simply serves as a caller to
ich9_lpc_add_properties. This simplifies the code a bit by eliminating
ich9_lpc_add_properties altogether and executing its logic in the parent
object initialiser function.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When QOM APIs were added to ich9 in 6f1426ab, the getter for sci_int was
written using uint32_t. However, the object property is uint8_t. This
fixes the getter for correctness.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Traditionally, the uint-specific property helpers only offer getters.
When adding object (or class) uint types, one must therefore use the
generic property helper if a setter is needed (and probably duplicate
some code writing their own getters/setters).
This enhances the uint-specific property helper APIs by adding a
bitwise-or'd 'flags' field and modifying all clients of that API to set
this paramater to OBJ_PROP_FLAG_READ. This maintains the current
behaviour whilst allowing others to also set OBJ_PROP_FLAG_WRITE (or use
the more convenient OBJ_PROP_FLAG_READWRITE) in the future (which will
automatically install a setter). Other flags may be added later.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Currently only the final page offset is being passed to the `log_clear`
hook via `memory_region_clear_dirty_bitmap` after it is used as an
iterator in `cpu_physical_memory_test_and_clear_dirty`. This patch
corrects the start address and size of the region.
Signed-off-by: Matt Borgerson <contact@mborgerson.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Install the SASL library to build the VNC SASL auth protocol code.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200309122454.22551-3-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The Simple Authentication and Security Layer (SASL) library
re-defines the struct iovec on Win32 [*]. QEMU also re-defines
it in "qemu/osdep.h". The two definitions then clash on a MinGW
build.
We can avoid the SASL definition by defining STRUCT_IOVEC_DEFINED.
Since QEMU already defines 'struct iovec' if it is missing, add
the definition to vnc_sasl_cflags to avoid SASL re-defining it.
[*] https://github.com/cyrusimap/cyrus-sasl/blob/cyrus-sasl-2.1.27/include/sasl.h#L187
Cc: Alexey Pavlov <alexpux@gmail.com>
Cc: Biswapriyo Nath <nathbappai@gmail.com>
Reported-by: Youry Metlitsky <winaes@yandex.ru>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200309122454.22551-2-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
On upgrades the old .so files usually are replaced. But on the other
hand since a qemu process represents a guest instance it is usually kept
around.
That makes late addition of dynamic features e.g. 'hot-attach of a ceph
disk' fail by trying to load a new version of e.f. block-rbd.so into an
old still running qemu binary.
This adds a fallback to also load modules from a versioned directory in the
temporary /var/run path. That way qemu is providing a way for packaging
to store modules of an upgraded qemu package as needed until the next reboot.
An example how that can then be used in packaging can be seen in:
https://git.launchpad.net/~paelzer/ubuntu/+source/qemu/log/?h=bug-1847361-miss-old-so-on-upgrade-UBUNTU
Fixes: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1847361
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200310145806.18335-2-christian.ehrhardt@canonical.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Since commit d52c454aad ("contrib: add
vhost-user-gpu"), qemu-ga is linking with pixman.
This is because the Make-based build-system use a global namespace for
variables, and we rely on "main.o-libs" for different linking targets.
Note: this kind of variable clashing is hard to fix or prevent
currently. meson should help, as declarations have a linear
dependency and doesn't rely so much on variables and clever tricks.
Note2: we have a lot of main.c (or other duplicated names!) in
tree. Imho, it would be annoying and a bad workaroud to rename all
those to avoid conflicts like I did here.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1811670
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200311160923.882474-1-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The mutex and condition variable were never initialized, causing
-mem-prealloc to abort with an assertion failure.
Fixes: 037fb5eb39
Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Cc: bauerchen <bauerchen@tencent.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
vtd_irte_get failed to check the index against the configured table
size, causing an out-of-bounds access on guest memory and potentially
misinterpreting the result.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Message-Id: <4b15b728-bdfe-3bbe-3a5c-ca3baeef3c5c@siemens.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
HAXM covers below files:
include/sysemu/hax.h
target/i386/hax-*
V2: Add HAXM github page for wiki and issue tracking.
Cc: Wenchao Wang <wenchao.wang@intel.com>
Cc: Hang Yuan <hang.yuan@intel.com>
Reviewed-by: Hang Yuan <hang.yuan@intel.com>
Signed-off-by: Colin Xu <colin.xu@intel.com>
Message-Id: <20200228012046.6629-1-colin.xu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Currently, WHPX is using some default values for the trapped CPUID
functions. These were not in sync with the QEMU values because the
CPUID values were never set with WHPX during VCPU initialization.
Additionally, at the moment, WHPX doesn't support setting CPUID
values in the hypervisor at runtime (i.e. after the partition has
been setup). That is needed to be able to set the CPUID values in
the hypervisor during VCPU init.
Until that support comes, use the QEMU values for the trapped CPUIDs.
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Message-Id: <SN4PR2101MB0880A8323EAD0CD0E8E2F423C0EB0@SN4PR2101MB0880.namprd21.prod.outlook.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Currently, TSC is set as part of the VM runtime state. Setting TSC at
runtime is heavy and additionally can have side effects on the guest,
which are not very resilient to variances in the TSC. This patch uses
the VM state to determine whether to set TSC or not. Some minor
enhancements for getting TSC values as well that considers the VM state.
Additionally, while setting the TSC, the partition is suspended to
reduce the variance in the TSC value across vCPUs.
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Message-Id: <SN4PR2101MB08804D23439166E81FF151F7C0EA0@SN4PR2101MB0880.namprd21.prod.outlook.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
And intialize buffer_is_zero() with it, when Intel AVX512F is
available on host.
This function utilizes Intel AVX512 fundamental instructions which
is faster than its implementation with AVX2 (in my unit test, with
4K buffer, on CascadeLake SP, ~36% faster, buffer_zero_avx512() V.S.
buffer_zero_avx2()).
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Description copied from Linux kernel commit from Gustavo A. R. Silva
(see [3]):
--v-- description start --v--
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to
declare variable-length types such as these ones is a flexible
array member [1], introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler
warning in case the flexible array does not occur last in the
structure, which will help us prevent some kind of undefined
behavior bugs from being unadvertenly introduced [2] to the
Linux codebase from now on.
--^-- description end --^--
Do the similar housekeeping in the QEMU codebase (which uses
C99 since commit 7be41675f7).
All these instances of code were found with the help of the
following command (then manual analysis, without modifying
structures only having a single flexible array member, such
QEDTable in block/qed.h):
git grep -F '[0];'
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76497732932f
[3] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?id=17642a2fbd2c1
Inspired-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Description copied from Linux kernel commit from Gustavo A. R. Silva
(see [3]):
--v-- description start --v--
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to
declare variable-length types such as these ones is a flexible
array member [1], introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler
warning in case the flexible array does not occur last in the
structure, which will help us prevent some kind of undefined
behavior bugs from being unadvertenly introduced [2] to the
Linux codebase from now on.
--^-- description end --^--
Do the similar housekeeping in the QEMU codebase (which uses
C99 since commit 7be41675f7).
All these instances of code were found with the help of the
following Coccinelle script:
@@
identifier s, m, a;
type t, T;
@@
struct s {
...
t m;
- T a[0];
+ T a[];
};
@@
identifier s, m, a;
type t, T;
@@
struct s {
...
t m;
- T a[0];
+ T a[];
} QEMU_PACKED;
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76497732932f
[3] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?id=17642a2fbd2c1
Inspired-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Right now the PVH option rom scans for the RSDP from 0xE0000 to
0xE1FFF. This is probobly a typo, it should scan from 0xE0000 to
0xFFFFF.
This is actually an issue on some QEMU versions/machines. For example,
when I run QEMU the RSDP is placed at 0xf5ad0 which will not be picked
up by the current implementation.
This bug still allows a Linux guest to boot (in most configurations) as
the kernel will just scan for the RSDP if one isn't provided.
Signed-off-by: Joe Richey <joerichey@google.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Fixes: 2785dc7b17 ("optionrom: add new PVH option rom")
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Compile error reported by gcc 10.0.1:
scsi/qemu-pr-helper.c: In function ‘multipath_pr_out’:
scsi/qemu-pr-helper.c:523:32: error: array subscript <unknown> is outside array bounds of ‘struct transportid *[0]’ [-Werror=array-bounds]
523 | paramp.trnptid_list[paramp.num_transportid++] = id;
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from scsi/qemu-pr-helper.c:36:
/usr/include/mpath_persist.h:168:22: note: while referencing ‘trnptid_list’
168 | struct transportid *trnptid_list[];
| ^~~~~~~~~~~~
scsi/qemu-pr-helper.c:424:35: note: defined here ‘paramp’
424 | struct prout_param_descriptor paramp;
| ^~~~~~
This highlights an actual implementation issue in function multipath_pr_out.
The variable paramp is declared with type `struct prout_param_descriptor`,
which is a struct terminated by an empty array in mpath_persist.h:
struct transportid *trnptid_list[];
That empty array was filled with code that looked like that:
trnptid_list[paramp.descr.num_transportid++] = id;
This is an actual out-of-bounds access.
The fix is to malloc `paramp`.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The bochs-display mmio bar has some sub-regions with the actual hardware
registers. What happens when the guest access something outside those
regions depends on the archirecture. On x86 those reads succeed (and
return 0xff I think). On risc-v qemu aborts.
This patch adds handlers for the parent region, to make the wanted
behavior explicit and to make things consistent across architectures.
v2:
- use existing unassigned_io_ops.
- also cover stdvga.
Cc: Alistair Francis <alistair23@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200309100009.17624-1-kraxel@redhat.com