On Mon, Jun 27, 2022 at 12:40:41PM +0200, Juergen Gross wrote: > On 27.06.22 12:28, Roger Pau Monné wrote: > > On Thu, Mar 24, 2022 at 03:01:31PM +0100, Juergen Gross wrote: > > > The entry point used for the vcpu_op hypercall on Arm is different > > > from the one on x86 today, as some of the common sub-ops are not > > > supported on Arm. The Arm specific handler filters out the not > > > supported sub-ops and then calls the common handler. This leads to the > > > weird call hierarchy: > > > > > > do_arm_vcpu_op() > > > do_vcpu_op() > > > arch_do_vcpu_op() > > > > > > Clean this up by renaming do_vcpu_op() to common_vcpu_op() and > > > arch_do_vcpu_op() in each architecture to do_vcpu_op(). This way one > > > of above calls can be avoided without restricting any potential > > > future use of common sub-ops for Arm. > > > > Wouldn't it be more natural to have do_vcpu_op() contain the common > > code (AFAICT handlers for > > VCPUOP_register_{vcpu_info,runstate_memory_area}) and then everything > > else handled by the x86 arch_do_vcpu_op() handler? > > > > I find the common prefix misleading, as not all the VCPUOP hypercalls > > are available to all the architectures. > > This would end up in Arm suddenly supporting the sub-ops it doesn't > (want to) support today. Otherwise it would make no sense that Arm has > a dedicated entry for this hypercall.
My preference would be for a common do_vcpu_op() that just contains handlers for VCPUOP_register_{vcpu_info,runstate_memory_area} and then an empty arch_ handler for Arm, and everything else moved to the x86 arch_ handler. That obviously implies some code churn, but results in a cleaner implementation IMO. Also has the nice benefit of removing unreachable code from the Arm build, which is also a MISRA-C rule. > The "common" just wants to express that the code is common. I'm open > for a better suggestion, though. :-) Right, it lives in common/ anyway, so there's not a much better name. Thanks, Roger.