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

Bernhard Beschow posted 11 patches 8 months, 1 week ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, David Hildenbrand <david@redhat.com>
[PATCH v5 11/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions
Posted by Bernhard Beschow 8 months, 1 week ago
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>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/vt82c686.c | 65 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 10 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d3e0f6d01f..485bb685b7 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,17 @@ 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);
+
+    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
+    for (int 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 +382,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 +427,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 +502,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 +565,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 +572,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;
-- 
2.43.0
Re: [PATCH v5 11/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
On 14/1/24 13:39, 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>
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/isa/vt82c686.c | 65 +++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 55 insertions(+), 10 deletions(-)


> +static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
> +{
> +    ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
> +

        memory_region_transaction_begin();

> +    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
> +    for (int 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));

        memory_region_transaction_commit();

> +}
> +
Re: [PATCH v5 11/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions
Posted by BALATON Zoltan 1 month, 3 weeks ago
On Mon, 29 Jul 2024, Philippe Mathieu-Daudé wrote:
> On 14/1/24 13:39, 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>
>> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/isa/vt82c686.c | 65 +++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 55 insertions(+), 10 deletions(-)
>
>
>> +static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
>> +{
>> +    ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
>> +
>
>       memory_region_transaction_begin();
>
>> +    isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
>> +    for (int 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));
>
>       memory_region_transaction_commit();

Is a transaction needed here? We're just enable/disable independent memory 
regions here and I don't think this function can be interrupted but having 
a transaction does not hurt but I don't understand why we would need it.

Regards,
BALATON Zoltan