Patchwork ahci: add port I/O index-data pair

login
register
mail settings
Submitter Daniel Verkamp
Date Aug. 27, 2011, 9:12 a.m.
Message ID <1314436348-28837-1-git-send-email-daniel@drv.nu>
Download mbox | patch
Permalink /patch/111856/
State New
Headers show

Comments

Daniel Verkamp - Aug. 27, 2011, 9:12 a.m.
Implement an I/O space index-data register pair as defined by the AHCI
spec, including the corresponding SATA PCI capability and BAR.

This allows real-mode code to access the AHCI registers; real-mode
code cannot address the memory-mapped register space because it is
beyond the first megabyte.

Signed-off-by: Daniel Verkamp <daniel@drv.nu>
---
 hw/ide/ahci.c |   42 +++++++++++++++++++++++++++++++++++++++++-
 hw/ide/ahci.h |    9 ++++++++-
 hw/ide/ich.c  |   27 ++++++++++++++++++++++++++-
 hw/pci_regs.h |    1 +
 4 files changed, 76 insertions(+), 3 deletions(-)
Alexander Graf - Aug. 28, 2011, 6:48 p.m.
On 27.08.2011, at 04:12, Daniel Verkamp wrote:

> Implement an I/O space index-data register pair as defined by the AHCI
> spec, including the corresponding SATA PCI capability and BAR.
> 
> This allows real-mode code to access the AHCI registers; real-mode
> code cannot address the memory-mapped register space because it is
> beyond the first megabyte.

Very nice patch! I'll check and compare with a real ICH-9 when I get back to .de, but I'd assume you also did that already ;). Once I checked that the IO region is set up similarly, I'll give you my ack.


Alex
Daniel Verkamp - Aug. 30, 2011, 3:07 a.m.
On Sun, Aug 28, 2011 at 11:48 AM, Alexander Graf <agraf@suse.de> wrote:
>
> On 27.08.2011, at 04:12, Daniel Verkamp wrote:
>
>> Implement an I/O space index-data register pair as defined by the AHCI
>> spec, including the corresponding SATA PCI capability and BAR.
>>
>> This allows real-mode code to access the AHCI registers; real-mode
>> code cannot address the memory-mapped register space because it is
>> beyond the first megabyte.
>
> Very nice patch! I'll check and compare with a real ICH-9 when I get
> back to .de, but I'd assume you also did that already ;). Once I checked
> that the IO region is set up similarly, I'll give you my ack.

Please do double check against real hardware if you get the chance - I
don't have a real ICH-9 handy to test against.  This is all written
based on my reading of the spec and testing with an internal DOS
developer tool from work.

I am mainly curious how the real thing handles writes to the index
register that aren't divisible by 4 or are beyond the end of the
register set (and how big that really is on ICH-9).  Judging by the
bits marked "RO" in the spec, I would guess writing 0x13 to the index
and then reading it back should give 0x10, but I haven't tested it on
real hw.

Thanks,
-- Daniel
Alexander Graf - Sept. 1, 2011, 2:58 p.m.
On 08/30/2011 05:07 AM, Daniel Verkamp wrote:
> On Sun, Aug 28, 2011 at 11:48 AM, Alexander Graf<agraf@suse.de>  wrote:
>> On 27.08.2011, at 04:12, Daniel Verkamp wrote:
>>
>>> Implement an I/O space index-data register pair as defined by the AHCI
>>> spec, including the corresponding SATA PCI capability and BAR.
>>>
>>> This allows real-mode code to access the AHCI registers; real-mode
>>> code cannot address the memory-mapped register space because it is
>>> beyond the first megabyte.
>> Very nice patch! I'll check and compare with a real ICH-9 when I get
>> back to .de, but I'd assume you also did that already ;). Once I checked
>> that the IO region is set up similarly, I'll give you my ack.
> Please do double check against real hardware if you get the chance - I
> don't have a real ICH-9 handy to test against.  This is all written
> based on my reading of the spec and testing with an internal DOS
> developer tool from work.
>
> I am mainly curious how the real thing handles writes to the index
> register that aren't divisible by 4 or are beyond the end of the
> register set (and how big that really is on ICH-9).  Judging by the
> bits marked "RO" in the spec, I would guess writing 0x13 to the index
> and then reading it back should give 0x10, but I haven't tested it on
> real hw.

Phew. So I finally got at least an ICH-9 system booting. This is what 
lspci -vvv tells me:

