[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 1/9] xen: move do_vcpu_op() to arch specific code



On 27.06.22 13:35, Roger Pau Monné wrote:
On Mon, Jun 27, 2022 at 01:08:11PM +0200, Juergen Gross wrote:
On 27.06.22 13:02, Roger Pau Monné wrote:
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.

I'd be fine with that.

I did it in V2 of the (then secret) series, and Jan replied:

   I'm afraid I don't agree with this movement. I could see things getting
   moved that are purely PV (on the assumption that no new PV ports will
   appear), but anything that's also usable by PVH / HVM ought to be usable
   in principle also by Arm or other PV-free ports.

I see. My opinion is that when other ports need those functions they
should be pulled out of arch code into common code, until then it just
adds confusion to have them in common code.

I will take a look at the current patch, as we need to make progress
on this.

Thanks for that.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.