Re: [PATCH 4/4] hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
On 4/27/21 7:34 PM, John Snow wrote: > On 4/15/21 6:23 AM, Philippe Mathieu-Daudé wrote: >> Some machines use floppy controllers via the SysBus interface, >> and don't need to pull in all the SysBus code. >> Extract the SysBus specific code to a new unit: fdc-sysbus.c, >> and add a new Kconfig symbol: "FDC_SYSBUS". >> >> Signed-off-by: Philippe Mathieu-Daudé > > LGTM, but again you'll want someone to review the Kconfig changes. It > looks reasonable to me at a glance, but I just don't know what I don't > know there. > > I'm trusting you somewhat that you've audited the proper dependencies > for each subsystem and that these have been accurately described via > Kconfig -- my knowledge of non-x86 platforms is a bit meager, so I am > relying on CI to tell me if this breaks anything I care about. The way I test these Kconfig changes is configuring with --without-default-devices, and build/test for each machine doing: $ echo $MACHINE=y > default-configs/devices/$arch-softmmu.mak (I overwrite because currently we don't support having a base config different than the default-configs one). For example for the SPARCbook machine: $ echo CONFIG_SUN4M=y > default-configs/devices/sparc-softmmu.mak $ ninja qemu-system-sparc $ qemu-system-sparc -M SPARCbook -S Or for the Jazz machine: $ echo CONFIG_JAZZ=y > default-configs/devices/mips64el-softmmu.mak $ echo CONFIG_SEMIHOSTING=y >> default-configs/devices/mips64el-softmmu.mak $ ninja qemu-system-mips64el $ ./qemu-system-mips64el -M pica61 -S (The CONFIG_SEMIHOSTING is a particular kludge pending another series) But unfortunately there are predating bugs breaking this testing. I'll add this information in the respin's cover. > Would love to get an ACK from Mark Cave-Ayland and Hervé Poussineau if > possible, but if they're not available to take a quick peek, we'll try > to get this in closer to the beginning of the dev window to maximize > problem-finding time. The sun4m maintainer is Artyom. > Reviewed-by: John Snow Thanks! > >> --- >> hw/block/fdc-sysbus.c | 252 ++ >> hw/block/fdc.c | 220 >> MAINTAINERS | 1 + >> hw/block/Kconfig | 4 + >> hw/block/meson.build | 1 + >> hw/block/trace-events | 2 + >> hw/mips/Kconfig | 2 +- >> hw/sparc/Kconfig | 2 +- >> 8 files changed, 262 insertions(+), 222 deletions(-) >> create mode 100644 hw/block/fdc-sysbus.c
Re: [PATCH 4/4] hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
On 4/15/21 6:23 AM, Philippe Mathieu-Daudé wrote: Some machines use floppy controllers via the SysBus interface, and don't need to pull in all the SysBus code. Extract the SysBus specific code to a new unit: fdc-sysbus.c, and add a new Kconfig symbol: "FDC_SYSBUS". Signed-off-by: Philippe Mathieu-Daudé LGTM, but again you'll want someone to review the Kconfig changes. It looks reasonable to me at a glance, but I just don't know what I don't know there. I'm trusting you somewhat that you've audited the proper dependencies for each subsystem and that these have been accurately described via Kconfig -- my knowledge of non-x86 platforms is a bit meager, so I am relying on CI to tell me if this breaks anything I care about. Would love to get an ACK from Mark Cave-Ayland and Hervé Poussineau if possible, but if they're not available to take a quick peek, we'll try to get this in closer to the beginning of the dev window to maximize problem-finding time. Reviewed-by: John Snow --- hw/block/fdc-sysbus.c | 252 ++ hw/block/fdc.c| 220 MAINTAINERS | 1 + hw/block/Kconfig | 4 + hw/block/meson.build | 1 + hw/block/trace-events | 2 + hw/mips/Kconfig | 2 +- hw/sparc/Kconfig | 2 +- 8 files changed, 262 insertions(+), 222 deletions(-) create mode 100644 hw/block/fdc-sysbus.c diff --git a/hw/block/fdc-sysbus.c b/hw/block/fdc-sysbus.c new file mode 100644 index 000..71755fd6ae4 --- /dev/null +++ b/hw/block/fdc-sysbus.c @@ -0,0 +1,252 @@ +/* + * QEMU Floppy disk emulator (Intel 82078) + * + * Copyright (c) 2003, 2007 Jocelyn Mayer + * Copyright (c) 2008 Hervé Poussineau + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "qom/object.h" +#include "hw/sysbus.h" +#include "hw/block/fdc.h" +#include "migration/vmstate.h" +#include "fdc-internal.h" +#include "trace.h" + +#define TYPE_SYSBUS_FDC "base-sysbus-fdc" +typedef struct FDCtrlSysBusClass FDCtrlSysBusClass; +typedef struct FDCtrlSysBus FDCtrlSysBus; +DECLARE_OBJ_CHECKERS(FDCtrlSysBus, FDCtrlSysBusClass, + SYSBUS_FDC, TYPE_SYSBUS_FDC) + +struct FDCtrlSysBusClass { +/*< private >*/ +SysBusDeviceClass parent_class; +/*< public >*/ + +bool use_strict_io; +}; + +struct FDCtrlSysBus { +/*< private >*/ +SysBusDevice parent_obj; +/*< public >*/ + +struct FDCtrl state; +}; + +static uint64_t fdctrl_read_mem(void *opaque, hwaddr reg, unsigned ize) +{ +return fdctrl_read(opaque, (uint32_t)reg); +} + +static void fdctrl_write_mem(void *opaque, hwaddr reg, + uint64_t value, unsigned size) +{ +fdctrl_write(opaque, (uint32_t)reg, value); +} + +static const MemoryRegionOps fdctrl_mem_ops = { +.read = fdctrl_read_mem, +.write = fdctrl_write_mem, +.endianness = DEVICE_NATIVE_ENDIAN, +}; + +static const MemoryRegionOps fdctrl_mem_strict_ops = { +.read = fdctrl_read_mem, +.write = fdctrl_write_mem, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 1, +.max_access_size = 1, +}, +}; + +static void fdctrl_external_reset_sysbus(DeviceState *d) +{ +FDCtrlSysBus *sys = SYSBUS_FDC(d); +FDCtrl *s = &sys->state; + +fdctrl_reset(s, 0); +} + +static void fdctrl_handle_tc(void *opaque, int irq, int level) +{ +trace_fdctrl_tc_pulse(level); +} + +void fdctrl_init_sysbus(qemu_irq irq, int dma_chann, +hwaddr mmio_base, DriveInfo **fds) +{ +FDCtrl *fdctrl; +DeviceState *dev; +SysBusDevice *sbd; +FDCtrlSysBus *sys; + +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_
[PATCH 4/4] hw/block/fdc: Extract SysBus floppy controllers to fdc-sysbus.c
Some machines use floppy controllers via the SysBus interface, and don't need to pull in all the SysBus code. Extract the SysBus specific code to a new unit: fdc-sysbus.c, and add a new Kconfig symbol: "FDC_SYSBUS". Signed-off-by: Philippe Mathieu-Daudé --- hw/block/fdc-sysbus.c | 252 ++ hw/block/fdc.c| 220 MAINTAINERS | 1 + hw/block/Kconfig | 4 + hw/block/meson.build | 1 + hw/block/trace-events | 2 + hw/mips/Kconfig | 2 +- hw/sparc/Kconfig | 2 +- 8 files changed, 262 insertions(+), 222 deletions(-) create mode 100644 hw/block/fdc-sysbus.c diff --git a/hw/block/fdc-sysbus.c b/hw/block/fdc-sysbus.c new file mode 100644 index 000..71755fd6ae4 --- /dev/null +++ b/hw/block/fdc-sysbus.c @@ -0,0 +1,252 @@ +/* + * QEMU Floppy disk emulator (Intel 82078) + * + * Copyright (c) 2003, 2007 Jocelyn Mayer + * Copyright (c) 2008 Hervé Poussineau + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "qom/object.h" +#include "hw/sysbus.h" +#include "hw/block/fdc.h" +#include "migration/vmstate.h" +#include "fdc-internal.h" +#include "trace.h" + +#define TYPE_SYSBUS_FDC "base-sysbus-fdc" +typedef struct FDCtrlSysBusClass FDCtrlSysBusClass; +typedef struct FDCtrlSysBus FDCtrlSysBus; +DECLARE_OBJ_CHECKERS(FDCtrlSysBus, FDCtrlSysBusClass, + SYSBUS_FDC, TYPE_SYSBUS_FDC) + +struct FDCtrlSysBusClass { +/*< private >*/ +SysBusDeviceClass parent_class; +/*< public >*/ + +bool use_strict_io; +}; + +struct FDCtrlSysBus { +/*< private >*/ +SysBusDevice parent_obj; +/*< public >*/ + +struct FDCtrl state; +}; + +static uint64_t fdctrl_read_mem(void *opaque, hwaddr reg, unsigned ize) +{ +return fdctrl_read(opaque, (uint32_t)reg); +} + +static void fdctrl_write_mem(void *opaque, hwaddr reg, + uint64_t value, unsigned size) +{ +fdctrl_write(opaque, (uint32_t)reg, value); +} + +static const MemoryRegionOps fdctrl_mem_ops = { +.read = fdctrl_read_mem, +.write = fdctrl_write_mem, +.endianness = DEVICE_NATIVE_ENDIAN, +}; + +static const MemoryRegionOps fdctrl_mem_strict_ops = { +.read = fdctrl_read_mem, +.write = fdctrl_write_mem, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 1, +.max_access_size = 1, +}, +}; + +static void fdctrl_external_reset_sysbus(DeviceState *d) +{ +FDCtrlSysBus *sys = SYSBUS_FDC(d); +FDCtrl *s = &sys->state; + +fdctrl_reset(s, 0); +} + +static void fdctrl_handle_tc(void *opaque, int irq, int level) +{ +trace_fdctrl_tc_pulse(level); +} + +void fdctrl_init_sysbus(qemu_irq irq, int dma_chann, +hwaddr mmio_base, DriveInfo **fds) +{ +FDCtrl *fdctrl; +DeviceState *dev; +SysBusDevice *sbd; +FDCtrlSysBus *sys; + +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); +sysbus_mmio_map(sbd, 0, mmio_base); + +fdctrl_init_drives(&sys->state.bus, fds); +} + +void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base, + DriveInfo **fds, qemu_irq *fdc_tc) +{ +DeviceState *dev; +FDCtrlSysBus *sys; + +dev = qdev_new("sun-fdtwo"); +sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); +sys = SYSBUS_FDC(dev); +sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq); +sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, io_base); +*fdc_tc = qdev_get_gpio_in(dev, 0); + +fdctrl_init_drives(&sys->state.bus, fds); +} + +static void sysbus_fdc_common_initfn(Object *obj) +{ +DeviceState *dev = DEVICE(obj); +FDCtrlSysBusClass *sbdc = SYSBUS_FDC_GET_CLASS(o