On Tue, 19 Dec 2023, Bernhard Beschow wrote:
Am 19. Dezember 2023 00:26:15 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
On Mon, 18 Dec 2023, 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 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 -- or in case of pegasos2 on board code if
no -bios is given -- to configure the functions accordingly.
Pegasos2 emulates firmware when no -bios is given, this was explained in
previos commit so maybe not needed to be explained it here again so you could
drop the comment between -- -- but I don't mind.
Signed-off-by: Bernhard Beschow <shen...@gmail.com>
---
hw/isa/vt82c686.c | 121 ++++++++++++++++++++++++++++++++++------------
1 file changed, 90 insertions(+), 31 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9c2333a277..be202d23cf 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"
@@ -343,6 +346,35 @@ static const TypeInfo via_superio_info = {
#define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
+static void vt82c686b_superio_update(ViaSuperIOState *s)
+{
+ isa_parallel_set_enabled(s->superio.parallel[0],
+ (s->regs[0xe2] & 0x3) != 3);
+ isa_serial_set_enabled(s->superio.serial[0], s->regs[0xe2] & BIT(2));
+ isa_serial_set_enabled(s->superio.serial[1], s->regs[0xe2] & BIT(3));
+ isa_fdc_set_enabled(s->superio.floppy, s->regs[0xe2] & BIT(4));
+
+ isa_fdc_set_iobase(s->superio.floppy, (s->regs[0xe3] & 0xfc) << 2);
+ isa_parallel_set_iobase(s->superio.parallel[0], s->regs[0xe6] << 2);
+ isa_serial_set_iobase(s->superio.serial[0], (s->regs[0xe7] & 0xfe) << 2);
+ isa_serial_set_iobase(s->superio.serial[1], (s->regs[0xe8] & 0xfe) << 2);
+}
I wonder if some code duplication could be saved by adding a shared
via_superio_update() for this further up in the abstract via-superio class
instead of this method and vt8231_superio_update() below. This common method in
abstract class would need to handle the differences which seem to be reg
addresses offset by 0x10 and VT8231 having only 1 serial port. These could
either be handled by adding function parameters or fields to ViaSuperIOState
for this that the subclasses can set and the method check. (Such as reg
base=0xe2 for vt82c686 and 0xf2 for vt8231 and num_serial or similar for how
many ports are there then can have a for loop for those that would only run
once for vt8231).
Only the enable bits and the parallel port base address line up, the serial
port(s) and the floppy would need special treatment. Not worth it IMO.
+static int vmstate_vt82c686b_superio_post_load(void *opaque, int version_id)
+{
+ ViaSuperIOState *s = opaque;
+
+ vt82c686b_superio_update(s);
+
+ return 0;
You could lose some blank lines here. You seem to love them, half of your cover
letter is just blank lines :-)
Yes, I want people to see the light rather than a wall (of text) ;-)
Well, information is in the text and anything else just distracts from it.
I guess you can configure your editor to present text the way you like but
it's hard for me or others to get rid of hard formatting so it's better to
only add the needed text.
but I'm the opposite and like more code to fit in one screen even on todays
displays that are wider than tall. So this funciton would take less space
without blank lines. (Even the local variable may not be necessary as you don't
access any fields within and void * should just cast without a warning but for
spelling out the desired type as a reminder I'm ok with leaving that but no
excessive blank lines please if you don't mind that much.)
In this case I'll remove the s variable and the blank line above the return if
that's so important to you.
I think it's simper and more readable that way that's why it's important
to me.
Regards,
BALATON Zoltan
