[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 4/8] vt82c686: Introduce abstract TYPE_VIA_ISA and base vt

From: BALATON Zoltan
Subject: Re: [PATCH v5 4/8] vt82c686: Introduce abstract TYPE_VIA_ISA and base vt82c686b_isa on it
Date: Tue, 9 Mar 2021 17:09:46 +0100 (CET)
User-agent: Alpine 2.03 (LMD 1266 2009-07-14)

On Tue, 9 Mar 2021, Dr. David Alan Gilbert wrote:
* David Gibson (david@gibson.dropbear.id.au) wrote:
On Thu, Mar 04, 2021 at 11:42:10PM +0100, Philippe Mathieu-Daudé wrote:
On 3/4/21 9:16 PM, BALATON Zoltan wrote:
On Thu, 4 Mar 2021, Philippe Mathieu-Daudé wrote:
On 3/2/21 10:11 PM, BALATON Zoltan wrote:
To allow reusing ISA bridge emulation for vt8231_isa move the device
state of vt82c686b_isa emulation in an abstract via_isa class.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
 hw/isa/vt82c686.c        | 70 ++++++++++++++++++++++------------------
 include/hw/pci/pci_ids.h |  2 +-
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 72234bc4d1..5137f97f37 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -609,24 +609,48 @@ static const TypeInfo vt8231_superio_info = {

+#define TYPE_VIA_ISA "via-isa"

-struct VT82C686BISAState {
+struct ViaISAState {
     PCIDevice dev;
     qemu_irq cpu_intr;
     ViaSuperIOState *via_sio;

+static const VMStateDescription vmstate_via = {
+    .name = "via-isa",

You changed the migration stream name, so I think we have
a problem with migration... No clue how to do that properly.

I don't think these machines support migration or state description of
vt86c686b was not missing something before these patches that would make
it not work anyway so I did not worry about this too much. I doubt
anybody wants to migrate a fuloong2e machine so this should not be a
problem in practice but maybe you can mention it in the release notes if
you think that would be necessary.

Maybe just add in the description:

 This change breaks migration back compatibility, but
 this is not an issue for the Fuloong2E machine.

Hrm.  If migration was never supported, why is there a vmstate
description there at all though?

That said, I don't think breaking compat is a problem: that's only an
issue where we actually have versioned machine types, which covers
only pc, pseries, arm virt and a very few others.  I don't think this
device was used on any of them.

Except 'vt82c686b-usb-uhci' is a generic PCI device that anyone can
instantiate, so it's not actually Fuloong specific.

This is the isa bridge part of vt82c686b which should not be used anywhere else than fuloong2e and should not change the usb part.

That the usb-uhci part of this chip appears in the PCI device list may be a bug as that's not much useful without the rest of the superio chip but fixing that is beyond this patch I think. There's no reason one would want to use vt82c686-usb device on a machine without the vt82c686 chip instead or another uhci device matching the hardware of that machine.

I've updated the commit message as Philippe suggested.



David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!

Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

reply via email to

[Prev in Thread] Current Thread [Next in Thread]