* [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators
@ 2010-11-02 9:32 Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 1/6] pcie_regs.h: more constants Isaku Yamahata
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
This patch series is v7 of the pcie switch emulators.
Now the express part has been merged, so the aer part is left.
This patch series is for the master branch because the current pci branch
seems a bit behind.
. I didn't changed the way to pass argument by pcie_aer_inject_error
qmp command.
It is overkill to add qlist support for qmp only for this command.
. the glue patch for pushing attention button is dropped for now,
it will be addressed later as another step.
. All forward declarations can't be eliminated because some functions
calls each other.
Changes v6 -> v7:
- the glue patch for pushing attention button is dropped for now.
This will be addressed later.
- various clean up of aer helper functions.
changes v5 -> v6:
- dropped already merged patches.
- add comment on hpev_intx
- updated the bridge fix patch
- update the aer patch.
- reordered the patch series to remove the aer dependency.
change v4 -> v5:
- introduced pci_xxx_test_and_clear/set_mask
- eliminated xxx_notify(msi_trigger, int_level)
- eliminated FLR bits.
FLR will be addressed at the next phase.
changes v3 -> v4:
- introduced new pci config helper functions.(clear set bit)
- various clean up and some bug fixes.
- dropped pci_shift_xxx().
- dropped function pointerin pcie_aer.h
- dropped pci_exp_cap(), pcie_aer_cap().
- file rename (pcie_{root, upstream, downsatrem} => ioh33420, x3130).
changes v2 -> v3:
- msi: improved commant and simplified shift/ffs dance
- pci w1c config register framework
- split pcie.[ch] into pcie_regs.h, pcie.[ch] and pcie_aer.[ch]
- pcie, aer: many changes by following reviews.
changes v1 -> v2:
- update msi
- dropped already pushed out patches.
- added msix patches.
Isaku Yamahata (6):
pcie_regs.h: more constants
pcie/aer: helper functions for pcie aer capability
pcie/aer: glue aer error injection into qemu monitor
ioh3420: support aer
x3130/upstream: support aer
x3130/downstream: support aer.
Makefile.objs | 2 +-
hmp-commands.hx | 23 ++
hw/ioh3420.c | 51 +++-
hw/pcie.h | 14 +
hw/pcie_aer.c | 912 +++++++++++++++++++++++++++++++++++++++++++++++
hw/pcie_aer.h | 94 +++++
hw/pcie_regs.h | 2 +
hw/xio3130_downstream.c | 13 +-
hw/xio3130_upstream.c | 12 +-
qemu-common.h | 3 +
sysemu.h | 5 +
11 files changed, 1114 insertions(+), 17 deletions(-)
create mode 100644 hw/pcie_aer.c
create mode 100644 hw/pcie_aer.h
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v7 1/6] pcie_regs.h: more constants
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
@ 2010-11-02 9:32 ` Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
remove unnecessary sizeof.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pcie_regs.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/hw/pcie_regs.h b/hw/pcie_regs.h
index 3461a1b..4d123d9 100644
--- a/hw/pcie_regs.h
+++ b/hw/pcie_regs.h
@@ -94,7 +94,9 @@
#define PCI_ERR_CAP_MHRE 0x00000400
#define PCI_ERR_CAP_TLP 0x00000800
+#define PCI_ERR_HEADER_LOG_SIZE 16
#define PCI_ERR_TLP_PREFIX_LOG 0x38
+#define PCI_ERR_TLP_PREFIX_LOG_SIZE 16
#define PCI_SEC_STATUS_RCV_SYSTEM_ERROR 0x4000
--
1.7.1.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 1/6] pcie_regs.h: more constants Isaku Yamahata
@ 2010-11-02 9:32 ` Isaku Yamahata
2010-11-02 12:57 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-02 13:54 ` Michael S. Tsirkin
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 3/6] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
` (4 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 UTC (permalink / raw)
To: qemu-devel; +Cc: skandasa, adnan, wexu2, mst, yamahata, etmartin
This patch implements helper functions for pcie aer capability
which will be used later.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
Changes v6 -> v7:
- make error log buffer non-ringed buffer
- refactor pcie_aer_error_inject()
- clean up of pcie_aer_write_config()
- make pcie_aer_inject_error() return error
- remove AERMsgResult
- remove PCIEAERSeverity
- reduce forward declarations
- style clean ups
- remove paren for sizeof.
Chnages v5 -> v6:
- cleaned up pcie_aer_write_config().
- enum definition.
Changes v4 -> v5:
- use pci_xxx_test_and_xxx_mask()
- rewrote PCIDevice::written bits.
- eliminated pcie_aer_notify()
- introduced PCIExpressDevice::aer_intx
Changes v3 -> v4:
- various naming fixes.
- use pci bit operation helper function
- eliminate errmsg function pointer
- replace pci_shift_xxx() with PCIDevice::written
- uncorrect error status register.
- dropped pcie_aer_cap()
Changes v2 -> v3:
- split out from pcie.[ch] to pcie_aer.[ch] to make the files sorter.
- embeded PCIExpressDevice into PCIDevice.
- CodingStyle fix
---
Makefile.objs | 2 +-
hw/pcie.h | 14 +
hw/pcie_aer.c | 825 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pcie_aer.h | 94 +++++++
qemu-common.h | 3 +
5 files changed, 937 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 faf485e..c5919af 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -207,7 +207,7 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
# PCI watchdog devices
hw-obj-y += wdt_i6300esb.o
-hw-obj-y += pcie.o pcie_port.o
+hw-obj-y += pcie.o pcie_aer.o pcie_port.o
hw-obj-y += msix.o msi.o
# PCI network cards
diff --git a/hw/pcie.h b/hw/pcie.h
index 8708504..7baa813 100644
--- a/hw/pcie.h
+++ b/hw/pcie.h
@@ -24,6 +24,7 @@
#include "hw.h"
#include "pci_regs.h"
#include "pcie_regs.h"
+#include "pcie_aer.h"
typedef enum {
/* for attention and power indicator */
@@ -79,6 +80,19 @@ struct PCIExpressDevice {
Software Notification of Hot-Plug Events, an interrupt
is sent whenever the logical and of these conditions
transitions from false to true. */
+
+ /* AER */
+ uint16_t aer_cap;
+ PCIEAERLog aer_log;
+ unsigned int aer_intx; /* INTx for error reporting
+ * default is 0 = INTA#
+ * If the chip wants to use other interrupt
+ * line, initialize this member with the
+ * desired number.
+ * If the chip dynamically changes this member,
+ * also initialize it when loaded as
+ * appropreately.
+ */
};
/* PCI express capability helper functions */
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
new file mode 100644
index 0000000..84c3457
--- /dev/null
+++ b/hw/pcie_aer.c
@@ -0,0 +1,825 @@
+/*
+ * 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 void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
+
+/* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
+static uint32_t pcie_aer_uncor_default_severity(uint32_t status)
+{
+ switch (status) {
+ case PCI_ERR_UNC_INTN:
+ case PCI_ERR_UNC_DLP:
+ case PCI_ERR_UNC_SDN:
+ case PCI_ERR_UNC_RX_OVER:
+ case PCI_ERR_UNC_FCP:
+ case PCI_ERR_UNC_MALF_TLP:
+ return PCI_ERR_ROOT_CMD_FATAL_EN;
+ case PCI_ERR_UNC_POISON_TLP:
+ case PCI_ERR_UNC_ECRC:
+ case PCI_ERR_UNC_UNSUP:
+ case PCI_ERR_UNC_COMP_TIME:
+ case PCI_ERR_UNC_COMP_ABORT:
+ case PCI_ERR_UNC_UNX_COMP:
+ case PCI_ERR_UNC_ACSV:
+ case PCI_ERR_UNC_MCBTLP:
+ case PCI_ERR_UNC_ATOP_EBLOCKED:
+ case PCI_ERR_UNC_TLP_PRF_BLOCKED:
+ return PCI_ERR_ROOT_CMD_NONFATAL_EN;
+ default:
+ abort();
+ break;
+ }
+ return PCI_ERR_ROOT_CMD_FATAL_EN;
+}
+
+static int aer_log_add_err(PCIEAERLog *aer_log, const PCIEAERErr *err)
+{
+ if (aer_log->log_num == aer_log->log_max) {
+ return -1;
+ }
+ memcpy(&aer_log->log[aer_log->log_num], err, sizeof *err);
+ aer_log->log_num++;
+ return 0;
+}
+
+static void aer_log_del_err(PCIEAERLog *aer_log, PCIEAERErr *err)
+{
+ assert(aer_log->log_num);
+ *err = aer_log->log[0];
+ aer_log->log_num--;
+ memmove(&aer_log->log[0], &aer_log->log[1],
+ aer_log->log_num * sizeof *err);
+}
+
+static void aer_log_clear_all_err(PCIEAERLog *aer_log)
+{
+ aer_log->log_num = 0;
+}
+
+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);
+}
+
+static void pcie_aer_rebuild_uncor_status(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ PCIEAERLog *aer_log = &dev->exp.aer_log;
+
+ uint16_t i;
+ for (i = 0; i < aer_log->log_num; i++) {
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
+ dev->exp.aer_log.log[i].status);
+ }
+}
+
+void pcie_aer_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ uint32_t first_error = 1U << PCI_ERR_CAP_FEP(errcap);
+ uint32_t uncorsta = pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS);
+
+ /* uncorrectable error */
+ if (!(uncorsta & first_error)) {
+ /* the bit that corresponds to the first error is cleared */
+ pcie_aer_clear_error(dev);
+ } else if (errcap & PCI_ERR_CAP_MHRE) {
+ /* When PCI_ERR_CAP_MHRE is enabled and the first error isn't cleared
+ * nothing should happen. So we have to revert the modification to
+ * the register.
+ */
+ pcie_aer_rebuild_uncor_status(dev);
+ } else {
+ /* capability & control
+ * PCI_ERR_CAP_MHRE might be cleared, so clear of header log.
+ */
+ aer_log_clear_all_err(&dev->exp.aer_log);
+ }
+}
+
+/*
+ * return value:
+ * true: error message is sent up
+ * false: error message is masked
+ */
+static bool
+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);
+ uint16_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 false;
+ }
+
+ /* send up error message */
+ if (pci_is_express(dev) &&
+ pcie_cap_get_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
+ /* Root port notify system itself,
+ or send the error message to root complex event collector. */
+ /*
+ * if root port is associated to event collector, set
+ * parent_port = root complex event collector
+ * For now root complex event collector isn't supported.
+ */
+ parent_port = NULL;
+ } else {
+ parent_port = pci_bridge_get_device(dev->bus);
+ }
+ if (parent_port) {
+ if (!pci_is_express(parent_port)) {
+ /* just ignore it */
+ return false;
+ }
+ pcie_aer_msg(parent_port, msg);
+ }
+ return true;
+}
+
+/*
+ * return value:
+ * true: error message is sent up
+ * false: error message is masked
+ */
+static bool pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ uint16_t bridge_control = pci_get_word(dev->config + PCI_BRIDGE_CONTROL);
+
+ if (pcie_aer_msg_is_uncor(msg)) {
+ /* Received System Error */
+ pci_word_test_and_set_mask(dev->config + PCI_SEC_STATUS,
+ PCI_SEC_STATUS_RCV_SYSTEM_ERROR);
+ }
+
+ if (!(bridge_control & PCI_BRIDGE_CTL_SERR)) {
+ return false;
+ }
+ return true;
+}
+
+void pcie_aer_root_set_vector(PCIDevice *dev, unsigned int vector)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ assert(vector < PCI_ERR_ROOT_IRQ_MAX);
+ pci_long_test_and_clear_mask(aer_cap + PCI_ERR_ROOT_STATUS,
+ PCI_ERR_ROOT_IRQ);
+ pci_long_test_and_set_mask(aer_cap + PCI_ERR_ROOT_STATUS,
+ ((uint32_t)vector) << PCI_ERR_ROOT_IRQ_SHIFT);
+}
+
+static unsigned int pcie_aer_root_get_vector(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+ return (root_status & PCI_ERR_ROOT_IRQ) >> PCI_ERR_ROOT_IRQ_SHIFT;
+}
+
+/*
+ * return value:
+ * true: error message is sent up
+ * false: error message is masked
+ */
+static bool pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ bool msg_sent;
+ uint16_t cmd;
+ uint8_t *aer_cap;
+ uint32_t root_cmd;
+ uint32_t root_sta;
+ bool msi_trigger;
+
+ msg_sent = false;
+ cmd = pci_get_word(dev->config + PCI_COMMAND);
+ aer_cap = dev->config + dev->exp.aer_cap;
+ root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
+ root_sta = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+ msi_trigger = false;
+
+ if (cmd & PCI_COMMAND_SERR) {
+ /* System Error. Platform Specific */
+ /* msg_sent = true; */
+ }
+
+ /* Errro Message Received: Root Error Status register */
+ switch (msg->severity) {
+ case PCI_ERR_ROOT_CMD_COR_EN:
+ 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 PCI_ERR_ROOT_CMD_NONFATAL_EN:
+ 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 PCI_ERR_ROOT_CMD_FATAL_EN:
+ 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;
+ default:
+ abort();
+ 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);
+ }
+ msg_sent = true;
+ }
+ return msg_sent;
+}
+
+static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+{
+ uint8_t type;
+ bool msg_sent;
+
+ assert(pci_is_express(dev));
+
+ type = pcie_cap_get_type(dev);
+ if (type == PCI_EXP_TYPE_ROOT_PORT ||
+ type == PCI_EXP_TYPE_UPSTREAM ||
+ type == PCI_EXP_TYPE_DOWNSTREAM) {
+ msg_sent = pcie_aer_msg_vbridge(dev, msg);
+ if (!msg_sent) {
+ return;
+ }
+ }
+ msg_sent= pcie_aer_msg_alldev(dev, msg);
+ if (type == PCI_EXP_TYPE_ROOT_PORT && msg_sent) {
+ pcie_aer_msg_root_port(dev, msg);
+ }
+}
+
+static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint8_t first_bit = ffsl(err->status) - 1;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ int i;
+
+ assert(err->status);
+ assert(err->status & (err->status - 1));
+
+ errcap &= ~(PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP);
+ errcap |= PCI_ERR_CAP_FEP(first_bit);
+
+ if (err->flags & PCIE_AER_ERR_HEADER_VALID) {
+ for (i = 0; i < ARRAY_SIZE(err->header); ++i) {
+ /* 7.10.8 Header Log Register */
+ uint8_t *header_log =
+ aer_cap + PCI_ERR_HEADER_LOG + i * sizeof err->header[0];
+ cpu_to_be32wu((uint32_t*)header_log, err->header[i]);
+ }
+ } else {
+ assert(!(err->flags & PCIE_AER_ERR_TLP_PRESENT));
+ memset(aer_cap + PCI_ERR_HEADER_LOG, 0, PCI_ERR_HEADER_LOG_SIZE);
+ }
+
+ 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 */
+ uint8_t *prefix_log =
+ aer_cap + PCI_ERR_TLP_PREFIX_LOG + i * sizeof err->prefix[0];
+ cpu_to_be32wu((uint32_t*)prefix_log, err->prefix[i]);
+ }
+ errcap |= PCI_ERR_CAP_TLP;
+ } else {
+ memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0,
+ PCI_ERR_TLP_PREFIX_LOG_SIZE);
+ }
+ pci_set_long(aer_cap + PCI_ERR_CAP, errcap);
+}
+
+static void pcie_aer_clear_log(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+
+ pci_long_test_and_clear_mask(aer_cap + PCI_ERR_CAP,
+ PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP);
+
+ memset(aer_cap + PCI_ERR_HEADER_LOG, 0, PCI_ERR_HEADER_LOG_SIZE);
+ memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, PCI_ERR_TLP_PREFIX_LOG_SIZE);
+}
+
+static int pcie_aer_record_error(PCIDevice *dev,
+ const PCIEAERErr *err)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ int fep = PCI_ERR_CAP_FEP(errcap);
+
+ assert(err->status);
+ assert(err->status & (err->status - 1));
+
+ if (errcap & PCI_ERR_CAP_MHRE &&
+ (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1U << fep))) {
+ /* Not first error. queue error */
+ if (aer_log_add_err(&dev->exp.aer_log, err) < 0) {
+ /* overflow */
+ return -1;
+ }
+ return 0;
+ }
+
+ pcie_aer_update_log(dev, err);
+ return 0;
+}
+
+static void pcie_aer_clear_error(PCIDevice *dev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+ uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
+ PCIEAERLog *aer_log = &dev->exp.aer_log;
+ PCIEAERErr err;
+
+ if (!(errcap & PCI_ERR_CAP_MHRE) || !aer_log->log_num) {
+ pcie_aer_clear_log(dev);
+ return;
+ }
+
+ /*
+ * If more errors are queued, set corresponding bits in uncorrectable
+ * error status.
+ * We emulate uncorrectable error status register as W1CS.
+ * So set bit in uncorrectable error status here again for multiple
+ * error recording support.
+ *
+ * 6.2.4.2 Multiple Error Handling(Advanced Error Reporting Capability)
+ */
+ pcie_aer_rebuild_uncor_status(dev);
+
+ aer_log_del_err(aer_log, &err);
+ pcie_aer_update_log(dev, &err);
+}
+
+typedef struct PCIEAERInject {
+ PCIDevice *dev;
+ uint8_t *aer_cap;
+ const PCIEAERErr *err;
+ uint16_t devctl;
+ uint16_t devsta;
+ uint32_t status;
+ bool is_unsupported_request;
+ int *is_header_log_overflowed;
+ PCIEAERMsg *msg;
+} PCIEAERInject;
+
+static bool pcie_aer_inject_cor_error(PCIEAERInject *inj,
+ uint32_t uncor_status,
+ bool is_advisory_nonfatal)
+{
+ PCIDevice *dev = inj->dev;
+
+ inj->devsta |= PCI_EXP_DEVSTA_CED;
+ if (inj->is_unsupported_request) {
+ inj->devsta |= PCI_EXP_DEVSTA_URD;
+ }
+ pci_set_word(dev->config + dev->exp.exp_cap + PCI_EXP_DEVSTA, inj->devsta);
+
+ if (inj->aer_cap) {
+ uint32_t mask;
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_COR_STATUS,
+ inj->status);
+ mask = pci_get_long(inj->aer_cap + PCI_ERR_COR_MASK);
+ if (mask & inj->status) {
+ return false;
+ }
+ if (is_advisory_nonfatal) {
+ uint32_t uncor_mask =
+ pci_get_long(inj->aer_cap + PCI_ERR_UNCOR_MASK);
+ if (!(uncor_mask & uncor_status)) {
+ *inj->is_header_log_overflowed =
+ pcie_aer_record_error(dev, inj->err);
+ }
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
+ uncor_status);
+ }
+ }
+
+ if (inj->is_unsupported_request && !(inj->devctl & PCI_EXP_DEVCTL_URRE)) {
+ return false;
+ }
+ if (!(inj->devctl & PCI_EXP_DEVCTL_CERE)) {
+ return false;
+ }
+
+ inj->msg->severity = PCI_ERR_ROOT_CMD_COR_EN;
+ return true;
+}
+
+static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal)
+{
+ PCIDevice *dev = inj->dev;
+ uint16_t cmd;
+
+ if (is_fatal) {
+ inj->devsta |= PCI_EXP_DEVSTA_FED;
+ } else {
+ inj->devsta |= PCI_EXP_DEVSTA_NFED;
+ }
+ if (inj->is_unsupported_request) {
+ inj->devsta |= PCI_EXP_DEVSTA_URD;
+ }
+ pci_set_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVSTA, inj->devsta);
+
+ if (inj->aer_cap) {
+ uint32_t mask = pci_get_long(inj->aer_cap + PCI_ERR_UNCOR_MASK);
+ if (mask & inj->status) {
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
+ inj->status);
+ return false;
+ }
+
+ *inj->is_header_log_overflowed = pcie_aer_record_error(dev, inj->err);
+ pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
+ inj->status);
+ }
+
+ cmd = pci_get_word(dev->config + PCI_COMMAND);
+ if (inj->is_unsupported_request &&
+ !(inj->devctl & PCI_EXP_DEVCTL_URRE) && !(cmd & PCI_COMMAND_SERR)) {
+ return false;
+ }
+ if (is_fatal) {
+ if (!((cmd & PCI_COMMAND_SERR) ||
+ (inj->devctl & PCI_EXP_DEVCTL_FERE))) {
+ return false;
+ }
+ inj->msg->severity = PCI_ERR_ROOT_CMD_FATAL_EN;
+ } else {
+ if (!((cmd & PCI_COMMAND_SERR) ||
+ (inj->devctl & PCI_EXP_DEVCTL_NFERE))) {
+ return false;
+ }
+ inj->msg->severity = PCI_ERR_ROOT_CMD_NONFATAL_EN;
+ }
+ return true;
+}
+
+/*
+ * non-Function specific error must be recorded in all functions.
+ * It is the responsibility of the caller of this function.
+ * It is also caller's responsiblity to determine which function should
+ * report the rerror.
+ *
+ * 6.2.4 Error Logging
+ * 6.2.5 Sqeunce of Device Error Signaling and Logging Operations
+ * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging
+ * Operations
+ */
+int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
+{
+ uint8_t *aer_cap = NULL;
+ uint16_t devctl = 0;
+ uint16_t devsta = 0;
+ uint32_t status = err->status;
+ bool is_unsupported_request =
+ (!(err->flags & PCIE_AER_ERR_IS_CORRECTABLE) &&
+ err->status == PCI_ERR_UNC_UNSUP);
+ PCIEAERMsg msg;
+ int is_header_log_overflowed = 0;
+
+ if (!pci_is_express(dev)) {
+ return -1;
+ }
+
+ if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
+ status &= PCI_ERR_COR_SUPPORTED;
+ } else {
+ status &= PCI_ERR_UNC_SUPPORTED;
+ }
+
+ /* invalid status bit. one and only one bit must be set */
+ if (!status || (status & (status - 1))) {
+ return -1;
+ }
+
+ if (dev->exp.aer_cap) {
+ uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
+ aer_cap = dev->config + dev->exp.aer_cap;
+ devctl = pci_get_long(exp_cap + PCI_EXP_DEVCTL);
+ devsta = pci_get_long(exp_cap + PCI_EXP_DEVSTA);
+ }
+ if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
+ PCIEAERInject inj = {
+ .dev = dev,
+ .aer_cap = aer_cap,
+ .err = err,
+ .devctl = devctl,
+ .devsta = devsta,
+ .status = status,
+ .is_unsupported_request = is_unsupported_request,
+ .is_header_log_overflowed = &is_header_log_overflowed,
+ .msg = &msg,
+ };
+ if (!pcie_aer_inject_cor_error(&inj, 0, false)) {
+ return 0;
+ }
+ } else {
+ bool is_fatal =
+ pcie_aer_uncor_default_severity(status) ==
+ PCI_ERR_ROOT_CMD_FATAL_EN;
+
+ 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)) {
+ PCIEAERInject inj = {
+ .dev = dev,
+ .aer_cap = aer_cap,
+ .err = err,
+ .devctl = devctl,
+ .devsta = devsta,
+ .status = PCI_ERR_COR_ADV_NONFATAL,
+ .is_unsupported_request = is_unsupported_request,
+ .is_header_log_overflowed = &is_header_log_overflowed,
+ .msg = &msg,
+ };
+ if (!pcie_aer_inject_cor_error(&inj, status, true)) {
+ return 0;
+ }
+ } else {
+ PCIEAERInject inj = {
+ .dev = dev,
+ .aer_cap = aer_cap,
+ .err = err,
+ .devctl = devctl,
+ .devsta = devsta,
+ .status = status,
+ .is_unsupported_request = is_unsupported_request,
+ .is_header_log_overflowed = &is_header_log_overflowed,
+ .msg = &msg,
+ };
+ if (!pcie_aer_inject_uncor_error(&inj, is_fatal)) {
+ return 0;
+ }
+ }
+ }
+
+ /* 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,
+ };
+ int ret = pcie_aer_inject_error(dev, &header_log_overflow);
+ assert(!ret);
+ }
+ return 0;
+}
+
+void pcie_aer_root_init(PCIDevice *dev)
+{
+ uint16_t pos = dev->exp.aer_cap;
+
+ pci_set_long(dev->wmask + pos + PCI_ERR_ROOT_COMMAND,
+ PCI_ERR_ROOT_CMD_EN_MASK);
+ pci_set_long(dev->w1cmask + pos + PCI_ERR_ROOT_STATUS,
+ PCI_ERR_ROOT_STATUS_REPORT_MASK);
+}
+
+void pcie_aer_root_reset(PCIDevice *dev)
+{
+ uint8_t* aer_cap = dev->config + dev->exp.aer_cap;
+
+ pci_set_long(aer_cap + PCI_ERR_ROOT_COMMAND, 0);
+
+ /*
+ * Advanced Error Interrupt Message Number in Root Error Status Register
+ * must be updated by chip dependent code because it's chip dependent
+ * which number is used.
+ */
+}
+
+static bool pcie_aer_root_does_trigger(uint32_t cmd, uint32_t status)
+{
+ return
+ ((cmd & PCI_ERR_ROOT_CMD_COR_EN) && (status & PCI_ERR_ROOT_COR_RCV)) ||
+ ((cmd & PCI_ERR_ROOT_CMD_NONFATAL_EN) &&
+ (status & PCI_ERR_ROOT_NONFATAL_RCV)) ||
+ ((cmd & PCI_ERR_ROOT_CMD_FATAL_EN) &&
+ (status & PCI_ERR_ROOT_FATAL_RCV));
+}
+
+void pcie_aer_root_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len,
+ uint32_t root_cmd_prev)
+{
+ uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+
+ /* root command register */
+ uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
+ if (root_cmd & PCI_ERR_ROOT_CMD_EN_MASK) {
+ /* 6.2.4.1.2 Interrupt Generation */
+
+ /* 0 -> 1 */
+ uint32_t root_cmd_set = (root_cmd_prev ^ root_cmd) & root_cmd;
+ uint32_t root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
+
+ if (pci_msi_enabled(dev)) {
+ if (pcie_aer_root_does_trigger(root_cmd_set, root_status)) {
+ pci_msi_notify(dev, pcie_aer_root_get_vector(dev));
+ }
+ } else {
+ int int_level = pcie_aer_root_does_trigger(root_cmd, root_status);
+ qemu_set_irq(dev->irq[dev->exp.aer_intx], int_level);
+ }
+ }
+}
+
+static const VMStateDescription vmstate_pcie_aer_err = {
+ .name = "PCIE_AER_ERROR",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(status, PCIEAERErr),
+ VMSTATE_UINT16(source_id, PCIEAERErr),
+ VMSTATE_UINT16(flags, PCIEAERErr),
+ VMSTATE_UINT32_ARRAY(header, PCIEAERErr, 4),
+ VMSTATE_UINT32_ARRAY(prefix, PCIEAERErr, 4),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+#define VMSTATE_PCIE_AER_ERRS(_field, _state, _field_num, _vmsd, _type) { \
+ .name = (stringify(_field)), \
+ .version_id = 0, \
+ .num_offset = vmstate_offset_value(_state, _field_num, uint16_t), \
+ .size = sizeof(_type), \
+ .vmsd = &(_vmsd), \
+ .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
+ .offset = vmstate_offset_pointer(_state, _field, _type), \
+}
+
+const VMStateDescription vmstate_pcie_aer_log = {
+ .name = "PCIE_AER_ERROR_LOG",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT16(log_num, PCIEAERLog),
+ VMSTATE_UINT16(log_max, PCIEAERLog),
+ VMSTATE_PCIE_AER_ERRS(log, PCIEAERLog, log_num,
+ vmstate_pcie_aer_err, PCIEAERErr),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
diff --git a/hw/pcie_aer.h b/hw/pcie_aer.h
new file mode 100644
index 0000000..0f4b4eb
--- /dev/null
+++ b/hw/pcie_aer.h
@@ -0,0 +1,94 @@
+/*
+ * pcie_aer.h
+ *
+ * Copyright (c) 2010 Isaku Yamahata <yamahata at valinux co jp>
+ * VA Linux Systems Japan K.K.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_PCIE_AER_H
+#define QEMU_PCIE_AER_H
+
+#include "hw.h"
+
+/* definitions which PCIExpressDevice uses */
+
+/* AER log */
+struct PCIEAERLog {
+ /* This structure is saved/loaded.
+ So explicitly size them instead of unsigned int */
+ uint16_t log_num;
+
+#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 message: error signaling message has only error sevirity and
+ source id. See 2.2.8.3 error signaling messages */
+struct PCIEAERMsg {
+ /*
+ * PCI_ERR_ROOT_CMD_{COR, NONFATAL, FATAL}_EN
+ * = PCI_EXP_DEVCTL_{CERE, NFERE, FERE}
+ */
+ uint32_t severity;
+
+ uint16_t source_id; /* bdf */
+};
+
+static inline bool
+pcie_aer_msg_is_uncor(const PCIEAERMsg *msg)
+{
+ return msg->severity == PCI_ERR_ROOT_CMD_NONFATAL_EN ||
+ msg->severity == PCI_ERR_ROOT_CMD_FATAL_EN;
+}
+
+/* error */
+struct PCIEAERErr {
+ uint32_t status; /* error status bits */
+ uint16_t source_id; /* bdf */
+
+#define PCIE_AER_ERR_IS_CORRECTABLE 0x1 /* correctable/uncorrectable */
+#define PCIE_AER_ERR_MAYBE_ADVISORY 0x2 /* maybe advisory non-fatal */
+#define PCIE_AER_ERR_HEADER_VALID 0x4 /* TLP header is logged */
+#define PCIE_AER_ERR_TLP_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);
+
+/* aer root port */
+void pcie_aer_root_set_vector(PCIDevice *dev, unsigned int vector);
+void pcie_aer_root_init(PCIDevice *dev);
+void pcie_aer_root_reset(PCIDevice *dev);
+void pcie_aer_root_write_config(PCIDevice *dev,
+ uint32_t addr, uint32_t val, int len,
+ uint32_t root_cmd_prev);
+
+/* error injection */
+int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err);
+
+#endif /* QEMU_PCIE_AER_H */
diff --git a/qemu-common.h b/qemu-common.h
index 21fc3a5..eb4d9bd 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -239,6 +239,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] 15+ messages in thread
* [Qemu-devel] [PATCH v7 3/6] pcie/aer: glue aer error injection into qemu monitor
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 1/6] pcie_regs.h: more constants Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-11-02 9:32 ` Isaku Yamahata
2010-11-02 12:10 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 4/6] ioh3420: support aer Isaku Yamahata
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 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 v6 -> v7:
- check return value.
Changes v3 -> v4:
- s/PCIE_AER/PCIEAER/g for structure names.
- compilation adjustment.
Changes v2 -> v3:
- compilation adjustment.
---
hmp-commands.hx | 23 ++++++++++++++
hw/pcie_aer.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
sysemu.h | 5 +++
3 files changed, 115 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..02eae7d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -835,6 +835,29 @@ Hot remove PCI device.
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_aer_inject_error
+@findex pcie_aer_inject_error
+Inject PCIe AER error
+ETEXI
+
+ {
.name = "host_net_add",
.args_type = "device:s,opts:s?",
.params = "tap|user|socket|vde|dump [options]",
diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
index 84c3457..30e39fd 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"
@@ -823,3 +825,88 @@ 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;
+ int ret;
+
+ /* 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);
+
+ ret = pcie_aer_inject_error(dev, &err);
+ *ret_data = qobject_from_jsonf("{ 'domain': %d, 'bus': %d, 'devfn': %d "
+ "'ret': %d}",
+ pci_find_domain(dev->bus),
+ pci_bus_num(dev->bus), dev->devfn,
+ ret);
+ assert(*ret_data);
+
+ return 0;
+}
diff --git a/sysemu.h b/sysemu.h
index b81a70e..99c7909 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -151,6 +151,11 @@ 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 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] 15+ messages in thread
* [Qemu-devel] [PATCH v7 4/6] ioh3420: support aer
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
` (2 preceding siblings ...)
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 3/6] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
@ 2010-11-02 9:32 ` Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 5/6] x3130/upstream: " Isaku Yamahata
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 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 | 51 +++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index 3cc129f..4eecf2f 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -36,25 +36,59 @@
#define IOH_EP_EXP_OFFSET 0x90
#define IOH_EP_AER_OFFSET 0x100
+/*
+ * If two MSI vector are allocated, Advanced Error Interrupt Message Number
+ * is 1. otherwise 0.
+ * 17.12.5.10 RPERRSTS, 32:27 bit Advanced Error Interrupt Message Number.
+ */
+static uint8_t ioh3420_aer_vector(const PCIDevice *d)
+{
+ switch (msi_nr_vectors_allocated(d)) {
+ case 1:
+ return 0;
+ case 2:
+ return 1;
+ case 4:
+ case 8:
+ case 16:
+ case 32:
+ default:
+ break;
+ }
+ abort();
+ return 0;
+}
+
+static void ioh3420_aer_vector_update(PCIDevice *d)
+{
+ pcie_aer_root_set_vector(d, ioh3420_aer_vector(d));
+}
+
static void ioh3420_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len)
{
+ uint32_t root_cmd =
+ pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
+
pci_bridge_write_config(d, address, val, len);
msi_write_config(d, address, val, len);
+ ioh3420_aer_vector_update(d);
pcie_cap_slot_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len);
+ pcie_aer_root_write_config(d, address, val, len, root_cmd);
}
static void ioh3420_reset(DeviceState *qdev)
{
PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
msi_reset(d);
+ ioh3420_aer_vector_update(d);
pcie_cap_root_reset(d);
pcie_cap_deverr_reset(d);
pcie_cap_slot_reset(d);
+ pcie_aer_root_reset(d);
pci_bridge_reset(qdev);
pci_bridge_disable_base_limit(d);
- /* TODO: AER */
}
static int ioh3420_initfn(PCIDevice *d)
@@ -98,13 +132,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);
@@ -142,7 +178,8 @@ static const VMStateDescription vmstate_ioh3420 = {
.post_load = pcie_cap_slot_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCIE_DEVICE(port.br.dev, PCIESlot),
- /* TODO: AER */
+ VMSTATE_STRUCT(port.br.dev.exp.aer_log, PCIESlot, 0,
+ vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
}
};
@@ -164,7 +201,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] 15+ messages in thread
* [Qemu-devel] [PATCH v7 5/6] x3130/upstream: support aer
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
` (3 preceding siblings ...)
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 4/6] ioh3420: support aer Isaku Yamahata
@ 2010-11-02 9:32 ` Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 6/6] x3130/downstream: " Isaku Yamahata
2010-11-02 14:05 ` [Qemu-devel] Re: [PATCH v7 0/6] pcie port switch emulators Michael S. Tsirkin
6 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 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 | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
index d9d637f..29bc814 100644
--- a/hw/xio3130_upstream.c
+++ b/hw/xio3130_upstream.c
@@ -41,7 +41,7 @@ static void xio3130_upstream_write_config(PCIDevice *d, uint32_t address,
pci_bridge_write_config(d, address, val, len);
pcie_cap_flr_write_config(d, address, val, len);
msi_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len);
}
static void xio3130_upstream_reset(DeviceState *qdev)
@@ -89,14 +89,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 +131,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 +152,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] 15+ messages in thread
* [Qemu-devel] [PATCH v7 6/6] x3130/downstream: support aer.
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
` (4 preceding siblings ...)
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 5/6] x3130/upstream: " Isaku Yamahata
@ 2010-11-02 9:32 ` Isaku Yamahata
2010-11-02 14:05 ` [Qemu-devel] Re: [PATCH v7 0/6] pcie port switch emulators Michael S. Tsirkin
6 siblings, 0 replies; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-02 9:32 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 | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
index 854eba8..f90415f 100644
--- a/hw/xio3130_downstream.c
+++ b/hw/xio3130_downstream.c
@@ -42,7 +42,7 @@ static void xio3130_downstream_write_config(PCIDevice *d, uint32_t address,
pcie_cap_flr_write_config(d, address, val, len);
pcie_cap_slot_write_config(d, address, val, len);
msi_write_config(d, address, val, len);
- /* TODO: AER */
+ pcie_aer_write_config(d, address, val, len);
}
static void xio3130_downstream_reset(DeviceState *qdev)
@@ -97,14 +97,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);
@@ -144,7 +144,8 @@ static const VMStateDescription vmstate_xio3130_downstream = {
.post_load = pcie_cap_slot_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCIE_DEVICE(port.br.dev, PCIESlot),
- /* TODO: AER */
+ VMSTATE_STRUCT(port.br.dev.exp.aer_log, PCIESlot, 0,
+ vmstate_pcie_aer_log, PCIEAERLog),
VMSTATE_END_OF_LIST()
}
};
@@ -166,7 +167,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] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 3/6] pcie/aer: glue aer error injection into qemu monitor
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 3/6] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
@ 2010-11-02 12:10 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-11-02 12:10 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 02, 2010 at 06:32:49PM +0900, Isaku Yamahata wrote:
> introduce pcie_aer_inject_error command.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> Changes v6 -> v7:
> - check return value.
>
> Changes v3 -> v4:
> - s/PCIE_AER/PCIEAER/g for structure names.
> - compilation adjustment.
>
> Changes v2 -> v3:
> - compilation adjustment.
> ---
> hmp-commands.hx | 23 ++++++++++++++
> hw/pcie_aer.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> sysemu.h | 5 +++
> 3 files changed, 115 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 81999aa..02eae7d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -835,6 +835,29 @@ Hot remove PCI device.
> ETEXI
>
> {
> + .name = "pcie_aer_inject_error",
> + .args_type = "advisory_non_fatal:-a,is_correctable:-c,"
is_correctable->correctable?
> + "pci_addr:s,error_status:i,"
> + "tlph0:i?,tlph1:i?,tlph2:i?,tlph3:i?,"
> + "hpfx0:i?,hpfx1:i?,hpfx2:i?,hpfx3:i?",
Do the field names have to be so cryptic?
> + .params = "[-a] [-c] [[<domain>:]<bus>:]<slot>.<func> "
> + "<error status:32bit> "
Only 1 bit is valid though, right?
So I would say let's pass symbolic name, not raw bit.
> + "[<tlp header:(32bit x 4)>] "
> + "[<tlp header prefix:(32bit x 4)>]",
Maybe -advisory -correctable?
> + .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_aer_inject_error
> +@findex pcie_aer_inject_error
> +Inject PCIe AER error
> +ETEXI
> +
> + {
> .name = "host_net_add",
> .args_type = "device:s,opts:s?",
> .params = "tap|user|socket|vde|dump [options]",
> diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
> index 84c3457..30e39fd 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"
> @@ -823,3 +825,88 @@ 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;
> + int ret;
> +
> + /* 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.
> + */
This does not look like such a big deal, and tying interface
to such implementation detail seems wrong to me.
> + 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;
> + }
So tlph0 is for header and hpfx0 is for TLP_PRESENT?
Not the other way around?
> +
> + 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);
> +
> + ret = pcie_aer_inject_error(dev, &err);
> + *ret_data = qobject_from_jsonf("{ 'domain': %d, 'bus': %d, 'devfn': %d "
> + "'ret': %d}",
> + pci_find_domain(dev->bus),
> + pci_bus_num(dev->bus), dev->devfn,
> + ret);
> + assert(*ret_data);
> +
> + return 0;
> +}
> diff --git a/sysemu.h b/sysemu.h
> index b81a70e..99c7909 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -151,6 +151,11 @@ 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 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] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
@ 2010-11-02 12:57 ` Michael S. Tsirkin
2010-11-03 1:24 ` Isaku Yamahata
2010-11-15 7:35 ` Isaku Yamahata
2010-11-02 13:54 ` Michael S. Tsirkin
1 sibling, 2 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-11-02 12:57 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 02, 2010 at 06:32:48PM +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>
> ---
> Changes v6 -> v7:
> - make error log buffer non-ringed buffer
> - refactor pcie_aer_error_inject()
> - clean up of pcie_aer_write_config()
> - make pcie_aer_inject_error() return error
> - remove AERMsgResult
> - remove PCIEAERSeverity
> - reduce forward declarations
> - style clean ups
> - remove paren for sizeof.
>
> Chnages v5 -> v6:
> - cleaned up pcie_aer_write_config().
> - enum definition.
>
> Changes v4 -> v5:
> - use pci_xxx_test_and_xxx_mask()
> - rewrote PCIDevice::written bits.
> - eliminated pcie_aer_notify()
> - introduced PCIExpressDevice::aer_intx
>
> Changes v3 -> v4:
> - various naming fixes.
> - use pci bit operation helper function
> - eliminate errmsg function pointer
> - replace pci_shift_xxx() with PCIDevice::written
> - uncorrect error status register.
> - dropped pcie_aer_cap()
>
> Changes v2 -> v3:
> - split out from pcie.[ch] to pcie_aer.[ch] to make the files sorter.
> - embeded PCIExpressDevice into PCIDevice.
> - CodingStyle fix
> ---
> Makefile.objs | 2 +-
> hw/pcie.h | 14 +
> hw/pcie_aer.c | 825 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/pcie_aer.h | 94 +++++++
> qemu-common.h | 3 +
> 5 files changed, 937 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 faf485e..c5919af 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -207,7 +207,7 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
> # PCI watchdog devices
> hw-obj-y += wdt_i6300esb.o
>
> -hw-obj-y += pcie.o pcie_port.o
> +hw-obj-y += pcie.o pcie_aer.o pcie_port.o
> hw-obj-y += msix.o msi.o
>
> # PCI network cards
> diff --git a/hw/pcie.h b/hw/pcie.h
> index 8708504..7baa813 100644
> --- a/hw/pcie.h
> +++ b/hw/pcie.h
> @@ -24,6 +24,7 @@
> #include "hw.h"
> #include "pci_regs.h"
> #include "pcie_regs.h"
> +#include "pcie_aer.h"
>
> typedef enum {
> /* for attention and power indicator */
> @@ -79,6 +80,19 @@ struct PCIExpressDevice {
> Software Notification of Hot-Plug Events, an interrupt
> is sent whenever the logical and of these conditions
> transitions from false to true. */
> +
> + /* AER */
> + uint16_t aer_cap;
> + PCIEAERLog aer_log;
> + unsigned int aer_intx; /* INTx for error reporting
> + * default is 0 = INTA#
> + * If the chip wants to use other interrupt
> + * line, initialize this member with the
> + * desired number.
> + * If the chip dynamically changes this member,
> + * also initialize it when loaded as
> + * appropreately.
> + */
> };
>
> /* PCI express capability helper functions */
> diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c
> new file mode 100644
> index 0000000..84c3457
> --- /dev/null
> +++ b/hw/pcie_aer.c
> @@ -0,0 +1,825 @@
> +/*
> + * 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 void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
> +
so what exactly is the order of calls that makes
removing the forward declarations impractical?
Is there a recursive call? If yes I'd like to
see it documented much better.
> +/* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
> +static uint32_t pcie_aer_uncor_default_severity(uint32_t status)
> +{
> + switch (status) {
> + case PCI_ERR_UNC_INTN:
> + case PCI_ERR_UNC_DLP:
> + case PCI_ERR_UNC_SDN:
> + case PCI_ERR_UNC_RX_OVER:
> + case PCI_ERR_UNC_FCP:
> + case PCI_ERR_UNC_MALF_TLP:
> + return PCI_ERR_ROOT_CMD_FATAL_EN;
> + case PCI_ERR_UNC_POISON_TLP:
> + case PCI_ERR_UNC_ECRC:
> + case PCI_ERR_UNC_UNSUP:
> + case PCI_ERR_UNC_COMP_TIME:
> + case PCI_ERR_UNC_COMP_ABORT:
> + case PCI_ERR_UNC_UNX_COMP:
> + case PCI_ERR_UNC_ACSV:
> + case PCI_ERR_UNC_MCBTLP:
> + case PCI_ERR_UNC_ATOP_EBLOCKED:
> + case PCI_ERR_UNC_TLP_PRF_BLOCKED:
> + return PCI_ERR_ROOT_CMD_NONFATAL_EN;
> + default:
> + abort();
> + break;
> + }
> + return PCI_ERR_ROOT_CMD_FATAL_EN;
> +}
> +
> +static int aer_log_add_err(PCIEAERLog *aer_log, const PCIEAERErr *err)
> +{
> + if (aer_log->log_num == aer_log->log_max) {
> + return -1;
> + }
> + memcpy(&aer_log->log[aer_log->log_num], err, sizeof *err);
> + aer_log->log_num++;
> + return 0;
> +}
> +
> +static void aer_log_del_err(PCIEAERLog *aer_log, PCIEAERErr *err)
> +{
> + assert(aer_log->log_num);
> + *err = aer_log->log[0];
> + aer_log->log_num--;
> + memmove(&aer_log->log[0], &aer_log->log[1],
> + aer_log->log_num * sizeof *err);
> +}
> +
> +static void aer_log_clear_all_err(PCIEAERLog *aer_log)
> +{
> + aer_log->log_num = 0;
> +}
> +
> +void pcie_aer_init(PCIDevice *dev, uint16_t offset)
> +{
> + PCIExpressDevice *exp;
> +
> + pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR);
> + pci_word_test_and_set_mask(dev->w1cmask + PCI_STATUS,
> + PCI_STATUS_SIG_SYSTEM_ERROR);
> +
I would say we should just set these for all devices.
But if we do my concern is that guest might write 1 to this register,
then we migrate to an old guest and that one can not clear this bit.
Thoughts? Let's add a flag so old machine types can disable this?
Also - what about other bits in the status register?
> + 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;
> + }
So someone should set log_max beforehand? And an illegal value is
rounded down? How is this API supposed to be used?
> + 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
> + */
I am not sure I agree. There are different kinds of reset. Full system
reset certainly clears this register. When we do add a concept of a hot
reset (e.g. with FLR), we should also add stickymask in the device. But
for now let's keep it simple.
> + 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);
> +}
> +
> +static void pcie_aer_rebuild_uncor_status(PCIDevice *dev)
rebuild -> update
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + PCIEAERLog *aer_log = &dev->exp.aer_log;
> +
> + uint16_t i;
> + for (i = 0; i < aer_log->log_num; i++) {
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_UNCOR_STATUS,
> + dev->exp.aer_log.log[i].status);
> + }
> +}
> +
> +void pcie_aer_write_config(PCIDevice *dev,
> + uint32_t addr, uint32_t val, int len)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
> + uint32_t first_error = 1U << PCI_ERR_CAP_FEP(errcap);
> + uint32_t uncorsta = pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS);
> +
> + /* uncorrectable error */
> + if (!(uncorsta & first_error)) {
> + /* the bit that corresponds to the first error is cleared */
> + pcie_aer_clear_error(dev);
> + } else if (errcap & PCI_ERR_CAP_MHRE) {
> + /* When PCI_ERR_CAP_MHRE is enabled and the first error isn't cleared
> + * nothing should happen. So we have to revert the modification to
> + * the register.
> + */
> + pcie_aer_rebuild_uncor_status(dev);
> + } else {
> + /* capability & control
> + * PCI_ERR_CAP_MHRE might be cleared, so clear of header log.
> + */
> + aer_log_clear_all_err(&dev->exp.aer_log);
> + }
> +}
> +
> +/*
> + * return value:
> + * true: error message is sent up
> + * false: error message is masked
> + */
OK I figured the return value out but what does this do?
Specifically what does alldev stand for?
> +static bool
> +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);
> + uint16_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 false;
> + }
transmit1 and transmit2 just seem to confuse. And transmit 1 already
includes pcie_aer_msg_is_uncor condition.
How about open-coding it all:
if (pcie_aer_msg_is_uncor(msg) &&
(pci_get_word(dev->config + PCI_COMMAND) & PCI_COMMAND_SERR)) {
/* Signaled System Error */
pci_word_test_and_set_mask(dev->config + PCI_STATUS,
PCI_STATUS_SIG_SYSTEM_ERROR);
} else if (!(msg->severity & pci_get_word(dev->config +
dev->exp.exp_cap + PCI_EXP_DEVCTL))) {
return false;
}
Took me a while to figure this out. Maybe add this bit from the spec:
When Set, this bit enables reporting of Non-fatal and Fatal
errors detected by the Function to the Root Complex. Note that
errors are reported if enabled either through this bit or through
the PCI Express specific bits in the Device Control register (see
Section 7.8.4).
> +
> + /* send up error message */
> + if (pci_is_express(dev) &&
> + pcie_cap_get_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
> + /* Root port notify system itself,
> + or send the error message to root complex event collector. */
> + /*
> + * if root port is associated to event collector, set
> + * parent_port = root complex event collector
> + * For now root complex event collector isn't supported.
> + */
> + parent_port = NULL;
> + } else {
> + parent_port = pci_bridge_get_device(dev->bus);
> + }
> + if (parent_port) {
> + if (!pci_is_express(parent_port)) {
> + /* just ignore it */
> + return false;
> + }
> + pcie_aer_msg(parent_port, msg);
> + }
> + return true;
> +}
> +
> +/*
> + * return value:
> + * true: error message is sent up
> + * false: error message is masked
> + */
> +static bool pcie_aer_msg_vbridge(PCIDevice *dev, const PCIEAERMsg *msg)
> +{
> + uint16_t bridge_control = pci_get_word(dev->config + PCI_BRIDGE_CONTROL);
> +
> + if (pcie_aer_msg_is_uncor(msg)) {
> + /* Received System Error */
> + pci_word_test_and_set_mask(dev->config + PCI_SEC_STATUS,
> + PCI_SEC_STATUS_RCV_SYSTEM_ERROR);
> + }
> +
> + if (!(bridge_control & PCI_BRIDGE_CTL_SERR)) {
> + return false;
> + }
> + return true;
> +}
> +
> +void pcie_aer_root_set_vector(PCIDevice *dev, unsigned int vector)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + assert(vector < PCI_ERR_ROOT_IRQ_MAX);
> + pci_long_test_and_clear_mask(aer_cap + PCI_ERR_ROOT_STATUS,
> + PCI_ERR_ROOT_IRQ);
> + pci_long_test_and_set_mask(aer_cap + PCI_ERR_ROOT_STATUS,
> + ((uint32_t)vector) << PCI_ERR_ROOT_IRQ_SHIFT);
We don't really need the cast, do we?
> +}
> +
> +static unsigned int pcie_aer_root_get_vector(PCIDevice *dev)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + uint32_t root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
> + return (root_status & PCI_ERR_ROOT_IRQ) >> PCI_ERR_ROOT_IRQ_SHIFT;
> +}
> +
> +/*
> + * return value:
> + * true: error message is sent up
> + * false: error message is masked
> + */
> +static bool pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
> +{
> + bool msg_sent;
> + uint16_t cmd;
> + uint8_t *aer_cap;
> + uint32_t root_cmd;
> + uint32_t root_sta;
root_sta -> root_status
> + bool msi_trigger;
> +
> + msg_sent = false;
> + cmd = pci_get_word(dev->config + PCI_COMMAND);
> + aer_cap = dev->config + dev->exp.aer_cap;
> + root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
> + root_sta = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
> + msi_trigger = false;
Seems unused?
> +
> + if (cmd & PCI_COMMAND_SERR) {
> + /* System Error. Platform Specific */
> + /* msg_sent = true; */
What goes on here?
> + }
> +
> + /* Errro Message Received: Root Error Status register */
> + switch (msg->severity) {
> + case PCI_ERR_ROOT_CMD_COR_EN:
> + 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 PCI_ERR_ROOT_CMD_NONFATAL_EN:
> + 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 PCI_ERR_ROOT_CMD_FATAL_EN:
> + 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;
> + default:
> + abort();
> + 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);
> + }
> + msg_sent = true;
> + }
> + return msg_sent;
> +}
> +
> +static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
> +{
> + uint8_t type;
> + bool msg_sent;
> +
> + assert(pci_is_express(dev));
> +
> + type = pcie_cap_get_type(dev);
> + if (type == PCI_EXP_TYPE_ROOT_PORT ||
> + type == PCI_EXP_TYPE_UPSTREAM ||
> + type == PCI_EXP_TYPE_DOWNSTREAM) {
> + msg_sent = pcie_aer_msg_vbridge(dev, msg);
> + if (!msg_sent) {
> + return;
> + }
> + }
> + msg_sent= pcie_aer_msg_alldev(dev, msg);
space before =
> + if (type == PCI_EXP_TYPE_ROOT_PORT && msg_sent) {
> + pcie_aer_msg_root_port(dev, msg);
> + }
> +}
> +
> +static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + uint8_t first_bit = ffsl(err->status) - 1;
> + uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
> + int i;
> +
> + assert(err->status);
> + assert(err->status & (err->status - 1));
> +
> + errcap &= ~(PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP);
> + errcap |= PCI_ERR_CAP_FEP(first_bit);
> +
> + if (err->flags & PCIE_AER_ERR_HEADER_VALID) {
> + for (i = 0; i < ARRAY_SIZE(err->header); ++i) {
> + /* 7.10.8 Header Log Register */
> + uint8_t *header_log =
> + aer_cap + PCI_ERR_HEADER_LOG + i * sizeof err->header[0];
> + cpu_to_be32wu((uint32_t*)header_log, err->header[i]);
> + }
> + } else {
> + assert(!(err->flags & PCIE_AER_ERR_TLP_PRESENT));
> + memset(aer_cap + PCI_ERR_HEADER_LOG, 0, PCI_ERR_HEADER_LOG_SIZE);
> + }
> +
> + 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 */
> + uint8_t *prefix_log =
> + aer_cap + PCI_ERR_TLP_PREFIX_LOG + i * sizeof err->prefix[0];
> + cpu_to_be32wu((uint32_t*)prefix_log, err->prefix[i]);
> + }
> + errcap |= PCI_ERR_CAP_TLP;
> + } else {
> + memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0,
> + PCI_ERR_TLP_PREFIX_LOG_SIZE);
> + }
> + pci_set_long(aer_cap + PCI_ERR_CAP, errcap);
> +}
> +
> +static void pcie_aer_clear_log(PCIDevice *dev)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> +
> + pci_long_test_and_clear_mask(aer_cap + PCI_ERR_CAP,
> + PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP);
> +
> + memset(aer_cap + PCI_ERR_HEADER_LOG, 0, PCI_ERR_HEADER_LOG_SIZE);
> + memset(aer_cap + PCI_ERR_TLP_PREFIX_LOG, 0, PCI_ERR_TLP_PREFIX_LOG_SIZE);
> +}
> +
> +static int pcie_aer_record_error(PCIDevice *dev,
> + const PCIEAERErr *err)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
> + int fep = PCI_ERR_CAP_FEP(errcap);
> +
> + assert(err->status);
> + assert(err->status & (err->status - 1));
> +
> + if (errcap & PCI_ERR_CAP_MHRE &&
> + (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1U << fep))) {
> + /* Not first error. queue error */
> + if (aer_log_add_err(&dev->exp.aer_log, err) < 0) {
> + /* overflow */
> + return -1;
> + }
> + return 0;
> + }
> +
> + pcie_aer_update_log(dev, err);
> + return 0;
> +}
> +
> +static void pcie_aer_clear_error(PCIDevice *dev)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> + uint32_t errcap = pci_get_long(aer_cap + PCI_ERR_CAP);
> + PCIEAERLog *aer_log = &dev->exp.aer_log;
> + PCIEAERErr err;
> +
> + if (!(errcap & PCI_ERR_CAP_MHRE) || !aer_log->log_num) {
> + pcie_aer_clear_log(dev);
> + return;
> + }
> +
> + /*
> + * If more errors are queued, set corresponding bits in uncorrectable
> + * error status.
> + * We emulate uncorrectable error status register as W1CS.
> + * So set bit in uncorrectable error status here again for multiple
> + * error recording support.
> + *
> + * 6.2.4.2 Multiple Error Handling(Advanced Error Reporting Capability)
> + */
> + pcie_aer_rebuild_uncor_status(dev);
> +
> + aer_log_del_err(aer_log, &err);
> + pcie_aer_update_log(dev, &err);
> +}
> +
> +typedef struct PCIEAERInject {
> + PCIDevice *dev;
> + uint8_t *aer_cap;
> + const PCIEAERErr *err;
> + uint16_t devctl;
> + uint16_t devsta;
> + uint32_t status;
Find some better names to avoid confusion between devsta and status?
Also, status is always a single bit. Pass bit number?
> + bool is_unsupported_request;
is_unsupported_request -> unsupported_request
> + int *is_header_log_overflowed;
is_header_log_overflowed -> log_overflow
(also - why int?)
> + PCIEAERMsg *msg;
This is too hacky. Just make msg and log overflow
structure members, don't pass pointers.
> +} PCIEAERInject;
> +
> +static bool pcie_aer_inject_cor_error(PCIEAERInject *inj,
> + uint32_t uncor_status,
> + bool is_advisory_nonfatal)
> +{
> + PCIDevice *dev = inj->dev;
> +
> + inj->devsta |= PCI_EXP_DEVSTA_CED;
> + if (inj->is_unsupported_request) {
> + inj->devsta |= PCI_EXP_DEVSTA_URD;
> + }
> + pci_set_word(dev->config + dev->exp.exp_cap + PCI_EXP_DEVSTA, inj->devsta);
> +
> + if (inj->aer_cap) {
> + uint32_t mask;
> + pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_COR_STATUS,
> + inj->status);
> + mask = pci_get_long(inj->aer_cap + PCI_ERR_COR_MASK);
> + if (mask & inj->status) {
> + return false;
> + }
> + if (is_advisory_nonfatal) {
> + uint32_t uncor_mask =
> + pci_get_long(inj->aer_cap + PCI_ERR_UNCOR_MASK);
> + if (!(uncor_mask & uncor_status)) {
> + *inj->is_header_log_overflowed =
> + pcie_aer_record_error(dev, inj->err);
> + }
> + pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
> + uncor_status);
> + }
> + }
> +
> + if (inj->is_unsupported_request && !(inj->devctl & PCI_EXP_DEVCTL_URRE)) {
> + return false;
> + }
> + if (!(inj->devctl & PCI_EXP_DEVCTL_CERE)) {
> + return false;
> + }
> +
> + inj->msg->severity = PCI_ERR_ROOT_CMD_COR_EN;
> + return true;
> +}
> +
> +static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal)
> +{
> + PCIDevice *dev = inj->dev;
> + uint16_t cmd;
> +
> + if (is_fatal) {
> + inj->devsta |= PCI_EXP_DEVSTA_FED;
> + } else {
> + inj->devsta |= PCI_EXP_DEVSTA_NFED;
> + }
> + if (inj->is_unsupported_request) {
> + inj->devsta |= PCI_EXP_DEVSTA_URD;
> + }
> + pci_set_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVSTA, inj->devsta);
> +
> + if (inj->aer_cap) {
> + uint32_t mask = pci_get_long(inj->aer_cap + PCI_ERR_UNCOR_MASK);
> + if (mask & inj->status) {
> + pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
> + inj->status);
> + return false;
> + }
> +
> + *inj->is_header_log_overflowed = pcie_aer_record_error(dev, inj->err);
> + pci_long_test_and_set_mask(inj->aer_cap + PCI_ERR_UNCOR_STATUS,
> + inj->status);
> + }
> +
> + cmd = pci_get_word(dev->config + PCI_COMMAND);
> + if (inj->is_unsupported_request &&
> + !(inj->devctl & PCI_EXP_DEVCTL_URRE) && !(cmd & PCI_COMMAND_SERR)) {
> + return false;
> + }
> + if (is_fatal) {
> + if (!((cmd & PCI_COMMAND_SERR) ||
> + (inj->devctl & PCI_EXP_DEVCTL_FERE))) {
> + return false;
> + }
> + inj->msg->severity = PCI_ERR_ROOT_CMD_FATAL_EN;
> + } else {
> + if (!((cmd & PCI_COMMAND_SERR) ||
> + (inj->devctl & PCI_EXP_DEVCTL_NFERE))) {
> + return false;
> + }
> + inj->msg->severity = PCI_ERR_ROOT_CMD_NONFATAL_EN;
> + }
> + return true;
> +}
> +
> +/*
> + * non-Function specific error must be recorded in all functions.
> + * It is the responsibility of the caller of this function.
> + * It is also caller's responsiblity to determine which function should
> + * report the rerror.
> + *
> + * 6.2.4 Error Logging
> + * 6.2.5 Sqeunce of Device Error Signaling and Logging Operations
> + * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging
> + * Operations
> + */
> +int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
> +{
> + uint8_t *aer_cap = NULL;
> + uint16_t devctl = 0;
> + uint16_t devsta = 0;
> + uint32_t status = err->status;
> + bool is_unsupported_request =
> + (!(err->flags & PCIE_AER_ERR_IS_CORRECTABLE) &&
> + err->status == PCI_ERR_UNC_UNSUP);
> + PCIEAERMsg msg;
> + int is_header_log_overflowed = 0;
> +
> + if (!pci_is_express(dev)) {
> + return -1;
> + }
> +
> + if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
> + status &= PCI_ERR_COR_SUPPORTED;
> + } else {
> + status &= PCI_ERR_UNC_SUPPORTED;
> + }
> +
> + /* invalid status bit. one and only one bit must be set */
> + if (!status || (status & (status - 1))) {
> + return -1;
> + }
> +
> + if (dev->exp.aer_cap) {
> + uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
> + aer_cap = dev->config + dev->exp.aer_cap;
> + devctl = pci_get_long(exp_cap + PCI_EXP_DEVCTL);
> + devsta = pci_get_long(exp_cap + PCI_EXP_DEVSTA);
> + }
> + if (err->flags & PCIE_AER_ERR_IS_CORRECTABLE) {
> + PCIEAERInject inj = {
> + .dev = dev,
> + .aer_cap = aer_cap,
> + .err = err,
> + .devctl = devctl,
> + .devsta = devsta,
> + .status = status,
> + .is_unsupported_request = is_unsupported_request,
> + .is_header_log_overflowed = &is_header_log_overflowed,
> + .msg = &msg,
> + };
> + if (!pcie_aer_inject_cor_error(&inj, 0, false)) {
> + return 0;
> + }
> + } else {
> + bool is_fatal =
> + pcie_aer_uncor_default_severity(status) ==
> + PCI_ERR_ROOT_CMD_FATAL_EN;
> +
> + 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)) {
> + PCIEAERInject inj = {
> + .dev = dev,
> + .aer_cap = aer_cap,
> + .err = err,
> + .devctl = devctl,
> + .devsta = devsta,
> + .status = PCI_ERR_COR_ADV_NONFATAL,
> + .is_unsupported_request = is_unsupported_request,
> + .is_header_log_overflowed = &is_header_log_overflowed,
> + .msg = &msg,
> + };
> + if (!pcie_aer_inject_cor_error(&inj, status, true)) {
> + return 0;
> + }
> + } else {
> + PCIEAERInject inj = {
> + .dev = dev,
> + .aer_cap = aer_cap,
> + .err = err,
> + .devctl = devctl,
> + .devsta = devsta,
> + .status = status,
> + .is_unsupported_request = is_unsupported_request,
> + .is_header_log_overflowed = &is_header_log_overflowed,
> + .msg = &msg,
> + };
> + if (!pcie_aer_inject_uncor_error(&inj, is_fatal)) {
> + return 0;
> + }
> + }
> + }
> +
> + /* 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,
> + };
> + int ret = pcie_aer_inject_error(dev, &header_log_overflow);
> + assert(!ret);
> + }
> + return 0;
> +}
> +
> +void pcie_aer_root_init(PCIDevice *dev)
> +{
> + uint16_t pos = dev->exp.aer_cap;
> +
> + pci_set_long(dev->wmask + pos + PCI_ERR_ROOT_COMMAND,
> + PCI_ERR_ROOT_CMD_EN_MASK);
> + pci_set_long(dev->w1cmask + pos + PCI_ERR_ROOT_STATUS,
> + PCI_ERR_ROOT_STATUS_REPORT_MASK);
> +}
> +
> +void pcie_aer_root_reset(PCIDevice *dev)
> +{
> + uint8_t* aer_cap = dev->config + dev->exp.aer_cap;
> +
> + pci_set_long(aer_cap + PCI_ERR_ROOT_COMMAND, 0);
> +
> + /*
> + * Advanced Error Interrupt Message Number in Root Error Status Register
> + * must be updated by chip dependent code because it's chip dependent
> + * which number is used.
> + */
> +}
> +
> +static bool pcie_aer_root_does_trigger(uint32_t cmd, uint32_t status)
> +{
> + return
> + ((cmd & PCI_ERR_ROOT_CMD_COR_EN) && (status & PCI_ERR_ROOT_COR_RCV)) ||
> + ((cmd & PCI_ERR_ROOT_CMD_NONFATAL_EN) &&
> + (status & PCI_ERR_ROOT_NONFATAL_RCV)) ||
> + ((cmd & PCI_ERR_ROOT_CMD_FATAL_EN) &&
> + (status & PCI_ERR_ROOT_FATAL_RCV));
> +}
> +
> +void pcie_aer_root_write_config(PCIDevice *dev,
> + uint32_t addr, uint32_t val, int len,
> + uint32_t root_cmd_prev)
> +{
> + uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
> +
> + /* root command register */
> + uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
> + if (root_cmd & PCI_ERR_ROOT_CMD_EN_MASK) {
> + /* 6.2.4.1.2 Interrupt Generation */
> +
> + /* 0 -> 1 */
> + uint32_t root_cmd_set = (root_cmd_prev ^ root_cmd) & root_cmd;
> + uint32_t root_status = pci_get_long(aer_cap + PCI_ERR_ROOT_STATUS);
> +
> + if (pci_msi_enabled(dev)) {
> + if (pcie_aer_root_does_trigger(root_cmd_set, root_status)) {
> + pci_msi_notify(dev, pcie_aer_root_get_vector(dev));
> + }
> + } else {
> + int int_level = pcie_aer_root_does_trigger(root_cmd, root_status);
> + qemu_set_irq(dev->irq[dev->exp.aer_intx], int_level);
> + }
> + }
> +}
> +
> +static const VMStateDescription vmstate_pcie_aer_err = {
> + .name = "PCIE_AER_ERROR",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(status, PCIEAERErr),
> + VMSTATE_UINT16(source_id, PCIEAERErr),
> + VMSTATE_UINT16(flags, PCIEAERErr),
> + VMSTATE_UINT32_ARRAY(header, PCIEAERErr, 4),
> + VMSTATE_UINT32_ARRAY(prefix, PCIEAERErr, 4),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +#define VMSTATE_PCIE_AER_ERRS(_field, _state, _field_num, _vmsd, _type) { \
> + .name = (stringify(_field)), \
> + .version_id = 0, \
> + .num_offset = vmstate_offset_value(_state, _field_num, uint16_t), \
> + .size = sizeof(_type), \
> + .vmsd = &(_vmsd), \
> + .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
> + .offset = vmstate_offset_pointer(_state, _field, _type), \
> +}
> +
> +const VMStateDescription vmstate_pcie_aer_log = {
> + .name = "PCIE_AER_ERROR_LOG",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT16(log_num, PCIEAERLog),
> + VMSTATE_UINT16(log_max, PCIEAERLog),
> + VMSTATE_PCIE_AER_ERRS(log, PCIEAERLog, log_num,
> + vmstate_pcie_aer_err, PCIEAERErr),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> diff --git a/hw/pcie_aer.h b/hw/pcie_aer.h
> new file mode 100644
> index 0000000..0f4b4eb
> --- /dev/null
> +++ b/hw/pcie_aer.h
> @@ -0,0 +1,94 @@
> +/*
> + * pcie_aer.h
> + *
> + * Copyright (c) 2010 Isaku Yamahata <yamahata at valinux co jp>
> + * VA Linux Systems Japan K.K.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef QEMU_PCIE_AER_H
> +#define QEMU_PCIE_AER_H
> +
> +#include "hw.h"
> +
> +/* definitions which PCIExpressDevice uses */
> +
> +/* AER log */
> +struct PCIEAERLog {
> + /* This structure is saved/loaded.
> + So explicitly size them instead of unsigned int */
> + uint16_t log_num;
> +
> +#define PCIE_AER_LOG_MAX_DEFAULT 8
> +#define PCIE_AER_LOG_MAX_MAX 128 /* what is appropriate? */
MAX MAX?
> +#define PCIE_AER_LOG_MAX_UNSET 0xffff
> + uint16_t log_max;
> +
> + PCIEAERErr *log; /* ringed buffer */
> +};
> +
> +/* aer error message: error signaling message has only error sevirity and
> + source id. See 2.2.8.3 error signaling messages */
> +struct PCIEAERMsg {
> + /*
> + * PCI_ERR_ROOT_CMD_{COR, NONFATAL, FATAL}_EN
> + * = PCI_EXP_DEVCTL_{CERE, NFERE, FERE}
> + */
> + uint32_t severity;
> +
> + uint16_t source_id; /* bdf */
> +};
> +
> +static inline bool
> +pcie_aer_msg_is_uncor(const PCIEAERMsg *msg)
> +{
> + return msg->severity == PCI_ERR_ROOT_CMD_NONFATAL_EN ||
> + msg->severity == PCI_ERR_ROOT_CMD_FATAL_EN;
> +}
> +
> +/* error */
> +struct PCIEAERErr {
> + uint32_t status; /* error status bits */
> + uint16_t source_id; /* bdf */
> +
> +#define PCIE_AER_ERR_IS_CORRECTABLE 0x1 /* correctable/uncorrectable */
> +#define PCIE_AER_ERR_MAYBE_ADVISORY 0x2 /* maybe advisory non-fatal */
> +#define PCIE_AER_ERR_HEADER_VALID 0x4 /* TLP header is logged */
> +#define PCIE_AER_ERR_TLP_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);
> +
> +/* aer root port */
> +void pcie_aer_root_set_vector(PCIDevice *dev, unsigned int vector);
> +void pcie_aer_root_init(PCIDevice *dev);
> +void pcie_aer_root_reset(PCIDevice *dev);
> +void pcie_aer_root_write_config(PCIDevice *dev,
> + uint32_t addr, uint32_t val, int len,
> + uint32_t root_cmd_prev);
> +
> +/* error injection */
> +int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err);
> +
> +#endif /* QEMU_PCIE_AER_H */
> diff --git a/qemu-common.h b/qemu-common.h
> index 21fc3a5..eb4d9bd 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -239,6 +239,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] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
2010-11-02 12:57 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-11-02 13:54 ` Michael S. Tsirkin
1 sibling, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-11-02 13:54 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 02, 2010 at 06:32:48PM +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>
Applying this gets:
Applying: pcie/aer: helper functions for pcie aer capability
/scm/qemu/.git/rebase-apply/patch:95: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 0/6] pcie port switch emulators
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
` (5 preceding siblings ...)
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 6/6] x3130/downstream: " Isaku Yamahata
@ 2010-11-02 14:05 ` Michael S. Tsirkin
6 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-11-02 14:05 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 02, 2010 at 06:32:46PM +0900, Isaku Yamahata wrote:
> This patch series is v7 of the pcie switch emulators.
> Now the express part has been merged, so the aer part is left.
I think figuring out the FLR first is a good idea.
This would include sticky bit implementation and figuring
out how to initiate, and handle, different kinds of reset.
That in turn is helpful because a lot of AER is sticky.
> This patch series is for the master branch because the current pci branch
> seems a bit behind.
I made them identical now so it should not matter.
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-02 12:57 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-11-03 1:24 ` Isaku Yamahata
2010-11-03 7:30 ` Michael S. Tsirkin
2010-11-15 7:35 ` Isaku Yamahata
1 sibling, 1 reply; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-03 1:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 02, 2010 at 02:57:12PM +0200, Michael S. Tsirkin wrote:
> > +static void pcie_aer_clear_error(PCIDevice *dev);
> > +static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
> > +
>
> so what exactly is the order of calls that makes
> removing the forward declarations impractical?
> Is there a recursive call? If yes I'd like to
> see it documented much better.
Why do you think forward declaration is so bad?
I don't see any such consensus and I don't think they aren't
accused generally like goto.
Can you please elaborate why you're trying so hard to prevent it?
--
yamahata
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-03 1:24 ` Isaku Yamahata
@ 2010-11-03 7:30 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-11-03 7:30 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Wed, Nov 03, 2010 at 10:24:30AM +0900, Isaku Yamahata wrote:
> On Tue, Nov 02, 2010 at 02:57:12PM +0200, Michael S. Tsirkin wrote:
> > > +static void pcie_aer_clear_error(PCIDevice *dev);
> > > +static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
> > > +
> >
> > so what exactly is the order of calls that makes
> > removing the forward declarations impractical?
> > Is there a recursive call? If yes I'd like to
> > see it documented much better.
>
> Why do you think forward declaration is so bad?
> I don't see any such consensus and I don't think they aren't
> accused generally like goto.
> Can you please elaborate why you're trying so hard to prevent it?
Well no, I do not claim they are that bad. In my opinion avoiding them
just makes for a slightly better code usually:
- They make for code duplication where if you change
a function there's another place to edit.
- Just generally add more code.
- Avoiding forward declarations makes you put functions
in some sensible order.
- Also makes you avoid recursion where a loop will do.
But all this doesn't always apply. I was trying to understand
whether there's recursion here that I am missing.
If there is a comment might be helpful.
> --
> yamahata
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-02 12:57 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-03 1:24 ` Isaku Yamahata
@ 2010-11-15 7:35 ` Isaku Yamahata
2010-11-15 7:44 ` Michael S. Tsirkin
1 sibling, 1 reply; 15+ messages in thread
From: Isaku Yamahata @ 2010-11-15 7:35 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Tue, Nov 02, 2010 at 02:57:12PM +0200, Michael S. Tsirkin wrote:
> > + 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;
> > + }
>
> So someone should set log_max beforehand? And an illegal value is
> rounded down? How is this API supposed to be used?
It's qdev property. If log_max is too big, should it return error
instead of silently rounding down?
--
yamahata
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] Re: [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability
2010-11-15 7:35 ` Isaku Yamahata
@ 2010-11-15 7:44 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2010-11-15 7:44 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: skandasa, adnan, etmartin, qemu-devel, wexu2
On Mon, Nov 15, 2010 at 04:35:12PM +0900, Isaku Yamahata wrote:
> On Tue, Nov 02, 2010 at 02:57:12PM +0200, Michael S. Tsirkin wrote:
> > > + 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;
> > > + }
> >
> > So someone should set log_max beforehand? And an illegal value is
> > rounded down? How is this API supposed to be used?
>
> It's qdev property. If log_max is too big, should it return error
> instead of silently rounding down?
I guess so. Ideally the legal range would be part of qdev, maybe we
can add this support.
> --
> yamahata
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2010-11-15 7:46 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-02 9:32 [Qemu-devel] [PATCH v7 0/6] pcie port switch emulators Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 1/6] pcie_regs.h: more constants Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 2/6] pcie/aer: helper functions for pcie aer capability Isaku Yamahata
2010-11-02 12:57 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-03 1:24 ` Isaku Yamahata
2010-11-03 7:30 ` Michael S. Tsirkin
2010-11-15 7:35 ` Isaku Yamahata
2010-11-15 7:44 ` Michael S. Tsirkin
2010-11-02 13:54 ` Michael S. Tsirkin
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 3/6] pcie/aer: glue aer error injection into qemu monitor Isaku Yamahata
2010-11-02 12:10 ` [Qemu-devel] " Michael S. Tsirkin
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 4/6] ioh3420: support aer Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 5/6] x3130/upstream: " Isaku Yamahata
2010-11-02 9:32 ` [Qemu-devel] [PATCH v7 6/6] x3130/downstream: " Isaku Yamahata
2010-11-02 14:05 ` [Qemu-devel] Re: [PATCH v7 0/6] pcie port switch emulators Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).