Page MenuHomeFreeBSD
Differential D35800 Diff 111905 sys/x86/xen/pv.c

Changeset View

Standalone View

View Options

sys/x86/xen/pv.c

Show First 20 Lines • Show All 88 Lines • ▼ Show 20 Lines
extern u_int64_t hammer_time(u_int64_t, u_int64_t); extern u_int64_t hammer_time(u_int64_t, u_int64_t);
/* Xen initial function */ /* Xen initial function */
uint64_t hammer_time_xen(vm_paddr_t); uint64_t hammer_time_xen(vm_paddr_t);
#define MAX_E820_ENTRIES 128 #define MAX_E820_ENTRIES 128
/*--------------------------- Forward Declarations ---------------------------*/ /*--------------------------- Forward Declarations ---------------------------*/
static caddr_t xen_pvh_parse_preload_data(uint64_t); static caddr_t xen_pvh_parse_preload_data(uint64_t);
static void xen_pvh_parse_memmap(caddr_t, vm_paddr_t *, int *); static void pvh_parse_memmap(caddr_t, vm_paddr_t *, int *);
/*---------------------------- Extern Declarations ---------------------------*/ /*---------------------------- Extern Declarations ---------------------------*/
/* /*
* Placed by the linker at the end of the bss section, which is the last * Placed by the linker at the end of the bss section, which is the last
* section loaded by Xen before loading the symtab and strtab. * section loaded by Xen before loading the symtab and strtab.
*/ */
extern uint32_t end; extern uint32_t end;
/*-------------------------------- Global Data -------------------------------*/ /*-------------------------------- Global Data -------------------------------*/
struct init_ops xen_pvh_init_ops = { struct init_ops xen_pvh_init_ops = {
.parse_preload_data = xen_pvh_parse_preload_data, .parse_preload_data = xen_pvh_parse_preload_data,
.early_clock_source_init = xen_clock_init, .early_clock_source_init = xen_clock_init,
.early_delay = xen_delay, .early_delay = xen_delay,
.parse_memmap = xen_pvh_parse_memmap, .parse_memmap = pvh_parse_memmap,
}; };
static struct bios_smap xen_smap[MAX_E820_ENTRIES]; static struct bios_smap xen_smap[MAX_E820_ENTRIES];
static struct hvm_start_info *start_info; static struct hvm_start_info *start_info;
/*-------------------------------- Xen PV init -------------------------------*/ /*-------------------------------- Xen PV init -------------------------------*/
▲ Show 20 Lines • Show All 271 Lines • ▼ Show 20 Lines
#ifdef DDB #ifdef DDB
xen_pvh_parse_symtab(); xen_pvh_parse_symtab();
#endif #endif
return (kmdp); return (kmdp);
} }
static void static void
pvh_parse_memmap_start_info(caddr_t kmdp, vm_paddr_t *physmap,
int *physmap_idx)
{
const struct hvm_memmap_table_entry * entries;
roygerUnsubmitted
Not Done
Inline Actions

This should be const.

royger: This should be const.
cpercivaAuthorUnsubmitted
Done
Inline Actions

Thanks, fixed.

cperciva: Thanks, fixed.
size_t nentries;
size_t i;
roygerUnsubmitted
Not Done
Inline Actions

It's fine to use unsigned int here for both nentries and i, as the size of those fields is 32bits (in fact I don't think you need a local variable for nentries, as you could just use start_info->memmap_entries IMO.

royger: It's fine to use unsigned int here for both nentries and i, as the size of those fields is…
cpercivaAuthorUnsubmitted
Done
Inline Actions

In practice I'm sure it would be fine, but as a matter of style I prefer to use size_t for sizes of and indexes into arrays.

cperciva: In practice I'm sure it would be fine, but as a matter of style I prefer to use `size_t` for…
/* Extract from HVM start_info. */
entries = (struct hvm_memmap_table_entry *)(start_info->memmap_paddr + KERNBASE);
nentries = start_info->memmap_entries;
roygerUnsubmitted
Not Done
Inline Actions

For safety you need to add KERNBASE here to the physical address, or you otherwise risk this only working with the initial page tables that have identity mappings.

royger: For safety you need to add KERNBASE here to the physical address, or you otherwise risk this…
cpercivaAuthorUnsubmitted
Done
Inline Actions

Thanks, fixed.

cperciva: Thanks, fixed.
/* Convert into E820 format and handle one by one. */
for (i = 0; i < nentries; i++) {
struct bios_smap entry;
roygerUnsubmitted
Not Done
Inline Actions

You could limit the scope of entry and define it inside the if:

struct bios_smap entry = {
    .base = entries[i].addr,
    .length = entries[i].size,
    .type = entries[i].type,
};

Or just cast the entry pointer into a struct bios_smap pointer, as the layouts are the same (at least for the 3 fields we care about).

royger: You could limit the scope of entry and define it inside the if: ``` struct bios_smap entry = {…
cpercivaAuthorUnsubmitted
Done
Inline Actions

I considered that, but I figured this way was clear and gave me a convenient place to include the comment
explaining that XEN_HVM_MEMMAP_TYPE_* values match SMAP_TYPE_* values and that copying them
directly isn't a bug.

cperciva: I considered that, but I figured this way was clear and gave me a convenient place to include…
roygerUnsubmitted
Done
Inline Actions

Could you at least define entry inside the for loop to limit the scope? It's only used there AFAICT.

royger: Could you at least define entry inside the for loop to limit the scope? It's only used there…
cpercivaAuthorUnsubmitted
Done
Inline Actions

Done.

cperciva: Done.
entry.base = entries[i].addr;
entry.length = entries[i].size;
/*
* Luckily for us, the XEN_HVM_MEMMAP_TYPE_* values exactly
* match the SMAP_TYPE_* values so we don't need to translate
* anything here.
*/
entry.type = entries[i].type;
bios_add_smap_entries(&entry, 1, physmap, physmap_idx);
}
}
static void
xen_pvh_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx) xen_pvh_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx)
{ {
struct xen_memory_map memmap; struct xen_memory_map memmap;
u_int32_t size; u_int32_t size;
int rc; int rc;
/* Fetch the E820 map from Xen */ /* Fetch the E820 map from Xen */
memmap.nr_entries = MAX_E820_ENTRIES; memmap.nr_entries = MAX_E820_ENTRIES;
set_xen_guest_handle(memmap.buffer, xen_smap); set_xen_guest_handle(memmap.buffer, xen_smap);
rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap); rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
if (rc) { if (rc) {
xc_printf("ERROR: unable to fetch Xen E820 memory map: %d\n", xc_printf("ERROR: unable to fetch Xen E820 memory map: %d\n",
rc); rc);
HYPERVISOR_shutdown(SHUTDOWN_crash); HYPERVISOR_shutdown(SHUTDOWN_crash);
} }
size = memmap.nr_entries * sizeof(xen_smap[0]); size = memmap.nr_entries * sizeof(xen_smap[0]);
bios_add_smap_entries(xen_smap, size, physmap, physmap_idx); bios_add_smap_entries(xen_smap, size, physmap, physmap_idx);
}
static void
pvh_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx)
{
/*
* If version >= 1 and memmap_paddr != 0, use the memory map provided
* in the start_info structure; if not, we're running under legacy
* Xen and need to use the Xen hypercall.
*/
if ((start_info->version >= 1) && (start_info->memmap_paddr != 0))
pvh_parse_memmap_start_info(kmdp, physmap, physmap_idx);
else
xen_pvh_parse_memmap(kmdp, physmap, physmap_idx);
roygerUnsubmitted
Not Done
Inline Actions

Might be worth adding a KASSERT that we only get here when isxen() == true.

royger: Might be worth adding a KASSERT that we only get here when isxen() == true.
} }