Page MenuHomeFreeBSD
Differential D40987 Diff 128252 usr.sbin/bhyve/bhyverun.c

Changeset View

Standalone View

View Options

usr.sbin/bhyve/bhyverun.c

Show First 20 Lines • Show All 470 Lines • ▼ Show 20 Lines#endif
vm_loop(vi->ctx, vi->vcpu); vm_loop(vi->ctx, vi->vcpu);
/* not reached */ /* not reached */
exit(1); exit(1);
return (NULL); return (NULL);
} }
static void void
fbsdrun_addcpu(struct vcpu_info *vi) fbsdrun_addcpu(int vcpuid)
jhbUnsubmitted
Done
Inline Actions

Maybe s/vcpu/vcpuid/ for this function? I've tried to use vcpuid for IDs and vcpu for struct vcpu * in more recent changes.

jhb: Maybe s/vcpu/vcpuid/ for this function? I've tried to use `vcpuid` for IDs and `vcpu` for…
corvinkUnsubmitted
Not Done
Inline Actions

I do agree with Johns suggestion. It makes things clearer.

Additionally, some parts of the commit (e.g. this input type change) could be done in a separate commit. Nevertheless, I don't have a strong preference on that.

corvink: I do agree with Johns suggestion. It makes things clearer. Additionally, some parts of the…
markjAuthorUnsubmitted
Done
Inline Actions

My main goal in splitting up the patches is to keep them reviewable, not necessarily to produce minimal diffs. If it would really help for me to split up some of these reviews further, please feel free to ask, but otherwise I'll avoid it, since it's more work and creates more opportunities to introduce bugs.

markj: My main goal in splitting up the patches is to keep them reviewable, not necessarily to produce…
markjAuthorUnsubmitted
Done
Inline Actions

I was matching fbsdrun_deletecpu() below, but I'll change it here.

