* [PATCH 00/12] pci: Move and clean up monitor command code
@ 2022-11-28 8:01 Markus Armbruster
2022-11-28 8:01 ` [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
` (13 more replies)
0 siblings, 14 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
This is mainly about splitting off monitor-related code. There's also
a few UI fixes to HMP command pcie_aer_inject_error. One UI issue
remains: when the second argument is symbolic (found in table
pcie_aer_error_list[]), then any -c is silently ignored. Should it be
rejected? Should it override the value from the table?
Markus Armbruster (12):
pci: Clean up a few things checkpatch.pl would flag later on
pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c
pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c
pci: Make query-pci stub consistent with the real one
pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
pci: Deduplicate get_class_desc()
pci: Move pcibus_dev_print() to pci-hmp-cmds.c
pci: Fix silent truncation of pcie_aer_inject_error argument
pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c
pci: Inline do_pcie_aer_inject_error() into its only caller
pci: Rename hmp_pcie_aer_inject_error()'s local variable @err
pci: Improve do_pcie_aer_inject_error()'s error messages
hw/pci/pci-internal.h | 25 +++++
include/monitor/hmp.h | 1 +
include/sysemu/sysemu.h | 3 -
hw/pci/pci-hmp-cmds.c | 234 ++++++++++++++++++++++++++++++++++++++++
hw/pci/pci-qmp-cmds.c | 201 ++++++++++++++++++++++++++++++++++
hw/pci/pci-stub.c | 9 +-
hw/pci/pci.c | 226 +-------------------------------------
hw/pci/pcie_aer.c | 113 +------------------
monitor/hmp-cmds.c | 107 ------------------
hw/pci/meson.build | 2 +
10 files changed, 476 insertions(+), 445 deletions(-)
create mode 100644 hw/pci/pci-internal.h
create mode 100644 hw/pci/pci-hmp-cmds.c
create mode 100644 hw/pci/pci-qmp-cmds.c
--
2.37.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:27 ` Philippe Mathieu-Daudé
2022-11-28 8:01 ` [PATCH 02/12] pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c Markus Armbruster
` (12 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
Fix a few style violations so that checkpatch.pl won't complain when I
move this code.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2f450f6a72..53ed447115 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1851,10 +1851,12 @@ static PciBridgeInfo *qmp_query_pci_bridge(PCIDevice *dev, PCIBus *bus,
range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
if (dev->config[PCI_SECONDARY_BUS] != 0) {
- PCIBus *child_bus = pci_find_bus_nr(bus, dev->config[PCI_SECONDARY_BUS]);
+ PCIBus *child_bus = pci_find_bus_nr(bus,
+ dev->config[PCI_SECONDARY_BUS]);
if (child_bus) {
info->has_devices = true;
- info->devices = qmp_query_pci_devices(child_bus, dev->config[PCI_SECONDARY_BUS]);
+ info->devices = qmp_query_pci_devices(child_bus,
+ dev->config[PCI_SECONDARY_BUS]);
}
}
@@ -2612,8 +2614,9 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
pci_get_word(d->config + PCI_SUBSYSTEM_ID));
for (i = 0; i < PCI_NUM_REGIONS; i++) {
r = &d->io_regions[i];
- if (!r->size)
+ if (!r->size) {
continue;
+ }
monitor_printf(mon, "%*sbar %d: %s at 0x%"FMT_PCIBUS
" [0x%"FMT_PCIBUS"]\n",
indent, "",
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 02/12] pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
2022-11-28 8:01 ` [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:01 ` [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c Markus Armbruster
` (11 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-internal.h | 20 +++++
hw/pci/pci-qmp-cmds.c | 201 ++++++++++++++++++++++++++++++++++++++++++
hw/pci/pci.c | 188 +--------------------------------------
hw/pci/meson.build | 1 +
4 files changed, 226 insertions(+), 184 deletions(-)
create mode 100644 hw/pci/pci-internal.h
create mode 100644 hw/pci/pci-qmp-cmds.c
diff --git a/hw/pci/pci-internal.h b/hw/pci/pci-internal.h
new file mode 100644
index 0000000000..4903a26cbf
--- /dev/null
+++ b/hw/pci/pci-internal.h
@@ -0,0 +1,20 @@
+#ifndef HW_PCI_PCI_INTERNAL_H
+#define HW_PCI_PCI_INTERNAL_H
+
+#include "qemu/queue.h"
+
+typedef struct {
+ uint16_t class;
+ const char *desc;
+ const char *fw_name;
+ uint16_t fw_ign_bits;
+} pci_class_desc;
+
+typedef QLIST_HEAD(, PCIHostState) PCIHostStateList;
+
+extern PCIHostStateList pci_host_bridges;
+
+const pci_class_desc *get_class_desc(int class);
+PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
+
+#endif
diff --git a/hw/pci/pci-qmp-cmds.c b/hw/pci/pci-qmp-cmds.c
new file mode 100644
index 0000000000..4b434dcfda
--- /dev/null
+++ b/hw/pci/pci-qmp-cmds.c
@@ -0,0 +1,201 @@
+/*
+ * QMP commands related to PCI
+ *
+ * Copyright (c) 2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
+#include "pci-internal.h"
+#include "qapi/qapi-commands-pci.h"
+
+static PciDeviceInfoList *qmp_query_pci_devices(PCIBus *bus, int bus_num);
+
+static PciMemoryRegionList *qmp_query_pci_regions(const PCIDevice *dev)
+{
+ PciMemoryRegionList *head = NULL, **tail = &head;
+ int i;
+
+ for (i = 0; i < PCI_NUM_REGIONS; i++) {
+ const PCIIORegion *r = &dev->io_regions[i];
+ PciMemoryRegion *region;
+
+ if (!r->size) {
+ continue;
+ }
+
+ region = g_malloc0(sizeof(*region));
+
+ if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+ region->type = g_strdup("io");
+ } else {
+ region->type = g_strdup("memory");
+ region->has_prefetch = true;
+ region->prefetch = !!(r->type & PCI_BASE_ADDRESS_MEM_PREFETCH);
+ region->has_mem_type_64 = true;
+ region->mem_type_64 = !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
+ }
+
+ region->bar = i;
+ region->address = r->addr;
+ region->size = r->size;
+
+ QAPI_LIST_APPEND(tail, region);
+ }
+
+ return head;
+}
+
+static PciBridgeInfo *qmp_query_pci_bridge(PCIDevice *dev, PCIBus *bus,
+ int bus_num)
+{
+ PciBridgeInfo *info;
+ PciMemoryRange *range;
+
+ info = g_new0(PciBridgeInfo, 1);
+
+ info->bus = g_new0(PciBusInfo, 1);
+ info->bus->number = dev->config[PCI_PRIMARY_BUS];
+ info->bus->secondary = dev->config[PCI_SECONDARY_BUS];
+ info->bus->subordinate = dev->config[PCI_SUBORDINATE_BUS];
+
+ range = info->bus->io_range = g_new0(PciMemoryRange, 1);
+ range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
+ range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
+
+ range = info->bus->memory_range = g_new0(PciMemoryRange, 1);
+ range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+ range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
+
+ range = info->bus->prefetchable_range = g_new0(PciMemoryRange, 1);
+ range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+ range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+
+ if (dev->config[PCI_SECONDARY_BUS] != 0) {
+ PCIBus *child_bus = pci_find_bus_nr(bus,
+ dev->config[PCI_SECONDARY_BUS]);
+ if (child_bus) {
+ info->has_devices = true;
+ info->devices = qmp_query_pci_devices(child_bus,
+ dev->config[PCI_SECONDARY_BUS]);
+ }
+ }
+
+ return info;
+}
+
+static PciDeviceInfo *qmp_query_pci_device(PCIDevice *dev, PCIBus *bus,
+ int bus_num)
+{
+ const pci_class_desc *desc;
+ PciDeviceInfo *info;
+ uint8_t type;
+ int class;
+
+ info = g_new0(PciDeviceInfo, 1);
+ info->bus = bus_num;
+ info->slot = PCI_SLOT(dev->devfn);
+ info->function = PCI_FUNC(dev->devfn);
+
+ info->class_info = g_new0(PciDeviceClass, 1);
+ class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
+ info->class_info->q_class = class;
+ desc = get_class_desc(class);
+ if (desc->desc) {
+ info->class_info->has_desc = true;
+ info->class_info->desc = g_strdup(desc->desc);
+ }
+
+ info->id = g_new0(PciDeviceId, 1);
+ info->id->vendor = pci_get_word(dev->config + PCI_VENDOR_ID);
+ info->id->device = pci_get_word(dev->config + PCI_DEVICE_ID);
+ info->regions = qmp_query_pci_regions(dev);
+ info->qdev_id = g_strdup(dev->qdev.id ? dev->qdev.id : "");
+
+ info->irq_pin = dev->config[PCI_INTERRUPT_PIN];
+ if (dev->config[PCI_INTERRUPT_PIN] != 0) {
+ info->has_irq = true;
+ info->irq = dev->config[PCI_INTERRUPT_LINE];
+ }
+
+ type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
+ if (type == PCI_HEADER_TYPE_BRIDGE) {
+ info->has_pci_bridge = true;
+ info->pci_bridge = qmp_query_pci_bridge(dev, bus, bus_num);
+ } else if (type == PCI_HEADER_TYPE_NORMAL) {
+ info->id->has_subsystem = info->id->has_subsystem_vendor = true;
+ info->id->subsystem = pci_get_word(dev->config + PCI_SUBSYSTEM_ID);
+ info->id->subsystem_vendor =
+ pci_get_word(dev->config + PCI_SUBSYSTEM_VENDOR_ID);
+ } else if (type == PCI_HEADER_TYPE_CARDBUS) {
+ info->id->has_subsystem = info->id->has_subsystem_vendor = true;
+ info->id->subsystem = pci_get_word(dev->config + PCI_CB_SUBSYSTEM_ID);
+ info->id->subsystem_vendor =
+ pci_get_word(dev->config + PCI_CB_SUBSYSTEM_VENDOR_ID);
+ }
+
+ return info;
+}
+
+static PciDeviceInfoList *qmp_query_pci_devices(PCIBus *bus, int bus_num)
+{
+ PciDeviceInfoList *head = NULL, **tail = &head;
+ PCIDevice *dev;
+ int devfn;
+
+ for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
+ dev = bus->devices[devfn];
+ if (dev) {
+ QAPI_LIST_APPEND(tail, qmp_query_pci_device(dev, bus, bus_num));
+ }
+ }
+
+ return head;
+}
+
+static PciInfo *qmp_query_pci_bus(PCIBus *bus, int bus_num)
+{
+ PciInfo *info = NULL;
+
+ bus = pci_find_bus_nr(bus, bus_num);
+ if (bus) {
+ info = g_malloc0(sizeof(*info));
+ info->bus = bus_num;
+ info->devices = qmp_query_pci_devices(bus, bus_num);
+ }
+
+ return info;
+}
+
+PciInfoList *qmp_query_pci(Error **errp)
+{
+ PciInfoList *head = NULL, **tail = &head;
+ PCIHostState *host_bridge;
+
+ QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
+ QAPI_LIST_APPEND(tail,
+ qmp_query_pci_bus(host_bridge->bus,
+ pci_bus_num(host_bridge->bus)));
+ }
+
+ return head;
+}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 53ed447115..81ffc74925 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -47,8 +47,8 @@
#include "hw/hotplug.h"
#include "hw/boards.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-pci.h"
#include "qemu/cutils.h"
+#include "pci-internal.h"
//#define DEBUG_PCI
#ifdef DEBUG_PCI
@@ -234,7 +234,6 @@ static const TypeInfo cxl_bus_info = {
.class_init = pcie_bus_class_init,
};
-static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
static void pci_update_mappings(PCIDevice *d);
static void pci_irq_handler(void *opaque, int irq_num, int level);
static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
@@ -243,7 +242,7 @@ static void pci_del_option_rom(PCIDevice *pdev);
static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
-static QLIST_HEAD(, PCIHostState) pci_host_bridges;
+PCIHostStateList pci_host_bridges;
int pci_bar(PCIDevice *d, int reg)
{
@@ -1662,13 +1661,6 @@ int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
/***********************************************************/
/* monitor info on PCI */
-typedef struct {
- uint16_t class;
- const char *desc;
- const char *fw_name;
- uint16_t fw_ign_bits;
-} pci_class_desc;
-
static const pci_class_desc pci_class_descriptions[] =
{
{ 0x0001, "VGA controller", "display"},
@@ -1776,7 +1768,7 @@ void pci_for_each_device(PCIBus *bus, int bus_num,
}
}
-static const pci_class_desc *get_class_desc(int class)
+const pci_class_desc *get_class_desc(int class)
{
const pci_class_desc *desc;
@@ -1788,178 +1780,6 @@ static const pci_class_desc *get_class_desc(int class)
return desc;
}
-static PciDeviceInfoList *qmp_query_pci_devices(PCIBus *bus, int bus_num);
-
-static PciMemoryRegionList *qmp_query_pci_regions(const PCIDevice *dev)
-{
- PciMemoryRegionList *head = NULL, **tail = &head;
- int i;
-
- for (i = 0; i < PCI_NUM_REGIONS; i++) {
- const PCIIORegion *r = &dev->io_regions[i];
- PciMemoryRegion *region;
-
- if (!r->size) {
- continue;
- }
-
- region = g_malloc0(sizeof(*region));
-
- if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
- region->type = g_strdup("io");
- } else {
- region->type = g_strdup("memory");
- region->has_prefetch = true;
- region->prefetch = !!(r->type & PCI_BASE_ADDRESS_MEM_PREFETCH);
- region->has_mem_type_64 = true;
- region->mem_type_64 = !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
- }
-
- region->bar = i;
- region->address = r->addr;
- region->size = r->size;
-
- QAPI_LIST_APPEND(tail, region);
- }
-
- return head;
-}
-
-static PciBridgeInfo *qmp_query_pci_bridge(PCIDevice *dev, PCIBus *bus,
- int bus_num)
-{
- PciBridgeInfo *info;
- PciMemoryRange *range;
-
- info = g_new0(PciBridgeInfo, 1);
-
- info->bus = g_new0(PciBusInfo, 1);
- info->bus->number = dev->config[PCI_PRIMARY_BUS];
- info->bus->secondary = dev->config[PCI_SECONDARY_BUS];
- info->bus->subordinate = dev->config[PCI_SUBORDINATE_BUS];
-
- range = info->bus->io_range = g_new0(PciMemoryRange, 1);
- range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO);
- range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO);
-
- range = info->bus->memory_range = g_new0(PciMemoryRange, 1);
- range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
- range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY);
-
- range = info->bus->prefetchable_range = g_new0(PciMemoryRange, 1);
- range->base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
- range->limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
-
- if (dev->config[PCI_SECONDARY_BUS] != 0) {
- PCIBus *child_bus = pci_find_bus_nr(bus,
- dev->config[PCI_SECONDARY_BUS]);
- if (child_bus) {
- info->has_devices = true;
- info->devices = qmp_query_pci_devices(child_bus,
- dev->config[PCI_SECONDARY_BUS]);
- }
- }
-
- return info;
-}
-
-static PciDeviceInfo *qmp_query_pci_device(PCIDevice *dev, PCIBus *bus,
- int bus_num)
-{
- const pci_class_desc *desc;
- PciDeviceInfo *info;
- uint8_t type;
- int class;
-
- info = g_new0(PciDeviceInfo, 1);
- info->bus = bus_num;
- info->slot = PCI_SLOT(dev->devfn);
- info->function = PCI_FUNC(dev->devfn);
-
- info->class_info = g_new0(PciDeviceClass, 1);
- class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
- info->class_info->q_class = class;
- desc = get_class_desc(class);
- if (desc->desc) {
- info->class_info->has_desc = true;
- info->class_info->desc = g_strdup(desc->desc);
- }
-
- info->id = g_new0(PciDeviceId, 1);
- info->id->vendor = pci_get_word(dev->config + PCI_VENDOR_ID);
- info->id->device = pci_get_word(dev->config + PCI_DEVICE_ID);
- info->regions = qmp_query_pci_regions(dev);
- info->qdev_id = g_strdup(dev->qdev.id ? dev->qdev.id : "");
-
- info->irq_pin = dev->config[PCI_INTERRUPT_PIN];
- if (dev->config[PCI_INTERRUPT_PIN] != 0) {
- info->has_irq = true;
- info->irq = dev->config[PCI_INTERRUPT_LINE];
- }
-
- type = dev->config[PCI_HEADER_TYPE] & ~PCI_HEADER_TYPE_MULTI_FUNCTION;
- if (type == PCI_HEADER_TYPE_BRIDGE) {
- info->has_pci_bridge = true;
- info->pci_bridge = qmp_query_pci_bridge(dev, bus, bus_num);
- } else if (type == PCI_HEADER_TYPE_NORMAL) {
- info->id->has_subsystem = info->id->has_subsystem_vendor = true;
- info->id->subsystem = pci_get_word(dev->config + PCI_SUBSYSTEM_ID);
- info->id->subsystem_vendor =
- pci_get_word(dev->config + PCI_SUBSYSTEM_VENDOR_ID);
- } else if (type == PCI_HEADER_TYPE_CARDBUS) {
- info->id->has_subsystem = info->id->has_subsystem_vendor = true;
- info->id->subsystem = pci_get_word(dev->config + PCI_CB_SUBSYSTEM_ID);
- info->id->subsystem_vendor =
- pci_get_word(dev->config + PCI_CB_SUBSYSTEM_VENDOR_ID);
- }
-
- return info;
-}
-
-static PciDeviceInfoList *qmp_query_pci_devices(PCIBus *bus, int bus_num)
-{
- PciDeviceInfoList *head = NULL, **tail = &head;
- PCIDevice *dev;
- int devfn;
-
- for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) {
- dev = bus->devices[devfn];
- if (dev) {
- QAPI_LIST_APPEND(tail, qmp_query_pci_device(dev, bus, bus_num));
- }
- }
-
- return head;
-}
-
-static PciInfo *qmp_query_pci_bus(PCIBus *bus, int bus_num)
-{
- PciInfo *info = NULL;
-
- bus = pci_find_bus_nr(bus, bus_num);
- if (bus) {
- info = g_malloc0(sizeof(*info));
- info->bus = bus_num;
- info->devices = qmp_query_pci_devices(bus, bus_num);
- }
-
- return info;
-}
-
-PciInfoList *qmp_query_pci(Error **errp)
-{
- PciInfoList *head = NULL, **tail = &head;
- PCIHostState *host_bridge;
-
- QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
- QAPI_LIST_APPEND(tail,
- qmp_query_pci_bus(host_bridge->bus,
- pci_bus_num(host_bridge->bus)));
- }
-
- return head;
-}
-
/* Initialize a PCI NIC. */
PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
const char *default_model,
@@ -2108,7 +1928,7 @@ static bool pci_root_bus_in_range(PCIBus *bus, int bus_num)
return false;
}
-static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num)
+PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num)
{
PCIBus *sec;
diff --git a/hw/pci/meson.build b/hw/pci/meson.build
index 5aff7ed1c6..40721f1514 100644
--- a/hw/pci/meson.build
+++ b/hw/pci/meson.build
@@ -5,6 +5,7 @@ pci_ss.add(files(
'pci.c',
'pci_bridge.c',
'pci_host.c',
+ 'pci-qmp-cmds.c',
'pcie_sriov.c',
'shpc.c',
'slotid_cap.c'
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
2022-11-28 8:01 ` [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
2022-11-28 8:01 ` [PATCH 02/12] pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:27 ` Philippe Mathieu-Daudé
2022-11-28 12:09 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 04/12] pci: Make query-pci stub consistent with the real one Markus Armbruster
` (10 subsequent siblings)
13 siblings, 2 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
This moves these commands from MAINTAINERS section "Human
Monitor (HMP)" to "PCI".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-hmp-cmds.c | 126 ++++++++++++++++++++++++++++++++++++++++++
monitor/hmp-cmds.c | 107 -----------------------------------
hw/pci/meson.build | 1 +
3 files changed, 127 insertions(+), 107 deletions(-)
create mode 100644 hw/pci/pci-hmp-cmds.c
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
new file mode 100644
index 0000000000..5adfe4f57f
--- /dev/null
+++ b/hw/pci/pci-hmp-cmds.c
@@ -0,0 +1,126 @@
+/*
+ * HMP commands related to PCI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-pci.h"
+
+static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
+{
+ PciMemoryRegionList *region;
+
+ monitor_printf(mon, " Bus %2" PRId64 ", ", dev->bus);
+ monitor_printf(mon, "device %3" PRId64 ", function %" PRId64 ":\n",
+ dev->slot, dev->function);
+ monitor_printf(mon, " ");
+
+ if (dev->class_info->has_desc) {
+ monitor_puts(mon, dev->class_info->desc);
+ } else {
+ monitor_printf(mon, "Class %04" PRId64, dev->class_info->q_class);
+ }
+
+ monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
+ dev->id->vendor, dev->id->device);
+ if (dev->id->has_subsystem_vendor && dev->id->has_subsystem) {
+ monitor_printf(mon, " PCI subsystem %04" PRIx64 ":%04" PRIx64 "\n",
+ dev->id->subsystem_vendor, dev->id->subsystem);
+ }
+
+ if (dev->has_irq) {
+ monitor_printf(mon, " IRQ %" PRId64 ", pin %c\n",
+ dev->irq, (char)('A' + dev->irq_pin - 1));
+ }
+
+ if (dev->has_pci_bridge) {
+ monitor_printf(mon, " BUS %" PRId64 ".\n",
+ dev->pci_bridge->bus->number);
+ monitor_printf(mon, " secondary bus %" PRId64 ".\n",
+ dev->pci_bridge->bus->secondary);
+ monitor_printf(mon, " subordinate bus %" PRId64 ".\n",
+ dev->pci_bridge->bus->subordinate);
+
+ monitor_printf(mon, " IO range [0x%04"PRIx64", 0x%04"PRIx64"]\n",
+ dev->pci_bridge->bus->io_range->base,
+ dev->pci_bridge->bus->io_range->limit);
+
+ monitor_printf(mon,
+ " memory range [0x%08"PRIx64", 0x%08"PRIx64"]\n",
+ dev->pci_bridge->bus->memory_range->base,
+ dev->pci_bridge->bus->memory_range->limit);
+
+ monitor_printf(mon, " prefetchable memory range "
+ "[0x%08"PRIx64", 0x%08"PRIx64"]\n",
+ dev->pci_bridge->bus->prefetchable_range->base,
+ dev->pci_bridge->bus->prefetchable_range->limit);
+ }
+
+ for (region = dev->regions; region; region = region->next) {
+ uint64_t addr, size;
+
+ addr = region->value->address;
+ size = region->value->size;
+
+ monitor_printf(mon, " BAR%" PRId64 ": ", region->value->bar);
+
+ if (!strcmp(region->value->type, "io")) {
+ monitor_printf(mon, "I/O at 0x%04" PRIx64
+ " [0x%04" PRIx64 "].\n",
+ addr, addr + size - 1);
+ } else {
+ monitor_printf(mon, "%d bit%s memory at 0x%08" PRIx64
+ " [0x%08" PRIx64 "].\n",
+ region->value->mem_type_64 ? 64 : 32,
+ region->value->prefetch ? " prefetchable" : "",
+ addr, addr + size - 1);
+ }
+ }
+
+ monitor_printf(mon, " id \"%s\"\n", dev->qdev_id);
+
+ if (dev->has_pci_bridge) {
+ if (dev->pci_bridge->has_devices) {
+ PciDeviceInfoList *cdev;
+ for (cdev = dev->pci_bridge->devices; cdev; cdev = cdev->next) {
+ hmp_info_pci_device(mon, cdev->value);
+ }
+ }
+ }
+}
+
+void hmp_info_pci(Monitor *mon, const QDict *qdict)
+{
+ PciInfoList *info_list, *info;
+ Error *err = NULL;
+
+ info_list = qmp_query_pci(&err);
+ if (err) {
+ monitor_printf(mon, "PCI devices not supported\n");
+ error_free(err);
+ return;
+ }
+
+ for (info = info_list; info; info = info->next) {
+ PciDeviceInfoList *dev;
+
+ for (dev = info->value->devices; dev; dev = dev->next) {
+ hmp_info_pci_device(mon, dev->value);
+ }
+ }
+
+ qapi_free_PciInfoList(info_list);
+}
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 01b789a79e..03eae10663 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -37,7 +37,6 @@
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-commands-net.h"
-#include "qapi/qapi-commands-pci.h"
#include "qapi/qapi-commands-rocker.h"
#include "qapi/qapi-commands-run-state.h"
#include "qapi/qapi-commands-stats.h"
@@ -701,89 +700,6 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
qapi_free_BalloonInfo(info);
}
-static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
-{
- PciMemoryRegionList *region;
-
- monitor_printf(mon, " Bus %2" PRId64 ", ", dev->bus);
- monitor_printf(mon, "device %3" PRId64 ", function %" PRId64 ":\n",
- dev->slot, dev->function);
- monitor_printf(mon, " ");
-
- if (dev->class_info->has_desc) {
- monitor_puts(mon, dev->class_info->desc);
- } else {
- monitor_printf(mon, "Class %04" PRId64, dev->class_info->q_class);
- }
-
- monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
- dev->id->vendor, dev->id->device);
- if (dev->id->has_subsystem_vendor && dev->id->has_subsystem) {
- monitor_printf(mon, " PCI subsystem %04" PRIx64 ":%04" PRIx64 "\n",
- dev->id->subsystem_vendor, dev->id->subsystem);
- }
-
- if (dev->has_irq) {
- monitor_printf(mon, " IRQ %" PRId64 ", pin %c\n",
- dev->irq, (char)('A' + dev->irq_pin - 1));
- }
-
- if (dev->has_pci_bridge) {
- monitor_printf(mon, " BUS %" PRId64 ".\n",
- dev->pci_bridge->bus->number);
- monitor_printf(mon, " secondary bus %" PRId64 ".\n",
- dev->pci_bridge->bus->secondary);
- monitor_printf(mon, " subordinate bus %" PRId64 ".\n",
- dev->pci_bridge->bus->subordinate);
-
- monitor_printf(mon, " IO range [0x%04"PRIx64", 0x%04"PRIx64"]\n",
- dev->pci_bridge->bus->io_range->base,
- dev->pci_bridge->bus->io_range->limit);
-
- monitor_printf(mon,
- " memory range [0x%08"PRIx64", 0x%08"PRIx64"]\n",
- dev->pci_bridge->bus->memory_range->base,
- dev->pci_bridge->bus->memory_range->limit);
-
- monitor_printf(mon, " prefetchable memory range "
- "[0x%08"PRIx64", 0x%08"PRIx64"]\n",
- dev->pci_bridge->bus->prefetchable_range->base,
- dev->pci_bridge->bus->prefetchable_range->limit);
- }
-
- for (region = dev->regions; region; region = region->next) {
- uint64_t addr, size;
-
- addr = region->value->address;
- size = region->value->size;
-
- monitor_printf(mon, " BAR%" PRId64 ": ", region->value->bar);
-
- if (!strcmp(region->value->type, "io")) {
- monitor_printf(mon, "I/O at 0x%04" PRIx64
- " [0x%04" PRIx64 "].\n",
- addr, addr + size - 1);
- } else {
- monitor_printf(mon, "%d bit%s memory at 0x%08" PRIx64
- " [0x%08" PRIx64 "].\n",
- region->value->mem_type_64 ? 64 : 32,
- region->value->prefetch ? " prefetchable" : "",
- addr, addr + size - 1);
- }
- }
-
- monitor_printf(mon, " id \"%s\"\n", dev->qdev_id);
-
- if (dev->has_pci_bridge) {
- if (dev->pci_bridge->has_devices) {
- PciDeviceInfoList *cdev;
- for (cdev = dev->pci_bridge->devices; cdev; cdev = cdev->next) {
- hmp_info_pci_device(mon, cdev->value);
- }
- }
- }
-}
-
static int hmp_info_pic_foreach(Object *obj, void *opaque)
{
InterruptStatsProvider *intc;
@@ -810,29 +726,6 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
hmp_info_pic_foreach, mon);
}
-void hmp_info_pci(Monitor *mon, const QDict *qdict)
-{
- PciInfoList *info_list, *info;
- Error *err = NULL;
-
- info_list = qmp_query_pci(&err);
- if (err) {
- monitor_printf(mon, "PCI devices not supported\n");
- error_free(err);
- return;
- }
-
- for (info = info_list; info; info = info->next) {
- PciDeviceInfoList *dev;
-
- for (dev = info->value->devices; dev; dev = dev->next) {
- hmp_info_pci_device(mon, dev->value);
- }
- }
-
- qapi_free_PciInfoList(info_list);
-}
-
void hmp_info_tpm(Monitor *mon, const QDict *qdict)
{
#ifdef CONFIG_TPM
diff --git a/hw/pci/meson.build b/hw/pci/meson.build
index 40721f1514..e42a133f3a 100644
--- a/hw/pci/meson.build
+++ b/hw/pci/meson.build
@@ -20,3 +20,4 @@ softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
+softmmu_ss.add(files('pci-hmp-cmds.c'))
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 04/12] pci: Make query-pci stub consistent with the real one
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (2 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-29 12:03 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI Markus Armbruster
` (9 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
QMP query-pci and HMP info pci can behave differently when there are
no PCI devices. They can report nothing, like this:
qemu-system-aarch64 -S -M spitz -display none -monitor stdio
QEMU 7.1.91 monitor - type 'help' for more information
(qemu) info pci
Or they can fail, like this:
qemu-system-microblaze -M petalogix-s3adsp1800 -display none -monitor stdio
QEMU 7.1.91 monitor - type 'help' for more information
(qemu) info pci
PCI devices not supported
They fail when none of the target's machines supports PCI, i.e. when
we're using qmp_query_pci() from hw/pci/pci-stub.c.
The error is not useful, and reporting nothing makes sense, so do that
in pci-stub.c, too.
Now qmp_query_pci() can't fail anymore. Drop the dead error handling
from hmp_info_pci().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-hmp-cmds.c | 8 +-------
hw/pci/pci-stub.c | 3 ---
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index 5adfe4f57f..e915fb9fe7 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -105,14 +105,8 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
void hmp_info_pci(Monitor *mon, const QDict *qdict)
{
PciInfoList *info_list, *info;
- Error *err = NULL;
- info_list = qmp_query_pci(&err);
- if (err) {
- monitor_printf(mon, "PCI devices not supported\n");
- error_free(err);
- return;
- }
+ info_list = qmp_query_pci(&error_abort);
for (info = info_list; info; info = info->next) {
PciDeviceInfoList *dev;
diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index 3a027c42e4..f29ecc999e 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -21,9 +21,7 @@
#include "qemu/osdep.h"
#include "sysemu/sysemu.h"
#include "monitor/monitor.h"
-#include "qapi/error.h"
#include "qapi/qapi-commands-pci.h"
-#include "qapi/qmp/qerror.h"
#include "hw/pci/pci.h"
#include "hw/pci/msi.h"
#include "hw/pci/msix.h"
@@ -33,7 +31,6 @@ bool pci_available;
PciInfoList *qmp_query_pci(Error **errp)
{
- error_setg(errp, QERR_UNSUPPORTED);
return NULL;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (3 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 04/12] pci: Make query-pci stub consistent with the real one Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:26 ` Philippe Mathieu-Daudé
2022-11-28 12:24 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 06/12] pci: Deduplicate get_class_desc() Markus Armbruster
` (8 subsequent siblings)
13 siblings, 2 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
!CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-stub.c | 5 +++++
hw/pci/meson.build | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index f29ecc999e..01d20a2f67 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -21,6 +21,7 @@
#include "qemu/osdep.h"
#include "sysemu/sysemu.h"
#include "monitor/monitor.h"
+#include "monitor/hmp.h"
#include "qapi/qapi-commands-pci.h"
#include "hw/pci/pci.h"
#include "hw/pci/msi.h"
@@ -34,6 +35,10 @@ PciInfoList *qmp_query_pci(Error **errp)
return NULL;
}
+void hmp_info_pci(Monitor *mon, const QDict *qdict)
+{
+}
+
void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
{
monitor_printf(mon, "PCI devices not supported\n");
diff --git a/hw/pci/meson.build b/hw/pci/meson.build
index e42a133f3a..4fcd888b27 100644
--- a/hw/pci/meson.build
+++ b/hw/pci/meson.build
@@ -5,6 +5,7 @@ pci_ss.add(files(
'pci.c',
'pci_bridge.c',
'pci_host.c',
+ 'pci-hmp-cmds.c',
'pci-qmp-cmds.c',
'pcie_sriov.c',
'shpc.c',
@@ -20,4 +21,3 @@ softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
-softmmu_ss.add(files('pci-hmp-cmds.c'))
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 06/12] pci: Deduplicate get_class_desc()
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (4 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:01 ` [PATCH 07/12] pci: Move pcibus_dev_print() to pci-hmp-cmds.c Markus Armbruster
` (7 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
pcibus_dev_print() contains a copy of get_class_desc(). Call the
function instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 81ffc74925..6711a75098 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2409,15 +2409,12 @@ uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id)
static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
{
PCIDevice *d = (PCIDevice *)dev;
- const pci_class_desc *desc;
+ int class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+ const pci_class_desc *desc = get_class_desc(class);
char ctxt[64];
PCIIORegion *r;
- int i, class;
+ int i;
- class = pci_get_word(d->config + PCI_CLASS_DEVICE);
- desc = pci_class_descriptions;
- while (desc->desc && class != desc->class)
- desc++;
if (desc->desc) {
snprintf(ctxt, sizeof(ctxt), "%s", desc->desc);
} else {
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 07/12] pci: Move pcibus_dev_print() to pci-hmp-cmds.c
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (5 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 06/12] pci: Deduplicate get_class_desc() Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:24 ` Philippe Mathieu-Daudé
2022-11-28 8:01 ` [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument Markus Armbruster
` (6 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
This method is for HMP command "info qtree".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-internal.h | 1 +
hw/pci/pci-hmp-cmds.c | 38 ++++++++++++++++++++++++++++++++++++++
hw/pci/pci.c | 38 --------------------------------------
3 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/hw/pci/pci-internal.h b/hw/pci/pci-internal.h
index 4903a26cbf..3199615e50 100644
--- a/hw/pci/pci-internal.h
+++ b/hw/pci/pci-internal.h
@@ -16,5 +16,6 @@ extern PCIHostStateList pci_host_bridges;
const pci_class_desc *get_class_desc(int class);
PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
+void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
#endif
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index e915fb9fe7..417f1ca607 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -14,8 +14,10 @@
*/
#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
#include "monitor/hmp.h"
#include "monitor/monitor.h"
+#include "pci-internal.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-pci.h"
@@ -118,3 +120,39 @@ void hmp_info_pci(Monitor *mon, const QDict *qdict)
qapi_free_PciInfoList(info_list);
}
+
+void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
+{
+ PCIDevice *d = (PCIDevice *)dev;
+ int class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+ const pci_class_desc *desc = get_class_desc(class);
+ char ctxt[64];
+ PCIIORegion *r;
+ int i;
+
+ if (desc->desc) {
+ snprintf(ctxt, sizeof(ctxt), "%s", desc->desc);
+ } else {
+ snprintf(ctxt, sizeof(ctxt), "Class %04x", class);
+ }
+
+ monitor_printf(mon, "%*sclass %s, addr %02x:%02x.%x, "
+ "pci id %04x:%04x (sub %04x:%04x)\n",
+ indent, "", ctxt, pci_dev_bus_num(d),
+ PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
+ pci_get_word(d->config + PCI_VENDOR_ID),
+ pci_get_word(d->config + PCI_DEVICE_ID),
+ pci_get_word(d->config + PCI_SUBSYSTEM_VENDOR_ID),
+ pci_get_word(d->config + PCI_SUBSYSTEM_ID));
+ for (i = 0; i < PCI_NUM_REGIONS; i++) {
+ r = &d->io_regions[i];
+ if (!r->size) {
+ continue;
+ }
+ monitor_printf(mon, "%*sbar %d: %s at 0x%"FMT_PCIBUS
+ " [0x%"FMT_PCIBUS"]\n",
+ indent, "",
+ i, r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
+ r->addr, r->addr + r->size - 1);
+ }
+}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6711a75098..d654045fe9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -34,7 +34,6 @@
#include "hw/qdev-properties-system.h"
#include "migration/qemu-file-types.h"
#include "migration/vmstate.h"
-#include "monitor/monitor.h"
#include "net/net.h"
#include "sysemu/numa.h"
#include "sysemu/sysemu.h"
@@ -59,7 +58,6 @@
bool pci_available = true;
-static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
static char *pcibus_get_dev_path(DeviceState *dev);
static char *pcibus_get_fw_dev_path(DeviceState *dev);
static void pcibus_reset(BusState *qbus);
@@ -2406,42 +2404,6 @@ uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id)
return pci_find_capability_list(pdev, cap_id, NULL);
}
-static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
-{
- PCIDevice *d = (PCIDevice *)dev;
- int class = pci_get_word(d->config + PCI_CLASS_DEVICE);
- const pci_class_desc *desc = get_class_desc(class);
- char ctxt[64];
- PCIIORegion *r;
- int i;
-
- if (desc->desc) {
- snprintf(ctxt, sizeof(ctxt), "%s", desc->desc);
- } else {
- snprintf(ctxt, sizeof(ctxt), "Class %04x", class);
- }
-
- monitor_printf(mon, "%*sclass %s, addr %02x:%02x.%x, "
- "pci id %04x:%04x (sub %04x:%04x)\n",
- indent, "", ctxt, pci_dev_bus_num(d),
- PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
- pci_get_word(d->config + PCI_VENDOR_ID),
- pci_get_word(d->config + PCI_DEVICE_ID),
- pci_get_word(d->config + PCI_SUBSYSTEM_VENDOR_ID),
- pci_get_word(d->config + PCI_SUBSYSTEM_ID));
- for (i = 0; i < PCI_NUM_REGIONS; i++) {
- r = &d->io_regions[i];
- if (!r->size) {
- continue;
- }
- monitor_printf(mon, "%*sbar %d: %s at 0x%"FMT_PCIBUS
- " [0x%"FMT_PCIBUS"]\n",
- indent, "",
- i, r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
- r->addr, r->addr + r->size - 1);
- }
-}
-
static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
{
PCIDevice *d = (PCIDevice *)dev;
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (6 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 07/12] pci: Move pcibus_dev_print() to pci-hmp-cmds.c Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-29 12:14 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c Markus Armbruster
` (5 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
PCI AER error status is 32 bit. When the HMP command's second
argument parses as a number, values greater than ULONG_MAX get
rejected, but values between UINT32_MAX+1 and ULONG_MAX get silently
truncated. Fix to reject them, too.
While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
won't complain.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pcie_aer.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index eff62f3945..ccca5a81cc 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -30,6 +30,7 @@
#include "hw/pci/pci_bus.h"
#include "hw/pci/pcie_regs.h"
#include "qapi/error.h"
+#include "qemu/cutils.h"
//#define DEBUG_PCIE
#ifdef DEBUG_PCIE
@@ -963,6 +964,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
const char *id = qdict_get_str(qdict, "id");
const char *error_name;
uint32_t error_status;
+ unsigned long num;
bool correctable;
PCIDevice *dev;
PCIEAERErr err;
@@ -983,14 +985,14 @@ static int do_pcie_aer_inject_error(Monitor *mon,
error_name = qdict_get_str(qdict, "error_status");
if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
- char *e = NULL;
- error_status = strtoul(error_name, &e, 0);
- correctable = qdict_get_try_bool(qdict, "correctable", false);
- if (!e || *e != '\0') {
+ if (qemu_strtoul(error_name, NULL, 0, &num) < 0
+ || num > UINT32_MAX) {
monitor_printf(mon, "invalid error status value. \"%s\"",
error_name);
return -EINVAL;
}
+ error_status = num;
+ correctable = qdict_get_try_bool(qdict, "correctable", false);
}
err.status = error_status;
err.source_id = pci_requester_id(dev);
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (7 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument Markus Armbruster
@ 2022-11-28 8:01 ` Markus Armbruster
2022-11-28 8:21 ` Philippe Mathieu-Daudé
2022-11-28 8:02 ` [PATCH 10/12] pci: Inline do_pcie_aer_inject_error() into its only caller Markus Armbruster
` (4 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-internal.h | 4 ++
include/monitor/hmp.h | 1 +
include/sysemu/sysemu.h | 3 --
hw/pci/pci-hmp-cmds.c | 105 ++++++++++++++++++++++++++++++++++++
hw/pci/pci-stub.c | 1 -
hw/pci/pcie_aer.c | 115 ++--------------------------------------
6 files changed, 114 insertions(+), 115 deletions(-)
diff --git a/hw/pci/pci-internal.h b/hw/pci/pci-internal.h
index 3199615e50..2ea356bdf5 100644
--- a/hw/pci/pci-internal.h
+++ b/hw/pci/pci-internal.h
@@ -18,4 +18,8 @@ const pci_class_desc *get_class_desc(int class);
PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
+int pcie_aer_parse_error_string(const char *error_name,
+ uint32_t *status, bool *correctable);
+int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err);
+
#endif
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index dfbc0c9a2f..27f86399f7 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -143,5 +143,6 @@ void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
void hmp_human_readable_text_helper(Monitor *mon,
HumanReadableText *(*qmp_handler)(Error **));
void hmp_info_stats(Monitor *mon, const QDict *qdict);
+void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict);
#endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6a7a31e64d..25be2a692e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -61,9 +61,6 @@ extern int nb_option_roms;
extern const char *prom_envs[MAX_PROM_ENVS];
extern unsigned int nb_prom_envs;
-/* pcie aer error injection */
-void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict);
-
/* serial ports */
/* Return the Chardev for serial port i, or NULL if none */
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index 417f1ca607..393ab4214a 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -19,7 +19,9 @@
#include "monitor/monitor.h"
#include "pci-internal.h"
#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
#include "qapi/qapi-commands-pci.h"
+#include "qemu/cutils.h"
static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
{
@@ -156,3 +158,106 @@ void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
r->addr, r->addr + r->size - 1);
}
}
+
+typedef struct PCIEErrorDetails {
+ const char *id;
+ const char *root_bus;
+ int bus;
+ int devfn;
+} PCIEErrorDetails;
+
+/*
+ * Inject an error described by @qdict.
+ * On success, set @details to show where error was sent.
+ * Return negative errno if injection failed and a message was emitted.
+ */
+static int do_pcie_aer_inject_error(Monitor *mon,
+ const QDict *qdict,
+ PCIEErrorDetails *details)
+{
+ const char *id = qdict_get_str(qdict, "id");
+ const char *error_name;
+ uint32_t error_status;
+ unsigned long num;
+ bool correctable;
+ PCIDevice *dev;
+ PCIEAERErr err;
+ int ret;
+
+ ret = pci_qdev_find_device(id, &dev);
+ if (ret < 0) {
+ monitor_printf(mon,
+ "id or pci device path is invalid or device not "
+ "found. %s\n", id);
+ return ret;
+ }
+ if (!pci_is_express(dev)) {
+ monitor_printf(mon, "the device doesn't support pci express. %s\n",
+ id);
+ return -ENOSYS;
+ }
+
+ error_name = qdict_get_str(qdict, "error_status");
+ if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
+ if (qemu_strtoul(error_name, NULL, 0, &num) < 0
+ || num > UINT32_MAX) {
+ monitor_printf(mon, "invalid error status value. \"%s\"",
+ error_name);
+ return -EINVAL;
+ }
+ error_status = num;
+ correctable = qdict_get_try_bool(qdict, "correctable", false);
+ }
+ err.status = error_status;
+ err.source_id = pci_requester_id(dev);
+
+ err.flags = 0;
+ if (correctable) {
+ err.flags |= PCIE_AER_ERR_IS_CORRECTABLE;
+ }
+ if (qdict_get_try_bool(qdict, "advisory_non_fatal", false)) {
+ err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY;
+ }
+ if (qdict_haskey(qdict, "header0")) {
+ err.flags |= PCIE_AER_ERR_HEADER_VALID;
+ }
+ if (qdict_haskey(qdict, "prefix0")) {
+ err.flags |= PCIE_AER_ERR_TLP_PREFIX_PRESENT;
+ }
+
+ err.header[0] = qdict_get_try_int(qdict, "header0", 0);
+ err.header[1] = qdict_get_try_int(qdict, "header1", 0);
+ err.header[2] = qdict_get_try_int(qdict, "header2", 0);
+ err.header[3] = qdict_get_try_int(qdict, "header3", 0);
+
+ err.prefix[0] = qdict_get_try_int(qdict, "prefix0", 0);
+ err.prefix[1] = qdict_get_try_int(qdict, "prefix1", 0);
+ err.prefix[2] = qdict_get_try_int(qdict, "prefix2", 0);
+ err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
+
+ ret = pcie_aer_inject_error(dev, &err);
+ if (ret < 0) {
+ monitor_printf(mon, "failed to inject error: %s\n",
+ strerror(-ret));
+ return ret;
+ }
+ details->id = id;
+ details->root_bus = pci_root_bus_path(dev);
+ details->bus = pci_dev_bus_num(dev);
+ details->devfn = dev->devfn;
+
+ return 0;
+}
+
+void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
+{
+ PCIEErrorDetails data;
+
+ if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
+ return;
+ }
+
+ monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
+ data.id, data.root_bus, data.bus,
+ PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
+}
diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index 01d20a2f67..f0508682d2 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -19,7 +19,6 @@
*/
#include "qemu/osdep.h"
-#include "sysemu/sysemu.h"
#include "monitor/monitor.h"
#include "monitor/hmp.h"
#include "qapi/qapi-commands-pci.h"
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ccca5a81cc..9a19be44ae 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -19,18 +19,14 @@
*/
#include "qemu/osdep.h"
-#include "sysemu/sysemu.h"
-#include "qapi/qmp/qdict.h"
#include "migration/vmstate.h"
-#include "monitor/monitor.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pcie.h"
#include "hw/pci/msix.h"
#include "hw/pci/msi.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pcie_regs.h"
-#include "qapi/error.h"
-#include "qemu/cutils.h"
+#include "pci-internal.h"
//#define DEBUG_PCIE
#ifdef DEBUG_PCIE
@@ -45,13 +41,6 @@
#define PCI_ERR_SRC_COR_OFFS 0
#define PCI_ERR_SRC_UNCOR_OFFS 2
-typedef struct PCIEErrorDetails {
- const char *id;
- const char *root_bus;
- int bus;
- int devfn;
-} PCIEErrorDetails;
-
/* 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)
{
@@ -632,7 +621,7 @@ static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal)
* Figure 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging
* Operations
*/
-static int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
+int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err)
{
uint8_t *aer_cap = NULL;
uint16_t devctl = 0;
@@ -934,8 +923,8 @@ static const struct PCIEAERErrorName pcie_aer_error_list[] = {
},
};
-static int pcie_aer_parse_error_string(const char *error_name,
- uint32_t *status, bool *correctable)
+int pcie_aer_parse_error_string(const char *error_name,
+ uint32_t *status, bool *correctable)
{
int i;
@@ -951,99 +940,3 @@ static int pcie_aer_parse_error_string(const char *error_name,
}
return -EINVAL;
}
-
-/*
- * Inject an error described by @qdict.
- * On success, set @details to show where error was sent.
- * Return negative errno if injection failed and a message was emitted.
- */
-static int do_pcie_aer_inject_error(Monitor *mon,
- const QDict *qdict,
- PCIEErrorDetails *details)
-{
- const char *id = qdict_get_str(qdict, "id");
- const char *error_name;
- uint32_t error_status;
- unsigned long num;
- bool correctable;
- PCIDevice *dev;
- PCIEAERErr err;
- int ret;
-
- ret = pci_qdev_find_device(id, &dev);
- if (ret < 0) {
- monitor_printf(mon,
- "id or pci device path is invalid or device not "
- "found. %s\n", id);
- return ret;
- }
- if (!pci_is_express(dev)) {
- monitor_printf(mon, "the device doesn't support pci express. %s\n",
- id);
- return -ENOSYS;
- }
-
- error_name = qdict_get_str(qdict, "error_status");
- if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
- if (qemu_strtoul(error_name, NULL, 0, &num) < 0
- || num > UINT32_MAX) {
- monitor_printf(mon, "invalid error status value. \"%s\"",
- error_name);
- return -EINVAL;
- }
- error_status = num;
- correctable = qdict_get_try_bool(qdict, "correctable", false);
- }
- err.status = error_status;
- err.source_id = pci_requester_id(dev);
-
- err.flags = 0;
- if (correctable) {
- err.flags |= PCIE_AER_ERR_IS_CORRECTABLE;
- }
- if (qdict_get_try_bool(qdict, "advisory_non_fatal", false)) {
- err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY;
- }
- if (qdict_haskey(qdict, "header0")) {
- err.flags |= PCIE_AER_ERR_HEADER_VALID;
- }
- if (qdict_haskey(qdict, "prefix0")) {
- err.flags |= PCIE_AER_ERR_TLP_PREFIX_PRESENT;
- }
-
- err.header[0] = qdict_get_try_int(qdict, "header0", 0);
- err.header[1] = qdict_get_try_int(qdict, "header1", 0);
- err.header[2] = qdict_get_try_int(qdict, "header2", 0);
- err.header[3] = qdict_get_try_int(qdict, "header3", 0);
-
- err.prefix[0] = qdict_get_try_int(qdict, "prefix0", 0);
- err.prefix[1] = qdict_get_try_int(qdict, "prefix1", 0);
- err.prefix[2] = qdict_get_try_int(qdict, "prefix2", 0);
- err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
-
- ret = pcie_aer_inject_error(dev, &err);
- if (ret < 0) {
- monitor_printf(mon, "failed to inject error: %s\n",
- strerror(-ret));
- return ret;
- }
- details->id = id;
- details->root_bus = pci_root_bus_path(dev);
- details->bus = pci_dev_bus_num(dev);
- details->devfn = dev->devfn;
-
- return 0;
-}
-
-void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
-{
- PCIEErrorDetails data;
-
- if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
- return;
- }
-
- monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
- data.id, data.root_bus, data.bus,
- PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
-}
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 10/12] pci: Inline do_pcie_aer_inject_error() into its only caller
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (8 preceding siblings ...)
2022-11-28 8:01 ` [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c Markus Armbruster
@ 2022-11-28 8:02 ` Markus Armbruster
2022-11-29 19:59 ` Dr. David Alan Gilbert
2022-11-28 8:02 ` [PATCH 11/12] pci: Rename hmp_pcie_aer_inject_error()'s local variable @err Markus Armbruster
` (3 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:02 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-hmp-cmds.c | 41 ++++++-----------------------------------
1 file changed, 6 insertions(+), 35 deletions(-)
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index 393ab4214a..b03badb1e6 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -159,21 +159,7 @@ void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
}
}
-typedef struct PCIEErrorDetails {
- const char *id;
- const char *root_bus;
- int bus;
- int devfn;
-} PCIEErrorDetails;
-
-/*
- * Inject an error described by @qdict.
- * On success, set @details to show where error was sent.
- * Return negative errno if injection failed and a message was emitted.
- */
-static int do_pcie_aer_inject_error(Monitor *mon,
- const QDict *qdict,
- PCIEErrorDetails *details)
+void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
{
const char *id = qdict_get_str(qdict, "id");
const char *error_name;
@@ -189,12 +175,12 @@ static int do_pcie_aer_inject_error(Monitor *mon,
monitor_printf(mon,
"id or pci device path is invalid or device not "
"found. %s\n", id);
- return ret;
+ return;
}
if (!pci_is_express(dev)) {
monitor_printf(mon, "the device doesn't support pci express. %s\n",
id);
- return -ENOSYS;
+ return;
}
error_name = qdict_get_str(qdict, "error_status");
@@ -203,7 +189,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
|| num > UINT32_MAX) {
monitor_printf(mon, "invalid error status value. \"%s\"",
error_name);
- return -EINVAL;
+ return;
}
error_status = num;
correctable = qdict_get_try_bool(qdict, "correctable", false);
@@ -239,25 +225,10 @@ static int do_pcie_aer_inject_error(Monitor *mon,
if (ret < 0) {
monitor_printf(mon, "failed to inject error: %s\n",
strerror(-ret));
- return ret;
- }
- details->id = id;
- details->root_bus = pci_root_bus_path(dev);
- details->bus = pci_dev_bus_num(dev);
- details->devfn = dev->devfn;
-
- return 0;
-}
-
-void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
-{
- PCIEErrorDetails data;
-
- if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
return;
}
monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
- data.id, data.root_bus, data.bus,
- PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
+ id, pci_root_bus_path(dev), pci_dev_bus_num(dev),
+ PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
}
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 11/12] pci: Rename hmp_pcie_aer_inject_error()'s local variable @err
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (9 preceding siblings ...)
2022-11-28 8:02 ` [PATCH 10/12] pci: Inline do_pcie_aer_inject_error() into its only caller Markus Armbruster
@ 2022-11-28 8:02 ` Markus Armbruster
2022-11-28 8:21 ` Philippe Mathieu-Daudé
2022-11-28 8:02 ` [PATCH 12/12] pci: Improve do_pcie_aer_inject_error()'s error messages Markus Armbruster
` (2 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:02 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
I'd like to use @err for an Error *err. Rename PCIEAERErr err to
aer_err.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-hmp-cmds.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index b03badb1e6..0807a206e4 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -167,7 +167,7 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
unsigned long num;
bool correctable;
PCIDevice *dev;
- PCIEAERErr err;
+ PCIEAERErr aer_err;
int ret;
ret = pci_qdev_find_device(id, &dev);
@@ -194,34 +194,34 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
error_status = num;
correctable = qdict_get_try_bool(qdict, "correctable", false);
}
- err.status = error_status;
- err.source_id = pci_requester_id(dev);
+ aer_err.status = error_status;
+ aer_err.source_id = pci_requester_id(dev);
- err.flags = 0;
+ aer_err.flags = 0;
if (correctable) {
- err.flags |= PCIE_AER_ERR_IS_CORRECTABLE;
+ aer_err.flags |= PCIE_AER_ERR_IS_CORRECTABLE;
}
if (qdict_get_try_bool(qdict, "advisory_non_fatal", false)) {
- err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY;
+ aer_err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY;
}
if (qdict_haskey(qdict, "header0")) {
- err.flags |= PCIE_AER_ERR_HEADER_VALID;
+ aer_err.flags |= PCIE_AER_ERR_HEADER_VALID;
}
if (qdict_haskey(qdict, "prefix0")) {
- err.flags |= PCIE_AER_ERR_TLP_PREFIX_PRESENT;
+ aer_err.flags |= PCIE_AER_ERR_TLP_PREFIX_PRESENT;
}
- err.header[0] = qdict_get_try_int(qdict, "header0", 0);
- err.header[1] = qdict_get_try_int(qdict, "header1", 0);
- err.header[2] = qdict_get_try_int(qdict, "header2", 0);
- err.header[3] = qdict_get_try_int(qdict, "header3", 0);
+ aer_err.header[0] = qdict_get_try_int(qdict, "header0", 0);
+ aer_err.header[1] = qdict_get_try_int(qdict, "header1", 0);
+ aer_err.header[2] = qdict_get_try_int(qdict, "header2", 0);
+ aer_err.header[3] = qdict_get_try_int(qdict, "header3", 0);
- err.prefix[0] = qdict_get_try_int(qdict, "prefix0", 0);
- err.prefix[1] = qdict_get_try_int(qdict, "prefix1", 0);
- err.prefix[2] = qdict_get_try_int(qdict, "prefix2", 0);
- err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
+ aer_err.prefix[0] = qdict_get_try_int(qdict, "prefix0", 0);
+ aer_err.prefix[1] = qdict_get_try_int(qdict, "prefix1", 0);
+ aer_err.prefix[2] = qdict_get_try_int(qdict, "prefix2", 0);
+ aer_err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
- ret = pcie_aer_inject_error(dev, &err);
+ ret = pcie_aer_inject_error(dev, &aer_err);
if (ret < 0) {
monitor_printf(mon, "failed to inject error: %s\n",
strerror(-ret));
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 12/12] pci: Improve do_pcie_aer_inject_error()'s error messages
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (10 preceding siblings ...)
2022-11-28 8:02 ` [PATCH 11/12] pci: Rename hmp_pcie_aer_inject_error()'s local variable @err Markus Armbruster
@ 2022-11-28 8:02 ` Markus Armbruster
2022-11-29 19:42 ` Dr. David Alan Gilbert
2022-11-28 9:25 ` [PATCH 00/12] pci: Move and clean up monitor command code Michael S. Tsirkin
2022-11-28 10:27 ` Michael S. Tsirkin
13 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 8:02 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/pci/pci-hmp-cmds.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
index 0807a206e4..279851bfe6 100644
--- a/hw/pci/pci-hmp-cmds.c
+++ b/hw/pci/pci-hmp-cmds.c
@@ -161,6 +161,7 @@ void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
{
+ Error *err = NULL;
const char *id = qdict_get_str(qdict, "id");
const char *error_name;
uint32_t error_status;
@@ -171,25 +172,21 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
int ret;
ret = pci_qdev_find_device(id, &dev);
- if (ret < 0) {
- monitor_printf(mon,
- "id or pci device path is invalid or device not "
- "found. %s\n", id);
- return;
+ if (ret == -ENODEV) {
+ error_setg(&err, "device '%s' not found", id);
+ goto out;
}
- if (!pci_is_express(dev)) {
- monitor_printf(mon, "the device doesn't support pci express. %s\n",
- id);
- return;
+ if (ret < 0 || !pci_is_express(dev)) {
+ error_setg(&err, "device '%s' is not a PCIe device", id);
+ goto out;
}
error_name = qdict_get_str(qdict, "error_status");
if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
if (qemu_strtoul(error_name, NULL, 0, &num) < 0
|| num > UINT32_MAX) {
- monitor_printf(mon, "invalid error status value. \"%s\"",
- error_name);
- return;
+ error_setg(&err, "invalid error status value '%s'", error_name);
+ goto out;
}
error_status = num;
correctable = qdict_get_try_bool(qdict, "correctable", false);
@@ -223,12 +220,15 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
ret = pcie_aer_inject_error(dev, &aer_err);
if (ret < 0) {
- monitor_printf(mon, "failed to inject error: %s\n",
- strerror(-ret));
- return;
+ error_setg_errno(&err, -ret, "failed to inject error");
+ goto out;
}
+
monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
id, pci_root_bus_path(dev), pci_dev_bus_num(dev),
PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+
+out:
+ hmp_handle_error(mon, err);
}
--
2.37.3
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c
2022-11-28 8:01 ` [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c Markus Armbruster
@ 2022-11-28 8:21 ` Philippe Mathieu-Daudé
2022-11-28 11:50 ` Markus Armbruster
0 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-28 8:21 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
On 28/11/22 09:01, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci-internal.h | 4 ++
> include/monitor/hmp.h | 1 +
> include/sysemu/sysemu.h | 3 --
> hw/pci/pci-hmp-cmds.c | 105 ++++++++++++++++++++++++++++++++++++
> hw/pci/pci-stub.c | 1 -
> hw/pci/pcie_aer.c | 115 ++--------------------------------------
> 6 files changed, 114 insertions(+), 115 deletions(-)
I suppose hmp_info_usb() is next :)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 11/12] pci: Rename hmp_pcie_aer_inject_error()'s local variable @err
2022-11-28 8:02 ` [PATCH 11/12] pci: Rename hmp_pcie_aer_inject_error()'s local variable @err Markus Armbruster
@ 2022-11-28 8:21 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-28 8:21 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
On 28/11/22 09:02, Markus Armbruster wrote:
> I'd like to use @err for an Error *err. Rename PCIEAERErr err to
> aer_err.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci-hmp-cmds.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 07/12] pci: Move pcibus_dev_print() to pci-hmp-cmds.c
2022-11-28 8:01 ` [PATCH 07/12] pci: Move pcibus_dev_print() to pci-hmp-cmds.c Markus Armbruster
@ 2022-11-28 8:24 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-28 8:24 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
On 28/11/22 09:01, Markus Armbruster wrote:
> This method is for HMP command "info qtree".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci-internal.h | 1 +
> hw/pci/pci-hmp-cmds.c | 38 ++++++++++++++++++++++++++++++++++++++
> hw/pci/pci.c | 38 --------------------------------------
> 3 files changed, 39 insertions(+), 38 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 8:01 ` [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI Markus Armbruster
@ 2022-11-28 8:26 ` Philippe Mathieu-Daudé
2022-11-28 10:21 ` Markus Armbruster
2022-11-28 12:24 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-28 8:26 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
On 28/11/22 09:01, Markus Armbruster wrote:
> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
> !CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci-stub.c | 5 +++++
> hw/pci/meson.build | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
Squash with patch #3?
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c
2022-11-28 8:01 ` [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c Markus Armbruster
@ 2022-11-28 8:27 ` Philippe Mathieu-Daudé
2022-11-28 12:09 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-28 8:27 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
On 28/11/22 09:01, Markus Armbruster wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" to "PCI".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci-hmp-cmds.c | 126 ++++++++++++++++++++++++++++++++++++++++++
> monitor/hmp-cmds.c | 107 -----------------------------------
> hw/pci/meson.build | 1 +
> 3 files changed, 127 insertions(+), 107 deletions(-)
> create mode 100644 hw/pci/pci-hmp-cmds.c
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on
2022-11-28 8:01 ` [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
@ 2022-11-28 8:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-28 8:27 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mst, marcel.apfelbaum, dgilbert
On 28/11/22 09:01, Markus Armbruster wrote:
> Fix a few style violations so that checkpatch.pl won't complain when I
> move this code.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/12] pci: Move and clean up monitor command code
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (11 preceding siblings ...)
2022-11-28 8:02 ` [PATCH 12/12] pci: Improve do_pcie_aer_inject_error()'s error messages Markus Armbruster
@ 2022-11-28 9:25 ` Michael S. Tsirkin
2022-11-28 11:52 ` Markus Armbruster
2022-11-28 10:27 ` Michael S. Tsirkin
13 siblings, 1 reply; 34+ messages in thread
From: Michael S. Tsirkin @ 2022-11-28 9:25 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, marcel.apfelbaum, dgilbert
On Mon, Nov 28, 2022 at 09:01:50AM +0100, Markus Armbruster wrote:
> This is mainly about splitting off monitor-related code. There's also
> a few UI fixes to HMP command pcie_aer_inject_error. One UI issue
> remains: when the second argument is symbolic (found in table
> pcie_aer_error_list[]), then any -c is silently ignored. Should it be
> rejected? Should it override the value from the table?
Rejected I'd say.
> Markus Armbruster (12):
> pci: Clean up a few things checkpatch.pl would flag later on
> pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c
> pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c
> pci: Make query-pci stub consistent with the real one
> pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
> pci: Deduplicate get_class_desc()
> pci: Move pcibus_dev_print() to pci-hmp-cmds.c
> pci: Fix silent truncation of pcie_aer_inject_error argument
> pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c
> pci: Inline do_pcie_aer_inject_error() into its only caller
> pci: Rename hmp_pcie_aer_inject_error()'s local variable @err
> pci: Improve do_pcie_aer_inject_error()'s error messages
>
> hw/pci/pci-internal.h | 25 +++++
> include/monitor/hmp.h | 1 +
> include/sysemu/sysemu.h | 3 -
> hw/pci/pci-hmp-cmds.c | 234 ++++++++++++++++++++++++++++++++++++++++
> hw/pci/pci-qmp-cmds.c | 201 ++++++++++++++++++++++++++++++++++
> hw/pci/pci-stub.c | 9 +-
> hw/pci/pci.c | 226 +-------------------------------------
> hw/pci/pcie_aer.c | 113 +------------------
> monitor/hmp-cmds.c | 107 ------------------
> hw/pci/meson.build | 2 +
> 10 files changed, 476 insertions(+), 445 deletions(-)
> create mode 100644 hw/pci/pci-internal.h
> create mode 100644 hw/pci/pci-hmp-cmds.c
> create mode 100644 hw/pci/pci-qmp-cmds.c
>
> --
> 2.37.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 8:26 ` Philippe Mathieu-Daudé
@ 2022-11-28 10:21 ` Markus Armbruster
2022-11-28 10:26 ` Michael S. Tsirkin
0 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 10:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, mst, marcel.apfelbaum, dgilbert
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 28/11/22 09:01, Markus Armbruster wrote:
>> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
>> CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
>> !CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/pci/pci-stub.c | 5 +++++
>> hw/pci/meson.build | 2 +-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> Squash with patch #3?
Could do, but the combined patch isn't pure code motion anymore, and I
get to explain that in the commit message.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 10:21 ` Markus Armbruster
@ 2022-11-28 10:26 ` Michael S. Tsirkin
0 siblings, 0 replies; 34+ messages in thread
From: Michael S. Tsirkin @ 2022-11-28 10:26 UTC (permalink / raw)
To: Markus Armbruster
Cc: Philippe Mathieu-Daudé, qemu-devel, marcel.apfelbaum, dgilbert
On Mon, Nov 28, 2022 at 11:21:36AM +0100, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > On 28/11/22 09:01, Markus Armbruster wrote:
> >> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> >> CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
> >> !CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> hw/pci/pci-stub.c | 5 +++++
> >> hw/pci/meson.build | 2 +-
> >> 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > Squash with patch #3?
>
> Could do, but the combined patch isn't pure code motion anymore, and I
> get to explain that in the commit message.
Yes I like it the way it is.
--
MST
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/12] pci: Move and clean up monitor command code
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
` (12 preceding siblings ...)
2022-11-28 9:25 ` [PATCH 00/12] pci: Move and clean up monitor command code Michael S. Tsirkin
@ 2022-11-28 10:27 ` Michael S. Tsirkin
13 siblings, 0 replies; 34+ messages in thread
From: Michael S. Tsirkin @ 2022-11-28 10:27 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, marcel.apfelbaum, dgilbert
On Mon, Nov 28, 2022 at 09:01:50AM +0100, Markus Armbruster wrote:
> This is mainly about splitting off monitor-related code. There's also
> a few UI fixes to HMP command pcie_aer_inject_error. One UI issue
> remains: when the second argument is symbolic (found in table
> pcie_aer_error_list[]), then any -c is silently ignored. Should it be
> rejected? Should it override the value from the table?
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Feel free to queue.
> Markus Armbruster (12):
> pci: Clean up a few things checkpatch.pl would flag later on
> pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c
> pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c
> pci: Make query-pci stub consistent with the real one
> pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
> pci: Deduplicate get_class_desc()
> pci: Move pcibus_dev_print() to pci-hmp-cmds.c
> pci: Fix silent truncation of pcie_aer_inject_error argument
> pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c
> pci: Inline do_pcie_aer_inject_error() into its only caller
> pci: Rename hmp_pcie_aer_inject_error()'s local variable @err
> pci: Improve do_pcie_aer_inject_error()'s error messages
>
> hw/pci/pci-internal.h | 25 +++++
> include/monitor/hmp.h | 1 +
> include/sysemu/sysemu.h | 3 -
> hw/pci/pci-hmp-cmds.c | 234 ++++++++++++++++++++++++++++++++++++++++
> hw/pci/pci-qmp-cmds.c | 201 ++++++++++++++++++++++++++++++++++
> hw/pci/pci-stub.c | 9 +-
> hw/pci/pci.c | 226 +-------------------------------------
> hw/pci/pcie_aer.c | 113 +------------------
> monitor/hmp-cmds.c | 107 ------------------
> hw/pci/meson.build | 2 +
> 10 files changed, 476 insertions(+), 445 deletions(-)
> create mode 100644 hw/pci/pci-internal.h
> create mode 100644 hw/pci/pci-hmp-cmds.c
> create mode 100644 hw/pci/pci-qmp-cmds.c
>
> --
> 2.37.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c
2022-11-28 8:21 ` Philippe Mathieu-Daudé
@ 2022-11-28 11:50 ` Markus Armbruster
0 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 11:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, mst, marcel.apfelbaum, dgilbert
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 28/11/22 09:01, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/pci/pci-internal.h | 4 ++
>> include/monitor/hmp.h | 1 +
>> include/sysemu/sysemu.h | 3 --
>> hw/pci/pci-hmp-cmds.c | 105 ++++++++++++++++++++++++++++++++++++
>> hw/pci/pci-stub.c | 1 -
>> hw/pci/pcie_aer.c | 115 ++--------------------------------------
>> 6 files changed, 114 insertions(+), 115 deletions(-)
>
> I suppose hmp_info_usb() is next :)
The smaller the monitor/{hmp,qmp)-cmds.c grabbags, the better.
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thanks!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/12] pci: Move and clean up monitor command code
2022-11-28 9:25 ` [PATCH 00/12] pci: Move and clean up monitor command code Michael S. Tsirkin
@ 2022-11-28 11:52 ` Markus Armbruster
0 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 11:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, marcel.apfelbaum, dgilbert
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, Nov 28, 2022 at 09:01:50AM +0100, Markus Armbruster wrote:
>> This is mainly about splitting off monitor-related code. There's also
>> a few UI fixes to HMP command pcie_aer_inject_error. One UI issue
>> remains: when the second argument is symbolic (found in table
>> pcie_aer_error_list[]), then any -c is silently ignored. Should it be
>> rejected? Should it override the value from the table?
>
> Rejected I'd say.
If this is consensus, I'll cook up a patch.
Thanks!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c
2022-11-28 8:01 ` [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c Markus Armbruster
2022-11-28 8:27 ` Philippe Mathieu-Daudé
@ 2022-11-28 12:09 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-28 12:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" to "PCI".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pci-hmp-cmds.c | 126 ++++++++++++++++++++++++++++++++++++++++++
> monitor/hmp-cmds.c | 107 -----------------------------------
> hw/pci/meson.build | 1 +
> 3 files changed, 127 insertions(+), 107 deletions(-)
> create mode 100644 hw/pci/pci-hmp-cmds.c
>
> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
> new file mode 100644
> index 0000000000..5adfe4f57f
> --- /dev/null
> +++ b/hw/pci/pci-hmp-cmds.c
> @@ -0,0 +1,126 @@
> +/*
> + * HMP commands related to PCI
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
Yes, unfortunately it looks like the bulk of this code was ~3 months
earlier than the cut off.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> + */
> +
> +#include "qemu/osdep.h"
> +#include "monitor/hmp.h"
> +#include "monitor/monitor.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-pci.h"
> +
> +static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
> +{
> + PciMemoryRegionList *region;
> +
> + monitor_printf(mon, " Bus %2" PRId64 ", ", dev->bus);
> + monitor_printf(mon, "device %3" PRId64 ", function %" PRId64 ":\n",
> + dev->slot, dev->function);
> + monitor_printf(mon, " ");
> +
> + if (dev->class_info->has_desc) {
> + monitor_puts(mon, dev->class_info->desc);
> + } else {
> + monitor_printf(mon, "Class %04" PRId64, dev->class_info->q_class);
> + }
> +
> + monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
> + dev->id->vendor, dev->id->device);
> + if (dev->id->has_subsystem_vendor && dev->id->has_subsystem) {
> + monitor_printf(mon, " PCI subsystem %04" PRIx64 ":%04" PRIx64 "\n",
> + dev->id->subsystem_vendor, dev->id->subsystem);
> + }
> +
> + if (dev->has_irq) {
> + monitor_printf(mon, " IRQ %" PRId64 ", pin %c\n",
> + dev->irq, (char)('A' + dev->irq_pin - 1));
> + }
> +
> + if (dev->has_pci_bridge) {
> + monitor_printf(mon, " BUS %" PRId64 ".\n",
> + dev->pci_bridge->bus->number);
> + monitor_printf(mon, " secondary bus %" PRId64 ".\n",
> + dev->pci_bridge->bus->secondary);
> + monitor_printf(mon, " subordinate bus %" PRId64 ".\n",
> + dev->pci_bridge->bus->subordinate);
> +
> + monitor_printf(mon, " IO range [0x%04"PRIx64", 0x%04"PRIx64"]\n",
> + dev->pci_bridge->bus->io_range->base,
> + dev->pci_bridge->bus->io_range->limit);
> +
> + monitor_printf(mon,
> + " memory range [0x%08"PRIx64", 0x%08"PRIx64"]\n",
> + dev->pci_bridge->bus->memory_range->base,
> + dev->pci_bridge->bus->memory_range->limit);
> +
> + monitor_printf(mon, " prefetchable memory range "
> + "[0x%08"PRIx64", 0x%08"PRIx64"]\n",
> + dev->pci_bridge->bus->prefetchable_range->base,
> + dev->pci_bridge->bus->prefetchable_range->limit);
> + }
> +
> + for (region = dev->regions; region; region = region->next) {
> + uint64_t addr, size;
> +
> + addr = region->value->address;
> + size = region->value->size;
> +
> + monitor_printf(mon, " BAR%" PRId64 ": ", region->value->bar);
> +
> + if (!strcmp(region->value->type, "io")) {
> + monitor_printf(mon, "I/O at 0x%04" PRIx64
> + " [0x%04" PRIx64 "].\n",
> + addr, addr + size - 1);
> + } else {
> + monitor_printf(mon, "%d bit%s memory at 0x%08" PRIx64
> + " [0x%08" PRIx64 "].\n",
> + region->value->mem_type_64 ? 64 : 32,
> + region->value->prefetch ? " prefetchable" : "",
> + addr, addr + size - 1);
> + }
> + }
> +
> + monitor_printf(mon, " id \"%s\"\n", dev->qdev_id);
> +
> + if (dev->has_pci_bridge) {
> + if (dev->pci_bridge->has_devices) {
> + PciDeviceInfoList *cdev;
> + for (cdev = dev->pci_bridge->devices; cdev; cdev = cdev->next) {
> + hmp_info_pci_device(mon, cdev->value);
> + }
> + }
> + }
> +}
> +
> +void hmp_info_pci(Monitor *mon, const QDict *qdict)
> +{
> + PciInfoList *info_list, *info;
> + Error *err = NULL;
> +
> + info_list = qmp_query_pci(&err);
> + if (err) {
> + monitor_printf(mon, "PCI devices not supported\n");
> + error_free(err);
> + return;
> + }
> +
> + for (info = info_list; info; info = info->next) {
> + PciDeviceInfoList *dev;
> +
> + for (dev = info->value->devices; dev; dev = dev->next) {
> + hmp_info_pci_device(mon, dev->value);
> + }
> + }
> +
> + qapi_free_PciInfoList(info_list);
> +}
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 01b789a79e..03eae10663 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -37,7 +37,6 @@
> #include "qapi/qapi-commands-migration.h"
> #include "qapi/qapi-commands-misc.h"
> #include "qapi/qapi-commands-net.h"
> -#include "qapi/qapi-commands-pci.h"
> #include "qapi/qapi-commands-rocker.h"
> #include "qapi/qapi-commands-run-state.h"
> #include "qapi/qapi-commands-stats.h"
> @@ -701,89 +700,6 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
> qapi_free_BalloonInfo(info);
> }
>
> -static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
> -{
> - PciMemoryRegionList *region;
> -
> - monitor_printf(mon, " Bus %2" PRId64 ", ", dev->bus);
> - monitor_printf(mon, "device %3" PRId64 ", function %" PRId64 ":\n",
> - dev->slot, dev->function);
> - monitor_printf(mon, " ");
> -
> - if (dev->class_info->has_desc) {
> - monitor_puts(mon, dev->class_info->desc);
> - } else {
> - monitor_printf(mon, "Class %04" PRId64, dev->class_info->q_class);
> - }
> -
> - monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
> - dev->id->vendor, dev->id->device);
> - if (dev->id->has_subsystem_vendor && dev->id->has_subsystem) {
> - monitor_printf(mon, " PCI subsystem %04" PRIx64 ":%04" PRIx64 "\n",
> - dev->id->subsystem_vendor, dev->id->subsystem);
> - }
> -
> - if (dev->has_irq) {
> - monitor_printf(mon, " IRQ %" PRId64 ", pin %c\n",
> - dev->irq, (char)('A' + dev->irq_pin - 1));
> - }
> -
> - if (dev->has_pci_bridge) {
> - monitor_printf(mon, " BUS %" PRId64 ".\n",
> - dev->pci_bridge->bus->number);
> - monitor_printf(mon, " secondary bus %" PRId64 ".\n",
> - dev->pci_bridge->bus->secondary);
> - monitor_printf(mon, " subordinate bus %" PRId64 ".\n",
> - dev->pci_bridge->bus->subordinate);
> -
> - monitor_printf(mon, " IO range [0x%04"PRIx64", 0x%04"PRIx64"]\n",
> - dev->pci_bridge->bus->io_range->base,
> - dev->pci_bridge->bus->io_range->limit);
> -
> - monitor_printf(mon,
> - " memory range [0x%08"PRIx64", 0x%08"PRIx64"]\n",
> - dev->pci_bridge->bus->memory_range->base,
> - dev->pci_bridge->bus->memory_range->limit);
> -
> - monitor_printf(mon, " prefetchable memory range "
> - "[0x%08"PRIx64", 0x%08"PRIx64"]\n",
> - dev->pci_bridge->bus->prefetchable_range->base,
> - dev->pci_bridge->bus->prefetchable_range->limit);
> - }
> -
> - for (region = dev->regions; region; region = region->next) {
> - uint64_t addr, size;
> -
> - addr = region->value->address;
> - size = region->value->size;
> -
> - monitor_printf(mon, " BAR%" PRId64 ": ", region->value->bar);
> -
> - if (!strcmp(region->value->type, "io")) {
> - monitor_printf(mon, "I/O at 0x%04" PRIx64
> - " [0x%04" PRIx64 "].\n",
> - addr, addr + size - 1);
> - } else {
> - monitor_printf(mon, "%d bit%s memory at 0x%08" PRIx64
> - " [0x%08" PRIx64 "].\n",
> - region->value->mem_type_64 ? 64 : 32,
> - region->value->prefetch ? " prefetchable" : "",
> - addr, addr + size - 1);
> - }
> - }
> -
> - monitor_printf(mon, " id \"%s\"\n", dev->qdev_id);
> -
> - if (dev->has_pci_bridge) {
> - if (dev->pci_bridge->has_devices) {
> - PciDeviceInfoList *cdev;
> - for (cdev = dev->pci_bridge->devices; cdev; cdev = cdev->next) {
> - hmp_info_pci_device(mon, cdev->value);
> - }
> - }
> - }
> -}
> -
> static int hmp_info_pic_foreach(Object *obj, void *opaque)
> {
> InterruptStatsProvider *intc;
> @@ -810,29 +726,6 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
> hmp_info_pic_foreach, mon);
> }
>
> -void hmp_info_pci(Monitor *mon, const QDict *qdict)
> -{
> - PciInfoList *info_list, *info;
> - Error *err = NULL;
> -
> - info_list = qmp_query_pci(&err);
> - if (err) {
> - monitor_printf(mon, "PCI devices not supported\n");
> - error_free(err);
> - return;
> - }
> -
> - for (info = info_list; info; info = info->next) {
> - PciDeviceInfoList *dev;
> -
> - for (dev = info->value->devices; dev; dev = dev->next) {
> - hmp_info_pci_device(mon, dev->value);
> - }
> - }
> -
> - qapi_free_PciInfoList(info_list);
> -}
> -
> void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> {
> #ifdef CONFIG_TPM
> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
> index 40721f1514..e42a133f3a 100644
> --- a/hw/pci/meson.build
> +++ b/hw/pci/meson.build
> @@ -20,3 +20,4 @@ softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
>
> softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
> softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
> +softmmu_ss.add(files('pci-hmp-cmds.c'))
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 8:01 ` [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI Markus Armbruster
2022-11-28 8:26 ` Philippe Mathieu-Daudé
@ 2022-11-28 12:24 ` Dr. David Alan Gilbert
2022-11-28 13:38 ` Markus Armbruster
1 sibling, 1 reply; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-28 12:24 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
> !CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Had you considered wrapping the hmp-commands-info.hx entry
with a #if defined instead?
Dave
> ---
> hw/pci/pci-stub.c | 5 +++++
> hw/pci/meson.build | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> index f29ecc999e..01d20a2f67 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -21,6 +21,7 @@
> #include "qemu/osdep.h"
> #include "sysemu/sysemu.h"
> #include "monitor/monitor.h"
> +#include "monitor/hmp.h"
> #include "qapi/qapi-commands-pci.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/msi.h"
> @@ -34,6 +35,10 @@ PciInfoList *qmp_query_pci(Error **errp)
> return NULL;
> }
>
> +void hmp_info_pci(Monitor *mon, const QDict *qdict)
> +{
> +}
> +
> void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> {
> monitor_printf(mon, "PCI devices not supported\n");
> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
> index e42a133f3a..4fcd888b27 100644
> --- a/hw/pci/meson.build
> +++ b/hw/pci/meson.build
> @@ -5,6 +5,7 @@ pci_ss.add(files(
> 'pci.c',
> 'pci_bridge.c',
> 'pci_host.c',
> + 'pci-hmp-cmds.c',
> 'pci-qmp-cmds.c',
> 'pcie_sriov.c',
> 'shpc.c',
> @@ -20,4 +21,3 @@ softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
>
> softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
> softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
> -softmmu_ss.add(files('pci-hmp-cmds.c'))
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 12:24 ` Dr. David Alan Gilbert
@ 2022-11-28 13:38 ` Markus Armbruster
2022-11-28 14:27 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 34+ messages in thread
From: Markus Armbruster @ 2022-11-28 13:38 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, mst, marcel.apfelbaum
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> * Markus Armbruster (armbru@redhat.com) wrote:
>> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
>> CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
>> !CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Had you considered wrapping the hmp-commands-info.hx entry
> with a #if defined instead?
No. Would you prefer that?
Code containing #ifdef CONFIG_PCI is target-dependent. Looks like the
affected monitor code already is, so no new headaches.
Aside: splitting off its target-independent parts could be nice. Not
today.
[...]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
2022-11-28 13:38 ` Markus Armbruster
@ 2022-11-28 14:27 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-28 14:27 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> >> CONFIG_PCI. hw/pci/pci-stub.c keeps the linker happy when
> >> !CONFIG_PCI. Build pci-hmp-cmds.c that way, too.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >
> > Had you considered wrapping the hmp-commands-info.hx entry
> > with a #if defined instead?
>
> No. Would you prefer that?
It seemed a bit simpler to me, but I'm not too fussed.
I kind of preferred the idea of the command giving an error if there's
no PCI built in.
> Code containing #ifdef CONFIG_PCI is target-dependent. Looks like the
> affected monitor code already is, so no new headaches.
> Aside: splitting off its target-independent parts could be nice. Not
> today.
Yeh.
Dave
> [...]
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 04/12] pci: Make query-pci stub consistent with the real one
2022-11-28 8:01 ` [PATCH 04/12] pci: Make query-pci stub consistent with the real one Markus Armbruster
@ 2022-11-29 12:03 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-29 12:03 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> QMP query-pci and HMP info pci can behave differently when there are
> no PCI devices. They can report nothing, like this:
>
> qemu-system-aarch64 -S -M spitz -display none -monitor stdio
> QEMU 7.1.91 monitor - type 'help' for more information
> (qemu) info pci
>
> Or they can fail, like this:
>
> qemu-system-microblaze -M petalogix-s3adsp1800 -display none -monitor stdio
> QEMU 7.1.91 monitor - type 'help' for more information
> (qemu) info pci
> PCI devices not supported
>
> They fail when none of the target's machines supports PCI, i.e. when
> we're using qmp_query_pci() from hw/pci/pci-stub.c.
>
> The error is not useful, and reporting nothing makes sense, so do that
> in pci-stub.c, too.
>
> Now qmp_query_pci() can't fail anymore. Drop the dead error handling
> from hmp_info_pci().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/pci/pci-hmp-cmds.c | 8 +-------
> hw/pci/pci-stub.c | 3 ---
> 2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
> index 5adfe4f57f..e915fb9fe7 100644
> --- a/hw/pci/pci-hmp-cmds.c
> +++ b/hw/pci/pci-hmp-cmds.c
> @@ -105,14 +105,8 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
> void hmp_info_pci(Monitor *mon, const QDict *qdict)
> {
> PciInfoList *info_list, *info;
> - Error *err = NULL;
>
> - info_list = qmp_query_pci(&err);
> - if (err) {
> - monitor_printf(mon, "PCI devices not supported\n");
> - error_free(err);
> - return;
> - }
> + info_list = qmp_query_pci(&error_abort);
>
> for (info = info_list; info; info = info->next) {
> PciDeviceInfoList *dev;
> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> index 3a027c42e4..f29ecc999e 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -21,9 +21,7 @@
> #include "qemu/osdep.h"
> #include "sysemu/sysemu.h"
> #include "monitor/monitor.h"
> -#include "qapi/error.h"
> #include "qapi/qapi-commands-pci.h"
> -#include "qapi/qmp/qerror.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> @@ -33,7 +31,6 @@ bool pci_available;
>
> PciInfoList *qmp_query_pci(Error **errp)
> {
> - error_setg(errp, QERR_UNSUPPORTED);
> return NULL;
> }
>
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument
2022-11-28 8:01 ` [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument Markus Armbruster
@ 2022-11-29 12:14 ` Dr. David Alan Gilbert
2022-11-30 18:40 ` Markus Armbruster
0 siblings, 1 reply; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-29 12:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> PCI AER error status is 32 bit. When the HMP command's second
> argument parses as a number, values greater than ULONG_MAX get
> rejected, but values between UINT32_MAX+1 and ULONG_MAX get silently
> truncated. Fix to reject them, too.
>
> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
> won't complain.
WOuldn't qemu_strtoui do the num > UINT32_MAX for you?
Dave
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/pci/pcie_aer.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index eff62f3945..ccca5a81cc 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -30,6 +30,7 @@
> #include "hw/pci/pci_bus.h"
> #include "hw/pci/pcie_regs.h"
> #include "qapi/error.h"
> +#include "qemu/cutils.h"
>
> //#define DEBUG_PCIE
> #ifdef DEBUG_PCIE
> @@ -963,6 +964,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
> const char *id = qdict_get_str(qdict, "id");
> const char *error_name;
> uint32_t error_status;
> + unsigned long num;
> bool correctable;
> PCIDevice *dev;
> PCIEAERErr err;
> @@ -983,14 +985,14 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>
> error_name = qdict_get_str(qdict, "error_status");
> if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
> - char *e = NULL;
> - error_status = strtoul(error_name, &e, 0);
> - correctable = qdict_get_try_bool(qdict, "correctable", false);
> - if (!e || *e != '\0') {
> + if (qemu_strtoul(error_name, NULL, 0, &num) < 0
> + || num > UINT32_MAX) {
> monitor_printf(mon, "invalid error status value. \"%s\"",
> error_name);
> return -EINVAL;
> }
> + error_status = num;
> + correctable = qdict_get_try_bool(qdict, "correctable", false);
> }
> err.status = error_status;
> err.source_id = pci_requester_id(dev);
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 12/12] pci: Improve do_pcie_aer_inject_error()'s error messages
2022-11-28 8:02 ` [PATCH 12/12] pci: Improve do_pcie_aer_inject_error()'s error messages Markus Armbruster
@ 2022-11-29 19:42 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-29 19:42 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/pci/pci-hmp-cmds.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
> index 0807a206e4..279851bfe6 100644
> --- a/hw/pci/pci-hmp-cmds.c
> +++ b/hw/pci/pci-hmp-cmds.c
> @@ -161,6 +161,7 @@ void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>
> void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> {
> + Error *err = NULL;
> const char *id = qdict_get_str(qdict, "id");
> const char *error_name;
> uint32_t error_status;
> @@ -171,25 +172,21 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> int ret;
>
> ret = pci_qdev_find_device(id, &dev);
> - if (ret < 0) {
> - monitor_printf(mon,
> - "id or pci device path is invalid or device not "
> - "found. %s\n", id);
> - return;
> + if (ret == -ENODEV) {
> + error_setg(&err, "device '%s' not found", id);
> + goto out;
> }
> - if (!pci_is_express(dev)) {
> - monitor_printf(mon, "the device doesn't support pci express. %s\n",
> - id);
> - return;
> + if (ret < 0 || !pci_is_express(dev)) {
> + error_setg(&err, "device '%s' is not a PCIe device", id);
> + goto out;
> }
>
> error_name = qdict_get_str(qdict, "error_status");
> if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
> if (qemu_strtoul(error_name, NULL, 0, &num) < 0
> || num > UINT32_MAX) {
> - monitor_printf(mon, "invalid error status value. \"%s\"",
> - error_name);
> - return;
> + error_setg(&err, "invalid error status value '%s'", error_name);
> + goto out;
> }
> error_status = num;
> correctable = qdict_get_try_bool(qdict, "correctable", false);
> @@ -223,12 +220,15 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
>
> ret = pcie_aer_inject_error(dev, &aer_err);
> if (ret < 0) {
> - monitor_printf(mon, "failed to inject error: %s\n",
> - strerror(-ret));
> - return;
> + error_setg_errno(&err, -ret, "failed to inject error");
> + goto out;
> }
>
> +
> monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
> id, pci_root_bus_path(dev), pci_dev_bus_num(dev),
> PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
> +
> +out:
> + hmp_handle_error(mon, err);
> }
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 10/12] pci: Inline do_pcie_aer_inject_error() into its only caller
2022-11-28 8:02 ` [PATCH 10/12] pci: Inline do_pcie_aer_inject_error() into its only caller Markus Armbruster
@ 2022-11-29 19:59 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 34+ messages in thread
From: Dr. David Alan Gilbert @ 2022-11-29 19:59 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mst, marcel.apfelbaum
* Markus Armbruster (armbru@redhat.com) wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Yeh that seems to have simplified out;
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/pci/pci-hmp-cmds.c | 41 ++++++-----------------------------------
> 1 file changed, 6 insertions(+), 35 deletions(-)
>
> diff --git a/hw/pci/pci-hmp-cmds.c b/hw/pci/pci-hmp-cmds.c
> index 393ab4214a..b03badb1e6 100644
> --- a/hw/pci/pci-hmp-cmds.c
> +++ b/hw/pci/pci-hmp-cmds.c
> @@ -159,21 +159,7 @@ void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
> }
> }
>
> -typedef struct PCIEErrorDetails {
> - const char *id;
> - const char *root_bus;
> - int bus;
> - int devfn;
> -} PCIEErrorDetails;
> -
> -/*
> - * Inject an error described by @qdict.
> - * On success, set @details to show where error was sent.
> - * Return negative errno if injection failed and a message was emitted.
> - */
> -static int do_pcie_aer_inject_error(Monitor *mon,
> - const QDict *qdict,
> - PCIEErrorDetails *details)
> +void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> {
> const char *id = qdict_get_str(qdict, "id");
> const char *error_name;
> @@ -189,12 +175,12 @@ static int do_pcie_aer_inject_error(Monitor *mon,
> monitor_printf(mon,
> "id or pci device path is invalid or device not "
> "found. %s\n", id);
> - return ret;
> + return;
> }
> if (!pci_is_express(dev)) {
> monitor_printf(mon, "the device doesn't support pci express. %s\n",
> id);
> - return -ENOSYS;
> + return;
> }
>
> error_name = qdict_get_str(qdict, "error_status");
> @@ -203,7 +189,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
> || num > UINT32_MAX) {
> monitor_printf(mon, "invalid error status value. \"%s\"",
> error_name);
> - return -EINVAL;
> + return;
> }
> error_status = num;
> correctable = qdict_get_try_bool(qdict, "correctable", false);
> @@ -239,25 +225,10 @@ static int do_pcie_aer_inject_error(Monitor *mon,
> if (ret < 0) {
> monitor_printf(mon, "failed to inject error: %s\n",
> strerror(-ret));
> - return ret;
> - }
> - details->id = id;
> - details->root_bus = pci_root_bus_path(dev);
> - details->bus = pci_dev_bus_num(dev);
> - details->devfn = dev->devfn;
> -
> - return 0;
> -}
> -
> -void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> -{
> - PCIEErrorDetails data;
> -
> - if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
> return;
> }
>
> monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
> - data.id, data.root_bus, data.bus,
> - PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
> + id, pci_root_bus_path(dev), pci_dev_bus_num(dev),
> + PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
> }
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument
2022-11-29 12:14 ` Dr. David Alan Gilbert
@ 2022-11-30 18:40 ` Markus Armbruster
0 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2022-11-30 18:40 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, mst, marcel.apfelbaum
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> * Markus Armbruster (armbru@redhat.com) wrote:
>> PCI AER error status is 32 bit. When the HMP command's second
>> argument parses as a number, values greater than ULONG_MAX get
>> rejected, but values between UINT32_MAX+1 and ULONG_MAX get silently
>> truncated. Fix to reject them, too.
>>
>> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
>> won't complain.
>
> WOuldn't qemu_strtoui do the num > UINT32_MAX for you?
Yes, that's better.
> Dave
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/pci/pcie_aer.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
>> index eff62f3945..ccca5a81cc 100644
>> --- a/hw/pci/pcie_aer.c
>> +++ b/hw/pci/pcie_aer.c
>> @@ -30,6 +30,7 @@
>> #include "hw/pci/pci_bus.h"
>> #include "hw/pci/pcie_regs.h"
>> #include "qapi/error.h"
>> +#include "qemu/cutils.h"
>>
>> //#define DEBUG_PCIE
>> #ifdef DEBUG_PCIE
>> @@ -963,6 +964,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>> const char *id = qdict_get_str(qdict, "id");
>> const char *error_name;
>> uint32_t error_status;
>> + unsigned long num;
>> bool correctable;
>> PCIDevice *dev;
>> PCIEAERErr err;
>> @@ -983,14 +985,14 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>>
>> error_name = qdict_get_str(qdict, "error_status");
>> if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
>> - char *e = NULL;
>> - error_status = strtoul(error_name, &e, 0);
>> - correctable = qdict_get_try_bool(qdict, "correctable", false);
>> - if (!e || *e != '\0') {
>> + if (qemu_strtoul(error_name, NULL, 0, &num) < 0
>> + || num > UINT32_MAX) {
>> monitor_printf(mon, "invalid error status value. \"%s\"",
>> error_name);
>> return -EINVAL;
>> }
>> + error_status = num;
>> + correctable = qdict_get_try_bool(qdict, "correctable", false);
>> }
>> err.status = error_status;
>> err.source_id = pci_requester_id(dev);
>> --
>> 2.37.3
>>
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2022-11-30 18:41 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28 8:01 [PATCH 00/12] pci: Move and clean up monitor command code Markus Armbruster
2022-11-28 8:01 ` [PATCH 01/12] pci: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
2022-11-28 8:27 ` Philippe Mathieu-Daudé
2022-11-28 8:01 ` [PATCH 02/12] pci: Move QMP commands to new hw/pci/pci-qmp-cmds.c Markus Armbruster
2022-11-28 8:01 ` [PATCH 03/12] pci: Move HMP commands from monitor/ to new hw/pci/pci-hmp-cmds.c Markus Armbruster
2022-11-28 8:27 ` Philippe Mathieu-Daudé
2022-11-28 12:09 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 04/12] pci: Make query-pci stub consistent with the real one Markus Armbruster
2022-11-29 12:03 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI Markus Armbruster
2022-11-28 8:26 ` Philippe Mathieu-Daudé
2022-11-28 10:21 ` Markus Armbruster
2022-11-28 10:26 ` Michael S. Tsirkin
2022-11-28 12:24 ` Dr. David Alan Gilbert
2022-11-28 13:38 ` Markus Armbruster
2022-11-28 14:27 ` Dr. David Alan Gilbert
2022-11-28 8:01 ` [PATCH 06/12] pci: Deduplicate get_class_desc() Markus Armbruster
2022-11-28 8:01 ` [PATCH 07/12] pci: Move pcibus_dev_print() to pci-hmp-cmds.c Markus Armbruster
2022-11-28 8:24 ` Philippe Mathieu-Daudé
2022-11-28 8:01 ` [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument Markus Armbruster
2022-11-29 12:14 ` Dr. David Alan Gilbert
2022-11-30 18:40 ` Markus Armbruster
2022-11-28 8:01 ` [PATCH 09/12] pci: Move HMP command from hw/pci/pcie_aer.c to pci-hmp-cmds.c Markus Armbruster
2022-11-28 8:21 ` Philippe Mathieu-Daudé
2022-11-28 11:50 ` Markus Armbruster
2022-11-28 8:02 ` [PATCH 10/12] pci: Inline do_pcie_aer_inject_error() into its only caller Markus Armbruster
2022-11-29 19:59 ` Dr. David Alan Gilbert
2022-11-28 8:02 ` [PATCH 11/12] pci: Rename hmp_pcie_aer_inject_error()'s local variable @err Markus Armbruster
2022-11-28 8:21 ` Philippe Mathieu-Daudé
2022-11-28 8:02 ` [PATCH 12/12] pci: Improve do_pcie_aer_inject_error()'s error messages Markus Armbruster
2022-11-29 19:42 ` Dr. David Alan Gilbert
2022-11-28 9:25 ` [PATCH 00/12] pci: Move and clean up monitor command code Michael S. Tsirkin
2022-11-28 11:52 ` Markus Armbruster
2022-11-28 10:27 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.