00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 
port SATA AHCI Controller (rev 02) (prog-if 01 [AHCI 1.0])
     Subsystem: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port 
SATA AHCI Controller
     Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
     Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
<TAbort- <MAbort- >SERR- <PERR- INTx-
     Latency: 0
     Interrupt: pin B routed to IRQ 26
     Region 0: I/O ports at d000 [size=8]
     Region 1: I/O ports at cc00 [size=4]
     Region 2: I/O ports at c880 [size=8]
     Region 3: I/O ports at c800 [size=4]
     Region 4: I/O ports at c480 [size=32]
     Region 5: Memory at ffaf9000 (32-bit, non-prefetchable) [size=2K]
     Capabilities: [80] MSI: Enable+ Count=1/16 Maskable- 64bit-
         Address: fee0f00c  Data: 4169
     Capabilities: [70] Power Management version 3
         Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA 
PME(D0-,D1-,D2-,D3hot+,D3cold-)
         Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
     Capabilities: [a8] SATA HBA v1.0 BAR4 Offset=00000004
     Capabilities: [b0] Vendor Specific Information: Len=06 <?>
     Kernel driver in use: ahci

So BAR4 is where the IDP info should be. Offset is 4 into that IO space 
and the space is 32 bytes long. Do you have the ICH-9 implementation 
spec? I can try to dig something up if you don't have it around.

Please send me a small test program I can run on the machine to find out 
what happens for unaligned I/O accesses. That would be very helpful!


Thanks,

Alex
Daniel Verkamp - Sept. 8, 2011, 7:33 a.m.
(Sorry for the slow response, was on vacation)

On Thu, Sep 1, 2011 at 7:58 AM, Alexander Graf <agraf@suse.de> wrote:
> On 08/30/2011 05:07 AM, Daniel Verkamp wrote:
>>
>> On Sun, Aug 28, 2011 at 11:48 AM, Alexander Graf<agraf@suse.de>  wrote:
>>>
>>> On 27.08.2011, at 04:12, Daniel Verkamp wrote:
>>>
>>>> Implement an I/O space index-data register pair as defined by the AHCI
>>>> spec, including the corresponding SATA PCI capability and BAR.
>>>>
>>>> This allows real-mode code to access the AHCI registers; real-mode
>>>> code cannot address the memory-mapped register space because it is
>>>> beyond the first megabyte.
>>>
>>> Very nice patch! I'll check and compare with a real ICH-9 when I get
>>> back to .de, but I'd assume you also did that already ;). Once I checked
>>> that the IO region is set up similarly, I'll give you my ack.
>>
>> Please do double check against real hardware if you get the chance - I
>> don't have a real ICH-9 handy to test against.  This is all written
>> based on my reading of the spec and testing with an internal DOS
>> developer tool from work.
>>
>> I am mainly curious how the real thing handles writes to the index
>> register that aren't divisible by 4 or are beyond the end of the
>> register set (and how big that really is on ICH-9).  Judging by the
>> bits marked "RO" in the spec, I would guess writing 0x13 to the index
>> and then reading it back should give 0x10, but I haven't tested it on
>> real hw.
>
> Phew. So I finally got at least an ICH-9 system booting. This is what lspci
> -vvv tells me:
>
> 00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
> port SATA AHCI Controller (rev 02) (prog-if 01 [AHCI 1.0])
>    Subsystem: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA AHCI
> Controller
>    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
> Stepping- SERR- FastB2B- DisINTx+
>    Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
> <MAbort- >SERR- <PERR- INTx-
>    Latency: 0
>    Interrupt: pin B routed to IRQ 26
>    Region 0: I/O ports at d000 [size=8]
>    Region 1: I/O ports at cc00 [size=4]
>    Region 2: I/O ports at c880 [size=8]
>    Region 3: I/O ports at c800 [size=4]
>    Region 4: I/O ports at c480 [size=32]
>    Region 5: Memory at ffaf9000 (32-bit, non-prefetchable) [size=2K]
>    Capabilities: [80] MSI: Enable+ Count=1/16 Maskable- 64bit-
>        Address: fee0f00c  Data: 4169
>    Capabilities: [70] Power Management version 3
>        Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
> PME(D0-,D1-,D2-,D3hot+,D3cold-)
>        Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
>    Capabilities: [a8] SATA HBA v1.0 BAR4 Offset=00000004
>    Capabilities: [b0] Vendor Specific Information: Len=06 <?>
>    Kernel driver in use: ahci
>
> So BAR4 is where the IDP info should be. Offset is 4 into that IO space and
> the space is 32 bytes long. Do you have the ICH-9 implementation spec? I can
> try to dig something up if you don't have it around.
>

