Re: [PATCH 3/7] hw/acpi/{ich9,piix4}: Resolve redundant io_base address attributes
Hi Bernhard,
On 22/1/23 18:07, Bernhard Beschow wrote:
A MemoryRegion has an addr attribute which gets set to the same values
as the redundant io_addr attributes.
Signed-off-by: Bernhard Beschow
---
include/hw/acpi/ich9.h | 1 -
include/hw/acpi/piix4.h | 2 --
hw/acpi/ich9.c | 17 -
hw/acpi/piix4.c | 11 ++-
4 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 370b34eacf..2e9bc63fca 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -91,13 +91,14 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
static void pm_io_space_update(PIIX4PMState *s)
{
PCIDevice *d = PCI_DEVICE(s);
+uint32_t io_base;
-s->io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
-s->io_base &= 0xffc0;
+io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
+io_base &= 0xffc0;
memory_region_transaction_begin();
memory_region_set_enabled(&s->io, d->config[0x80] & 1);
-memory_region_set_address(&s->io, s->io_base);
+memory_region_set_address(&s->io, io_base);
OK for this part.
memory_region_transaction_commit();
}
@@ -433,8 +434,8 @@ static void piix4_pm_add_properties(PIIX4PMState *s)
&s->ar.gpe.len, OBJ_PROP_FLAG_READ);
object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT,
&sci_int, OBJ_PROP_FLAG_READ);
-object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
- &s->io_base, OBJ_PROP_FLAG_READ);
+object_property_add_uint64_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
+ &s->io.addr, OBJ_PROP_FLAG_READ);
+Eduardo/Mark
We shouldn't do that IMO, because we access an internal field from
another QOM object.
We can however alias the MR property:
object_property_add_alias(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
OBJECT(&s->io), "addr");
}
Re: [PATCH 6/7] hw/acpi: Trace GPE access in all device models, not just PIIX4
On 22/1/23 18:07, Bernhard Beschow wrote: Signed-off-by: Bernhard Beschow --- hw/acpi/core.c | 5 + hw/acpi/piix4.c | 3 --- hw/acpi/trace-events | 8 3 files changed, 9 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC PATCH v5 3/9] target/arm: Use "max" as default cpu for the virt machine with KVM
On 20/01/2023 19.48, Fabiano Rosas wrote:
Now that the cortex-a15 is under CONFIG_TCG, use as default CPU for a
KVM-only build the 'max' cpu.
Note that we cannot use 'host' here because the qtests can run without
any other accelerator (than qtest) and 'host' depends on KVM being
enabled.
Signed-off-by: Fabiano Rosas
Cc: Daniel P. Berrangé
Cc: Thomas Huth
---
hw/arm/virt.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b5c711c919..8091c74e3d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3005,7 +3005,11 @@ static void virt_machine_class_init(ObjectClass *oc,
void *data)
mc->minimum_page_bits = 12;
mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids;
mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
+#ifdef CONFIG_TCG
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
+#else
+mc->default_cpu_type = ARM_CPU_TYPE_NAME("max");
+#endif
mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
mc->kvm_type = virt_kvm_type;
assert(!mc->get_hotplug_handler);
Thanks for the update, this sounds like the best option to me.
Reviewed-by: Thomas Huth
Re: [PATCH v2 06/10] tcg/loongarch64: Introduce tcg_out_addi
On 1/18/23 09:11, Richard Henderson wrote: Adjust the constraints to allow any int32_t for immediate addition. Split immediate adds into addu16i + addi, which covers quite a lot of the immediate space. For the hole in the middle, load the constant into TMP0 instead. Signed-off-by: Richard Henderson --- tcg/loongarch64/tcg-target-con-set.h | 4 +- tcg/loongarch64/tcg-target-con-str.h | 2 +- tcg/loongarch64/tcg-target.c.inc | 57 3 files changed, 53 insertions(+), 10 deletions(-) I've checked some generated code and this indeed benefits. Reviewed-by: WANG Xuerui
Re: [PATCH v2 07/10] tcg/loongarch64: Improve setcond expansion
On 1/18/23 09:11, Richard Henderson wrote: Split out a helper function, tcg_out_setcond_int, which does not always produce the complete boolean result, but returns a set of flags to do so. Accept all int32_t as constant input, so that LE/GT can adjust the constant to LT. Signed-off-by: Richard Henderson --- tcg/loongarch64/tcg-target.c.inc | 165 +-- 1 file changed, 115 insertions(+), 50 deletions(-) Reviewed-by: WANG Xuerui Thanks!
Re: [PATCH 0/2] vhost-user: Remove the nested event loop to unbreak the DPDK use case
On Thu, 19 Jan 2023 18:24:22 +0100 Greg Kurz wrote: > The nested event loop was introduced in QEMU 6.0 to allow servicing > of requests coming from the slave channel while waiting for an ack > from the back-end on the master socket. It turns out this is fragile > and breaks if the servicing of the slave channel causes a new message > to be sent on the master socket. This is exactly what happens when > using DPDK as reported in [0]. > > The only identified user for the nested loop is DAX enablement that > isn't upstream yet. Just drop the code for now. Some more clever > solution should be designed when the need to service concurrent > requests from both channels arises again. > > Greg Kurz (2): > Revert "vhost-user: Monitor slave channel in vhost_user_read()" > Revert "vhost-user: Introduce nested event loop in vhost_user_read()" > > hw/virtio/vhost-user.c | 100 - > 1 file changed, 8 insertions(+), 92 deletions(-) > Hi Michael, Can you please merge this series as you kindly proposed in [0] ? This will help to fix [1] which is currently blocking downstream testing. Cheers, -- Greg [0] https://lore.kernel.org/qemu-devel/20230118060102-mutt-send-email-...@kernel.org/ [1] https://bugzilla.redhat.com/show_bug.cgi?id=2155173
Re: [PATCH qemu v3] x86: don't let decompressed kernel image clobber setup_data
On 30/12/22 23:07, Jason A. Donenfeld wrote:
The setup_data links are appended to the compressed kernel image. Since
the kernel image is typically loaded at 0x10, setup_data lives at
`0x10 + compressed_size`, which does not get relocated during the
kernel's boot process.
The kernel typically decompresses the image starting at address
0x100 (note: there's one more zero there than the compressed image
above). This usually is fine for most kernels.
However, if the compressed image is actually quite large, then
setup_data will live at a `0x10 + compressed_size` that extends into
the decompressed zone at 0x100. In other words, if compressed_size
is larger than `0x100 - 0x10`, then the decompression step will
clobber setup_data, resulting in crashes.
Visually, what happens now is that QEMU appends setup_data to the kernel
image:
kernel imagesetup_data
|--|||
0x10 0x10+l1 0x10+l1+l2
The problem is that this decompresses to 0x100 (one more zero). So
if l1 is > (0x100-0x10), then this winds up looking like:
kernel imagesetup_data
|--|||
0x10 0x10+l1 0x10+l1+l2
d e c o m p r e s s e d k e r n e l
|-|
0x100
0x100+l3
The decompressed kernel seemingly overwriting the compressed kernel
image isn't a problem, because that gets relocated to a higher address
early on in the boot process, at the end of startup_64. setup_data,
however, stays in the same place, since those links are self referential
and nothing fixes them up. So the decompressed kernel clobbers it.
Fix this by appending setup_data to the cmdline blob rather than the
kernel image blob, which remains at a lower address that won't get
clobbered.
This could have been done by overwriting the initrd blob instead, but
that poses big difficulties, such as no longer being able to use memory
mapped files for initrd, hurting performance, and, more importantly, the
initrd address calculation is hard coded in qboot, and it always grows
down rather than up, which means lots of brittle semantics would have to
be changed around, incurring more complexity. In contrast, using cmdline
is simple and doesn't interfere with anything.
The microvm machine has a gross hack where it fiddles with fw_cfg data
after the fact. So this hack is updated to account for this appending,
by reserving some bytes.
Cc: x...@kernel.org
Cc: Philippe Mathieu-Daudé
Cc: H. Peter Anvin
Cc: Borislav Petkov
Cc: Eric Biggers
Signed-off-by: Jason A. Donenfeld
---
Changes v2->v3:
- Fix mistakes in string handling.
Changes v1->v2:
- Append setup_data to cmdline instead of kernel image.
hw/i386/microvm.c | 13 ++
hw/i386/x86.c | 50 +++
hw/nvram/fw_cfg.c | 9 +++
include/hw/i386/microvm.h | 5 ++--
include/hw/nvram/fw_cfg.h | 9 +++
5 files changed, 54 insertions(+), 32 deletions(-)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index a00881bc64..432754eda4 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -741,6 +741,15 @@ void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void
*data, size_t len)
fw_cfg_add_bytes_callback(s, key, NULL, NULL, NULL, data, len, true);
}
+void *fw_cfg_read_bytes_ptr(FWCfgState *s, uint16_t key)
+{
+int arch = !!(key & FW_CFG_ARCH_LOCAL);
+
+key &= FW_CFG_ENTRY_MASK;
+assert(key < fw_cfg_max_entry(s));
+return s->entries[arch][key].data;
Shouldn't it be safer to provide a size argument, and return
NULL if s->entries[arch][key].len < size?
Maybe this API should return a (casted) const pointer, so the
only way to update the key is via fw_cfg_add_bytes().
+}
+
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 2e503904dc..990dcdbb2e 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -139,6 +139,15 @@ void fw_cfg_add_bytes_callback(FWCfgState *s, uint16_t key,
void *data, size_t len,
bool read_only);
+/**
+ * fw_cfg_read_bytes_ptr:
+ * @s: fw_cfg device being modified
+ * @key: selector key value for new fw_cfg item
+ *
+ * Reads an existing fw_cfg data pointer.
+ */
+void *fw_cfg_read_bytes_ptr(FWCfgState *s, uint16_t key);
+
/**
* fw_cfg_add_string:
* @s: fw_cfg device being modified
Re: [PATCH v2 09/10] tcg/loongarch64: Use tcg_pcrel_diff in tcg_out_ldst
On 18/1/23 02:11, Richard Henderson wrote: Take the w^x split into account when computing the pc-relative distance to an absolute pointer. Signed-off-by: Richard Henderson --- tcg/loongarch64/tcg-target.c.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 05/10] tcg/loongarch64: Update tcg-insn-defs.c.inc
On 18/1/23 02:11, Richard Henderson wrote: Regenerate with ADDU16I included: $ cd loongarch-opcodes/scripts/go $ go run ./genqemutcgdefs > $QEMU/tcg/loongarch64/tcg-insn-defs.c.inc Signed-off-by: Richard Henderson --- tcg/loongarch64/tcg-insn-defs.c.inc | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) mode change 100644 => 100755 tcg/loongarch64/tcg-insn-defs.c.inc Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 0/3] Misc sm501 clean ups
On 21/1/23 21:35, BALATON Zoltan wrote: Some small trivial clean ups I've found while looking at this file. BALATON Zoltan (3): hw/display/sm501: Remove parenthesis around consant macro definitions hw/display/sm501: Remove unneeded casts from void pointer hw/display/sm501: Code style fix hw/display/sm501.c | 419 +++-- 1 file changed, 210 insertions(+), 209 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[PATCH v2 2/2] qapi, audio: Make introspection reflect build configuration more closely
From: Daniel P. Berrangé
Currently the -audiodev accepts any audiodev type regardless of what is
built in to QEMU. An error only occurs later at runtime when a sound
device tries to use the audio backend.
With this change QEMU will immediately reject -audiodev args that are
not compiled into the binary. The QMP schema will also be introspectable
to identify what is compiled in.
This also helps to avoid compiling code that is not required in the
binary. Note: When building the audiodevs as modules, the patch only
compiles out code for modules that we don't build at all.
Signed-off-by: Daniel P. Berrangé
[thuth: Rebase, take sndio and dbus devices into account]
Signed-off-by: Thomas Huth
---
qapi/audio.json| 44 ++
audio/audio_template.h | 20 +++
audio/audio.c | 20 +++
audio/audio_legacy.c | 41 ++-
4 files changed, 112 insertions(+), 13 deletions(-)
diff --git a/qapi/audio.json b/qapi/audio.json
index c7aafa2763..4e54c00f51 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -408,8 +408,18 @@
# Since: 4.0
##
{ 'enum': 'AudiodevDriver',
- 'data': [ 'none', 'alsa', 'coreaudio', 'dbus', 'dsound', 'jack', 'oss', 'pa',
-'sdl', 'sndio', 'spice', 'wav' ] }
+ 'data': [ 'none',
+{ 'name': 'alsa', 'if': 'CONFIG_AUDIO_ALSA' },
+{ 'name': 'coreaudio', 'if': 'CONFIG_AUDIO_COREAUDIO' },
+{ 'name': 'dbus', 'if': 'CONFIG_DBUS_DISPLAY' },
+{ 'name': 'dsound', 'if': 'CONFIG_AUDIO_DSOUND' },
+{ 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
+{ 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
+{ 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
+{ 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
+{ 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
+{ 'name': 'spice', 'if': 'CONFIG_SPICE' },
+'wav' ] }
##
# @Audiodev:
@@ -432,16 +442,26 @@
'discriminator': 'driver',
'data': {
'none': 'AudiodevGenericOptions',
-'alsa': 'AudiodevAlsaOptions',
-'coreaudio': 'AudiodevCoreaudioOptions',
-'dbus': 'AudiodevGenericOptions',
-'dsound':'AudiodevDsoundOptions',
-'jack': 'AudiodevJackOptions',
-'oss': 'AudiodevOssOptions',
-'pa':'AudiodevPaOptions',
-'sdl': 'AudiodevSdlOptions',
-'sndio': 'AudiodevSndioOptions',
-'spice': 'AudiodevGenericOptions',
+'alsa': { 'type': 'AudiodevAlsaOptions',
+ 'if': 'CONFIG_AUDIO_ALSA' },
+'coreaudio': { 'type': 'AudiodevCoreaudioOptions',
+ 'if': 'CONFIG_AUDIO_COREAUDIO' },
+'dbus': { 'type': 'AudiodevGenericOptions',
+ 'if': 'CONFIG_DBUS_DISPLAY' },
+'dsound':{ 'type': 'AudiodevDsoundOptions',
+ 'if': 'CONFIG_AUDIO_DSOUND' },
+'jack': { 'type': 'AudiodevJackOptions',
+ 'if': 'CONFIG_AUDIO_JACK' },
+'oss': { 'type': 'AudiodevOssOptions',
+ 'if': 'CONFIG_AUDIO_OSS' },
+'pa':{ 'type': 'AudiodevPaOptions',
+ 'if': 'CONFIG_AUDIO_PA' },
+'sdl': { 'type': 'AudiodevSdlOptions',
+ 'if': 'CONFIG_AUDIO_SDL' },
+'sndio': { 'type': 'AudiodevSndioOptions',
+ 'if': 'CONFIG_AUDIO_SNDIO' },
+'spice': { 'type': 'AudiodevGenericOptions',
+ 'if': 'CONFIG_SPICE' },
'wav': 'AudiodevWavOptions' } }
##
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 720a32e57e..42b4712acb 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -326,27 +326,47 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
TYPE)(Audiodev *dev)
switch (dev->driver) {
case AUDIODEV_DRIVER_NONE:
return dev->u.none.TYPE;
+#ifdef CONFIG_AUDIO_ALSA
case AUDIODEV_DRIVER_ALSA:
return qapi_AudiodevAlsaPerDirectionOptions_base(dev->u.alsa.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_COREAUDIO
case AUDIODEV_DRIVER_COREAUDIO:
return qapi_AudiodevCoreaudioPerDirectionOptions_base(
dev->u.coreaudio.TYPE);
+#endif
+#ifdef CONFIG_DBUS_DISPLAY
case AUDIODEV_DRIVER_DBUS:
return dev->u.dbus.TYPE;
+#endif
+#ifdef CONFIG_AUDIO_DSOUND
case AUDIODEV_DRIVER_DSOUND:
return dev->u.dsound.TYPE;
+#endif
+#ifdef CONFIG_AUDIO_JACK
case AUDIODEV_DRIVER_JACK:
return qapi_AudiodevJackPerDirectionOptions_base(dev->u.jack.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_OSS
case AUDIODEV_DRIVER_OSS:
return qapi_AudiodevOssPerDirectionOptions_base(dev->u.oss.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_PA
case AUDIODEV_DRIVER_PA:
return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_SDL
case AUDIODEV_DRIVER_SDL:
return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE)
Re: [PATCH v2 10/10] tcg/loongarch64: Reorg goto_tb implementation
On 1/18/23 09:11, Richard Henderson wrote: The old implementation replaces two insns, swapping between b nop and pcaddu18i tmp, jirl zero, tmp, & 0x There is a race condition in which a thread could be stopped at the jirl, i.e. with the top of the address loaded, and when restarted we have re-linked to a different TB, so that the top half no longer matches the bottom half. Note that while we never directly re-link to a different TB, we can link, unlink, and link again all while the stopped thread remains stopped. The new implementation replaces only one insn, swapping between b and pcadd tmp, falling through to load the address from tmp, and branch. Signed-off-by: Richard Henderson --- tcg/loongarch64/tcg-target.h | 7 +--- tcg/loongarch64/tcg-target.c.inc | 72 ++-- 2 files changed, 33 insertions(+), 46 deletions(-) I've tested this on my 3A5000 box and things seem to work, thanks. Reviewed-by: WANG Xuerui
Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation
On 20/1/23 16:54, Evgeny Iakovlev wrote:
UART should be enabled in general and have RX enabled specifically to be
able to receive data from peripheral device. Same goes for transmitting
data to peripheral device and a TXE flag.
Check if UART CR register has EN and RXE or TXE bits enabled before
trying to receive or transmit data.
Signed-off-by: Evgeny Iakovlev
Reviewed-by: Peter Maydell
---
hw/char/pl011.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
+static inline bool pl011_can_transmit(PL011State *s)
+{
+return s->cr & PL011_CR_UARTEN && s->cr & PL011_CR_TXE;
+}
+
static void pl011_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
@@ -221,7 +231,9 @@ static void pl011_write(void *opaque, hwaddr offset,
switch (offset >> 2) {
case 0: /* UARTDR */
-/* ??? Check if transmitter is enabled. */
+if (!pl011_can_transmit(s)) {
+break;
+}
ch = value;
/* XXX this blocks entire thread. Rewrite to use
* qemu_chr_fe_write and background I/O callbacks */
@@ -292,7 +304,11 @@ static int pl011_can_receive(void *opaque)
PL011State *s = (PL011State *)opaque;
int r;
-r = s->read_count < pl011_get_fifo_depth(s);
+if (!(s->cr & PL011_CR_UARTEN) || !(s->cr & PL011_CR_RXE)) {
Maybe add pl011_can_receive() similarly to pl011_can_transmit().
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé
+r = 0;
+} else {
+r = s->read_count < pl011_get_fifo_depth(s);
+}
trace_pl011_can_receive(s->lcr, s->read_count, r);
return r;
}
[PATCH v2 1/2] qapi, audio: add query-audiodev command
From: Daniel P. Berrangé
Way back in QEMU 4.0, the -audiodev command line option was introduced
for configuring audio backends. This CLI option does not use QemuOpts
so it is not visible for introspection in 'query-command-line-options',
instead using the QAPI Audiodev type. Unfortunately there is also no
QMP command that uses the Audiodev type, so it is not introspectable
with 'query-qmp-schema' either.
This introduces a 'query-audiodev' command that simply reflects back
the list of configured -audiodev command line options. This alone is
maybe not very useful by itself, but it makes Audiodev introspectable
via 'query-qmp-schema', so that libvirt (and other upper layer tools)
can discover the available audiodevs.
Signed-off-by: Daniel P. Berrangé
[thuth: Update for upcoming QEMU v8.0, and use QAPI_LIST_PREPEND]
Signed-off-by: Thomas Huth
---
qapi/audio.json | 13 +
audio/audio.c | 12
2 files changed, 25 insertions(+)
diff --git a/qapi/audio.json b/qapi/audio.json
index 1e0a24bdfc..c7aafa2763 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -443,3 +443,16 @@
'sndio': 'AudiodevSndioOptions',
'spice': 'AudiodevGenericOptions',
'wav': 'AudiodevWavOptions' } }
+
+##
+# @query-audiodevs:
+#
+# Returns information about audiodev configuration
+#
+# Returns: array of @Audiodev
+#
+# Since: 8.0
+#
+##
+{ 'command': 'query-audiodevs',
+ 'returns': ['Audiodev'] }
diff --git a/audio/audio.c b/audio/audio.c
index d849a94a81..6f270c07b7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -28,8 +28,10 @@
#include "monitor/monitor.h"
#include "qemu/timer.h"
#include "qapi/error.h"
+#include "qapi/clone-visitor.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qapi-visit-audio.h"
+#include "qapi/qapi-commands-audio.h"
#include "qemu/cutils.h"
#include "qemu/module.h"
#include "qemu/help_option.h"
@@ -2311,3 +2313,13 @@ size_t audio_rate_get_bytes(RateCtl *rate, struct
audio_pcm_info *info,
return bytes;
}
+
+AudiodevList *qmp_query_audiodevs(Error **errp)
+{
+AudiodevList *ret = NULL;
+AudiodevListEntry *e;
+QSIMPLEQ_FOREACH(e, &audiodevs, next) {
+QAPI_LIST_PREPEND(ret, QAPI_CLONE(Audiodev, e->dev));
+}
+return ret;
+}
--
2.31.1
[PATCH v2 0/2] audio: make audiodev introspectable by management apps
Here's a respin from Daniel's audiodev introspection patches from 2021. I've rebased them to the current master branch and addressed the review comments from v1. The Audiodev QAPI type is not introspectable via query-qmp-schema as nothing in QMP uses it. "-audiodev" is not introspectable via query-command-line-options because it avoided legacy QemuOpts. To fix it, introduce a tiny "query-audiodev" QMP command that uses the "Audiodev" QAPI structure, so that it shows up in the schema. Then mark the various backend types with conditionals so that only the ones that were available at compile time show up in the schema. Daniel P. Berrangé (2): qapi, audio: add query-audiodev command qapi, audio: Make introspection reflect build configuration more closely qapi/audio.json| 57 +- audio/audio_template.h | 20 +++ audio/audio.c | 32 audio/audio_legacy.c | 41 +- 4 files changed, 137 insertions(+), 13 deletions(-) -- 2.31.1
Re: [PATCH v2 01/10] target/loongarch: Enable the disassembler for host tcg
On 18/1/23 02:11, Richard Henderson wrote: Reuse the decodetree based disassembler from target/loongarch/ for tcg/loongarch64/. The generation of decode-insns.c.inc into ./libcommon.fa.p/ could eventually result in conflict, if any other host requires the same trick, but this is good enough for now. Signed-off-by: Richard Henderson --- disas.c | 2 ++ target/loongarch/meson.build | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
[RFC v2 2/2] spice: Add an option to forward the dmabuf directly to the encoder (v2)
This patch adds support for gl=on and port != 0. In other words,
with this option enabled, it should be possible to stream the
content associated with the dmabuf to a remote client.
Here is the flow of things from the Qemu side:
- Call gl_scanout (to update the fd) and gl_draw_async just like
in the local display case.
- Additionally, create an update with the cmd set to QXL_CMD_DRAW
to trigger the creation of a new drawable (associated with the fd)
by the Spice server.
- Wait (or block) until the Encoder is done encoding the content.
- Unblock the pipeline once the async completion cookie is received.
v2:
- Use the existing gl_scanout and gl_draw_async APIs instead of
adding new ones.
Cc: Gerd Hoffmann
Cc: Marc-André Lureau
Cc: Dongwon Kim
Signed-off-by: Vivek Kasireddy
---
include/ui/spice-display.h | 1 +
qemu-options.hx| 6 ++-
ui/spice-core.c| 22 +--
ui/spice-display.c | 75 --
4 files changed, 81 insertions(+), 23 deletions(-)
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index e271e011da..df74f5ee9b 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -153,6 +153,7 @@ struct SimpleSpiceCursor {
};
extern bool spice_opengl;
+extern bool spice_dmabuf_encode;
int qemu_spice_rect_is_empty(const QXLRect* r);
void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r);
diff --git a/qemu-options.hx b/qemu-options.hx
index aab8df0922..3016f8a6f7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2143,7 +2143,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
" [,disable-agent-file-xfer=on|off][,agent-mouse=[on|off]]\n"
" [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n"
" [,preferred-codec=:\n"
-" [,gl=[on|off]][,rendernode=]\n"
+" [,gl=[on|off]][,rendernode=][,dmabuf-encode=[on|off]]\n"
" enable spice\n"
" at least one of {port, tls-port} is mandatory\n",
QEMU_ARCH_ALL)
@@ -2248,6 +2248,10 @@ SRST
``rendernode=``
DRM render node for OpenGL rendering. If not specified, it will
pick the first available. (Since 2.9)
+
+``dmabuf-encode=[on|off]``
+Forward the dmabuf directly to the encoder (Gstreamer).
+Default is off.
ERST
DEF("portrait", 0, QEMU_OPTION_portrait,
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6e00211e3a..c9b856b056 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -494,6 +494,9 @@ static QemuOptsList qemu_spice_opts = {
},{
.name = "rendernode",
.type = QEMU_OPT_STRING,
+},{
+.name = "dmabuf-encode",
+.type = QEMU_OPT_BOOL,
#endif
},
{ /* end of list */ }
@@ -843,11 +846,24 @@ static void qemu_spice_init(void)
g_free(password);
#ifdef HAVE_SPICE_GL
+if (qemu_opt_get_bool(opts, "dmabuf-encode", 0)) {
+spice_dmabuf_encode = 1;
+}
if (qemu_opt_get_bool(opts, "gl", 0)) {
-if ((port != 0) || (tls_port != 0)) {
-error_report("SPICE GL support is local-only for now and "
- "incompatible with -spice port/tls-port");
+if (((port != 0) || (tls_port != 0)) && !spice_dmabuf_encode) {
+error_report("Add dmabuf-encode=on option to enable GL streaming");
exit(1);
+} else if (spice_dmabuf_encode) {
+if (port == 0 && tls_port == 0) {
+error_report("dmabuf-encode=on is only meant to be used for "
+ "non-local displays");
+exit(1);
+}
+if (g_strcmp0(preferred_codec, "gstreamer:h264")) {
+error_report("dmabuf-encode=on currently only works and tested"
+ "with gstreamer:h264");
+exit(1);
+}
}
if (egl_rendernode_init(qemu_opt_get(opts, "rendernode"),
DISPLAYGL_MODE_ON) != 0) {
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 494168e7fe..90ada643a2 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -28,6 +28,7 @@
#include "ui/spice-display.h"
bool spice_opengl;
+bool spice_dmabuf_encode;
int qemu_spice_rect_is_empty(const QXLRect* r)
{
@@ -117,7 +118,7 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
}
static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
- QXLRect *rect)
+ QXLRect *rect, bool dmabuf)
{
SimpleSpiceUpdate *update;
QXLDrawable *drawable;
@@ -168,15 +169,17 @@ static void
qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
image->bitmap.palette = 0;
image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
-dest = pixman_image_create_bits(PIXMAN_LE_x8r8g8b8, bw, bh,
-(void *)update->bitmap, bw * 4);
-pixman_image_composit
[RFC v2 0/2] spice: Add an option to forward the dmabuf directly to the encoder (v2)
This patch series adds options to select a preferred codec and also to forward a dmabuf directly to the encoder module that is part of the Spice server. Currently, gstreamer:h264 is the only combination tested but additional work is ongoing to test other combinations. Tested with: -device virtio-gpu-pci,max_outputs=1,blob=true,xres=1920,yres=1080 -spice port=3001,gl=on,disable-ticketing=on,dmabuf-encode=on, preferred-codec=gstreamer:h264 and remote-viewer --spice-debug spice://x.x.x.x:3001 on the client side. Associated Spice server patches (v1) can be found here: https://lists.freedesktop.org/archives/spice-devel/2023-January/052927.html v2: - Used the already available gl_scanout and gl_draw_async APIs instead of adding new ones to Spice. - Improved the commit message of the second patch Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Dongwon Kim Vivek Kasireddy (2): spice: Add an option for users to provide a preferred codec spice: Add an option to forward the dmabuf directly to the encoder (v2) include/ui/spice-display.h | 1 + qemu-options.hx| 11 +- ui/spice-core.c| 36 -- ui/spice-display.c | 75 -- 4 files changed, 100 insertions(+), 23 deletions(-) -- 2.37.2
Re: [PATCH v0 0/4] backends/hostmem: add an ability to specify prealloc timeout
On 20.01.23 14:47, Daniil Tatianin wrote: This series introduces new qemu_prealloc_mem_with_timeout() api, which allows limiting the maximum amount of time to be spent on memory preallocation. It also adds prealloc statistics collection that is exposed via an optional timeout handler. This new api is then utilized by hostmem for guest RAM preallocation controlled via new object properties called 'prealloc-timeout' and 'prealloc-timeout-fatal'. This is useful for limiting VM startup time on systems with unpredictable page allocation delays due to memory fragmentation or the backing storage. The timeout can be configured to either simply emit a warning and continue VM startup without having preallocated the entire guest RAM or just abort startup entirely if that is not acceptable for a specific use case. The major use case for preallocation is memory resources that cannot be overcommitted (hugetlb, file blocks, ...), to avoid running out of such resources later, while the guest is already running, and crashing it. Allocating only a fraction "because it takes too long" looks quite useless in that (main use-case) context. We shouldn't encourage QEMU users to play with fire in such a way. IOW, there should be no way around "prealloc-timeout-fatal". Either preallocation succeeded and the guest can run, or it failed, and the guest can't run. ... but then, management tools can simply start QEMU with "-S", start an own timer, and zap QEMU if it didn't manage to come up in time, and simply start a new QEMU instance without preallocation enabled. The "good" thing about that approach is that it will also cover any implicit memory preallocation, like using mlock() or VFIO, that don't run in ordinary per-hostmem preallocation context. If setting QEMU up takes to long, you might want to try on a different hypervisor in your cluster instead. I don't immediately see why we want to make our preallcoation+hostmem implementation in QEMU more complicated for such a use case. -- Thanks, David / dhildenb
[PATCH v6 0/5] riscv: Allow user to set the satp mode
This introduces new properties to allow the user to set the satp mode, see patch 3 for full syntax. In addition, it prevents cpus to boot in a satp mode they do not support (see patch 5). v6: - Remove the valid_vm check in validate_vm and add it to the finalize function so that map already contains the constraint, Alex - Add forgotten mbare to satp_mode_from_str, Alex - Move satp mode properties handling to riscv_cpu_satp_mode_finalize, Andrew - Only add satp mode properties corresponding to the cpu, and then remove the check against valid_vm_1_10_32/64 in riscv_cpu_satp_mode_finalize, Andrew/Alistair/Alex - Move mmu-type setting to its own patch, Andrew - patch 5 is new and is a fix, Alex v5: - Simplify v4 implementation by leveraging valid_vm_1_10_32/64, as suggested by Andrew - Split the v4 patch into 2 patches as suggested by Andrew - Lot of other minor corrections, from Andrew - Set the satp mode N by disabling the satp mode N + 1 - Add a helper to set satp mode from a string, as suggested by Frank v4: - Use custom boolean properties instead of OnOffAuto properties, based on ARMVQMap, as suggested by Andrew v3: - Free sv_name as pointed by Bin - Replace satp-mode with boolean properties as suggested by Andrew - Removed RB from Atish as the patch considerably changed v2: - Use error_setg + return as suggested by Alistair - Add RB from Atish - Fixed checkpatch issues missed in v1 - Replaced Ludovic email address with the rivos one Alexandre Ghiti (5): riscv: Pass Object to register_cpu_props instead of DeviceState riscv: Change type of valid_vm_1_10_[32|64] to bool riscv: Allow user to set the satp mode riscv: Correctly set the device-tree entry 'mmu-type' riscv: Introduce satp mode hw capabilities hw/riscv/virt.c| 19 ++-- target/riscv/cpu.c | 247 +++-- target/riscv/cpu.h | 23 + target/riscv/csr.c | 29 +++--- 4 files changed, 287 insertions(+), 31 deletions(-) -- 2.37.2
[PATCH v6 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool
This array is actually used as a boolean so swap its current char type
to a boolean and at the same time, change the type of validate_vm to
bool since it returns valid_vm_1_10_[32|64].
Signed-off-by: Alexandre Ghiti
---
target/riscv/csr.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0db2c233e5..6b157806a5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1117,16 +1117,16 @@ static const target_ulong hip_writable_mask = MIP_VSSIP;
static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP |
MIP_VSEIP;
static const target_ulong vsip_writable_mask = MIP_VSSIP;
-static const char valid_vm_1_10_32[16] = {
-[VM_1_10_MBARE] = 1,
-[VM_1_10_SV32] = 1
+static const bool valid_vm_1_10_32[16] = {
+[VM_1_10_MBARE] = true,
+[VM_1_10_SV32] = true
};
-static const char valid_vm_1_10_64[16] = {
-[VM_1_10_MBARE] = 1,
-[VM_1_10_SV39] = 1,
-[VM_1_10_SV48] = 1,
-[VM_1_10_SV57] = 1
+static const bool valid_vm_1_10_64[16] = {
+[VM_1_10_MBARE] = true,
+[VM_1_10_SV39] = true,
+[VM_1_10_SV48] = true,
+[VM_1_10_SV57] = true
};
/* Machine Information Registers */
@@ -1209,7 +1209,7 @@ static RISCVException read_mstatus(CPURISCVState *env,
int csrno,
return RISCV_EXCP_NONE;
}
-static int validate_vm(CPURISCVState *env, target_ulong vm)
+static bool validate_vm(CPURISCVState *env, target_ulong vm)
{
if (riscv_cpu_mxl(env) == MXL_RV32) {
return valid_vm_1_10_32[vm & 0xf];
@@ -2648,7 +2648,8 @@ static RISCVException read_satp(CPURISCVState *env, int
csrno,
static RISCVException write_satp(CPURISCVState *env, int csrno,
target_ulong val)
{
-target_ulong vm, mask;
+target_ulong mask;
+bool vm;
if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
return RISCV_EXCP_NONE;
--
2.37.2
[PATCH v6 3/5] riscv: Allow user to set the satp mode
RISC-V specifies multiple sizes for addressable memory and Linux probes for
the machine's support at startup via the satp CSR register (done in
csr.c:validate_vm).
As per the specification, sv64 must support sv57, which in turn must
support sv48...etc. So we can restrict machine support by simply setting the
"highest" supported mode and the bare mode is always supported.
You can set the satp mode using the new properties "sv32", "sv39", "sv48",
"sv57" and "sv64" as follows:
-cpu rv64,sv57=on # Linux will boot using sv57 scheme
-cpu rv64,sv39=on # Linux will boot using sv39 scheme
-cpu rv64,sv57=off # Linux will boot using sv48 scheme
-cpu rv64 # Linux will boot using sv57 scheme by default
We take the highest level set by the user:
-cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme
We make sure that invalid configurations are rejected:
-cpu rv64,sv32=on # Can't enable 32-bit satp mode in 64-bit
-cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are
# enabled
We accept "redundant" configurations:
-cpu rv64,sv48=on,sv57=off # Linux will boot using sv48 scheme
And contradictory configurations:
-cpu rv64,sv48=on,sv48=off # Linux will boot using sv39 scheme
In addition, we now correctly set the device-tree entry 'mmu-type' using
those new properties.
Co-Developed-by: Ludovic Henry
Signed-off-by: Ludovic Henry
Signed-off-by: Alexandre Ghiti
---
target/riscv/cpu.c | 204 +
target/riscv/cpu.h | 19 +
target/riscv/csr.c | 12 ++-
3 files changed, 228 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7181b34f86..e409e6ab64 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -27,6 +27,7 @@
#include "time_helper.h"
#include "exec/exec-all.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "qemu/error-report.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
@@ -229,6 +230,79 @@ static void set_vext_version(CPURISCVState *env, int
vext_ver)
env->vext_ver = vext_ver;
}
+static uint8_t satp_mode_from_str(const char *satp_mode_str)
+{
+if (!strncmp(satp_mode_str, "mbare", 5)) {
+return VM_1_10_MBARE;
+}
+
+if (!strncmp(satp_mode_str, "sv32", 4)) {
+return VM_1_10_SV32;
+}
+
+if (!strncmp(satp_mode_str, "sv39", 4)) {
+return VM_1_10_SV39;
+}
+
+if (!strncmp(satp_mode_str, "sv48", 4)) {
+return VM_1_10_SV48;
+}
+
+if (!strncmp(satp_mode_str, "sv57", 4)) {
+return VM_1_10_SV57;
+}
+
+if (!strncmp(satp_mode_str, "sv64", 4)) {
+return VM_1_10_SV64;
+}
+
+g_assert_not_reached();
+}
+
+uint8_t satp_mode_max_from_map(uint32_t map)
+{
+/* map here has at least one bit set, so no problem with clz */
+return 31 - __builtin_clz(map);
+}
+
+const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
+{
+if (is_32_bit) {
+switch (satp_mode) {
+case VM_1_10_SV32:
+return "sv32";
+case VM_1_10_MBARE:
+return "none";
+}
+} else {
+switch (satp_mode) {
+case VM_1_10_SV64:
+return "sv64";
+case VM_1_10_SV57:
+return "sv57";
+case VM_1_10_SV48:
+return "sv48";
+case VM_1_10_SV39:
+return "sv39";
+case VM_1_10_MBARE:
+return "none";
+}
+}
+
+g_assert_not_reached();
+}
+
+/* Sets the satp mode to the max supported */
+static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
+{
+if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
+cpu->cfg.satp_mode.map |=
+(1 << satp_mode_from_str(is_32_bit ? "sv32" : "sv57"));
+} else {
+cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
+}
+}
+
static void riscv_any_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -619,6 +693,82 @@ static void riscv_cpu_disas_set_info(CPUState *s,
disassemble_info *info)
}
}
+static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
+{
+bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
+const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
+
+if (cpu->cfg.satp_mode.map == 0) {
+/*
+ * If unset by both the user and the cpu, we fallback to the default
+ * satp mode.
+ */
+if (cpu->cfg.satp_mode.init == 0) {
+set_satp_mode_default(cpu, rv32);
+} else {
+/*
+ * Find the lowest level that was disabled and then enable the
+ * first valid level below which can be found in
+ * valid_vm_1_10_32/64.
+ */
+for (int i = 1; i < 16; ++i) {
+if (!(cpu->cfg.satp_mode.map & (1 << i)) &&
+(cpu->cfg.satp_mode.init & (1 << i)) &&
+valid_vm[i]) {
+
[PATCH v6 4/5] riscv: Correctly set the device-tree entry 'mmu-type'
The 'mmu-type' should reflect what the hardware is capable of so use the
new satp_mode field in RISCVCPUConfig to do that.
Signed-off-by: Alexandre Ghiti
---
hw/riscv/virt.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 94ff2a1584..48d034a5f7 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -228,7 +228,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int
socket,
int cpu;
uint32_t cpu_phandle;
MachineState *mc = MACHINE(s);
-char *name, *cpu_name, *core_name, *intc_name;
+uint8_t satp_mode_max;
+char *name, *cpu_name, *core_name, *intc_name, *sv_name;
for (cpu = s->soc[socket].num_harts - 1; cpu >= 0; cpu--) {
cpu_phandle = (*phandle)++;
@@ -236,14 +237,14 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int
socket,
cpu_name = g_strdup_printf("/cpus/cpu@%d",
s->soc[socket].hartid_base + cpu);
qemu_fdt_add_subnode(mc->fdt, cpu_name);
-if (riscv_feature(&s->soc[socket].harts[cpu].env,
- RISCV_FEATURE_MMU)) {
-qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
-(is_32_bit) ? "riscv,sv32" : "riscv,sv48");
-} else {
-qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type",
-"riscv,none");
-}
+
+satp_mode_max = satp_mode_max_from_map(
+s->soc[socket].harts[cpu].cfg.satp_mode.map);
+sv_name = g_strdup_printf("riscv,%s",
+ satp_mode_str(satp_mode_max, is_32_bit));
+qemu_fdt_setprop_string(mc->fdt, cpu_name, "mmu-type", sv_name);
+g_free(sv_name);
+
name = riscv_isa_string(&s->soc[socket].harts[cpu]);
qemu_fdt_setprop_string(mc->fdt, cpu_name, "riscv,isa", name);
g_free(name);
--
2.37.2
[RFC v2 1/2] spice: Add an option for users to provide a preferred codec
Giving users an option to choose a particular codec will enable
them to make an appropriate decision based on their hardware and
use-case.
Cc: Gerd Hoffmann
Cc: Marc-André Lureau
Cc: Dongwon Kim
Signed-off-by: Vivek Kasireddy
---
qemu-options.hx | 5 +
ui/spice-core.c | 14 ++
2 files changed, 19 insertions(+)
diff --git a/qemu-options.hx b/qemu-options.hx
index 3aa3a2f5a3..aab8df0922 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2142,6 +2142,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
" [,streaming-video=[off|all|filter]][,disable-copy-paste=on|off]\n"
" [,disable-agent-file-xfer=on|off][,agent-mouse=[on|off]]\n"
" [,playback-compression=[on|off]][,seamless-migration=[on|off]]\n"
+" [,preferred-codec=:\n"
" [,gl=[on|off]][,rendernode=]\n"
" enable spice\n"
" at least one of {port, tls-port} is mandatory\n",
@@ -2237,6 +2238,10 @@ SRST
``seamless-migration=[on|off]``
Enable/disable spice seamless migration. Default is off.
+``preferred-codec=:``
+Provide the preferred codec the Spice server should use.
+Default would be spice:mjpeg.
+
``gl=[on|off]``
Enable/disable OpenGL context. Default is off.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 72f8f1681c..6e00211e3a 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -469,6 +469,9 @@ static QemuOptsList qemu_spice_opts = {
},{
.name = "streaming-video",
.type = QEMU_OPT_STRING,
+},{
+.name = "preferred-codec",
+.type = QEMU_OPT_STRING,
},{
.name = "agent-mouse",
.type = QEMU_OPT_BOOL,
@@ -644,6 +647,7 @@ static void qemu_spice_init(void)
char *x509_key_file = NULL,
*x509_cert_file = NULL,
*x509_cacert_file = NULL;
+const char *preferred_codec = NULL;
int port, tls_port, addr_flags;
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
@@ -795,6 +799,16 @@ static void qemu_spice_init(void)
spice_server_set_streaming_video(spice_server, SPICE_STREAM_VIDEO_OFF);
}
+preferred_codec = qemu_opt_get(opts, "preferred-codec");
+if (preferred_codec) {
+if (spice_server_set_video_codecs(spice_server, preferred_codec)) {
+error_report("Preferred codec name is not valid");
+exit(1);
+}
+} else {
+spice_server_set_video_codecs(spice_server, "spice:mjpeg");
+}
+
spice_server_set_agent_mouse
(spice_server, qemu_opt_get_bool(opts, "agent-mouse", 1));
spice_server_set_playback_compression
--
2.37.2
Re: [PATCH] migration: Fix migration crash when target psize larger than host
On 20/01/2023 17.31, Peter Xu wrote:
Commit d9e474ea56 overlooked the case where the target psize is even larger
than the host psize. One example is Alpha has 8K page size and migration
will start to crash the source QEMU when running Alpha migration on x86.
Fix it by detecting that case and set host start/end just to cover the
single page to be migrated.
This will slightly optimize the common case where host psize equals to
guest psize so we don't even need to do the roundups, but that's trivial.
Cc: qemu-sta...@nongnu.org
Reported-by: Thomas Huth
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1456
Fixes: d9e474ea56 ("migration: Teach PSS about host page")
Signed-off-by: Peter Xu
---
migration/ram.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
Thanks, I've checked that this fixes the issue for me, indeed!
Tested-by: Thomas Huth
[PATCH v6 5/5] riscv: Introduce satp mode hw capabilities
Currently, the max satp mode is set with the only constraint that it must be
implemented in qemu, i.e. set in valid_vm_1_10_[32|64].
But we actually need to add another level of constraint: what the hw is
actually capable of, because currently, a linux booting on a sifive-u54
boots in sv57 mode which is incompatible with the cpu's sv39 max
capability.
So add a new bitmap to RISCVSATPMap which contains this capability and
initialize it in every XXX_cpu_init.
Finally, we have the following chain of constraints:
Qemu capability > HW capability > User choice > Software capability
Signed-off-by: Alexandre Ghiti
---
target/riscv/cpu.c | 78 +++---
target/riscv/cpu.h | 8 +++--
2 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e409e6ab64..19a37fee2b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -292,24 +292,39 @@ const char *satp_mode_str(uint8_t satp_mode, bool
is_32_bit)
g_assert_not_reached();
}
-/* Sets the satp mode to the max supported */
-static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
+static void set_satp_mode_max_supported(RISCVCPU *cpu,
+const char *satp_mode_str,
+bool is_32_bit)
{
-if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
-cpu->cfg.satp_mode.map |=
-(1 << satp_mode_from_str(is_32_bit ? "sv32" : "sv57"));
-} else {
-cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
+uint8_t satp_mode = satp_mode_from_str(satp_mode_str);
+const bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
+
+for (int i = 0; i <= satp_mode; ++i) {
+if (valid_vm[i]) {
+cpu->cfg.satp_mode.supported |= (1 << i);
+}
}
}
+/* Sets the satp mode to the max supported */
+static void set_satp_mode_default(RISCVCPU *cpu)
+{
+uint8_t satp_mode = satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
+
+cpu->cfg.satp_mode.map |= (1 << satp_mode);
+}
+
static void riscv_any_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+
#if defined(TARGET_RISCV32)
set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+set_satp_mode_max_supported(cpu, "sv32", true);
#elif defined(TARGET_RISCV64)
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+set_satp_mode_max_supported(cpu, "sv57", false);
#endif
set_priv_version(env, PRIV_VERSION_1_12_0);
register_cpu_props(obj);
@@ -319,18 +334,24 @@ static void riscv_any_cpu_init(Object *obj)
static void rv64_base_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+
/* We set this in the realise function */
set_misa(env, MXL_RV64, 0);
register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
+set_satp_mode_max_supported(cpu, "sv57", false);
}
static void rv64_sifive_u_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
+set_satp_mode_max_supported(cpu, "sv39", false);
}
static void rv64_sifive_e_cpu_init(Object *obj)
@@ -341,6 +362,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
cpu->cfg.mmu = false;
+set_satp_mode_max_supported(cpu, "mbare", false);
}
static void rv128_base_cpu_init(Object *obj)
@@ -352,11 +374,13 @@ static void rv128_base_cpu_init(Object *obj)
exit(EXIT_FAILURE);
}
CPURISCVState *env = &RISCV_CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
/* We set this in the realise function */
set_misa(env, MXL_RV128, 0);
register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
+set_satp_mode_max_supported(cpu, "sv57", false);
}
#else
static void rv32_base_cpu_init(Object *obj)
@@ -367,13 +391,17 @@ static void rv32_base_cpu_init(Object *obj)
register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
+set_satp_mode_max_supported(cpu, "sv32", true);
}
static void rv32_sifive_u_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
+RISCVCPU *cpu = RISCV_CPU(obj);
+
set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
set_priv_version(env, PRIV_VERSION_1_10_0);
+set_satp_mode_max_supported(cpu, "sv32", true);
}
static void rv32_sifive_e_cpu_init(Object *obj)
@@ -384,6 +412,7 @@ static void rv32_sifive_e_
Re: [RFC PATCH v5 7/9] target/avocado: Pass parameters to migration test on aarch64
On 20/1/23 19:48, Fabiano Rosas wrote:
The migration tests are currently broken for an aarch64 host because
the tests pass no 'machine' and 'cpu' options on the QEMU command
line. Most other architectures define a default value in QEMU for
these options, but arm does not.
There was some discussions around that in the past:
https://lore.kernel.org/qemu-devel/20190621153806.13489-1-waine...@redhat.com/
https://lore.kernel.org/qemu-devel/cafeaca9nbu+l4whfkltv93wy90wjnv05ez12pt6pmljdz5h...@mail.gmail.com/
Add these options to the test class in case the test is being executed
in an aarch64 host.
I'm not sure what we are aiming to test here.
Migration in general? If so, any random machine should work.
By hardcoding the 'virt' machine, at least this test is reproducible.
I'd rather fix that generically as "if a test requires a default
machine and the target doesn't provide any default, then SKIP the
test". Then adding machine-specific tests. Can be done on top, so
Acked-by: Philippe Mathieu-Daudé
Signed-off-by: Fabiano Rosas
Reviewed-by: Richard Henderson
---
tests/avocado/migration.py | 10 ++
1 file changed, 10 insertions(+)
diff --git a/tests/avocado/migration.py b/tests/avocado/migration.py
index 4b25680c50..ffd3db0f35 100644
--- a/tests/avocado/migration.py
+++ b/tests/avocado/migration.py
@@ -11,6 +11,8 @@
import tempfile
+import os
+
from avocado_qemu import QemuSystemTest
from avocado import skipUnless
@@ -26,6 +28,14 @@ class Migration(QemuSystemTest):
timeout = 10
+def setUp(self):
+super().setUp()
+
+arch = os.uname()[4]
+if arch == 'aarch64':
+self.machine = 'virt'
+self.cpu = 'max'
+
@staticmethod
def migration_finished(vm):
return vm.command('query-migrate')['status'] in ('completed',
'failed')
Re: [PATCH v6 00/33] Consolidate PIIX south bridges
On 20/1/23 13:22, Bernhard Beschow wrote: Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow : Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" : On 9/1/23 18:23, Bernhard Beschow wrote: This series consolidates the implementations of the PIIX3 and PIIX4 south bridges and is an extended version of [1]. The motivation is to share as much code as possible and to bring both device models to feature parity such that perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this list before. Bernhard Beschow (30): hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs() hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig hw/usb/hcd-uhci: Introduce TYPE_ defines for device models hw/intc/i8259: Make using the isa_pic singleton more type-safe hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC hw/i386/pc: Create RTC controllers in south bridges hw/i386/pc: No need for rtc_state to be an out-parameter hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge hw/isa/piix3: Create USB controller in host device hw/isa/piix3: Create power management controller in host device hw/isa/piix3: Create TYPE_ISA_PIC in host device hw/isa/piix3: Create IDE controller in host device hw/isa/piix3: Wire up ACPI interrupt internally hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 hw/isa/piix3: Drop the "3" from PIIX base class hw/isa/piix4: Make PIIX4's ACPI and USB functions optional hw/isa/piix4: Remove unused inbound ISA interrupt lines hw/isa/piix4: Use TYPE_ISA_PIC device hw/isa/piix4: Reuse struct PIIXState from PIIX3 hw/isa/piix4: Rename reset control operations to match PIIX3 hw/isa/piix3: Merge hw/isa/piix4.c hw/isa/piix: Harmonize names of reset control memory regions hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 hw/isa/piix: Rename functions to be shared for interrupt triggering hw/isa/piix: Consolidate IRQ triggering hw/isa/piix: Share PIIX3's base class with PIIX4 Philippe Mathieu-Daudé (3): hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader hw/isa/piix4: Correct IRQRC[A:D] reset values I'm queuing the first 10 patches for now to alleviate the size of this series, and I'll respin a v7 with the rest to avoid making you suffer any longer :/ Thanks for insisting in this effort and I apologize it is taking me so long... Okay... What's the further plan? Is there anything missing? Ping The plan is "I'll respin a v7 with the rest".
[PATCH v6 1/5] riscv: Pass Object to register_cpu_props instead of DeviceState
One can extract the DeviceState pointer from the Object pointer, so pass
the Object for future commits to access other fields of Object.
No functional changes intended.
Reviewed-by: Alistair Francis
Reviewed-by: Frank Chang
Reviewed-by: Andrew Jones
Signed-off-by: Alexandre Ghiti
---
target/riscv/cpu.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..7181b34f86 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -200,7 +200,7 @@ static const char * const riscv_intr_names[] = {
"reserved"
};
-static void register_cpu_props(DeviceState *dev);
+static void register_cpu_props(Object *obj);
const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
{
@@ -238,7 +238,7 @@ static void riscv_any_cpu_init(Object *obj)
set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
#endif
set_priv_version(env, PRIV_VERSION_1_12_0);
-register_cpu_props(DEVICE(obj));
+register_cpu_props(obj);
}
#if defined(TARGET_RISCV64)
@@ -247,7 +247,7 @@ static void rv64_base_cpu_init(Object *obj)
CPURISCVState *env = &RISCV_CPU(obj)->env;
/* We set this in the realise function */
set_misa(env, MXL_RV64, 0);
-register_cpu_props(DEVICE(obj));
+register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
}
@@ -280,7 +280,7 @@ static void rv128_base_cpu_init(Object *obj)
CPURISCVState *env = &RISCV_CPU(obj)->env;
/* We set this in the realise function */
set_misa(env, MXL_RV128, 0);
-register_cpu_props(DEVICE(obj));
+register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
}
@@ -290,7 +290,7 @@ static void rv32_base_cpu_init(Object *obj)
CPURISCVState *env = &RISCV_CPU(obj)->env;
/* We set this in the realise function */
set_misa(env, MXL_RV32, 0);
-register_cpu_props(DEVICE(obj));
+register_cpu_props(obj);
/* Set latest version of privileged specification */
set_priv_version(env, PRIV_VERSION_1_12_0);
}
@@ -343,7 +343,7 @@ static void riscv_host_cpu_init(Object *obj)
#elif defined(TARGET_RISCV64)
set_misa(env, MXL_RV64, 0);
#endif
-register_cpu_props(DEVICE(obj));
+register_cpu_props(obj);
}
#endif
@@ -1083,9 +1083,10 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static void register_cpu_props(DeviceState *dev)
+static void register_cpu_props(Object *obj)
{
Property *prop;
+DeviceState *dev = DEVICE(obj);
for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
qdev_property_add_static(dev, prop);
--
2.37.2
Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
On 23/1/23 09:39, Thomas Huth wrote:
From: Daniel P. Berrangé
Way back in QEMU 4.0, the -audiodev command line option was introduced
for configuring audio backends. This CLI option does not use QemuOpts
so it is not visible for introspection in 'query-command-line-options',
instead using the QAPI Audiodev type. Unfortunately there is also no
QMP command that uses the Audiodev type, so it is not introspectable
with 'query-qmp-schema' either.
This introduces a 'query-audiodev' command that simply reflects back
the list of configured -audiodev command line options. This alone is
maybe not very useful by itself, but it makes Audiodev introspectable
via 'query-qmp-schema', so that libvirt (and other upper layer tools)
can discover the available audiodevs.
Signed-off-by: Daniel P. Berrangé
[thuth: Update for upcoming QEMU v8.0, and use QAPI_LIST_PREPEND]
Signed-off-by: Thomas Huth
---
qapi/audio.json | 13 +
audio/audio.c | 12
2 files changed, 25 insertions(+)
diff --git a/qapi/audio.json b/qapi/audio.json
index 1e0a24bdfc..c7aafa2763 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -443,3 +443,16 @@
'sndio': 'AudiodevSndioOptions',
'spice': 'AudiodevGenericOptions',
'wav': 'AudiodevWavOptions' } }
+
+##
+# @query-audiodevs:
+#
+# Returns information about audiodev configuration
Maybe clearer as 'audio backends'?
So similarly, wouldn't be clearer to name this command
'query-audio-backends'? Otherwise we need to go read QEMU
source to understand what is 'audiodevs'.
+#
+# Returns: array of @Audiodev
+#
+# Since: 8.0
+#
+##
+{ 'command': 'query-audiodevs',
+ 'returns': ['Audiodev'] }
diff --git a/audio/audio.c b/audio/audio.c
index d849a94a81..6f270c07b7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -28,8 +28,10 @@
#include "monitor/monitor.h"
#include "qemu/timer.h"
#include "qapi/error.h"
+#include "qapi/clone-visitor.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qapi-visit-audio.h"
+#include "qapi/qapi-commands-audio.h"
#include "qemu/cutils.h"
#include "qemu/module.h"
#include "qemu/help_option.h"
@@ -2311,3 +2313,13 @@ size_t audio_rate_get_bytes(RateCtl *rate, struct
audio_pcm_info *info,
return bytes;
}
+
+AudiodevList *qmp_query_audiodevs(Error **errp)
+{
+AudiodevList *ret = NULL;
+AudiodevListEntry *e;
+QSIMPLEQ_FOREACH(e, &audiodevs, next) {
I am a bit confused here, isn't &audiodevs containing what the user
provided from CLI? How is that useful to libvirt? Maybe the corner case
of a user hand-modifying the QEMU launch arguments from a XML config?
Wouldn't a list of linked in AudiodevDriver be more useful to libvirt
so it could pick the best available backend to start a VM?
+QAPI_LIST_PREPEND(ret, QAPI_CLONE(Audiodev, e->dev));
+}
+return ret;
+}
Re: [PATCH v6 2/5] riscv: Change type of valid_vm_1_10_[32|64] to bool
On Mon, Jan 23, 2023 at 10:03:21AM +0100, Alexandre Ghiti wrote:
> This array is actually used as a boolean so swap its current char type
> to a boolean and at the same time, change the type of validate_vm to
> bool since it returns valid_vm_1_10_[32|64].
>
> Signed-off-by: Alexandre Ghiti
Suggested-by: Andrew Jones
Reviewed-by: Andrew Jones
> ---
> target/riscv/csr.c | 21 +++--
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0db2c233e5..6b157806a5 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1117,16 +1117,16 @@ static const target_ulong hip_writable_mask =
> MIP_VSSIP;
> static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP |
> MIP_VSEIP;
> static const target_ulong vsip_writable_mask = MIP_VSSIP;
>
> -static const char valid_vm_1_10_32[16] = {
> -[VM_1_10_MBARE] = 1,
> -[VM_1_10_SV32] = 1
> +static const bool valid_vm_1_10_32[16] = {
> +[VM_1_10_MBARE] = true,
> +[VM_1_10_SV32] = true
> };
>
> -static const char valid_vm_1_10_64[16] = {
> -[VM_1_10_MBARE] = 1,
> -[VM_1_10_SV39] = 1,
> -[VM_1_10_SV48] = 1,
> -[VM_1_10_SV57] = 1
> +static const bool valid_vm_1_10_64[16] = {
> +[VM_1_10_MBARE] = true,
> +[VM_1_10_SV39] = true,
> +[VM_1_10_SV48] = true,
> +[VM_1_10_SV57] = true
> };
>
> /* Machine Information Registers */
> @@ -1209,7 +1209,7 @@ static RISCVException read_mstatus(CPURISCVState *env,
> int csrno,
> return RISCV_EXCP_NONE;
> }
>
> -static int validate_vm(CPURISCVState *env, target_ulong vm)
> +static bool validate_vm(CPURISCVState *env, target_ulong vm)
> {
> if (riscv_cpu_mxl(env) == MXL_RV32) {
> return valid_vm_1_10_32[vm & 0xf];
> @@ -2648,7 +2648,8 @@ static RISCVException read_satp(CPURISCVState *env, int
> csrno,
> static RISCVException write_satp(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> -target_ulong vm, mask;
> +target_ulong mask;
> +bool vm;
>
> if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
> return RISCV_EXCP_NONE;
> --
> 2.37.2
>
Re: cxl nvdimm Potential probe ordering issues.
On Fri, 20 Jan 2023 14:41:05 -0800 Dan Williams wrote: > Gregory Price wrote: > > On Fri, Jan 20, 2023 at 09:38:13AM -0800, Dan Williams wrote: > > > As it stands currently that dax device and the cxl device are not > > > related since a default dax-device is loaded just based on the presence > > > of an EFI_MEMORY_SP address range in the address map. With the new ram > > > enabling that default device will be elided and CXL will register a > > > dax-device parented by a cxl region. > > > > > > > - The memory *does not* auto-online, instead the dax device > > > > can be > > > >onlined as system-ram *manually* via ndctl and friends > > > > > > That *manually* part is the problem that needs distro help to solve. It > > > should be the case that by default all Linux distributions auto-online > > > all dax-devices. If that happens to online memory that is too slow for > > > general use, or too high-performance / precious for general purpose use > > > then the administrator can set policy after the fact. Unfortunately user > > > policy can not be applied if these memory ranges were onlined by the > > > kernel at boot , so that's why the kernel policy defaults to not-online. > > > > > > In other words, there is no guarantee that memory that was assigned to > > > the general purpose pool at boot can be removed. The only guaranteed > > > behavior is to never give the memory to the core kernel in the first > > > instance and always let user policy route the memory. > > > > > > > 3) The code creates an nvdimm_bridge IFF a CFMW is defined - regardless > > > >of the type-3 device configuration (pmem-only or vmem-only) > > > > > > Correct, the top-level bus code (cxl_acpi) and the endpoint code > > > (cxl_mem, cxl_port) need to handshake before establishing regions. For > > > pmem regions the platform needs to claim the availability of a pmem > > > capable CXL window. > > > > > > > 4) As you can see above, multiple decoders are registered. I'm not sure > > > >if that's correct or not, but it does seem odd given there's only one > > > > cxl type-3 device. Odd that decoder0.0 shows up when CFMW is > > > > there, > > > > but not when it isn't. > > > > > > CXL windows are modeled as decoders hanging off the the CXL root device > > > (ACPI0017 on ACPI based platforms). An endpoint decoder can then map a > > > selection of that window. > > > > > > > Don't know why I haven't thought of this until now, but is the CFMW code > > > > reporting something odd about what's behind it? Is it assuming the > > > > devices are pmem? > > > > > > No, the cxl_acpi code is just advertising platform decode possibilities > > > independent of what devices show up. Think of this like the PCI MMIO > > > space that gets allocated to a root bridge at the beginning of time. > > > That space may or may not get consumed based on what devices show up > > > downstream. > > > > Thank you for the explanation Dan, and thank you for you patience > > @JCameron. I'm fairly sure I grok it now. > > > > Summarizing to make sure: the cxl driver is providing what would be the > > CXL.io (control) path, and the CXL.mem path is basically being simulated > > by what otherwise would be a traditional PCI memory region. This explains > > why turning off Legacy mode drops the dax devices, and why the topology > > looks strange - the devices are basically attached in 2 different ways. > > > > Might there be interest from the QEMU community to implement this > > legacy-style setup in the short term, in an effort to test the the > > control path of type-3 devices while we wait for the kernel to catch up? I'd happily review such code, but it's not on my list of things to work on otherwise. Too many other things to support! Jonathan > > > > Or should we forget this mode ever existed and just barrel forward > > with HDM decoders and writing the kernel code to hook up the underlying > > devices in drivers/cxl? > > Which mode are you referring? > > The next steps for the kernel enabling relevant to this thread are: > > * ram region discovery (platform firmware or kexec established) > * ram region creation > * pmem region discovery (from labels)
Re: [PULL 09/12] include/hw/ppc: Split pnv_chip.h off pnv.h
Hi Markus,
On 20/1/23 08:01, Markus Armbruster wrote:
PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip are defined
in pnv.h. Many users of the header don't actually need them. One
instance is this inclusion loop: hw/ppc/pnv_homer.h includes
hw/ppc/pnv.h for typedef PnvChip, and vice versa for struct PnvHomer.
Similar structs live in their own headers: PnvHomerClass and PnvHomer
in pnv_homer.h, PnvLpcClass and PnvLpcController in pci_lpc.h,
PnvPsiClass, PnvPsi, Pnv8Psi, Pnv9Psi, Pnv10Psi in pnv_psi.h, ...
Move PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip to new
pnv_chip.h, and adjust include directives. This breaks the inclusion
loop mentioned above.
Signed-off-by: Markus Armbruster
Reviewed-by: Cédric Le Goater
Reviewed-by: Daniel Henrique Barboza
Message-Id: <20221222104628.659681-2-arm...@redhat.com>
---
include/hw/ppc/pnv.h | 143 +---
include/hw/ppc/pnv_chip.h | 147 +
hw/intc/pnv_xive.c | 1 +
hw/intc/pnv_xive2.c| 1 +
hw/pci-host/pnv_phb3.c | 1 +
hw/pci-host/pnv_phb4_pec.c | 1 +
hw/ppc/pnv.c | 3 +
hw/ppc/pnv_core.c | 1 +
hw/ppc/pnv_homer.c | 1 +
hw/ppc/pnv_lpc.c | 1 +
hw/ppc/pnv_xscom.c | 1 +
11 files changed, 160 insertions(+), 141 deletions(-)
create mode 100644 include/hw/ppc/pnv_chip.h
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 9ef7e2d0dc..ca49e4281d 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -20,158 +20,19 @@
#ifndef PPC_PNV_H
#define PPC_PNV_H
+#include "cpu.h"
Why is "cpu.h" required here? For pnv_chip_find_cpu()?
Isn't "target/ppc/cpu-qom.h" enough?
#include "hw/boards.h"
#include "hw/sysbus.h"
#include "hw/ipmi/ipmi.h"
-#include "hw/ppc/pnv_lpc.h"
#include "hw/ppc/pnv_pnor.h"
-#include "hw/ppc/pnv_psi.h"
-#include "hw/ppc/pnv_occ.h"
-#include "hw/ppc/pnv_sbe.h"
-#include "hw/ppc/pnv_homer.h"
-#include "hw/ppc/pnv_xive.h"
-#include "hw/ppc/pnv_core.h"
-#include "hw/pci-host/pnv_phb3.h"
-#include "hw/pci-host/pnv_phb4.h"
#include "hw/pci-host/pnv_phb.h"
-#include "qom/object.h"
#define TYPE_PNV_CHIP "pnv-chip"
-OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,
-PNV_CHIP)
-struct PnvChip {
-/*< private >*/
-SysBusDevice parent_obj;
-
-/*< public >*/
-uint32_t chip_id;
-uint64_t ram_start;
-uint64_t ram_size;
-
-uint32_t nr_cores;
-uint32_t nr_threads;
-uint64_t cores_mask;
-PnvCore **cores;
-
-uint32_t num_pecs;
-
-MemoryRegion xscom_mmio;
-MemoryRegion xscom;
-AddressSpace xscom_as;
-
-MemoryRegion *fw_mr;
-gchar*dt_isa_nodename;
-};
-
-#define TYPE_PNV8_CHIP "pnv8-chip"
+typedef struct PnvChip PnvChip;
typedef struct Pnv8Chip Pnv8Chip;
-DECLARE_INSTANCE_CHECKER(Pnv8Chip, PNV8_CHIP,
- TYPE_PNV8_CHIP)
-
-struct Pnv8Chip {
-/*< private >*/
-PnvChip parent_obj;
-
-/*< public >*/
-MemoryRegion icp_mmio;
-
-PnvLpcController lpc;
-Pnv8Psi psi;
-PnvOCC occ;
-PnvHomer homer;
-
-#define PNV8_CHIP_PHB3_MAX 4
-/*
- * The array is used to allow quick access to the phbs by
- * pnv_ics_get_child() and pnv_ics_resend_child().
- */
-PnvPHB *phbs[PNV8_CHIP_PHB3_MAX];
-uint32_t num_phbs;
-
-XICSFabric*xics;
-};
-
-#define TYPE_PNV9_CHIP "pnv9-chip"
typedef struct Pnv9Chip Pnv9Chip;
-DECLARE_INSTANCE_CHECKER(Pnv9Chip, PNV9_CHIP,
- TYPE_PNV9_CHIP)
-
-struct Pnv9Chip {
-/*< private >*/
-PnvChip parent_obj;
-
-/*< public >*/
-PnvXive xive;
-Pnv9Psi psi;
-PnvLpcController lpc;
-PnvOCC occ;
-PnvSBE sbe;
-PnvHomer homer;
-
-uint32_t nr_quads;
-PnvQuad *quads;
-
-#define PNV9_CHIP_MAX_PEC 3
-PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
-};
-
-/*
- * A SMT8 fused core is a pair of SMT4 cores.
- */
-#define PNV9_PIR2FUSEDCORE(pir) (((pir) >> 3) & 0xf)
-#define PNV9_PIR2CHIP(pir) (((pir) >> 8) & 0x7f)
-
-#define TYPE_PNV10_CHIP "pnv10-chip"
typedef struct Pnv10Chip Pnv10Chip;
-DECLARE_INSTANCE_CHECKER(Pnv10Chip, PNV10_CHIP,
- TYPE_PNV10_CHIP)
-
-struct Pnv10Chip {
-/*< private >*/
-PnvChip parent_obj;
-
-/*< public >*/
-PnvXive2 xive;
-Pnv9Psi psi;
-PnvLpcController lpc;
-PnvOCC occ;
-PnvSBE sbe;
-PnvHomer homer;
-
-uint32_t nr_quads;
-PnvQuad *quads;
-
-#define PNV10_CHIP_MAX_PEC 2
-PnvPhb4PecState pecs[PNV10_CHIP_MAX_PEC];
-};
-
-#define PNV10_PIR2FUSEDCORE(pir) (((pir) >> 3) & 0xf)
-#define PNV10_PIR2CHIP(pir) (((pir) >> 8) & 0x7f)
-
-struct PnvChipClass {
-/*< private >*/
-SysBusDeviceClass parent_class;
-
-/*< public >*/
-uint64_t chip_cfam_id;
-u
Re: [PULL 09/12] include/hw/ppc: Split pnv_chip.h off pnv.h
Philippe Mathieu-Daudé writes: > Hi Markus, > > On 20/1/23 08:01, Markus Armbruster wrote: >> PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip are defined >> in pnv.h. Many users of the header don't actually need them. One >> instance is this inclusion loop: hw/ppc/pnv_homer.h includes >> hw/ppc/pnv.h for typedef PnvChip, and vice versa for struct PnvHomer. >> Similar structs live in their own headers: PnvHomerClass and PnvHomer >> in pnv_homer.h, PnvLpcClass and PnvLpcController in pci_lpc.h, >> PnvPsiClass, PnvPsi, Pnv8Psi, Pnv9Psi, Pnv10Psi in pnv_psi.h, ... >> Move PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip to new >> pnv_chip.h, and adjust include directives. This breaks the inclusion >> loop mentioned above. >> Signed-off-by: Markus Armbruster >> Reviewed-by: Cédric Le Goater >> Reviewed-by: Daniel Henrique Barboza >> Message-Id: <20221222104628.659681-2-arm...@redhat.com> >> --- >> include/hw/ppc/pnv.h | 143 +--- >> include/hw/ppc/pnv_chip.h | 147 + >> hw/intc/pnv_xive.c | 1 + >> hw/intc/pnv_xive2.c| 1 + >> hw/pci-host/pnv_phb3.c | 1 + >> hw/pci-host/pnv_phb4_pec.c | 1 + >> hw/ppc/pnv.c | 3 + >> hw/ppc/pnv_core.c | 1 + >> hw/ppc/pnv_homer.c | 1 + >> hw/ppc/pnv_lpc.c | 1 + >> hw/ppc/pnv_xscom.c | 1 + >> 11 files changed, 160 insertions(+), 141 deletions(-) >> create mode 100644 include/hw/ppc/pnv_chip.h >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >> index 9ef7e2d0dc..ca49e4281d 100644 >> --- a/include/hw/ppc/pnv.h >> +++ b/include/hw/ppc/pnv.h >> @@ -20,158 +20,19 @@ >> #ifndef PPC_PNV_H >> #define PPC_PNV_H >> +#include "cpu.h" > > Why is "cpu.h" required here? For pnv_chip_find_cpu()? Yes: ../include/hw/ppc/pnv.h:58:1: error: unknown type name ‘PowerPCCPU’ 58 | PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir); | ^~ > Isn't "target/ppc/cpu-qom.h" enough? Seems to suffice. Would you like to post a followup?
Re: [PATCH v6 3/5] riscv: Allow user to set the satp mode
On Mon, Jan 23, 2023 at 10:03:22AM +0100, Alexandre Ghiti wrote:
> RISC-V specifies multiple sizes for addressable memory and Linux probes for
> the machine's support at startup via the satp CSR register (done in
> csr.c:validate_vm).
>
> As per the specification, sv64 must support sv57, which in turn must
> support sv48...etc. So we can restrict machine support by simply setting the
> "highest" supported mode and the bare mode is always supported.
>
> You can set the satp mode using the new properties "sv32", "sv39", "sv48",
> "sv57" and "sv64" as follows:
> -cpu rv64,sv57=on # Linux will boot using sv57 scheme
> -cpu rv64,sv39=on # Linux will boot using sv39 scheme
> -cpu rv64,sv57=off # Linux will boot using sv48 scheme
> -cpu rv64 # Linux will boot using sv57 scheme by default
>
> We take the highest level set by the user:
> -cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme
>
> We make sure that invalid configurations are rejected:
> -cpu rv64,sv32=on # Can't enable 32-bit satp mode in 64-bit
The property doesn't exist for rv64 anymore, so I'm not sure we need
this info in the commit message.
> -cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are
># enabled
>
> We accept "redundant" configurations:
> -cpu rv64,sv48=on,sv57=off # Linux will boot using sv48 scheme
>
> And contradictory configurations:
> -cpu rv64,sv48=on,sv48=off # Linux will boot using sv39 scheme
>
> In addition, we now correctly set the device-tree entry 'mmu-type' using
> those new properties.
>
> Co-Developed-by: Ludovic Henry
> Signed-off-by: Ludovic Henry
> Signed-off-by: Alexandre Ghiti
> ---
> target/riscv/cpu.c | 204 +
> target/riscv/cpu.h | 19 +
> target/riscv/csr.c | 12 ++-
> 3 files changed, 228 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7181b34f86..e409e6ab64 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -27,6 +27,7 @@
> #include "time_helper.h"
> #include "exec/exec-all.h"
> #include "qapi/error.h"
> +#include "qapi/visitor.h"
> #include "qemu/error-report.h"
> #include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> @@ -229,6 +230,79 @@ static void set_vext_version(CPURISCVState *env, int
> vext_ver)
> env->vext_ver = vext_ver;
> }
>
> +static uint8_t satp_mode_from_str(const char *satp_mode_str)
> +{
> +if (!strncmp(satp_mode_str, "mbare", 5)) {
> +return VM_1_10_MBARE;
> +}
> +
> +if (!strncmp(satp_mode_str, "sv32", 4)) {
> +return VM_1_10_SV32;
> +}
> +
> +if (!strncmp(satp_mode_str, "sv39", 4)) {
> +return VM_1_10_SV39;
> +}
> +
> +if (!strncmp(satp_mode_str, "sv48", 4)) {
> +return VM_1_10_SV48;
> +}
> +
> +if (!strncmp(satp_mode_str, "sv57", 4)) {
> +return VM_1_10_SV57;
> +}
> +
> +if (!strncmp(satp_mode_str, "sv64", 4)) {
> +return VM_1_10_SV64;
> +}
> +
> +g_assert_not_reached();
> +}
> +
> +uint8_t satp_mode_max_from_map(uint32_t map)
> +{
> +/* map here has at least one bit set, so no problem with clz */
> +return 31 - __builtin_clz(map);
> +}
> +
> +const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit)
> +{
> +if (is_32_bit) {
> +switch (satp_mode) {
> +case VM_1_10_SV32:
> +return "sv32";
> +case VM_1_10_MBARE:
> +return "none";
> +}
> +} else {
> +switch (satp_mode) {
> +case VM_1_10_SV64:
> +return "sv64";
> +case VM_1_10_SV57:
> +return "sv57";
> +case VM_1_10_SV48:
> +return "sv48";
> +case VM_1_10_SV39:
> +return "sv39";
> +case VM_1_10_MBARE:
> +return "none";
> +}
> +}
> +
> +g_assert_not_reached();
> +}
> +
> +/* Sets the satp mode to the max supported */
> +static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
> +{
> +if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> +cpu->cfg.satp_mode.map |=
> +(1 << satp_mode_from_str(is_32_bit ? "sv32" :
> "sv57"));
> +} else {
> +cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> +}
> +}
> +
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -619,6 +693,82 @@ static void riscv_cpu_disas_set_info(CPUState *s,
> disassemble_info *info)
> }
> }
>
> +static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
> +{
> +bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> +const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
> +
> +if (cpu->cfg.satp_mode.map == 0) {
> +/*
> + * If unset by both the user and the cpu, we fallback to the default
> + * satp mode.
> + */
nit: I'd put the above comment under the if init == 0 below.
> +
Re: [PATCH v6 4/5] riscv: Correctly set the device-tree entry 'mmu-type'
On Mon, Jan 23, 2023 at 10:03:23AM +0100, Alexandre Ghiti wrote: > The 'mmu-type' should reflect what the hardware is capable of so use the > new satp_mode field in RISCVCPUConfig to do that. > > Signed-off-by: Alexandre Ghiti > --- > hw/riscv/virt.c | 19 ++- > 1 file changed, 10 insertions(+), 9 deletions(-) Reviewed-by: Andrew Jones
Re: [RFC v2 2/2] spice: Add an option to forward the dmabuf directly to the encoder (v2)
Hi,
> Here is the flow of things from the Qemu side:
> - Call gl_scanout (to update the fd) and gl_draw_async just like
> in the local display case.
Ok.
> - Additionally, create an update with the cmd set to QXL_CMD_DRAW
> to trigger the creation of a new drawable (associated with the fd)
> by the Spice server.
> - Wait (or block) until the Encoder is done encoding the content.
> - Unblock the pipeline once the async completion cookie is received.
Care to explain? For qemu it should make a difference what spice-server
does with the dma-bufs passed (local display / encode video + send to
remote).
> #ifdef HAVE_SPICE_GL
> +} else if (spice_dmabuf_encode) {
> +if (g_strcmp0(preferred_codec, "gstreamer:h264")) {
> +error_report("dmabuf-encode=on currently only works and
> tested"
> + "with gstreamer:h264");
> +exit(1);
> +}
IMHO we should not hard-code todays spice-server capabilities like this.
For starters this isn't true for spice-server versions which don't (yet)
have your patches. Also the capability might depend on hardware
support. IMHO we need some feature negotiation between qemu and spice
here.
take care,
Gerd
Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function
On 1/22/23 19:53, Alistair Francis wrote: On Sun, Jan 22, 2023 at 5:16 AM Daniel Henrique Barboza wrote: Conor, Thanks for the Icicle-kit walk-through! I'll not claim that I fully understood it, but I understood enough to handle the situation ATM. Without this change, this is where the FDT is being installed in the board when I start it with 8Gb of RAM (retrieved via 'info roms'): addr=bfe0 size=0x00a720 mem=ram name="fdt" Which surprised me at first because this is almost at the end of the LO area which has 1Gb and I figured it would be in the middle of another RAM area. I took another read at what we're doing in riscv_load_fdt(): --- temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB); --- This code can be read as "if the starting address of the RAM is lower than 3Gb, put the FDT no further than 3Gb (0xc000). Otherwise, put it at the end of dram", where "dram_base" is the starting address of the RAM block that the function receives. For icicle-kit, this is being passed as memmap[MICROCHIP_PFSOC_DRAM_LO].base, 0x8000, which is 2Gb. So, regardless of how much RAM we have (dram_end), the FDT will always be capped at 3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit. Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix 32 bit guest boot and it happened to also work for the Microchip SoC. So yeah, I thought that I was fixing a bug and in the end I caused one. This patch needs to go. Alistair, I believe I should re-send v2, this time explaining why the existing function will not break the Microchip board because we'll never put the FDT out of the LO area of the board. Does this work for you? I think that's fine. My only worry is that we are losing some flexibility that some future board might want. What if we change riscv_load_fdt() parameters to pass a MemoryRegion/MemMapEntry instead of just dram_base? Instead of this: uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) We would have this: uint64_t riscv_load_fdt(MemMapEntry mem, uint64_t mem_size, void *fdt) Or even this: uint64_t riscv_load_fdt(hwaddr dram_base, hwaddr dram_size, uint64_t mem_size, void *fdt) And then we can make assumptions based on the actual memory region that the fdt is going to fit into, instead of having a starting address and a total memory size and have to deal with issues such as sparse memory. We can keep all the assumptions already made today (e.g. the 3Gb maximum addr) while also having a guarantee that the fdt isn't going to be put in the wrong memory region/spot if we decide to change the assumptions later on. Thanks, Daniel Alistair
Re: [PATCH] hw/riscv: boot: Don't use CSRs if they are disabled
On 1/23/23 00:57, Alistair Francis wrote:
From: Alistair Francis
If the CSRs and CSR instructions are disabled because the Zicsr
extension isn't enabled then we want to make sure we don't run any CSR
instructions in the boot ROM.
This patches removes the CSR instructions from the reset-vec if the
extension isn't enabled. We replace the instruction with a NOP instead.
Note that we don't do this for the SiFive U machine, as we are modelling
the hardware in that case.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1447
Signed-off-by: Alistair Francis
---
Shouldn't we also handle the sifive_u/sifive_e boards? Their reset vectors
aren't being covered by riscv_set_rom_reset_vec() (it's on my TODO, didn't
send it yet because sifive uses an extra MSEL pin at the start of the vector).
Daniel
hw/riscv/boot.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 2594276223..cb27798a25 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -356,6 +356,15 @@ void riscv_setup_rom_reset_vec(MachineState *machine,
RISCVHartArrayState *harts
reset_vec[4] = 0x0182b283; /* ld t0, 24(t0) */
}
+if (!harts->harts[0].cfg.ext_icsr) {
+/*
+ * The Zicsr extension has been disabled, so let's ensure we don't
+ * run the CSR instruction. Let's fill the address with a non
+ * compressed nop.
+ */
+reset_vec[2] = 0x0013; /* addi x0, x0, 0 */
+}
+
/* copy in the reset vector in little_endian byte order */
for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
reset_vec[i] = cpu_to_le32(reset_vec[i]);
Re: [PATCH] configure: Expand test which disable -Wmissing-braces
On 06/01/2023 15.21, Anthony PERARD wrote:
From: Anthony PERARD
With "clang 6.0.0-1ubuntu2" on Ubuntu Bionic, the test with build
fine, but clang still suggest braces around the zero initializer in a
few places, where there is a subobject. Expand test to include a sub
struct which doesn't build on clang 6.0.0-1ubuntu2, and give:
config-temp/qemu-conf.c:7:8: error: suggest braces around initialization
of subobject [-Werror,-Wmissing-braces]
} x = {0};
^
{}
These are the error reported by clang on QEMU's code (v7.2.0):
hw/pci-bridge/cxl_downstream.c:101:51: error: suggest braces around
initialization of subobject [-Werror,-Wmissing-braces]
dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
hw/pci-bridge/cxl_root_port.c:62:51: error: suggest braces around
initialization of subobject [-Werror,-Wmissing-braces]
dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
tests/qtest/virtio-net-test.c:322:34: error: suggest braces around
initialization of subobject [-Werror,-Wmissing-braces]
QOSGraphTestOptions opts = { 0 };
Reported-by: Andrew Cooper
Signed-off-by: Anthony PERARD
---
While Ubuntu Bionic isn't supposed to be supported anymore, clang v6
is still the minimum required as tested by ./configure.
---
configure | 4
1 file changed, 4 insertions(+)
diff --git a/configure b/configure
index 9f0bc57546..3cd9b8bad4 100755
--- a/configure
+++ b/configure
@@ -1290,7 +1290,11 @@ fi
# Disable -Wmissing-braces on older compilers that warn even for
# the "universal" C zero initializer {0}.
cat > $TMPC << EOF
+struct s {
+ void *a;
+};
struct {
+ struct s s;
int a[2];
} x = {0};
EOF
Not sure whether this is really a good fix...
Nobody is really still testing such old compiler versions, so it would be
better to adjust our minimum compiler version, I think.
According to repology.org, our supported distros ship these versions of
Clang these days:
Fedora 36: 14.0.5
CentOS 8 (RHEL-8): 12.0.1
Debian 11: 13.0.1
OpenSUSE Leap 15.4: 13.0.1
Ubuntu LTS 20.04: 12.0.0
FreeBSD Ports: 15.0.7
NetBSD pkgsrc: 15.0.7
Homebrew: 15.0.7
MSYS2 mingw: 15.0.7
Haiku ports: 12.0.1
The Xcode that we are testing in the gitlab-CI seems to be "Apple clang
version 13" which should correspond to LLVM 12.0.0 according to
https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)_2
.
So from that numbers, it seems we could increase the minimal Clang version
to 12.0.0 nowadays.
However, looking at our CI jobs, the Debian runner still seems to use Clang
11 instead:
https://gitlab.com/qemu-project/qemu/-/jobs/3637272746
And our tests/docker/dockerfiles/ubuntu2004.docker file still seems to use
Clang version 10.0.0 ...
So without much additional effort, I think it should be fine to bump our
minimal required version of Clang to 10.0.0 nowadays, thus I'd suggest that
you send a patch for the configure script to do that instead. (Xcode version
should then be bumped from v10 to v12 instead)
Then you can simply remove that old test for missing-braces from the
configure script completely.
Thomas
Re: [PATCH v6 3/5] riscv: Allow user to set the satp mode
On Mon, Jan 23, 2023 at 10:03:22AM +0100, Alexandre Ghiti wrote: > RISC-V specifies multiple sizes for addressable memory and Linux probes for > the machine's support at startup via the satp CSR register (done in > csr.c:validate_vm). > > As per the specification, sv64 must support sv57, which in turn must > support sv48...etc. So we can restrict machine support by simply setting the > "highest" supported mode and the bare mode is always supported. > > You can set the satp mode using the new properties "sv32", "sv39", "sv48", > "sv57" and "sv64" as follows: > -cpu rv64,sv57=on # Linux will boot using sv57 scheme > -cpu rv64,sv39=on # Linux will boot using sv39 scheme > -cpu rv64,sv57=off # Linux will boot using sv48 scheme > -cpu rv64 # Linux will boot using sv57 scheme by default > > We take the highest level set by the user: > -cpu rv64,sv48=on,sv57=on # Linux will boot using sv57 scheme > > We make sure that invalid configurations are rejected: > -cpu rv64,sv32=on # Can't enable 32-bit satp mode in 64-bit > -cpu rv64,sv39=off,sv48=on # sv39 must be supported if higher modes are ># enabled > > We accept "redundant" configurations: > -cpu rv64,sv48=on,sv57=off # Linux will boot using sv48 scheme > > And contradictory configurations: > -cpu rv64,sv48=on,sv48=off # Linux will boot using sv39 scheme > > In addition, we now correctly set the device-tree entry 'mmu-type' using > those new properties. This sentence no longer applies to this patch. Thanks, drew
Re: [PATCH v6 3/5] riscv: Allow user to set the satp mode
On Mon, Jan 23, 2023 at 10:03:22AM +0100, Alexandre Ghiti wrote:
...
> +/* Sets the satp mode to the max supported */
> +static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
> +{
nit: When passing in the cpu object pointer there's no need to also pass
is_32_bit, we can just use it from the pointer, cpu->env.misa_mxl == MXL_RV32
Thanks,
drew
Re: [PATCH] linux-user: Add support for LoongArch64's old world ABI
On Mon, 23 Jan 2023 at 10:27, WANG Xuerui wrote: > > From: WANG Xuerui > > This patch adds a "loongarch64ow-linux-user" target and a > corresponding "qemu-loongarch64ow" binary, for supporting user-mode > emulation of old-world LoongArch applications in the wild. > > Although the old-world LoongArch is already being (slowly) phased out, > there are already a number of deployments (mainly as a result of > LoongArch's early commercial growth), whose migration path is something > software developers have to care about. Support for user-mode emulation > in addition to system-level emulation would help development of such > migration & compatibility solutions. Is this 'old-world' ABI supported by the upstream Linux kernel? I can't see signs of it from a quick grep. If it isn't, then I'm not sure if we should support it in QEMU user-mode emulation. We've always set "upstream Linux" as our definition of what the official ABI and featureset is for usermode emulation. thanks -- PMM
Re: [PATCH v6 5/5] riscv: Introduce satp mode hw capabilities
On Mon, Jan 23, 2023 at 10:03:24AM +0100, Alexandre Ghiti wrote:
> Currently, the max satp mode is set with the only constraint that it must be
> implemented in qemu, i.e. set in valid_vm_1_10_[32|64].
>
> But we actually need to add another level of constraint: what the hw is
> actually capable of, because currently, a linux booting on a sifive-u54
> boots in sv57 mode which is incompatible with the cpu's sv39 max
> capability.
>
> So add a new bitmap to RISCVSATPMap which contains this capability and
> initialize it in every XXX_cpu_init.
>
> Finally, we have the following chain of constraints:
>
> Qemu capability > HW capability > User choice > Software capability
^ What software is this?
I'd think the user's choice would always be last.
>
> Signed-off-by: Alexandre Ghiti
> ---
> target/riscv/cpu.c | 78 +++---
> target/riscv/cpu.h | 8 +++--
> 2 files changed, 59 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e409e6ab64..19a37fee2b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -292,24 +292,39 @@ const char *satp_mode_str(uint8_t satp_mode, bool
> is_32_bit)
> g_assert_not_reached();
> }
>
> -/* Sets the satp mode to the max supported */
> -static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> +const char *satp_mode_str,
> +bool is_32_bit)
> {
> -if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> -cpu->cfg.satp_mode.map |=
> -(1 << satp_mode_from_str(is_32_bit ? "sv32" :
> "sv57"));
> -} else {
> -cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> +uint8_t satp_mode = satp_mode_from_str(satp_mode_str);
> +const bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
> +
> +for (int i = 0; i <= satp_mode; ++i) {
> +if (valid_vm[i]) {
> +cpu->cfg.satp_mode.supported |= (1 << i);
I don't think we need a new 'supported' bitmap, I think each board that
needs to further constrain va-bits from what QEMU supports should just set
valid_vm_1_10_32/64. I.e. drop const from the arrays and add an init
function something like
#define QEMU_SATP_MODE_MAX VM_1_10_SV64
void riscv_cpu_set_satp_mode_max(RISCVCPU *cpu, uint8_t satp_mode_max)
{
bool is_32_bit = cpu->env.misa_mxl == MXL_RV32;
bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
g_assert(satp_mode_max <= QEMU_SATP_MODE_MAX);
g_assert(!is_32_bit || satp_mode_max < 2);
memset(valid_vm, 0, sizeof(*valid_vm));
for (int i = 0; i <= satp_mode_max; i++) {
valid_vm[i] = true;
}
}
The valid_vm[] checks already in finalize should then manage the
validation needed to constrain boards. Only boards that care about
this need to call this function, otherwise they'll get the default.
Also, this patch should come before the patch that changes the default
for all boards to sv57 in order to avoid breaking bisection.
Thanks,
drew
Re: [PATCH v3 07/11] tests/qtest/migration-test: Build command line using GString API (1/4)
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> Part 1/4: Convert memory & machine options.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Dr. David Alan Gilbert
> ---
> tests/qtest/migration-test.c | 24 ++--
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index f96c73f552..9cdef4fa65 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -582,6 +582,7 @@ typedef struct {
> static int test_migrate_start(QTestState **from, QTestState **to,
>const char *uri, MigrateStart *args)
> {
> +g_autoptr(GString) cmd_common = NULL;
> g_autofree gchar *arch_source = NULL;
> g_autofree gchar *arch_target = NULL;
> g_autofree gchar *cmd_target = NULL;
> @@ -601,6 +602,9 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> }
>
> got_stop = false;
> +
> +cmd_common = g_string_new("");
> +
> bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> /* the assembled x86 boot sector should be exactly one sector large
> */
> @@ -644,6 +648,10 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> } else {
> g_assert_not_reached();
> }
> +if (machine_opts) {
> +g_string_append_printf(cmd_common, " -machine %s ", machine_opts);
> +}
> +g_string_append_printf(cmd_common, "-m %s ", memory_size);
>
> if (!getenv("QTEST_LOG") && args->hide_stderr) {
> #ifdef _WIN32
> @@ -674,33 +682,29 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> if (!args->only_target) {
> g_autofree gchar *cmd_source = NULL;
>
> -cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
> +cmd_source = g_strdup_printf("-accel kvm%s -accel tcg %s "
> "-name source,debug-threads=on "
> - "-m %s "
> "-serial file:%s/src_serial "
> "%s %s %s %s",
> args->use_dirty_ring ?
> ",dirty-ring-size=4096" : "",
> - machine_opts ? " -machine " : "",
> - machine_opts ? machine_opts : "",
> - memory_size, tmpfs,
> + cmd_common->str,
> + tmpfs,
> arch_source, shmem_opts,
> args->opts_source ? args->opts_source :
> "",
> ignore_stderr);
> *from = qtest_init(cmd_source);
> }
>
> -cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
> +cmd_target = g_strdup_printf("-accel kvm%s -accel tcg %s "
> "-name target,debug-threads=on "
> - "-m %s "
> "-serial file:%s/dest_serial "
> "-incoming %s "
> "%s %s %s %s",
> args->use_dirty_ring ?
> ",dirty-ring-size=4096" : "",
> - machine_opts ? " -machine " : "",
> - machine_opts ? machine_opts : "",
> - memory_size, tmpfs, uri,
> + cmd_common->str,
> + tmpfs, uri,
> arch_target, shmem_opts,
> args->opts_target ? args->opts_target : "",
> ignore_stderr);
> --
> 2.38.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v3 09/11] tests/qtest/migration-test: Build command line using GString API (3/4)
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> Part 3/4: Convert accelerator options.
>
> Reviewed-by: Richard Henderson
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tests/qtest/migration-test.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 670097a956..1ed3505c91 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -603,6 +603,13 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> got_stop = false;
>
> cmd_common = g_string_new("");
> +/* KVM first */
> +if (args->use_dirty_ring) {
> +g_string_append(cmd_common, "-accel kvm,dirty-ring-size=4096 ");
> +} else {
> +g_string_append(cmd_common, "-accel kvm ");
> +}
> +g_string_append(cmd_common, "-accel tcg ");
Yep, that's the right way around this time :-)
Reviewed-by: Dr. David Alan Gilbert
> bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> @@ -678,12 +685,10 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> if (!args->only_target) {
> g_autofree gchar *cmd_source = NULL;
>
> -cmd_source = g_strdup_printf("-accel kvm%s -accel tcg %s "
> +cmd_source = g_strdup_printf("%s "
> "-name source,debug-threads=on "
> "-serial file:%s/src_serial "
> "%s %s %s",
> - args->use_dirty_ring ?
> - ",dirty-ring-size=4096" : "",
> cmd_common->str,
> tmpfs,
> arch_source,
> @@ -692,13 +697,11 @@ static int test_migrate_start(QTestState **from,
> QTestState **to,
> *from = qtest_init(cmd_source);
> }
>
> -cmd_target = g_strdup_printf("-accel kvm%s -accel tcg %s "
> +cmd_target = g_strdup_printf("%s "
> "-name target,debug-threads=on "
> "-serial file:%s/dest_serial "
> "-incoming %s "
> "%s %s %s",
> - args->use_dirty_ring ?
> - ",dirty-ring-size=4096" : "",
> cmd_common->str,
> tmpfs, uri,
> arch_target,
> --
> 2.38.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH] configure: Expand test which disable -Wmissing-braces
On Fri, Jan 06, 2023 at 03:21:10PM +0100, Anthony PERARD via wrote:
> From: Anthony PERARD
>
> With "clang 6.0.0-1ubuntu2" on Ubuntu Bionic, the test with build
> fine, but clang still suggest braces around the zero initializer in a
> few places, where there is a subobject. Expand test to include a sub
> struct which doesn't build on clang 6.0.0-1ubuntu2, and give:
> config-temp/qemu-conf.c:7:8: error: suggest braces around initialization
> of subobject [-Werror,-Wmissing-braces]
> } x = {0};
>^
>{}
>
> These are the error reported by clang on QEMU's code (v7.2.0):
> hw/pci-bridge/cxl_downstream.c:101:51: error: suggest braces around
> initialization of subobject [-Werror,-Wmissing-braces]
> dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
>
> hw/pci-bridge/cxl_root_port.c:62:51: error: suggest braces around
> initialization of subobject [-Werror,-Wmissing-braces]
> dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
>
> tests/qtest/virtio-net-test.c:322:34: error: suggest braces around
> initialization of subobject [-Werror,-Wmissing-braces]
> QOSGraphTestOptions opts = { 0 };
>
> Reported-by: Andrew Cooper
> Signed-off-by: Anthony PERARD
> ---
>
> While Ubuntu Bionic isn't supposed to be supported anymore, clang v6
> is still the minimum required as tested by ./configure.
The configure checks don't always keep track with our supported OS
versions, because we often don't update the min versions until we
find a technical issue that motivates it. IOW, the supported OS
versions are considered to be the overriding policy regardless of
what the code tool/library versions checks currently implement.
So if the compile problem only exists on OS versions that are outside
our support matrix, then we should be bumping the minimum version
check in configure, which could possibly enable us to get rid of the
entire check for broken -Wmissing-braces.
> ---
> configure | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/configure b/configure
> index 9f0bc57546..3cd9b8bad4 100755
> --- a/configure
> +++ b/configure
> @@ -1290,7 +1290,11 @@ fi
> # Disable -Wmissing-braces on older compilers that warn even for
> # the "universal" C zero initializer {0}.
> cat > $TMPC << EOF
> +struct s {
> + void *a;
> +};
> struct {
> + struct s s;
>int a[2];
> } x = {0};
> EOF
> --
> Anthony PERARD
>
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] coverity: physmem: use simple assertions instead of modelling
On 26.12.22 23:03, Vladimir Sementsov-Ogievskiy wrote:
Unfortunately Coverity doesn't follow the logic aroung "len" and "l"
variables in stacks finishing with flatview_{read,write}_continue() and
generate a lot of OVERRUN false-positives. When small buffer (2 or 4
bytes) is passed to mem read/write path, Coverity assumes the worst
case of sz=8 in stn_he_p()/ldn_he_p() (defined in
include/qemu/bswap.h), and reports buffer overrun.
To silence these false-positives we have model functions, which hide
real logic from Coverity.
However, it turned out that these new two assertions are enough to
quiet Coverity.
Assertions are better than hiding the logic, so let's drop the
modelling and move to assertions for memory r/w call stacks.
After patch, the sequence
cov-make-library --output-file /tmp/master.xmldb \
scripts/coverity-scan/model.c
cov-build --dir ~/covtmp/master make -j9
cov-analyze --user-model-file /tmp/master.xmldb \
--dir ~/covtmp/master --all --strip-path "$(pwd)
cov-format-errors --dir ~/covtmp/master \
--html-output ~/covtmp/master_html_report
Generate for me the same big set of CIDs excepept for 6 disappeared (so
it becomes even better).
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
scripts/coverity-scan/model.c | 88 ---
softmmu/physmem.c | 18 +++
2 files changed, 18 insertions(+), 88 deletions(-)
diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
index 686d1a3008..a064d84084 100644
--- a/scripts/coverity-scan/model.c
+++ b/scripts/coverity-scan/model.c
@@ -42,94 +42,6 @@ typedef _Bool bool;
typedef struct va_list_str *va_list;
-/* exec.c */
-
-typedef struct AddressSpace AddressSpace;
-typedef struct MemoryRegionCache MemoryRegionCache;
-typedef uint64_t hwaddr;
-typedef uint32_t MemTxResult;
-typedef struct MemTxAttrs {} MemTxAttrs;
-
-static void __bufwrite(uint8_t *buf, ssize_t len)
-{
-int first, last;
-__coverity_negative_sink__(len);
-if (len == 0) return;
-buf[0] = first;
-buf[len-1] = last;
-__coverity_writeall__(buf);
-}
-
-static void __bufread(uint8_t *buf, ssize_t len)
-{
-__coverity_negative_sink__(len);
-if (len == 0) return;
-int first = buf[0];
-int last = buf[len-1];
-}
-
-MemTxResult address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
- MemTxAttrs attrs,
- void *buf, int len)
-{
-MemTxResult result;
-// TODO: investigate impact of treating reads as producing
-// tainted data, with __coverity_tainted_data_argument__(buf).
-__bufwrite(buf, len);
-return result;
-}
-
-MemTxResult address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
-MemTxAttrs attrs,
-const void *buf, int len)
-{
-MemTxResult result;
-__bufread(buf, len);
-return result;
-}
-
-MemTxResult address_space_rw_cached(MemoryRegionCache *cache, hwaddr addr,
-MemTxAttrs attrs,
-void *buf, int len, bool is_write)
-{
-if (is_write) {
-return address_space_write_cached(cache, addr, attrs, buf, len);
-} else {
-return address_space_read_cached(cache, addr, attrs, buf, len);
-}
-}
-
-MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs,
- void *buf, int len)
-{
-MemTxResult result;
-// TODO: investigate impact of treating reads as producing
-// tainted data, with __coverity_tainted_data_argument__(buf).
-__bufwrite(buf, len);
-return result;
-}
-
-MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
-MemTxAttrs attrs,
-const void *buf, int len)
-{
-MemTxResult result;
-__bufread(buf, len);
-return result;
-}
-
-MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
- MemTxAttrs attrs,
- void *buf, int len, bool is_write)
-{
-if (is_write) {
-return address_space_write(as, addr, attrs, buf, len);
-} else {
-return address_space_read(as, addr, attrs, buf, len);
-}
-}
-
/* Tainting */
typedef struct {} name2keysym_t;
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index edec095c7a..24571002b3 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2821,6 +2821,15 @@ static MemTxResult flatview_write_continue(FlatView *fv,
hwaddr addr,
l = memory_access_size(mr, l, addr1);
/* XXX: could force current_cpu to NULL to avoid
potential bugs */
+
+/*
+ * Assure Coverity (and ourselves) that we are not going to OVERRUN
+ * the buffer by following ldn_he_p().
+ */
+assert((l == 1 && len >= 1) ||
Re: [PATCH v2 07/11] audio/audio_template: use g_malloc0() to replace audio_calloc()
On Sat, Jan 21, 2023 at 10:47:31AM +0100, Volker Rümelin wrote: > Use g_malloc0() as a direct replacement for audio_calloc(). > > Since the type of the parameter n_bytes of the function g_malloc0() > is unsigned, the type of the variables voice_size_out and > voice_size_in has been changed to size_t. This means that the > function argument no longer has to be checked for negative values. > > Signed-off-by: Volker Rümelin > --- > audio/audio_int.h | 4 ++-- > audio/audio_template.h | 18 -- > 2 files changed, 10 insertions(+), 12 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 08/11] audio/audio_template: use g_new0() to replace audio_calloc()
On Sat, Jan 21, 2023 at 10:47:32AM +0100, Volker Rümelin wrote: > Replace audio_calloc() with the equivalent g_new0(). > > With a n_structs argument >= 1, g_new0() never returns NULL. > Also remove the unnecessary NULL checks. > > Signed-off-by: Volker Rümelin > --- > audio/audio_template.h | 29 - > 1 file changed, 12 insertions(+), 17 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
On Mon, Jan 23, 2023 at 10:20:29AM +0100, Philippe Mathieu-Daudé wrote:
> On 23/1/23 09:39, Thomas Huth wrote:
> > From: Daniel P. Berrangé
> >
> > Way back in QEMU 4.0, the -audiodev command line option was introduced
> > for configuring audio backends. This CLI option does not use QemuOpts
> > so it is not visible for introspection in 'query-command-line-options',
> > instead using the QAPI Audiodev type. Unfortunately there is also no
> > QMP command that uses the Audiodev type, so it is not introspectable
> > with 'query-qmp-schema' either.
> >
> > This introduces a 'query-audiodev' command that simply reflects back
> > the list of configured -audiodev command line options. This alone is
> > maybe not very useful by itself, but it makes Audiodev introspectable
> > via 'query-qmp-schema', so that libvirt (and other upper layer tools)
> > can discover the available audiodevs.
> >
> > Signed-off-by: Daniel P. Berrangé
> > [thuth: Update for upcoming QEMU v8.0, and use QAPI_LIST_PREPEND]
> > Signed-off-by: Thomas Huth
> > ---
> > qapi/audio.json | 13 +
> > audio/audio.c | 12
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/qapi/audio.json b/qapi/audio.json
> > index 1e0a24bdfc..c7aafa2763 100644
> > --- a/qapi/audio.json
> > +++ b/qapi/audio.json
> > @@ -443,3 +443,16 @@
> > 'sndio': 'AudiodevSndioOptions',
> > 'spice': 'AudiodevGenericOptions',
> > 'wav': 'AudiodevWavOptions' } }
> > +
> > +##
> > +# @query-audiodevs:
> > +#
> > +# Returns information about audiodev configuration
>
> Maybe clearer as 'audio backends'?
>
> So similarly, wouldn't be clearer to name this command
> 'query-audio-backends'? Otherwise we need to go read QEMU
> source to understand what is 'audiodevs'.
The command line parameter is called '-audiodev' and this
query-audiodevs command reports the same data, so that
looks easy enough to understand IMHO.
> > +#
> > +# Returns: array of @Audiodev
> > +#
> > +# Since: 8.0
> > +#
> > +##
> > +{ 'command': 'query-audiodevs',
> > + 'returns': ['Audiodev'] }
> > diff --git a/audio/audio.c b/audio/audio.c
> > index d849a94a81..6f270c07b7 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -28,8 +28,10 @@
> > #include "monitor/monitor.h"
> > #include "qemu/timer.h"
> > #include "qapi/error.h"
> > +#include "qapi/clone-visitor.h"
> > #include "qapi/qobject-input-visitor.h"
> > #include "qapi/qapi-visit-audio.h"
> > +#include "qapi/qapi-commands-audio.h"
> > #include "qemu/cutils.h"
> > #include "qemu/module.h"
> > #include "qemu/help_option.h"
> > @@ -2311,3 +2313,13 @@ size_t audio_rate_get_bytes(RateCtl *rate, struct
> > audio_pcm_info *info,
> > return bytes;
> > }
> > +
> > +AudiodevList *qmp_query_audiodevs(Error **errp)
> > +{
> > +AudiodevList *ret = NULL;
> > +AudiodevListEntry *e;
> > +QSIMPLEQ_FOREACH(e, &audiodevs, next) {
>
> I am a bit confused here, isn't &audiodevs containing what the user provided
> from CLI? How is that useful to libvirt? Maybe the corner case
> of a user hand-modifying the QEMU launch arguments from a XML config?
>
> Wouldn't a list of linked in AudiodevDriver be more useful to libvirt
> so it could pick the best available backend to start a VM?
On the libvirt side we're never going to need to actually call the
query-audiodevs commands. The mere existance of the command, means
that the QMP schema now exposes information about what audio backends
have been compiled into the binary. This is the same trick we've used
for other aspects of QMP. IOW we don't need a separate command just
for the purpose of listing AudiodevDrivers.
The idea of a query-audiodevs command is useful in general. When we
later gain support for hotplug/unplug of audio, the set of audiodevs
will no longer be guaranteed to match the original CLI args.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v6 5/5] riscv: Introduce satp mode hw capabilities
On Mon, Jan 23, 2023 at 11:51 AM Andrew Jones wrote:
>
> On Mon, Jan 23, 2023 at 10:03:24AM +0100, Alexandre Ghiti wrote:
> > Currently, the max satp mode is set with the only constraint that it must be
> > implemented in qemu, i.e. set in valid_vm_1_10_[32|64].
> >
> > But we actually need to add another level of constraint: what the hw is
> > actually capable of, because currently, a linux booting on a sifive-u54
> > boots in sv57 mode which is incompatible with the cpu's sv39 max
> > capability.
> >
> > So add a new bitmap to RISCVSATPMap which contains this capability and
> > initialize it in every XXX_cpu_init.
> >
> > Finally, we have the following chain of constraints:
> >
> > Qemu capability > HW capability > User choice > Software capability
>
> ^ What software is this?
> I'd think the user's choice would always be last.
>
> >
> > Signed-off-by: Alexandre Ghiti
> > ---
> > target/riscv/cpu.c | 78 +++---
> > target/riscv/cpu.h | 8 +++--
> > 2 files changed, 59 insertions(+), 27 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index e409e6ab64..19a37fee2b 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -292,24 +292,39 @@ const char *satp_mode_str(uint8_t satp_mode, bool
> > is_32_bit)
> > g_assert_not_reached();
> > }
> >
> > -/* Sets the satp mode to the max supported */
> > -static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
> > +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> > +const char *satp_mode_str,
> > +bool is_32_bit)
> > {
> > -if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> > -cpu->cfg.satp_mode.map |=
> > -(1 << satp_mode_from_str(is_32_bit ? "sv32" :
> > "sv57"));
> > -} else {
> > -cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> > +uint8_t satp_mode = satp_mode_from_str(satp_mode_str);
> > +const bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
> > +
> > +for (int i = 0; i <= satp_mode; ++i) {
> > +if (valid_vm[i]) {
> > +cpu->cfg.satp_mode.supported |= (1 << i);
>
> I don't think we need a new 'supported' bitmap, I think each board that
> needs to further constrain va-bits from what QEMU supports should just set
> valid_vm_1_10_32/64. I.e. drop const from the arrays and add an init
> function something like
This was my first idea too, but those arrays are global and I have to
admit that I thought it was possible to emulate a cpu with different
cores. Anyway, isn't it a bit weird to store this into some global
array whereas it is intimately linked to the CPU? To me, it makes
sense to keep those variables as a way to know what qemu is able to
emulate and have a CPU specific map like in this patch for the hw
capabilities. Does it make sense to you?
>
> #define QEMU_SATP_MODE_MAX VM_1_10_SV64
>
> void riscv_cpu_set_satp_mode_max(RISCVCPU *cpu, uint8_t satp_mode_max)
> {
> bool is_32_bit = cpu->env.misa_mxl == MXL_RV32;
> bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
>
> g_assert(satp_mode_max <= QEMU_SATP_MODE_MAX);
> g_assert(!is_32_bit || satp_mode_max < 2);
>
> memset(valid_vm, 0, sizeof(*valid_vm));
>
> for (int i = 0; i <= satp_mode_max; i++) {
> valid_vm[i] = true;
> }
> }
>
> The valid_vm[] checks already in finalize should then manage the
> validation needed to constrain boards. Only boards that care about
> this need to call this function, otherwise they'll get the default.
>
> Also, this patch should come before the patch that changes the default
> for all boards to sv57 in order to avoid breaking bisection.
As I explained earlier, I didn't change the default to sv57! Just
fixed what was passed via the device tree, which should not be used
anyway :)
Alex
>
> Thanks,
> drew
Re: [PATCH] linux-user: Add support for LoongArch64's old world ABI
On 1/23/23 18:47, Peter Maydell wrote: On Mon, 23 Jan 2023 at 10:27, WANG Xuerui wrote: From: WANG Xuerui This patch adds a "loongarch64ow-linux-user" target and a corresponding "qemu-loongarch64ow" binary, for supporting user-mode emulation of old-world LoongArch applications in the wild. Although the old-world LoongArch is already being (slowly) phased out, there are already a number of deployments (mainly as a result of LoongArch's early commercial growth), whose migration path is something software developers have to care about. Support for user-mode emulation in addition to system-level emulation would help development of such migration & compatibility solutions. Is this 'old-world' ABI supported by the upstream Linux kernel? I can't see signs of it from a quick grep. If it isn't, then I'm not sure if we should support it in QEMU user-mode emulation. We've always set "upstream Linux" as our definition of what the official ABI and featureset is for usermode emulation. No, the old-world is not, and will not be, supported by upstream Linux, as it is strictly the premature state of this architecture only born for commercial reasons. After sending the patch I've discussed with several other maintainers of various LoongArch ports, and it seems even Loongson the corporation itself is seeing to completely abandon old-world development in favor of a 100% new world future (which is even more courageous than me, an unaffiliated hobbyist user). Given the current old-world deployments all have commercial support, it's probably best to leave the migration work for them and keep upstream clean of any such legacy. So I'm dropping this patch now, and thanks for the feedback!
[PATCH] linux-user: Add support for LoongArch64's old world ABI
From: WANG Xuerui This patch adds a "loongarch64ow-linux-user" target and a corresponding "qemu-loongarch64ow" binary, for supporting user-mode emulation of old-world LoongArch applications in the wild. Although the old-world LoongArch is already being (slowly) phased out, there are already a number of deployments (mainly as a result of LoongArch's early commercial growth), whose migration path is something software developers have to care about. Support for user-mode emulation in addition to system-level emulation would help development of such migration & compatibility solutions. The differences we have to care about are: - TARGET_NSIG: 64 for new-world, 128 for old-world - System calls: fstat, newfstatat, getrlimit and setrlimit only present on old-world Other incompatibilities exist, but they are irrelevant to emulators like QEMU, as these are entirely contained within the particular firmware image or sysroot in use. Corresponding binfmt_misc magic has been updated to allow somewhat proper classification of LoongArch executables based on the object ABI version bitfield [1] in the ELF header's e_flags field. We make the assumption that all old-world binaries are compiled with an older toolchain that produce v0 binaries, and that all new-world users have migrated to object ABI v1 by now -- the initial new-world userbase is small and technical enough that anyone who has not migrated by now is simply advised to rebuild world or reinstall. As for why this cannot be done within one binary: currently TARGET_NSIG is a compile-time constant, hence it is not currently possible for us to implement an allegedly more elegant solution, of checking the .note.ABI-tag section (then falling back to e_flags heuristics if the optional section is absent). [1]: https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html#_e_flags_identifies_abi_type_and_version Signed-off-by: WANG Xuerui --- Tested by successfully emulating a Loongnix chroot, running old-world LoongArch apps such as the bundled Kingsoft Office 2019, in addition to common CLI utilities and some of the MATE apps included in the distro's live ISO. configs/targets/loongarch64ow-linux-user.mak | 4 + linux-user/loongarch64/syscall_base_nr.h | 312 ++ linux-user/loongarch64/syscall_nr.h | 316 +-- linux-user/syscall_defs.h| 9 +- scripts/gensyscalls.sh | 2 +- scripts/qemu-binfmt-conf.sh | 12 +- 6 files changed, 340 insertions(+), 315 deletions(-) create mode 100644 configs/targets/loongarch64ow-linux-user.mak create mode 100644 linux-user/loongarch64/syscall_base_nr.h diff --git a/configs/targets/loongarch64ow-linux-user.mak b/configs/targets/loongarch64ow-linux-user.mak new file mode 100644 index 00..41eb8c16a7 --- /dev/null +++ b/configs/targets/loongarch64ow-linux-user.mak @@ -0,0 +1,4 @@ +# Default configuration for loongarch64-linux-user +TARGET_ARCH=loongarch64 +TARGET_BASE_ARCH=loongarch +TARGET_ABI_LOONGARCH64_OW=y diff --git a/linux-user/loongarch64/syscall_base_nr.h b/linux-user/loongarch64/syscall_base_nr.h new file mode 100644 index 00..f84434bfa2 --- /dev/null +++ b/linux-user/loongarch64/syscall_base_nr.h @@ -0,0 +1,312 @@ +/* + * This file contains the system call numbers. + * Do not modify. + * This file is generated by scripts/gensyscalls.sh + */ +#ifndef LINUX_USER_LOONGARCH_SYSCALL_BASE_NR_H +#define LINUX_USER_LOONGARCH_SYSCALL_BASE_NR_H + +#define TARGET_NR_io_setup 0 +#define TARGET_NR_io_destroy 1 +#define TARGET_NR_io_submit 2 +#define TARGET_NR_io_cancel 3 +#define TARGET_NR_io_getevents 4 +#define TARGET_NR_setxattr 5 +#define TARGET_NR_lsetxattr 6 +#define TARGET_NR_fsetxattr 7 +#define TARGET_NR_getxattr 8 +#define TARGET_NR_lgetxattr 9 +#define TARGET_NR_fgetxattr 10 +#define TARGET_NR_listxattr 11 +#define TARGET_NR_llistxattr 12 +#define TARGET_NR_flistxattr 13 +#define TARGET_NR_removexattr 14 +#define TARGET_NR_lremovexattr 15 +#define TARGET_NR_fremovexattr 16 +#define TARGET_NR_getcwd 17 +#define TARGET_NR_lookup_dcookie 18 +#define TARGET_NR_eventfd2 19 +#define TARGET_NR_epoll_create1 20 +#define TARGET_NR_epoll_ctl 21 +#define TARGET_NR_epoll_pwait 22 +#define TARGET_NR_dup 23 +#define TARGET_NR_dup3 24 +#define TARGET_NR_fcntl 25 +#define TARGET_NR_inotify_init1 26 +#define TARGET_NR_inotify_add_watch 27 +#define TARGET_NR_inotify_rm_watch 28 +#define TARGET_NR_ioctl 29 +#define TARGET_NR_ioprio_set 30 +#define TARGET_NR_ioprio_get 31 +#define TARGET_NR_flock 32 +#define TARGET_NR_mknodat 33 +#define TARGET_NR_mkdirat 34 +#define TARGET_NR_unlinkat 35 +#define TARGET_NR_symlinkat 36 +#define TARGET_NR_linkat 37 +#define TARGET_NR_umount2 39 +#define TARGET_NR_mount 40 +#define TARGET_NR_pivot_root 41 +#define TARGET_NR_nfsservctl 42 +#define TARGET_NR_statfs 43 +#define TARGET_NR_fstatfs 44 +#define TARGET_NR_truncate 45 +#define TARGET_NR_ftruncate 46 +#define
Re: [PATCH v2 2/2] target/riscv: redirect XVentanaCondOps to use the Zicond functions
On Mon, Jan 23, 2023 at 11:37 AM Philipp Tomsich
wrote:
>
> On Mon, 23 Jan 2023 at 02:29, Alistair Francis wrote:
> >
> > On Sat, Jan 21, 2023 at 12:36 PM Philipp Tomsich
> > wrote:
> > >
> > > The Zicond standard extension implements the same instruction
> > > semantics as XVentanaCondOps, although using different mnemonics and
> > > opcodes.
> > >
> > > Point XVentanaCondOps to the (newly implemented) Zicond implementation
> > > to reduce the future maintenance burden.
> > >
> > > Also updating MAINTAINERS as trans_xventanacondops.c.inc will not see
> > > active maintenance from here forward.
> > >
> > > Signed-off-by: Philipp Tomsich
> > > ---
> > >
> > > Changes in v2:
> > > - Calls into the gen_czero_{eqz,nez} helpers instead of calling
> > > trans_czero_{eqz,nez} to bypass the require-check and ensure that
> > > XVentanaCondOps can be enabled/disabled independently of Zicond.
> > >
> > > MAINTAINERS| 2 +-
> > > .../insn_trans/trans_xventanacondops.c.inc | 18 +++---
> > > 2 files changed, 4 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index ca914c42fa..293a9d1c8c 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -305,7 +305,7 @@ F: target/riscv/insn_trans/trans_zicond.c.inc
> > > RISC-V XVentanaCondOps extension
> > > M: Philipp Tomsich
> > > L: qemu-ri...@nongnu.org
> > > -S: Supported
> > > +S: Odd Fixes
> >
> > Should this extension be deprecated then?
>
> The extension is out in the wild (as the Ventana Veyron V1 core
> implements it), so we shouldn't deprecate it.
> However, this now is the thinnest possible layer of implementation
> (and will pick up any fixes/updates from Zicond).
>
> I felt that downgrading it to "Odd Fixes" was the right way to
> indicate this. Let me know if you would like to handle it
> differently.
It probably makes sense to just leave it as supported then. It's up to
a vendor to support their extensions, so I feel that marking it as
"Off Fixes" is a little strange.
Alistair
>
> Philipp.
Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function
On Mon, Jan 23, 2023 at 8:19 PM Daniel Henrique Barboza wrote: > > > > On 1/22/23 19:53, Alistair Francis wrote: > > On Sun, Jan 22, 2023 at 5:16 AM Daniel Henrique Barboza > > wrote: > >> > >> Conor, > >> > >> Thanks for the Icicle-kit walk-through! I'll not claim that I fully > >> understood it, > >> but I understood enough to handle the situation ATM. > >> > >> Without this change, this is where the FDT is being installed in the board > >> when > >> I start it with 8Gb of RAM (retrieved via 'info roms'): > >> > >> addr=bfe0 size=0x00a720 mem=ram name="fdt" > >> > >> Which surprised me at first because this is almost at the end of the LO > >> area which has > >> 1Gb and I figured it would be in the middle of another RAM area. I took > >> another read > >> at what we're doing in riscv_load_fdt(): > >> > >> --- > >> temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; > >> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB); > >> --- > >> > >> This code can be read as "if the starting address of the RAM is lower than > >> 3Gb, put > >> the FDT no further than 3Gb (0xc000). Otherwise, put it at the end of > >> dram", > >> where "dram_base" is the starting address of the RAM block that the > >> function > >> receives. > >> > >> For icicle-kit, this is being passed as > >> memmap[MICROCHIP_PFSOC_DRAM_LO].base, > >> 0x8000, which is 2Gb. > >> > >> So, regardless of how much RAM we have (dram_end), the FDT will always be > >> capped at > >> 3Gb. At this moment, this fits exactly at the end of the LO area for the > >> Icicle Kit. > >> Which is funny because this 3Gb restriction was added by commit > >> 1a475d39ef54 to fix > >> 32 bit guest boot and it happened to also work for the Microchip SoC. > >> > >> So yeah, I thought that I was fixing a bug and in the end I caused one. > >> This patch > >> needs to go. > >> > >> > >> Alistair, I believe I should re-send v2, this time explaining why the > >> existing function > >> will not break the Microchip board because we'll never put the FDT out of > >> the LO area > >> of the board. Does this work for you? > > > > I think that's fine. My only worry is that we are losing some > > flexibility that some future board might want. > > What if we change riscv_load_fdt() parameters to pass a > MemoryRegion/MemMapEntry > instead of just dram_base? > > Instead of this: > > uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > > We would have this: > > uint64_t riscv_load_fdt(MemMapEntry mem, uint64_t mem_size, void *fdt) > > Or even this: > > uint64_t riscv_load_fdt(hwaddr dram_base, hwaddr dram_size, > uint64_t mem_size, void *fdt) > > > And then we can make assumptions based on the actual memory region that the > fdt > is going to fit into, instead of having a starting address and a total memory > size and have to deal with issues such as sparse memory. > > We can keep all the assumptions already made today (e.g. the 3Gb maximum addr) > while also having a guarantee that the fdt isn't going to be put in the wrong > memory region/spot if we decide to change the assumptions later on. That seems like a good direction. We currently don't need this though, so don't feel like it needs to be done today. Alistair > > > Thanks, > > Daniel > > > > > > > Alistair
Re: [PATCH] hw/riscv: boot: Don't use CSRs if they are disabled
On Mon, Jan 23, 2023 at 8:25 PM Daniel Henrique Barboza
wrote:
>
>
>
> On 1/23/23 00:57, Alistair Francis wrote:
> > From: Alistair Francis
> >
> > If the CSRs and CSR instructions are disabled because the Zicsr
> > extension isn't enabled then we want to make sure we don't run any CSR
> > instructions in the boot ROM.
> >
> > This patches removes the CSR instructions from the reset-vec if the
> > extension isn't enabled. We replace the instruction with a NOP instead.
> >
> > Note that we don't do this for the SiFive U machine, as we are modelling
> > the hardware in that case.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1447
> > Signed-off-by: Alistair Francis
> > ---
>
> Shouldn't we also handle the sifive_u/sifive_e boards? Their reset vectors
> aren't being covered by riscv_set_rom_reset_vec() (it's on my TODO, didn't
> send it yet because sifive uses an extra MSEL pin at the start of the vector).
I feel that those boards are modelling hardware, so in this case we
should model what the hardware does. I don't even think that a user
could disable the CSR extension on those boards.
Alistair
>
>
>
> Daniel
>
>
>
> > hw/riscv/boot.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 2594276223..cb27798a25 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -356,6 +356,15 @@ void riscv_setup_rom_reset_vec(MachineState *machine,
> > RISCVHartArrayState *harts
> > reset_vec[4] = 0x0182b283; /* ld t0, 24(t0) */
> > }
> >
> > +if (!harts->harts[0].cfg.ext_icsr) {
> > +/*
> > + * The Zicsr extension has been disabled, so let's ensure we don't
> > + * run the CSR instruction. Let's fill the address with a non
> > + * compressed nop.
> > + */
> > +reset_vec[2] = 0x0013; /* addi x0, x0, 0 */
> > +}
> > +
> > /* copy in the reset vector in little_endian byte order */
> > for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
> > reset_vec[i] = cpu_to_le32(reset_vec[i]);
Re: [PATCH] hw/riscv: boot: Don't use CSRs if they are disabled
On 1/23/23 00:57, Alistair Francis wrote:
From: Alistair Francis
If the CSRs and CSR instructions are disabled because the Zicsr
extension isn't enabled then we want to make sure we don't run any CSR
instructions in the boot ROM.
This patches removes the CSR instructions from the reset-vec if the
extension isn't enabled. We replace the instruction with a NOP instead.
Note that we don't do this for the SiFive U machine, as we are modelling
the hardware in that case.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1447
Signed-off-by: Alistair Francis
---
Reviewed-by: Daniel Henrique Barboza
hw/riscv/boot.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 2594276223..cb27798a25 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -356,6 +356,15 @@ void riscv_setup_rom_reset_vec(MachineState *machine,
RISCVHartArrayState *harts
reset_vec[4] = 0x0182b283; /* ld t0, 24(t0) */
}
+if (!harts->harts[0].cfg.ext_icsr) {
+/*
+ * The Zicsr extension has been disabled, so let's ensure we don't
+ * run the CSR instruction. Let's fill the address with a non
+ * compressed nop.
+ */
+reset_vec[2] = 0x0013; /* addi x0, x0, 0 */
+}
+
/* copy in the reset vector in little_endian byte order */
for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
reset_vec[i] = cpu_to_le32(reset_vec[i]);
Re: [RFC PATCH 2/2] hw/sd: skip double power-up in sd_vmstate_pre_load()
* Philippe Mathieu-Daudé (phi...@linaro.org) wrote:
> +David / Juan / Peter for migration and timers.
>
> On 20/1/23 13:01, Daniel Henrique Barboza wrote:
> > At this moment any migration with the RISC-V sifive_u machine
> > fails with the following error:
> >
> > qemu-system-riscv64: ../hw/sd/sd.c:297: sd_ocr_powerup: Assertion
> > `!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)' failed.
> >
> > The assert was introduced by dd26eb43337a ("hw/sd: model a power-up
> > delay, as a workaround for an EDK2 bug"). It introduced a delayed timer
> > of 0.5ms to power up the card after the first ACMD41 command. The assert
> > prevents the card from being turned on twice.
> >
> > When migrating a machine that uses a sd card, e.g. RISC-V sifive_u, the
> > card is turned on during machine_init() in both source and destination
> > hosts.
If that's a problem, then why don't you make that machine_init code
check whether we're INMIGRATE, and skip the power on?
> When the migration stream finishes in the destination, the
> > pre_load() hook will attempt to turn on the card before loading its
> > vmstate. The assert() is always going to hit because the card was
> > already on.
> >
> > Change sd_vmstate_pre_load() to check first if the sd card is turned on
> > before executing a sd_ocr_powerup() and triggering the assert.
Again, not knowing this device, but the other thought is a Post_load
that determines if the OCR state hasn't been loaded, and then turns it
on.
Dave
> > Signed-off-by: Daniel Henrique Barboza
> > ---
> > hw/sd/sd.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index bd88c1a8f0..4add719643 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -664,11 +664,19 @@ static int sd_vmstate_pre_load(void *opaque)
> > {
> > SDState *sd = opaque;
> > -/* If the OCR state is not included (prior versions, or not
> > +/*
> > + * If the OCR state is not included (prior versions, or not
> >* needed), then the OCR must be set as powered up. If the OCR state
> >* is included, this will be replaced by the state restore.
> > + *
> > + * However, there's a chance that the board will powerup the SD
> > + * before reaching INMIGRATE state in the destination host.
> > + * Powering up the SD again in this case will cause an assert
> > + * inside sd_ocr_powerup(). Skip sd_ocr_powerup() in this case.
> >*/
> > -sd_ocr_powerup(sd);
> > +if (!sd_card_powered_up(sd)) {
> > +sd_ocr_powerup(sd);
> > +}
> > return 0;
> > }
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PULL 09/12] include/hw/ppc: Split pnv_chip.h off pnv.h
On 23/1/23 11:07, Markus Armbruster wrote: Philippe Mathieu-Daudé writes: Hi Markus, On 20/1/23 08:01, Markus Armbruster wrote: PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip are defined in pnv.h. Many users of the header don't actually need them. One instance is this inclusion loop: hw/ppc/pnv_homer.h includes hw/ppc/pnv.h for typedef PnvChip, and vice versa for struct PnvHomer. Similar structs live in their own headers: PnvHomerClass and PnvHomer in pnv_homer.h, PnvLpcClass and PnvLpcController in pci_lpc.h, PnvPsiClass, PnvPsi, Pnv8Psi, Pnv9Psi, Pnv10Psi in pnv_psi.h, ... Move PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip to new pnv_chip.h, and adjust include directives. This breaks the inclusion loop mentioned above. Signed-off-by: Markus Armbruster Reviewed-by: Cédric Le Goater Reviewed-by: Daniel Henrique Barboza Message-Id: <20221222104628.659681-2-arm...@redhat.com> --- include/hw/ppc/pnv.h | 143 +--- include/hw/ppc/pnv_chip.h | 147 + hw/intc/pnv_xive.c | 1 + hw/intc/pnv_xive2.c| 1 + hw/pci-host/pnv_phb3.c | 1 + hw/pci-host/pnv_phb4_pec.c | 1 + hw/ppc/pnv.c | 3 + hw/ppc/pnv_core.c | 1 + hw/ppc/pnv_homer.c | 1 + hw/ppc/pnv_lpc.c | 1 + hw/ppc/pnv_xscom.c | 1 + 11 files changed, 160 insertions(+), 141 deletions(-) create mode 100644 include/hw/ppc/pnv_chip.h diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index 9ef7e2d0dc..ca49e4281d 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -20,158 +20,19 @@ #ifndef PPC_PNV_H #define PPC_PNV_H +#include "cpu.h" Why is "cpu.h" required here? For pnv_chip_find_cpu()? Yes: ../include/hw/ppc/pnv.h:58:1: error: unknown type name ‘PowerPCCPU’ 58 | PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir); | ^~ Isn't "target/ppc/cpu-qom.h" enough? Seems to suffice. Would you like to post a followup? Good. First I need to finish the ARM part and deal with this comment: https://lore.kernel.org/qemu-devel/325310d0-aad6-fc39-748a-80762d644...@linaro.org/ Then this change will be included in the PPC equivalent series. Thanks, Phil.
Re: [PATCH qemu v3] x86: don't let decompressed kernel image clobber setup_data
On Sun, Jan 22, 2023 at 08:21:30PM -0800, Eric Biggers wrote: > Hi Michael, > > On Tue, Jan 10, 2023 at 12:50:42PM -0500, Michael S. Tsirkin wrote: > > On Tue, Jan 10, 2023 at 04:34:49PM +0100, Jason A. Donenfeld wrote: > > > Hi Michael, > > > > > > Could you queue up this patch and mark it as a fix for 7.2.1? It is a > > > straight-up bug fix for a 7.2 regression that's now affected several > > > users. > > > > OK. In the future pls cc me if you want me to merge a patch. Thanks! > > > > > - It has two Tested-by tags on the thread. > > > - hpa, the maintainer of the kernel side of this, confirmed on one of > > > the various tributary threads that this approach is a correct one. > > > - It doesn't introduce any new functionality. > > > > > > For your convenience, you can grab this out of lore here: > > > > > > https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/ > > > > > > Or if you want to yolo it: > > > > > > curl > > > https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/raw > > > | git am -s > > > > > > It's now sat silent on the mailing list for a while. So let's please get > > > this committed and backported so that the bug reports stop coming in. > > > > > This patch still isn't on QEMU's master branch. What happened to it? > > - Eric Indeed though I remember picking it up. Tagged again now. Thanks!
[RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation
Until now, testing using CXL has relied up always using two root ports below a host bridge, to work around a current assumption in the Linux kernel support that, in the single root port case, the implementation will use the allowed passthrough decoder implementation choice. If that choice is made all accesses are routed from the host bridge to the single root port that is present. Effectively we have a pass through decoder (it is called that in the kernel driver). This patch series implements that functionality and makes it the default See patch 2 for a discussion of why I think we can make this change without backwards compatibility issues (basically if it didn't work before who are we breaking by making it work?) Whilst this limitation has been known since the initial QEMU patch postings / kernel CXL region support, Fan Ni Ran into it recently reminding me that we should solve it. https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ Tree with a large set of patches before this at: https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 I've done some basic testing, though I did hit what appears to be a kernel race on region bring up of existing region / namespace in a 1HB 2RP 2EP test case. That is proving hard to replicate consistently but doesn't seem to have anything to do with the emulation other than perhaps we are opening up a race by responding slowly to something. Jonathan Cameron (2): hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers hw/pxb-cxl: Support passthrough HDM Decoders unless overridden hw/cxl/cxl-host.c | 31 + hw/pci-bridge/pci_expander_bridge.c | 43 + hw/pci/pcie_port.c | 38 + include/hw/cxl/cxl.h| 1 + include/hw/cxl/cxl_component.h | 1 + include/hw/pci/pci_bridge.h | 1 + include/hw/pci/pcie_port.h | 2 ++ 7 files changed, 100 insertions(+), 17 deletions(-) -- 2.37.2
[RFC PATCH 2/2] hw/pxb-cxl: Support passthrough HDM Decoders unless overridden
The CXL r3.0 specification allows for there to be no HDM decoders on CXL
Host Bridges if they have only a single root port. Instead, all accesses
directed to the host bridge (as specified in CXL Fixed Memory Windows)
are assumed to be routed to the single root port.
Linux currently assumes this implementation choice. So to simplify testing,
make QEMU emulation also default to no HDM decoders under these particular
circumstances, but provide a hdm_for_passthrough boolean option to have
HDM decoders as previously.
Technically this is breaking backwards compatibility, but given the only
known software stack used with the QEMU emulation is the Linux kernel
and this configuration did not work before this change, there are
unlikely to be any complaints that it now works. The option is retained
to allow testing of software that does allow for these HDM decoders to exist,
once someone writes it.
Reported-by: Fan Ni
Signed-off-by: Jonathan Cameron
---
hw/cxl/cxl-host.c | 31 +
hw/pci-bridge/pci_expander_bridge.c | 43 +
include/hw/cxl/cxl.h| 1 +
include/hw/cxl/cxl_component.h | 1 +
include/hw/pci/pci_bridge.h | 1 +
5 files changed, 60 insertions(+), 17 deletions(-)
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 3c1ec8732a..6e923ceeaf 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -146,21 +146,28 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow
*fw, hwaddr addr)
return NULL;
}
-hb_cstate = cxl_get_hb_cstate(hb);
-if (!hb_cstate) {
-return NULL;
-}
+if (cxl_get_hb_passthrough(hb)) {
+rp = pcie_find_port_first(hb->bus);
+if (!rp) {
+return NULL;
+}
+} else {
+hb_cstate = cxl_get_hb_cstate(hb);
+if (!hb_cstate) {
+return NULL;
+}
-cache_mem = hb_cstate->crb.cache_mem_registers;
+cache_mem = hb_cstate->crb.cache_mem_registers;
-target_found = cxl_hdm_find_target(cache_mem, addr, &target);
-if (!target_found) {
-return NULL;
-}
+target_found = cxl_hdm_find_target(cache_mem, addr, &target);
+if (!target_found) {
+return NULL;
+}
-rp = pcie_find_port_by_pn(hb->bus, target);
-if (!rp) {
-return NULL;
+rp = pcie_find_port_by_pn(hb->bus, target);
+if (!rp) {
+return NULL;
+}
}
d = pci_bridge_get_sec_bus(PCI_BRIDGE(rp))->devices[0];
diff --git a/hw/pci-bridge/pci_expander_bridge.c
b/hw/pci-bridge/pci_expander_bridge.c
index e752a21292..e2aabc12aa 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -15,6 +15,7 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_port.h"
#include "hw/qdev-properties.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci-bridge/pci_expander_bridge.h"
@@ -79,6 +80,13 @@ CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb)
return &host->cxl_cstate;
}
+bool cxl_get_hb_passthrough(PCIHostState *hb)
+{
+CXLHost *host = PXB_CXL_HOST(hb);
+
+return host->passthrough;
+}
+
static int pxb_bus_num(PCIBus *bus)
{
PXBDev *pxb = convert_to_pxb(bus->parent_dev);
@@ -289,15 +297,31 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin)
return pin - PCI_SLOT(pxb->devfn);
}
-static void pxb_dev_reset(DeviceState *dev)
+static void pxb_cxl_dev_reset(DeviceState *dev)
{
CXLHost *cxl = PXB_CXL_DEV(dev)->cxl.cxl_host_bridge;
CXLComponentState *cxl_cstate = &cxl->cxl_cstate;
+PCIHostState *hb = PCI_HOST_BRIDGE(cxl);
uint32_t *reg_state = cxl_cstate->crb.cache_mem_registers;
uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
+int dsp_count = 0;
cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
-ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8);
+/*
+ * The CXL specification allows for host bridges with no HDM decoders
+ * if they only have a single root port.
+ */
+if (!PXB_DEV(dev)->hdm_for_passthrough) {
+dsp_count = pcie_count_ds_ports(hb->bus);
+}
+/* Initial reset will have 0 dsp so wait until > 0 */
+if (dsp_count == 1) {
+cxl->passthrough = true;
+/* Set Capability ID in header to NONE */
+ARRAY_FIELD_DP32(reg_state, CXL_HDM_CAPABILITY_HEADER, ID, 0);
+} else {
+ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT,
8);
+}
}
static gint pxb_compare(gconstpointer a, gconstpointer b)
@@ -481,9 +505,18 @@ static void pxb_cxl_dev_realize(PCIDevice *dev, Error
**errp)
}
pxb_dev_realize_common(dev, CXL, errp);
-pxb_dev_reset(DEVICE(dev));
+pxb_cxl_dev_reset(DEVICE(dev));
}
+static Property pxb_cxl_dev_properties[] = {
+/* Note: 0 is not a legal PXB bus number. */
+DEFINE_
[RFC PATCH 1/2] hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers
These two helpers enable host bridges to operate differently depending on
the number of downstream ports, in particular if there is only a single
port.
Useful for CXL where HDM address decoders are allowed to be implicit in
the host bridge if there is only a single root port.
Signed-off-by: Jonathan Cameron
---
hw/pci/pcie_port.c | 38 ++
include/hw/pci/pcie_port.h | 2 ++
2 files changed, 40 insertions(+)
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index 687e4e763a..b709777e0c 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -161,6 +161,44 @@ PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn)
return NULL;
}
+/* Find first port in devfn number */
+PCIDevice *pcie_find_port_first(PCIBus *bus)
+{
+int devfn;
+
+for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
+PCIDevice *d = bus->devices[devfn];
+
+if (!d || !pci_is_express(d) || !d->exp.exp_cap) {
+continue;
+}
+
+if (object_dynamic_cast(OBJECT(d), TYPE_PCIE_PORT)) {
+return d;
+}
+}
+
+return NULL;
+}
+
+int pcie_count_ds_ports(PCIBus *bus)
+{
+int dsp_count = 0;
+int devfn;
+
+for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
+PCIDevice *d = bus->devices[devfn];
+
+if (!d || !pci_is_express(d) || !d->exp.exp_cap) {
+continue;
+}
+if (object_dynamic_cast(OBJECT(d), TYPE_PCIE_PORT)) {
+dsp_count++;
+}
+}
+return dsp_count;
+}
+
static const TypeInfo pcie_port_type_info = {
.name = TYPE_PCIE_PORT,
.parent = TYPE_PCI_BRIDGE,
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index fd484afb30..2cbad72555 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -41,6 +41,8 @@ struct PCIEPort {
void pcie_port_init_reg(PCIDevice *d);
PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn);
+PCIDevice *pcie_find_port_first(PCIBus *bus);
+int pcie_count_ds_ports(PCIBus *bus);
#define TYPE_PCIE_SLOT "pcie-slot"
OBJECT_DECLARE_SIMPLE_TYPE(PCIESlot, PCIE_SLOT)
--
2.37.2
Re: [PATCH] qga/linux: add usb support to guest-get-fsinfo
Reviewed-by: Konstantin Kostiuk
On Sun, Jan 22, 2023 at 5:33 PM Kfir Manor wrote:
> ---
> qga/commands-posix.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index ebd33a643c..aab9d3bd50 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -880,7 +880,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const
> *syspath,
> g_str_equal(driver, "sym53c8xx") ||
> g_str_equal(driver, "virtio-pci") ||
> g_str_equal(driver, "ahci") ||
> - g_str_equal(driver, "nvme"))) {
> + g_str_equal(driver, "nvme") ||
> + g_str_equal(driver, "xhci_hcd") ||
> + g_str_equal(driver, "ehci-pci"))) {
> break;
> }
>
> @@ -977,6 +979,8 @@ static bool build_guest_fsinfo_for_pci_dev(char const
> *syspath,
> }
> } else if (strcmp(driver, "nvme") == 0) {
> disk->bus_type = GUEST_DISK_BUS_TYPE_NVME;
> +} else if (strcmp(driver, "ehci-pci") == 0 || strcmp(driver,
> "xhci_hcd") == 0) {
> +disk->bus_type = GUEST_DISK_BUS_TYPE_USB;
> } else {
> g_debug("unknown driver '%s' (sysfs path '%s')", driver, syspath);
> goto cleanup;
> --
> 2.38.1
>
>
Re: [PATCH] qga/linux: add usb support to guest-get-fsinfo
Hi Kfir,
You missed adding the Signed-off-by line to the patch.
Please resend.
Best Regards,
Konstantin Kostiuk.
On Sun, Jan 22, 2023 at 5:33 PM Kfir Manor wrote:
> ---
> qga/commands-posix.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index ebd33a643c..aab9d3bd50 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -880,7 +880,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const
> *syspath,
> g_str_equal(driver, "sym53c8xx") ||
> g_str_equal(driver, "virtio-pci") ||
> g_str_equal(driver, "ahci") ||
> - g_str_equal(driver, "nvme"))) {
> + g_str_equal(driver, "nvme") ||
> + g_str_equal(driver, "xhci_hcd") ||
> + g_str_equal(driver, "ehci-pci"))) {
> break;
> }
>
> @@ -977,6 +979,8 @@ static bool build_guest_fsinfo_for_pci_dev(char const
> *syspath,
> }
> } else if (strcmp(driver, "nvme") == 0) {
> disk->bus_type = GUEST_DISK_BUS_TYPE_NVME;
> +} else if (strcmp(driver, "ehci-pci") == 0 || strcmp(driver,
> "xhci_hcd") == 0) {
> +disk->bus_type = GUEST_DISK_BUS_TYPE_USB;
> } else {
> g_debug("unknown driver '%s' (sysfs path '%s')", driver, syspath);
> goto cleanup;
> --
> 2.38.1
>
>
Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
On 23/1/23 12:11, Daniel P. Berrangé wrote:
On Mon, Jan 23, 2023 at 10:20:29AM +0100, Philippe Mathieu-Daudé wrote:
On 23/1/23 09:39, Thomas Huth wrote:
From: Daniel P. Berrangé
Way back in QEMU 4.0, the -audiodev command line option was introduced
for configuring audio backends. This CLI option does not use QemuOpts
so it is not visible for introspection in 'query-command-line-options',
instead using the QAPI Audiodev type. Unfortunately there is also no
QMP command that uses the Audiodev type, so it is not introspectable
with 'query-qmp-schema' either.
This introduces a 'query-audiodev' command that simply reflects back
the list of configured -audiodev command line options. This alone is
maybe not very useful by itself, but it makes Audiodev introspectable
via 'query-qmp-schema', so that libvirt (and other upper layer tools)
can discover the available audiodevs.
Signed-off-by: Daniel P. Berrangé
[thuth: Update for upcoming QEMU v8.0, and use QAPI_LIST_PREPEND]
Signed-off-by: Thomas Huth
---
qapi/audio.json | 13 +
audio/audio.c | 12
2 files changed, 25 insertions(+)
diff --git a/qapi/audio.json b/qapi/audio.json
index 1e0a24bdfc..c7aafa2763 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -443,3 +443,16 @@
'sndio': 'AudiodevSndioOptions',
'spice': 'AudiodevGenericOptions',
'wav': 'AudiodevWavOptions' } }
+
+##
+# @query-audiodevs:
+#
+# Returns information about audiodev configuration
Maybe clearer as 'audio backends'?
So similarly, wouldn't be clearer to name this command
'query-audio-backends'? Otherwise we need to go read QEMU
source to understand what is 'audiodevs'.
The command line parameter is called '-audiodev' and this
query-audiodevs command reports the same data, so that
looks easy enough to understand IMHO.
+#
+# Returns: array of @Audiodev
+#
+# Since: 8.0
+#
+##
+{ 'command': 'query-audiodevs',
+ 'returns': ['Audiodev'] }
diff --git a/audio/audio.c b/audio/audio.c
index d849a94a81..6f270c07b7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -28,8 +28,10 @@
#include "monitor/monitor.h"
#include "qemu/timer.h"
#include "qapi/error.h"
+#include "qapi/clone-visitor.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qapi-visit-audio.h"
+#include "qapi/qapi-commands-audio.h"
#include "qemu/cutils.h"
#include "qemu/module.h"
#include "qemu/help_option.h"
@@ -2311,3 +2313,13 @@ size_t audio_rate_get_bytes(RateCtl *rate, struct
audio_pcm_info *info,
return bytes;
}
+
+AudiodevList *qmp_query_audiodevs(Error **errp)
+{
+AudiodevList *ret = NULL;
+AudiodevListEntry *e;
+QSIMPLEQ_FOREACH(e, &audiodevs, next) {
I am a bit confused here, isn't &audiodevs containing what the user provided
from CLI? How is that useful to libvirt? Maybe the corner case
of a user hand-modifying the QEMU launch arguments from a XML config?
Wouldn't a list of linked in AudiodevDriver be more useful to libvirt
so it could pick the best available backend to start a VM?
On the libvirt side we're never going to need to actually call the
query-audiodevs commands. The mere existance of the command, means
that the QMP schema now exposes information about what audio backends
have been compiled into the binary. This is the same trick we've used
for other aspects of QMP. IOW we don't need a separate command just
for the purpose of listing AudiodevDrivers.
I understand having "what audio backends have been compiled into the
binary" is useful, but I am missing how you get that from &audiodevs.
AFAICT &audiodevs is for the CLI parsed backends, not all the backends
linked within a binary. I probably need sugar / coffee and will revisit
after lunch.
The idea of a query-audiodevs command is useful in general. When we
later gain support for hotplug/unplug of audio, the set of audiodevs
will no longer be guaranteed to match the original CLI args.
With regards,
Daniel
Re: [PATCH qemu v3] x86: don't let decompressed kernel image clobber setup_data
On Mon, Jan 23, 2023 at 6:12 AM Michael S. Tsirkin wrote: > > On Sun, Jan 22, 2023 at 08:21:30PM -0800, Eric Biggers wrote: > > Hi Michael, > > > > On Tue, Jan 10, 2023 at 12:50:42PM -0500, Michael S. Tsirkin wrote: > > > On Tue, Jan 10, 2023 at 04:34:49PM +0100, Jason A. Donenfeld wrote: > > > > Hi Michael, > > > > > > > > Could you queue up this patch and mark it as a fix for 7.2.1? It is a > > > > straight-up bug fix for a 7.2 regression that's now affected several > > > > users. > > > > > > OK. In the future pls cc me if you want me to merge a patch. Thanks! > > > > > > > - It has two Tested-by tags on the thread. > > > > - hpa, the maintainer of the kernel side of this, confirmed on one of > > > > the various tributary threads that this approach is a correct one. > > > > - It doesn't introduce any new functionality. > > > > > > > > For your convenience, you can grab this out of lore here: > > > > > > > > https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/ > > > > > > > > Or if you want to yolo it: > > > > > > > > curl > > > > https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/raw > > > > | git am -s > > > > > > > > It's now sat silent on the mailing list for a while. So let's please get > > > > this committed and backported so that the bug reports stop coming in. > > > > > > > > This patch still isn't on QEMU's master branch. What happened to it? > > > > - Eric > > Indeed though I remember picking it up. Tagged again now. Thanks! Thanks. What branch is this in? I didn't see it on: https://gitlab.com/mstredhat/qemu/-/branches/active https://github.com/mstsirkin/qemu/branches
[PATCH] block/blkio: Fix inclusion of required headers
After recent header file inclusion rework the build fails when the blkio module is enabled: ../block/blkio.c: In function ‘blkio_detach_aio_context’: ../block/blkio.c:321:24: error: implicit declaration of function ‘bdrv_get_aio_context’; did you mean ‘qemu_get_aio_context’? [-Werror=implicit-function-declaration] 321 | aio_set_fd_handler(bdrv_get_aio_context(bs), |^~~~ |qemu_get_aio_context ../block/blkio.c:321:24: error: nested extern declaration of ‘bdrv_get_aio_context’ [-Werror=nested-externs] ../block/blkio.c:321:24: error: passing argument 1 of ‘aio_set_fd_handler’ makes pointer from integer without a cast [-Werror=int-conversion] 321 | aio_set_fd_handler(bdrv_get_aio_context(bs), |^~~~ || |int In file included from /home/pipo/git/qemu.git/include/qemu/job.h:33, from /home/pipo/git/qemu.git/include/block/blockjob.h:30, from /home/pipo/git/qemu.git/include/block/block_int-global-state.h:28, from /home/pipo/git/qemu.git/include/block/block_int.h:27, from ../block/blkio.c:13: /home/pipo/git/qemu.git/include/block/aio.h:476:37: note: expected ‘AioContext *’ but argument is of type ‘int’ 476 | void aio_set_fd_handler(AioContext *ctx, | ^~~ ../block/blkio.c: In function ‘blkio_file_open’: ../block/blkio.c:821:34: error: passing argument 2 of ‘blkio_attach_aio_context’ makes pointer from integer without a cast [-Werror=int-conversion] 821 | blkio_attach_aio_context(bs, bdrv_get_aio_context(bs)); | ^~~~ | | | int Fix it by including 'block/block-io.h' which contains the required declarations. Fixes: e2c1c34f139f49ef909bb4322607fb8b39002312 Signed-off-by: Peter Krempa --- block/blkio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/blkio.c b/block/blkio.c index 5eae3adfaf..6ad86b23d1 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -19,6 +19,8 @@ #include "qemu/module.h" #include "exec/memory.h" /* for ram_block_discard_disable() */ +#include "block/block-io.h" + /* * Keep the QEMU BlockDriver names identical to the libblkio driver names. * Using macros instead of typing out the string literals avoids typos. -- 2.38.1
Re: [PATCH 2/2] target/arm: Look up ARMCPRegInfo at runtime
On Fri, 6 Jan 2023 at 19:45, Richard Henderson wrote: > > Do not encode the pointer as a constant in the opcode stream. > This pointer is specific to the cpu that first generated the > translation, which runs into problems with both hot-pluggable > cpus and user-only threads, as cpus are removed. > > Perform the lookup in either helper_access_check_cp_reg, > or a new helper_lookup_cp_reg. As well as the use-after-free, this is also a correctness bug, isn't it? If we hardwire in the cpregs pointer for CPU 0 into the TB, and then CPU 1 with a slightly different config executes the TB, it will get the cpregs of CPU 0, not its own, so it might see a register it should not or vice-versa. So I think we need this patch anyway, even if we're going to try to do something to improve sharing of cpreg hashtables across CPUs. Reviewed-by: Peter Maydell thanks -- PMM
[PATCH] qga/linux: add usb support to guest-get-fsinfo
Signed-off-by: Kfir Manor
---
qga/commands-posix.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ebd33a643c..aab9d3bd50 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -880,7 +880,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const
*syspath,
g_str_equal(driver, "sym53c8xx") ||
g_str_equal(driver, "virtio-pci") ||
g_str_equal(driver, "ahci") ||
- g_str_equal(driver, "nvme"))) {
+ g_str_equal(driver, "nvme") ||
+ g_str_equal(driver, "xhci_hcd") ||
+ g_str_equal(driver, "ehci-pci"))) {
break;
}
@@ -977,6 +979,8 @@ static bool build_guest_fsinfo_for_pci_dev(char const
*syspath,
}
} else if (strcmp(driver, "nvme") == 0) {
disk->bus_type = GUEST_DISK_BUS_TYPE_NVME;
+} else if (strcmp(driver, "ehci-pci") == 0 || strcmp(driver, "xhci_hcd")
== 0) {
+disk->bus_type = GUEST_DISK_BUS_TYPE_USB;
} else {
g_debug("unknown driver '%s' (sysfs path '%s')", driver, syspath);
goto cleanup;
--
2.38.1
Re: [PATCH 0/2] target/arm: Look up ARMCPRegInfo at runtime
On Fri, 6 Jan 2023 at 19:45, Richard Henderson
wrote:
>
> Here's a short-to-medium term alternative to moving all of the ARMCPU
> cp_regs hash table to the ARMCPUClass, so that we're no longer leaving
> dangling pointers to freed objects encoded in the compiled
> TranslationBlocks. (I still think we ought to do less work at
> object_{init,realize}, but that may be a much longer term project.)
>
> Instead of giving the helper a direct pointer, pass the cpreg hash key,
> which will be constant across cpus. Perform this lookup in the existing
> helper_access_check_cp_reg (which had a return value going spare), or a
> new helper_lookup_cp_reg. The other cp_regs functions are unchanged,
> because they still get a pointer.
>
> This ought to be enough to re-instate Alex's linux-user patch
> to free the cpu object after thread termination.
Applied to target-arm.next, thanks.
-- PMM
Re: [PATCH v6 5/5] riscv: Introduce satp mode hw capabilities
On Mon, Jan 23, 2023 at 12:15:08PM +0100, Alexandre Ghiti wrote:
> On Mon, Jan 23, 2023 at 11:51 AM Andrew Jones wrote:
> >
> > On Mon, Jan 23, 2023 at 10:03:24AM +0100, Alexandre Ghiti wrote:
> > > Currently, the max satp mode is set with the only constraint that it must
> > > be
> > > implemented in qemu, i.e. set in valid_vm_1_10_[32|64].
> > >
> > > But we actually need to add another level of constraint: what the hw is
> > > actually capable of, because currently, a linux booting on a sifive-u54
> > > boots in sv57 mode which is incompatible with the cpu's sv39 max
> > > capability.
> > >
> > > So add a new bitmap to RISCVSATPMap which contains this capability and
> > > initialize it in every XXX_cpu_init.
> > >
> > > Finally, we have the following chain of constraints:
> > >
> > > Qemu capability > HW capability > User choice > Software capability
> >
> > ^ What software is this?
> > I'd think the user's choice would always be last.
> >
> > >
> > > Signed-off-by: Alexandre Ghiti
> > > ---
> > > target/riscv/cpu.c | 78 +++---
> > > target/riscv/cpu.h | 8 +++--
> > > 2 files changed, 59 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index e409e6ab64..19a37fee2b 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -292,24 +292,39 @@ const char *satp_mode_str(uint8_t satp_mode, bool
> > > is_32_bit)
> > > g_assert_not_reached();
> > > }
> > >
> > > -/* Sets the satp mode to the max supported */
> > > -static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
> > > +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> > > +const char *satp_mode_str,
> > > +bool is_32_bit)
> > > {
> > > -if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> > > -cpu->cfg.satp_mode.map |=
> > > -(1 << satp_mode_from_str(is_32_bit ? "sv32" :
> > > "sv57"));
> > > -} else {
> > > -cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> > > +uint8_t satp_mode = satp_mode_from_str(satp_mode_str);
> > > +const bool *valid_vm = is_32_bit ? valid_vm_1_10_32 :
> > > valid_vm_1_10_64;
> > > +
> > > +for (int i = 0; i <= satp_mode; ++i) {
> > > +if (valid_vm[i]) {
> > > +cpu->cfg.satp_mode.supported |= (1 << i);
> >
> > I don't think we need a new 'supported' bitmap, I think each board that
> > needs to further constrain va-bits from what QEMU supports should just set
> > valid_vm_1_10_32/64. I.e. drop const from the arrays and add an init
> > function something like
>
> This was my first idea too, but those arrays are global and I have to
> admit that I thought it was possible to emulate a cpu with different
> cores. Anyway, isn't it a bit weird to store this into some global
> array whereas it is intimately linked to the CPU? To me, it makes
> sense to keep those variables as a way to know what qemu is able to
> emulate and have a CPU specific map like in this patch for the hw
> capabilities. Does it make sense to you?
Ah, yes, to support heterogeneous configs it's best to keep this
information per-cpu. I'll take another look at the patch.
>
> >
> > #define QEMU_SATP_MODE_MAX VM_1_10_SV64
> >
> > void riscv_cpu_set_satp_mode_max(RISCVCPU *cpu, uint8_t satp_mode_max)
> > {
> > bool is_32_bit = cpu->env.misa_mxl == MXL_RV32;
> > bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
> >
> > g_assert(satp_mode_max <= QEMU_SATP_MODE_MAX);
> > g_assert(!is_32_bit || satp_mode_max < 2);
> >
> > memset(valid_vm, 0, sizeof(*valid_vm));
> >
> > for (int i = 0; i <= satp_mode_max; i++) {
> > valid_vm[i] = true;
> > }
> > }
> >
> > The valid_vm[] checks already in finalize should then manage the
> > validation needed to constrain boards. Only boards that care about
> > this need to call this function, otherwise they'll get the default.
> >
> > Also, this patch should come before the patch that changes the default
> > for all boards to sv57 in order to avoid breaking bisection.
>
> As I explained earlier, I didn't change the default to sv57! Just
> fixed what was passed via the device tree, which should not be used
> anyway :)
OK, I keep misunderstanding how we're "fixing" something which is
is wrong, but apparently doesn't exhibit any symptoms. So, assuming
it doesn't matter, then I guess it can come anywhere in the series.
Thanks,
drew
Re: [PATCH] tests/qtest: Plug memory leaks in qtest_get_machines
Thomas Huth writes:
> On 20/01/2023 20.44, Fabiano Rosas wrote:
>> These leaks can be avoided:
>>
>> 759 bytes in 61 blocks are still reachable in loss record 56 of 60
>> at 0x4034744: malloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> by 0x4A88518: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.5)
>> by 0x4AA313E: g_strdup (in /usr/lib64/libglib-2.0.so.0.7000.5)
>> by 0x12083E: qtest_get_machines (libqtest.c:1323)
>> by 0x12098C: qtest_cb_for_every_machine (libqtest.c:1348)
>> by 0x11556C: main (test-hmp.c:160)
>>
>> 992 bytes in 1 blocks are still reachable in loss record 57 of 60
>> at 0x4034744: malloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> by 0x4A88518: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.5)
>> by 0x120725: qtest_get_machines (libqtest.c:1313)
>> by 0x12098C: qtest_cb_for_every_machine (libqtest.c:1348)
>> by 0x11556C: main (test-hmp.c:160)
>>
>> Signed-off-by: Fabiano Rosas
>> ---
>> tests/qtest/libqtest.c | 14 ++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index 6b2216cb20..65abac5029 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -1285,6 +1285,18 @@ struct MachInfo {
>> char *alias;
>> };
>>
>> +static void qtest_free_machine_info(gpointer data)
>> +{
>> +struct MachInfo *machines = data;
>> +int i;
>> +
>> +for (i = 0; machines[i].name != NULL; i++) {
>> +g_free((void *)machines[i].name); > +g_free((void
>> *)machines[i].alias);
>
> I'd suggest setting .name and .alias to NULL after freeing them, to avoid
> that danling pointers are left behind.
>
>> +}
>> +g_free(machines);
>> +}
>> +
>> /*
>>* Returns an array with pointers to the available machine names.
>>* The terminating entry has the name set to NULL.
>> @@ -1336,6 +1348,8 @@ static struct MachInfo *qtest_get_machines(void)
>> qobject_unref(response);
>>
>> memset(&machines[idx], 0, sizeof(struct MachInfo)); /* Terminating
>> entry */
>> +g_test_queue_destroy(qtest_free_machine_info, machines);
>
> So this frees the machines structure...
>
>> return machines;
>
> ... but here it gets returned, too? ... that looks wrong. Did you maybe
> rather want to free it at the end of qtest_cb_for_every_machine() and
> qtest_has_machine ?
g_test_queue_destroy will only call qtest_free_machine_info during the
test teardown phase:
#0 qtest_free_machine_info (data=0x55677870) at
../tests/qtest/libqtest.c:1289
#1 0x77b1d9d1 in ?? () from /usr/lib64/libglib-2.0.so.0
#2 0x77b1d8b3 in ?? () from /usr/lib64/libglib-2.0.so.0
#3 0x77b1d8b3 in ?? () from /usr/lib64/libglib-2.0.so.0
#4 0x77b1de82 in g_test_run_suite () from /usr/lib64/libglib-2.0.so.0
#5 0x77b1deab in g_test_run () from /usr/lib64/libglib-2.0.so.0
#6 0x55561221 in main (argc=, argv=) at ../tests/qtest/qom-test.c:12
As long as 'machines' is static and not being exposed to the tests, I
think this should be fine.
Re: [PATCH v0 0/4] backends/hostmem: add an ability to specify prealloc timeout
On 1/23/23 11:57 AM, David Hildenbrand wrote: On 20.01.23 14:47, Daniil Tatianin wrote: This series introduces new qemu_prealloc_mem_with_timeout() api, which allows limiting the maximum amount of time to be spent on memory preallocation. It also adds prealloc statistics collection that is exposed via an optional timeout handler. This new api is then utilized by hostmem for guest RAM preallocation controlled via new object properties called 'prealloc-timeout' and 'prealloc-timeout-fatal'. This is useful for limiting VM startup time on systems with unpredictable page allocation delays due to memory fragmentation or the backing storage. The timeout can be configured to either simply emit a warning and continue VM startup without having preallocated the entire guest RAM or just abort startup entirely if that is not acceptable for a specific use case. The major use case for preallocation is memory resources that cannot be overcommitted (hugetlb, file blocks, ...), to avoid running out of such resources later, while the guest is already running, and crashing it. Wouldn't you say that preallocating memory for the sake of speeding up guest kernel startup & runtime is a valid use case of prealloc? This way we can avoid expensive (for a multitude of reasons) page faults that will otherwise slow down the guest significantly at runtime and affect the user experience. Allocating only a fraction "because it takes too long" looks quite useless in that (main use-case) context. We shouldn't encourage QEMU users to play with fire in such a way. IOW, there should be no way around "prealloc-timeout-fatal". Either preallocation succeeded and the guest can run, or it failed, and the guest can't run. Here we basically accept the fact that e.g with fragmented memory the kernel might take a while in a page fault handler especially for hugetlb because of page compaction that has to run for every fault. This way we can prefault at least some number of pages and let the guest fault the rest on demand later on during runtime even if it's slow and would cause a noticeable lag. ... but then, management tools can simply start QEMU with "-S", start an own timer, and zap QEMU if it didn't manage to come up in time, and simply start a new QEMU instance without preallocation enabled. The "good" thing about that approach is that it will also cover any implicit memory preallocation, like using mlock() or VFIO, that don't run in ordinary per-hostmem preallocation context. If setting QEMU up takes to long, you might want to try on a different hypervisor in your cluster instead. This approach definitely works too but again it assumes that we always want 'prealloc-timeout-fatal' to be on, which is, for the most part only the case for working around issues that might be caused by overcommit. I don't immediately see why we want to make our preallcoation+hostmem implementation in QEMU more complicated for such a use case.
[PATCH v2] qga/linux: add usb support to guest-get-fsinfo
Signed-off-by: Kfir Manor
---
qga/commands-posix.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ebd33a643c..aab9d3bd50 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -880,7 +880,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const
*syspath,
g_str_equal(driver, "sym53c8xx") ||
g_str_equal(driver, "virtio-pci") ||
g_str_equal(driver, "ahci") ||
- g_str_equal(driver, "nvme"))) {
+ g_str_equal(driver, "nvme") ||
+ g_str_equal(driver, "xhci_hcd") ||
+ g_str_equal(driver, "ehci-pci"))) {
break;
}
@@ -977,6 +979,8 @@ static bool build_guest_fsinfo_for_pci_dev(char const
*syspath,
}
} else if (strcmp(driver, "nvme") == 0) {
disk->bus_type = GUEST_DISK_BUS_TYPE_NVME;
+} else if (strcmp(driver, "ehci-pci") == 0 || strcmp(driver, "xhci_hcd")
== 0) {
+disk->bus_type = GUEST_DISK_BUS_TYPE_USB;
} else {
g_debug("unknown driver '%s' (sysfs path '%s')", driver, syspath);
goto cleanup;
--
2.38.1
[PULL 21/26] target/arm: Fix in_debug path in S1_ptw_translate
From: Richard Henderson
During the conversion, the test against get_phys_addr_lpae got inverted,
meaning that successful translations went to the 'failed' label.
Cc: qemu-sta...@nongnu.org
Fixes: f3639a64f60 ("target/arm: Use softmmu tlbs for page table walking")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1417
Signed-off-by: Richard Henderson
Message-id: 20230114054605.2977022-1-richard.hender...@linaro.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
target/arm/ptw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 4bda0590c7c..57f3615a66d 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -238,8 +238,8 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate
*ptw,
};
GetPhysAddrResult s2 = { };
-if (!get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
-false, &s2, fi)) {
+if (get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
+ false, &s2, fi)) {
goto fail;
}
ptw->out_phys = s2.f.phys_addr;
--
2.34.1
[PULL 00/26] target-arm queue
The following changes since commit 65cc5ccf06a74c98de73ec683d9a543baa302a12:
Merge tag 'pull-riscv-to-apply-20230120' of
https://github.com/alistair23/qemu into staging (2023-01-20 16:17:56 +)
are available in the Git repository at:
https://git.linaro.org/people/pmaydell/qemu-arm.git
tags/pull-target-arm-20230123
for you to fetch changes up to 3b07a936d3bfe97b07ddffcfbb532985a88033dd:
target/arm: Look up ARMCPRegInfo at runtime (2023-01-23 13:32:38 +)
target-arm queue:
* Widen cnthctl_el2 to uint64_t
* Unify checking for M Main Extension in MRS/MSR
* bitbang_i2c, versatile_i2c: code cleanups
* SME: refactor SME SM/ZA handling
* Fix physical address resolution for MTE
* Fix in_debug path in S1_ptw_translate
* Don't set EXC_RETURN.ES if Security Extension not present
* Implement DBGCLAIM registers
* Provide stubs for more external debug registers
* Look up ARMCPRegInfo at runtime, not translate time
David Reiss (1):
target/arm: Unify checking for M Main Extension in MRS/MSR
Evgeny Iakovlev (2):
target/arm: implement DBGCLAIM registers
target/arm: provide stubs for more external debug registers
Peter Maydell (1):
target/arm: Don't set EXC_RETURN.ES if Security Extension not present
Philippe Mathieu-Daudé (10):
hw/i2c/bitbang_i2c: Define TYPE_GPIO_I2C in public header
hw/i2c/bitbang_i2c: Remove unused dummy MemoryRegion
hw/i2c/bitbang_i2c: Change state calling bitbang_i2c_set_state() helper
hw/i2c/bitbang_i2c: Trace state changes
hw/i2c/bitbang_i2c: Convert DPRINTF() to trace events
hw/i2c/versatile_i2c: Drop useless casts from void * to pointer
hw/i2c/versatile_i2c: Replace VersatileI2CState -> ArmSbconI2CState
hw/i2c/versatile_i2c: Replace TYPE_VERSATILE_I2C -> TYPE_ARM_SBCON_I2C
hw/i2c/versatile_i2c: Use ARM_SBCON_I2C() macro
hw/i2c/versatile_i2c: Rename versatile_i2c -> arm_sbcon_i2c
Richard Henderson (12):
target/arm: Widen cnthctl_el2 to uint64_t
target/arm/sme: Reorg SME access handling in handle_msr_i()
target/arm/sme: Rebuild hflags in set_pstate() helpers
target/arm/sme: Introduce aarch64_set_svcr()
target/arm/sme: Reset SVE state in aarch64_set_svcr()
target/arm/sme: Reset ZA state in aarch64_set_svcr()
target/arm/sme: Rebuild hflags in aarch64_set_svcr()
target/arm/sme: Unify set_pstate() SM/ZA helpers as set_svcr()
target/arm: Fix physical address resolution for MTE
target/arm: Fix in_debug path in S1_ptw_translate
target/arm: Reorg do_coproc_insn
target/arm: Look up ARMCPRegInfo at runtime
MAINTAINERS | 1 +
include/hw/i2c/arm_sbcon_i2c.h | 6 +-
include/hw/i2c/bitbang_i2c.h| 2 +
target/arm/cpu.h| 5 +-
target/arm/helper-sme.h | 3 +-
target/arm/helper.h | 11 +-
target/arm/translate.h | 7 +
hw/arm/musicpal.c | 3 +-
hw/arm/realview.c | 2 +-
hw/arm/versatilepb.c| 2 +-
hw/arm/vexpress.c | 2 +-
hw/i2c/{versatile_i2c.c => arm_sbcon_i2c.c} | 39 ++-
hw/i2c/bitbang_i2c.c| 80 --
linux-user/aarch64/cpu_loop.c | 11 +-
linux-user/aarch64/signal.c | 13 +-
target/arm/debug_helper.c | 54
target/arm/helper.c | 41 ++-
target/arm/m_helper.c | 24 +-
target/arm/mte_helper.c | 2 +-
target/arm/op_helper.c | 27 +-
target/arm/ptw.c| 4 +-
target/arm/sme_helper.c | 37 +--
target/arm/translate-a64.c | 68 +++--
target/arm/translate.c | 430 +++-
hw/arm/Kconfig | 4 +-
hw/i2c/Kconfig | 2 +-
hw/i2c/meson.build | 2 +-
hw/i2c/trace-events | 7 +
28 files changed, 506 insertions(+), 383 deletions(-)
rename hw/i2c/{versatile_i2c.c => arm_sbcon_i2c.c} (70%)
Re: [PATCH] vdpa: fix VHOST_BACKEND_F_IOTLB_ASID flag check
On 1/17/23 11:53, Eugenio Pérez wrote:
VHOST_BACKEND_F_IOTLB_ASID is the feature bit, not the bitmask. Since
the device under test also provided VHOST_BACKEND_F_IOTLB_MSG_V2 and
VHOST_BACKEND_F_IOTLB_BATCH, this went unnoticed.
Fixes: c1a1008685 ("vdpa: always start CVQ in SVQ mode if possible")
Signed-off-by: Eugenio Pérez
Acked-by: Jason Wang
---
Originally on SUSPEND series, but it is a fix that it is worth to send
and apply individually:
https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg02574.html
---
net/vhost-vdpa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 1a13a34d35..de5ed8ff22 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -384,7 +384,7 @@ static int vhost_vdpa_net_cvq_start(NetClientState *nc)
g_strerror(errno), errno);
return -1;
}
-if (!(backend_features & VHOST_BACKEND_F_IOTLB_ASID) ||
+if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID)) ||
!vhost_vdpa_net_valid_svq_features(v->dev->features, NULL)) {
return 0;
}
--
2.31.1
Reviewed-by: Laurent Vivier
[PULL 16/26] target/arm/sme: Reset SVE state in aarch64_set_svcr()
From: Richard Henderson
Move arm_reset_sve_state() calls to aarch64_set_svcr().
Signed-off-by: Richard Henderson
Reviewed-by: Fabiano Rosas
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20230112102436.1913-5-phi...@linaro.org
Message-Id: <20230112004322.161330-1-richard.hender...@linaro.org>
[PMD: Split patch in multiple tiny steps]
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Maydell
---
target/arm/cpu.h | 1 -
linux-user/aarch64/cpu_loop.c | 1 -
linux-user/aarch64/signal.c | 8 +---
target/arm/helper.c | 13 +
target/arm/sme_helper.c | 10 --
5 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ef61849eb1d..f3ddc3b7793 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1124,7 +1124,6 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
void aarch64_sve_change_el(CPUARMState *env, int old_el,
int new_el, bool el0_a64);
void aarch64_set_svcr(CPUARMState *env, uint64_t new, uint64_t mask);
-void arm_reset_sve_state(CPUARMState *env);
/*
* SVE registers are encoded in KVM's memory in an endianness-invariant format.
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index d53742e10bb..5e93d27d8f6 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -96,7 +96,6 @@ void cpu_loop(CPUARMState *env)
aarch64_set_svcr(env, 0, R_SVCR_SM_MASK);
if (FIELD_EX64(env->svcr, SVCR, SM)) {
arm_rebuild_hflags(env);
-arm_reset_sve_state(env);
}
ret = do_syscall(env,
env->xregs[8],
diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index b6e4dcb494d..a326a6def5e 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -665,14 +665,8 @@ static void target_setup_frame(int usig, struct
target_sigaction *ka,
env->btype = 2;
}
-/*
- * Invoke the signal handler with both SM and ZA disabled.
- * When clearing SM, ResetSVEState, per SMSTOP.
- */
+/* Invoke the signal handler with both SM and ZA disabled. */
aarch64_set_svcr(env, 0, R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
-if (FIELD_EX64(env->svcr, SVCR, SM)) {
-arm_reset_sve_state(env);
-}
if (env->svcr) {
arm_rebuild_hflags(env);
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 24c069b8acf..0ac867c4119 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6725,11 +6725,24 @@ static CPAccessResult access_esm(CPUARMState *env,
const ARMCPRegInfo *ri,
return CP_ACCESS_OK;
}
+/* ResetSVEState */
+static void arm_reset_sve_state(CPUARMState *env)
+{
+memset(env->vfp.zregs, 0, sizeof(env->vfp.zregs));
+/* Recall that FFR is stored as pregs[16]. */
+memset(env->vfp.pregs, 0, sizeof(env->vfp.pregs));
+vfp_set_fpcr(env, 0x089f);
+}
+
void aarch64_set_svcr(CPUARMState *env, uint64_t new, uint64_t mask)
{
uint64_t change = (env->svcr ^ new) & mask;
env->svcr ^= change;
+
+if (change & R_SVCR_SM_MASK) {
+arm_reset_sve_state(env);
+}
}
static void svcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
diff --git a/target/arm/sme_helper.c b/target/arm/sme_helper.c
index 94dc084135d..f73bf4d2853 100644
--- a/target/arm/sme_helper.c
+++ b/target/arm/sme_helper.c
@@ -29,22 +29,12 @@
#include "vec_internal.h"
#include "sve_ldst_internal.h"
-/* ResetSVEState */
-void arm_reset_sve_state(CPUARMState *env)
-{
-memset(env->vfp.zregs, 0, sizeof(env->vfp.zregs));
-/* Recall that FFR is stored as pregs[16]. */
-memset(env->vfp.pregs, 0, sizeof(env->vfp.pregs));
-vfp_set_fpcr(env, 0x089f);
-}
-
void helper_set_pstate_sm(CPUARMState *env, uint32_t i)
{
if (i == FIELD_EX64(env->svcr, SVCR, SM)) {
return;
}
aarch64_set_svcr(env, 0, R_SVCR_SM_MASK);
-arm_reset_sve_state(env);
arm_rebuild_hflags(env);
}
--
2.34.1
[PULL 02/26] target/arm: Unify checking for M Main Extension in MRS/MSR
From: David Reiss
BASEPRI, FAULTMASK, and their _NS equivalents only exist on devices with
the Main Extension. However, the MRS instruction did not check this,
and the MSR instruction handled it inconsistently (warning BASEPRI, but
silently ignoring writes to BASEPRI_NS). Unify this behavior and always
warn when reading or writing any of these registers if the extension is
not present.
Signed-off-by: David Reiss
Message-id: 167330628518.10497.1310042578726892778...@git.sr.ht
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
target/arm/m_helper.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 033a4d92614..d87b9ecd123 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -2465,11 +2465,17 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
}
return env->v7m.primask[M_REG_NS];
case 0x91: /* BASEPRI_NS */
+if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+goto bad_reg;
+}
if (!env->v7m.secure) {
return 0;
}
return env->v7m.basepri[M_REG_NS];
case 0x93: /* FAULTMASK_NS */
+if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+goto bad_reg;
+}
if (!env->v7m.secure) {
return 0;
}
@@ -2515,8 +2521,14 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
return env->v7m.primask[env->v7m.secure];
case 17: /* BASEPRI */
case 18: /* BASEPRI_MAX */
+if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+goto bad_reg;
+}
return env->v7m.basepri[env->v7m.secure];
case 19: /* FAULTMASK */
+if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+goto bad_reg;
+}
return env->v7m.faultmask[env->v7m.secure];
default:
bad_reg:
@@ -2581,13 +2593,19 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t
maskreg, uint32_t val)
env->v7m.primask[M_REG_NS] = val & 1;
return;
case 0x91: /* BASEPRI_NS */
-if (!env->v7m.secure || !arm_feature(env, ARM_FEATURE_M_MAIN)) {
+if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+goto bad_reg;
+}
+if (!env->v7m.secure) {
return;
}
env->v7m.basepri[M_REG_NS] = val & 0xff;
return;
case 0x93: /* FAULTMASK_NS */
-if (!env->v7m.secure || !arm_feature(env, ARM_FEATURE_M_MAIN)) {
+if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
+goto bad_reg;
+}
+if (!env->v7m.secure) {
return;
}
env->v7m.faultmask[M_REG_NS] = val & 1;
--
2.34.1
[PULL 19/26] target/arm/sme: Unify set_pstate() SM/ZA helpers as set_svcr()
From: Richard Henderson
Unify the two helper_set_pstate_{sm,za} in this function.
Do not call helper_* functions from svcr_write.
Signed-off-by: Richard Henderson
Reviewed-by: Fabiano Rosas
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20230112102436.1913-8-phi...@linaro.org
Message-Id: <20230112004322.161330-1-richard.hender...@linaro.org>
[PMD: Split patch in multiple tiny steps]
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Maydell
---
target/arm/helper-sme.h| 3 +--
target/arm/helper.c| 2 --
target/arm/sme_helper.c| 9 ++---
target/arm/translate-a64.c | 10 ++
4 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/target/arm/helper-sme.h b/target/arm/helper-sme.h
index d2d544a6961..27eef49a11e 100644
--- a/target/arm/helper-sme.h
+++ b/target/arm/helper-sme.h
@@ -17,8 +17,7 @@
* License along with this library; if not, see
[PULL 26/26] target/arm: Look up ARMCPRegInfo at runtime
From: Richard Henderson
Do not encode the pointer as a constant in the opcode stream.
This pointer is specific to the cpu that first generated the
translation, which runs into problems with both hot-pluggable
cpus and user-only threads, as cpus are removed. It's also a
potential correctness issue in the theoretical case of a
slightly-heterogenous system, because if CPU 0 generates a
TB and then CPU 1 executes it, CPU 1 will end up using CPU 0's
hash table, which might have a wrong set of registers in it.
(All our current systems are either completely homogenous,
M-profile, or have CPUs sufficiently different that they
wouldn't be sharing TBs anyway because the differences would
show up in the TB flags, so the correctness issue is only
theoretical, not practical.)
Perform the lookup in either helper_access_check_cp_reg,
or a new helper_lookup_cp_reg.
Signed-off-by: Richard Henderson
Message-id: 20230106194451.1213153-3-richard.hender...@linaro.org
[PMM: added note in commit message about correctness issue]
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
target/arm/helper.h| 11 +
target/arm/translate.h | 7 ++
target/arm/op_helper.c | 27 ++--
target/arm/translate-a64.c | 49 ++---
target/arm/translate.c | 50 +-
5 files changed, 95 insertions(+), 49 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 92f36d9dbb7..018b00ea75b 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -79,11 +79,12 @@ DEF_HELPER_2(v8m_stackcheck, void, env, i32)
DEF_HELPER_FLAGS_2(check_bxj_trap, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_4(access_check_cp_reg, void, env, ptr, i32, i32)
-DEF_HELPER_3(set_cp_reg, void, env, ptr, i32)
-DEF_HELPER_2(get_cp_reg, i32, env, ptr)
-DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64)
-DEF_HELPER_2(get_cp_reg64, i64, env, ptr)
+DEF_HELPER_4(access_check_cp_reg, cptr, env, i32, i32, i32)
+DEF_HELPER_FLAGS_2(lookup_cp_reg, TCG_CALL_NO_RWG_SE, cptr, env, i32)
+DEF_HELPER_3(set_cp_reg, void, env, cptr, i32)
+DEF_HELPER_2(get_cp_reg, i32, env, cptr)
+DEF_HELPER_3(set_cp_reg64, void, env, cptr, i64)
+DEF_HELPER_2(get_cp_reg64, i64, env, cptr)
DEF_HELPER_2(get_r13_banked, i32, env, i32)
DEF_HELPER_3(set_r13_banked, void, env, i32, i32)
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 3cdc7dbc2fb..f17f095cbe2 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -610,6 +610,13 @@ static inline void set_disas_label(DisasContext *s,
DisasLabel l)
s->pc_save = l.pc_save;
}
+static inline TCGv_ptr gen_lookup_cp_reg(uint32_t key)
+{
+TCGv_ptr ret = tcg_temp_new_ptr();
+gen_helper_lookup_cp_reg(ret, cpu_env, tcg_constant_i32(key));
+return ret;
+}
+
/*
* Helpers for implementing sets of trans_* functions.
* Defer the implementation of NAME to FUNC, with optional extra arguments.
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 70672bcd9fc..31f89db8997 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -624,14 +624,16 @@ uint32_t HELPER(mrs_banked)(CPUARMState *env, uint32_t
tgtmode, uint32_t regno)
}
}
-void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t
syndrome,
- uint32_t isread)
+const void *HELPER(access_check_cp_reg)(CPUARMState *env, uint32_t key,
+uint32_t syndrome, uint32_t isread)
{
ARMCPU *cpu = env_archcpu(env);
-const ARMCPRegInfo *ri = rip;
+const ARMCPRegInfo *ri = get_arm_cp_reginfo(cpu->cp_regs, key);
CPAccessResult res = CP_ACCESS_OK;
int target_el;
+assert(ri != NULL);
+
if (arm_feature(env, ARM_FEATURE_XSCALE) && ri->cp < 14
&& extract32(env->cp15.c15_cpar, ri->cp, 1) == 0) {
res = CP_ACCESS_TRAP;
@@ -663,7 +665,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void
*rip, uint32_t syndrome,
res = ri->accessfn(env, ri, isread);
}
if (likely(res == CP_ACCESS_OK)) {
-return;
+return ri;
}
fail:
@@ -705,7 +707,16 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void
*rip, uint32_t syndrome,
raise_exception(env, EXCP_UDEF, syndrome, target_el);
}
-void HELPER(set_cp_reg)(CPUARMState *env, void *rip, uint32_t value)
+const void *HELPER(lookup_cp_reg)(CPUARMState *env, uint32_t key)
+{
+ARMCPU *cpu = env_archcpu(env);
+const ARMCPRegInfo *ri = get_arm_cp_reginfo(cpu->cp_regs, key);
+
+assert(ri != NULL);
+return ri;
+}
+
+void HELPER(set_cp_reg)(CPUARMState *env, const void *rip, uint32_t value)
{
const ARMCPRegInfo *ri = rip;
@@ -718,7 +729,7 @@ void HELPER(set_cp_reg)(CPUARMState *env, void *rip,
uint32_t value)
}
}
-uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip)
+uint32_t HELPER(get_cp_reg)(CPUARMState *env, const void *rip)
{
const ARMCPRegInfo *ri = rip;
u
[PULL 18/26] target/arm/sme: Rebuild hflags in aarch64_set_svcr()
From: Richard Henderson
Signed-off-by: Richard Henderson
Reviewed-by: Fabiano Rosas
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20230112102436.1913-7-phi...@linaro.org
Message-Id: <20230112004322.161330-1-richard.hender...@linaro.org>
[PMD: Split patch in multiple tiny steps]
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Maydell
---
linux-user/aarch64/cpu_loop.c | 8 +---
linux-user/aarch64/signal.c | 3 ---
target/arm/helper.c | 6 +-
target/arm/sme_helper.c | 8
4 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 5e93d27d8f6..2e2f7cf2188 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -89,14 +89,8 @@ void cpu_loop(CPUARMState *env)
switch (trapnr) {
case EXCP_SWI:
-/*
- * On syscall, PSTATE.ZA is preserved, along with the ZA matrix.
- * PSTATE.SM is cleared, per SMSTOP, which does ResetSVEState.
- */
+/* On syscall, PSTATE.ZA is preserved, PSTATE.SM is cleared. */
aarch64_set_svcr(env, 0, R_SVCR_SM_MASK);
-if (FIELD_EX64(env->svcr, SVCR, SM)) {
-arm_rebuild_hflags(env);
-}
ret = do_syscall(env,
env->xregs[8],
env->xregs[0],
diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index a326a6def5e..b265cfd4706 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -667,9 +667,6 @@ static void target_setup_frame(int usig, struct
target_sigaction *ka,
/* Invoke the signal handler with both SM and ZA disabled. */
aarch64_set_svcr(env, 0, R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
-if (env->svcr) {
-arm_rebuild_hflags(env);
-}
if (info) {
tswap_siginfo(&frame->info, info);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 564c5d93320..80779678499 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6738,6 +6738,9 @@ void aarch64_set_svcr(CPUARMState *env, uint64_t new,
uint64_t mask)
{
uint64_t change = (env->svcr ^ new) & mask;
+if (change == 0) {
+return;
+}
env->svcr ^= change;
if (change & R_SVCR_SM_MASK) {
@@ -6755,6 +6758,8 @@ void aarch64_set_svcr(CPUARMState *env, uint64_t new,
uint64_t mask)
if (change & new & R_SVCR_ZA_MASK) {
memset(env->zarray, 0, sizeof(env->zarray));
}
+
+arm_rebuild_hflags(env);
}
static void svcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -6763,7 +6768,6 @@ static void svcr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
helper_set_pstate_sm(env, FIELD_EX64(value, SVCR, SM));
helper_set_pstate_za(env, FIELD_EX64(value, SVCR, ZA));
aarch64_set_svcr(env, value, -1);
-arm_rebuild_hflags(env);
}
static void smcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
diff --git a/target/arm/sme_helper.c b/target/arm/sme_helper.c
index e146c17ba19..3abe03e4cb3 100644
--- a/target/arm/sme_helper.c
+++ b/target/arm/sme_helper.c
@@ -31,20 +31,12 @@
void helper_set_pstate_sm(CPUARMState *env, uint32_t i)
{
-if (i == FIELD_EX64(env->svcr, SVCR, SM)) {
-return;
-}
aarch64_set_svcr(env, 0, R_SVCR_SM_MASK);
-arm_rebuild_hflags(env);
}
void helper_set_pstate_za(CPUARMState *env, uint32_t i)
{
-if (i == FIELD_EX64(env->svcr, SVCR, ZA)) {
-return;
-}
aarch64_set_svcr(env, 0, R_SVCR_ZA_MASK);
-arm_rebuild_hflags(env);
}
void helper_sme_zero(CPUARMState *env, uint32_t imm, uint32_t svl)
--
2.34.1
[PULL 11/26] hw/i2c/versatile_i2c: Use ARM_SBCON_I2C() macro
From: Philippe Mathieu-Daudé
ARM_SBCON_I2C() macro and ArmSbconI2CState typedef are
already declared via the QOM DECLARE_INSTANCE_CHECKER()
macro in "hw/i2c/arm_sbcon_i2c.h". Drop the VERSATILE_I2C
declarations from versatile_i2c.c.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-id: 20230110082508.24038-5-phi...@linaro.org
Signed-off-by: Peter Maydell
---
hw/i2c/versatile_i2c.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/i2c/versatile_i2c.c b/hw/i2c/versatile_i2c.c
index b95c70608b2..d19df62265a 100644
--- a/hw/i2c/versatile_i2c.c
+++ b/hw/i2c/versatile_i2c.c
@@ -29,11 +29,6 @@
#include "qemu/module.h"
#include "qom/object.h"
-typedef ArmSbconI2CState VersatileI2CState;
-DECLARE_INSTANCE_CHECKER(ArmSbconI2CState, VERSATILE_I2C,
- TYPE_ARM_SBCON_I2C)
-
-
REG32(CONTROL_GET, 0)
REG32(CONTROL_SET, 0)
@@ -86,7 +81,7 @@ static const MemoryRegionOps versatile_i2c_ops = {
static void versatile_i2c_init(Object *obj)
{
DeviceState *dev = DEVICE(obj);
-ArmSbconI2CState *s = VERSATILE_I2C(obj);
+ArmSbconI2CState *s = ARM_SBCON_I2C(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
I2CBus *bus;
--
2.34.1
[PULL 03/26] hw/i2c/bitbang_i2c: Define TYPE_GPIO_I2C in public header
From: Philippe Mathieu-Daudé
Define TYPE_GPIO_I2C in the public "hw/i2c/bitbang_i2c.h"
header and use it in hw/arm/musicpal.c.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Acked-by: Corey Minyard
Message-id: 20230111085016.44551-2-phi...@linaro.org
Signed-off-by: Peter Maydell
---
include/hw/i2c/bitbang_i2c.h | 2 ++
hw/arm/musicpal.c| 3 ++-
hw/i2c/bitbang_i2c.c | 1 -
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/hw/i2c/bitbang_i2c.h b/include/hw/i2c/bitbang_i2c.h
index 92334e9016a..a079e6d70f9 100644
--- a/include/hw/i2c/bitbang_i2c.h
+++ b/include/hw/i2c/bitbang_i2c.h
@@ -3,6 +3,8 @@
#include "hw/i2c/i2c.h"
+#define TYPE_GPIO_I2C "gpio_i2c"
+
typedef struct bitbang_i2c_interface bitbang_i2c_interface;
#define BITBANG_I2C_SDA 0
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 73e2b7e4cef..89b66606c32 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -26,6 +26,7 @@
#include "hw/block/flash.h"
#include "ui/console.h"
#include "hw/i2c/i2c.h"
+#include "hw/i2c/bitbang_i2c.h"
#include "hw/irq.h"
#include "hw/or-irq.h"
#include "hw/audio/wm8750.h"
@@ -1303,7 +1304,7 @@ static void musicpal_init(MachineState *machine)
dev = sysbus_create_simple(TYPE_MUSICPAL_GPIO, MP_GPIO_BASE,
qdev_get_gpio_in(pic, MP_GPIO_IRQ));
-i2c_dev = sysbus_create_simple("gpio_i2c", -1, NULL);
+i2c_dev = sysbus_create_simple(TYPE_GPIO_I2C, -1, NULL);
i2c = (I2CBus *)qdev_get_child_bus(i2c_dev, "i2c");
lcd_dev = sysbus_create_simple(TYPE_MUSICPAL_LCD, MP_LCD_BASE, NULL);
diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c
index e9a0612a043..ac84bf02624 100644
--- a/hw/i2c/bitbang_i2c.c
+++ b/hw/i2c/bitbang_i2c.c
@@ -162,7 +162,6 @@ void bitbang_i2c_init(bitbang_i2c_interface *s, I2CBus *bus)
/* GPIO interface. */
-#define TYPE_GPIO_I2C "gpio_i2c"
OBJECT_DECLARE_SIMPLE_TYPE(GPIOI2CState, GPIO_I2C)
struct GPIOI2CState {
--
2.34.1
[PULL 09/26] hw/i2c/versatile_i2c: Replace VersatileI2CState -> ArmSbconI2CState
From: Philippe Mathieu-Daudé
In order to rename TYPE_VERSATILE_I2C as TYPE_ARM_SBCON_I2C
(the formal ARM naming), start renaming its state.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-id: 20230110082508.24038-3-phi...@linaro.org
Signed-off-by: Peter Maydell
---
include/hw/i2c/arm_sbcon_i2c.h | 3 +--
hw/i2c/versatile_i2c.c | 10 +-
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/hw/i2c/arm_sbcon_i2c.h b/include/hw/i2c/arm_sbcon_i2c.h
index f54d1e54135..0101422d9dc 100644
--- a/include/hw/i2c/arm_sbcon_i2c.h
+++ b/include/hw/i2c/arm_sbcon_i2c.h
@@ -21,8 +21,7 @@
#define TYPE_ARM_SBCON_I2C TYPE_VERSATILE_I2C
typedef struct ArmSbconI2CState ArmSbconI2CState;
-DECLARE_INSTANCE_CHECKER(ArmSbconI2CState, ARM_SBCON_I2C,
- TYPE_ARM_SBCON_I2C)
+DECLARE_INSTANCE_CHECKER(ArmSbconI2CState, ARM_SBCON_I2C, TYPE_ARM_SBCON_I2C)
struct ArmSbconI2CState {
/*< private >*/
diff --git a/hw/i2c/versatile_i2c.c b/hw/i2c/versatile_i2c.c
index 52a650f45ee..ee095762e57 100644
--- a/hw/i2c/versatile_i2c.c
+++ b/hw/i2c/versatile_i2c.c
@@ -30,7 +30,7 @@
#include "qom/object.h"
typedef ArmSbconI2CState VersatileI2CState;
-DECLARE_INSTANCE_CHECKER(VersatileI2CState, VERSATILE_I2C,
+DECLARE_INSTANCE_CHECKER(ArmSbconI2CState, VERSATILE_I2C,
TYPE_VERSATILE_I2C)
@@ -45,7 +45,7 @@ REG32(CONTROL_CLR, 4)
static uint64_t versatile_i2c_read(void *opaque, hwaddr offset,
unsigned size)
{
-VersatileI2CState *s = opaque;
+ArmSbconI2CState *s = opaque;
switch (offset) {
case A_CONTROL_SET:
@@ -60,7 +60,7 @@ static uint64_t versatile_i2c_read(void *opaque, hwaddr
offset,
static void versatile_i2c_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
-VersatileI2CState *s = opaque;
+ArmSbconI2CState *s = opaque;
switch (offset) {
case A_CONTROL_SET:
@@ -86,7 +86,7 @@ static const MemoryRegionOps versatile_i2c_ops = {
static void versatile_i2c_init(Object *obj)
{
DeviceState *dev = DEVICE(obj);
-VersatileI2CState *s = VERSATILE_I2C(obj);
+ArmSbconI2CState *s = VERSATILE_I2C(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
I2CBus *bus;
@@ -100,7 +100,7 @@ static void versatile_i2c_init(Object *obj)
static const TypeInfo versatile_i2c_info = {
.name = TYPE_VERSATILE_I2C,
.parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(VersatileI2CState),
+.instance_size = sizeof(ArmSbconI2CState),
.instance_init = versatile_i2c_init,
};
--
2.34.1
[PULL 08/26] hw/i2c/versatile_i2c: Drop useless casts from void * to pointer
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-id: 20230110082508.24038-2-phi...@linaro.org
Signed-off-by: Peter Maydell
---
hw/i2c/versatile_i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i2c/versatile_i2c.c b/hw/i2c/versatile_i2c.c
index 3a04ba39691..52a650f45ee 100644
--- a/hw/i2c/versatile_i2c.c
+++ b/hw/i2c/versatile_i2c.c
@@ -45,7 +45,7 @@ REG32(CONTROL_CLR, 4)
static uint64_t versatile_i2c_read(void *opaque, hwaddr offset,
unsigned size)
{
-VersatileI2CState *s = (VersatileI2CState *)opaque;
+VersatileI2CState *s = opaque;
switch (offset) {
case A_CONTROL_SET:
@@ -60,7 +60,7 @@ static uint64_t versatile_i2c_read(void *opaque, hwaddr
offset,
static void versatile_i2c_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
-VersatileI2CState *s = (VersatileI2CState *)opaque;
+VersatileI2CState *s = opaque;
switch (offset) {
case A_CONTROL_SET:
--
2.34.1
[PULL 24/26] target/arm: provide stubs for more external debug registers
From: Evgeny Iakovlev
Qemu doesn't implement Debug Communication Channel, as well as the rest
of external debug interface. However, Microsoft Hyper-V in tries to
access some of those registers during an EL2 context switch.
Since there is no architectural way to not advertise support for external
debug, provide RAZ/WI stubs for OSDTRRX_EL1, OSDTRTX_EL1 and OSECCR_EL1
registers in the same way the rest of DCM is currently done. Do account
for access traps though with access_tda.
Signed-off-by: Evgeny Iakovlev
Reviewed-by: Peter Maydell
Message-id: 20230120155929.32384-3-eiakov...@linux.microsoft.com
Signed-off-by: Peter Maydell
---
target/arm/debug_helper.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index f95a73329db..cced3f168d0 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -682,6 +682,27 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
.opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
.access = PL0_R, .accessfn = access_tda,
.type = ARM_CP_CONST, .resetvalue = 0 },
+/*
+ * OSDTRRX_EL1/OSDTRTX_EL1 are used for save and restore of DBGDTRRX_EL0.
+ * It is a component of the Debug Communications Channel, which is not
implemented.
+ */
+{ .name = "OSDTRRX_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
+ .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 2,
+ .access = PL1_RW, .accessfn = access_tda,
+ .type = ARM_CP_CONST, .resetvalue = 0 },
+{ .name = "OSDTRTX_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
+ .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 2,
+ .access = PL1_RW, .accessfn = access_tda,
+ .type = ARM_CP_CONST, .resetvalue = 0 },
+/*
+ * OSECCR_EL1 provides a mechanism for an operating system
+ * to access the contents of EDECCR. EDECCR is not implemented though,
+ * as is the rest of external device mechanism.
+ */
+{ .name = "OSECCR_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
+ .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 2,
+ .access = PL1_RW, .accessfn = access_tda,
+ .type = ARM_CP_CONST, .resetvalue = 0 },
/*
* DBGDSCRint[15,12,5:2] map to MDSCR_EL1[15,12,5:2]. Map all bits as
* it is unlikely a guest will care.
--
2.34.1
[PULL 25/26] target/arm: Reorg do_coproc_insn
From: Richard Henderson
Move the ri == NULL case to the top of the function and return.
This allows the else to be removed and the code unindented.
Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
Message-id: 20230106194451.1213153-2-richard.hender...@linaro.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
target/arm/translate.c | 406 -
1 file changed, 203 insertions(+), 203 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 1dcaefb8e75..40f9f07ea30 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -4715,220 +4715,220 @@ static void do_coproc_insn(DisasContext *s, int
cpnum, int is64,
bool isread, int rt, int rt2)
{
const ARMCPRegInfo *ri;
+bool need_exit_tb;
ri = get_arm_cp_reginfo(s->cp_regs,
ENCODE_CP_REG(cpnum, is64, s->ns, crn, crm, opc1, opc2));
-if (ri) {
-bool need_exit_tb;
-/* Check access permissions */
-if (!cp_access_ok(s->current_el, ri, isread)) {
-unallocated_encoding(s);
-return;
-}
-
-if (s->hstr_active || ri->accessfn ||
-(arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) {
-/* Emit code to perform further access permissions checks at
- * runtime; this may result in an exception.
- * Note that on XScale all cp0..c13 registers do an access check
- * call in order to handle c15_cpar.
- */
-uint32_t syndrome;
-
-/* Note that since we are an implementation which takes an
- * exception on a trapped conditional instruction only if the
- * instruction passes its condition code check, we can take
- * advantage of the clause in the ARM ARM that allows us to set
- * the COND field in the instruction to 0xE in all cases.
- * We could fish the actual condition out of the insn (ARM)
- * or the condexec bits (Thumb) but it isn't necessary.
- */
-switch (cpnum) {
-case 14:
-if (is64) {
-syndrome = syn_cp14_rrt_trap(1, 0xe, opc1, crm, rt, rt2,
- isread, false);
-} else {
-syndrome = syn_cp14_rt_trap(1, 0xe, opc1, opc2, crn, crm,
-rt, isread, false);
-}
-break;
-case 15:
-if (is64) {
-syndrome = syn_cp15_rrt_trap(1, 0xe, opc1, crm, rt, rt2,
- isread, false);
-} else {
-syndrome = syn_cp15_rt_trap(1, 0xe, opc1, opc2, crn, crm,
-rt, isread, false);
-}
-break;
-default:
-/* ARMv8 defines that only coprocessors 14 and 15 exist,
- * so this can only happen if this is an ARMv7 or earlier CPU,
- * in which case the syndrome information won't actually be
- * guest visible.
- */
-assert(!arm_dc_feature(s, ARM_FEATURE_V8));
-syndrome = syn_uncategorized();
-break;
-}
-
-gen_set_condexec(s);
-gen_update_pc(s, 0);
-gen_helper_access_check_cp_reg(cpu_env,
- tcg_constant_ptr(ri),
- tcg_constant_i32(syndrome),
- tcg_constant_i32(isread));
-} else if (ri->type & ARM_CP_RAISES_EXC) {
-/*
- * The readfn or writefn might raise an exception;
- * synchronize the CPU state in case it does.
- */
-gen_set_condexec(s);
-gen_update_pc(s, 0);
-}
-
-/* Handle special cases first */
-switch (ri->type & ARM_CP_SPECIAL_MASK) {
-case 0:
-break;
-case ARM_CP_NOP:
-return;
-case ARM_CP_WFI:
-if (isread) {
-unallocated_encoding(s);
-return;
-}
-gen_update_pc(s, curr_insn_len(s));
-s->base.is_jmp = DISAS_WFI;
-return;
-default:
-g_assert_not_reached();
-}
-
-if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO))
{
-gen_io_start();
-}
-
-if (isread) {
-/* Read */
-if (is64) {
-TCGv_i64 tmp64;
-TCGv_i32 tmp;
-if (ri->type & ARM_CP_CONST) {
-tmp64 = tcg_constant_i64(ri->resetvalue);
-} else if (ri->readfn) {
-tmp64 = tcg_temp_new_i64();
-
Re: [PATCH v0 0/4] backends/hostmem: add an ability to specify prealloc timeout
On Mon, Jan 23, 2023 at 04:30:03PM +0300, Daniil Tatianin wrote: > On 1/23/23 11:57 AM, David Hildenbrand wrote: > > On 20.01.23 14:47, Daniil Tatianin wrote: > > > This series introduces new qemu_prealloc_mem_with_timeout() api, > > > which allows limiting the maximum amount of time to be spent on memory > > > preallocation. It also adds prealloc statistics collection that is > > > exposed via an optional timeout handler. > > > > > > This new api is then utilized by hostmem for guest RAM preallocation > > > controlled via new object properties called 'prealloc-timeout' and > > > 'prealloc-timeout-fatal'. > > > > > > This is useful for limiting VM startup time on systems with > > > unpredictable page allocation delays due to memory fragmentation or the > > > backing storage. The timeout can be configured to either simply emit a > > > warning and continue VM startup without having preallocated the entire > > > guest RAM or just abort startup entirely if that is not acceptable for > > > a specific use case. > > > > The major use case for preallocation is memory resources that cannot be > > overcommitted (hugetlb, file blocks, ...), to avoid running out of such > > resources later, while the guest is already running, and crashing it. > > Wouldn't you say that preallocating memory for the sake of speeding up guest > kernel startup & runtime is a valid use case of prealloc? This way we can > avoid expensive (for a multitude of reasons) page faults that will otherwise > slow down the guest significantly at runtime and affect the user experience. > > > Allocating only a fraction "because it takes too long" looks quite > > useless in that (main use-case) context. We shouldn't encourage QEMU > > users to play with fire in such a way. IOW, there should be no way > > around "prealloc-timeout-fatal". Either preallocation succeeded and the > > guest can run, or it failed, and the guest can't run. > > Here we basically accept the fact that e.g with fragmented memory the kernel > might take a while in a page fault handler especially for hugetlb because of > page compaction that has to run for every fault. > > This way we can prefault at least some number of pages and let the guest > fault the rest on demand later on during runtime even if it's slow and would > cause a noticeable lag. Rather than treat this as a problem that needs a timeout, can we restate it as situations need synchronous vs asynchronous preallocation ? For the case where we need synchronous prealloc, current QEMU deals with that. If it doesn't work quickly enough, mgmt can just kill QEMU already today. For the case where you would like some prealloc, but don't mind if it runs without full prealloc, then why not just treat it as an entirely asynchronous task ? Instead of calling qemu_prealloc_mem and waiting for it to complete, just spawn a thread to run qemu_prealloc_mem, so it doesn't block QEMU startup. This will have minimal maint burden on the existing code, and will avoid need for mgmt apps to think about what timeout value to give, which is good because timeouts are hard to get right. Most of the time that async background prealloc will still finish before the guest even gets out of the firmware phase, but if it takes longer it is no big deal. You don't need to quit the prealloc job early, you just need it to not delay the guest OS boot IIUC. This impl could be done with the 'prealloc' property turning from a boolean on/off, to a enum on/async/off, where 'on' == sync prealloc. Or add a separate 'prealloc-async' bool property With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL 12/26] hw/i2c/versatile_i2c: Rename versatile_i2c -> arm_sbcon_i2c
From: Philippe Mathieu-Daudé
This device model started with the Versatile board, named
TYPE_VERSATILE_I2C, then ended up renamed TYPE_ARM_SBCON_I2C
as per the official "ARM SBCon two-wire serial bus interface"
description from:
https://developer.arm.com/documentation/dui0440/b/programmer-s-reference/two-wire-serial-bus-interface--sbcon
Use the latter name as a better description.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-id: 20230110082508.24038-6-phi...@linaro.org
Signed-off-by: Peter Maydell
---
MAINTAINERS | 1 +
hw/i2c/{versatile_i2c.c => arm_sbcon_i2c.c} | 24 ++---
hw/arm/Kconfig | 4 ++--
hw/i2c/Kconfig | 2 +-
hw/i2c/meson.build | 2 +-
5 files changed, 17 insertions(+), 16 deletions(-)
rename hw/i2c/{versatile_i2c.c => arm_sbcon_i2c.c} (81%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 08ad1e5341e..c581c11a645 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -942,6 +942,7 @@ M: Peter Maydell
L: qemu-...@nongnu.org
S: Maintained
F: hw/*/versatile*
+F: hw/i2c/arm_sbcon_i2c.c
F: include/hw/i2c/arm_sbcon_i2c.h
F: hw/misc/arm_sysctl.c
F: docs/system/arm/versatile.rst
diff --git a/hw/i2c/versatile_i2c.c b/hw/i2c/arm_sbcon_i2c.c
similarity index 81%
rename from hw/i2c/versatile_i2c.c
rename to hw/i2c/arm_sbcon_i2c.c
index d19df62265a..979ccbe0ed6 100644
--- a/hw/i2c/versatile_i2c.c
+++ b/hw/i2c/arm_sbcon_i2c.c
@@ -37,7 +37,7 @@ REG32(CONTROL_CLR, 4)
#define SCL BIT(0)
#define SDA BIT(1)
-static uint64_t versatile_i2c_read(void *opaque, hwaddr offset,
+static uint64_t arm_sbcon_i2c_read(void *opaque, hwaddr offset,
unsigned size)
{
ArmSbconI2CState *s = opaque;
@@ -52,7 +52,7 @@ static uint64_t versatile_i2c_read(void *opaque, hwaddr
offset,
}
}
-static void versatile_i2c_write(void *opaque, hwaddr offset,
+static void arm_sbcon_i2c_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
ArmSbconI2CState *s = opaque;
@@ -72,13 +72,13 @@ static void versatile_i2c_write(void *opaque, hwaddr offset,
s->in = bitbang_i2c_set(&s->bitbang, BITBANG_I2C_SDA, (s->out & SDA) != 0);
}
-static const MemoryRegionOps versatile_i2c_ops = {
-.read = versatile_i2c_read,
-.write = versatile_i2c_write,
+static const MemoryRegionOps arm_sbcon_i2c_ops = {
+.read = arm_sbcon_i2c_read,
+.write = arm_sbcon_i2c_write,
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void versatile_i2c_init(Object *obj)
+static void arm_sbcon_i2c_init(Object *obj)
{
DeviceState *dev = DEVICE(obj);
ArmSbconI2CState *s = ARM_SBCON_I2C(obj);
@@ -87,21 +87,21 @@ static void versatile_i2c_init(Object *obj)
bus = i2c_init_bus(dev, "i2c");
bitbang_i2c_init(&s->bitbang, bus);
-memory_region_init_io(&s->iomem, obj, &versatile_i2c_ops, s,
+memory_region_init_io(&s->iomem, obj, &arm_sbcon_i2c_ops, s,
"arm_sbcon_i2c", 0x1000);
sysbus_init_mmio(sbd, &s->iomem);
}
-static const TypeInfo versatile_i2c_info = {
+static const TypeInfo arm_sbcon_i2c_info = {
.name = TYPE_ARM_SBCON_I2C,
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(ArmSbconI2CState),
-.instance_init = versatile_i2c_init,
+.instance_init = arm_sbcon_i2c_init,
};
-static void versatile_i2c_register_types(void)
+static void arm_sbcon_i2c_register_types(void)
{
-type_register_static(&versatile_i2c_info);
+type_register_static(&arm_sbcon_i2c_info);
}
-type_init(versatile_i2c_register_types)
+type_init(arm_sbcon_i2c_register_types)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 19d6b9d95f5..2d157de9b8b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -211,7 +211,7 @@ config REALVIEW
select PL110
select PL181 # display
select PL310 # cache controller
-select VERSATILE_I2C
+select ARM_SBCON_I2C
select DS1338 # I2C RTC+NVRAM
select USB_OHCI
@@ -481,7 +481,7 @@ config MPS2
select SPLIT_IRQ
select UNIMP
select CMSDK_APB_WATCHDOG
-select VERSATILE_I2C
+select ARM_SBCON_I2C
config FSL_IMX7
bool
diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
index f8ec461be3d..14886b35dac 100644
--- a/hw/i2c/Kconfig
+++ b/hw/i2c/Kconfig
@@ -14,7 +14,7 @@ config SMBUS_EEPROM
bool
select SMBUS
-config VERSATILE_I2C
+config ARM_SBCON_I2C
bool
select BITBANG_I2C
diff --git a/hw/i2c/meson.build b/hw/i2c/meson.build
index e4c8e14a527..3996564c25c 100644
--- a/hw/i2c/meson.build
+++ b/hw/i2c/meson.build
@@ -12,7 +12,7 @@ i2c_ss.add(when: 'CONFIG_ALLWINNER_I2C', if_true:
files('allwinner-i2c.c'))
i2c_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('microbit_i2c.c'))
i2c_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_smbus.c'))
i2c_ss.add(when: 'CONFIG_SMBUS_EEPROM', if_true: files('smbus_eep
[PULL 04/26] hw/i2c/bitbang_i2c: Remove unused dummy MemoryRegion
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Acked-by: Corey Minyard
Message-id: 20230111085016.44551-3-phi...@linaro.org
Signed-off-by: Peter Maydell
---
hw/i2c/bitbang_i2c.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c
index ac84bf02624..e41cb63daa7 100644
--- a/hw/i2c/bitbang_i2c.c
+++ b/hw/i2c/bitbang_i2c.c
@@ -165,9 +165,10 @@ void bitbang_i2c_init(bitbang_i2c_interface *s, I2CBus
*bus)
OBJECT_DECLARE_SIMPLE_TYPE(GPIOI2CState, GPIO_I2C)
struct GPIOI2CState {
+/*< private >*/
SysBusDevice parent_obj;
+/*< public >*/
-MemoryRegion dummy_iomem;
bitbang_i2c_interface bitbang;
int last_level;
qemu_irq out;
@@ -188,12 +189,8 @@ static void gpio_i2c_init(Object *obj)
{
DeviceState *dev = DEVICE(obj);
GPIOI2CState *s = GPIO_I2C(obj);
-SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
I2CBus *bus;
-memory_region_init(&s->dummy_iomem, obj, "gpio_i2c", 0);
-sysbus_init_mmio(sbd, &s->dummy_iomem);
-
bus = i2c_init_bus(dev, "i2c");
bitbang_i2c_init(&s->bitbang, bus);
--
2.34.1
[PULL 15/26] target/arm/sme: Introduce aarch64_set_svcr()
From: Richard Henderson
Signed-off-by: Richard Henderson
Reviewed-by: Fabiano Rosas
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20230112102436.1913-4-phi...@linaro.org
Message-Id: <20230112004322.161330-1-richard.hender...@linaro.org>
[PMD: Split patch in multiple tiny steps]
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Maydell
---
target/arm/cpu.h | 1 +
linux-user/aarch64/cpu_loop.c | 2 +-
linux-user/aarch64/signal.c | 2 +-
target/arm/helper.c | 8
target/arm/sme_helper.c | 4 ++--
5 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1feb63b4d73..ef61849eb1d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1123,6 +1123,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t
*buf, int reg);
void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
void aarch64_sve_change_el(CPUARMState *env, int old_el,
int new_el, bool el0_a64);
+void aarch64_set_svcr(CPUARMState *env, uint64_t new, uint64_t mask);
void arm_reset_sve_state(CPUARMState *env);
/*
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 9875d609a91..d53742e10bb 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -93,8 +93,8 @@ void cpu_loop(CPUARMState *env)
* On syscall, PSTATE.ZA is preserved, along with the ZA matrix.
* PSTATE.SM is cleared, per SMSTOP, which does ResetSVEState.
*/
+aarch64_set_svcr(env, 0, R_SVCR_SM_MASK);
if (FIELD_EX64(env->svcr, SVCR, SM)) {
-env->svcr = FIELD_DP64(env->svcr, SVCR, SM, 0);
arm_rebuild_hflags(env);
arm_reset_sve_state(env);
}
diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index 6a2c6e06d28..b6e4dcb494d 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -669,11 +669,11 @@ static void target_setup_frame(int usig, struct
target_sigaction *ka,
* Invoke the signal handler with both SM and ZA disabled.
* When clearing SM, ResetSVEState, per SMSTOP.
*/
+aarch64_set_svcr(env, 0, R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
if (FIELD_EX64(env->svcr, SVCR, SM)) {
arm_reset_sve_state(env);
}
if (env->svcr) {
-env->svcr = 0;
arm_rebuild_hflags(env);
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 22ea8fbe368..24c069b8acf 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6725,11 +6725,19 @@ static CPAccessResult access_esm(CPUARMState *env,
const ARMCPRegInfo *ri,
return CP_ACCESS_OK;
}
+void aarch64_set_svcr(CPUARMState *env, uint64_t new, uint64_t mask)
+{
+uint64_t change = (env->svcr ^ new) & mask;
+
+env->svcr ^= change;
+}
+
static void svcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
helper_set_pstate_sm(env, FIELD_EX64(value, SVCR, SM));
helper_set_pstate_za(env, FIELD_EX64(value, SVCR, ZA));
+aarch64_set_svcr(env, value, -1);
arm_rebuild_hflags(env);
}
diff --git a/target/arm/sme_helper.c b/target/arm/sme_helper.c
index b5aefa3edaf..94dc084135d 100644
--- a/target/arm/sme_helper.c
+++ b/target/arm/sme_helper.c
@@ -43,7 +43,7 @@ void helper_set_pstate_sm(CPUARMState *env, uint32_t i)
if (i == FIELD_EX64(env->svcr, SVCR, SM)) {
return;
}
-env->svcr ^= R_SVCR_SM_MASK;
+aarch64_set_svcr(env, 0, R_SVCR_SM_MASK);
arm_reset_sve_state(env);
arm_rebuild_hflags(env);
}
@@ -53,7 +53,7 @@ void helper_set_pstate_za(CPUARMState *env, uint32_t i)
if (i == FIELD_EX64(env->svcr, SVCR, ZA)) {
return;
}
-env->svcr ^= R_SVCR_ZA_MASK;
+aarch64_set_svcr(env, 0, R_SVCR_ZA_MASK);
/*
* ResetSMEState.
--
2.34.1
[PULL 07/26] hw/i2c/bitbang_i2c: Convert DPRINTF() to trace events
From: Philippe Mathieu-Daudé
Convert the remaining DPRINTF debug macro uses to tracepoints.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Acked-by: Corey Minyard
Message-id: 20230111085016.44551-6-phi...@linaro.org
Signed-off-by: Peter Maydell
---
hw/i2c/bitbang_i2c.c | 18 ++
hw/i2c/trace-events | 4
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c
index efc49b7502f..bb189547651 100644
--- a/hw/i2c/bitbang_i2c.c
+++ b/hw/i2c/bitbang_i2c.c
@@ -18,14 +18,6 @@
#include "qom/object.h"
#include "trace.h"
-//#define DEBUG_BITBANG_I2C
-
-#ifdef DEBUG_BITBANG_I2C
-#define DPRINTF(fmt, ...) \
-do { printf("bitbang_i2c: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
-#endif
/* bitbang_i2c_state enum to name */
static const char * const sname[] = {
@@ -71,8 +63,10 @@ static void bitbang_i2c_enter_stop(bitbang_i2c_interface
*i2c)
/* Set device data pin. */
static int bitbang_i2c_ret(bitbang_i2c_interface *i2c, int level)
{
+trace_bitbang_i2c_data(i2c->last_clock, i2c->last_data,
+ i2c->device_out, level);
i2c->device_out = level;
-//DPRINTF("%d %d %d\n", i2c->last_clock, i2c->last_data, i2c->device_out);
+
return level & i2c->last_data;
}
@@ -137,11 +131,11 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line,
int level)
if (i2c->current_addr < 0) {
i2c->current_addr = i2c->buffer;
-DPRINTF("Address 0x%02x\n", i2c->current_addr);
+trace_bitbang_i2c_addr(i2c->current_addr);
ret = i2c_start_transfer(i2c->bus, i2c->current_addr >> 1,
i2c->current_addr & 1);
} else {
-DPRINTF("Sent 0x%02x\n", i2c->buffer);
+trace_bitbang_i2c_send(i2c->buffer);
ret = i2c_send(i2c->bus, i2c->buffer);
}
if (ret) {
@@ -161,7 +155,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line,
int level)
}
case RECEIVING_BIT7:
i2c->buffer = i2c_recv(i2c->bus);
-DPRINTF("RX byte 0x%02x\n", i2c->buffer);
+trace_bitbang_i2c_recv(i2c->buffer);
/* Fall through... */
case RECEIVING_BIT6 ... RECEIVING_BIT0:
data = i2c->buffer >> 7;
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index 48aee4887c4..8e88aa24c1a 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -2,6 +2,10 @@
# bitbang_i2c.c
bitbang_i2c_state(const char *old_state, const char *new_state) "state %s ->
%s"
+bitbang_i2c_addr(uint8_t addr) "Address 0x%02x"
+bitbang_i2c_send(uint8_t byte) "TX byte 0x%02x"
+bitbang_i2c_recv(uint8_t byte) "RX byte 0x%02x"
+bitbang_i2c_data(unsigned dat, unsigned clk, unsigned old_out, unsigned
new_out) "dat %u clk %u out %u -> %u"
# core.c
--
2.34.1
[PULL 22/26] target/arm: Don't set EXC_RETURN.ES if Security Extension not present
In v7m_exception_taken(), for v8M we set the EXC_RETURN.ES bit if
either the exception targets Secure or if the CPU doesn't implement
the Security Extension. This is incorrect: the v8M Arm ARM specifies
that the ES bit should be RES0 if the Security Extension is not
implemented, and the pseudocode agrees.
Remove the incorrect condition, so that we leave the ES bit 0
if the Security Extension isn't implemented.
This doesn't have any guest-visible effects for our current set of
emulated CPUs, because all our v8M CPUs implement the Security
Extension; but it's worth fixing in case we add a v8M CPU without
the extension in future.
Reported-by: Igor Kotrasinski
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
---
target/arm/m_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index d87b9ecd123..e7e746ea182 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -879,7 +879,7 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr,
bool dotailchain,
}
lr &= ~R_V7M_EXCRET_ES_MASK;
-if (targets_secure || !arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+if (targets_secure) {
lr |= R_V7M_EXCRET_ES_MASK;
}
lr &= ~R_V7M_EXCRET_SPSEL_MASK;
--
2.34.1
Re: [PATCH v4 1/2] arm/kvm: add support for MTE
Hi Connie,
On 1/11/23 17:13, Cornelia Huck wrote:
> Introduce a new cpu feature flag to control MTE support. To preserve
> backwards compatibility for tcg, MTE will continue to be enabled as
> long as tag memory has been provided.
>
> If MTE has been enabled, we need to disable migration, as we do not
this only applies to KVM acceleration
> yet have a way to migrate the tags as well. Therefore, MTE will stay
> off with KVM unless requested explicitly.
>
> Signed-off-by: Cornelia Huck
> ---
> docs/system/arm/cpu-features.rst | 21 +
> hw/arm/virt.c| 2 +-
> target/arm/cpu.c | 18 ++---
> target/arm/cpu.h | 1 +
> target/arm/cpu64.c | 133 +++
> target/arm/internals.h | 1 +
> target/arm/kvm64.c | 5 ++
> target/arm/kvm_arm.h | 12 +++
> target/arm/monitor.c | 1 +
> 9 files changed, 181 insertions(+), 13 deletions(-)
>
> diff --git a/docs/system/arm/cpu-features.rst
> b/docs/system/arm/cpu-features.rst
> index 00c444042ff5..e278650c837e 100644
> --- a/docs/system/arm/cpu-features.rst
> +++ b/docs/system/arm/cpu-features.rst
> @@ -443,3 +443,24 @@ As with ``sve-default-vector-length``, if the default
> length is larger
> than the maximum vector length enabled, the actual vector length will
> be reduced. If this property is set to ``-1`` then the default vector
> length is set to the maximum possible length.
> +
> +MTE CPU Property
> +
> +
> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it
> requires
> +presence of tag memory (which can be turned on for the ``virt`` machine via
> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
> +proper migration support is implemented, enabling MTE will install a
> migration
> +blocker.
maybe re-emphasize: when KVM is enabled
> +
> +If not specified explicitly via ``on`` or ``off``, MTE will be available
> +according to the following rules:
> +
> +* When TCG is used, MTE will be available iff tag memory is available; i.e.
> it
suggestion: is available at machine level
> + preserves the behaviour prior to introduction of the feature.
s/prior to/prior to the ?
> +
> +* When KVM is used, MTE will default to off, so that migration will not
> + unintentionally be blocked.
> +
> +* Other accelerators currently don't support MTE.
> +
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ea2413a0bad7..42359e256ad0 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2136,7 +2136,7 @@ static void machvirt_init(MachineState *machine)
>
> if (vms->mte && (kvm_enabled() || hvf_enabled())) {
> error_report("mach-virt: %s does not support providing "
> - "MTE to the guest CPU",
> + "emulated MTE to the guest CPU",
each time I read this message I feel difficult to understand it. Why not
replacing by
"mach-virt does not support tag memory with %s acceleration" or
something alike?
> kvm_enabled() ? "KVM" : "HVF");
> exit(1);
> }
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 5f63316dbf22..decab743d0d5 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1529,6 +1529,11 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error
> **errp)
> error_propagate(errp, local_err);
> return;
> }
> +arm_cpu_mte_finalize(cpu, &local_err);
> +if (local_err != NULL) {
> +error_propagate(errp, local_err);
> +return;
> +}
> }
> #endif
>
> @@ -1605,7 +1610,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> }
> if (cpu->tag_memory) {
> error_setg(errp,
> - "Cannot enable %s when guest CPUs has MTE enabled",
> + "Cannot enable %s when guest CPUs has tag memory
> enabled",
> current_accel_name());
> return;
> }
> @@ -1984,17 +1989,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> ID_PFR1, VIRTUALIZATION, 0);
> }
>
> -#ifndef CONFIG_USER_ONLY
> -if (cpu->tag_memory == NULL && cpu_isar_feature(aa64_mte, cpu)) {
> -/*
> - * Disable the MTE feature bits if we do not have tag-memory
> - * provided by the machine.
> - */
> -cpu->isar.id_aa64pfr1 =
> -FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 0);
> -}
> -#endif
> -
> if (tcg_enabled()) {
> /*
> * Don't report the Statistical Profiling Extension in the ID
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index bf2bce046d56..f1a9015a7ed7 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1038,6 +1038,7 @@ struct ArchCPU {
> bool prop_pauth;
> bool prop_pauth_impdef;
> bool prop_lpa2;
> +OnOffAuto p
Re: [PATCH v6 5/5] riscv: Introduce satp mode hw capabilities
On Mon, Jan 23, 2023 at 10:03:24AM +0100, Alexandre Ghiti wrote:
> Currently, the max satp mode is set with the only constraint that it must be
> implemented in qemu, i.e. set in valid_vm_1_10_[32|64].
>
> But we actually need to add another level of constraint: what the hw is
> actually capable of, because currently, a linux booting on a sifive-u54
> boots in sv57 mode which is incompatible with the cpu's sv39 max
> capability.
>
> So add a new bitmap to RISCVSATPMap which contains this capability and
> initialize it in every XXX_cpu_init.
>
> Finally, we have the following chain of constraints:
>
> Qemu capability > HW capability > User choice > Software capability
>
> Signed-off-by: Alexandre Ghiti
> ---
> target/riscv/cpu.c | 78 +++---
> target/riscv/cpu.h | 8 +++--
> 2 files changed, 59 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e409e6ab64..19a37fee2b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -292,24 +292,39 @@ const char *satp_mode_str(uint8_t satp_mode, bool
> is_32_bit)
> g_assert_not_reached();
> }
>
> -/* Sets the satp mode to the max supported */
> -static void set_satp_mode_default(RISCVCPU *cpu, bool is_32_bit)
> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> +const char *satp_mode_str,
> +bool is_32_bit)
I'd drop 'is_32_bit' and get it from 'cpu', which would "clean up" all the
callsites by getting rid of all the true/false stuff. Also, why take the
string instead of the VM_1_10_SV* define?
> {
> -if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> -cpu->cfg.satp_mode.map |=
> -(1 << satp_mode_from_str(is_32_bit ? "sv32" :
> "sv57"));
> -} else {
> -cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> +uint8_t satp_mode = satp_mode_from_str(satp_mode_str);
> +const bool *valid_vm = is_32_bit ? valid_vm_1_10_32 : valid_vm_1_10_64;
> +
> +for (int i = 0; i <= satp_mode; ++i) {
> +if (valid_vm[i]) {
> +cpu->cfg.satp_mode.supported |= (1 << i);
> +}
> }
> }
>
> +/* Sets the satp mode to the max supported */
> +static void set_satp_mode_default(RISCVCPU *cpu)
> +{
> +uint8_t satp_mode = satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
> +
> +cpu->cfg.satp_mode.map |= (1 << satp_mode);
Let's do 'cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported' to make
sure 'map' has all supported bits set for property probing.
> +}
> +
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> +RISCVCPU *cpu = RISCV_CPU(obj);
> +
> #if defined(TARGET_RISCV32)
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> +set_satp_mode_max_supported(cpu, "sv32", true);
> #elif defined(TARGET_RISCV64)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> +set_satp_mode_max_supported(cpu, "sv57", false);
> #endif
> set_priv_version(env, PRIV_VERSION_1_12_0);
> register_cpu_props(obj);
> @@ -319,18 +334,24 @@ static void riscv_any_cpu_init(Object *obj)
> static void rv64_base_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> +RISCVCPU *cpu = RISCV_CPU(obj);
> +
> /* We set this in the realise function */
> set_misa(env, MXL_RV64, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> +set_satp_mode_max_supported(cpu, "sv57", false);
> }
>
> static void rv64_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> +RISCVCPU *cpu = RISCV_CPU(obj);
> +
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> +set_satp_mode_max_supported(cpu, "sv39", false);
> }
>
> static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -341,6 +362,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
> set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> cpu->cfg.mmu = false;
> +set_satp_mode_max_supported(cpu, "mbare", false);
> }
>
> static void rv128_base_cpu_init(Object *obj)
> @@ -352,11 +374,13 @@ static void rv128_base_cpu_init(Object *obj)
> exit(EXIT_FAILURE);
> }
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> +RISCVCPU *cpu = RISCV_CPU(obj);
> /* We set this in the realise function */
> set_misa(env, MXL_RV128, 0);
> register_cpu_props(obj);
> /* Set latest version of privileged specification */
> set_priv_version(env, PRIV_VERSION_1_12_0);
> +set_satp_mode_max_supported(cpu, "sv57", false);
> }
> #else
> static void rv32_base_cpu_init(Object *obj)
> @@ -367,13 +391,17 @@ static void rv32_bas
[PULL 14/26] target/arm/sme: Rebuild hflags in set_pstate() helpers
From: Richard Henderson
Signed-off-by: Richard Henderson
Reviewed-by: Fabiano Rosas
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20230112102436.1913-3-phi...@linaro.org
Message-Id: <20230112004322.161330-1-richard.hender...@linaro.org>
[PMD: Split patch in multiple tiny steps]
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Maydell
---
target/arm/sme_helper.c| 2 ++
target/arm/translate-a64.c | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/arm/sme_helper.c b/target/arm/sme_helper.c
index f891306bb98..b5aefa3edaf 100644
--- a/target/arm/sme_helper.c
+++ b/target/arm/sme_helper.c
@@ -45,6 +45,7 @@ void helper_set_pstate_sm(CPUARMState *env, uint32_t i)
}
env->svcr ^= R_SVCR_SM_MASK;
arm_reset_sve_state(env);
+arm_rebuild_hflags(env);
}
void helper_set_pstate_za(CPUARMState *env, uint32_t i)
@@ -65,6 +66,7 @@ void helper_set_pstate_za(CPUARMState *env, uint32_t i)
if (i) {
memset(env->zarray, 0, sizeof(env->zarray));
}
+arm_rebuild_hflags(env);
}
void helper_sme_zero(CPUARMState *env, uint32_t imm, uint32_t svl)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 35cc851246f..035e63bdc51 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1855,7 +1855,6 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
if ((crm & 4) && i != s->pstate_za) {
gen_helper_set_pstate_za(cpu_env, tcg_constant_i32(i));
}
-gen_rebuild_hflags(s);
} else {
s->base.is_jmp = DISAS_NEXT;
}
--
2.34.1
[PULL 05/26] hw/i2c/bitbang_i2c: Change state calling bitbang_i2c_set_state() helper
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Acked-by: Corey Minyard
Message-id: 20230111085016.44551-4-phi...@linaro.org
Signed-off-by: Peter Maydell
---
hw/i2c/bitbang_i2c.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c
index e41cb63daa7..bf4b781393d 100644
--- a/hw/i2c/bitbang_i2c.c
+++ b/hw/i2c/bitbang_i2c.c
@@ -26,13 +26,19 @@ do { printf("bitbang_i2c: " fmt , ## __VA_ARGS__); } while
(0)
#define DPRINTF(fmt, ...) do {} while(0)
#endif
+static void bitbang_i2c_set_state(bitbang_i2c_interface *i2c,
+ bitbang_i2c_state state)
+{
+i2c->state = state;
+}
+
static void bitbang_i2c_enter_stop(bitbang_i2c_interface *i2c)
{
DPRINTF("STOP\n");
if (i2c->current_addr >= 0)
i2c_end_transfer(i2c->bus);
i2c->current_addr = -1;
-i2c->state = STOPPED;
+bitbang_i2c_set_state(i2c, STOPPED);
}
/* Set device data pin. */
@@ -69,7 +75,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int
level)
if (level == 0) {
DPRINTF("START\n");
/* START condition. */
-i2c->state = SENDING_BIT7;
+bitbang_i2c_set_state(i2c, SENDING_BIT7);
i2c->current_addr = -1;
} else {
/* STOP condition. */
@@ -96,7 +102,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line,
int level)
case SENDING_BIT7 ... SENDING_BIT0:
i2c->buffer = (i2c->buffer << 1) | data;
/* will end up in WAITING_FOR_ACK */
-i2c->state++;
+bitbang_i2c_set_state(i2c, i2c->state + 1);
return bitbang_i2c_ret(i2c, 1);
case WAITING_FOR_ACK:
@@ -117,13 +123,14 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line,
int level)
* device we were sending to decided to NACK us).
*/
DPRINTF("Got NACK\n");
+bitbang_i2c_set_state(i2c, SENT_NACK);
bitbang_i2c_enter_stop(i2c);
return bitbang_i2c_ret(i2c, 1);
}
if (i2c->current_addr & 1) {
-i2c->state = RECEIVING_BIT7;
+bitbang_i2c_set_state(i2c, RECEIVING_BIT7);
} else {
-i2c->state = SENDING_BIT7;
+bitbang_i2c_set_state(i2c, SENDING_BIT7);
}
return bitbang_i2c_ret(i2c, 0);
}
@@ -134,18 +141,18 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line,
int level)
case RECEIVING_BIT6 ... RECEIVING_BIT0:
data = i2c->buffer >> 7;
/* will end up in SENDING_ACK */
-i2c->state++;
+bitbang_i2c_set_state(i2c, i2c->state + 1);
i2c->buffer <<= 1;
return bitbang_i2c_ret(i2c, data);
case SENDING_ACK:
-i2c->state = RECEIVING_BIT7;
if (data != 0) {
DPRINTF("NACKED\n");
-i2c->state = SENT_NACK;
+bitbang_i2c_set_state(i2c, SENT_NACK);
i2c_nack(i2c->bus);
} else {
DPRINTF("ACKED\n");
+bitbang_i2c_set_state(i2c, RECEIVING_BIT7);
}
return bitbang_i2c_ret(i2c, 1);
}
--
2.34.1
