Add support for specifying properties of dynamically-registered devices via the command line

The previous approach of traversing the machine and its device tree
at startup to register CLI11 options was not working for dynamically
registered devices like PCI cards. This meant that options like
gfxmem_size or mon_id from the video cards could not be set.

Switch to instead registering in MachineFactory a hook function that
provides CLI flag values. We can call it when registering any property,
whether at startup or dynamically.
This commit is contained in:
Mihai Parparita 2024-11-21 22:51:38 -08:00
parent 5a0a7b12e4
commit a941836a4d
4 changed files with 68 additions and 47 deletions

View File

@ -119,9 +119,8 @@ PCIBase *PCIHost::attach_pci_device(const std::string& dev_name, int slot_id, co
}
// attempt to create device object
MachineFactory::register_device_settings(dev_name);
auto desc = DeviceRegistry::get_descriptor(dev_name);
map<string, string> settings;
MachineFactory::get_device_settings(desc, settings);
auto dev_obj = desc.m_create_func();
if (!dev_obj->supports_type(HWCompType::PCI_DEV)) {

View File

@ -203,7 +203,7 @@ void MachineFactory::list_device_settings(DeviceDescription& dev)
print_settings(dev.properties);
}
void MachineFactory::print_settings(PropMap& prop_map)
void MachineFactory::print_settings(const PropMap& prop_map)
{
string help;
@ -237,27 +237,25 @@ void MachineFactory::print_settings(PropMap& prop_map)
}
}
void MachineFactory::get_device_settings(DeviceDescription& dev, map<string, string> &settings)
void MachineFactory::register_device_settings(const std::string& name)
{
auto dev = DeviceRegistry::get_descriptor(name);
for (auto& d : dev.subdev_list) {
get_device_settings(DeviceRegistry::get_descriptor(d), settings);
register_device_settings(d);
}
for (auto& p : dev.properties) {
if (settings.count(p.first)) {
// This is a hack. Need to implement hierarchical paths and per device properties.
LOG_F(ERROR, "Duplicate setting \"%s\".", p.first.c_str());
}
else {
settings[p.first] = p.second->get_string();
// populate dynamic machine settings from presets
gMachineSettings[p.first] = unique_ptr<BasicProperty>(p.second->clone());
register_settings(dev.properties);
if (!dev.properties.empty()) {
std::cout << "Device " << name << " Settings" << endl
<< std::string(36, '-') << endl;
for (auto& p : dev.properties) {
std::cout << std::setw(24) << std::right << p.first << " : "
<< gMachineSettings[p.first]->get_string() << endl;
}
}
}
int MachineFactory::get_machine_settings(const string& id, map<string, string> &settings)
int MachineFactory::register_machine_settings(const std::string& id)
{
auto it = get_registry().find(id);
if (it != get_registry().end()) {
@ -265,15 +263,17 @@ int MachineFactory::get_machine_settings(const string& id, map<string, string> &
gMachineSettings.clear();
for (auto& p : props) {
settings[p.first] = p.second->get_string();
register_settings(props);
// populate dynamic machine settings from presets
gMachineSettings[p.first] = unique_ptr<BasicProperty>(p.second->clone());
std::cout << endl << "Machine " << id << " Settings" << endl
<< std::string(36, '-') << endl;
for (auto& p : props) {
std::cout << std::setw(24) << std::right << p.first << " : "
<< gMachineSettings[p.first]->get_string() << endl;
}
for (auto& dev : it->second.devices) {
get_device_settings(DeviceRegistry::get_descriptor(dev), settings);
register_device_settings(dev);
}
} else {
LOG_F(ERROR, "Unknown machine id %s", id.c_str());
@ -283,17 +283,22 @@ int MachineFactory::get_machine_settings(const string& id, map<string, string> &
return 0;
}
void MachineFactory::set_machine_settings(map<string, string> &settings) {
for (auto& s : settings) {
gMachineSettings.at(s.first)->set_string(s.second);
void MachineFactory::register_settings(const PropMap& props) {
for (auto& p : props) {
if (gMachineSettings.count(p.first)) {
// This is a hack. Need to implement hierarchical paths and per device properties.
LOG_F(ERROR, "Duplicate setting \"%s\".", p.first.c_str());
}
else {
auto clone = p.second->clone();
auto override_value = get_setting_value(p.first);
if (override_value) {
clone->set_string(*override_value);
}
gMachineSettings[p.first] = std::unique_ptr<BasicProperty>(clone);
}
}
// print machine settings summary
cout << endl << "Machine settings summary: " << endl;
for (auto& p : gMachineSettings) {
cout << p.first << " : " << p.second->get_string() << endl;
}
}
string MachineFactory::machine_name_from_rom(string& rom_filepath) {
@ -412,3 +417,5 @@ int MachineFactory::create_machine_for_id(string& id, string& rom_filepath) {
return 0;
}
GetSettingValueFunc MachineFactory::get_setting_value;

View File

@ -29,7 +29,9 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
#include <machines/machineproperties.h>
#include <functional>
#include <map>
#include <optional>
#include <string>
#include <vector>
@ -45,6 +47,8 @@ struct MachineDescription {
function<int(string&)> init_func;
};
typedef std::function<std::optional<std::string>(const std::string&)> GetSettingValueFunc;
class MachineFactory
{
public:
@ -57,18 +61,20 @@ public:
static int create(string& mach_id);
static int create_machine_for_id(string& id, string& rom_filepath);
static void get_device_settings(DeviceDescription& dev, map<string, string> &settings);
static int get_machine_settings(const string& id, map<string, string> &settings);
static void set_machine_settings(map<string, string> &settings);
static void register_device_settings(const std::string &name);
static int register_machine_settings(const std::string& id);
static void list_machines();
static void list_properties();
static GetSettingValueFunc get_setting_value;
private:
static void create_device(string& dev_name, DeviceDescription& dev);
static void print_settings(PropMap& p);
static void print_settings(const PropMap& p);
static void list_device_settings(DeviceDescription& dev);
static int load_boot_rom(string& rom_filepath);
static void register_settings(const PropMap& p);
static map<string, MachineDescription> & get_registry() {
static map<string, MachineDescription> machine_registry;

View File

@ -162,22 +162,31 @@ int main(int argc, char** argv) {
}
}
/* handle overriding of machine settings from command line */
map<string, string> settings;
if (MachineFactory::get_machine_settings(machine_str, settings) < 0) {
// Hook to allow properties to be read from the command-line, regardless
// of when they are registered.
MachineFactory::get_setting_value = [&](const std::string& name) -> std::optional<std::string> {
CLI::App sa;
sa.allow_extras();
std::string value;
sa.add_option("--" + name, value);
try {
sa.parse(app.remaining_for_passthrough());
} catch (const CLI::Error& e) {
ABORT_F("Cannot parse CLI: %s", e.get_name().c_str());
}
if (sa.count("--" + name) > 0) {
return value;
} else {
return std::nullopt;
}
};
if (MachineFactory::register_machine_settings(machine_str) < 0) {
return 1;
}
CLI::App sa;
sa.allow_extras();
for (auto& s : settings) {
sa.add_option("--" + s.first, s.second);
}
sa.parse(app.remaining_for_passthrough()); /* TODO: handle exceptions! */
MachineFactory::set_machine_settings(settings);
cout << "BootROM path: " << bootrom_path << endl;
cout << "Execution mode: " << execution_mode << endl;
if (is_deterministic) {