Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Mon, 23 Oct 2023 11:19:23 +0200
From:   Andrew Jones <ajones@ventanamicro.com>
To:     Charlie Jenkins <charlie@rivosinc.com>
Cc:     =?utf-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Jessica Clarke <jrtc27@jrtc27.com>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] riscv: Script to autogenerate instruction handling code
Message-ID: <20231023-aa3f789ccb483afbbdd55b38@orel>
References: <ZRD8RzBeA2RMrcki@ghost>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZRD8RzBeA2RMrcki@ghost>
Precedence: bulk

On Sun, Sep 24, 2023 at 08:19:35PM -0700, Charlie Jenkins wrote:
> I have heeded Andrew Jones' advice and written a script to generate the
> instruction handling code. It is still in development, but currently
> lives on a fork of riscv-opcodes [1]. I am interested if what I have
> produced so far is in line with what people would want to see.

Hi Charlie,

Sorry for my slow response. I'm glad to see that we're going in a
direction where we generate these functions and reuse an existing
generator to do it.

> 
> An insn.h file can be generated by running the following in the repo:
> 
> make
> python3 parse_linux.py instr_dict.yaml insn.h opcodes_config variable_field_data.yaml
> 
> I have pushed the generated files to the repo so people do not need to
> run the script.

I couldn't find the generated files, not even [3] from your references
seems to be present.

> 
> Each instruction has "variable fields" such as registers and immediates.
> For each variable field that appears in any provided instruction 3 functions
> are provided: extract a variable field from an instruction, insert a value into
> a variable field of an instruction, update a value into a variable field of an
> instruction. Update first clears the previous value of the variable field of the
> instruction. Then for each instruction, the script generates a function to check if an
> arbitrary 32-bit value matches the given instruction, and a function to
> generate the binary for the instruction given the required variable
> fields.
> 
> I was able to use riscv-opcodes to parse the instruction files, but
> needed to create a new data structure in variable_field_data.py [2] which
> holds the positioning of immediates inside of an instruction.
> 
> I envision that opcodes_config [3] would live inside of the kernel alongside
> a simple script to call riscv-opcodes (that resides somewhere in the
> user's file system) with appropriate parameters. When somebody wants to
> add a new instruction, they can add an instruction to opcodes_config,
> run the script, and commit the resulting generated file.

That sounds good to me. (They may hand craft the functions for a single
instruction too, by just using the other functions as templates, but even
if the script isn't used all the time in the future, the initial
conversion of many instructions makes it worth while, IMO.)

> 
> If this script is in a direction that people like, I will continue to
> fix up the issues in it and try to get it upstreamed to riscv-opcodes
> before I send a kernel patch.

Please send me a pointer to opcodes_config and insn.h. Also, since you're
extending riscv-opcodes with variable_field_data.py, have you found a way
to verify that all the immediate offsets are correct? Or were the offsets
extracted from the spec/tool directly somehow? I.e. was
variable_field_data.py mostly generated itself?

Thanks,
drew

> 
> - Charlie
> 
> [1] https://github.com/charlie-rivos/riscv-opcodes/tree/linux_parsing
> [2] https://github.com/charlie-rivos/riscv-opcodes/blob/linux_parsing/variable_field_data.py
> [3] https://github.com/charlie-rivos/riscv-opcodes/blob/linux_parsing/opcodes_config