On 2017-12-13 07:22, Devshatwar, Nikhil wrote:
> Hello all,
>
> I am profiling the impact of reloading a cell on the root cell.
> My observation is that the hypercall for cell_load and cell_start ends up
> suspending the root cell while the operation is being performed.
>
> This causes delays in the execution of a real time application running in the
> root cell.
> I can understand that the cell_suspend might be necessary when creating or
> destroying cells because it affects resources being used by root cell.
> But the loading and starting of other non-root cell can happen with root cell
> running in parallel.
>
> Was this decision taken to avoid making Jailhouse re-entrant?
> If so, we can hold some locks in the hypercall to avoid that
>
> For now, I tried[1 HACK patch] to remove the cell_suspend and cell_resume and
> it seems to work well.
> The correct way would be to suspend only when creating or destroying cells.
> Let me know if there will be any repercussions of this change?
>
There are two reasons why we stop the root cell on load and start
operations (you need both for a cell reload):
- primitive lock to protect the management API against reentry
- need for atomic modification of the 2nd-stage page table
(we map in/out the loadable non-root cell memory regions to/from the
root cell)
While the former might be resolvable with a hypervisor internal lock +
usage counter, the latter is more complex. I'm not saying it's
impossible, but the result will require more code and more review to
ensure the correct behavior.
>
>
> diff --git a/hypervisor/control.c b/hypervisor/control.c
> index 72d3c7e..a25da91 100644
> --- a/hypervisor/control.c
> +++ b/hypervisor/control.c
> @@ -514,26 +514,21 @@ static int cell_management_prologue(enum
> management_task task,
> if (cpu_data->cell != &root_cell)
> return -EPERM;
>
> - cell_suspend(&root_cell, cpu_data);
> -
> for_each_cell(*cell_ptr)
> if ((*cell_ptr)->config->id == id)
> break;
>
> if (!*cell_ptr) {
> - cell_resume(cpu_data);
> return -ENOENT;
> }
>
> /* root cell cannot be managed */
> if (*cell_ptr == &root_cell) {
> - cell_resume(cpu_data);
> return -EINVAL;
> }
>
> if ((task == CELL_DESTROY && !cell_reconfig_ok(*cell_ptr)) ||
> !cell_shutdown_ok(*cell_ptr)) {
> - cell_resume(cpu_data);
> return -EPERM;
> }
>
> @@ -582,8 +577,6 @@ static int cell_start(struct per_cpu *cpu_data, unsigned
> long id)
> printk("Started cell \"%s\"\n", cell->config->name);
>
> out_resume:
> - cell_resume(cpu_data);
> -
> return err;
> }
Oh, and this patch is definitely broken as it also affects cell
destruction and - indirectly - creation.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP 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.
