Re: [Qemu-devel] [PATCH v3 26/32] ahci: MSI capability should be at 0x80, not 0x50.

[Michael S. Tsirkin]

On Wed, Aug 13, 2014 at 05:56:09PM -0400, John Snow wrote:
> In the Intel ICH9 data sheet, the MSI capability offset
> in the PCI configuration space for ICH9 AHCI devices is
> specified to be 0x80.
> 
> Further, the PCI capability pointer should always point
> to 0x80 in ICH9 devices, despite the fact that AHCI 1.3
> specifies that it should be pointing to PMCAP (Which in
> this instance would be 0x70) to maintain adherence to
> the Intel data sheet specifications and real observed behavior.
> 
> Signed-off-by: John Snow <address@hidden>
> ---
>  hw/ide/ich.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
> index a2f1639..d2a3ac2 100644
> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -71,6 +71,7 @@
>  #include <hw/ide/pci.h>
>  #include <hw/ide/ahci.h>
>  
> +#define ICH9_MSI_CAP_OFFSET     0x80
>  #define ICH9_SATA_CAP_OFFSET    0xA8
>  
>  #define ICH9_IDP_BAR            4
> @@ -115,7 +116,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>      /* XXX Software should program this register */
>      dev->config[0x90]   = 1 << 6; /* Address Map Register - AHCI mode */
>  
> -    msi_init(dev, 0x50, 1, true, false);
>      d->ahci.irq = pci_allocate_irq(dev);
>  
>      pci_register_bar(dev, ICH9_IDP_BAR, PCI_BASE_ADDRESS_SPACE_IO,
> @@ -135,6 +135,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>                   (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4));
>      d->ahci.idp_offset = ICH9_IDP_INDEX;
>  
> +    /* MSI cap should be added last to be first. */

I would put the explanation from the commit log
somewhere around this comment otherwise we risk
someone noting ahci spec says otherwise and
sending a patch "fixing" it.

> +    msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
> +
>      return 0;
>  }
>  
> -- 
> 1.9.3