Approach for exposing HTIF options to other tools #25
Conversation
This fixes the `syscall_t::set_chroot` method so that it will write out the location that it failed to chroot to, `where,` as opposed to the unset attribtue `chroot`.
This changes the `htif_t` constructor to use getopt for option parsing
adding support for POSIX-like `--` long options while preserving
support for "legacy" `+` args. The options supported by `htif_t` are
exposed to programs importing the `htif.h` header.
This results in a change of the `htif_t` and `dtm_t` constructor APIs
so that it can directly process command line arguments. Previously
the `htif_t` constructor was:
htif_t::htif_t(const std::vector<std::string>& target_args)
Now it is:
htif_t::htif_t(int argc, char** argv)
As `htif_t` is a secondary consumer of command line arguments (e.g.,
it is constructed after the rocket-chip emulator), the options
supported by `htif_t` are exposed via three new macros in `htif_t.h`:
* HTIF_USAGE_OPTIONS -- usage string suitable to be appended to any
program that will pass arguments to `htif_t`
* HTIF_LONG_OPTIONS_OPTIND -- Base val for `htif_t` options
* HTIF_LONG_OPTIONS -- `struct option` fragment for `htif_t`. All
options index starting from `HTIF_LONG_OPTIONS_OPTIND`.
Support for the currently unsupported option `+disk` now raises an
exception.
Space is provided for non-standard options which may be exposed to
`htif_t`. Currently only the VCS options of the form `-cm XXX` are
ignored.
Does some minor cleanup of usage formatting. Adds a htif_t() constructor that does not do argument parsing. This is needed for something like `elf2hex` which wants to pass no arguments to htif_t. Note: `elf2hex` is currently non-working on master and not as a result of this. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@gmail.com>
This retains the legacy htif_t constructor using a vector of string to pass command line arguments. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@gmail.com>
|
Is there a getopt_long that isn’t GPL? I thought it was infectious, but maybe I’m misremembering. |
|
Yes, there's a BSD implementation Also, I defer to an expert here, as this isn't something that you want to get wrong... Some earlier discussion of this here: chipsalliance/rocket-chip#484 (comment) |
|
Indeed, my memory is faulty. @palmer-dabbelt any objections? |
|
It looks like it's a GNU extension, but that's not a GPL thing -- we're in the same license territory as linking against anything else in libc. I'm OK with it. |
|
Hi, should I be filing a new issue or would you prefer opening this again? |
|
@neelgala: thanks, you are right about this. Spike should be able to handle a one-argument option. I think the fix is a lot simpler initially, however... spike creates a sim_t object before it knows if it's going to exit (https://github.com/riscv/riscv-isa-sim/blob/9d1e10a36e771bf8cfbf515e07e856e021c1007a/spike_main/spike.cc#L157). For the time being, the binary doesn't appear to have to actually exist: This is a new issue on spike. I expect I can get a PR together that will fix this and a second that aligns spike's option parsing with fesvr. Edit: Ignore my comment about moving the sim_t object. Totally wrong. I looked at this too quickly. |
|
Digging into this more... the existing (though undocumented) way to specify a "no-binary htif" is by passing "none" as the program (see: https://github.com/riscv/riscv-fesvr/blob/master/fesvr/htif.cc#L79). This code path is never hit by This probably warrants a broader discussion related to whether or not htif should be mandating a program to load. @aswaterman: what do you think? |
This provides one strategy for exposing what options are supported by the host to other tools. This uses
getopt_longfor all argument parsing and exposes the data structures necessary for the parsing inhtif_t.h.As it's necessary to still provide backwards support for
spikeandelf2hex, two additional legacy constructors are provided forhtif_t:elf2hex)std::vector<std::string>(forspike)Summarily:
htif_tconstructor that uses(int argc, char** argv)instead of(const std::vector<std::string>)getopt_longfor option parsing and add equivalent--long options to all existing+optionshtif_toptions to external programs via macros inhtif_t.h(see Add option checks for HTIF chipsalliance/rocket-chip#597)+diskoption will throw astd::invalid_argumentexception (as I'm still not aware of this being supported)-cm XXX+YYYset_chrooterror messageFixes #24.
Due to the use of the legacy
std::vector<std::string>this should, theoretically, not break anything. However, it may... I was unaware until trying to buildriscv-torturethatelf2hexwants to pass empty options into thehtif_tconstructor.This is intended to be backed up by chipsalliance/rocket-chip#597.