Re: [PATCH 6/9] target/riscv/cpu: Share RISCVCPUConfig with disassembler

From: LIU Zhiwei <baxiantai@gmail.com>
To: "Christoph Müllner" <christoph.muellner@vrull.eu>
Cc: qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
	Alistair Francis <alistair.francis@wdc.com>,
	Bin Meng <bin.meng@windriver.com>,
	Philipp Tomsich <philipp.tomsich@vrull.eu>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Zhiwei Liu <zhiwei_liu@linux.alibaba.com>,
	Weiwei Li <liweiwei@iscas.ac.cn>,
	Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Subject: Re: [PATCH 6/9] target/riscv/cpu: Share RISCVCPUConfig with disassembler
Date: Mon, 12 Jun 2023 19:56:01 +0800	[thread overview]
Message-ID: <bf9ca2e7-5bbe-6415-fbaf-57854d058e47@gmail.com> (raw)
In-Reply-To: <CAEg0e7iAjgRPGxOKEA9BmfZ5M8tc_CzyU4jogwM8d9LSMob0mg@mail.gmail.com>

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: