Order args_regs_reset() and vcpu_on() are called between primary vs. secondary VM

[Hong-Seok Kim]

I is subtle so there is a chance that I have read the code incorrectly. Risking that, I would like to ask if the orders that args_regs_reset() and vcpu_on() are called is different between primary vs secondary VMs, and if they are, why they should be different.

I didn't read the code deeply so vaguely understand what args_regs_reset() and vcpu_on() do. My current understanding is as follows.

- arch_regs_reset() is clearing registers to a desired initiali state

- vcpu_on() calls arch_regs_set_pc_arg() then change vcpu's state to READY

By looking at the code, at least between arch_regs_reset() should be called before arch_regs_set_pc_arg(). Thus, the correct order between arch_regs_reset() and vcpu_on() appears arch_regs_reset() -> vcpu_on().

That is certainly the case for secondary VMs. For initializing vcPU registers, secondary VMs use vcpu_secondary_reset(). This function calls arch_regs_reset() and then vcpu_on() if vcpu was off before.

The situation is more complicated for primary VMs. For vCPU[0], vcpu_on() is called at the end of load_primary(). Then, arch_regs_reset() is called before returning from cpu_main(). For other vCPUs, vcpu_on() is called by cpu_on() when processing PSCI_CPU_ON in psci_primary_vm_handlder(). In later in the same code, an smc call is made, asking TrustZone to power on the cpu and start it at cpu_entry. Therefore, the cpu eventually calls args_regs_reset() for its corresponding vCPU at the end of cpu_main().

 

Effectively, for primary VMs, vcpu_on() is called before arch_regs_reset() is called. So, the orderings between primary and secondary VMs are different. I am wondering if (1) I read the code correctly and (2) if so, what's the reasoning behind it.

[Hong-Seok Kim]

This could be related to the other question that I sent in a separate email but also somewhat independent. Any thoughts on this as well?

[Andrew Walbran]

On Fri, 5 Jul 2019 at 14:50, 'Hong-Seok Kim' via Hafnium <hafnium...@googlegroups.com> wrote:

I is subtle so there is a chance that I have read the code incorrectly. Risking that, I would like to ask if the orders that args_regs_reset() and vcpu_on() are called is different between primary vs secondary VMs, and if they are, why they should be different.

I didn't read the code deeply so vaguely understand what args_regs_reset() and vcpu_on() do. My current understanding is as follows.

- arch_regs_reset() is clearing registers to a desired initiali state

- vcpu_on() calls arch_regs_set_pc_arg() then change vcpu's state to READY

By looking at the code, at least between arch_regs_reset() should be called before arch_regs_set_pc_arg(). Thus, the correct order between arch_regs_reset() and vcpu_on() appears arch_regs_reset() -> vcpu_on().

arch_regs_reset resets everything except the program counter and first general-purpose register (i.e. the 'arg'), so the order doesn't actually matter.

 

That is certainly the case for secondary VMs. For initializing vcPU registers, secondary VMs use vcpu_secondary_reset(). This function calls arch_regs_reset() and then vcpu_on() if vcpu was off before.

The situation is more complicated for primary VMs. For vCPU[0], vcpu_on() is called at the end of load_primary(). Then, arch_regs_reset() is called before returning from cpu_main(). For other vCPUs, vcpu_on() is called by cpu_on() when processing PSCI_CPU_ON in psci_primary_vm_handlder(). In later in the same code, an smc call is made, asking TrustZone to power on the cpu and start it at cpu_entry. Therefore, the cpu eventually calls args_regs_reset() for its corresponding vCPU at the end of cpu_main().

 

Effectively, for primary VMs, vcpu_on() is called before arch_regs_reset() is called. So, the orderings between primary and secondary VMs are different. I am wondering if (1) I read the code correctly and (2) if so, what's the reasoning behind it.

I think you are correct. I'm not aware of any particular reason why we chose to do it this way rather than in a different order; I don't think it makes a difference. If you have an idea for how to refactor the code to make it cleaner then send me a CL. I've attached a call graph I put together when I was trying to understand and refactor some of this code for the PSCI implementation a while ago, in case that's helpful to you.

 

[Hong-Seok Kim]

I don't think there is a magic how we could potentially improve the current code structure. Now that we have a pretty solid working version, we just need to carefully review how we initialize VMs, physical CPUs and virtual CPUs. Writing a document is actually a good way to review what the implementation really does and reflect what we really want it to be.

It'd be great if someone from the Hafnium team could volunteer working with the Oak team to document this part in detail and, as a by product, make a proposal on how to improve the structure if possible.