Did you see my patches at
https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg02895.html ?
Patches 01/11 and 02/11 prevent the disturbing error message from
audio_calloc and later patches remove the audio_calloc function.
Oh, I missed these. Thank you for pointing it out.
I think the subject of your patch isn't correct. Your patch doesn't fix
an abort in audio_calloc. In
https://gitlab.com/qemu-project/qemu/-/issues/1393 you correctly notice
this was already fixed.
Fair enough.
> Section 5.10.2 of the AC97 specification (https://hands.com/~lkcl/ac97_r23.pdf)
> shows the feasibility to support for rates other than 48kHZ. Specifically,
> AC97_PCM_Front_DAC_Rate (reg 2Ch) should be from 8kHZ to 48kHZ.
I think you misread section 5.10.2 of the AC97 Revision 2.3
specification. Section 5.10 is about S/PDIF concurrency. It doesn't say
anything about the lowest sample rate limit without concurrent S/PDIF
transmission. The emulated SigmaTel STAC9700 codec doesn't even have a
S/PDIF output. But I have an example for sample rates lower than 8kHz.
The Texas Instruments LM4546B is an AC97 codec which supports sample
rates from 4kHz - 48kHz.
Thank you for pointing it out! Now, I understand better!
The STAC9700 is a 48kHz fixed rate codec. You won't find anything about
the highest or lowest sample rate in its data sheet. Someone added the
VRA and VRM modes in QEMU and the guest drivers don't seem to mind.
I would like to keep the ability to select a 1Hz sample rate, as there
is no reason to artificially limit the lowest supported sample rate. See
https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg03987.html
I would support a patch to limit the VRA and VRM modes to the highest
supported rate of 48kHz. This is a technical limit for single data rate.
> Before Volker Rümelin fixed it in 12f4abf6a245 and 0cbc8bd4694f, an adversary
> could leverage this to crash QEMU.
>
> Fixes: e5c9a13e2670 ("PCI AC97 emulation by malc.")
> Reported-by: Volker Rümelin<vr_qemu@t-online.de>
> Reported-by: Qiang Liu<cyruscyliu@gmail.com>
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1393
> Signed-off-by: Qiang Liu<cyruscyliu@gmail.com>
> ---
> hw/audio/ac97.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
> index be2dd701a4..826411e462 100644
> --- a/hw/audio/ac97.c
> +++ b/hw/audio/ac97.c
> @@ -625,9 +625,14 @@ static void nam_writew(void *opaque, uint32_t addr, uint32_t val)
> break;
> case AC97_PCM_Front_DAC_Rate:
> if (mixer_load(s, AC97_Extended_Audio_Ctrl_Stat) & EACS_VRA) {
> - mixer_store(s, addr, val);
> - dolog("Set front DAC rate to %d\n", val);
> - open_voice(s, PO_INDEX, val);
> + if (val >= 8000 && val <= 48000) {
> + mixer_store(s, addr, val);
> + dolog("Set front DAC rate to %d\n", val);
> + open_voice(s, PO_INDEX, val);
> + } else {
> + dolog("Attempt to set front DAC rate to %d, but valid is"
> + "8-48kHZ\n", val);
This is not correct. If you limit the sample rate, you should echo back
the closest supported sample rate. See AC'97 2.3 Section 5.8.3. It's not
a guest error if the guest writes an unsupported sample rate to the
Audio Sample Rate Control Registers, which means it's also not necessary
to log this.
With best regards,
Volker
Best,
Qiang