Hi Volker,
> 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_q...@t-online.de> > > Reported-by: Qiang Liu<cyruscy...@gmail.com> > > Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1393 > > Signed-off-by: Qiang Liu<cyruscy...@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 > I rethink and want to say that a low sample rate could be a problem. Have checked the missed patch https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg02895.html, I think you have addressed my concern. Thank all your suggestions again! Let's close this discussion and I will close the issue. Best, Qiang
