[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 14/28] m68k: replace cpu_m68k_init() with cpu_ge
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 14/28] m68k: replace cpu_m68k_init() with cpu_generic_init() |
Date: |
Mon, 17 Jul 2017 17:23:22 +0200 |
On Mon, 17 Jul 2017 17:05:15 +0200
Andreas Färber <address@hidden> wrote:
> Am 17.07.2017 um 12:41 schrieb Igor Mammedov:
> > On Sat, 15 Jul 2017 08:08:58 -1000
> > Richard Henderson <address@hidden> wrote:
> >
> >> On 07/14/2017 03:52 AM, Igor Mammedov wrote:
> >>> @@ -230,6 +230,8 @@ static void m68k_cpu_realizefn(DeviceState *dev,
> >>> Error **errp)
> >>> M68kCPUClass *mcc = M68K_CPU_GET_CLASS(dev);
> >>> Error *local_err = NULL;
> >>>
> >>> + register_m68k_insns(&cpu->env);
> >>> +
> >>
> >> I think it would make more sense to do this during m68k_tcg_init.
> >>
> > it seems that m68k_cpu_initfn accesses 'env' via some global,
> > while cpu_mk68k_init() used to access concrete pointer of just created cpu,
> >
> > how about moving register_m68k_insns() to m68k_cpu_initfn(), instead?
> > it should be equivalent to what cpu_mk68k_init() used to do.
>
> As a general note, realize should be re-entrant. Can't tell from the
> above diff whether that is the case here.
Looking at
void register_m68k_insns (CPUM68KState *env)
{
/* Build the opcode table only once to avoid
multithreading issues. */
if (opcode_table[0] != NULL) {
return;
}
it is save to use multiple times,
also looking further in it:
#define BASE(name, opcode, mask) \
register_opcode(disas_##name, 0x##opcode, 0x##mask)
#define INSN(name, opcode, mask, feature) do { \
if (m68k_feature(env, M68K_FEATURE_##feature)) \
BASE(name, opcode, mask); \
} while(0)
BASE(undef, 0000, 0000);
INSN(arith_im, 0080, fff8, CF_ISA_A);
INSN macro depends on enabled features, it might work with current code that
has no user settable features but it will break once that is available.
So I retract my suggestion to move register_m68k_insns() into m68k_cpu_initfn()
and keep it as it's in this patch (in m68k_cpu_realizefn()),
that way features theoretically set between initfn(and m68k_tcg_init) and
realize() will
have effect on created cpu and we won't have to fix it in future.
> Regards,
> Andreas
>
- [Qemu-devel] [PATCH 12/28] alpha: replace cpu_alpha_init() with cpu_generic_init(), (continued)
[Qemu-devel] [PATCH 16/28] nios2: replace cpu_nios2_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 15/28] microblaze: replace cpu_mb_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 17/28] tilegx: replace cpu_tilegx_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 18/28] xtensa: replace cpu_xtensa_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 20/28] sh4: replace cpu_sh4_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 19/28] tricore: replace cpu_tricore_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 22/28] cris: replace cpu_cris_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14
[Qemu-devel] [PATCH 21/28] arm: replace cpu_arm_init() with cpu_generic_init(), Igor Mammedov, 2017/07/14