Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances

[Philippe Mathieu-Daudé]

On 5/2/23 23:32, Mark Cave-Ayland wrote:
> On 05/02/2023 22:21, BALATON Zoltan wrote:
>
> > On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:
> > > On 26/01/2023 21:17, Bernhard Beschow wrote:
> > > > Internal instances now defer interrupt wiring to the caller which
> > > > decouples them from the ISABus. User-created devices still fish out the
> > > > ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
> > > > The latter mechanism is considered a workaround and intended to be
> > > > removed once a deprecation period for user-created PIIX IDE devices is
> > > > over.
> > > >
> > > > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > > > ---
> > > >   include/hw/ide/pci.h |  1 +
> > > >   hw/ide/piix.c        | 64 
> > > > ++++++++++++++++++++++++++++++++++----------
> > > >   hw/isa/piix.c        |  5 ++++
> > > >   3 files changed, 56 insertions(+), 14 deletions(-)

> > > I haven't checked the datasheet, but I suspect this will be similar 
> > > to the cmd646/via PCI-IDE interfaces in that there will be a PCI 
> > > configuration register that will switch between ISA compatibility 
> > > mode (and ISA irqs) and PCI mode (with PCI IRQs). So it would be the 
> > > device configuration that would specify PCI or ISA mode, rather than 
> > > the presence of an ISABus.
> >
> > I forgot about this topic already and haven't follwed this series 
> > either so what I say may not fully make sense but I think CMD646 and 
> > via-ide are different. CMD646 is a PCI device and should use PCI 
> > interrupts while via-ide is part of a southbridge/superio complex and 
> > connected to the ISA PICs within that southbride, so I think via-ide 
> > always uses ISA IRQs and the ISA btidge within the same chip may 
> > convert that to PCI IRQs or not (that part is where I'm lost also 
> > because we may not actually model it that way). After a long debate we 
> > managed to find a solution back then that works for every guest we use 
> > it for now so I think we don't want to touch it now until some real 
> > need arises. It does not worth the trouble and added complexity to 
> > model something that is not used just for the sake of correctness. By 
> > the time we find a use for that, the ISA emulation may evolve so it's 
> > easier to implement the missing switching between isa and native mode 
> > or we may want to do it differently (such as we do things differently 
> > now compared to what we did years ago). So I think it does not worth 
> > keeping the ISA model from being simplified for some theoretical uses 
> > in the future which we may not actually do any time soon. But I don't 
> > want to get into this again so just shared my thoughts and feel free 
> > to ignore it. I don't care where these patches go as long as the VIA 
> > model keeps working for me.
>
> I have a vague memory that ISA compatibility mode was part of the 
> original PCI-BMDMA specification, but it has been a while since I last 
> looked.
>
> Bernhard, is there any mention of this in the PIIX datasheet(s)? For 
> reference the cmd646 datasheet specifies that ISA mode or PCI mode is 
> determined by register PROG_IF (0x9) in PCI configuration space.

Orthogonal to this discussion, one problem I see here is a device
exposing 2 interfaces: ISA and PCI. QOM does support interfaces,
but ISA and PCI aren't QOM interfaces. The QOM cast macros are
written as a QOM object can only inherit one parent. Should we
stick to QDev and convert ISA/PCI as QOM interfaces? That could
solve some QDev IDE limitations...