On Tue, 12 Apr 2022 at 17:49, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> The sysbus floppy controllers (devices sysbus-fdc and sun-fdtwo)
> don't support DMA. The core floppy controller code expects this to
> be indicated by setting FDCtrl::dma_chann to -1. This used to be
> done in the device instance_init functions sysbus_fdc_initfn() and
> sun4m_fdc_initfn(), but in commit 1430759ec3e we refactored this code
> and accidentally lost the setting of dma_chann.
>
> For sysbus-fdc this has no ill effects because we were redundantly
> also setting dma_chann in fdctrl_init_sysbus(), but for sun-fdtwo
> this means that guests which try to enable DMA on the floppy
> controller will cause QEMU to crash because FDCtrl::dma is NULL.
>
> Set dma_chann to -1 in the common instance init, and remove the
> redundant code in fdctrl_init_sysbus() that is also setting it.
>
> There is a six-year-old FIXME comment in the jazz board code to the
> effect that in theory it should support doing DMA via a custom DMA
> controller. If anybody ever chooses to fix that they can do it by
> adding support for setting both FDCtrl::dma_chann and FDCtrl::dma.
> (A QOM link property 'dma-controller' on the sysbus device which can
> be set to an instance of IsaDmaClass is probably the way to go.)
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/958
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> -void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
> - hwaddr mmio_base, DriveInfo **fds)
> +void fdctrl_init_sysbus(qemu_irq irq, hwaddr mmio_base, DriveInfo **fds)
> {
> FDCtrl *fdctrl;
> DeviceState *dev;
> @@ -105,7 +104,6 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
> dev = qdev_new("sysbus-fdc");
> sys = SYSBUS_FDC(dev);
> fdctrl = &sys->state;
> - fdctrl->dma_chann = dma_chann; /* FIXME */
> sbd = SYS_BUS_DEVICE(dev);
> sysbus_realize_and_unref(sbd, &error_fatal);
> sysbus_connect_irq(sbd, 0, irq);
Just noticed that deleting this line removes the only use
of the 'fdctrl' local in this function, which then means
we can delete it. I'll send a v2 that does that.
-- PMM