I'm not sure I understand what you mean, but I think everything is in
the right spot - compare with the real ICH-9 dump you provide
(relevant parts quoted below; full lspci dump from QEMU device at
end):

Real: Region 4: I/O ports at c480 [size=32]
QEMU: Region 4: I/O ports at c040 [size=32] (I/O address is different,
but that is ok)

Real: Capabilities: [a8] SATA HBA v1.0 BAR4 Offset=00000004
QEMU: Capabilities: [a8] SATA HBA v1.0 BAR4 Offset=00000004 (identical)

> Please send me a small test program I can run on the machine to find out
> what happens for unaligned I/O accesses. That would be very helpful!
>

I will try to put something together in the next few days and send it along;
is a DOS test app suitable?

Thanks,
-- Daniel Verkamp


Here is the lspci -vvv -nn -x dump of the QEMU-emulated AHCI
controller with the patch applied:

00:04.0 SATA controller [0106]: Intel Corporation 82801IR/IO/IH
(ICH9R/DO/DH) 6 port SATA AHCI Controller [8086:2922] (rev 02)
(prog-if 01 [AHCI 1.0])
	Subsystem: Red Hat, Inc Device [1af4:1100]
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 24
	Region 4: I/O ports at c040 [size=32]
	Region 5: Memory at febf1000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [a8] SATA HBA v1.0 BAR4 Offset=00000004
	Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
		Address: 00000000fee0100c  Data: 4149
	Kernel driver in use: ahci
00: 86 80 22 29 07 04 10 00 02 01 06 01 00 00 00 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 41 c0 00 00 00 10 bf fe 00 00 00 00 f4 1a 00 11
30: 00 00 00 00 a8 00 00 00 00 00 00 00 0b 01 00 00
40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50: 05 00 81 00 0c 10 e0 fe 00 00 00 00 49 41 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: 40 00 3f 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 12 50 10 00 48 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
Kevin Wolf - Sept. 20, 2011, 1:39 p.m.
Am 28.08.2011 20:48, schrieb Alexander Graf:
> 
> On 27.08.2011, at 04:12, Daniel Verkamp wrote:
> 
>> Implement an I/O space index-data register pair as defined by the AHCI
>> spec, including the corresponding SATA PCI capability and BAR.
>>
>> This allows real-mode code to access the AHCI registers; real-mode
>> code cannot address the memory-mapped register space because it is
>> beyond the first megabyte.
> 
> Very nice patch! I'll check and compare with a real ICH-9 when I get back to .de, but I'd assume you also did that already ;). Once I checked that the IO region is set up similarly, I'll give you my ack.

What's the status with review/testing of this patch, Alex? Is it ready
to be merged, should I drop it or do you just need some more time?

Kevin
Alexander Graf - Sept. 20, 2011, 2:02 p.m.
On 09/20/2011 03:39 PM, Kevin Wolf wrote:
> Am 28.08.2011 20:48, schrieb Alexander Graf:
>> On 27.08.2011, at 04:12, Daniel Verkamp wrote:
>>
>>> Implement an I/O space index-data register pair as defined by the AHCI
>>> spec, including the corresponding SATA PCI capability and BAR.
>>>
>>> This allows real-mode code to access the AHCI registers; real-mode
>>> code cannot address the memory-mapped register space because it is
>>> beyond the first megabyte.
>> Very nice patch! I'll check and compare with a real ICH-9 when I get back to .de, but I'd assume you also did that already ;). Once I checked that the IO region is set up similarly, I'll give you my ack.
> What's the status with review/testing of this patch, Alex? Is it ready
> to be merged, should I drop it or do you just need some more time?

If you don't see regressions with it, I'd say we're good. I still don't 
have a validation program, but considering that it's a reasonably simple 
change and makes us more conforming to real hardware, I don't see why we 
shouldn't have it.


Alex
Kevin Wolf - Sept. 21, 2011, 12:34 p.m.
Am 27.08.2011 11:12, schrieb Daniel Verkamp:
> Implement an I/O space index-data register pair as defined by the AHCI
> spec, including the corresponding SATA PCI capability and BAR.
> 
> This allows real-mode code to access the AHCI registers; real-mode
> code cannot address the memory-mapped register space because it is
> beyond the first megabyte.
> 
> Signed-off-by: Daniel Verkamp <daniel@drv.nu>

