* [PATCH v2 0/4] Clean up includes
@ 2022-12-22 12:08 Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 1/4] include/hw/virtio: Break inclusion loop Markus Armbruster
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Markus Armbruster @ 2022-12-22 12:08 UTC (permalink / raw)
To: qemu-devel
Cc: mst, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex
Back in 2016, we discussed[1] rules for headers, and these were
generally liked:
1. Have a carefully curated header that's included everywhere first. We
got that already thanks to Peter: osdep.h.
2. Headers should normally include everything they need beyond osdep.h.
If exceptions are needed for some reason, they must be documented in
the header. If all that's needed from a header is typedefs, put
those into qemu/typedefs.h instead of including the header.
3. Cyclic inclusion is forbidden.
This series fixes a number of rule violations.
It is based on
[PATCH v2 0/4] hw/ppc: Clean up includes
[PATCH v2 0/7] include/hw/pci include/hw/cxl: Clean up includes
[PATCH v2 0/3] block: Clean up includes
[PATCH v3 0/5] coroutine: Clean up includes
With all of these applied, just three inclusion loops remain reachable
from include/:
target/microblaze/cpu.h target/microblaze/mmu.h
target/nios2/cpu.h target/nios2/mmu.h
target/riscv/cpu.h target/riscv/pmp.h
Breaking them would be nice, but I'm out of steam.
v2:
* Rebased
* PATCH 3: v1 posted separately
* PATCH 4: New
[1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
Based-on: <20221222104628.659681-1-armbru@redhat.com>
Markus Armbruster (4):
include/hw/virtio: Break inclusion loop
include: Include headers where needed
include: Don't include qemu/osdep.h
docs/devel: Rules on #include in headers
docs/devel/style.rst | 7 +++++++
bsd-user/qemu.h | 1 -
crypto/block-luks-priv.h | 1 -
include/exec/plugin-gen.h | 1 +
include/hw/acpi/erst.h | 3 +++
include/hw/char/cmsdk-apb-uart.h | 1 +
include/hw/char/goldfish_tty.h | 1 +
include/hw/char/xilinx_uartlite.h | 1 +
include/hw/cris/etraxfs.h | 1 +
include/hw/cxl/cxl_host.h | 1 -
include/hw/display/macfb.h | 3 ++-
include/hw/dma/sifive_pdma.h | 2 ++
include/hw/i386/ioapic_internal.h | 1 +
include/hw/i386/sgx-epc.h | 1 +
include/hw/input/pl050.h | 1 -
include/hw/intc/goldfish_pic.h | 2 ++
include/hw/intc/loongarch_pch_msi.h | 2 ++
include/hw/intc/loongarch_pch_pic.h | 2 ++
include/hw/intc/nios2_vic.h | 2 ++
include/hw/misc/mchp_pfsoc_dmc.h | 2 ++
include/hw/misc/mchp_pfsoc_ioscb.h | 2 ++
include/hw/misc/mchp_pfsoc_sysreg.h | 2 ++
include/hw/misc/pvpanic.h | 1 +
include/hw/misc/sifive_e_prci.h | 3 ++-
include/hw/misc/sifive_u_otp.h | 3 ++-
include/hw/misc/sifive_u_prci.h | 3 ++-
include/hw/misc/virt_ctrl.h | 2 ++
include/hw/misc/xlnx-versal-pmc-iou-slcr.h | 1 +
include/hw/net/lasi_82596.h | 2 +-
include/hw/net/xlnx-zynqmp-can.h | 1 +
include/hw/ppc/pnv_psi.h | 2 +-
include/hw/riscv/boot_opensbi.h | 2 ++
include/hw/riscv/microchip_pfsoc.h | 3 +++
include/hw/riscv/numa.h | 1 +
include/hw/riscv/sifive_u.h | 2 ++
include/hw/riscv/spike.h | 2 +-
include/hw/riscv/virt.h | 2 +-
include/hw/ssi/sifive_spi.h | 3 +++
include/hw/timer/sse-timer.h | 1 +
include/hw/tricore/triboard.h | 1 -
include/hw/usb/hcd-dwc3.h | 1 +
include/hw/usb/hcd-musb.h | 2 ++
include/hw/usb/xlnx-usb-subsystem.h | 2 ++
include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 3 +++
include/hw/virtio/virtio-mmio.h | 2 +-
include/hw/virtio/virtio.h | 1 -
include/qemu/plugin-memory.h | 3 +++
include/qemu/userfaultfd.h | 1 -
include/sysemu/dirtyrate.h | 2 ++
include/sysemu/dump.h | 1 +
include/user/syscall-trace.h | 1 +
net/vmnet_int.h | 1 -
qga/cutils.h | 1 -
target/hexagon/hex_arch_types.h | 1 -
target/hexagon/mmvec/macros.h | 1 -
target/riscv/pmu.h | 1 -
hw/virtio/virtio-qmp.c | 1 +
hw/virtio/virtio.c | 1 +
qga/cutils.c | 3 ++-
59 files changed, 82 insertions(+), 22 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] include/hw/virtio: Break inclusion loop
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
@ 2022-12-22 12:08 ` Markus Armbruster
2022-12-23 2:58 ` Jason Wang
2022-12-22 12:08 ` [PATCH v2 2/4] include: Include headers where needed Markus Armbruster
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2022-12-22 12:08 UTC (permalink / raw)
To: qemu-devel
Cc: mst, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex,
Philippe Mathieu-Daudé, Alistair Francis, Stefano Garzarella
hw/virtio/virtio.h and hw/virtio/vhost.h include each other. The
former doesn't actually need the latter, so drop that inclusion to
break the loop.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
include/hw/virtio/virtio.h | 1 -
hw/virtio/virtio-qmp.c | 1 +
hw/virtio/virtio.c | 1 +
3 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 24561e933a..48ab2117b5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -22,7 +22,6 @@
#include "standard-headers/linux/virtio_config.h"
#include "standard-headers/linux/virtio_ring.h"
#include "qom/object.h"
-#include "hw/virtio/vhost.h"
/*
* A guest should never accept this. It implies negotiation is broken
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 8e7282658f..3d4497da99 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include "hw/virtio/virtio.h"
+#include "hw/virtio/vhost.h"
#include "virtio-qmp.h"
#include "standard-headers/linux/virtio_ids.h"
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 289eb71045..0ec6ff9ae4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -25,6 +25,7 @@
#include "qom/object_interfaces.h"
#include "hw/core/cpu.h"
#include "hw/virtio/virtio.h"
+#include "hw/virtio/vhost.h"
#include "migration/qemu-file-types.h"
#include "qemu/atomic.h"
#include "hw/virtio/virtio-bus.h"
--
2.38.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] include: Include headers where needed
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 1/4] include/hw/virtio: Break inclusion loop Markus Armbruster
@ 2022-12-22 12:08 ` Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 3/4] include: Don't include qemu/osdep.h Markus Armbruster
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2022-12-22 12:08 UTC (permalink / raw)
To: qemu-devel
Cc: mst, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex,
Alistair Francis
A number of headers neglect to include everything they need. They
compile only if the headers they need are already included from
elsewhere. Fix that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
include/exec/plugin-gen.h | 1 +
include/hw/acpi/erst.h | 3 +++
include/hw/char/cmsdk-apb-uart.h | 1 +
include/hw/char/goldfish_tty.h | 1 +
include/hw/char/xilinx_uartlite.h | 1 +
include/hw/cris/etraxfs.h | 1 +
include/hw/display/macfb.h | 3 ++-
include/hw/dma/sifive_pdma.h | 2 ++
include/hw/i386/ioapic_internal.h | 1 +
include/hw/i386/sgx-epc.h | 1 +
include/hw/intc/goldfish_pic.h | 2 ++
include/hw/intc/loongarch_pch_msi.h | 2 ++
include/hw/intc/loongarch_pch_pic.h | 2 ++
include/hw/intc/nios2_vic.h | 2 ++
include/hw/misc/mchp_pfsoc_dmc.h | 2 ++
include/hw/misc/mchp_pfsoc_ioscb.h | 2 ++
include/hw/misc/mchp_pfsoc_sysreg.h | 2 ++
include/hw/misc/pvpanic.h | 1 +
include/hw/misc/sifive_e_prci.h | 3 ++-
include/hw/misc/sifive_u_otp.h | 3 ++-
include/hw/misc/sifive_u_prci.h | 3 ++-
include/hw/misc/virt_ctrl.h | 2 ++
include/hw/misc/xlnx-versal-pmc-iou-slcr.h | 1 +
include/hw/net/lasi_82596.h | 2 +-
include/hw/net/xlnx-zynqmp-can.h | 1 +
include/hw/ppc/pnv_psi.h | 2 +-
include/hw/riscv/boot_opensbi.h | 2 ++
include/hw/riscv/microchip_pfsoc.h | 3 +++
include/hw/riscv/numa.h | 1 +
include/hw/riscv/sifive_u.h | 2 ++
include/hw/riscv/spike.h | 2 +-
include/hw/riscv/virt.h | 2 +-
include/hw/ssi/sifive_spi.h | 3 +++
include/hw/timer/sse-timer.h | 1 +
include/hw/usb/hcd-dwc3.h | 1 +
include/hw/usb/hcd-musb.h | 2 ++
include/hw/usb/xlnx-usb-subsystem.h | 2 ++
include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 3 +++
include/hw/virtio/virtio-mmio.h | 2 +-
include/qemu/plugin-memory.h | 3 +++
include/sysemu/dirtyrate.h | 2 ++
include/sysemu/dump.h | 1 +
include/user/syscall-trace.h | 1 +
43 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/include/exec/plugin-gen.h b/include/exec/plugin-gen.h
index 5004728c61..5f5506f1cc 100644
--- a/include/exec/plugin-gen.h
+++ b/include/exec/plugin-gen.h
@@ -12,6 +12,7 @@
#ifndef QEMU_PLUGIN_GEN_H
#define QEMU_PLUGIN_GEN_H
+#include "exec/cpu_ldst.h"
#include "qemu/plugin.h"
#include "tcg/tcg.h"
diff --git a/include/hw/acpi/erst.h b/include/hw/acpi/erst.h
index b747fe7739..b2ff663ddc 100644
--- a/include/hw/acpi/erst.h
+++ b/include/hw/acpi/erst.h
@@ -11,6 +11,9 @@
#ifndef HW_ACPI_ERST_H
#define HW_ACPI_ERST_H
+#include "hw/acpi/bios-linker-loader.h"
+#include "qom/object.h"
+
void build_erst(GArray *table_data, BIOSLinker *linker, Object *erst_dev,
const char *oem_id, const char *oem_table_id);
diff --git a/include/hw/char/cmsdk-apb-uart.h b/include/hw/char/cmsdk-apb-uart.h
index 9daff0eeee..64b0a3d534 100644
--- a/include/hw/char/cmsdk-apb-uart.h
+++ b/include/hw/char/cmsdk-apb-uart.h
@@ -15,6 +15,7 @@
#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
#include "chardev/char-fe.h"
+#include "qapi/error.h"
#include "qom/object.h"
#define TYPE_CMSDK_APB_UART "cmsdk-apb-uart"
diff --git a/include/hw/char/goldfish_tty.h b/include/hw/char/goldfish_tty.h
index 7503d2fa1e..d59733e5ae 100644
--- a/include/hw/char/goldfish_tty.h
+++ b/include/hw/char/goldfish_tty.h
@@ -12,6 +12,7 @@
#include "qemu/fifo8.h"
#include "chardev/char-fe.h"
+#include "hw/sysbus.h"
#define TYPE_GOLDFISH_TTY "goldfish_tty"
OBJECT_DECLARE_SIMPLE_TYPE(GoldfishTTYState, GOLDFISH_TTY)
diff --git a/include/hw/char/xilinx_uartlite.h b/include/hw/char/xilinx_uartlite.h
index bb32d0fcb3..dd09c06801 100644
--- a/include/hw/char/xilinx_uartlite.h
+++ b/include/hw/char/xilinx_uartlite.h
@@ -17,6 +17,7 @@
#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
+#include "qapi/error.h"
static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
qemu_irq irq,
diff --git a/include/hw/cris/etraxfs.h b/include/hw/cris/etraxfs.h
index 8b01ed67d3..467b529dc0 100644
--- a/include/hw/cris/etraxfs.h
+++ b/include/hw/cris/etraxfs.h
@@ -29,6 +29,7 @@
#include "hw/cris/etraxfs_dma.h"
#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
+#include "qapi/error.h"
DeviceState *etraxfs_eth_init(NICInfo *nd, hwaddr base, int phyaddr,
struct etraxfs_dma_client *dma_out,
diff --git a/include/hw/display/macfb.h b/include/hw/display/macfb.h
index 55a50d3fb0..27cebefc9e 100644
--- a/include/hw/display/macfb.h
+++ b/include/hw/display/macfb.h
@@ -15,9 +15,10 @@
#include "exec/memory.h"
#include "hw/irq.h"
+#include "hw/nubus/nubus.h"
+#include "hw/sysbus.h"
#include "ui/console.h"
#include "qemu/timer.h"
-#include "qom/object.h"
typedef enum {
MACFB_DISPLAY_APPLE_21_COLOR = 0,
diff --git a/include/hw/dma/sifive_pdma.h b/include/hw/dma/sifive_pdma.h
index e319bbd6c4..8c6cfa7f32 100644
--- a/include/hw/dma/sifive_pdma.h
+++ b/include/hw/dma/sifive_pdma.h
@@ -23,6 +23,8 @@
#ifndef SIFIVE_PDMA_H
#define SIFIVE_PDMA_H
+#include "hw/sysbus.h"
+
struct sifive_pdma_chan {
uint32_t control;
uint32_t next_config;
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index 9880443cc7..e8ff338d7f 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -23,6 +23,7 @@
#define QEMU_IOAPIC_INTERNAL_H
#include "exec/memory.h"
+#include "hw/i386/ioapic.h"
#include "hw/sysbus.h"
#include "qemu/notify.h"
#include "qom/object.h"
diff --git a/include/hw/i386/sgx-epc.h b/include/hw/i386/sgx-epc.h
index 581fac389a..3e00efd870 100644
--- a/include/hw/i386/sgx-epc.h
+++ b/include/hw/i386/sgx-epc.h
@@ -12,6 +12,7 @@
#ifndef QEMU_SGX_EPC_H
#define QEMU_SGX_EPC_H
+#include "hw/qdev-core.h"
#include "hw/i386/hostmem-epc.h"
#define TYPE_SGX_EPC "sgx-epc"
diff --git a/include/hw/intc/goldfish_pic.h b/include/hw/intc/goldfish_pic.h
index e9d552f796..3e79580367 100644
--- a/include/hw/intc/goldfish_pic.h
+++ b/include/hw/intc/goldfish_pic.h
@@ -10,6 +10,8 @@
#ifndef HW_INTC_GOLDFISH_PIC_H
#define HW_INTC_GOLDFISH_PIC_H
+#include "hw/sysbus.h"
+
#define TYPE_GOLDFISH_PIC "goldfish_pic"
OBJECT_DECLARE_SIMPLE_TYPE(GoldfishPICState, GOLDFISH_PIC)
diff --git a/include/hw/intc/loongarch_pch_msi.h b/include/hw/intc/loongarch_pch_msi.h
index 6d67560dea..2810665ef7 100644
--- a/include/hw/intc/loongarch_pch_msi.h
+++ b/include/hw/intc/loongarch_pch_msi.h
@@ -5,6 +5,8 @@
* Copyright (C) 2021 Loongson Technology Corporation Limited
*/
+#include "hw/sysbus.h"
+
#define TYPE_LOONGARCH_PCH_MSI "loongarch_pch_msi"
OBJECT_DECLARE_SIMPLE_TYPE(LoongArchPCHMSI, LOONGARCH_PCH_MSI)
diff --git a/include/hw/intc/loongarch_pch_pic.h b/include/hw/intc/loongarch_pch_pic.h
index 2d4aa9ed6f..5d5dee9280 100644
--- a/include/hw/intc/loongarch_pch_pic.h
+++ b/include/hw/intc/loongarch_pch_pic.h
@@ -5,6 +5,8 @@
* Copyright (c) 2021 Loongson Technology Corporation Limited
*/
+#include "hw/sysbus.h"
+
#define TYPE_LOONGARCH_PCH_PIC "loongarch_pch_pic"
#define PCH_PIC_NAME(name) TYPE_LOONGARCH_PCH_PIC#name
OBJECT_DECLARE_SIMPLE_TYPE(LoongArchPCHPIC, LOONGARCH_PCH_PIC)
diff --git a/include/hw/intc/nios2_vic.h b/include/hw/intc/nios2_vic.h
index ac507b9d74..5c975a2ac4 100644
--- a/include/hw/intc/nios2_vic.h
+++ b/include/hw/intc/nios2_vic.h
@@ -35,6 +35,8 @@
#ifndef HW_INTC_NIOS2_VIC_H
#define HW_INTC_NIOS2_VIC_H
+#include "hw/sysbus.h"
+
#define TYPE_NIOS2_VIC "nios2-vic"
OBJECT_DECLARE_SIMPLE_TYPE(Nios2VIC, NIOS2_VIC)
diff --git a/include/hw/misc/mchp_pfsoc_dmc.h b/include/hw/misc/mchp_pfsoc_dmc.h
index 2baa1413b0..3bc1581e0f 100644
--- a/include/hw/misc/mchp_pfsoc_dmc.h
+++ b/include/hw/misc/mchp_pfsoc_dmc.h
@@ -23,6 +23,8 @@
#ifndef MCHP_PFSOC_DMC_H
#define MCHP_PFSOC_DMC_H
+#include "hw/sysbus.h"
+
/* DDR SGMII PHY module */
#define MCHP_PFSOC_DDR_SGMII_PHY_REG_SIZE 0x1000
diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h b/include/hw/misc/mchp_pfsoc_ioscb.h
index 9235523e33..bab83a96a6 100644
--- a/include/hw/misc/mchp_pfsoc_ioscb.h
+++ b/include/hw/misc/mchp_pfsoc_ioscb.h
@@ -23,6 +23,8 @@
#ifndef MCHP_PFSOC_IOSCB_H
#define MCHP_PFSOC_IOSCB_H
+#include "hw/sysbus.h"
+
typedef struct MchpPfSoCIoscbState {
SysBusDevice parent;
MemoryRegion container;
diff --git a/include/hw/misc/mchp_pfsoc_sysreg.h b/include/hw/misc/mchp_pfsoc_sysreg.h
index 546ba68f6a..a2fd1c9f07 100644
--- a/include/hw/misc/mchp_pfsoc_sysreg.h
+++ b/include/hw/misc/mchp_pfsoc_sysreg.h
@@ -23,6 +23,8 @@
#ifndef MCHP_PFSOC_SYSREG_H
#define MCHP_PFSOC_SYSREG_H
+#include "hw/sysbus.h"
+
#define MCHP_PFSOC_SYSREG_REG_SIZE 0x2000
typedef struct MchpPfSoCSysregState {
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index e520566ab0..fab94165d0 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -15,6 +15,7 @@
#ifndef HW_MISC_PVPANIC_H
#define HW_MISC_PVPANIC_H
+#include "exec/memory.h"
#include "qom/object.h"
#define TYPE_PVPANIC_ISA_DEVICE "pvpanic"
diff --git a/include/hw/misc/sifive_e_prci.h b/include/hw/misc/sifive_e_prci.h
index 262ca16181..6aa949e910 100644
--- a/include/hw/misc/sifive_e_prci.h
+++ b/include/hw/misc/sifive_e_prci.h
@@ -18,7 +18,8 @@
#ifndef HW_SIFIVE_E_PRCI_H
#define HW_SIFIVE_E_PRCI_H
-#include "qom/object.h"
+
+#include "hw/sysbus.h"
enum {
SIFIVE_E_PRCI_HFROSCCFG = 0x0,
diff --git a/include/hw/misc/sifive_u_otp.h b/include/hw/misc/sifive_u_otp.h
index 5d0d7df455..170d2148f2 100644
--- a/include/hw/misc/sifive_u_otp.h
+++ b/include/hw/misc/sifive_u_otp.h
@@ -18,7 +18,8 @@
#ifndef HW_SIFIVE_U_OTP_H
#define HW_SIFIVE_U_OTP_H
-#include "qom/object.h"
+
+#include "hw/sysbus.h"
#define SIFIVE_U_OTP_PA 0x00
#define SIFIVE_U_OTP_PAIO 0x04
diff --git a/include/hw/misc/sifive_u_prci.h b/include/hw/misc/sifive_u_prci.h
index d9ebf40b7f..4d2491ad46 100644
--- a/include/hw/misc/sifive_u_prci.h
+++ b/include/hw/misc/sifive_u_prci.h
@@ -18,7 +18,8 @@
#ifndef HW_SIFIVE_U_PRCI_H
#define HW_SIFIVE_U_PRCI_H
-#include "qom/object.h"
+
+#include "hw/sysbus.h"
#define SIFIVE_U_PRCI_HFXOSCCFG 0x00
#define SIFIVE_U_PRCI_COREPLLCFG0 0x04
diff --git a/include/hw/misc/virt_ctrl.h b/include/hw/misc/virt_ctrl.h
index 25a237e518..81346cf017 100644
--- a/include/hw/misc/virt_ctrl.h
+++ b/include/hw/misc/virt_ctrl.h
@@ -7,6 +7,8 @@
#ifndef VIRT_CTRL_H
#define VIRT_CTRL_H
+#include "hw/sysbus.h"
+
#define TYPE_VIRT_CTRL "virt-ctrl"
OBJECT_DECLARE_SIMPLE_TYPE(VirtCtrlState, VIRT_CTRL)
diff --git a/include/hw/misc/xlnx-versal-pmc-iou-slcr.h b/include/hw/misc/xlnx-versal-pmc-iou-slcr.h
index 2170420f01..f7d24c93c4 100644
--- a/include/hw/misc/xlnx-versal-pmc-iou-slcr.h
+++ b/include/hw/misc/xlnx-versal-pmc-iou-slcr.h
@@ -54,6 +54,7 @@
#ifndef XLNX_VERSAL_PMC_IOU_SLCR_H
#define XLNX_VERSAL_PMC_IOU_SLCR_H
+#include "hw/sysbus.h"
#include "hw/register.h"
#define TYPE_XILINX_VERSAL_PMC_IOU_SLCR "xlnx.versal-pmc-iou-slcr"
diff --git a/include/hw/net/lasi_82596.h b/include/hw/net/lasi_82596.h
index 7b62b04833..3ef2f47ba2 100644
--- a/include/hw/net/lasi_82596.h
+++ b/include/hw/net/lasi_82596.h
@@ -10,7 +10,7 @@
#include "net/net.h"
#include "hw/net/i82596.h"
-#include "qom/object.h"
+#include "hw/sysbus.h"
#define TYPE_LASI_82596 "lasi_82596"
typedef struct SysBusI82596State SysBusI82596State;
diff --git a/include/hw/net/xlnx-zynqmp-can.h b/include/hw/net/xlnx-zynqmp-can.h
index eb1558708b..fd2aa77760 100644
--- a/include/hw/net/xlnx-zynqmp-can.h
+++ b/include/hw/net/xlnx-zynqmp-can.h
@@ -30,6 +30,7 @@
#ifndef XLNX_ZYNQMP_CAN_H
#define XLNX_ZYNQMP_CAN_H
+#include "hw/sysbus.h"
#include "hw/register.h"
#include "net/can_emu.h"
#include "net/can_host.h"
diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
index 8253469b8f..2a6f715350 100644
--- a/include/hw/ppc/pnv_psi.h
+++ b/include/hw/ppc/pnv_psi.h
@@ -23,7 +23,7 @@
#include "hw/sysbus.h"
#include "hw/ppc/xics.h"
#include "hw/ppc/xive.h"
-#include "qom/object.h"
+#include "hw/qdev-core.h"
#define TYPE_PNV_PSI "pnv-psi"
OBJECT_DECLARE_TYPE(PnvPsi, PnvPsiClass,
diff --git a/include/hw/riscv/boot_opensbi.h b/include/hw/riscv/boot_opensbi.h
index c19cad4818..1b749663dc 100644
--- a/include/hw/riscv/boot_opensbi.h
+++ b/include/hw/riscv/boot_opensbi.h
@@ -8,6 +8,8 @@
#ifndef RISCV_BOOT_OPENSBI_H
#define RISCV_BOOT_OPENSBI_H
+#include "exec/cpu-defs.h"
+
/** Expected value of info magic ('OSBI' ascii string in hex) */
#define FW_DYNAMIC_INFO_MAGIC_VALUE 0x4942534f
diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index a757b240e0..9e806b09b1 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -22,13 +22,16 @@
#ifndef HW_MICROCHIP_PFSOC_H
#define HW_MICROCHIP_PFSOC_H
+#include "hw/boards.h"
#include "hw/char/mchp_pfsoc_mmuart.h"
+#include "hw/cpu/cluster.h"
#include "hw/dma/sifive_pdma.h"
#include "hw/misc/mchp_pfsoc_dmc.h"
#include "hw/misc/mchp_pfsoc_ioscb.h"
#include "hw/misc/mchp_pfsoc_sysreg.h"
#include "hw/net/cadence_gem.h"
#include "hw/sd/cadence_sdhci.h"
+#include "hw/riscv/riscv_hart.h"
typedef struct MicrochipPFSoCState {
/*< private >*/
diff --git a/include/hw/riscv/numa.h b/include/hw/riscv/numa.h
index fcce942cee..1a9cce3344 100644
--- a/include/hw/riscv/numa.h
+++ b/include/hw/riscv/numa.h
@@ -19,6 +19,7 @@
#ifndef RISCV_NUMA_H
#define RISCV_NUMA_H
+#include "hw/boards.h"
#include "hw/sysbus.h"
#include "sysemu/numa.h"
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 8f63a183c4..a43304292c 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -19,6 +19,8 @@
#ifndef HW_SIFIVE_U_H
#define HW_SIFIVE_U_H
+#include "hw/boards.h"
+#include "hw/cpu/cluster.h"
#include "hw/dma/sifive_pdma.h"
#include "hw/net/cadence_gem.h"
#include "hw/riscv/riscv_hart.h"
diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index 73d69234de..73bf2a9aad 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -19,9 +19,9 @@
#ifndef HW_RISCV_SPIKE_H
#define HW_RISCV_SPIKE_H
+#include "hw/boards.h"
#include "hw/riscv/riscv_hart.h"
#include "hw/sysbus.h"
-#include "qom/object.h"
#define SPIKE_CPUS_MAX 8
#define SPIKE_SOCKETS_MAX 8
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index be4ab8fe7f..3007bb3646 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -19,10 +19,10 @@
#ifndef HW_RISCV_VIRT_H
#define HW_RISCV_VIRT_H
+#include "hw/boards.h"
#include "hw/riscv/riscv_hart.h"
#include "hw/sysbus.h"
#include "hw/block/flash.h"
-#include "qom/object.h"
#define VIRT_CPUS_MAX_BITS 9
#define VIRT_CPUS_MAX (1 << VIRT_CPUS_MAX_BITS)
diff --git a/include/hw/ssi/sifive_spi.h b/include/hw/ssi/sifive_spi.h
index 47d0d6a47c..d0c40cdb11 100644
--- a/include/hw/ssi/sifive_spi.h
+++ b/include/hw/ssi/sifive_spi.h
@@ -22,6 +22,9 @@
#ifndef HW_SIFIVE_SPI_H
#define HW_SIFIVE_SPI_H
+#include "qemu/fifo8.h"
+#include "hw/sysbus.h"
+
#define SIFIVE_SPI_REG_NUM (0x78 / 4)
#define TYPE_SIFIVE_SPI "sifive.spi"
diff --git a/include/hw/timer/sse-timer.h b/include/hw/timer/sse-timer.h
index b4ee8e7f6c..265ad32400 100644
--- a/include/hw/timer/sse-timer.h
+++ b/include/hw/timer/sse-timer.h
@@ -25,6 +25,7 @@
#define SSE_TIMER_H
#include "hw/sysbus.h"
+#include "qemu/timer.h"
#include "qom/object.h"
#include "hw/timer/sse-counter.h"
diff --git a/include/hw/usb/hcd-dwc3.h b/include/hw/usb/hcd-dwc3.h
index 7c804d536d..f752a27e94 100644
--- a/include/hw/usb/hcd-dwc3.h
+++ b/include/hw/usb/hcd-dwc3.h
@@ -26,6 +26,7 @@
#ifndef HCD_DWC3_H
#define HCD_DWC3_H
+#include "hw/register.h"
#include "hw/usb/hcd-xhci.h"
#include "hw/usb/hcd-xhci-sysbus.h"
diff --git a/include/hw/usb/hcd-musb.h b/include/hw/usb/hcd-musb.h
index f30a26f7f4..4d4b1ec0fc 100644
--- a/include/hw/usb/hcd-musb.h
+++ b/include/hw/usb/hcd-musb.h
@@ -13,6 +13,8 @@
#ifndef HW_USB_HCD_MUSB_H
#define HW_USB_HCD_MUSB_H
+#include "exec/hwaddr.h"
+
enum musb_irq_source_e {
musb_irq_suspend = 0,
musb_irq_resume,
diff --git a/include/hw/usb/xlnx-usb-subsystem.h b/include/hw/usb/xlnx-usb-subsystem.h
index 5b730abd84..40f9e97e09 100644
--- a/include/hw/usb/xlnx-usb-subsystem.h
+++ b/include/hw/usb/xlnx-usb-subsystem.h
@@ -25,6 +25,8 @@
#ifndef XLNX_USB_SUBSYSTEM_H
#define XLNX_USB_SUBSYSTEM_H
+#include "hw/register.h"
+#include "hw/sysbus.h"
#include "hw/usb/xlnx-versal-usb2-ctrl-regs.h"
#include "hw/usb/hcd-dwc3.h"
diff --git a/include/hw/usb/xlnx-versal-usb2-ctrl-regs.h b/include/hw/usb/xlnx-versal-usb2-ctrl-regs.h
index 633bf3013a..6a502006b0 100644
--- a/include/hw/usb/xlnx-versal-usb2-ctrl-regs.h
+++ b/include/hw/usb/xlnx-versal-usb2-ctrl-regs.h
@@ -26,6 +26,9 @@
#ifndef XLNX_VERSAL_USB2_CTRL_REGS_H
#define XLNX_VERSAL_USB2_CTRL_REGS_H
+#include "hw/register.h"
+#include "hw/sysbus.h"
+
#define TYPE_XILINX_VERSAL_USB2_CTRL_REGS "xlnx.versal-usb2-ctrl-regs"
#define XILINX_VERSAL_USB2_CTRL_REGS(obj) \
diff --git a/include/hw/virtio/virtio-mmio.h b/include/hw/virtio/virtio-mmio.h
index 090f7730e7..aa49262022 100644
--- a/include/hw/virtio/virtio-mmio.h
+++ b/include/hw/virtio/virtio-mmio.h
@@ -22,8 +22,8 @@
#ifndef HW_VIRTIO_MMIO_H
#define HW_VIRTIO_MMIO_H
+#include "hw/sysbus.h"
#include "hw/virtio/virtio-bus.h"
-#include "qom/object.h"
/* QOM macros */
/* virtio-mmio-bus */
diff --git a/include/qemu/plugin-memory.h b/include/qemu/plugin-memory.h
index 8ad13c110c..6fd539022a 100644
--- a/include/qemu/plugin-memory.h
+++ b/include/qemu/plugin-memory.h
@@ -9,6 +9,9 @@
#ifndef PLUGIN_MEMORY_H
#define PLUGIN_MEMORY_H
+#include "exec/cpu-defs.h"
+#include "exec/hwaddr.h"
+
struct qemu_plugin_hwaddr {
bool is_io;
bool is_store;
diff --git a/include/sysemu/dirtyrate.h b/include/sysemu/dirtyrate.h
index 4d3b9a4902..20813f303f 100644
--- a/include/sysemu/dirtyrate.h
+++ b/include/sysemu/dirtyrate.h
@@ -13,6 +13,8 @@
#ifndef QEMU_DIRTYRATE_H
#define QEMU_DIRTYRATE_H
+#include "qapi/qapi-types-migration.h"
+
typedef struct VcpuStat {
int nvcpu; /* number of vcpu */
DirtyRateVcpu *rates; /* array of dirty rate for each vcpu */
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 4ffed0b659..7008d43d04 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -15,6 +15,7 @@
#define DUMP_H
#include "qapi/qapi-types-dump.h"
+#include "qemu/thread.h"
#define MAKEDUMPFILE_SIGNATURE "makedumpfile"
#define MAX_SIZE_MDF_HEADER (4096) /* max size of makedumpfile_header */
diff --git a/include/user/syscall-trace.h b/include/user/syscall-trace.h
index b4e53d3870..c5a220da34 100644
--- a/include/user/syscall-trace.h
+++ b/include/user/syscall-trace.h
@@ -10,6 +10,7 @@
#ifndef SYSCALL_TRACE_H
#define SYSCALL_TRACE_H
+#include "exec/user/abitypes.h"
#include "trace/trace-root.h"
/*
--
2.38.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] include: Don't include qemu/osdep.h
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 1/4] include/hw/virtio: Break inclusion loop Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 2/4] include: Include headers where needed Markus Armbruster
@ 2022-12-22 12:08 ` Markus Armbruster
2023-01-08 6:51 ` Michael S. Tsirkin
2022-12-22 12:08 ` [PATCH v2 4/4] docs/devel: Rules on #include in headers Markus Armbruster
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2022-12-22 12:08 UTC (permalink / raw)
To: qemu-devel
Cc: mst, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex,
Philippe Mathieu-Daudé, Bin Meng, Taylor Simpson,
Alistair Francis
docs/devel/style.rst mandates:
The "qemu/osdep.h" header contains preprocessor macros that affect
the behavior of core system headers like <stdint.h>. It must be
the first include so that core system headers included by external
libraries get the preprocessor macros that QEMU depends on.
Do not include "qemu/osdep.h" from header files since the .c file
will have already included it.
A few violations have crept in. Fix them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
bsd-user/qemu.h | 1 -
crypto/block-luks-priv.h | 1 -
include/hw/cxl/cxl_host.h | 1 -
include/hw/input/pl050.h | 1 -
include/hw/tricore/triboard.h | 1 -
include/qemu/userfaultfd.h | 1 -
net/vmnet_int.h | 1 -
qga/cutils.h | 1 -
target/hexagon/hex_arch_types.h | 1 -
target/hexagon/mmvec/macros.h | 1 -
target/riscv/pmu.h | 1 -
qga/cutils.c | 3 ++-
12 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index be6105385e..0ceecfb6df 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -17,7 +17,6 @@
#ifndef QEMU_H
#define QEMU_H
-#include "qemu/osdep.h"
#include "cpu.h"
#include "qemu/units.h"
#include "exec/cpu_ldst.h"
diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h
index dc2dd14e52..8fc967afcb 100644
--- a/crypto/block-luks-priv.h
+++ b/crypto/block-luks-priv.h
@@ -18,7 +18,6 @@
*
*/
-#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu/bswap.h"
diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index a1b662ce40..c9bc9c7c50 100644
--- a/include/hw/cxl/cxl_host.h
+++ b/include/hw/cxl/cxl_host.h
@@ -7,7 +7,6 @@
* COPYING file in the top-level directory.
*/
-#include "qemu/osdep.h"
#include "hw/cxl/cxl.h"
#include "hw/boards.h"
diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h
index 89ec4fafc9..4cb8985f31 100644
--- a/include/hw/input/pl050.h
+++ b/include/hw/input/pl050.h
@@ -10,7 +10,6 @@
#ifndef HW_PL050_H
#define HW_PL050_H
-#include "qemu/osdep.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
#include "hw/input/ps2.h"
diff --git a/include/hw/tricore/triboard.h b/include/hw/tricore/triboard.h
index 094c8bd563..4fdd2d7d97 100644
--- a/include/hw/tricore/triboard.h
+++ b/include/hw/tricore/triboard.h
@@ -18,7 +18,6 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
-#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/boards.h"
#include "sysemu/sysemu.h"
diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
index 6b74f92792..55c95998e8 100644
--- a/include/qemu/userfaultfd.h
+++ b/include/qemu/userfaultfd.h
@@ -13,7 +13,6 @@
#ifndef USERFAULTFD_H
#define USERFAULTFD_H
-#include "qemu/osdep.h"
#include "exec/hwaddr.h"
#include <linux/userfaultfd.h>
diff --git a/net/vmnet_int.h b/net/vmnet_int.h
index adf6e8c20d..d0b90594f2 100644
--- a/net/vmnet_int.h
+++ b/net/vmnet_int.h
@@ -10,7 +10,6 @@
#ifndef VMNET_INT_H
#define VMNET_INT_H
-#include "qemu/osdep.h"
#include "vmnet_int.h"
#include "clients.h"
diff --git a/qga/cutils.h b/qga/cutils.h
index f0f30a7d28..2bfaf554a8 100644
--- a/qga/cutils.h
+++ b/qga/cutils.h
@@ -1,7 +1,6 @@
#ifndef CUTILS_H_
#define CUTILS_H_
-#include "qemu/osdep.h"
int qga_open_cloexec(const char *name, int flags, mode_t mode);
diff --git a/target/hexagon/hex_arch_types.h b/target/hexagon/hex_arch_types.h
index 885f68f760..52a7f2b2f3 100644
--- a/target/hexagon/hex_arch_types.h
+++ b/target/hexagon/hex_arch_types.h
@@ -18,7 +18,6 @@
#ifndef HEXAGON_HEX_ARCH_TYPES_H
#define HEXAGON_HEX_ARCH_TYPES_H
-#include "qemu/osdep.h"
#include "mmvec/mmvec.h"
#include "qemu/int128.h"
diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
index 8c864e8c68..1201d778d0 100644
--- a/target/hexagon/mmvec/macros.h
+++ b/target/hexagon/mmvec/macros.h
@@ -18,7 +18,6 @@
#ifndef HEXAGON_MMVEC_MACROS_H
#define HEXAGON_MMVEC_MACROS_H
-#include "qemu/osdep.h"
#include "qemu/host-utils.h"
#include "arch.h"
#include "mmvec/system_ext_mmvec.h"
diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 3004ce37b6..0c819ca983 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -16,7 +16,6 @@
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include "qemu/osdep.h"
#include "qemu/log.h"
#include "cpu.h"
#include "qemu/main-loop.h"
diff --git a/qga/cutils.c b/qga/cutils.c
index b8e142ef64..b21bcf3683 100644
--- a/qga/cutils.c
+++ b/qga/cutils.c
@@ -2,8 +2,9 @@
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
*/
-#include "cutils.h"
+#include "qemu/osdep.h"
+#include "cutils.h"
#include "qapi/error.h"
/**
--
2.38.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] docs/devel: Rules on #include in headers
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
` (2 preceding siblings ...)
2022-12-22 12:08 ` [PATCH v2 3/4] include: Don't include qemu/osdep.h Markus Armbruster
@ 2022-12-22 12:08 ` Markus Armbruster
2022-12-23 10:47 ` Bernhard Beschow
2022-12-23 23:41 ` Alex Bennée
2022-12-22 12:10 ` [PATCH v2 0/4] Clean up includes Markus Armbruster
` (2 subsequent siblings)
6 siblings, 2 replies; 14+ messages in thread
From: Markus Armbruster @ 2022-12-22 12:08 UTC (permalink / raw)
To: qemu-devel
Cc: mst, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex,
Bernhard Beschow
Rules for headers were proposed a long time ago, and generally liked:
Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
Wortk them into docs/devel/style.rst.
Suggested-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/devel/style.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 7ddd42b6c2..68aa776930 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -293,6 +293,13 @@ that QEMU depends on.
Do not include "qemu/osdep.h" from header files since the .c file will have
already included it.
+Headers should normally include everything they need beyond osdep.h.
+If exceptions are needed for some reason, they must be documented in
+the header. If all that's needed from a header is typedefs, consider
+putting those into qemu/typedefs.h instead of including the header.
+
+Cyclic inclusion is forbidden.
+
C types
=======
--
2.38.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] Clean up includes
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
` (3 preceding siblings ...)
2022-12-22 12:08 ` [PATCH v2 4/4] docs/devel: Rules on #include in headers Markus Armbruster
@ 2022-12-22 12:10 ` Markus Armbruster
2023-01-05 1:50 ` Michael S. Tsirkin
2023-01-08 6:49 ` Michael S. Tsirkin
6 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2022-12-22 12:10 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex
Michael, I forgot to add your R-bys. I'll fix that in the next
revision if I need one, else in the pull request.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] include/hw/virtio: Break inclusion loop
2022-12-22 12:08 ` [PATCH v2 1/4] include/hw/virtio: Break inclusion loop Markus Armbruster
@ 2022-12-23 2:58 ` Jason Wang
2022-12-23 3:31 ` Edgar E. Iglesias
0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2022-12-23 2:58 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, mst, imammedo, ani, peter.maydell, laurent,
edgar.iglesias, Alistair.Francis, bin.meng, palmer,
marcel.apfelbaum, yangxiaojuan, gaosong, richard.henderson,
deller, vikram.garhwal, francisco.iglesias, clg, kraxel,
marcandre.lureau, riku.voipio, qemu-arm, qemu-riscv, qemu-ppc,
crwulff, marex, Philippe Mathieu-Daudé, Stefano Garzarella
On Thu, Dec 22, 2022 at 8:08 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> hw/virtio/virtio.h and hw/virtio/vhost.h include each other. The
> former doesn't actually need the latter, so drop that inclusion to
> break the loop.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> ---
> include/hw/virtio/virtio.h | 1 -
> hw/virtio/virtio-qmp.c | 1 +
> hw/virtio/virtio.c | 1 +
> 3 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 24561e933a..48ab2117b5 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -22,7 +22,6 @@
> #include "standard-headers/linux/virtio_config.h"
> #include "standard-headers/linux/virtio_ring.h"
> #include "qom/object.h"
> -#include "hw/virtio/vhost.h"
>
> /*
> * A guest should never accept this. It implies negotiation is broken
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index 8e7282658f..3d4497da99 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -11,6 +11,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/virtio/virtio.h"
> +#include "hw/virtio/vhost.h"
> #include "virtio-qmp.h"
>
> #include "standard-headers/linux/virtio_ids.h"
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 289eb71045..0ec6ff9ae4 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -25,6 +25,7 @@
> #include "qom/object_interfaces.h"
> #include "hw/core/cpu.h"
> #include "hw/virtio/virtio.h"
> +#include "hw/virtio/vhost.h"
> #include "migration/qemu-file-types.h"
> #include "qemu/atomic.h"
> #include "hw/virtio/virtio-bus.h"
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] include/hw/virtio: Break inclusion loop
2022-12-23 2:58 ` Jason Wang
@ 2022-12-23 3:31 ` Edgar E. Iglesias
0 siblings, 0 replies; 14+ messages in thread
From: Edgar E. Iglesias @ 2022-12-23 3:31 UTC (permalink / raw)
To: Jason Wang
Cc: Markus Armbruster, qemu-devel, mst, imammedo, ani, peter.maydell,
laurent, Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, vikram.garhwal,
francisco.iglesias, clg, kraxel, marcandre.lureau, riku.voipio,
qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex,
Philippe Mathieu-Daudé, Stefano Garzarella
On Fri, Dec 23, 2022 at 10:58:06AM +0800, Jason Wang wrote:
> On Thu, Dec 22, 2022 at 8:08 PM Markus Armbruster <armbru@redhat.com> wrote:
> >
> > hw/virtio/virtio.h and hw/virtio/vhost.h include each other. The
> > former doesn't actually need the latter, so drop that inclusion to
> > break the loop.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Edgar E. Iglesias <edgar@zeroasic.com>
>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
> Thanks
>
> > ---
> > include/hw/virtio/virtio.h | 1 -
> > hw/virtio/virtio-qmp.c | 1 +
> > hw/virtio/virtio.c | 1 +
> > 3 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 24561e933a..48ab2117b5 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -22,7 +22,6 @@
> > #include "standard-headers/linux/virtio_config.h"
> > #include "standard-headers/linux/virtio_ring.h"
> > #include "qom/object.h"
> > -#include "hw/virtio/vhost.h"
> >
> > /*
> > * A guest should never accept this. It implies negotiation is broken
> > diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> > index 8e7282658f..3d4497da99 100644
> > --- a/hw/virtio/virtio-qmp.c
> > +++ b/hw/virtio/virtio-qmp.c
> > @@ -11,6 +11,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "hw/virtio/virtio.h"
> > +#include "hw/virtio/vhost.h"
> > #include "virtio-qmp.h"
> >
> > #include "standard-headers/linux/virtio_ids.h"
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 289eb71045..0ec6ff9ae4 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -25,6 +25,7 @@
> > #include "qom/object_interfaces.h"
> > #include "hw/core/cpu.h"
> > #include "hw/virtio/virtio.h"
> > +#include "hw/virtio/vhost.h"
> > #include "migration/qemu-file-types.h"
> > #include "qemu/atomic.h"
> > #include "hw/virtio/virtio-bus.h"
> > --
> > 2.38.1
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] docs/devel: Rules on #include in headers
2022-12-22 12:08 ` [PATCH v2 4/4] docs/devel: Rules on #include in headers Markus Armbruster
@ 2022-12-23 10:47 ` Bernhard Beschow
2023-01-09 12:01 ` Markus Armbruster
2022-12-23 23:41 ` Alex Bennée
1 sibling, 1 reply; 14+ messages in thread
From: Bernhard Beschow @ 2022-12-23 10:47 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: mst, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex
Am 22. Dezember 2022 12:08:13 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Rules for headers were proposed a long time ago, and generally liked:
>
> Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
>
>Wortk them into docs/devel/style.rst.
>
>Suggested-by: Bernhard Beschow <shentey@gmail.com>
>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>---
> docs/devel/style.rst | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/docs/devel/style.rst b/docs/devel/style.rst
>index 7ddd42b6c2..68aa776930 100644
>--- a/docs/devel/style.rst
>+++ b/docs/devel/style.rst
>@@ -293,6 +293,13 @@ that QEMU depends on.
> Do not include "qemu/osdep.h" from header files since the .c file will have
> already included it.
>
>+Headers should normally include everything they need beyond osdep.h.
>+If exceptions are needed for some reason, they must be documented in
>+the header. If all that's needed from a header is typedefs, consider
>+putting those into qemu/typedefs.h instead of including the header.
>+
>+Cyclic inclusion is forbidden.
>+
Nice!
I wonder if these should be bullet points like in your mail from 2016. I found them crystal clear since they looked like a todo list for review.
Feel free to respin. Either way:
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
> C types
> =======
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] docs/devel: Rules on #include in headers
2022-12-22 12:08 ` [PATCH v2 4/4] docs/devel: Rules on #include in headers Markus Armbruster
2022-12-23 10:47 ` Bernhard Beschow
@ 2022-12-23 23:41 ` Alex Bennée
1 sibling, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2022-12-23 23:41 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, mst, imammedo, ani, peter.maydell, laurent,
edgar.iglesias, Alistair.Francis, bin.meng, palmer,
marcel.apfelbaum, yangxiaojuan, gaosong, richard.henderson,
deller, jasowang, vikram.garhwal, francisco.iglesias, clg, kraxel,
marcandre.lureau, riku.voipio, qemu-riscv, qemu-ppc, crwulff,
marex, Bernhard Beschow, qemu-arm
Markus Armbruster <armbru@redhat.com> writes:
> Rules for headers were proposed a long time ago, and generally liked:
>
> Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
>
> Wortk them into docs/devel/style.rst.
nit: spelling Work
>
> Suggested-by: Bernhard Beschow <shentey@gmail.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> docs/devel/style.rst | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 7ddd42b6c2..68aa776930 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -293,6 +293,13 @@ that QEMU depends on.
> Do not include "qemu/osdep.h" from header files since the .c file will have
> already included it.
>
> +Headers should normally include everything they need beyond osdep.h.
> +If exceptions are needed for some reason, they must be documented in
> +the header. If all that's needed from a header is typedefs, consider
> +putting those into qemu/typedefs.h instead of including the header.
> +
> +Cyclic inclusion is forbidden.
> +
> C types
> =======
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] Clean up includes
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
` (4 preceding siblings ...)
2022-12-22 12:10 ` [PATCH v2 0/4] Clean up includes Markus Armbruster
@ 2023-01-05 1:50 ` Michael S. Tsirkin
2023-01-08 6:49 ` Michael S. Tsirkin
6 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2023-01-05 1:50 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex
On Thu, Dec 22, 2022 at 01:08:09PM +0100, Markus Armbruster wrote:
> Back in 2016, we discussed[1] rules for headers, and these were
> generally liked:
>
> 1. Have a carefully curated header that's included everywhere first. We
> got that already thanks to Peter: osdep.h.
>
> 2. Headers should normally include everything they need beyond osdep.h.
> If exceptions are needed for some reason, they must be documented in
> the header. If all that's needed from a header is typedefs, put
> those into qemu/typedefs.h instead of including the header.
>
> 3. Cyclic inclusion is forbidden.
>
> This series fixes a number of rule violations.
Conflicted with some patches I'm merging so I queued this up too.
Thanks!
> It is based on
>
> [PATCH v2 0/4] hw/ppc: Clean up includes
> [PATCH v2 0/7] include/hw/pci include/hw/cxl: Clean up includes
> [PATCH v2 0/3] block: Clean up includes
> [PATCH v3 0/5] coroutine: Clean up includes
>
> With all of these applied, just three inclusion loops remain reachable
> from include/:
>
> target/microblaze/cpu.h target/microblaze/mmu.h
>
> target/nios2/cpu.h target/nios2/mmu.h
>
> target/riscv/cpu.h target/riscv/pmp.h
>
> Breaking them would be nice, but I'm out of steam.
>
> v2:
> * Rebased
> * PATCH 3: v1 posted separately
> * PATCH 4: New
>
> [1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
>
> Based-on: <20221222104628.659681-1-armbru@redhat.com>
>
> Markus Armbruster (4):
> include/hw/virtio: Break inclusion loop
> include: Include headers where needed
> include: Don't include qemu/osdep.h
> docs/devel: Rules on #include in headers
>
> docs/devel/style.rst | 7 +++++++
> bsd-user/qemu.h | 1 -
> crypto/block-luks-priv.h | 1 -
> include/exec/plugin-gen.h | 1 +
> include/hw/acpi/erst.h | 3 +++
> include/hw/char/cmsdk-apb-uart.h | 1 +
> include/hw/char/goldfish_tty.h | 1 +
> include/hw/char/xilinx_uartlite.h | 1 +
> include/hw/cris/etraxfs.h | 1 +
> include/hw/cxl/cxl_host.h | 1 -
> include/hw/display/macfb.h | 3 ++-
> include/hw/dma/sifive_pdma.h | 2 ++
> include/hw/i386/ioapic_internal.h | 1 +
> include/hw/i386/sgx-epc.h | 1 +
> include/hw/input/pl050.h | 1 -
> include/hw/intc/goldfish_pic.h | 2 ++
> include/hw/intc/loongarch_pch_msi.h | 2 ++
> include/hw/intc/loongarch_pch_pic.h | 2 ++
> include/hw/intc/nios2_vic.h | 2 ++
> include/hw/misc/mchp_pfsoc_dmc.h | 2 ++
> include/hw/misc/mchp_pfsoc_ioscb.h | 2 ++
> include/hw/misc/mchp_pfsoc_sysreg.h | 2 ++
> include/hw/misc/pvpanic.h | 1 +
> include/hw/misc/sifive_e_prci.h | 3 ++-
> include/hw/misc/sifive_u_otp.h | 3 ++-
> include/hw/misc/sifive_u_prci.h | 3 ++-
> include/hw/misc/virt_ctrl.h | 2 ++
> include/hw/misc/xlnx-versal-pmc-iou-slcr.h | 1 +
> include/hw/net/lasi_82596.h | 2 +-
> include/hw/net/xlnx-zynqmp-can.h | 1 +
> include/hw/ppc/pnv_psi.h | 2 +-
> include/hw/riscv/boot_opensbi.h | 2 ++
> include/hw/riscv/microchip_pfsoc.h | 3 +++
> include/hw/riscv/numa.h | 1 +
> include/hw/riscv/sifive_u.h | 2 ++
> include/hw/riscv/spike.h | 2 +-
> include/hw/riscv/virt.h | 2 +-
> include/hw/ssi/sifive_spi.h | 3 +++
> include/hw/timer/sse-timer.h | 1 +
> include/hw/tricore/triboard.h | 1 -
> include/hw/usb/hcd-dwc3.h | 1 +
> include/hw/usb/hcd-musb.h | 2 ++
> include/hw/usb/xlnx-usb-subsystem.h | 2 ++
> include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 3 +++
> include/hw/virtio/virtio-mmio.h | 2 +-
> include/hw/virtio/virtio.h | 1 -
> include/qemu/plugin-memory.h | 3 +++
> include/qemu/userfaultfd.h | 1 -
> include/sysemu/dirtyrate.h | 2 ++
> include/sysemu/dump.h | 1 +
> include/user/syscall-trace.h | 1 +
> net/vmnet_int.h | 1 -
> qga/cutils.h | 1 -
> target/hexagon/hex_arch_types.h | 1 -
> target/hexagon/mmvec/macros.h | 1 -
> target/riscv/pmu.h | 1 -
> hw/virtio/virtio-qmp.c | 1 +
> hw/virtio/virtio.c | 1 +
> qga/cutils.c | 3 ++-
> 59 files changed, 82 insertions(+), 22 deletions(-)
>
> --
> 2.38.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] Clean up includes
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
` (5 preceding siblings ...)
2023-01-05 1:50 ` Michael S. Tsirkin
@ 2023-01-08 6:49 ` Michael S. Tsirkin
6 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2023-01-08 6:49 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex
On Thu, Dec 22, 2022 at 01:08:09PM +0100, Markus Armbruster wrote:
> Back in 2016, we discussed[1] rules for headers, and these were
> generally liked:
>
> 1. Have a carefully curated header that's included everywhere first. We
> got that already thanks to Peter: osdep.h.
>
> 2. Headers should normally include everything they need beyond osdep.h.
> If exceptions are needed for some reason, they must be documented in
> the header. If all that's needed from a header is typedefs, put
> those into qemu/typedefs.h instead of including the header.
>
> 3. Cyclic inclusion is forbidden.
>
> This series fixes a number of rule violations.
I had to drop this for now due to failures on bsd in particular.
See Peter's answer to my pull.
Markus when you merge this feel free to use:
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> It is based on
>
> [PATCH v2 0/4] hw/ppc: Clean up includes
> [PATCH v2 0/7] include/hw/pci include/hw/cxl: Clean up includes
> [PATCH v2 0/3] block: Clean up includes
> [PATCH v3 0/5] coroutine: Clean up includes
>
> With all of these applied, just three inclusion loops remain reachable
> from include/:
>
> target/microblaze/cpu.h target/microblaze/mmu.h
>
> target/nios2/cpu.h target/nios2/mmu.h
>
> target/riscv/cpu.h target/riscv/pmp.h
>
> Breaking them would be nice, but I'm out of steam.
>
> v2:
> * Rebased
> * PATCH 3: v1 posted separately
> * PATCH 4: New
>
> [1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
>
> Based-on: <20221222104628.659681-1-armbru@redhat.com>
>
> Markus Armbruster (4):
> include/hw/virtio: Break inclusion loop
> include: Include headers where needed
> include: Don't include qemu/osdep.h
> docs/devel: Rules on #include in headers
>
> docs/devel/style.rst | 7 +++++++
> bsd-user/qemu.h | 1 -
> crypto/block-luks-priv.h | 1 -
> include/exec/plugin-gen.h | 1 +
> include/hw/acpi/erst.h | 3 +++
> include/hw/char/cmsdk-apb-uart.h | 1 +
> include/hw/char/goldfish_tty.h | 1 +
> include/hw/char/xilinx_uartlite.h | 1 +
> include/hw/cris/etraxfs.h | 1 +
> include/hw/cxl/cxl_host.h | 1 -
> include/hw/display/macfb.h | 3 ++-
> include/hw/dma/sifive_pdma.h | 2 ++
> include/hw/i386/ioapic_internal.h | 1 +
> include/hw/i386/sgx-epc.h | 1 +
> include/hw/input/pl050.h | 1 -
> include/hw/intc/goldfish_pic.h | 2 ++
> include/hw/intc/loongarch_pch_msi.h | 2 ++
> include/hw/intc/loongarch_pch_pic.h | 2 ++
> include/hw/intc/nios2_vic.h | 2 ++
> include/hw/misc/mchp_pfsoc_dmc.h | 2 ++
> include/hw/misc/mchp_pfsoc_ioscb.h | 2 ++
> include/hw/misc/mchp_pfsoc_sysreg.h | 2 ++
> include/hw/misc/pvpanic.h | 1 +
> include/hw/misc/sifive_e_prci.h | 3 ++-
> include/hw/misc/sifive_u_otp.h | 3 ++-
> include/hw/misc/sifive_u_prci.h | 3 ++-
> include/hw/misc/virt_ctrl.h | 2 ++
> include/hw/misc/xlnx-versal-pmc-iou-slcr.h | 1 +
> include/hw/net/lasi_82596.h | 2 +-
> include/hw/net/xlnx-zynqmp-can.h | 1 +
> include/hw/ppc/pnv_psi.h | 2 +-
> include/hw/riscv/boot_opensbi.h | 2 ++
> include/hw/riscv/microchip_pfsoc.h | 3 +++
> include/hw/riscv/numa.h | 1 +
> include/hw/riscv/sifive_u.h | 2 ++
> include/hw/riscv/spike.h | 2 +-
> include/hw/riscv/virt.h | 2 +-
> include/hw/ssi/sifive_spi.h | 3 +++
> include/hw/timer/sse-timer.h | 1 +
> include/hw/tricore/triboard.h | 1 -
> include/hw/usb/hcd-dwc3.h | 1 +
> include/hw/usb/hcd-musb.h | 2 ++
> include/hw/usb/xlnx-usb-subsystem.h | 2 ++
> include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 3 +++
> include/hw/virtio/virtio-mmio.h | 2 +-
> include/hw/virtio/virtio.h | 1 -
> include/qemu/plugin-memory.h | 3 +++
> include/qemu/userfaultfd.h | 1 -
> include/sysemu/dirtyrate.h | 2 ++
> include/sysemu/dump.h | 1 +
> include/user/syscall-trace.h | 1 +
> net/vmnet_int.h | 1 -
> qga/cutils.h | 1 -
> target/hexagon/hex_arch_types.h | 1 -
> target/hexagon/mmvec/macros.h | 1 -
> target/riscv/pmu.h | 1 -
> hw/virtio/virtio-qmp.c | 1 +
> hw/virtio/virtio.c | 1 +
> qga/cutils.c | 3 ++-
> 59 files changed, 82 insertions(+), 22 deletions(-)
>
> --
> 2.38.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/4] include: Don't include qemu/osdep.h
2022-12-22 12:08 ` [PATCH v2 3/4] include: Don't include qemu/osdep.h Markus Armbruster
@ 2023-01-08 6:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2023-01-08 6:51 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, imammedo, ani, peter.maydell, laurent, edgar.iglesias,
Alistair.Francis, bin.meng, palmer, marcel.apfelbaum,
yangxiaojuan, gaosong, richard.henderson, deller, jasowang,
vikram.garhwal, francisco.iglesias, clg, kraxel, marcandre.lureau,
riku.voipio, qemu-arm, qemu-riscv, qemu-ppc, crwulff, marex,
Philippe Mathieu-Daudé, Bin Meng, Taylor Simpson
On Thu, Dec 22, 2022 at 01:08:12PM +0100, Markus Armbruster wrote:
> docs/devel/style.rst mandates:
>
> The "qemu/osdep.h" header contains preprocessor macros that affect
> the behavior of core system headers like <stdint.h>. It must be
> the first include so that core system headers included by external
> libraries get the preprocessor macros that QEMU depends on.
>
> Do not include "qemu/osdep.h" from header files since the .c file
> will have already included it.
>
> A few violations have crept in. Fix them.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Dropped this one due to CI failures.
> ---
> bsd-user/qemu.h | 1 -
> crypto/block-luks-priv.h | 1 -
> include/hw/cxl/cxl_host.h | 1 -
> include/hw/input/pl050.h | 1 -
> include/hw/tricore/triboard.h | 1 -
> include/qemu/userfaultfd.h | 1 -
> net/vmnet_int.h | 1 -
> qga/cutils.h | 1 -
> target/hexagon/hex_arch_types.h | 1 -
> target/hexagon/mmvec/macros.h | 1 -
> target/riscv/pmu.h | 1 -
> qga/cutils.c | 3 ++-
> 12 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index be6105385e..0ceecfb6df 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -17,7 +17,6 @@
> #ifndef QEMU_H
> #define QEMU_H
>
> -#include "qemu/osdep.h"
> #include "cpu.h"
> #include "qemu/units.h"
> #include "exec/cpu_ldst.h"
> diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h
> index dc2dd14e52..8fc967afcb 100644
> --- a/crypto/block-luks-priv.h
> +++ b/crypto/block-luks-priv.h
> @@ -18,7 +18,6 @@
> *
> */
>
> -#include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/bswap.h"
>
> diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
> index a1b662ce40..c9bc9c7c50 100644
> --- a/include/hw/cxl/cxl_host.h
> +++ b/include/hw/cxl/cxl_host.h
> @@ -7,7 +7,6 @@
> * COPYING file in the top-level directory.
> */
>
> -#include "qemu/osdep.h"
> #include "hw/cxl/cxl.h"
> #include "hw/boards.h"
>
> diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h
> index 89ec4fafc9..4cb8985f31 100644
> --- a/include/hw/input/pl050.h
> +++ b/include/hw/input/pl050.h
> @@ -10,7 +10,6 @@
> #ifndef HW_PL050_H
> #define HW_PL050_H
>
> -#include "qemu/osdep.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> #include "hw/input/ps2.h"
> diff --git a/include/hw/tricore/triboard.h b/include/hw/tricore/triboard.h
> index 094c8bd563..4fdd2d7d97 100644
> --- a/include/hw/tricore/triboard.h
> +++ b/include/hw/tricore/triboard.h
> @@ -18,7 +18,6 @@
> * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> */
>
> -#include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "hw/boards.h"
> #include "sysemu/sysemu.h"
> diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
> index 6b74f92792..55c95998e8 100644
> --- a/include/qemu/userfaultfd.h
> +++ b/include/qemu/userfaultfd.h
> @@ -13,7 +13,6 @@
> #ifndef USERFAULTFD_H
> #define USERFAULTFD_H
>
> -#include "qemu/osdep.h"
> #include "exec/hwaddr.h"
> #include <linux/userfaultfd.h>
>
> diff --git a/net/vmnet_int.h b/net/vmnet_int.h
> index adf6e8c20d..d0b90594f2 100644
> --- a/net/vmnet_int.h
> +++ b/net/vmnet_int.h
> @@ -10,7 +10,6 @@
> #ifndef VMNET_INT_H
> #define VMNET_INT_H
>
> -#include "qemu/osdep.h"
> #include "vmnet_int.h"
> #include "clients.h"
>
> diff --git a/qga/cutils.h b/qga/cutils.h
> index f0f30a7d28..2bfaf554a8 100644
> --- a/qga/cutils.h
> +++ b/qga/cutils.h
> @@ -1,7 +1,6 @@
> #ifndef CUTILS_H_
> #define CUTILS_H_
>
> -#include "qemu/osdep.h"
>
> int qga_open_cloexec(const char *name, int flags, mode_t mode);
>
> diff --git a/target/hexagon/hex_arch_types.h b/target/hexagon/hex_arch_types.h
> index 885f68f760..52a7f2b2f3 100644
> --- a/target/hexagon/hex_arch_types.h
> +++ b/target/hexagon/hex_arch_types.h
> @@ -18,7 +18,6 @@
> #ifndef HEXAGON_HEX_ARCH_TYPES_H
> #define HEXAGON_HEX_ARCH_TYPES_H
>
> -#include "qemu/osdep.h"
> #include "mmvec/mmvec.h"
> #include "qemu/int128.h"
>
> diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
> index 8c864e8c68..1201d778d0 100644
> --- a/target/hexagon/mmvec/macros.h
> +++ b/target/hexagon/mmvec/macros.h
> @@ -18,7 +18,6 @@
> #ifndef HEXAGON_MMVEC_MACROS_H
> #define HEXAGON_MMVEC_MACROS_H
>
> -#include "qemu/osdep.h"
> #include "qemu/host-utils.h"
> #include "arch.h"
> #include "mmvec/system_ext_mmvec.h"
> diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
> index 3004ce37b6..0c819ca983 100644
> --- a/target/riscv/pmu.h
> +++ b/target/riscv/pmu.h
> @@ -16,7 +16,6 @@
> * this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -#include "qemu/osdep.h"
> #include "qemu/log.h"
> #include "cpu.h"
> #include "qemu/main-loop.h"
> diff --git a/qga/cutils.c b/qga/cutils.c
> index b8e142ef64..b21bcf3683 100644
> --- a/qga/cutils.c
> +++ b/qga/cutils.c
> @@ -2,8 +2,9 @@
> * This work is licensed under the terms of the GNU GPL, version 2 or later.
> * See the COPYING file in the top-level directory.
> */
> -#include "cutils.h"
>
> +#include "qemu/osdep.h"
> +#include "cutils.h"
> #include "qapi/error.h"
>
> /**
> --
> 2.38.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] docs/devel: Rules on #include in headers
2022-12-23 10:47 ` Bernhard Beschow
@ 2023-01-09 12:01 ` Markus Armbruster
0 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2023-01-09 12:01 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, mst, imammedo, ani, peter.maydell, laurent,
edgar.iglesias, Alistair.Francis, bin.meng, palmer,
marcel.apfelbaum, yangxiaojuan, gaosong, richard.henderson,
deller, jasowang, vikram.garhwal, francisco.iglesias, clg, kraxel,
marcandre.lureau, riku.voipio, qemu-arm, qemu-riscv, qemu-ppc,
crwulff, marex
Bernhard Beschow <shentey@gmail.com> writes:
> Am 22. Dezember 2022 12:08:13 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>Rules for headers were proposed a long time ago, and generally liked:
>>
>> Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
>> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
>>
>>Wortk them into docs/devel/style.rst.
>>
>>Suggested-by: Bernhard Beschow <shentey@gmail.com>
>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>---
>> docs/devel/style.rst | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>>diff --git a/docs/devel/style.rst b/docs/devel/style.rst
>>index 7ddd42b6c2..68aa776930 100644
>>--- a/docs/devel/style.rst
>>+++ b/docs/devel/style.rst
>>@@ -293,6 +293,13 @@ that QEMU depends on.
>> Do not include "qemu/osdep.h" from header files since the .c file will have
>> already included it.
>>
>>+Headers should normally include everything they need beyond osdep.h.
>>+If exceptions are needed for some reason, they must be documented in
>>+the header. If all that's needed from a header is typedefs, consider
>>+putting those into qemu/typedefs.h instead of including the header.
>>+
>>+Cyclic inclusion is forbidden.
>>+
>
> Nice!
>
> I wonder if these should be bullet points like in your mail from 2016. I found them crystal clear since they looked like a todo list for review.
I tried to blend my change in with the existing text.
> Feel free to respin. Either way:
>
> Reviewed-by: Bernhard Beschow <shentey@gmail.com>
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-01-09 12:26 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-22 12:08 [PATCH v2 0/4] Clean up includes Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 1/4] include/hw/virtio: Break inclusion loop Markus Armbruster
2022-12-23 2:58 ` Jason Wang
2022-12-23 3:31 ` Edgar E. Iglesias
2022-12-22 12:08 ` [PATCH v2 2/4] include: Include headers where needed Markus Armbruster
2022-12-22 12:08 ` [PATCH v2 3/4] include: Don't include qemu/osdep.h Markus Armbruster
2023-01-08 6:51 ` Michael S. Tsirkin
2022-12-22 12:08 ` [PATCH v2 4/4] docs/devel: Rules on #include in headers Markus Armbruster
2022-12-23 10:47 ` Bernhard Beschow
2023-01-09 12:01 ` Markus Armbruster
2022-12-23 23:41 ` Alex Bennée
2022-12-22 12:10 ` [PATCH v2 0/4] Clean up includes Markus Armbruster
2023-01-05 1:50 ` Michael S. Tsirkin
2023-01-08 6:49 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).