On 2018-05-22 22:16, 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]>
> ---
> 
> NOTE: only tested on ARM, compile-time tested on x86. Currently lacking
>       an x86 target.
> 
>  hypervisor/arch/x86/control.c | 25 +++++++++++++------------
>  hypervisor/control.c          | 20 ++++++++++++--------
>  2 files changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/hypervisor/arch/x86/control.c b/hypervisor/arch/x86/control.c
> index e51cf52c..4a6776ce 100644
> --- a/hypervisor/arch/x86/control.c
> +++ b/hypervisor/arch/x86/control.c
> @@ -32,24 +32,12 @@ 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;
>  
>       err = vcpu_cell_init(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 +91,19 @@ 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;
> +     unsigned int cpu;
> +
> +     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;

/* comm_region is zero-initialized */

> +     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..78a7c221 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;
> @@ -582,8 +578,16 @@ static int cell_start(struct per_cpu *cpu_data, unsigned 
> long id)
>       }
>  
>       /* 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;
> +     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));
> +
> +     comm_region->cell_state = JAILHOUSE_CELL_RUNNING;
> +     comm_region->msg_to_cell = JAILHOUSE_MSG_NONE;

With the memset, these two can become comments:

/*
 * Includes:
 *   cell_state = JAILHOUSE_CELL_RUNNING (0)
 *   msg_to_cell = JAILHOUSE_MSG_NONE (0)
 */
memset(...);

Jan

>  
>       pci_cell_reset(cell);
>       arch_cell_reset(cell);
> 

-- 
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.

Reply via email to