[PATCH] hw/audio/lm4549: Don't try to open a zero-frequency audio voice

[PATCH] hw/audio/lm4549: Don't try to open a zero-frequency audio voice

[Peter Maydell]

If the guest incorrectly programs the lm4549 audio chip with a zero
frequency, we will pass this to AUD_open_out(), which will complain:

   A bug was just triggered in AUD_open_out
   Save all your work and restart without audio
   I am sorry
   Context:
   audio: frequency=0 nchannels=2 fmt=S16 endianness=little

The datasheet doesn't say what we should do here, only that the valid
range for the freqency is 8000 to 48000 Hz; we choose to log the
guest error and ignore an attempt to change the DAC rate to something
outside the valid range.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/410
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This bug has been around for so long and is a weird edge case whose
only effect is to print a debug message, so it doesn't really seem
worth cc'ing stable on.
---
 hw/audio/lm4549.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
index 745441bd790..bf711c49c04 100644
--- a/hw/audio/lm4549.c
+++ b/hw/audio/lm4549.c
@@ -15,6 +15,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/hw.h"
+#include "qemu/log.h"
 #include "qemu/audio.h"
 #include "lm4549.h"
 #include "migration/vmstate.h"
@@ -179,9 +180,23 @@ void lm4549_write(lm4549_state *s,
         break;
 
     case LM4549_PCM_Front_DAC_Rate:
-        regfile[LM4549_PCM_Front_DAC_Rate] = value;
         DPRINTF("DAC rate change = %i\n", value);
 
+        /*
+         * Valid sample rates are 4kHz to 48kHz.
+         * The datasheet doesn't say what happens if you try to
+         * set the frequency to zero. AUD_open_out() will print
+         * a bug message if we pass it a zero frequency, so just
+         * ignore attempts to set the DAC frequency to zero.
+         */
+        if (value < 4000 || value > 48000) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: DAC sample rate %d Hz is invalid, ignoring it\n",
+                          __func__, value);
+            break;
+        }
+        regfile[LM4549_PCM_Front_DAC_Rate] = value;
+
         /* Re-open a voice with the new sample rate */
         struct audsettings as;
         as.freq = value;
-- 
2.43.0

Re: [PATCH] hw/audio/lm4549: Don't try to open a zero-frequency audio voice

[BALATON Zoltan]

On Fri, 7 Nov 2025, Peter Maydell wrote:
> If the guest incorrectly programs the lm4549 audio chip with a zero
> frequency, we will pass this to AUD_open_out(), which will complain:
>
>   A bug was just triggered in AUD_open_out
>   Save all your work and restart without audio
>   I am sorry
>   Context:
>   audio: frequency=0 nchannels=2 fmt=S16 endianness=little
>
> The datasheet doesn't say what we should do here, only that the valid
> range for the freqency is 8000 to 48000 Hz; we choose to log the
> guest error and ignore an attempt to change the DAC rate to something
> outside the valid range.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/410
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This bug has been around for so long and is a weird edge case whose
> only effect is to print a debug message, so it doesn't really seem
> worth cc'ing stable on.
> ---
> hw/audio/lm4549.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
> index 745441bd790..bf711c49c04 100644
> --- a/hw/audio/lm4549.c
> +++ b/hw/audio/lm4549.c
> @@ -15,6 +15,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/hw.h"
> +#include "qemu/log.h"
> #include "qemu/audio.h"
> #include "lm4549.h"
> #include "migration/vmstate.h"
> @@ -179,9 +180,23 @@ void lm4549_write(lm4549_state *s,
>         break;
>
>     case LM4549_PCM_Front_DAC_Rate:
> -        regfile[LM4549_PCM_Front_DAC_Rate] = value;
>         DPRINTF("DAC rate change = %i\n", value);
>
> +        /*
> +         * Valid sample rates are 4kHz to 48kHz.

Commit message says minimum is 8kHz. One of these is likely incorrect.
Maybe a typo in commit message?

Regards,
BALATON Zoltan

> +         * The datasheet doesn't say what happens if you try to
> +         * set the frequency to zero. AUD_open_out() will print
> +         * a bug message if we pass it a zero frequency, so just
> +         * ignore attempts to set the DAC frequency to zero.
> +         */
> +        if (value < 4000 || value > 48000) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: DAC sample rate %d Hz is invalid, ignoring it\n",
> +                          __func__, value);
> +            break;
> +        }
> +        regfile[LM4549_PCM_Front_DAC_Rate] = value;
> +
>         /* Re-open a voice with the new sample rate */
>         struct audsettings as;
>         as.freq = value;
>

Re: [PATCH] hw/audio/lm4549: Don't try to open a zero-frequency audio voice

[Peter Maydell]

On Sat, 8 Nov 2025 at 00:22, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Fri, 7 Nov 2025, Peter Maydell wrote:
> > If the guest incorrectly programs the lm4549 audio chip with a zero
> > frequency, we will pass this to AUD_open_out(), which will complain:
> >
> >   A bug was just triggered in AUD_open_out
> >   Save all your work and restart without audio
> >   I am sorry
> >   Context:
> >   audio: frequency=0 nchannels=2 fmt=S16 endianness=little
> >
> > The datasheet doesn't say what we should do here, only that the valid
> > range for the freqency is 8000 to 48000 Hz; we choose to log the
> > guest error and ignore an attempt to change the DAC rate to something
> > outside the valid range.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/410
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > This bug has been around for so long and is a weird edge case whose
> > only effect is to print a debug message, so it doesn't really seem
> > worth cc'ing stable on.
> > ---
> > hw/audio/lm4549.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
> > index 745441bd790..bf711c49c04 100644
> > --- a/hw/audio/lm4549.c
> > +++ b/hw/audio/lm4549.c
> > @@ -15,6 +15,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "hw/hw.h"
> > +#include "qemu/log.h"
> > #include "qemu/audio.h"
> > #include "lm4549.h"
> > #include "migration/vmstate.h"
> > @@ -179,9 +180,23 @@ void lm4549_write(lm4549_state *s,
> >         break;
> >
> >     case LM4549_PCM_Front_DAC_Rate:
> > -        regfile[LM4549_PCM_Front_DAC_Rate] = value;
> >         DPRINTF("DAC rate change = %i\n", value);
> >
> > +        /*
> > +         * Valid sample rates are 4kHz to 48kHz.
>
> Commit message says minimum is 8kHz. One of these is likely incorrect.
> Maybe a typo in commit message?

Whoops; yes, you're right, the commit message is the one
that's wrong. The datasheet for this is at
https://www.ti.com/lit/ds/symlink/lm4549b.pdf
and it says 4kHz.

-- PMM

Re: [PATCH] hw/audio/lm4549: Don't try to open a zero-frequency audio voice

[Philippe Mathieu-Daudé]

On 7/11/25 16:41, Peter Maydell wrote:
> If the guest incorrectly programs the lm4549 audio chip with a zero
> frequency, we will pass this to AUD_open_out(), which will complain:
> 
>     A bug was just triggered in AUD_open_out
>     Save all your work and restart without audio
>     I am sorry
>     Context:
>     audio: frequency=0 nchannels=2 fmt=S16 endianness=little
> 
> The datasheet doesn't say what we should do here, only that the valid
> range for the freqency is 8000 to 48000 Hz; we choose to log the
> guest error and ignore an attempt to change the DAC rate to something
> outside the valid range.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/410
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This bug has been around for so long and is a weird edge case whose
> only effect is to print a debug message, so it doesn't really seem
> worth cc'ing stable on.
> ---
>   hw/audio/lm4549.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)

With s/8000/4000/ in description:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>