* [Qemu-devel] [PATCH v9 1/8] pci: revise pci command register initialization
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-16 10:50 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 2/8] pci: fix accesses to pci status register Isaku Yamahata
` (8 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
This patch cleans up command register initialization with
comments. It also fixes the initialization of io/memory bit of command register.
Those bits for type 1 device is RW.
Those bits for type 0 device is
RO = 0 if it has no io/memory BAR
RW if it has io/memory BAR
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v8 -> v9
- patch squash
---
hw/pci.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 57 insertions(+), 1 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 962886e..2fc8ab1 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -544,8 +544,53 @@ static void pci_init_wmask(PCIDevice *dev)
dev->wmask[PCI_CACHE_LINE_SIZE] = 0xff;
dev->wmask[PCI_INTERRUPT_LINE] = 0xff;
+
+ /*
+ * bit 0: PCI_COMMAND_IO
+ * type 0: if IO BAR is used, RW
+ * This is handled by pci_register_bar()
+ * type 1: RW:
+ * This is fixed by pci_init_wmask_bridge()
+ * bit 1: PCI_COMMAND_MEMORY
+ * type 0: if IO BAR is used, RW
+ * This is handled by pci_register_bar()
+ * type 1: RW
+ * This is fixed by pci_init_wmask_bridge()
+ * bit 2: PCI_COMMAND_MASTER
+ * type 0: RW if bus master
+ * type 1: RW
+ * bit 3: PCI_COMMAND_SPECIAL
+ * RO=0, optionally RW: Such device should set this bit itself
+ * bit 4: PCI_COMMAND_INVALIDATE
+ * RO=0, optionally RW: Such device should set this bit itself
+ * bit 5: PCI_COMMAND_VGA_PALETTE
+ * RO=0, optionally RW: Such device should set this bit itself
+ * bit 6: PCI_COMMAND_PARITY
+ * RW with exceptions: Such device should clear this bit itself
+ * Given that qemu doesn't emulate pci bus cycles, so that there
+ * is no place to generate parity error. So just making this
+ * register RW is okay because there is no place which refers
+ * this bit.
+ * TODO: When device assignment tried to inject PERR# into qemu,
+ * some extra work would be needed.
+ * bit 7: PCI_COMMAND_WAIT: reserved (PCI 3.0)
+ * RO=0
+ * bit 8: PCI_COMMAND_SERR
+ * RW with exceptions: Such device should clear this bit itself
+ * Given that qemu doesn't emulate pci bus cycles, so that there
+ * is no place to generate system error. So just making this
+ * register RW is okay because there is no place which refers
+ * this bit.
+ * TODO: When device assignment tried to inject SERR# into qemu,
+ * some extra work would be needed.
+ * bit 9: PCI_COMMAND_FAST_BACK
+ * RO=0, optionally RW: Such device should set this bit itself
+ * bit 10: PCI_COMMAND_INTX_DISABLE
+ * RW
+ * bit 11-15: reserved
+ */
pci_set_word(dev->wmask + PCI_COMMAND,
- PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
+ PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
PCI_COMMAND_INTX_DISABLE);
memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
@@ -554,6 +599,9 @@ static void pci_init_wmask(PCIDevice *dev)
static void pci_init_wmask_bridge(PCIDevice *d)
{
+ pci_word_test_and_set_mask(d->wmask + PCI_COMMAND,
+ PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
+
/* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
PCI_SEC_LETENCY_TIMER */
memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
@@ -791,6 +839,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
if (region_num == PCI_ROM_SLOT) {
/* ROM enable bit is writeable */
wmask |= PCI_ROM_ADDRESS_ENABLE;
+ } else {
+ if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+ pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
+ PCI_COMMAND_IO);
+ } else {
+ pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
+ PCI_COMMAND_MEMORY);
+ }
}
pci_set_long(pci_dev->config + addr, type);
if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 1/8] pci: revise pci command register initialization Isaku Yamahata
@ 2010-11-16 10:50 ` Michael S. Tsirkin
2010-11-17 2:03 ` Isaku Yamahata
0 siblings, 1 reply; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-16 10:50 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 05:26:05PM +0900, Isaku Yamahata wrote:
> This patch cleans up command register initialization with
> comments. It also fixes the initialization of io/memory bit of command register.
> Those bits for type 1 device is RW.
> Those bits for type 0 device is
> RO = 0 if it has no io/memory BAR
> RW if it has io/memory BAR
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
There's a bug here: you can not assume that device that has
no io BAR claims no io transactions.
Another bug is that migrating from qemu where a bit is writeable to one
where it's RO creates a situation where a RW bit becomes RO, or the
reverse, which might confuse guests. So we will need a compatibility
flag and set it for old machine types.
> ---
> Changes v8 -> v9
> - patch squash
> ---
> hw/pci.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 57 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 962886e..2fc8ab1 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -544,8 +544,53 @@ static void pci_init_wmask(PCIDevice *dev)
>
> dev->wmask[PCI_CACHE_LINE_SIZE] = 0xff;
> dev->wmask[PCI_INTERRUPT_LINE] = 0xff;
> +
> + /*
> + * bit 0: PCI_COMMAND_IO
> + * type 0: if IO BAR is used, RW
> + * This is handled by pci_register_bar()
> + * type 1: RW:
> + * This is fixed by pci_init_wmask_bridge()
> + * bit 1: PCI_COMMAND_MEMORY
> + * type 0: if IO BAR is used, RW
> + * This is handled by pci_register_bar()
> + * type 1: RW
> + * This is fixed by pci_init_wmask_bridge()
> + * bit 2: PCI_COMMAND_MASTER
> + * type 0: RW if bus master
> + * type 1: RW
> + * bit 3: PCI_COMMAND_SPECIAL
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 4: PCI_COMMAND_INVALIDATE
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 5: PCI_COMMAND_VGA_PALETTE
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 6: PCI_COMMAND_PARITY
> + * RW with exceptions: Such device should clear this bit itself
> + * Given that qemu doesn't emulate pci bus cycles, so that there
> + * is no place to generate parity error. So just making this
> + * register RW is okay because there is no place which refers
> + * this bit.
> + * TODO: When device assignment tried to inject PERR# into qemu,
> + * some extra work would be needed.
> + * bit 7: PCI_COMMAND_WAIT: reserved (PCI 3.0)
> + * RO=0
> + * bit 8: PCI_COMMAND_SERR
> + * RW with exceptions: Such device should clear this bit itself
> + * Given that qemu doesn't emulate pci bus cycles, so that there
> + * is no place to generate system error. So just making this
> + * register RW is okay because there is no place which refers
> + * this bit.
> + * TODO: When device assignment tried to inject SERR# into qemu,
> + * some extra work would be needed.
> + * bit 9: PCI_COMMAND_FAST_BACK
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 10: PCI_COMMAND_INTX_DISABLE
> + * RW
> + * bit 11-15: reserved
> + */
Let's document non-obvious things, like maybe COMMAND_PARITY/COMMAND_SERR.
I don't cherish writing each bit up in two places.
> pci_set_word(dev->wmask + PCI_COMMAND,
> - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
> + PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> PCI_COMMAND_INTX_DISABLE);
>
> memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
> @@ -554,6 +599,9 @@ static void pci_init_wmask(PCIDevice *dev)
>
> static void pci_init_wmask_bridge(PCIDevice *d)
> {
> + pci_word_test_and_set_mask(d->wmask + PCI_COMMAND,
> + PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
> +
> /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
> PCI_SEC_LETENCY_TIMER */
> memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
> @@ -791,6 +839,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> if (region_num == PCI_ROM_SLOT) {
> /* ROM enable bit is writeable */
> wmask |= PCI_ROM_ADDRESS_ENABLE;
> + } else {
> + if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> + pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
> + PCI_COMMAND_IO);
> + } else {
> + pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
> + PCI_COMMAND_MEMORY);
> + }
> }
> pci_set_long(pci_dev->config + addr, type);
> if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization
2010-11-16 10:50 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-11-17 2:03 ` Isaku Yamahata
2010-11-17 12:02 ` Michael S. Tsirkin
0 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-17 2:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 12:50:19PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 16, 2010 at 05:26:05PM +0900, Isaku Yamahata wrote:
> > This patch cleans up command register initialization with
> > comments. It also fixes the initialization of io/memory bit of command register.
> > Those bits for type 1 device is RW.
> > Those bits for type 0 device is
> > RO = 0 if it has no io/memory BAR
> > RW if it has io/memory BAR
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> There's a bug here: you can not assume that device that has
> no io BAR claims no io transactions.
Which device are you mentioning?
Such device should set the bit by itself, not by pci generic layer.
> Another bug is that migrating from qemu where a bit is writeable to one
> where it's RO creates a situation where a RW bit becomes RO, or the
> reverse, which might confuse guests. So we will need a compatibility
> flag and set it for old machine types.
We needs to keep compatibility. Which way do you prefer?
- don't care: no way
- introduce global property to indicate compat qemu version or flags
something like if (compat version <= 0.13) old behaviour...
or if (flags & ...)
- introduce global-pci property
- introduce pci bus property
Users needs to specify this property for all pci devices.
- Don't change common code(pci.c), and provide a helper function.
Each device which needs new behavior like pcie calls it.
Probably each device may provide property to specify compat behavior
- any other?
>
> > ---
> > Changes v8 -> v9
> > - patch squash
> > ---
> > hw/pci.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 files changed, 57 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 962886e..2fc8ab1 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -544,8 +544,53 @@ static void pci_init_wmask(PCIDevice *dev)
> >
> > dev->wmask[PCI_CACHE_LINE_SIZE] = 0xff;
> > dev->wmask[PCI_INTERRUPT_LINE] = 0xff;
> > +
> > + /*
> > + * bit 0: PCI_COMMAND_IO
> > + * type 0: if IO BAR is used, RW
> > + * This is handled by pci_register_bar()
> > + * type 1: RW:
> > + * This is fixed by pci_init_wmask_bridge()
> > + * bit 1: PCI_COMMAND_MEMORY
> > + * type 0: if IO BAR is used, RW
> > + * This is handled by pci_register_bar()
> > + * type 1: RW
> > + * This is fixed by pci_init_wmask_bridge()
> > + * bit 2: PCI_COMMAND_MASTER
> > + * type 0: RW if bus master
> > + * type 1: RW
> > + * bit 3: PCI_COMMAND_SPECIAL
> > + * RO=0, optionally RW: Such device should set this bit itself
> > + * bit 4: PCI_COMMAND_INVALIDATE
> > + * RO=0, optionally RW: Such device should set this bit itself
> > + * bit 5: PCI_COMMAND_VGA_PALETTE
> > + * RO=0, optionally RW: Such device should set this bit itself
> > + * bit 6: PCI_COMMAND_PARITY
> > + * RW with exceptions: Such device should clear this bit itself
> > + * Given that qemu doesn't emulate pci bus cycles, so that there
> > + * is no place to generate parity error. So just making this
> > + * register RW is okay because there is no place which refers
> > + * this bit.
> > + * TODO: When device assignment tried to inject PERR# into qemu,
> > + * some extra work would be needed.
> > + * bit 7: PCI_COMMAND_WAIT: reserved (PCI 3.0)
> > + * RO=0
> > + * bit 8: PCI_COMMAND_SERR
> > + * RW with exceptions: Such device should clear this bit itself
> > + * Given that qemu doesn't emulate pci bus cycles, so that there
> > + * is no place to generate system error. So just making this
> > + * register RW is okay because there is no place which refers
> > + * this bit.
> > + * TODO: When device assignment tried to inject SERR# into qemu,
> > + * some extra work would be needed.
> > + * bit 9: PCI_COMMAND_FAST_BACK
> > + * RO=0, optionally RW: Such device should set this bit itself
> > + * bit 10: PCI_COMMAND_INTX_DISABLE
> > + * RW
> > + * bit 11-15: reserved
> > + */
>
> Let's document non-obvious things, like maybe COMMAND_PARITY/COMMAND_SERR.
> I don't cherish writing each bit up in two places.
>
> > pci_set_word(dev->wmask + PCI_COMMAND,
> > - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
> > + PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> > PCI_COMMAND_INTX_DISABLE);
> >
> > memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
> > @@ -554,6 +599,9 @@ static void pci_init_wmask(PCIDevice *dev)
> >
> > static void pci_init_wmask_bridge(PCIDevice *d)
> > {
> > + pci_word_test_and_set_mask(d->wmask + PCI_COMMAND,
> > + PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
> > +
> > /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
> > PCI_SEC_LETENCY_TIMER */
> > memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
> > @@ -791,6 +839,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> > if (region_num == PCI_ROM_SLOT) {
> > /* ROM enable bit is writeable */
> > wmask |= PCI_ROM_ADDRESS_ENABLE;
> > + } else {
> > + if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> > + pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
> > + PCI_COMMAND_IO);
> > + } else {
> > + pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
> > + PCI_COMMAND_MEMORY);
> > + }
> > }
> > pci_set_long(pci_dev->config + addr, type);
> > if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> > --
> > 1.7.1.1
>
--
yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization
2010-11-17 2:03 ` Isaku Yamahata
@ 2010-11-17 12:02 ` Michael S. Tsirkin
2010-11-18 2:08 ` Isaku Yamahata
0 siblings, 1 reply; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-17 12:02 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Nov 17, 2010 at 11:03:14AM +0900, Isaku Yamahata wrote:
> On Tue, Nov 16, 2010 at 12:50:19PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Nov 16, 2010 at 05:26:05PM +0900, Isaku Yamahata wrote:
> > > This patch cleans up command register initialization with
> > > comments. It also fixes the initialization of io/memory bit of command register.
> > > Those bits for type 1 device is RW.
> > > Those bits for type 0 device is
> > > RO = 0 if it has no io/memory BAR
> > > RW if it has io/memory BAR
> > >
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > There's a bug here: you can not assume that device that has
> > no io BAR claims no io transactions.
>
> Which device are you mentioning?
Look at appendix D in PCI spec. There are many programming
classes like display controllers that claim specific
addresses without using a BAR. We could code it all
up with a huge table. But what we have is much simpler
and works well AFAIK.
> Such device should set the bit by itself, not by pci generic layer.
Since this is PCI spec specified behaviour, I think it's better
to have it in a common place. Devices are sure to get it wrong.
> > Another bug is that migrating from qemu where a bit is writeable to one
> > where it's RO creates a situation where a RW bit becomes RO, or the
> > reverse, which might confuse guests. So we will need a compatibility
> > flag and set it for old machine types.
>
> We needs to keep compatibility. Which way do you prefer?
>
> - don't care: no way
>
> - introduce global property to indicate compat qemu version or flags
> something like if (compat version <= 0.13) old behaviour...
> or if (flags & ...)
>
> - introduce global-pci property
>
> - introduce pci bus property
> Users needs to specify this property for all pci devices.
>
> - Don't change common code(pci.c), and provide a helper function.
> Each device which needs new behavior like pcie calls it.
> Probably each device may provide property to specify compat behavior
>
> - any other?
- Don't change behaviour at all.
What is the motivation for the change? Why do we bother? What we have
is spec compliant, I think, so it's hard for me to believe pcie *needs*
the new behaviour.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization
2010-11-17 12:02 ` Michael S. Tsirkin
@ 2010-11-18 2:08 ` Isaku Yamahata
2010-11-18 6:42 ` Michael S. Tsirkin
0 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-18 2:08 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Nov 17, 2010 at 02:02:00PM +0200, Michael S. Tsirkin wrote:
> > > Another bug is that migrating from qemu where a bit is writeable to one
> > > where it's RO creates a situation where a RW bit becomes RO, or the
> > > reverse, which might confuse guests. So we will need a compatibility
> > > flag and set it for old machine types.
> >
> > We needs to keep compatibility. Which way do you prefer?
> >
> > - don't care: no way
> >
> > - introduce global property to indicate compat qemu version or flags
> > something like if (compat version <= 0.13) old behaviour...
> > or if (flags & ...)
> >
> > - introduce global-pci property
> >
> > - introduce pci bus property
> > Users needs to specify this property for all pci devices.
> >
> > - Don't change common code(pci.c), and provide a helper function.
> > Each device which needs new behavior like pcie calls it.
> > Probably each device may provide property to specify compat behavior
> >
> > - any other?
>
> - Don't change behaviour at all.
>
> What is the motivation for the change? Why do we bother? What we have
> is spec compliant, I think, so it's hard for me to believe pcie *needs*
> the new behaviour.
AER wants SERR bit to be writable and you requested it as below.
I thought, you wanted me to revise PCI_COMMAND and PCI_STATUS initialization.
If I misunderstood, can you please elaborate on it?
If you accept the following PCI_COMMAND line,
I'm fine with dropping this clean up patch.
http://lists.nongnu.org/archive/html/qemu-devel/2010-11/msg00131.html
> > +void pcie_aer_init(PCIDevice *dev, uint16_t offset)
> > +{
> > + PCIExpressDevice *exp;
> > +
> > + pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR);
> > + pci_word_test_and_set_mask(dev->w1cmask + PCI_STATUS,
> > + PCI_STATUS_SIG_SYSTEM_ERROR);
> > +
>
> I would say we should just set these for all devices.
> But if we do my concern is that guest might write 1 to this register,
> then we migrate to an old guest and that one can not clear this bit.
> Thoughts? Let's add a flag so old machine types can disable this?
>
>
> Also - what about other bits in the status register?
--
yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 1/8] pci: revise pci command register initialization
2010-11-18 2:08 ` Isaku Yamahata
@ 2010-11-18 6:42 ` Michael S. Tsirkin
0 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-18 6:42 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Thu, Nov 18, 2010 at 11:08:40AM +0900, Isaku Yamahata wrote:
> On Wed, Nov 17, 2010 at 02:02:00PM +0200, Michael S. Tsirkin wrote:
> > > > Another bug is that migrating from qemu where a bit is writeable to one
> > > > where it's RO creates a situation where a RW bit becomes RO, or the
> > > > reverse, which might confuse guests. So we will need a compatibility
> > > > flag and set it for old machine types.
> > >
> > > We needs to keep compatibility. Which way do you prefer?
> > >
> > > - don't care: no way
> > >
> > > - introduce global property to indicate compat qemu version or flags
> > > something like if (compat version <= 0.13) old behaviour...
> > > or if (flags & ...)
> > >
> > > - introduce global-pci property
> > >
> > > - introduce pci bus property
> > > Users needs to specify this property for all pci devices.
> > >
> > > - Don't change common code(pci.c), and provide a helper function.
> > > Each device which needs new behavior like pcie calls it.
> > > Probably each device may provide property to specify compat behavior
> > >
> > > - any other?
> >
> > - Don't change behaviour at all.
> >
> > What is the motivation for the change? Why do we bother? What we have
> > is spec compliant, I think, so it's hard for me to believe pcie *needs*
> > the new behaviour.
>
> AER wants SERR bit to be writable and you requested it as below.
> I thought, you wanted me to revise PCI_COMMAND and PCI_STATUS initialization.
> If I misunderstood, can you please elaborate on it?
> If you accept the following PCI_COMMAND line,
> I'm fine with dropping this clean up patch.
>
> http://lists.nongnu.org/archive/html/qemu-devel/2010-11/msg00131.html
> > > +void pcie_aer_init(PCIDevice *dev, uint16_t offset)
> > > +{
> > > + PCIExpressDevice *exp;
> > > +
> > > + pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR);
> > > + pci_word_test_and_set_mask(dev->w1cmask + PCI_STATUS,
> > > + PCI_STATUS_SIG_SYSTEM_ERROR);
> > > +
> >
> > I would say we should just set these for all devices.
> > But if we do my concern is that guest might write 1 to this register,
> > then we migrate to an old guest and that one can not clear this bit.
> > Thoughts? Let's add a flag so old machine types can disable this?
> >
> >
> > Also - what about other bits in the status register?
I think what you did for PCI_STATUS addressed this comment. Thanks!
Yes, for AER we need to enable SERR, and just for SERR, I think
a global property or a bus property will be the simplest (I think
properties are inherited from bus to device, right?)
Something like pci_command_serr_enable, should be a bit property.
Default to on and disable for 0.13 and back. Hmm?
Also, need to check this and fail aer initialization if not
there. Maybe simply don't add aer capability if aer_init fails?
Or make it yet another property ...
--
MST
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v9 2/8] pci: fix accesses to pci status register
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 1/8] pci: revise pci command register initialization Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-16 10:52 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 3/8] pci: clean up of " Isaku Yamahata
` (7 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
pci status register is 16 bit, not 8 bit.
So use helper function to manipulate status register.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 2fc8ab1..52fe655 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -127,9 +127,11 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
static void pci_update_irq_status(PCIDevice *dev)
{
if (dev->irq_state) {
- dev->config[PCI_STATUS] |= PCI_STATUS_INTERRUPT;
+ pci_word_test_and_set_mask(dev->config + PCI_STATUS,
+ PCI_STATUS_INTERRUPT);
} else {
- dev->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
+ pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
+ PCI_STATUS_INTERRUPT);
}
}
@@ -404,7 +406,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
* in irq_state which we are saving.
* This makes us compatible with old devices
* which never set or clear this bit. */
- s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
+ pci_word_test_and_clear_mask(s->config + PCI_STATUS, PCI_STATUS_INTERRUPT);
vmstate_save_state(f, pci_get_vmstate(s), s);
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
@@ -530,7 +532,7 @@ static void pci_init_cmask(PCIDevice *dev)
{
pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff);
pci_set_word(dev->cmask + PCI_DEVICE_ID, 0xffff);
- dev->cmask[PCI_STATUS] = PCI_STATUS_CAP_LIST;
+ pci_set_word(dev->cmask + PCI_STATUS, PCI_STATUS_CAP_LIST);
dev->cmask[PCI_REVISION_ID] = 0xff;
dev->cmask[PCI_CLASS_PROG] = 0xff;
pci_set_word(dev->cmask + PCI_CLASS_DEVICE, 0xffff);
@@ -1697,8 +1699,9 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
{
uint8_t next, prev;
- if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST))
+ if (!(pci_get_word(pdev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST)) {
return 0;
+ }
for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
prev = next + PCI_CAP_LIST_NEXT)
@@ -1804,7 +1807,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
config[PCI_CAP_LIST_ID] = cap_id;
config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
pdev->config[PCI_CAPABILITY_LIST] = offset;
- pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+ pci_word_test_and_set_mask(pdev->config + PCI_STATUS, PCI_STATUS_CAP_LIST);
memset(pdev->used + offset, 0xFF, size);
/* Make capability read-only by default */
memset(pdev->wmask + offset, 0, size);
@@ -1827,8 +1830,10 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
memset(pdev->cmask + offset, 0, size);
memset(pdev->used + offset, 0, size);
- if (!pdev->config[PCI_CAPABILITY_LIST])
- pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
+ if (!pdev->config[PCI_CAPABILITY_LIST]) {
+ pci_word_test_and_clear_mask(pdev->config + PCI_STATUS,
+ PCI_STATUS_CAP_LIST);
+ }
}
/* Reserve space for capability at a known offset (to call after load). */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 2/8] pci: fix accesses to pci status register
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 2/8] pci: fix accesses to pci status register Isaku Yamahata
@ 2010-11-16 10:52 ` Michael S. Tsirkin
2010-11-17 4:17 ` Isaku Yamahata
0 siblings, 1 reply; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-16 10:52 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 05:26:06PM +0900, Isaku Yamahata wrote:
> pci status register is 16 bit, not 8 bit.
> So use helper function to manipulate status register.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
At least the subject is wrong: the relevant bit is in the low byte. So
the code is correct as written on BE machines.
> ---
> hw/pci.c | 21 +++++++++++++--------
> 1 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 2fc8ab1..52fe655 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -127,9 +127,11 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
> static void pci_update_irq_status(PCIDevice *dev)
> {
> if (dev->irq_state) {
> - dev->config[PCI_STATUS] |= PCI_STATUS_INTERRUPT;
> + pci_word_test_and_set_mask(dev->config + PCI_STATUS,
> + PCI_STATUS_INTERRUPT);
> } else {
> - dev->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> + pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> + PCI_STATUS_INTERRUPT);
> }
> }
>
> @@ -404,7 +406,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> * in irq_state which we are saving.
> * This makes us compatible with old devices
> * which never set or clear this bit. */
> - s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> + pci_word_test_and_clear_mask(s->config + PCI_STATUS, PCI_STATUS_INTERRUPT);
> vmstate_save_state(f, pci_get_vmstate(s), s);
> /* Restore the interrupt status bit. */
> pci_update_irq_status(s);
> @@ -530,7 +532,7 @@ static void pci_init_cmask(PCIDevice *dev)
> {
> pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff);
> pci_set_word(dev->cmask + PCI_DEVICE_ID, 0xffff);
> - dev->cmask[PCI_STATUS] = PCI_STATUS_CAP_LIST;
> + pci_set_word(dev->cmask + PCI_STATUS, PCI_STATUS_CAP_LIST);
> dev->cmask[PCI_REVISION_ID] = 0xff;
> dev->cmask[PCI_CLASS_PROG] = 0xff;
> pci_set_word(dev->cmask + PCI_CLASS_DEVICE, 0xffff);
> @@ -1697,8 +1699,9 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
> {
> uint8_t next, prev;
>
> - if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST))
> + if (!(pci_get_word(pdev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST)) {
> return 0;
> + }
>
> for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
> prev = next + PCI_CAP_LIST_NEXT)
> @@ -1804,7 +1807,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> config[PCI_CAP_LIST_ID] = cap_id;
> config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
> pdev->config[PCI_CAPABILITY_LIST] = offset;
> - pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
> + pci_word_test_and_set_mask(pdev->config + PCI_STATUS, PCI_STATUS_CAP_LIST);
> memset(pdev->used + offset, 0xFF, size);
> /* Make capability read-only by default */
> memset(pdev->wmask + offset, 0, size);
> @@ -1827,8 +1830,10 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
> memset(pdev->cmask + offset, 0, size);
> memset(pdev->used + offset, 0, size);
>
> - if (!pdev->config[PCI_CAPABILITY_LIST])
> - pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
> + if (!pdev->config[PCI_CAPABILITY_LIST]) {
> + pci_word_test_and_clear_mask(pdev->config + PCI_STATUS,
> + PCI_STATUS_CAP_LIST);
> + }
> }
>
> /* Reserve space for capability at a known offset (to call after load). */
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 2/8] pci: fix accesses to pci status register
2010-11-16 10:52 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-11-17 4:17 ` Isaku Yamahata
0 siblings, 0 replies; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-17 4:17 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 12:52:31PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 16, 2010 at 05:26:06PM +0900, Isaku Yamahata wrote:
> > pci status register is 16 bit, not 8 bit.
> > So use helper function to manipulate status register.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> At least the subject is wrong: the relevant bit is in the low byte. So
> the code is correct as written on BE machines.
The code is correct, but also inconsistent and prone to errors.
Except the commit message, are you objecting the patch contents itself?
How about the following commit log?
pci: make accesses to pci status register consistent
pci status register is 16 bits, not 8 bits.
It is inconsistent to access 16 bits register as uint8_t.
So use pci config space functions to manipulate status register
for consistency.
>
> > ---
> > hw/pci.c | 21 +++++++++++++--------
> > 1 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 2fc8ab1..52fe655 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -127,9 +127,11 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
> > static void pci_update_irq_status(PCIDevice *dev)
> > {
> > if (dev->irq_state) {
> > - dev->config[PCI_STATUS] |= PCI_STATUS_INTERRUPT;
> > + pci_word_test_and_set_mask(dev->config + PCI_STATUS,
> > + PCI_STATUS_INTERRUPT);
> > } else {
> > - dev->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > + pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> > + PCI_STATUS_INTERRUPT);
> > }
> > }
> >
> > @@ -404,7 +406,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
> > * in irq_state which we are saving.
> > * This makes us compatible with old devices
> > * which never set or clear this bit. */
> > - s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> > + pci_word_test_and_clear_mask(s->config + PCI_STATUS, PCI_STATUS_INTERRUPT);
> > vmstate_save_state(f, pci_get_vmstate(s), s);
> > /* Restore the interrupt status bit. */
> > pci_update_irq_status(s);
> > @@ -530,7 +532,7 @@ static void pci_init_cmask(PCIDevice *dev)
> > {
> > pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff);
> > pci_set_word(dev->cmask + PCI_DEVICE_ID, 0xffff);
> > - dev->cmask[PCI_STATUS] = PCI_STATUS_CAP_LIST;
> > + pci_set_word(dev->cmask + PCI_STATUS, PCI_STATUS_CAP_LIST);
> > dev->cmask[PCI_REVISION_ID] = 0xff;
> > dev->cmask[PCI_CLASS_PROG] = 0xff;
> > pci_set_word(dev->cmask + PCI_CLASS_DEVICE, 0xffff);
> > @@ -1697,8 +1699,9 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
> > {
> > uint8_t next, prev;
> >
> > - if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST))
> > + if (!(pci_get_word(pdev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST)) {
> > return 0;
> > + }
> >
> > for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]);
> > prev = next + PCI_CAP_LIST_NEXT)
> > @@ -1804,7 +1807,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> > config[PCI_CAP_LIST_ID] = cap_id;
> > config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];
> > pdev->config[PCI_CAPABILITY_LIST] = offset;
> > - pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
> > + pci_word_test_and_set_mask(pdev->config + PCI_STATUS, PCI_STATUS_CAP_LIST);
> > memset(pdev->used + offset, 0xFF, size);
> > /* Make capability read-only by default */
> > memset(pdev->wmask + offset, 0, size);
> > @@ -1827,8 +1830,10 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size)
> > memset(pdev->cmask + offset, 0, size);
> > memset(pdev->used + offset, 0, size);
> >
> > - if (!pdev->config[PCI_CAPABILITY_LIST])
> > - pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
> > + if (!pdev->config[PCI_CAPABILITY_LIST]) {
> > + pci_word_test_and_clear_mask(pdev->config + PCI_STATUS,
> > + PCI_STATUS_CAP_LIST);
> > + }
> > }
> >
> > /* Reserve space for capability at a known offset (to call after load). */
> > --
> > 1.7.1.1
>
--
yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v9 3/8] pci: clean up of pci status register
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 1/8] pci: revise pci command register initialization Isaku Yamahata
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 2/8] pci: fix accesses to pci status register Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-16 14:01 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 4/8] pcie_regs.h: more constants Isaku Yamahata
` (6 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
This patch refine the initialization/reset of
pci status registers.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 41 +++++++++++++++++++++++++++++++++++++++--
1 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 52fe655..fba765b 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -145,6 +145,9 @@ static void pci_device_reset(PCIDevice *dev)
pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
pci_get_word(dev->wmask + PCI_COMMAND) |
pci_get_word(dev->w1cmask + PCI_COMMAND));
+ pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
+ pci_get_word(dev->wmask + PCI_STATUS) |
+ pci_get_word(dev->w1cmask + PCI_STATUS));
dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
dev->config[PCI_INTERRUPT_LINE] = 0x0;
for (r = 0; r < PCI_NUM_REGIONS; ++r) {
@@ -540,7 +543,7 @@ static void pci_init_cmask(PCIDevice *dev)
dev->cmask[PCI_CAPABILITY_LIST] = 0xff;
}
-static void pci_init_wmask(PCIDevice *dev)
+static void pci_init_wmask_w1cmask(PCIDevice *dev)
{
int config_size = pci_config_size(dev);
@@ -595,6 +598,40 @@ static void pci_init_wmask(PCIDevice *dev)
PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
PCI_COMMAND_INTX_DISABLE);
+ /*
+ * bit 0-2: reserved
+ * bit 3: PCI_STATUS_INTERRUPT: RO
+ * bit 4: PCI_STATUS_CAP_LIST: RO
+ * bit 5: PCI_STATUS_66MHZ: RO
+ * bit 6: PCI_STATUS_UDF: reserved (PCI 2.2-)
+ * bit 7: PCI_STATUS_FAST_BACK: RO
+ * bit 8: PCI_STATUS_PARITY
+ * type 0: RW for bus master
+ * type 1: RW1C
+ * bit 9-10: PCI_STATUS_DEVSEL: RO
+ * bit 11: PCI_STATUS_SIG_TARGET_ABORT
+ * type 0: RW1C for targets that is capable of terminating
+ * a transaction.
+ * type 1: RW1C
+ * bit 12: PCI_STATUS_REC_TARGET_ABORT
+ * type 0: RW1C for masters
+ * type 1: RW1C
+ * bit 13: PCI_STATUS_REC_MASTER_ABORT
+ * type 0: RW1C for masters
+ * type 1: RW1C
+ * bit 14: PCI_STATUS_SIG_SYSTEM_ERROR
+ * type 0: RW1C with execptions
+ * type 1: RW1C
+ * bit : PCI_STATUS_DETECTED_PARITY: RW1C
+ *
+ * It's okay to set w1mask even for RO=0(i.e. reserved) because
+ * writing value 1 to w1c bit whose value is 0 has no effect.
+ */
+ pci_set_word(dev->w1cmask + PCI_STATUS,
+ PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
+ PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
+ PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
+
memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
config_size - PCI_CONFIG_HEADER_SIZE);
}
@@ -725,7 +762,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
pci_set_default_subsystem_id(pci_dev);
}
pci_init_cmask(pci_dev);
- pci_init_wmask(pci_dev);
+ pci_init_wmask_w1cmask(pci_dev);
if (is_bridge) {
pci_init_wmask_bridge(pci_dev);
}
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 3/8] pci: clean up of pci status register
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 3/8] pci: clean up of " Isaku Yamahata
@ 2010-11-16 14:01 ` Michael S. Tsirkin
0 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-16 14:01 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 05:26:07PM +0900, Isaku Yamahata wrote:
> This patch refine the initialization/reset of
> pci status registers.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
This one seems good. Applied with some tweaks: I cut down the comment:
we don't really need to repeat what code does IMO, rather why it does
it, and clarified the comment text. Also split init_w1c out to a
function so that each function does one thing.
> ---
> hw/pci.c | 41 +++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 52fe655..fba765b 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -145,6 +145,9 @@ static void pci_device_reset(PCIDevice *dev)
> pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
> pci_get_word(dev->wmask + PCI_COMMAND) |
> pci_get_word(dev->w1cmask + PCI_COMMAND));
> + pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
> + pci_get_word(dev->wmask + PCI_STATUS) |
> + pci_get_word(dev->w1cmask + PCI_STATUS));
> dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
> dev->config[PCI_INTERRUPT_LINE] = 0x0;
> for (r = 0; r < PCI_NUM_REGIONS; ++r) {
> @@ -540,7 +543,7 @@ static void pci_init_cmask(PCIDevice *dev)
> dev->cmask[PCI_CAPABILITY_LIST] = 0xff;
> }
>
> -static void pci_init_wmask(PCIDevice *dev)
> +static void pci_init_wmask_w1cmask(PCIDevice *dev)
> {
> int config_size = pci_config_size(dev);
>
> @@ -595,6 +598,40 @@ static void pci_init_wmask(PCIDevice *dev)
> PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> PCI_COMMAND_INTX_DISABLE);
>
> + /*
> + * bit 0-2: reserved
> + * bit 3: PCI_STATUS_INTERRUPT: RO
> + * bit 4: PCI_STATUS_CAP_LIST: RO
> + * bit 5: PCI_STATUS_66MHZ: RO
> + * bit 6: PCI_STATUS_UDF: reserved (PCI 2.2-)
> + * bit 7: PCI_STATUS_FAST_BACK: RO
> + * bit 8: PCI_STATUS_PARITY
> + * type 0: RW for bus master
> + * type 1: RW1C
> + * bit 9-10: PCI_STATUS_DEVSEL: RO
> + * bit 11: PCI_STATUS_SIG_TARGET_ABORT
> + * type 0: RW1C for targets that is capable of terminating
> + * a transaction.
> + * type 1: RW1C
> + * bit 12: PCI_STATUS_REC_TARGET_ABORT
> + * type 0: RW1C for masters
> + * type 1: RW1C
> + * bit 13: PCI_STATUS_REC_MASTER_ABORT
> + * type 0: RW1C for masters
> + * type 1: RW1C
> + * bit 14: PCI_STATUS_SIG_SYSTEM_ERROR
> + * type 0: RW1C with execptions
> + * type 1: RW1C
> + * bit : PCI_STATUS_DETECTED_PARITY: RW1C
> + *
> + * It's okay to set w1mask even for RO=0(i.e. reserved) because
> + * writing value 1 to w1c bit whose value is 0 has no effect.
> + */
> + pci_set_word(dev->w1cmask + PCI_STATUS,
> + PCI_STATUS_PARITY | PCI_STATUS_SIG_TARGET_ABORT |
> + PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_REC_MASTER_ABORT |
> + PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY);
> +
> memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
> config_size - PCI_CONFIG_HEADER_SIZE);
> }
> @@ -725,7 +762,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> pci_set_default_subsystem_id(pci_dev);
> }
> pci_init_cmask(pci_dev);
> - pci_init_wmask(pci_dev);
> + pci_init_wmask_w1cmask(pci_dev);
> if (is_bridge) {
> pci_init_wmask_bridge(pci_dev);
> }
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v9 4/8] pcie_regs.h: more constants
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
` (2 preceding siblings ...)
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 3/8] pci: clean up of " Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
` (5 subsequent siblings)
9 siblings, 0 replies; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
remove unnecessary sizeof.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pcie_regs.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/hw/pcie_regs.h b/hw/pcie_regs.h
index 3461a1b..4d123d9 100644
--- a/hw/pcie_regs.h
+++ b/hw/pcie_regs.h
@@ -94,7 +94,9 @@
#define PCI_ERR_CAP_MHRE 0x00000400
#define PCI_ERR_CAP_TLP 0x00000800
+#define PCI_ERR_HEADER_LOG_SIZE 16
#define PCI_ERR_TLP_PREFIX_LOG 0x38
+#define PCI_ERR_TLP_PREFIX_LOG_SIZE 16
#define PCI_SEC_STATUS_RCV_SYSTEM_ERROR 0x4000
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
` (3 preceding siblings ...)
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 4/8] pcie_regs.h: more constants Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-17 14:06 ` [Qemu-devel] [PATCH 0/2] " Michael S. Tsirkin
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 6/8] ioh3420: support aer Isaku Yamahata
` (4 subsequent siblings)
9 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
This patch implements helper functions for pcie aer capability
which will be used later.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v8 -> v9:
- make pcie_aer_init() return error
Changes v7 -> v8:
- various changes to follow the review.
- s/TLP_PRESENT/TLP_PREFIX_PRESENT/g
Changes v6 -> v7:
- make error log buffer non-ringed buffer
- refactor pcie_aer_error_inject()
- clean up of pcie_aer_write_config()
- make pcie_aer_inject_error() return error
- remove AERMsgResult
- remove PCIEAERSeverity
- reduce forward declarations
- style clean ups
- remove paren for sizeof.
Chnages v5 -> v6:
- cleaned up pcie_aer_write_config().
- enum definition.
Changes v4 -> v5:
- use pci_xxx_test_and_xxx_mask()
- rewrote PCIDevice::written bits.
- eliminated pcie_aer_notify()
- introduced PCIExpressDevice::aer_intx
Changes v3 -> v4:
- various naming fixes.
- use pci bit operation helper function
- eliminate errmsg function pointer
- replace pci_shift_xxx() with PCIDevice::written
- uncorrect error status register.
- dropped pcie_aer_cap()
Changes v2 -> v3:
- split out from pcie.[ch] to pcie_aer.[ch] to make the files sorter.
- embeded PCIExpressDevice into PCIDevice.
- CodingStyle fix
---
Makefile.objs | 2 +-
hw/pcie.h | 14 +
hw/pcie_aer.c | 827 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pcie_aer.h | 106 ++++++++
qemu-common.h | 3 +
5 files changed, 951 insertions(+), 1 deletions(-)
create mode 100644 hw/pcie_aer.c
create mode 100644 hw/pcie_aer.h
diff --git a/Makefile.objs b/Makefile.objs
index 15569af..6f01220 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -208,7 +208,7 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
# PCI watchdog devices
hw-obj-y += wdt_i6300esb.o
-hw-obj-y += pcie.o pcie_port.o
+hw-obj-y += pcie.o pcie_aer.o pcie_port.o
hw-obj-y += msix.o msi.o
# PCI network cards
diff --git a/hw/pcie.h b/hw/pcie.h
index 8708504..7baa813 100644
--- a/hw/pcie.h
+++ b/hw/pcie.h
@@ -24,6 +24,7 @@
#include "hw.h"
#include "pci_regs.h"
#include "pcie_regs.h"
+#include "pcie_aer.h"
typedef enum {
/* for attention and power indicator */
@@ -79,6 +80,19 @@ struct PCIExpressDevice {
Software Notification of Hot-Plug Events, an interrupt
is sent whenever the logical and of these conditions
transitions from false to true. */
+
+ /* AER */
+ uint16_t aer_cap;
+ PCIEAERLog aer_log;
+ unsigned int aer_intx; /* INTx for error reporting
+ * default is 0 = INTA#
+ * If the chip wants to use other interrupt
+ * line, initialize this member with the
+ * desired number.
+ * If the chip dynamically changes this member,
+ * also initialize it when loaded as
+ * appropreately.
+ */
};
/* PCI express capability helper functions */
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
new file mode 100644
index 0000000..acf826a
--- /dev/null
+++ b/hw/pcie_aer.c
@@ -0,0 +1,827 @@
+/*
+ * pcie_aer.c
+ *
+ * Copyright (c) 2010 Isaku Yamahata <yamahata at valinux co jp>
+ * VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "sysemu.h"
+#include "pci_bridge.h"
+#include "pcie.h"
+#include "msix.h"
+#include "msi.h"
+#include "pci_internals.h"
+#include "pcie_regs.h"
+
+//#define DEBUG_PCIE
+#ifdef DEBUG_PCIE
+# define PCIE_DPRINTF(fmt, ...) \
+ fprintf(stderr, "%s:%d " fmt, __func__, __LINE__, ## __VA_ARGS__)
+#else
+# define PCIE_DPRINTF(fmt, ...) do {} while (0)
+#endif
+#define PCIE_DEV_PRINTF(dev, fmt, ...) \
+ PCIE_DPRINTF("%s:%x "fmt, (dev)->name, (dev)->devfn, ## __VA_ARGS__)
+
+/* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
+static uint32_t pcie_aer_uncor_default_severity(uint32_t status)
+{
+ switch (status) {
+ case PCI_ERR_UNC_INTN:
+ case PCI_ERR_UNC_DLP:
+ case PCI_ERR_UNC_SDN:
+ case PCI_ERR_UNC_RX_OVER:
+ case PCI_ERR_UNC_FCP:
+ case PCI_ERR_UNC_MALF_TLP:
+ return PCI_ERR_ROOT_CMD_FATAL_EN;
+ case PCI_ERR_UNC_POISON_TLP:
+ case PCI_ERR_UNC_ECRC:
+ case PCI_ERR_UNC_UNSUP:
+ case PCI_ERR_UNC_COMP_TIME:
+ case PCI_ERR_UNC_COMP_ABORT:
+ case PCI_ERR_UNC_UNX_COMP:
+ case PCI_ERR_UNC_ACSV:
+ case PCI_ERR_UNC_MCBTLP:
+ case PCI_ERR_UNC_ATOP_EBLOCKED:
+ case PCI_ERR_UNC_TLP_PRF_BLOCKED:
+ return PCI_ERR_ROOT_CMD_NONFATAL_EN;
+ default:
+ abort();
+ break;
+ }
+ return PCI_ERR_ROOT_CMD_FATAL_EN;
+}
+
+static int aer_log_add_err(PCIEAERLog *aer_log, const PCIEAERErr *err)
+{
+ if (aer_log->log_num == aer_log->log_max) {
+ return -1;
+ }
+ memcpy(&aer_log->log[aer_log->log_num], err, sizeof *err);
+ aer_log->log_num++;
+ return 0;
+}
+
+static void aer_log_del_err(PCIEAERLog *aer_log, PCIEAERErr *err)
+{
+ assert(aer_log->log_num);
+ *err = aer_log->log[0];
+ aer_log->log_num--;
+ memmove(&aer_log->log[0], &aer_log->log[1],
+ aer_log->log_num * sizeof *err);
+}
+
+static void aer_log_clear_all_err(PCIEAERLog *aer_log)
+{
+ aer_log->log_num = 0;
+}
+
+int pcie_aer_init(PCIDevice *dev, uint16_t offset)
+{
+ PCIExpressDevice *exp;
+
+ pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
+ offset, PCI_ERR_SIZEOF);
+ exp = &dev->exp;
+ exp->aer_cap = offset;
+
+ /* log_max is property */
+ if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) {
+ dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
+ }
+ /* clip down the value to avoid unreasobale memory usage */
+ if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) {
+ return -EINVAL;
+ }
+ dev->exp.aer_log.log = qemu_mallocz(sizeof dev->exp.aer_log.log[0] *
+ dev->exp.aer_log.log_max);
+
+ pci_set_long(dev->w1cmask + offset + PCI_ERR_UNCOR_STATUS,
+ PCI_ERR_UNC_SUPPORTED);
+
+ pci_set_long(dev->config + offset + PCI_ERR_UNCOR_SEVER,
+ PCI_ERR_UNC_SEVERITY_DEFAULT);
+ pci_set_long(dev->wmask + offset + PCI_ERR_UNCOR_SEVER,
+ PCI_ERR_UNC_SUPPORTED);
+
+ pci_long_test_and_set_mask(dev->w1cmask + offset + PCI_ERR_COR_STATUS,
+ PCI_ERR_COR_STATUS);
+
+ pci_set_long(dev->config + offset + PCI_ERR_COR_MASK,
+ PCI_ERR_COR_MASK_DEFAULT);
+ pci_set_long(dev->wmask + offset + PCI_ERR_COR_MASK,
+ PCI_ERR_COR_SUPPORTED);
+
+ /* capabilities and control. multiple header logging is supported */
+ if (dev->exp.aer_log.log_max > 0) {
+ pci_set_long(dev->config + offset + PCI_ERR_CAP,
+ PCI_ERR_CAP_ECRC_GENC | PCI_ERR_CAP_ECRC_CHKC |
+ PCI_ERR_CAP_MHRC);
+ pci_set_long(dev->wmask + offset + PCI_ERR_CAP,
+ PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE |
+ PCI_ERR_CAP_MHRE);
+ } else {
+ pci_set_long(dev->config + offset + PCI_ERR_CAP,
+ PCI_ERR_CAP_ECRC_GENC | PCI_ERR_CAP_ECRC_CHKC);
+ pci_set_long(dev->wmask + offset + PCI_ERR_CAP,
+ PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
+ }
+
+ switch (pcie_cap_get_type(dev)) {
+ case PCI_EXP_TYPE_ROOT_PORT:
+ /* this case will be set by pcie_aer_root_init() */
+ /* fallthrough */
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ case PCI_EXP_TYPE_UPSTREAM:
+ pci_word_test_and_set_mask(dev->wmask + PCI_BRIDGE_CONTROL,
+ PCI_BRIDGE_CTL_SERR);
+ pci_long_test_and_set_mask(dev->w1cmask + PCI_STATUS,
+ PCI_SEC_STATUS_RCV_SYSTEM_ERROR);
+ break;
+ default:
+ /* nothing */
+ break;
+ }
+ return 0;
+}
+
+void pcie_aer_exit(PCIDevice *dev)
+{
+ qemu_free(dev->exp.aer_log.log);
+}
+
+static void pcie_aer_update_uncor_status(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ PCIEAERLog *aer_log = &dev->exp.aer_log;
+
+ uint16_t i;
+ for (i = 0; i < aer_log->log_num; i++) {
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
+ dev->exp.aer_log.log[i].status);
+ }
+}
+
+/*
+ * pcie_aer_msg() is called recursively by
+ * pcie_aer_msg_alldev(), pci_aer_msg_vbridge() and pcie_aer_msg_root_port()
+ */
+static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
+
+/*
+ * return value:
+ * true: error message is sent up
+ * false: error message is masked
+ *
+ * 6.2.6 Error Message Control
+ * Figure 6-3
+ * all pci express devices part
+ */
+static bool
+pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ PCIDevice *parent_port;
+
+ if (!(pcie_aer_msg_is_uncor(msg) &&
+ (pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_SERR))) {
+ return false;
+ }
+
+ /* Signaled System Error
+ *
+ * 7.5.1.1 Command register
+ * Bit 8 SERR# Enable
+ *
+ * When Set, this bit enables reporting of Non-fatal and Fatal
+ * errors detected by the Function to the Root Complex. Note that
+ * errors are reported if enabled either through this bit or through
+ * the PCI Express specific bits in the Device Control register (see
+ * Section 7.8.4).
+ */
+ pci_word_test_and_set_mask(dev->config + PCI_STATUS,
+ PCI_STATUS_SIG_SYSTEM_ERROR);
+
+ if (!(msg->severity &
+ pci_get_word(dev->config + dev->exp.exp_cap + PCI_EXP_DEVCTL))) {
+ return false;
+ }
+
+ /* send up error message */
+ if (pci_is_express(dev) &&
+ pcie_cap_get_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
+ /* Root port notify system itself,
+ or send the error message to root complex event collector. */
+ /*
+ * if root port is associated to event collector, set
+ * parent_port = root complex event collector
+ * For now root complex event collector isn't supported.
+ */
+ parent_port = NULL;
+ } else {
+ parent_port = pci_bridge_get_device(dev->bus);
+ }
+ if (parent_port) {
+ if (!pci_is_express(parent_port)) {
+ /* just ignore it */
+ return false;
+ }
+ pcie_aer_msg(parent_port, msg);
+ }
+ return true;
+}
+
+/*
+ * return value:
+ * true: error message is sent up
+ * false: error message is masked
+ *
+ * 6.2.6 Error Message Control
+ * Figure 6-3
+ * virtual pci bridge part
+ */
+static bool pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ uint16_t bridge_control = pci_get_word(dev->config + PCI_BRIDGE_CONTROL);
+
+ if (pcie_aer_msg_is_uncor(msg)) {
+ /* Received System Error */
+ pci_word_test_and_set_mask(dev->config + PCI_SEC_STATUS,
+ PCI_SEC_STATUS_RCV_SYSTEM_ERROR);
+ }
+
+ if (!(bridge_control & PCI_BRIDGE_CTL_SERR)) {
+ return false;
+ }
+ return true;
+}
+
+void pcie_aer_root_set_vector(PCIDevice *dev, unsigned int vector)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ assert(vector < PCI_ERR_ROOT_IRQ_MAX);
+ pci_long_test_and_clear_mask(aer_cap + PCI_ERR_ROOT_STATUS,
+ PCI_ERR_ROOT_IRQ);
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_ROOT_STATUS,
+ vector << PCI_ERR_ROOT_IRQ_SHIFT);
+}
+
+static unsigned int pcie_aer_root_get_vector(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+ return (root_status & PCI_ERR_ROOT_IRQ) >> PCI_ERR_ROOT_IRQ_SHIFT;
+}
+
+/*
+ * return value:
+ * true: error message is sent up
+ * false: error message is masked
+ *
+ * 6.2.6 Error Message Control
+ * Figure 6-3
+ * root port part
+ */
+static bool pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ bool msg_sent;
+ uint16_t cmd;
+ uint8_t *aer_cap;
+ uint32_t root_cmd;
+ uint32_t root_status;
+ bool msi_trigger;
+
+ msg_sent = false;
+ cmd = pci_get_word(dev->config + PCI_COMMAND);
+ aer_cap = dev->config + dev->exp.aer_cap;
+ root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
+ root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+ msi_trigger = false;
+
+ if (cmd & PCI_COMMAND_SERR) {
+ /* System Error.
+ *
+ * The way to report System Error is platform specific and
+ * it isn't implemented in qemu right now.
+ * So just discard the error for now.
+ * OS which cares of aer would receive errors via
+ * native aer mechanims, so this wouldn't matter.
+ */
+ }
+
+ /* Errro Message Received: Root Error Status register */
+ switch (msg->severity) {
+ case PCI_ERR_ROOT_CMD_COR_EN:
+ if (root_status & PCI_ERR_ROOT_COR_RCV) {
+ root_status |= PCI_ERR_ROOT_MULTI_COR_RCV;
+ } else {
+ if (root_cmd & PCI_ERR_ROOT_CMD_COR_EN) {
+ msi_trigger = true;
+ }
+ pci_set_word(aer_cap + PCI_ERR_ROOT_COR_SRC, msg->source_id);
+ }
+ root_status |= PCI_ERR_ROOT_COR_RCV;
+ break;
+ case PCI_ERR_ROOT_CMD_NONFATAL_EN:
+ if (!(root_status & PCI_ERR_ROOT_NONFATAL_RCV) &&
+ root_cmd & PCI_ERR_ROOT_CMD_NONFATAL_EN) {
+ msi_trigger = true;
+ }
+ root_status |= PCI_ERR_ROOT_NONFATAL_RCV;
+ break;
+ case PCI_ERR_ROOT_CMD_FATAL_EN:
+ if (!(root_status & PCI_ERR_ROOT_FATAL_RCV) &&
+ root_cmd & PCI_ERR_ROOT_CMD_FATAL_EN) {
+ msi_trigger = true;
+ }
+ if (!(root_status & PCI_ERR_ROOT_UNCOR_RCV)) {
+ root_status |= PCI_ERR_ROOT_FIRST_FATAL;
+ }
+ root_status |= PCI_ERR_ROOT_FATAL_RCV;
+ break;
+ default:
+ abort();
+ break;
+ }
+ if (pcie_aer_msg_is_uncor(msg)) {
+ if (root_status & PCI_ERR_ROOT_UNCOR_RCV) {
+ root_status |= PCI_ERR_ROOT_MULTI_UNCOR_RCV;
+ } else {
+ pci_set_word(aer_cap + PCI_ERR_ROOT_SRC, msg->source_id);
+ }
+ root_status |= PCI_ERR_ROOT_UNCOR_RCV;
+ }
+ pci_set_long(aer_cap + PCI_ERR_ROOT_STATUS, root_status);
+
+ if (root_cmd & msg->severity) {
+ /* 6.2.4.1.2 Interrupt Generation */
+ if (pci_msi_enabled(dev)) {
+ if (msi_trigger) {
+ pci_msi_notify(dev, pcie_aer_root_get_vector(dev));
+ }
+ } else {
+ qemu_set_irq(dev->irq[dev->exp.aer_intx], 1);
+ }
+ msg_sent = true;
+ }
+ return msg_sent;
+}
+
+/*
+ * 6.2.6 Error Message Control Figure 6-3
+ */
+static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ uint8_t type;
+ bool msg_sent;
+
+ assert(pci_is_express(dev));
+
+ type = pcie_cap_get_type(dev);
+ if (type == PCI_EXP_TYPE_ROOT_PORT ||
+ type == PCI_EXP_TYPE_UPSTREAM ||
+ type == PCI_EXP_TYPE_DOWNSTREAM) {
+ msg_sent = pcie_aer_msg_vbridge(dev, msg);
+ if (!msg_sent) {
+ return;
+ }
+ }
+ msg_sent = pcie_aer_msg_alldev(dev, msg);
+ if (type == PCI_EXP_TYPE_ROOT_PORT && msg_sent) {
+ pcie_aer_msg_root_port(dev, msg);
+ }
+}
+
+static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint8_t first_bit = ffsl(err->status) - 1;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ int i;
+
+ assert(err->status);
+ assert(err->status & (err->status - 1));
+
+ errcap &= ~(PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP);
+ errcap |= PCI_ERR_CAP_FEP(first_bit);
+
+ if (err->flags & PCIE_AER_ERR_HEADER_VALID) {
+ for (i = 0; i < ARRAY_SIZE(err->header); ++i) {
+ /* 7.10.8 Header Log Register */
+ uint8_t *header_log =
+ aer_cap + PCI_ERR_HEADER_LOG + i * sizeof err->header[0];
+ cpu_to_be32wu((uint32_t*)header_log, err->header[i]);
+ }
+ } else {
+ assert(!(err->flags & PCIE_AER_ERR_TLP_PREFIX_PRESENT));
+ memset(aer_cap + PCI_ERR_HEADER_LOG, 0, PCI_ERR_HEADER_LOG_SIZE);
+ }
+
+ if ((err->flags & PCIE_AER_ERR_TLP_PREFIX_PRESENT) &&
+ (pci_get_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVCTL2) &
+ PCI_EXP_DEVCAP2_EETLPP)) {
+ for (i = 0; i < ARRAY_SIZE(err->prefix); ++i) {
+ /* 7.10.12 tlp prefix log register */
+ uint8_t *prefix_log =
+ aer_cap + PCI_ERR_TLP_PREFIX_LOG + i * sizeof err->prefix[0];
+ cpu_to_be32wu((uint32_t*)prefix_log, err->prefix[i]);
+ }
+ errcap |= PCI_ERR_CAP_TLP;
+ } else {
+ memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0,
+ PCI_ERR_TLP_PREFIX_LOG_SIZE);
+ }
+ pci_set_long(aer_cap + PCI_ERR_CAP, errcap);
+}
+
+static void pcie_aer_clear_log(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+
+ pci_long_test_and_clear_mask(aer_cap + PCI_ERR_CAP,
+ PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP);
+
+ memset(aer_cap + PCI_ERR_HEADER_LOG, 0, PCI_ERR_HEADER_LOG_SIZE);
+ memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, PCI_ERR_TLP_PREFIX_LOG_SIZE);
+}
+
+static void pcie_aer_clear_error(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ PCIEAERLog *aer_log = &dev->exp.aer_log;
+ PCIEAERErr err;
+
+ if (!(errcap & PCI_ERR_CAP_MHRE) || !aer_log->log_num) {
+ pcie_aer_clear_log(dev);
+ return;
+ }
+
+ /*
+ * If more errors are queued, set corresponding bits in uncorrectable
+ * error status.
+ * We emulate uncorrectable error status register as W1CS.
+ * So set bit in uncorrectable error status here again for multiple
+ * error recording support.
+ *
+ * 6.2.4.2 Multiple Error Handling(Advanced Error Reporting Capability)
+ */
+ pcie_aer_update_uncor_status(dev);
+
+ aer_log_del_err(aer_log, &err);
+ pcie_aer_update_log(dev, &err);
+}
+
+static int pcie_aer_record_error(PCIDevice *dev,
+ const PCIEAERErr *err)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ int fep = PCI_ERR_CAP_FEP(errcap);
+
+ assert(err->status);
+ assert(err->status & (err->status - 1));
+
+ if (errcap & PCI_ERR_CAP_MHRE &&
+ (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1U << fep))) {
+ /* Not first error. queue error */
+ if (aer_log_add_err(&dev->exp.aer_log, err) < 0) {
+ /* overflow */
+ return -1;
+ }
+ return 0;
+ }
+
+ pcie_aer_update_log(dev, err);
+ return 0;
+}
+
+typedef struct PCIEAERInject {
+ PCIDevice *dev;
+ uint8_t *aer_cap;
+ const PCIEAERErr *err;
+ uint16_t devctl;
+ uint16_t devsta;
+ uint32_t error_status;
+ bool unsupported_request;
+ bool log_overflow;
+ PCIEAERMsg msg;
+} PCIEAERInject;
+
+static bool pcie_aer_inject_cor_error(PCIEAERInject *inj,
+ uint32_t uncor_status,
+ bool is_advisory_nonfatal)
+{
+ PCIDevice *dev = inj->dev;
+
+ inj->devsta |= PCI_EXP_DEVSTA_CED;
+ if (inj->unsupported_request) {
+ inj->devsta |= PCI_EXP_DEVSTA_URD;
+ }
+ pci_set_word(dev->config + dev->exp.exp_cap + PCI_EXP_DEVSTA, inj->devsta);
+
+ if (inj->aer_cap) {
+ uint32_t mask;
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_COR_STATUS,
+ inj->error_status);
+ mask = pci_get_long(inj->aer_cap + PCI_ERR_COR_MASK);
+ if (mask & inj->error_status) {
+ return false;
+ }
+ if (is_advisory_nonfatal) {
+ uint32_t uncor_mask =
+ pci_get_long(inj->aer_cap + PCI_ERR_UNCOR_MASK);
+ if (!(uncor_mask & uncor_status)) {
+ inj->log_overflow = !!pcie_aer_record_error(dev, inj->err);
+ }
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
+ uncor_status);
+ }
+ }
+
+ if (inj->unsupported_request && !(inj->devctl & PCI_EXP_DEVCTL_URRE)) {
+ return false;
+ }
+ if (!(inj->devctl & PCI_EXP_DEVCTL_CERE)) {
+ return false;
+ }
+
+ inj->msg.severity = PCI_ERR_ROOT_CMD_COR_EN;
+ return true;
+}
+
+static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal)
+{
+ PCIDevice *dev = inj->dev;
+ uint16_t cmd;
+
+ if (is_fatal) {
+ inj->devsta |= PCI_EXP_DEVSTA_FED;
+ } else {
+ inj->devsta |= PCI_EXP_DEVSTA_NFED;
+ }
+ if (inj->unsupported_request) {
+ inj->devsta |= PCI_EXP_DEVSTA_URD;
+ }
+ pci_set_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVSTA, inj->devsta);
+
+ if (inj->aer_cap) {
+ uint32_t mask = pci_get_long(inj->aer_cap + PCI_ERR_UNCOR_MASK);
+ if (mask & inj->error_status) {
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
+ inj->error_status);
+ return false;
+ }
+
+ inj->log_overflow = !!pcie_aer_record_error(dev, inj->err);
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
+ inj->error_status);
+ }
+
+ cmd = pci_get_word(dev->config + PCI_COMMAND);
+ if (inj->unsupported_request &&
+ !(inj->devctl & PCI_EXP_DEVCTL_URRE) && !(cmd & PCI_COMMAND_SERR)) {
+ return false;
+ }
+ if (is_fatal) {
+ if (!((cmd & PCI_COMMAND_SERR) ||
+ (inj->devctl & PCI_EXP_DEVCTL_FERE))) {
+ return false;
+ }
+ inj->msg.severity = PCI_ERR_ROOT_CMD_FATAL_EN;
+ } else {
+ if (!((cmd & PCI_COMMAND_SERR) ||
+ (inj->devctl & PCI_EXP_DEVCTL_NFERE))) {
+ return false;
+ }
+ inj->msg.severity = PCI_ERR_ROOT_CMD_NONFATAL_EN;
+ }
+ return true;
+}
+
+/*
+ * non-Function specific error must be recorded in all functions.
+ * It is the responsibility of the caller of this function.
+ * It is also caller's responsiblity to determine which function should
+ * report the rerror.
+ *
+ * 6.2.4 Error Logging
+ * 6.2.5 Sqeunce of Device Error Signaling and Logging Operations
+ * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging
+ * Operations
+ */
+int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
+{
+ uint8_t *aer_cap = NULL;
+ uint16_t devctl = 0;
+ uint16_t devsta = 0;
+ uint32_t error_status = err->status;
+ PCIEAERInject inj;
+
+ if (!pci_is_express(dev)) {
+ return -ENOSYS;
+ }
+
+ if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
+ error_status &= PCI_ERR_COR_SUPPORTED;
+ } else {
+ error_status &= PCI_ERR_UNC_SUPPORTED;
+ }
+
+ /* invalid status bit. one and only one bit must be set */
+ if (!error_status || (error_status & (error_status - 1))) {
+ return -EINVAL;
+ }
+
+ if (dev->exp.aer_cap) {
+ uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
+ aer_cap = dev->config + dev->exp.aer_cap;
+ devctl = pci_get_long(exp_cap + PCI_EXP_DEVCTL);
+ devsta = pci_get_long(exp_cap + PCI_EXP_DEVSTA);
+ }
+
+ inj.dev = dev;
+ inj.aer_cap = aer_cap;
+ inj.err = err;
+ inj.devctl = devctl;
+ inj.devsta = devsta;
+ inj.error_status = error_status;
+ inj.unsupported_request = !(err->flags & PCIE_AER_ERR_IS_CORRECTABLE) &&
+ err->status == PCI_ERR_UNC_UNSUP;
+ inj.log_overflow = false;
+
+ if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
+ if (!pcie_aer_inject_cor_error(&inj, 0, false)) {
+ return 0;
+ }
+ } else {
+ bool is_fatal =
+ pcie_aer_uncor_default_severity(error_status) ==
+ PCI_ERR_ROOT_CMD_FATAL_EN;
+ if (aer_cap) {
+ is_fatal =
+ error_status & pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
+ }
+ if (!is_fatal && (err->flags & PCIE_AER_ERR_MAYBE_ADVISORY)) {
+ inj.error_status = PCI_ERR_COR_ADV_NONFATAL;
+ if (!pcie_aer_inject_cor_error(&inj, error_status, true)) {
+ return 0;
+ }
+ } else {
+ if (!pcie_aer_inject_uncor_error(&inj, is_fatal)) {
+ return 0;
+ }
+ }
+ }
+
+ /* send up error message */
+ inj.msg.source_id = err->source_id;
+ pcie_aer_msg(dev, &inj.msg);
+
+ if (inj.log_overflow) {
+ PCIEAERErr header_log_overflow = {
+ .status = PCI_ERR_COR_HL_OVERFLOW,
+ .flags = PCIE_AER_ERR_IS_CORRECTABLE,
+ };
+ int ret = pcie_aer_inject_error(dev, &header_log_overflow);
+ assert(!ret);
+ }
+ return 0;
+}
+
+void pcie_aer_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ uint32_t first_error = 1U << PCI_ERR_CAP_FEP(errcap);
+ uint32_t uncorsta = pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS);
+
+ /* uncorrectable error */
+ if (!(uncorsta & first_error)) {
+ /* the bit that corresponds to the first error is cleared */
+ pcie_aer_clear_error(dev);
+ } else if (errcap & PCI_ERR_CAP_MHRE) {
+ /* When PCI_ERR_CAP_MHRE is enabled and the first error isn't cleared
+ * nothing should happen. So we have to revert the modification to
+ * the register.
+ */
+ pcie_aer_update_uncor_status(dev);
+ } else {
+ /* capability & control
+ * PCI_ERR_CAP_MHRE might be cleared, so clear of header log.
+ */
+ aer_log_clear_all_err(&dev->exp.aer_log);
+ }
+}
+
+void pcie_aer_root_init(PCIDevice *dev)
+{
+ uint16_t pos = dev->exp.aer_cap;
+
+ pci_set_long(dev->wmask + pos + PCI_ERR_ROOT_COMMAND,
+ PCI_ERR_ROOT_CMD_EN_MASK);
+ pci_set_long(dev->w1cmask + pos + PCI_ERR_ROOT_STATUS,
+ PCI_ERR_ROOT_STATUS_REPORT_MASK);
+}
+
+void pcie_aer_root_reset(PCIDevice *dev)
+{
+ uint8_t* aer_cap = dev->config + dev->exp.aer_cap;
+
+ pci_set_long(aer_cap + PCI_ERR_ROOT_COMMAND, 0);
+
+ /*
+ * Advanced Error Interrupt Message Number in Root Error Status Register
+ * must be updated by chip dependent code because it's chip dependent
+ * which number is used.
+ */
+}
+
+static bool pcie_aer_root_does_trigger(uint32_t cmd, uint32_t status)
+{
+ return
+ ((cmd & PCI_ERR_ROOT_CMD_COR_EN) && (status & PCI_ERR_ROOT_COR_RCV)) ||
+ ((cmd & PCI_ERR_ROOT_CMD_NONFATAL_EN) &&
+ (status & PCI_ERR_ROOT_NONFATAL_RCV)) ||
+ ((cmd & PCI_ERR_ROOT_CMD_FATAL_EN) &&
+ (status & PCI_ERR_ROOT_FATAL_RCV));
+}
+
+void pcie_aer_root_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len,
+ uint32_t root_cmd_prev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+
+ /* root command register */
+ uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
+ if (root_cmd & PCI_ERR_ROOT_CMD_EN_MASK) {
+ /* 6.2.4.1.2 Interrupt Generation */
+
+ /* 0 -> 1 */
+ uint32_t root_cmd_set = (root_cmd_prev ^ root_cmd) & root_cmd;
+ uint32_t root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+
+ if (pci_msi_enabled(dev)) {
+ if (pcie_aer_root_does_trigger(root_cmd_set, root_status)) {
+ pci_msi_notify(dev, pcie_aer_root_get_vector(dev));
+ }
+ } else {
+ int int_level = pcie_aer_root_does_trigger(root_cmd, root_status);
+ qemu_set_irq(dev->irq[dev->exp.aer_intx], int_level);
+ }
+ }
+}
+
+static const VMStateDescription vmstate_pcie_aer_err = {
+ .name = "PCIE_AER_ERROR",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(status, PCIEAERErr),
+ VMSTATE_UINT16(source_id, PCIEAERErr),
+ VMSTATE_UINT16(flags, PCIEAERErr),
+ VMSTATE_UINT32_ARRAY(header, PCIEAERErr, 4),
+ VMSTATE_UINT32_ARRAY(prefix, PCIEAERErr, 4),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+#define VMSTATE_PCIE_AER_ERRS(_field, _state, _field_num, _vmsd, _type) { \
+ .name = (stringify(_field)), \
+ .version_id = 0, \
+ .num_offset = vmstate_offset_value(_state, _field_num, uint16_t), \
+ .size = sizeof(_type), \
+ .vmsd = &(_vmsd), \
+ .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
+ .offset = vmstate_offset_pointer(_state, _field, _type), \
+}
+
+const VMStateDescription vmstate_pcie_aer_log = {
+ .name = "PCIE_AER_ERROR_LOG",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT16(log_num, PCIEAERLog),
+ VMSTATE_UINT16(log_max, PCIEAERLog),
+ VMSTATE_PCIE_AER_ERRS(log, PCIEAERLog, log_num,
+ vmstate_pcie_aer_err, PCIEAERErr),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
diff --git a/hw/pcie_aer.h b/hw/pcie_aer.h
new file mode 100644
index 0000000..7539500
--- /dev/null
+++ b/hw/pcie_aer.h
@@ -0,0 +1,106 @@
+/*
+ * pcie_aer.h
+ *
+ * Copyright (c) 2010 Isaku Yamahata <yamahata at valinux co jp>
+ * VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_PCIE_AER_H
+#define QEMU_PCIE_AER_H
+
+#include "hw.h"
+
+/* definitions which PCIExpressDevice uses */
+
+/* AER log */
+struct PCIEAERLog {
+ /* This structure is saved/loaded.
+ So explicitly size them instead of unsigned int */
+
+ /* the number of currently recorded log in log member */
+ uint16_t log_num;
+
+ /*
+ * The maximum number of the log. Errors can be logged up to this.
+ *
+ * This is configurable property.
+ * The specified value will be clipped down to PCIE_AER_LOG_MAX_LIMIT
+ * to avoid unreasonable memory usage.
+ * I bet that 128 log size would be big enough, otherwise too many errors
+ * for system to function normaly. But could consecutive errors occur?
+ */
+#define PCIE_AER_LOG_MAX_DEFAULT 8
+#define PCIE_AER_LOG_MAX_LIMIT 128
+#define PCIE_AER_LOG_MAX_UNSET 0xffff
+ uint16_t log_max;
+
+ /* Error log. log_max-sized array */
+ PCIEAERErr *log;
+};
+
+/* aer error message: error signaling message has only error sevirity and
+ source id. See 2.2.8.3 error signaling messages */
+struct PCIEAERMsg {
+ /*
+ * PCI_ERR_ROOT_CMD_{COR, NONFATAL, FATAL}_EN
+ * = PCI_EXP_DEVCTL_{CERE, NFERE, FERE}
+ */
+ uint32_t severity;
+
+ uint16_t source_id; /* bdf */
+};
+
+static inline bool
+pcie_aer_msg_is_uncor(const PCIEAERMsg *msg)
+{
+ return msg->severity == PCI_ERR_ROOT_CMD_NONFATAL_EN ||
+ msg->severity == PCI_ERR_ROOT_CMD_FATAL_EN;
+}
+
+/* error */
+struct PCIEAERErr {
+ uint32_t status; /* error status bits */
+ uint16_t source_id; /* bdf */
+
+#define PCIE_AER_ERR_IS_CORRECTABLE 0x1 /* correctable/uncorrectable */
+#define PCIE_AER_ERR_MAYBE_ADVISORY 0x2 /* maybe advisory non-fatal */
+#define PCIE_AER_ERR_HEADER_VALID 0x4 /* TLP header is logged */
+#define PCIE_AER_ERR_TLP_PREFIX_PRESENT 0x8 /* TLP Prefix is logged */
+ uint16_t flags;
+
+ uint32_t header[4]; /* TLP header */
+ uint32_t prefix[4]; /* TLP header prefix */
+};
+
+extern const VMStateDescription vmstate_pcie_aer_log;
+
+int pcie_aer_init(PCIDevice *dev, uint16_t offset);
+void pcie_aer_exit(PCIDevice *dev);
+void pcie_aer_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len);
+
+/* aer root port */
+void pcie_aer_root_set_vector(PCIDevice *dev, unsigned int vector);
+void pcie_aer_root_init(PCIDevice *dev);
+void pcie_aer_root_reset(PCIDevice *dev);
+void pcie_aer_root_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len,
+ uint32_t root_cmd_prev);
+
+/* error injection */
+int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err);
+
+#endif /* QEMU_PCIE_AER_H */
diff --git a/qemu-common.h b/qemu-common.h
index b3957f1..de82c2e 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -240,6 +240,9 @@ typedef struct PCIBus PCIBus;
typedef struct PCIDevice PCIDevice;
typedef struct PCIExpressDevice PCIExpressDevice;
typedef struct PCIBridge PCIBridge;
+typedef struct PCIEAERMsg PCIEAERMsg;
+typedef struct PCIEAERLog PCIEAERLog;
+typedef struct PCIEAERErr PCIEAERErr;
typedef struct PCIEPort PCIEPort;
typedef struct PCIESlot PCIESlot;
typedef struct SerialState SerialState;
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-11-17 14:06 ` Michael S. Tsirkin
2010-11-17 14:06 ` [Qemu-devel] [PATCH 1/2] pcie_aer: get rid of recursion Michael S. Tsirkin
` (3 more replies)
0 siblings, 4 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-17 14:06 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 05:26:09PM +0900, Isaku Yamahata wrote:
> This patch implements helper functions for pcie aer capability
> which will be used later.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
OK, I applied this and tried to get rid of recursion,
and clean up some whitespace and english mistakes.
Patcheset attached, pls review.
I'll push the patches out on pci branch so we can
make progress more easily.
Please, try to address the TODO: I think the case of
PCIE device behind a pci bridge is not covered properly.
Michael S. Tsirkin (2):
pcie_aer: get rid of recursion
pcie_aer: complete unwinding recursion
hw/pcie_aer.c | 72 +++++++++++++++++++++++---------------------------------
1 files changed, 30 insertions(+), 42 deletions(-)
--
1.7.3.2.91.g446ac
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 1/2] pcie_aer: get rid of recursion
2010-11-17 14:06 ` [Qemu-devel] [PATCH 0/2] " Michael S. Tsirkin
@ 2010-11-17 14:06 ` Michael S. Tsirkin
2010-11-17 14:06 ` [Qemu-devel] [PATCH 2/2] pcie_aer: complete unwinding recursion Michael S. Tsirkin
` (2 subsequent siblings)
3 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-17 14:06 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
Added some TODOs: they are trivial but omitted here
to make the patch logic as transparent as possible.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pcie_aer.c | 48 +++++++++++++++++++++++++++++++-----------------
1 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
index acf826a..c565d39 100644
--- a/hw/pcie_aer.c
+++ b/hw/pcie_aer.c
@@ -176,14 +176,8 @@ static void pcie_aer_update_uncor_status(PCIDevice *dev)
}
/*
- * pcie_aer_msg() is called recursively by
- * pcie_aer_msg_alldev(), pci_aer_msg_vbridge() and pcie_aer_msg_root_port()
- */
-static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
-
-/*
* return value:
- * true: error message is sent up
+ * true: error message needs to be sent up
* false: error message is masked
*
* 6.2.6 Error Message Control
@@ -193,8 +187,6 @@ static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
static bool
pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
{
- PCIDevice *parent_port;
-
if (!(pcie_aer_msg_is_uncor(msg) &&
(pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_SERR))) {
return false;
@@ -220,13 +212,21 @@ pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
}
/* send up error message */
+ return true;
+}
+
+/* Get parent port to send up error message on.
+ * TODO: clean up and open-code this logic */
+static PCIDevice *pcie_aer_parent_port(PCIDevice *dev)
+{
+ PCIDevice *parent_port;
if (pci_is_express(dev) &&
pcie_cap_get_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
- /* Root port notify system itself,
+ /* Root port can notify system itself,
or send the error message to root complex event collector. */
/*
- * if root port is associated to event collector, set
- * parent_port = root complex event collector
+ * if root port is associated with an event collector,
+ * return the root complex event collector here.
* For now root complex event collector isn't supported.
*/
parent_port = NULL;
@@ -236,11 +236,10 @@ pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
if (parent_port) {
if (!pci_is_express(parent_port)) {
/* just ignore it */
- return false;
+ return NULL;
}
- pcie_aer_msg(parent_port, msg);
}
- return true;
+ return parent_port;
}
/*
@@ -381,8 +380,12 @@ static bool pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
/*
* 6.2.6 Error Message Control Figure 6-3
+ *
+ * Returns true in case the error needs to
+ * be propagated up.
+ * TODO: open-code.
*/
-static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+static bool pcie_send_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
{
uint8_t type;
bool msg_sent;
@@ -402,6 +405,18 @@ static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
if (type == PCI_EXP_TYPE_ROOT_PORT && msg_sent) {
pcie_aer_msg_root_port(dev, msg);
}
+ return msg_sent;
+}
+
+static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ bool send_to_parent;
+ while (dev) {
+ if (!pcie_send_aer_msg(dev, msg)) {
+ return;
+ }
+ dev = pcie_aer_parent_port(dev);
+ }
}
static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
@@ -824,4 +839,3 @@ const VMStateDescription vmstate_pcie_aer_log = {
VMSTATE_END_OF_LIST()
}
};
-
--
1.7.3.2.91.g446ac
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH 2/2] pcie_aer: complete unwinding recursion
2010-11-17 14:06 ` [Qemu-devel] [PATCH 0/2] " Michael S. Tsirkin
2010-11-17 14:06 ` [Qemu-devel] [PATCH 1/2] pcie_aer: get rid of recursion Michael S. Tsirkin
@ 2010-11-17 14:06 ` Michael S. Tsirkin
2010-11-18 8:11 ` [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
2010-11-19 9:42 ` Isaku Yamahata
3 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-17 14:06 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
Open-code functions created in the previous patch,
to make code more compact and clear.
Detcted and documented what looks like a bug in code
that becomes apparent from this refactoring.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pcie_aer.c | 80 +++++++++++++++++++-------------------------------------
1 files changed, 27 insertions(+), 53 deletions(-)
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
index c565d39..6621918 100644
--- a/hw/pcie_aer.c
+++ b/hw/pcie_aer.c
@@ -215,33 +215,6 @@ pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
return true;
}
-/* Get parent port to send up error message on.
- * TODO: clean up and open-code this logic */
-static PCIDevice *pcie_aer_parent_port(PCIDevice *dev)
-{
- PCIDevice *parent_port;
- if (pci_is_express(dev) &&
- pcie_cap_get_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
- /* Root port can notify system itself,
- or send the error message to root complex event collector. */
- /*
- * if root port is associated with an event collector,
- * return the root complex event collector here.
- * For now root complex event collector isn't supported.
- */
- parent_port = NULL;
- } else {
- parent_port = pci_bridge_get_device(dev->bus);
- }
- if (parent_port) {
- if (!pci_is_express(parent_port)) {
- /* just ignore it */
- return NULL;
- }
- }
- return parent_port;
-}
-
/*
* return value:
* true: error message is sent up
@@ -381,41 +354,42 @@ static bool pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
/*
* 6.2.6 Error Message Control Figure 6-3
*
- * Returns true in case the error needs to
- * be propagated up.
- * TODO: open-code.
+ * Walk up the bus tree from the device, propagate the error message.
*/
-static bool pcie_send_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
{
uint8_t type;
- bool msg_sent;
-
- assert(pci_is_express(dev));
- type = pcie_cap_get_type(dev);
- if (type == PCI_EXP_TYPE_ROOT_PORT ||
- type == PCI_EXP_TYPE_UPSTREAM ||
- type == PCI_EXP_TYPE_DOWNSTREAM) {
- msg_sent = pcie_aer_msg_vbridge(dev, msg);
- if (!msg_sent) {
+ while (dev) {
+ if (!pci_is_express(parent_port)) {
+ /* just ignore it */
+ /* TODO: Shouldn't we set PCI_STATUS_SIG_SYSTEM_ERROR?
+ * Consider e.g. a PCI bridge above a PCI Express device. */
return;
}
- }
- msg_sent = pcie_aer_msg_alldev(dev, msg);
- if (type == PCI_EXP_TYPE_ROOT_PORT && msg_sent) {
- pcie_aer_msg_root_port(dev, msg);
- }
- return msg_sent;
-}
-static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
-{
- bool send_to_parent;
- while (dev) {
- if (!pcie_send_aer_msg(dev, msg)) {
+ type = pcie_cap_get_type(dev);
+ if ((type == PCI_EXP_TYPE_ROOT_PORT ||
+ type == PCI_EXP_TYPE_UPSTREAM ||
+ type == PCI_EXP_TYPE_DOWNSTREAM) &&
+ !pcie_aer_msg_vbridge(dev, msg)) {
+ return;
+ }
+ if (!pcie_aer_msg_alldev(dev, msg)) {
+ return;
+ }
+ if (type == PCI_EXP_TYPE_ROOT_PORT) {
+ pcie_aer_msg_root_port(dev, msg);
+ /* Root port can notify system itself,
+ or send the error message to root complex event collector. */
+ /*
+ * if root port is associated with an event collector,
+ * return the root complex event collector here.
+ * For now root complex event collector isn't supported.
+ */
return;
}
- dev = pcie_aer_parent_port(dev);
+ dev = pci_bridge_get_device(dev->bus);
}
}
--
1.7.3.2.91.g446ac
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability
2010-11-17 14:06 ` [Qemu-devel] [PATCH 0/2] " Michael S. Tsirkin
2010-11-17 14:06 ` [Qemu-devel] [PATCH 1/2] pcie_aer: get rid of recursion Michael S. Tsirkin
2010-11-17 14:06 ` [Qemu-devel] [PATCH 2/2] pcie_aer: complete unwinding recursion Michael S. Tsirkin
@ 2010-11-18 8:11 ` Isaku Yamahata
2010-11-18 8:52 ` Michael S. Tsirkin
2010-11-19 9:42 ` Isaku Yamahata
3 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-18 8:11 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Nov 17, 2010 at 04:06:38PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 16, 2010 at 05:26:09PM +0900, Isaku Yamahata wrote:
> > This patch implements helper functions for pcie aer capability
> > which will be used later.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> OK, I applied this and tried to get rid of recursion,
> and clean up some whitespace and english mistakes.
>
> Patcheset attached, pls review.
>
> I'll push the patches out on pci branch so we can
> make progress more easily.
Thank you for cleaning it up.
Basically looks good. Except the following patch.
> Please, try to address the TODO: I think the case of
> PCIE device behind a pci bridge is not covered properly.
Will do.
>From 166886f7f3e423812f4f3f467e2071c53e9dde01 Mon Sep 17 00:00:00 2001
Message-Id: <166886f7f3e423812f4f3f467e2071c53e9dde01.1290067665.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1290067665.git.yamahata@valinux.co.jp>
References: <cover.1290067665.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Thu, 18 Nov 2010 17:03:23 +0900
Subject: [PATCH] pcie/aer: typo
Compilation fix.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pcie_aer.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
index 03cc6fb..c72cbc6 100644
--- a/hw/pcie_aer.c
+++ b/hw/pcie_aer.c
@@ -363,7 +363,7 @@ static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
uint8_t type;
while (dev) {
- if (!pci_is_express(parent_port)) {
+ if (!pci_is_express(dev)) {
/* just ignore it */
/* TODO: Shouldn't we set PCI_STATUS_SIG_SYSTEM_ERROR?
* Consider e.g. a PCI bridge above a PCI Express device. */
--
1.7.1.1
--
yamahata
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability
2010-11-18 8:11 ` [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-11-18 8:52 ` Michael S. Tsirkin
0 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-18 8:52 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Thu, Nov 18, 2010 at 05:11:17PM +0900, Isaku Yamahata wrote:
> On Wed, Nov 17, 2010 at 04:06:38PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Nov 16, 2010 at 05:26:09PM +0900, Isaku Yamahata wrote:
> > > This patch implements helper functions for pcie aer capability
> > > which will be used later.
> > >
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > OK, I applied this and tried to get rid of recursion,
> > and clean up some whitespace and english mistakes.
> >
> > Patcheset attached, pls review.
> >
> > I'll push the patches out on pci branch so we can
> > make progress more easily.
>
> Thank you for cleaning it up.
> Basically looks good. Except the following patch.
Right. Fixed up, thanks!
>
> > Please, try to address the TODO: I think the case of
> > PCIE device behind a pci bridge is not covered properly.
>
> Will do.
>
>
> >From 166886f7f3e423812f4f3f467e2071c53e9dde01 Mon Sep 17 00:00:00 2001
> Message-Id: <166886f7f3e423812f4f3f467e2071c53e9dde01.1290067665.git.yamahata@valinux.co.jp>
> In-Reply-To: <cover.1290067665.git.yamahata@valinux.co.jp>
> References: <cover.1290067665.git.yamahata@valinux.co.jp>
> From: Isaku Yamahata <yamahata@valinux.co.jp>
> Date: Thu, 18 Nov 2010 17:03:23 +0900
> Subject: [PATCH] pcie/aer: typo
>
> Compilation fix.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/pcie_aer.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
> index 03cc6fb..c72cbc6 100644
> --- a/hw/pcie_aer.c
> +++ b/hw/pcie_aer.c
> @@ -363,7 +363,7 @@ static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
> uint8_t type;
>
> while (dev) {
> - if (!pci_is_express(parent_port)) {
> + if (!pci_is_express(dev)) {
> /* just ignore it */
> /* TODO: Shouldn't we set PCI_STATUS_SIG_SYSTEM_ERROR?
> * Consider e.g. a PCI bridge above a PCI Express device. */
> --
> 1.7.1.1
>
>
> --
> yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability
2010-11-17 14:06 ` [Qemu-devel] [PATCH 0/2] " Michael S. Tsirkin
` (2 preceding siblings ...)
2010-11-18 8:11 ` [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-11-19 9:42 ` Isaku Yamahata
2010-11-19 12:03 ` Michael S. Tsirkin
3 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-19 9:42 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Nov 17, 2010 at 04:06:38PM +0200, Michael S. Tsirkin wrote:
> Please, try to address the TODO: I think the case of
> PCIE device behind a pci bridge is not covered properly.
Right, pci-to-pcie bridge case isn't covered.
Although SERR bit can be set, the error can't be propagated up further
because qemu doesn't emulate PERR#/SERR#.
There is no use case of pci-to-pcie bridge in qemu, I think.
If good error handling is wanted, express should be used.
So it isn't worthwhile to implement pci-to-pcie bridge.
--
yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH 0/2] Re: [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability
2010-11-19 9:42 ` Isaku Yamahata
@ 2010-11-19 12:03 ` Michael S. Tsirkin
0 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-19 12:03 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Fri, Nov 19, 2010 at 06:42:27PM +0900, Isaku Yamahata wrote:
> On Wed, Nov 17, 2010 at 04:06:38PM +0200, Michael S. Tsirkin wrote:
> > Please, try to address the TODO: I think the case of
> > PCIE device behind a pci bridge is not covered properly.
>
> Right, pci-to-pcie bridge case isn't covered.
> Although SERR bit can be set, the error can't be propagated up further
> because qemu doesn't emulate PERR#/SERR#.
>
> There is no use case of pci-to-pcie bridge in qemu, I think.
I think it's useful to put an express device behind
a pci bridge in qemu, because pci bridges are tested
and supported by more guests.
> If good error handling is wanted, express should be used.
> So it isn't worthwhile to implement pci-to-pcie bridge.
Okay, we can look into this later.
> --
> yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v9 6/8] ioh3420: support aer
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
` (4 preceding siblings ...)
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 5/8] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 7/8] x3130/upstream: " Isaku Yamahata
` (3 subsequent siblings)
9 siblings, 0 replies; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
Add aer support.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v8 -> v9:
- error path in initialization
---
hw/ioh3420.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 70 insertions(+), 10 deletions(-)
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index 3cc129f..95adf09 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -36,25 +36,59 @@
#define IOH_EP_EXP_OFFSET 0x90
#define IOH_EP_AER_OFFSET 0x100
+/*
+ * If two MSI vector are allocated, Advanced Error Interrupt Message Number
+ * is 1. otherwise 0.
+ * 17.12.5.10 RPERRSTS, 32:27 bit Advanced Error Interrupt Message Number.
+ */
+static uint8_t ioh3420_aer_vector(const PCIDevice *d)
+{
+ switch (msi_nr_vectors_allocated(d)) {
+ case 1:
+ return 0;
+ case 2:
+ return 1;
+ case 4:
+ case 8:
+ case 16:
+ case 32:
+ default:
+ break;
+ }
+ abort();
+ return 0;
+}
+
+static void ioh3420_aer_vector_update(PCIDevice *d)
+{
+ pcie_aer_root_set_vector(d, ioh3420_aer_vector(d));
+}
+
static void ioh3420_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len)
{
+ uint32_t root_cmd =
+ pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
+
pci_bridge_write_config(d, address, val, len);
msi_write_config(d, address, val, len);
+ ioh3420_aer_vector_update(d);
pcie_cap_slot_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len);
+ pcie_aer_root_write_config(d, address, val, len, root_cmd);
}
static void ioh3420_reset(DeviceState *qdev)
{
PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
msi_reset(d);
+ ioh3420_aer_vector_update(d);
pcie_cap_root_reset(d);
pcie_cap_deverr_reset(d);
pcie_cap_slot_reset(d);
+ pcie_aer_root_reset(d);
pci_bridge_reset(qdev);
pci_bridge_disable_base_limit(d);
- /* TODO: AER */
}
static int ioh3420_initfn(PCIDevice *d)
@@ -63,6 +97,7 @@ static int ioh3420_initfn(PCIDevice *d)
PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
int rc;
+ int tmp;
rc = pci_bridge_initfn(d);
if (rc < 0) {
@@ -78,35 +113,57 @@ static int ioh3420_initfn(PCIDevice *d)
rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
IOH_EP_SSVID_SVID, IOH_EP_SSVID_SSID);
if (rc < 0) {
- return rc;
+ goto err_bridge;
}
rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
if (rc < 0) {
- return rc;
+ goto err_bridge;
}
rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, p->port);
if (rc < 0) {
- return rc;
+ goto err_msi;
}
pcie_cap_deverr_init(d);
pcie_cap_slot_init(d, s->slot);
pcie_chassis_create(s->chassis);
rc = pcie_chassis_add_slot(s);
if (rc < 0) {
+ goto err_pcie_cap;
return rc;
}
pcie_cap_root_init(d);
- /* TODO: AER */
+ rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
+ if (rc < 0) {
+ goto err;
+ }
+ pcie_aer_root_init(d);
+ ioh3420_aer_vector_update(d);
return 0;
+
+err:
+ pcie_chassis_del_slot(s);
+err_pcie_cap:
+ pcie_cap_exit(d);
+err_msi:
+ msi_uninit(d);
+err_bridge:
+ tmp = pci_bridge_exitfn(d);
+ assert(!tmp);
+ return rc;
}
static int ioh3420_exitfn(PCIDevice *d)
{
- /* TODO: AER */
- msi_uninit(d);
+ PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
+ PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
+ PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
+
+ pcie_aer_exit(d);
+ pcie_chassis_del_slot(s);
pcie_cap_exit(d);
+ msi_uninit(d);
return pci_bridge_exitfn(d);
}
@@ -142,7 +199,8 @@ static const VMStateDescription vmstate_ioh3420 = {
.post_load = pcie_cap_slot_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCIE_DEVICE(port.br.dev, PCIESlot),
- /* TODO: AER */
+ VMSTATE_STRUCT(port.br.dev.exp.aer_log, PCIESlot, 0,
+ vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
}
};
@@ -164,7 +222,9 @@ static PCIDeviceInfo ioh3420_info = {
DEFINE_PROP_UINT8("port", PCIESlot, port.port, 0),
DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
- /* TODO: AER */
+ DEFINE_PROP_UINT16("aer_log_max", PCIESlot,
+ port.br.dev.exp.aer_log.log_max,
+ PCIE_AER_LOG_MAX_DEFAULT),
DEFINE_PROP_END_OF_LIST(),
}
};
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v9 7/8] x3130/upstream: support aer
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
` (5 preceding siblings ...)
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 6/8] ioh3420: support aer Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 8/8] x3130/downstream: " Isaku Yamahata
` (2 subsequent siblings)
9 siblings, 0 replies; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
add aer support.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v8 -> v9
- error path in initialization.
---
hw/xio3130_upstream.c | 33 ++++++++++++++++++++++++---------
1 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
index d9d637f..387bf6c 100644
--- a/hw/xio3130_upstream.c
+++ b/hw/xio3130_upstream.c
@@ -41,7 +41,7 @@ static void xio3130_upstream_write_config(PCIDevice *d, uint32_t address,
pci_bridge_write_config(d, address, val, len);
pcie_cap_flr_write_config(d, address, val, len);
msi_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len);
}
static void xio3130_upstream_reset(DeviceState *qdev)
@@ -57,6 +57,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
int rc;
+ int tmp;
rc = pci_bridge_initfn(d);
if (rc < 0) {
@@ -72,33 +73,45 @@ static int xio3130_upstream_initfn(PCIDevice *d)
XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
if (rc < 0) {
- return rc;
+ goto err_bridge;
}
rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
if (rc < 0) {
- return rc;
+ goto err_bridge;
}
rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_UPSTREAM,
p->port);
if (rc < 0) {
- return rc;
+ goto err_msi;
}
/* TODO: implement FLR */
pcie_cap_flr_init(d);
pcie_cap_deverr_init(d);
- /* TODO: AER */
+ rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+ if (rc < 0) {
+ goto err;
+ }
return 0;
+
+err:
+ pcie_cap_exit(d);
+err_msi:
+ msi_uninit(d);
+err_bridge:
+ tmp = pci_bridge_exitfn(d);
+ assert(!tmp);
+ return rc;
}
static int xio3130_upstream_exitfn(PCIDevice *d)
{
- /* TODO: AER */
- msi_uninit(d);
+ pcie_aer_exit(d);
pcie_cap_exit(d);
+ msi_uninit(d);
return pci_bridge_exitfn(d);
}
@@ -131,7 +144,8 @@ static const VMStateDescription vmstate_xio3130_upstream = {
.minimum_version_id_old = 1,
.fields = (VMStateField[]) {
VMSTATE_PCIE_DEVICE(br.dev, PCIEPort),
- /* TODO: AER */
+ VMSTATE_STRUCT(br.dev.exp.aer_log, PCIEPort, 0, vmstate_pcie_aer_log,
+ PCIEAERLog),
VMSTATE_END_OF_LIST()
}
};
@@ -151,7 +165,8 @@ static PCIDeviceInfo xio3130_upstream_info = {
.qdev.props = (Property[]) {
DEFINE_PROP_UINT8("port", PCIEPort, port, 0),
- /* TODO: AER */
+ DEFINE_PROP_UINT16("aer_log_max", PCIEPort, br.dev.exp.aer_log.log_max,
+ PCIE_AER_LOG_MAX_DEFAULT),
DEFINE_PROP_END_OF_LIST(),
}
};
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v9 8/8] x3130/downstream: support aer.
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
` (6 preceding siblings ...)
2010-11-16 8:26 ` [Qemu-devel] [PATCH v9 7/8] x3130/upstream: " Isaku Yamahata
@ 2010-11-16 8:26 ` Isaku Yamahata
[not found] ` <1289930315.27724.18.camel@etmartin-lnx>
2010-11-17 14:38 ` [Qemu-devel] Re: [PATCH v9 0/8] pcie port switch emulators Michael S. Tsirkin
9 siblings, 0 replies; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-16 8:26 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
add aer support.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v8 -> v9:
- error path in initialization
---
hw/xio3130_downstream.c | 43 +++++++++++++++++++++++++++++++++----------
1 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
index 854eba8..1a2d258 100644
--- a/hw/xio3130_downstream.c
+++ b/hw/xio3130_downstream.c
@@ -42,7 +42,7 @@ static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address,
pcie_cap_flr_write_config(d, address, val, len);
pcie_cap_slot_write_config(d, address, val, len);
msi_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len);
}
static void xio3130_downstream_reset(DeviceState *qdev)
@@ -61,6 +61,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
int rc;
+ int tmp;
rc = pci_bridge_initfn(d);
if (rc < 0) {
@@ -76,17 +77,17 @@ static int xio3130_downstream_initfn(PCIDevice *d)
XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
if (rc < 0) {
- return rc;
+ goto err_bridge;
}
rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
if (rc < 0) {
- return rc;
+ goto err_bridge;
}
rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_DOWNSTREAM,
p->port);
if (rc < 0) {
- return rc;
+ goto err_msi;
}
pcie_cap_flr_init(d); /* TODO: implement FLR */
pcie_cap_deverr_init(d);
@@ -94,19 +95,38 @@ static int xio3130_downstream_initfn(PCIDevice *d)
pcie_chassis_create(s->chassis);
rc = pcie_chassis_add_slot(s);
if (rc < 0) {
- return rc;
+ goto err_pcie_cap;
}
pcie_cap_ari_init(d);
- /* TODO: AER */
+ rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+ if (rc < 0) {
+ goto err;
+ }
return 0;
+
+err:
+ pcie_chassis_del_slot(s);
+err_pcie_cap:
+ pcie_cap_exit(d);
+err_msi:
+ msi_uninit(d);
+err_bridge:
+ tmp = pci_bridge_exitfn(d);
+ assert(!tmp);
+ return rc;
}
static int xio3130_downstream_exitfn(PCIDevice *d)
{
- /* TODO: AER */
- msi_uninit(d);
+ PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
+ PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
+ PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
+
+ pcie_aer_exit(d);
+ pcie_chassis_del_slot(s);
pcie_cap_exit(d);
+ msi_uninit(d);
return pci_bridge_exitfn(d);
}
@@ -144,7 +164,8 @@ static const VMStateDescription vmstate_xio3130_downstream = {
.post_load = pcie_cap_slot_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCIE_DEVICE(port.br.dev, PCIESlot),
- /* TODO: AER */
+ VMSTATE_STRUCT(port.br.dev.exp.aer_log, PCIESlot, 0,
+ vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
}
};
@@ -166,7 +187,9 @@ static PCIDeviceInfo xio3130_downstream_info = {
DEFINE_PROP_UINT8("port", PCIESlot, port.port, 0),
DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
- /* TODO: AER */
+ DEFINE_PROP_UINT16("aer_log_max", PCIESlot,
+ port.br.dev.exp.aer_log.log_max,
+ PCIE_AER_LOG_MAX_DEFAULT),
DEFINE_PROP_END_OF_LIST(),
}
};
--
1.7.1.1
^ permalink raw reply related [flat|nested] 29+ messages in thread[parent not found: <1289930315.27724.18.camel@etmartin-lnx>]
* [Qemu-devel] Re: [PATCH v9 8/8] x3130/downstream: support aer.
[not found] ` <1289930315.27724.18.camel@etmartin-lnx>
@ 2010-11-16 18:57 ` Etienne Martineau
2010-11-17 4:07 ` Isaku Yamahata
0 siblings, 1 reply; 29+ messages in thread
From: Etienne Martineau @ 2010-11-16 18:57 UTC (permalink / raw)
To: Isaku Yamahata
Cc: skandasa, adnan, wexu2, Michael S. Tsirkin, qemu-devel,
Etienne Martineau
On Tue, 2010-11-16 at 17:26 +0900, Isaku Yamahata wrote:
> add aer support.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
I'm actually working on a scheme to provide support to handle PCI
errors related to assigned devices. The goal is to notify the
coresponding driver so that all his I/O access are stop quickly and to
provide that same driver a chance to get back in sync with the device.
I'm just wondering how can I make use of your aer support in Q35?
As you already know, error detection and error recovery phases are driven
by a state machine where the next state depends on the returned value from
the current callback. Also only the host can performed the link recovery
sequence for assigned devices.
Because of such it seems like the only way to maintain consistency between
the assigned device and it's corresponding driver is to perform the error
detection/recovery phase in lockstep with the host?
thanks,
-Etienne
^ permalink raw reply [flat|nested] 29+ messages in thread* [Qemu-devel] Re: [PATCH v9 8/8] x3130/downstream: support aer.
2010-11-16 18:57 ` [Qemu-devel] " Etienne Martineau
@ 2010-11-17 4:07 ` Isaku Yamahata
2010-11-17 5:31 ` Etienne Martineau
0 siblings, 1 reply; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-17 4:07 UTC (permalink / raw)
To: Etienne Martineau; +Cc: skandasa, adnan, qemu-devel, wexu2, Michael S. Tsirkin
On Tue, Nov 16, 2010 at 10:57:59AM -0800, Etienne Martineau wrote:
>
> On Tue, 2010-11-16 at 17:26 +0900, Isaku Yamahata wrote:
> > add aer support.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> I'm actually working on a scheme to provide support to handle PCI
> errors related to assigned devices. The goal is to notify the
> coresponding driver so that all his I/O access are stop quickly and to
> provide that same driver a chance to get back in sync with the device.
>
> I'm just wondering how can I make use of your aer support in Q35?
>
> As you already know, error detection and error recovery phases are driven
> by a state machine where the next state depends on the returned value from
> the current callback. Also only the host can performed the link recovery
> sequence for assigned devices.
>
> Because of such it seems like the only way to maintain consistency between
> the assigned device and it's corresponding driver is to perform the error
> detection/recovery phase in lockstep with the host?
Maybe. At least at the first implementation, I suppose.
Then we would learn from its experience, then move on to next generation
implementation.
To be honest, what I have in my mind very vaguely is
- something like pcie aer fd driver.
or enhancement to vfio
qemu polls the fd.
- error recovery in host will be directed by qemu
in concert with guest recovery action.
For latency necessary information would be shared by
qemu and host kernel, so that the aer driver in host kernel
could take responsibility to eliminate the latency caused by
qemu process.
I suppose there is no single right way for recovery action
in host/guest. So there should be room for recovery policies.
--
yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] Re: [PATCH v9 8/8] x3130/downstream: support aer.
2010-11-17 4:07 ` Isaku Yamahata
@ 2010-11-17 5:31 ` Etienne Martineau
2010-11-18 2:19 ` Isaku Yamahata
0 siblings, 1 reply; 29+ messages in thread
From: Etienne Martineau @ 2010-11-17 5:31 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, qemu-devel, wexu2, Michael S. Tsirkin
On Wed, 17 Nov 2010, Isaku Yamahata wrote:
> > Because of such it seems like the only way to maintain consistency between
> > the assigned device and it's corresponding driver is to perform the error
> > detection/recovery phase in lockstep with the host?
>
> Maybe. At least at the first implementation, I suppose.
> Then we would learn from its experience, then move on to next generation
> implementation.
>
> To be honest, what I have in my mind very vaguely is
> - something like pcie aer fd driver.
> or enhancement to vfio
> qemu polls the fd.
I'm currently working on a pcie aer driver. Few weeks ago I sent some rfc
patches. I'm about to send another version.
It's basically a simple UIO based pci-stub driver for AER and PM.
Notification goes through eventfd and error code / error result are mmap
directly over a 'logical' BAR. Qemu consume the eventfd or it goes
directly to the guest with irqfd.
> - error recovery in host will be directed by qemu
> in concert with guest recovery action.
To my view, this is the tricky part. Error recovery can be directed by
qemu indeed but how do you get the information about the guest recovery
action for every error callback?
I think that because aer handling effectively 'merge' callback return code
from multiple source it's hard to discriminate what value should be given
back to the host for the corresponding assigned device (at least from the
qemu side)
> For latency necessary information would be shared by
> qemu and host kernel, so that the aer driver in host kernel
> could take responsibility to eliminate the latency caused by
> qemu process.
I'm sorry but I'm not sure to follow here. Can you elaborate more on this
topic?
>
> I suppose there is no single right way for recovery action
> in host/guest. So there should be room for recovery policies.
Yes I agree. There is already a policy argument part of the uio
pci_stub driver that I'm working on.
-Etienne
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] Re: [PATCH v9 8/8] x3130/downstream: support aer.
2010-11-17 5:31 ` Etienne Martineau
@ 2010-11-18 2:19 ` Isaku Yamahata
0 siblings, 0 replies; 29+ messages in thread
From: Isaku Yamahata @ 2010-11-18 2:19 UTC (permalink / raw)
To: Etienne Martineau; +Cc: skandasa, adnan, qemu-devel, wexu2, Michael S. Tsirkin
On Tue, Nov 16, 2010 at 09:31:28PM -0800, Etienne Martineau wrote:
>
> On Wed, 17 Nov 2010, Isaku Yamahata wrote:
>
> > > Because of such it seems like the only way to maintain consistency between
> > > the assigned device and it's corresponding driver is to perform the error
> > > detection/recovery phase in lockstep with the host?
> >
> > Maybe. At least at the first implementation, I suppose.
> > Then we would learn from its experience, then move on to next generation
> > implementation.
> >
> > To be honest, what I have in my mind very vaguely is
> > - something like pcie aer fd driver.
> > or enhancement to vfio
> > qemu polls the fd.
>
> I'm currently working on a pcie aer driver. Few weeks ago I sent some rfc
> patches. I'm about to send another version.
>
> It's basically a simple UIO based pci-stub driver for AER and PM.
> Notification goes through eventfd and error code / error result are mmap
> directly over a 'logical' BAR. Qemu consume the eventfd or it goes
> directly to the guest with irqfd.
You mean '[RFC PATCH] kvm: BSimple stub driver with AER capabilities'.
Yes, that is exactly what I've thought.
Can you please add me to CC?
> > - error recovery in host will be directed by qemu
> > in concert with guest recovery action.
>
> To my view, this is the tricky part. Error recovery can be directed by
> qemu indeed but how do you get the information about the guest recovery
> action for every error callback?
>
> I think that because aer handling effectively 'merge' callback return code
> from multiple source it's hard to discriminate what value should be given
> back to the host for the corresponding assigned device (at least from the
> qemu side)
I don't have any clear idea yet. I've just figured it would be tricky
like you.
Maybe listing what kind of aer recovery we want would help, I suppose.
> > For latency necessary information would be shared by
> > qemu and host kernel, so that the aer driver in host kernel
> > could take responsibility to eliminate the latency caused by
> > qemu process.
> I'm sorry but I'm not sure to follow here. Can you elaborate more on this
> topic?
I mean that we might want to move the code in qemu aer recovery into
kvm host kernel for latency. something like in-kernel pic emulator.
> > I suppose there is no single right way for recovery action
> > in host/guest. So there should be room for recovery policies.
>
> Yes I agree. There is already a policy argument part of the uio
> pci_stub driver that I'm working on.
Great.
--
yamahata
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] Re: [PATCH v9 0/8] pcie port switch emulators
2010-11-16 8:26 [Qemu-devel] [PATCH v9 0/8] pcie port switch emulators Isaku Yamahata
` (8 preceding siblings ...)
[not found] ` <1289930315.27724.18.camel@etmartin-lnx>
@ 2010-11-17 14:38 ` Michael S. Tsirkin
9 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2010-11-17 14:38 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 16, 2010 at 05:26:04PM +0900, Isaku Yamahata wrote:
> Now v9 of pcie aer patch series.
> I dropped qmp patch to inject aer error because it will depends
> on Gleb's openfirmware path patches.
> Once his patches are merged, the glue patch will be respined.
>
> Patch description:
> The patch series adds pcie/aer functionality to the pcie port emulators
> and adds new qemu command to inject aer into the guest.
>
> Change v8 -> v9:
> - dropped qmp glue aer error injection.
> - folded pci command register initialization patches
> - make pcie_aer_init() return error
>
> Changes v7 -> v8:
> - Added command to the forward declaration.
> - revise pci command/status register initialization
> - various changes to follow the review
> - use domain:slot.func:slot.func..:slot.func to specify pci function
> instead of domain:bus:slot.func
> - allow symbolic name for aer error name in addition to 32 bit value
>
> Changes v6 -> v7:
> - the glue patch for pushing attention button is dropped for now.
> This will be addressed later.
> - various clean up of aer helper functions.
>
> changes v5 -> v6:
> - dropped already merged patches.
> - add comment on hpev_intx
> - updated the bridge fix patch
> - update the aer patch.
> - reordered the patch series to remove the aer dependency.
>
> change v4 -> v5:
> - introduced pci_xxx_test_and_clear/set_mask
> - eliminated xxx_notify(msi_trigger, int_level)
> - eliminated FLR bits.
> FLR will be addressed at the next phase.
>
> changes v3 -> v4:
> - introduced new pci config helper functions.(clear set bit)
> - various clean up and some bug fixes.
> - dropped pci_shift_xxx().
> - dropped function pointerin pcie_aer.h
> - dropped pci_exp_cap(), pcie_aer_cap().
> - file rename (pcie_{root, upstream, downsatrem} => ioh33420, x3130).
>
> changes v2 -> v3:
> - msi: improved commant and simplified shift/ffs dance
> - pci w1c config register framework
> - split pcie.[ch] into pcie_regs.h, pcie.[ch] and pcie_aer.[ch]
> - pcie, aer: many changes by following reviews.
>
> changes v1 -> v2:
> - update msi
> - dropped already pushed out patches.
> - added msix patches.
>
> Isaku Yamahata (8):
> pci: revise pci command register initialization
> pci: fix accesses to pci status register
I dropped these for now. Maybe for AER
we'll need a small patch to make SERR enable
writeable + a compatibility flag set in
machine description for 0.13 and back?
If yes let's do just this, leave IO/MEMORY alone.
> pci: clean up of pci status register
Applied a minimal subset for AER.
> pcie_regs.h: more constants
Applied after fixing up the comment.
> pcie/aer: helper functions for pcie aer capability
Applied, added cleanup patches on top to remove recursion
> ioh3420: support aer
> x3130/upstream: support aer
> x3130/downstream: support aer.
Applied.
>
> Makefile.objs | 2 +-
> hw/ioh3420.c | 80 ++++-
> hw/pci.c | 120 +++++++-
> hw/pcie.h | 14 +
> hw/pcie_aer.c | 827 +++++++++++++++++++++++++++++++++++++++++++++++
> hw/pcie_aer.h | 106 ++++++
> hw/pcie_regs.h | 2 +
> hw/xio3130_downstream.c | 43 ++-
> hw/xio3130_upstream.c | 33 ++-
> qemu-common.h | 3 +
> 10 files changed, 1189 insertions(+), 41 deletions(-)
> create mode 100644 hw/pcie_aer.c
> create mode 100644 hw/pcie_aer.h
^ permalink raw reply [flat|nested] 29+ messages in thread