Thanks, applied to the block branch.

> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -72,6 +72,14 @@
>  #include <hw/ide/pci.h>
>  #include <hw/ide/ahci.h>
>  
> +#define ICH9_SATA_CAP_OFFSET    0xA8
> +
> +#define ICH9_IDP_BAR            4
> +#define ICH9_MEM_BAR            5
> +
> +#define ICH9_IDP_INDEX          0x10
> +#define ICH9_IDP_INDEX_LOG2     0x04

Just wondering, why did you choose 0x10 and not 0? The spec reads as if
the implementation could freely choose this, and I can't see what the
first 16 Bytes are used for.

Doesn't make it less correct, of course.

Kevin
Kevin Wolf - Sept. 21, 2011, 12:57 p.m.
Am 21.09.2011 14:34, schrieb Kevin Wolf:
> Am 27.08.2011 11:12, schrieb Daniel Verkamp:
>> Implement an I/O space index-data register pair as defined by the AHCI
>> spec, including the corresponding SATA PCI capability and BAR.
>>
>> This allows real-mode code to access the AHCI registers; real-mode
>> code cannot address the memory-mapped register space because it is
>> beyond the first megabyte.
>>
>> Signed-off-by: Daniel Verkamp <daniel@drv.nu>
> 
> Thanks, applied to the block branch.
> 
>> --- a/hw/ide/ich.c
>> +++ b/hw/ide/ich.c
>> @@ -72,6 +72,14 @@
>>  #include <hw/ide/pci.h>
>>  #include <hw/ide/ahci.h>
>>  
>> +#define ICH9_SATA_CAP_OFFSET    0xA8
>> +
>> +#define ICH9_IDP_BAR            4
>> +#define ICH9_MEM_BAR            5
>> +
>> +#define ICH9_IDP_INDEX          0x10
>> +#define ICH9_IDP_INDEX_LOG2     0x04
> 
> Just wondering, why did you choose 0x10 and not 0? The spec reads as if
> the implementation could freely choose this, and I can't see what the
> first 16 Bytes are used for.

I forgot that we're not implementing a generic AHCI controller here but
an ICH9 one, which does define the offsets used. Now I'm convinced. :-)

Kevin

