Re: [PATCH 0/5] linux-user: clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
Review would be nice; cc'ing a few people who have left their mark in
git.




Re: [PATCH] hw/cxl: Fix local variable shadowing of cap_hdrs

2023-09-29 Thread Markus Armbruster
Jonathan Cameron  writes:

> On Wed, 27 Sep 2023 19:13:35 +
> Fan Ni  wrote:
>
>> On Mon, Sep 25, 2023 at 04:22:58PM +0100, Jonathan Cameron wrote:
>> 
>> > Rename the version not burried in the macro to cap_h.  
>> The change looks good to me. Just one minor thing. why "version" get
>> involved here?
>>
>
> Used in the sense of two copies of something with slightly differences
> given if it were straight code without a macro, we'd have just
> have used the copy being changed here for all of the calls. 
> With hindsight, not the best word to choose given the many other meanings!

Fan, good enough to get your R-by?




Re: [PATCH 0/4] ui/console: multihead: fix crash, simplify logic

2023-09-29 Thread Mark Cave-Ayland

On 26/09/2023 09:00, Marc-André Lureau wrote:


Hi Laszlo

On Mon, Sep 25, 2023 at 7:36 PM Laszlo Ersek  wrote:

Has this been queued by someone? Both Gerd and Marc-André are "odd
fixers", so I'm not sure who should be sending a PR with these patches
(and I don't see a pending PULL at

with these patch subjects included).


I have the series in my "ui" branch. I was waiting for a few more
patches to be accumulated. But if someone else takes this first, I'll
drop them.


Does this series fix the "../ui/console.c:818: dpy_get_ui_info: Assertion 
`dpy_ui_info_supported(con)' failed." assert() on startup when using gtk? It would be 
good to get this fixed in git master soon, as it has been broken for a couple of 
weeks now, and -display sdl has issues tracking the mouse correctly on my laptop here :(



ATB,

Mark.




Re: [PATCH] target/ppc: Rename variables to avoid local variable shadowing in VUPKPX

2023-09-29 Thread Markus Armbruster
Cédric Le Goater  writes:

> On 9/23/23 10:25, Michael Tokarev wrote:
>> 23.09.2023 10:12, Cédric Le Goater:
>> 
>>> --- a/target/ppc/int_helper.c
>>> +++ b/target/ppc/int_helper.c
>>> @@ -2022,11 +2022,11 @@ void helper_vsum4ubs(CPUPPCState *env, ppc_avr_t 
>>> *r, ppc_avr_t *a, ppc_avr_t *b)
>>>   for (i = 0; i < ARRAY_SIZE(r->u32); i++) {  \
>>>   uint16_t e = b->u16[hi ? i : i + 4];    \
>>>   uint8_t a = (e >> 15) ? 0xff : 0;   \
>>> -    uint8_t r = (e >> 10) & 0x1f;   \
>>> +    uint8_t _r = (e >> 10) & 0x1f;  \
>>>   uint8_t g = (e >> 5) & 0x1f;    \
>>> -    uint8_t b = e & 0x1f;   \
>>> +    uint8_t _b = e & 0x1f;  \
>> I'd suggest to rename all of them here to have the same pattern.  Maybe.. :)
>
> or maybe use the field names from the ISA : VRT,VRA,VRB ?

Should I expect a respin?

If not, anyone ready to give an R-by as is?


Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

2023-09-29 Thread Markus Armbruster
Ani Sinha  writes:

>> On 29-Sep-2023, at 11:43 AM, Markus Armbruster  wrote:
>> 
>> Ani Sinha  writes:
>> 
 On 29-Sep-2023, at 11:17 AM, Markus Armbruster  wrote:
 
 Ani Sinha  writes:
 
> Code changes in acpi that addresses all compiler complaints coming from 
> enabling
> -Wshadow flags. Enabling -Wshadow catches cases of local variables 
> shadowing
> other local variables or parameters. These makes the code confusing 
> and/or adds
> bugs that are difficult to catch.
> 
> The code is tested to build with and without the flag turned on.
> 
> CC: Markus Armbruster 
> CC: Philippe Mathieu-Daude 
> CC: m...@redhat.com
> CC: imamm...@redhat.com
> Message-Id: <87r0mqlf9x@pond.sub.org>
 
 This is my "Help wanted for enabling -Wshadow=local" post.
>>> 
>>> Yes indeed. I wanted to refer to that thread for context in the commit log.
>> 
>> I appreciate your diligence.  We just don't have an established tag
>> convention for "see also" references to e-mail.  I could append
>> 
>>See also
>> 
>>Subject: Help wanted for enabling -Wshadow=local
>>Message-Id: <87r0mqlf9x@pond.sub.org>
>>https://lore.kernel.org/qemu-devel/87r0mqlf9x@pond.sub.org
>> 
>> to your first paragraph.  Want me to?
>
> Sure, if that is ok.

Done!




Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

2023-09-29 Thread Ani Sinha



> On 29-Sep-2023, at 1:32 PM, Markus Armbruster  wrote:
> 
> Ani Sinha  writes:
> 
>>> On 29-Sep-2023, at 11:43 AM, Markus Armbruster  wrote:
>>> 
>>> Ani Sinha  writes:
>>> 
> On 29-Sep-2023, at 11:17 AM, Markus Armbruster  wrote:
> 
> Ani Sinha  writes:
> 
>> Code changes in acpi that addresses all compiler complaints coming from 
>> enabling
>> -Wshadow flags. Enabling -Wshadow catches cases of local variables 
>> shadowing
>> other local variables or parameters. These makes the code confusing 
>> and/or adds
>> bugs that are difficult to catch.
>> 
>> The code is tested to build with and without the flag turned on.
>> 
>> CC: Markus Armbruster 
>> CC: Philippe Mathieu-Daude 
>> CC: m...@redhat.com
>> CC: imamm...@redhat.com
>> Message-Id: <87r0mqlf9x@pond.sub.org>
> 
> This is my "Help wanted for enabling -Wshadow=local" post.
 
 Yes indeed. I wanted to refer to that thread for context in the commit log.
>>> 
>>> I appreciate your diligence.  We just don't have an established tag
>>> convention for "see also" references to e-mail.  I could append
>>> 
>>>   See also
>>> 
>>>   Subject: Help wanted for enabling -Wshadow=local
>>>   Message-Id: <87r0mqlf9x@pond.sub.org>
>>>   https://lore.kernel.org/qemu-devel/87r0mqlf9x@pond.sub.org
>>> 
>>> to your first paragraph.  Want me to?
>> 
>> Sure, if that is ok.
> 
> Done!

Your shadow-next has no changes. Have you not pushed to that branch?



Re: [PATCH] target/ppc: Rename variables to avoid local variable shadowing in VUPKPX

2023-09-29 Thread Cédric Le Goater

On 9/29/23 10:00, Markus Armbruster wrote:

Cédric Le Goater  writes:


On 9/23/23 10:25, Michael Tokarev wrote:

23.09.2023 10:12, Cédric Le Goater:


--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -2022,11 +2022,11 @@ void helper_vsum4ubs(CPUPPCState *env, ppc_avr_t *r, 
ppc_avr_t *a, ppc_avr_t *b)
   for (i = 0; i < ARRAY_SIZE(r->u32); i++) {  \
   uint16_t e = b->u16[hi ? i : i + 4];    \
   uint8_t a = (e >> 15) ? 0xff : 0;   \
-    uint8_t r = (e >> 10) & 0x1f;   \
+    uint8_t _r = (e >> 10) & 0x1f;  \
   uint8_t g = (e >> 5) & 0x1f;    \
-    uint8_t b = e & 0x1f;   \
+    uint8_t _b = e & 0x1f;  \

I'd suggest to rename all of them here to have the same pattern.  Maybe.. :)


or maybe use the field names from the ISA : VRT,VRA,VRB ?


Should I expect a respin?

If not, anyone ready to give an R-by as is?


This one I can respin. I agree with Michael that some consistency is
preferable. Preparing a v2 full of '_'.

Thanks,

C.





[PATCH v2] target/ppc: Rename variables to avoid local variable shadowing in VUPKPX

2023-09-29 Thread Cédric Le Goater
and fix such warnings :

  ../target/ppc/int_helper.c: In function ‘helper_vupklpx’:
  ../target/ppc/int_helper.c:2025:21: warning: declaration of ‘r’ shadows a 
parameter [-Wshadow=local]
   2025 | uint8_t r = (e >> 10) & 0x1f; 
  \
| ^
  ../target/ppc/int_helper.c:2033:1: note: in expansion of macro ‘VUPKPX’
   2033 | VUPKPX(lpx, UPKLO)
| ^~
  ../target/ppc/int_helper.c:2017:41: note: shadowed declaration is here
   2017 | void helper_vupk##suffix(ppc_avr_t *r, ppc_avr_t *b)  
  \
|  ~~~^
  ../target/ppc/int_helper.c:2033:1: note: in expansion of macro ‘VUPKPX’
   2033 | VUPKPX(lpx, UPKLO)
| ^~

Signed-off-by: Cédric Le Goater 
---

 v2: changed all locals to start with '_' (Michael)

 target/ppc/int_helper.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 6fd00684a5b9..0a5c3e78a413 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -2020,13 +2020,13 @@ void helper_vsum4ubs(CPUPPCState *env, ppc_avr_t *r, 
ppc_avr_t *a, ppc_avr_t *b)
 ppc_avr_t result;   \
 \
 for (i = 0; i < ARRAY_SIZE(r->u32); i++) {  \
-uint16_t e = b->u16[hi ? i : i + 4];\
-uint8_t a = (e >> 15) ? 0xff : 0;   \
-uint8_t r = (e >> 10) & 0x1f;   \
-uint8_t g = (e >> 5) & 0x1f;\
-uint8_t b = e & 0x1f;   \
+uint16_t _e = b->u16[hi ? i : i + 4];   \
+uint8_t _a = (_e >> 15) ? 0xff : 0; \
+uint8_t _r = (_e >> 10) & 0x1f; \
+uint8_t _g = (_e >> 5) & 0x1f;  \
+uint8_t _b = _e & 0x1f; \
 \
-result.u32[i] = (a << 24) | (r << 16) | (g << 8) | b;   \
+result.u32[i] = (_a << 24) | (_r << 16) | (_g << 8) | _b;   \
 }   \
 *r = result;\
 }
-- 
2.41.0




Re: [PATCH] hw/acpi: changes towards enabling -Wshadow=local

2023-09-29 Thread Markus Armbruster
Ani Sinha  writes:

> Your shadow-next has no changes. Have you not pushed to that branch?

I did, but only after I was done updating patches.  Intend to post my
pull request shortly.




[PULL 03/56] ui: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster 
Reviewed-by: Peter Maydell 
Message-ID: <20230921121312.1301864-4-arm...@redhat.com>
---
 ui/gtk.c  | 14 +++---
 ui/spice-display.c|  9 +
 ui/vnc-palette.c  |  2 --
 ui/vnc.c  | 12 ++--
 ui/vnc-enc-zrle.c.inc |  9 -
 5 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index e09f97a86b..3373427c9b 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -930,8 +930,8 @@ static gboolean gd_motion_event(GtkWidget *widget, 
GdkEventMotion *motion,
 GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
 GdkRectangle geometry;
 
-int x = (int)motion->x_root;
-int y = (int)motion->y_root;
+int xr = (int)motion->x_root;
+int yr = (int)motion->y_root;
 
 gdk_monitor_get_geometry(monitor, &geometry);
 
@@ -942,13 +942,13 @@ static gboolean gd_motion_event(GtkWidget *widget, 
GdkEventMotion *motion,
  * may still be only half way across the screen. Without
  * this warp, the server pointer would thus appear to hit
  * an invisible wall */
-if (x <= geometry.x || x - geometry.x >= geometry.width - 1 ||
-y <= geometry.y || y - geometry.y >= geometry.height - 1) {
+if (xr <= geometry.x || xr - geometry.x >= geometry.width - 1 ||
+yr <= geometry.y || yr - geometry.y >= geometry.height - 1) {
 GdkDevice *dev = gdk_event_get_device((GdkEvent *)motion);
-x = geometry.x + geometry.width / 2;
-y = geometry.y + geometry.height / 2;
+xr = geometry.x + geometry.width / 2;
+yr = geometry.y + geometry.height / 2;
 
-gdk_device_warp(dev, screen, x, y);
+gdk_device_warp(dev, screen, xr, yr);
 s->last_set = FALSE;
 return FALSE;
 }
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 5cc47bd668..6eb98a5a5c 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -1081,15 +1081,16 @@ static void qemu_spice_gl_update(DisplayChangeListener 
*dcl,
 }
 
 if (render_cursor) {
-int x, y;
+int ptr_x, ptr_y;
+
 qemu_mutex_lock(&ssd->lock);
-x = ssd->ptr_x;
-y = ssd->ptr_y;
+ptr_x = ssd->ptr_x;
+ptr_y = ssd->ptr_y;
 qemu_mutex_unlock(&ssd->lock);
 egl_texture_blit(ssd->gls, &ssd->blit_fb, &ssd->guest_fb,
  !y_0_top);
 egl_texture_blend(ssd->gls, &ssd->blit_fb, &ssd->cursor_fb,
-  !y_0_top, x, y, 1.0, 1.0);
+  !y_0_top, ptr_x, ptr_y, 1.0, 1.0);
 glFlush();
 }
 
diff --git a/ui/vnc-palette.c b/ui/vnc-palette.c
index dc7c0ba997..4e88c412f0 100644
--- a/ui/vnc-palette.c
+++ b/ui/vnc-palette.c
@@ -86,8 +86,6 @@ int palette_put(VncPalette *palette, uint32_t color)
 return 0;
 }
 if (!entry) {
-VncPaletteEntry *entry;
-
 entry = &palette->pool[palette->size];
 entry->color = color;
 entry->idx = idx;
diff --git a/ui/vnc.c b/ui/vnc.c
index c302bb07a5..523eb1f5b0 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1584,15 +1584,15 @@ static void vnc_jobs_bh(void *opaque)
  */
 static int vnc_client_read(VncState *vs)
 {
-size_t ret;
+size_t sz;
 
 #ifdef CONFIG_VNC_SASL
 if (vs->sasl.conn && vs->sasl.runSSF)
-ret = vnc_client_read_sasl(vs);
+sz = vnc_client_read_sasl(vs);
 else
 #endif /* CONFIG_VNC_SASL */
-ret = vnc_client_read_plain(vs);
-if (!ret) {
+sz = vnc_client_read_plain(vs);
+if (!sz) {
 if (vs->disconnecting) {
 vnc_disconnect_finish(vs);
 return -1;
@@ -3118,8 +3118,8 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
 cmp_bytes = MIN(VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES,
 server_stride);
 if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
-int width = pixman_image_get_width(vd->server);
-tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width);
+int w = pixman_image_get_width(vd->server);
+tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, w);
 } else {
 int guest_bpp =
 PIXMAN_FORMAT_BPP(pixman_image_get_format(vd->guest.fb));
diff --git a/ui/vnc-enc-zrle.c.inc b/ui/vnc-enc-zrle.c.inc
index a8ca37d05e..2ef7501d52 100644
--- a/ui/vnc-enc-zrle.c.inc
+++ b/ui/vnc-enc-zrle.c.inc
@@ -153,11 +153,12 @@ static void ZRLE_ENCODE_TILE(VncState *vs, ZRLE_PIXEL 
*data, int w, int h,
 }
 
 if (use_rle) {
-ZRLE_PIXEL *ptr = data;
-ZRLE_PIXEL *end = ptr + w * h;
 ZRLE_PIXEL *run_start;
 ZRLE_PIXEL pix;
 
+pt

[PULL 49/56] seccomp: avoid shadowing of 'action' variable

2023-09-29 Thread Markus Armbruster
From: Daniel P. Berrangé 

This is confusing as one 'action' variable is used for storing
a SCMP_ enum value, while the other 'action' variable is used
for storing a SECCOMP_ enum value.

Signed-off-by: Daniel P. Berrangé 
Message-ID: <20230922160644.438631-3-berra...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 softmmu/qemu-seccomp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/softmmu/qemu-seccomp.c b/softmmu/qemu-seccomp.c
index d66a2a1226..4d7439e7f7 100644
--- a/softmmu/qemu-seccomp.c
+++ b/softmmu/qemu-seccomp.c
@@ -283,9 +283,9 @@ static uint32_t qemu_seccomp_update_action(uint32_t action)
 if (action == SCMP_ACT_TRAP) {
 static int kill_process = -1;
 if (kill_process == -1) {
-uint32_t action = SECCOMP_RET_KILL_PROCESS;
+uint32_t testaction = SECCOMP_RET_KILL_PROCESS;
 
-if (qemu_seccomp(SECCOMP_GET_ACTION_AVAIL, 0, &action) == 0) {
+if (qemu_seccomp(SECCOMP_GET_ACTION_AVAIL, 0, &testaction) == 0) {
 kill_process = 1;
 } else {
 kill_process = 0;
-- 
2.41.0




[PULL 34/56] spapr/drc: Clean up local variable shadowing in rtas_ibm_configure_connector()

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Remove extra 'drc_index' variable to avoid this warning :

  ../hw/ppc/spapr_drc.c: In function ‘rtas_ibm_configure_connector’:
  ../hw/ppc/spapr_drc.c:1240:26: warning: declaration of ‘drc_index’ shadows a 
previous local [-Wshadow=compatible-local]
   1240 | uint32_t drc_index = spapr_drc_index(drc);
|  ^
  ../hw/ppc/spapr_drc.c:1155:14: note: shadowed declaration is here
   1155 | uint32_t drc_index;
|  ^

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-7-...@kaod.org>
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/spapr_drc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index b5c400a94d..843e318312 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -1237,8 +1237,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 case FDT_END_NODE:
 drc->ccs_depth--;
 if (drc->ccs_depth == 0) {
-uint32_t drc_index = spapr_drc_index(drc);
-
 /* done sending the device tree, move to configured state */
 trace_spapr_drc_set_configured(drc_index);
 drc->state = drck->ready_state;
-- 
2.41.0




[PULL 32/56] spapr: Clean up local variable shadowing in spapr_init_cpus()

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Remove extra 'i' variable to fix this warning :

  ../hw/ppc/spapr.c: In function ‘spapr_init_cpus’:
  ../hw/ppc/spapr.c:2668:13: warning: declaration of ‘i’ shadows a previous 
local [-Wshadow=compatible-local]
   2668 | int i;
| ^
  ../hw/ppc/spapr.c:2645:9: note: shadowed declaration is here
   2645 | int i;
| ^

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-5-...@kaod.org>
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/spapr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 612dbdf356..4fe82e490a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2665,8 +2665,6 @@ static void spapr_init_cpus(SpaprMachineState *spapr)
 }
 
 if (smc->pre_2_10_has_unused_icps) {
-int i;
-
 for (i = 0; i < spapr_max_server_number(spapr); i++) {
 /* Dummy entries get deregistered when real ICPState objects
  * are registered during CPU core hotplug.
-- 
2.41.0




[PULL 28/56] hw/intc/openpic: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/intc/openpic.c: In function ‘openpic_gbl_write’:
  hw/intc/openpic.c:614:17: warning: declaration of ‘idx’ shadows a previous 
local [-Wshadow=compatible-local]
614 | int idx;
| ^~~
  hw/intc/openpic.c:568:9: note: shadowed declaration is here
568 | int idx;
| ^~~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904162824.85385-3-phi...@linaro.org>
Reviewed-by: Peter Maydell 
Signed-off-by: Markus Armbruster 
---
 hw/intc/openpic.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index c757adbe53..a6f91d4bcd 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -610,11 +610,8 @@ static void openpic_gbl_write(void *opaque, hwaddr addr, 
uint64_t val,
 case 0x10B0:
 case 0x10C0:
 case 0x10D0:
-{
-int idx;
-idx = (addr - 0x10A0) >> 4;
-write_IRQreg_ivpr(opp, opp->irq_ipi0 + idx, val);
-}
+idx = (addr - 0x10A0) >> 4;
+write_IRQreg_ivpr(opp, opp->irq_ipi0 + idx, val);
 break;
 case 0x10E0: /* SPVE */
 opp->spve = val & opp->vector_mask;
-- 
2.41.0




[PATCH v3 05/14] audio: commonize voice initialization

2023-09-29 Thread Paolo Bonzini
Move some mostly irrelevant code out of audio_init.

Signed-off-by: Paolo Bonzini 
---
 audio/audio.c  | 19 ++-
 audio/audio_template.h |  9 -
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index d4263976a5b..2014a2904e8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1570,8 +1570,8 @@ static int audio_driver_init(AudioState *s, struct 
audio_driver *drv,
 drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
 }
 
-audio_init_nb_voices_out(s, drv);
-audio_init_nb_voices_in(s, drv);
+audio_init_nb_voices_out(s, drv, 1);
+audio_init_nb_voices_in(s, drv, 0);
 s->drv = drv;
 return 0;
 } else {
@@ -1774,21 +1774,6 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 
 s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
 
-s->nb_hw_voices_out = audio_get_pdo_out(dev)->voices;
-s->nb_hw_voices_in = audio_get_pdo_in(dev)->voices;
-
-if (s->nb_hw_voices_out < 1) {
-dolog ("Bogus number of playback voices %d, setting to 1\n",
-   s->nb_hw_voices_out);
-s->nb_hw_voices_out = 1;
-}
-
-if (s->nb_hw_voices_in < 0) {
-dolog ("Bogus number of capture voices %d, setting to 0\n",
-   s->nb_hw_voices_in);
-s->nb_hw_voices_in = 0;
-}
-
 if (drvname) {
 driver = audio_driver_lookup(drvname);
 if (driver) {
diff --git a/audio/audio_template.h b/audio/audio_template.h
index dc0c74aa746..7ccfec01168 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -37,11 +37,12 @@
 #endif
 
 static void glue(audio_init_nb_voices_, TYPE)(AudioState *s,
-  struct audio_driver *drv)
+  struct audio_driver *drv, int 
min_voices)
 {
 int max_voices = glue (drv->max_voices_, TYPE);
 size_t voice_size = glue(drv->voice_size_, TYPE);
 
+glue (s->nb_hw_voices_, TYPE) = glue(audio_get_pdo_, TYPE)(s->dev)->voices;
 if (glue (s->nb_hw_voices_, TYPE) > max_voices) {
 if (!max_voices) {
 #ifdef DAC
@@ -56,6 +57,12 @@ static void glue(audio_init_nb_voices_, TYPE)(AudioState *s,
 glue (s->nb_hw_voices_, TYPE) = max_voices;
 }
 
+if (glue (s->nb_hw_voices_, TYPE) < min_voices) {
+dolog ("Bogus number of " NAME " voices %d, setting to %d\n",
+   glue (s->nb_hw_voices_, TYPE),
+   min_voices);
+}
+
 if (audio_bug(__func__, !voice_size && max_voices)) {
 dolog ("drv=`%s' voice_size=0 max_voices=%d\n",
drv->name, max_voices);
-- 
2.41.0




[PATCH v3 03/14] audio: allow returning an error from the driver init

2023-09-29 Thread Paolo Bonzini
An error is already printed by audio_driver_init, but we can make
it more precise if the driver can return an Error *.

Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Paolo Bonzini 
---
 audio/alsaaudio.c   |  2 +-
 audio/audio.c   | 13 ++---
 audio/audio_int.h   |  2 +-
 audio/coreaudio.m   |  2 +-
 audio/dbusaudio.c   |  2 +-
 audio/dsoundaudio.c |  2 +-
 audio/jackaudio.c   |  2 +-
 audio/noaudio.c |  2 +-
 audio/ossaudio.c| 11 ---
 audio/paaudio.c |  7 +--
 audio/pwaudio.c | 16 +---
 audio/sdlaudio.c|  5 +++--
 audio/sndioaudio.c  |  2 +-
 audio/spiceaudio.c  |  5 -
 audio/wavaudio.c|  2 +-
 15 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 057571dd1e0..6fb78e5b972 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -904,7 +904,7 @@ static void 
alsa_init_per_direction(AudiodevAlsaPerDirectionOptions *apdo)
 }
 }
 
