[Qemu-devel] PIIX/IDE: ports disabled in PCI config space?

[Qemu-devel] PIIX/IDE: ports disabled in PCI config space?

[Luca Tettamanti]

Hello,
I'm testing the new Fedora7 under KVM. As you may know Fedora has
migrated to the new libata drivers.

ata_piix is unhappy with the PIIX IDE controller provided by QEmu/KVM:

libata version 2.20 loaded.
ata_piix 0000:00:01.1: version 2.10ac1
PCI: Setting latency timer of device 0000:00:01.1 to 64
ata1: PATA max MWDMA2 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x00011400 irq 14
ata2: PATA max MWDMA2 cmd 0x00010170 ctl 0x00010376 bmdma 0x00011408 irq 15
scsi0 : ata_piix
ata1: port disabled. ignoring.
scsi1 : ata_piix
ata2: port disabled. ignoring.

The "port disabled" messages are generated by piix_pata_prereset
(called by piix_pata_error_handler), see drivers/ata/ata-piix.c.
piix_pata_prereset checks PCI config space to see whether the port is
active or not:

static int piix_pata_prereset(struct ata_port *ap, unsigned long deadline)
{
        struct pci_dev *pdev = to_pci_dev(ap->host->dev);
        
        if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no]))
                return -ENOENT;
        
        return ata_std_prereset(ap, deadline);
}

with:

static struct pci_bits piix_enable_bits[] = {
        { 0x41U, 1U, 0x80UL, 0x80UL },  /* port 0 */
        { 0x43U, 1U, 0x80UL, 0x80UL },  /* port 1 */
};

which means that it will read 1 byte at offset 0x41 (or 0x43), mask it
with 0x80 and the result shall be 0x80 (i.e. it checks bit 7).

Bit 7 in Intel docs is described in this way:

"IDE Decode Enable (IDE). 1=Enable; 0=Disable. When enabled, I/O
 transactions on PCI targeting the IDE ATA register blocks (command block
 and control block) are positively decoded on PCI and driven on the IDE
 interface. When disabled, these accesses are subtractively decoded to
 ISA."

Now this is config space of the IDE controller:

00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] (prog-if 80 [Master])
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 64
        Region 0: [virtual] Memory at 000001f0 (32-bit, non-prefetchable) [size=8]
        Region 1: [virtual] Memory at 000003f0 (type 3, non-prefetchable) [size=1]
        Region 2: [virtual] Memory at 00000170 (32-bit, non-prefetchable) [size=8]
        Region 3: [virtual] Memory at 00000370 (type 3, non-prefetchable) [size=1]
        Region 4: I/O ports at 1400 [size=16]
00: 86 80 10 70 07 00 80 02 00 80 01 01 00 40 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 01 14 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
       ^^    ^^
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

As you can see those 2 bytes are 0, and the libata driver considers them
disabled (the legacy driver works fine).

The following patch fixes the problem (i.e. ata_piix finds both the HD
and the cdrom):

--- a/hw/ide.c	2007-06-04 19:34:25.000000000 +0200
+++ b/hw/ide.c	2007-06-04 21:45:28.000000000 +0200
@@ -2586,6 +2586,8 @@ static void piix3_reset(PCIIDEState *d)
     pci_conf[0x06] = 0x80; /* FBC */
     pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
     pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+    pci_conf[0x41] = 0x80; // enable port 0
+    pci_conf[0x43] = 0x80; // enable port 1
 }
 
 void pci_piix_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn)


It has been run-time tested with KVM-27. Applies cleanly to qemu 0.9 and
with offset to qemu-snapshot (I've not tested it though).

Luca
-- 
This message will self destruct in 5 seconds.

[Qemu-devel] Re: [kvm-devel] PIIX/IDE: ports disabled in PCI config space?

[Avi Kivity]

Luca Tettamanti wrote:
> Hello,
> I'm testing the new Fedora7 under KVM. As you may know Fedora has
> migrated to the new libata drivers.
>
> ata_piix is unhappy with the PIIX IDE controller provided by QEmu/KVM:
>
>
>   

[...]

> The following patch fixes the problem (i.e. ata_piix finds both the HD
> and the cdrom):
>
> --- a/hw/ide.c	2007-06-04 19:34:25.000000000 +0200
> +++ b/hw/ide.c	2007-06-04 21:45:28.000000000 +0200
> @@ -2586,6 +2586,8 @@ static void piix3_reset(PCIIDEState *d)
>      pci_conf[0x06] = 0x80; /* FBC */
>      pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
>      pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
> +    pci_conf[0x41] = 0x80; // enable port 0
> +    pci_conf[0x43] = 0x80; // enable port 1
>  }
>  
>  void pci_piix_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn)
>   

I imagine the reset state in the spec is disabled?  If so, then the 
long-term fix is to enable these bits in the bios.

In any case, I applied this to the kvm repo.  Thanks.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [kvm-devel] PIIX/IDE: ports disabled in PCI config space?

[Luca]

On 6/5/07, Avi Kivity <avi@qumranet.com> wrote:
> Luca Tettamanti wrote:
> > Hello,
> > I'm testing the new Fedora7 under KVM. As you may know Fedora has
> > migrated to the new libata drivers.
> >
> > ata_piix is unhappy with the PIIX IDE controller provided by QEmu/KVM:
> >
> >
> >
>
> [...]
>
> > The following patch fixes the problem (i.e. ata_piix finds both the HD
> > and the cdrom):
> >
> > --- a/hw/ide.c        2007-06-04 19:34:25.000000000 +0200
> > +++ b/hw/ide.c        2007-06-04 21:45:28.000000000 +0200
> > @@ -2586,6 +2586,8 @@ static void piix3_reset(PCIIDEState *d)
> >      pci_conf[0x06] = 0x80; /* FBC */
> >      pci_conf[0x07] = 0x02; // PCI_status_devsel_medium
> >      pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
> > +    pci_conf[0x41] = 0x80; // enable port 0
> > +    pci_conf[0x43] = 0x80; // enable port 1
> >  }
> >
> >  void pci_piix_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn)
> >
>
> I imagine the reset state in the spec is disabled?

Yes, you are right.
Btw, this is the doc:
http://www.intel.com/design/intarch/datashts/290550.htm

> If so, then the
> long-term fix is to enable these bits in the bios.

Good point. I'm looking at bochs right now: the BIOS doesn't touch
that bits. The 2 ports are enabled/disabled - using values from the
config file - by the reset function of the emulated controller
(iodev/pci_ide.cc), so they're doing the same thing I've done for
QEMU.
I'd rather not touch the BIOS, it's a bit obscure ;-)

Luca

[Qemu-devel] Re: [kvm-devel] PIIX/IDE: ports disabled in PCI config space?

[Avi Kivity]

Luca wrote:
>
> Good point. I'm looking at bochs right now: the BIOS doesn't touch
> that bits. The 2 ports are enabled/disabled - using values from the
> config file - by the reset function of the emulated controller
> (iodev/pci_ide.cc), so they're doing the same thing I've done for
> QEMU.
> I'd rather not touch the BIOS, it's a bit obscure ;-)

No kidding :-)

Maybe if you post your findings to bochs-devel they'd make the change.

-- 
error compiling committee.c: too many arguments to function