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

[Philippe Mathieu-Daudé]

On 7/2/23 00:40, Bernhard Beschow wrote:
> Am 5. Februar 2023 22:32:03 UTC schrieb Mark Cave-Ayland 
> <mark.cave-ayland@ilande.co.uk>:
> > 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.
>
> I've found the following:
>
>    "Only PCI masters have access to the IDE port. ISA Bus masters cannot access the 
> IDE I/O port addresses. Memory targeted by the IDE interface acting as a PCI Bus master 
> on behalf of IDE DMA slaves must reside on PCI, usually main memory implemented by the 
> host-to-PCI bridge."
>
> And:
>
>    "PIIX4 can act as a PCI Bus master on behalf of an IDE slave device."
>
> Does this perhaps mean that piix-ide does indeed have no ISA bus?

Yes :)