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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers





On 06/07/2016 20:23, Tamas K Lengyel wrote:
On Wed, Jul 6, 2016 at 1:43 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
On 05.07.16 at 20:37, <tamas.lengyel@xxxxxxxxxxxx> wrote:
+struct vm_event_regs_arm64 {
+    uint64_t x0;
+    uint64_t x1;
+    uint64_t x2;
+    uint64_t x3;
+    uint64_t x4;
+    uint64_t x5;
+    uint64_t x6;
+    uint64_t x7;
+    uint64_t x8;
+    uint64_t x9;
+    uint64_t x10;
+    uint64_t x11;
+    uint64_t x12;
+    uint64_t x13;
+    uint64_t x14;
+    uint64_t x15;
+    uint64_t x16;
+    uint64_t x17;
+    uint64_t x18;
+    uint64_t x19;
+    uint64_t x20;
+    uint64_t x21;
+    uint64_t x22;
+    uint64_t x23;
+    uint64_t x24;
+    uint64_t x25;
+    uint64_t x26;
+    uint64_t x27;
+    uint64_t x28;
+    uint64_t x29;
+    uint64_t x30;
+    uint64_t pc;
+};

Isn't the stack pointer a fully separate register in aarch64? Not
making available something as essential as that seems wrong to
me.


The register is available for access already, so unless there is an
actual use-case that requires it to be transmitted through vm_event I
don't see the point for transmitting it. So as I mentioned in my other
response, I'm inclined to reduce this patch to the bare essentials my
use-case requires at this point and leave the extensions up for the
future when - and if - it will be of use. Since this patch is just an
optimization, if transmitting such reduced set is not acceptable for
some reason, I'll forgo this patch entirely.

Here we go with again the same argument: "this is not necessary for my use-case". This data structure is part of the ABI between the hypervisor and the vm-event app, i.e modifying this structure for adding ARM64/ARM registers will result to an incompatibility with a previous version of the hypervisor. Better to do it now than in a couple of years when vm-event will have more users. I agree that it is time consuming to get an ABI correct, but it will save users to recompile/ship another version of their vm-event app because of this incompatibility.

As mentioned in a previous thread, the main use-case for trapping an SMC is emulating the call, hence a vm-event app would want to have access to the general-purpose registers. And yes, I know that your use-case is different and does not require those registers, I already expressed my concerns about it.

Now, if you drop this patch, how will you retrieve the vCPU register? I guess via DOMCTL_getvcpucontext? If so, if the vCPU has not been paused, you will get a context which is different compare to the time the vm-event has occurred. And yes, I know that in your use-case the vCPU is paused. This call will always be more expensive than passing the registers with event.

Anyway, I really don't think we ask for something really difficult to accomplish.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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