qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 06/11] mac_oldworld: Rename ppc_heathrow_reset to ppc_heat


From: BALATON Zoltan
Subject: Re: [PATCH v5 06/11] mac_oldworld: Rename ppc_heathrow_reset to ppc_heathrow_cpu_reset
Date: Sat, 27 Jun 2020 00:22:42 +0200 (CEST)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

On Fri, 26 Jun 2020, Mark Cave-Ayland wrote:
On 16/06/2020 14:47, BALATON Zoltan wrote:

This function resets a CPU not the whole machine so reflect that in
its name.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ppc/mac_oldworld.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 4200008851..f97f241e0c 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -73,7 +73,7 @@ static uint64_t translate_kernel_address(void *opaque, 
uint64_t addr)
     return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
 }

-static void ppc_heathrow_reset(void *opaque)
+static void ppc_heathrow_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;

@@ -112,7 +112,7 @@ static void ppc_heathrow_init(MachineState *machine)

         /* Set time-base frequency to 16.6 Mhz */
         cpu_ppc_tb_init(env,  TBFREQ);
-        qemu_register_reset(ppc_heathrow_reset, cpu);
+        qemu_register_reset(ppc_heathrow_cpu_reset, cpu);
     }

     /* allocate RAM */

As per my previous comment on your earlier version, I don't agree with this - 
the
reset is being registered at board level, it just so happens that as it's only
touching the CPU due to the opaque being passed in.

I'd be inclined to pass in a suitable HeathrowMachineState object containing a
reference to the CPU instead.

It's not a board level reset func but a CPU level one. See where it's registered here:

https://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/mac_oldworld.c;h=f8c204ead73843098084bf5213ac4046d7d843c4;hb=HEAD#l111

One for each CPU and as there could be more than one CPU, this won't work with a single reference in HeathrowMachineState. We could reset CPUs in a board level reset func (added by next patch) but I don't know how to access CPU objects from MachineState (it did not look trivial when I've looked) so I just left it as it is for later clean up separate from this series. I've just renamed it to avoid confusion with board level reset func which is usually named *_reset but I could call that ppc_heathrow_board_reset and then we don't need this patch but I think this is cleaner.

I don't even know why we need a reset func to reset the CPU, I'd expect that to be the default behaviour of any board reset without needing to register a func to do it.

Regards,
BALATON Zoltan

reply via email to

[Prev in Thread] Current Thread [Next in Thread]