| Message ID | 52628688d4f3b34cefd0ebab12dd0f1ec8fe3618.1434432813.git.crosthwaite.peter@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 16 June 2015 at 06:46, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: > Use cpu_set_pc for setting program counters when bootloading. This > removes an instance of system level code having to reach into the CPU > env. > > Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> > --- > hw/microblaze/boot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c > index 4c44317..ec68479 100644 > --- a/hw/microblaze/boot.c > +++ b/hw/microblaze/boot.c > @@ -54,7 +54,7 @@ static void main_cpu_reset(void *opaque) > env->regs[5] = boot_info.cmdline; > env->regs[6] = boot_info.initrd_start; > env->regs[7] = boot_info.fdt; > - env->sregs[SR_PC] = boot_info.bootstrap_pc; > + cpu_set_pc(CPU(cpu), boot_info.bootstrap_pc, &error_abort); Well, it sort of removes an instance of reaching into the CPU env, but there's all those other ones in plain sight just above. Is there much point in setting SR_PC indirectly if we don't have a mechanism for setting the other regs indirectly? -- PMM
On Tue, Jun 16, 2015 at 4:33 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 16 June 2015 at 06:46, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: >> Use cpu_set_pc for setting program counters when bootloading. This >> removes an instance of system level code having to reach into the CPU >> env. >> >> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> >> --- >> hw/microblaze/boot.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c >> index 4c44317..ec68479 100644 >> --- a/hw/microblaze/boot.c >> +++ b/hw/microblaze/boot.c >> @@ -54,7 +54,7 @@ static void main_cpu_reset(void *opaque) >> env->regs[5] = boot_info.cmdline; >> env->regs[6] = boot_info.initrd_start; >> env->regs[7] = boot_info.fdt; >> - env->sregs[SR_PC] = boot_info.bootstrap_pc; >> + cpu_set_pc(CPU(cpu), boot_info.bootstrap_pc, &error_abort); > > Well, it sort of removes an instance of reaching into the CPU > env, but there's all those other ones in plain sight just above. > Is there much point in setting SR_PC indirectly if we don't > have a mechanism for setting the other regs indirectly? > Yes. Needs more patches :). I'm starting with the easy stuff, and I am actually more interested in getting rid of the SR_PC macro usage at this stage. ARMs solution to this is the machine code bootloader. That would be one way. But what I want to propose was that we add a virtual method to CPUs that sets these offsets that bootloaders can call. This brings us closer to one bootloader that can do it all. Regards, Peter > -- PMM >
Am 16.06.2015 um 07:46 schrieb Peter Crosthwaite: > Use cpu_set_pc for setting program counters when bootloading. This > removes an instance of system level code having to reach into the CPU > env. > > Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> Reviewed-by: Andreas Färber <afaerber@suse.de> but please remember to use "cpu_set_pc()" convention here and elsewhere in the commit messages including subjects. Regards, Andreas
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c index 4c44317..ec68479 100644 --- a/hw/microblaze/boot.c +++ b/hw/microblaze/boot.c @@ -54,7 +54,7 @@ static void main_cpu_reset(void *opaque) env->regs[5] = boot_info.cmdline; env->regs[6] = boot_info.initrd_start; env->regs[7] = boot_info.fdt; - env->sregs[SR_PC] = boot_info.bootstrap_pc; + cpu_set_pc(CPU(cpu), boot_info.bootstrap_pc, &error_abort); if (boot_info.machine_cpu_reset) { boot_info.machine_cpu_reset(cpu); }
Use cpu_set_pc for setting program counters when bootloading. This removes an instance of system level code having to reach into the CPU env. Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> --- hw/microblaze/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)