Patch

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 29521ba..431e58e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -370,6 +370,43 @@  static MemoryRegionOps ahci_mem_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static uint64_t ahci_idp_read(void *opaque, target_phys_addr_t addr,
+                              unsigned size)
+{
+    AHCIState *s = opaque;
+
+    if (addr == s->idp_offset) {
+        /* index register */
+        return s->idp_index;
+    } else if (addr == s->idp_offset + 4) {
+        /* data register - do memory read at location selected by index */
+        return ahci_mem_read(opaque, s->idp_index, size);
+    } else {
+        return 0;
+    }
+}
+
+static void ahci_idp_write(void *opaque, target_phys_addr_t addr,
+                           uint64_t val, unsigned size)
+{
+    AHCIState *s = opaque;
+
+    if (addr == s->idp_offset) {
+        /* index register - mask off reserved bits */
+        s->idp_index = (uint32_t)val & ((AHCI_MEM_BAR_SIZE - 1) & ~3);
+    } else if (addr == s->idp_offset + 4) {
+        /* data register - do memory write at location selected by index */
+        ahci_mem_write(opaque, s->idp_index, val, size);
+    }
+}
+
+static MemoryRegionOps ahci_idp_ops = {
+    .read = ahci_idp_read,
+    .write = ahci_idp_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+
 static void ahci_reg_init(AHCIState *s)
 {
     int i;
@@ -1126,7 +1163,9 @@  void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
     s->dev = g_malloc0(sizeof(AHCIDevice) * ports);
     ahci_reg_init(s);
     /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for now */
-    memory_region_init_io(&s->mem, &ahci_mem_ops, s, "ahci", 0x1000);
+    memory_region_init_io(&s->mem, &ahci_mem_ops, s, "ahci", AHCI_MEM_BAR_SIZE);
+    memory_region_init_io(&s->idp, &ahci_idp_ops, s, "ahci-idp", 32);
+
     irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
 
     for (i = 0; i < s->ports; i++) {
@@ -1146,6 +1185,7 @@  void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
 void ahci_uninit(AHCIState *s)
 {
     memory_region_destroy(&s->mem);
+    memory_region_destroy(&s->idp);
     g_free(s->dev);
 }
 
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index e456193..a2a9400 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -24,7 +24,7 @@ 
 #ifndef HW_IDE_AHCI_H
 #define HW_IDE_AHCI_H
 
-#define AHCI_PCI_BAR              5
+#define AHCI_MEM_BAR_SIZE         0x1000
 #define AHCI_MAX_PORTS            32
 #define AHCI_MAX_SG               168 /* hardware max is 64K */
 #define AHCI_DMA_BOUNDARY         0xffffffff
@@ -212,6 +212,10 @@ 
 #define RES_FIS_SDBFIS                     0x58
 #define RES_FIS_UFIS                       0x60
 
+#define SATA_CAP_SIZE           0x8
+#define SATA_CAP_REV            0x2
+#define SATA_CAP_BAR            0x4
+
 typedef struct AHCIControlRegs {
     uint32_t    cap;
     uint32_t    ghc;
@@ -290,6 +294,9 @@  typedef struct AHCIState {
     AHCIDevice *dev;
     AHCIControlRegs control_regs;
     MemoryRegion mem;
+    MemoryRegion idp;       /* Index-Data Pair I/O port space */
+    unsigned idp_offset;    /* Offset of index in I/O port space */
+    uint32_t idp_index;     /* Current IDP index */
     int ports;
     qemu_irq irq;
 } AHCIState;
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 5278bc4..4ff68a8 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -72,6 +72,14 @@ 
 #include <hw/ide/pci.h>
 #include <hw/ide/ahci.h>
 
+#define ICH9_SATA_CAP_OFFSET    0xA8
+
+#define ICH9_IDP_BAR            4
+#define ICH9_MEM_BAR            5
+
+#define ICH9_IDP_INDEX          0x10
+#define ICH9_IDP_INDEX_LOG2     0x04
+
 static const VMStateDescription vmstate_ahci = {
     .name = "ahci",
     .unmigratable = 1,
@@ -80,6 +88,8 @@  static const VMStateDescription vmstate_ahci = {
 static int pci_ich9_ahci_init(PCIDevice *dev)
 {
     struct AHCIPCIState *d;
+    int sata_cap_offset;
+    uint8_t *sata_cap;
     d = DO_UPCAST(struct AHCIPCIState, card, dev);
 
     ahci_init(&d->ahci, &dev->qdev, 6);
@@ -98,7 +108,22 @@  static int pci_ich9_ahci_init(PCIDevice *dev)
     msi_init(dev, 0x50, 1, true, false);
     d->ahci.irq = d->card.irq[0];
 
-    pci_register_bar(&d->card, 5, 0, &d->ahci.mem);
+    pci_register_bar(&d->card, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
+                     &d->ahci.idp);
+    pci_register_bar(&d->card, ICH9_MEM_BAR, PCI_BASE_ADDRESS_SPACE_MEMORY,
+                     &d->ahci.mem);
+
+    sata_cap_offset = pci_add_capability(&d->card, PCI_CAP_ID_SATA,
+                                         ICH9_SATA_CAP_OFFSET, SATA_CAP_SIZE);
+    if (sata_cap_offset < 0) {
+        return sata_cap_offset;
+    }
+
+    sata_cap = d->card.config + sata_cap_offset;
+    pci_set_word(sata_cap + SATA_CAP_REV, 0x10);
+    pci_set_long(sata_cap + SATA_CAP_BAR,
+                 (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4));
+    d->ahci.idp_offset = ICH9_IDP_INDEX;
 
     return 0;
 }
diff --git a/hw/pci_regs.h b/hw/pci_regs.h
index e884096..e8357c3 100644
--- a/hw/pci_regs.h
+++ b/hw/pci_regs.h
@@ -211,6 +211,7 @@ 
 #define  PCI_CAP_ID_AGP3	0x0E	/* AGP Target PCI-PCI bridge */
 #define  PCI_CAP_ID_EXP 	0x10	/* PCI Express */
 #define  PCI_CAP_ID_MSIX	0x11	/* MSI-X */
+#define  PCI_CAP_ID_SATA	0x12	/* Serial ATA */
 #define  PCI_CAP_ID_AF		0x13	/* PCI Advanced Features */
 #define PCI_CAP_LIST_NEXT	1	/* Next capability in the list */
 #define PCI_CAP_FLAGS		2	/* Capability defined flags (16 bits) */