From: BALATON Zoltan <balaton@eik.bme.hu>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: qemu-ppc@nongnu.org,
Daniel Henrique Barboza <danielhb413@gmail.com>,
qemu-devel@nongnu.org, Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH v4 08/24] ppc/ppc4xx: Introduce a DCR device model
Date: Tue, 9 Aug 2022 19:21:49 +0200 (CEST) [thread overview]
Message-ID: <8dcf2a12-f799-673f-d5bf-1cecba42447a@eik.bme.hu> (raw)
In-Reply-To: <20220809153904.485018-9-clg@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 4374 bytes --]
On Tue, 9 Aug 2022, Cédric Le Goater wrote:
> The Device Control Registers (DCR) of on-SoC devices are accessed by
> software through the use of the mtdcr and mfdcr instructions. These
> are converted in transactions on a side band bus, the DCR bus, which
> connects the on-SoC devices to the CPU.
>
> Ideally, we should model these accesses with a DCR namespace and DCR
> memory regions but today the DCR handlers are installed in a DCR table
> under the CPU. Instead introduce a little device model wrapper to hold
> a CPU link and handle registration of DCR handlers.
>
> The DCR device inherits from SysBus because most of these devices also
> have MMIO regions and/or IRQs. Being a SysBusDevice makes things easier
> to install the device model in the overall SoC.
>
> The "cpu" link should be considered as modeling the piece of HW logic
> connecting the device to the DCR bus.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/ppc/ppc4xx.h | 17 ++++++++++++++++
> hw/ppc/ppc4xx_devs.c | 44 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index 591e2421a343..82e60b0e0742 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -27,6 +27,7 @@
>
> #include "hw/ppc/ppc.h"
> #include "exec/memory.h"
> +#include "hw/sysbus.h"
>
> void ppc4xx_sdram_banks(MemoryRegion *ram, int nr_banks,
> MemoryRegion ram_memories[],
> @@ -44,4 +45,20 @@ void ppc4xx_mal_init(CPUPPCState *env, uint8_t txcnum, uint8_t rxcnum,
>
> #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
>
> +/*
> + * Generic DCR device
> + */
> +#define TYPE_PPC4xx_DCR_DEVICE "ppc4xx-dcr-device"
> +OBJECT_DECLARE_SIMPLE_TYPE(Ppc4xxDcrDeviceState, PPC4xx_DCR_DEVICE);
> +struct Ppc4xxDcrDeviceState {
> + SysBusDevice parent_obj;
> +
> + PowerPCCPU *cpu;
> +};
> +
> +void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int dcrn,
> + dcr_read_cb dcr_read, dcr_write_cb dcr_write);
> +bool ppc4xx_dcr_realize(Ppc4xxDcrDeviceState *dev, PowerPCCPU *cpu,
> + Error **errp);
> +
> #endif /* PPC4XX_H */
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index 069b51195160..bce7ef461346 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -664,3 +664,47 @@ void ppc4xx_mal_init(CPUPPCState *env, uint8_t txcnum, uint8_t rxcnum,
> mal, &dcr_read_mal, &dcr_write_mal);
> }
> }
> +
> +void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int dcrn,
> + dcr_read_cb dcr_read, dcr_write_cb dcr_write)
I still think this should have a separate void *opaque parameter for the
callbacks and not pass dev for that as the callbacks could use anything
they wish for that parameter. (Additionally this allows dropping a lot of
QOM casts. If you want to see how often these are accessed, you can try
-trace enable="ppc_dcr*"; on the machines and OS I've tested some are
read/written frequently so I'd not add unnecessary overhead without a good
reason.)
Otherwise:
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATOn Zoltan
> +{
> + CPUPPCState *env;
> +
> + assert(dev->cpu);
> +
> + env = &dev->cpu->env;
> +
> + ppc_dcr_register(env, dcrn, dev, dcr_read, dcr_write);
> +}
> +
> +bool ppc4xx_dcr_realize(Ppc4xxDcrDeviceState *dev, PowerPCCPU *cpu,
> + Error **errp)
> +{
> + object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
> + return sysbus_realize(SYS_BUS_DEVICE(dev), errp);
> +}
> +
> +static Property ppc4xx_dcr_properties[] = {
> + DEFINE_PROP_LINK("cpu", Ppc4xxDcrDeviceState, cpu, TYPE_POWERPC_CPU,
> + PowerPCCPU *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void ppc4xx_dcr_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + device_class_set_props(dc, ppc4xx_dcr_properties);
> +}
> +
> +static const TypeInfo ppc4xx_types[] = {
> + {
> + .name = TYPE_PPC4xx_DCR_DEVICE,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(Ppc4xxDcrDeviceState),
> + .class_init = ppc4xx_dcr_class_init,
> + .abstract = true,
> + }
> +};
> +
> +DEFINE_TYPES(ppc4xx_types)
>
next prev parent reply other threads:[~2022-08-09 17:24 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-09 15:38 [PATCH v4 00/24] ppc: QOM'ify 405 board Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 01/24] ppc/ppc405: Remove taihu machine Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 02/24] ppc/ppc405: Introduce a PPC405 generic machine Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 03/24] ppc/ppc405: Move devices under the ref405ep machine Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 04/24] ppc/ppc405: Move SRAM " Cédric Le Goater
2022-08-09 16:53 ` BALATON Zoltan
2022-08-09 17:42 ` Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 05/24] ppc/ppc405: Introduce a PPC405 SoC Cédric Le Goater
2022-08-09 16:59 ` BALATON Zoltan
2022-08-09 15:38 ` [PATCH v4 06/24] ppc/ppc405: Start QOMification of the SoC Cédric Le Goater
2022-08-09 17:12 ` BALATON Zoltan
2022-08-09 15:38 ` [PATCH v4 07/24] ppc/ppc405: QOM'ify CPU Cédric Le Goater
2022-08-09 17:15 ` BALATON Zoltan
2022-08-09 17:39 ` BALATON Zoltan
2022-08-09 15:38 ` [PATCH v4 08/24] ppc/ppc4xx: Introduce a DCR device model Cédric Le Goater
2022-08-09 17:21 ` BALATON Zoltan [this message]
2022-08-10 12:38 ` Cédric Le Goater
2022-08-10 13:28 ` BALATON Zoltan
2022-08-10 13:57 ` Cédric Le Goater
2022-08-10 14:48 ` BALATON Zoltan
2022-08-11 7:09 ` Cédric Le Goater
2022-08-11 11:39 ` BALATON Zoltan
2022-08-11 12:20 ` Cédric Le Goater
2022-08-11 21:55 ` BALATON Zoltan
2022-08-09 15:38 ` [PATCH v4 09/24] ppc/ppc405: QOM'ify CPC Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 10/24] ppc/ppc405: QOM'ify GPT Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 11/24] ppc/ppc405: QOM'ify OCM Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 12/24] ppc/ppc405: QOM'ify GPIO Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 13/24] ppc/ppc405: QOM'ify DMA Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 14/24] ppc/ppc405: QOM'ify EBC Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 15/24] ppc/ppc405: QOM'ify OPBA Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 16/24] ppc/ppc405: QOM'ify POB Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 17/24] ppc/ppc405: QOM'ify PLB Cédric Le Goater
2022-08-09 15:38 ` [PATCH v4 18/24] ppc/ppc405: QOM'ify MAL Cédric Le Goater
2022-08-09 17:34 ` BALATON Zoltan
2022-08-10 6:17 ` Cédric Le Goater
2022-08-10 21:35 ` BALATON Zoltan
2022-08-09 15:38 ` [PATCH v4 19/24] ppc/ppc405: QOM'ify FPGA Cédric Le Goater
2022-08-09 17:37 ` BALATON Zoltan
2022-08-10 6:22 ` Cédric Le Goater
2022-08-10 11:40 ` BALATON Zoltan
2022-08-10 17:22 ` Daniel Henrique Barboza
2022-08-10 17:32 ` BALATON Zoltan
2022-08-09 15:39 ` [PATCH v4 20/24] ppc/ppc405: Use an embedded PPCUIC model in SoC state Cédric Le Goater
2022-08-09 17:40 ` BALATON Zoltan
2022-08-09 15:39 ` [PATCH v4 21/24] ppc/ppc405: Use an explicit I2C object Cédric Le Goater
2022-08-09 17:45 ` BALATON Zoltan
2022-08-09 15:39 ` [PATCH v4 22/24] ppc/ppc4xx: Fix sdram trace events Cédric Le Goater
2022-08-09 17:47 ` BALATON Zoltan
2022-08-09 15:39 ` [PATCH v4 23/24] ppc/ppc405: QOM'ify SDRAM Cédric Le Goater
2022-08-09 17:53 ` BALATON Zoltan
2022-08-10 6:26 ` Cédric Le Goater
2022-08-10 11:39 ` BALATON Zoltan
2022-08-09 15:39 ` [PATCH v4 24/24] ppc/ppc405: Add check on minimum RAM size Cédric Le Goater
2022-08-09 17:55 ` BALATON Zoltan
2022-08-10 6:24 ` Cédric Le Goater
2022-08-11 8:24 ` [PATCH v4 00/24] ppc: QOM'ify 405 board Daniel Henrique Barboza
2022-08-11 8:33 ` Cédric Le Goater
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=8dcf2a12-f799-673f-d5bf-1cecba42447a@eik.bme.hu \
--to=balaton@eik.bme.hu \
--cc=clg@kaod.org \
--cc=danielhb413@gmail.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@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 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.