markj: I was matching fbsdrun_deletecpu() below, but I'll change it here.
{ {
struct vcpu_info *vi;
pthread_t thr; pthread_t thr;
int error; int error;
vi = &vcpu_info[vcpuid];
error = vm_activate_cpu(vi->vcpu); error = vm_activate_cpu(vi->vcpu);
if (error != 0) if (error != 0)
err(EX_OSERR, "could not activate CPU %d", vi->vcpuid); err(EX_OSERR, "could not activate CPU %d", vi->vcpuid);
CPU_SET_ATOMIC(vi->vcpuid, &cpumask); CPU_SET_ATOMIC(vcpuid, &cpumask);
vm_suspend_cpu(vi->vcpu); vm_suspend_cpu(vi->vcpu);
error = pthread_create(&thr, NULL, fbsdrun_start_thread, vi); error = pthread_create(&thr, NULL, fbsdrun_start_thread, vi);
assert(error == 0); assert(error == 0);
} }
void void
▲ Show 20 Lines • Show All 87 Lines • ▼ Show 20 Linesnum_vcpus_allowed(struct vmctx *ctx, struct vcpu *vcpu)
error = vm_get_topology(ctx, &sockets, &cores, &threads, &maxcpus); error = vm_get_topology(ctx, &sockets, &cores, &threads, &maxcpus);
if (error == 0) if (error == 0)
return (maxcpus); return (maxcpus);
else else
return (1); return (1);
} }
static void
fbsdrun_set_capabilities(struct vcpu *vcpu)
{
int err, tmp;
if (get_config_bool_default("x86.vmexit_on_hlt", false)) {
err = vm_get_capability(vcpu, VM_CAP_HALT_EXIT, &tmp);
if (err < 0) {
fprintf(stderr, "VM exit on HLT not supported\n");
exit(4);
}
vm_set_capability(vcpu, VM_CAP_HALT_EXIT, 1);
}
if (get_config_bool_default("x86.vmexit_on_pause", false)) {
/*
* pause exit support required for this mode
*/
err = vm_get_capability(vcpu, VM_CAP_PAUSE_EXIT, &tmp);
if (err < 0) {
fprintf(stderr,
"SMP mux requested, no pause support\n");
exit(4);
}
vm_set_capability(vcpu, VM_CAP_PAUSE_EXIT, 1);
}
if (get_config_bool_default("x86.x2apic", false))
err = vm_set_x2apic_state(vcpu, X2APIC_ENABLED);
else
err = vm_set_x2apic_state(vcpu, X2APIC_DISABLED);
if (err) {
fprintf(stderr, "Unable to set x2apic state (%d)\n", err);
exit(4);
}
vm_set_capability(vcpu, VM_CAP_ENABLE_INVPCID, 1);
err = vm_set_capability(vcpu, VM_CAP_IPI_EXIT, 1);
assert(err == 0);
}
static struct vmctx * static struct vmctx *
do_open(const char *vmname) do_open(const char *vmname)
{ {
struct vmctx *ctx; struct vmctx *ctx;
int error; int error;
bool reinit, romboot; bool reinit, romboot;
reinit = romboot = false; reinit = romboot = false;
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines if (reinit) {
} }
} }
error = vm_set_topology(ctx, cpu_sockets, cpu_cores, cpu_threads, 0); error = vm_set_topology(ctx, cpu_sockets, cpu_cores, cpu_threads, 0);
if (error) if (error)
errx(EX_OSERR, "vm_set_topology"); errx(EX_OSERR, "vm_set_topology");
return (ctx); return (ctx);
} }
static void
spinup_vcpu(struct vcpu_info *vi, bool bsp)
{
int error;
if (!bsp) {
fbsdrun_set_capabilities(vi->vcpu);
/*
* Enable the 'unrestricted guest' mode for APs.
*
* APs startup in power-on 16-bit mode.
*/
error = vm_set_capability(vi->vcpu, VM_CAP_UNRESTRICTED_GUEST, 1);
assert(error == 0);
}
fbsdrun_addcpu(vi);
}
static bool static bool
parse_config_option(const char *option) parse_config_option(const char *option)
{ {
const char *value; const char *value;
char *path; char *path;
value = strchr(option, '='); value = strchr(option, '=');
if (value == NULL || value[1] == '\0') if (value == NULL || value[1] == '\0')
▲ Show 20 Lines • Show All 54 Lines • ▼ Show 20 Lines if (colon == NULL) {
sport = colon; sport = colon;
set_config_value("gdb.address", opt); set_config_value("gdb.address", opt);
} }
set_config_value("gdb.port", sport); set_config_value("gdb.port", sport);
} }
#endif #endif
static void
set_defaults(void)
{
set_config_bool("acpi_tables", true);
set_config_bool("acpi_tables_in_memory", true);
set_config_value("memory.size", "256M");
set_config_bool("x86.strictmsr", true);
set_config_value("lpc.fwcfg", "bhyve");
}
int int
main(int argc, char *argv[]) main(int argc, char *argv[])
{ {
int c, error; int c, error;
int max_vcpus, memflags; int max_vcpus, memflags;
struct vcpu *bsp; struct vcpu *bsp;
struct vmctx *ctx; struct vmctx *ctx;
size_t memsize; size_t memsize;
const char *optstr, *value, *vmname; const char *optstr, *value, *vmname;
#ifdef BHYVE_SNAPSHOT #ifdef BHYVE_SNAPSHOT
char *restore_file; char *restore_file;
struct restore_state rstate; struct restore_state rstate;
restore_file = NULL; restore_file = NULL;
#endif #endif
init_config(); bhyve_init_config();
jhbUnsubmitted
Not Done
Inline Actions

While set_defaults certainly seems MD, the call to init_config seems MI?

jhb: While set_defaults certainly seems MD, the call to `init_config` seems MI?
markjAuthorUnsubmitted
Done
Inline Actions

It just seemed a bit cleaner to have all config initialization done with one function call from main(), even though that creates a bit of redundancy. In particular, init_config() needs to be called before bhyve_init_config(). If you prefer to keep the init_config() call here, I can move it back.

markj: It just seemed a bit cleaner to have all config initialization done with one function call from…
jhbUnsubmitted
Not Done
Inline Actions

I do think in this case it would be best to keep init_config just before bhyve_init_config in main. You could perhaps even rename bhyve_init_config to bhyve_set_defaults in that case if you wanted?

jhb: I do think in this case it would be best to keep `init_config` just before `bhyve_init_config`…
set_defaults();
progname = basename(argv[0]); progname = basename(argv[0]);
#ifdef BHYVE_SNAPSHOT #ifdef BHYVE_SNAPSHOT
optstr = "aehuwxACDHIPSWYk:f:o:p:G:c:s:m:l:K:U:r:"; optstr = "aehuwxACDHIPSWYk:f:o:p:G:c:s:m:l:K:U:r:";
#else #else
optstr = "aehuwxACDHIPSWYk:f:o:p:G:c:s:m:l:K:U:"; optstr = "aehuwxACDHIPSWYk:f:o:p:G:c:s:m:l:K:U:";
#endif #endif
while ((c = getopt(argc, argv, optstr)) != -1) { while ((c = getopt(argc, argv, optstr)) != -1) {
switch (c) { switch (c) {
#ifdef __amd64__
case 'a': case 'a':
set_config_bool("x86.x2apic", false); set_config_bool("x86.x2apic", false);
break; break;
#endif
case 'A': case 'A':
/* /*
* NOP. For backward compatibility. Most systems don't * NOP. For backward compatibility. Most systems don't
* work properly without sane ACPI tables. Therefore, * work properly without sane ACPI tables. Therefore,
* we're always generating them. * we're always generating them.
*/ */
break; break;
case 'D': case 'D':
▲ Show 20 Lines • Show All 59 Lines • ▼ Show 20 Lines case 'S':
break; break;
case 'm': case 'm':
set_config_value("memory.size", optarg); set_config_value("memory.size", optarg);
break; break;
case 'o': case 'o':
if (!parse_config_option(optarg)) if (!parse_config_option(optarg))
errx(EX_USAGE, "invalid configuration option '%s'", optarg); errx(EX_USAGE, "invalid configuration option '%s'", optarg);
break; break;
#ifdef __amd64__
case 'H': case 'H':
set_config_bool("x86.vmexit_on_hlt", true); set_config_bool("x86.vmexit_on_hlt", true);
break; break;
case 'I': case 'I':
/* /*
* The "-I" option was used to add an ioapic to the * The "-I" option was used to add an ioapic to the
* virtual machine. * virtual machine.
* *
* An ioapic is now provided unconditionally for each * An ioapic is now provided unconditionally for each
* virtual machine and this option is now deprecated. * virtual machine and this option is now deprecated.
*/ */
break; break;
case 'P': case 'P':
set_config_bool("x86.vmexit_on_pause", true); set_config_bool("x86.vmexit_on_pause", true);
break; break;
case 'e': case 'e':
set_config_bool("x86.strictio", true); set_config_bool("x86.strictio", true);
break; break;
#ifdef __amd64__
case 'u': case 'u':
set_config_bool("rtc.use_localtime", false); set_config_bool("rtc.use_localtime", false);
break; break;
#endif #endif
case 'U': case 'U':
set_config_value("uuid", optarg); set_config_value("uuid", optarg);
break; break;
#ifdef __amd64__
case 'w': case 'w':
set_config_bool("x86.strictmsr", false); set_config_bool("x86.strictmsr", false);
break; break;
#endif
case 'W': case 'W':
set_config_bool("virtio_msix", false); set_config_bool("virtio_msix", false);
break; break;
#ifdef __amd64__
case 'x': case 'x':
set_config_bool("x86.x2apic", true); set_config_bool("x86.x2apic", true);
break; break;
#ifdef __amd64__
case 'Y': case 'Y':
set_config_bool("x86.mptable", false); set_config_bool("x86.mptable", false);
break; break;
#endif #endif
case 'h': case 'h':
usage(0); usage(0);
default: default:
usage(1); usage(1);
▲ Show 20 Lines • Show All 57 Lines • ▼ Show 20 Lines#endif
bsp = vm_vcpu_open(ctx, BSP); bsp = vm_vcpu_open(ctx, BSP);
max_vcpus = num_vcpus_allowed(ctx, bsp); max_vcpus = num_vcpus_allowed(ctx, bsp);
if (guest_ncpus > max_vcpus) { if (guest_ncpus > max_vcpus) {
fprintf(stderr, "%d vCPUs requested but only %d available\n", fprintf(stderr, "%d vCPUs requested but only %d available\n",
guest_ncpus, max_vcpus); guest_ncpus, max_vcpus);
exit(4); exit(4);
} }
fbsdrun_set_capabilities(bsp); bhyve_init_vcpu(bsp);
/* Allocate per-VCPU resources. */ /* Allocate per-VCPU resources. */
vcpu_info = calloc(guest_ncpus, sizeof(*vcpu_info)); vcpu_info = calloc(guest_ncpus, sizeof(*vcpu_info));
for (int vcpuid = 0; vcpuid < guest_ncpus; vcpuid++) { for (int vcpuid = 0; vcpuid < guest_ncpus; vcpuid++) {
vcpu_info[vcpuid].ctx = ctx; vcpu_info[vcpuid].ctx = ctx;
vcpu_info[vcpuid].vcpuid = vcpuid; vcpu_info[vcpuid].vcpuid = vcpuid;
if (vcpuid == BSP) if (vcpuid == BSP)
vcpu_info[vcpuid].vcpu = bsp; vcpu_info[vcpuid].vcpu = bsp;
▲ Show 20 Lines • Show All 74 Lines • ▼ Show 20 Lines#endif
*/ */
if (get_config_bool("acpi_tables")) if (get_config_bool("acpi_tables"))
vmgenc_init(ctx); vmgenc_init(ctx);
#ifdef BHYVE_GDB #ifdef BHYVE_GDB
init_gdb(ctx); init_gdb(ctx);
#endif #endif
#ifdef __amd64__
if (lpc_bootrom()) {
if (vm_set_capability(bsp, VM_CAP_UNRESTRICTED_GUEST, 1)) {
fprintf(stderr, "ROM boot failed: unrestricted guest "
"capability not available\n");
exit(4);
}
error = vcpu_reset(bsp);
assert(error == 0);
}
#endif
/* /*
* Add all vCPUs. * Add all vCPUs.
*/ */
for (int vcpuid = 0; vcpuid < guest_ncpus; vcpuid++) for (int vcpuid = 0; vcpuid < guest_ncpus; vcpuid++)
spinup_vcpu(&vcpu_info[vcpuid], vcpuid == BSP); bhyve_start_vcpu(vcpu_info[vcpuid].vcpu, vcpuid == BSP);
#ifdef BHYVE_SNAPSHOT #ifdef BHYVE_SNAPSHOT
if (restore_file != NULL) { if (restore_file != NULL) {
fprintf(stdout, "Pausing pci devs...\r\n"); fprintf(stdout, "Pausing pci devs...\r\n");
if (vm_pause_devices() != 0) { if (vm_pause_devices() != 0) {
fprintf(stderr, "Failed to pause PCI device state.\n"); fprintf(stderr, "Failed to pause PCI device state.\n");
exit(1); exit(1);
} }
▲ Show 20 Lines • Show All 103 Lines • Show Last 20 Lines