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). Thanks, Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux -- 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.
