Changeset View
Changeset View
Standalone View
Standalone View
usr.sbin/bhyve/pci_lpc.c
| Show First 20 Lines • Show All 95 Lines • ▼ Show 20 Lines | lpc_device_parse(const char *opts) | |||||||||||||||||||||||
| int unit, error; | int unit, error; | |||||||||||||||||||||||
| char *str, *cpy, *lpcdev, *node_name; | char *str, *cpy, *lpcdev, *node_name; | |||||||||||||||||||||||
| error = -1; | error = -1; | |||||||||||||||||||||||
| str = cpy = strdup(opts); | str = cpy = strdup(opts); | |||||||||||||||||||||||
| lpcdev = strsep(&str, ","); | lpcdev = strsep(&str, ","); | |||||||||||||||||||||||
| if (lpcdev != NULL) { | if (lpcdev != NULL) { | |||||||||||||||||||||||
| if (strcasecmp(lpcdev, "bootrom") == 0) { | if (strcasecmp(lpcdev, "bootrom") == 0) { | |||||||||||||||||||||||
| set_config_value("lpc.bootrom", str); | nvlist_t *const nvl = create_config_node("lpc.bootrom"); | |||||||||||||||||||||||
| const char *const romfile = strsep(&str, ","); | ||||||||||||||||||||||||
| const char *const varfile = strsep(&str, ","); | ||||||||||||||||||||||||
| set_config_value_node(nvl, "romfile", romfile); | ||||||||||||||||||||||||
rew: I don't think libnv will like it when varfile is NULL, it looks like it will put the nvlist in… | ||||||||||||||||||||||||
| set_config_value_node(nvl, "varfile", varfile); | ||||||||||||||||||||||||
| error = 0; | error = 0; | |||||||||||||||||||||||
| goto done; | goto done; | |||||||||||||||||||||||
| } | } | |||||||||||||||||||||||
| for (unit = 0; unit < LPC_UART_NUM; unit++) { | for (unit = 0; unit < LPC_UART_NUM; unit++) { | |||||||||||||||||||||||
| if (strcasecmp(lpcdev, lpc_uart_names[unit]) == 0) { | if (strcasecmp(lpcdev, lpc_uart_names[unit]) == 0) { | |||||||||||||||||||||||
| asprintf(&node_name, "lpc.%s.path", | asprintf(&node_name, "lpc.%s.path", | |||||||||||||||||||||||
| lpc_uart_names[unit]); | lpc_uart_names[unit]); | |||||||||||||||||||||||
| set_config_value(node_name, str); | set_config_value(node_name, str); | |||||||||||||||||||||||
| Show All 26 Lines | lpc_print_supported_devices() | |||||||||||||||||||||||
| for (i = 0; i < LPC_UART_NUM; i++) | for (i = 0; i < LPC_UART_NUM; i++) | |||||||||||||||||||||||
| printf("%s\n", lpc_uart_names[i]); | printf("%s\n", lpc_uart_names[i]); | |||||||||||||||||||||||
| printf("%s\n", pctestdev_getname()); | printf("%s\n", pctestdev_getname()); | |||||||||||||||||||||||
| } | } | |||||||||||||||||||||||
| const char * | const char * | |||||||||||||||||||||||
| lpc_bootrom(void) | lpc_bootrom(void) | |||||||||||||||||||||||
| { | { | |||||||||||||||||||||||
| return (get_config_value("lpc.bootrom.romfile")); | ||||||||||||||||||||||||
| return (get_config_value("lpc.bootrom")); | ||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||
| static void | static void | |||||||||||||||||||||||
| lpc_uart_intr_assert(void *arg) | lpc_uart_intr_assert(void *arg) | |||||||||||||||||||||||
| { | { | |||||||||||||||||||||||
| struct lpc_uart_softc *sc = arg; | struct lpc_uart_softc *sc = arg; | |||||||||||||||||||||||
| assert(sc->irq >= 0); | assert(sc->irq >= 0); | |||||||||||||||||||||||
| Show All 40 Lines | lpc_uart_io_handler(struct vmctx *ctx, int vcpu, int in, int port, int bytes, | |||||||||||||||||||||||
| } | } | |||||||||||||||||||||||
| return (0); | return (0); | |||||||||||||||||||||||
| } | } | |||||||||||||||||||||||
| static int | static int | |||||||||||||||||||||||
| lpc_init(struct vmctx *ctx) | lpc_init(struct vmctx *ctx) | |||||||||||||||||||||||
| { | { | |||||||||||||||||||||||
| struct lpc_uart_softc *sc; | struct lpc_uart_softc *sc; | |||||||||||||||||||||||
| struct inout_port iop; | struct inout_port iop; | |||||||||||||||||||||||
| const char *backend, *name, *romfile; | const char *backend, *name; | |||||||||||||||||||||||
| char *node_name; | char *node_name; | |||||||||||||||||||||||
| int unit, error; | int unit, error; | |||||||||||||||||||||||
| romfile = get_config_value("lpc.bootrom"); | const nvlist_t *const nvl_bootrom = find_config_node("lpc.bootrom"); | |||||||||||||||||||||||
| if (romfile != NULL) { | if (nvl_bootrom != NULL) { | |||||||||||||||||||||||
| error = bootrom_loadrom(ctx, romfile); | error = bootrom_loadrom(ctx, nvl_bootrom); | |||||||||||||||||||||||
Not Done Inline Actionswhy so much const? as opposed to const nvlist_t *nvl? also curious, why not declare nvl at the beginning of the function? I see there are other occurrences like this within the patch..I'm only commenting on this one. I don't mean to nitpick here. rew: why so much const? as opposed to const nvlist_t *nvl?
also curious, why not declare nvl at the… | ||||||||||||||||||||||||
Not Done Inline ActionsIt is true that style(9) discourages C++ style variable declarations in the middle of scopes (though it has recently gained some exceptions for things like declaring a an integrator in a for statement). jhb: It is true that style(9) discourages C++ style variable declarations in the middle of scopes… | ||||||||||||||||||||||||
| if (error) | if (error) | |||||||||||||||||||||||
| return (error); | return (error); | |||||||||||||||||||||||
| } | } | |||||||||||||||||||||||
Done Inline Actions
This doesn't look correct. If a bootrom isn't being used, bootrom_loadrom() will exit with an error. Either need to do something like this or have bootrom_loadrom() return success when a bootrom is not being used. rew: This doesn't look correct.
If a bootrom isn't being used, bootrom_loadrom() will exit with an… | ||||||||||||||||||||||||
| /* COM1 and COM2 */ | /* COM1 and COM2 */ | |||||||||||||||||||||||
| for (unit = 0; unit < LPC_UART_NUM; unit++) { | for (unit = 0; unit < LPC_UART_NUM; unit++) { | |||||||||||||||||||||||
| sc = &lpc_uart_softc[unit]; | sc = &lpc_uart_softc[unit]; | |||||||||||||||||||||||
| name = lpc_uart_names[unit]; | name = lpc_uart_names[unit]; | |||||||||||||||||||||||
| if (uart_legacy_alloc(unit, &sc->iobase, &sc->irq) != 0) { | if (uart_legacy_alloc(unit, &sc->iobase, &sc->irq) != 0) { | |||||||||||||||||||||||
| EPRINTLN("Unable to allocate resources for " | EPRINTLN("Unable to allocate resources for " | |||||||||||||||||||||||
| ▲ Show 20 Lines • Show All 292 Lines • Show Last 20 Lines | ||||||||||||||||||||||||
I don't think libnv will like it when varfile is NULL, it looks like it will put the nvlist in an error state.
Maybe something like? Should lpc.bootrom get a check like this too?