Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode

[BALATON Zoltan]

On Thu, 5 Mar 2020, Mark Cave-Ayland wrote:
> On 04/03/2020 22:33, BALATON Zoltan wrote:
> > > > > > > AFAICT this then only leaves the question: why does the firmware set
> > > > > > > PCI_INTERRUPT_LINE to 9, which is presumably why you are seeing problems running
> > > > > > > MorphOS under QEMU.
> > > > > >
> > > > > > Linux does try to handle both true native mode and half-native mode. It only 
> > > > > > uses
> > > > > > half-native mode if finds IRQ14 on Pegasos, otherwise skips Pegasos specific 
> > > > > > fixup
> > > > > > and uses true native mode setup. I don't know what MorphOS does but I think it
> > > > > > justs
> > > > > > knows that Pegasos2 has this quirk and does not look at the device tree at all.
>
> I just a quick look at the PCI specification and found this interesting 
> paragraph in
> the section about "Interrupt Line":
>
>
> "The Interrupt Line register is an eight-bit register used to communicate 
> interrupt
> line routing information. The register is read/write and must be implemented by 
> any
> device (or device function) that uses an interrupt pin. POST software will 
> write the
> routing information into this register as it initializes and configures the 
> system."
>
> "The value in this register tells which input of the system interrupt 
> controller(s)
> the device's interrupt pin is connected to. The device itself does not use this
> value, rather it is used by device drivers and operating systems. Device 
> drivers and
> operating systems can use this information to determine priority and vector
> information. Values in this register are architecture-specific [43]."
>
> [43] For x86 based PCs, the values in this register correspond to IRQ numbers 
> (0-15)
> of the standard dual 8259 configuration. The value 255 is defined as meaning
> "unknown" or "no connection" to the interrupt controller. Values between 15 and 
> 254
> are reserved.
>
>
> The key part here is "The device itself does not use this value, rather it is 
> used by
> device drivers and operating systems" since this immediately tells us that the
> existing code in hw/ide/via.c which uses the interrupt line value for IRQ 
> routing is
> incorrect and should be removed.

On real hardware this may be true but in QEMU how would it otherwise raise 
the correct interrupt line the guest expects? This probably does not 
matter for pegasos2 but I think is needed for 100% native mode used with 
the fulong2e so it gets the IRQ it expects.

> If we do that the next question is how does the VIA know whether the use the PCI
> interrupt or the legacy interrupt? Another look at the datasheet showed that 
> there is

I don't think via-ide ever uses a PCI interrupt, if you look at its 
datasheet the description of the prog-if reg (0x9) says in native mode irq 
is programmable via config reg 0x3c which then lists all the ISA IRQs as 
possible values, default 14 and 0 meaning disable.

> another possibility: PCI configuration space register 0x3d (Interrupt pin) is
> documented as having value 0 == Legacy IRQ routing which should be the initial 
> value
> on reset, but QEMU incorrectly sets it to 1 which indicates PCI IRQ routing.

The VT8231 docs say this should always read 1 but may be this is somehow 
set to 0 on the Pegasos2. What does that mean? Should we use this value 
instead of the feature bit to force using legacy interrupts? We'd still 
need a property in via-ide to set this reg or is it possible to set it 
from board code overriding the default after device is created? That would 
allow to drop patch 1. I can try this.

> In your previous email you included a trace of the PCI configuration accesses 
> to the
> via-ide device. Can you try this again with the following diff and post the same
> output once again?
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 096de8dba0..db9f4af861 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -139,7 +139,7 @@ static void via_ide_reset(DeviceState *dev)
>     pci_set_long(pci_conf + PCI_BASE_ADDRESS_2, 0x00000170);
>     pci_set_long(pci_conf + PCI_BASE_ADDRESS_3, 0x00000374);
>     pci_set_long(pci_conf + PCI_BASE_ADDRESS_4, 0x0000cc01); /* BMIBA: 20-23h */
> -    pci_set_long(pci_conf + PCI_INTERRUPT_LINE, 0x0000010e);
> +    pci_set_long(pci_conf + PCI_INTERRUPT_LINE, 0x0000000e);
>
>     /* IDE chip enable, IDE configuration 1/2, IDE FIFO Configuration*/
>     pci_set_long(pci_conf + 0x40, 0x0a090600);

This does not change much:

pci_cfg_write via-ide 12:1 @0x9 <- 0xf
pci_cfg_write via-ide 12:1 @0x40 <- 0xb
pci_cfg_write via-ide 12:1 @0x41 <- 0xf2
pci_cfg_write via-ide 12:1 @0x43 <- 0x35
pci_cfg_write via-ide 12:1 @0x44 <- 0x18
pci_cfg_write via-ide 12:1 @0x45 <- 0x1c
pci_cfg_write via-ide 12:1 @0x46 <- 0xc0
pci_cfg_write via-ide 12:1 @0x50 <- 0x17171717
pci_cfg_write via-ide 12:1 @0x54 <- 0x14
pci_cfg_read via-ide 12:1 @0x0 -> 0x5711106
pci_cfg_read via-ide 12:1 @0x0 -> 0x5711106
pci_cfg_read via-ide 12:1 @0x8 -> 0x1018f06
pci_cfg_read via-ide 12:1 @0xc -> 0x0
pci_cfg_read via-ide 12:1 @0x2c -> 0x11001af4
pci_cfg_read via-ide 12:1 @0x3c -> 0xe
pci_cfg_read via-ide 12:1 @0x4 -> 0x2800080
pci_cfg_read via-ide 12:1 @0x3c -> 0xe
pci_cfg_write via-ide 12:1 @0x3c <- 0x9

compared to

> pci_cfg_write via-ide 12:1 @0x9 <- 0xf
> pci_cfg_write via-ide 12:1 @0x40 <- 0xb
> pci_cfg_write via-ide 12:1 @0x41 <- 0xf2
> pci_cfg_write via-ide 12:1 @0x43 <- 0x35
> pci_cfg_write via-ide 12:1 @0x44 <- 0x18
> pci_cfg_write via-ide 12:1 @0x45 <- 0x1c
> pci_cfg_write via-ide 12:1 @0x46 <- 0xc0
> pci_cfg_write via-ide 12:1 @0x50 <- 0x17171717
> pci_cfg_write via-ide 12:1 @0x54 <- 0x14
> pci_cfg_read via-ide 12:1 @0x0 -> 0x5711106
> pci_cfg_read via-ide 12:1 @0x0 -> 0x5711106
> pci_cfg_read via-ide 12:1 @0x8 -> 0x1018f06
> pci_cfg_read via-ide 12:1 @0xc -> 0x0
> pci_cfg_read via-ide 12:1 @0x2c -> 0x11001af4
> pci_cfg_read via-ide 12:1 @0x3c -> 0x10e
> pci_cfg_read via-ide 12:1 @0x4 -> 0x2800080
> pci_cfg_read via-ide 12:1 @0x3c -> 0x10e
> pci_cfg_write via-ide 12:1 @0x3c <- 0x109

firmware does not seem to care about this value.

Regards,
BALATON Zoltan