diff mbox series

[v4,11/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions

Message ID 20240106210531.140542-12-shentey@gmail.com
State New
Headers show
Series hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions | expand

Commit Message

Bernhard Beschow Jan. 6, 2024, 9:05 p.m. UTC
The VIA south bridges are able to relocate and toggle (enable or disable) their
SuperI/O functions. So far this is hardcoded such that all functions are always
enabled and are located at fixed addresses.

Some PC BIOSes seem to probe for I/O occupancy before activating such a function
and issue an error in case of a conflict. Since the functions are currently
enabled on reset, conflicts are always detected. Prevent that by implementing
relocation and toggling of the SuperI/O functions.

Note that all SuperI/O functions are now deactivated upon reset (except for
VT82C686B's serial ports where Fuloong 2e's rescue-yl seems to expect them to be
enabled by default). Rely on firmware to configure the functions accordingly.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c | 66 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 10 deletions(-)

Comments

BALATON Zoltan Jan. 7, 2024, 1:59 p.m. UTC | #1
On Sat, 6 Jan 2024, Bernhard Beschow wrote:
> The VIA south bridges are able to relocate and toggle (enable or disable) their
> SuperI/O functions. So far this is hardcoded such that all functions are always
> enabled and are located at fixed addresses.
>
> Some PC BIOSes seem to probe for I/O occupancy before activating such a function
> and issue an error in case of a conflict. Since the functions are currently
> enabled on reset, conflicts are always detected. Prevent that by implementing
> relocation and toggling of the SuperI/O functions.
>
> Note that all SuperI/O functions are now deactivated upon reset (except for
> VT82C686B's serial ports where Fuloong 2e's rescue-yl seems to expect them to be
> enabled by default). Rely on firmware to configure the functions accordingly.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/isa/vt82c686.c | 66 ++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 56 insertions(+), 10 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index d3e0f6d01f..9f62fb5964 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -15,6 +15,9 @@
>
> #include "qemu/osdep.h"
> #include "hw/isa/vt82c686.h"
> +#include "hw/block/fdc.h"
> +#include "hw/char/parallel-isa.h"
> +#include "hw/char/serial.h"
> #include "hw/pci/pci.h"
> #include "hw/qdev-properties.h"
> #include "hw/ide/pci.h"
> @@ -323,6 +326,18 @@ static uint64_t via_superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
>     return val;
> }
>
> +static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
> +{
> +    ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
> +    size_t i;

The expected value for i is 0 or 1 (maybe up to 3 sometimes it there are 
more serial ports in a chip). so why use such big type? This should just 
be int. Newly it's also allowed to declare it within the for so if you 
want that you could do so but I have no preference on that and declaring 
it here is also OK. Otherwise:

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> +
> +    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
> +    for (i = 0; i < ic->serial.count; i++) {
> +        isa_serial_set_enabled(s->superio.serial[i], data & BIT(i + 2));
> +    }
> +    isa_fdc_set_enabled(s->superio.floppy, data & BIT(4));
> +}
> +
> static void via_superio_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -368,7 +383,25 @@ static void vt82c686b_superio_cfg_write(void *opaque, hwaddr addr,
>     case 0xfd ... 0xff:
>         /* ignore write to read only registers */
>         return;
> -    /* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
> +    case 0xe2:
> +        data &= 0x1f;
> +        via_superio_devices_enable(sc, data);
> +        break;
> +    case 0xe3:
> +        data &= 0xfc;
> +        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
> +        break;
> +    case 0xe6:
> +        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
> +        break;
> +    case 0xe7:
> +        data &= 0xfe;
> +        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
> +        break;
> +    case 0xe8:
> +        data &= 0xfe;
> +        isa_serial_set_iobase(sc->superio.serial[1], data << 2);
> +        break;
>     default:
>         qemu_log_mask(LOG_UNIMP,
>                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
> @@ -395,9 +428,14 @@ static void vt82c686b_superio_reset(DeviceState *dev)
>     /* Device ID */
>     vt82c686b_superio_cfg_write(s, 0, 0xe0, 1);
>     vt82c686b_superio_cfg_write(s, 1, 0x3c, 1);
> -    /* Function select - all disabled */
> +    /*
> +     * Function select - only serial enabled
> +     * Fuloong 2e's rescue-yl prints to the serial console w/o enabling it. This
> +     * suggests that the serial ports are enabled by default, so override the
> +     * datasheet.
> +     */
>     vt82c686b_superio_cfg_write(s, 0, 0xe2, 1);
> -    vt82c686b_superio_cfg_write(s, 1, 0x03, 1);
> +    vt82c686b_superio_cfg_write(s, 1, 0x0f, 1);
>     /* Floppy ctrl base addr 0x3f0-7 */
>     vt82c686b_superio_cfg_write(s, 0, 0xe3, 1);
>     vt82c686b_superio_cfg_write(s, 1, 0xfc, 1);
> @@ -465,6 +503,21 @@ static void vt8231_superio_cfg_write(void *opaque, hwaddr addr,
>     case 0xfd:
>         /* ignore write to read only registers */
>         return;
> +    case 0xf2:
> +        data &= 0x17;
> +        via_superio_devices_enable(sc, data);
> +        break;
> +    case 0xf4:
> +        data &= 0xfe;
> +        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
> +        break;
> +    case 0xf6:
> +        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
> +        break;
> +    case 0xf7:
> +        data &= 0xfc;
> +        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
> +        break;
>     default:
>         qemu_log_mask(LOG_UNIMP,
>                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
> @@ -513,12 +566,6 @@ static void vt8231_superio_init(Object *obj)
>     VIA_SUPERIO(obj)->io_ops = &vt8231_superio_cfg_ops;
> }
>
> -static uint16_t vt8231_superio_serial_iobase(ISASuperIODevice *sio,
> -                                             uint8_t index)
> -{
> -        return 0x2f8; /* FIXME: This should be settable via registers f2-f4 */
> -}
> -
> static void vt8231_superio_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -526,7 +573,6 @@ static void vt8231_superio_class_init(ObjectClass *klass, void *data)
>
>     dc->reset = vt8231_superio_reset;
>     sc->serial.count = 1;
> -    sc->serial.get_iobase = vt8231_superio_serial_iobase;
>     sc->parallel.count = 1;
>     sc->ide.count = 0; /* emulated by via-ide */
>     sc->floppy.count = 1;
>
Bernhard Beschow Jan. 8, 2024, 7:53 p.m. UTC | #2
Am 7. Januar 2024 13:59:44 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sat, 6 Jan 2024, Bernhard Beschow wrote:
>> The VIA south bridges are able to relocate and toggle (enable or disable) their
>> SuperI/O functions. So far this is hardcoded such that all functions are always
>> enabled and are located at fixed addresses.
>> 
>> Some PC BIOSes seem to probe for I/O occupancy before activating such a function
>> and issue an error in case of a conflict. Since the functions are currently
>> enabled on reset, conflicts are always detected. Prevent that by implementing
>> relocation and toggling of the SuperI/O functions.
>> 
>> Note that all SuperI/O functions are now deactivated upon reset (except for
>> VT82C686B's serial ports where Fuloong 2e's rescue-yl seems to expect them to be
>> enabled by default). Rely on firmware to configure the functions accordingly.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c | 66 ++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 56 insertions(+), 10 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index d3e0f6d01f..9f62fb5964 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -15,6 +15,9 @@
>> 
>> #include "qemu/osdep.h"
>> #include "hw/isa/vt82c686.h"
>> +#include "hw/block/fdc.h"
>> +#include "hw/char/parallel-isa.h"
>> +#include "hw/char/serial.h"
>> #include "hw/pci/pci.h"
>> #include "hw/qdev-properties.h"
>> #include "hw/ide/pci.h"
>> @@ -323,6 +326,18 @@ static uint64_t via_superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
>>     return val;
>> }
>> 
>> +static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
>> +{
>> +    ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
>> +    size_t i;
>
>The expected value for i is 0 or 1 (maybe up to 3 sometimes it there are more serial ports in a chip). so why use such big type?

serial.count is of type size_t, that's why I chose it. Let me know if you still want an int, otherwise I'd leave it as is.

Best regards,
Bernhard

> This should just be int. Newly it's also allowed to declare it within the for so if you want that you could do so but I have no preference on that and declaring it here is also OK. Otherwise:
>
>Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>
>> +
>> +    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
>> +    for (i = 0; i < ic->serial.count; i++) {
>> +        isa_serial_set_enabled(s->superio.serial[i], data & BIT(i + 2));
>> +    }
>> +    isa_fdc_set_enabled(s->superio.floppy, data & BIT(4));
>> +}
>> +
>> static void via_superio_class_init(ObjectClass *klass, void *data)
>> {
>>     DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -368,7 +383,25 @@ static void vt82c686b_superio_cfg_write(void *opaque, hwaddr addr,
>>     case 0xfd ... 0xff:
>>         /* ignore write to read only registers */
>>         return;
>> -    /* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
>> +    case 0xe2:
>> +        data &= 0x1f;
>> +        via_superio_devices_enable(sc, data);
>> +        break;
>> +    case 0xe3:
>> +        data &= 0xfc;
>> +        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
>> +        break;
>> +    case 0xe6:
>> +        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
>> +        break;
>> +    case 0xe7:
>> +        data &= 0xfe;
>> +        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
>> +        break;
>> +    case 0xe8:
>> +        data &= 0xfe;
>> +        isa_serial_set_iobase(sc->superio.serial[1], data << 2);
>> +        break;
>>     default:
>>         qemu_log_mask(LOG_UNIMP,
>>                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
>> @@ -395,9 +428,14 @@ static void vt82c686b_superio_reset(DeviceState *dev)
>>     /* Device ID */
>>     vt82c686b_superio_cfg_write(s, 0, 0xe0, 1);
>>     vt82c686b_superio_cfg_write(s, 1, 0x3c, 1);
>> -    /* Function select - all disabled */
>> +    /*
>> +     * Function select - only serial enabled
>> +     * Fuloong 2e's rescue-yl prints to the serial console w/o enabling it. This
>> +     * suggests that the serial ports are enabled by default, so override the
>> +     * datasheet.
>> +     */
>>     vt82c686b_superio_cfg_write(s, 0, 0xe2, 1);
>> -    vt82c686b_superio_cfg_write(s, 1, 0x03, 1);
>> +    vt82c686b_superio_cfg_write(s, 1, 0x0f, 1);
>>     /* Floppy ctrl base addr 0x3f0-7 */
>>     vt82c686b_superio_cfg_write(s, 0, 0xe3, 1);
>>     vt82c686b_superio_cfg_write(s, 1, 0xfc, 1);
>> @@ -465,6 +503,21 @@ static void vt8231_superio_cfg_write(void *opaque, hwaddr addr,
>>     case 0xfd:
>>         /* ignore write to read only registers */
>>         return;
>> +    case 0xf2:
>> +        data &= 0x17;
>> +        via_superio_devices_enable(sc, data);
>> +        break;
>> +    case 0xf4:
>> +        data &= 0xfe;
>> +        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
>> +        break;
>> +    case 0xf6:
>> +        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
>> +        break;
>> +    case 0xf7:
>> +        data &= 0xfc;
>> +        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
>> +        break;
>>     default:
>>         qemu_log_mask(LOG_UNIMP,
>>                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
>> @@ -513,12 +566,6 @@ static void vt8231_superio_init(Object *obj)
>>     VIA_SUPERIO(obj)->io_ops = &vt8231_superio_cfg_ops;
>> }
>> 
>> -static uint16_t vt8231_superio_serial_iobase(ISASuperIODevice *sio,
>> -                                             uint8_t index)
>> -{
>> -        return 0x2f8; /* FIXME: This should be settable via registers f2-f4 */
>> -}
>> -
>> static void vt8231_superio_class_init(ObjectClass *klass, void *data)
>> {
>>     DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -526,7 +573,6 @@ static void vt8231_superio_class_init(ObjectClass *klass, void *data)
>> 
>>     dc->reset = vt8231_superio_reset;
>>     sc->serial.count = 1;
>> -    sc->serial.get_iobase = vt8231_superio_serial_iobase;
>>     sc->parallel.count = 1;
>>     sc->ide.count = 0; /* emulated by via-ide */
>>     sc->floppy.count = 1;
>>
BALATON Zoltan Jan. 8, 2024, 8:57 p.m. UTC | #3
On Mon, 8 Jan 2024, Bernhard Beschow wrote:
> Am 7. Januar 2024 13:59:44 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Sat, 6 Jan 2024, Bernhard Beschow wrote:
>>> The VIA south bridges are able to relocate and toggle (enable or disable) their
>>> SuperI/O functions. So far this is hardcoded such that all functions are always
>>> enabled and are located at fixed addresses.
>>>
>>> Some PC BIOSes seem to probe for I/O occupancy before activating such a function
>>> and issue an error in case of a conflict. Since the functions are currently
>>> enabled on reset, conflicts are always detected. Prevent that by implementing
>>> relocation and toggling of the SuperI/O functions.
>>>
>>> Note that all SuperI/O functions are now deactivated upon reset (except for
>>> VT82C686B's serial ports where Fuloong 2e's rescue-yl seems to expect them to be
>>> enabled by default). Rely on firmware to configure the functions accordingly.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>> hw/isa/vt82c686.c | 66 ++++++++++++++++++++++++++++++++++++++++-------
>>> 1 file changed, 56 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index d3e0f6d01f..9f62fb5964 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -15,6 +15,9 @@
>>>
>>> #include "qemu/osdep.h"
>>> #include "hw/isa/vt82c686.h"
>>> +#include "hw/block/fdc.h"
>>> +#include "hw/char/parallel-isa.h"
>>> +#include "hw/char/serial.h"
>>> #include "hw/pci/pci.h"
>>> #include "hw/qdev-properties.h"
>>> #include "hw/ide/pci.h"
>>> @@ -323,6 +326,18 @@ static uint64_t via_superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
>>>     return val;
>>> }
>>>
>>> +static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
>>> +{
>>> +    ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
>>> +    size_t i;
>>
>> The expected value for i is 0 or 1 (maybe up to 3 sometimes it there are more serial ports in a chip). so why use such big type?
>
> serial.count is of type size_t, that's why I chose it. Let me know if you still want an int, otherwise I'd leave it as is.

I think int would suffice here but it's not a big deal. The count is 
indeed size_t, not sure why. How many components were expected? But the 
practical value is just a few so larger type seems to be an overkill.

Regards,
BALATON Zoltan

> Best regards,
> Bernhard
>
>> This should just be int. Newly it's also allowed to declare it within the for so if you want that you could do so but I have no preference on that and declaring it here is also OK. Otherwise:
>>
>> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>>
>>> +
>>> +    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
>>> +    for (i = 0; i < ic->serial.count; i++) {
>>> +        isa_serial_set_enabled(s->superio.serial[i], data & BIT(i + 2));
>>> +    }
>>> +    isa_fdc_set_enabled(s->superio.floppy, data & BIT(4));
>>> +}
>>> +
>>> static void via_superio_class_init(ObjectClass *klass, void *data)
>>> {
>>>     DeviceClass *dc = DEVICE_CLASS(klass);
>>> @@ -368,7 +383,25 @@ static void vt82c686b_superio_cfg_write(void *opaque, hwaddr addr,
>>>     case 0xfd ... 0xff:
>>>         /* ignore write to read only registers */
>>>         return;
>>> -    /* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
>>> +    case 0xe2:
>>> +        data &= 0x1f;
>>> +        via_superio_devices_enable(sc, data);
>>> +        break;
>>> +    case 0xe3:
>>> +        data &= 0xfc;
>>> +        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
>>> +        break;
>>> +    case 0xe6:
>>> +        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
>>> +        break;
>>> +    case 0xe7:
>>> +        data &= 0xfe;
>>> +        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
>>> +        break;
>>> +    case 0xe8:
>>> +        data &= 0xfe;
>>> +        isa_serial_set_iobase(sc->superio.serial[1], data << 2);
>>> +        break;
>>>     default:
>>>         qemu_log_mask(LOG_UNIMP,
>>>                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
>>> @@ -395,9 +428,14 @@ static void vt82c686b_superio_reset(DeviceState *dev)
>>>     /* Device ID */
>>>     vt82c686b_superio_cfg_write(s, 0, 0xe0, 1);
>>>     vt82c686b_superio_cfg_write(s, 1, 0x3c, 1);
>>> -    /* Function select - all disabled */
>>> +    /*
>>> +     * Function select - only serial enabled
>>> +     * Fuloong 2e's rescue-yl prints to the serial console w/o enabling it. This
>>> +     * suggests that the serial ports are enabled by default, so override the
>>> +     * datasheet.
>>> +     */
>>>     vt82c686b_superio_cfg_write(s, 0, 0xe2, 1);
>>> -    vt82c686b_superio_cfg_write(s, 1, 0x03, 1);
>>> +    vt82c686b_superio_cfg_write(s, 1, 0x0f, 1);
>>>     /* Floppy ctrl base addr 0x3f0-7 */
>>>     vt82c686b_superio_cfg_write(s, 0, 0xe3, 1);
>>>     vt82c686b_superio_cfg_write(s, 1, 0xfc, 1);
>>> @@ -465,6 +503,21 @@ static void vt8231_superio_cfg_write(void *opaque, hwaddr addr,
>>>     case 0xfd:
>>>         /* ignore write to read only registers */
>>>         return;
>>> +    case 0xf2:
>>> +        data &= 0x17;
>>> +        via_superio_devices_enable(sc, data);
>>> +        break;
>>> +    case 0xf4:
>>> +        data &= 0xfe;
>>> +        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
>>> +        break;
>>> +    case 0xf6:
>>> +        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
>>> +        break;
>>> +    case 0xf7:
>>> +        data &= 0xfc;
>>> +        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
>>> +        break;
>>>     default:
>>>         qemu_log_mask(LOG_UNIMP,
>>>                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
>>> @@ -513,12 +566,6 @@ static void vt8231_superio_init(Object *obj)
>>>     VIA_SUPERIO(obj)->io_ops = &vt8231_superio_cfg_ops;
>>> }
>>>
>>> -static uint16_t vt8231_superio_serial_iobase(ISASuperIODevice *sio,
>>> -                                             uint8_t index)
>>> -{
>>> -        return 0x2f8; /* FIXME: This should be settable via registers f2-f4 */
>>> -}
>>> -
>>> static void vt8231_superio_class_init(ObjectClass *klass, void *data)
>>> {
>>>     DeviceClass *dc = DEVICE_CLASS(klass);
>>> @@ -526,7 +573,6 @@ static void vt8231_superio_class_init(ObjectClass *klass, void *data)
>>>
>>>     dc->reset = vt8231_superio_reset;
>>>     sc->serial.count = 1;
>>> -    sc->serial.get_iobase = vt8231_superio_serial_iobase;
>>>     sc->parallel.count = 1;
>>>     sc->ide.count = 0; /* emulated by via-ide */
>>>     sc->floppy.count = 1;
>>>
>
>
diff mbox series

Patch

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d3e0f6d01f..9f62fb5964 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -15,6 +15,9 @@ 
 
 #include "qemu/osdep.h"
 #include "hw/isa/vt82c686.h"
+#include "hw/block/fdc.h"
+#include "hw/char/parallel-isa.h"
+#include "hw/char/serial.h"
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/ide/pci.h"
@@ -323,6 +326,18 @@  static uint64_t via_superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
     return val;
 }
 
+static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
+{
+    ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
+    size_t i;
+
+    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
+    for (i = 0; i < ic->serial.count; i++) {
+        isa_serial_set_enabled(s->superio.serial[i], data & BIT(i + 2));
+    }
+    isa_fdc_set_enabled(s->superio.floppy, data & BIT(4));
+}
+
 static void via_superio_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -368,7 +383,25 @@  static void vt82c686b_superio_cfg_write(void *opaque, hwaddr addr,
     case 0xfd ... 0xff:
         /* ignore write to read only registers */
         return;
-    /* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
+    case 0xe2:
+        data &= 0x1f;
+        via_superio_devices_enable(sc, data);
+        break;
+    case 0xe3:
+        data &= 0xfc;
+        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
+        break;
+    case 0xe6:
+        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
+        break;
+    case 0xe7:
+        data &= 0xfe;
+        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
+        break;
+    case 0xe8:
+        data &= 0xfe;
+        isa_serial_set_iobase(sc->superio.serial[1], data << 2);
+        break;
     default:
         qemu_log_mask(LOG_UNIMP,
                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
@@ -395,9 +428,14 @@  static void vt82c686b_superio_reset(DeviceState *dev)
     /* Device ID */
     vt82c686b_superio_cfg_write(s, 0, 0xe0, 1);
     vt82c686b_superio_cfg_write(s, 1, 0x3c, 1);
-    /* Function select - all disabled */
+    /*
+     * Function select - only serial enabled
+     * Fuloong 2e's rescue-yl prints to the serial console w/o enabling it. This
+     * suggests that the serial ports are enabled by default, so override the
+     * datasheet.
+     */
     vt82c686b_superio_cfg_write(s, 0, 0xe2, 1);
-    vt82c686b_superio_cfg_write(s, 1, 0x03, 1);
+    vt82c686b_superio_cfg_write(s, 1, 0x0f, 1);
     /* Floppy ctrl base addr 0x3f0-7 */
     vt82c686b_superio_cfg_write(s, 0, 0xe3, 1);
     vt82c686b_superio_cfg_write(s, 1, 0xfc, 1);
@@ -465,6 +503,21 @@  static void vt8231_superio_cfg_write(void *opaque, hwaddr addr,
     case 0xfd:
         /* ignore write to read only registers */
         return;
+    case 0xf2:
+        data &= 0x17;
+        via_superio_devices_enable(sc, data);
+        break;
+    case 0xf4:
+        data &= 0xfe;
+        isa_serial_set_iobase(sc->superio.serial[0], data << 2);
+        break;
+    case 0xf6:
+        isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
+        break;
+    case 0xf7:
+        data &= 0xfc;
+        isa_fdc_set_iobase(sc->superio.floppy, data << 2);
+        break;
     default:
         qemu_log_mask(LOG_UNIMP,
                       "via_superio_cfg: unimplemented register 0x%x\n", idx);
@@ -513,12 +566,6 @@  static void vt8231_superio_init(Object *obj)
     VIA_SUPERIO(obj)->io_ops = &vt8231_superio_cfg_ops;
 }
 
-static uint16_t vt8231_superio_serial_iobase(ISASuperIODevice *sio,
-                                             uint8_t index)
-{
-        return 0x2f8; /* FIXME: This should be settable via registers f2-f4 */
-}
-
 static void vt8231_superio_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -526,7 +573,6 @@  static void vt8231_superio_class_init(ObjectClass *klass, void *data)
 
     dc->reset = vt8231_superio_reset;
     sc->serial.count = 1;
-    sc->serial.get_iobase = vt8231_superio_serial_iobase;
     sc->parallel.count = 1;
     sc->ide.count = 0; /* emulated by via-ide */
     sc->floppy.count = 1;