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

Re: [Xen-devel] [PATCH] arm/monitor vm-events: Implement guest-request support


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
  • Date: Fri, 19 Feb 2016 18:25:12 +0200
  • Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
  • Comment: DomainKeys? See http://domainkeys.sourceforge.net/
  • Delivery-date: Fri, 19 Feb 2016 16:25:24 +0000
  • Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=bitdefender.com; b=pBrXwTPdy+OU+oPkU6U1tOOT5mrqWGnBn0jkuJar9Aug9nn1lGskzRvTXaYhtKWgr6HJR1z4U1RdJhZOxphvsAoAo6E3Q8fjmp6aMVuV05oZiHRUOKxppgxOhfiIvsozq7u4xXziC8wxbMS7B3HwMDcAty+Hzq+w/hU2bQeb9UkQIPA1uKEdK/O4tLiAMHiaIHwLR6gUKbOCV/zb1+o93zK4yyu1epGGrRfDVIDz/eiySc5M07UMV/u12Fhi9EI5sPzvEnzersPXOEThCIyYseBjwmq89Y4uUTXjSY1fPOcOk6otUR65DYp3pFGshnkbYgwItllDgbbk+2hVa0X8oA==; h=Received:Received:Received:Received:Received:Subject:To:References:Cc:From:Message-ID:Date:User-Agent:MIME-Version:In-Reply-To:Content-Type:Content-Transfer-Encoding:X-BitDefender-Scanner:X-BitDefender-Spam:X-BitDefender-SpamStamp:X-BitDefender-CF-Stamp;
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>

On 2/19/2016 4:26 PM, Jan Beulich wrote:
On 18.02.16 at 20:35, <czuzu@xxxxxxxxxxxxxxx> wrote:
---
  MAINTAINERS                     |   1 +
  xen/arch/arm/hvm.c              |   8 +++
  xen/arch/x86/hvm/event.c        | 116 ++++++----------------------------------
  xen/arch/x86/hvm/hvm.c          |   1 +
  xen/arch/x86/monitor.c          |  14 -----
  xen/arch/x86/vm_event.c         |   1 +
  xen/common/Makefile             |   2 +-
  xen/common/hvm/Makefile         |   3 +-
  xen/common/hvm/event.c          |  96 +++++++++++++++++++++++++++++++++
So here you _again_ try to introduce something HVM-ish for ARM.
Why? Why can't this code live in common/vm_event.c?

Are you referring to "xen/arch/arm/hvm.c"? If so, I did not introduce that file, it was already there, all I did is add handling of HVMOP_guest_request_vm_event to ARM-side's already existing do_hvm_op :). On the "HVM-ish" note, is there some incompatibility between ARM and the concept of HVM?


--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -376,17 +376,15 @@ struct arch_domain
      unsigned long *pirq_eoi_map;
      unsigned long pirq_eoi_map_mfn;
- /* Monitor options */
+    /* Arch-specific monitor options */
      struct {
-        unsigned int write_ctrlreg_enabled       : 4;
-        unsigned int write_ctrlreg_sync          : 4;
-        unsigned int write_ctrlreg_onchangeonly  : 4;
-        unsigned int mov_to_msr_enabled          : 1;
-        unsigned int mov_to_msr_extended         : 1;
-        unsigned int singlestep_enabled          : 1;
-        unsigned int software_breakpoint_enabled : 1;
-        unsigned int guest_request_enabled       : 1;
-        unsigned int guest_request_sync          : 1;
+        uint16_t write_ctrlreg_enabled       : 4;
+        uint16_t write_ctrlreg_sync          : 4;
+        uint16_t write_ctrlreg_onchangeonly  : 4;
+        uint16_t mov_to_msr_enabled          : 1;
+        uint16_t mov_to_msr_extended         : 1;
+        uint16_t singlestep_enabled          : 1;
+        uint16_t software_breakpoint_enabled : 1;
      } monitor;
What is this type change supposed to achieve in general, and in
particular in the context of this patch?

Some time before this patch there was a discussion I had w/ ARM maintainer Ian Campbell who made the suggestion to try and move parts of the monitor vm-events code to the common-side. (you can find that discussion here: https://www.mail-archive.com/xen-devel@xxxxxxxxxxxxx/msg54139.html). In short, the reason for his suggestion was that that previous attempt of mine to add guest-request support for ARM highlighted code duplication between X86 and ARM if I were to leave the monitor vm-events code as it was (that is, completely arch-specific). In an effort to avoid that, I first submitted a 7 patch-series which tried to move as much as possible to the common-side. "As much as possible" meant guest-request, ctrl-reg write, single-step and software breakpoints, all moved to the common-side. That also meant introducing some Kconfigs to selectively activate some parts of that now-common code, since not all of it was used on ARM at that moment. Although conceptually that code could have remained on the common-side, Tamas pointed out that we could get rid of the Kconfigs (which in his opinion bloated the code) by only moving at the moment what is implemented on both X86 and ARM. As for the rest, he noted that when I add implementations for the other monitor vm-events on ARM as well, I could move that to common as well. The above is a result of that - guest-request is implemented on both X86 and ARM with this patch, hence I also moved it to common.

--- a/xen/include/asm-x86/hvm/event.h
+++ b/xen/include/asm-x86/hvm/event.h
@@ -1,5 +1,9 @@
  /*
- * event.h: Hardware virtual machine assist events.
+ * include/asm-x86/hvm/event.h
+ *
+ * Arch-specific hardware virtual machine event abstractions.
+ *
+ * Copyright (c) 2016, Bitdefender S.R.L.
Is this true? Namely, was _all_ of the code here written by people
on your company, including what may have got moved here from
elsewhere?

Jan



My bad. Removing that line would be satisfactory, yes?

Corneliu.

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


 


Rackspace

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