| Message ID | 20230530131843.1186637-7-christoph.muellner@vrull.eu |
|---|---|
| State | New |
| Headers | show |
| Series | disas/riscv: Add vendor extension support | expand |
On Tue, May 30, 2023 at 11:20 PM Christoph Muellner <christoph.muellner@vrull.eu> wrote: > > From: Christoph Müllner <christoph.muellner@vrull.eu> > > The disassembler needs the available extensions in order > to properly decode instructions in case of overlapping > encodings (e.g. for vendor extensions). > > Let's use the field 'disassemble_info::private_data' to store > our RISCVCPUConfig pointer. > > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> Acked-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 5b7818dbd1..6f0cd9a0bb 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) > { > RISCVCPU *cpu = RISCV_CPU(s); > CPURISCVState *env = &cpu->env; > + RISCVCPUConfig *cfg = &cpu->cfg; > + > + info->private_data = cfg; > > switch (env->xl) { > case MXL_RV32: > -- > 2.40.1 > >
On 2023/5/30 21:18, Christoph Muellner wrote: > From: Christoph Müllner <christoph.muellner@vrull.eu> > > The disassembler needs the available extensions in order > to properly decode instructions in case of overlapping > encodings (e.g. for vendor extensions). > > Let's use the field 'disassemble_info::private_data' to store > our RISCVCPUConfig pointer. > > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> > --- > target/riscv/cpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 5b7818dbd1..6f0cd9a0bb 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) > { > RISCVCPU *cpu = RISCV_CPU(s); > CPURISCVState *env = &cpu->env; > + RISCVCPUConfig *cfg = &cpu->cfg; > + > + info->private_data = cfg; I don't know if this field will be overridden by the binutils. Can we extend the struct disassemble_info, and add some fields like supporting for Capstone? Zhiwei > > switch (env->xl) { > case MXL_RV32:
On Mon, Jun 12, 2023 at 8:25 AM LIU Zhiwei <baxiantai@gmail.com> wrote: > > > On 2023/5/30 21:18, Christoph Muellner wrote: > > From: Christoph Müllner <christoph.muellner@vrull.eu> > > > > The disassembler needs the available extensions in order > > to properly decode instructions in case of overlapping > > encodings (e.g. for vendor extensions). > > > > Let's use the field 'disassemble_info::private_data' to store > > our RISCVCPUConfig pointer. > > > > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> > > --- > > target/riscv/cpu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index 5b7818dbd1..6f0cd9a0bb 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) > > { > > RISCVCPU *cpu = RISCV_CPU(s); > > CPURISCVState *env = &cpu->env; > > + RISCVCPUConfig *cfg = &cpu->cfg; > > + > > + info->private_data = cfg; > > I don't know if this field will be overridden by the binutils. Can we > extend the struct disassemble_info, and add some fields like supporting > for Capstone? Initially I wanted to add a new field, but then I noticed that the field 'disassemble_info::private_data' is used for a similar purpose by disas/cris.c, disas/m68k.c, and dias/xtensa.c. So I decided to not add yet another field, which only serves one architecture. But if that's the preferred way, then I can change. Thanks Christoph > > Zhiwei > > > > > switch (env->xl) { > > case MXL_RV32:
On 2023/6/12 17:47, Christoph Müllner wrote: > On Mon, Jun 12, 2023 at 8:25 AM LIU Zhiwei <baxiantai@gmail.com> wrote: >> >> On 2023/5/30 21:18, Christoph Muellner wrote: >>> From: Christoph Müllner <christoph.muellner@vrull.eu> >>> >>> The disassembler needs the available extensions in order >>> to properly decode instructions in case of overlapping >>> encodings (e.g. for vendor extensions). >>> >>> Let's use the field 'disassemble_info::private_data' to store >>> our RISCVCPUConfig pointer. >>> >>> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> >>> --- >>> target/riscv/cpu.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >>> index 5b7818dbd1..6f0cd9a0bb 100644 >>> --- a/target/riscv/cpu.c >>> +++ b/target/riscv/cpu.c >>> @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) >>> { >>> RISCVCPU *cpu = RISCV_CPU(s); >>> CPURISCVState *env = &cpu->env; >>> + RISCVCPUConfig *cfg = &cpu->cfg; >>> + >>> + info->private_data = cfg; >> I don't know if this field will be overridden by the binutils. Can we >> extend the struct disassemble_info, and add some fields like supporting >> for Capstone? > > Initially I wanted to add a new field, but then I noticed that the field > 'disassemble_info::private_data' is used for a similar purpose by > disas/cris.c, disas/m68k.c, and dias/xtensa.c. > So I decided to not add yet another field, which only serves one architecture. I think you can CC these arch maintainers to see if it need some specially process before using the private_data. > > But if that's the preferred way, then I can change. I prefer this way, but not insist on if it really works using the private_data. Zhiwei > > Thanks > Christoph > >> Zhiwei >> >>> switch (env->xl) { >>> case MXL_RV32:
On Mon, Jun 12, 2023 at 12:01 PM LIU Zhiwei <baxiantai@gmail.com> wrote: > > > On 2023/6/12 17:47, Christoph Müllner wrote: > > On Mon, Jun 12, 2023 at 8:25 AM LIU Zhiwei <baxiantai@gmail.com> wrote: > >> > >> On 2023/5/30 21:18, Christoph Muellner wrote: > >>> From: Christoph Müllner <christoph.muellner@vrull.eu> > >>> > >>> The disassembler needs the available extensions in order > >>> to properly decode instructions in case of overlapping > >>> encodings (e.g. for vendor extensions). > >>> > >>> Let's use the field 'disassemble_info::private_data' to store > >>> our RISCVCPUConfig pointer. > >>> > >>> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> > >>> --- > >>> target/riscv/cpu.c | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > >>> index 5b7818dbd1..6f0cd9a0bb 100644 > >>> --- a/target/riscv/cpu.c > >>> +++ b/target/riscv/cpu.c > >>> @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) > >>> { > >>> RISCVCPU *cpu = RISCV_CPU(s); > >>> CPURISCVState *env = &cpu->env; > >>> + RISCVCPUConfig *cfg = &cpu->cfg; > >>> + > >>> + info->private_data = cfg; > >> I don't know if this field will be overridden by the binutils. Can we > >> extend the struct disassemble_info, and add some fields like supporting > >> for Capstone? > > > > Initially I wanted to add a new field, but then I noticed that the field > > 'disassemble_info::private_data' is used for a similar purpose by > > disas/cris.c, disas/m68k.c, and dias/xtensa.c. > > So I decided to not add yet another field, which only serves one architecture. > I think you can CC these arch maintainers to see if it need some > specially process before using the private_data. > > > > But if that's the preferred way, then I can change. > > I prefer this way, but not insist on if it really works using the > private_data. This topic is already resolved by using the field 'info->target_info'. So I dropped this patch anyway. BR Christoph > > Zhiwei > > > > > Thanks > > Christoph > > > >> Zhiwei > >> > >>> switch (env->xl) { > >>> case MXL_RV32:
On 2023/6/12 18:04, Christoph Müllner wrote: > On Mon, Jun 12, 2023 at 12:01 PM LIU Zhiwei <baxiantai@gmail.com> wrote: >> >> On 2023/6/12 17:47, Christoph Müllner wrote: >>> On Mon, Jun 12, 2023 at 8:25 AM LIU Zhiwei <baxiantai@gmail.com> wrote: >>>> On 2023/5/30 21:18, Christoph Muellner wrote: >>>>> From: Christoph Müllner <christoph.muellner@vrull.eu> >>>>> >>>>> The disassembler needs the available extensions in order >>>>> to properly decode instructions in case of overlapping >>>>> encodings (e.g. for vendor extensions). >>>>> >>>>> Let's use the field 'disassemble_info::private_data' to store >>>>> our RISCVCPUConfig pointer. >>>>> >>>>> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> >>>>> --- >>>>> target/riscv/cpu.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >>>>> index 5b7818dbd1..6f0cd9a0bb 100644 >>>>> --- a/target/riscv/cpu.c >>>>> +++ b/target/riscv/cpu.c >>>>> @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) >>>>> { >>>>> RISCVCPU *cpu = RISCV_CPU(s); >>>>> CPURISCVState *env = &cpu->env; >>>>> + RISCVCPUConfig *cfg = &cpu->cfg; >>>>> + >>>>> + info->private_data = cfg; >>>> I don't know if this field will be overridden by the binutils. Can we >>>> extend the struct disassemble_info, and add some fields like supporting >>>> for Capstone? >>> Initially I wanted to add a new field, but then I noticed that the field >>> 'disassemble_info::private_data' is used for a similar purpose by >>> disas/cris.c, disas/m68k.c, and dias/xtensa.c. >>> So I decided to not add yet another field, which only serves one architecture. >> I think you can CC these arch maintainers to see if it need some >> specially process before using the private_data. >>> But if that's the preferred way, then I can change. >> I prefer this way, but not insist on if it really works using the >> private_data. > This topic is already resolved by using the field 'info->target_info'. > So I dropped this patch anyway. OK. I remembered I also used this field to pass the ISA information in the multi-path disassemble patch set. Zhiwei > > BR > Christoph > >> Zhiwei >> >>> Thanks >>> Christoph >>> >>>> Zhiwei >>>> >>>>> switch (env->xl) { >>>>> case MXL_RV32:
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 5b7818dbd1..6f0cd9a0bb 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -819,6 +819,9 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info) { RISCVCPU *cpu = RISCV_CPU(s); CPURISCVState *env = &cpu->env; + RISCVCPUConfig *cfg = &cpu->cfg; + + info->private_data = cfg; switch (env->xl) { case MXL_RV32: