From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, "Cédric Le Goater" <clg@kaod.org>,
"Daniel Henrique Barboza" <danielhb413@gmail.com>
Subject: Re: [PULL 09/12] include/hw/ppc: Split pnv_chip.h off pnv.h
Date: Mon, 23 Jan 2023 10:44:14 +0100 [thread overview]
Message-ID: <d8106f3d-ee31-ca51-40ba-4329238851de@linaro.org> (raw)
In-Reply-To: <20230120070122.3982588-10-armbru@redhat.com>
Hi Markus,
On 20/1/23 08:01, Markus Armbruster wrote:
> PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip are defined
> in pnv.h. Many users of the header don't actually need them. One
> instance is this inclusion loop: hw/ppc/pnv_homer.h includes
> hw/ppc/pnv.h for typedef PnvChip, and vice versa for struct PnvHomer.
>
> Similar structs live in their own headers: PnvHomerClass and PnvHomer
> in pnv_homer.h, PnvLpcClass and PnvLpcController in pci_lpc.h,
> PnvPsiClass, PnvPsi, Pnv8Psi, Pnv9Psi, Pnv10Psi in pnv_psi.h, ...
>
> Move PnvChipClass, PnvChip, Pnv8Chip, Pnv9Chip, and Pnv10Chip to new
> pnv_chip.h, and adjust include directives. This breaks the inclusion
> loop mentioned above.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Message-Id: <20221222104628.659681-2-armbru@redhat.com>
> ---
> include/hw/ppc/pnv.h | 143 +-----------------------------------
> include/hw/ppc/pnv_chip.h | 147 +++++++++++++++++++++++++++++++++++++
> hw/intc/pnv_xive.c | 1 +
> hw/intc/pnv_xive2.c | 1 +
> hw/pci-host/pnv_phb3.c | 1 +
> hw/pci-host/pnv_phb4_pec.c | 1 +
> hw/ppc/pnv.c | 3 +
> hw/ppc/pnv_core.c | 1 +
> hw/ppc/pnv_homer.c | 1 +
> hw/ppc/pnv_lpc.c | 1 +
> hw/ppc/pnv_xscom.c | 1 +
> 11 files changed, 160 insertions(+), 141 deletions(-)
> create mode 100644 include/hw/ppc/pnv_chip.h
>
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 9ef7e2d0dc..ca49e4281d 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -20,158 +20,19 @@
> #ifndef PPC_PNV_H
> #define PPC_PNV_H
>
> +#include "cpu.h"
Why is "cpu.h" required here? For pnv_chip_find_cpu()?
Isn't "target/ppc/cpu-qom.h" enough?
> #include "hw/boards.h"
> #include "hw/sysbus.h"
> #include "hw/ipmi/ipmi.h"
> -#include "hw/ppc/pnv_lpc.h"
> #include "hw/ppc/pnv_pnor.h"
> -#include "hw/ppc/pnv_psi.h"
> -#include "hw/ppc/pnv_occ.h"
> -#include "hw/ppc/pnv_sbe.h"
> -#include "hw/ppc/pnv_homer.h"
> -#include "hw/ppc/pnv_xive.h"
> -#include "hw/ppc/pnv_core.h"
> -#include "hw/pci-host/pnv_phb3.h"
> -#include "hw/pci-host/pnv_phb4.h"
> #include "hw/pci-host/pnv_phb.h"
> -#include "qom/object.h"
>
> #define TYPE_PNV_CHIP "pnv-chip"
> -OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,
> - PNV_CHIP)
>
> -struct PnvChip {
> - /*< private >*/
> - SysBusDevice parent_obj;
> -
> - /*< public >*/
> - uint32_t chip_id;
> - uint64_t ram_start;
> - uint64_t ram_size;
> -
> - uint32_t nr_cores;
> - uint32_t nr_threads;
> - uint64_t cores_mask;
> - PnvCore **cores;
> -
> - uint32_t num_pecs;
> -
> - MemoryRegion xscom_mmio;
> - MemoryRegion xscom;
> - AddressSpace xscom_as;
> -
> - MemoryRegion *fw_mr;
> - gchar *dt_isa_nodename;
> -};
> -
> -#define TYPE_PNV8_CHIP "pnv8-chip"
> +typedef struct PnvChip PnvChip;
> typedef struct Pnv8Chip Pnv8Chip;
> -DECLARE_INSTANCE_CHECKER(Pnv8Chip, PNV8_CHIP,
> - TYPE_PNV8_CHIP)
> -
> -struct Pnv8Chip {
> - /*< private >*/
> - PnvChip parent_obj;
> -
> - /*< public >*/
> - MemoryRegion icp_mmio;
> -
> - PnvLpcController lpc;
> - Pnv8Psi psi;
> - PnvOCC occ;
> - PnvHomer homer;
> -
> -#define PNV8_CHIP_PHB3_MAX 4
> - /*
> - * The array is used to allow quick access to the phbs by
> - * pnv_ics_get_child() and pnv_ics_resend_child().
> - */
> - PnvPHB *phbs[PNV8_CHIP_PHB3_MAX];
> - uint32_t num_phbs;
> -
> - XICSFabric *xics;
> -};
> -
> -#define TYPE_PNV9_CHIP "pnv9-chip"
> typedef struct Pnv9Chip Pnv9Chip;
> -DECLARE_INSTANCE_CHECKER(Pnv9Chip, PNV9_CHIP,
> - TYPE_PNV9_CHIP)
> -
> -struct Pnv9Chip {
> - /*< private >*/
> - PnvChip parent_obj;
> -
> - /*< public >*/
> - PnvXive xive;
> - Pnv9Psi psi;
> - PnvLpcController lpc;
> - PnvOCC occ;
> - PnvSBE sbe;
> - PnvHomer homer;
> -
> - uint32_t nr_quads;
> - PnvQuad *quads;
> -
> -#define PNV9_CHIP_MAX_PEC 3
> - PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
> -};
> -
> -/*
> - * A SMT8 fused core is a pair of SMT4 cores.
> - */
> -#define PNV9_PIR2FUSEDCORE(pir) (((pir) >> 3) & 0xf)
> -#define PNV9_PIR2CHIP(pir) (((pir) >> 8) & 0x7f)
> -
> -#define TYPE_PNV10_CHIP "pnv10-chip"
> typedef struct Pnv10Chip Pnv10Chip;
> -DECLARE_INSTANCE_CHECKER(Pnv10Chip, PNV10_CHIP,
> - TYPE_PNV10_CHIP)
> -
> -struct Pnv10Chip {
> - /*< private >*/
> - PnvChip parent_obj;
> -
> - /*< public >*/
> - PnvXive2 xive;
> - Pnv9Psi psi;
> - PnvLpcController lpc;
> - PnvOCC occ;
> - PnvSBE sbe;
> - PnvHomer homer;
> -
> - uint32_t nr_quads;
> - PnvQuad *quads;
> -
> -#define PNV10_CHIP_MAX_PEC 2
> - PnvPhb4PecState pecs[PNV10_CHIP_MAX_PEC];
> -};
> -
> -#define PNV10_PIR2FUSEDCORE(pir) (((pir) >> 3) & 0xf)
> -#define PNV10_PIR2CHIP(pir) (((pir) >> 8) & 0x7f)
> -
> -struct PnvChipClass {
> - /*< private >*/
> - SysBusDeviceClass parent_class;
> -
> - /*< public >*/
> - uint64_t chip_cfam_id;
> - uint64_t cores_mask;
> - uint32_t num_pecs;
> - uint32_t num_phbs;
> -
> - DeviceRealize parent_realize;
> -
> - uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> - void (*intc_create)(PnvChip *chip, PowerPCCPU *cpu, Error **errp);
> - void (*intc_reset)(PnvChip *chip, PowerPCCPU *cpu);
> - void (*intc_destroy)(PnvChip *chip, PowerPCCPU *cpu);
> - void (*intc_print_info)(PnvChip *chip, PowerPCCPU *cpu, Monitor *mon);
> - ISABus *(*isa_create)(PnvChip *chip, Error **errp);
> - void (*dt_populate)(PnvChip *chip, void *fdt);
> - void (*pic_print_info)(PnvChip *chip, Monitor *mon);
> - uint64_t (*xscom_core_base)(PnvChip *chip, uint32_t core_id);
> - uint32_t (*xscom_pcba)(PnvChip *chip, uint64_t addr);
> -};
>
> #define PNV_CHIP_TYPE_SUFFIX "-" TYPE_PNV_CHIP
> #define PNV_CHIP_TYPE_NAME(cpu_model) cpu_model PNV_CHIP_TYPE_SUFFIX
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> new file mode 100644
> index 0000000000..53e1d921d7
> --- /dev/null
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -0,0 +1,147 @@
> +#ifndef PPC_PNV_CHIP_H
> +#define PPC_PNV_CHIP_H
> +
> +#include "hw/pci-host/pnv_phb4.h"
> +#include "hw/ppc/pnv_core.h"
> +#include "hw/ppc/pnv_homer.h"
> +#include "hw/ppc/pnv_lpc.h"
> +#include "hw/ppc/pnv_occ.h"
> +#include "hw/ppc/pnv_psi.h"
> +#include "hw/ppc/pnv_sbe.h"
> +#include "hw/ppc/pnv_xive.h"
> +#include "hw/sysbus.h"
> +
> +OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,
> + PNV_CHIP)
> +
> +struct PnvChip {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> + uint32_t chip_id;
> + uint64_t ram_start;
> + uint64_t ram_size;
> +
> + uint32_t nr_cores;
> + uint32_t nr_threads;
> + uint64_t cores_mask;
> + PnvCore **cores;
> +
> + uint32_t num_pecs;
> +
> + MemoryRegion xscom_mmio;
> + MemoryRegion xscom;
> + AddressSpace xscom_as;
> +
> + MemoryRegion *fw_mr;
> + gchar *dt_isa_nodename;
> +};
> +
> +#define TYPE_PNV8_CHIP "pnv8-chip"
> +DECLARE_INSTANCE_CHECKER(Pnv8Chip, PNV8_CHIP,
> + TYPE_PNV8_CHIP)
> +
> +struct Pnv8Chip {
> + /*< private >*/
> + PnvChip parent_obj;
> +
> + /*< public >*/
> + MemoryRegion icp_mmio;
> +
> + PnvLpcController lpc;
> + Pnv8Psi psi;
> + PnvOCC occ;
> + PnvHomer homer;
> +
> +#define PNV8_CHIP_PHB3_MAX 4
> + /*
> + * The array is used to allow quick access to the phbs by
> + * pnv_ics_get_child() and pnv_ics_resend_child().
> + */
> + PnvPHB *phbs[PNV8_CHIP_PHB3_MAX];
> + uint32_t num_phbs;
> +
> + XICSFabric *xics;
> +};
> +
> +#define TYPE_PNV9_CHIP "pnv9-chip"
> +DECLARE_INSTANCE_CHECKER(Pnv9Chip, PNV9_CHIP,
> + TYPE_PNV9_CHIP)
> +
> +struct Pnv9Chip {
> + /*< private >*/
> + PnvChip parent_obj;
> +
> + /*< public >*/
> + PnvXive xive;
> + Pnv9Psi psi;
> + PnvLpcController lpc;
> + PnvOCC occ;
> + PnvSBE sbe;
> + PnvHomer homer;
> +
> + uint32_t nr_quads;
> + PnvQuad *quads;
> +
> +#define PNV9_CHIP_MAX_PEC 3
> + PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
> +};
> +
> +/*
> + * A SMT8 fused core is a pair of SMT4 cores.
> + */
> +#define PNV9_PIR2FUSEDCORE(pir) (((pir) >> 3) & 0xf)
> +#define PNV9_PIR2CHIP(pir) (((pir) >> 8) & 0x7f)
> +
> +#define TYPE_PNV10_CHIP "pnv10-chip"
> +DECLARE_INSTANCE_CHECKER(Pnv10Chip, PNV10_CHIP,
> + TYPE_PNV10_CHIP)
> +
> +struct Pnv10Chip {
> + /*< private >*/
> + PnvChip parent_obj;
> +
> + /*< public >*/
> + PnvXive2 xive;
> + Pnv9Psi psi;
> + PnvLpcController lpc;
> + PnvOCC occ;
> + PnvSBE sbe;
> + PnvHomer homer;
> +
> + uint32_t nr_quads;
> + PnvQuad *quads;
> +
> +#define PNV10_CHIP_MAX_PEC 2
> + PnvPhb4PecState pecs[PNV10_CHIP_MAX_PEC];
> +};
> +
> +#define PNV10_PIR2FUSEDCORE(pir) (((pir) >> 3) & 0xf)
> +#define PNV10_PIR2CHIP(pir) (((pir) >> 8) & 0x7f)
> +
> +struct PnvChipClass {
> + /*< private >*/
> + SysBusDeviceClass parent_class;
> +
> + /*< public >*/
> + uint64_t chip_cfam_id;
> + uint64_t cores_mask;
> + uint32_t num_pecs;
> + uint32_t num_phbs;
> +
> + DeviceRealize parent_realize;
> +
> + uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> + void (*intc_create)(PnvChip *chip, PowerPCCPU *cpu, Error **errp);
> + void (*intc_reset)(PnvChip *chip, PowerPCCPU *cpu);
> + void (*intc_destroy)(PnvChip *chip, PowerPCCPU *cpu);
> + void (*intc_print_info)(PnvChip *chip, PowerPCCPU *cpu, Monitor *mon);
> + ISABus *(*isa_create)(PnvChip *chip, Error **errp);
> + void (*dt_populate)(PnvChip *chip, void *fdt);
> + void (*pic_print_info)(PnvChip *chip, Monitor *mon);
> + uint64_t (*xscom_core_base)(PnvChip *chip, uint32_t core_id);
> + uint32_t (*xscom_pcba)(PnvChip *chip, uint64_t addr);
> +};
> +
> +#endif
next prev parent reply other threads:[~2023-01-23 9:45 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-20 7:01 [PULL 00/12] Header cleanup patches for 2023-01-20 Markus Armbruster
2023-01-20 7:01 ` [PULL 01/12] coroutine: Clean up superfluous inclusion of qemu/coroutine.h Markus Armbruster
2023-01-20 7:01 ` [PULL 02/12] coroutine: Move coroutine_fn to qemu/osdep.h, trim includes Markus Armbruster
2023-01-20 7:01 ` [PULL 03/12] coroutine: Clean up superfluous inclusion of qemu/lockable.h Markus Armbruster
2023-01-20 7:01 ` [PULL 04/12] coroutine: Split qemu/coroutine-core.h off qemu/coroutine.h Markus Armbruster
2023-01-20 7:01 ` [PULL 05/12] coroutine: Use Coroutine typedef name instead of structure tag Markus Armbruster
2023-01-20 7:01 ` [PULL 06/12] include/block: Untangle inclusion loops Markus Armbruster
2023-01-20 7:01 ` [PULL 07/12] hw/sparc64/niagara: Use blk_name() instead of open-coding it Markus Armbruster
2023-01-20 7:01 ` [PULL 08/12] include/hw/block: Include hw/block/block.h where needed Markus Armbruster
2023-01-20 7:01 ` [PULL 09/12] include/hw/ppc: Split pnv_chip.h off pnv.h Markus Armbruster
2023-01-23 9:44 ` Philippe Mathieu-Daudé [this message]
2023-01-23 10:07 ` Markus Armbruster
2023-01-23 12:10 ` Philippe Mathieu-Daudé
2023-01-20 7:01 ` [PULL 10/12] include/hw/ppc: Supply a few missing includes Markus Armbruster
2023-01-20 7:01 ` [PULL 11/12] include/hw/ppc: Don't include hw/pci-host/pnv_phb.h from pnv.h Markus Armbruster
2023-01-20 7:01 ` [PULL 12/12] include/hw/ppc include/hw/pci-host: Drop extra typedefs Markus Armbruster
2023-01-20 16:17 ` [PULL 00/12] Header cleanup patches for 2023-01-20 Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d8106f3d-ee31-ca51-40ba-4329238851de@linaro.org \
--to=philmd@linaro.org \
--cc=armbru@redhat.com \
--cc=clg@kaod.org \
--cc=danielhb413@gmail.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).