From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com [IPv6:2607:f8b0:4864:20::72c]) by sourceware.org (Postfix) with ESMTPS id 1EC923888018 for ; Tue, 13 Jul 2021 05:42:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1EC923888018 Received: by mail-qk1-x72c.google.com with SMTP id p202so4042091qka.12 for ; Mon, 12 Jul 2021 22:42:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=kEb9W7Z6hvcV971kSUgOWdtxB5a5GMm/pV+yOqtZ15w=; b=sJL/JQIpOj0hi+OW8aDhZ4v8ozEcjR+hQBdYSnKpa7Q6E7PI1zfL1XQbJykYItf0As NS5cBCuR4shmUb/ENXGZ29xmcZVcDmwdBeHcHTzRU1LyLwURpR2UFlzsct3TPUDm9e8b 0iXll+RHn4rvidPNcWFF1X4UR4oCGTKnumqiPTFtX9UNfxE8Rd/YX1nWwXPAbyGocfBK 2lKyjCuFptiMFPCl0DNShj53etoeTqHxxXPC6rOTiLT6+DcHGlkOMse5Y5nTVtnEQLRN 0FOUGMUx5LahB1sXvNJK9FD2X6vRFG2O0aR785UdAVJP/1UcygEpIbYPjIiy14QIZkw4 lBug== X-Gm-Message-State: AOAM532zsJ5IN3w4bNSiSA18Aa3HBweSyVNQkXyHtgdgTcz74gyiImTe oMsqzcY6UR3Z0PSzqjDF01Byb7JrrxP6DSmQ4oU= X-Google-Smtp-Source: ABdhPJwapjPIeDOJMgtADwyz/P+g6Y2Vr+L6aEiDhx1w/Ngv+dZymnDowmHhyyl/+sxIpe2rLMWOsyxjA9AUmhGk2b4= X-Received: by 2002:a05:620a:4151:: with SMTP id k17mr2512324qko.10.1626154935592; Mon, 12 Jul 2021 22:42:15 -0700 (PDT) MIME-Version: 1.0 References: <5a120c4d-630a-08f5-fb81-d35237267adb@simark.ca> <20200925114042.14337-1-Sergey.Belyashov@gmail.com> In-Reply-To: From: Sergey Belyashov Date: Tue, 13 Jul 2021 08:42:02 +0300 Message-ID: Subject: Re: [PATCH v4] [gdb] Add basic Z80 CPU support To: Simon Marchi Cc: gdb-patches@sourceware.org X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Jul 2021 05:42:18 -0000 Hi Simon, Thank you. You can safely remove these comments, but keep last one - it is reserved for GameBoy Z80 & Z80 Next CPUs. Sergey Belyashov =D0=B2=D1=82, 13 =D0=B8=D1=8E=D0=BB=D1=8F 2021 =D0=B3., 0:37 Simon Marchi <= simon.marchi@polymtl.ca>: > Hi Sergey, > > I'm fine with merging this pretty much as-is. I certainly won't have > the time to do an in-depth / functional review, but I trust that if it > is useful to you, it can be for others. I can take care of rebasing and > doing the fixups to make sure it compiles, as well as make a few style > changes. > > I'm just wondering about these, if you could help me: > > > diff --git a/gdb/features/z80-cpu.xml b/gdb/features/z80-cpu.xml > > new file mode 100644 > > index 0000000000..d8093d68b9 > > --- /dev/null > > +++ b/gdb/features/z80-cpu.xml > > @@ -0,0 +1,34 @@ > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > Is this commented out on purpose, can we remove it? > > > +static int > > +z80_scan_prologue (struct gdbarch *gdbarch, CORE_ADDR pc_beg, CORE_ADD= R > pc_end, > > + struct z80_unwind_cache *info) > > +{ > > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > > + int addr_len =3D gdbarch_tdep (gdbarch)->addr_length; > > + gdb_byte prologue[32]; /* max prologue is 24 bytes: __interrupt with > local array */ > > + int pos =3D 0; > > + int len; > > + int reg; > > + CORE_ADDR value; > > + > > + len =3D pc_end - pc_beg; > > + if (len > (int)sizeof(prologue)) > > + len =3D sizeof(prologue); > > + > > + read_memory (pc_beg, prologue, len); > > + > > + /* stage0: check for series of POPs and then PUSHs */ > > + if ((reg =3D z80_is_pop_rr(prologue, &pos))) > > + { > > + int i; > > + int size =3D pos; > > + gdb_byte regs[8]; /* Z80 have only 6 register pairs */ > > + regs[0] =3D reg & 0xff; > > + for (i =3D 1; i < 8 && (regs[i] =3D z80_is_pop_rr (&prologue[pos= ], > &size)); > > + ++i, pos +=3D size); > > + /* now we expect series of PUSHs in reverse order */ > > + for (--i; i >=3D 0 && regs[i] =3D=3D z80_is_push_rr (&prologue[p= os], > &size); > > + --i, pos +=3D size); > > + if (i =3D=3D -1 && pos > 0) > > + info->prologue_type.load_args =3D 1; > > + else > > + pos =3D 0; > > + } > > + /* stage1: check for __interrupt handlers and __critical functions *= / > > + else if (!memcmp (&prologue[pos], "\355\127\363\365", 4)) > > + { /* ld a, i; di; push af */ > > + info->prologue_type.critical =3D 1; > > + pos +=3D 4; > > + info->state_size +=3D addr_len; > > + } > > + else if (!memcmp (&prologue[pos], "\365\305\325\345\375\345", 6)) > > + { /* push af; push bc; push de; push hl; push iy */ > > + info->prologue_type.interrupt =3D 1; > > + pos +=3D 6; > > + info->state_size +=3D addr_len * 5; > > + } > > + > > + /* stage2: check for FP saving scheme */ > > + if (prologue[pos] =3D=3D 0xcd) /* call nn */ > > + { > > + struct bound_minimal_symbol msymbol; > > + msymbol =3D lookup_minimal_symbol ("__sdcc_enter_ix", NULL, NULL= ); > > + if (msymbol.minsym) > > + { > > + value =3D BMSYMBOL_VALUE_ADDRESS (msymbol); > > + if (value =3D=3D extract_unsigned_integer (&prologue[pos+1], > addr_len, byte_order)) > > + { > > + pos +=3D 1 + addr_len; > > + info->prologue_type.fp_sdcc =3D 1; > > + } > > + } > > + } > > + else if (!memcmp (&prologue[pos], "\335\345\335\041\000\000", > 4+addr_len) && > > + !memcmp (&prologue[pos+4+addr_len], "\335\071\335\371", 4)) > > + { /* push ix; ld ix, #0; add ix, sp; ld sp, ix */ > > + pos +=3D 4 + addr_len + 4; > > + info->prologue_type.fp_sdcc =3D 1; > > + } > > + else if (!memcmp (&prologue[pos], "\335\345", 2)) > > + { /* push ix */ > > + pos +=3D 2; > > + info->prologue_type.fp_sdcc =3D 1; > > + } > > + > > + /* stage3: check for local variables allocation */ > > + switch (prologue[pos]) > > + { > > + case 0xf5: /* push af */ > > + info->size =3D 0; > > + while (prologue[pos] =3D=3D 0xf5) > > + { > > + info->size +=3D addr_len; > > + pos++; > > + } > > + if (prologue[pos] =3D=3D 0x3b) /* dec sp */ > > + { > > + info->size++; > > + pos++; > > + } > > + break; > > + case 0x3b: /* dec sp */ > > + info->size =3D 0; > > + while (prologue[pos] =3D=3D 0x3b) > > + { > > + info->size++; > > + pos++; > > + } > > + break; > > + case 0x21: /*ld hl, -nn */ > > + if (prologue[pos+addr_len] =3D=3D 0x39 && prologue[pos+addr_len] = >=3D > 0x80 && > > + prologue[pos+addr_len+1] =3D=3D 0xf9) > > + { /* add hl, sp; ld sp, hl */ > > + info->size =3D -extract_signed_integer(&prologue[pos+1], > addr_len, byte_order); > > + pos +=3D 1 + addr_len + 2; > > + } > > + break; > > + case 0xfd: /* ld iy, -nn */ > > + if (prologue[pos+1] =3D=3D 0x21 && prologue[pos+1+addr_len] >=3D = 0x80 && > > + !memcmp (&prologue[pos+2+addr_len], "\375\071\375\371", 4)) > > + { > > + info->size =3D -extract_signed_integer(&prologue[pos+2], > addr_len, byte_order); > > + pos +=3D 2 + addr_len + 4; > > + } > > + break; > > + case 0xed: /* check for lea xx, ix - n */ > > + switch (prologue[pos+1]) > > + { > > + case 0x22: /* lea hl, ix - n */ > > + if (prologue[pos+2] >=3D 0x80 && prologue[pos+3] =3D=3D 0xf9) > > + { /* ld sp, hl */ > > + info->size =3D -extract_signed_integer(&prologue[pos+2], = 1, > byte_order); > > + pos +=3D 4; > > + } > > + break; > > + case 0x55: /* lea iy, ix - n */ > > + if (prologue[pos+2] >=3D 0x80 && prologue[pos+3] =3D=3D 0xfd = && > > + prologue[pos+4] =3D=3D 0xf9) > > + { /* ld sp, iy */ > > + info->size =3D -extract_signed_integer(&prologue[pos+2], = 1, > byte_order); > > + pos +=3D 5; > > + } > > + break; > > + } > > + break; > > + } > > + len =3D 0; > > + //info->saved_regs[Z80_PC_REGNUM].addr =3D len++ > > Is this commented out on purpose? Can we remove it? > > > +/* returns pointer to instruction information structure corresponded t= o > opcode > > + in buf */ > > +static const struct insn_info * > > +z80_get_insn_info (struct gdbarch *gdbarch, const gdb_byte *buf, int > *size) > > +{ > > + int code; > > + const struct insn_info *info; > > + unsigned long mach =3D gdbarch_bfd_arch_info (gdbarch)->mach; > > + *size =3D 0; > > + switch (mach) > > + { > > + case bfd_mach_ez80_z80: > > + info =3D &ez80_main_insn_table[4]; /* skip force_nops */ > > + break; > > + case bfd_mach_ez80_adl: > > + info =3D &ez80_adl_main_insn_table[4]; /* skip force_nops */ > > + break; > > +/* > > + case bfd_mach_gbz80: > > + info =3D &gbz80_main_insn_table[0]; > > + break; > > + case bfd_mach_z80n: > > + info =3D &z80n_main_insn_table[0]; > > + break; > > +*/ > > Is this commented out on purpose, can we remove it? > > Thanks, > > Simon >