| Message ID | 1408643079-30675-3-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2014-08-21 19:44, 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 <jsnow@redhat.com> > --- > hw/ide/ich.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/ide/ich.c b/hw/ide/ich.c > index a2f1639..8eb77a1 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,11 @@ static int pci_ich9_ahci_init(PCIDevice *dev) > (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4)); > d->ahci.idp_offset = ICH9_IDP_INDEX; > > + /* Although the AHCI 1.3 specification states that the first capability > + * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 > + * AHCI device puts the MSI capability first, pointing to 0x80. */ > + msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); > + > return 0; > } > > I did this for HDA recently, and it became clear that this requires a compat switch to expose the old, broken layout to older guests. Otherwise you can't migrate from them. Jan
On 09/17/2014 12:42 PM, Michael S. Tsirkin wrote: > On Wed, Sep 17, 2014 at 05:54:28PM +0200, Jan Kiszka wrote: >> On 2014-08-21 19:44, 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 <jsnow@redhat.com> >>> --- >>> hw/ide/ich.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ide/ich.c b/hw/ide/ich.c >>> index a2f1639..8eb77a1 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,11 @@ static int pci_ich9_ahci_init(PCIDevice *dev) >>> (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4)); >>> d->ahci.idp_offset = ICH9_IDP_INDEX; >>> >>> + /* Although the AHCI 1.3 specification states that the first capability >>> + * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 >>> + * AHCI device puts the MSI capability first, pointing to 0x80. */ >>> + msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); >>> + >>> return 0; >>> } >>> >>> >> >> I did this for HDA recently, and it became clear that this requires a >> compat switch to expose the old, broken layout to older guests. >> Otherwise you can't migrate from them. >> >> Jan > > Absolutely but ahci still disables migration :) I was halfway to preparing a v5, but you're right ... We've disabled AHCI migration until we iron out the most egregious kinks. > >> -- >> Siemens AG, Corporate Technology, CT RTC ITP SES-DE >> Corporate Competence Center Embedded Linux
On Wed, Sep 17, 2014 at 05:54:28PM +0200, Jan Kiszka wrote: > On 2014-08-21 19:44, 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 <jsnow@redhat.com> > > --- > > hw/ide/ich.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ide/ich.c b/hw/ide/ich.c > > index a2f1639..8eb77a1 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,11 @@ static int pci_ich9_ahci_init(PCIDevice *dev) > > (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4)); > > d->ahci.idp_offset = ICH9_IDP_INDEX; > > > > + /* Although the AHCI 1.3 specification states that the first capability > > + * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 > > + * AHCI device puts the MSI capability first, pointing to 0x80. */ > > + msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); > > + > > return 0; > > } > > > > > > I did this for HDA recently, and it became clear that this requires a > compat switch to expose the old, broken layout to older guests. > Otherwise you can't migrate from them. > > Jan Absolutely but ahci still disables migration :) > -- > Siemens AG, Corporate Technology, CT RTC ITP SES-DE > Corporate Competence Center Embedded Linux
diff --git a/hw/ide/ich.c b/hw/ide/ich.c index a2f1639..8eb77a1 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,11 @@ static int pci_ich9_ahci_init(PCIDevice *dev) (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4)); d->ahci.idp_offset = ICH9_IDP_INDEX; + /* Although the AHCI 1.3 specification states that the first capability + * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 + * AHCI device puts the MSI capability first, pointing to 0x80. */ + msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); + return 0; }
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 <jsnow@redhat.com> --- hw/ide/ich.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)