On 05/23/2018 02:08 PM, Jan Kiszka wrote: > On 2018-05-23 12:27, Ralf Ramsauer wrote: >> The communication region is RW for both, the inmate and the hypervisor. >> As a consequence, an inmate may overwrite the whole region. Currently, >> the region is filled in cell_create. >> >> Move initialisation to cell_start to provide a proper initial state when >> starting the cell. Move architecture specific initialisation to >> arch_cell_reset, which will be called by cell_start. >> >> Leave the cell's shut down state in cell_create to maintain a valid >> state after cell creation. >> >> Signed-off-by: Ralf Ramsauer <[email protected]> >> --- >> since v1: >> - assume that all fields are zero initialised, and save some >> assignements. Leave comments. >> >> hypervisor/arch/x86/control.c | 23 ++++++++++++----------- >> hypervisor/control.c | 23 ++++++++++++++--------- >> 2 files changed, 26 insertions(+), 20 deletions(-) >> >> diff --git a/hypervisor/arch/x86/control.c b/hypervisor/arch/x86/control.c >> index e51cf52c..a13ea711 100644 >> --- a/hypervisor/arch/x86/control.c >> +++ b/hypervisor/arch/x86/control.c >> @@ -32,7 +32,6 @@ struct exception_frame { >> >> int arch_cell_create(struct cell *cell) >> { >> - struct jailhouse_comm_region *comm_region = >> &cell->comm_page.comm_region; >> unsigned int cpu; >> int err; >> >> @@ -40,16 +39,6 @@ int arch_cell_create(struct cell *cell) >> if (err) >> return err; >> >> - comm_region->pm_timer_address = >> - system_config->platform_info.x86.pm_timer_address; >> - comm_region->pci_mmconfig_base = >> - system_config->platform_info.pci_mmconfig_base; >> - comm_region->num_cpus = 0; >> - for_each_cpu(cpu, cell->cpu_set) >> - comm_region->num_cpus++; >> - comm_region->tsc_khz = system_config->platform_info.x86.tsc_khz; >> - comm_region->apic_khz = system_config->platform_info.x86.apic_khz; >> - >> return 0; >> } >> >> @@ -103,6 +92,18 @@ void arch_cell_destroy(struct cell *cell) >> >> void arch_cell_reset(struct cell *cell) >> { >> + struct jailhouse_comm_region *comm_region = >> &cell->comm_page.comm_region; >> + >> + comm_region->pm_timer_address = >> + system_config->platform_info.x86.pm_timer_address; >> + comm_region->pci_mmconfig_base = >> + system_config->platform_info.pci_mmconfig_base; >> + /* comm_region, and hence num_cpus, is zero-initialised */ >> + for_each_cpu(cpu, cell->cpu_set) >> + comm_region->num_cpus++; >> + comm_region->tsc_khz = system_config->platform_info.x86.tsc_khz; >> + comm_region->apic_khz = system_config->platform_info.x86.apic_khz; >> + >> ioapic_cell_reset(cell); >> } >> >> diff --git a/hypervisor/control.c b/hypervisor/control.c >> index 1025852f..9bb1c865 100644 >> --- a/hypervisor/control.c >> +++ b/hypervisor/control.c >> @@ -341,7 +341,6 @@ static int cell_create(struct per_cpu *cpu_data, >> unsigned long config_address) >> { >> unsigned long cfg_page_offs = config_address & ~PAGE_MASK; >> unsigned int cfg_pages, cell_pages, cpu, n; >> - struct jailhouse_comm_region *comm_region; >> const struct jailhouse_memory *mem; >> struct jailhouse_cell_desc *cfg; >> unsigned long cfg_total_size; >> @@ -476,11 +475,7 @@ static int cell_create(struct per_cpu *cpu_data, >> unsigned long config_address) >> >> config_commit(cell); >> >> - comm_region = &cell->comm_page.comm_region; >> - memcpy(comm_region->signature, COMM_REGION_MAGIC, >> - sizeof(comm_region->signature)); >> - comm_region->revision = COMM_REGION_ABI_REVISION; >> - comm_region->cell_state = JAILHOUSE_CELL_SHUT_DOWN; >> + cell->comm_page.comm_region.cell_state = JAILHOUSE_CELL_SHUT_DOWN; >> >> last = &root_cell; >> while (last->next) >> @@ -558,6 +553,7 @@ static int cell_management_prologue(enum management_task >> task, >> >> static int cell_start(struct per_cpu *cpu_data, unsigned long id) >> { >> + struct jailhouse_comm_region *comm_region; >> const struct jailhouse_memory *mem; >> unsigned int cpu, n; >> struct cell *cell; >> @@ -581,9 +577,18 @@ static int cell_start(struct per_cpu *cpu_data, >> unsigned long id) >> cell->loadable = false; >> } >> >> - /* present a consistent Communication Region state to the cell */ >> - cell->comm_page.comm_region.cell_state = JAILHOUSE_CELL_RUNNING; >> - cell->comm_page.comm_region.msg_to_cell = JAILHOUSE_MSG_NONE; >> + /* Present a consistent Communication Region state to the cell. Zero the >> + * whole region,+ as it might be dirty. This implies: >> + * - cell_state = JAILHOUSE_CELL_RUNNING (0) >> + * - msg_to_cell = JAILHOUSE_MSG_NONE (0) >> + */ >> + comm_region = &cell->comm_page.comm_region; >> + memset(&cell->comm_page, 0, sizeof(cell->comm_page)); >> + >> + comm_region->revision = COMM_REGION_ABI_REVISION; >> + memcpy(comm_region->signature, COMM_REGION_MAGIC, >> + sizeof(comm_region->signature)); >> + >> >> pci_cell_reset(cell); >> arch_cell_reset(cell); >> > > Applied to next, with some some massaging (cpu variable had to be moved > as well, comment in hunk above adjusted).
Hmm. Must have happened while rebasing, I hit this when compile-time testing it yesterday. Anyway, thanks. Ralf > > Thanks, > Jan > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
