| Message ID | 1450074970-28562-2-git-send-email-chrisfriedt@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 14 December 2015 at 06:36, Christopher Friedt <chrisfriedt@gmail.com> wrote: > * allow overriding the default xml descriptor with gdb_xml_descriptor() > * read cortex-m registers using arm_cortexm_gdb_read_register() > * write cortex-m registers using arm_cortexm_gdb_write_register() > * correct the number of cortex-m core regs to 23 > > Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com> > --- > gdbstub.c | 29 ++++--- > include/qom/cpu.h | 1 + > target-arm/cpu-qom.h | 4 + > target-arm/cpu.c | 5 +- > target-arm/gdbstub.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 241 insertions(+), 13 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index 9c29aa0..4684a4b 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -540,19 +540,24 @@ static const char *get_feature_xml(const char *p, const char **newp, > GDBRegisterState *r; > CPUState *cpu = first_cpu; > > - snprintf(target_xml, sizeof(target_xml), > - "<?xml version=\"1.0\"?>" > - "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" > - "<target>" > - "<xi:include href=\"%s\"/>", > - cc->gdb_core_xml_file); > - > - for (r = cpu->gdb_regs; r; r = r->next) { > - pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\""); > - pstrcat(target_xml, sizeof(target_xml), r->xml); > - pstrcat(target_xml, sizeof(target_xml), "\"/>"); > + if (cc->gdb_xml_descriptor) { > + cc->gdb_xml_descriptor(cpu, target_xml, sizeof(target_xml)); > + } else { > + snprintf(target_xml, sizeof(target_xml), > + "<?xml version=\"1.0\"?>" > + "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" > + "<target>" > + "<xi:include href=\"%s\"/>", > + cc->gdb_core_xml_file); > + > + for (r = cpu->gdb_regs; r; r = r->next) { > + pstrcat(target_xml, sizeof(target_xml), > + "<xi:include href=\""); > + pstrcat(target_xml, sizeof(target_xml), r->xml); > + pstrcat(target_xml, sizeof(target_xml), "\"/>"); > + } > + pstrcat(target_xml, sizeof(target_xml), "</target>"); This patch seems to be creating a completely new method of constructing the XML to send to gdb. Is it really not possible to do this using the existing mechanisms we have for selecting XML to send to gdb and handling the registers it implies? thanks -- PMM
On Mon, Dec 14, 2015 at 3:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > This patch seems to be creating a completely new method of > constructing the XML to send to gdb. Is it really not possible > to do this using the existing mechanisms we have for selecting > XML to send to gdb and handling the registers it implies? Hehehe... I only just saw the gdb-xml/ directory. I'll create a gdb-xml/arm-cortex-m-core.xml then, and refactor some of the remaining lines of code. Thanks for the feedback ;-)
On 14 December 2015 at 13:07, Christopher Friedt <chrisfriedt@gmail.com> wrote: > On Mon, Dec 14, 2015 at 3:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> This patch seems to be creating a completely new method of >> constructing the XML to send to gdb. Is it really not possible >> to do this using the existing mechanisms we have for selecting >> XML to send to gdb and handling the registers it implies? > > Hehehe... I only just saw the gdb-xml/ directory. I'll create a > gdb-xml/arm-cortex-m-core.xml then, and refactor some of the remaining > lines of code. Note that our XML files are from gdb itself, so you should start by checking whether gdb has a suitable Cortex-M xml file. thanks -- PMM
On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Note that our XML files are from gdb itself, so you should start > by checking whether gdb has a suitable Cortex-M xml file. They do indeed. Thanks for the tip.
On Mon, Dec 14, 2015 at 8:16 AM, Christopher Friedt <chrisfriedt@gmail.com> wrote: > On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> Note that our XML files are from gdb itself, so you should start >> by checking whether gdb has a suitable Cortex-M xml file. > > They do indeed. Thanks for the tip. binutils-gdb arm-m-profile.xml: https://goo.gl/hpTye8 openocd armv7m.c: http://goo.gl/FFn56X There are 2 (major) differences from what I've seen: 1) xpsr is regnum 25 instead of 16 (what OpenOCD uses), and I'm fine with that. 2) binutils-gdb does not specify anything for the org.gnu.gdb.arm.m-system group of core registers in any xml file. It also seems very clear that the binutils people and the openocd people have diverged at some point in their assignment of regnum values; in openocd, the registers are mostly all consecutive with moderate reuse between cores, whereas in binutils-gdb, their are occasional gaps and extensive reuse between cores. The differences seem primarily technical, but it's unclear as to why binutils-gdb does *not* include the m-system group of core registers. The m-system group of core registers are *incredibly* useful, but I'm also inclined not to clobber binutils-gdb's register numbering convention. I think it would be most ideal to append the crucial m-system information directly [1] in arm-m-profile.xml from binutils-gdb (or possibly declare it as an include [2]): <feature name="org.gnu.gdb.arm.m-system"> <reg name="msp" bitsize="32" type="data_ptr"/> <reg name="psp" bitsize="32" type="data_ptr"/> <reg name="primask" bitsize="1" type="int8"/> <reg name="basepri" bitsize="8" type="int8"/> <reg name="faultmask" bitsize="1" type="int8"/> <reg name="control" bitsize="3" type="int8"/> </feature> However, if the worry there is that it diverges from binutils-gdb, then the next best solution would be to create a separate arm-m-system.xml, and to append that to the cpu->gdb_reg linked list in cortex_m3_initfn(), cortex_m4_initfn(), and any other m's [3]. Which solution would work best for qemu? C
On 14 December 2015 at 14:22, Christopher Friedt <chrisfriedt@gmail.com> wrote: > On Mon, Dec 14, 2015 at 8:16 AM, Christopher Friedt > <chrisfriedt@gmail.com> wrote: >> On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> Note that our XML files are from gdb itself, so you should start >>> by checking whether gdb has a suitable Cortex-M xml file. >> >> They do indeed. Thanks for the tip. > > binutils-gdb arm-m-profile.xml: https://goo.gl/hpTye8 > openocd armv7m.c: http://goo.gl/FFn56X > > There are 2 (major) differences from what I've seen: > > 1) xpsr is regnum 25 instead of 16 (what OpenOCD uses), and I'm fine with that. > 2) binutils-gdb does not specify anything for the > org.gnu.gdb.arm.m-system group of core registers in any xml file. > > It also seems very clear that the binutils people and the openocd > people have diverged at some point in their assignment of regnum > values; in openocd, the registers are mostly all consecutive with > moderate reuse between cores, whereas in binutils-gdb, their are > occasional gaps and extensive reuse between cores. The differences > seem primarily technical, but it's unclear as to why binutils-gdb does > *not* include the m-system group of core registers. My guess would be because gdb is primarily thinking of itself as a user-mode debugger, and system registers aren't accessible from there. And/or "nobody asked for it". > The m-system group of core registers are *incredibly* useful, but I'm > also inclined not to clobber binutils-gdb's register numbering > convention. > > I think it would be most ideal to append the crucial m-system > information directly [1] in arm-m-profile.xml from binutils-gdb (or > possibly declare it as an include [2]): > > <feature name="org.gnu.gdb.arm.m-system"> > <reg name="msp" bitsize="32" type="data_ptr"/> > <reg name="psp" bitsize="32" type="data_ptr"/> > <reg name="primask" bitsize="1" type="int8"/> > <reg name="basepri" bitsize="8" type="int8"/> > <reg name="faultmask" bitsize="1" type="int8"/> > <reg name="control" bitsize="3" type="int8"/> > </feature> > > However, if the worry there is that it diverges from binutils-gdb, > then the next best solution would be to create a separate > arm-m-system.xml, and to append that to the cpu->gdb_reg linked list > in cortex_m3_initfn(), cortex_m4_initfn(), and any other m's [3]. > > Which solution would work best for qemu? I'd rather we didn't diverge from upstream gdb too. On the other hand I'm not sure how much it matters if we all end up using different XML to describe the same target hardware. It would be nice to ask the gdb folks first though, maybe. rth: do you know how this stuff works? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 14 December 2015 at 14:22, Christopher Friedt <chrisfriedt@gmail.com> wrote: >> On Mon, Dec 14, 2015 at 8:16 AM, Christopher Friedt >> <chrisfriedt@gmail.com> wrote: >>> On Mon, Dec 14, 2015 at 8:14 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> Note that our XML files are from gdb itself, so you should start >>>> by checking whether gdb has a suitable Cortex-M xml file. >>> >>> They do indeed. Thanks for the tip. >> >> binutils-gdb arm-m-profile.xml: https://goo.gl/hpTye8 >> openocd armv7m.c: http://goo.gl/FFn56X >> >> There are 2 (major) differences from what I've seen: >> >> 1) xpsr is regnum 25 instead of 16 (what OpenOCD uses), and I'm fine with that. >> 2) binutils-gdb does not specify anything for the >> org.gnu.gdb.arm.m-system group of core registers in any xml file. >> >> It also seems very clear that the binutils people and the openocd >> people have diverged at some point in their assignment of regnum >> values; in openocd, the registers are mostly all consecutive with >> moderate reuse between cores, whereas in binutils-gdb, their are >> occasional gaps and extensive reuse between cores. The differences >> seem primarily technical, but it's unclear as to why binutils-gdb does >> *not* include the m-system group of core registers. > > My guess would be because gdb is primarily thinking of itself > as a user-mode debugger, and system registers aren't accessible > from there. And/or "nobody asked for it". > >> The m-system group of core registers are *incredibly* useful, but I'm >> also inclined not to clobber binutils-gdb's register numbering >> convention. >> >> I think it would be most ideal to append the crucial m-system >> information directly [1] in arm-m-profile.xml from binutils-gdb (or >> possibly declare it as an include [2]): >> >> <feature name="org.gnu.gdb.arm.m-system"> >> <reg name="msp" bitsize="32" type="data_ptr"/> >> <reg name="psp" bitsize="32" type="data_ptr"/> >> <reg name="primask" bitsize="1" type="int8"/> >> <reg name="basepri" bitsize="8" type="int8"/> >> <reg name="faultmask" bitsize="1" type="int8"/> >> <reg name="control" bitsize="3" type="int8"/> >> </feature> >> >> However, if the worry there is that it diverges from binutils-gdb, >> then the next best solution would be to create a separate >> arm-m-system.xml, and to append that to the cpu->gdb_reg linked list >> in cortex_m3_initfn(), cortex_m4_initfn(), and any other m's [3]. >> >> Which solution would work best for qemu? > > I'd rather we didn't diverge from upstream gdb too. On the > other hand I'm not sure how much it matters if we all end up > using different XML to describe the same target hardware. It > would be nice to ask the gdb folks first though, maybe. > > rth: do you know how this stuff works? IIRC last time I played with this when adding aarch64 system registers for debugging is the number is irrelevant to gdb, its all dependant on what the stub sends. As long as the coprocessor get/set functions agree on the order with the xml everything should be fine. > > thanks > -- PMM -- Alex Bennée
On Mon, Dec 14, 2015 at 10:56 AM, Alex Bennée <alex.bennee@linaro.org> wrote: > IIRC last time I played with this when adding aarch64 system registers > for debugging is the number is irrelevant to gdb, its all dependant on > what the stub sends. As long as the coprocessor get/set functions agree > on the order with the xml everything should be fine. That's right - the question isn't really how get the two to communicate, but really which registers numbers to choose and how to choose them in such a way that it doesn't make anyone angry :-) And Peter is likely right that we should ask the gdb folks input - Peter, do you have someone particular in mind?
Just to update everyone, there is a thread on gdb-patches here [1] that is awaiting consensus before a patch is submitted that we may pull into qemu. [1] https://goo.gl/FyUvQu
On 16 December 2015 at 00:16, Christopher Friedt <chrisfriedt@gmail.com> wrote: > Just to update everyone, there is a thread on gdb-patches here [1] > that is awaiting consensus before a patch is submitted that we may > pull into qemu. > > [1] https://goo.gl/FyUvQu Thanks for following up with the GDB developers. -- PMM
diff --git a/gdbstub.c b/gdbstub.c index 9c29aa0..4684a4b 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -540,19 +540,24 @@ static const char *get_feature_xml(const char *p, const char **newp, GDBRegisterState *r; CPUState *cpu = first_cpu; - snprintf(target_xml, sizeof(target_xml), - "<?xml version=\"1.0\"?>" - "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" - "<target>" - "<xi:include href=\"%s\"/>", - cc->gdb_core_xml_file); - - for (r = cpu->gdb_regs; r; r = r->next) { - pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\""); - pstrcat(target_xml, sizeof(target_xml), r->xml); - pstrcat(target_xml, sizeof(target_xml), "\"/>"); + if (cc->gdb_xml_descriptor) { + cc->gdb_xml_descriptor(cpu, target_xml, sizeof(target_xml)); + } else { + snprintf(target_xml, sizeof(target_xml), + "<?xml version=\"1.0\"?>" + "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" + "<target>" + "<xi:include href=\"%s\"/>", + cc->gdb_core_xml_file); + + for (r = cpu->gdb_regs; r; r = r->next) { + pstrcat(target_xml, sizeof(target_xml), + "<xi:include href=\""); + pstrcat(target_xml, sizeof(target_xml), r->xml); + pstrcat(target_xml, sizeof(target_xml), "\"/>"); + } + pstrcat(target_xml, sizeof(target_xml), "</target>"); } - pstrcat(target_xml, sizeof(target_xml), "</target>"); } return target_xml; } diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 51a1323..7b8d875 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -152,6 +152,7 @@ typedef struct CPUClass { int (*handle_mmu_fault)(CPUState *cpu, vaddr address, int rw, int mmu_index); hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr); + int (*gdb_xml_descriptor)(CPUState *cpu, char *buf, size_t len); int (*gdb_read_register)(CPUState *cpu, uint8_t *buf, int reg); int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg); void (*debug_excp_handler)(CPUState *cpu); diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h index 25fb1ce..eafae6b 100644 --- a/target-arm/cpu-qom.h +++ b/target-arm/cpu-qom.h @@ -221,6 +221,10 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr); int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg); int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); +int arm_cortexm_gdb_xml_descriptor(CPUState *cpu, char *buf, size_t len); +int arm_cortexm_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg); +int arm_cortexm_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); + /* Callback functions for the generic timer's timers. */ void arm_gt_ptimer_cb(void *opaque); void arm_gt_vtimer_cb(void *opaque); diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 30739fc..e56b77a 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -553,7 +553,6 @@ static void arm_cpu_post_init(Object *obj) &error_abort); } } - } static void arm_cpu_finalizefn(Object *obj) @@ -910,6 +909,10 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data) #endif cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt; + cc->gdb_num_core_regs = 23; + cc->gdb_xml_descriptor = arm_cortexm_gdb_xml_descriptor; + cc->gdb_read_register = arm_cortexm_gdb_read_register; + cc->gdb_write_register = arm_cortexm_gdb_write_register; } static const ARMCPRegInfo cortexr5_cp_reginfo[] = { diff --git a/target-arm/gdbstub.c b/target-arm/gdbstub.c index 1c34396..eb39757 100644 --- a/target-arm/gdbstub.c +++ b/target-arm/gdbstub.c @@ -3,6 +3,7 @@ * * Copyright (c) 2003-2005 Fabrice Bellard * Copyright (c) 2013 SUSE LINUX Products GmbH + * Copyright (c) 2015 Christopher Friedt * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -100,3 +101,217 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) /* Unknown register. */ return 0; } + +enum cm_gdb_regnr { + R0, R1, R2, R3, + R4, R5, R6, R7, + R8, R9, R10, R11, + R12, SP, LR, PC, + XPSR, MSP, PSP, PRIMASK, + BASEPRI, FAULTMASK, CONTROL, +}; +struct cm_gdb_regdesc { + const char *const name; + int bitsz; + int nr; + const char *const type; + const char *const grp; +}; +static const struct cm_gdb_regdesc cm_gdb_regdesc[] = { + { "r0", 32, R0, "int", "general", }, + { "r1", 32, R1, "int", "general", }, + { "r2", 32, R2, "int", "general", }, + { "r3", 32, R3, "int", "general", }, + { "r4", 32, R4, "int", "general", }, + { "r5", 32, R5, "int", "general", }, + { "r6", 32, R6, "int", "general", }, + { "r7", 32, R7, "int", "general", }, + { "r8", 32, R8, "int", "general", }, + { "r9", 32, R9, "int", "general", }, + { "r10", 32, R10, "int", "general", }, + { "r11", 32, R11, "int", "general", }, + { "r12", 32, R12, "int", "general", }, + { "sp", 32, SP, "data_ptr", "general", }, + { "lr", 32, LR, "int", "general", }, + { "pc", 32, PC, "code_ptr", "general", }, + { "xpsr", 32, XPSR, "int", "general", }, + { "msp", 32, MSP, "data_ptr", "system", }, + { "psp", 32, PSP, "data_ptr", "system", }, + { "primask", 1, PRIMASK, "int8", "system", }, + { "basepri", 8, BASEPRI, "int8", "system", }, + { "faultmask", 1, FAULTMASK, "int8", "system", }, + { "control", 2, CONTROL, "int8", "system", }, +}; + +int arm_cortexm_gdb_xml_descriptor(CPUState *cs, char *buf, size_t len) +{ + int r; + int i; + ssize_t l; + char *d; + ARMCPU *cpu; + CPUARMState *env; + + l = len; + d = buf; + cpu = ARM_CPU(cs); + env = &cpu->env; + +#define _update_buf_or_err() \ + do { \ + d += r; \ + len -= r; \ + if (r < 0 || l <= 0) { \ + goto nospace; \ + } \ + } while (0); + + r = snprintf(d, l, + "<?xml version=\"1.0\"?>" + "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">" + "<target version=\"1.0\">" + "<feature name=\"org.gnu.gdb.arm.m-profile\">" + ); + _update_buf_or_err(); + + for ( + i = 0; + i < sizeof(cm_gdb_regdesc) / sizeof(cm_gdb_regdesc[0]); + i++ + ) { + r = snprintf(d, l, + "<reg name=\"%s\" bitsize=\"%d\" " + "regnum=\"%d\" type=\"%s\" group=\"%s\"/>", + cm_gdb_regdesc[i].name, + cm_gdb_regdesc[i].bitsz, + cm_gdb_regdesc[i].nr, + cm_gdb_regdesc[i].type, + cm_gdb_regdesc[i].grp + ); + _update_buf_or_err(); + + if (XPSR == i) { + r = snprintf(d, l, + "</feature>" + "<feature name=\"org.gnu.gdb.arm.m-system\">" + ); + _update_buf_or_err(); + } + } + + r = snprintf(d, l, + "</feature>" + "</target>" + ); + _update_buf_or_err(); + + r = 0; + goto out; + +nospace: + buf[0] = '\0'; + r = -1; +out: + return r; + +#undef _update_buf_or_err +} + +#ifndef PSTATE_F +#define PSTATE_F (1U << 6) +#endif +#ifndef PSTATE_I +#define PSTATE_I (1U << 7) +#endif + +int arm_cortexm_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + + if (n < XPSR) { + /* Core integer register. */ + return gdb_get_reg32(mem_buf, env->regs[n]); + } + + switch (n) { + case XPSR: + return gdb_get_reg32(mem_buf, xpsr_read(env)); + case MSP: + return gdb_get_reg32(mem_buf, + env->v7m.current_sp ? env->v7m.other_sp : env->regs[SP]); + case PSP: + return gdb_get_reg32(mem_buf, + env->v7m.current_sp ? env->regs[SP] : env->v7m.other_sp); + case PRIMASK: + return gdb_get_reg8(mem_buf, !!(env->daif & PSTATE_I)); + case BASEPRI: + return gdb_get_reg8(mem_buf, env->v7m.basepri); + case FAULTMASK: + return gdb_get_reg8(mem_buf, !!(env->daif & PSTATE_F)); + case CONTROL: + return gdb_get_reg8(mem_buf, env->v7m.control); + } + /* Unknown register. */ + return 0; +} + +int arm_cortexm_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + uint32_t tmp; + + tmp = ldl_p(mem_buf); + + /* Mask out low bit of PC to workaround gdb bugs. This will probably + cause problems if we ever implement the Jazelle DBX extensions. */ + if (n == 15) { + tmp &= ~1; + } + + if (n < 16) { + /* Core integer register. */ + env->regs[n] = tmp; + return 4; + } + switch (n) { + case XPSR: return 4; + case MSP: + if (env->v7m.current_sp) { + env->v7m.other_sp = tmp; + } else { + env->regs[SP] = tmp; + } + return 4; + case PSP: + if (env->v7m.current_sp) { + env->regs[SP] = tmp; + } else { + env->v7m.other_sp = tmp; + } + return 4; + case PRIMASK: + if (tmp & 1) { + env->daif |= PSTATE_I; + } else { + env->daif &= ~PSTATE_I; + } + return 1; + case BASEPRI: + env->v7m.basepri = tmp & 0xff; + return 1; + case FAULTMASK: + if (tmp & 1) { + env->daif |= PSTATE_F; + } else { + env->daif &= ~PSTATE_F; + } + return 1; + case CONTROL: + env->v7m.control = tmp & 0x3; + return 1; + } + /* Unknown register. */ + return 0; +}
* allow overriding the default xml descriptor with gdb_xml_descriptor() * read cortex-m registers using arm_cortexm_gdb_read_register() * write cortex-m registers using arm_cortexm_gdb_write_register() * correct the number of cortex-m core regs to 23 Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com> --- gdbstub.c | 29 ++++--- include/qom/cpu.h | 1 + target-arm/cpu-qom.h | 4 + target-arm/cpu.c | 5 +- target-arm/gdbstub.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 241 insertions(+), 13 deletions(-)