* [Qemu-devel] [PATCH v6 01/12] pcie: comment on hpev_intx
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset() Isaku Yamahata
` (11 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
document hpev_intx.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pcie.h | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/hw/pcie.h b/hw/pcie.h
index 68327d8..2871e27 100644
--- a/hw/pcie.h
+++ b/hw/pcie.h
@@ -65,7 +65,15 @@ struct PCIExpressDevice {
/* TODO FLR */
/* SLOT */
- unsigned int hpev_intx; /* INTx for hot plug event */
+ unsigned int hpev_intx; /* INTx for hot plug event (0-3:INT[A-D]#)
+ * 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 */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset()
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 01/12] pcie: comment on hpev_intx Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:49 ` [Qemu-devel] " Michael S. Tsirkin
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 03/12] pcie port: define struct PCIEPort/PCIESlot and helper functions Isaku Yamahata
` (10 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
The default value of base/limit registers aren't specified in the spec.
So pci_bridge_reset() shouldn't touch them.
Instead, introduced two functions to reset those registers in a way
of typical implementation. zero base/limit registers or disable forwarding.
They will be used later.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v5 -> v6:
- pci_bridge_disable_base_limit()
Changes v4 -> v5:
- drop the lines in pci_bridge_reset()
- introduced two functions to reset base/limit registers.
---
hw/pci_bridge.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
hw/pci_bridge.h | 1 +
2 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 638e3b3..7e8488a 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -151,6 +151,26 @@ void pci_bridge_write_config(PCIDevice *d,
}
}
+void pci_bridge_disable_base_limit(PCIDevice *dev)
+{
+ uint8_t *conf = dev->config;
+
+ pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_word_test_and_set_mask(conf + PCI_MEMORY_BASE,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_set_mask(conf + PCI_PREF_MEMORY_BASE,
+ PCI_PREF_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_LIMIT,
+ PCI_PREF_RANGE_MASK & 0xffff);
+ pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
+ pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
+}
+
/* reset bridge specific configuration registers */
void pci_bridge_reset_reg(PCIDevice *dev)
{
@@ -161,12 +181,28 @@ void pci_bridge_reset_reg(PCIDevice *dev)
conf[PCI_SUBORDINATE_BUS] = 0;
conf[PCI_SEC_LATENCY_TIMER] = 0;
- conf[PCI_IO_BASE] = 0;
- conf[PCI_IO_LIMIT] = 0;
- pci_set_word(conf + PCI_MEMORY_BASE, 0);
- pci_set_word(conf + PCI_MEMORY_LIMIT, 0);
- pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0);
- pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0);
+ /*
+ * the default values for base/limit registers aren't specified
+ * in the PCI-to-PCI-bridge spec. So we don't thouch them here.
+ * Each implementation can override it.
+ * typical implementation does
+ * zero base/limit registers or
+ * disable forwarding: pci_bridge_disable_base_limit()
+ * If disable forwarding is wanted, call pci_bridge_disable_base_limit()
+ * after this function.
+ */
+ pci_byte_test_and_clear_mask(conf + PCI_IO_BASE,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_word_test_and_clear_mask(conf + PCI_MEMORY_BASE,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_BASE,
+ PCI_PREF_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_LIMIT,
+ PCI_PREF_RANGE_MASK & 0xffff);
pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
index f6fade0..84411a6 100644
--- a/hw/pci_bridge.h
+++ b/hw/pci_bridge.h
@@ -39,6 +39,7 @@ pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
void pci_bridge_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len);
+void pci_bridge_disable_base_limit(PCIDevice *dev);
void pci_bridge_reset_reg(PCIDevice *dev);
void pci_bridge_reset(DeviceState *qdev);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset()
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset() Isaku Yamahata
@ 2010-10-20 8:49 ` Michael S. Tsirkin
2010-10-20 9:04 ` Isaku Yamahata
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-20 8:49 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Oct 20, 2010 at 05:18:51PM +0900, Isaku Yamahata wrote:
> The default value of base/limit registers aren't specified in the spec.
> So pci_bridge_reset() shouldn't touch them.
> Instead, introduced two functions to reset those registers in a way
> of typical implementation. zero base/limit registers or disable forwarding.
> They will be used later.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
The commit message seems to be out of date?
> ---
> Changes v5 -> v6:
> - pci_bridge_disable_base_limit()
>
> Changes v4 -> v5:
> - drop the lines in pci_bridge_reset()
> - introduced two functions to reset base/limit registers.
> ---
> hw/pci_bridge.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
> hw/pci_bridge.h | 1 +
> 2 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index 638e3b3..7e8488a 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -151,6 +151,26 @@ void pci_bridge_write_config(PCIDevice *d,
> }
> }
>
> +void pci_bridge_disable_base_limit(PCIDevice *dev)
> +{
> + uint8_t *conf = dev->config;
> +
> + pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_word_test_and_set_mask(conf + PCI_MEMORY_BASE,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_set_mask(conf + PCI_PREF_MEMORY_BASE,
> + PCI_PREF_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_LIMIT,
> + PCI_PREF_RANGE_MASK & 0xffff);
> + pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
> + pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
> +}
> +
> /* reset bridge specific configuration registers */
> void pci_bridge_reset_reg(PCIDevice *dev)
> {
> @@ -161,12 +181,28 @@ void pci_bridge_reset_reg(PCIDevice *dev)
> conf[PCI_SUBORDINATE_BUS] = 0;
> conf[PCI_SEC_LATENCY_TIMER] = 0;
>
> - conf[PCI_IO_BASE] = 0;
> - conf[PCI_IO_LIMIT] = 0;
> - pci_set_word(conf + PCI_MEMORY_BASE, 0);
> - pci_set_word(conf + PCI_MEMORY_LIMIT, 0);
> - pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0);
> - pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0);
> + /*
> + * the default values for base/limit registers aren't specified
> + * in the PCI-to-PCI-bridge spec. So we don't thouch them here.
> + * Each implementation can override it.
> + * typical implementation does
> + * zero base/limit registers or
> + * disable forwarding: pci_bridge_disable_base_limit()
> + * If disable forwarding is wanted, call pci_bridge_disable_base_limit()
> + * after this function.
> + */
> + pci_byte_test_and_clear_mask(conf + PCI_IO_BASE,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
> + PCI_IO_RANGE_MASK & 0xff);
> + pci_word_test_and_clear_mask(conf + PCI_MEMORY_BASE,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
> + PCI_MEMORY_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_BASE,
> + PCI_PREF_RANGE_MASK & 0xffff);
> + pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_LIMIT,
> + PCI_PREF_RANGE_MASK & 0xffff);
> pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
> pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
>
> diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
> index f6fade0..84411a6 100644
> --- a/hw/pci_bridge.h
> +++ b/hw/pci_bridge.h
> @@ -39,6 +39,7 @@ pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
>
> void pci_bridge_write_config(PCIDevice *d,
> uint32_t address, uint32_t val, int len);
> +void pci_bridge_disable_base_limit(PCIDevice *dev);
> void pci_bridge_reset_reg(PCIDevice *dev);
> void pci_bridge_reset(DeviceState *qdev);
>
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset()
2010-10-20 8:49 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-10-20 9:04 ` Isaku Yamahata
2010-10-20 10:00 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 9:04 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Oct 20, 2010 at 10:49:20AM +0200, Michael S. Tsirkin wrote:
> On Wed, Oct 20, 2010 at 05:18:51PM +0900, Isaku Yamahata wrote:
> > The default value of base/limit registers aren't specified in the spec.
> > So pci_bridge_reset() shouldn't touch them.
> > Instead, introduced two functions to reset those registers in a way
> > of typical implementation. zero base/limit registers or disable forwarding.
> > They will be used later.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> The commit message seems to be out of date?
Oops. Here's the update one. Only the commit log change.
Should I resend the whole series?
>From a3e0fd4d19879156d40f87228d09c660fc512b16 Mon Sep 17 00:00:00 2001
Message-Id: <a3e0fd4d19879156d40f87228d09c660fc512b16.1287565438.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1287565438.git.yamahata@valinux.co.jp>
References: <cover.1287565438.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Fri, 15 Oct 2010 19:33:50 +0900
Subject: [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset()
The lower bits of base/limit registers is RO and shouldn't be zero cleared
on reset. This patch fixes it.
In fact, the default value of base/limit registers aren't specified
in the spec. And some bridges disable forwarding on reset instead of
zeroing base/limit registers.
So introduce one function to disable bridge forwarding so that
such bridges can use it. It will be used later.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v5 -> v6:
- pci_bridge_disable_base_limit()
Changes v4 -> v5:
- drop the lines in pci_bridge_reset()
- introduced two functions to reset base/limit registers.
---
hw/pci_bridge.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
hw/pci_bridge.h | 1 +
2 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 638e3b3..7e8488a 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -151,6 +151,26 @@ void pci_bridge_write_config(PCIDevice *d,
}
}
+void pci_bridge_disable_base_limit(PCIDevice *dev)
+{
+ uint8_t *conf = dev->config;
+
+ pci_byte_test_and_set_mask(conf + PCI_IO_BASE,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_word_test_and_set_mask(conf + PCI_MEMORY_BASE,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_set_mask(conf + PCI_PREF_MEMORY_BASE,
+ PCI_PREF_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_LIMIT,
+ PCI_PREF_RANGE_MASK & 0xffff);
+ pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
+ pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
+}
+
/* reset bridge specific configuration registers */
void pci_bridge_reset_reg(PCIDevice *dev)
{
@@ -161,12 +181,28 @@ void pci_bridge_reset_reg(PCIDevice *dev)
conf[PCI_SUBORDINATE_BUS] = 0;
conf[PCI_SEC_LATENCY_TIMER] = 0;
- conf[PCI_IO_BASE] = 0;
- conf[PCI_IO_LIMIT] = 0;
- pci_set_word(conf + PCI_MEMORY_BASE, 0);
- pci_set_word(conf + PCI_MEMORY_LIMIT, 0);
- pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0);
- pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0);
+ /*
+ * the default values for base/limit registers aren't specified
+ * in the PCI-to-PCI-bridge spec. So we don't thouch them here.
+ * Each implementation can override it.
+ * typical implementation does
+ * zero base/limit registers or
+ * disable forwarding: pci_bridge_disable_base_limit()
+ * If disable forwarding is wanted, call pci_bridge_disable_base_limit()
+ * after this function.
+ */
+ pci_byte_test_and_clear_mask(conf + PCI_IO_BASE,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT,
+ PCI_IO_RANGE_MASK & 0xff);
+ pci_word_test_and_clear_mask(conf + PCI_MEMORY_BASE,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_MEMORY_LIMIT,
+ PCI_MEMORY_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_BASE,
+ PCI_PREF_RANGE_MASK & 0xffff);
+ pci_word_test_and_clear_mask(conf + PCI_PREF_MEMORY_LIMIT,
+ PCI_PREF_RANGE_MASK & 0xffff);
pci_set_word(conf + PCI_PREF_BASE_UPPER32, 0);
pci_set_word(conf + PCI_PREF_LIMIT_UPPER32, 0);
diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h
index f6fade0..84411a6 100644
--- a/hw/pci_bridge.h
+++ b/hw/pci_bridge.h
@@ -39,6 +39,7 @@ pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type);
void pci_bridge_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len);
+void pci_bridge_disable_base_limit(PCIDevice *dev);
void pci_bridge_reset_reg(PCIDevice *dev);
void pci_bridge_reset(DeviceState *qdev);
--
1.7.1.1
--
yamahata
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset()
2010-10-20 9:04 ` Isaku Yamahata
@ 2010-10-20 10:00 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-20 10:00 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Oct 20, 2010 at 06:04:49PM +0900, Isaku Yamahata wrote:
> On Wed, Oct 20, 2010 at 10:49:20AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Oct 20, 2010 at 05:18:51PM +0900, Isaku Yamahata wrote:
> > > The default value of base/limit registers aren't specified in the spec.
> > > So pci_bridge_reset() shouldn't touch them.
> > > Instead, introduced two functions to reset those registers in a way
> > > of typical implementation. zero base/limit registers or disable forwarding.
> > > They will be used later.
> > >
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > The commit message seems to be out of date?
>
> Oops. Here's the update one. Only the commit log change.
> Should I resend the whole series?
No, I can handle that.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH v6 03/12] pcie port: define struct PCIEPort/PCIESlot and helper functions
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 01/12] pcie: comment on hpev_intx Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 02/12] pci/bridge: fix pci_bridge_reset() Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 04/12] ioh3420: pcie root port in X58 ioh Isaku Yamahata
` (9 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
define struct PCIEPort which represents common part
of pci express port.(root, upstream and downstream.)
add a helper function for pcie port which can be used commonly by
root/upstream/downstream port.
define struct PCIESlot which represents common part of
pcie slot.(root and downstream.) and helper functions for it.
helper functions for chassis, slot -> PCIESlot conversion.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v4 -> v5:
- use pci_xxx_test_and_xxx_mask()
Changes v3 -> v4:
- Initialize prefetchable memory base/limit registers correctly.
They must support 64bit.
- compilation adjustment.
Changes v2 -> v3:
- static'fy chassis.
- compilation adjustment.
---
Makefile.objs | 2 +-
hw/pcie_port.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pcie_port.h | 51 ++++++++++++++++++++++++
qemu-common.h | 2 +
4 files changed, 170 insertions(+), 1 deletions(-)
create mode 100644 hw/pcie_port.c
create mode 100644 hw/pcie_port.h
diff --git a/Makefile.objs b/Makefile.objs
index eeb5134..c73d12b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -186,7 +186,7 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
# PCI watchdog devices
hw-obj-y += wdt_i6300esb.o
-hw-obj-y += pcie.o
+hw-obj-y += pcie.o pcie_port.o
hw-obj-y += msix.o msi.o
# PCI network cards
diff --git a/hw/pcie_port.c b/hw/pcie_port.c
new file mode 100644
index 0000000..117de61
--- /dev/null
+++ b/hw/pcie_port.c
@@ -0,0 +1,116 @@
+/*
+ * pcie_port.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 "pcie_port.h"
+
+void pcie_port_init_reg(PCIDevice *d)
+{
+ /* Unlike pci bridge,
+ 66MHz and fast back to back don't apply to pci express port. */
+ pci_set_word(d->config + PCI_STATUS, 0);
+ pci_set_word(d->config + PCI_SEC_STATUS, 0);
+
+ /* 7.5.3.5 Prefetchable Memory Base Limit
+ * The Prefetchable Memory Base and Prefetchable Memory Limit registers
+ * must indicate that 64-bit addresses are supported, as defined in
+ * PCI-to-PCI Bridge Architecture Specification, Revision 1.2.
+ */
+ pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_BASE,
+ PCI_PREF_RANGE_TYPE_64);
+ pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_LIMIT,
+ PCI_PREF_RANGE_TYPE_64);
+}
+
+/**************************************************************************
+ * (chassis number, pcie physical slot number) -> pcie slot conversion
+ */
+struct PCIEChassis {
+ uint8_t number;
+
+ QLIST_HEAD(, PCIESlot) slots;
+ QLIST_ENTRY(PCIEChassis) next;
+};
+
+static QLIST_HEAD(, PCIEChassis) chassis = QLIST_HEAD_INITIALIZER(chassis);
+
+static struct PCIEChassis *pcie_chassis_find(uint8_t chassis_number)
+{
+ struct PCIEChassis *c;
+ QLIST_FOREACH(c, &chassis, next) {
+ if (c->number == chassis_number) {
+ break;
+ }
+ }
+ return c;
+}
+
+void pcie_chassis_create(uint8_t chassis_number)
+{
+ struct PCIEChassis *c;
+ c = pcie_chassis_find(chassis_number);
+ if (c) {
+ return;
+ }
+ c = qemu_mallocz(sizeof(*c));
+ c->number = chassis_number;
+ QLIST_INIT(&c->slots);
+ QLIST_INSERT_HEAD(&chassis, c, next);
+}
+
+static PCIESlot *pcie_chassis_find_slot_with_chassis(struct PCIEChassis *c,
+ uint8_t slot)
+{
+ PCIESlot *s;
+ QLIST_FOREACH(s, &c->slots, next) {
+ if (s->slot == slot) {
+ break;
+ }
+ }
+ return s;
+}
+
+PCIESlot *pcie_chassis_find_slot(uint8_t chassis_number, uint16_t slot)
+{
+ struct PCIEChassis *c;
+ c = pcie_chassis_find(chassis_number);
+ if (!c) {
+ return NULL;
+ }
+ return pcie_chassis_find_slot_with_chassis(c, slot);
+}
+
+int pcie_chassis_add_slot(struct PCIESlot *slot)
+{
+ struct PCIEChassis *c;
+ c = pcie_chassis_find(slot->chassis);
+ if (!c) {
+ return -ENODEV;
+ }
+ if (pcie_chassis_find_slot_with_chassis(c, slot->slot)) {
+ return -EBUSY;
+ }
+ QLIST_INSERT_HEAD(&c->slots, slot, next);
+ return 0;
+}
+
+void pcie_chassis_del_slot(PCIESlot *s)
+{
+ QLIST_REMOVE(s, next);
+}
diff --git a/hw/pcie_port.h b/hw/pcie_port.h
new file mode 100644
index 0000000..3709583
--- /dev/null
+++ b/hw/pcie_port.h
@@ -0,0 +1,51 @@
+/*
+ * pcie_port.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_PORT_H
+#define QEMU_PCIE_PORT_H
+
+#include "pci_bridge.h"
+#include "pci_internals.h"
+
+struct PCIEPort {
+ PCIBridge br;
+
+ /* pci express switch port */
+ uint8_t port;
+};
+
+void pcie_port_init_reg(PCIDevice *d);
+
+struct PCIESlot {
+ PCIEPort port;
+
+ /* pci express switch port with slot */
+ uint8_t chassis;
+ uint16_t slot;
+ QLIST_ENTRY(PCIESlot) next;
+};
+
+void pcie_chassis_create(uint8_t chassis_number);
+void pcie_main_chassis_create(void);
+PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
+int pcie_chassis_add_slot(struct PCIESlot *slot);
+void pcie_chassis_del_slot(PCIESlot *s);
+
+#endif /* QEMU_PCIE_PORT_H */
diff --git a/qemu-common.h b/qemu-common.h
index 6d9ee26..b97b16e 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -221,6 +221,8 @@ typedef struct PCIBus PCIBus;
typedef struct PCIDevice PCIDevice;
typedef struct PCIExpressDevice PCIExpressDevice;
typedef struct PCIBridge PCIBridge;
+typedef struct PCIEPort PCIEPort;
+typedef struct PCIESlot PCIESlot;
typedef struct SerialState SerialState;
typedef struct IRQState *qemu_irq;
typedef struct PCMCIACardState PCMCIACardState;
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] [PATCH v6 04/12] ioh3420: pcie root port in X58 ioh
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (2 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 03/12] pcie port: define struct PCIEPort/PCIESlot and helper functions Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 05/12] x3130: pcie upstream port Isaku Yamahata
` (8 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
Implements pcie root port switch in intel X58 ioh
whose device id is 0x3420.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v5 -> v6:
- compilation adjustment.
- eliminated aer bits.
Changes v4 -> v5:
- use pci_xxx_test_and_xxx_mask()
Changes v3 -> v4:
- rename pcie_root -> ioh3420
- compilation adjustment.
Changes v2 -> v3:
- compilation adjustment.
---
Makefile.objs | 1 +
hw/ioh3420.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/ioh3420.h | 10 +++
3 files changed, 199 insertions(+), 0 deletions(-)
create mode 100644 hw/ioh3420.c
create mode 100644 hw/ioh3420.h
diff --git a/Makefile.objs b/Makefile.objs
index c73d12b..3a05322 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -140,6 +140,7 @@ hw-obj-y =
hw-obj-y += vl.o loader.o
hw-obj-y += virtio.o virtio-console.o
hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o pci_bridge.o
+hw-obj-y += ioh3420.o
hw-obj-y += watchdog.o
hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
hw-obj-$(CONFIG_ECC) += ecc.o
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
new file mode 100644
index 0000000..1f340d3
--- /dev/null
+++ b/hw/ioh3420.c
@@ -0,0 +1,188 @@
+/*
+ * ioh3420.c
+ * Intel X58 north bridge IOH
+ * PCI Express root port device id 3420
+ *
+ * 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 "pci_ids.h"
+#include "msi.h"
+#include "pcie.h"
+#include "ioh3420.h"
+
+#define PCI_DEVICE_ID_IOH_EPORT 0x3420 /* D0:F0 express mode */
+#define PCI_DEVICE_ID_IOH_REV 0x2
+#define IOH_EP_SSVID_OFFSET 0x40
+#define IOH_EP_SSVID_SVID PCI_VENDOR_ID_INTEL
+#define IOH_EP_SSVID_SSID 0
+#define IOH_EP_MSI_OFFSET 0x60
+#define IOH_EP_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_MASKBIT
+#define IOH_EP_MSI_NR_VECTOR 2
+#define IOH_EP_EXP_OFFSET 0x90
+#define IOH_EP_AER_OFFSET 0x100
+
+static void ioh3420_write_config(PCIDevice *d,
+ uint32_t address, uint32_t val, int len)
+{
+ uint16_t sltctl =
+ pci_get_word(d->config + d->exp.exp_cap + PCI_EXP_SLTCTL);
+
+ pci_bridge_write_config(d, address, val, len);
+ msi_write_config(d, address, val, len);
+ pcie_cap_slot_write_config(d, address, val, len, sltctl);
+ /* TODO: AER */
+}
+
+static void ioh3420_reset(DeviceState *qdev)
+{
+ PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
+ msi_reset(d);
+ pcie_cap_root_reset(d);
+ pcie_cap_deverr_reset(d);
+ pcie_cap_slot_reset(d);
+ pci_bridge_reset(qdev);
+ pci_bridge_disable_base_limit(d);
+ /* TODO: AER */
+}
+
+static int ioh3420_initfn(PCIDevice *d)
+{
+ PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
+ PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
+ PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
+ int rc;
+
+ rc = pci_bridge_initfn(d);
+ if (rc < 0) {
+ return rc;
+ }
+
+ d->config[PCI_REVISION_ID] = PCI_DEVICE_ID_IOH_REV;
+ pcie_port_init_reg(d);
+
+ pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_INTEL);
+ pci_config_set_device_id(d->config, PCI_DEVICE_ID_IOH_EPORT);
+
+ rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
+ IOH_EP_SSVID_SVID, IOH_EP_SSVID_SSID);
+ if (rc < 0) {
+ return rc;
+ }
+ 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;
+ }
+ rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, p->port);
+ if (rc < 0) {
+ return rc;
+ }
+ 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) {
+ return rc;
+ }
+ pcie_cap_root_init(d);
+ /* TODO: AER */
+ return 0;
+}
+
+static int ioh3420_exitfn(PCIDevice *d)
+{
+ /* TODO: AER */
+ msi_uninit(d);
+ pcie_cap_exit(d);
+ return pci_bridge_exitfn(d);
+}
+
+PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool multifunction,
+ const char *bus_name, pci_map_irq_fn map_irq,
+ uint8_t port, uint8_t chassis, uint16_t slot)
+{
+ PCIDevice *d;
+ PCIBridge *br;
+ DeviceState *qdev;
+
+ d = pci_create_multifunction(bus, devfn, multifunction, "ioh3420");
+ if (!d) {
+ return NULL;
+ }
+ br = DO_UPCAST(PCIBridge, dev, d);
+
+ qdev = &br->dev.qdev;
+ pci_bridge_map_irq(br, bus_name, map_irq);
+ qdev_prop_set_uint8(qdev, "port", port);
+ qdev_prop_set_uint8(qdev, "chassis", chassis);
+ qdev_prop_set_uint16(qdev, "slot", slot);
+ qdev_init_nofail(qdev);
+
+ return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
+}
+
+static const VMStateDescription vmstate_ioh3420 = {
+ .name = "ioh-3240-express-root-port",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCIE_DEVICE(port.br.dev, PCIESlot),
+ /* TODO: AER */
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static PCIDeviceInfo ioh3420_info = {
+ .qdev.name = "ioh3420",
+ .qdev.desc = "Intel IOH device id 3420 PCIE Root Port",
+ .qdev.size = sizeof(PCIESlot),
+ .qdev.reset = ioh3420_reset,
+ .qdev.vmsd = &vmstate_ioh3420,
+
+ .is_express = 1,
+ .is_bridge = 1,
+ .config_write = ioh3420_write_config,
+ .init = ioh3420_initfn,
+ .exit = ioh3420_exitfn,
+
+ .qdev.props = (Property[]) {
+ 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_END_OF_LIST(),
+ }
+};
+
+static void ioh3420_register(void)
+{
+ pci_qdev_register(&ioh3420_info);
+}
+
+device_init(ioh3420_register);
+
+/*
+ * Local variables:
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 8
+ * indent-tab-mode: nil
+ * End:
+ */
diff --git a/hw/ioh3420.h b/hw/ioh3420.h
new file mode 100644
index 0000000..68c523a
--- /dev/null
+++ b/hw/ioh3420.h
@@ -0,0 +1,10 @@
+#ifndef QEMU_IOH3420_H
+#define QEMU_IOH3420_H
+
+#include "pcie_port.h"
+
+PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool multifunction,
+ const char *bus_name, pci_map_irq_fn map_irq,
+ uint8_t port, uint8_t chassis, uint16_t slot);
+
+#endif /* QEMU_IOH3420_H */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] [PATCH v6 05/12] x3130: pcie upstream port
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (3 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 04/12] ioh3420: pcie root port in X58 ioh Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 06/12] x3130: pcie downstream port Isaku Yamahata
` (7 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
Implement TI x3130 pcie upstream port switch.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v5 -> v6:
- compilation adjustment.
- delete aer bits.
Changes v4 -> v5:
- remove flr related stuff.
This will be addressed at the next phase.
- use pci_xxx_test_and_xxx_mask().
Chnages v3 -> v4:
- rename pcie_upstream -> x3130_upstream.
- compilation adjustment.
Changes v2 -> v3:
- compilation adjustment.
---
Makefile.objs | 2 +-
hw/xio3130_upstream.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
hw/xio3130_upstream.h | 10 +++
3 files changed, 185 insertions(+), 1 deletions(-)
create mode 100644 hw/xio3130_upstream.c
create mode 100644 hw/xio3130_upstream.h
diff --git a/Makefile.objs b/Makefile.objs
index 3a05322..b1ef2bb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -140,7 +140,7 @@ hw-obj-y =
hw-obj-y += vl.o loader.o
hw-obj-y += virtio.o virtio-console.o
hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o pci_bridge.o
-hw-obj-y += ioh3420.o
+hw-obj-y += ioh3420.o xio3130_upstream.o
hw-obj-y += watchdog.o
hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
hw-obj-$(CONFIG_ECC) += ecc.o
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
new file mode 100644
index 0000000..d9d637f
--- /dev/null
+++ b/hw/xio3130_upstream.c
@@ -0,0 +1,174 @@
+/*
+ * xio3130_upstream.c
+ * TI X3130 pci express upstream port switch
+ *
+ * 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 "pci_ids.h"
+#include "msi.h"
+#include "pcie.h"
+#include "xio3130_upstream.h"
+
+#define PCI_DEVICE_ID_TI_XIO3130U 0x8232 /* upstream port */
+#define XIO3130_REVISION 0x2
+#define XIO3130_MSI_OFFSET 0x70
+#define XIO3130_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_64BIT
+#define XIO3130_MSI_NR_VECTOR 1
+#define XIO3130_SSVID_OFFSET 0x80
+#define XIO3130_SSVID_SVID 0
+#define XIO3130_SSVID_SSID 0
+#define XIO3130_EXP_OFFSET 0x90
+#define XIO3130_AER_OFFSET 0x100
+
+static void xio3130_upstream_write_config(PCIDevice *d, uint32_t address,
+ uint32_t val, int len)
+{
+ 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 */
+}
+
+static void xio3130_upstream_reset(DeviceState *qdev)
+{
+ PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
+ msi_reset(d);
+ pci_bridge_reset(qdev);
+ pcie_cap_deverr_reset(d);
+}
+
+static int xio3130_upstream_initfn(PCIDevice *d)
+{
+ PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
+ PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
+ int rc;
+
+ rc = pci_bridge_initfn(d);
+ if (rc < 0) {
+ return rc;
+ }
+
+ pcie_port_init_reg(d);
+ pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_TI);
+ pci_config_set_device_id(d->config, PCI_DEVICE_ID_TI_XIO3130U);
+ d->config[PCI_REVISION_ID] = XIO3130_REVISION;
+
+ rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
+ XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
+ XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
+ if (rc < 0) {
+ return rc;
+ }
+ rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
+ XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
+ if (rc < 0) {
+ return rc;
+ }
+ rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_UPSTREAM,
+ p->port);
+ if (rc < 0) {
+ return rc;
+ }
+
+ /* TODO: implement FLR */
+ pcie_cap_flr_init(d);
+
+ pcie_cap_deverr_init(d);
+ /* TODO: AER */
+
+ return 0;
+}
+
+static int xio3130_upstream_exitfn(PCIDevice *d)
+{
+ /* TODO: AER */
+ msi_uninit(d);
+ pcie_cap_exit(d);
+ return pci_bridge_exitfn(d);
+}
+
+PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
+ const char *bus_name, pci_map_irq_fn map_irq,
+ uint8_t port)
+{
+ PCIDevice *d;
+ PCIBridge *br;
+ DeviceState *qdev;
+
+ d = pci_create_multifunction(bus, devfn, multifunction, "x3130-upstream");
+ if (!d) {
+ return NULL;
+ }
+ br = DO_UPCAST(PCIBridge, dev, d);
+
+ qdev = &br->dev.qdev;
+ pci_bridge_map_irq(br, bus_name, map_irq);
+ qdev_prop_set_uint8(qdev, "port", port);
+ qdev_init_nofail(qdev);
+
+ return DO_UPCAST(PCIEPort, br, br);
+}
+
+static const VMStateDescription vmstate_xio3130_upstream = {
+ .name = "xio3130-express-upstream-port",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCIE_DEVICE(br.dev, PCIEPort),
+ /* TODO: AER */
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static PCIDeviceInfo xio3130_upstream_info = {
+ .qdev.name = "x3130-upstream",
+ .qdev.desc = "TI X3130 Upstream Port of PCI Express Switch",
+ .qdev.size = sizeof(PCIEPort),
+ .qdev.reset = xio3130_upstream_reset,
+ .qdev.vmsd = &vmstate_xio3130_upstream,
+
+ .is_express = 1,
+ .is_bridge = 1,
+ .config_write = xio3130_upstream_write_config,
+ .init = xio3130_upstream_initfn,
+ .exit = xio3130_upstream_exitfn,
+
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_UINT8("port", PCIEPort, port, 0),
+ /* TODO: AER */
+ DEFINE_PROP_END_OF_LIST(),
+ }
+};
+
+static void xio3130_upstream_register(void)
+{
+ pci_qdev_register(&xio3130_upstream_info);
+}
+
+device_init(xio3130_upstream_register);
+
+
+/*
+ * Local variables:
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 8
+ * indent-tab-mode: nil
+ * End:
+ */
diff --git a/hw/xio3130_upstream.h b/hw/xio3130_upstream.h
new file mode 100644
index 0000000..e996997
--- /dev/null
+++ b/hw/xio3130_upstream.h
@@ -0,0 +1,10 @@
+#ifndef QEMU_XIO3130_UPSTREAM_H
+#define QEMU_XIO3130_UPSTREAM_H
+
+#include "pcie_port.h"
+
+PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
+ const char *bus_name, pci_map_irq_fn map_irq,
+ uint8_t port);
+
+#endif /* QEMU_XIO3130_H */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] [PATCH v6 06/12] x3130: pcie downstream port
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (4 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 05/12] x3130: pcie upstream port Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command Isaku Yamahata
` (6 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
Implement TI x3130 pcie downstream port switch.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v5 -> v6:
- compilation adjustment.
- eliminate aer bits.
Changes v4 -> v5:
- use pci_xxx_test_and_xxx_mask().
- removed flr related stuff.
Changes v3 -> v4:
- rename: pcie_downstream -> x3130_downstream
- compilation adjustment.
Changes v2 -> v3:
- compilation adjustment.
---
Makefile.objs | 2 +-
hw/xio3130_downstream.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++
hw/xio3130_downstream.h | 11 +++
3 files changed, 202 insertions(+), 1 deletions(-)
create mode 100644 hw/xio3130_downstream.c
create mode 100644 hw/xio3130_downstream.h
diff --git a/Makefile.objs b/Makefile.objs
index b1ef2bb..138e545 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -140,7 +140,7 @@ hw-obj-y =
hw-obj-y += vl.o loader.o
hw-obj-y += virtio.o virtio-console.o
hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o pci_bridge.o
-hw-obj-y += ioh3420.o xio3130_upstream.o
+hw-obj-y += ioh3420.o xio3130_upstream.o xio3130_downstream.o
hw-obj-y += watchdog.o
hw-obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
hw-obj-$(CONFIG_ECC) += ecc.o
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
new file mode 100644
index 0000000..a44e188
--- /dev/null
+++ b/hw/xio3130_downstream.c
@@ -0,0 +1,190 @@
+/*
+ * x3130_downstream.c
+ * TI X3130 pci express downstream port switch
+ *
+ * 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 "pci_ids.h"
+#include "msi.h"
+#include "pcie.h"
+#include "xio3130_downstream.h"
+
+#define PCI_DEVICE_ID_TI_XIO3130D 0x8233 /* downstream port */
+#define XIO3130_REVISION 0x1
+#define XIO3130_MSI_OFFSET 0x70
+#define XIO3130_MSI_SUPPORTED_FLAGS PCI_MSI_FLAGS_64BIT
+#define XIO3130_MSI_NR_VECTOR 1
+#define XIO3130_SSVID_OFFSET 0x80
+#define XIO3130_SSVID_SVID 0
+#define XIO3130_SSVID_SSID 0
+#define XIO3130_EXP_OFFSET 0x90
+#define XIO3130_AER_OFFSET 0x100
+
+static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address,
+ uint32_t val, int len)
+{
+ uint16_t sltctl =
+ pci_get_word(d->config + d->exp.exp_cap + PCI_EXP_SLTCTL);
+
+ pci_bridge_write_config(d, address, val, len);
+ pcie_cap_flr_write_config(d, address, val, len);
+ pcie_cap_slot_write_config(d, address, val, len, sltctl);
+ msi_write_config(d, address, val, len);
+ /* TODO: AER */
+}
+
+static void xio3130_downstream_reset(DeviceState *qdev)
+{
+ PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
+ msi_reset(d);
+ pcie_cap_deverr_reset(d);
+ pcie_cap_slot_reset(d);
+ pcie_cap_ari_reset(d);
+ pci_bridge_reset(qdev);
+}
+
+static int xio3130_downstream_initfn(PCIDevice *d)
+{
+ PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
+ PCIEPort *p = DO_UPCAST(PCIEPort, br, br);
+ PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
+ int rc;
+
+ rc = pci_bridge_initfn(d);
+ if (rc < 0) {
+ return rc;
+ }
+
+ pcie_port_init_reg(d);
+ pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_TI);
+ pci_config_set_device_id(d->config, PCI_DEVICE_ID_TI_XIO3130D);
+ d->config[PCI_REVISION_ID] = XIO3130_REVISION;
+
+ rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
+ XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
+ XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
+ if (rc < 0) {
+ return rc;
+ }
+ rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
+ XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
+ if (rc < 0) {
+ return rc;
+ }
+ rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_DOWNSTREAM,
+ p->port);
+ if (rc < 0) {
+ return rc;
+ }
+ pcie_cap_flr_init(d); /* TODO: implement FLR */
+ 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) {
+ return rc;
+ }
+ pcie_cap_ari_init(d);
+ /* TODO: AER */
+
+ return 0;
+}
+
+static int xio3130_downstream_exitfn(PCIDevice *d)
+{
+ /* TODO: AER */
+ msi_uninit(d);
+ pcie_cap_exit(d);
+ return pci_bridge_exitfn(d);
+}
+
+PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,
+ const char *bus_name, pci_map_irq_fn map_irq,
+ uint8_t port, uint8_t chassis,
+ uint16_t slot)
+{
+ PCIDevice *d;
+ PCIBridge *br;
+ DeviceState *qdev;
+
+ d = pci_create_multifunction(bus, devfn, multifunction,
+ "xio3130-downstream");
+ if (!d) {
+ return NULL;
+ }
+ br = DO_UPCAST(PCIBridge, dev, d);
+
+ qdev = &br->dev.qdev;
+ pci_bridge_map_irq(br, bus_name, map_irq);
+ qdev_prop_set_uint8(qdev, "port", port);
+ qdev_prop_set_uint8(qdev, "chassis", chassis);
+ qdev_prop_set_uint16(qdev, "slot", slot);
+ qdev_init_nofail(qdev);
+
+ return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br));
+}
+
+static const VMStateDescription vmstate_xio3130_downstream = {
+ .name = "xio3130-express-downstream-port",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCIE_DEVICE(port.br.dev, PCIESlot),
+ /* TODO: AER */
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static PCIDeviceInfo xio3130_downstream_info = {
+ .qdev.name = "xio3130-downstream",
+ .qdev.desc = "TI X3130 Downstream Port of PCI Express Switch",
+ .qdev.size = sizeof(PCIESlot),
+ .qdev.reset = xio3130_downstream_reset,
+ .qdev.vmsd = &vmstate_xio3130_downstream,
+
+ .is_express = 1,
+ .is_bridge = 1,
+ .config_write = xio3130_downstream_write_config,
+ .init = xio3130_downstream_initfn,
+ .exit = xio3130_downstream_exitfn,
+
+ .qdev.props = (Property[]) {
+ 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_END_OF_LIST(),
+ }
+};
+
+static void xio3130_downstream_register(void)
+{
+ pci_qdev_register(&xio3130_downstream_info);
+}
+
+device_init(xio3130_downstream_register);
+
+/*
+ * Local variables:
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 8
+ * indent-tab-mode: nil
+ * End:
+ */
diff --git a/hw/xio3130_downstream.h b/hw/xio3130_downstream.h
new file mode 100644
index 0000000..010487f
--- /dev/null
+++ b/hw/xio3130_downstream.h
@@ -0,0 +1,11 @@
+#ifndef QEMU_XIO3130_DOWNSTREAM_H
+#define QEMU_XIO3130_DOWNSTREAM_H
+
+#include "pcie_port.h"
+
+PCIESlot *xio3130_downstream_init(PCIBus *bus, int devfn, bool multifunction,
+ const char *bus_name, pci_map_irq_fn map_irq,
+ uint8_t port, uint8_t chassis,
+ uint16_t slot);
+
+#endif /* QEMU_XIO3130_DOWNSTREAM_H */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (5 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 06/12] x3130: pcie downstream port Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 10:00 ` [Qemu-devel] " Michael S. Tsirkin
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
` (5 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
glue pcie_push_attention_button command.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pcie_port.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-monitor.hx | 14 +++++++++
sysemu.h | 4 +++
3 files changed, 100 insertions(+), 0 deletions(-)
diff --git a/hw/pcie_port.c b/hw/pcie_port.c
index 117de61..f43a1c7 100644
--- a/hw/pcie_port.c
+++ b/hw/pcie_port.c
@@ -18,6 +18,10 @@
* with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "qemu-objects.h"
+#include "sysemu.h"
+#include "monitor.h"
+#include "pcie.h"
#include "pcie_port.h"
void pcie_port_init_reg(PCIDevice *d)
@@ -114,3 +118,81 @@ void pcie_chassis_del_slot(PCIESlot *s)
{
QLIST_REMOVE(s, next);
}
+
+/**************************************************************************
+ * glue for qemu monitor
+ */
+
+/* Parse [<chassis>.]<slot>, return -1 on error */
+static int pcie_parse_slot_addr(const char* slot_addr,
+ uint8_t *chassisp, uint16_t *slotp)
+{
+ const char *p;
+ char *e;
+ unsigned long val;
+ unsigned long chassis = 0;
+ unsigned long slot;
+
+ p = slot_addr;
+ val = strtoul(p, &e, 0);
+ if (e == p) {
+ return -1;
+ }
+ if (*e == '.') {
+ chassis = val;
+ p = e + 1;
+ val = strtoul(p, &e, 0);
+ if (e == p) {
+ return -1;
+ }
+ }
+ slot = val;
+
+ if (*e) {
+ return -1;
+ }
+
+ if (chassis > 0xff || slot > 0xffff) {
+ return -1;
+ }
+
+ *chassisp = chassis;
+ *slotp = slot;
+ return 0;
+}
+
+void pcie_attention_button_push_print(Monitor *mon, const QObject *data)
+{
+ QDict *qdict;
+
+ assert(qobject_type(data) == QTYPE_QDICT);
+ qdict = qobject_to_qdict(data);
+
+ monitor_printf(mon, "OK chassis %d, slot %d\n",
+ (int) qdict_get_int(qdict, "chassis"),
+ (int) qdict_get_int(qdict, "slot"));
+}
+
+int pcie_attention_button_push(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+ const char* pcie_slot = qdict_get_str(qdict, "pcie_slot");
+ uint8_t chassis;
+ uint16_t slot;
+ PCIESlot *s;
+
+ if (pcie_parse_slot_addr(pcie_slot, &chassis, &slot) < 0) {
+ monitor_printf(mon, "invalid pcie slot address %s\n", pcie_slot);
+ return -1;
+ }
+ s = pcie_chassis_find_slot(chassis, slot);
+ if (!s) {
+ monitor_printf(mon, "slot is not found. %s\n", pcie_slot);
+ return -1;
+ }
+ pcie_cap_slot_push_attention_button(&s->port.br.dev);
+ *ret_data = qobject_from_jsonf("{ 'chassis': %d, 'slot': %d}",
+ chassis, slot);
+ assert(*ret_data);
+ return 0;
+}
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 2af3de6..965c754 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1154,6 +1154,20 @@ Hot remove PCI device.
ETEXI
{
+ .name = "pcie_push_attention_button",
+ .args_type = "pcie_slot:s",
+ .params = "[<chassis>.]<slot>",
+ .help = "push pci express attention button",
+ .user_print = pcie_attention_button_push_print,
+ .mhandler.cmd_new = pcie_attention_button_push,
+ },
+
+STEXI
+@item pcie_abp
+Push PCI express attention button
+ETEXI
+
+ {
.name = "host_net_add",
.args_type = "device:s,opts:s?",
.params = "tap|user|socket|vde|dump [options]",
diff --git a/sysemu.h b/sysemu.h
index 9c988bb..cca411d 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -150,6 +150,10 @@ extern unsigned int nb_prom_envs;
void pci_device_hot_add(Monitor *mon, const QDict *qdict);
void drive_hot_add(Monitor *mon, const QDict *qdict);
void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
+/* pcie hotplug */
+void pcie_attention_button_push_print(Monitor *mon, const QObject *data);
+int pcie_attention_button_push(Monitor *mon, const QDict *qdict,
+ QObject **ret_data);
/* serial ports */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command Isaku Yamahata
@ 2010-10-20 10:00 ` Michael S. Tsirkin
2010-10-21 3:46 ` Isaku Yamahata
2010-10-22 11:35 ` Markus Armbruster
0 siblings, 2 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-20 10:00 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, lmr, etmartin, wexu2, qemu-devel, kraxel
On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> glue pcie_push_attention_button command.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
So as a high level command, I think we need to
think about how to tie this into pci_add/pci_del.
Right?
As a low level command, this is not really useful unless
there is an event on LED status change and a way
to get info on LED status.
Right?
> ---
> hw/pcie_port.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-monitor.hx | 14 +++++++++
> sysemu.h | 4 +++
> 3 files changed, 100 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pcie_port.c b/hw/pcie_port.c
> index 117de61..f43a1c7 100644
> --- a/hw/pcie_port.c
> +++ b/hw/pcie_port.c
> @@ -18,6 +18,10 @@
> * with this program; if not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include "qemu-objects.h"
> +#include "sysemu.h"
> +#include "monitor.h"
> +#include "pcie.h"
> #include "pcie_port.h"
>
> void pcie_port_init_reg(PCIDevice *d)
> @@ -114,3 +118,81 @@ void pcie_chassis_del_slot(PCIESlot *s)
> {
> QLIST_REMOVE(s, next);
> }
> +
> +/**************************************************************************
> + * glue for qemu monitor
> + */
> +
> +/* Parse [<chassis>.]<slot>, return -1 on error */
> +static int pcie_parse_slot_addr(const char* slot_addr,
> + uint8_t *chassisp, uint16_t *slotp)
> +{
> + const char *p;
> + char *e;
> + unsigned long val;
> + unsigned long chassis = 0;
> + unsigned long slot;
> +
> + p = slot_addr;
> + val = strtoul(p, &e, 0);
> + if (e == p) {
> + return -1;
> + }
> + if (*e == '.') {
> + chassis = val;
> + p = e + 1;
> + val = strtoul(p, &e, 0);
> + if (e == p) {
> + return -1;
> + }
> + }
> + slot = val;
> +
> + if (*e) {
> + return -1;
> + }
> +
> + if (chassis > 0xff || slot > 0xffff) {
> + return -1;
> + }
> +
> + *chassisp = chassis;
> + *slotp = slot;
> + return 0;
> +}
> +
> +void pcie_attention_button_push_print(Monitor *mon, const QObject *data)
> +{
> + QDict *qdict;
> +
> + assert(qobject_type(data) == QTYPE_QDICT);
> + qdict = qobject_to_qdict(data);
> +
> + monitor_printf(mon, "OK chassis %d, slot %d\n",
> + (int) qdict_get_int(qdict, "chassis"),
> + (int) qdict_get_int(qdict, "slot"));
> +}
> +
> +int pcie_attention_button_push(Monitor *mon, const QDict *qdict,
> + QObject **ret_data)
> +{
> + const char* pcie_slot = qdict_get_str(qdict, "pcie_slot");
> + uint8_t chassis;
> + uint16_t slot;
> + PCIESlot *s;
> +
> + if (pcie_parse_slot_addr(pcie_slot, &chassis, &slot) < 0) {
> + monitor_printf(mon, "invalid pcie slot address %s\n", pcie_slot);
> + return -1;
> + }
> + s = pcie_chassis_find_slot(chassis, slot);
> + if (!s) {
> + monitor_printf(mon, "slot is not found. %s\n", pcie_slot);
> + return -1;
> + }
> + pcie_cap_slot_push_attention_button(&s->port.br.dev);
> + *ret_data = qobject_from_jsonf("{ 'chassis': %d, 'slot': %d}",
> + chassis, slot);
> + assert(*ret_data);
> + return 0;
> +}
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 2af3de6..965c754 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -1154,6 +1154,20 @@ Hot remove PCI device.
> ETEXI
>
> {
> + .name = "pcie_push_attention_button",
> + .args_type = "pcie_slot:s",
> + .params = "[<chassis>.]<slot>",
> + .help = "push pci express attention button",
> + .user_print = pcie_attention_button_push_print,
> + .mhandler.cmd_new = pcie_attention_button_push,
> + },
> +
> +STEXI
> +@item pcie_abp
> +Push PCI express attention button
> +ETEXI
> +
> + {
> .name = "host_net_add",
> .args_type = "device:s,opts:s?",
> .params = "tap|user|socket|vde|dump [options]",
> diff --git a/sysemu.h b/sysemu.h
> index 9c988bb..cca411d 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -150,6 +150,10 @@ extern unsigned int nb_prom_envs;
> void pci_device_hot_add(Monitor *mon, const QDict *qdict);
> void drive_hot_add(Monitor *mon, const QDict *qdict);
> void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
> +/* pcie hotplug */
> +void pcie_attention_button_push_print(Monitor *mon, const QObject *data);
> +int pcie_attention_button_push(Monitor *mon, const QDict *qdict,
> + QObject **ret_data);
>
> /* serial ports */
>
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-20 10:00 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-10-21 3:46 ` Isaku Yamahata
2010-10-21 8:02 ` Michael S. Tsirkin
2010-10-22 11:35 ` Markus Armbruster
1 sibling, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-21 3:46 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, lmr, etmartin, wexu2, qemu-devel, kraxel
On Wed, Oct 20, 2010 at 12:00:11PM +0200, Michael S. Tsirkin wrote:
> On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > glue pcie_push_attention_button command.
> >
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> So as a high level command, I think we need to
> think about how to tie this into pci_add/pci_del.
> Right?
Maybe or maybe not.
I'm not sure it's a good idea to tie it to pci_add/pci_del
in the first place.
The specification says only that pushing the button is just to
initiate the hot-plug operation. It means only a notification.
The spec says nothing about the concrete action from OS when
the button is pushed. It's up to OS.
For example.
OS may start to probe the slot.
OS may start to quiescence the device.
OS is allowed to ignore the notification.
OS may propagate the notification to the management software,
and it would pop up the dialog to the user for further operation.
> As a low level command, this is not really useful unless
> there is an event on LED status change and a way
> to get info on LED status.
> Right?
No. Guest OS can provide users those infos, and Linux does
via sysfs. So there already is a way to know LED status.
This is the reason why LED status change event stuff has low
priority in my TODO list.
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-21 3:46 ` Isaku Yamahata
@ 2010-10-21 8:02 ` Michael S. Tsirkin
2010-10-21 9:41 ` Isaku Yamahata
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-21 8:02 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, lmr, etmartin, wexu2, qemu-devel, kraxel
On Thu, Oct 21, 2010 at 12:46:33PM +0900, Isaku Yamahata wrote:
> On Wed, Oct 20, 2010 at 12:00:11PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > glue pcie_push_attention_button command.
> > >
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > So as a high level command, I think we need to
> > think about how to tie this into pci_add/pci_del.
> > Right?
>
> Maybe or maybe not.
> I'm not sure it's a good idea to tie it to pci_add/pci_del
> in the first place.
> The specification says only that pushing the button is just to
> initiate the hot-plug operation. It means only a notification.
> The spec says nothing about the concrete action from OS when
> the button is pushed. It's up to OS.
>
> For example.
> OS may start to probe the slot.
> OS may start to quiescence the device.
> OS is allowed to ignore the notification.
> OS may propagate the notification to the management software,
> and it would pop up the dialog to the user for further operation.
>
pci_add/pci_del really behave in the same way. There's no way to force
the OS to respond. We only use ACPI for now, but for express I expect
standard interfaces will work better long term.
> > As a low level command, this is not really useful unless
> > there is an event on LED status change and a way
> > to get info on LED status.
> > Right?
>
> No. Guest OS can provide users those infos, and Linux does
> via sysfs. So there already is a way to know LED status.
> This is the reason why LED status change event stuff has low
> priority in my TODO list.
But that's in the guest.
--
MST
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-21 8:02 ` Michael S. Tsirkin
@ 2010-10-21 9:41 ` Isaku Yamahata
0 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-21 9:41 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, lmr, etmartin, wexu2, qemu-devel, kraxel
On Thu, Oct 21, 2010 at 10:02:37AM +0200, Michael S. Tsirkin wrote:
> On Thu, Oct 21, 2010 at 12:46:33PM +0900, Isaku Yamahata wrote:
> > On Wed, Oct 20, 2010 at 12:00:11PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > > glue pcie_push_attention_button command.
> > > >
> > > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > >
> > > So as a high level command, I think we need to
> > > think about how to tie this into pci_add/pci_del.
> > > Right?
> >
> > Maybe or maybe not.
> > I'm not sure it's a good idea to tie it to pci_add/pci_del
> > in the first place.
> > The specification says only that pushing the button is just to
> > initiate the hot-plug operation. It means only a notification.
> > The spec says nothing about the concrete action from OS when
> > the button is pushed. It's up to OS.
> >
> > For example.
> > OS may start to probe the slot.
> > OS may start to quiescence the device.
> > OS is allowed to ignore the notification.
> > OS may propagate the notification to the management software,
> > and it would pop up the dialog to the user for further operation.
> >
>
> pci_add/pci_del really behave in the same way. There's no way to force
> the OS to respond. We only use ACPI for now, but for express I expect
> standard interfaces will work better long term.
The current qemu pci hot plug requires guest OS intervention as you said.
On the other hand, The express hot plug doesn't requires the intervention.
pci_add/pci_del for express simply populates/deletes PCIDvice
(+ interrupt). No guest OS intervention. It really corresponds to physical
insertion/removal of the pcie card into/from the slot.
Probably I don't understand what you mean by "tie it to pci_add/pci_del".
The command shouldn't be express specific, but more generic?
or something else?
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-20 10:00 ` [Qemu-devel] " Michael S. Tsirkin
2010-10-21 3:46 ` Isaku Yamahata
@ 2010-10-22 11:35 ` Markus Armbruster
2010-10-22 14:38 ` Michael S. Tsirkin
1 sibling, 1 reply; 40+ messages in thread
From: Markus Armbruster @ 2010-10-22 11:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, lmr, etmartin, wexu2, qemu-devel, Isaku Yamahata,
kraxel
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
>> glue pcie_push_attention_button command.
>>
>> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>
> So as a high level command, I think we need to
> think about how to tie this into pci_add/pci_del.
> Right?
[...]
Do we have consensus how our set of commands for hot plug should look
like? We talked about it, but did we reach consensus? If yes, did we
write it down somewhere?
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-22 11:35 ` Markus Armbruster
@ 2010-10-22 14:38 ` Michael S. Tsirkin
2010-10-22 14:52 ` Anthony Liguori
2010-10-25 3:29 ` Isaku Yamahata
0 siblings, 2 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-22 14:38 UTC (permalink / raw)
To: Markus Armbruster
Cc: skandasa, lmr, etmartin, wexu2, qemu-devel, Isaku Yamahata,
kraxel
On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
> > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> >> glue pcie_push_attention_button command.
> >>
> >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> > So as a high level command, I think we need to
> > think about how to tie this into pci_add/pci_del.
> > Right?
> [...]
>
> Do we have consensus how our set of commands for hot plug should look
> like? We talked about it, but did we reach consensus? If yes, did we
> write it down somewhere?
I think for simple things yes:
- command to send hotplug notification to the guest
- command to immediately add/remove the device
- event to notify about guest ack
- way to poll status: did guest ack last command?
Existing ones will keep function:
- send notification and when acked remove device
- add device and send notification
These are useful for human monitor but maybe not
for management.
--
MST
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-22 14:38 ` Michael S. Tsirkin
@ 2010-10-22 14:52 ` Anthony Liguori
2010-10-25 4:16 ` Michael S. Tsirkin
2010-10-25 3:29 ` Isaku Yamahata
1 sibling, 1 reply; 40+ messages in thread
From: Anthony Liguori @ 2010-10-22 14:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
Isaku Yamahata, kraxel
On 10/22/2010 09:38 AM, Michael S. Tsirkin wrote:
> On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
>
>> "Michael S. Tsirkin"<mst@redhat.com> writes:
>>
>>
>>> On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
>>>
>>>> glue pcie_push_attention_button command.
>>>>
>>>> Signed-off-by: Isaku Yamahata<yamahata@valinux.co.jp>
>>>>
>>> So as a high level command, I think we need to
>>> think about how to tie this into pci_add/pci_del.
>>> Right?
>>>
>> [...]
>>
>> Do we have consensus how our set of commands for hot plug should look
>> like? We talked about it, but did we reach consensus? If yes, did we
>> write it down somewhere?
>>
> I think for simple things yes:
> - command to send hotplug notification to the guest
> - command to immediately add/remove the device
> - event to notify about guest ack
>
Is it a guest ack? I thought it was actually an eject that can be
initiated without the notification being sent to the guest.
If so, polling doesn't really make much sense.
Regards,
Anthony Liguori
> - way to poll status: did guest ack last command?
>
> Existing ones will keep function:
> - send notification and when acked remove device
> - add device and send notification
> These are useful for human monitor but maybe not
> for management.
>
>
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-22 14:52 ` Anthony Liguori
@ 2010-10-25 4:16 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-25 4:16 UTC (permalink / raw)
To: Anthony Liguori
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
Isaku Yamahata, kraxel
On Fri, Oct 22, 2010 at 09:52:19AM -0500, Anthony Liguori wrote:
> On 10/22/2010 09:38 AM, Michael S. Tsirkin wrote:
> >On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> >>"Michael S. Tsirkin"<mst@redhat.com> writes:
> >>
> >>>On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> >>>>glue pcie_push_attention_button command.
> >>>>
> >>>>Signed-off-by: Isaku Yamahata<yamahata@valinux.co.jp>
> >>>So as a high level command, I think we need to
> >>>think about how to tie this into pci_add/pci_del.
> >>>Right?
> >>[...]
> >>
> >>Do we have consensus how our set of commands for hot plug should look
> >>like? We talked about it, but did we reach consensus? If yes, did we
> >>write it down somewhere?
> >I think for simple things yes:
> >- command to send hotplug notification to the guest
> >- command to immediately add/remove the device
> >- event to notify about guest ack
>
> Is it a guest ack? I thought it was actually an eject that can be
> initiated without the notification being sent to the guest.
Yes.
> If so, polling doesn't really make much sense.
Who said something about polling?
> Regards,
>
> Anthony Liguori
>
> >- way to poll status: did guest ack last command?
> >
> >Existing ones will keep function:
> >- send notification and when acked remove device
> >- add device and send notification
> >These are useful for human monitor but maybe not
> >for management.
> >
> >
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-22 14:38 ` Michael S. Tsirkin
2010-10-22 14:52 ` Anthony Liguori
@ 2010-10-25 3:29 ` Isaku Yamahata
2010-10-25 4:15 ` Michael S. Tsirkin
1 sibling, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-25 3:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Fri, Oct 22, 2010 at 04:38:49PM +0200, Michael S. Tsirkin wrote:
> On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> >
> > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > >> glue pcie_push_attention_button command.
> > >>
> > >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > >
> > > So as a high level command, I think we need to
> > > think about how to tie this into pci_add/pci_del.
> > > Right?
> > [...]
> >
> > Do we have consensus how our set of commands for hot plug should look
> > like? We talked about it, but did we reach consensus? If yes, did we
> > write it down somewhere?
>
> I think for simple things yes:
> - command to send hotplug notification to the guest
> - command to immediately add/remove the device
> - event to notify about guest ack
> - way to poll status: did guest ack last command?
I'm not sure about guest ack. Let me check my understanding.
The current qemu pci hot plug has its own hot plug controler,
PIIX4_PM, which relies on ACPI.
- command to add the device into the slot
This corresponds to physically inserting the device into the slot.
- qemu pci hot plug case: device_add/pci_add command.
The qemu pci hot plug controller, PIIX4_PM, detects the insertion,
then notify the guest OS of the event via ACPI, _L01.
The guest OS would start to probe the device.
- pci express native hot plug case: device_add/pci_add command.
The pcie hot plug controller detects the the insertion,
then notify the guest OS of the event via interrupt.
The guest OS would start to probe the device.
- command to remove the device from the slot
This corresponds to physically removing the device from the slot.
- qemu pci hot plug case: No corresponding command.
There is no way to remove the pci card forcibly from the slot.
- pci express native hot plug case: device_del/pci_del
After the removal of the card, the hot plug controller notifies
the guest OS via interrupt.
- command to send hotplug notification to the guest
command to push attention button.
This corresponds to pushing the button near the slot.
- qemu pci hot plug case: device_del/pci_del command
Maybe the button is called an eject button.
When the button is pushed, the hot plug controller notifies
the guest OS via ACPI, _L01.
Then, guest OS reacts the event by calling ACPI \_SB.PCI0.S<n>._EJ0
method. It program the hot plug controller to eject the device
in the given slot. As a result, the device is removed from the slot.
If the guest OS doesn't call _EJ0 (nor programs the controller directly),
the device stays there.
There is no way to remove the pci card without the guest OS
intervention.
- pci express native hot plug case:
pcie_push_attention_button command with my patch series.
The hot plug controller raise the interrupt to the guest OS.
There is no specified action from the OS.
- a way to get the slot status
new command for QMP/HMP? or enhance info pci?
- QMP event for qemu to notify the slot status change
e.g. when LED status is changed, qmp event will be sent.
> Existing ones will keep function:
> - send notification and when acked remove device
> - add device and send notification
> These are useful for human monitor but maybe not
> for management.
>
>
> --
> MST
>
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-25 3:29 ` Isaku Yamahata
@ 2010-10-25 4:15 ` Michael S. Tsirkin
2010-10-25 5:53 ` Isaku Yamahata
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-25 4:15 UTC (permalink / raw)
To: Isaku Yamahata
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Mon, Oct 25, 2010 at 12:29:57PM +0900, Isaku Yamahata wrote:
> On Fri, Oct 22, 2010 at 04:38:49PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > >
> > > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > >> glue pcie_push_attention_button command.
> > > >>
> > > >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > > >
> > > > So as a high level command, I think we need to
> > > > think about how to tie this into pci_add/pci_del.
> > > > Right?
> > > [...]
> > >
> > > Do we have consensus how our set of commands for hot plug should look
> > > like? We talked about it, but did we reach consensus? If yes, did we
> > > write it down somewhere?
> >
> > I think for simple things yes:
> > - command to send hotplug notification to the guest
> > - command to immediately add/remove the device
> > - event to notify about guest ack
> > - way to poll status: did guest ack last command?
>
> I'm not sure about guest ack. Let me check my understanding.
Your understanding is correct.
> The current qemu pci hot plug has its own hot plug controler,
> PIIX4_PM, which relies on ACPI.
>
> - command to add the device into the slot
> This corresponds to physically inserting the device into the slot.
> - qemu pci hot plug case: device_add/pci_add command.
> The qemu pci hot plug controller, PIIX4_PM, detects the insertion,
> then notify the guest OS of the event via ACPI, _L01.
> The guest OS would start to probe the device.
>
> - pci express native hot plug case: device_add/pci_add command.
> The pcie hot plug controller detects the the insertion,
> then notify the guest OS of the event via interrupt.
> The guest OS would start to probe the device.
>
> - command to remove the device from the slot
> This corresponds to physically removing the device from the slot.
> - qemu pci hot plug case: No corresponding command.
> There is no way to remove the pci card forcibly from the slot.
>
> - pci express native hot plug case: device_del/pci_del
> After the removal of the card, the hot plug controller notifies
> the guest OS via interrupt.
>
> - command to send hotplug notification to the guest
> command to push attention button.
> This corresponds to pushing the button near the slot.
> - qemu pci hot plug case: device_del/pci_del command
> Maybe the button is called an eject button.
> When the button is pushed, the hot plug controller notifies
> the guest OS via ACPI, _L01.
> Then, guest OS reacts the event by calling ACPI \_SB.PCI0.S<n>._EJ0
> method. It program the hot plug controller to eject the device
> in the given slot. As a result, the device is removed from the slot.
> If the guest OS doesn't call _EJ0 (nor programs the controller directly),
> the device stays there.
> There is no way to remove the pci card without the guest OS
> intervention.
>
> - pci express native hot plug case:
> pcie_push_attention_button command with my patch series.
> The hot plug controller raise the interrupt to the guest OS.
> There is no specified action from the OS.
Right. So what is suggested is that we have
A. a single command to push the attention button
B. a single command to remove pci card without guest OS interaction
C. a way to interact with the guest
management tools or human monitor will be able to tie A,B,C together
in various interesting ways
What I am also saying is that the same command should be able
to work for pci and express I think.
> - a way to get the slot status
> new command for QMP/HMP? or enhance info pci?
>
> - QMP event for qemu to notify the slot status change
> e.g. when LED status is changed, qmp event will be sent.
>
> > Existing ones will keep function:
> > - send notification and when acked remove device
> > - add device and send notification
> > These are useful for human monitor but maybe not
> > for management.
> >
> >
> > --
> > MST
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-25 4:15 ` Michael S. Tsirkin
@ 2010-10-25 5:53 ` Isaku Yamahata
2010-10-25 5:55 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-25 5:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Mon, Oct 25, 2010 at 06:15:37AM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 25, 2010 at 12:29:57PM +0900, Isaku Yamahata wrote:
> > On Fri, Oct 22, 2010 at 04:38:49PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > >
> > > > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > > >> glue pcie_push_attention_button command.
> > > > >>
> > > > >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > > > >
> > > > > So as a high level command, I think we need to
> > > > > think about how to tie this into pci_add/pci_del.
> > > > > Right?
> > > > [...]
> > > >
> > > > Do we have consensus how our set of commands for hot plug should look
> > > > like? We talked about it, but did we reach consensus? If yes, did we
> > > > write it down somewhere?
> > >
> > > I think for simple things yes:
> > > - command to send hotplug notification to the guest
> > > - command to immediately add/remove the device
> > > - event to notify about guest ack
> > > - way to poll status: did guest ack last command?
> >
> > I'm not sure about guest ack. Let me check my understanding.
>
>
> Your understanding is correct.
>
> > The current qemu pci hot plug has its own hot plug controler,
> > PIIX4_PM, which relies on ACPI.
> >
> > - command to add the device into the slot
> > This corresponds to physically inserting the device into the slot.
> > - qemu pci hot plug case: device_add/pci_add command.
> > The qemu pci hot plug controller, PIIX4_PM, detects the insertion,
> > then notify the guest OS of the event via ACPI, _L01.
> > The guest OS would start to probe the device.
> >
> > - pci express native hot plug case: device_add/pci_add command.
> > The pcie hot plug controller detects the the insertion,
> > then notify the guest OS of the event via interrupt.
> > The guest OS would start to probe the device.
> >
> > - command to remove the device from the slot
> > This corresponds to physically removing the device from the slot.
> > - qemu pci hot plug case: No corresponding command.
> > There is no way to remove the pci card forcibly from the slot.
> >
> > - pci express native hot plug case: device_del/pci_del
> > After the removal of the card, the hot plug controller notifies
> > the guest OS via interrupt.
> >
> > - command to send hotplug notification to the guest
> > command to push attention button.
> > This corresponds to pushing the button near the slot.
> > - qemu pci hot plug case: device_del/pci_del command
> > Maybe the button is called an eject button.
> > When the button is pushed, the hot plug controller notifies
> > the guest OS via ACPI, _L01.
> > Then, guest OS reacts the event by calling ACPI \_SB.PCI0.S<n>._EJ0
> > method. It program the hot plug controller to eject the device
> > in the given slot. As a result, the device is removed from the slot.
> > If the guest OS doesn't call _EJ0 (nor programs the controller directly),
> > the device stays there.
> > There is no way to remove the pci card without the guest OS
> > intervention.
> >
> > - pci express native hot plug case:
> > pcie_push_attention_button command with my patch series.
> > The hot plug controller raise the interrupt to the guest OS.
> > There is no specified action from the OS.
>
> Right. So what is suggested is that we have
> A. a single command to push the attention button
> B. a single command to remove pci card without guest OS interaction
> C. a way to interact with the guest
> management tools or human monitor will be able to tie A,B,C together
> in various interesting ways
>
Makes sense. How about the following?
For A. => pci_push_attention_button command
qemu pci hot plug case: device_del/pci_del is internally aliased to
this command for compatibility.
pci express native hot plug case: this command is exposed to
the user as is.
For B. => pci_unplug command (or other better name?)
qemu pci hot plug case: -ENOSYS. (until someone implements it.)
pci express native hot plug case: device_del/pci_del is internally
aliased to this command.
For C. => I'm not sure what kind of command is wanted. Any comments?
- new QMP/HMP command
enhance info pci command? or info pci-slot command?
Probably we want to allow hot-plug controller specific infos.
- new QMP event
- LED change status
power led: on, off, blinking
attention led: on, off, blinking
- slot status change?
card: inserted, removed
electromechanical lock: engaged, un-engaged
Probably we want to allow events specific to the hot-plug controller.
- any other?
> What I am also saying is that the same command should be able
> to work for pci and express I think.
I see. Then, I think that the slot numbering needs to be discussed.
More concretely, it's what type of argument is passed for push attention
button command.
Here the slot number in general means that the number that is printed near
the physical slot. Not pci (segment, bus, device) triplets.
The current qemu pci hot plug abuses the triplets as the slot number
with the assumption of (segment = 0, bus = 0, device = slot number).
On the other hand, pcie hot plug has its own scheme to number the slots.
The correspondence between the triplets and the slot is provided
to the OS. (The standard pci hot plug has its own too which is different
from the pcie one.)
So users may want to operate with the slot number.
User's opinions are needed. Any comments?
thanks,
> > - a way to get the slot status
> > new command for QMP/HMP? or enhance info pci?
> >
> > - QMP event for qemu to notify the slot status change
> > e.g. when LED status is changed, qmp event will be sent.
> >
> > > Existing ones will keep function:
> > > - send notification and when acked remove device
> > > - add device and send notification
> > > These are useful for human monitor but maybe not
> > > for management.
> > >
> > >
> > > --
> > > MST
> > >
> >
> > --
> > yamahata
>
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-25 5:53 ` Isaku Yamahata
@ 2010-10-25 5:55 ` Michael S. Tsirkin
2010-10-25 7:02 ` Isaku Yamahata
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-25 5:55 UTC (permalink / raw)
To: Isaku Yamahata
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Mon, Oct 25, 2010 at 02:53:16PM +0900, Isaku Yamahata wrote:
> On Mon, Oct 25, 2010 at 06:15:37AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Oct 25, 2010 at 12:29:57PM +0900, Isaku Yamahata wrote:
> > > On Fri, Oct 22, 2010 at 04:38:49PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> > > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > >
> > > > > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > > > >> glue pcie_push_attention_button command.
> > > > > >>
> > > > > >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > > > > >
> > > > > > So as a high level command, I think we need to
> > > > > > think about how to tie this into pci_add/pci_del.
> > > > > > Right?
> > > > > [...]
> > > > >
> > > > > Do we have consensus how our set of commands for hot plug should look
> > > > > like? We talked about it, but did we reach consensus? If yes, did we
> > > > > write it down somewhere?
> > > >
> > > > I think for simple things yes:
> > > > - command to send hotplug notification to the guest
> > > > - command to immediately add/remove the device
> > > > - event to notify about guest ack
> > > > - way to poll status: did guest ack last command?
> > >
> > > I'm not sure about guest ack. Let me check my understanding.
> >
> >
> > Your understanding is correct.
> >
> > > The current qemu pci hot plug has its own hot plug controler,
> > > PIIX4_PM, which relies on ACPI.
> > >
> > > - command to add the device into the slot
> > > This corresponds to physically inserting the device into the slot.
> > > - qemu pci hot plug case: device_add/pci_add command.
> > > The qemu pci hot plug controller, PIIX4_PM, detects the insertion,
> > > then notify the guest OS of the event via ACPI, _L01.
> > > The guest OS would start to probe the device.
> > >
> > > - pci express native hot plug case: device_add/pci_add command.
> > > The pcie hot plug controller detects the the insertion,
> > > then notify the guest OS of the event via interrupt.
> > > The guest OS would start to probe the device.
> > >
> > > - command to remove the device from the slot
> > > This corresponds to physically removing the device from the slot.
> > > - qemu pci hot plug case: No corresponding command.
> > > There is no way to remove the pci card forcibly from the slot.
> > >
> > > - pci express native hot plug case: device_del/pci_del
> > > After the removal of the card, the hot plug controller notifies
> > > the guest OS via interrupt.
> > >
> > > - command to send hotplug notification to the guest
> > > command to push attention button.
> > > This corresponds to pushing the button near the slot.
> > > - qemu pci hot plug case: device_del/pci_del command
> > > Maybe the button is called an eject button.
> > > When the button is pushed, the hot plug controller notifies
> > > the guest OS via ACPI, _L01.
> > > Then, guest OS reacts the event by calling ACPI \_SB.PCI0.S<n>._EJ0
> > > method. It program the hot plug controller to eject the device
> > > in the given slot. As a result, the device is removed from the slot.
> > > If the guest OS doesn't call _EJ0 (nor programs the controller directly),
> > > the device stays there.
> > > There is no way to remove the pci card without the guest OS
> > > intervention.
> > >
> > > - pci express native hot plug case:
> > > pcie_push_attention_button command with my patch series.
> > > The hot plug controller raise the interrupt to the guest OS.
> > > There is no specified action from the OS.
> >
> > Right. So what is suggested is that we have
> > A. a single command to push the attention button
> > B. a single command to remove pci card without guest OS interaction
> > C. a way to interact with the guest
> > management tools or human monitor will be able to tie A,B,C together
> > in various interesting ways
> >
> Makes sense. How about the following?
>
> For A. => pci_push_attention_button command
> qemu pci hot plug case: device_del/pci_del is internally aliased to
> this command for compatibility.
> pci express native hot plug case: this command is exposed to
> the user as is.
>
> For B. => pci_unplug command (or other better name?)
> qemu pci hot plug case: -ENOSYS. (until someone implements it.)
> pci express native hot plug case: device_del/pci_del is internally
> aliased to this command.
>
> For C. => I'm not sure what kind of command is wanted. Any comments?
> - new QMP/HMP command
> enhance info pci command? or info pci-slot command?
> Probably we want to allow hot-plug controller specific infos.
Yes. It probably does not matter which command this is.
>
> - new QMP event
> - LED change status
> power led: on, off, blinking
> attention led: on, off, blinking
> - slot status change?
> card: inserted, removed
> electromechanical lock: engaged, un-engaged
> Probably we want to allow events specific to the hot-plug controller.
>
> - any other?
I am really interested in high level QMP event that
tells us guest ejected the device.
This should work for pci and for express.
Low level stuff for led status etc is noce for a debugging,
not really sure about value for users.
>
>
> > What I am also saying is that the same command should be able
> > to work for pci and express I think.
>
> I see. Then, I think that the slot numbering needs to be discussed.
Yes.
> More concretely, it's what type of argument is passed for push attention
> button command.
> Here the slot number in general means that the number that is printed near
> the physical slot. Not pci (segment, bus, device) triplets.
>
> The current qemu pci hot plug abuses the triplets as the slot number
> with the assumption of (segment = 0, bus = 0, device = slot number).
> On the other hand, pcie hot plug has its own scheme to number the slots.
> The correspondence between the triplets and the slot is provided
> to the OS. (The standard pci hot plug has its own too which is different
> from the pcie one.)
> So users may want to operate with the slot number.
> User's opinions are needed. Any comments?
>
> thanks,
Can we just use the topological address everywhere?
Bus numbers are guest assigned, so we can not always
rely on them. They are nice for human monitor
though and should be way to get from them to
topology and back.
And for bus 0 they are equivalent.
> > > - a way to get the slot status
> > > new command for QMP/HMP? or enhance info pci?
> > >
> > > - QMP event for qemu to notify the slot status change
> > > e.g. when LED status is changed, qmp event will be sent.
> > >
> > > > Existing ones will keep function:
> > > > - send notification and when acked remove device
> > > > - add device and send notification
> > > > These are useful for human monitor but maybe not
> > > > for management.
> > > >
> > > >
> > > > --
> > > > MST
> > > >
> > >
> > > --
> > > yamahata
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-25 5:55 ` Michael S. Tsirkin
@ 2010-10-25 7:02 ` Isaku Yamahata
2010-10-25 7:27 ` Michael S. Tsirkin
2010-10-27 1:47 ` Isaku Yamahata
0 siblings, 2 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-25 7:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Mon, Oct 25, 2010 at 07:55:57AM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 25, 2010 at 02:53:16PM +0900, Isaku Yamahata wrote:
> > On Mon, Oct 25, 2010 at 06:15:37AM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Oct 25, 2010 at 12:29:57PM +0900, Isaku Yamahata wrote:
> > > > On Fri, Oct 22, 2010 at 04:38:49PM +0200, Michael S. Tsirkin wrote:
> > > > > On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> > > > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > >
> > > > > > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > > > > >> glue pcie_push_attention_button command.
> > > > > > >>
> > > > > > >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > > > > > >
> > > > > > > So as a high level command, I think we need to
> > > > > > > think about how to tie this into pci_add/pci_del.
> > > > > > > Right?
> > > > > > [...]
> > > > > >
> > > > > > Do we have consensus how our set of commands for hot plug should look
> > > > > > like? We talked about it, but did we reach consensus? If yes, did we
> > > > > > write it down somewhere?
> > > > >
> > > > > I think for simple things yes:
> > > > > - command to send hotplug notification to the guest
> > > > > - command to immediately add/remove the device
> > > > > - event to notify about guest ack
> > > > > - way to poll status: did guest ack last command?
> > > >
> > > > I'm not sure about guest ack. Let me check my understanding.
> > >
> > >
> > > Your understanding is correct.
> > >
> > > > The current qemu pci hot plug has its own hot plug controler,
> > > > PIIX4_PM, which relies on ACPI.
> > > >
> > > > - command to add the device into the slot
> > > > This corresponds to physically inserting the device into the slot.
> > > > - qemu pci hot plug case: device_add/pci_add command.
> > > > The qemu pci hot plug controller, PIIX4_PM, detects the insertion,
> > > > then notify the guest OS of the event via ACPI, _L01.
> > > > The guest OS would start to probe the device.
> > > >
> > > > - pci express native hot plug case: device_add/pci_add command.
> > > > The pcie hot plug controller detects the the insertion,
> > > > then notify the guest OS of the event via interrupt.
> > > > The guest OS would start to probe the device.
> > > >
> > > > - command to remove the device from the slot
> > > > This corresponds to physically removing the device from the slot.
> > > > - qemu pci hot plug case: No corresponding command.
> > > > There is no way to remove the pci card forcibly from the slot.
> > > >
> > > > - pci express native hot plug case: device_del/pci_del
> > > > After the removal of the card, the hot plug controller notifies
> > > > the guest OS via interrupt.
> > > >
> > > > - command to send hotplug notification to the guest
> > > > command to push attention button.
> > > > This corresponds to pushing the button near the slot.
> > > > - qemu pci hot plug case: device_del/pci_del command
> > > > Maybe the button is called an eject button.
> > > > When the button is pushed, the hot plug controller notifies
> > > > the guest OS via ACPI, _L01.
> > > > Then, guest OS reacts the event by calling ACPI \_SB.PCI0.S<n>._EJ0
> > > > method. It program the hot plug controller to eject the device
> > > > in the given slot. As a result, the device is removed from the slot.
> > > > If the guest OS doesn't call _EJ0 (nor programs the controller directly),
> > > > the device stays there.
> > > > There is no way to remove the pci card without the guest OS
> > > > intervention.
> > > >
> > > > - pci express native hot plug case:
> > > > pcie_push_attention_button command with my patch series.
> > > > The hot plug controller raise the interrupt to the guest OS.
> > > > There is no specified action from the OS.
> > >
> > > Right. So what is suggested is that we have
> > > A. a single command to push the attention button
> > > B. a single command to remove pci card without guest OS interaction
> > > C. a way to interact with the guest
> > > management tools or human monitor will be able to tie A,B,C together
> > > in various interesting ways
> > >
> > Makes sense. How about the following?
> >
> > For A. => pci_push_attention_button command
> > qemu pci hot plug case: device_del/pci_del is internally aliased to
> > this command for compatibility.
> > pci express native hot plug case: this command is exposed to
> > the user as is.
> >
> > For B. => pci_unplug command (or other better name?)
> > qemu pci hot plug case: -ENOSYS. (until someone implements it.)
> > pci express native hot plug case: device_del/pci_del is internally
> > aliased to this command.
> >
> > For C. => I'm not sure what kind of command is wanted. Any comments?
> > - new QMP/HMP command
> > enhance info pci command? or info pci-slot command?
> > Probably we want to allow hot-plug controller specific infos.
>
> Yes. It probably does not matter which command this is.
>
> >
> > - new QMP event
> > - LED change status
> > power led: on, off, blinking
> > attention led: on, off, blinking
> > - slot status change?
> > card: inserted, removed
> > electromechanical lock: engaged, un-engaged
> > Probably we want to allow events specific to the hot-plug controller.
> >
> > - any other?
>
>
> I am really interested in high level QMP event that
> tells us guest ejected the device.
> This should work for pci and for express.
Express doesn't have a way for OS to eject the device.
Neither Standard pci hot plug.
So ejecting the device would be a event specific to qemu pci hot plug.
> Low level stuff for led status etc is noce for a debugging,
> not really sure about value for users.
>
> >
> >
> > > What I am also saying is that the same command should be able
> > > to work for pci and express I think.
> >
> > I see. Then, I think that the slot numbering needs to be discussed.
>
> Yes.
>
> > More concretely, it's what type of argument is passed for push attention
> > button command.
> > Here the slot number in general means that the number that is printed near
> > the physical slot. Not pci (segment, bus, device) triplets.
> >
> > The current qemu pci hot plug abuses the triplets as the slot number
> > with the assumption of (segment = 0, bus = 0, device = slot number).
> > On the other hand, pcie hot plug has its own scheme to number the slots.
> > The correspondence between the triplets and the slot is provided
> > to the OS. (The standard pci hot plug has its own too which is different
> > from the pcie one.)
> > So users may want to operate with the slot number.
> > User's opinions are needed. Any comments?
> >
> > thanks,
>
> Can we just use the topological address everywhere?
> Bus numbers are guest assigned, so we can not always
> rely on them. They are nice for human monitor
> though and should be way to get from them to
> topology and back.
> And for bus 0 they are equivalent.
Yes, with some kind of conversion.
We could go with whichever addressing with (slight?) effort
to implement conversion.
The point is what users(libvirt? virt-manager?) want.
thanks,
> > > > - a way to get the slot status
> > > > new command for QMP/HMP? or enhance info pci?
> > > >
> > > > - QMP event for qemu to notify the slot status change
> > > > e.g. when LED status is changed, qmp event will be sent.
> > > >
> > > > > Existing ones will keep function:
> > > > > - send notification and when acked remove device
> > > > > - add device and send notification
> > > > > These are useful for human monitor but maybe not
> > > > > for management.
> > > > >
> > > > >
> > > > > --
> > > > > MST
> > > > >
> > > >
> > > > --
> > > > yamahata
> > >
> >
> > --
> > yamahata
>
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-25 7:02 ` Isaku Yamahata
@ 2010-10-25 7:27 ` Michael S. Tsirkin
2010-10-27 1:47 ` Isaku Yamahata
1 sibling, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-25 7:27 UTC (permalink / raw)
To: Isaku Yamahata
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Mon, Oct 25, 2010 at 04:02:36PM +0900, Isaku Yamahata wrote:
> On Mon, Oct 25, 2010 at 07:55:57AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Oct 25, 2010 at 02:53:16PM +0900, Isaku Yamahata wrote:
> > > On Mon, Oct 25, 2010 at 06:15:37AM +0200, Michael S. Tsirkin wrote:
> > > > On Mon, Oct 25, 2010 at 12:29:57PM +0900, Isaku Yamahata wrote:
> > > > > On Fri, Oct 22, 2010 at 04:38:49PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Fri, Oct 22, 2010 at 01:35:47PM +0200, Markus Armbruster wrote:
> > > > > > > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > > > > > >
> > > > > > > > On Wed, Oct 20, 2010 at 05:18:56PM +0900, Isaku Yamahata wrote:
> > > > > > > >> glue pcie_push_attention_button command.
> > > > > > > >>
> > > > > > > >> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > > > > > > >
> > > > > > > > So as a high level command, I think we need to
> > > > > > > > think about how to tie this into pci_add/pci_del.
> > > > > > > > Right?
> > > > > > > [...]
> > > > > > >
> > > > > > > Do we have consensus how our set of commands for hot plug should look
> > > > > > > like? We talked about it, but did we reach consensus? If yes, did we
> > > > > > > write it down somewhere?
> > > > > >
> > > > > > I think for simple things yes:
> > > > > > - command to send hotplug notification to the guest
> > > > > > - command to immediately add/remove the device
> > > > > > - event to notify about guest ack
> > > > > > - way to poll status: did guest ack last command?
> > > > >
> > > > > I'm not sure about guest ack. Let me check my understanding.
> > > >
> > > >
> > > > Your understanding is correct.
> > > >
> > > > > The current qemu pci hot plug has its own hot plug controler,
> > > > > PIIX4_PM, which relies on ACPI.
> > > > >
> > > > > - command to add the device into the slot
> > > > > This corresponds to physically inserting the device into the slot.
> > > > > - qemu pci hot plug case: device_add/pci_add command.
> > > > > The qemu pci hot plug controller, PIIX4_PM, detects the insertion,
> > > > > then notify the guest OS of the event via ACPI, _L01.
> > > > > The guest OS would start to probe the device.
> > > > >
> > > > > - pci express native hot plug case: device_add/pci_add command.
> > > > > The pcie hot plug controller detects the the insertion,
> > > > > then notify the guest OS of the event via interrupt.
> > > > > The guest OS would start to probe the device.
> > > > >
> > > > > - command to remove the device from the slot
> > > > > This corresponds to physically removing the device from the slot.
> > > > > - qemu pci hot plug case: No corresponding command.
> > > > > There is no way to remove the pci card forcibly from the slot.
> > > > >
> > > > > - pci express native hot plug case: device_del/pci_del
> > > > > After the removal of the card, the hot plug controller notifies
> > > > > the guest OS via interrupt.
> > > > >
> > > > > - command to send hotplug notification to the guest
> > > > > command to push attention button.
> > > > > This corresponds to pushing the button near the slot.
> > > > > - qemu pci hot plug case: device_del/pci_del command
> > > > > Maybe the button is called an eject button.
> > > > > When the button is pushed, the hot plug controller notifies
> > > > > the guest OS via ACPI, _L01.
> > > > > Then, guest OS reacts the event by calling ACPI \_SB.PCI0.S<n>._EJ0
> > > > > method. It program the hot plug controller to eject the device
> > > > > in the given slot. As a result, the device is removed from the slot.
> > > > > If the guest OS doesn't call _EJ0 (nor programs the controller directly),
> > > > > the device stays there.
> > > > > There is no way to remove the pci card without the guest OS
> > > > > intervention.
> > > > >
> > > > > - pci express native hot plug case:
> > > > > pcie_push_attention_button command with my patch series.
> > > > > The hot plug controller raise the interrupt to the guest OS.
> > > > > There is no specified action from the OS.
> > > >
> > > > Right. So what is suggested is that we have
> > > > A. a single command to push the attention button
> > > > B. a single command to remove pci card without guest OS interaction
> > > > C. a way to interact with the guest
> > > > management tools or human monitor will be able to tie A,B,C together
> > > > in various interesting ways
> > > >
> > > Makes sense. How about the following?
> > >
> > > For A. => pci_push_attention_button command
> > > qemu pci hot plug case: device_del/pci_del is internally aliased to
> > > this command for compatibility.
> > > pci express native hot plug case: this command is exposed to
> > > the user as is.
> > >
> > > For B. => pci_unplug command (or other better name?)
> > > qemu pci hot plug case: -ENOSYS. (until someone implements it.)
> > > pci express native hot plug case: device_del/pci_del is internally
> > > aliased to this command.
> > >
> > > For C. => I'm not sure what kind of command is wanted. Any comments?
> > > - new QMP/HMP command
> > > enhance info pci command? or info pci-slot command?
> > > Probably we want to allow hot-plug controller specific infos.
> >
> > Yes. It probably does not matter which command this is.
> >
> > >
> > > - new QMP event
> > > - LED change status
> > > power led: on, off, blinking
> > > attention led: on, off, blinking
> > > - slot status change?
> > > card: inserted, removed
> > > electromechanical lock: engaged, un-engaged
> > > Probably we want to allow events specific to the hot-plug controller.
> > >
> > > - any other?
> >
> >
> > I am really interested in high level QMP event that
> > tells us guest ejected the device.
> > This should work for pci and for express.
>
> Express doesn't have a way for OS to eject the device.
> Neither Standard pci hot plug.
> So ejecting the device would be a event specific to qemu pci hot plug.
Isn't this what LED is used for?
> > Low level stuff for led status etc is noce for a debugging,
> > not really sure about value for users.
> >
> > >
> > >
> > > > What I am also saying is that the same command should be able
> > > > to work for pci and express I think.
> > >
> > > I see. Then, I think that the slot numbering needs to be discussed.
> >
> > Yes.
> >
> > > More concretely, it's what type of argument is passed for push attention
> > > button command.
> > > Here the slot number in general means that the number that is printed near
> > > the physical slot. Not pci (segment, bus, device) triplets.
> > >
> > > The current qemu pci hot plug abuses the triplets as the slot number
> > > with the assumption of (segment = 0, bus = 0, device = slot number).
> > > On the other hand, pcie hot plug has its own scheme to number the slots.
> > > The correspondence between the triplets and the slot is provided
> > > to the OS. (The standard pci hot plug has its own too which is different
> > > from the pcie one.)
> > > So users may want to operate with the slot number.
> > > User's opinions are needed. Any comments?
> > >
> > > thanks,
> >
> > Can we just use the topological address everywhere?
> > Bus numbers are guest assigned, so we can not always
> > rely on them. They are nice for human monitor
> > though and should be way to get from them to
> > topology and back.
> > And for bus 0 they are equivalent.
>
> Yes, with some kind of conversion.
> We could go with whichever addressing with (slight?) effort
> to implement conversion.
> The point is what users(libvirt? virt-manager?) want.
>
> thanks,
>
> > > > > - a way to get the slot status
> > > > > new command for QMP/HMP? or enhance info pci?
> > > > >
> > > > > - QMP event for qemu to notify the slot status change
> > > > > e.g. when LED status is changed, qmp event will be sent.
> > > > >
> > > > > > Existing ones will keep function:
> > > > > > - send notification and when acked remove device
> > > > > > - add device and send notification
> > > > > > These are useful for human monitor but maybe not
> > > > > > for management.
> > > > > >
> > > > > >
> > > > > > --
> > > > > > MST
> > > > > >
> > > > >
> > > > > --
> > > > > yamahata
> > > >
> > >
> > > --
> > > yamahata
> >
>
> --
> yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-25 7:02 ` Isaku Yamahata
2010-10-25 7:27 ` Michael S. Tsirkin
@ 2010-10-27 1:47 ` Isaku Yamahata
2010-10-27 13:31 ` Michael S. Tsirkin
1 sibling, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-27 1:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Mon, Oct 25, 2010 at 04:02:36PM +0900, Isaku Yamahata wrote:
> > > > What I am also saying is that the same command should be able
> > > > to work for pci and express I think.
> > >
> > > I see. Then, I think that the slot numbering needs to be discussed.
> >
> > Yes.
> >
> > > More concretely, it's what type of argument is passed for push attention
> > > button command.
> > > Here the slot number in general means that the number that is printed near
> > > the physical slot. Not pci (segment, bus, device) triplets.
> > >
> > > The current qemu pci hot plug abuses the triplets as the slot number
> > > with the assumption of (segment = 0, bus = 0, device = slot number).
> > > On the other hand, pcie hot plug has its own scheme to number the slots.
> > > The correspondence between the triplets and the slot is provided
> > > to the OS. (The standard pci hot plug has its own too which is different
> > > from the pcie one.)
> > > So users may want to operate with the slot number.
> > > User's opinions are needed. Any comments?
> > >
> > > thanks,
> >
> > Can we just use the topological address everywhere?
> > Bus numbers are guest assigned, so we can not always
> > rely on them. They are nice for human monitor
> > though and should be way to get from them to
> > topology and back.
> > And for bus 0 they are equivalent.
>
> Yes, with some kind of conversion.
> We could go with whichever addressing with (slight?) effort
> to implement conversion.
> The point is what users(libvirt? virt-manager?) want.
When pushing the attention button, we are talking to the hot plug
controller, not the device in the slot.
The button can be pushed even when the slot is empty.
In qemu pci hot plug case the button is used only for device
ejection, so (segment, bus, device) triplet (or any address to specify
the device in the slot) works.
But such address doesn't work for pcie case. (nor for standard pci hot
plug case)
Address that will be used should be able to specify the slot itself
without the device.
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command
2010-10-27 1:47 ` Isaku Yamahata
@ 2010-10-27 13:31 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-27 13:31 UTC (permalink / raw)
To: Isaku Yamahata
Cc: skandasa, lmr, etmartin, wexu2, Markus Armbruster, qemu-devel,
kraxel
On Wed, Oct 27, 2010 at 10:47:11AM +0900, Isaku Yamahata wrote:
> On Mon, Oct 25, 2010 at 04:02:36PM +0900, Isaku Yamahata wrote:
> > > > > What I am also saying is that the same command should be able
> > > > > to work for pci and express I think.
> > > >
> > > > I see. Then, I think that the slot numbering needs to be discussed.
> > >
> > > Yes.
> > >
> > > > More concretely, it's what type of argument is passed for push attention
> > > > button command.
> > > > Here the slot number in general means that the number that is printed near
> > > > the physical slot. Not pci (segment, bus, device) triplets.
> > > >
> > > > The current qemu pci hot plug abuses the triplets as the slot number
> > > > with the assumption of (segment = 0, bus = 0, device = slot number).
> > > > On the other hand, pcie hot plug has its own scheme to number the slots.
> > > > The correspondence between the triplets and the slot is provided
> > > > to the OS. (The standard pci hot plug has its own too which is different
> > > > from the pcie one.)
> > > > So users may want to operate with the slot number.
> > > > User's opinions are needed. Any comments?
> > > >
> > > > thanks,
> > >
> > > Can we just use the topological address everywhere?
> > > Bus numbers are guest assigned, so we can not always
> > > rely on them. They are nice for human monitor
> > > though and should be way to get from them to
> > > topology and back.
> > > And for bus 0 they are equivalent.
> >
> > Yes, with some kind of conversion.
> > We could go with whichever addressing with (slight?) effort
> > to implement conversion.
> > The point is what users(libvirt? virt-manager?) want.
>
> When pushing the attention button, we are talking to the hot plug
> controller, not the device in the slot.
> The button can be pushed even when the slot is empty.
>
> In qemu pci hot plug case the button is used only for device
> ejection, so (segment, bus, device) triplet (or any address to specify
> the device in the slot) works.
> But such address doesn't work for pcie case. (nor for standard pci hot
> plug case)
> Address that will be used should be able to specify the slot itself
> without the device.
Not sure I understand. The device number is the number of the slot on
the bus anyway, isn't it? The only issue is that the bus must be
specified by the topology used to reach it, not bus number
since that is guest assigned expect for the root which is always 0.
> --
> yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (6 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 07/12] pcie/hotplug: introduce pushing attention button command Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 9:56 ` [Qemu-devel] " Michael S. Tsirkin
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 09/12] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
` (4 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 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>
---
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 | 780 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pcie_aer.h | 105 ++++++++
qemu-common.h | 3 +
5 files changed, 903 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 138e545..48f98f3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -187,7 +187,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 2871e27..415a680 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 */
@@ -74,6 +75,19 @@ struct PCIExpressDevice {
* also initialize it when loaded as
* appropreately.
*/
+
+ /* 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..b8cede3
--- /dev/null
+++ b/hw/pcie_aer.c
@@ -0,0 +1,780 @@
+/*
+ * 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__)
+
+static void pcie_aer_clear_error(PCIDevice *dev);
+static uint8_t pcie_aer_root_get_vector(PCIDevice *dev);
+static AERMsgResult
+pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg);
+static AERMsgResult
+pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg *msg);
+static AERMsgResult
+pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg);
+
+/* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
+static PCIEAERSeverity 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 AER_ERR_FATAL;
+ 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 AER_ERR_NONFATAL;
+ default:
+ break;
+ }
+ abort();
+ return AER_ERR_FATAL;
+}
+
+static uint32_t aer_log_next(uint32_t i, uint32_t max)
+{
+ return (i + 1) % max;
+}
+
+static bool aer_log_empty_index(uint32_t producer, uint32_t consumer)
+{
+ return producer == consumer;
+}
+
+static bool aer_log_empty(PCIEAERLog *aer_log)
+{
+ return aer_log_empty_index(aer_log->producer, aer_log->consumer);
+}
+
+static bool aer_log_full(PCIEAERLog *aer_log)
+{
+ return aer_log_next(aer_log->producer, aer_log->log_max) ==
+ aer_log->consumer;
+}
+
+static uint32_t aer_log_add(PCIEAERLog *aer_log)
+{
+ uint32_t i = aer_log->producer;
+ aer_log->producer = aer_log_next(aer_log->producer, aer_log->log_max);
+ return i;
+}
+
+static uint32_t aer_log_del(PCIEAERLog *aer_log)
+{
+ uint32_t i = aer_log->consumer;
+ aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max);
+ return i;
+}
+
+static int aer_log_add_err(PCIEAERLog *aer_log, const PCIEAERErr *err)
+{
+ uint32_t i;
+ if (aer_log_full(aer_log)) {
+ return -1;
+ }
+ i = aer_log_add(aer_log);
+ memcpy(&aer_log->log[i], err, sizeof(*err));
+ return 0;
+}
+
+static const PCIEAERErr* aer_log_del_err(PCIEAERLog *aer_log)
+{
+ uint32_t i;
+ assert(!aer_log_empty(aer_log));
+ i = aer_log_del(aer_log);
+ return &aer_log->log[i];
+}
+
+static void aer_log_clear_all_err(PCIEAERLog *aer_log)
+{
+ aer_log->producer = 0;
+ aer_log->consumer = 0;
+}
+
+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);
+
+ pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
+ offset, PCI_ERR_SIZEOF);
+ exp = &dev->exp;
+ exp->aer_cap = offset;
+ if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) {
+ dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
+ }
+ if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_MAX) {
+ dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_MAX;
+ }
+ dev->exp.aer_log.log = qemu_mallocz(sizeof(dev->exp.aer_log.log[0]) *
+ dev->exp.aer_log.log_max);
+
+ /* On reset PCI_ERR_CAP_MHRE is disabled
+ * PCI_ERR_CAP_MHRE is RWS so that reset doesn't affect related
+ * registers
+ */
+ 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;
+ }
+}
+
+void pcie_aer_exit(PCIDevice *dev)
+{
+ qemu_free(dev->exp.aer_log.log);
+}
+
+void pcie_aer_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len,
+ uint32_t uncorsta_old)
+{
+ uint32_t pos = dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(dev->config + pos + PCI_ERR_CAP);
+ uint32_t first_error = 1U << PCI_ERR_CAP_FEP(errcap);
+ uint32_t uncorsta = pci_get_long(dev->config + pos + PCI_ERR_UNCOR_STATUS);
+
+ /* uncorrectable error */
+ if ((uncorsta_old & first_error) && !(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.
+ */
+ pci_set_long(dev->config + pos + PCI_ERR_UNCOR_STATUS, uncorsta_old);
+ }
+
+ /* capability & control */
+ if (ranges_overlap(addr, len, pos + PCI_ERR_CAP, 4)) {
+ if (!(errcap & PCI_ERR_CAP_MHRE)) {
+ aer_log_clear_all_err(&dev->exp.aer_log);
+ }
+ }
+}
+
+static inline void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ uint8_t type;
+ AERMsgResult result;
+
+ 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) {
+ result = pcie_aer_msg_vbridge(dev, msg);
+ if (result != AER_MSG_SENT) {
+ return;
+ }
+ }
+ result = pcie_aer_msg_alldev(dev, msg);
+ if (type == PCI_EXP_TYPE_ROOT_PORT && result == AER_MSG_SENT) {
+ pcie_aer_msg_root_port(dev, msg);
+ }
+}
+
+static AERMsgResult
+pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ uint16_t cmd = pci_get_word(dev->config + PCI_COMMAND);
+ bool transmit1 =
+ pcie_aer_msg_is_uncor(msg) && (cmd & PCI_COMMAND_SERR);
+ uint32_t devctl = pci_get_word(dev->config +
+ dev->exp.exp_cap + PCI_EXP_DEVCTL);
+ bool transmit2 = msg->severity & devctl;
+ PCIDevice *parent_port;
+
+ if (transmit1) {
+ if (pcie_aer_msg_is_uncor(msg)) {
+ /* Signaled System Error */
+ pci_word_test_and_set_mask(dev->config + PCI_STATUS,
+ PCI_STATUS_SIG_SYSTEM_ERROR);
+ }
+ }
+
+ if (!(transmit1 || transmit2)) {
+ return AER_MSG_MASKED;
+ }
+
+ /* 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)) {
+ /* What to do? */
+ return AER_MSG_MASKED;
+ }
+ pcie_aer_msg(parent_port, msg);
+ }
+ return AER_MSG_SENT;
+}
+
+static AERMsgResult
+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 AER_MSG_MASKED;
+ }
+ return AER_MSG_SENT;
+}
+
+static AERMsgResult
+pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ AERMsgResult ret;
+ uint16_t cmd;
+ uint8_t *aer_cap;
+ uint32_t root_cmd;
+ uint32_t root_sta;
+ bool msi_trigger;
+
+ ret = AER_MSG_MASKED;
+ 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_sta = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+ msi_trigger = false;
+
+ if (cmd & PCI_COMMAND_SERR) {
+ /* System Error. Platform Specific */
+ /* ret = AER_MSG_SENT; */
+ }
+
+ /* Errro Message Received: Root Error Status register */
+ switch (msg->severity) {
+ case AER_ERR_COR:
+ if (root_sta & PCI_ERR_ROOT_COR_RCV) {
+ root_sta |= 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_sta |= PCI_ERR_ROOT_COR_RCV;
+ break;
+ case AER_ERR_NONFATAL:
+ if (!(root_sta & PCI_ERR_ROOT_NONFATAL_RCV) &&
+ root_cmd & PCI_ERR_ROOT_CMD_NONFATAL_EN) {
+ msi_trigger = true;
+ }
+ root_sta |= PCI_ERR_ROOT_NONFATAL_RCV;
+ break;
+ case AER_ERR_FATAL:
+ if (!(root_sta & PCI_ERR_ROOT_FATAL_RCV) &&
+ root_cmd & PCI_ERR_ROOT_CMD_FATAL_EN) {
+ msi_trigger = true;
+ }
+ if (!(root_sta & PCI_ERR_ROOT_UNCOR_RCV)) {
+ root_sta |= PCI_ERR_ROOT_FIRST_FATAL;
+ }
+ root_sta |= PCI_ERR_ROOT_FATAL_RCV;
+ break;
+ }
+ if (pcie_aer_msg_is_uncor(msg)) {
+ if (root_sta & PCI_ERR_ROOT_UNCOR_RCV) {
+ root_sta |= PCI_ERR_ROOT_MULTI_UNCOR_RCV;
+ } else {
+ pci_set_word(aer_cap + PCI_ERR_ROOT_SRC, msg->source_id);
+ }
+ root_sta |= PCI_ERR_ROOT_UNCOR_RCV;
+ }
+ pci_set_long(aer_cap + PCI_ERR_ROOT_STATUS, root_sta);
+
+ if (root_cmd & msg->severity) {
+ /* 6.2.4.1.2 Interrupt Generation */
+ if (pci_msi_enabled(dev)) {
+ pci_msi_notify(dev, pcie_aer_root_get_vector(dev));
+ } else {
+ qemu_set_irq(dev->irq[dev->exp.aer_intx], 1);
+ }
+ ret = AER_MSG_SENT;
+ }
+ return ret;
+}
+
+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;
+ uint32_t dw;
+
+ 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 */
+ cpu_to_be32wu(&dw, err->header[i]);
+ memcpy(aer_cap + PCI_ERR_HEADER_LOG + sizeof(err->header[0]) * i,
+ &dw, sizeof(dw));
+ }
+ } else {
+ assert(!(err->flags & PCIE_AER_ERR_TLP_PRESENT));
+ memset(aer_cap + PCI_ERR_HEADER_LOG, 0, sizeof(err->header));
+ }
+
+ if ((err->flags & PCIE_AER_ERR_TLP_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 */
+ cpu_to_be32wu(&dw, err->prefix[i]);
+ memcpy(aer_cap + PCI_ERR_TLP_PREFIX_LOG +
+ sizeof(err->prefix[0]) * i, &dw, sizeof(dw));
+ }
+ errcap |= PCI_ERR_CAP_TLP;
+ } else {
+ memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, sizeof(err->prefix));
+ }
+ pci_set_long(aer_cap + PCI_ERR_CAP, errcap);
+}
+
+static void pcie_aer_clear_log(PCIDevice *dev)
+{
+ PCIEAERErr *err;
+ 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, sizeof(err->header));
+ memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, sizeof(err->prefix));
+}
+
+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);
+
+ if (errcap & PCI_ERR_CAP_MHRE &&
+ (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1ULL << 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;
+}
+
+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;
+ const PCIEAERErr *err;
+ uint32_t consumer;
+
+ if (!(errcap & PCI_ERR_CAP_MHRE) || aer_log_empty(aer_log)) {
+ pcie_aer_clear_log(dev);
+ return;
+ }
+
+ /*
+ * If more errors are queued, set corresponding bits in uncorrectable
+ * error status.
+ * We emulates 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)
+ */
+ for (consumer = dev->exp.aer_log.consumer;
+ !aer_log_empty_index(dev->exp.aer_log.producer, consumer);
+ consumer = aer_log_next(consumer, dev->exp.aer_log.log_max)) {
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
+ dev->exp.aer_log.log[consumer].status);
+ }
+
+ err = aer_log_del_err(aer_log);
+ pcie_aer_update_log(dev, err);
+}
+
+/*
+ * 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 Sqeucne of Device Error Signaling and Logging Operations
+ * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging
+ * Operations
+ *
+ * Although this implementation can be shortened/optimized, this is kept
+ * parallel to table 6-2.
+ */
+void pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
+{
+ uint8_t *exp_cap;
+ uint8_t *aer_cap = NULL;
+ uint32_t devctl = 0;
+ uint32_t devsta = 0;
+ uint32_t status = err->status;
+ uint32_t mask;
+ bool is_unsupported_request =
+ (!(err->flags & PCIE_AER_ERR_IS_CORRECTABLE) &&
+ err->status == PCI_ERR_UNC_UNSUP);
+ bool is_advisory_nonfatal = false; /* for advisory non-fatal error */
+ uint32_t uncor_status = 0; /* for advisory non-fatal error */
+ PCIEAERMsg msg;
+ int is_header_log_overflowed = 0;
+
+ if (!pci_is_express(dev)) {
+ /* What to do? */
+ return;
+ }
+
+ if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
+ status &= PCI_ERR_COR_SUPPORTED;
+ } else {
+ status &= PCI_ERR_UNC_SUPPORTED;
+ }
+ if (!status || status & (status - 1)) {
+ /* invalid status bit. one and only one bit must be set */
+ return;
+ }
+
+ exp_cap = dev->config + dev->exp.exp_cap;
+ if (dev->exp.aer_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);
+ }
+ if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
+ correctable_error:
+ devsta |= PCI_EXP_DEVSTA_CED;
+ if (is_unsupported_request) {
+ devsta |= PCI_EXP_DEVSTA_URD;
+ }
+ pci_set_word(exp_cap + PCI_EXP_DEVSTA, devsta);
+
+ if (aer_cap) {
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_COR_STATUS, status);
+ mask = pci_get_long(aer_cap + PCI_ERR_COR_MASK);
+ if (mask & status) {
+ return;
+ }
+ if (is_advisory_nonfatal) {
+ uint32_t uncor_mask =
+ pci_get_long(aer_cap + PCI_ERR_UNCOR_MASK);
+ if (!(uncor_mask & uncor_status)) {
+ is_header_log_overflowed = pcie_aer_record_error(dev, err);
+ }
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
+ uncor_status);
+ }
+ }
+
+ if (is_unsupported_request && !(devctl & PCI_EXP_DEVCTL_URRE)) {
+ return;
+ }
+ if (!(devctl & PCI_EXP_DEVCTL_CERE)) {
+ return;
+ }
+ msg.severity = AER_ERR_COR;
+ } else {
+ bool is_fatal =
+ (pcie_aer_uncor_default_severity(status) == AER_ERR_FATAL);
+ uint16_t cmd;
+
+ if (aer_cap) {
+ is_fatal = status & pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
+ }
+ if (!is_fatal && (err->flags & PCIE_AER_ERR_MAYBE_ADVISORY)) {
+ is_advisory_nonfatal = true;
+ uncor_status = status;
+ status = PCI_ERR_COR_ADV_NONFATAL;
+ goto correctable_error;
+ }
+ if (is_fatal) {
+ devsta |= PCI_EXP_DEVSTA_FED;
+ } else {
+ devsta |= PCI_EXP_DEVSTA_NFED;
+ }
+ if (is_unsupported_request) {
+ devsta |= PCI_EXP_DEVSTA_URD;
+ }
+ pci_set_long(exp_cap + PCI_EXP_DEVSTA, devsta);
+
+ if (aer_cap) {
+ mask = pci_get_long(aer_cap + PCI_ERR_UNCOR_MASK);
+ if (mask & status) {
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
+ status);
+ return;
+ }
+
+ is_header_log_overflowed = pcie_aer_record_error(dev, err);
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS, status);
+ }
+
+ cmd = pci_get_word(dev->config + PCI_COMMAND);
+ if (is_unsupported_request &&
+ !(devctl & PCI_EXP_DEVCTL_URRE) && !(cmd & PCI_COMMAND_SERR)) {
+ return;
+ }
+ if (is_fatal) {
+ if (!((cmd & PCI_COMMAND_SERR) ||
+ (devctl & PCI_EXP_DEVCTL_FERE))) {
+ return;
+ }
+ msg.severity = AER_ERR_FATAL;
+ } else {
+ if (!((cmd & PCI_COMMAND_SERR) ||
+ (devctl & PCI_EXP_DEVCTL_NFERE))) {
+ return;
+ }
+ msg.severity = AER_ERR_NONFATAL;
+ }
+ }
+
+ /* send up error message */
+ msg.source_id = err->source_id;
+ pcie_aer_msg(dev, &msg);
+
+ if (is_header_log_overflowed) {
+ PCIEAERErr header_log_overflow = {
+ .status = PCI_ERR_COR_HL_OVERFLOW,
+ .flags = PCIE_AER_ERR_IS_CORRECTABLE,
+ .header = {0, 0, 0, 0},
+ .prefix = {0, 0, 0, 0},
+ };
+ pcie_aer_inject_error(dev, &header_log_overflow);
+ }
+}
+
+void pcie_aer_root_set_vector(PCIDevice *dev, uint8_t 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,
+ ((uint32_t)vector) << PCI_ERR_ROOT_IRQ_SHIFT);
+}
+
+static uint8_t 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;
+}
+
+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)
+{
+ uint16_t pos = dev->exp.aer_cap;
+ uint8_t *aer_cap = dev->config + pos;
+
+ /* 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_UINT32(producer, PCIEAERLog),
+ VMSTATE_UINT32(consumer, PCIEAERLog),
+ VMSTATE_UINT16(log_max, PCIEAERLog),
+ VMSTATE_PCIE_AER_ERRS(log, PCIEAERLog, log_max,
+ 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..db9f87c
--- /dev/null
+++ b/hw/pcie_aer.h
@@ -0,0 +1,105 @@
+/*
+ * 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 */
+typedef enum {
+ AER_MSG_MASKED,
+ AER_MSG_SENT,
+} AERMsgResult;
+
+/* AER log */
+struct PCIEAERLog {
+ /* This structure is saved/loaded.
+ So explicitly size them instead of unsigned int */
+ uint32_t producer;
+ uint32_t consumer;
+
+#define PCIE_AER_LOG_MAX_DEFAULT 8
+#define PCIE_AER_LOG_MAX_MAX 128 /* what is appropriate? */
+#define PCIE_AER_LOG_MAX_UNSET 0xffff
+ uint16_t log_max;
+
+ PCIEAERErr *log; /* ringed buffer */
+};
+
+/* aer error severity */
+typedef enum {
+ /* those value are same as
+ * Root error command register in aer extended cap and
+ * root control register in pci express cap.
+ */
+ AER_ERR_COR = PCI_ERR_ROOT_CMD_COR_EN,
+ AER_ERR_NONFATAL = PCI_ERR_ROOT_CMD_NONFATAL_EN,
+ AER_ERR_FATAL = PCI_ERR_ROOT_CMD_FATAL_EN,
+} PCIEAERSeverity;
+
+/* aer error message: error signaling message has only error sevirity and
+ source id. See 2.2.8.3 error signaling messages */
+struct PCIEAERMsg {
+ PCIEAERSeverity severity;
+ uint16_t source_id; /* bdf */
+};
+
+static inline bool
+pcie_aer_msg_is_uncor(const PCIEAERMsg *msg)
+{
+ return msg->severity == AER_ERR_NONFATAL || msg->severity == AER_ERR_FATAL;
+}
+
+/* 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_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;
+
+void 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,
+ uint32_t uncorsta_prev);
+
+/* aer root port */
+void pcie_aer_root_set_vector(PCIDevice *dev, uint8_t 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 */
+void 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 b97b16e..63ee8c3 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -221,6 +221,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] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-10-20 9:56 ` Michael S. Tsirkin
2010-10-21 5:15 ` Isaku Yamahata
2010-10-22 11:28 ` Markus Armbruster
0 siblings, 2 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-20 9:56 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Oct 20, 2010 at 05:18:57PM +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>
Some style comments and a couple of minor bugs.
> ---
> 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 | 780 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/pcie_aer.h | 105 ++++++++
> qemu-common.h | 3 +
> 5 files changed, 903 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 138e545..48f98f3 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -187,7 +187,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 2871e27..415a680 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 */
> @@ -74,6 +75,19 @@ struct PCIExpressDevice {
> * also initialize it when loaded as
> * appropreately.
> */
> +
> + /* 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..b8cede3
> --- /dev/null
> +++ b/hw/pcie_aer.c
> @@ -0,0 +1,780 @@
> +/*
> + * 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__)
> +
> +static void pcie_aer_clear_error(PCIDevice *dev);
> +static uint8_t pcie_aer_root_get_vector(PCIDevice *dev);
> +static AERMsgResult
> +pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg);
> +static AERMsgResult
> +pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg *msg);
> +static AERMsgResult
> +pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg);
Just make the above bool, e.g. true if sent?
Also don't forward declare static functions. Just order them properly in the
file.
> +
> +/* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
> +static PCIEAERSeverity 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 AER_ERR_FATAL;
> + 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 AER_ERR_NONFATAL;
> + default:
> + break;
> + }
> + abort();
> + return AER_ERR_FATAL;
> +}
> +
> +static uint32_t aer_log_next(uint32_t i, uint32_t max)
> +{
> + return (i + 1) % max;
> +}
> +
Pass in PCIEAERLog* as 1st parameter instead of max.
This way we get it right.
> +static bool aer_log_empty_index(uint32_t producer, uint32_t consumer)
> +{
> + return producer == consumer;
> +}
> +
This one is better opencoded.
> +static bool aer_log_empty(PCIEAERLog *aer_log)
> +{
> + return aer_log_empty_index(aer_log->producer, aer_log->consumer);
> +}
> +
> +static bool aer_log_full(PCIEAERLog *aer_log)
> +{
> + return aer_log_next(aer_log->producer, aer_log->log_max) ==
> + aer_log->consumer;
> +}
> +
> +static uint32_t aer_log_add(PCIEAERLog *aer_log)
> +{
> + uint32_t i = aer_log->producer;
> + aer_log->producer = aer_log_next(aer_log->producer, aer_log->log_max);
> + return i;
> +}
> +
> +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> +{
> + uint32_t i = aer_log->consumer;
> + aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max);
> + return i;
> +}
Please just use 'int' or 'unsigned' instead of uint32_t if you simply
want to say 'a number'. Using specific length makes it impossible to
say where you *really* want a value.
> +
> +static int aer_log_add_err(PCIEAERLog *aer_log, const PCIEAERErr *err)
> +{
> + uint32_t i;
> + if (aer_log_full(aer_log)) {
> + return -1;
> + }
> + i = aer_log_add(aer_log);
> + memcpy(&aer_log->log[i], err, sizeof(*err));
sizeof is not a function, you don't need ()
> + return 0;
> +}
> +
> +static const PCIEAERErr* aer_log_del_err(PCIEAERLog *aer_log)
> +{
> + uint32_t i;
> + assert(!aer_log_empty(aer_log));
> + i = aer_log_del(aer_log);
> + return &aer_log->log[i];
> +}
> +
> +static void aer_log_clear_all_err(PCIEAERLog *aer_log)
> +{
> + aer_log->producer = 0;
> + aer_log->consumer = 0;
> +}
> +
> +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);
> +
> + pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
> + offset, PCI_ERR_SIZEOF);
> + exp = &dev->exp;
> + exp->aer_cap = offset;
> + if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) {
> + dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
> + }
> + if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_MAX) {
> + dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_MAX;
> + }
> + dev->exp.aer_log.log = qemu_mallocz(sizeof(dev->exp.aer_log.log[0]) *
> + dev->exp.aer_log.log_max);
> +
> + /* On reset PCI_ERR_CAP_MHRE is disabled
> + * PCI_ERR_CAP_MHRE is RWS so that reset doesn't affect related
> + * registers
> + */
> + 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;
> + }
> +}
> +
> +void pcie_aer_exit(PCIDevice *dev)
> +{
> + qemu_free(dev->exp.aer_log.log);
> +}
> +
> +void pcie_aer_write_config(PCIDevice *dev,
> + uint32_t addr, uint32_t val, int len,
> + uint32_t uncorsta_old)
> +{
> + uint32_t pos = dev->exp.aer_cap;
dev->config + pos would be a better variable to use.
> + uint32_t errcap = pci_get_long(dev->config + pos + PCI_ERR_CAP);
> + uint32_t first_error = 1U << PCI_ERR_CAP_FEP(errcap);
> + uint32_t uncorsta = pci_get_long(dev->config + pos + PCI_ERR_UNCOR_STATUS);
> +
> + /* uncorrectable error */
> + if ((uncorsta_old & first_error) && !(uncorsta & first_error)) {
> + /* the bit that corresponds to the first error is cleared */
> + pcie_aer_clear_error(dev);
So why not just call this unconditionally?
> + } 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.
> + */
> + pci_set_long(dev->config + pos + PCI_ERR_UNCOR_STATUS, uncorsta_old);
Why do we need uncorsta_old? Why can't we get the info from the log?
> + }
> +
> + /* capability & control */
> + if (ranges_overlap(addr, len, pos + PCI_ERR_CAP, 4)) {
I can not figure this out. What does this range check do?
And why use capability number as offset?
> + if (!(errcap & PCI_ERR_CAP_MHRE)) {
also checking twise is ugly.
> + aer_log_clear_all_err(&dev->exp.aer_log);
> + }
> + }
> +}
> +
> +static inline void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
> +{
> + uint8_t type;
> + AERMsgResult result;
> +
> + 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) {
> + result = pcie_aer_msg_vbridge(dev, msg);
> + if (result != AER_MSG_SENT) {
> + return;
> + }
> + }
> + result = pcie_aer_msg_alldev(dev, msg);
> + if (type == PCI_EXP_TYPE_ROOT_PORT && result == AER_MSG_SENT) {
> + pcie_aer_msg_root_port(dev, msg);
> + }
> +}
> +
> +static AERMsgResult
> +pcie_aer_msg_alldev(PCIDevice *dev, const PCIEAERMsg *msg)
> +{
> + uint16_t cmd = pci_get_word(dev->config + PCI_COMMAND);
> + bool transmit1 =
> + pcie_aer_msg_is_uncor(msg) && (cmd & PCI_COMMAND_SERR);
> + uint32_t devctl = pci_get_word(dev->config +
> + dev->exp.exp_cap + PCI_EXP_DEVCTL);
Word is uint16, so devctl should be too.
> + bool transmit2 = msg->severity & devctl;
> + PCIDevice *parent_port;
> +
> + if (transmit1) {
> + if (pcie_aer_msg_is_uncor(msg)) {
> + /* Signaled System Error */
> + pci_word_test_and_set_mask(dev->config + PCI_STATUS,
> + PCI_STATUS_SIG_SYSTEM_ERROR);
> + }
> + }
> +
> + if (!(transmit1 || transmit2)) {
> + return AER_MSG_MASKED;
> + }
> +
> + /* 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)) {
> + /* What to do? */
> + return AER_MSG_MASKED;
> + }
> + pcie_aer_msg(parent_port, msg);
> + }
> + return AER_MSG_SENT;
> +}
> +
> +static AERMsgResult
> +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 AER_MSG_MASKED;
> + }
> + return AER_MSG_SENT;
> +}
> +
> +static AERMsgResult
> +pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
> +{
> + AERMsgResult ret;
> + uint16_t cmd;
> + uint8_t *aer_cap;
> + uint32_t root_cmd;
> + uint32_t root_sta;
> + bool msi_trigger;
> +
> + ret = AER_MSG_MASKED;
> + 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_sta = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
> + msi_trigger = false;
> +
> + if (cmd & PCI_COMMAND_SERR) {
> + /* System Error. Platform Specific */
> + /* ret = AER_MSG_SENT; */
> + }
> +
> + /* Errro Message Received: Root Error Status register */
> + switch (msg->severity) {
> + case AER_ERR_COR:
> + if (root_sta & PCI_ERR_ROOT_COR_RCV) {
> + root_sta |= 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_sta |= PCI_ERR_ROOT_COR_RCV;
> + break;
> + case AER_ERR_NONFATAL:
> + if (!(root_sta & PCI_ERR_ROOT_NONFATAL_RCV) &&
> + root_cmd & PCI_ERR_ROOT_CMD_NONFATAL_EN) {
> + msi_trigger = true;
> + }
> + root_sta |= PCI_ERR_ROOT_NONFATAL_RCV;
> + break;
> + case AER_ERR_FATAL:
> + if (!(root_sta & PCI_ERR_ROOT_FATAL_RCV) &&
> + root_cmd & PCI_ERR_ROOT_CMD_FATAL_EN) {
> + msi_trigger = true;
> + }
> + if (!(root_sta & PCI_ERR_ROOT_UNCOR_RCV)) {
> + root_sta |= PCI_ERR_ROOT_FIRST_FATAL;
> + }
> + root_sta |= PCI_ERR_ROOT_FATAL_RCV;
> + break;
> + }
> + if (pcie_aer_msg_is_uncor(msg)) {
> + if (root_sta & PCI_ERR_ROOT_UNCOR_RCV) {
> + root_sta |= PCI_ERR_ROOT_MULTI_UNCOR_RCV;
> + } else {
> + pci_set_word(aer_cap + PCI_ERR_ROOT_SRC, msg->source_id);
> + }
> + root_sta |= PCI_ERR_ROOT_UNCOR_RCV;
> + }
> + pci_set_long(aer_cap + PCI_ERR_ROOT_STATUS, root_sta);
> +
> + if (root_cmd & msg->severity) {
> + /* 6.2.4.1.2 Interrupt Generation */
> + if (pci_msi_enabled(dev)) {
> + pci_msi_notify(dev, pcie_aer_root_get_vector(dev));
> + } else {
> + qemu_set_irq(dev->irq[dev->exp.aer_intx], 1);
> + }
> + ret = AER_MSG_SENT;
> + }
> + return ret;
> +}
> +
> +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;
What guarantees err->status is not 0 here?
> + uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
> + int i;
> + uint32_t dw;
> +
> + 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 */
> + cpu_to_be32wu(&dw, err->header[i]);
> + memcpy(aer_cap + PCI_ERR_HEADER_LOG + sizeof(err->header[0]) * i,
> + &dw, sizeof(dw));
memcpy into config space is almost sure to be wrong on BE hosts.
Please fill all fields with appropriate pci_ macros.
This also ensures you don't depend on specific layout for correctness.
sizeof(err->header) is confusing here. Please just use a macro.
> + }
> + } else {
> + assert(!(err->flags & PCIE_AER_ERR_TLP_PRESENT));
> + memset(aer_cap + PCI_ERR_HEADER_LOG, 0, sizeof(err->header));
sizeof(err->header) is confusing here. Please just use a macro.
> + }
> +
> + if ((err->flags & PCIE_AER_ERR_TLP_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 */
> + cpu_to_be32wu(&dw, err->prefix[i]);
> + memcpy(aer_cap + PCI_ERR_TLP_PREFIX_LOG +
> + sizeof(err->prefix[0]) * i, &dw, sizeof(dw));
as above
> + }
> + errcap |= PCI_ERR_CAP_TLP;
> + } else {
> + memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, sizeof(err->prefix));
> + }
> + pci_set_long(aer_cap + PCI_ERR_CAP, errcap);
> +}
> +
> +static void pcie_aer_clear_log(PCIDevice *dev)
> +{
> + PCIEAERErr *err;
> + 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, sizeof(err->header));
> + memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, sizeof(err->prefix));
So err is in fact unused?
This assumes that layout for PCIEAERErr matches config space.
This will lead with time to attempts to copy structure directly.
This is wrong e.g. because of endianness. Please don't do
these tricks: just a macro defining size will be enough.
> +}
> +
> +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);
> +
> + if (errcap & PCI_ERR_CAP_MHRE &&
> + (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1ULL << fep))) {
get_long returns 32 bit
1ULL gives you 64 bit
So why bother with it?
Looking at code fep is 0 to 31, so 1U would be what you want.
> + /* 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;
> +}
> +
> +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;
> + const PCIEAERErr *err;
> + uint32_t consumer;
> +
> + if (!(errcap & PCI_ERR_CAP_MHRE) || aer_log_empty(aer_log)) {
> + pcie_aer_clear_log(dev);
> + return;
> + }
> +
> + /*
> + * If more errors are queued, set corresponding bits in uncorrectable
> + * error status.
> + * We emulates uncorrectable error status register as W1CS.
We emulate
> + * 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)
> + */
> + for (consumer = dev->exp.aer_log.consumer;
> + !aer_log_empty_index(dev->exp.aer_log.producer, consumer);
> + consumer = aer_log_next(consumer, dev->exp.aer_log.log_max)) {
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
> + dev->exp.aer_log.log[consumer].status);
> + }
> +
> + err = aer_log_del_err(aer_log);
> + pcie_aer_update_log(dev, err);
> +}
> +
> +/*
> + * 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 Sqeucne of Device Error Signaling and Logging Operations
typo?
> + * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging
> + * Operations
> + *
> + * Although this implementation can be shortened/optimized, this is kept
> + * parallel to table 6-2.
Yes but this does not mean this can not be split up
into subfunctions.
> + */
> +void pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
> +{
> + uint8_t *exp_cap;
> + uint8_t *aer_cap = NULL;
> + uint32_t devctl = 0;
> + uint32_t devsta = 0;
> + uint32_t status = err->status;
> + uint32_t mask;
> + bool is_unsupported_request =
> + (!(err->flags & PCIE_AER_ERR_IS_CORRECTABLE) &&
> + err->status == PCI_ERR_UNC_UNSUP);
> + bool is_advisory_nonfatal = false; /* for advisory non-fatal error */
> + uint32_t uncor_status = 0; /* for advisory non-fatal error */
> + PCIEAERMsg msg;
> + int is_header_log_overflowed = 0;
> +
> + if (!pci_is_express(dev)) {
> + /* What to do? */
This is used from command, right?
So return error?
> + return;
> + }
> +
> + if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
> + status &= PCI_ERR_COR_SUPPORTED;
> + } else {
> + status &= PCI_ERR_UNC_SUPPORTED;
> + }
> + if (!status || status & (status - 1)) {
So you want to rely on & being higher precedence than && or not?
I prefer when we do, use less (), but the rest of the file seems
to use (). Let's be consistent.
> + /* invalid status bit. one and only one bit must be set */
Pls put comments on same or above lines as the code, not after :)
> + return;
So return error?
> + }
> +
> + exp_cap = dev->config + dev->exp.exp_cap;
> + if (dev->exp.aer_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);
> + }
> + if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
> + correctable_error:
Please, don't use goto to implement logic.
Split correctable/uncorrectable into subfunctions and call them any
number of times.
> + devsta |= PCI_EXP_DEVSTA_CED;
> + if (is_unsupported_request) {
> + devsta |= PCI_EXP_DEVSTA_URD;
> + }
> + pci_set_word(exp_cap + PCI_EXP_DEVSTA, devsta);
> +
> + if (aer_cap) {
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_COR_STATUS, status);
> + mask = pci_get_long(aer_cap + PCI_ERR_COR_MASK);
> + if (mask & status) {
> + return;
> + }
> + if (is_advisory_nonfatal) {
> + uint32_t uncor_mask =
> + pci_get_long(aer_cap + PCI_ERR_UNCOR_MASK);
> + if (!(uncor_mask & uncor_status)) {
> + is_header_log_overflowed = pcie_aer_record_error(dev, err);
> + }
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
> + uncor_status);
> + }
> + }
> +
> + if (is_unsupported_request && !(devctl & PCI_EXP_DEVCTL_URRE)) {
> + return;
> + }
> + if (!(devctl & PCI_EXP_DEVCTL_CERE)) {
> + return;
> + }
> + msg.severity = AER_ERR_COR;
> + } else {
> + bool is_fatal =
> + (pcie_aer_uncor_default_severity(status) == AER_ERR_FATAL);
Don't need (); also
> + uint16_t cmd;
> +
> + if (aer_cap) {
> + is_fatal = status & pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
> + }
> + if (!is_fatal && (err->flags & PCIE_AER_ERR_MAYBE_ADVISORY)) {
> + is_advisory_nonfatal = true;
> + uncor_status = status;
> + status = PCI_ERR_COR_ADV_NONFATAL;
> + goto correctable_error;
> + }
> + if (is_fatal) {
> + devsta |= PCI_EXP_DEVSTA_FED;
> + } else {
> + devsta |= PCI_EXP_DEVSTA_NFED;
> + }
> + if (is_unsupported_request) {
> + devsta |= PCI_EXP_DEVSTA_URD;
> + }
> + pci_set_long(exp_cap + PCI_EXP_DEVSTA, devsta);
> +
> + if (aer_cap) {
> + mask = pci_get_long(aer_cap + PCI_ERR_UNCOR_MASK);
> + if (mask & status) {
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
> + status);
> + return;
> + }
> +
> + is_header_log_overflowed = pcie_aer_record_error(dev, err);
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS, status);
> + }
> +
> + cmd = pci_get_word(dev->config + PCI_COMMAND);
> + if (is_unsupported_request &&
> + !(devctl & PCI_EXP_DEVCTL_URRE) && !(cmd & PCI_COMMAND_SERR)) {
> + return;
> + }
> + if (is_fatal) {
> + if (!((cmd & PCI_COMMAND_SERR) ||
> + (devctl & PCI_EXP_DEVCTL_FERE))) {
> + return;
> + }
> + msg.severity = AER_ERR_FATAL;
> + } else {
> + if (!((cmd & PCI_COMMAND_SERR) ||
> + (devctl & PCI_EXP_DEVCTL_NFERE))) {
> + return;
> + }
> + msg.severity = AER_ERR_NONFATAL;
> + }
> + }
> +
> + /* send up error message */
> + msg.source_id = err->source_id;
> + pcie_aer_msg(dev, &msg);
> +
> + if (is_header_log_overflowed) {
> + PCIEAERErr header_log_overflow = {
> + .status = PCI_ERR_COR_HL_OVERFLOW,
> + .flags = PCIE_AER_ERR_IS_CORRECTABLE,
> + .header = {0, 0, 0, 0},
> + .prefix = {0, 0, 0, 0},
You don't need the 0,0,0 thing. Whatever you don't specify is 0.
> + };
> + pcie_aer_inject_error(dev, &header_log_overflow);
> + }
> +}
> +
> +void pcie_aer_root_set_vector(PCIDevice *dev, uint8_t vector)
Just get vector as unsigned, you won't have to cast it down below.
> +{
> + 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,
> + ((uint32_t)vector) << PCI_ERR_ROOT_IRQ_SHIFT);
> +}
> +
> +static uint8_t 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;
> +}
> +
> +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)
> +{
> + uint16_t pos = dev->exp.aer_cap;
> + uint8_t *aer_cap = dev->config + pos;
> +
> + /* 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_UINT32(producer, PCIEAERLog),
> + VMSTATE_UINT32(consumer, PCIEAERLog),
> + VMSTATE_UINT16(log_max, PCIEAERLog),
> + VMSTATE_PCIE_AER_ERRS(log, PCIEAERLog, log_max,
> + 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..db9f87c
> --- /dev/null
> +++ b/hw/pcie_aer.h
> @@ -0,0 +1,105 @@
> +/*
> + * 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 */
> +typedef enum {
> + AER_MSG_MASKED,
> + AER_MSG_SENT,
> +} AERMsgResult;
We don't really need this, especially not in an external header.
> +
> +/* AER log */
> +struct PCIEAERLog {
> + /* This structure is saved/loaded.
> + So explicitly size them instead of unsigned int */
> + uint32_t producer;
> + uint32_t consumer;
> +
> +#define PCIE_AER_LOG_MAX_DEFAULT 8
> +#define PCIE_AER_LOG_MAX_MAX 128 /* what is appropriate? */
> +#define PCIE_AER_LOG_MAX_UNSET 0xffff
> + uint16_t log_max;
> +
> + PCIEAERErr *log; /* ringed buffer */
> +};
> +
> +/* aer error severity */
> +typedef enum {
> + /* those value are same as
> + * Root error command register in aer extended cap and
> + * root control register in pci express cap.
> + */
> + AER_ERR_COR = PCI_ERR_ROOT_CMD_COR_EN,
> + AER_ERR_NONFATAL = PCI_ERR_ROOT_CMD_NONFATAL_EN,
> + AER_ERR_FATAL = PCI_ERR_ROOT_CMD_FATAL_EN,
> +} PCIEAERSeverity;
> +
I think we are better off without the enum above.
It just adds 2 ways to get the same value and
thre will always be confusion.
And C compiler does not give us type checking anyway.
> +/* aer error message: error signaling message has only error sevirity and
> + source id. See 2.2.8.3 error signaling messages */
> +struct PCIEAERMsg {
> + PCIEAERSeverity severity;
> + uint16_t source_id; /* bdf */
> +};
> +
> +static inline bool
> +pcie_aer_msg_is_uncor(const PCIEAERMsg *msg)
> +{
> + return msg->severity == AER_ERR_NONFATAL || msg->severity == AER_ERR_FATAL;
> +}
> +
> +/* 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_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;
> +
> +void 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,
> + uint32_t uncorsta_prev);
> +
> +/* aer root port */
> +void pcie_aer_root_set_vector(PCIDevice *dev, uint8_t 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 */
> +void 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 b97b16e..63ee8c3 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -221,6 +221,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 [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-20 9:56 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-10-21 5:15 ` Isaku Yamahata
2010-10-21 8:07 ` Michael S. Tsirkin
2010-10-22 11:28 ` Markus Armbruster
1 sibling, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-21 5:15 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
Thank you for detailed review.
On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote:
> > +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> > +{
> > + uint32_t i = aer_log->consumer;
> > + aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max);
> > + return i;
> > +}
>
>
> Please just use 'int' or 'unsigned' instead of uint32_t if you simply
> want to say 'a number'. Using specific length makes it impossible to
> say where you *really* want a value.
PCIEAERLog is saved/loaded. So explicit sized number is chosen.
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-21 5:15 ` Isaku Yamahata
@ 2010-10-21 8:07 ` Michael S. Tsirkin
2010-10-21 23:53 ` Isaku Yamahata
0 siblings, 1 reply; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-21 8:07 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Thu, Oct 21, 2010 at 02:15:24PM +0900, Isaku Yamahata wrote:
> Thank you for detailed review.
>
> On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote:
> > > +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> > > +{
> > > + uint32_t i = aer_log->consumer;
> > > + aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max);
> > > + return i;
> > > +}
> >
> >
> > Please just use 'int' or 'unsigned' instead of uint32_t if you simply
> > want to say 'a number'. Using specific length makes it impossible to
> > say where you *really* want a value.
>
> PCIEAERLog is saved/loaded. So explicit sized number is chosen.
I didn't notice. But consumer/producer are not guest visible, are they?
If yes I think it's a mistake to save/load them, tying us to
a specific implementation: just calculate and save the # of
valid entries in the log.
Also I put the comment here but it is a general one: pls go over the
code, and just take a look at what types you use all over and think
whether size really matters. In most places it does not, it just must be
big enough, so use int or unsigned there. It will be much harder for
others to do so later.
> --
> yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-21 8:07 ` Michael S. Tsirkin
@ 2010-10-21 23:53 ` Isaku Yamahata
2010-10-22 9:27 ` Michael S. Tsirkin
0 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-21 23:53 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Thu, Oct 21, 2010 at 10:07:01AM +0200, Michael S. Tsirkin wrote:
> On Thu, Oct 21, 2010 at 02:15:24PM +0900, Isaku Yamahata wrote:
> > Thank you for detailed review.
> >
> > On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote:
> > > > +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> > > > +{
> > > > + uint32_t i = aer_log->consumer;
> > > > + aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max);
> > > > + return i;
> > > > +}
> > >
> > >
> > > Please just use 'int' or 'unsigned' instead of uint32_t if you simply
> > > want to say 'a number'. Using specific length makes it impossible to
> > > say where you *really* want a value.
> >
> > PCIEAERLog is saved/loaded. So explicit sized number is chosen.
>
> I didn't notice. But consumer/producer are not guest visible, are they?
> If yes I think it's a mistake to save/load them, tying us to
> a specific implementation: just calculate and save the # of
> valid entries in the log.
ACIEAERLog and PCIEAERErr themselves are the implementation specific
internal states which is not visible to guest.
So there is no point of arguing that consumer/producer are
specific to implementation.
> Also I put the comment here but it is a general one: pls go over the
> code, and just take a look at what types you use all over and think
> whether size really matters. In most places it does not, it just must be
> big enough, so use int or unsigned there. It will be much harder for
> others to do so later.
Will do.
--
yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-21 23:53 ` Isaku Yamahata
@ 2010-10-22 9:27 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-22 9:27 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Fri, Oct 22, 2010 at 08:53:15AM +0900, Isaku Yamahata wrote:
> On Thu, Oct 21, 2010 at 10:07:01AM +0200, Michael S. Tsirkin wrote:
> > On Thu, Oct 21, 2010 at 02:15:24PM +0900, Isaku Yamahata wrote:
> > > Thank you for detailed review.
> > >
> > > On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote:
> > > > > +static uint32_t aer_log_del(PCIEAERLog *aer_log)
> > > > > +{
> > > > > + uint32_t i = aer_log->consumer;
> > > > > + aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max);
> > > > > + return i;
> > > > > +}
> > > >
> > > >
> > > > Please just use 'int' or 'unsigned' instead of uint32_t if you simply
> > > > want to say 'a number'. Using specific length makes it impossible to
> > > > say where you *really* want a value.
> > >
> > > PCIEAERLog is saved/loaded. So explicit sized number is chosen.
> >
> > I didn't notice. But consumer/producer are not guest visible, are they?
> > If yes I think it's a mistake to save/load them, tying us to
> > a specific implementation: just calculate and save the # of
> > valid entries in the log.
>
> ACIEAERLog and PCIEAERErr themselves are the implementation specific
> internal states which is not visible to guest.
> So there is no point of arguing that consumer/producer are
> specific to implementation.
Well the errors can be read out by the guest, so they are
potentially guest visible. Thus I think the only thing
we want to migrate is the list of errors logged.
> > Also I put the comment here but it is a general one: pls go over the
> > code, and just take a look at what types you use all over and think
> > whether size really matters. In most places it does not, it just must be
> > big enough, so use int or unsigned there. It will be much harder for
> > others to do so later.
>
> Will do.
> --
> yamahata
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability
2010-10-20 9:56 ` [Qemu-devel] " Michael S. Tsirkin
2010-10-21 5:15 ` Isaku Yamahata
@ 2010-10-22 11:28 ` Markus Armbruster
1 sibling, 0 replies; 40+ messages in thread
From: Markus Armbruster @ 2010-10-22 11:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: skandasa, etmartin, wexu2, qemu-devel, Isaku Yamahata, adnan
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Wed, Oct 20, 2010 at 05:18:57PM +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>
>
> Some style comments and a couple of minor bugs.
[...]
>> +
>> +static int aer_log_add_err(PCIEAERLog *aer_log, const PCIEAERErr *err)
>> +{
>> + uint32_t i;
>> + if (aer_log_full(aer_log)) {
>> + return -1;
>> + }
>> + i = aer_log_add(aer_log);
>> + memcpy(&aer_log->log[i], err, sizeof(*err));
>
> sizeof is not a function, you don't need ()
Matter of taste. Yes, it's an operator, but it's as function-like as
operators get.
For what it's worth, grep says roughly 98% of our sizeof uses are
followed by '('. I doubt the parenthesis are required for grouping that
often.
[...]
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH v6 09/12] pcie/aer: glue aer error injection into qemu monitor
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (7 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 9:44 ` [Qemu-devel] " Michael S. Tsirkin
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 10/12] ioh3420: support aer Isaku Yamahata
` (3 subsequent siblings)
12 siblings, 1 reply; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
introduce pcie_aer_inject_error command.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v3 -> v4:
- s/PCIE_AER/PCIEAER/g for structure names.
- compilation adjustment.
Changes v2 -> v3:
- compilation adjustment.
---
hw/pcie_aer.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-monitor.hx | 22 ++++++++++++++
sysemu.h | 5 +++
3 files changed, 111 insertions(+), 0 deletions(-)
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
index b8cede3..459e0d7 100644
--- a/hw/pcie_aer.c
+++ b/hw/pcie_aer.c
@@ -19,6 +19,8 @@
*/
#include "sysemu.h"
+#include "qemu-objects.h"
+#include "monitor.h"
#include "pci_bridge.h"
#include "pcie.h"
#include "msix.h"
@@ -778,3 +780,85 @@ const VMStateDescription vmstate_pcie_aer_log = {
}
};
+void pcie_aer_inject_error_print(Monitor *mon, const QObject *data)
+{
+ QDict *qdict;
+ int devfn;
+ assert(qobject_type(data) == QTYPE_QDICT);
+ qdict = qobject_to_qdict(data);
+
+ devfn = (int)qdict_get_int(qdict, "devfn");
+ monitor_printf(mon, "OK domain: %x, bus: %x devfn: %x.%x\n",
+ (int) qdict_get_int(qdict, "domain"),
+ (int) qdict_get_int(qdict, "bus"),
+ PCI_SLOT(devfn), PCI_FUNC(devfn));
+}
+
+int do_pcie_aer_inejct_error(Monitor *mon,
+ const QDict *qdict, QObject **ret_data)
+{
+ const char *pci_addr = qdict_get_str(qdict, "pci_addr");
+ int dom;
+ int bus;
+ unsigned int slot;
+ unsigned int func;
+ PCIDevice *dev;
+ PCIEAERErr err;
+
+ /* Ideally qdev device path should be used.
+ * However at the moment there is no reliable way to determine
+ * wheher a given qdev is pci device or not.
+ * so pci_addr is used.
+ */
+ if (pci_parse_devaddr(pci_addr, &dom, &bus, &slot, &func)) {
+ monitor_printf(mon, "invalid pci address %s\n", pci_addr);
+ return -1;
+ }
+ dev = pci_find_device(pci_find_root_bus(dom), bus, slot, func);
+ if (!dev) {
+ monitor_printf(mon, "device is not found. 0x%x:0x%x.0x%x\n",
+ bus, slot, func);
+ return -1;
+ }
+ if (!pci_is_express(dev)) {
+ monitor_printf(mon, "the device doesn't support pci express. "
+ "0x%x:0x%x.0x%x\n",
+ bus, slot, func);
+ return -1;
+ }
+
+ err.status = qdict_get_int(qdict, "error_status");
+ err.source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn;
+
+ err.flags = 0;
+ if (qdict_get_int(qdict, "is_correctable")) {
+ err.flags |= PCIE_AER_ERR_IS_CORRECTABLE;
+ }
+ if (qdict_get_int(qdict, "advisory_non_fatal")) {
+ err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY;
+ }
+ if (qdict_haskey(qdict, "tlph0")) {
+ err.flags |= PCIE_AER_ERR_HEADER_VALID;
+ }
+ if (qdict_haskey(qdict, "hpfx0")) {
+ err.flags |= PCIE_AER_ERR_TLP_PRESENT;
+ }
+
+ err.header[0] = qdict_get_try_int(qdict, "tlph0", 0);
+ err.header[1] = qdict_get_try_int(qdict, "tlph1", 0);
+ err.header[2] = qdict_get_try_int(qdict, "tlph2", 0);
+ err.header[3] = qdict_get_try_int(qdict, "tlph3", 0);
+
+ err.prefix[0] = qdict_get_try_int(qdict, "hpfx0", 0);
+ err.prefix[1] = qdict_get_try_int(qdict, "hpfx1", 0);
+ err.prefix[2] = qdict_get_try_int(qdict, "hpfx2", 0);
+ err.prefix[3] = qdict_get_try_int(qdict, "hpfx3", 0);
+
+ pcie_aer_inject_error(dev, &err);
+ *ret_data = qobject_from_jsonf("{ 'domain': %d, 'bus': %d, 'devfn': %d }",
+ pci_find_domain(dev->bus),
+ pci_bus_num(dev->bus), dev->devfn);
+ assert(*ret_data);
+
+ return 0;
+}
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 965c754..ccb3d0e 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1168,6 +1168,28 @@ Push PCI express attention button
ETEXI
{
+ .name = "pcie_aer_inject_error",
+ .args_type = "advisory_non_fatal:-a,is_correctable:-c,"
+ "pci_addr:s,error_status:i,"
+ "tlph0:i?,tlph1:i?,tlph2:i?,tlph3:i?,"
+ "hpfx0:i?,hpfx1:i?,hpfx2:i?,hpfx3:i?",
+ .params = "[-a] [-c] [[<domain>:]<bus>:]<slot>.<func> "
+ "<error status:32bit> "
+ "[<tlp header:(32bit x 4)>] "
+ "[<tlp header prefix:(32bit x 4)>]",
+ .help = "inject pcie aer error "
+ "(use -a for advisory non fatal error) "
+ "(use -c for correctrable error)",
+ .user_print = pcie_aer_inject_error_print,
+ .mhandler.cmd_new = do_pcie_aer_inejct_error,
+ },
+
+STEXI
+@item pcie_abp
+Push PCI express attention button
+ETEXI
+
+ {
.name = "host_net_add",
.args_type = "device:s,opts:s?",
.params = "tap|user|socket|vde|dump [options]",
diff --git a/sysemu.h b/sysemu.h
index cca411d..2f7157c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -155,6 +155,11 @@ void pcie_attention_button_push_print(Monitor *mon, const QObject *data);
int pcie_attention_button_push(Monitor *mon, const QDict *qdict,
QObject **ret_data);
+/* pcie aer error injection */
+void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
+int do_pcie_aer_inejct_error(Monitor *mon,
+ const QDict *qdict, QObject **ret_data);
+
/* serial ports */
#define MAX_SERIAL_PORTS 4
--
1.7.1.1
^ permalink raw reply related [flat|nested] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 09/12] pcie/aer: glue aer error injection into qemu monitor
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 09/12] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
@ 2010-10-20 9:44 ` Michael S. Tsirkin
0 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-20 9:44 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Oct 20, 2010 at 05:18:58PM +0900, Isaku Yamahata wrote:
> introduce pcie_aer_inject_error command.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> Changes v3 -> v4:
> - s/PCIE_AER/PCIEAER/g for structure names.
> - compilation adjustment.
>
> Changes v2 -> v3:
> - compilation adjustment.
> ---
> hw/pcie_aer.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-monitor.hx | 22 ++++++++++++++
> sysemu.h | 5 +++
> 3 files changed, 111 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
> index b8cede3..459e0d7 100644
> --- a/hw/pcie_aer.c
> +++ b/hw/pcie_aer.c
> @@ -19,6 +19,8 @@
> */
>
> #include "sysemu.h"
> +#include "qemu-objects.h"
> +#include "monitor.h"
> #include "pci_bridge.h"
> #include "pcie.h"
> #include "msix.h"
> @@ -778,3 +780,85 @@ const VMStateDescription vmstate_pcie_aer_log = {
> }
> };
>
> +void pcie_aer_inject_error_print(Monitor *mon, const QObject *data)
> +{
> + QDict *qdict;
> + int devfn;
> + assert(qobject_type(data) == QTYPE_QDICT);
> + qdict = qobject_to_qdict(data);
> +
> + devfn = (int)qdict_get_int(qdict, "devfn");
> + monitor_printf(mon, "OK domain: %x, bus: %x devfn: %x.%x\n",
> + (int) qdict_get_int(qdict, "domain"),
> + (int) qdict_get_int(qdict, "bus"),
> + PCI_SLOT(devfn), PCI_FUNC(devfn));
> +}
> +
> +int do_pcie_aer_inejct_error(Monitor *mon,
> + const QDict *qdict, QObject **ret_data)
> +{
> + const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> + int dom;
> + int bus;
> + unsigned int slot;
> + unsigned int func;
> + PCIDevice *dev;
> + PCIEAERErr err;
> +
> + /* Ideally qdev device path should be used.
> + * However at the moment there is no reliable way to determine
> + * wheher a given qdev is pci device or not.
> + * so pci_addr is used.
> + */
> + if (pci_parse_devaddr(pci_addr, &dom, &bus, &slot, &func)) {
> + monitor_printf(mon, "invalid pci address %s\n", pci_addr);
> + return -1;
> + }
> + dev = pci_find_device(pci_find_root_bus(dom), bus, slot, func);
> + if (!dev) {
> + monitor_printf(mon, "device is not found. 0x%x:0x%x.0x%x\n",
> + bus, slot, func);
> + return -1;
> + }
> + if (!pci_is_express(dev)) {
> + monitor_printf(mon, "the device doesn't support pci express. "
> + "0x%x:0x%x.0x%x\n",
> + bus, slot, func);
> + return -1;
> + }
> +
> + err.status = qdict_get_int(qdict, "error_status");
> + err.source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn;
> +
> + err.flags = 0;
> + if (qdict_get_int(qdict, "is_correctable")) {
> + err.flags |= PCIE_AER_ERR_IS_CORRECTABLE;
> + }
> + if (qdict_get_int(qdict, "advisory_non_fatal")) {
> + err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY;
> + }
> + if (qdict_haskey(qdict, "tlph0")) {
> + err.flags |= PCIE_AER_ERR_HEADER_VALID;
> + }
> + if (qdict_haskey(qdict, "hpfx0")) {
> + err.flags |= PCIE_AER_ERR_TLP_PRESENT;
> + }
> +
> + err.header[0] = qdict_get_try_int(qdict, "tlph0", 0);
> + err.header[1] = qdict_get_try_int(qdict, "tlph1", 0);
> + err.header[2] = qdict_get_try_int(qdict, "tlph2", 0);
> + err.header[3] = qdict_get_try_int(qdict, "tlph3", 0);
> +
> + err.prefix[0] = qdict_get_try_int(qdict, "hpfx0", 0);
> + err.prefix[1] = qdict_get_try_int(qdict, "hpfx1", 0);
> + err.prefix[2] = qdict_get_try_int(qdict, "hpfx2", 0);
> + err.prefix[3] = qdict_get_try_int(qdict, "hpfx3", 0);
Can't we use a list or something? Sticking index in key name in this way
seems ugly.
> + pcie_aer_inject_error(dev, &err);
> + *ret_data = qobject_from_jsonf("{ 'domain': %d, 'bus': %d, 'devfn': %d }",
> + pci_find_domain(dev->bus),
> + pci_bus_num(dev->bus), dev->devfn);
> + assert(*ret_data);
> +
> + return 0;
> +}
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 965c754..ccb3d0e 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -1168,6 +1168,28 @@ Push PCI express attention button
> ETEXI
>
> {
> + .name = "pcie_aer_inject_error",
> + .args_type = "advisory_non_fatal:-a,is_correctable:-c,"
> + "pci_addr:s,error_status:i,"
> + "tlph0:i?,tlph1:i?,tlph2:i?,tlph3:i?,"
> + "hpfx0:i?,hpfx1:i?,hpfx2:i?,hpfx3:i?",
> + .params = "[-a] [-c] [[<domain>:]<bus>:]<slot>.<func> "
> + "<error status:32bit> "
> + "[<tlp header:(32bit x 4)>] "
> + "[<tlp header prefix:(32bit x 4)>]",
> + .help = "inject pcie aer error "
> + "(use -a for advisory non fatal error) "
> + "(use -c for correctrable error)",
> + .user_print = pcie_aer_inject_error_print,
> + .mhandler.cmd_new = do_pcie_aer_inejct_error,
> + },
> +
> +STEXI
> +@item pcie_abp
> +Push PCI express attention button
> +ETEXI
> +
> + {
> .name = "host_net_add",
> .args_type = "device:s,opts:s?",
> .params = "tap|user|socket|vde|dump [options]",
> diff --git a/sysemu.h b/sysemu.h
> index cca411d..2f7157c 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -155,6 +155,11 @@ void pcie_attention_button_push_print(Monitor *mon, const QObject *data);
> int pcie_attention_button_push(Monitor *mon, const QDict *qdict,
> QObject **ret_data);
>
> +/* pcie aer error injection */
> +void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
> +int do_pcie_aer_inejct_error(Monitor *mon,
> + const QDict *qdict, QObject **ret_data);
> +
> /* serial ports */
>
> #define MAX_SERIAL_PORTS 4
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 40+ messages in thread
* [Qemu-devel] [PATCH v6 10/12] ioh3420: support aer
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (8 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 09/12] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
@ 2010-10-20 8:18 ` Isaku Yamahata
2010-10-20 8:19 ` [Qemu-devel] [PATCH v6 11/12] x3130/upstream: " Isaku Yamahata
` (2 subsequent siblings)
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:18 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>
---
hw/ioh3420.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index 1f340d3..09c94f9 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -36,28 +36,63 @@
#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)
{
uint16_t sltctl =
pci_get_word(d->config + d->exp.exp_cap + PCI_EXP_SLTCTL);
+ uint32_t uncorsta =
+ pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_UNCOR_STATUS);
+ 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, sltctl);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len, uncorsta);
+ 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)
@@ -101,13 +136,15 @@ static int ioh3420_initfn(PCIDevice *d)
return rc;
}
pcie_cap_root_init(d);
- /* TODO: AER */
+ pcie_aer_init(d, IOH_EP_AER_OFFSET);
+ pcie_aer_root_init(d);
+ ioh3420_aer_vector_update(d);
return 0;
}
static int ioh3420_exitfn(PCIDevice *d)
{
- /* TODO: AER */
+ pcie_aer_exit(d);
msi_uninit(d);
pcie_cap_exit(d);
return pci_bridge_exitfn(d);
@@ -144,7 +181,8 @@ static const VMStateDescription vmstate_ioh3420 = {
.minimum_version_id_old = 1,
.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 +204,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] 40+ messages in thread* [Qemu-devel] [PATCH v6 11/12] x3130/upstream: support aer
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (9 preceding siblings ...)
2010-10-20 8:18 ` [Qemu-devel] [PATCH v6 10/12] ioh3420: support aer Isaku Yamahata
@ 2010-10-20 8:19 ` Isaku Yamahata
2010-10-20 8:19 ` [Qemu-devel] [PATCH v6 12/12] x3130/downstream: " Isaku Yamahata
2010-10-20 10:09 ` [Qemu-devel] Re: [PATCH v6 00/12] pcie port switch emulators Michael S. Tsirkin
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:19 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>
---
hw/xio3130_upstream.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
index d9d637f..36ed4b1 100644
--- a/hw/xio3130_upstream.c
+++ b/hw/xio3130_upstream.c
@@ -38,10 +38,13 @@
static void xio3130_upstream_write_config(PCIDevice *d, uint32_t address,
uint32_t val, int len)
{
+ uint32_t uncorsta =
+ pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_UNCOR_STATUS);
+
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, uncorsta);
}
static void xio3130_upstream_reset(DeviceState *qdev)
@@ -89,14 +92,14 @@ static int xio3130_upstream_initfn(PCIDevice *d)
pcie_cap_flr_init(d);
pcie_cap_deverr_init(d);
- /* TODO: AER */
+ pcie_aer_init(d, XIO3130_AER_OFFSET);
return 0;
}
static int xio3130_upstream_exitfn(PCIDevice *d)
{
- /* TODO: AER */
+ pcie_aer_exit(d);
msi_uninit(d);
pcie_cap_exit(d);
return pci_bridge_exitfn(d);
@@ -131,7 +134,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 +155,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] 40+ messages in thread* [Qemu-devel] [PATCH v6 12/12] x3130/downstream: support aer.
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (10 preceding siblings ...)
2010-10-20 8:19 ` [Qemu-devel] [PATCH v6 11/12] x3130/upstream: " Isaku Yamahata
@ 2010-10-20 8:19 ` Isaku Yamahata
2010-10-20 10:09 ` [Qemu-devel] Re: [PATCH v6 00/12] pcie port switch emulators Michael S. Tsirkin
12 siblings, 0 replies; 40+ messages in thread
From: Isaku Yamahata @ 2010-10-20 8:19 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>
---
hw/xio3130_downstream.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
index a44e188..9087c0b 100644
--- a/hw/xio3130_downstream.c
+++ b/hw/xio3130_downstream.c
@@ -40,12 +40,14 @@ static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address,
{
uint16_t sltctl =
pci_get_word(d->config + d->exp.exp_cap + PCI_EXP_SLTCTL);
+ uint32_t uncorsta =
+ pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_UNCOR_STATUS);
pci_bridge_write_config(d, address, val, len);
pcie_cap_flr_write_config(d, address, val, len);
pcie_cap_slot_write_config(d, address, val, len, sltctl);
msi_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len, uncorsta);
}
static void xio3130_downstream_reset(DeviceState *qdev)
@@ -100,14 +102,14 @@ static int xio3130_downstream_initfn(PCIDevice *d)
return rc;
}
pcie_cap_ari_init(d);
- /* TODO: AER */
+ pcie_aer_init(d, XIO3130_AER_OFFSET);
return 0;
}
static int xio3130_downstream_exitfn(PCIDevice *d)
{
- /* TODO: AER */
+ pcie_aer_exit(d);
msi_uninit(d);
pcie_cap_exit(d);
return pci_bridge_exitfn(d);
@@ -146,7 +148,8 @@ static const VMStateDescription vmstate_xio3130_downstream = {
.minimum_version_id_old = 1,
.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()
}
};
@@ -168,7 +171,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] 40+ messages in thread* [Qemu-devel] Re: [PATCH v6 00/12] pcie port switch emulators
2010-10-20 8:18 [Qemu-devel] [PATCH v6 00/12] pcie port switch emulators Isaku Yamahata
` (11 preceding siblings ...)
2010-10-20 8:19 ` [Qemu-devel] [PATCH v6 12/12] x3130/downstream: " Isaku Yamahata
@ 2010-10-20 10:09 ` Michael S. Tsirkin
12 siblings, 0 replies; 40+ messages in thread
From: Michael S. Tsirkin @ 2010-10-20 10:09 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Oct 20, 2010 at 05:18:49PM +0900, Isaku Yamahata wrote:
> Isaku Yamahata (12):
> pcie: comment on hpev_intx
> pci/bridge: fix pci_bridge_reset()
> pcie port: define struct PCIEPort/PCIESlot and helper functions
> ioh3420: pcie root port in X58 ioh
> x3130: pcie upstream port
> x3130: pcie downstream port
Applied patched 1-6 on my branch.
I'll give people a chance to comment and then we can merge.
^ permalink raw reply [flat|nested] 40+ messages in thread