Re: [PATCH for-7.1] hw/block/fdc-sysbus: Always mark sysbus floppy controllers as not having DMA

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 12 Apr 2022 at 17:49, Peter Maydell <peter.maydell@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.maydell@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