-static void *alsa_audio_init(Audiodev *dev)
+static void *alsa_audio_init(Audiodev *dev, Error **errp)
 {
 AudiodevAlsaOptions *aopts;
 assert(dev->driver == AUDIODEV_DRIVER_ALSA);
diff --git a/audio/audio.c b/audio/audio.c
index 4332c4c6ce8..3d664503521 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -33,6 +33,7 @@
 #include "qapi/qapi-visit-audio.h"
 #include "qapi/qapi-commands-audio.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/help_option.h"
@@ -1555,7 +1556,9 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, 
size_t size)
 static int audio_driver_init(AudioState *s, struct audio_driver *drv,
  bool msg, Audiodev *dev)
 {
-s->drv_opaque = drv->init(dev);
+Error *local_err = NULL;
+
+s->drv_opaque = drv->init(dev, &local_err);
 
 if (s->drv_opaque) {
 if (!drv->pcm_ops->get_buffer_in) {
@@ -1572,8 +1575,12 @@ static int audio_driver_init(AudioState *s, struct 
audio_driver *drv,
 s->drv = drv;
 return 0;
 } else {
-if (msg) {
-dolog("Could not init `%s' audio driver\n", drv->name);
+if (!msg) {
+error_free(local_err);
+} else if (local_err) {
+error_report_err(local_err);
+} else {
+error_report("Could not init `%s' audio driver", drv->name);
 }
 return -1;
 }
diff --git a/audio/audio_int.h b/audio/audio_int.h
index e57ff50155a..06e815de9f6 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -140,7 +140,7 @@ typedef struct audio_driver audio_driver;
 struct audio_driver {
 const char *name;
 const char *descr;
-void *(*init) (Audiodev *);
+void *(*init) (Audiodev *, Error **);
 void (*fini) (void *);
 #ifdef CONFIG_GIO
 void (*set_dbus_server) (AudioState *s, GDBusObjectManagerServer *manager, 
bool p2p);
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 4695291621a..7cfb38fb6ae 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -644,7 +644,7 @@ static void coreaudio_enable_out(HWVoiceOut *hw, bool 
enable)
 update_device_playback_state(core);
 }
 
-static void *coreaudio_audio_init(Audiodev *dev)
+static void *coreaudio_audio_init(Audiodev *dev, Error **errp)
 {
 return dev;
 }
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index 7a11fbfb420..310ca997ff4 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -395,7 +395,7 @@ dbus_enable_in(HWVoiceIn *hw, bool enable)
 }
 
 static void *
-dbus_audio_init(Audiodev *dev)
+dbus_audio_init(Audiodev *dev, Error **errp)
 {
 DBusAudio *da = g_new0(DBusAudio, 1);
 
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 3fb67ec3eed..eefde88edcb 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -619,7 +619,7 @@ static void dsound_audio_fini (void *opaque)
 g_free(s);
 }
 
-static void *dsound_audio_init(Audiodev *dev)
+static void *dsound_audio_init(Audiodev *dev, Error **errp)
 {
 int err;
 HRESULT hr;
diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index e1eaa3477dc..823e7d96bae 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -645,7 +645,7 @@ static int qjack_thread_creator(jack_native_thread_t 
*thread,
 }
 #endif
 
-static void *qjack_init(Audiodev *dev)
+static void *qjack_init(Audiodev *dev, Error **errp)
 {
 assert(dev->driver == AUDIODEV_DRIVER_JACK);
 return dev;
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 4fdee5adecf..a36bfeffd14 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -104,7 +104,7 @@ static void no_enable_in(HWVoiceIn *hw, bool enable)
 }
 }
 
-static void *no_audio_init(Audiodev *dev)
+static void *no_audio_init(Audiodev *dev, Error **errp)
 {
 return &no_audio_init;
 }
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index e8d732b612c..ec4448d573d 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -28,6 +28,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/host-util

[PULL 08/56] tcg: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  tcg/tcg.c:2551:27: error: declaration shadows a local variable 
[-Werror,-Wshadow]
MemOp op = get_memop(oi);
  ^
  tcg/tcg.c:2437:12: note: previous declaration is here
TCGOp *op;
   ^
  accel/tcg/tb-maint.c:245:18: error: declaration shadows a local variable 
[-Werror,-Wshadow]
for (int i = 0; i < V_L2_SIZE; i++) {
 ^
  accel/tcg/tb-maint.c:210:9: note: previous declaration is here
int i;
^

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-2-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 accel/tcg/tb-maint.c |  3 +--
 tcg/tcg.c| 16 
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 32ae8af61c..8c71cebabd 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -207,13 +207,12 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, 
bool alloc)
 {
 PageDesc *pd;
 void **lp;
-int i;
 
 /* Level 1.  Always allocated.  */
 lp = l1_map + ((index >> v_l1_shift) & (v_l1_size - 1));
 
 /* Level 2..N-1.  */
-for (i = v_l2_levels; i > 0; i--) {
+for (int i = v_l2_levels; i > 0; i--) {
 void **p = qatomic_rcu_read(lp);
 
 if (p == NULL) {
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 604fa9bf3e..ea94d0fbff 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2549,21 +2549,21 @@ static void tcg_dump_ops(TCGContext *s, FILE *f, bool 
have_prefs)
 {
 const char *s_al, *s_op, *s_at;
 MemOpIdx oi = op->args[k++];
-MemOp op = get_memop(oi);
+MemOp mop = get_memop(oi);
 unsigned ix = get_mmuidx(oi);
 
-s_al = alignment_name[(op & MO_AMASK) >> MO_ASHIFT];
-s_op = ldst_name[op & (MO_BSWAP | MO_SSIZE)];
-s_at = atom_name[(op & MO_ATOM_MASK) >> MO_ATOM_SHIFT];
-op &= ~(MO_AMASK | MO_BSWAP | MO_SSIZE | MO_ATOM_MASK);
+s_al = alignment_name[(mop & MO_AMASK) >> MO_ASHIFT];
+s_op = ldst_name[mop & (MO_BSWAP | MO_SSIZE)];
+s_at = atom_name[(mop & MO_ATOM_MASK) >> MO_ATOM_SHIFT];
+mop &= ~(MO_AMASK | MO_BSWAP | MO_SSIZE | MO_ATOM_MASK);
 
 /* If all fields are accounted for, print symbolically. */
-if (!op && s_al && s_op && s_at) {
+if (!mop && s_al && s_op && s_at) {
 col += ne_fprintf(f, ",%s%s%s,%u",
   s_at, s_al, s_op, ix);
 } else {
-op = get_memop(oi);
-col += ne_fprintf(f, ",$0x%x,%u", op, ix);
+mop = get_memop(oi);
+col += ne_fprintf(f, ",$0x%x,%u", mop, ix);
 }
 i = 1;
 }
-- 
2.41.0




[PULL 43/56] aspeed/i2c: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Remove superfluous local 'data' variable and use the one define at the
top of the routine. This fixes :

  ../hw/i2c/aspeed_i2c.c: In function ‘aspeed_i2c_bus_recv’:
  ../hw/i2c/aspeed_i2c.c:315:17: warning: declaration of ‘data’ shadows a 
previous local [-Wshadow=compatible-local]
315 | uint8_t data;
| ^~~~
  ../hw/i2c/aspeed_i2c.c:288:13: note: shadowed declaration is here
288 | uint8_t data;
| ^~~~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230922155924.1172019-2-...@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Joel Stanley 
Signed-off-by: Markus Armbruster 
---
 hw/i2c/aspeed_i2c.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 7275d40749..1037c22b2f 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -312,7 +312,6 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
 SHARED_ARRAY_FIELD_DP32(bus->regs, reg_pool_ctrl, RX_COUNT, i & 0xff);
 SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, RX_BUFF_EN, 0);
 } else if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, RX_DMA_EN)) {
-uint8_t data;
 /* In new mode, clear how many bytes we RXed */
 if (aspeed_i2c_is_new_mode(bus->controller)) {
 ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN_STS, RX_LEN, 0);
-- 
2.41.0




[PATCH v3 04/14] audio: return Error ** from audio_state_by_name

2023-09-29 Thread Paolo Bonzini
Remove duplicate error formatting code.

Signed-off-by: Paolo Bonzini 
---
 audio/audio-hmp-cmds.c   |  6 --
 audio/audio.c|  3 ++-
 audio/audio.h|  2 +-
 hw/core/qdev-properties-system.c | 16 
 ui/dbus.c|  3 +--
 ui/vnc.c |  3 +--
 6 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/audio/audio-hmp-cmds.c b/audio/audio-hmp-cmds.c
index 1237ce9e750..c9608b715b8 100644
--- a/audio/audio-hmp-cmds.c
+++ b/audio/audio-hmp-cmds.c
@@ -26,6 +26,7 @@
 #include "audio/audio.h"
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
+#include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
 
 static QLIST_HEAD (capture_list_head, CaptureState) capture_head;
@@ -65,10 +66,11 @@ void hmp_wavcapture(Monitor *mon, const QDict *qdict)
 int nchannels = qdict_get_try_int(qdict, "nchannels", 2);
 const char *audiodev = qdict_get_str(qdict, "audiodev");
 CaptureState *s;
-AudioState *as = audio_state_by_name(audiodev);
+Error *local_err = NULL;
+AudioState *as = audio_state_by_name(audiodev, &local_err);
 
 if (!as) {
-monitor_printf(mon, "Audiodev '%s' not found\n", audiodev);
+error_report_err(local_err);
 return;
 }
 
diff --git a/audio/audio.c b/audio/audio.c
index 3d664503521..d4263976a5b 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2261,7 +2261,7 @@ int audio_buffer_bytes(AudiodevPerDirectionOptions *pdo,
 audioformat_bytes_per_sample(as->fmt);
 }
 
-AudioState *audio_state_by_name(const char *name)
+AudioState *audio_state_by_name(const char *name, Error **errp)
 {
 AudioState *s;
 QTAILQ_FOREACH(s, &audio_states, list) {
@@ -2270,6 +2270,7 @@ AudioState *audio_state_by_name(const char *name)
 return s;
 }
 }
+error_setg(errp, "audiodev '%s' not found", name);
 return NULL;
 }
 
diff --git a/audio/audio.h b/audio/audio.h
index 01bdc567fb1..e0c13b5dcdf 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -174,7 +174,7 @@ bool audio_init_audiodevs(void);
 void audio_help(void);
 void audio_legacy_help(void);
 
-AudioState *audio_state_by_name(const char *name);
+AudioState *audio_state_by_name(const char *name, Error **errp);
 const char *audio_get_id(QEMUSoundCard *card);
 
 #define DEFINE_AUDIO_PROPERTIES(_s, _f) \
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 41b7e682c78..688340610ec 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -480,24 +480,16 @@ static void set_audiodev(Object *obj, Visitor *v, const 
char* name,
 Property *prop = opaque;
 QEMUSoundCard *card = object_field_prop_ptr(obj, prop);
 AudioState *state;
-int err = 0;
-char *str;
+g_autofree char *str = NULL;
 
 if (!visit_type_str(v, name, &str, errp)) {
 return;
 }
 
-state = audio_state_by_name(str);
-
-if (!state) {
-err = -ENOENT;
-goto out;
+state = audio_state_by_name(str, errp);
+if (state) {
+card->state = state;
 }
-card->state = state;
-
-out:
-error_set_from_qdev_prop_error(errp, err, obj, name, str);
-g_free(str);
 }
 
 const PropertyInfo qdev_prop_audiodev = {
diff --git a/ui/dbus.c b/ui/dbus.c
index 32f1bbe81ae..866467ad2e3 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -220,9 +220,8 @@ dbus_display_complete(UserCreatable *uc, Error **errp)
 }
 
 if (dd->audiodev && *dd->audiodev) {
-AudioState *audio_state = audio_state_by_name(dd->audiodev);
+AudioState *audio_state = audio_state_by_name(dd->audiodev, errp);
 if (!audio_state) {
-error_setg(errp, "Audiodev '%s' not found", dd->audiodev);
 return;
 }
 if (!g_str_equal(audio_state->drv->name, "dbus")) {
diff --git a/ui/vnc.c b/ui/vnc.c
index acb56461b2d..82929469130 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4181,9 +4181,8 @@ void vnc_display_open(const char *id, Error **errp)
 
 audiodev = qemu_opt_get(opts, "audiodev");
 if (audiodev) {
-vd->audio_state = audio_state_by_name(audiodev);
+vd->audio_state = audio_state_by_name(audiodev, errp);
 if (!vd->audio_state) {
-error_setg(errp, "Audiodev '%s' not found", audiodev);
 goto fail;
 }
 }
-- 
2.41.0




[PULL 31/56] spapr: Clean up local variable shadowing in spapr_dt_cpus()

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Introduce a helper routine defining one CPU device node to fix this
warning :

  ../hw/ppc/spapr.c: In function ‘spapr_dt_cpus’:
  ../hw/ppc/spapr.c:812:19: warning: declaration of ‘cs’ shadows a previous 
local [-Wshadow=compatible-local]
812 | CPUState *cs = rev[i];
|   ^~
  ../hw/ppc/spapr.c:786:15: note: shadowed declaration is here
786 | CPUState *cs;
|   ^~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-4-...@kaod.org>
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/spapr.c | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1f1aa2a6d4..612dbdf356 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -780,6 +780,26 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int 
offset,
   pcc->lrg_decr_bits)));
 }
 
+static void spapr_dt_one_cpu(void *fdt, SpaprMachineState *spapr, CPUState *cs,
+ int cpus_offset)
+{
+PowerPCCPU *cpu = POWERPC_CPU(cs);
+int index = spapr_get_vcpu_id(cpu);
+DeviceClass *dc = DEVICE_GET_CLASS(cs);
+g_autofree char *nodename = NULL;
+int offset;
+
+if (!spapr_is_thread0_in_vcore(spapr, cpu)) {
+return;
+}
+
+nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
+offset = fdt_add_subnode(fdt, cpus_offset, nodename);
+_FDT(offset);
+spapr_dt_cpu(cs, fdt, offset, spapr);
+}
+
+
 static void spapr_dt_cpus(void *fdt, SpaprMachineState *spapr)
 {
 CPUState **rev;
@@ -809,21 +829,7 @@ static void spapr_dt_cpus(void *fdt, SpaprMachineState 
*spapr)
 }
 
 for (i = n_cpus - 1; i >= 0; i--) {
-CPUState *cs = rev[i];
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-int index = spapr_get_vcpu_id(cpu);
-DeviceClass *dc = DEVICE_GET_CLASS(cs);
-g_autofree char *nodename = NULL;
-int offset;
-
-if (!spapr_is_thread0_in_vcore(spapr, cpu)) {
-continue;
-}
-
-nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
-offset = fdt_add_subnode(fdt, cpus_offset, nodename);
-_FDT(offset);
-spapr_dt_cpu(cs, fdt, offset, spapr);
+spapr_dt_one_cpu(fdt, spapr, rev[i], cpus_offset);
 }
 
 g_free(rev);
-- 
2.41.0




[PATCH v3 13/14] audio: forbid default audiodev backend with -nodefaults

2023-09-29 Thread Paolo Bonzini
Now that all callers support setting an audiodev, forbid using the default
audiodev if -nodefaults is provided on the command line.

Signed-off-by: Paolo Bonzini 
---
 audio/audio.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index f0788345bf8..7cfcbfb6ef1 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1806,10 +1806,12 @@ static AudioState *audio_init(Audiodev *dev)
 bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
 {
 if (!card->state) {
-if (!QSIMPLEQ_EMPTY(&audiodevs)) {
+if (!QSIMPLEQ_EMPTY(&audiodevs) || !defaults_enabled()) {
 error_setg(errp, "No audiodev specified for %s", name);
-error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?",
-  QSIMPLEQ_FIRST(&audiodevs)->dev->id);
+if (!QSIMPLEQ_EMPTY(&audiodevs)) {
+error_append_hint(errp, "Perhaps you wanted to set 
audiodev=%s?",
+  QSIMPLEQ_FIRST(&audiodevs)->dev->id);
+}
 return false;
 }
 
-- 
2.41.0




[PULL 15/56] hw/arm/virt: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/arm/virt.c:821:22: error: declaration shadows a local variable 
[-Werror,-Wshadow]
qemu_irq irq = qdev_get_gpio_in(vms->gic,
 ^
  hw/arm/virt.c:803:13: note: previous declaration is here
int irq;
^

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 
Message-ID: <20230904161235.84651-9-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 hw/arm/virt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8ad78b23c2..15e74249f9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -801,7 +801,6 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
 for (i = 0; i < smp_cpus; i++) {
 DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
 int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
-int irq;
 /* Mapping from the output timer irq lines from the CPU to the
  * GIC PPI inputs we use for the virt board.
  */
@@ -812,7 +811,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion 
*mem)
 [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
 };
 
-for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+for (unsigned irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
 qdev_connect_gpio_out(cpudev, irq,
   qdev_get_gpio_in(vms->gic,
ppibase + timer_irq[irq]));
-- 
2.41.0




[PATCH v3 08/14] Introduce machine property "audiodev"

2023-09-29 Thread Paolo Bonzini
From: Martin Kletzander 

Many machine types have default audio devices with no way to set the underlying
audiodev.  Instead of adding an option for each and every one of them, this new
property can be used as a default during machine initialisation when creating
such devices.

Signed-off-by: Martin Kletzander 
[Make the property optional, instead of including it in all machines. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 hw/core/machine.c   | 33 +
 include/hw/boards.h |  9 +
 2 files changed, 42 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cb38b8cf4cb..6aa49c8d4f1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-net.h"
+#include "audio/audio.h"
 
 GlobalProperty hw_compat_8_1[] = {};
 const size_t hw_compat_8_1_len = G_N_ELEMENTS(hw_compat_8_1);
@@ -686,6 +687,26 @@ bool device_type_is_dynamic_sysbus(MachineClass *mc, const 
char *type)
 return allowed;
 }
 
+static char *machine_get_audiodev(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return g_strdup(ms->audiodev);
+}
+
+static void machine_set_audiodev(Object *obj, const char *value,
+ Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+if (!audio_state_by_name(value, errp)) {
+return;
+}
+
+g_free(ms->audiodev);
+ms->audiodev = g_strdup(value);
+}
+
 HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
 {
 int i;
@@ -931,6 +952,17 @@ out_free:
 qapi_free_BootConfiguration(config);
 }
 
+void machine_add_audiodev_property(MachineClass *mc)
+{
+ObjectClass *oc = OBJECT_CLASS(mc);
+
+object_class_property_add_str(oc, "audiodev",
+  machine_get_audiodev,
+  machine_set_audiodev);
+object_class_property_set_description(oc, "audiodev",
+  "Audiodev to use for default machine 
devices");
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -1136,6 +1168,7 @@ static void machine_finalize(Object *obj)
 g_free(ms->device_memory);
 g_free(ms->nvdimms_state);
 g_free(ms->numa_state);
+g_free(ms->audiodev);
 }
 
 bool machine_usb(MachineState *machine)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 6c67af196a3..55a64a13fdf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -24,6 +24,7 @@ OBJECT_DECLARE_TYPE(MachineState, MachineClass, MACHINE)
 
 extern MachineState *current_machine;
 
+void machine_add_audiodev_property(MachineClass *mc);
 void machine_run_board_init(MachineState *machine, const char *mem_path, Error 
**errp);
 bool machine_usb(MachineState *machine);
 int machine_phandle_start(MachineState *machine);
@@ -358,6 +359,14 @@ struct MachineState {
 MemoryRegion *ram;
 DeviceMemoryState *device_memory;
 
+/*
+ * Included in MachineState for simplicity, but not supported
+ * unless machine_add_audiodev_property is called.  Boards
+ * that have embedded audio devices can call it from the
+ * machine init function and forward the property to the device.
+ */
+char *audiodev;
+
 ram_addr_t ram_size;
 ram_addr_t maxram_size;
 uint64_t   ram_slots;
-- 
2.41.0




[PULL 05/56] block/vdi: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Kevin Wolf 
Message-ID: <20230921121312.1301864-6-arm...@redhat.com>
---
 block/vdi.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 6c35309e04..934e1b849b 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -634,7 +634,6 @@ vdi_co_pwritev(BlockDriverState *bs, int64_t offset, 
int64_t bytes,
 bmap_entry = le32_to_cpu(s->bmap[block_index]);
 if (!VDI_IS_ALLOCATED(bmap_entry)) {
 /* Allocate new block and write to it. */
-uint64_t data_offset;
 qemu_co_rwlock_upgrade(&s->bmap_lock);
 bmap_entry = le32_to_cpu(s->bmap[block_index]);
 if (VDI_IS_ALLOCATED(bmap_entry)) {
@@ -700,7 +699,7 @@ nonallocating_write:
 /* One or more new blocks were allocated. */
 VdiHeader *header;
 uint8_t *base;
-uint64_t offset;
+uint64_t bmap_offset;
 uint32_t n_sectors;
 
 g_free(block);
@@ -723,11 +722,11 @@ nonallocating_write:
 bmap_first /= (SECTOR_SIZE / sizeof(uint32_t));
 bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
 n_sectors = bmap_last - bmap_first + 1;
-offset = s->bmap_sector + bmap_first;
+bmap_offset = s->bmap_sector + bmap_first;
 base = ((uint8_t *)&s->bmap[0]) + bmap_first * SECTOR_SIZE;
 logout("will write %u block map sectors starting from entry %u\n",
n_sectors, bmap_first);
-ret = bdrv_co_pwrite(bs->file, offset * SECTOR_SIZE,
+ret = bdrv_co_pwrite(bs->file, bmap_offset * SECTOR_SIZE,
  n_sectors * SECTOR_SIZE, base, 0);
 }
 
-- 
2.41.0




[PATCH 2/3] virtio-net: added replay blocker for guest_announce

2023-09-29 Thread pavel . dovgalyuk
From: Pavel Dovgalyuk 

This patch blocks record/replay when guest_announce is enabled,
because this flag breaks loading the snapshots in deterministic
execution mode.

Signed-off-by: Pavel Dovgalyuk 
---
 hw/net/virtio-net.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 50156e8002..eb50b7e030 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3603,6 +3603,10 @@ static void virtio_net_device_realize(DeviceState *dev, 
Error **errp)
 n->host_features |= (1ULL << VIRTIO_NET_F_MTU);
 }
 
+if (n->host_features & (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE)) {
+replay_add_blocker("-device virtio-net-device,guest_announce=true");
+}
+
 if (n->net_conf.duplex_str) {
 if (strncmp(n->net_conf.duplex_str, "half", 5) == 0) {
 n->net_conf.duplex = DUPLEX_HALF;
-- 
2.34.1




[PATCH v3 01/14] ui/vnc: Require audiodev= to enable audio

2023-09-29 Thread Paolo Bonzini
If there is no audiodev do not send the audio ack in response to
VNC_ENCODING_AUDIO, so that clients aren't told audio exists, and
immediately drop the client if they try to send any audio control messages
when audio is not advertised.

Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Paolo Bonzini 
---
 docs/about/deprecated.rst   |  8 +++-
 docs/about/removed-features.rst |  6 ++
 ui/vnc.c| 11 ++-
 ui/vnc.h|  2 ++
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 8f3fef97bd4..c07bf58dde1 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -45,13 +45,11 @@ backend settings instead of environment variables.  To ease 
migration to
 the new format, the ``-audiodev-help`` option can be used to convert
 the current values of the environment variables to ``-audiodev`` options.
 
-Creating sound card devices and vnc without ``audiodev=`` property (since 4.2)
-''
+Creating sound card devices without ``audiodev=`` property (since 4.2)
+''
 
 When not using the deprecated legacy audio config, each sound card
-should specify an ``audiodev=`` property.  Additionally, when using
-vnc, you should specify an ``audiodev=`` property if you plan to
-transmit audio through the VNC protocol.
+should specify an ``audiodev=`` property.
 
 Short-form boolean options (since 6.0)
 ''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 97ec47f1d25..276060b320c 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -436,6 +436,12 @@ the process listing. This was replaced by the new 
``password-secret``
 option which lets the password be securely provided on the command
 line using a ``secret`` object instance.
 
+Creating vnc without ``audiodev=`` property (removed in 8.2)
+
+
+When using vnc, you should specify an ``audiodev=`` property if
+you plan to transmit audio through the VNC protocol.
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/ui/vnc.c b/ui/vnc.c
index c302bb07a5b..acb56461b2d 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2195,7 +2195,10 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 send_ext_key_event_ack(vs);
 break;
 case VNC_ENCODING_AUDIO:
-send_ext_audio_ack(vs);
+if (vs->vd->audio_state) {
+vs->features |= VNC_FEATURE_AUDIO_MASK;
+send_ext_audio_ack(vs);
+}
 break;
 case VNC_ENCODING_WMVi:
 vs->features |= VNC_FEATURE_WMVI_MASK;
@@ -2502,6 +2505,12 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
   read_u32(data, 4), read_u32(data, 8));
 break;
 case VNC_MSG_CLIENT_QEMU_AUDIO:
+if (!vnc_has_feature(vs, VNC_FEATURE_AUDIO)) {
+error_report("Audio message %d with audio disabled", 
read_u8(data, 2));
+vnc_client_error(vs);
+break;
+}
+
 if (len == 2)
 return 4;
 
diff --git a/ui/vnc.h b/ui/vnc.h
index 757fa83044e..96d19dce199 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -464,6 +464,7 @@ enum VncFeatures {
 VNC_FEATURE_LED_STATE,
 VNC_FEATURE_XVP,
 VNC_FEATURE_CLIPBOARD_EXT,
+VNC_FEATURE_AUDIO,
 };
 
 #define VNC_FEATURE_RESIZE_MASK  (1 << VNC_FEATURE_RESIZE)
@@ -481,6 +482,7 @@ enum VncFeatures {
 #define VNC_FEATURE_LED_STATE_MASK   (1 << VNC_FEATURE_LED_STATE)
 #define VNC_FEATURE_XVP_MASK (1 << VNC_FEATURE_XVP)
 #define VNC_FEATURE_CLIPBOARD_EXT_MASK   (1 <<  VNC_FEATURE_CLIPBOARD_EXT)
+#define VNC_FEATURE_AUDIO_MASK   (1 <<  VNC_FEATURE_AUDIO)
 
 
 /* Client -> Server message IDs */
-- 
2.41.0




[PULL 17/56] hw/m68k: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/m68k/virt.c:263:13: error: declaration shadows a local variable 
[-Werror,-Wshadow]
BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
^
  hw/m68k/bootinfo.h:47:13: note: expanded from macro 'BOOTINFOSTR'
int i; \
^
  hw/m68k/virt.c:130:9: note: previous declaration is here
int i;
^

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-13-phi...@linaro.org>
Reviewed-by: Thomas Huth 
Signed-off-by: Markus Armbruster 
---
 hw/m68k/bootinfo.h | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index a3d37e3c80..0e6e3eea87 100644
--- a/hw/m68k/bootinfo.h
+++ b/hw/m68k/bootinfo.h
@@ -44,15 +44,14 @@
 
 #define BOOTINFOSTR(base, id, string) \
 do { \
-int i; \
 stw_p(base, id); \
 base += 2; \
 stw_p(base, \
  (sizeof(struct bi_record) + strlen(string) + \
   1 /* null termination */ + 3 /* padding */) & ~3); \
 base += 2; \
-for (i = 0; string[i]; i++) { \
-stb_p(base++, string[i]); \
+for (unsigned i_ = 0; string[i_]; i_++) { \
+stb_p(base++, string[i_]); \
 } \
 stb_p(base++, 0); \
 base = QEMU_ALIGN_PTR_UP(base, 4); \
@@ -60,7 +59,6 @@
 
 #define BOOTINFODATA(base, id, data, len) \
 do { \
-int i; \
 stw_p(base, id); \
 base += 2; \
 stw_p(base, \
@@ -69,8 +67,8 @@
 base += 2; \
 stw_p(base, len); \
 base += 2; \
-for (i = 0; i < len; ++i) { \
-stb_p(base++, data[i]); \
+for (unsigned i_ = 0; i_ < len; ++i_) { \
+stb_p(base++, data[i_]); \
 } \
 base = QEMU_ALIGN_PTR_UP(base, 4); \
 } while (0)
-- 
2.41.0




[PULL 23/56] linux-user/strace: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  linux-user/strace.c: In function ‘print_sockaddr’:
  linux-user/strace.c:370:17: warning: declaration of ‘i’ shadows a previous 
local [-Wshadow=compatible-local]
370 | int i;
| ^
  linux-user/strace.c:361:9: note: shadowed declaration is here
361 | int i;
| ^

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-20-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 linux-user/strace.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index e0ab8046ec..cf26e55264 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -367,7 +367,6 @@ print_sockaddr(abi_ulong addr, abi_long addrlen, int last)
 switch (sa_family) {
 case AF_UNIX: {
 struct target_sockaddr_un *un = (struct target_sockaddr_un *)sa;
-int i;
 qemu_log("{sun_family=AF_UNIX,sun_path=\"");
 for (i = 0; i < addrlen -
 offsetof(struct target_sockaddr_un, sun_path) &&
-- 
2.41.0




[PULL 00/56] -Wshadow=local patches patches for 2023-09-29

2023-09-29 Thread Markus Armbruster
The following changes since commit 36e9aab3c569d4c9ad780473596e18479838d1aa:

  migration: Move return path cleanup to main migration thread (2023-09-27 
13:58:02 -0400)

are available in the Git repository at:

  https://repo.or.cz/qemu/armbru.git tags/pull-shadow-2023-09-29

for you to fetch changes up to 4dba9141f97e66fdd920df37c4aa7b2ffe0d6a4a:

  disas/m68k: clean up local variable shadowing (2023-09-29 10:07:21 +0200)


-Wshadow=local patches patches for 2023-09-29


Alberto Garcia (1):
  test-throttle: don't shadow 'index' variable in do_test_accounting()

Alistair Francis (4):
  hw/riscv: opentitan: Fixup local variables shadowing
  target/riscv: cpu: Fixup local variables shadowing
  target/riscv: vector_helper: Fixup local variables shadowing
  softmmu/device_tree: Fixup local variables shadowing

Ani Sinha (1):
  hw/acpi: changes towards enabling -Wshadow=local

Cédric Le Goater (12):
  hw/ppc: Clean up local variable shadowing in _FDT helper routine
  pnv/psi: Clean up local variable shadowing
  spapr: Clean up local variable shadowing in spapr_dt_cpus()
  spapr: Clean up local variable shadowing in spapr_init_cpus()
  spapr: Clean up local variable shadowing in spapr_get_fw_dev_path()
  spapr/drc: Clean up local variable shadowing in 
rtas_ibm_configure_connector()
  spapr/pci: Clean up local variable shadowing in spapr_phb_realize()
  spapr/drc: Clean up local variable shadowing in prop_get_fdt()
  aspeed/i2c: Clean up local variable shadowing
  aspeed: Clean up local variable shadowing
  aspeed/i3c: Rename variable shadowing a local
  aspeed/timer: Clean up local variable shadowing

Daniel P. Berrangé (2):
  crypto: remove shadowed 'ret' variable
  seccomp: avoid shadowing of 'action' variable

Eric Blake (1):
  qemu-nbd: changes towards enabling -Wshadow=local

Klaus Jensen (1):
  hw/nvme: Clean up local variable shadowing in nvme_ns_init()

Laurent Vivier (1):
  disas/m68k: clean up local variable shadowing

Markus Armbruster (7):
  migration/rdma: Fix save_page method to fail on polling error
  migration: Clean up local variable shadowing
  ui: Clean up local variable shadowing
  block/dirty-bitmap: Clean up local variable shadowing
  block/vdi: Clean up local variable shadowing
  block: Clean up local variable shadowing
  qobject atomics osdep: Make a few macros more hygienic

Peter Maydell (4):
  hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()
  hw/misc/arm_sysctl.c: Avoid shadowing local variable
  hw/arm/smmuv3.c: Avoid shadowing variable
  hw/arm/smmuv3-internal.h: Don't use locals in statement macros

Peter Xu (1):
  intel_iommu: Fix shadow local variables on "size"

Philippe Mathieu-Daudé (21):
  tcg: Clean up local variable shadowing
  target/arm/tcg: Clean up local variable shadowing
  target/arm/hvf: Clean up local variable shadowing
  target/mips: Clean up local variable shadowing
  target/m68k: Clean up local variable shadowing
  target/tricore: Clean up local variable shadowing
  hw/arm/armv7m: Clean up local variable shadowing
  hw/arm/virt: Clean up local variable shadowing
  hw/arm/allwinner: Clean up local variable shadowing
  hw/m68k: Clean up local variable shadowing
  hw/microblaze: Clean up local variable shadowing
  hw/nios2: Clean up local variable shadowing
  net/eth: Clean up local variable shadowing
  crypto/cipher-gnutls.c: Clean up local variable shadowing
  util/vhost-user-server: Clean up local variable shadowing
  linux-user/strace: Clean up local variable shadowing
  sysemu/device_tree: Clean up local variable shadowing
  softmmu/memory: Clean up local variable shadowing
  softmmu/physmem: Clean up local variable shadowing
  hw/core/machine: Clean up local variable shadowing
  hw/intc/openpic: Clean up local variable shadowing

 hw/arm/smmuv3-internal.h | 41 ++-
 hw/m68k/bootinfo.h   | 10 +++-
 include/hw/ppc/fdt.h |  8 +++---
 include/qapi/qmp/qobject.h   | 10 ++--
 include/qemu/atomic.h| 17 +
 include/qemu/compiler.h  |  3 +++
 include/qemu/osdep.h | 27 ++--
 include/sysemu/device_tree.h |  6 ++---
 accel/tcg/tb-maint.c |  3 +--
 block.c  |  9 ---
 block/monitor/bitmap-qmp-cmds.c  | 19 ---
 block/qcow2-bitmap.c |  3 +--
 block/rbd.c  |  2 +-
 block/stream.c   |  1 -
 block/vdi.c  |  7 +++---
 block/vvfat.c

[PULL 18/56] hw/microblaze: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/microblaze/petalogix_ml605_mmu.c: In function ‘petalogix_ml605_init’:
  hw/microblaze/petalogix_ml605_mmu.c:186:24: warning: declaration of ‘dinfo’ 
shadows a previous local [-Wshadow=compatible-local]
186 | DriveInfo *dinfo = drive_get(IF_MTD, 0, i);
|^
  hw/microblaze/petalogix_ml605_mmu.c:78:16: note: shadowed declaration is here
 78 | DriveInfo *dinfo;
|^

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-14-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 hw/microblaze/petalogix_ml605_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index ea0fb68cf0..fb7889cf67 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -183,7 +183,7 @@ petalogix_ml605_init(MachineState *machine)
 spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
 
 for (i = 0; i < NUM_SPI_FLASHES; i++) {
-DriveInfo *dinfo = drive_get(IF_MTD, 0, i);
+dinfo = drive_get(IF_MTD, 0, i);
 qemu_irq cs_line;
 
 dev = qdev_new("n25q128");
-- 
2.41.0




[PULL 13/56] target/tricore: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  target/tricore/translate.c:5016:18: warning: declaration of ‘temp’ shadows a 
previous local [-Wshadow=compatible-local]
   5016 | TCGv temp = tcg_constant_i32(const9);
|  ^~~~
  target/tricore/translate.c:4958:10: note: shadowed declaration is here
   4958 | TCGv temp;
|  ^~~~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-7-phi...@linaro.org>
Reviewed-by: Bastian Koppelmann 
Signed-off-by: Markus Armbruster 
---
 target/tricore/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 6ae5ccbf72..9ca211b2a8 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -4962,8 +4962,6 @@ static void decode_rc_logical_shift(DisasContext *ctx)
 const9 = MASK_OP_RC_CONST9(ctx->opcode);
 op2 = MASK_OP_RC_OP2(ctx->opcode);
 
-temp = tcg_temp_new();
-
 switch (op2) {
 case OPC2_32_RC_AND:
 tcg_gen_andi_tl(cpu_gpr_d[r2], cpu_gpr_d[r1], const9);
@@ -4972,10 +4970,12 @@ static void decode_rc_logical_shift(DisasContext *ctx)
 tcg_gen_andi_tl(cpu_gpr_d[r2], cpu_gpr_d[r1], ~const9);
 break;
 case OPC2_32_RC_NAND:
+temp = tcg_temp_new();
 tcg_gen_movi_tl(temp, const9);
 tcg_gen_nand_tl(cpu_gpr_d[r2], cpu_gpr_d[r1], temp);
 break;
 case OPC2_32_RC_NOR:
+temp = tcg_temp_new();
 tcg_gen_movi_tl(temp, const9);
 tcg_gen_nor_tl(cpu_gpr_d[r2], cpu_gpr_d[r1], temp);
 break;
@@ -5013,7 +5013,7 @@ static void decode_rc_logical_shift(DisasContext *ctx)
 break;
 case OPC2_32_RC_SHUFFLE:
 if (has_feature(ctx, TRICORE_FEATURE_162)) {
-TCGv temp = tcg_constant_i32(const9);
+temp = tcg_constant_i32(const9);
 gen_helper_shuffle(cpu_gpr_d[r2], cpu_gpr_d[r1], temp);
 } else {
 generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
-- 
2.41.0




[PULL 12/56] target/m68k: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  target/m68k/translate.c:828:18: error: declaration shadows a local variable 
[-Werror,-Wshadow]
TCGv tmp = tcg_temp_new();
 ^
  target/m68k/translate.c:801:15: note: previous declaration is here
TCGv reg, tmp, result;
  ^

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 
Message-ID: <20230904161235.84651-6-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 target/m68k/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 9e224fe796..15c9ddf427 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -824,7 +824,7 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, 
int mode, int reg0,
 reg = get_areg(s, reg0);
 result = gen_ldst(s, opsize, reg, val, what, index);
 if (what == EA_STORE || !addrp) {
-TCGv tmp = tcg_temp_new();
+tmp = tcg_temp_new();
 if (reg0 == 7 && opsize == OS_BYTE &&
 m68k_feature(s->env, M68K_FEATURE_M68K)) {
 tcg_gen_addi_i32(tmp, reg, 2);
-- 
2.41.0




[PULL 25/56] softmmu/memory: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  softmmu/memory.c: In function ‘mtree_print_mr’:
  softmmu/memory.c:3236:27: warning: declaration of ‘ml’ shadows a previous 
local [-Wshadow=compatible-local]
   3236 | MemoryRegionList *ml;
|   ^~
  softmmu/memory.c:3213:32: note: shadowed declaration is here
   3213 | MemoryRegionList *new_ml, *ml, *next_ml;
|^~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-22-phi...@linaro.org>
Reviewed-by: Peter Xu 
Signed-off-by: Markus Armbruster 
---
 softmmu/memory.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index c0383a163d..234bd7b116 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3245,7 +3245,6 @@ static void mtree_print_mr(const MemoryRegion *mr, 
unsigned int level,
 }
 
 if (mr->alias) {
-MemoryRegionList *ml;
 bool found = false;
 
 /* check if the alias is already in the queue */
-- 
2.41.0




[PULL 07/56] qobject atomics osdep: Make a few macros more hygienic

2023-09-29 Thread Markus Armbruster
Variables declared in macros can shadow other variables.  Much of the
time, this is harmless, e.g.:

#define _FDT(exp)  \
do {   \
int ret = (exp);   \
if (ret < 0) { \
error_report("error creating device tree: %s: %s",   \
#exp, fdt_strerror(ret));  \
exit(1);   \
}  \
} while (0)

Harmless shadowing in h_client_architecture_support():

target_ulong ret;

[...]

ret = do_client_architecture_support(cpu, spapr, vec, fdt_bufsize);
if (ret == H_SUCCESS) {
_FDT((fdt_pack(spapr->fdt_blob)));
[...]
}

return ret;

However, we can get in trouble when the shadowed variable is used in a
macro argument:

#define QOBJECT(obj) ({ \
typeof(obj) o = (obj);  \
o ? container_of(&(o)->base, QObject, base) : NULL; \
 })

QOBJECT(o) expands into

({
--->typeof(o) o = (o);
o ? container_of(&(o)->base, QObject, base) : NULL;
})

Unintended variable name capture at --->.  We'd be saved by
-Winit-self.  But I could certainly construct more elaborate death
traps that don't trigger it.

To reduce the risk of trapping ourselves, we use variable names in
macros that no sane person would use elsewhere.  Here's our actual
definition of QOBJECT():

#define QOBJECT(obj) ({ \
typeof(obj) _obj = (obj);   \
_obj ? container_of(&(_obj)->base, QObject, base) : NULL;   \
})

Works well enough until we nest macro calls.  For instance, with

#define qobject_ref(obj) ({ \
typeof(obj) _obj = (obj);   \
qobject_ref_impl(QOBJECT(_obj));\
_obj;   \
})

the expression qobject_ref(obj) expands into

({
typeof(obj) _obj = (obj);
qobject_ref_impl(
({
--->typeof(_obj) _obj = (_obj);
_obj ? container_of(&(_obj)->base, QObject, base) : NULL;
}));
_obj;
})

Unintended variable name capture at --->.

The only reliable way to prevent unintended variable name capture is
-Wshadow.

One blocker for enabling it is shadowing hiding in function-like
macros like

 qdict_put(dict, "name", qobject_ref(...))

qdict_put() wraps its last argument in QOBJECT(), and the last
argument here contains another QOBJECT().

Use dark preprocessor sorcery to make the macros that give us this
problem use different variable names on every call.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Message-ID: <20230921121312.1301864-8-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
---
 include/qapi/qmp/qobject.h | 10 --
 include/qemu/atomic.h  | 17 -
 include/qemu/compiler.h|  3 +++
 include/qemu/osdep.h   | 27 ---
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 9003b71fd3..89b97d88bc 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -45,10 +45,16 @@ struct QObject {
 struct QObjectBase_ base;
 };
 
-#define QOBJECT(obj) ({ \
+/*
+ * Preprocessor sorcery ahead: use a different identifier for the
+ * local variable in each expansion, so we can nest macro calls
+ * without shadowing variables.
+ */
+#define QOBJECT_INTERNAL(obj, _obj) ({  \
 typeof(obj) _obj = (obj);   \
-_obj ? container_of(&(_obj)->base, QObject, base) : NULL;   \
+_obj ? container_of(&_obj->base, QObject, base) : NULL; \
 })
+#define QOBJECT(obj) QOBJECT_INTERNAL((obj), MAKE_IDENTFIER(_obj))
 
 /* Required for qobject_to() */
 #define QTYPE_CAST_TO_QNull QTYPE_QNULL
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index d95612f7a0..f1d3d1702a 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -157,13 +157,20 @@
 smp_read_barrier_depends();
 #endif
 
-#define qatomic_rcu_read(ptr)  \
-({ \
+/*
+ * Preprocessor sorcery ahead: use a different identifier for the
+ * local variable in each expansion, so we can nest macro calls
+ * without shadowing variables.
+ */
+#define qatomic_rcu_read_internal(ptr, _val)\
+({  \
 qemu_build_assert(sizeof(*ptr) <= ATOMIC_REG_SIZE); \
-

[PULL 24/56] sysemu/device_tree: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/mips/boston.c:472:5: error: declaration shadows a local variable 
[-Werror,-Wshadow]
qemu_fdt_setprop_cells(fdt, name, "reg", reg_base, reg_size);
^
  include/sysemu/device_tree.h:129:13: note: expanded from macro 
'qemu_fdt_setprop_cells'
int i;
^
  hw/mips/boston.c:461:9: note: previous declaration is here
int i;
^

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-21-phi...@linaro.org>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 include/sysemu/device_tree.h | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ca5339beae..8eab395934 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -126,10 +126,8 @@ int qemu_fdt_add_path(void *fdt, const char *path);
 #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
 do {  \
 uint32_t qdt_tmp[] = { __VA_ARGS__ }; \
-int i;\
-  \
-for (i = 0; i < ARRAY_SIZE(qdt_tmp); i++) {   \
-qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); \
+for (unsigned i_ = 0; i_ < ARRAY_SIZE(qdt_tmp); i_++) {   \
+qdt_tmp[i_] = cpu_to_be32(qdt_tmp[i_]);   \
 } \
 qemu_fdt_setprop(fdt, node_path, property, qdt_tmp,   \
  sizeof(qdt_tmp));\
-- 
2.41.0




[PATCH v3 02/14] audio: Require AudioState in AUD_add_capture

2023-09-29 Thread Paolo Bonzini
From: Martin Kletzander 

Since all callers require a valid audiodev this function can now safely
abort in case of missing AudioState.

Signed-off-by: Martin Kletzander 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
 audio/audio.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 2f479657117..4332c4c6ce8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1876,10 +1876,9 @@ CaptureVoiceOut *AUD_add_capture(
 struct capture_callback *cb;
 
 if (!s) {
-if (!legacy_config) {
-dolog("Capturing without setting an audiodev is deprecated\n");
-}
-s = audio_init(NULL, NULL);
+error_setg(&error_abort,
+   "Capturing without setting an audiodev is not supported");
+abort();
 }
 
 if (!audio_get_pdo_out(s->dev)->mixing_engine) {
-- 
2.41.0




[PULL 09/56] target/arm/tcg: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  target/arm/tcg/translate-m-nocp.c: In function ‘gen_M_fp_sysreg_read’:
  target/arm/tcg/translate-m-nocp.c:509:18: warning: declaration of ‘tmp’ 
shadows a previous local [-Wshadow=compatible-local]
509 | TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
|  ^~~
  target/arm/tcg/translate-m-nocp.c:433:14: note: shadowed declaration is here
433 | TCGv_i32 tmp;
|  ^~~
   ---

  target/arm/tcg/mve_helper.c: In function ‘helper_mve_vqshlsb’:
  target/arm/tcg/mve_helper.c:1259:19: warning: declaration of ‘r’ shadows a 
previous local [-Wshadow=compatible-local]
   1259 | typeof(N) r = FN(N, (int8_t)(M), sizeof(N) * 8, ROUND, 
&su32);  \
|   ^
  target/arm/tcg/mve_helper.c:1267:5: note: in expansion of macro 
‘WRAP_QRSHL_HELPER’
   1267 | WRAP_QRSHL_HELPER(do_sqrshl_bhs, N, M, false, satp)
| ^
  target/arm/tcg/mve_helper.c:927:22: note: in expansion of macro ‘DO_SQSHL_OP’
927 | TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);
  \
|  ^~
  target/arm/tcg/mve_helper.c:945:5: note: in expansion of macro ‘DO_2OP_SAT’
945 | DO_2OP_SAT(OP##b, 1, int8_t, FN)\
| ^~
  target/arm/tcg/mve_helper.c:1277:1: note: in expansion of macro ‘DO_2OP_SAT_S’
   1277 | DO_2OP_SAT_S(vqshls, DO_SQSHL_OP)
| ^~~~
   ---

  target/arm/tcg/mve_helper.c: In function ‘do_sqrshl48_d’:
  target/arm/tcg/mve_helper.c:2463:17: warning: declaration of ‘extval’ shadows 
a previous local [-Wshadow=compatible-local]
   2463 | int64_t extval = sextract64(src << shift, 0, 48);
| ^~
  target/arm/tcg/mve_helper.c:2443:18: note: shadowed declaration is here
   2443 | int64_t val, extval;
|  ^~
   ---

  target/arm/tcg/mve_helper.c: In function ‘do_uqrshl48_d’:
  target/arm/tcg/mve_helper.c:2495:18: warning: declaration of ‘extval’ shadows 
a previous local [-Wshadow=compatible-local]
   2495 | uint64_t extval = extract64(src << shift, 0, 48);
|  ^~
  target/arm/tcg/mve_helper.c:2479:19: note: shadowed declaration is here
   2479 | uint64_t val, extval;
|   ^~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-3-phi...@linaro.org>
Reviewed-by: Peter Maydell 
Signed-off-by: Markus Armbruster 
---
 target/arm/tcg/mve_helper.c   | 16 
 target/arm/tcg/translate-m-nocp.c |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/arm/tcg/mve_helper.c b/target/arm/tcg/mve_helper.c
index c666a96ba1..8b99736aad 100644
--- a/target/arm/tcg/mve_helper.c
+++ b/target/arm/tcg/mve_helper.c
@@ -925,8 +925,8 @@ DO_1OP_IMM(vorri, DO_ORRI)
 bool qc = false;\
 for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {  \
 bool sat = false;   \
-TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);  \
-mergemask(&d[H##ESIZE(e)], r, mask);\
+TYPE r_ = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat); \
+mergemask(&d[H##ESIZE(e)], r_, mask);   \
 qc |= sat & mask & 1;   \
 }   \
 if (qc) {   \
@@ -1250,11 +1250,11 @@ DO_2OP_SAT(vqsubsw, 4, int32_t, DO_SQSUB_W)
 #define WRAP_QRSHL_HELPER(FN, N, M, ROUND, satp)\
 ({  \
 uint32_t su32 = 0;  \
-typeof(N) r = FN(N, (int8_t)(M), sizeof(N) * 8, ROUND, &su32);  \
+typeof(N) qrshl_ret = FN(N, (int8_t)(M), sizeof(N) * 8, ROUND, &su32); 
\
 if (su32) { \
 *satp = true;   \
 }   \
-r;  \
+qrshl_ret;  \
 })
 
 #define DO_SQSHL_OP(N, M, satp) \
@@ -1292,12 +1292,12 @@ DO_2OP_SAT_U(vqrshlu, DO_UQRSHL_OP)
 for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {  \
 bool sat = false;   \
 if ((e & 1) == XCHG) {  \
-TYPE r = FN(n[H##ESIZE(e)], \
+TYPE vqdmladh_ret = FN(n[H##ESIZE(e)],  \
 m[H##ESIZE(

[PATCH v3 00/14] Cleanup deprecated audio features, take 2

2023-09-29 Thread Paolo Bonzini
In this version the QEMU_AUDIO_* options go away, but it is still
possible to pick a default audio backend from the list provided to
--audio-drv-list.  The code is still simplified a lot compared to
having all the legacy parsing code and the -audio-help function.
I had to keep QEMU_AUDIO_DRV=none because it is used by libqtest;
a possibility for the future could be to add a "-audio none" option
without a model.  For now I kept this setting as it is a subset of the
previous accepted values, and it can be deprecated separately.

At the end of this series, all devices can be configured with
-audiodev.  Therefore, I decided to forbid mixing the default
audio backend with audiodevs or with -nodefaults.

Patches 1-2 are Martin's patches that didn't end up in the previous
pull request.

Patches 3-4 change audio.c to use Error ** a bit more.

Patches 5-7 introduce the minimal code to create a default audio
backend.

Patches 8-11 introduce a machine property "audiodev" and plumb
it into all machines with an embedded sound card.

Patches 12-13 forbid some not-so-sensible usage of default
audio backends.

Patch 14 is an Error** propagation cleanup to avoid giving
Markus a heart attack if he ever looks at this code.  Or
at least moderating the intensity.

Paolo

v1->v2:
- do not expose audio encoding feature if VNC audio is not enabled [Daniel]
- add hint to error messages when -audiodev and default backend are mixed
- context changes due to shadowed variable fixes

v2->v3:
- adjust comment on "char *audiodev" field of MachineState
- use error_fatal instead of error_abort in OMAP and TSC210x's calls
  to AUD_register_card
- redo vt82c686 patches to remove aliased audiodev property
- fix leak in audio_driver_init when msg == false
- add patch to use Error in audio_init

Martin Kletzander (4):
  audio: Require AudioState in AUD_add_capture
  Introduce machine property "audiodev"
  hw/arm: Support machine-default audiodev with fallback
  hw/ppc: Support machine-default audiodev with fallback

Paolo Bonzini (10):
  ui/vnc: Require audiodev= to enable audio
  audio: allow returning an error from the driver init
  audio: return Error ** from audio_state_by_name
  audio: commonize voice initialization
  audio: simplify flow in audio_init
  audio: remove QEMU_AUDIO_* and -audio-help support
  vt82c686: Support machine-default audiodev with fallback
  audio: forbid mixing default audiodev backend and -audiodev
  audio: forbid default audiodev backend with -nodefaults
  audio: propagate Error out of audio_driver_init

 audio/alsaaudio.c|   3 +-
 audio/audio-hmp-cmds.c   |   6 +-
 audio/audio.c| 228 ++--
 audio/audio.h|   7 +-
 audio/audio_int.h|   7 +-
 audio/audio_legacy.c | 591 ---
 audio/audio_template.h   |   9 +-
 audio/coreaudio.m|   3 +-
 audio/dbusaudio.c|   3 +-
 audio/dsoundaudio.c  |   3 +-
 audio/jackaudio.c|   3 +-
 audio/meson.build|   1 -
 audio/noaudio.c  |   3 +-
 audio/ossaudio.c |  12 +-
 audio/paaudio.c  |   8 +-
 audio/pwaudio.c  |  17 +-
 audio/sdlaudio.c |   6 +-
 audio/sndioaudio.c   |   3 +-
 audio/spiceaudio.c   |   5 +-
 audio/wavaudio.c |   3 +-
 docs/about/deprecated.rst|  16 +-
 docs/about/removed-features.rst  |  12 +
 hw/arm/integratorcp.c|  11 +-
 hw/arm/musicpal.c|  11 +-
 hw/arm/nseries.c |   4 +
 hw/arm/omap2.c   |   7 +-
 hw/arm/palm.c|   2 +
 hw/arm/realview.c|  12 +
 hw/arm/spitz.c   |  17 +-
 hw/arm/versatilepb.c |   8 +
 hw/arm/vexpress.c|   5 +
 hw/arm/xlnx-zcu102.c |   6 +
 hw/arm/z2.c  |  15 +-
 hw/audio/ac97.c  |   6 +-
 hw/audio/adlib.c |   6 +-
 hw/audio/cs4231a.c   |   6 +-
 hw/audio/es1370.c|   5 +-
 hw/audio/gus.c   |   6 +-
 hw/audio/hda-codec.c |   5 +-
 hw/audio/lm4549.c|   8 +-
 hw/audio/pcspk.c |   4 +-
 hw/audio/sb16.c  |   6 +-
 hw/audio/via-ac97.c  |   6 +-
 hw/audio/wm8750.c|   5 +-
 hw/core/machine.c|  33 ++
 hw/core/qdev-properties-system.c |  16 +-
 hw/display/xlnx_dp.c |   6 +-
 hw/input/tsc210x.c   |   7 +-
 hw/mips/fuloong2e.c  |  15 +-
 hw/ppc/pegasos2.c|  12 +-
 hw/ppc/prep.c|   7 +
 hw/usb/dev-audio.c   |   5 +-
 include/hw/boards.h  |   9 +
 qemu-options.hx  |  10 -
 softmmu/vl.c |   8 +-
 ui/dbus.c|   3 +-
 ui/vnc.c

[PULL 38/56] hw/acpi: changes towards enabling -Wshadow=local

2023-09-29 Thread Markus Armbruster
From: Ani Sinha 

Code changes in acpi that addresses all compiler complaints coming from enabling
-Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
other local variables or parameters. These makes the code confusing and/or adds
bugs that are difficult to catch.  See also

Subject: Help wanted for enabling -Wshadow=local
Message-Id: <87r0mqlf9x@pond.sub.org>
https://lore.kernel.org/qemu-devel/87r0mqlf9x@pond.sub.org

The code is tested to build with and without the flag turned on.

CC: Markus Armbruster 
CC: Philippe Mathieu-Daude 
CC: m...@redhat.com
CC: imamm...@redhat.com
Signed-off-by: Ani Sinha 
Message-ID: <20230922124203.127110-1-anisi...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
[Commit message tweaked]
Signed-off-by: Markus Armbruster 
---
 hw/acpi/cpu_hotplug.c | 25 +
 hw/i386/acpi-build.c  | 24 
 hw/smbios/smbios.c| 37 +++--
 3 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index ff14c3f410..634bbecb31 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -265,26 +265,27 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState 
*machine,
 
 /* build Processor object for each processor */
 for (i = 0; i < apic_ids->len; i++) {
-int apic_id = apic_ids->cpus[i].arch_id;
+int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
 
-dev = aml_processor(i, 0, 0, "CP%.02X", apic_id);
+dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id);
 
 method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call2(CPU_MAT_METHOD, aml_int(apic_id), aml_int(i))
+aml_return(aml_call2(CPU_MAT_METHOD,
+ aml_int(cpu_apic_id), aml_int(i))
 ));
 aml_append(dev, method);
 
 method = aml_method("_STA", 0, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id;
+aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(cpu_apic_id;
 aml_append(dev, method);
 
 method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id),
 aml_arg(0)))
 );
 aml_append(dev, method);
@@ -298,11 +299,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState 
*machine,
 /* Arg0 = APIC ID */
 method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
 for (i = 0; i < apic_ids->len; i++) {
-int apic_id = apic_ids->cpus[i].arch_id;
+int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
+if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id)));
 aml_append(if_ctx,
-aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
+aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1))
 );
 aml_append(method, if_ctx);
 }
@@ -319,13 +320,13 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState 
*machine,
 aml_varpackage(x86ms->apic_id_limit);
 
 for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
-int apic_id = apic_ids->cpus[i].arch_id;
+int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-for (; apic_idx < apic_id; apic_idx++) {
+for (; apic_idx < cpu_apic_id; apic_idx++) {
 aml_append(pkg, aml_int(0));
 }
 aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
-apic_idx = apic_id + 1;
+apic_idx = cpu_apic_id + 1;
 }
 aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
 aml_append(ctx, sb_scope);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4d2d40bab5..95199c8900 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1585,12 +1585,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
 aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
 if (pci_bus_is_cxl(bus)) {
-struct Aml *pkg = aml_package(2);
+struct Aml *aml_pkg = aml_package(2);
 
 aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0016")));
-aml_append(pkg, aml_eisaid("PNP0A08"));
-aml_append(pkg, aml_eisaid("PNP0A03"));
-aml_append(dev, aml_name_decl("_CID", pkg));
+aml_append(aml_pkg, aml_eisaid("PNP0A08"));
+aml_append(aml_pkg, aml_eisaid("PNP0A03"));
+aml_append(dev, aml_name_decl("_CID"

[PATCH v3 06/14] audio: simplify flow in audio_init

2023-09-29 Thread Paolo Bonzini
Merge two ifs into one.

Signed-off-by: Paolo Bonzini 
---
 audio/audio.c | 76 +--
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 2014a2904e8..e3ca5fe65ec 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1707,12 +1707,12 @@ static AudiodevListEntry *audiodev_find(
  * if dev == NULL => legacy implicit initialization, return the already created
  *   state or create a new one
  */
-static AudioState *audio_init(Audiodev *dev, const char *name)
+static AudioState *audio_init(Audiodev *dev)
 {
 static bool atexit_registered;
 size_t i;
 int done = 0;
-const char *drvname = NULL;
+const char *drvname;
 VMChangeStateEntry *vmse;
 AudioState *s;
 struct audio_driver *driver;
@@ -1736,17 +1736,32 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 }
 }
 
+s = g_new0(AudioState, 1);
+
+QLIST_INIT (&s->hw_head_out);
+QLIST_INIT (&s->hw_head_in);
+QLIST_INIT (&s->cap_head);
+if (!atexit_registered) {
+atexit(audio_cleanup);
+atexit_registered = true;
+}
+
+s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
+
 if (dev) {
 /* -audiodev option */
-legacy_config = false;
+s->dev = dev;
 drvname = AudiodevDriver_str(dev->driver);
-} else if (!QTAILQ_EMPTY(&audio_states)) {
-if (!legacy_config) {
-dolog("Device %s: audiodev default parameter is deprecated, please 
"
-  "specify audiodev=%s\n", name,
-  QTAILQ_FIRST(&audio_states)->dev->id);
+driver = audio_driver_lookup(drvname);
+if (driver) {
+done = !audio_driver_init(s, driver, true, dev);
+} else {
+dolog ("Unknown audio driver `%s'\n", drvname);
+}
+if (!done) {
+free_audio_state(s);
+return NULL;
 }
-return QTAILQ_FIRST(&audio_states);
 } else {
 /* legacy implicit initialization */
 head = audio_handle_legacy_opts();
@@ -1759,33 +1774,7 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
  */
 dev = QSIMPLEQ_FIRST(&head)->dev;
 audio_validate_opts(dev, &error_abort);
-}
 
-s = g_new0(AudioState, 1);
-s->dev = dev;
-
-QLIST_INIT (&s->hw_head_out);
-QLIST_INIT (&s->hw_head_in);
-QLIST_INIT (&s->cap_head);
-if (!atexit_registered) {
-atexit(audio_cleanup);
-atexit_registered = true;
-}
-
-s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
-
-if (drvname) {
-driver = audio_driver_lookup(drvname);
-if (driver) {
-done = !audio_driver_init(s, driver, true, dev);
-} else {
-dolog ("Unknown audio driver `%s'\n", drvname);
-}
-if (!done) {
-free_audio_state(s);
-return NULL;
-}
-} else {
 for (i = 0; audio_prio_list[i]; i++) {
 AudiodevListEntry *e = audiodev_find(&head, audio_prio_list[i]);
 driver = audio_driver_lookup(audio_prio_list[i]);
@@ -1800,8 +1789,9 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 }
 }
 }
+
+audio_free_audiodev_list(&head);
 }
-audio_free_audiodev_list(&head);
 
 if (!done) {
 driver = audio_driver_lookup("none");
@@ -1841,7 +1831,16 @@ void audio_free_audiodev_list(AudiodevListHead *head)
 void AUD_register_card (const char *name, QEMUSoundCard *card)
 {
 if (!card->state) {
-card->state = audio_init(NULL, name);
+if (!QTAILQ_EMPTY(&audio_states)) {
+if (!legacy_config) {
+dolog("Device %s: audiodev default parameter is deprecated, 
please "
+  "specify audiodev=%s\n", name,
+  QTAILQ_FIRST(&audio_states)->dev->id);
+}
+card->state = QTAILQ_FIRST(&audio_states);
+} else {
+card->state = audio_init(NULL);
+}
 }
 
 card->name = g_strdup (name);
@@ -2172,6 +2171,7 @@ void audio_define(Audiodev *dev)
 e = g_new0(AudiodevListEntry, 1);
 e->dev = dev;
 QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
+legacy_config = false;
 }
 
 bool audio_init_audiodevs(void)
@@ -2179,7 +2179,7 @@ bool audio_init_audiodevs(void)
 AudiodevListEntry *e;
 
 QSIMPLEQ_FOREACH(e, &audiodevs, next) {
-if (!audio_init(e->dev, NULL)) {
+if (!audio_init(e->dev)) {
 return false;
 }
 }
-- 
2.41.0




[PULL 06/56] block: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
Acked-by: Anthony PERARD 
Acked-by: Ilya Dryomov 
Reviewed-by: Kevin Wolf 
Message-ID: <20230921121312.1301864-7-arm...@redhat.com>
---
 block.c  |  9 +
 block/rbd.c  |  2 +-
 block/stream.c   |  1 -
 block/vvfat.c| 35 ++-
 hw/block/xen-block.c |  6 +++---
 5 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/block.c b/block.c
index e7f349b25c..af04c8ac6f 100644
--- a/block.c
+++ b/block.c
@@ -3072,18 +3072,19 @@ bdrv_attach_child_common(BlockDriverState *child_bs,
   &local_err);
 
 if (ret < 0 && child_class->change_aio_ctx) {
-Transaction *tran = tran_new();
+Transaction *aio_ctx_tran = tran_new();
 GHashTable *visited = g_hash_table_new(NULL, NULL);
 bool ret_child;
 
 g_hash_table_add(visited, new_child);
 ret_child = child_class->change_aio_ctx(new_child, child_ctx,
-visited, tran, NULL);
+visited, aio_ctx_tran,
+NULL);
 if (ret_child == true) {
 error_free(local_err);
 ret = 0;
 }
-tran_finalize(tran, ret_child == true ? 0 : -1);
+tran_finalize(aio_ctx_tran, ret_child == true ? 0 : -1);
 g_hash_table_destroy(visited);
 }
 
@@ -6208,12 +6209,12 @@ void bdrv_iterate_format(void (*it)(void *opaque, const 
char *name),
 QLIST_FOREACH(drv, &bdrv_drivers, list) {
 if (drv->format_name) {
 bool found = false;
-int i = count;
 
 if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, read_only)) {
 continue;
 }
 
+i = count;
 while (formats && i && !found) {
 found = !strcmp(formats[--i], drv->format_name);
 }
diff --git a/block/rbd.c b/block/rbd.c
index 978671411e..472ca05cba 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1290,7 +1290,7 @@ static int coroutine_fn 
qemu_rbd_start_co(BlockDriverState *bs,
  * operations that exceed the current size.
  */
 if (offset + bytes > s->image_size) {
-int r = qemu_rbd_resize(bs, offset + bytes);
+r = qemu_rbd_resize(bs, offset + bytes);
 if (r < 0) {
 return r;
 }
diff --git a/block/stream.c b/block/stream.c
index e4da214f1f..133cb72fb4 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -292,7 +292,6 @@ void stream_start(const char *job_id, BlockDriverState *bs,
 /* Make sure that the image is opened in read-write mode */
 bs_read_only = bdrv_is_read_only(bs);
 if (bs_read_only) {
-int ret;
 /* Hold the chain during reopen */
 if (bdrv_freeze_backing_chain(bs, above_base, errp) < 0) {
 return;
diff --git a/block/vvfat.c b/block/vvfat.c
index 0ddc91fc09..856b479c91 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -777,7 +777,6 @@ static int read_directory(BDRVVVFATState* s, int 
mapping_index)
 while((entry=readdir(dir))) {
 unsigned int length=strlen(dirname)+2+strlen(entry->d_name);
 char* buffer;
-direntry_t* direntry;
 struct stat st;
 int is_dot=!strcmp(entry->d_name,".");
 int is_dotdot=!strcmp(entry->d_name,"..");
@@ -857,7 +856,7 @@ static int read_directory(BDRVVVFATState* s, int 
mapping_index)
 
 /* fill with zeroes up to the end of the cluster */
 while(s->directory.next%(0x10*s->sectors_per_cluster)) {
-direntry_t* direntry=array_get_next(&(s->directory));
+direntry = array_get_next(&(s->directory));
 memset(direntry,0,sizeof(direntry_t));
 }
 
@@ -1962,24 +1961,24 @@ get_cluster_count_for_direntry(BDRVVVFATState* s, 
direntry_t* direntry, const ch
  * This is horribly inefficient, but that is okay, since
  * it is rarely executed, if at all.
  */
-int64_t offset = cluster2sector(s, cluster_num);
+int64_t offs = cluster2sector(s, cluster_num);
 
 vvfat_close_current_file(s);
 for (i = 0; i < s->sectors_per_cluster; i++) {
 int res;
 
 res = bdrv_is_allocated(s->qcow->bs,
-(offset + i) * BDRV_SECTOR_SIZE,
+(offs + i) * BDRV_SECTOR_SIZE,
 BDRV_SECTOR_SIZE, NULL);
 i

[PULL 37/56] test-throttle: don't shadow 'index' variable in do_test_accounting()

2023-09-29 Thread Markus Armbruster
From: Alberto Garcia 

Fixes build with -Wshadow=local

Signed-off-by: Alberto Garcia 
Message-ID: <20230922105742.81317-1-be...@igalia.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 tests/unit/test-throttle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index cb587e33e7..ac35d65d19 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -625,7 +625,7 @@ static bool do_test_accounting(bool is_ops, /* are we 
testing bps or ops */
 throttle_config_init(&cfg);
 
 for (i = 0; i < 3; i++) {
-BucketType index = to_test[is_ops][i];
+index = to_test[is_ops][i];
 cfg.buckets[index].avg = avg;
 }
 
-- 
2.41.0




[PATCH 3/3] replay: fix for loading non-replay snapshots

2023-09-29 Thread pavel . dovgalyuk
From: Pavel Dovgalyuk 

Snapshots created in regular icount execution mode can't be loaded
in recording mode, because icount value advances only by 32-bit value.
This patch initializes replay icount initial value after loading
the snapshot.

Cc: Pizarro Solar Rafael Ulises Luzius 
Signed-off-by: Pavel Dovgalyuk 
---
 replay/replay-snapshot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 10a7cf7992..1e32ada1f6 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -75,6 +75,7 @@ void replay_vmstate_init(void)
 
 if (replay_snapshot) {
 if (replay_mode == REPLAY_MODE_RECORD) {
+replay_state.current_icount = replay_get_current_icount();
 if (!save_snapshot(replay_snapshot,
true, NULL, false, NULL, &err)) {
 error_report_err(err);
-- 
2.34.1




[PATCH v3 11/14] vt82c686: Support machine-default audiodev with fallback

2023-09-29 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/mips/fuloong2e.c | 15 ---
 hw/ppc/pegasos2.c   | 12 ++--
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..c6109633fee 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -295,9 +295,17 @@ static void mips_fuloong2e_init(MachineState *machine)
 pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
 
 /* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
-  PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
-  TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+
+/* Set properties on individual devices before realizing the south bridge 
*/
+if (machine->audiodev) {
+dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ac97"));
+qdev_prop_set_string(dev, "audiodev", machine->audiodev);
+}
+
+pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
+
 object_property_add_alias(OBJECT(machine), "rtc-time",
   object_resolve_path_component(OBJECT(pci_dev),
 "rtc"),
@@ -337,6 +345,7 @@ static void mips_fuloong2e_machine_init(MachineClass *mc)
 mc->default_ram_size = 256 * MiB;
 mc->default_ram_id = "fuloong2e.ram";
 mc->minimum_page_bits = 14;
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..3203a4a7289 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -180,8 +180,15 @@ static void pegasos2_init(MachineState *machine)
 pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
 
 /* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA));
+
+/* Set properties on individual devices before realizing the south bridge 
*/
+if (machine->audiodev) {
+dev = PCI_DEVICE(object_resolve_path_component(via, "ac97"));
+qdev_prop_set_string(DEVICE(dev), "audiodev", machine->audiodev);
+}
+
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
 for (i = 0; i < PCI_NUM_PINS; i++) {
 pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
 }
@@ -556,6 +563,7 @@ static void pegasos2_machine_class_init(ObjectClass *oc, 
void *data)
 mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7457_v1.2");
 mc->default_ram_id = "pegasos2.ram";
 mc->default_ram_size = 512 * MiB;
+machine_add_audiodev_property(mc);
 
 vhc->cpu_in_nested = pegasos2_cpu_in_nested;
 vhc->hypercall = pegasos2_hypercall;
-- 
2.41.0




[PATCH v3 14/14] audio: propagate Error out of audio_driver_init

2023-09-29 Thread Paolo Bonzini
Now that all the callers of audio_init can report failure, pass the Error
from audio_driver_init to audio_init instead of reporting it directly
in audio_driver_init.  This eliminates more complex logic that calls
error_report_err and error_init, replacing it with just &error_fatal
(when creating command line audiodevs) and error propagation
(when creating default audiodevs from AUD_register_card).

Signed-off-by: Paolo Bonzini 
---
 audio/audio.c | 31 ++-
 audio/audio.h |  2 +-
 softmmu/vl.c  |  4 +---
 3 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 7cfcbfb6ef1..380da72a7b0 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1556,7 +1556,7 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, 
size_t size)
 }
 
 static int audio_driver_init(AudioState *s, struct audio_driver *drv,
- bool msg, Audiodev *dev)
+ Audiodev *dev, Error **errp)
 {
 Error *local_err = NULL;
 
@@ -1577,12 +1577,10 @@ static int audio_driver_init(AudioState *s, struct 
audio_driver *drv,
 s->drv = drv;
 return 0;
 } else {
-if (!msg) {
-error_free(local_err);
-} else if (local_err) {
-error_report_err(local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 } else {
-error_report("Could not init `%s' audio driver", drv->name);
+error_setg(errp, "Could not init `%s' audio driver", drv->name);
 }
 return -1;
 }
@@ -1733,7 +1731,7 @@ static void audio_create_default_audiodevs(void)
  * if dev == NULL => legacy implicit initialization, return the already created
  *   state or create a new one
  */
-static AudioState *audio_init(Audiodev *dev)
+static AudioState *audio_init(Audiodev *dev, Error **errp)
 {
 static bool atexit_registered;
 int done = 0;
@@ -1760,9 +1758,9 @@ static AudioState *audio_init(Audiodev *dev)
 drvname = AudiodevDriver_str(dev->driver);
 driver = audio_driver_lookup(drvname);
 if (driver) {
-done = !audio_driver_init(s, driver, true, dev);
+done = !audio_driver_init(s, driver, dev, errp);
 } else {
-dolog ("Unknown audio driver `%s'\n", drvname);
+error_setg(errp, "Unknown audio driver `%s'\n", drvname);
 }
 if (!done) {
 free_audio_state(s);
@@ -1778,7 +1776,7 @@ static AudioState *audio_init(Audiodev *dev)
 s->dev = dev = e->dev;
 drvname = AudiodevDriver_str(dev->driver);
 driver = audio_driver_lookup(drvname);
-if (!audio_driver_init(s, driver, false, dev)) {
+if (!audio_driver_init(s, driver, dev, NULL)) {
 break;
 }
 QSIMPLEQ_REMOVE_HEAD(&default_audiodevs, next);
@@ -1821,7 +1819,10 @@ bool AUD_register_card (const char *name, QEMUSoundCard 
*card, Error **errp)
 if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
 audio_create_default_audiodevs();
 }
-card->state = audio_init(NULL);
+card->state = audio_init(NULL, errp);
+if (!card->state) {
+return false;
+}
 }
 }
 
@@ -2157,17 +2158,13 @@ void audio_define(Audiodev *dev)
 QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
 }
 
-bool audio_init_audiodevs(void)
+void audio_init_audiodevs(void)
 {
 AudiodevListEntry *e;
 
 QSIMPLEQ_FOREACH(e, &audiodevs, next) {
-if (!audio_init(e->dev)) {
-return false;
-}
+audio_init(e->dev, &error_fatal);
 }
-
-return true;
 }
 
 audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
diff --git a/audio/audio.h b/audio/audio.h
index 70b264d897d..80f3f92124d 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -170,7 +170,7 @@ void audio_sample_from_uint64(void *samples, int pos,
 
 void audio_define(Audiodev *audio);
 void audio_parse_option(const char *opt);
-bool audio_init_audiodevs(void);
+void audio_init_audiodevs(void);
 void audio_help(void);
 
 AudioState *audio_state_by_name(const char *name, Error **errp);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index cafb1a98427..98e071e63bb 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1962,9 +1962,7 @@ static void qemu_create_early_backends(void)
  * setting machine properties, so they can be referred to.
  */
 configure_blockdev(&bdo_queue, machine_class, snapshot);
-if (!audio_init_audiodevs()) {
-exit(1);
-}
+audio_init_audiodevs();
 }
 
 
-- 
2.41.0




[PATCH 1/3] replay: improve determinism of virtio-net

2023-09-29 Thread pavel . dovgalyuk
From: Pavel Dovgalyuk 

virtio-net device uses bottom halves for callbacks.
These callbacks should be deterministic, because they affect VM state.
This patch replaces BH invocations with corresponding replay functions,
making them deterministic in record/replay mode.

Signed-off-by: Pavel Dovgalyuk 
---
 hw/net/virtio-net.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 5a0201c423..50156e8002 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -46,6 +46,7 @@
 #include "net_rx_pkt.h"
 #include "hw/virtio/vhost.h"
 #include "sysemu/qtest.h"
+#include "sysemu/replay.h"
 
 #define VIRTIO_NET_VM_VERSION11
 
@@ -417,7 +418,7 @@ static void virtio_net_set_status(struct VirtIODevice 
*vdev, uint8_t status)
 timer_mod(q->tx_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 
n->tx_timeout);
 } else {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
 }
 } else {
 if (q->tx_timer) {
@@ -2822,7 +2823,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, 
VirtQueue *vq)
 return;
 }
 virtio_queue_set_notification(vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
 }
 
 static void virtio_net_tx_timer(void *opaque)
@@ -2905,7 +2906,7 @@ static void virtio_net_tx_bh(void *opaque)
 /* If we flush a full burst of packets, assume there are
  * more coming and immediately reschedule */
 if (ret >= n->tx_burst) {
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
 q->tx_waiting = 1;
 return;
 }
@@ -2919,7 +2920,7 @@ static void virtio_net_tx_bh(void *opaque)
 return;
 } else if (ret > 0) {
 virtio_queue_set_notification(q->tx_vq, 0);
-qemu_bh_schedule(q->tx_bh);
+replay_bh_schedule_event(q->tx_bh);
 q->tx_waiting = 1;
 }
 }
-- 
2.34.1




[PULL 41/56] hw/arm/smmuv3.c: Avoid shadowing variable

2023-09-29 Thread Markus Armbruster
From: Peter Maydell 

Avoid shadowing a variable in smmuv3_notify_iova():

../../hw/arm/smmuv3.c: In function ‘smmuv3_notify_iova’:
../../hw/arm/smmuv3.c:1043:23: warning: declaration of ‘event’ shadows a 
previous local [-Wshadow=local]
 1043 | SMMUEventInfo event = {.inval_ste_allowed = true};
  |   ^
../../hw/arm/smmuv3.c:1038:19: note: shadowed declaration is here
 1038 | IOMMUTLBEvent event;
  |   ^

Signed-off-by: Peter Maydell 
Message-ID: <20230922152944.3583438-4-peter.mayd...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Auger 
Signed-off-by: Markus Armbruster 
---
 hw/arm/smmuv3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 1e9be8e89a..6f2b2bd45f 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1040,8 +1040,8 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
 SMMUv3State *s = sdev->smmu;
 
 if (!tg) {
-SMMUEventInfo event = {.inval_ste_allowed = true};
-SMMUTransCfg *cfg = smmuv3_get_config(sdev, &event);
+SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
+SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
 SMMUTransTableInfo *tt;
 
 if (!cfg) {
-- 
2.41.0




[PULL 22/56] util/vhost-user-server: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  util/vhost-user-server.c: In function ‘set_watch’:
  util/vhost-user-server.c:274:20: warning: declaration of ‘vu_fd_watch’ 
shadows a previous local [-Wshadow=compatible-local]
274 | VuFdWatch *vu_fd_watch = g_new0(VuFdWatch, 1);
|^~~
  util/vhost-user-server.c:271:16: note: shadowed declaration is here
271 | VuFdWatch *vu_fd_watch = find_vu_fd_watch(server, fd);
|^~~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-18-phi...@linaro.org>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Markus Armbruster 
---
 util/vhost-user-server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
index b4b6bf30a2..5ccc6d24a0 100644
--- a/util/vhost-user-server.c
+++ b/util/vhost-user-server.c
@@ -278,7 +278,7 @@ set_watch(VuDev *vu_dev, int fd, int vu_evt,
 VuFdWatch *vu_fd_watch = find_vu_fd_watch(server, fd);
 
 if (!vu_fd_watch) {
-VuFdWatch *vu_fd_watch = g_new0(VuFdWatch, 1);
+vu_fd_watch = g_new0(VuFdWatch, 1);
 
 QTAILQ_INSERT_TAIL(&server->vu_fd_watches, vu_fd_watch, next);
 
-- 
2.41.0




[PULL 04/56] block/dirty-bitmap: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: rename both the pair of parameters and the pair of local
variables.  While there, move the local variables to function scope.

Suggested-by: Kevin Wolf 
Signed-off-by: Markus Armbruster 
Reviewed-by: Kevin Wolf 
Message-ID: <20230921121312.1301864-5-arm...@redhat.com>
---
 block/monitor/bitmap-qmp-cmds.c | 19 ++-
 block/qcow2-bitmap.c|  3 +--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/block/monitor/bitmap-qmp-cmds.c b/block/monitor/bitmap-qmp-cmds.c
index 55f778f5af..70d01a3776 100644
--- a/block/monitor/bitmap-qmp-cmds.c
+++ b/block/monitor/bitmap-qmp-cmds.c
@@ -258,37 +258,38 @@ void qmp_block_dirty_bitmap_disable(const char *node, 
const char *name,
 bdrv_disable_dirty_bitmap(bitmap);
 }
 
-BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target,
+BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *dst_node,
+  const char *dst_bitmap,
   BlockDirtyBitmapOrStrList *bms,
   HBitmap **backup, Error **errp)
 {
 BlockDriverState *bs;
 BdrvDirtyBitmap *dst, *src;
 BlockDirtyBitmapOrStrList *lst;
+const char *src_node, *src_bitmap;
 HBitmap *local_backup = NULL;
 
 GLOBAL_STATE_CODE();
 
-dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
+dst = block_dirty_bitmap_lookup(dst_node, dst_bitmap, &bs, errp);
 if (!dst) {
 return NULL;
 }
 
 for (lst = bms; lst; lst = lst->next) {
 switch (lst->value->type) {
-const char *name, *node;
 case QTYPE_QSTRING:
-name = lst->value->u.local;
-src = bdrv_find_dirty_bitmap(bs, name);
+src_bitmap = lst->value->u.local;
+src = bdrv_find_dirty_bitmap(bs, src_bitmap);
 if (!src) {
-error_setg(errp, "Dirty bitmap '%s' not found", name);
+error_setg(errp, "Dirty bitmap '%s' not found", src_bitmap);
 goto fail;
 }
 break;
 case QTYPE_QDICT:
-node = lst->value->u.external.node;
-name = lst->value->u.external.name;
-src = block_dirty_bitmap_lookup(node, name, NULL, errp);
+src_node = lst->value->u.external.node;
+src_bitmap = lst->value->u.external.name;
+src = block_dirty_bitmap_lookup(src_node, src_bitmap, NULL, errp);
 if (!src) {
 goto fail;
 }
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 037fa2d435..ffd5cd3b23 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1555,7 +1555,6 @@ bool 
qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
 FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
 const char *name = bdrv_dirty_bitmap_name(bitmap);
 uint32_t granularity = bdrv_dirty_bitmap_granularity(bitmap);
-Qcow2Bitmap *bm;
 
 if (!bdrv_dirty_bitmap_get_persistence(bitmap) ||
 bdrv_dirty_bitmap_inconsistent(bitmap)) {
@@ -1625,7 +1624,7 @@ bool 
qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
 
 /* allocate clusters and store bitmaps */
 QSIMPLEQ_FOREACH(bm, bm_list, entry) {
-BdrvDirtyBitmap *bitmap = bm->dirty_bitmap;
+bitmap = bm->dirty_bitmap;
 
 if (bitmap == NULL || bdrv_dirty_bitmap_readonly(bitmap)) {
 continue;
-- 
2.41.0




[PATCH 0/3] Record/replay patches

2023-09-29 Thread pavel . dovgalyuk
From: Pavel Dovgalyuk 

The set of patches include the following modifications:
 - fix for allowing record/replay with virtio-net
 - fix for loading non-replay snapshots

Pavel Dovgalyuk (3):
  replay: improve determinism of virtio-net
  virtio-net: added replay blocker for guest_announce
  replay: fix for loading non-replay snapshots

 hw/net/virtio-net.c  | 13 +
 replay/replay-snapshot.c |  1 +
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.34.1




[PATCH v3 09/14] hw/arm: Support machine-default audiodev with fallback

2023-09-29 Thread Paolo Bonzini
From: Martin Kletzander 

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 hw/arm/integratorcp.c | 11 ++-
 hw/arm/musicpal.c | 11 +--
 hw/arm/nseries.c  |  4 
 hw/arm/omap2.c|  5 +
 hw/arm/palm.c |  2 ++
 hw/arm/realview.c | 12 
 hw/arm/spitz.c| 17 -
 hw/arm/versatilepb.c  |  8 
 hw/arm/vexpress.c |  5 +
 hw/arm/xlnx-zcu102.c  |  6 ++
 hw/arm/z2.c   | 15 ++-
 hw/input/tsc210x.c|  5 +
 12 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index b109ece3ae0..d176e9af7ee 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -27,6 +27,7 @@
 #include "hw/irq.h"
 #include "hw/sd/sd.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 #define TYPE_INTEGRATOR_CM "integrator_core"
 OBJECT_DECLARE_SIMPLE_TYPE(IntegratorCMState, INTEGRATOR_CM)
@@ -660,7 +661,13 @@ static void integratorcp_init(MachineState *machine)
&error_fatal);
 }
 
-sysbus_create_varargs("pl041", 0x1d00, pic[25], NULL);
+dev = qdev_new("pl041");
+if (machine->audiodev) {
+qdev_prop_set_string(dev, "audiodev", machine->audiodev);
+}
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x1d00);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[25]);
 
 if (nd_table[0].used)
 smc91c111_init(&nd_table[0], 0xc800, pic[27]);
@@ -678,6 +685,8 @@ static void integratorcp_machine_init(MachineClass *mc)
 mc->ignore_memory_transaction_failures = true;
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
 mc->default_ram_id = "integrator.ram";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index dc4e43e0ee2..9703bfb97fb 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -37,9 +37,9 @@
 #include "qemu/cutils.h"
 #include "qom/object.h"
 #include "hw/net/mv88w8618_eth.h"
+#include "audio/audio.h"
 #include "qemu/error-report.h"
 
-
 #define MP_MISC_BASE0x80002000
 #define MP_MISC_SIZE0x1000
 
@@ -1326,7 +1326,12 @@ static void musicpal_init(MachineState *machine)
 qdev_connect_gpio_out(key_dev, i, qdev_get_gpio_in(dev, i + 15));
 }
 
-wm8750_dev = i2c_slave_create_simple(i2c, TYPE_WM8750, MP_WM_ADDR);
+wm8750_dev = i2c_slave_new(TYPE_WM8750, MP_WM_ADDR);
+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(wm8750_dev), "audiodev", 
machine->audiodev);
+}
+i2c_slave_realize_and_unref(wm8750_dev, i2c, &error_abort);
+
 dev = qdev_new(TYPE_MV88W8618_AUDIO);
 s = SYS_BUS_DEVICE(dev);
 object_property_set_link(OBJECT(dev), "wm8750", OBJECT(wm8750_dev),
@@ -1347,6 +1352,8 @@ static void musicpal_machine_init(MachineClass *mc)
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
 mc->default_ram_size = MP_RAM_DEFAULT_SIZE;
 mc->default_ram_id = "musicpal.ram";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("musicpal", musicpal_machine_init)
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 9e49e9e1776..35aff46b4b4 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1432,6 +1432,8 @@ static void n800_class_init(ObjectClass *oc, void *data)
 /* Actually two chips of 0x400 bytes each */
 mc->default_ram_size = 0x0800;
 mc->default_ram_id = "omap2.dram";
+
+machine_add_audiodev_property(mc);
 }
 
 static const TypeInfo n800_type = {
@@ -1452,6 +1454,8 @@ static void n810_class_init(ObjectClass *oc, void *data)
 /* Actually two chips of 0x400 bytes each */
 mc->default_ram_size = 0x0800;
 mc->default_ram_id = "omap2.dram";
+
+machine_add_audiodev_property(mc);
 }
 
 static const TypeInfo n810_type = {
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index d5a2ae7af6e..41b1f596dca 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -37,6 +37,7 @@
 #include "hw/block/flash.h"
 #include "hw/arm/soc_dma.h"
 #include "hw/sysbus.h"
+#include "hw/boards.h"
 #include "audio/audio.h"
 
 /* Enhanced Audio Controller (CODEC only) */
@@ -609,6 +610,10 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
 s->codec.txdrq = *drq;
 omap_eac_reset(s);
 
+if (current_machine->audiodev) {
+s->codec.card.name = g_strdup(current_machine->audiodev);
+s->codec.card.state = audio_state_by_name(s->codec.card.name, 
&error_fatal);
+}
 AUD_register_card("OMAP EAC", &s->codec.card);
 
 memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 17c11ac4cec..b86f2c331bb 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -310,6 +310,8 @@ static void palmte_machine_init(MachineClass *mc)
 mc->default_cpu_type = ARM_CPU_TYPE_NAM

[PULL 21/56] crypto/cipher-gnutls.c: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  In file included from crypto/cipher.c:140:
  crypto/cipher-gnutls.c.inc: In function ‘qcrypto_gnutls_cipher_encrypt’:
  crypto/cipher-gnutls.c.inc:116:17: warning: declaration of ‘err’ shadows a 
previous local [-Wshadow=compatible-local]
116 | int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, 
NULL);
| ^~~
  crypto/cipher-gnutls.c.inc:94:9: note: shadowed declaration is here
 94 | int err;
| ^~~
   ---

  crypto/cipher-gnutls.c.inc: In function ‘qcrypto_gnutls_cipher_decrypt’:
  crypto/cipher-gnutls.c.inc:177:17: warning: declaration of ‘err’ shadows a 
previous local [-Wshadow=compatible-local]
177 | int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, 
NULL);
| ^~~
  crypto/cipher-gnutls.c.inc:154:9: note: shadowed declaration is here
154 | int err;
| ^~~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-17-phi...@linaro.org>
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Markus Armbruster 
---
 crypto/cipher-gnutls.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/cipher-gnutls.c.inc b/crypto/cipher-gnutls.c.inc
index 501e4e07a5..d3e231c13c 100644
--- a/crypto/cipher-gnutls.c.inc
+++ b/crypto/cipher-gnutls.c.inc
@@ -113,7 +113,7 @@ qcrypto_gnutls_cipher_encrypt(QCryptoCipher *cipher,
 while (len) {
 gnutls_cipher_hd_t handle;
 gnutls_datum_t gkey = { (unsigned char *)ctx->key, ctx->nkey };
-int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
+err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
 if (err != 0) {
 error_setg(errp, "Cannot initialize cipher: %s",
gnutls_strerror(err));
@@ -174,7 +174,7 @@ qcrypto_gnutls_cipher_decrypt(QCryptoCipher *cipher,
 while (len) {
 gnutls_cipher_hd_t handle;
 gnutls_datum_t gkey = { (unsigned char *)ctx->key, ctx->nkey };
-int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
+err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
 if (err != 0) {
 error_setg(errp, "Cannot initialize cipher: %s",
gnutls_strerror(err));
-- 
2.41.0




[PULL 01/56] migration/rdma: Fix save_page method to fail on polling error

2023-09-29 Thread Markus Armbruster
qemu_rdma_save_page() reports polling error with error_report(), then
succeeds anyway.  This is because the variable holding the polling
status *shadows* the variable the function returns.  The latter
remains zero.

Broken since day one, and duplicated more recently.

Fixes: 2da776db4846 (rdma: core logic)
Fixes: b390afd8c50b (migration/rdma: Fix out of order wrid)
Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Peter Xu 
Reviewed-by: Li Zhijian 
Message-ID: <20230921121312.1301864-2-arm...@redhat.com>
---
 migration/rdma.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index a2a3db35b1..3915d1d7c9 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3282,7 +3282,8 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
  */
 while (1) {
 uint64_t wr_id, wr_id_in;
-int ret = qemu_rdma_poll(rdma, rdma->recv_cq, &wr_id_in, NULL);
+ret = qemu_rdma_poll(rdma, rdma->recv_cq, &wr_id_in, NULL);
+
 if (ret < 0) {
 error_report("rdma migration: polling error! %d", ret);
 goto err;
@@ -3297,7 +3298,8 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
 
 while (1) {
 uint64_t wr_id, wr_id_in;
-int ret = qemu_rdma_poll(rdma, rdma->send_cq, &wr_id_in, NULL);
+ret = qemu_rdma_poll(rdma, rdma->send_cq, &wr_id_in, NULL);
+
 if (ret < 0) {
 error_report("rdma migration: polling error! %d", ret);
 goto err;
-- 
2.41.0




[PULL 36/56] spapr/drc: Clean up local variable shadowing in prop_get_fdt()

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Rename 'name' variable to avoid this warning :

  ../hw/ppc/spapr_drc.c: In function ‘prop_get_fdt’:
  ../hw/ppc/spapr_drc.c:344:21: warning: declaration of ‘name’ shadows a 
parameter [-Wshadow=compatible-local]
344 | const char *name = NULL;
| ^~~~
  ../hw/ppc/spapr_drc.c:325:63: note: shadowed declaration is here
325 | static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
|   ^~~~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-9-...@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/spapr_drc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 843e318312..2b99d3b4b1 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -341,7 +341,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const 
char *name,
 fdt_depth = 0;
 
 do {
-const char *name = NULL;
+const char *dt_name = NULL;
 const struct fdt_property *prop = NULL;
 int prop_len = 0, name_len = 0;
 uint32_t tag;
@@ -351,8 +351,8 @@ static void prop_get_fdt(Object *obj, Visitor *v, const 
char *name,
 switch (tag) {
 case FDT_BEGIN_NODE:
 fdt_depth++;
-name = fdt_get_name(fdt, fdt_offset, &name_len);
-if (!visit_start_struct(v, name, NULL, 0, errp)) {
+dt_name = fdt_get_name(fdt, fdt_offset, &name_len);
+if (!visit_start_struct(v, dt_name, NULL, 0, errp)) {
 return;
 }
 break;
@@ -369,8 +369,8 @@ static void prop_get_fdt(Object *obj, Visitor *v, const 
char *name,
 case FDT_PROP: {
 int i;
 prop = fdt_get_property_by_offset(fdt, fdt_offset, &prop_len);
-name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
-if (!visit_start_list(v, name, NULL, 0, errp)) {
+dt_name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
+if (!visit_start_list(v, dt_name, NULL, 0, errp)) {
 return;
 }
 for (i = 0; i < prop_len; i++) {
-- 
2.41.0




[PULL 29/56] hw/ppc: Clean up local variable shadowing in _FDT helper routine

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

this fixes numerous warnings of this type :

  In file included from ../hw/ppc/spapr_pci.c:43:
  ../hw/ppc/spapr_pci.c: In function ‘spapr_dt_phb’:
  ../include/hw/ppc/fdt.h:18:13: warning: declaration of ‘ret’ shadows a 
previous local [-Wshadow=compatible-local]
 18 | int ret = (exp);   \
| ^~~
  ../hw/ppc/spapr_pci.c:2355:5: note: in expansion of macro ‘_FDT’
   2355 | _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
| ^~~~
  ../hw/ppc/spapr_pci.c:2311:24: note: shadowed declaration is here
   2311 | int bus_off, i, j, ret;
|^~~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-2-...@kaod.org>
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 include/hw/ppc/fdt.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/ppc/fdt.h b/include/hw/ppc/fdt.h
index a8cd85069f..b56ac2a8cb 100644
--- a/include/hw/ppc/fdt.h
+++ b/include/hw/ppc/fdt.h
@@ -15,10 +15,10 @@
 
 #define _FDT(exp)  \
 do {   \
-int ret = (exp);   \
-if (ret < 0) { \
-error_report("error creating device tree: %s: %s",   \
-#exp, fdt_strerror(ret));  \
+int _ret = (exp);  \
+if (_ret < 0) {\
+error_report("error creating device tree: %s: %s", \
+#exp, fdt_strerror(_ret)); \
 exit(1);   \
 }  \
 } while (0)
-- 
2.41.0




[PULL 44/56] aspeed: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Remove superfluous local 'irq' variables and use the one define at the
top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
such as :

  ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
  ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a 
previous local [-Wshadow=compatible-local]
420 | qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
|  ^~~
  ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
312 | qemu_irq irq;
|  ^~~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230922155924.1172019-3-...@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
---
 hw/arm/aspeed_ast2600.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a..e122e1c32d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -388,7 +388,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->timerctrl), 0,
 sc->memmap[ASPEED_DEV_TIMER1]);
 for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
-qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
+irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
 }
 
@@ -413,8 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 }
 aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i2c), 0, sc->memmap[ASPEED_DEV_I2C]);
 for (i = 0; i < ASPEED_I2C_GET_CLASS(&s->i2c)->num_busses; i++) {
-qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
-sc->irqmap[ASPEED_DEV_I2C] + i);
+irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+   sc->irqmap[ASPEED_DEV_I2C] + i);
 /* The AST2600 I2C controller has one IRQ per bus. */
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c.busses[i]), 0, irq);
 }
@@ -611,8 +611,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 }
 aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
 for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
-qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
-sc->irqmap[ASPEED_DEV_I3C] + i);
+irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+   sc->irqmap[ASPEED_DEV_I3C] + i);
 /* The AST2600 I3C controller has one IRQ per bus. */
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->i3c.devices[i]), 0, irq);
 }
-- 
2.41.0




[PATCH v3 07/14] audio: remove QEMU_AUDIO_* and -audio-help support

2023-09-29 Thread Paolo Bonzini
These have been deprecated for a long time, and the introduction of
-audio in 7.1.0 has cemented the new way of specifying an audio backend's
parameters.  However, there is still a need for simple configuration
of the audio backend in the desktop case; therefore, if no audiodev is
passed to audio_init(), go through a bunch of simple Audiodev* structures
and pick the first that can be initialized successfully.

The only QEMU_AUDIO_* option that is left in, waiting for a better idea,
is QEMU_AUDIO_DRV=none which is used by qtest.

Remove all the parsing code, including the concept of "can_be_default"
audio drivers: now that audio_prio_list[] is only used in a single place,
wav can be excluded directly in that function.

Signed-off-by: Paolo Bonzini 
---
 audio/alsaaudio.c   |   1 -
 audio/audio.c   | 131 +++
 audio/audio.h   |   1 -
 audio/audio_int.h   |   5 -
 audio/audio_legacy.c| 591 
 audio/coreaudio.m   |   1 -
 audio/dbusaudio.c   |   1 -
 audio/dsoundaudio.c |   1 -
 audio/jackaudio.c   |   1 -
 audio/meson.build   |   1 -
 audio/noaudio.c |   1 -
 audio/ossaudio.c|   1 -
 audio/paaudio.c |   1 -
 audio/pwaudio.c |   1 -
 audio/sdlaudio.c|   1 -
 audio/sndioaudio.c  |   1 -
 audio/wavaudio.c|   1 -
 docs/about/deprecated.rst   |   8 -
 docs/about/removed-features.rst |   6 +
 qemu-options.hx |  10 -
 softmmu/vl.c|   4 -
 21 files changed, 60 insertions(+), 709 deletions(-)
 delete mode 100644 audio/audio_legacy.c

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 6fb78e5b972..cacae1ea59c 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -960,7 +960,6 @@ static struct audio_driver alsa_audio_driver = {
 .init   = alsa_audio_init,
 .fini   = alsa_audio_fini,
 .pcm_ops= &alsa_pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof (ALSAVoiceOut),
diff --git a/audio/audio.c b/audio/audio.c
index e3ca5fe65ec..9da2eaece03 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -32,6 +32,7 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-visit-audio.h"
 #include "qapi/qapi-commands-audio.h"
+#include "qapi/qmp/qdict.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
@@ -62,19 +63,22 @@ const char *audio_prio_list[] = {
 "spice",
 CONFIG_AUDIO_DRIVERS
 "none",
-"wav",
 NULL
 };
 
 static QLIST_HEAD(, audio_driver) audio_drivers;
-static AudiodevListHead audiodevs = QSIMPLEQ_HEAD_INITIALIZER(audiodevs);
+static AudiodevListHead audiodevs =
+QSIMPLEQ_HEAD_INITIALIZER(audiodevs);
+static AudiodevListHead default_audiodevs =
+QSIMPLEQ_HEAD_INITIALIZER(default_audiodevs);
+
 
 void audio_driver_register(audio_driver *drv)
 {
 QLIST_INSERT_HEAD(&audio_drivers, drv, next);
 }
 
-audio_driver *audio_driver_lookup(const char *name)
+static audio_driver *audio_driver_lookup(const char *name)
 {
 struct audio_driver *d;
 Error *local_err = NULL;
@@ -112,8 +116,6 @@ const struct mixeng_volume nominal_volume = {
 #endif
 };
 
-static bool legacy_config = true;
-
 int audio_bug (const char *funcname, int cond)
 {
 if (cond) {
@@ -1688,17 +1690,41 @@ static const VMStateDescription vmstate_audio = {
 
 static void audio_validate_opts(Audiodev *dev, Error **errp);
 
-static AudiodevListEntry *audiodev_find(
-AudiodevListHead *head, const char *drvname)
+static void audio_create_default_audiodevs(void)
 {
-AudiodevListEntry *e;
-QSIMPLEQ_FOREACH(e, head, next) {
-if (strcmp(AudiodevDriver_str(e->dev->driver), drvname) == 0) {
-return e;
-}
+const char *drvname = getenv("QEMU_AUDIO_DRV");
+
+/* QEMU_AUDIO_DRV=none is used by libqtest.  */
+if (drvname && !g_str_equal(drvname, "none")) {
+error_report("Please use -audiodev instead of QEMU_AUDIO_*");
+exit(1);
 }
 
-return NULL;
+for (int i = 0; audio_prio_list[i]; i++) {
+if (drvname && !g_str_equal(drvname, audio_prio_list[i])) {
+continue;
+}
+
+if (audio_driver_lookup(audio_prio_list[i])) {
+QDict *dict = qdict_new();
+Audiodev *dev = NULL;
+AudiodevListEntry *e;
+Visitor *v;
+
+qdict_put_str(dict, "driver", audio_prio_list[i]);
+qdict_put_str(dict, "id", "#default");
+
+v = qobject_input_visitor_new_keyval(QOBJECT(dict));
+qobject_unref(dict);
+visit_type_Audiodev(v, NULL, &dev, &error_fatal);
+visit_free(v);
+
+audio_validate_opts(dev, &error_abort);
+e = g_new0(AudiodevListEntry, 1);
+e->dev

[PATCH v3 10/14] hw/ppc: Support machine-default audiodev with fallback

2023-09-29 Thread Paolo Bonzini
From: Martin Kletzander 

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 hw/ppc/prep.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index f6fd35fcb9e..137276bcb92 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -45,6 +45,7 @@
 #include "trace.h"
 #include "elf.h"
 #include "qemu/units.h"
+#include "audio/audio.h"
 
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
@@ -310,6 +311,10 @@ static void ibm_40p_init(MachineState *machine)
 dev = DEVICE(isa_dev);
 qdev_prop_set_uint32(dev, "iobase", 0x830);
 qdev_prop_set_uint32(dev, "irq", 10);
+
+if (machine->audiodev) {
+qdev_prop_set_string(dev, "audiodev", machine->audiodev);
+}
 isa_realize_and_unref(isa_dev, isa_bus, &error_fatal);
 
 isa_dev = isa_new("pc87312");
@@ -426,6 +431,8 @@ static void ibm_40p_machine_init(MachineClass *mc)
 mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("604");
 mc->default_display = "std";
 mc->default_nic = "pcnet";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("40p", ibm_40p_machine_init)
-- 
2.41.0




[PULL 39/56] hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()

2023-09-29 Thread Markus Armbruster
From: Peter Maydell 

Avoid shadowing a local variable in do_process_its_cmd():

../../hw/intc/arm_gicv3_its.c:548:17: warning: declaration of ‘ite’ shadows a 
previous local [-Wshadow=compatible-local]
  548 | ITEntry ite = {};
  | ^~~
../../hw/intc/arm_gicv3_its.c:518:13: note: shadowed declaration is here
  518 | ITEntry ite;
  | ^~~

Signed-off-by: Peter Maydell 
Message-ID: <20230922152944.3583438-2-peter.mayd...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Auger 
Signed-off-by: Markus Armbruster 
---
 hw/intc/arm_gicv3_its.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 5f552b4d37..52e9aca9c6 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -545,10 +545,10 @@ static ItsCmdResult do_process_its_cmd(GICv3ITSState *s, 
uint32_t devid,
 }
 
 if (cmdres == CMD_CONTINUE_OK && cmd == DISCARD) {
-ITEntry ite = {};
+ITEntry i = {};
 /* remove mapping from interrupt translation table */
-ite.valid = false;
-return update_ite(s, eventid, &dte, &ite) ? CMD_CONTINUE_OK : 
CMD_STALL;
+i.valid = false;
+return update_ite(s, eventid, &dte, &i) ? CMD_CONTINUE_OK : CMD_STALL;
 }
 return CMD_CONTINUE_OK;
 }
-- 
2.41.0




[PULL 19/56] hw/nios2: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/nios2/10m50_devboard.c: In function ‘nios2_10m50_ghrd_init’:
  hw/nios2/10m50_devboard.c:101:22: warning: declaration of ‘dev’ shadows a 
previous local [-Wshadow=compatible-local]
101 | DeviceState *dev = qdev_new(TYPE_NIOS2_VIC);
|  ^~~
  hw/nios2/10m50_devboard.c:60:18: note: shadowed declaration is here
 60 | DeviceState *dev;
|  ^~~

  hw/nios2/10m50_devboard.c:110:18: warning: declaration of ‘i’ shadows a 
previous local [-Wshadow=compatible-local]
110 | for (int i = 0; i < 32; i++) {
|  ^
  hw/nios2/10m50_devboard.c:67:9: note: shadowed declaration is here
 67 | int i;
| ^

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-15-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 hw/nios2/10m50_devboard.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c
index 91383fb097..952a0dc33e 100644
--- a/hw/nios2/10m50_devboard.c
+++ b/hw/nios2/10m50_devboard.c
@@ -98,7 +98,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
 qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal);
 
 if (nms->vic) {
-DeviceState *dev = qdev_new(TYPE_NIOS2_VIC);
+dev = qdev_new(TYPE_NIOS2_VIC);
 MemoryRegion *dev_mr;
 qemu_irq cpu_irq;
 
@@ -107,7 +107,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
 
 cpu_irq = qdev_get_gpio_in_named(DEVICE(cpu), "EIC", 0);
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irq);
-for (int i = 0; i < 32; i++) {
+for (i = 0; i < 32; i++) {
 irq[i] = qdev_get_gpio_in(dev, i);
 }
 
-- 
2.41.0




[PULL 26/56] softmmu/physmem: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  softmmu/physmem.c: In function ‘cpu_physical_memory_snapshot_and_clear_dirty’:
  softmmu/physmem.c:916:27: warning: declaration of ‘offset’ shadows a 
parameter [-Wshadow=compatible-local]
916 | unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
|   ^~
  softmmu/physmem.c:892:31: note: shadowed declaration is here
892 | (MemoryRegion *mr, hwaddr offset, hwaddr length, unsigned client)
|~~~^~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-23-phi...@linaro.org>
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Peter Xu 
Signed-off-by: Markus Armbruster 
---
 softmmu/physmem.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 4f6ca653b3..309653c722 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -913,16 +913,16 @@ DirtyBitmapSnapshot 
*cpu_physical_memory_snapshot_and_clear_dirty
 
 while (page < end) {
 unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE;
-unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
+unsigned long ofs = page % DIRTY_MEMORY_BLOCK_SIZE;
 unsigned long num = MIN(end - page,
-DIRTY_MEMORY_BLOCK_SIZE - offset);
+DIRTY_MEMORY_BLOCK_SIZE - ofs);
 
-assert(QEMU_IS_ALIGNED(offset, (1 << BITS_PER_LEVEL)));
+assert(QEMU_IS_ALIGNED(ofs, (1 << BITS_PER_LEVEL)));
 assert(QEMU_IS_ALIGNED(num,(1 << BITS_PER_LEVEL)));
-offset >>= BITS_PER_LEVEL;
+ofs >>= BITS_PER_LEVEL;
 
 bitmap_copy_and_clear_atomic(snap->dirty + dest,
- blocks->blocks[idx] + offset,
+ blocks->blocks[idx] + ofs,
  num);
 page += num;
 dest += num >> BITS_PER_LEVEL;
-- 
2.41.0




[PULL 45/56] aspeed/i3c: Rename variable shadowing a local

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

to fix warning :

  ../hw/i3c/aspeed_i3c.c: In function ‘aspeed_i3c_realize’:
  ../hw/i3c/aspeed_i3c.c:1959:17: warning: declaration of ‘dev’ shadows a 
parameter [-Wshadow=local]
   1959 | Object *dev = OBJECT(&s->devices[i]);
| ^~~
  ../hw/i3c/aspeed_i3c.c:1942:45: note: shadowed declaration is here
   1942 | static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
|~^~~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230922155924.1172019-4-...@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Andrew Jeffery 
Signed-off-by: Markus Armbruster 
---
 hw/misc/aspeed_i3c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
index f54f5da522..d1ff617671 100644
--- a/hw/misc/aspeed_i3c.c
+++ b/hw/misc/aspeed_i3c.c
@@ -296,13 +296,13 @@ static void aspeed_i3c_realize(DeviceState *dev, Error 
**errp)
 memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
 
 for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) {
-Object *dev = OBJECT(&s->devices[i]);
+Object *i3c_dev = OBJECT(&s->devices[i]);
 
-if (!object_property_set_uint(dev, "device-id", i, errp)) {
+if (!object_property_set_uint(i3c_dev, "device-id", i, errp)) {
 return;
 }
 
-if (!sysbus_realize(SYS_BUS_DEVICE(dev), errp)) {
+if (!sysbus_realize(SYS_BUS_DEVICE(i3c_dev), errp)) {
 return;
 }
 
-- 
2.41.0




[PULL 02/56] migration: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster 
Reviewed-by: Peter Xu 
Reviewed-by: Li Zhijian 
Message-ID: <20230921121312.1301864-3-arm...@redhat.com>
---
 migration/block.c   | 4 ++--
 migration/ram.c | 8 +++-
 migration/rdma.c| 8 +---
 migration/vmstate.c | 2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 86c2256a2b..eb6aafeb9e 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -440,8 +440,8 @@ static int init_blk_migration(QEMUFile *f)
 /* Can only insert new BDSes now because doing so while iterating block
  * devices may end up in a deadlock (iterating the new BDSes, too). */
 for (i = 0; i < num_bs; i++) {
-BlkMigDevState *bmds = bmds_bs[i].bmds;
-BlockDriverState *bs = bmds_bs[i].bs;
+bmds = bmds_bs[i].bmds;
+bs = bmds_bs[i].bs;
 
 if (bmds) {
 ret = blk_insert_bs(bmds->blk, bs, &local_err);
diff --git a/migration/ram.c b/migration/ram.c
index 9040d66e61..0c202f8109 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3517,8 +3517,6 @@ int colo_init_ram_cache(void)
 * we use the same name 'ram_bitmap' as for migration.
 */
 if (ram_bytes_total()) {
-RAMBlock *block;
-
 RAMBLOCK_FOREACH_NOT_IGNORED(block) {
 unsigned long pages = block->max_length >> TARGET_PAGE_BITS;
 block->bmap = bitmap_new(pages);
@@ -3998,12 +3996,12 @@ static int ram_load_precopy(QEMUFile *f)
 }
 }
 if (migrate_ignore_shared()) {
-hwaddr addr = qemu_get_be64(f);
+hwaddr addr2 = qemu_get_be64(f);
 if (migrate_ram_is_ignored(block) &&
-block->mr->addr != addr) {
+block->mr->addr != addr2) {
 error_report("Mismatched GPAs for block %s "
  "%" PRId64 "!= %" PRId64,
- id, (uint64_t)addr,
+ id, (uint64_t)addr2,
  (uint64_t)block->mr->addr);
 ret = -EINVAL;
 }
diff --git a/migration/rdma.c b/migration/rdma.c
index 3915d1d7c9..c78ddfcb74 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1902,9 +1902,11 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, 
RDMAControlHeader *head,
  * by waiting for a READY message.
  */
 if (rdma->control_ready_expected) {
-RDMAControlHeader resp;
-ret = qemu_rdma_exchange_get_response(rdma,
-&resp, RDMA_CONTROL_READY, 
RDMA_WRID_READY);
+RDMAControlHeader resp_ignored;
+
+ret = qemu_rdma_exchange_get_response(rdma, &resp_ignored,
+  RDMA_CONTROL_READY,
+  RDMA_WRID_READY);
 if (ret < 0) {
 return ret;
 }
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 31842c3afb..438ea77cfa 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -97,7 +97,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription 
*vmsd,
 return -EINVAL;
 }
 if (vmsd->pre_load) {
-int ret = vmsd->pre_load(opaque);
+ret = vmsd->pre_load(opaque);
 if (ret) {
 return ret;
 }
-- 
2.41.0




[PULL 20/56] net/eth: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  net/eth.c:435:20: error: declaration shadows a local variable 
[-Werror,-Wshadow]
size_t input_size = iov_size(pkt, pkt_frags);
   ^
  net/eth.c:413:16: note: previous declaration is here
size_t input_size = iov_size(pkt, pkt_frags);
   ^

Suggested-by: Akihiko Odaki 
Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-16-phi...@linaro.org>
Reviewed-by: Eric Blake 
Reviewed-by: Akihiko Odaki 
Signed-off-by: Markus Armbruster 
---
 net/eth.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 649e66bb1f..3f680cc033 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -432,8 +432,6 @@ _eth_get_rss_ex_src_addr(const struct iovec *pkt, int 
pkt_frags,
 }
 
 if (opthdr.type == IP6_OPT_HOME) {
-size_t input_size = iov_size(pkt, pkt_frags);
-
 if (input_size < opt_offset + sizeof(opthdr)) {
 return false;
 }
-- 
2.41.0




[PULL 10/56] target/arm/hvf: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Per Peter Maydell analysis [*]:

  The hvf_vcpu_exec() function is not documented, but in practice
  its caller expects it to return either EXCP_DEBUG (for "this was
  a guest debug exception you need to deal with") or something else
  (presumably the intention being 0 for OK).

  The hvf_sysreg_read() and hvf_sysreg_write() functions are also not
  documented, but they return 0 on success, or 1 for a completely
  unrecognized sysreg where we've raised the UNDEF exception (but
  not if we raised an UNDEF exception for an unrecognized GIC sysreg --
  I think this is a bug). We use this return value to decide whether
  we need to advance the PC past the insn or not. It's not the same
  as the return value we want to return from hvf_vcpu_exec().

  Retain the variable as locally scoped but give it a name that
  doesn't clash with the other function-scoped variable.

This fixes:

  target/arm/hvf/hvf.c:1936:13: error: declaration shadows a local variable 
[-Werror,-Wshadow]
int ret = 0;
^
  target/arm/hvf/hvf.c:1807:9: note: previous declaration is here
int ret;
^
[*] 
https://lore.kernel.org/qemu-devel/cafeaca_e+fu6jkts+w63wr9ccj6btu_ht_ydzwowc0kbkdy...@mail.gmail.com/

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-4-phi...@linaro.org>
Reviewed-by: Peter Maydell 
Signed-off-by: Markus Armbruster 
---
 target/arm/hvf/hvf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 546c0e817f..757e13b0f9 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1934,16 +1934,16 @@ int hvf_vcpu_exec(CPUState *cpu)
 uint32_t rt = (syndrome >> 5) & 0x1f;
 uint32_t reg = syndrome & SYSREG_MASK;
 uint64_t val;
-int ret = 0;
+int sysreg_ret = 0;
 
 if (isread) {
-ret = hvf_sysreg_read(cpu, reg, rt);
+sysreg_ret = hvf_sysreg_read(cpu, reg, rt);
 } else {
 val = hvf_get_reg(cpu, rt);
-ret = hvf_sysreg_write(cpu, reg, val);
+sysreg_ret = hvf_sysreg_write(cpu, reg, val);
 }
 
-advance_pc = !ret;
+advance_pc = !sysreg_ret;
 break;
 }
 case EC_WFX_TRAP:
-- 
2.41.0




[PATCH v3 12/14] audio: forbid mixing default audiodev backend and -audiodev

2023-09-29 Thread Paolo Bonzini
Now that all callers support setting an audiodev, forbid using the default
audiodev if any audiodev is defined on the command line.  To make the
detection easier make AUD_register_card() return false on error.

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 audio/audio.c| 16 ++--
 audio/audio.h|  2 +-
 hw/arm/omap2.c   |  2 +-
 hw/audio/ac97.c  |  6 +-
 hw/audio/adlib.c |  6 --
 hw/audio/cs4231a.c   |  6 --
 hw/audio/es1370.c|  5 -
 hw/audio/gus.c   |  6 --
 hw/audio/hda-codec.c |  5 -
 hw/audio/lm4549.c|  8 +---
 hw/audio/pcspk.c |  4 +---
 hw/audio/sb16.c  |  6 --
 hw/audio/via-ac97.c  |  6 --
 hw/audio/wm8750.c|  5 -
 hw/display/xlnx_dp.c |  6 --
 hw/input/tsc210x.c   |  2 +-
 hw/usb/dev-audio.c   |  5 -
 17 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 9da2eaece03..f0788345bf8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1803,15 +1803,17 @@ static AudioState *audio_init(Audiodev *dev)
 return s;
 }
 
-void AUD_register_card (const char *name, QEMUSoundCard *card)
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
 {
 if (!card->state) {
+if (!QSIMPLEQ_EMPTY(&audiodevs)) {
+error_setg(errp, "No audiodev specified for %s", name);
+error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?",
+  QSIMPLEQ_FIRST(&audiodevs)->dev->id);
+return false;
+}
+
 if (!QTAILQ_EMPTY(&audio_states)) {
-if (!legacy_config) {
-dolog("Device %s: audiodev default parameter is deprecated, 
please "
-  "specify audiodev=%s\n", name,
-  QTAILQ_FIRST(&audio_states)->dev->id);
-}
 card->state = QTAILQ_FIRST(&audio_states);
 } else {
 if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
@@ -1824,6 +1826,8 @@ void AUD_register_card (const char *name, QEMUSoundCard 
*card)
 card->name = g_strdup (name);
 memset (&card->entries, 0, sizeof (card->entries));
 QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
+
+return true;
 }
 
 void AUD_remove_card (QEMUSoundCard *card)
diff --git a/audio/audio.h b/audio/audio.h
index 34df8962a66..70b264d897d 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp {
 void AUD_vlog (const char *cap, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 
0);
 void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 
-void AUD_register_card (const char *name, QEMUSoundCard *card);
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp);
 void AUD_remove_card (QEMUSoundCard *card);
 CaptureVoiceOut *AUD_add_capture(
 AudioState *s,
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 41b1f596dca..f170728e7ec 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -614,7 +614,7 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
 s->codec.card.name = g_strdup(current_machine->audiodev);
 s->codec.card.state = audio_state_by_name(s->codec.card.name, 
&error_fatal);
 }
-AUD_register_card("OMAP EAC", &s->codec.card);
+AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal);
 
 memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
   omap_l4_region_size(ta, 0));
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index c2a5ce062a1..6a7a2dc80c4 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1273,6 +1273,10 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
 AC97LinkState *s = AC97(dev);
 uint8_t *c = s->dev.config;
 
+if (!AUD_register_card ("ac97", &s->card, errp)) {
+return;
+}
+
 /* TODO: no need to override */
 c[PCI_COMMAND] = 0x00;  /* pcicmd pci command rw, ro */
 c[PCI_COMMAND + 1] = 0x00;
@@ -1306,7 +1310,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
   "ac97-nabm", 256);
 pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
 pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
-AUD_register_card("ac97", &s->card);
+
 ac97_on_reset(DEVICE(s));
 }
 
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 5f979b1487d..bd73806d83a 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -255,6 +255,10 @@ static void adlib_realizefn (DeviceState *dev, Error 
**errp)
 AdlibState *s = ADLIB(dev);
 struct audsettings as;
 
+if (!AUD_register_card ("adlib", &s->card, errp)) {
+return;
+}
+
 s->opl = OPLCreate (3579545, s->freq);
 if (!s->opl) {
 error_setg (errp, "OPLCreate %d failed", s->freq);
@@ -270,8 +274,6 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
 as.fmt = AUDIO_FORMAT_S16;
 as.endianness = A

[PULL 16/56] hw/arm/allwinner: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/arm/allwinner-r40.c:412:14: error: declaration shadows a local variable 
[-Werror,-Wshadow]
for (int i = 0; i < AW_R40_NUM_MMCS; i++) {
 ^
  hw/arm/allwinner-r40.c:299:14: note: previous declaration is here
unsigned i;
 ^

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 
Message-ID: <20230904161235.84651-10-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 hw/arm/allwinner-r40.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
index 7d29eb224f..a0d367c60d 100644
--- a/hw/arm/allwinner-r40.c
+++ b/hw/arm/allwinner-r40.c
@@ -296,10 +296,9 @@ static void allwinner_r40_realize(DeviceState *dev, Error 
**errp)
 {
 const char *r40_nic_models[] = { "gmac", "emac", NULL };
 AwR40State *s = AW_R40(dev);
-unsigned i;
 
 /* CPUs */
-for (i = 0; i < AW_R40_NUM_CPUS; i++) {
+for (unsigned i = 0; i < AW_R40_NUM_CPUS; i++) {
 
 /*
  * Disable secondary CPUs. Guest EL3 firmware will start
@@ -335,7 +334,7 @@ static void allwinner_r40_realize(DeviceState *dev, Error 
**errp)
  * maintenance interrupt signal to the appropriate GIC PPI inputs,
  * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
  */
-for (i = 0; i < AW_R40_NUM_CPUS; i++) {
+for (unsigned i = 0; i < AW_R40_NUM_CPUS; i++) {
 DeviceState *cpudev = DEVICE(&s->cpus[i]);
 int ppibase = AW_R40_GIC_NUM_SPI + i * GIC_INTERNAL + GIC_NR_SGIS;
 int irq;
@@ -494,7 +493,7 @@ static void allwinner_r40_realize(DeviceState *dev, Error 
**errp)
qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_EMAC));
 
 /* Unimplemented devices */
-for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
+for (unsigned i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
 create_unimplemented_device(r40_unimplemented[i].device_name,
 r40_unimplemented[i].base,
 r40_unimplemented[i].size);
-- 
2.41.0




[PULL 14/56] hw/arm/armv7m: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/arm/armv7m.c: In function ‘armv7m_realize’:
  hw/arm/armv7m.c:520:27: warning: declaration of ‘sbd’ shadows a previous 
local [-Wshadow=compatible-local]
520 | SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
|   ^~~
  hw/arm/armv7m.c:278:19: note: shadowed declaration is here
278 | SysBusDevice *sbd;
|   ^~~
   ---

  hw/arm/armsse.c: In function ‘armsse_realize’:
  hw/arm/armsse.c:1471:27: warning: declaration of ‘mr’ shadows a previous 
local [-Wshadow=compatible-local]
   1471 | MemoryRegion *mr;
|   ^~
  hw/arm/armsse.c:917:19: note: shadowed declaration is here
917 | MemoryRegion *mr;
|   ^~
   ---

  hw/arm/armsse.c:1608:22: warning: declaration of ‘dev_splitter’ shadows a 
previous local [-Wshadow=compatible-local]
   1608 | DeviceState *dev_splitter = DEVICE(splitter);
|  ^~~~
  hw/arm/armsse.c:923:18: note: shadowed declaration is here
923 | DeviceState *dev_splitter;
|  ^~~~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-8-phi...@linaro.org>
Reviewed-by: Peter Maydell 
Signed-off-by: Markus Armbruster 
---
 hw/arm/armsse.c | 16 ++--
 hw/arm/armv7m.c |  2 +-
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 11cd08b6c1..31acbf7347 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1468,7 +1468,6 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 if (info->has_cachectrl) {
 for (i = 0; i < info->num_cpus; i++) {
 char *name = g_strdup_printf("cachectrl%d", i);
-MemoryRegion *mr;
 
 qdev_prop_set_string(DEVICE(&s->cachectrl[i]), "name", name);
 g_free(name);
@@ -1484,7 +1483,6 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 if (info->has_cpusecctrl) {
 for (i = 0; i < info->num_cpus; i++) {
 char *name = g_strdup_printf("CPUSECCTRL%d", i);
-MemoryRegion *mr;
 
 qdev_prop_set_string(DEVICE(&s->cpusecctrl[i]), "name", name);
 g_free(name);
@@ -1499,7 +1497,6 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 }
 if (info->has_cpuid) {
 for (i = 0; i < info->num_cpus; i++) {
-MemoryRegion *mr;
 
 qdev_prop_set_uint32(DEVICE(&s->cpuid[i]), "CPUID", i);
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpuid[i]), errp)) {
@@ -1512,7 +1509,6 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 }
 if (info->has_cpu_pwrctrl) {
 for (i = 0; i < info->num_cpus; i++) {
-MemoryRegion *mr;
 
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpu_pwrctrl[i]), errp)) {
 return;
@@ -1605,7 +1601,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 /* Wire up the splitters for the MPC IRQs */
 for (i = 0; i < IOTS_NUM_EXP_MPC + info->sram_banks; i++) {
 SplitIRQ *splitter = &s->mpc_irq_splitter[i];
-DeviceState *dev_splitter = DEVICE(splitter);
+DeviceState *devs = DEVICE(splitter);
 
 if (!object_property_set_int(OBJECT(splitter), "num-lines", 2,
  errp)) {
@@ -1617,22 +1613,22 @@ static void armsse_realize(DeviceState *dev, Error 
**errp)
 
 if (i < IOTS_NUM_EXP_MPC) {
 /* Splitter input is from GPIO input line */
-s->mpcexp_status_in[i] = qdev_get_gpio_in(dev_splitter, 0);
-qdev_connect_gpio_out(dev_splitter, 0,
+s->mpcexp_status_in[i] = qdev_get_gpio_in(devs, 0);
+qdev_connect_gpio_out(devs, 0,
   qdev_get_gpio_in_named(dev_secctl,
  "mpcexp_status", i));
 } else {
 /* Splitter input is from our own MPC */
 qdev_connect_gpio_out_named(DEVICE(&s->mpc[i - IOTS_NUM_EXP_MPC]),
 "irq", 0,
-qdev_get_gpio_in(dev_splitter, 0));
-qdev_connect_gpio_out(dev_splitter, 0,
+qdev_get_gpio_in(devs, 0));
+qdev_connect_gpio_out(devs, 0,
   qdev_get_gpio_in_named(dev_secctl,
  "mpc_status",
  i - 
IOTS_NUM_EXP_MPC));
 }
 
-qdev_connect_gpio_out(dev_splitter, 1,
+qdev_connect_gpio_out(devs, 1,
   qdev_get_gpio_in(DEVICE(&s->mpc_irq_orgate), i));
 }
 /* Create GPIO inputs which will pass the line state for our
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index bf173b10b8..1f78e18872 10

[PULL 27/56] hw/core/machine: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  hw/core/machine.c: In function ‘machine_initfn’:
  hw/core/machine.c:1081:17: warning: declaration of ‘obj’ shadows a parameter 
[-Wshadow=compatible-local]
   1081 | Object *obj = OBJECT(ms);
| ^~~
  hw/core/machine.c:1065:36: note: shadowed declaration is here
   1065 | static void machine_initfn(Object *obj)
|^~~

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904162824.85385-2-phi...@linaro.org>
Reviewed-by: Peter Maydell 
Signed-off-by: Markus Armbruster 
---
 hw/core/machine.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cb38b8cf4c..a232ae0bcd 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1082,8 +1082,6 @@ static void machine_initfn(Object *obj)
 ms->maxram_size = mc->default_ram_size;
 
 if (mc->nvdimm_supported) {
-Object *obj = OBJECT(ms);
-
 ms->nvdimms_state = g_new0(NVDIMMState, 1);
 object_property_add_bool(obj, "nvdimm",
  machine_get_nvdimm, machine_set_nvdimm);
-- 
2.41.0




[PULL 30/56] pnv/psi: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

to fix :

  ../hw/ppc/pnv_psi.c: In function ‘pnv_psi_p9_mmio_write’:
  ../hw/ppc/pnv_psi.c:741:24: warning: declaration of ‘addr’ shadows a 
parameter [-Wshadow=compatible-local]
741 | hwaddr addr = val & ~(PSIHB9_ESB_CI_VALID | 
PSIHB10_ESB_CI_64K);
|^~~~
  ../hw/ppc/pnv_psi.c:702:56: note: shadowed declaration is here
702 | static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
| ~~~^~~~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-3-...@kaod.org>
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/pnv_psi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index daaa2f0575..26460d210d 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -738,8 +738,9 @@ static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
 }
 } else {
 if (!(psi->regs[reg] & PSIHB9_ESB_CI_VALID)) {
-hwaddr addr = val & ~(PSIHB9_ESB_CI_VALID | 
PSIHB10_ESB_CI_64K);
-memory_region_add_subregion(sysmem, addr,
+hwaddr esb_addr =
+val & ~(PSIHB9_ESB_CI_VALID | PSIHB10_ESB_CI_64K);
+memory_region_add_subregion(sysmem, esb_addr,
 &psi9->source.esb_mmio);
 }
 }
-- 
2.41.0




[PULL 35/56] spapr/pci: Clean up local variable shadowing in spapr_phb_realize()

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Rename SysBusDevice variable to avoid this warning :

  ../hw/ppc/spapr_pci.c: In function ‘spapr_phb_realize’:
  ../hw/ppc/spapr_pci.c:1872:24: warning: declaration of ‘s’ shadows a previous 
local [-Wshadow=local]
   1872 | SpaprPhbState *s;
|^
  ../hw/ppc/spapr_pci.c:1829:19: note: shadowed declaration is here
   1829 | SysBusDevice *s = SYS_BUS_DEVICE(dev);
|   ^

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-8-...@kaod.org>
Reviewed-by: Harsh Prateek Bora 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/spapr_pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index ce14959317..370c5a90f2 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1826,9 +1826,9 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
 (SpaprMachineState *) object_dynamic_cast(qdev_get_machine(),
   TYPE_SPAPR_MACHINE);
 SpaprMachineClass *smc = spapr ? SPAPR_MACHINE_GET_CLASS(spapr) : NULL;
-SysBusDevice *s = SYS_BUS_DEVICE(dev);
-SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
-PCIHostState *phb = PCI_HOST_BRIDGE(s);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(sbd);
+PCIHostState *phb = PCI_HOST_BRIDGE(sbd);
 MachineState *ms = MACHINE(spapr);
 char *namebuf;
 int i;
-- 
2.41.0




[PULL 33/56] spapr: Clean up local variable shadowing in spapr_get_fw_dev_path()

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

Rename PCIDevice variable to avoid this warning :

  ../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
  ../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a 
previous local [-Wshadow=compatible-local]
   3217 | PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
|^~
  ../hw/ppc/spapr.c:3147:16: note: shadowed declaration is here
   3147 | PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
|^~

Signed-off-by: Cédric Le Goater 
Message-ID: <20230918145850.241074-6-...@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
---
 hw/ppc/spapr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4fe82e490a..d4230d3647 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3214,8 +3214,8 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, 
BusState *bus,
 
 if (g_str_equal("pci-bridge", qdev_fw_name(dev))) {
 /* SLOF uses "pci" instead of "pci-bridge" for PCI bridges */
-PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
-return g_strdup_printf("pci@%x", PCI_SLOT(pcidev->devfn));
+PCIDevice *pdev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
+return g_strdup_printf("pci@%x", PCI_SLOT(pdev->devfn));
 }
 
 if (pcidev) {
-- 
2.41.0




[PULL 47/56] intel_iommu: Fix shadow local variables on "size"

2023-09-29 Thread Markus Armbruster
From: Peter Xu 

This patch fixes the warning of shadowed local variable:

../hw/i386/intel_iommu.c: In function ‘vtd_address_space_unmap’:
../hw/i386/intel_iommu.c:3773:18: warning: declaration of ‘size’ shadows a 
previous local [-Wshadow=compatible-local]
 3773 | uint64_t size = mask + 1;
  |  ^~~~
../hw/i386/intel_iommu.c:3747:12: note: shadowed declaration is here
 3747 | hwaddr size, remain;
  |^~~~

Cc: Jason Wang 
Cc: Eric Auger 
Cc: Michael S. Tsirkin 
Cc: Markus Armbruster 
Signed-off-by: Peter Xu 
Message-ID: <20230922160410.138786-1-pet...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Markus Armbruster 
---
 hw/i386/intel_iommu.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index c0ce896668..2c832ab68b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3744,7 +3744,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, 
PCIBus *bus,
 /* Unmap the whole range in the notifier's scope. */
 static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
 {
-hwaddr size, remain;
+hwaddr total, remain;
 hwaddr start = n->start;
 hwaddr end = n->end;
 IntelIOMMUState *s = as->iommu_state;
@@ -3765,7 +3765,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, 
IOMMUNotifier *n)
 }
 
 assert(start <= end);
-size = remain = end - start + 1;
+total = remain = end - start + 1;
 
 while (remain >= VTD_PAGE_SIZE) {
 IOMMUTLBEvent event;
@@ -3793,10 +3793,10 @@ static void vtd_address_space_unmap(VTDAddressSpace 
*as, IOMMUNotifier *n)
 trace_vtd_as_unmap_whole(pci_bus_num(as->bus),
  VTD_PCI_SLOT(as->devfn),
  VTD_PCI_FUNC(as->devfn),
- n->start, size);
+ n->start, total);
 
 map.iova = n->start;
-map.size = size - 1; /* Inclusive */
+map.size = total - 1; /* Inclusive */
 iova_tree_remove(as->iova_tree, map);
 }
 
-- 
2.41.0




[PULL 11/56] target/mips: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Fix:

  target/mips/tcg/nanomips_translate.c.inc:4410:33: error: declaration shadows 
a local variable [-Werror,-Wshadow]
int32_t imm = extract32(ctx->opcode, 1, 13) |
^
  target/mips/tcg/nanomips_translate.c.inc:3577:9: note: previous declaration 
is here
int imm;
^
  target/mips/tcg/translate.c:15578:19: error: declaration shadows a local 
variable [-Werror,-Wshadow]
for (unsigned i = 1; i < 32; i++) {
  ^
  target/mips/tcg/translate.c:15567:9: note: previous declaration is here
int i;
^
  target/mips/tcg/msa_helper.c:7478:13: error: declaration shadows a local 
variable [-Werror,-Wshadow]
MSA_FLOAT_MAXOP(pwx->w[0], min, pws->w[0], pws->w[0], 32);
^
  target/mips/tcg/msa_helper.c:7434:23: note: expanded from macro 
'MSA_FLOAT_MAXOP'
float_status *status = &env->active_tc.msa_fp_status;
  ^

Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20230904161235.84651-5-phi...@linaro.org>
Signed-off-by: Markus Armbruster 
---
 target/mips/tcg/msa_helper.c | 8 
 target/mips/tcg/translate.c  | 8 +++-
 target/mips/tcg/nanomips_translate.c.inc | 6 +++---
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/target/mips/tcg/msa_helper.c b/target/mips/tcg/msa_helper.c
index c314a74397..7a8dbada5d 100644
--- a/target/mips/tcg/msa_helper.c
+++ b/target/mips/tcg/msa_helper.c
@@ -7432,15 +7432,15 @@ void helper_msa_ftq_df(CPUMIPSState *env, uint32_t df, 
uint32_t wd,
 
 #define MSA_FLOAT_MAXOP(DEST, OP, ARG1, ARG2, BITS) \
 do {\
-float_status *status = &env->active_tc.msa_fp_status;   \
+float_status *status_ = &env->active_tc.msa_fp_status;  \
 int c;  \
 \
-set_float_exception_flags(0, status);   \
-DEST = float ## BITS ## _ ## OP(ARG1, ARG2, status);\
+set_float_exception_flags(0, status_);  \
+DEST = float ## BITS ## _ ## OP(ARG1, ARG2, status_);   \
 c = update_msacsr(env, 0, 0);   \
 \
 if (get_enabled_exceptions(env, c)) {   \
-DEST = ((FLOAT_SNAN ## BITS(status) >> 6) << 6) | c;\
+DEST = ((FLOAT_SNAN ## BITS(status_) >> 6) << 6) | c;   \
 }   \
 } while (0)
 
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 9bb40f1849..26d741d960 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -15564,10 +15564,8 @@ void gen_intermediate_code(CPUState *cs, 
TranslationBlock *tb, int *max_insns,
 
 void mips_tcg_init(void)
 {
-int i;
-
 cpu_gpr[0] = NULL;
-for (i = 1; i < 32; i++)
+for (unsigned i = 1; i < 32; i++)
 cpu_gpr[i] = tcg_global_mem_new(cpu_env,
 offsetof(CPUMIPSState,
  active_tc.gpr[i]),
@@ -15584,7 +15582,7 @@ void mips_tcg_init(void)
rname);
 }
 #endif /* !TARGET_MIPS64 */
-for (i = 0; i < 32; i++) {
+for (unsigned i = 0; i < 32; i++) {
 int off = offsetof(CPUMIPSState, active_fpu.fpr[i].wr.d[0]);
 
 fpu_f64[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
@@ -15592,7 +15590,7 @@ void mips_tcg_init(void)
 msa_translate_init();
 cpu_PC = tcg_global_mem_new(cpu_env,
 offsetof(CPUMIPSState, active_tc.PC), "PC");
-for (i = 0; i < MIPS_DSP_ACC; i++) {
+for (unsigned i = 0; i < MIPS_DSP_ACC; i++) {
 cpu_HI[i] = tcg_global_mem_new(cpu_env,
offsetof(CPUMIPSState, active_tc.HI[i]),
regnames_HI[i]);
diff --git a/target/mips/tcg/nanomips_translate.c.inc 
b/target/mips/tcg/nanomips_translate.c.inc
index a98dde0d2e..d81a7c2d11 100644
--- a/target/mips/tcg/nanomips_translate.c.inc
+++ b/target/mips/tcg/nanomips_translate.c.inc
@@ -4407,8 +4407,8 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, 
DisasContext *ctx)
 case NM_BPOSGE32C:
 check_dsp_r3(ctx);
 {
-int32_t imm = extract32(ctx->opcode, 1, 13) |
-  extract32(ctx->opcode, 0, 1) << 13;
+imm = extract32(ctx->opcode, 1, 13)
+| extract

[PULL 46/56] aspeed/timer: Clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Cédric Le Goater 

commit 8137355e850f ("aspeed/timer: Fix behaviour running Linux")
introduced a MAX() expression to calculate the next timer deadline :

return calculate_time(t, MAX(MAX(t->match[0], t->match[1]), 0));

The second MAX() is not necessary since the compared values are an
unsigned and 0. Simply remove it and fix warning :

  ../hw/timer/aspeed_timer.c: In function ‘calculate_next’:
  ../include/qemu/osdep.h:396:31: warning: declaration of ‘_a’ shadows a 
previous local [-Wshadow=compatible-local]
396 | typeof(1 ? (a) : (b)) _a = (a), _b = (b);   \
|   ^~
  ../hw/timer/aspeed_timer.c:170:12: note: in expansion of macro ‘MAX’
170 | next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
|^~~
  ../hw/timer/aspeed_timer.c:170:16: note: in expansion of macro ‘MAX’
170 | next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
|^~~
  /home/legoater/work/qemu/qemu-aspeed.git/include/qemu/osdep.h:396:31: note: 
shadowed declaration is here
396 | typeof(1 ? (a) : (b)) _a = (a), _b = (b);   \
|   ^~
  ../hw/timer/aspeed_timer.c:170:12: note: in expansion of macro ‘MAX’
170 | next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
|^~~

Cc: Joel Stanley 
Cc: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
Message-ID: <20230922155924.1172019-5-...@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
---
 hw/timer/aspeed_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 9c20b3d6ad..72161f07bb 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -167,7 +167,7 @@ static uint64_t calculate_next(struct AspeedTimer *t)
 qemu_set_irq(t->irq, t->level);
 }
 
-next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
+next = MAX(calculate_match(t, 0), calculate_match(t, 1));
 t->start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
 return calculate_time(t, next);
-- 
2.41.0




[PULL 55/56] hw/nvme: Clean up local variable shadowing in nvme_ns_init()

2023-09-29 Thread Markus Armbruster
From: Klaus Jensen 

Fix local variable shadowing in nvme_ns_init().

Reported-by: Markus Armbruster 
Signed-off-by: Klaus Jensen 
Message-ID: <20230925-fix-local-shadowing-v1-1-3a1172132...@samsung.com>
Reviewed-by: Jesper Wendel Devantier 
Signed-off-by: Markus Armbruster 
---
 hw/nvme/ns.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 44aba8f4d9..0eabcf5cf5 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -107,7 +107,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 ns->pif = ns->params.pif;
 
-static const NvmeLBAF lbaf[16] = {
+static const NvmeLBAF defaults[16] = {
 [0] = { .ds =  9   },
 [1] = { .ds =  9, .ms =  8 },
 [2] = { .ds =  9, .ms = 16 },
@@ -120,7 +120,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 ns->nlbaf = 8;
 
-memcpy(&id_ns->lbaf, &lbaf, sizeof(lbaf));
+memcpy(&id_ns->lbaf, &defaults, sizeof(defaults));
 
 for (i = 0; i < ns->nlbaf; i++) {
 NvmeLBAF *lbaf = &id_ns->lbaf[i];
-- 
2.41.0




[PULL 54/56] softmmu/device_tree: Fixup local variables shadowing

2023-09-29 Thread Markus Armbruster
From: Alistair Francis 

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
Message-ID: <20230925043023.71448-5-alistair.fran...@wdc.com>
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Markus Armbruster 
---
 softmmu/device_tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 30aa3aea9f..eb5166ca36 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -418,9 +418,9 @@ int qemu_fdt_setprop_string_array(void *fdt, const char 
*node_path,
 }
 p = str = g_malloc0(total_len);
 for (i = 0; i < len; i++) {
-int len = strlen(array[i]) + 1;
-pstrcpy(p, len, array[i]);
-p += len;
+int offset = strlen(array[i]) + 1;
+pstrcpy(p, offset, array[i]);
+p += offset;
 }
 
 ret = qemu_fdt_setprop(fdt, node_path, prop, str, total_len);
-- 
2.41.0




[PULL 50/56] qemu-nbd: changes towards enabling -Wshadow=local

2023-09-29 Thread Markus Armbruster
From: Eric Blake 

Address all compiler complaints from -Wshadow in qemu-nbd.  Several
instances of 'int ret' became shadows when commit 4fbec260 added 'ret'
at a higher scope in main.  More interesting was the 'void *ret'
capturing the result of a pthread; where we were conceptually doing
'(void*)(intptr_t)EXIT_FAILURE != NULL' which just feels wrong (even
though it happens to compile correctly), so it was worth a better
cleanup.

Signed-off-by: Eric Blake 
Message-ID: <20230922205019.2755352-2-ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Markus Armbruster 
---
 qemu-nbd.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 70aa3c487a..54faa87a0c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -939,7 +939,6 @@ int main(int argc, char **argv)
 g_autoptr(GError) err = NULL;
 int stderr_fd[2];
 pid_t pid;
-int ret;
 
 if (!g_unix_open_pipe(stderr_fd, FD_CLOEXEC, &err)) {
 error_report("Error setting up communication pipe: %s",
@@ -1172,7 +1171,6 @@ int main(int argc, char **argv)
 
 if (opts.device) {
 #if HAVE_NBD_DEVICE
-int ret;
 ret = pthread_create(&client_thread, NULL, nbd_client_thread, &opts);
 if (ret != 0) {
 error_report("Failed to create client thread: %s", strerror(ret));
@@ -1219,9 +1217,10 @@ int main(int argc, char **argv)
 qemu_opts_del(sn_opts);
 
 if (opts.device) {
-void *ret;
-pthread_join(client_thread, &ret);
-exit(ret != NULL);
+void *result;
+pthread_join(client_thread, &result);
+ret = (intptr_t)result;
+exit(ret);
 } else {
 exit(EXIT_SUCCESS);
 }
-- 
2.41.0




[PULL 40/56] hw/misc/arm_sysctl.c: Avoid shadowing local variable

2023-09-29 Thread Markus Armbruster
From: Peter Maydell 

Avoid shadowing a local variable in arm_sysctl_write():

../../hw/misc/arm_sysctl.c: In function ‘arm_sysctl_write’:
../../hw/misc/arm_sysctl.c:537:26: warning: declaration of ‘val’ shadows a 
parameter [-Wshadow=local]
  537 | uint32_t val;
  |  ^~~
../../hw/misc/arm_sysctl.c:388:39: note: shadowed declaration is here
  388 |  uint64_t val, unsigned size)
  |  ~^~~

Signed-off-by: Peter Maydell 
Message-ID: <20230922152944.3583438-3-peter.mayd...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Auger 
Signed-off-by: Markus Armbruster 
---
 hw/misc/arm_sysctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/arm_sysctl.c b/hw/misc/arm_sysctl.c
index 42d4693854..3e4f4b0524 100644
--- a/hw/misc/arm_sysctl.c
+++ b/hw/misc/arm_sysctl.c
@@ -534,12 +534,12 @@ static void arm_sysctl_write(void *opaque, hwaddr offset,
 s->sys_cfgstat |= 2;/* error */
 }
 } else {
-uint32_t val;
+uint32_t data;
 if (!vexpress_cfgctrl_read(s, dcc, function, site, position,
-   device, &val)) {
+   device, &data)) {
 s->sys_cfgstat |= 2;/* error */
 } else {
-s->sys_cfgdata = val;
+s->sys_cfgdata = data;
 }
 }
 }
-- 
2.41.0




[PULL 42/56] hw/arm/smmuv3-internal.h: Don't use locals in statement macros

2023-09-29 Thread Markus Armbruster
From: Peter Maydell 

The STE_CTXPTR() and STE_S2TTB() macros both extract two halves
of an address from fields in the STE and combine them into a
single value to return. The current code for this uses a GCC
statement expression. There are two problems with this:

(1) The type chosen for the variable in the statement expr
is 'unsigned long', which might not be 64 bits

(2) the name chosen for the variable causes -Wshadow warnings
because it's the same as a variable in use at the callsite:

In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmu_get_cd’:
../../hw/arm/smmuv3-internal.h:538:23: warning: declaration of ‘addr’ shadows a 
previous local [-Wshadow=compatible-local]
  538 | unsigned long addr; \
  |   ^~~~
../../hw/arm/smmuv3.c:339:23: note: in expansion of macro ‘STE_CTXPTR’
  339 | dma_addr_t addr = STE_CTXPTR(ste);
  |   ^~
../../hw/arm/smmuv3.c:339:16: note: shadowed declaration is here
  339 | dma_addr_t addr = STE_CTXPTR(ste);
  |^~~~

Sidestep both of these problems by just using a single
expression rather than a statement expr.

For CMD_ADDR, we got the type of the variable right but still
run into -Wshadow problems:

In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmuv3_range_inval’:
../../hw/arm/smmuv3-internal.h:334:22: warning: declaration of ‘addr’ shadows a 
previous local [-Wshadow=compatible-local]
  334 | uint64_t addr = high << 32 | (low << 12); \
  |  ^~~~
../../hw/arm/smmuv3.c:1104:28: note: in expansion of macro ‘CMD_ADDR’
 1104 | dma_addr_t end, addr = CMD_ADDR(cmd);
  |^~~~
../../hw/arm/smmuv3.c:1104:21: note: shadowed declaration is here
 1104 | dma_addr_t end, addr = CMD_ADDR(cmd);
  | ^~~~

so convert it too.

CD_TTB has neither problem, but it is the only other macro in
the file that uses this pattern, so we convert it also for
consistency's sake.

We use extract64() rather than extract32() to avoid having
to explicitly cast the result to uint64_t.

Signed-off-by: Peter Maydell 
Message-ID: <20230922152944.3583438-5-peter.mayd...@linaro.org>
Reviewed-by: Eric Auger 
Signed-off-by: Markus Armbruster 
---
 hw/arm/smmuv3-internal.h | 41 +---
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 6d1c1edab7..648c2e37a2 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -328,12 +328,9 @@ enum { /* Command completion notification */
 #define CMD_TTL(x)  extract32((x)->word[2], 8 , 2)
 #define CMD_TG(x)   extract32((x)->word[2], 10, 2)
 #define CMD_STE_RANGE(x)extract32((x)->word[2], 0 , 5)
-#define CMD_ADDR(x) ({\
-uint64_t high = (uint64_t)(x)->word[3];   \
-uint64_t low = extract32((x)->word[2], 12, 20);\
-uint64_t addr = high << 32 | (low << 12); \
-addr; \
-})
+#define CMD_ADDR(x) \
+(((uint64_t)((x)->word[3]) << 32) | \
+ ((extract64((x)->word[2], 12, 20)) << 12))
 
 #define SMMU_FEATURE_2LVL_STE (1 << 0)
 
@@ -533,21 +530,13 @@ typedef struct CD {
 #define STE_S2S(x) extract32((x)->word[5], 25, 1)
 #define STE_S2R(x) extract32((x)->word[5], 26, 1)
 
-#define STE_CTXPTR(x)   \
-({  \
-unsigned long addr; \
-addr = (uint64_t)extract32((x)->word[1], 0, 16) << 32;  \
-addr |= (uint64_t)((x)->word[0] & 0xffc0);  \
-addr;   \
-})
+#define STE_CTXPTR(x)   \
+((extract64((x)->word[1], 0, 16) << 32) |   \
+ ((x)->word[0] & 0xffc0))
 
-#define STE_S2TTB(x)\
-({  \
-unsigned long addr; \
-addr = (uint64_t)extract32((x)->word[7], 0, 16) << 32;  \
-addr |= (uint64_t)((x)->word[6] & 0xfff0);  \
-addr;   \
-})
+#define STE_S2TTB(x)\
+((extract64((x)->word[7], 0, 16) << 32) |   \
+ ((x)->word[6] & 0xfff0))
 
 static inline int oas2bits(int oas_field)
 {
@@ -585,14 +574,10 @@ static inline int pa_range(STE *ste)
 
 #define CD_VALID(x)   extract32((x)->word[0], 31, 1)
 #define CD_ASID(x)extract32((x)->word[1], 16, 16)
-#define CD_TTB(x, sel) 

[PULL 56/56] disas/m68k: clean up local variable shadowing

2023-09-29 Thread Markus Armbruster
From: Laurent Vivier 

Fix following warnings

.../disas/m68k.c: In function ‘print_insn_arg’:
.../disas/m68k.c:1635:13: warning: declaration of ‘val’ shadows a previous 
local [-Wshadow=compatible-local]
 1635 | int val = fetch_arg (buffer, place, 5, info);
  | ^~~
.../disas/m68k.c:1093:7: note: shadowed declaration is here
 1093 |   int val = 0;
  |   ^~~

Signed-off-by: Laurent Vivier 
Message-ID: <20230925084455.395150-1-laur...@vivier.eu>
Reviewed-by: Thomas Huth 
Signed-off-by: Markus Armbruster 
---
 disas/m68k.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/disas/m68k.c b/disas/m68k.c
index aefaecfbd6..1f16e295ab 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -1632,10 +1632,10 @@ print_insn_arg (const char *d,
 case '2':
 case '3':
   {
-   int val = fetch_arg (buffer, place, 5, info);
+   int reg = fetch_arg (buffer, place, 5, info);
 const char *name = 0;
 
-   switch (val)
+   switch (reg)
  {
  case 2: name = "%tt0"; break;
  case 3: name = "%tt1"; break;
@@ -1655,12 +1655,12 @@ print_insn_arg (const char *d,
  int break_reg = ((buffer[3] >> 2) & 7);
 
  (*info->fprintf_func)
-   (info->stream, val == 0x1c ? "%%bad%d" : "%%bac%d",
+   (info->stream, reg == 0x1c ? "%%bad%d" : "%%bac%d",
 break_reg);
}
break;
  default:
-   (*info->fprintf_func) (info->stream, "", val);
+   (*info->fprintf_func) (info->stream, "", reg);
  }
if (name)
  (*info->fprintf_func) (info->stream, "%s", name);
-- 
2.41.0




[PULL 52/56] target/riscv: cpu: Fixup local variables shadowing

2023-09-29 Thread Markus Armbruster
From: Alistair Francis 

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
Message-ID: <20230925043023.71448-3-alistair.fran...@wdc.com>
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Markus Armbruster 
---
 target/riscv/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f227c7664e..4140899c52 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -704,7 +704,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 CSR_MPMMASK,
 };
 
-for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
+for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
 int csrno = dump_csrs[i];
 target_ulong val = 0;
 RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
@@ -747,7 +747,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 CSR_VTYPE,
 CSR_VLENB,
 };
-for (int i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
+for (i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
 int csrno = dump_rvv_csrs[i];
 target_ulong val = 0;
 RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
-- 
2.41.0




[PULL 48/56] crypto: remove shadowed 'ret' variable

2023-09-29 Thread Markus Armbruster
From: Daniel P. Berrangé 

Both instances of 'ret' are used to store a gnutls API return code.

Signed-off-by: Daniel P. Berrangé 
Message-ID: <20230922160644.438631-2-berra...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
---
 crypto/tls-cipher-suites.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crypto/tls-cipher-suites.c b/crypto/tls-cipher-suites.c
index 5e4f597464..d0df4badc0 100644
--- a/crypto/tls-cipher-suites.c
+++ b/crypto/tls-cipher-suites.c
@@ -52,7 +52,6 @@ GByteArray 
*qcrypto_tls_cipher_suites_get_data(QCryptoTLSCipherSuites *obj,
 byte_array = g_byte_array_new();
 
 for (i = 0;; i++) {
-int ret;
 unsigned idx;
 const char *name;
 IANA_TLS_CIPHER cipher;
-- 
2.41.0




[PULL 51/56] hw/riscv: opentitan: Fixup local variables shadowing

2023-09-29 Thread Markus Armbruster
From: Alistair Francis 

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
Message-ID: <20230925043023.71448-2-alistair.fran...@wdc.com>
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Markus Armbruster 
---
 hw/riscv/opentitan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 6a2fcc4ade..436503f1ba 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -227,7 +227,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, 
Error **errp)
IRQ_M_TIMER));
 
 /* SPI-Hosts */
-for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
+for (i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
 dev = DEVICE(&(s->spi_host[i]));
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi_host[i]), errp)) {
 return;
-- 
2.41.0




[PULL 53/56] target/riscv: vector_helper: Fixup local variables shadowing

2023-09-29 Thread Markus Armbruster
From: Alistair Francis 

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
Message-ID: <20230925043023.71448-4-alistair.fran...@wdc.com>
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Markus Armbruster 
---
 target/riscv/vector_helper.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 3fb05cc3d6..cba02c1320 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -516,7 +516,7 @@ ProbeSuccess:
 k++;
 continue;
 }
-target_ulong addr = base + ((i * nf + k) << log2_esz);
+addr = base + ((i * nf + k) << log2_esz);
 ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
 k++;
 }
@@ -4791,9 +4791,10 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, 
void *vs2, \
 uint32_t total_elems = vext_get_total_elems(env, desc, esz);  \
 uint32_t vta = vext_vta(desc);\
 uint32_t vma = vext_vma(desc);\
-target_ulong i_max, i;\
+target_ulong i_max, i_min, i; \
   \
-i_max = MAX(MIN(s1 < vlmax ? vlmax - s1 : 0, vl), env->vstart);   \
+i_min = MIN(s1 < vlmax ? vlmax - s1 : 0, vl); \
+i_max = MAX(i_min, env->vstart);  \
 for (i = env->vstart; i < i_max; ++i) {   \
 if (!vm && !vext_elem_mask(v0, i)) {  \
 /* set masked-off elements to 1s */   \
-- 
2.41.0




Re: [PATCH v7 07/12] nbd/client: Initial support for extended headers

2023-09-29 Thread Vladimir Sementsov-Ogievskiy

On 25.09.23 22:22, Eric Blake wrote:

  nbd_receive_simple_reply(int32_t error, const char *errname, uint64_t cookie) "Got simple reply: { 
.error = %" PRId32 " (%s), cookie = %" PRIu64" }"
-nbd_receive_structured_reply_chunk(uint16_t flags, uint16_t type, const char *name, uint64_t cookie, uint32_t length) 
"Got structured reply chunk: { flags = 0x%" PRIx16 ", type = %d (%s), cookie = %" PRIu64 ", 
length = %" PRIu32 " }"
+nbd_receive_reply_chunk_header(uint16_t flags, uint16_t type, const char *name, uint64_t cookie, uint32_t length) 
"Got reply chunk header: { flags = 0x%" PRIx16 ", type = %d (%s), cookie = %" PRIu64 ", length 
= %" PRIu32 " }"


preexisting, but better would be s/%d/%" PRIu16 "/


+nbd_receive_wrong_header(uint32_t magic, const char *mode) "Server sent unexpected magic 
0x%" PRIx32 " for negotiated mode %s"



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir




Re: [PATCH 0/6] riscv: RVA22U64 profile support

2023-09-29 Thread Andrea Bolognani
On Tue, Sep 26, 2023 at 04:49:44PM -0300, Daniel Henrique Barboza wrote:
> Based-on: 20230926183109.165878-1-dbarb...@ventanamicro.com
> ("[PATCH 0/2] riscv: add extension properties for all cpus")
>
> Hi,
>
> These patches implements the base profile support for qemu-riscv and the
> first profile, RVA22U64.
>
> As discussed in this thread [1] we're aiming for a flag that enables all
> mandatory extensions of a profile. Optional extensions were left behind
> and must be enabled by hand if desired. Since this is the first profile
> we're adding, we'll need to add the base framework as well.
>
> The RVA22U64 profile was chosen because qemu-riscv implements all its
> extensions, both mandatory and optional. That includes 'zicntr' and
> 'zihpm', which we support for awhile but aren't adverting to userspace.
>
> Other design decisions made:
>
> - disabling a profile flag does nothing, i.e. we won't mass disable
>   mandatory extensions of the rva22U64 profile if the user sets
>   rva22u64=false;

Wouldn't it make more sense to error out when this is requested?

Silently ignoring an explicit request made by the user is pretty much
never a good idea in my experience.

-- 
Andrea Bolognani / Red Hat / Virtualization




Re: [PATCH 2/2] target/riscv/tcg-cpu.c: add extension properties for all cpus

2023-09-29 Thread Daniel P . Berrangé
On Tue, Sep 26, 2023 at 03:31:09PM -0300, Daniel Henrique Barboza wrote:
> At this moment we do not expose extension properties for vendor CPUs
> because that would allow users to change them via command line. The
> drawback is that if we were to add an API that shows all CPU properties,
> e.g. qmp-query-cpu-model-expansion, we won't be able to show extensions
> state of vendor CPUs.
> 
> We have the required machinery to create extension properties for vendor
> CPUs while not allowing users to enable extensions. Disabling existing
> extensions is allowed since it can be useful for debugging.
> 
> Change the set() callback cpu_set_multi_ext_cfg() to allow enabling
> extensions only for generic CPUs. In cpu_add_multi_ext_prop() let's not
> set the default values for the properties if we're not dealing with
> generic CPUs, otherwise the values set in cpu_init() of vendor CPUs will
> be overwritten. And finally, in tcg_cpu_instance_init(), add cpu user
> properties for all CPUs.
> 
> For the veyron-v1 CPU, we're now able to disable existing extensions
> like smstateen:
> 
> $ ./build/qemu-system-riscv64 --nographic -M virt \
> -cpu veyron-v1,smstateen=false
> 
> But setting extensions that the CPU didn't set during cpu_init(), like
> V, is not allowed:
> 
> $ ./build/qemu-system-riscv64 --nographic -M virt \
> -cpu veyron-v1,v=true
> qemu-system-riscv64: can't apply global veyron-v1-riscv-cpu.v=true:
>   'veyron-v1' CPU does not allow enabling extensions

Why should we block the user if they want to enable an extra
feature, over and above what is built-in to the CPU model ?
Is there some technical reason that prevents this from working ?

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH 0/6] riscv: RVA22U64 profile support

2023-09-29 Thread Daniel P . Berrangé
On Tue, Sep 26, 2023 at 04:49:44PM -0300, Daniel Henrique Barboza wrote:
> Based-on: 20230926183109.165878-1-dbarb...@ventanamicro.com
> ("[PATCH 0/2] riscv: add extension properties for all cpus")
> 
> Hi,
> 
> These patches implements the base profile support for qemu-riscv and the
> first profile, RVA22U64. 
> 
> As discussed in this thread [1] we're aiming for a flag that enables all
> mandatory extensions of a profile. Optional extensions were left behind
> and must be enabled by hand if desired. Since this is the first profile
> we're adding, we'll need to add the base framework as well. 
> 
> The RVA22U64 profile was chosen because qemu-riscv implements all its
> extensions, both mandatory and optional. That includes 'zicntr' and
> 'zihpm', which we support for awhile but aren't adverting to userspace.
> 
> Other design decisions made:
> 
> - disabling a profile flag does nothing, i.e. we won't mass disable
>   mandatory extensions of the rva22U64 profile if the user sets
>   rva22u64=false;

Why shouldn't this be allowed ?

IIUC, a profile is syntactic sugar for a group of features. If
we can disable individual features explicitly, why should we
not allow use of the profile as sugar to disable them en-mass ?


BTW, I would caution that the semantics of mixing groups of
features, with individual features in -cpu is likely to be
ill defined, as you cannot rely on left-to-right processing
of the -cpu arguments.

IOW, if you say  -cpu $foo,$group=on,$feature=off

you might expect this to result in '$feature' being disabled
if it were implied by $group. This is not guaranteed as the
QDict processing of options could result in us effectively
processing   -cpu $foo,$feature=off,$group=on

This brokeness with CPU feature groups and their interaction
with CPU feature flags already impacts s390x:

  https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg00981.html

There is a possible way to fix it by declaring an ordering
such that all groups will be processed fully, before any
individual features are processed, and declaring that group
or feature names must not be repeated. This avoids a reliance
on left-to-right ordering:

  https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg01005.html

that is still likely broken, however, if multiple different
groups are listed, and there is overlap in their feature
sets.

TL;DR: feature groups are pretty error prone if more than
one is listed by the user, or they're combined with individual
features.

> 
> - profile support for vendor CPUs consists into checking if the CPU
>   happens to have the mandatory extensions required for it. In case it
>   doesn't we'll error out. This is done to follow the same prerogative
>   we always had of not allowing extensions being enabled for vendor
>   CPUs;

Why shouldn't this be allowed ?

> - the KVM driver doesn't support profiles. In theory we could apply the
>   same logic as for the vendor CPUs, but KVM has a long way to go before
>   that becomes a factor. We'll revisit this decision when KVM is able to
>   support at least one profile.
> 
> Patch 5 ("enable profile support for vendor CPUs") needs the following
> series to be applied beforehand:
> 
> "[PATCH 0/2] riscv: add extension properties for all cpus"
> 
> Otherwise we won't be able to add the profile flag to vendor CPUs.
> 
> [1] 
> https://lore.kernel.org/qemu-riscv/35a847a1-2720-14ab-61b0-c72d77d5f...@ventanamicro.com/
> 
> Daniel Henrique Barboza (6):
>   target/riscv/cpu.c: add zicntr extension flag
>   target/riscv/cpu.c: add zihpm extension flag
>   target/riscv: add rva22u64 profile definition
>   target/riscv/tcg: implement rva22u64 profile
>   target/riscv/tcg-cpu.c: enable profile support for vendor CPUs
>   target/riscv/kvm: add 'rva22u64' flag as unavailable
> 
>  target/riscv/cpu.c | 25 ++
>  target/riscv/cpu.h | 10 
>  target/riscv/cpu_cfg.h |  2 +
>  target/riscv/kvm/kvm-cpu.c |  5 +-
>  target/riscv/tcg/tcg-cpu.c | 98 ++
>  5 files changed, 139 insertions(+), 1 deletion(-)
> 
> -- 
> 2.41.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH] hw/display/ramfb: plug slight guest-triggerable leak on mode setting

2023-09-29 Thread Marc-André Lureau
Hi

On Wed, Sep 27, 2023 at 7:46 PM Laszlo Ersek  wrote:
>
> On 9/19/23 15:19, Laszlo Ersek wrote:
> > The fw_cfg DMA write callback in ramfb prepares a new display surface in
> > QEMU; this new surface is put to use ("swapped in") upon the next display
> > update. At that time, the old surface (if any) is released.
> >
> > If the guest triggers the fw_cfg DMA write callback at least twice between
> > two adjacent display updates, then the second callback (and further such
> > callbacks) will leak the previously prepared (but not yet swapped in)
> > display surface.
> >
> > The issue can be shown by:
> >
> > (1) starting QEMU with "-trace displaysurface_free", and
> >
> > (2) running the following program in the guest UEFI shell:
> >
> >> #include// ShellAppMain()
> >> #include  // gBS
> >> #include   // 
> >> EFI_GRAPHICS_OUTPUT_PROTOCOL
> >>
> >> INTN
> >> EFIAPI
> >> ShellAppMain (
> >>   IN UINTN   Argc,
> >>   IN CHAR16  **Argv
> >>   )
> >> {
> >>   EFI_STATUSStatus;
> >>   VOID  *Interface;
> >>   EFI_GRAPHICS_OUTPUT_PROTOCOL  *Gop;
> >>   UINT32Mode;
> >>
> >>   Status = gBS->LocateProtocol (
> >>   &gEfiGraphicsOutputProtocolGuid,
> >>   NULL,
> >>   &Interface
> >>   );
> >>   if (EFI_ERROR (Status)) {
> >> return 1;
> >>   }
> >>
> >>   Gop = Interface;
> >>
> >>   Mode = 1;
> >>   for ( ; ;) {
> >> Status = Gop->SetMode (Gop, Mode);
> >> if (EFI_ERROR (Status)) {
> >>   break;
> >> }
> >>
> >> Mode = 1 - Mode;
> >>   }
> >>
> >>   return 1;
> >> }
> >
> > The symptom is then that:
> >
> > - only one trace message appears periodically,
> >
> > - the time between adjacent messages keeps increasing -- implying that
> >   some list structure (containing the leaked resources) keeps growing,
> >
> > - the "surface" pointer is ever different.
> >
> >> 18566@1695127471.449586:displaysurface_free surface=0x7f2fcc09a7c0
> >> 18566@1695127471.529559:displaysurface_free surface=0x7f2fcc9dac10
> >> 18566@1695127471.659812:displaysurface_free surface=0x7f2fcc441dd0
> >> 18566@1695127471.839669:displaysurface_free surface=0x7f2fcc0363d0
> >> 18566@1695127472.069674:displaysurface_free surface=0x7f2fcc413a80
> >> 18566@1695127472.349580:displaysurface_free surface=0x7f2fcc09cd00
> >> 18566@1695127472.679783:displaysurface_free surface=0x7f2fcc1395f0
> >> 18566@1695127473.059848:displaysurface_free surface=0x7f2fcc1cae50
> >> 18566@1695127473.489724:displaysurface_free surface=0x7f2fcc42fc50
> >> 18566@1695127473.969791:displaysurface_free surface=0x7f2fcc45dcc0
> >> 18566@1695127474.499708:displaysurface_free surface=0x7f2fcc70b9d0
> >> 18566@1695127475.079769:displaysurface_free surface=0x7f2fcc82acc0
> >> 18566@1695127475.709941:displaysurface_free surface=0x7f2fcc369c00
> >> 18566@1695127476.389619:displaysurface_free surface=0x7f2fcc32b910
> >> 18566@1695127477.119772:displaysurface_free surface=0x7f2fcc0d5a20
> >> 18566@1695127477.899517:displaysurface_free surface=0x7f2fcc086c40
> >> 18566@1695127478.729962:displaysurface_free surface=0x7f2fccc72020
> >> 18566@1695127479.609839:displaysurface_free surface=0x7f2fcc185160
> >> 18566@1695127480.539688:displaysurface_free surface=0x7f2fcc23a7e0
> >> 18566@1695127481.519759:displaysurface_free surface=0x7f2fcc3ec870
> >> 18566@1695127482.549930:displaysurface_free surface=0x7f2fcc634960
> >> 18566@1695127483.629661:displaysurface_free surface=0x7f2fcc26b140
> >> 18566@1695127484.759987:displaysurface_free surface=0x7f2fcc321700
> >> 18566@1695127485.940289:displaysurface_free surface=0x7f2fccaad100
> >
> > We figured this wasn't a CVE-worthy problem, as only small amounts of
> > memory were leaked (the framebuffer itself is mapped from guest RAM, QEMU
> > only allocates administrative structures), plus libvirt restricts QEMU
> > memory footprint anyway, thus the guest can only DoS itself.
> >
> > Plug the leak, by releasing the last prepared (not yet swapped in) display
> > surface, if any, in the fw_cfg DMA write callback.
> >
> > Regarding the "reproducer", with the fix in place, the log is flooded with
> > trace messages (one per fw_cfg write), *and* the trace message alternates
> > between just two "surface" pointer values (i.e., nothing is leaked, the
> > allocator flip-flops between two objects in effect).
> >
> > This issue appears to date back to the introducion of ramfb (995b30179bdc,
> > "hw/display: add ramfb, a simple boot framebuffer living in guest ram",
> > 2018-06-18).
> >
> > Cc: Gerd Hoffmann  (maintainer:ramfb)
> > Cc: qemu-sta...@nongnu.org
> > Fixes: 995b30179bdc
> > Signed-off-by: Laszlo Ersek 
> > ---
> >  hw/display/ramfb.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
> > index 79b9754a5820..c2b002d53480 100644
> > --- a/hw/display/ramfb.c
> > +++ b/hw/display/ramfb.c
> > @@ -97,6 +97,7 @@ static void ramf

Re: [PATCH 0/6] riscv: RVA22U64 profile support

2023-09-29 Thread Daniel Henrique Barboza




On 9/29/23 07:46, Daniel P. Berrangé wrote:

On Tue, Sep 26, 2023 at 04:49:44PM -0300, Daniel Henrique Barboza wrote:

Based-on: 20230926183109.165878-1-dbarb...@ventanamicro.com
("[PATCH 0/2] riscv: add extension properties for all cpus")

Hi,

These patches implements the base profile support for qemu-riscv and the
first profile, RVA22U64.

As discussed in this thread [1] we're aiming for a flag that enables all
mandatory extensions of a profile. Optional extensions were left behind
and must be enabled by hand if desired. Since this is the first profile
we're adding, we'll need to add the base framework as well.

The RVA22U64 profile was chosen because qemu-riscv implements all its
extensions, both mandatory and optional. That includes 'zicntr' and
'zihpm', which we support for awhile but aren't adverting to userspace.

Other design decisions made:

- disabling a profile flag does nothing, i.e. we won't mass disable
   mandatory extensions of the rva22U64 profile if the user sets
   rva22u64=false;


Why shouldn't this be allowed ?

IIUC, a profile is syntactic sugar for a group of features. If
we can disable individual features explicitly, why should we
not allow use of the profile as sugar to disable them en-mass ?


In theory there's no harm in allowing mass disabling of extensions but, given
it's a whole profile, we would end up disabling most/all CPU extensions and
the guest would do nothing.

There is a thread in the ML:

https://lore.kernel.org/qemu-riscv/cabjz62nyvnu4z1qmcg7myjkgg_9ywxjufhhwjmoqep6unrr...@mail.gmail.com/

Where we discussed the possibility of having a minimal CPU extension set. We 
didn't
reach a consensus because the definition of "minimal CPU extension set" vary 
between
OSes (Linux requires IMAFD, FreeBSD might require something differ).

Assuming we reach a consensus on what a minimal set is, we could allow 
disabling mass
extensions via probile but keeping this minimal set, for example. At very least 
we
shouldn't allow users to disable 'I' because that would kill the CPU, so RV64I 
is
the minimum set that I would assume for now.





BTW, I would caution that the semantics of mixing groups of
features, with individual features in -cpu is likely to be
ill defined, as you cannot rely on left-to-right processing
of the -cpu arguments.

IOW, if you say  -cpu $foo,$group=on,$feature=off

you might expect this to result in '$feature' being disabled
if it were implied by $group. This is not guaranteed as the
QDict processing of options could result in us effectively
processing   -cpu $foo,$feature=off,$group=on

This brokeness with CPU feature groups and their interaction
with CPU feature flags already impacts s390x:

   https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg00981.html

There is a possible way to fix it by declaring an ordering
such that all groups will be processed fully, before any
individual features are processed, and declaring that group
or feature names must not be repeated. This avoids a reliance
on left-to-right ordering:

   https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg01005.html

that is still likely broken, however, if multiple different
groups are listed, and there is overlap in their feature
sets.


Just read the discussion. I agree with restricting the command line flexibility
to make things more consistent, but IIRC libvirt (and others) uses a lot of
command line appending and restricting these use cases now will break stuff
up. I recall pc64 domains using  to add extra cpu/machine
flags to the domain back in the day. Surely we can claim (I guess) that command
line pass-through is unstable and users shouldn't expect extended support for
it, but we all know that users will be less than pleased when their domains
start breaking because QEMU restricted the command line.

As for the current RISC-V case, one thing we can do is to postpone feature group
processing to realize() time, since in that stage we're already done with the
command line processing. We can make a sanity check between the feature group
flags and error out if there's something off. That would make things a little
less fragile that what they are, albeit I'm sure that there will be some cases
that will be left uncovered.



TL;DR: feature groups are pretty error prone if more than
one is listed by the user, or they're combined with individual
features.



- profile support for vendor CPUs consists into checking if the CPU
   happens to have the mandatory extensions required for it. In case it
   doesn't we'll error out. This is done to follow the same prerogative
   we always had of not allowing extensions being enabled for vendor
   CPUs;


Why shouldn't this be allowed ?


There's no technical reason to not allow it. The reason it's forbid is to be
closer to what the real hardware would do. E.g. the real hardware doesn't allow
users to enable Vector if the hardware doesn't support it. Vendor CPUs also has
a privileged spec restriction as well, so if a CPU is run

[PULL 0/2] Firmware/seabios 20230929 patches

2023-09-29 Thread Gerd Hoffmann
The following changes since commit 36e9aab3c569d4c9ad780473596e18479838d1aa:

  migration: Move return path cleanup to main migration thread (2023-09-27 
13:58:02 -0400)

are available in the Git repository at:

  https://gitlab.com/kraxel/qemu.git tags/firmware/seabios-20230929-pull-request

for you to fetch changes up to 1f75b1beeb8d958cc56113ba229348d6a0be9d9d:

  seabios: update binaries to git snapshot (2023-09-29 13:15:44 +0200)


seabios: update to git snapshot

Give seabios a bit real world testing before tagging a release.
Update to release will follow later in the devel cycle.



Gerd Hoffmann (2):
  seabios: update submodule to git snapshot
  seabios: update binaries to git snapshot

 pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes
 pc-bios/bios-microvm.bin  | Bin 131072 -> 131072 bytes
 pc-bios/bios.bin  | Bin 131072 -> 131072 bytes
 pc-bios/vgabios-ati.bin   | Bin 39936 -> 39424 bytes
 pc-bios/vgabios-bochs-display.bin | Bin 28672 -> 28672 bytes
 pc-bios/vgabios-cirrus.bin| Bin 39424 -> 38912 bytes
 pc-bios/vgabios-qxl.bin   | Bin 39936 -> 39424 bytes
 pc-bios/vgabios-ramfb.bin | Bin 29184 -> 28672 bytes
 pc-bios/vgabios-stdvga.bin| Bin 39936 -> 39424 bytes
 pc-bios/vgabios-virtio.bin| Bin 39936 -> 39424 bytes
 pc-bios/vgabios-vmware.bin| Bin 39936 -> 39424 bytes
 pc-bios/vgabios.bin   | Bin 39424 -> 38912 bytes
 roms/seabios  |   2 +-
 13 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.41.0




[PULL 1/2] seabios: update submodule to git snapshot

2023-09-29 Thread Gerd Hoffmann
git shortlog


Gerd Hoffmann (7):
  disable array bounds warning
  better kvm detection
  detect physical address space size
  move 64bit pci window to end of address space
  be less conservative with the 64bit pci io window
  qemu: log reservations in fw_cfg e820 table
  check for e820 conflict

José Martínez (1):
  Fix high memory zone initialization in CSM mode

Lukas Stockner via SeaBIOS (1):
  virtio-blk: Fix integer overflow for large max IO sizes

Mark Cave-Ayland (3):
  esp-scsi: flush FIFO before sending SCSI command
  esp-scsi: check for INTR_BS/INTR_FC instead of STAT_TC for command 
completion
  esp-scsi: handle non-DMA SCSI commands with no data phase

Niklas Cassel via SeaBIOS (1):
  ahci: handle TFES irq correctly

Tony Titus via SeaBIOS (1):
  Increase BUILD_MAX_E820 to 128

Signed-off-by: Gerd Hoffmann 
---
 roms/seabios | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roms/seabios b/roms/seabios
index ea1b7a073390..1e1da7a96300 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit ea1b7a0733906b8425d948ae94fba63c32b1d425
+Subproject commit 1e1da7a963007d03a4e0e9a9e0ff17990bb1608d
-- 
2.41.0




Re: [PATCH v3 02/14] audio: Require AudioState in AUD_add_capture

2023-09-29 Thread BALATON Zoltan

On Fri, 29 Sep 2023, Paolo Bonzini wrote:

From: Martin Kletzander 

Since all callers require a valid audiodev this function can now safely
abort in case of missing AudioState.

Signed-off-by: Martin Kletzander 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
audio/audio.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 2f479657117..4332c4c6ce8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1876,10 +1876,9 @@ CaptureVoiceOut *AUD_add_capture(
struct capture_callback *cb;

if (!s) {
-if (!legacy_config) {
-dolog("Capturing without setting an audiodev is deprecated\n");
-}
-s = audio_init(NULL, NULL);
+error_setg(&error_abort,
+   "Capturing without setting an audiodev is not supported");
+abort();


This patch is still here but now Markus replied and quoted error.h that 
says this is wrong, error_abort should not be used in error_setg so this

should use error_report instead.

Regards,
BALATON Zoltan


}

if (!audio_get_pdo_out(s->dev)->mixing_engine) {





Re: [PATCH] ramfb: implement migration support

2023-09-29 Thread Gerd Hoffmann
On Wed, Sep 20, 2023 at 12:26:51PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Implementing RAMFB migration seems quite straightforward. Unfortunately,
> current QEMU didn't block migration when RAMFB was used.
> 
> Having an extra "ramfb" VMState doesn't seem to "break" migration
> forward compatibility. Surprisingly, missing sections are being ignored?
> 
> Backward compatiblity however will have this extra error:
> qemu: Unknown savevm section or instance 'ramfb' 0. Make sure that your 
> current VM setup matches your saved VM setup, including any hotplugged 
> devices.

I guess that is only fixable by making old machine types bug-compatible,
i.e. register the ramfb vmstate only for qemu 8.2+ machine types.

Otherwise looks fine to me.

take care,
  Gerd




Re: [PATCH] hw/display/ramfb: plug slight guest-triggerable leak on mode setting

2023-09-29 Thread Gerd Hoffmann
On Wed, Sep 27, 2023 at 05:45:25PM +0200, Laszlo Ersek wrote:
> On 9/19/23 15:19, Laszlo Ersek wrote:
> > The fw_cfg DMA write callback in ramfb prepares a new display surface in
> > QEMU; this new surface is put to use ("swapped in") upon the next display
> > update. At that time, the old surface (if any) is released.
> > 
> > If the guest triggers the fw_cfg DMA write callback at least twice between
> > two adjacent display updates, then the second callback (and further such
> > callbacks) will leak the previously prepared (but not yet swapped in)
> > display surface.

[ ... ]

> >  s->width = width;
> >  s->height = height;
> > +qemu_free_displaysurface(s->ds);
> >  s->ds = surface;
> >  }
> >  
> 
> Ping.

Reviewed-by: Gerd Hoffmann 

take care,
  Gerd




MAINTAINERS still leaves more files uncovered than I'd like

2023-09-29 Thread Markus Armbruster
Back in 2014 (time flies), I posted

Subject: MAINTAINERS leaves too many files uncovered
Message-ID: <87mw8rumhb@blackfin.pond.sub.org>
https://lore.kernel.org/qemu-devel/87mw8rumhb@blackfin.pond.sub.org/

I updated my findings in 2015, 2016 (at commit e00da552a0d), and 2018
(at v3.1.0-rc2).  This is another update, at commit 36e9aab3c56.

Unsurprisingly, the number of files in the tree

$ git-ls-files | wc -l

grows over time:

year  2014  2015  2016  2018  2023
# 3746  4387  4921  6461  9788

Looks exponential to me, doubling every seven years or so.

The number of .c files has grown more slowly:

year  2014  2015  2016  2018  2023
# 1836  1945  2132  2633  3588

The number of .c files not covered by MAINTAINERS

$ for i in `git-ls-files`; do [ "`scripts/get_maintainer.pl -f 
--no-git-fallback $i | grep -v '^qemu-devel@nongnu\.org'`" ] || echo $i; done 
>unmaintained-files
$ grep -c '\.c$' unmaintained-files

went down a lot after my first post, but has since flatlined:

year  2014  2015  2016  2018  2023
# 1066   461   402   259   246

It looks like we've pretty much stopped adding more unmaintained .c
files, i.e. cherry-picking the kernel's 13f1937ef33 (checkpatch: emit a
warning on file add/move/delete) as commit 4be6131e329 worked.

On the other hand, we're not making progress on the remaining old ones
anymore.


Currently unmaintained files by extension, with a long tail that doesn't
add up to anything interesting omitted:

#filespercent
ext unmaintained  total unmaintained
h   3292504 13%
c   2463588  7%
rst 103 240 43%
   95 716 13%
build52 211 25%
mak  47 104 45%
decode   44  77 57%
txt  23  53 43%
bin  20  27 75%
rom  15  15100%
inc  14 147 10%
cocci14  23 61%

Observations:

1. More unmaintained .h than .c.  Suggests sloppy accounting in
   MAINTAINERS.  Low-hanging fruit?

2. The number of unmaintained .rst and .txt suggests we're less
   interested in maintaining documentation.

3. The number of unmaintained .build and .mak suggests we're less
   interested in maintaining the build system.


Where are the remaining unmaintained files now?  Top-scoring
directories, files in sub-directories not counted:

$ sed 's,/[^/]*$,/,;s,^[^/]*$,./,' unmaintained-files | sort | uniq -c | 
sort -nr

  # directory
 84 include/qemu/
 56 util/
 50 pc-bios/
 46 tests/unit/
 45 tests/decode/
 40 include/standard-headers/linux/
 38 docs/system/
 35 pc-bios/keymaps/
 30 docs/devel/
 26 configs/targets/
 26 ./
 25 roms/
 22 target/i386/
 22 hw/core/
 19 scripts/
 19 include/exec/
 19 hw/display/
 14 tests/multiboot/
 14 scripts/coccinelle/
 14 include/sysemu/
 14 include/hw/
 12 po/
 12 hw/misc/
 12 hw/intc/
 12 docs/
 11 pc-bios/optionrom/
 11 docs/specs/
 10 tests/tcg/riscv64/
 10 tests/tcg/ppc64/
 10 disas/


Which unmaintained files are we still changing?  Unmaintained files
sorted by number of commits in the past year (since commit 5433af7697b):

$ for i in `cat unmaintained-files`; do echo -n "$i "; git-rev-list 
5433af7697b.. $i | wc -l; done | awk '{ printf "%7d %s\n", $2, $1 }' | sort -rn

186 MAINTAINERS
 65 target/i386/cpu.c
 54 qemu-options.hx
 45 target/i386/cpu.h
 22 tests/unit/test-bdrv-drain.c
 21 target/i386/ops_sse.h
 21 VERSION
 19 tests/unit/test-cutils.c
 19 include/monitor/hmp.h
 18 docs/system/arm/emulation.rst
 16 include/qemu/osdep.h
 16 hw/misc/meson.build
 15 hw/virtio/meson.build
 14 util/meson.build
 12 include/qemu/typedefs.h
 12 docs/about/removed-features.rst
 11 util/cutils.c
 10 tests/unit/test-block-iothread.c
[82 more with fewer than 10 changes]

Several of these we clearly need to cover in MAINTAINERS.


Full list of unmaintained files:

.dir-locals.el
.editorconfig
.exrc
.gdbinit
.git-blame-ignore-revs
.gitattributes
.gitignore
.gitlab/issue_templates/bug.md
.gitlab/issue_templates/feature_request.md
.gitmodules
.gitpublish
.mailmap
.patchew.yml
.readthedocs.yml
COPYING
COPYING.LIB
LICENSE
MAINTAINERS
README.rst
VERSION
accel/accel-softmmu.h
accel/dummy-cpus.c
accel/meson.build
accel/stubs/meson.build
accel/stubs/xen-stub.c
backends/confidential-guest-support.c
backends/dbus-vmstate1.xml
backends/meson.build
backends/trace-events
backends/trace.h
configs/devices/aarch64-softmmu/default.mak
configs/devices/aarch64-softmmu/minimal.mak
configs/devices/alpha-softmmu/default.mak
configs/devices/arm-softmmu/default.mak
configs/devices/cris-softmmu

Re: [PATCH v3 11/14] vt82c686: Support machine-default audiodev with fallback

2023-09-29 Thread BALATON Zoltan

On Fri, 29 Sep 2023, Paolo Bonzini wrote:

Signed-off-by: Paolo Bonzini 


This does not touch vt82c686 any more so maybe subject line could be 
changed to via-ac97: to be more specific. I like to keep all via stuff 
together with empty lines only between different components so I think 
empty lines added here are unnecessary but that's a minor thing.


Reviewed-by: BALATON Zoltan 

Regards,
BALATON Zoltan


---
hw/mips/fuloong2e.c | 15 ---
hw/ppc/pegasos2.c   | 12 ++--
2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..c6109633fee 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -295,9 +295,17 @@ static void mips_fuloong2e_init(MachineState *machine)
pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));

/* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
-  PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
-  TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+
+/* Set properties on individual devices before realizing the south bridge 
*/
+if (machine->audiodev) {
+dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ac97"));
+qdev_prop_set_string(dev, "audiodev", machine->audiodev);
+}
+
+pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
+
object_property_add_alias(OBJECT(machine), "rtc-time",
  object_resolve_path_component(OBJECT(pci_dev),
"rtc"),
@@ -337,6 +345,7 @@ static void mips_fuloong2e_machine_init(MachineClass *mc)
mc->default_ram_size = 256 * MiB;
mc->default_ram_id = "fuloong2e.ram";
mc->minimum_page_bits = 14;
+machine_add_audiodev_property(mc);
}

DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..3203a4a7289 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -180,8 +180,15 @@ static void pegasos2_init(MachineState *machine)
pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);

/* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA));
+
+/* Set properties on individual devices before realizing the south bridge 
*/
+if (machine->audiodev) {
+dev = PCI_DEVICE(object_resolve_path_component(via, "ac97"));
+qdev_prop_set_string(DEVICE(dev), "audiodev", machine->audiodev);
+}
+
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
for (i = 0; i < PCI_NUM_PINS; i++) {
pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
}
@@ -556,6 +563,7 @@ static void pegasos2_machine_class_init(ObjectClass *oc, 
void *data)
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7457_v1.2");
mc->default_ram_id = "pegasos2.ram";
mc->default_ram_size = 512 * MiB;
+machine_add_audiodev_property(mc);

vhc->cpu_in_nested = pegasos2_cpu_in_nested;
vhc->hypercall = pegasos2_hypercall;





Re: [PATCH v7 11/12] nbd/server: Prepare for per-request filtering of BLOCK_STATUS

2023-09-29 Thread Vladimir Sementsov-Ogievskiy

On 25.09.23 22:22, Eric Blake wrote:

The next commit will add support for the optional extension
NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
request that the server only return a subset of negotiated contexts,
rather than all contexts.  To make that task easier, this patch
populates the list of contexts to return on a per-command basis (for
now, identical to the full set of negotiated contexts).

Signed-off-by: Eric Blake


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir




Re: [PATCH v3 12/14] audio: forbid mixing default audiodev backend and -audiodev

2023-09-29 Thread BALATON Zoltan

On Fri, 29 Sep 2023, Paolo Bonzini wrote:

Now that all callers support setting an audiodev, forbid using the default
audiodev if any audiodev is defined on the command line.  To make the
detection easier make AUD_register_card() return false on error.


So this means that now qemu-system-ppc -M pegasos2 -audiodev sdl,id=au 
will give an error saying that audiodev must be specified but does not say 
it should be set on the machine so users will not know, as oppsed to now 
just getting a warning but things still working. This is not helpful. Why 
is this feature deprecated and what does removing bring that justifies 
this? Could we keep the more intuitive current behaviour and drop the 
deprecation instead unless removing this is needed for later patches as 
the current default handling is more convenient for command line users.


Regards,
BALATON Zoltan


Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
audio/audio.c| 16 ++--
audio/audio.h|  2 +-
hw/arm/omap2.c   |  2 +-
hw/audio/ac97.c  |  6 +-
hw/audio/adlib.c |  6 --
hw/audio/cs4231a.c   |  6 --
hw/audio/es1370.c|  5 -
hw/audio/gus.c   |  6 --
hw/audio/hda-codec.c |  5 -
hw/audio/lm4549.c|  8 +---
hw/audio/pcspk.c |  4 +---
hw/audio/sb16.c  |  6 --
hw/audio/via-ac97.c  |  6 --
hw/audio/wm8750.c|  5 -
hw/display/xlnx_dp.c |  6 --
hw/input/tsc210x.c   |  2 +-
hw/usb/dev-audio.c   |  5 -
17 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 9da2eaece03..f0788345bf8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1803,15 +1803,17 @@ static AudioState *audio_init(Audiodev *dev)
return s;
}

-void AUD_register_card (const char *name, QEMUSoundCard *card)
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
{
if (!card->state) {
+if (!QSIMPLEQ_EMPTY(&audiodevs)) {
+error_setg(errp, "No audiodev specified for %s", name);
+error_append_hint(errp, "Perhaps you wanted to set audiodev=%s?",
+  QSIMPLEQ_FIRST(&audiodevs)->dev->id);
+return false;
+}
+
if (!QTAILQ_EMPTY(&audio_states)) {
-if (!legacy_config) {
-dolog("Device %s: audiodev default parameter is deprecated, please 
"
-  "specify audiodev=%s\n", name,
-  QTAILQ_FIRST(&audio_states)->dev->id);
-}
card->state = QTAILQ_FIRST(&audio_states);
} else {
if (QSIMPLEQ_EMPTY(&default_audiodevs)) {
@@ -1824,6 +1826,8 @@ void AUD_register_card (const char *name, QEMUSoundCard 
*card)
card->name = g_strdup (name);
memset (&card->entries, 0, sizeof (card->entries));
QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
+
+return true;
}

void AUD_remove_card (QEMUSoundCard *card)
diff --git a/audio/audio.h b/audio/audio.h
index 34df8962a66..70b264d897d 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp {
void AUD_vlog (const char *cap, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 
0);
void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3);

-void AUD_register_card (const char *name, QEMUSoundCard *card);
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp);
void AUD_remove_card (QEMUSoundCard *card);
CaptureVoiceOut *AUD_add_capture(
AudioState *s,
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 41b1f596dca..f170728e7ec 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -614,7 +614,7 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
s->codec.card.name = g_strdup(current_machine->audiodev);
s->codec.card.state = audio_state_by_name(s->codec.card.name, 
&error_fatal);
}
-AUD_register_card("OMAP EAC", &s->codec.card);
+AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal);

memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
  omap_l4_region_size(ta, 0));
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index c2a5ce062a1..6a7a2dc80c4 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1273,6 +1273,10 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
AC97LinkState *s = AC97(dev);
uint8_t *c = s->dev.config;

+if (!AUD_register_card ("ac97", &s->card, errp)) {
+return;
+}
+
/* TODO: no need to override */
c[PCI_COMMAND] = 0x00;  /* pcicmd pci command rw, ro */
c[PCI_COMMAND + 1] = 0x00;
@@ -1306,7 +1310,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
  "ac97-nabm", 256);
pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
-AUD_register_card("ac97", &s->card);
+
ac97_on_reset(DEVICE(s));
}

diff --git a/hw/audi

  1   2   3   >