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

2014-08-14 Thread 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 js...@redhat.com
 ---
  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_OFFSET0xA8
  
  #define ICH9_IDP_BAR4
 @@ -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



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

2014-08-14 Thread 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 js...@redhat.com


Besides the comment I sent previously:

Reviewed-by: Michael S. Tsirkin m...@redhat.com


 ---
  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_OFFSET0xA8
  
  #define ICH9_IDP_BAR4
 @@ -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. */
 +msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
 +
  return 0;
  }
  
 -- 
 1.9.3



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

2014-08-13 Thread John Snow
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 js...@redhat.com
---
 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_OFFSET0xA8
 
 #define ICH9_IDP_BAR4
@@ -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. */
+msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false);
+
 return 0;
 }
 
-- 
1.9.3