Re: [PATCH v1 0/4] HyperV: Synthetic Debugging device
On 04/02/2022, Jon Doron wrote:
Ping
This patchset adds support for the synthetic debugging device.
HyperV supports a special transport layer for the kernel debugger when
running in HyperV.
This patchset add supports for this device so you could have a setup
fast windows kernel debugging.
At this point of time, DHCP is not implmeneted so to set this up few
things need to be noted.
The scenario I used to test is having 2 VMs in the same virtual network
i.e a Debugger VM with the NIC:
-nic tap,model=virtio,mac=02:ca:01:01:01:01,script=/etc/qemu-ifup
And it's IP is going to be static 192.168.53.12
And the VM we want to debug, to which we need to have the englightments
and vmbus configured:
-cpu
host,hv-relaxed,hv_spinlocks=0x1fff,hv_time,+vmx,invtsc,hv-vapic,hv-vpindex,hv-synic,hv-syndbg
\
-device vmbus-bridge \
-device hv-syndbg,host_ip=192.168.53.12,host_port=5,use_hcalls=false \
-nic tap,model=virtio,mac=02:ca:01:01:01:02,script=/etc/qemu-ifup \
Then in the debuggee VM we would setup the kernel debugging in the
following way:
If the VM is older than Win8:
* Copy the proper platform kdvm.dll (make sure it's called kdvm.dll even if
platform is 32bit)
bcdedit /set {GUID} dbgtransport kdvm.dll
bcdedit /set {GUID} loadoptions host_ip="1.2.3.4",host_port="5",nodhcp
bcdedit /set {GUID} debug on
bcdedit /set {GUID} halbreakpoint on
Win8 and late:
bcdedit /dbgsettings net hostip:7.7.7.7 port:5 nodhcp
This is all the setup that is required to get the synthetic debugger
configured correctly.
Jon Doron (4):
hyperv: SControl is optional to enable SynIc
hyperv: Add definitions for syndbg
hyperv: Add support to process syndbg commands
hw: hyperv: Initial commit for Synthetic Debugging device
docs/hyperv.txt | 15 +
hw/hyperv/Kconfig| 5 +
hw/hyperv/hyperv.c | 475 +--
hw/hyperv/meson.build| 1 +
hw/hyperv/syndbg.c | 407 ++
include/hw/hyperv/hyperv-proto.h | 52
include/hw/hyperv/hyperv.h | 60
target/i386/cpu.c| 2 +
target/i386/cpu.h| 7 +
target/i386/kvm/hyperv-proto.h | 37 +++
target/i386/kvm/hyperv-stub.c| 6 +
target/i386/kvm/hyperv.c | 52 +++-
target/i386/kvm/kvm.c| 76 -
13 files changed, 1105 insertions(+), 90 deletions(-)
create mode 100644 hw/hyperv/syndbg.c
--
2.34.1
Re: [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map
On 2022/01/20 21:36, Peter Maydell wrote:
From: Marc Zyngier
Even when the VM is configured with highmem=off, the highest_gpa
field includes devices that are above the 4GiB limit.
Similarily, nothing seem to check that the memory is within
the limit set by the highmem=off option.
This leads to failures in virt_kvm_type() on systems that have
a crippled IPA range, as the reported IPA space is larger than
what it should be.
Instead, honor the user-specified limit to only use the devices
at the lowest end of the spectrum, and fail if we have memory
crossing the 4GiB limit.
Reviewed-by: Andrew Jones
Reviewed-by: Eric Auger
Signed-off-by: Marc Zyngier
Message-id: 20220114140741.1358263-4-...@kernel.org
Signed-off-by: Peter Maydell
---
hw/arm/virt.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 62bdce1eb4b..3b839ba78ba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1670,7 +1670,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState
*vms, int idx)
static void virt_set_memmap(VirtMachineState *vms)
{
MachineState *ms = MACHINE(vms);
-hwaddr base, device_memory_base, device_memory_size;
+hwaddr base, device_memory_base, device_memory_size, memtop;
int i;
vms->memmap = extended_memmap;
@@ -1697,7 +1697,11 @@ static void virt_set_memmap(VirtMachineState *vms)
device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
/* Base address of the high IO region */
-base = device_memory_base + ROUND_UP(device_memory_size, GiB);
+memtop = base = device_memory_base + ROUND_UP(device_memory_size, GiB);
+if (!vms->highmem && memtop > 4 * GiB) {
+error_report("highmem=off, but memory crosses the 4GiB limit\n");
+exit(EXIT_FAILURE);
+}
if (base < device_memory_base) {
error_report("maxmem/slots too huge");
exit(EXIT_FAILURE);
@@ -1714,7 +1718,7 @@ static void virt_set_memmap(VirtMachineState *vms)
vms->memmap[i].size = size;
base += size;
}
-vms->highest_gpa = base - 1;
+vms->highest_gpa = (vms->highmem ? base : memtop) - 1;
if (device_memory_size > 0) {
ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
ms->device_memory->base = device_memory_base;
Hi,
This breaks in a case where highmem is disabled but can have more than 4
GiB of RAM. M1 (Apple Silicon) actually can have 36-bit PA with HVF,
which is not enough for highmem MMIO but is enough to contain 32 GiB of RAM.
Where the magic number of 4 GiB / 32-bit came from? I also don't quite
understand what failures virt_kvm_type() had.
Regards,
Akihiko Odaki
[PATCH v2] target/arm: Support PSCI 1.1 and SMCCC 1.0
Support the latest PSCI on TCG and HVF. A 64-bit function called from
AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC
Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since
they do not implement mandatory functions.
Signed-off-by: Akihiko Odaki
---
hw/arm/boot.c | 12 +---
target/arm/cpu.c| 5 +++--
target/arm/hvf/hvf.c| 27 ++-
target/arm/kvm-consts.h | 8 ++--
target/arm/kvm64.c | 2 +-
target/arm/psci.c | 35 ---
6 files changed, 77 insertions(+), 12 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index b1e95978f26..0eeef94ceb5 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -488,9 +488,15 @@ static void fdt_add_psci_node(void *fdt)
}
qemu_fdt_add_subnode(fdt, "/psci");
-if (armcpu->psci_version == 2) {
-const char comp[] = "arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+if (armcpu->psci_version == QEMU_PSCI_VERSION_0_2 ||
+armcpu->psci_version == QEMU_PSCI_VERSION_1_1) {
+if (armcpu->psci_version == QEMU_PSCI_VERSION_0_2) {
+const char comp[] = "arm,psci-0.2\0arm,psci";
+qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+} else {
+const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci";
+qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+}
cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5a9c02a2561..307a83a7bb6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1110,11 +1110,12 @@ static void arm_cpu_initfn(Object *obj)
* picky DTB consumer will also provide a helpful error message.
*/
cpu->dtb_compatible = "qemu,unknown";
-cpu->psci_version = 1; /* By default assume PSCI v0.1 */
+cpu->psci_version = QEMU_PSCI_VERSION_0_1; /* By default assume PSCI v0.1
*/
cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
if (tcg_enabled() || hvf_enabled()) {
-cpu->psci_version = 2; /* TCG and HVF implement PSCI 0.2 */
+/* TCG and HVF implement PSCI 1.1 */
+cpu->psci_version = QEMU_PSCI_VERSION_1_1;
}
}
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 0dc96560d34..1701fb8bbdb 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -653,7 +653,7 @@ static bool hvf_handle_psci_call(CPUState *cpu)
switch (param[0]) {
case QEMU_PSCI_0_2_FN_PSCI_VERSION:
-ret = QEMU_PSCI_0_2_RET_VERSION_0_2;
+ret = QEMU_PSCI_VERSION_1_1;
break;
case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
ret = QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED; /* No trusted OS */
@@ -721,6 +721,31 @@ static bool hvf_handle_psci_call(CPUState *cpu)
case QEMU_PSCI_0_2_FN_MIGRATE:
ret = QEMU_PSCI_RET_NOT_SUPPORTED;
break;
+case QEMU_PSCI_1_0_FN_PSCI_FEATURES:
+switch (param[1]) {
+case QEMU_PSCI_0_2_FN_PSCI_VERSION:
+case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+case QEMU_PSCI_0_2_FN_AFFINITY_INFO:
+case QEMU_PSCI_0_2_FN64_AFFINITY_INFO:
+case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
+case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
+case QEMU_PSCI_0_1_FN_CPU_ON:
+case QEMU_PSCI_0_2_FN_CPU_ON:
+case QEMU_PSCI_0_2_FN64_CPU_ON:
+case QEMU_PSCI_0_1_FN_CPU_OFF:
+case QEMU_PSCI_0_2_FN_CPU_OFF:
+case QEMU_PSCI_0_1_FN_CPU_SUSPEND:
+case QEMU_PSCI_0_2_FN_CPU_SUSPEND:
+case QEMU_PSCI_0_2_FN64_CPU_SUSPEND:
+case QEMU_PSCI_1_0_FN_PSCI_FEATURES:
+ret = 0;
+break;
+case QEMU_PSCI_0_1_FN_MIGRATE:
+case QEMU_PSCI_0_2_FN_MIGRATE:
+default:
+ret = QEMU_PSCI_RET_NOT_SUPPORTED;
+}
+break;
default:
return false;
}
diff --git a/target/arm/kvm-consts.h b/target/arm/kvm-consts.h
index 580f1c1fee0..ee877aa3a5c 100644
--- a/target/arm/kvm-consts.h
+++ b/target/arm/kvm-consts.h
@@ -77,6 +77,8 @@ MISMATCH_CHECK(QEMU_PSCI_0_1_FN_MIGRATE, KVM_PSCI_FN_MIGRATE);
#define QEMU_PSCI_0_2_FN64_AFFINITY_INFO QEMU_PSCI_0_2_FN64(4)
#define QEMU_PSCI_0_2_FN64_MIGRATE QEMU_PSCI_0_2_FN64(5)
+#define QEMU_PSCI_1_0_FN_PSCI_FEATURES QEMU_PSCI_0_2_FN(10)
+
MISMATCH_CHECK(QEMU_PSCI_0_2_FN_CPU_SUSPEND, PSCI_0_2_FN_CPU_SUSPEND);
MISMATCH_CHECK(QEMU_PSCI_0_2_FN_CPU_OFF, PSCI_0_2_FN_CPU_OFF);
MISMATCH_CHECK(QEMU_PSCI_0_2_FN_CPU_ON, PSCI_0_2_FN_CPU_ON);
@@ -84,14 +86,16 @@ MISMATCH_CHECK(QEMU_PSCI_0_2_FN_MIGRATE,
PSCI_0_2_FN_MIGRATE);
MISMATCH_CHECK(QEMU_PSCI_0_2_FN64_CPU_SUSPEND, PSCI_0_2_FN64_CPU_SUSPEND);
MISMATCH_CHECK(QEMU_PSCI_0_2_FN64_CPU_ON, PSCI_0_2_FN64_CPU_ON);
MISMATCH_CHECK(QEMU_PSCI_0_2_FN64_MIGRATE, PSCI_0_2_FN64_MIGRATE);
+MISMATCH_CHECK(QEMU_PSCI_1_0_FN_PSCI_FEATURES, P
[PATCH 6/6] Revert "ui: factor out qemu_console_set_display_gl_ctx()"
This reverts commit 4f4181499170dcf80182745b319607802ea32896.
This eliminates the situation where a display is registered as a GL
context provider but not as a listener.
Signed-off-by: Akihiko Odaki
---
include/ui/console.h | 3 ---
ui/console.c | 22 --
2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 58722312534..760dde770d1 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -409,9 +409,6 @@ void graphic_hw_gl_block(QemuConsole *con, bool block);
void qemu_console_early_init(void);
-void qemu_console_set_display_gl_ctx(QemuConsole *con,
- DisplayChangeListener *dcl);
-
QemuConsole *qemu_console_lookup_by_index(unsigned int index);
QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
QemuConsole *qemu_console_lookup_by_device_name(const char *device_id,
diff --git a/ui/console.c b/ui/console.c
index f3d72655bb6..dce2ed3e1de 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1458,19 +1458,6 @@ static bool dpy_compatible_with(QemuConsole *con,
return true;
}
-void qemu_console_set_display_gl_ctx(QemuConsole *con,
- DisplayChangeListener *dcl)
-{
-/* display has opengl support */
-assert(dcl->con);
-if (dcl->con->gl) {
-fprintf(stderr, "can't register two opengl displays (%s, %s)\n",
-dcl->ops->dpy_name, dcl->con->gl->ops->dpy_name);
-exit(1);
-}
-dcl->con->gl = dcl;
-}
-
void register_displaychangelistener(DisplayChangeListener *dcl)
{
static const char nodev[] =
@@ -1481,7 +1468,14 @@ void
register_displaychangelistener(DisplayChangeListener *dcl)
assert(!dcl->ds);
if (dcl->ops->dpy_gl_ctx_create) {
-qemu_console_set_display_gl_ctx(dcl->con, dcl);
+/* display has opengl support */
+assert(dcl->con);
+if (dcl->con->gl) {
+error_report("can't register two opengl displays (%s, %s)\n",
+ dcl->ops->dpy_name, dcl->con->gl->ops->dpy_name);
+exit(1);
+}
+dcl->con->gl = dcl;
}
if (dcl->con) {
--
2.32.0 (Apple Git-132)
[PATCH 2/6] Revert "console: save current scanout details"
This reverts commit ebced091854517f063c46ce8e522ebc45e9bccdf.
This fixes the compatibility between egl-headless and displays
using console_select, most importantly vnc.
Signed-off-by: Akihiko Odaki
---
include/ui/console.h | 27 ---
ui/console.c | 165 ++-
2 files changed, 54 insertions(+), 138 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index f590819880b..eefd7e4dc1f 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -108,17 +108,6 @@ struct QemuConsoleClass {
#define QEMU_ALLOCATED_FLAG 0x01
#define QEMU_PLACEHOLDER_FLAG 0x02
-typedef struct ScanoutTexture {
-uint32_t backing_id;
-bool backing_y_0_top;
-uint32_t backing_width;
-uint32_t backing_height;
-uint32_t x;
-uint32_t y;
-uint32_t width;
-uint32_t height;
-} ScanoutTexture;
-
typedef struct DisplaySurface {
pixman_format_code_t format;
pixman_image_t *image;
@@ -189,22 +178,6 @@ typedef struct QemuDmaBuf {
bool draw_submitted;
} QemuDmaBuf;
-enum display_scanout {
-SCANOUT_NONE,
-SCANOUT_SURFACE,
-SCANOUT_TEXTURE,
-SCANOUT_DMABUF,
-};
-
-typedef struct DisplayScanout {
-enum display_scanout kind;
-union {
-/* DisplaySurface *surface; is kept in QemuConsole */
-ScanoutTexture texture;
-QemuDmaBuf *dmabuf;
-};
-} DisplayScanout;
-
typedef struct DisplayState DisplayState;
typedef struct DisplayGLCtx DisplayGLCtx;
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2cc..78583df9203 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -77,7 +77,6 @@ struct QemuConsole {
console_type_t console_type;
DisplayState *ds;
DisplaySurface *surface;
-DisplayScanout scanout;
int dcls;
DisplayGLCtx *gl;
int gl_block;
@@ -147,7 +146,6 @@ static void dpy_refresh(DisplayState *s);
static DisplayState *get_alloc_displaystate(void);
static void text_console_update_cursor_timer(void);
static void text_console_update_cursor(void *opaque);
-static bool displaychangelistener_has_dmabuf(DisplayChangeListener *dcl);
static void gui_update(void *opaque)
{
@@ -483,8 +481,6 @@ static void text_console_resize(QemuConsole *s)
TextCell *cells, *c, *c1;
int w1, x, y, last_width;
-assert(s->scanout.kind == SCANOUT_SURFACE);
-
last_width = s->width;
s->width = surface_width(s->surface) / FONT_WIDTH;
s->height = surface_height(s->surface) / FONT_HEIGHT;
@@ -1056,48 +1052,6 @@ static void console_putchar(QemuConsole *s, int ch)
}
}
-static void displaychangelistener_display_console(DisplayChangeListener *dcl,
- QemuConsole *con)
-{
-static const char nodev[] =
-"This VM has no graphic display device.";
-static DisplaySurface *dummy;
-
-if (!con) {
-if (!dcl->ops->dpy_gfx_switch) {
-return;
-}
-if (!dummy) {
-dummy = qemu_create_placeholder_surface(640, 480, nodev);
-}
-dcl->ops->dpy_gfx_switch(dcl, dummy);
-return;
-}
-
-if (con->scanout.kind == SCANOUT_DMABUF &&
-displaychangelistener_has_dmabuf(dcl)) {
-dcl->ops->dpy_gl_scanout_dmabuf(dcl, con->scanout.dmabuf);
-} else if (con->scanout.kind == SCANOUT_TEXTURE &&
- dcl->ops->dpy_gl_scanout_texture) {
-dcl->ops->dpy_gl_scanout_texture(dcl,
- con->scanout.texture.backing_id,
- con->scanout.texture.backing_y_0_top,
- con->scanout.texture.backing_width,
- con->scanout.texture.backing_height,
- con->scanout.texture.x,
- con->scanout.texture.y,
- con->scanout.texture.width,
- con->scanout.texture.height);
-} else if (con->scanout.kind == SCANOUT_SURFACE &&
- dcl->ops->dpy_gfx_switch) {
-dcl->ops->dpy_gfx_switch(dcl, con->surface);
-}
-
-dcl->ops->dpy_gfx_update(dcl, 0, 0,
- qemu_console_get_width(con, 0),
- qemu_console_get_height(con, 0));
-}
-
void console_select(unsigned int index)
{
DisplayChangeListener *dcl;
@@ -1114,7 +1068,13 @@ void console_select(unsigned int index)
if (dcl->con != NULL) {
continue;
}
-displaychangelistener_display_console(dcl, s);
+if (dcl->ops->dpy_gfx_switch) {
+dcl->ops->dpy_gfx_switch(dcl, s->surface);
+}
+}
+if (s->surface) {
+dpy_gfx_update(s, 0, 0, surface_width(s->surface),
+ surface_height(s->surface));
}
[PATCH 4/6] Revert "ui: dispatch GL events to all listeners"
This reverts commit 7cc712e9862ffdbe4161dbdf3bbf41bcbe547472.
Signed-off-by: Akihiko Odaki
---
ui/console.c | 58 +++-
1 file changed, 16 insertions(+), 42 deletions(-)
diff --git a/ui/console.c b/ui/console.c
index 13c0d001c09..6f21007737e 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1824,12 +1824,8 @@ int dpy_gl_ctx_make_current(QemuConsole *con,
QEMUGLContext ctx)
void dpy_gl_scanout_disable(QemuConsole *con)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
-
-QLIST_FOREACH(dcl, &s->listeners, next) {
-dcl->ops->dpy_gl_scanout_disable(dcl);
-}
+assert(con->gl);
+con->gl->ops->dpy_gl_scanout_disable(con->gl);
}
void dpy_gl_scanout_texture(QemuConsole *con,
@@ -1840,80 +1836,58 @@ void dpy_gl_scanout_texture(QemuConsole *con,
uint32_t x, uint32_t y,
uint32_t width, uint32_t height)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
-
-QLIST_FOREACH(dcl, &s->listeners, next) {
-dcl->ops->dpy_gl_scanout_texture(dcl, backing_id,
+assert(con->gl);
+con->gl->ops->dpy_gl_scanout_texture(con->gl, backing_id,
backing_y_0_top,
backing_width, backing_height,
x, y, width, height);
-}
}
void dpy_gl_scanout_dmabuf(QemuConsole *con,
QemuDmaBuf *dmabuf)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
-
-QLIST_FOREACH(dcl, &s->listeners, next) {
-dcl->ops->dpy_gl_scanout_dmabuf(dcl, dmabuf);
-}
+assert(con->gl);
+con->gl->ops->dpy_gl_scanout_dmabuf(con->gl, dmabuf);
}
void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf *dmabuf,
bool have_hot, uint32_t hot_x, uint32_t hot_y)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
+assert(con->gl);
-QLIST_FOREACH(dcl, &s->listeners, next) {
-if (dcl->ops->dpy_gl_cursor_dmabuf) {
-dcl->ops->dpy_gl_cursor_dmabuf(dcl, dmabuf,
+if (con->gl->ops->dpy_gl_cursor_dmabuf) {
+con->gl->ops->dpy_gl_cursor_dmabuf(con->gl, dmabuf,
have_hot, hot_x, hot_y);
-}
}
}
void dpy_gl_cursor_position(QemuConsole *con,
uint32_t pos_x, uint32_t pos_y)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
+assert(con->gl);
-QLIST_FOREACH(dcl, &s->listeners, next) {
-if (dcl->ops->dpy_gl_cursor_position) {
-dcl->ops->dpy_gl_cursor_position(dcl, pos_x, pos_y);
-}
+if (con->gl->ops->dpy_gl_cursor_position) {
+con->gl->ops->dpy_gl_cursor_position(con->gl, pos_x, pos_y);
}
}
void dpy_gl_release_dmabuf(QemuConsole *con,
QemuDmaBuf *dmabuf)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
+assert(con->gl);
-QLIST_FOREACH(dcl, &s->listeners, next) {
-if (dcl->ops->dpy_gl_release_dmabuf) {
-dcl->ops->dpy_gl_release_dmabuf(dcl, dmabuf);
-}
+if (con->gl->ops->dpy_gl_release_dmabuf) {
+con->gl->ops->dpy_gl_release_dmabuf(con->gl, dmabuf);
}
}
void dpy_gl_update(QemuConsole *con,
uint32_t x, uint32_t y, uint32_t w, uint32_t h)
{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
-
assert(con->gl);
graphic_hw_gl_block(con, true);
-QLIST_FOREACH(dcl, &s->listeners, next) {
-dcl->ops->dpy_gl_update(dcl, x, y, w, h);
-}
+con->gl->ops->dpy_gl_update(con->gl, x, y, w, h);
graphic_hw_gl_block(con, false);
}
--
2.32.0 (Apple Git-132)
[PATCH 5/6] Revert "ui: associate GL context outside of display listener registration"
This reverts commit ac32b2fff127843355b4f7e7ac9f93dd4a395adf.
Signed-off-by: Akihiko Odaki
---
ui/console.c | 7 ++-
ui/egl-headless.c | 1 -
ui/gtk.c | 3 ---
ui/sdl2.c | 3 ---
ui/spice-display.c | 3 ---
5 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/ui/console.c b/ui/console.c
index 6f21007737e..f3d72655bb6 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1480,11 +1480,8 @@ void
register_displaychangelistener(DisplayChangeListener *dcl)
assert(!dcl->ds);
-if (dcl->con && dcl->con->gl &&
-dcl->con->gl != dcl) {
-error_report("Display %s is incompatible with the GL context",
- dcl->ops->dpy_name);
-exit(1);
+if (dcl->ops->dpy_gl_ctx_create) {
+qemu_console_set_display_gl_ctx(dcl->con, dcl);
}
if (dcl->con) {
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index 08327c40c6e..a26a2520c49 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -197,7 +197,6 @@ static void egl_headless_init(DisplayState *ds,
DisplayOptions *opts)
edpy->dcl.con = con;
edpy->dcl.ops = &egl_ops;
edpy->gls = qemu_gl_init_shader();
-qemu_console_set_display_gl_ctx(con, &edpy->dcl);
register_displaychangelistener(&edpy->dcl);
}
}
diff --git a/ui/gtk.c b/ui/gtk.c
index ca29dde6cbd..b31e900ebda 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2112,9 +2112,6 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s,
VirtualConsole *vc,
vc->gfx.kbd = qkbd_state_init(con);
vc->gfx.dcl.con = con;
-if (display_opengl) {
-qemu_console_set_display_gl_ctx(con, &vc->gfx.dcl);
-}
register_displaychangelistener(&vc->gfx.dcl);
gd_connect_vc_gfx_signals(vc);
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 628ae33245f..4117b4ac6e7 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -871,9 +871,6 @@ static void sdl2_display_init(DisplayState *ds,
DisplayOptions *o)
#endif
sdl2_console[i].dcl.con = con;
sdl2_console[i].kbd = qkbd_state_init(con);
-if (display_opengl) {
-qemu_console_set_display_gl_ctx(con, &sdl2_console[i].dcl);
-}
register_displaychangelistener(&sdl2_console[i].dcl);
#if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index bf057bc95f5..07234d49594 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -1158,9 +1158,6 @@ static void qemu_spice_display_init_one(QemuConsole *con)
qemu_spice_create_host_memslot(ssd);
-if (spice_opengl) {
-qemu_console_set_display_gl_ctx(con, &ssd->dcl);
-}
register_displaychangelistener(&ssd->dcl);
}
--
2.32.0 (Apple Git-132)
[PATCH 1/6] ui/dbus: Share one listener for a console
This fixes surface texture double free with multiple connections and
out-of-sync display size with multiple displays.
This also reduces resource usage a little and allows to remove code to
support multiple listeners for OpenGL displays.
Signed-off-by: Akihiko Odaki
---
ui/dbus-console.c | 109 +++-
ui/dbus-listener.c | 401 +
ui/dbus.h | 32 +++-
3 files changed, 344 insertions(+), 198 deletions(-)
diff --git a/ui/dbus-console.c b/ui/dbus-console.c
index e062f721d76..ec035c427db 100644
--- a/ui/dbus-console.c
+++ b/ui/dbus-console.c
@@ -33,11 +33,10 @@
struct _DBusDisplayConsole {
GDBusObjectSkeleton parent_instance;
-DisplayChangeListener dcl;
DBusDisplay *display;
QemuConsole *con;
-GHashTable *listeners;
+DBusDisplayListener *listener;
QemuDBusDisplay1Console *iface;
QemuDBusDisplay1Keyboard *iface_kbd;
@@ -54,7 +53,7 @@ G_DEFINE_TYPE(DBusDisplayConsole,
dbus_display_console,
G_TYPE_DBUS_OBJECT_SKELETON)
-static void
+void
dbus_display_console_set_size(DBusDisplayConsole *ddc,
uint32_t width, uint32_t height)
{
@@ -64,78 +63,9 @@ dbus_display_console_set_size(DBusDisplayConsole *ddc,
NULL);
}
-static void
-dbus_gfx_switch(DisplayChangeListener *dcl,
-struct DisplaySurface *new_surface)
-{
-DBusDisplayConsole *ddc = container_of(dcl, DBusDisplayConsole, dcl);
-
-dbus_display_console_set_size(ddc,
- surface_width(new_surface),
- surface_height(new_surface));
-}
-
-static void
-dbus_gfx_update(DisplayChangeListener *dcl,
-int x, int y, int w, int h)
-{
-}
-
-static void
-dbus_gl_scanout_disable(DisplayChangeListener *dcl)
-{
-}
-
-static void
-dbus_gl_scanout_texture(DisplayChangeListener *dcl,
-uint32_t tex_id,
-bool backing_y_0_top,
-uint32_t backing_width,
-uint32_t backing_height,
-uint32_t x, uint32_t y,
-uint32_t w, uint32_t h)
-{
-DBusDisplayConsole *ddc = container_of(dcl, DBusDisplayConsole, dcl);
-
-dbus_display_console_set_size(ddc, w, h);
-}
-
-static void
-dbus_gl_scanout_dmabuf(DisplayChangeListener *dcl,
- QemuDmaBuf *dmabuf)
-{
-DBusDisplayConsole *ddc = container_of(dcl, DBusDisplayConsole, dcl);
-
-dbus_display_console_set_size(ddc,
- dmabuf->width,
- dmabuf->height);
-}
-
-static void
-dbus_gl_scanout_update(DisplayChangeListener *dcl,
- uint32_t x, uint32_t y,
- uint32_t w, uint32_t h)
-{
-}
-
-static const DisplayChangeListenerOps dbus_console_dcl_ops = {
-.dpy_name= "dbus-console",
-.dpy_gfx_switch = dbus_gfx_switch,
-.dpy_gfx_update = dbus_gfx_update,
-.dpy_gl_scanout_disable = dbus_gl_scanout_disable,
-.dpy_gl_scanout_texture = dbus_gl_scanout_texture,
-.dpy_gl_scanout_dmabuf = dbus_gl_scanout_dmabuf,
-.dpy_gl_update = dbus_gl_scanout_update,
-};
-
static void
dbus_display_console_init(DBusDisplayConsole *object)
{
-DBusDisplayConsole *ddc = DBUS_DISPLAY_CONSOLE(object);
-
-ddc->listeners = g_hash_table_new_full(g_str_hash, g_str_equal,
-NULL, g_object_unref);
-ddc->dcl.ops = &dbus_console_dcl_ops;
}
static void
@@ -143,10 +73,10 @@ dbus_display_console_dispose(GObject *object)
{
DBusDisplayConsole *ddc = DBUS_DISPLAY_CONSOLE(object);
-unregister_displaychangelistener(&ddc->dcl);
g_clear_object(&ddc->iface_kbd);
g_clear_object(&ddc->iface);
-g_clear_pointer(&ddc->listeners, g_hash_table_unref);
+dbus_display_listener_unref_all_connections(ddc->listener);
+g_clear_object(&ddc->listener);
g_clear_pointer(&ddc->kbd, qkbd_state_free);
G_OBJECT_CLASS(dbus_display_console_parent_class)->dispose(object);
@@ -161,14 +91,14 @@ dbus_display_console_class_init(DBusDisplayConsoleClass
*klass)
}
static void
-listener_vanished_cb(DBusDisplayListener *listener)
+listener_vanished_cb(DBusDisplayListenerConnection *ddlc)
{
-DBusDisplayConsole *ddc = dbus_display_listener_get_console(listener);
-const char *name = dbus_display_listener_get_bus_name(listener);
+DBusDisplayConsole *ddc =
dbus_display_listener_connection_get_console(ddlc);
+const char *name = dbus_display_listener_connection_get_bus_name(ddlc);
trace_dbus_listener_vanished(name);
-g_hash_table_remove(ddc->listeners, name);
+g_object_unref(ddlc);
qkbd_state_lift_all_keys(ddc->kbd);
}
@@ -211,15 +141,15 @@ dbus_console_register_listener(DBusDisplayConsole *ddc,
GVariant *arg_listener)
{
c
[PATCH 3/6] Revert "ui: split the GL context in a different object"
This reverts commit 5e79d516e8ac818d2a90aae9f787775055434ee9.
Signed-off-by: Akihiko Odaki
---
include/ui/console.h | 34 --
include/ui/egl-context.h | 6 +++---
include/ui/gtk.h | 11 +--
include/ui/sdl2.h | 7 +++
include/ui/spice-display.h | 1 -
ui/console.c | 26 ++
ui/dbus-listener.c | 16 ++--
ui/dbus.c | 22 --
ui/dbus.h | 4
ui/egl-context.c | 6 +++---
ui/egl-headless.c | 21 +++--
ui/gtk-egl.c | 10 +-
ui/gtk-gl-area.c | 8
ui/gtk.c | 24
ui/sdl2-gl.c | 10 +-
ui/sdl2.c | 13 -
ui/spice-display.c | 18 +++---
17 files changed, 90 insertions(+), 147 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index eefd7e4dc1f..58722312534 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -179,7 +179,6 @@ typedef struct QemuDmaBuf {
} QemuDmaBuf;
typedef struct DisplayState DisplayState;
-typedef struct DisplayGLCtx DisplayGLCtx;
typedef struct DisplayChangeListenerOps {
const char *dpy_name;
@@ -214,6 +213,16 @@ typedef struct DisplayChangeListenerOps {
void (*dpy_cursor_define)(DisplayChangeListener *dcl,
QEMUCursor *cursor);
+/* required if GL */
+QEMUGLContext (*dpy_gl_ctx_create)(DisplayChangeListener *dcl,
+ QEMUGLParams *params);
+/* required if GL */
+void (*dpy_gl_ctx_destroy)(DisplayChangeListener *dcl,
+ QEMUGLContext ctx);
+/* required if GL */
+int (*dpy_gl_ctx_make_current)(DisplayChangeListener *dcl,
+ QEMUGLContext ctx);
+
/* required if GL */
void (*dpy_gl_scanout_disable)(DisplayChangeListener *dcl);
/* required if GL */
@@ -254,26 +263,6 @@ struct DisplayChangeListener {
QLIST_ENTRY(DisplayChangeListener) next;
};
-typedef struct DisplayGLCtxOps {
-/*
- * We only check if the GLCtx is compatible with a DCL via ops. A natural
- * evolution of this would be a callback to check some runtime requirements
- * and allow various DCL kinds.
- */
-const DisplayChangeListenerOps *compatible_dcl;
-
-QEMUGLContext (*dpy_gl_ctx_create)(DisplayGLCtx *dgc,
- QEMUGLParams *params);
-void (*dpy_gl_ctx_destroy)(DisplayGLCtx *dgc,
- QEMUGLContext ctx);
-int (*dpy_gl_ctx_make_current)(DisplayGLCtx *dgc,
- QEMUGLContext ctx);
-} DisplayGLCtxOps;
-
-struct DisplayGLCtx {
-const DisplayGLCtxOps *ops;
-};
-
DisplayState *init_displaystate(void);
DisplaySurface *qemu_create_displaysurface_from(int width, int height,
pixman_format_code_t format,
@@ -420,7 +409,8 @@ void graphic_hw_gl_block(QemuConsole *con, bool block);
void qemu_console_early_init(void);
-void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *ctx);
+void qemu_console_set_display_gl_ctx(QemuConsole *con,
+ DisplayChangeListener *dcl);
QemuConsole *qemu_console_lookup_by_index(unsigned int index);
QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
diff --git a/include/ui/egl-context.h b/include/ui/egl-context.h
index c2761d747a4..9374fe41e32 100644
--- a/include/ui/egl-context.h
+++ b/include/ui/egl-context.h
@@ -4,10 +4,10 @@
#include "ui/console.h"
#include "ui/egl-helpers.h"
-QEMUGLContext qemu_egl_create_context(DisplayGLCtx *dgc,
+QEMUGLContext qemu_egl_create_context(DisplayChangeListener *dcl,
QEMUGLParams *params);
-void qemu_egl_destroy_context(DisplayGLCtx *dgc, QEMUGLContext ctx);
-int qemu_egl_make_context_current(DisplayGLCtx *dgc,
+void qemu_egl_destroy_context(DisplayChangeListener *dcl, QEMUGLContext ctx);
+int qemu_egl_make_context_current(DisplayChangeListener *dcl,
QEMUGLContext ctx);
#endif /* EGL_CONTEXT_H */
diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 101b147d1b9..7d22affd381 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -35,7 +35,6 @@ typedef struct GtkDisplayState GtkDisplayState;
typedef struct VirtualGfxConsole {
GtkWidget *drawing_area;
-DisplayGLCtx dgc;
DisplayChangeListener dcl;
QKbdState *kbd;
DisplaySurface *ds;
@@ -166,7 +165,7 @@ void gd_egl_update(DisplayChangeListener *dcl,
void gd_egl_refresh(DisplayChangeListener *dcl);
void gd_egl_switch(DisplayChangeListener *dcl,
DisplaySurface *surface);
-QEMUGLContext gd_egl_create_context(DisplayGLCtx *dgc,
+QEMUGLContext gd_egl_create_contex
[PATCH 0/6] ui/dbus: Share one listener for a console
ui/dbus required to have multiple DisplayChangeListeners (possibly with OpenGL) for a console but that caused several problems: - It broke egl-headless, an unusual display which implements OpenGL rendering for non-OpenGL displays. The code to support multiple DisplayChangeListeners does not consider the case where non-OpenGL displays listens OpenGL consoles. - Multiple OpenGL DisplayChangeListeners of dbus shares one DisplaySurface and modifies its texture field, causing OpenGL texture leak and use-after-free. - Introduced extra code to check the compatibility of OpenGL context providers and OpenGL texture renderers where those are often inherently tightly coupled since they must share the same hardware. - Needed extra work to broadcast the same change to multiple dbus listeners. This series solve them by implementing the change broadcast in ui/dbus, which knows exactly what is needed. Changes for the common code to support multiple OpenGL DisplayChangeListeners were reverted to fix egl-headless and reduce the code size. Akihiko Odaki (6): ui/dbus: Share one listener for a console Revert "console: save current scanout details" Revert "ui: split the GL context in a different object" Revert "ui: dispatch GL events to all listeners" Revert "ui: associate GL context outside of display listener registration" Revert "ui: factor out qemu_console_set_display_gl_ctx()" include/ui/console.h | 60 +- include/ui/egl-context.h | 6 +- include/ui/gtk.h | 11 +- include/ui/sdl2.h | 7 +- include/ui/spice-display.h | 1 - ui/console.c | 258 +++ ui/dbus-console.c | 109 ++ ui/dbus-listener.c | 417 +++-- ui/dbus.c | 22 -- ui/dbus.h | 36 +++- ui/egl-context.c | 6 +- ui/egl-headless.c | 20 +- ui/gtk-egl.c | 10 +- ui/gtk-gl-area.c | 8 +- ui/gtk.c | 25 +-- ui/sdl2-gl.c | 10 +- ui/sdl2.c | 14 +- ui/spice-display.c | 19 +- 18 files changed, 498 insertions(+), 541 deletions(-) -- 2.32.0 (Apple Git-132)
[PATCH] softmmu/qdev-monitor: Add virtio-gpu-gl aliases
Signed-off-by: Akihiko Odaki
---
softmmu/qdev-monitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 01f3834db57..a0df820b9de 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -83,6 +83,8 @@ static const QDevAlias qdev_alias_table[] = {
{ "virtio-gpu-device", "virtio-gpu", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-gpu-gl-device", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_MMIO },
+{ "virtio-gpu-gl-pci", "virtio-gpu-gl", QEMU_ARCH_VIRTIO_PCI },
{ "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI },
--
2.32.0 (Apple Git-132)
[PATCH] edid: Fix clock of Detailed Timing Descriptor
The clock field is 16-bits in EDID Detailed Timing Descriptor, but
edid_desc_timing assumed it is 32-bit. Write the 16-bit value if it fits
in 16-bit. Write DisplayID otherwise.
Signed-off-by: Akihiko Odaki
---
hw/display/edid-generate.c | 66 ++
1 file changed, 32 insertions(+), 34 deletions(-)
diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index bccf32af69c..2cb819675e0 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -255,33 +255,31 @@ static void edid_desc_dummy(uint8_t *desc)
edid_desc_type(desc, 0x10);
}
-static void edid_desc_timing(uint8_t *desc, uint32_t refresh_rate,
+static void edid_desc_timing(uint8_t *desc, const Timings *timings,
uint32_t xres, uint32_t yres,
uint32_t xmm, uint32_t ymm)
{
-Timings timings;
-generate_timings(&timings, refresh_rate, xres, yres);
-stl_le_p(desc, timings.clock);
+stw_le_p(desc, timings->clock);
desc[2] = xres & 0xff;
-desc[3] = timings.xblank & 0xff;
+desc[3] = timings->xblank & 0xff;
desc[4] = (((xres & 0xf00) >> 4) |
- ((timings.xblank & 0xf00) >> 8));
+ ((timings->xblank & 0xf00) >> 8));
desc[5] = yres & 0xff;
-desc[6] = timings.yblank & 0xff;
+desc[6] = timings->yblank & 0xff;
desc[7] = (((yres & 0xf00) >> 4) |
- ((timings.yblank & 0xf00) >> 8));
+ ((timings->yblank & 0xf00) >> 8));
-desc[8] = timings.xfront & 0xff;
-desc[9] = timings.xsync & 0xff;
+desc[8] = timings->xfront & 0xff;
+desc[9] = timings->xsync & 0xff;
-desc[10] = (((timings.yfront & 0x00f) << 4) |
-((timings.ysync & 0x00f) << 0));
-desc[11] = (((timings.xfront & 0x300) >> 2) |
-((timings.xsync & 0x300) >> 4) |
-((timings.yfront & 0x030) >> 2) |
-((timings.ysync & 0x030) >> 4));
+desc[10] = (((timings->yfront & 0x00f) << 4) |
+((timings->ysync & 0x00f) << 0));
+desc[11] = (((timings->xfront & 0x300) >> 2) |
+((timings->xsync & 0x300) >> 4) |
+((timings->yfront & 0x030) >> 2) |
+((timings->ysync & 0x030) >> 4));
desc[12] = xmm & 0xff;
desc[13] = ymm & 0xff;
@@ -348,13 +346,10 @@ static void init_displayid(uint8_t *did)
edid_checksum(did + 1, did[2] + 4);
}
-static void qemu_displayid_generate(uint8_t *did, uint32_t refresh_rate,
+static void qemu_displayid_generate(uint8_t *did, const Timings *timings,
uint32_t xres, uint32_t yres,
uint32_t xmm, uint32_t ymm)
{
-Timings timings;
-generate_timings(&timings, refresh_rate, xres, yres);
-
did[0] = 0x70; /* display id extension */
did[1] = 0x13; /* version 1.3 */
did[2] = 23; /* length */
@@ -364,21 +359,21 @@ static void qemu_displayid_generate(uint8_t *did,
uint32_t refresh_rate,
did[6] = 0x00; /* revision */
did[7] = 0x14; /* block length */
-did[8] = timings.clock & 0xff;
-did[9] = (timings.clock & 0xff00) >> 8;
-did[10] = (timings.clock & 0xff) >> 16;
+did[8] = timings->clock & 0xff;
+did[9] = (timings->clock & 0xff00) >> 8;
+did[10] = (timings->clock & 0xff) >> 16;
did[11] = 0x88; /* leave aspect ratio undefined */
stw_le_p(did + 12, 0x & (xres - 1));
-stw_le_p(did + 14, 0x & (timings.xblank - 1));
-stw_le_p(did + 16, 0x & (timings.xfront - 1));
-stw_le_p(did + 18, 0x & (timings.xsync - 1));
+stw_le_p(did + 14, 0x & (timings->xblank - 1));
+stw_le_p(did + 16, 0x & (timings->xfront - 1));
+stw_le_p(did + 18, 0x & (timings->xsync - 1));
stw_le_p(did + 20, 0x & (yres - 1));
-stw_le_p(did + 22, 0x & (timings.yblank - 1));
-stw_le_p(did + 24, 0x & (timings.yfront - 1));
-stw_le_p(did + 26, 0x & (timings.ysync - 1));
+stw_le_p(did + 22, 0x & (timings->yblank - 1));
+stw_le_p(did + 24, 0x & (timings->yfront - 1));
+stw_le_p(did + 26, 0x & (timings->ysync - 1));
edid_checksum(did + 1, did[2] + 4);
}
@@ -386,6 +381,7 @@ static void qemu_displayid_generate(uint8_t *did, uint32_t
refresh_rate,
void qemu_edid_generate(uint8_t *edid, size_t size,
qemu_edid_info *info)
{
+Timings timings;
uint8_t *desc = edid + 54;
uint8_t *xtra3 = NULL;
uint8_t *dta = NULL;
@@ -409,9 +405,6 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
if (!info->prefy) {
info->prefy = 800;
}
-if (info->prefx >= 4096 || info->prefy >= 4096) {
-large_screen = 1;
-}
if (info->width_mm && info->height_mm) {
width_mm = info->width_mm;
height_mm = info->height_mm;
@@ -421,6 +414,11 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
[PATCH] ui/cocoa: Do not alert even without block devices
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 5 -
1 file changed, 5 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index ac18e14ce01..271a2676026 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1715,11 +1715,6 @@ static void addRemovableDevicesMenuItems(void)
currentDevice = qmp_query_block(NULL);
pointerToFree = currentDevice;
-if(currentDevice == NULL) {
-NSBeep();
-QEMU_Alert(@"Failed to query for block devices!");
-return;
-}
menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
--
2.32.0 (Apple Git-132)
[PATCH] ui/cocoa: Fix the leak of qemu_console_get_label
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index ac18e14ce01..fdf52a7c2f7 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1680,7 +1680,10 @@ static void create_initial_menus(void)
/* Returns a name for a given console */
static NSString * getConsoleName(QemuConsole * console)
{
-return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
+char *label = qemu_console_get_label(console);
+NSString *nslabel = [NSString stringWithUTF8String:label];
+g_free(label);
+return nslabel;
}
/* Add an entry to the View menu for each console */
--
2.32.0 (Apple Git-132)
[PATCH] MAINTAINERS: Add Akihiko Odaki to macOS-relateds
Signed-off-by: Akihiko Odaki --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 2fd74c46426..5aefb5b431a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2333,6 +2333,7 @@ F: audio/alsaaudio.c Core Audio framework backend M: Gerd Hoffmann R: Christian Schoenebeck +R: Akihiko Odaki S: Odd Fixes F: audio/coreaudio.c @@ -2585,6 +2586,7 @@ F: util/drm.c Cocoa graphics M: Peter Maydell +R: Akihiko Odaki S: Odd Fixes F: ui/cocoa.m -- 2.32.0 (Apple Git-132)
[PATCH] target/arm: Support PSCI 1.1 and SMCCC 1.0
Support the latest PSCI on TCG and HVF. It has optional functions and
none of them are implemented. Unimplemented functions now return
NOT_SUPPORTED, which automatically makes TCG compliant to SMC Calling
Convention 1.0. HVF had already complied to SMCCC 1.0 for the
compatibility with Windows and this change eliminates the inconsistency
between TCG and HVF.
Signed-off-by: Akihiko Odaki
---
hw/arm/boot.c | 12 +++--
target/arm/cpu.c | 5 +-
target/arm/helper.c| 4 +-
target/arm/hvf/hvf.c | 33 +++--
target/arm/internals.h | 29 +--
target/arm/kvm-consts.h| 8 ++-
target/arm/kvm64.c | 2 +-
target/arm/meson.build | 2 +-
target/arm/op_helper.c | 19 +++-
target/arm/{psci.c => smccc.c} | 89 +-
10 files changed, 116 insertions(+), 87 deletions(-)
rename target/arm/{psci.c => smccc.c} (76%)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 399f8e837ce..0269d2aba03 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -487,9 +487,15 @@ static void fdt_add_psci_node(void *fdt)
}
qemu_fdt_add_subnode(fdt, "/psci");
-if (armcpu->psci_version == 2) {
-const char comp[] = "arm,psci-0.2\0arm,psci";
-qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+if (armcpu->psci_version == QEMU_PSCI_VERSION_0_2 ||
+armcpu->psci_version == QEMU_PSCI_VERSION_1_1) {
+if (armcpu->psci_version == QEMU_PSCI_VERSION_0_2) {
+const char comp[] = "arm,psci-0.2\0arm,psci";
+qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+} else {
+const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci";
+qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+}
cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a211804fd3d..e108fb0abb8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1110,11 +1110,12 @@ static void arm_cpu_initfn(Object *obj)
* picky DTB consumer will also provide a helpful error message.
*/
cpu->dtb_compatible = "qemu,unknown";
-cpu->psci_version = 1; /* By default assume PSCI v0.1 */
+cpu->psci_version = QEMU_PSCI_VERSION_0_1; /* By default assume PSCI v0.1
*/
cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE;
if (tcg_enabled() || hvf_enabled()) {
-cpu->psci_version = 2; /* TCG and HVF implement PSCI 0.2 */
+/* TCG and HVF implement PSCI 1.1 */
+cpu->psci_version = QEMU_PSCI_VERSION_1_1;
}
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index cfca0f5ba6d..f844f4d41f7 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10195,8 +10195,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
env->exception.syndrome);
}
-if (arm_is_psci_call(cpu, cs->exception_index)) {
-arm_handle_psci_call(cpu);
+if (arm_is_smccc_call(cpu, cs->exception_index)) {
+arm_handle_smccc_call(cpu);
qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
return;
}
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 0dc96560d34..e8ac9a6d14e 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -653,7 +653,7 @@ static bool hvf_handle_psci_call(CPUState *cpu)
switch (param[0]) {
case QEMU_PSCI_0_2_FN_PSCI_VERSION:
-ret = QEMU_PSCI_0_2_RET_VERSION_0_2;
+ret = QEMU_PSCI_VERSION_1_1;
break;
case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
ret = QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED; /* No trusted OS */
@@ -721,6 +721,31 @@ static bool hvf_handle_psci_call(CPUState *cpu)
case QEMU_PSCI_0_2_FN_MIGRATE:
ret = QEMU_PSCI_RET_NOT_SUPPORTED;
break;
+case QEMU_PSCI_1_0_FN_PSCI_FEATURES:
+switch (param[1]) {
+case QEMU_PSCI_0_2_FN_PSCI_VERSION:
+case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+case QEMU_PSCI_0_2_FN_AFFINITY_INFO:
+case QEMU_PSCI_0_2_FN64_AFFINITY_INFO:
+case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
+case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
+case QEMU_PSCI_0_1_FN_CPU_ON:
+case QEMU_PSCI_0_2_FN_CPU_ON:
+case QEMU_PSCI_0_2_FN64_CPU_ON:
+case QEMU_PSCI_0_1_FN_CPU_OFF:
+case QEMU_PSCI_0_2_FN_CPU_OFF:
+case QEMU_PSCI_0_1_FN_CPU_SUSPEND:
+case QEMU_PSCI_0_2_FN_CPU_SUSPEND:
+case QEMU_PSCI_0_2_FN64_CPU_SUSPEND:
+case QEMU_PSCI_0_1_FN_MIGRATE:
+case QEMU_PSCI_0_2_FN_MIGRATE:
+case QEMU_PSCI_1_0_FN_PSCI_FEATURES:
+ret = 0;
+break;
+default:
+ret = QEMU_PSCI_RET_NOT_SUPPORTED;
+}
+break;
default:
return false;
}
@@ -1208,8 +1233,7 @@ int hvf_vcpu_exec(CPUState *cpu)
if (arm_cpu->psci_conduit
Re: [PULL 00/28] testing and plugin updates
On Wed, 9 Feb 2022 at 14:15, Alex Bennée wrote: > > The following changes since commit 0a301624c2f4ced3331ffd5bce85b4274fe132af: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20220208' into staging (2022-02-08 > 11:40:08 +) > > are available in the Git repository at: > > https://github.com/stsquad/qemu.git tags/pull-testing-and-plugins-090222-1 > > for you to fetch changes up to 514f9f8eb64bfd5d6c15024db93f83bd81998de5: > > include/exec: fix softmmu version of TARGET_ABI_FMT_lx (2022-02-09 13:29:38 > +) > > > Testing and plugin updates: > > - include vhost tests in qtest > - clean-up gcov ephemera in clean/.gitignore > - lcitool and docker updates > - mention .editorconfig in devel notes > - switch Centos8 to Centos Stream 8 > - remove TCG tracing support > - add coverage plugin using drcov format > - expand abilities of libinsn.so plugin > - use correct logging for i386 int cases > - move reset of plugin data to start of block > - deprecate ppc6432abi > - fix TARGET_ABI_FMT_ptr for softmmu builds > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0 for any user-visible changes. -- PMM
Re: [PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
Am 12. Februar 2022 19:30:43 MEZ schrieb Peter Maydell
:
>On Sat, 12 Feb 2022 at 17:02, BALATON Zoltan wrote:
>>
>> On Sat, 12 Feb 2022, Peter Maydell wrote:
>> > On Sat, 12 Feb 2022 at 13:42, BALATON Zoltan wrote:
>> >> By the way the corresponding member in struct PIIXState in
>> >> include/hw/southbridge/piix.h has a comment saying:
>> >>
>> >> /* This member isn't used. Just for save/load compatibility */
>> >> int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>> >>
>> >> and only seems to be filled in piix3_pre_save() but never used. So what's
>> >> the point of this then? Maybe piix3 also uses a bitmap to store these
>> >> levels instead? There's a uint64_t pic_levels member above the unused
>> >> array but I haven't checked the implementation.
>> >
>> > I think what has happened here is that originally piix3 used
>> > the same implementation piix4 currently does -- where it stores
>> > locally the value of the (incoming) PCI IRQ levels, and then when it wants
>> > to know the value of the (outgoing) PIC IRQ levels it loops round
>> > to effectively OR together all the PCI IRQ levels for those PCI
>> > IRQs which are configured to that particular PIC interrupt.
>> >
>> > Then in commit e735b55a8c11 (in 2011) piix3 was changed to call
>> > pci_bus_get_irq_level() to get the value of a PCI IRQ rather than
>> > looking at its local cache of that information in the pci_irq_levels[]
>> > array. This is the source of the "save/load compatibility" thing --
>> > before doing a vmsave piix3_pre_save() fills in that field so that
>> > it appears in the outbound data stream and thus a migration from
>> > a new QEMU to an old pre-e735b55a8c11 QEMU will still work. (This
>> > was important at the time, but could probably be cleaned up now.)
>> >
>> > The next commit after that one is ab431c283e7055bcd, which
>> > is an optimization that fixes the equivalent of the "XXX: optimize"
>> > marker in the gt64120_pci_set_irq()/piix4 code -- this does
>> > something slightly more complicated involving the pic_levels
>> > field, in order to avoid having to do the "loop over all the
>> > PCI IRQ levels" whenever it needs to set/reset a PIC interrupt.
>> >
>> > We could pick up one or both (or none!) of these two changes
>> > for the piix4 code. (If we're breaking migration compat anyway
>> > we wouldn't need to include the save-load compat part of
>> > the first change.)
>>
>> I'm not sure I fully get this. Currently (before this series) PIIX4State
>> does not seem to have any fields to store irq state (in hw/isa/piix4.c):
>>
>> struct PIIX4State {
>> PCIDevice dev;
>> qemu_irq cpu_intr;
>> qemu_irq *isa;
>>
>> RTCState rtc;
>> /* Reset Control Register */
>> MemoryRegion rcr_mem;
>> uint8_t rcr;
>> };
>>
>> Patch 1 in this series introduces that by moving it from MaltaState. Then
>> we could have a patch 2 to clean it up and change to the way piix3 does it
>> and skip introducing the saving of this array into the migration state. It
>> may still break migration but not sure if MaltaState was saved already so
>> this may be already missing from migration anyway and if we do the same as
>> piix3 then maybe we don't need to change the piix4 state either (as this
>> is saved as part of the PCIHost state?) but I don't know much about this
>> so maybe I'm wrong.
>
>Yeah, that would work -- we weren't saving the old gt64xxx_pci.c
>pci_irq_levels[] global, so we don't break anything that wasn't
>already broken by not putting the newly-introduced PIIX4State
>array into migration state. Then when we do the equivalent of
>e735b55a8c11 the array can just be deleted again. (We can't
>conveniently switch to using pci_bus_get_irq_level() before doing
>patch 1 of this series, because we need the pointer to the
>piix4 device object and gt64120_pci_set_irq() is only passed
>a pointer directly to a qemu_irq array.)
>
>> In any case PIIX3 and PIIX4 state seem to be different so there's no
>> reason to worry aobut compatibility between them.
>
>Yep, that's right. The only reasons to copy changes from piix3
>are (a) because they're worth having in themselves and (b)
>because having the two devices be the same is maybe less
>confusing. (b)'s a pretty weak reason, and (a) depends on
>the individual change. In this case I think making the equivalent
>of e735b55a8c11 is worthwhile because it saves us having an
>extra array field and migrating it, and the change is pretty
>small. For ab431c283e7055bcd you could argue either way -- it's
>clearly a better way to structure the irq handling, but it's only
>an optimisation, not a bug fix.
e735b55a8c11 seems like a very elegant way for fixing migration of the IRQ
levels. I'll have a look.
Regards,
Bernhard
>
>-- PMM
Re: [PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
Am 12. Februar 2022 19:30:43 MEZ schrieb Peter Maydell
:
>On Sat, 12 Feb 2022 at 17:02, BALATON Zoltan wrote:
>>
>> On Sat, 12 Feb 2022, Peter Maydell wrote:
>> > On Sat, 12 Feb 2022 at 13:42, BALATON Zoltan wrote:
>> >> By the way the corresponding member in struct PIIXState in
>> >> include/hw/southbridge/piix.h has a comment saying:
>> >>
>> >> /* This member isn't used. Just for save/load compatibility */
>> >> int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
>> >>
>> >> and only seems to be filled in piix3_pre_save() but never used. So what's
>> >> the point of this then? Maybe piix3 also uses a bitmap to store these
>> >> levels instead? There's a uint64_t pic_levels member above the unused
>> >> array but I haven't checked the implementation.
>> >
>> > I think what has happened here is that originally piix3 used
>> > the same implementation piix4 currently does -- where it stores
>> > locally the value of the (incoming) PCI IRQ levels, and then when it wants
>> > to know the value of the (outgoing) PIC IRQ levels it loops round
>> > to effectively OR together all the PCI IRQ levels for those PCI
>> > IRQs which are configured to that particular PIC interrupt.
>> >
>> > Then in commit e735b55a8c11 (in 2011) piix3 was changed to call
>> > pci_bus_get_irq_level() to get the value of a PCI IRQ rather than
>> > looking at its local cache of that information in the pci_irq_levels[]
>> > array. This is the source of the "save/load compatibility" thing --
>> > before doing a vmsave piix3_pre_save() fills in that field so that
>> > it appears in the outbound data stream and thus a migration from
>> > a new QEMU to an old pre-e735b55a8c11 QEMU will still work. (This
>> > was important at the time, but could probably be cleaned up now.)
>> >
>> > The next commit after that one is ab431c283e7055bcd, which
>> > is an optimization that fixes the equivalent of the "XXX: optimize"
>> > marker in the gt64120_pci_set_irq()/piix4 code -- this does
>> > something slightly more complicated involving the pic_levels
>> > field, in order to avoid having to do the "loop over all the
>> > PCI IRQ levels" whenever it needs to set/reset a PIC interrupt.
>> >
>> > We could pick up one or both (or none!) of these two changes
>> > for the piix4 code. (If we're breaking migration compat anyway
>> > we wouldn't need to include the save-load compat part of
>> > the first change.)
>>
>> I'm not sure I fully get this. Currently (before this series) PIIX4State
>> does not seem to have any fields to store irq state (in hw/isa/piix4.c):
>>
>> struct PIIX4State {
>> PCIDevice dev;
>> qemu_irq cpu_intr;
>> qemu_irq *isa;
>>
>> RTCState rtc;
>> /* Reset Control Register */
>> MemoryRegion rcr_mem;
>> uint8_t rcr;
>> };
>>
>> Patch 1 in this series introduces that by moving it from MaltaState. Then
>> we could have a patch 2 to clean it up and change to the way piix3 does it
>> and skip introducing the saving of this array into the migration state. It
>> may still break migration but not sure if MaltaState was saved already so
>> this may be already missing from migration anyway and if we do the same as
>> piix3 then maybe we don't need to change the piix4 state either (as this
>> is saved as part of the PCIHost state?) but I don't know much about this
>> so maybe I'm wrong.
>
>Yeah, that would work -- we weren't saving the old gt64xxx_pci.c
>pci_irq_levels[] global, so we don't break anything that wasn't
>already broken by not putting the newly-introduced PIIX4State
>array into migration state. Then when we do the equivalent of
>e735b55a8c11 the array can just be deleted again. (We can't
>conveniently switch to using pci_bus_get_irq_level() before doing
>patch 1 of this series, because we need the pointer to the
>piix4 device object and gt64120_pci_set_irq() is only passed
>a pointer directly to a qemu_irq array.)
>
>> In any case PIIX3 and PIIX4 state seem to be different so there's no
>> reason to worry aobut compatibility between them.
>
>Yep, that's right. The only reasons to copy changes from piix3
>are (a) because they're worth having in themselves and (b)
>because having the two devices be the same is maybe less
>confusing. (b)'s a pretty weak reason, and (a) depends on
>the individual change. In this case I think making the equivalent
>of e735b55a8c11 is worthwhile because it saves us having an
>extra array field and migrating it, and the change is pretty
>small. For ab431c283e7055bcd you could argue either way -- it's
>clearly a better way to structure the irq handling, but it's only
>an optimisation, not a bug fix.
e735b55a8c11 seems like a very elegant way for fixing migration of the IRQ
levels. I'll have a look.
Regards,
Bernhard
>
>-- PMM
[PATCH] net/eth: Don't consider ESP to be an IPv6 option header
The IPv6 option headers all have in common that they start with some
common fields, in particular the type of the next header followed by the
extention header length. This is used to traverse the list of the
options. The ESP header does not follow that format, which can break the
IPv6 option header traversal code in eth_parse_ipv6_hdr().
The effect of that is that network interfaces such as vmxnet3 that use
the following call chain
eth_is_ip6_extension_header_type
eth_parse_ipv6_hdr
net_tx_pkt_parse_headers
net_tx_pkt_parse
vmxnet3_process_tx_queue
to send packets from the VM out to the host will drop packets of the
following structure:
Ethernet-Header(IPv6-Header(ESP(encrypted data)))
Note that not all types of network interfaces use the net_tx_pkt_parse
function though, leading to inconsistent behavior regarding sending
those packets. The e1000 network interface for example does not suffer
from this limitation.
By not considering ESP to be an IPv6 header we can allow sending those
packets out to the host on all types of network interfaces.
Fixes: 75020a702151 ("Common definitions for VMWARE devices")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/149
Buglink: https://bugs.launchpad.net/qemu/+bug/1758091
Signed-off-by: Thomas Jansen
---
net/eth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/eth.c b/net/eth.c
index fe876d1a55..f074b2f9f3 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -389,7 +389,6 @@ eth_is_ip6_extension_header_type(uint8_t hdr_type)
case IP6_HOP_BY_HOP:
case IP6_ROUTING:
case IP6_FRAGMENT:
-case IP6_ESP:
case IP6_AUTHENTICATION:
case IP6_DESTINATON:
case IP6_MOBILITY:
--
2.34.1
Re: [RFC PATCH 14/25] virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler
Le 11/02/2022 à 23:13, Shreyansh Chouhan a écrit :
Signed-off-by: Shreyansh Chouhan
---
hw/audio/virtio-snd.c | 88 ++-
1 file changed, 87 insertions(+), 1 deletion(-)
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index aec3e86db2..a53a6be168 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -188,6 +188,91 @@ static uint32_t virtio_snd_handle_jack_remap(VirtIOSound
*s,
return sz;
}
+/*
+ * Get a specific stream from the virtio sound card device.
+ *
+ * @s: VirtIOSound card device
+ * @stream: Stream id
+ *
+ * Returns NULL if function fails.
+ * TODO: Make failure more explicit. Output can be NULL if the stream number
+ * is valid but the stream hasn't been allocated yet.
+ */
+static virtio_snd_pcm_stream *virtio_snd_pcm_get_stream(VirtIOSound *s,
+uint32_t stream)
+{
+if (stream >= s->snd_conf.streams) {
+virtio_snd_err("Invalid stream request %d\n", stream);
+return NULL;
+}
+return s->streams[stream];
+}
+
+/*
+ * Handle the VIRTIO_SND_R_PCM_INFO request.
+ * The function writes the info structs to the request element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card device
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_pcm_info(VirtIOSound *s,
+ VirtQueueElement *elem)
+{
+virtio_snd_query_info req;
+uint32_t sz;
+sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+assert(sz == sizeof(virtio_snd_query_info));
+
+virtio_snd_hdr resp;
+if (iov_size(elem->in_sg, elem->in_num) <
+sizeof(virtio_snd_hdr) + req.size * req.count) {
+virtio_snd_err("pcm info: buffer too small, got: %lu, needed: %lu\n",
+iov_size(elem->in_sg, elem->in_num),
+sizeof(virtio_snd_pcm_info));
+resp.code = VIRTIO_SND_S_BAD_MSG;
+goto done;
+}
+
+virtio_snd_pcm_stream *stream;
+virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info, req.count);
+for (int i = req.start_id; i < req.start_id + req.count; i++) {
+stream = virtio_snd_pcm_get_stream(s, i);
+
+if (!stream) {
+virtio_snd_err("Invalid stream id: %d\n", i);
+resp.code = VIRTIO_SND_S_BAD_MSG;
+goto done;
+}
+
+pcm_info[i - req.start_id].hdr.hda_fn_nid = stream->hda_fn_nid;
+pcm_info[i - req.start_id].features = stream->features;
+pcm_info[i - req.start_id].formats = stream->formats;
+pcm_info[i - req.start_id].rates = stream->rates;
+pcm_info[i - req.start_id].direction = stream->direction;
+pcm_info[i - req.start_id].channels_min = stream->channels_min;
+pcm_info[i - req.start_id].channels_max = stream->channels_max;
+
+memset(&pcm_info[i].padding, 0, sizeof(pcm_info[i].padding));
+}
+
+resp.code = VIRTIO_SND_S_OK;
+done:
+sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+assert(sz == sizeof(virtio_snd_hdr));
+
+if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+g_free(pcm_info);
+return sz;
+}
+
+sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+ pcm_info, sizeof(virtio_snd_pcm_info) * req.count);
Same problem here:
In file included from ../../../Projects/qemu-next/hw/audio/virtio-snd.c:7:
.../hw/audio/virtio-snd.c: In function 'virtio_snd_handle_ctrl':
.../include/qemu/iov.h:49:16: error: 'pcm_info' may be used uninitialized in this function
[-Werror=maybe-uninitialized]
49 | return iov_from_buf_full(iov, iov_cnt, offset, buf, bytes);
|^~~
.../hw/audio/virtio-snd.c:238:26: note: 'pcm_info' was declared here
238 | virtio_snd_pcm_info *pcm_info = g_new0(virtio_snd_pcm_info,
req.count);
| ^~~~
+assert(sz == req.size * req.count);
+g_free(pcm_info);
+return sizeof(virtio_snd_hdr) + sz;
+}
+
/* The control queue handler. Pops an element from the control virtqueue,
* checks the header and performs the requested action. Finally marks the
* element as used.
@@ -233,7 +318,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
sz = virtio_snd_handle_jack_remap(s, elem);
goto done;
} else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
-virtio_snd_log("VIRTIO_SND_R_PCM_INFO");
+sz = virtio_snd_handle_pcm_info(s, elem);
+goto done;
} else if (ctrl.code == VIRTIO_SND_R_PCM_SET_PARAMS) {
virtio_snd_log("VIRTIO_SND_R_PCM_SET_PARAMS");
} else if (ctrl.code == VIRTIO_SND_R_PCM_PREPARE) {
Re: [RFC PATCH 12/25] virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler
Le 11/02/2022 à 23:13, Shreyansh Chouhan a écrit :
Signed-off-by: Shreyansh Chouhan
---
hw/audio/virtio-snd.c | 81 +--
1 file changed, 79 insertions(+), 2 deletions(-)
diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a87922f91b..c2af26f3cb 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -92,6 +92,80 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev,
uint64_t features,
{
return vdev->host_features;
}
+/*
+ * Get a specific jack from the VirtIOSound card.
+ *
+ * @s: VirtIOSound card device.
+ * @id: Jack id
+ */
+static virtio_snd_jack *virtio_snd_get_jack(VirtIOSound *s, uint32_t id)
+{
+if (id >= s->snd_conf.jacks) {
+return NULL;
+}
+return s->jacks[id];
+}
+
+/*
+ * Handles VIRTIO_SND_R_JACK_INFO.
+ * The function writes the info structs and response to the virtqueue element.
+ * Returns the used size in bytes.
+ *
+ * @s: VirtIOSound card
+ * @elem: The request element from control queue
+ */
+static uint32_t virtio_snd_handle_jack_info(VirtIOSound *s,
+VirtQueueElement *elem)
+{
+virtio_snd_query_info req;
+size_t sz = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+assert(sz == sizeof(virtio_snd_query_info));
+
+virtio_snd_hdr resp;
+
+if (iov_size(elem->in_sg, elem->in_num) <
+sizeof(virtio_snd_hdr) + req.count * req.size) {
+virtio_snd_err("jack info: buffer too small got: %lu needed: %lu\n",
+ iov_size(elem->in_sg, elem->in_num),
+ sizeof(virtio_snd_hdr) + req.count * req.size);
+resp.code = VIRTIO_SND_S_BAD_MSG;
+goto done;
+}
+
+virtio_snd_jack_info *jack_info = g_new0(virtio_snd_jack_info, req.count);
+for (int i = req.start_id; i < req.count + req.start_id; i++) {
+virtio_snd_jack *jack = virtio_snd_get_jack(s, i);
+if (!jack) {
+virtio_snd_err("Invalid jack id: %d\n", i);
+resp.code = VIRTIO_SND_S_BAD_MSG;
+goto done;
+}
+
+jack_info[i - req.start_id].hdr.hda_fn_nid = jack->hda_fn_nid;
+jack_info[i - req.start_id].features = jack->features;
+jack_info[i - req.start_id].hda_reg_defconf = jack->hda_reg_defconf;
+jack_info[i - req.start_id].hda_reg_caps = jack->hda_reg_caps;
+jack_info[i - req.start_id].connected = jack->connected;
+memset(jack_info[i - req.start_id].padding, 0,
+ sizeof(jack_info[i - req.start_id].padding));
+}
+
+resp.code = VIRTIO_SND_S_OK;
+done:
+sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
+assert(sz == sizeof(virtio_snd_hdr));
+
+if (resp.code == VIRTIO_SND_S_BAD_MSG) {
+g_free(jack_info);
+return sz;
+}
+
+sz = iov_from_buf(elem->in_sg, elem->in_num, sizeof(virtio_snd_hdr),
+ jack_info, sizeof(virtio_snd_jack_info) * req.count);
+assert(sz == req.count * req.size);
+g_free(jack_info);
+return sizeof(virtio_snd_hdr) + sz;
+}
/* The control queue handler. Pops an element from the control virtqueue,
* checks the header and performs the requested action. Finally marks the
@@ -102,6 +176,7 @@ static uint64_t virtio_snd_get_features(VirtIODevice *vdev,
uint64_t features,
*/
static void virtio_snd_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
{
+VirtIOSound *s = VIRTIO_SOUND(vdev);
virtio_snd_hdr ctrl;
VirtQueueElement *elem = NULL;
@@ -131,7 +206,8 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
/* error */
virtio_snd_err("virtio snd ctrl could not read header\n");
} else if (ctrl.code == VIRTIO_SND_R_JACK_INFO) {
-virtio_snd_log("VIRTIO_SND_R_JACK_INFO");
+sz = virtio_snd_handle_jack_info(s, elem);
+goto done;
} else if (ctrl.code == VIRTIO_SND_R_JACK_REMAP) {
virtio_snd_log("VIRTIO_SND_R_JACK_REMAP");
} else if (ctrl.code == VIRTIO_SND_R_PCM_INFO) {
@@ -156,8 +232,9 @@ static void virtio_snd_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
virtio_snd_hdr resp;
resp.code = VIRTIO_SND_S_OK;
sz = iov_from_buf(elem->in_sg, elem->in_num, 0, &resp, sizeof(resp));
-virtqueue_push(vq, elem, sz);
+done:
+virtqueue_push(vq, elem, sz);
virtio_notify(vdev, vq);
g_free(iov2);
g_free(elem);
This patch has a warning:
.../hw/audio/virtio-snd.c: In function 'virtio_snd_handle_ctrl':
.../include/qemu/iov.h:49:16: error: 'jack_info' may be used uninitialized in this function
[-Werror=maybe-uninitialized]
49 | return iov_from_buf_full(iov, iov_cnt, offset, buf, bytes);
|^~~
...hw/audio/virtio-snd.c:135:27: note: 'jack_info' was declared here
135 | vir
Re: [RFC PATCH v2 00/25] Virtio Sound card Implementation
Le 11/02/2022 à 23:12, Shreyansh Chouhan a écrit : The second RFC for implementing the VirtIO Sound card as described in the virtio specs. Sorry for the absence of activity on this. The output from the sound card works. What remains to be done: - Features defined in PCM features. (Eg message polling) - Channel maps - Jack remaps - Input I will work on the input after I have implemented the output along with all the features since at that point it should just be a matter of reversing a few things in the code that writes the audio. I can work on this patchset mostly on weekends now but I will try to be more regular with this. Reviews are welcome :) Shreyansh Chouhan (25): virtio-snd: Add virtio sound header file virtio-snd: Add jack control structures virtio-snd: Add PCM control structures virtio-snd: Add chmap control structures virtio-snd: Add device implementation structures virtio-snd: Add PCI wrapper code for VirtIOSound virtio-snd: Add properties for class init virtio-snd: Add code for get config function virtio-snd: Add code for the realize function virtio-snd: Add macros for logging virtio-snd: Add control virtqueue handler virtio-snd: Add VIRTIO_SND_R_JACK_INFO handler virtio-snd: Add stub for VIRTIO_SND_R_JACK_REMAP handler virtio-snd: Add VIRTIO_SND_R_PCM_INFO handler virtio-snd: Add VIRITO_SND_R_PCM_SET_PARAMS handle virtio-snd: Add VIRTIO_SND_R_PCM_PREPARE handler virtio-snd: Add default configs to realize fn virtio-snd: Add callback for SWVoiceOut virtio-snd: Add start/stop handler virtio-snd: Add VIRTIO_SND_R_PCM_RELEASE handler virtio-snd: Replaced goto with if else virtio-snd: Add code to device unrealize function virtio-snd: Add xfer handler virtio-snd: Add event vq and a handler stub virtio-snd: Replaced AUD_log with tracepoints hw/audio/Kconfig |5 + hw/audio/meson.build |1 + hw/audio/trace-events | 14 + hw/audio/virtio-snd.c | 1241 hw/virtio/meson.build |1 + hw/virtio/virtio-snd-pci.c | 72 ++ include/hw/virtio/virtio-snd.h | 383 ++ 7 files changed, 1717 insertions(+) create mode 100644 hw/audio/virtio-snd.c create mode 100644 hw/virtio/virtio-snd-pci.c create mode 100644 include/hw/virtio/virtio-snd.h Thank you for your work. IMHO, all your patches can be merged in only one. Morever it would help for review as some patches remove code done in previous patches. The "v2" tag is missing in the subject of the patches of your series. And don't send a series as a reply of a previous one. You can use "git-publish" it helps a lot when we have to send several versions of a series. Thanks, Laurent
Re: [PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
On Sat, 12 Feb 2022 at 17:02, BALATON Zoltan wrote:
>
> On Sat, 12 Feb 2022, Peter Maydell wrote:
> > On Sat, 12 Feb 2022 at 13:42, BALATON Zoltan wrote:
> >> By the way the corresponding member in struct PIIXState in
> >> include/hw/southbridge/piix.h has a comment saying:
> >>
> >> /* This member isn't used. Just for save/load compatibility */
> >> int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
> >>
> >> and only seems to be filled in piix3_pre_save() but never used. So what's
> >> the point of this then? Maybe piix3 also uses a bitmap to store these
> >> levels instead? There's a uint64_t pic_levels member above the unused
> >> array but I haven't checked the implementation.
> >
> > I think what has happened here is that originally piix3 used
> > the same implementation piix4 currently does -- where it stores
> > locally the value of the (incoming) PCI IRQ levels, and then when it wants
> > to know the value of the (outgoing) PIC IRQ levels it loops round
> > to effectively OR together all the PCI IRQ levels for those PCI
> > IRQs which are configured to that particular PIC interrupt.
> >
> > Then in commit e735b55a8c11 (in 2011) piix3 was changed to call
> > pci_bus_get_irq_level() to get the value of a PCI IRQ rather than
> > looking at its local cache of that information in the pci_irq_levels[]
> > array. This is the source of the "save/load compatibility" thing --
> > before doing a vmsave piix3_pre_save() fills in that field so that
> > it appears in the outbound data stream and thus a migration from
> > a new QEMU to an old pre-e735b55a8c11 QEMU will still work. (This
> > was important at the time, but could probably be cleaned up now.)
> >
> > The next commit after that one is ab431c283e7055bcd, which
> > is an optimization that fixes the equivalent of the "XXX: optimize"
> > marker in the gt64120_pci_set_irq()/piix4 code -- this does
> > something slightly more complicated involving the pic_levels
> > field, in order to avoid having to do the "loop over all the
> > PCI IRQ levels" whenever it needs to set/reset a PIC interrupt.
> >
> > We could pick up one or both (or none!) of these two changes
> > for the piix4 code. (If we're breaking migration compat anyway
> > we wouldn't need to include the save-load compat part of
> > the first change.)
>
> I'm not sure I fully get this. Currently (before this series) PIIX4State
> does not seem to have any fields to store irq state (in hw/isa/piix4.c):
>
> struct PIIX4State {
> PCIDevice dev;
> qemu_irq cpu_intr;
> qemu_irq *isa;
>
> RTCState rtc;
> /* Reset Control Register */
> MemoryRegion rcr_mem;
> uint8_t rcr;
> };
>
> Patch 1 in this series introduces that by moving it from MaltaState. Then
> we could have a patch 2 to clean it up and change to the way piix3 does it
> and skip introducing the saving of this array into the migration state. It
> may still break migration but not sure if MaltaState was saved already so
> this may be already missing from migration anyway and if we do the same as
> piix3 then maybe we don't need to change the piix4 state either (as this
> is saved as part of the PCIHost state?) but I don't know much about this
> so maybe I'm wrong.
Yeah, that would work -- we weren't saving the old gt64xxx_pci.c
pci_irq_levels[] global, so we don't break anything that wasn't
already broken by not putting the newly-introduced PIIX4State
array into migration state. Then when we do the equivalent of
e735b55a8c11 the array can just be deleted again. (We can't
conveniently switch to using pci_bus_get_irq_level() before doing
patch 1 of this series, because we need the pointer to the
piix4 device object and gt64120_pci_set_irq() is only passed
a pointer directly to a qemu_irq array.)
> In any case PIIX3 and PIIX4 state seem to be different so there's no
> reason to worry aobut compatibility between them.
Yep, that's right. The only reasons to copy changes from piix3
are (a) because they're worth having in themselves and (b)
because having the two devices be the same is maybe less
confusing. (b)'s a pretty weak reason, and (a) depends on
the individual change. In this case I think making the equivalent
of e735b55a8c11 is worthwhile because it saves us having an
extra array field and migrating it, and the change is pretty
small. For ab431c283e7055bcd you could argue either way -- it's
clearly a better way to structure the irq handling, but it's only
an optimisation, not a bug fix.
-- PMM
Re: [PATCH v4 10/13] audio/coreaudio: Remove a deprecation warning on macOS 12
On Samstag, 12. Februar 2022 18:27:18 CET Christian Schoenebeck wrote:
> On Samstag, 12. Februar 2022 16:23:49 CET Akihiko Odaki wrote:
> > On 2022/02/12 1:34, Philippe Mathieu-Daudé via wrote:
> > > When building on macOS 12 we get:
> > >audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is
> > >deprecated: first deprecated in macOS 12.0
> > >[-Werror,-Wdeprecated-declarations]>
> > >
> > >kAudioObjectPropertyElementMaster
> > >^
> > >kAudioObjectPropertyElementMain
> > >
> > >/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Fr
> > >am
> > >eworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: note:
> > >'kAudioObjectPropertyElementMaster' has been explicitly marked
> > >deprecated here>
> > >
> > >kAudioObjectPropertyElementMaster
> > >API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain"
> > >,
> > >macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0,
> > >15.0)) = kAudioObjectPropertyElementMain ^
> > >
> > > Replace by kAudioObjectPropertyElementMain, redefining it to
> > > kAudioObjectPropertyElementMaster if not available.
> > >
> > > Suggested-by: Akihiko Odaki
> > > Suggested-by: Christian Schoenebeck
> > > Suggested-by: Roman Bolshakov
> > > Signed-off-by: Philippe Mathieu-Daudé
> > > Reviewed-by: Christian Schoenebeck
> > > ---
> > >
> > > audio/coreaudio.c | 17 +++--
> > > 1 file changed, 11 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> > > index d8a21d3e50..5b3aeaced0 100644
> > > --- a/audio/coreaudio.c
> > > +++ b/audio/coreaudio.c
> > > @@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
> > >
> > > bool enabled;
> > >
> > > } coreaudioVoiceOut;
> > >
> > > +#if !defined(MAC_OS_VERSION_12_0) \
> > > +|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
> > > +#define kAudioObjectPropertyElementMain
> > > kAudioObjectPropertyElementMaster
> > > +#endif
> > > +
> >
> > Unless I have missed something, we have found
> > MAC_OS_X_VERSION_MIN_REQUIRED is better even for a constant in the
> > following thread:
> > https://lore.kernel.org/all/524515d6-2fb5-15c1-0aaf-bcda3684c...@gmail.com
> > /
> >
> > Regards,
> > Akihiko Odaki
>
> Well, MAC_OS_X_VERSION_MIN_REQUIRED would work as well, note though that it
> would effectively result with older SDKs (Xcode <= 13.0) to this:
>
> enum {
> MAIN,
> MASTER = MAIN
> };
>
> #define MAIN MASTER
>
> int main() {
>int k = MAIN;
> }
>
> Which compiles and works (as both enums reflect the same value anyway), but
> strictly the defined preprocessor macro would mask (with older SDKs) the
> already existing enum. Not that I would care, just noting.
>
> On practical side though, your solution (MAC_OS_X_VERSION_MIN_REQUIRED)
> would avoid deprecation warnings in future. So yes, maybe it's a bit
> better.
Correction: it would result in this masking scenario with recent SDK (e.g.
Xcode 13.2.1) and targeting a minimum deployment target macOs <12.0 (not when
compiling directly with Xcode <13.1).
Best regards,
Christian Schoenebeck
Re: [PATCH v4 10/13] audio/coreaudio: Remove a deprecation warning on macOS 12
On Samstag, 12. Februar 2022 16:23:49 CET Akihiko Odaki wrote:
> On 2022/02/12 1:34, Philippe Mathieu-Daudé via wrote:
> > When building on macOS 12 we get:
> >audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is
> >deprecated: first deprecated in macOS 12.0
> >[-Werror,-Wdeprecated-declarations]>
> >kAudioObjectPropertyElementMaster
> >^
> >kAudioObjectPropertyElementMain
> >
> >/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Fram
> >eworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: note:
> >'kAudioObjectPropertyElementMaster' has been explicitly marked
> >deprecated here>
> >kAudioObjectPropertyElementMaster
> >API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain",
> >macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0,
> >15.0)) = kAudioObjectPropertyElementMain ^
> >
> > Replace by kAudioObjectPropertyElementMain, redefining it to
> > kAudioObjectPropertyElementMaster if not available.
> >
> > Suggested-by: Akihiko Odaki
> > Suggested-by: Christian Schoenebeck
> > Suggested-by: Roman Bolshakov
> > Signed-off-by: Philippe Mathieu-Daudé
> > Reviewed-by: Christian Schoenebeck
> > ---
> >
> > audio/coreaudio.c | 17 +++--
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> > index d8a21d3e50..5b3aeaced0 100644
> > --- a/audio/coreaudio.c
> > +++ b/audio/coreaudio.c
> > @@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
> >
> > bool enabled;
> >
> > } coreaudioVoiceOut;
> >
> > +#if !defined(MAC_OS_VERSION_12_0) \
> > +|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
> > +#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
> > +#endif
> > +
>
> Unless I have missed something, we have found
> MAC_OS_X_VERSION_MIN_REQUIRED is better even for a constant in the
> following thread:
> https://lore.kernel.org/all/524515d6-2fb5-15c1-0aaf-bcda3684c...@gmail.com/
>
> Regards,
> Akihiko Odaki
Well, MAC_OS_X_VERSION_MIN_REQUIRED would work as well, note though that it
would effectively result with older SDKs (Xcode <= 13.0) to this:
enum {
MAIN,
MASTER = MAIN
};
#define MAIN MASTER
int main() {
int k = MAIN;
}
Which compiles and works (as both enums reflect the same value anyway), but
strictly the defined preprocessor macro would mask (with older SDKs) the
already existing enum. Not that I would care, just noting.
On practical side though, your solution (MAC_OS_X_VERSION_MIN_REQUIRED) would
avoid deprecation warnings in future. So yes, maybe it's a bit better.
Best regards,
Christian Schoenebeck
>
> > static const AudioObjectPropertyAddress voice_addr = {
> >
> > kAudioHardwarePropertyDefaultOutputDevice,
> > kAudioObjectPropertyScopeGlobal,
> >
> > -kAudioObjectPropertyElementMaster
> > +kAudioObjectPropertyElementMain
> >
> > };
> >
> > static OSStatus coreaudio_get_voice(AudioDeviceID *id)
> >
> > @@ -69,7 +74,7 @@ static OSStatus
> > coreaudio_get_framesizerange(AudioDeviceID id,>
> > AudioObjectPropertyAddress addr = {
> >
> > kAudioDevicePropertyBufferFrameSizeRange,
> > kAudioDevicePropertyScopeOutput,
> >
> > -kAudioObjectPropertyElementMaster
> > +kAudioObjectPropertyElementMain
> >
> > };
> >
> > return AudioObjectGetPropertyData(id,
> >
> > @@ -86,7 +91,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID
> > id, UInt32 *framesize)>
> > AudioObjectPropertyAddress addr = {
> >
> > kAudioDevicePropertyBufferFrameSize,
> > kAudioDevicePropertyScopeOutput,
> >
> > -kAudioObjectPropertyElementMaster
> > +kAudioObjectPropertyElementMain
> >
> > };
> >
> > return AudioObjectGetPropertyData(id,
> >
> > @@ -103,7 +108,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID
> > id, UInt32 *framesize)>
> > AudioObjectPropertyAddress addr = {
> >
> > kAudioDevicePropertyBufferFrameSize,
> > kAudioDevicePropertyScopeOutput,
> >
> > -kAudioObjectPropertyElementMaster
> > +kAudioObjectPropertyElementMain
> >
> > };
> >
> > return AudioObjectSetPropertyData(id,
> >
> > @@ -121,7 +126,7 @@ static OSStatus
> > coreaudio_set_streamformat(AudioDeviceID id,>
> > AudioObjectPropertyAddress addr = {
> >
> > kAudioDevicePropertyStreamFormat,
> > kAudioDevicePropertyScopeOutput,
> >
> > -kAudioObjectPropertyElementMaster
> > +kAudioObjectPropertyElementMain
> >
> > };
> >
> > return AudioObjectSetPropertyData(id,
> >
> > @@ -138,7 +143,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID
> > id, UInt32 *result)>
> > AudioObjectPropertyAddress a
Re: [PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
On Sat, 12 Feb 2022, Peter Maydell wrote:
On Sat, 12 Feb 2022 at 13:42, BALATON Zoltan wrote:
By the way the corresponding member in struct PIIXState in
include/hw/southbridge/piix.h has a comment saying:
/* This member isn't used. Just for save/load compatibility */
int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
and only seems to be filled in piix3_pre_save() but never used. So what's
the point of this then? Maybe piix3 also uses a bitmap to store these
levels instead? There's a uint64_t pic_levels member above the unused
array but I haven't checked the implementation.
I think what has happened here is that originally piix3 used
the same implementation piix4 currently does -- where it stores
locally the value of the (incoming) PCI IRQ levels, and then when it wants
to know the value of the (outgoing) PIC IRQ levels it loops round
to effectively OR together all the PCI IRQ levels for those PCI
IRQs which are configured to that particular PIC interrupt.
Then in commit e735b55a8c11 (in 2011) piix3 was changed to call
pci_bus_get_irq_level() to get the value of a PCI IRQ rather than
looking at its local cache of that information in the pci_irq_levels[]
array. This is the source of the "save/load compatibility" thing --
before doing a vmsave piix3_pre_save() fills in that field so that
it appears in the outbound data stream and thus a migration from
a new QEMU to an old pre-e735b55a8c11 QEMU will still work. (This
was important at the time, but could probably be cleaned up now.)
The next commit after that one is ab431c283e7055bcd, which
is an optimization that fixes the equivalent of the "XXX: optimize"
marker in the gt64120_pci_set_irq()/piix4 code -- this does
something slightly more complicated involving the pic_levels
field, in order to avoid having to do the "loop over all the
PCI IRQ levels" whenever it needs to set/reset a PIC interrupt.
We could pick up one or both (or none!) of these two changes
for the piix4 code. (If we're breaking migration compat anyway
we wouldn't need to include the save-load compat part of
the first change.)
I'm not sure I fully get this. Currently (before this series) PIIX4State
does not seem to have any fields to store irq state (in hw/isa/piix4.c):
struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
RTCState rtc;
/* Reset Control Register */
MemoryRegion rcr_mem;
uint8_t rcr;
};
Patch 1 in this series introduces that by moving it from MaltaState. Then
we could have a patch 2 to clean it up and change to the way piix3 does it
and skip introducing the saving of this array into the migration state. It
may still break migration but not sure if MaltaState was saved already so
this may be already missing from migration anyway and if we do the same as
piix3 then maybe we don't need to change the piix4 state either (as this
is saved as part of the PCIHost state?) but I don't know much about this
so maybe I'm wrong.
In any case PIIX3 and PIIX4 state seem to be different so there's no
reason to worry aobut compatibility between them. It's just confusing that
there's a common piix.h which defines a PIIXState that looks like it could
be common but maybe it's not used by PIIX4 but only by PIIX3 or I've
missed something as I've only looked at this briefly.
Regards,
BALATON Zoltan
Re: [PATCH v2 1/5] malta: Move PCI interrupt handling from gt64xxx to piix4
On Sat, 12 Feb 2022, BALATON Zoltan wrote:
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
Handling PCI interrupts in piix4 increases cohesion and reduces differences
between piix4 and piix3.
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Sorry for being late in commenting, I've missed the first round. Apologies if
this causes a delay or another version.
---
hw/isa/piix4.c | 58 +++
hw/mips/gt64xxx_pci.c | 62 --
hw/mips/malta.c| 6 +---
include/hw/mips/mips.h | 2 +-
4 files changed, 65 insertions(+), 63 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0fe7b69bc4..5a86308689 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -45,6 +45,7 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
+qemu_irq i8259[ISA_NUM_IRQS];
RTCState rtc;
/* Reset Control Register */
@@ -54,6 +55,30 @@ struct PIIX4State {
OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
+static int pci_irq_levels[4];
+
+static void piix4_set_irq(void *opaque, int irq_num, int level)
+{
+int i, pic_irq, pic_level;
+qemu_irq *pic = opaque;
+
+pci_irq_levels[irq_num] = level;
+
+/* now we change the pic irq level according to the piix irq mappings
*/
+/* XXX: optimize */
+pic_irq = piix4_dev->config[PIIX_PIRQCA + irq_num];
+if (pic_irq < 16) {
+/* The pic level is the logical OR of all the PCI irqs mapped to
it. */
+pic_level = 0;
+for (i = 0; i < 4; i++) {
+if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
+pic_level |= pci_irq_levels[i];
+}
+}
+qemu_set_irq(pic[pic_irq], pic_level);
+}
+}
+
static void piix4_isa_reset(DeviceState *dev)
{
PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -248,8 +273,34 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+int slot;
+
+slot = PCI_SLOT(pci_dev->devfn);
+
+switch (slot) {
+/* PIIX4 USB */
+case 10:
+return 3;
+/* AMD 79C973 Ethernet */
+case 11:
+return 1;
+/* Crystal 4281 Sound */
+case 12:
+return 2;
+/* PCI slot 1 to 4 */
+case 18 ... 21:
+return ((slot - 18) + irq_num) & 0x03;
+/* Unknown device, don't do any translation */
+default:
+return irq_num;
+}
+}
+
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus
**smbus)
{
+PIIX4State *s;
PCIDevice *pci;
DeviceState *dev;
int devfn = PCI_DEVFN(10, 0);
@@ -257,6 +308,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci = pci_create_simple_multifunction(pci_bus, devfn, true,
TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
+s = PIIX4_PCI_DEVICE(pci);
if (isa_bus) {
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
@@ -271,5 +323,11 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
NULL, 0, NULL);
}
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s->i8259, 4);
+
+for (int i = 0; i < ISA_NUM_IRQS; i++) {
Sorry one more nit. This is code movement but are we OK with declaring
local variable within for? I thinks coding style advises against this, not
sure if checkpatch accepts it or not. This could be cleaned up the next
patch together with removing the i8259 field which are simple enough to do
in one patch then this one stays as code movement and changes in next
patch.
+s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
+}
+
return dev;
}
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
[...]
diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 6c9c8805f3..ff88942e63 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -10,7 +10,7 @@
#include "exec/memory.h"
/* gt64xxx.c */
-PCIBus *gt64120_register(qemu_irq *pic);
+PCIBus *gt64120_register(void);
Now that you don't need to pass anything to it, do you still need this
function? Maybe what it does now could be done in the gt64120 device's
realize functions (there seems to be at least two: gt64120_realize and
gt64120_pci_realize but haven't checked which is more appropriate to put this
init in) or in an init function then you can just create the gt64120 device
in malta.c with qdev_new as is more usual to do in other boards. This
register function looks like the legacy init functions we're trying to get
rid of so this seems to be an opportunity to clean this up. This could be
done in a separate follow up though so may not need to be part of this series
but may be nice to have.
I meant this comment at the end of patch 1. But maybe this is too much to
do in this series as it needs more understanding of the gt64120
implement
Re: [PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
On Sat, 12 Feb 2022 at 13:42, BALATON Zoltan wrote: > By the way the corresponding member in struct PIIXState in > include/hw/southbridge/piix.h has a comment saying: > > /* This member isn't used. Just for save/load compatibility */ > int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; > > and only seems to be filled in piix3_pre_save() but never used. So what's > the point of this then? Maybe piix3 also uses a bitmap to store these > levels instead? There's a uint64_t pic_levels member above the unused > array but I haven't checked the implementation. I think what has happened here is that originally piix3 used the same implementation piix4 currently does -- where it stores locally the value of the (incoming) PCI IRQ levels, and then when it wants to know the value of the (outgoing) PIC IRQ levels it loops round to effectively OR together all the PCI IRQ levels for those PCI IRQs which are configured to that particular PIC interrupt. Then in commit e735b55a8c11 (in 2011) piix3 was changed to call pci_bus_get_irq_level() to get the value of a PCI IRQ rather than looking at its local cache of that information in the pci_irq_levels[] array. This is the source of the "save/load compatibility" thing -- before doing a vmsave piix3_pre_save() fills in that field so that it appears in the outbound data stream and thus a migration from a new QEMU to an old pre-e735b55a8c11 QEMU will still work. (This was important at the time, but could probably be cleaned up now.) The next commit after that one is ab431c283e7055bcd, which is an optimization that fixes the equivalent of the "XXX: optimize" marker in the gt64120_pci_set_irq()/piix4 code -- this does something slightly more complicated involving the pic_levels field, in order to avoid having to do the "loop over all the PCI IRQ levels" whenever it needs to set/reset a PIC interrupt. We could pick up one or both (or none!) of these two changes for the piix4 code. (If we're breaking migration compat anyway we wouldn't need to include the save-load compat part of the first change.) thanks -- PMM
Re: [PATCH v2 2/5] pci: Always pass own DeviceState to pci_map_irq_fn's
On Sat, 12 Feb 2022 at 13:27, BALATON Zoltan wrote: > > On Sat, 12 Feb 2022, Bernhard Beschow wrote: > > Passing own DeviceState rather than just the IRQs allows for resolving > > global variables. > > Do you mean pci_set_irq_fn instead of pci_map_irq_fn in the patch title? > > > Signed-off-by: Bernhard Beschow > > Reviewed-by: Peter Maydell > > Reviewed-by: Philippe Mathieu-Daudé > > --- > > hw/isa/piix4.c | 6 +++--- > > hw/pci-host/sh_pci.c| 6 +++--- > > hw/pci-host/versatile.c | 6 +++--- > > hw/ppc/ppc440_pcix.c| 6 +++--- > > hw/ppc/ppc4xx_pci.c | 6 +++--- > > 5 files changed, 15 insertions(+), 15 deletions(-) > > You don't seem to change any global function definition that reqires this > change in all these devices so why can't these decide on their own what > their preferred opaque data is for their set irq function and only change > piix4? Am I missing something? Yeah, we don't necessarily need to change all these devices, but I think this is an area where over time we're shifting from an older school of API design that was "we have some basically standalone functions that take an arbitrary opaque pointer" towards a more object-oriented design, where the opaque pointer should probably be the pointer-to-this-object unless there's a good reason why it needs to be something else[*], because having a function that's part of a device being passed something other than the device-instance pointer is a bit unexpected. In some cases there is a good reason for opaque pointers for function callbacks to be something else, because passing in the device pointer would make the code noticeably more complicated. But in the specific cases here, the only change is that the callback functions use "s->somearray[i]" instead of "an_argument[i]", which doesn't seem to me like a significant complexity change. thanks -- PMM
Re: [PATCH v2 2/5] pci: Always pass own DeviceState to pci_map_irq_fn's
On Sat, 12 Feb 2022, Bernhard Beschow wrote: Am 12. Februar 2022 14:27:32 MEZ schrieb BALATON Zoltan : On Sat, 12 Feb 2022, Bernhard Beschow wrote: Passing own DeviceState rather than just the IRQs allows for resolving global variables. Do you mean pci_set_irq_fn instead of pci_map_irq_fn in the patch title? I'm referring to the typedef in pci.h because the patch establishes consistency across the board. Signed-off-by: Bernhard Beschow Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé --- hw/isa/piix4.c | 6 +++--- hw/pci-host/sh_pci.c| 6 +++--- hw/pci-host/versatile.c | 6 +++--- hw/ppc/ppc440_pcix.c| 6 +++--- hw/ppc/ppc4xx_pci.c | 6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) You don't seem to change any global function definition that reqires this change in all these devices so why can't these decide on their own what their preferred opaque data is for their set irq function and only change piix4? Am I missing something? I'm not opposed to this change but it seems to be unnecessary to touch other device implementations for this and may just make them more complex for no good reason. This patch is a segway into a direction where the type of the opaque parameter of the pci_map_irq_fn typedef could be changed from void* to DeviceState* in I'm still not quite sure what you mean considering these typedefs in include/hw/pci/pci.h: typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level); typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num); typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); pci_map_irq_fn already has PCIDevice *, it's pci_set_irq_fn that has void *opaque and is what this patch appears to be changing. Am I looking at the wrong typedefs? order to facilitate the more typesafe QOM casting. I see, though, why this is confusing in the context of this patch series. I don't have strong feelings for converting the other devices or not. So I can only change piix4 if desired. Yes that seems to be an independent cahange so maybe it's better to just change piix4 in this series and then have a separate patch for changing the void *opaque to DeviceState * independent of this series. But I'm not sure that's necessarily a good idea. It may give some more checks but for callbacks used internally by device implementations I think it can be expected that code that registers the callback also knows what its opaque data should be so it does not have to be checked additionally, especially in code that may be called frequently. Although in a similar via-ide callback I could not measure a big penalty the last time I tried but I suspect there still mey be some overhead involving QOM casts (maybe there are just bigger bottle necks in ide emulation so it was masked) so I'm not sure it's worth the added complexity but if others prefer that I'm not that much opposed to it but it's clearer as a separate change anyway. Regards, BALATON Zoltan
Re: [PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
Am 12. Februar 2022 14:19:51 MEZ schrieb BALATON Zoltan :
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
This is a follow-up on patch "malta: Move PCI interrupt handling from
gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
to make the code movement more obvious. However, i8259[] seems redundant
to *isa, so remove it.
Should this be squashed in patch 1 or at least come after it? Or are there
some other changes needed before this patch?
I didn't want to change the patch order since they've been reviewed already.
But yeah, you're right: It makes sense to get this out of the way early in the
patch series. I will do a v3.
I had another comment in the bottom of this message, not sure you've
found that too, I forgot to put a warning that there are more comments
below here. Changing the patch order or patches according to review is OK
and adding a new patch between already reviewed ones is OK too then you
can keep Reviewed-by on patches that did not change.
Regards,
BALATON Zoltan
Regards,
Bernhard
Regards,
BALATON Zoltan
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a9322851db..692fa8d1f9 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -43,7 +43,6 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
-qemu_irq i8259[ISA_NUM_IRQS];
int32_t pci_irq_levels[PIIX_NUM_PIRQS];
@@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
pic_level |= s->pci_irq_levels[i];
}
}
-qemu_set_irq(s->i8259[pic_irq], pic_level);
+qemu_set_irq(s->isa[pic_irq], pic_level);
}
}
@@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
-for (int i = 0; i < ISA_NUM_IRQS; i++) {
-s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
-}
-
return dev;
}
Re: [PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute
On Sat, 12 Feb 2022, BALATON Zoltan wrote:
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
Am 12. Februar 2022 14:19:51 MEZ schrieb BALATON Zoltan
:
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
This is a follow-up on patch "malta: Move PCI interrupt handling from
gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
to make the code movement more obvious. However, i8259[] seems redundant
to *isa, so remove it.
Should this be squashed in patch 1 or at least come after it? Or are there
some other changes needed before this patch?
I didn't want to change the patch order since they've been reviewed
already. But yeah, you're right: It makes sense to get this out of the way
early in the patch series. I will do a v3.
I had another comment in the bottom of this message, not sure you've found
that too, I forgot to put a warning that there are more comments below here.
I mean at the end of patch 1 not this one, sorry was too quick to send it.
Changing the patch order or patches according to review is OK and adding a
new patch between already reviewed ones is OK too then you can keep
Reviewed-by on patches that did not change.
Regards,
BALATON Zoltan
Regards,
Bernhard
Regards,
BALATON Zoltan
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a9322851db..692fa8d1f9 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -43,7 +43,6 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
-qemu_irq i8259[ISA_NUM_IRQS];
int32_t pci_irq_levels[PIIX_NUM_PIRQS];
@@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num,
int level)
pic_level |= s->pci_irq_levels[i];
}
}
-qemu_set_irq(s->i8259[pic_irq], pic_level);
+qemu_set_irq(s->isa[pic_irq], pic_level);
}
}
@@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s,
PIIX_NUM_PIRQS);
-for (int i = 0; i < ISA_NUM_IRQS; i++) {
-s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
-}
-
return dev;
}
Re: [PATCH v5 1/2] semihosting/arm-compat: replace heuristic for softmmu SYS_HEAPINFO
On Fri, 11 Feb 2022 at 16:18, Philippe Mathieu-Daudé wrote: > For user-mode, this patch makes sense. For system-mode it is > not obvious to make sense of SYS_HEAPINFO (except focusing in > cores targeting embedded systems eventually). The main user of semihosting in system mode is standalone binaries compiled with the gnu toolchain using a newlib that targets semihosting. These generally use SYS_HEAPINFO on startup to find out where to put the stack etc, rather than hardwiring it in the linker map. This applies to both M-profile and A-profile. -- PMM
Re: [PATCH v4 10/13] audio/coreaudio: Remove a deprecation warning on macOS 12
On 2022/02/12 1:34, Philippe Mathieu-Daudé via wrote:
When building on macOS 12 we get:
audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is
deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations]
kAudioObjectPropertyElementMaster
^
kAudioObjectPropertyElementMain
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5:
note: 'kAudioObjectPropertyElementMaster' has been explicitly marked
deprecated here
kAudioObjectPropertyElementMaster
API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", macos(10.0,
12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) =
kAudioObjectPropertyElementMain
^
Replace by kAudioObjectPropertyElementMain, redefining it to
kAudioObjectPropertyElementMaster if not available.
Suggested-by: Akihiko Odaki
Suggested-by: Christian Schoenebeck
Suggested-by: Roman Bolshakov
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Christian Schoenebeck
---
audio/coreaudio.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index d8a21d3e50..5b3aeaced0 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
bool enabled;
} coreaudioVoiceOut;
+#if !defined(MAC_OS_VERSION_12_0) \
+|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
+#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
+#endif
+
Unless I have missed something, we have found
MAC_OS_X_VERSION_MIN_REQUIRED is better even for a constant in the
following thread:
https://lore.kernel.org/all/524515d6-2fb5-15c1-0aaf-bcda3684c...@gmail.com/
Regards,
Akihiko Odaki
static const AudioObjectPropertyAddress voice_addr = {
kAudioHardwarePropertyDefaultOutputDevice,
kAudioObjectPropertyScopeGlobal,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
static OSStatus coreaudio_get_voice(AudioDeviceID *id)
@@ -69,7 +74,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id,
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyBufferFrameSizeRange,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectGetPropertyData(id,
@@ -86,7 +91,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id,
UInt32 *framesize)
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyBufferFrameSize,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectGetPropertyData(id,
@@ -103,7 +108,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id,
UInt32 *framesize)
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyBufferFrameSize,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectSetPropertyData(id,
@@ -121,7 +126,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id,
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyStreamFormat,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectSetPropertyData(id,
@@ -138,7 +143,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id,
UInt32 *result)
AudioObjectPropertyAddress addr = {
kAudioDevicePropertyDeviceIsRunning,
kAudioDevicePropertyScopeOutput,
-kAudioObjectPropertyElementMaster
+kAudioObjectPropertyElementMain
};
return AudioObjectGetPropertyData(id,
Re: [PATCH v4 01/13] lcitool: refresh
On 2022/02/12 1:34, Philippe Mathieu-Daudé via wrote: Signed-off-by: Philippe Mathieu-Daudé --- tests/docker/dockerfiles/ubuntu1804.docker | 2 -- tests/docker/dockerfiles/ubuntu2004.docker | 2 -- 2 files changed, 4 deletions(-) diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker index 699f2dfc6a..040938277a 100644 --- a/tests/docker/dockerfiles/ubuntu1804.docker +++ b/tests/docker/dockerfiles/ubuntu1804.docker @@ -65,7 +65,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ libpam0g-dev \ libpcre2-dev \ libpixman-1-dev \ -libpmem-dev \ libpng-dev \ libpulse-dev \ librbd-dev \ @@ -89,7 +88,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ libvdeplug-dev \ libvirglrenderer-dev \ libvte-2.91-dev \ -libxen-dev \ libzstd-dev \ llvm \ locales \ diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker index 87513125b8..159e7f60c9 100644 --- a/tests/docker/dockerfiles/ubuntu2004.docker +++ b/tests/docker/dockerfiles/ubuntu2004.docker @@ -66,7 +66,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ libpam0g-dev \ libpcre2-dev \ libpixman-1-dev \ -libpmem-dev \ libpng-dev \ libpulse-dev \ librbd-dev \ @@ -91,7 +90,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ libvdeplug-dev \ libvirglrenderer-dev \ libvte-2.91-dev \ -libxen-dev \ libzstd-dev \ llvm \ locales \ This can't be applied to master. % git am ~/mbox.txt Applying: lcitool: refresh error: patch failed: tests/docker/dockerfiles/ubuntu1804.docker:89 error: tests/docker/dockerfiles/ubuntu1804.docker: patch does not apply error: patch failed: tests/docker/dockerfiles/ubuntu2004.docker:91 error: tests/docker/dockerfiles/ubuntu2004.docker: patch does not apply Patch failed at 0001 lcitool: refresh Regards, Akihiko Odaki
Re: [PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute
Am 12. Februar 2022 14:19:51 MEZ schrieb BALATON Zoltan :
>On Sat, 12 Feb 2022, Bernhard Beschow wrote:
>> This is a follow-up on patch "malta: Move PCI interrupt handling from
>> gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
>> to make the code movement more obvious. However, i8259[] seems redundant
>> to *isa, so remove it.
>
>Should this be squashed in patch 1 or at least come after it? Or are there
>some other changes needed before this patch?
I didn't want to change the patch order since they've been reviewed already.
But yeah, you're right: It makes sense to get this out of the way early in the
patch series. I will do a v3.
Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> Signed-off-by: Bernhard Beschow
>> ---
>> hw/isa/piix4.c | 7 +--
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>> index a9322851db..692fa8d1f9 100644
>> --- a/hw/isa/piix4.c
>> +++ b/hw/isa/piix4.c
>> @@ -43,7 +43,6 @@ struct PIIX4State {
>> PCIDevice dev;
>> qemu_irq cpu_intr;
>> qemu_irq *isa;
>> -qemu_irq i8259[ISA_NUM_IRQS];
>>
>> int32_t pci_irq_levels[PIIX_NUM_PIRQS];
>>
>> @@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
>> level)
>> pic_level |= s->pci_irq_levels[i];
>> }
>> }
>> -qemu_set_irq(s->i8259[pic_irq], pic_level);
>> +qemu_set_irq(s->isa[pic_irq], pic_level);
>> }
>> }
>>
>> @@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
>> **isa_bus, I2CBus **smbus)
>>
>> pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s,
>> PIIX_NUM_PIRQS);
>>
>> -for (int i = 0; i < ISA_NUM_IRQS; i++) {
>> -s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
>> -}
>> -
>> return dev;
>> }
>>
Re: [PULL 0/7] nbd: handle AioContext change correctly
On Freitag, 11. Februar 2022 14:04:44 CET Vladimir Sementsov-Ogievskiy wrote: > 11.02.2022 15:52, Peter Maydell wrote: > > On Wed, 9 Feb 2022 at 14:03, Vladimir Sementsov-Ogievskiy > > > > wrote: > >> The following changes since commit 0a301624c2f4ced3331ffd5bce85b4274fe132af: > >>Merge remote-tracking branch > >>'remotes/pmaydell/tags/pull-target-arm-20220208' into staging > >>(2022-02-08 11:40:08 +)>> > >> are available in the Git repository at: > >>https://src.openvz.org/scm/~vsementsov/qemu.git > >>tags/pull-nbd-2022-02-09 > >> > >> for you to fetch changes up to 1bd4523c2ded28b7805b971b9d3d746beabd0a94: > >>iotests/281: Let NBD connection yield in iothread (2022-02-09 14:15:29 > >>+0100)>> > >> > >> nbd: handle AioContext change correctly > > > > Hi; this pullreq is OK content-wise, but the commits are missing > > your Signed-off-by: line as the submaintainer/submitter of the > > pull request. Could you add them and resend, please? > > Oops, sorry. I thought I'm already an experienced maintainer and don't need > to re-read "Submitting a Pull Request" instruction every time. I was > wrong:) Assuming you are using pwclient to pull patchtes, just add signoff=on to your .pwclientrc config file to automatically add your signoff tag to pulled patches. Additionally I would recommend adding: msgid=on which will add the email message id of each patch, so that people can review the discussions on qemu-devel later on if needed. On Linux kernel side it's now more common to add a Link: tag, which is a bit more convenient as people can just click on it to get to the discussion. Not sure if there is config option for pwclient for that as well. Best regards, Christian Schoenebeck
Re: [PATCH v2 2/5] pci: Always pass own DeviceState to pci_map_irq_fn's
Am 12. Februar 2022 14:27:32 MEZ schrieb BALATON Zoltan :
>On Sat, 12 Feb 2022, Bernhard Beschow wrote:
>> Passing own DeviceState rather than just the IRQs allows for resolving
>> global variables.
>
>Do you mean pci_set_irq_fn instead of pci_map_irq_fn in the patch title?
I'm referring to the typedef in pci.h because the patch establishes
consistency across the board.
>> Signed-off-by: Bernhard Beschow
>> Reviewed-by: Peter Maydell
>> Reviewed-by: Philippe Mathieu-Daudé
>> ---
>> hw/isa/piix4.c | 6 +++---
>> hw/pci-host/sh_pci.c| 6 +++---
>> hw/pci-host/versatile.c | 6 +++---
>> hw/ppc/ppc440_pcix.c| 6 +++---
>> hw/ppc/ppc4xx_pci.c | 6 +++---
>> 5 files changed, 15 insertions(+), 15 deletions(-)
>
>You don't seem to change any global function definition that reqires this
>change in all these devices so why can't these decide on their own what
>their preferred opaque data is for their set irq function and only change
>piix4? Am I missing something? I'm not opposed to this change but it seems
>to be unnecessary to touch other device implementations for this and may
>just make them more complex for no good reason.
This patch is a segway into a direction where the type of the opaque parameter
of the pci_map_irq_fn typedef could be changed from void* to DeviceState* in
order to facilitate the more typesafe QOM casting. I see, though, why this is
confusing in the context of this patch series. I don't have strong feelings for
converting the other devices or not. So I can only change piix4 if desired.
Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>> index 5a86308689..a31e9714cf 100644
>> --- a/hw/isa/piix4.c
>> +++ b/hw/isa/piix4.c
>> @@ -60,7 +60,7 @@ static int pci_irq_levels[4];
>> static void piix4_set_irq(void *opaque, int irq_num, int level)
>> {
>> int i, pic_irq, pic_level;
>> -qemu_irq *pic = opaque;
>> +PIIX4State *s = opaque;
>>
>> pci_irq_levels[irq_num] = level;
>>
>> @@ -75,7 +75,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
>> level)
>> pic_level |= pci_irq_levels[i];
>> }
>> }
>> -qemu_set_irq(pic[pic_irq], pic_level);
>> +qemu_set_irq(s->i8259[pic_irq], pic_level);
>> }
>> }
>>
>> @@ -323,7 +323,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
>> **isa_bus, I2CBus **smbus)
>>NULL, 0, NULL);
>> }
>>
>> -pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s->i8259, 4);
>> +pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, 4);
>>
>> for (int i = 0; i < ISA_NUM_IRQS; i++) {
>> s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
>> diff --git a/hw/pci-host/sh_pci.c b/hw/pci-host/sh_pci.c
>> index 719d6ca2a6..ae0aa462b3 100644
>> --- a/hw/pci-host/sh_pci.c
>> +++ b/hw/pci-host/sh_pci.c
>> @@ -111,9 +111,9 @@ static int sh_pci_map_irq(PCIDevice *d, int irq_num)
>>
>> static void sh_pci_set_irq(void *opaque, int irq_num, int level)
>> {
>> -qemu_irq *pic = opaque;
>> +SHPCIState *s = opaque;
>>
>> -qemu_set_irq(pic[irq_num], level);
>> +qemu_set_irq(s->irq[irq_num], level);
>> }
>>
>> static void sh_pci_device_realize(DeviceState *dev, Error **errp)
>> @@ -128,7 +128,7 @@ static void sh_pci_device_realize(DeviceState *dev,
>> Error **errp)
>> }
>> phb->bus = pci_register_root_bus(dev, "pci",
>> sh_pci_set_irq, sh_pci_map_irq,
>> - s->irq,
>> + s,
>> get_system_memory(),
>> get_system_io(),
>> PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
>> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
>> index f66384fa02..5fbcb72d7d 100644
>> --- a/hw/pci-host/versatile.c
>> +++ b/hw/pci-host/versatile.c
>> @@ -362,9 +362,9 @@ static int pci_vpb_rv_map_irq(PCIDevice *d, int irq_num)
>>
>> static void pci_vpb_set_irq(void *opaque, int irq_num, int level)
>> {
>> -qemu_irq *pic = opaque;
>> +PCIVPBState *s = opaque;
>>
>> -qemu_set_irq(pic[irq_num], level);
>> +qemu_set_irq(s->irq[irq_num], level);
>> }
>>
>> static void pci_vpb_reset(DeviceState *d)
>> @@ -422,7 +422,7 @@ static void pci_vpb_realize(DeviceState *dev, Error
>> **errp)
>> mapfn = pci_vpb_map_irq;
>> }
>>
>> -pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4);
>> +pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s, 4);
>>
>> /* Our memory regions are:
>> * 0 : our control registers
>> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
>> index 788d25514a..291c1bfbe7 100644
>> --- a/hw/ppc/ppc440_pcix.c
>> +++ b/hw/ppc/ppc440_pcix.c
>> @@ -431,14 +431,14 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int
>> irq_num)
>>
>> static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
>> {
>> -
Re: [PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute
Am 12. Februar 2022 14:19:51 MEZ schrieb BALATON Zoltan :
>On Sat, 12 Feb 2022, Bernhard Beschow wrote:
>> This is a follow-up on patch "malta: Move PCI interrupt handling from
>> gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
>> to make the code movement more obvious. However, i8259[] seems redundant
>> to *isa, so remove it.
>
>Should this be squashed in patch 1 or at least come after it? Or are there
>some other changes needed before this patch?
I didn't want to change the patch order since they've been reviewed already.
But yeah, you're right: It makes sense to get this out of the way early in the
patch series. I will do a v3.
Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> Signed-off-by: Bernhard Beschow
>> ---
>> hw/isa/piix4.c | 7 +--
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>> index a9322851db..692fa8d1f9 100644
>> --- a/hw/isa/piix4.c
>> +++ b/hw/isa/piix4.c
>> @@ -43,7 +43,6 @@ struct PIIX4State {
>> PCIDevice dev;
>> qemu_irq cpu_intr;
>> qemu_irq *isa;
>> -qemu_irq i8259[ISA_NUM_IRQS];
>>
>> int32_t pci_irq_levels[PIIX_NUM_PIRQS];
>>
>> @@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
>> level)
>> pic_level |= s->pci_irq_levels[i];
>> }
>> }
>> -qemu_set_irq(s->i8259[pic_irq], pic_level);
>> +qemu_set_irq(s->isa[pic_irq], pic_level);
>> }
>> }
>>
>> @@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
>> **isa_bus, I2CBus **smbus)
>>
>> pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s,
>> PIIX_NUM_PIRQS);
>>
>> -for (int i = 0; i < ISA_NUM_IRQS; i++) {
>> -s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
>> -}
>> -
>> return dev;
>> }
>>
Re: [PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
Now that pci_irq_levels[] is part of PIIX4State, the PCI IRQ levels can
be saved and restored via the VMState mechanism. This fixes migrated VMs
to start with PCI IRQ levels zeroed, which is a bug.
Suggested-by: Peter Maydell
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 964e09cf7f..a9322851db 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -45,7 +45,7 @@ struct PIIX4State {
qemu_irq *isa;
qemu_irq i8259[ISA_NUM_IRQS];
-int pci_irq_levels[PIIX_NUM_PIRQS];
+int32_t pci_irq_levels[PIIX_NUM_PIRQS];
Do you really need 32 bits to store a value of 0 or 1? I saw piix3 has
that too but do we need to do the same here? Could this be just an uint8_t
array? That's still an overkill to store 4 bits of information but less so
than with int32_t.
By the way the corresponding member in struct PIIXState in
include/hw/southbridge/piix.h has a comment saying:
/* This member isn't used. Just for save/load compatibility */
int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
and only seems to be filled in piix3_pre_save() but never used. So what's
the point of this then? Maybe piix3 also uses a bitmap to store these
levels instead? There's a uint64_t pic_levels member above the unused
array but I haven't checked the implementation.
Regards,
BALATON Zoltan
RTCState rtc;
/* Reset Control Register */
@@ -128,12 +128,14 @@ static int piix4_ide_post_load(void *opaque, int
version_id)
static const VMStateDescription vmstate_piix4 = {
.name = "PIIX4",
-.version_id = 3,
+.version_id = 4,
.minimum_version_id = 2,
.post_load = piix4_ide_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCI_DEVICE(dev, PIIX4State),
VMSTATE_UINT8_V(rcr, PIIX4State, 3),
+VMSTATE_INT32_ARRAY_V(pci_irq_levels, PIIX4State,
+ PIIX_NUM_PIRQS, 4),
VMSTATE_END_OF_LIST()
}
};
Re: [PATCH v4 0/6] Introduce CanoKey QEMU
Hi, Is there any further feedback on this patch set. Regards, Zenithal
Re: [PATCH v4 09/13] block/file-posix: Remove a deprecation warning on macOS 12
On Freitag, 11. Februar 2022 17:34:30 CET Philippe Mathieu-Daudé via wrote:
> When building on macOS 12 we get:
>
> block/file-posix.c:3335:18: warning: 'IOMasterPort' is deprecated: first
> deprecated in macOS 12.0 [-Wdeprecated-declarations] kernResult =
> IOMasterPort( MACH_PORT_NULL, &masterPort );
>^~~~
>IOMainPort
>
> Replace by IOMainPort, redefining it to IOMasterPort if not available.
>
> Suggested-by: Akihiko Odaki
> Signed-off-by: Philippe Mathieu-Daudé
> ---
Reviewed-by: Christian Schoenebeck
> block/file-posix.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 1f1756e192..13393ad296 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -3319,17 +3319,23 @@ BlockDriver bdrv_file = {
> #if defined(__APPLE__) && defined(__MACH__)
> static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
> CFIndex maxPathSize, int flags);
> +
> +#if !defined(MAC_OS_VERSION_12_0) \
> +|| (MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_VERSION_12_0)
> +#define IOMainPort IOMasterPort
> +#endif
> +
> static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator)
> {
> kern_return_t kernResult = KERN_FAILURE;
> -mach_port_t masterPort;
> +mach_port_t mainPort;
> CFMutableDictionaryRef classesToMatch;
> const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass};
> char *mediaType = NULL;
>
> -kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort );
> +kernResult = IOMainPort(MACH_PORT_NULL, &mainPort);
> if ( KERN_SUCCESS != kernResult ) {
> -printf( "IOMasterPort returned %d\n", kernResult );
> +printf("IOMainPort returned %d\n", kernResult);
> }
>
> int index;
> @@ -3342,7 +3348,7 @@ static char *FindEjectableOpticalMedia(io_iterator_t
> *mediaIterator) }
> CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),
> kCFBooleanTrue);
> -kernResult = IOServiceGetMatchingServices(masterPort,
> classesToMatch, +kernResult =
> IOServiceGetMatchingServices(mainPort, classesToMatch, mediaIterator); if
> (kernResult != KERN_SUCCESS) {
> error_report("Note: IOServiceGetMatchingServices returned %d",
Re: [PATCH v2 2/5] pci: Always pass own DeviceState to pci_map_irq_fn's
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
Passing own DeviceState rather than just the IRQs allows for resolving
global variables.
Do you mean pci_set_irq_fn instead of pci_map_irq_fn in the patch title?
Signed-off-by: Bernhard Beschow
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix4.c | 6 +++---
hw/pci-host/sh_pci.c| 6 +++---
hw/pci-host/versatile.c | 6 +++---
hw/ppc/ppc440_pcix.c| 6 +++---
hw/ppc/ppc4xx_pci.c | 6 +++---
5 files changed, 15 insertions(+), 15 deletions(-)
You don't seem to change any global function definition that reqires this
change in all these devices so why can't these decide on their own what
their preferred opaque data is for their set irq function and only change
piix4? Am I missing something? I'm not opposed to this change but it seems
to be unnecessary to touch other device implementations for this and may
just make them more complex for no good reason.
Regards,
BALATON Zoltan
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 5a86308689..a31e9714cf 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -60,7 +60,7 @@ static int pci_irq_levels[4];
static void piix4_set_irq(void *opaque, int irq_num, int level)
{
int i, pic_irq, pic_level;
-qemu_irq *pic = opaque;
+PIIX4State *s = opaque;
pci_irq_levels[irq_num] = level;
@@ -75,7 +75,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
pic_level |= pci_irq_levels[i];
}
}
-qemu_set_irq(pic[pic_irq], pic_level);
+qemu_set_irq(s->i8259[pic_irq], pic_level);
}
}
@@ -323,7 +323,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
NULL, 0, NULL);
}
-pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s->i8259, 4);
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, 4);
for (int i = 0; i < ISA_NUM_IRQS; i++) {
s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
diff --git a/hw/pci-host/sh_pci.c b/hw/pci-host/sh_pci.c
index 719d6ca2a6..ae0aa462b3 100644
--- a/hw/pci-host/sh_pci.c
+++ b/hw/pci-host/sh_pci.c
@@ -111,9 +111,9 @@ static int sh_pci_map_irq(PCIDevice *d, int irq_num)
static void sh_pci_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pic = opaque;
+SHPCIState *s = opaque;
-qemu_set_irq(pic[irq_num], level);
+qemu_set_irq(s->irq[irq_num], level);
}
static void sh_pci_device_realize(DeviceState *dev, Error **errp)
@@ -128,7 +128,7 @@ static void sh_pci_device_realize(DeviceState *dev, Error
**errp)
}
phb->bus = pci_register_root_bus(dev, "pci",
sh_pci_set_irq, sh_pci_map_irq,
- s->irq,
+ s,
get_system_memory(),
get_system_io(),
PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index f66384fa02..5fbcb72d7d 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -362,9 +362,9 @@ static int pci_vpb_rv_map_irq(PCIDevice *d, int irq_num)
static void pci_vpb_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pic = opaque;
+PCIVPBState *s = opaque;
-qemu_set_irq(pic[irq_num], level);
+qemu_set_irq(s->irq[irq_num], level);
}
static void pci_vpb_reset(DeviceState *d)
@@ -422,7 +422,7 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
mapfn = pci_vpb_map_irq;
}
-pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4);
+pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s, 4);
/* Our memory regions are:
* 0 : our control registers
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 788d25514a..291c1bfbe7 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -431,14 +431,14 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int
irq_num)
static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pci_irq = opaque;
+PPC440PCIXState *s = opaque;
trace_ppc440_pcix_set_irq(irq_num);
if (irq_num < 0) {
error_report("%s: PCI irq %d", __func__, irq_num);
return;
}
-qemu_set_irq(*pci_irq, level);
+qemu_set_irq(s->irq, level);
}
static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
@@ -492,7 +492,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
**errp)
sysbus_init_irq(sbd, &s->irq);
memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
- ppc440_pcix_map_irq, &s->irq, &s->busmem,
+ ppc440_pcix_map_irq, s, &s->busmem,
get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
s->dev = pci_create_simple(h->bus, PCI
Re: [PATCH v4 12/13] ui/cocoa: Remove allowedFileTypes restriction in SavePanel
On Freitag, 11. Februar 2022 17:34:33 CET Philippe Mathieu-Daudé via wrote:
> setAllowedFileTypes is deprecated in macOS 12.
>
> Per Akihiko Odaki [*]:
>
> An image file, which is being chosen by the panel, can be a
> raw file and have a variety of file extensions and many are not
> covered by the provided list (e.g. "udf"). Other platforms like
> GTK can provide an option to open a file with an extension not
> listed, but Cocoa can't. It forces the user to rename the file
> to give an extension in the list. Moreover, Cocoa does not tell
> which extensions are in the list so the user needs to read the
> source code, which is pretty bad.
>
> Since this code is harming the usability rather than improving it,
> simply remove the [NSSavePanel allowedFileTypes:] call, fixing:
>
> [2789/6622] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
> ui/cocoa.m:1411:16: error: 'setAllowedFileTypes:' is deprecated: first
> deprecated in macOS 12.0 - Use -allowedContentTypes instead
> [-Werror,-Wdeprecated-declarations] [openPanel setAllowedFileTypes:
> supportedImageFileTypes];
> ^
>
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Framewor
> ks/AppKit.framework/Headers/NSSavePanel.h:215:49: note: property
> 'allowedFileTypes' is declared deprecated here @property (nullable, copy)
> NSArray *allowedFileTypes API_DEPRECATED("Use
> -allowedContentTypes instead", macos(10.3,12.0)); ^
>
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Framewor
> ks/AppKit.framework/Headers/NSSavePanel.h:215:49: note:
> 'setAllowedFileTypes:' has been explicitly marked deprecated here FAILED:
> libcommon.fa.p/ui_cocoa.m.o
>
> [*]
> https://lore.kernel.org/qemu-devel/4dde2e66-63cb-4390-9538-c032310db3e3@gma
> il.com/
>
> Suggested-by: Akihiko Odaki
> Reviewed-by: Roman Bolshakov
> Tested-by: Roman Bolshakov
> Signed-off-by: Philippe Mathieu-Daudé
> ---
Reviewed-by: Christian Schoenebeck
> ui/cocoa.m | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index ac18e14ce0..7a1ddd4075 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -100,7 +100,6 @@ static int gArgc;
> static char **gArgv;
> static bool stretch_video;
> static NSTextField *pauseLabel;
> -static NSArray * supportedImageFileTypes;
>
> static QemuSemaphore display_init_sem;
> static QemuSemaphore app_started_sem;
> @@ -1168,10 +1167,6 @@ QemuCocoaView *cocoaView;
> [pauseLabel setTextColor: [NSColor blackColor]];
> [pauseLabel sizeToFit];
>
> -// set the supported image file types that can be opened
> -supportedImageFileTypes = [NSArray arrayWithObjects: @"img",
> @"iso", @"dmg", - @"qcow", @"qcow2",
> @"cloop", @"vmdk", @"cdr", - @"toast",
> nil];
> [self make_about_window];
> }
> return self;
> @@ -1414,7 +1409,6 @@ QemuCocoaView *cocoaView;
> openPanel = [NSOpenPanel openPanel];
> [openPanel setCanChooseFiles: YES];
> [openPanel setAllowsMultipleSelection: NO];
> -[openPanel setAllowedFileTypes: supportedImageFileTypes];
> if([openPanel runModal] == NSModalResponseOK) {
> NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
> if(file == nil) {
Re: [PATCH v2 1/5] malta: Move PCI interrupt handling from gt64xxx to piix4
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
Handling PCI interrupts in piix4 increases cohesion and reduces differences
between piix4 and piix3.
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Sorry for being late in commenting, I've missed the first round. Apologies
if this causes a delay or another version.
---
hw/isa/piix4.c | 58 +++
hw/mips/gt64xxx_pci.c | 62 --
hw/mips/malta.c| 6 +---
include/hw/mips/mips.h | 2 +-
4 files changed, 65 insertions(+), 63 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0fe7b69bc4..5a86308689 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -45,6 +45,7 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
+qemu_irq i8259[ISA_NUM_IRQS];
RTCState rtc;
/* Reset Control Register */
@@ -54,6 +55,30 @@ struct PIIX4State {
OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
+static int pci_irq_levels[4];
+
+static void piix4_set_irq(void *opaque, int irq_num, int level)
+{
+int i, pic_irq, pic_level;
+qemu_irq *pic = opaque;
+
+pci_irq_levels[irq_num] = level;
+
+/* now we change the pic irq level according to the piix irq mappings */
+/* XXX: optimize */
+pic_irq = piix4_dev->config[PIIX_PIRQCA + irq_num];
+if (pic_irq < 16) {
+/* The pic level is the logical OR of all the PCI irqs mapped to it. */
+pic_level = 0;
+for (i = 0; i < 4; i++) {
+if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
+pic_level |= pci_irq_levels[i];
+}
+}
+qemu_set_irq(pic[pic_irq], pic_level);
+}
+}
+
static void piix4_isa_reset(DeviceState *dev)
{
PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -248,8 +273,34 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+int slot;
+
+slot = PCI_SLOT(pci_dev->devfn);
+
+switch (slot) {
+/* PIIX4 USB */
+case 10:
+return 3;
+/* AMD 79C973 Ethernet */
+case 11:
+return 1;
+/* Crystal 4281 Sound */
+case 12:
+return 2;
+/* PCI slot 1 to 4 */
+case 18 ... 21:
+return ((slot - 18) + irq_num) & 0x03;
+/* Unknown device, don't do any translation */
+default:
+return irq_num;
+}
+}
+
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
+PIIX4State *s;
PCIDevice *pci;
DeviceState *dev;
int devfn = PCI_DEVFN(10, 0);
@@ -257,6 +308,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci = pci_create_simple_multifunction(pci_bus, devfn, true,
TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
+s = PIIX4_PCI_DEVICE(pci);
if (isa_bus) {
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
@@ -271,5 +323,11 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
NULL, 0, NULL);
}
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s->i8259, 4);
+
+for (int i = 0; i < ISA_NUM_IRQS; i++) {
+s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
+}
+
return dev;
}
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index c7480bd019..9e23e32eff 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -981,56 +981,6 @@ static const MemoryRegionOps isd_mem_ops = {
},
};
-static int gt64120_pci_map_irq(PCIDevice *pci_dev, int irq_num)
-{
-int slot;
-
-slot = PCI_SLOT(pci_dev->devfn);
-
-switch (slot) {
-/* PIIX4 USB */
-case 10:
-return 3;
-/* AMD 79C973 Ethernet */
-case 11:
-return 1;
-/* Crystal 4281 Sound */
-case 12:
-return 2;
-/* PCI slot 1 to 4 */
-case 18 ... 21:
-return ((slot - 18) + irq_num) & 0x03;
-/* Unknown device, don't do any translation */
-default:
-return irq_num;
-}
-}
-
-static int pci_irq_levels[4];
-
-static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
-{
-int i, pic_irq, pic_level;
-qemu_irq *pic = opaque;
-
-pci_irq_levels[irq_num] = level;
-
-/* now we change the pic irq level according to the piix irq mappings */
-/* XXX: optimize */
-pic_irq = piix4_dev->config[PIIX_PIRQCA + irq_num];
-if (pic_irq < 16) {
-/* The pic level is the logical OR of all the PCI irqs mapped to it. */
-pic_level = 0;
-for (i = 0; i < 4; i++) {
-if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
-pic_level |= pci_irq_levels[i];
-}
-}
-qemu_set_irq(pic[pic_irq], pic_level);
-}
-}
-
-
static void gt64120_reset(DeviceState *dev)
{
GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
@@ -1207,7 +1157,7 @@ static void gt64120_realize(
Re: [PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute
On Sat, 12 Feb 2022, Bernhard Beschow wrote:
This is a follow-up on patch "malta: Move PCI interrupt handling from
gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
to make the code movement more obvious. However, i8259[] seems redundant
to *isa, so remove it.
Should this be squashed in patch 1 or at least come after it? Or are there
some other changes needed before this patch?
Regards,
BALATON Zoltan
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a9322851db..692fa8d1f9 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -43,7 +43,6 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
-qemu_irq i8259[ISA_NUM_IRQS];
int32_t pci_irq_levels[PIIX_NUM_PIRQS];
@@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
pic_level |= s->pci_irq_levels[i];
}
}
-qemu_set_irq(s->i8259[pic_irq], pic_level);
+qemu_set_irq(s->isa[pic_irq], pic_level);
}
}
@@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
-for (int i = 0; i < ISA_NUM_IRQS; i++) {
-s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
-}
-
return dev;
}
Re: [PATCH v4 10/13] audio/coreaudio: Remove a deprecation warning on macOS 12
On Freitag, 11. Februar 2022 17:34:31 CET Philippe Mathieu-Daudé via wrote:
> When building on macOS 12 we get:
>
> audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is
> deprecated: first deprecated in macOS 12.0
> [-Werror,-Wdeprecated-declarations] kAudioObjectPropertyElementMaster
> ^
> kAudioObjectPropertyElementMain
>
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Framewor
> ks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: note:
> 'kAudioObjectPropertyElementMaster' has been explicitly marked deprecated
> here kAudioObjectPropertyElementMaster
> API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain",
> macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) =
> kAudioObjectPropertyElementMain ^
>
> Replace by kAudioObjectPropertyElementMain, redefining it to
> kAudioObjectPropertyElementMaster if not available.
>
> Suggested-by: Akihiko Odaki
> Suggested-by: Christian Schoenebeck
> Suggested-by: Roman Bolshakov
> Signed-off-by: Philippe Mathieu-Daudé
> ---
Reviewed-by: Christian Schoenebeck
> audio/coreaudio.c | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index d8a21d3e50..5b3aeaced0 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -44,10 +44,15 @@ typedef struct coreaudioVoiceOut {
> bool enabled;
> } coreaudioVoiceOut;
>
> +#if !defined(MAC_OS_VERSION_12_0) \
> +|| (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
> +#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
> +#endif
> +
> static const AudioObjectPropertyAddress voice_addr = {
> kAudioHardwarePropertyDefaultOutputDevice,
> kAudioObjectPropertyScopeGlobal,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
> };
>
> static OSStatus coreaudio_get_voice(AudioDeviceID *id)
> @@ -69,7 +74,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID
> id, AudioObjectPropertyAddress addr = {
> kAudioDevicePropertyBufferFrameSizeRange,
> kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
> };
>
> return AudioObjectGetPropertyData(id,
> @@ -86,7 +91,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id,
> UInt32 *framesize) AudioObjectPropertyAddress addr = {
> kAudioDevicePropertyBufferFrameSize,
> kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
> };
>
> return AudioObjectGetPropertyData(id,
> @@ -103,7 +108,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID
> id, UInt32 *framesize) AudioObjectPropertyAddress addr = {
> kAudioDevicePropertyBufferFrameSize,
> kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
> };
>
> return AudioObjectSetPropertyData(id,
> @@ -121,7 +126,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID
> id, AudioObjectPropertyAddress addr = {
> kAudioDevicePropertyStreamFormat,
> kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
> };
>
> return AudioObjectSetPropertyData(id,
> @@ -138,7 +143,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID
> id, UInt32 *result) AudioObjectPropertyAddress addr = {
> kAudioDevicePropertyDeviceIsRunning,
> kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
> };
>
> return AudioObjectGetPropertyData(id,
[PATCH] multifd: ensure multifd threads are terminated before cleanup params
In multifd_save_cleanup(), we terminate all multifd threads and destroy
the 'p->mutex', while the mutex may still be held by multifd send thread,
this causes qemu to crash.
It's because the multifd_send_thread maybe scheduled out after setting
'p->running' to false. To reproduce the problem, we put
'multifd_send_thread' to sleep seconds before unlock 'p->mutex':
function multifd_send_thread()
{
...
qemu_mutex_lock(&p->mutex);
p->running = false;
usleep(500);
qemu_mutex_unlock(&p->mutex);
...
}
As the 'p->running' is used to indicate whether the multifd_send/recv thread
is created, it should be set to false after the thread terminate.
Signed-off-by: Wang Xin
Signed-off-by: Huangyu Zhai
diff --git a/migration/multifd.c b/migration/multifd.c
index 76b57a7177..d8fc7d319e 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -526,6 +526,7 @@ void multifd_save_cleanup(void)
if (p->running) {
qemu_thread_join(&p->thread);
+p->running = false;
}
}
for (i = 0; i < migrate_multifd_channels(); i++) {
@@ -707,10 +708,6 @@ out:
qemu_sem_post(&multifd_send_state->channels_ready);
}
-qemu_mutex_lock(&p->mutex);
-p->running = false;
-qemu_mutex_unlock(&p->mutex);
-
rcu_unregister_thread();
trace_multifd_send_thread_end(p->id, p->num_packets,
p->total_normal_pages);
@@ -995,6 +992,7 @@ int multifd_load_cleanup(Error **errp)
*/
qemu_sem_post(&p->sem_sync);
qemu_thread_join(&p->thread);
+p->running = false;
}
}
for (i = 0; i < migrate_multifd_channels(); i++) {
@@ -1110,9 +1108,6 @@ static void *multifd_recv_thread(void *opaque)
multifd_recv_terminate_threads(local_err);
error_free(local_err);
}
-qemu_mutex_lock(&p->mutex);
-p->running = false;
-qemu_mutex_unlock(&p->mutex);
rcu_unregister_thread();
trace_multifd_recv_thread_end(p->id, p->num_packets,
p->total_normal_pages);
--
2.26.0.windows.1
[PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute
This is a follow-up on patch "malta: Move PCI interrupt handling from
gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
to make the code movement more obvious. However, i8259[] seems redundant
to *isa, so remove it.
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a9322851db..692fa8d1f9 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -43,7 +43,6 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
-qemu_irq i8259[ISA_NUM_IRQS];
int32_t pci_irq_levels[PIIX_NUM_PIRQS];
@@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
pic_level |= s->pci_irq_levels[i];
}
}
-qemu_set_irq(s->i8259[pic_irq], pic_level);
+qemu_set_irq(s->isa[pic_irq], pic_level);
}
}
@@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
-for (int i = 0; i < ISA_NUM_IRQS; i++) {
-s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
-}
-
return dev;
}
--
2.35.1
[PATCH v2 4/5] isa/piix4: Fix PCI IRQ levels to be preserved in VMState
Now that pci_irq_levels[] is part of PIIX4State, the PCI IRQ levels can
be saved and restored via the VMState mechanism. This fixes migrated VMs
to start with PCI IRQ levels zeroed, which is a bug.
Suggested-by: Peter Maydell
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Bernhard Beschow
---
hw/isa/piix4.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 964e09cf7f..a9322851db 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -45,7 +45,7 @@ struct PIIX4State {
qemu_irq *isa;
qemu_irq i8259[ISA_NUM_IRQS];
-int pci_irq_levels[PIIX_NUM_PIRQS];
+int32_t pci_irq_levels[PIIX_NUM_PIRQS];
RTCState rtc;
/* Reset Control Register */
@@ -128,12 +128,14 @@ static int piix4_ide_post_load(void *opaque, int
version_id)
static const VMStateDescription vmstate_piix4 = {
.name = "PIIX4",
-.version_id = 3,
+.version_id = 4,
.minimum_version_id = 2,
.post_load = piix4_ide_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCI_DEVICE(dev, PIIX4State),
VMSTATE_UINT8_V(rcr, PIIX4State, 3),
+VMSTATE_INT32_ARRAY_V(pci_irq_levels, PIIX4State,
+ PIIX_NUM_PIRQS, 4),
VMSTATE_END_OF_LIST()
}
};
--
2.35.1
[PATCH v2 3/5] isa/piix4: Resolve global variables
Now that piix4_set_irq's opaque parameter references own PIIX4State,
piix4_dev becomes redundant and pci_irq_levels can be moved into PIIX4State.
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix4.c| 22 +-
include/hw/southbridge/piix.h | 2 --
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a31e9714cf..964e09cf7f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -39,14 +39,14 @@
#include "sysemu/runstate.h"
#include "qom/object.h"
-PCIDevice *piix4_dev;
-
struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
qemu_irq i8259[ISA_NUM_IRQS];
+int pci_irq_levels[PIIX_NUM_PIRQS];
+
RTCState rtc;
/* Reset Control Register */
MemoryRegion rcr_mem;
@@ -55,24 +55,22 @@ struct PIIX4State {
OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
-static int pci_irq_levels[4];
-
static void piix4_set_irq(void *opaque, int irq_num, int level)
{
int i, pic_irq, pic_level;
PIIX4State *s = opaque;
-pci_irq_levels[irq_num] = level;
+s->pci_irq_levels[irq_num] = level;
/* now we change the pic irq level according to the piix irq mappings */
/* XXX: optimize */
-pic_irq = piix4_dev->config[PIIX_PIRQCA + irq_num];
-if (pic_irq < 16) {
+pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
+if (pic_irq < ISA_NUM_IRQS) {
/* The pic level is the logical OR of all the PCI irqs mapped to it. */
pic_level = 0;
-for (i = 0; i < 4; i++) {
-if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
-pic_level |= pci_irq_levels[i];
+for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
+pic_level |= s->pci_irq_levels[i];
}
}
qemu_set_irq(s->i8259[pic_irq], pic_level);
@@ -223,8 +221,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
return;
}
isa_init_irq(ISA_DEVICE(&s->rtc), &s->rtc.irq, RTC_ISA_IRQ);
-
-piix4_dev = dev;
}
static void piix4_init(Object *obj)
@@ -323,7 +319,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
NULL, 0, NULL);
}
-pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, 4);
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
for (int i = 0; i < ISA_NUM_IRQS; i++) {
s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 6387f2b612..f63f83e5c6 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,8 +70,6 @@ typedef struct PIIXState PIIX3State;
DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
TYPE_PIIX3_PCI_DEVICE)
-extern PCIDevice *piix4_dev;
-
PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
--
2.35.1
[PATCH v2 2/5] pci: Always pass own DeviceState to pci_map_irq_fn's
Passing own DeviceState rather than just the IRQs allows for resolving
global variables.
Signed-off-by: Bernhard Beschow
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix4.c | 6 +++---
hw/pci-host/sh_pci.c| 6 +++---
hw/pci-host/versatile.c | 6 +++---
hw/ppc/ppc440_pcix.c| 6 +++---
hw/ppc/ppc4xx_pci.c | 6 +++---
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 5a86308689..a31e9714cf 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -60,7 +60,7 @@ static int pci_irq_levels[4];
static void piix4_set_irq(void *opaque, int irq_num, int level)
{
int i, pic_irq, pic_level;
-qemu_irq *pic = opaque;
+PIIX4State *s = opaque;
pci_irq_levels[irq_num] = level;
@@ -75,7 +75,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
pic_level |= pci_irq_levels[i];
}
}
-qemu_set_irq(pic[pic_irq], pic_level);
+qemu_set_irq(s->i8259[pic_irq], pic_level);
}
}
@@ -323,7 +323,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
NULL, 0, NULL);
}
-pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s->i8259, 4);
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, 4);
for (int i = 0; i < ISA_NUM_IRQS; i++) {
s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
diff --git a/hw/pci-host/sh_pci.c b/hw/pci-host/sh_pci.c
index 719d6ca2a6..ae0aa462b3 100644
--- a/hw/pci-host/sh_pci.c
+++ b/hw/pci-host/sh_pci.c
@@ -111,9 +111,9 @@ static int sh_pci_map_irq(PCIDevice *d, int irq_num)
static void sh_pci_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pic = opaque;
+SHPCIState *s = opaque;
-qemu_set_irq(pic[irq_num], level);
+qemu_set_irq(s->irq[irq_num], level);
}
static void sh_pci_device_realize(DeviceState *dev, Error **errp)
@@ -128,7 +128,7 @@ static void sh_pci_device_realize(DeviceState *dev, Error
**errp)
}
phb->bus = pci_register_root_bus(dev, "pci",
sh_pci_set_irq, sh_pci_map_irq,
- s->irq,
+ s,
get_system_memory(),
get_system_io(),
PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index f66384fa02..5fbcb72d7d 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -362,9 +362,9 @@ static int pci_vpb_rv_map_irq(PCIDevice *d, int irq_num)
static void pci_vpb_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pic = opaque;
+PCIVPBState *s = opaque;
-qemu_set_irq(pic[irq_num], level);
+qemu_set_irq(s->irq[irq_num], level);
}
static void pci_vpb_reset(DeviceState *d)
@@ -422,7 +422,7 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
mapfn = pci_vpb_map_irq;
}
-pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4);
+pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s, 4);
/* Our memory regions are:
* 0 : our control registers
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 788d25514a..291c1bfbe7 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -431,14 +431,14 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int
irq_num)
static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pci_irq = opaque;
+PPC440PCIXState *s = opaque;
trace_ppc440_pcix_set_irq(irq_num);
if (irq_num < 0) {
error_report("%s: PCI irq %d", __func__, irq_num);
return;
}
-qemu_set_irq(*pci_irq, level);
+qemu_set_irq(s->irq, level);
}
static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
@@ -492,7 +492,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
**errp)
sysbus_init_irq(sbd, &s->irq);
memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
- ppc440_pcix_map_irq, &s->irq, &s->busmem,
+ ppc440_pcix_map_irq, s, &s->busmem,
get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 5df97e6d15..f6718746a1 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -256,11 +256,11 @@ static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int
irq_num)
static void ppc4xx_pci_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pci_irqs = opaque;
+PPC4xxPCIState *s = opaque;
trace_ppc4xx_pci_set_irq(irq_num);
assert(irq_num >= 0 && irq_num < PPC4xx_PCI_NUM_DE
[PATCH v2 1/5] malta: Move PCI interrupt handling from gt64xxx to piix4
Handling PCI interrupts in piix4 increases cohesion and reduces differences
between piix4 and piix3.
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix4.c | 58 +++
hw/mips/gt64xxx_pci.c | 62 --
hw/mips/malta.c| 6 +---
include/hw/mips/mips.h | 2 +-
4 files changed, 65 insertions(+), 63 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0fe7b69bc4..5a86308689 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -45,6 +45,7 @@ struct PIIX4State {
PCIDevice dev;
qemu_irq cpu_intr;
qemu_irq *isa;
+qemu_irq i8259[ISA_NUM_IRQS];
RTCState rtc;
/* Reset Control Register */
@@ -54,6 +55,30 @@ struct PIIX4State {
OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
+static int pci_irq_levels[4];
+
+static void piix4_set_irq(void *opaque, int irq_num, int level)
+{
+int i, pic_irq, pic_level;
+qemu_irq *pic = opaque;
+
+pci_irq_levels[irq_num] = level;
+
+/* now we change the pic irq level according to the piix irq mappings */
+/* XXX: optimize */
+pic_irq = piix4_dev->config[PIIX_PIRQCA + irq_num];
+if (pic_irq < 16) {
+/* The pic level is the logical OR of all the PCI irqs mapped to it. */
+pic_level = 0;
+for (i = 0; i < 4; i++) {
+if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
+pic_level |= pci_irq_levels[i];
+}
+}
+qemu_set_irq(pic[pic_irq], pic_level);
+}
+}
+
static void piix4_isa_reset(DeviceState *dev)
{
PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -248,8 +273,34 @@ static void piix4_register_types(void)
type_init(piix4_register_types)
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+int slot;
+
+slot = PCI_SLOT(pci_dev->devfn);
+
+switch (slot) {
+/* PIIX4 USB */
+case 10:
+return 3;
+/* AMD 79C973 Ethernet */
+case 11:
+return 1;
+/* Crystal 4281 Sound */
+case 12:
+return 2;
+/* PCI slot 1 to 4 */
+case 18 ... 21:
+return ((slot - 18) + irq_num) & 0x03;
+/* Unknown device, don't do any translation */
+default:
+return irq_num;
+}
+}
+
DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
{
+PIIX4State *s;
PCIDevice *pci;
DeviceState *dev;
int devfn = PCI_DEVFN(10, 0);
@@ -257,6 +308,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
pci = pci_create_simple_multifunction(pci_bus, devfn, true,
TYPE_PIIX4_PCI_DEVICE);
dev = DEVICE(pci);
+s = PIIX4_PCI_DEVICE(pci);
if (isa_bus) {
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
@@ -271,5 +323,11 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus, I2CBus **smbus)
NULL, 0, NULL);
}
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s->i8259, 4);
+
+for (int i = 0; i < ISA_NUM_IRQS; i++) {
+s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
+}
+
return dev;
}
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index c7480bd019..9e23e32eff 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -981,56 +981,6 @@ static const MemoryRegionOps isd_mem_ops = {
},
};
-static int gt64120_pci_map_irq(PCIDevice *pci_dev, int irq_num)
-{
-int slot;
-
-slot = PCI_SLOT(pci_dev->devfn);
-
-switch (slot) {
-/* PIIX4 USB */
-case 10:
-return 3;
-/* AMD 79C973 Ethernet */
-case 11:
-return 1;
-/* Crystal 4281 Sound */
-case 12:
-return 2;
-/* PCI slot 1 to 4 */
-case 18 ... 21:
-return ((slot - 18) + irq_num) & 0x03;
-/* Unknown device, don't do any translation */
-default:
-return irq_num;
-}
-}
-
-static int pci_irq_levels[4];
-
-static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
-{
-int i, pic_irq, pic_level;
-qemu_irq *pic = opaque;
-
-pci_irq_levels[irq_num] = level;
-
-/* now we change the pic irq level according to the piix irq mappings */
-/* XXX: optimize */
-pic_irq = piix4_dev->config[PIIX_PIRQCA + irq_num];
-if (pic_irq < 16) {
-/* The pic level is the logical OR of all the PCI irqs mapped to it. */
-pic_level = 0;
-for (i = 0; i < 4; i++) {
-if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
-pic_level |= pci_irq_levels[i];
-}
-}
-qemu_set_irq(pic[pic_irq], pic_level);
-}
-}
-
-
static void gt64120_reset(DeviceState *dev)
{
GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
@@ -1207,7 +1157,7 @@ static void gt64120_realize(DeviceState *dev, Error
**errp)
"gt64120-isd", 0x1000);
}
-PCIBus *gt64120_register(qemu_irq *pic)
[PATCH v2 0/5] malta: Fix PCI IRQ levels to be preserved during migration
The intention behind v1 [1] was just to remove some global variables from gt64xxx and piix4. During review it was noticed that the Malta board misses to preserve the PCI IRQ levels during migration. Since the patch series offered an easy fix v2 was born. Furthermore, i8259[] was moved to PIIX4State in patch 1. This attribute seems quite redundant to *isa to me. I therefore attempt to resolve it. Tested with [2]: qemu-system-mips64 -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda \ debian_wheezy_mips_standard.qcow2 -append "root=/dev/sda1 console=tty0" It was possible to log in as root and `poweroff` the machine. [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg02786.html [2] https://people.debian.org/~aurel32/qemu/mips/ v2: isa/piix4: Fix PCI IRQ levels to be preserved in VMState isa/piix4: Resolve redundant i8259[] attribute Bernhard Beschow (5): malta: Move PCI interrupt handling from gt64xxx to piix4 pci: Always pass own DeviceState to pci_map_irq_fn's isa/piix4: Resolve global variables isa/piix4: Fix PCI IRQ levels to be preserved in VMState isa/piix4: Resolve redundant i8259[] attribute hw/isa/piix4.c| 61 +++--- hw/mips/gt64xxx_pci.c | 62 +++ hw/mips/malta.c | 6 +--- hw/pci-host/sh_pci.c | 6 ++-- hw/pci-host/versatile.c | 6 ++-- hw/ppc/ppc440_pcix.c | 6 ++-- hw/ppc/ppc4xx_pci.c | 6 ++-- include/hw/mips/mips.h| 2 +- include/hw/southbridge/piix.h | 2 -- 9 files changed, 75 insertions(+), 82 deletions(-) -- 2.35.1
[PATCH v2] hw/net: e1000e: Clear ICR on read when using non MSI-X interrupts
In section 7.4.3 of the 82574 datasheet it states that
"In systems that do not support MSI-X, reading the ICR
register clears it's bits..."
Some OSes rely on this.
Signed-off-by: Nick Hudson
---
hw/net/e1000e_core.c | 5 +
hw/net/trace-events | 1 +
2 files changed, 6 insertions(+)
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 8ae6fb7e14..2c51089a82 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -2607,6 +2607,11 @@ e1000e_mac_icr_read(E1000ECore *core, int index)
core->mac[ICR] = 0;
}
+if (!msix_enabled(core->owner)) {
+trace_e1000e_irq_icr_clear_nonmsix_icr_read();
+core->mac[ICR] = 0;
+}
+
if ((core->mac[ICR] & E1000_ICR_ASSERTED) &&
(core->mac[CTRL_EXT] & E1000_CTRL_EXT_IAME)) {
trace_e1000e_irq_icr_clear_iame();
diff --git a/hw/net/trace-events b/hw/net/trace-events
index 643338f610..4c0ec3fda1 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -221,6 +221,7 @@ e1000e_irq_write_ics(uint32_t val) "Adding ICR bits 0x%x"
e1000e_irq_icr_process_iame(void) "Clearing IMS bits due to IAME"
e1000e_irq_read_ics(uint32_t ics) "Current ICS: 0x%x"
e1000e_irq_read_ims(uint32_t ims) "Current IMS: 0x%x"
+e1000e_irq_icr_clear_nonmsix_icr_read(void) "Clearing ICR on read due to non
MSI-X int"
e1000e_irq_icr_read_entry(uint32_t icr) "Starting ICR read. Current ICR: 0x%x"
e1000e_irq_icr_read_exit(uint32_t icr) "Ending ICR read. Current ICR: 0x%x"
e1000e_irq_icr_clear_zero_ims(void) "Clearing ICR on read due to zero IMS"
--
2.25.1
