[PATCH v2 0/2] Make hvm_fep available to non-debug build

[PATCH v2 0/2] Make hvm_fep available to non-debug build

[Wei Liu]

Wei Liu (2):
  xen/kernel: document 'C' in print_tainted
  xen: make available hvm_fep to non-debug build as well

 docs/misc/xen-command-line.markdown |  8 ++++++--
 xen/arch/x86/Kconfig                | 14 ++++++++++++++
 xen/arch/x86/hvm/hvm.c              | 28 ++++++++++++++++++++++++++--
 xen/common/kernel.c                 |  7 +++++--
 xen/include/asm-x86/hvm/hvm.h       |  2 +-
 xen/include/xen/lib.h               |  1 +
 6 files changed, 53 insertions(+), 7 deletions(-)

-- 
2.1.4


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

[PATCH v2 1/2] xen/kernel: document 'C' in print_tainted

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/common/kernel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 1a6823a..dae7e35 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -174,6 +174,7 @@ int __init parse_bool(const char *s)
  *  'S' - SMP with CPUs not designed for SMP.
  *  'M' - Machine had a machine check experience.
  *  'B' - System has hit bad_page.
+ *  'C' - Console output is synchronous.
  *
  *      The string is overwritten by the next call to print_taint().
  */
-- 
2.1.4


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

[PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

Originally hvm_fep was guarded by NDEBUG, which means it was only
available to debug builds.

However there is value to have it for non-debug builds as well. User can
use that to run tests in setup that replicates production setup.

Make it clear with a sync_console style warning that this option can't
be used in production setup. Update command line documentation
accordingly. Finally mark Xen as tainted when this option is enabled.

Add a kconfig option under x86 to configure hvm_fep.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Doug Goldstein <cardoe@cardoe.com>

v2:
1. unsigned -> unsigned int
2. %d -> %u
3. Add spaces around "-"
4. Update warning message
5. Only taint hv when fep is used
6. Add kconfig option
---
 docs/misc/xen-command-line.markdown |  8 ++++++--
 xen/arch/x86/Kconfig                | 14 ++++++++++++++
 xen/arch/x86/hvm/hvm.c              | 28 ++++++++++++++++++++++++++--
 xen/common/kernel.c                 |  6 ++++--
 xen/include/asm-x86/hvm/hvm.h       |  2 +-
 xen/include/xen/lib.h               |  1 +
 6 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index fed732c..dc53e24 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -878,8 +878,12 @@ Recognized in debug builds of the hypervisor only.
 Allow use of the Forced Emulation Prefix in HVM guests, to allow emulation of
 arbitrary instructions.
 
-This option is intended for development purposes, and is only available in
-debug builds of the hypervisor.
+This option is intended for development and testing purposes.
+
+*Warning*
+As this feature opens up the instruction emulator to HVM guest, don't
+use this in production system. No security support is provided when
+this flag is set.
 
 ### hvm\_port80
 > `= <boolean>`
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 73f79cc..5e3b04a 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -59,6 +59,20 @@ config BIGMEM
 
 	  If unsure, say N.
 
+config HVM_FEP
+	bool "HVM Forced Emulation Prefix support"
+	default y
+	---help---
+
+	  Compiles in a feature that allows HVM guest to enter
+	  instruction emulator with forced emulation prefix.
+
+	  This feature can only be enabled during boot time with
+	  appropriate hypervisor command line option. Please read
+	  hypervisor command line documentation before trying to use
+	  this feature.
+
+	  If unsure, say Y.
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 78db903..373b78e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -37,6 +37,7 @@
 #include <xen/mem_access.h>
 #include <xen/rangeset.h>
 #include <xen/vm_event.h>
+#include <xen/delay.h>
 #include <asm/shadow.h>
 #include <asm/hap.h>
 #include <asm/current.h>
@@ -95,9 +96,9 @@ unsigned long __section(".bss.page_aligned")
 static bool_t __initdata opt_hap_enabled = 1;
 boolean_param("hap", opt_hap_enabled);
 
-#ifndef opt_hvm_fep
+#if CONFIG_HVM_FEP
 /* Permit use of the Forced Emulation Prefix in HVM guests */
-bool_t opt_hvm_fep;
+bool_t __read_mostly opt_hvm_fep;
 boolean_param("hvm_fep", opt_hvm_fep);
 #endif
 
@@ -182,6 +183,28 @@ static int __init hvm_enable(void)
     if ( !opt_altp2m_enabled )
         hvm_funcs.altp2m_supported = 0;
 
+    if ( opt_hvm_fep )
+    {
+        unsigned int i, j;
+
+        printk("**********************************************\n");
+        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS AVAILABLE\n");
+        printk("******* This option is *ONLY* intended to aid testing of Xen.\n");
+        printk("******* It has implications on the security of the system.\n");
+        printk("******* Please *DO NOT* use this in production.\n");
+        printk("**********************************************\n");
+        for ( i = 0; i < 3; i++ )
+        {
+            printk("%u... ", 3 - i);
+            for ( j = 0; j < 100; j++ )
+            {
+                process_pending_softirqs();
+                mdelay(10);
+            }
+        }
+        printk("\n");
+    }
+
     /*
      * Allow direct access to the PC debug ports 0x80 and 0xed (they are
      * often used for I/O delays, but the vmexits simply slow things down).
@@ -3905,6 +3928,7 @@ void hvm_ud_intercept(struct cpu_user_regs *regs)
         {
             regs->eip += sizeof(sig);
             regs->eflags &= ~X86_EFLAGS_RF;
+            add_taint(TAINT_HVM_FEP);
         }
     }
 
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index dae7e35..5bf77aa 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -175,6 +175,7 @@ int __init parse_bool(const char *s)
  *  'M' - Machine had a machine check experience.
  *  'B' - System has hit bad_page.
  *  'C' - Console output is synchronous.
+ *  'H' - HVM forced emulation prefix is permitted.
  *
  *      The string is overwritten by the next call to print_taint().
  */
@@ -182,11 +183,12 @@ char *print_tainted(char *str)
 {
     if ( tainted )
     {
-        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c",
+        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c",
                  tainted & TAINT_UNSAFE_SMP ? 'S' : ' ',
                  tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
                  tainted & TAINT_BAD_PAGE ? 'B' : ' ',
-                 tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ');
+                 tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
+                 tainted & TAINT_HVM_FEP ? 'H' : ' ');
     }
     else
     {
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index f486ee9..d2e0ae5 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -27,7 +27,7 @@
 #include <public/hvm/save.h>
 #include <xen/mm.h>
 
-#ifndef NDEBUG
+#if CONFIG_HVM_FEP
 /* Permit use of the Forced Emulation Prefix in HVM guests */
 extern bool_t opt_hvm_fep;
 #else
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 1c652bb..b1b0fb2 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -142,6 +142,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
 #define TAINT_BAD_PAGE                  (1<<2)
 #define TAINT_SYNC_CONSOLE              (1<<3)
 #define TAINT_ERROR_INJECT              (1<<4)
+#define TAINT_HVM_FEP                   (1<<5)
 extern int tainted;
 #define TAINT_STRING_MAX_LEN            20
 extern char *print_tainted(char *str);
-- 
2.1.4


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -59,6 +59,20 @@ config BIGMEM
>  
>  	  If unsure, say N.
>  
> +config HVM_FEP
> +	bool "HVM Forced Emulation Prefix support"

And no "if EXPERT"?

> @@ -95,9 +96,9 @@ unsigned long __section(".bss.page_aligned")
>  static bool_t __initdata opt_hap_enabled = 1;
>  boolean_param("hap", opt_hap_enabled);
>  
> -#ifndef opt_hvm_fep
> +#if CONFIG_HVM_FEP

#ifdef please. And I think this would better be left alone anyway
(and then the comment only applies to the other instance).

Jan


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Fri, Jun 17, 2016 at 05:34:07AM -0600, Jan Beulich wrote:
> >>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
> > --- a/xen/arch/x86/Kconfig
> > +++ b/xen/arch/x86/Kconfig
> > @@ -59,6 +59,20 @@ config BIGMEM
> >  
> >  	  If unsure, say N.
> >  
> > +config HVM_FEP
> > +	bool "HVM Forced Emulation Prefix support"
> 
> And no "if EXPERT"?
> 

Done.

> > @@ -95,9 +96,9 @@ unsigned long __section(".bss.page_aligned")
> >  static bool_t __initdata opt_hap_enabled = 1;
> >  boolean_param("hap", opt_hap_enabled);
> >  
> > -#ifndef opt_hvm_fep
> > +#if CONFIG_HVM_FEP
> 
> #ifdef please. And I think this would better be left alone anyway
> (and then the comment only applies to the other instance).
> 

Change this instance back to what is was and fix the other.

Wei.

> Jan
> 

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Andrew Cooper]

On 17/06/16 12:34, Jan Beulich wrote:
>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -59,6 +59,20 @@ config BIGMEM
>>  
>>  	  If unsure, say N.
>>  
>> +config HVM_FEP
>> +	bool "HVM Forced Emulation Prefix support"
> And no "if EXPERT"?

Is that wise? Does that mean we get a default on option which can't be
selected in menuconfig?

~Andrew

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 17.06.16 at 15:51, <andrew.cooper3@citrix.com> wrote:
> On 17/06/16 12:34, Jan Beulich wrote:
>>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
>>> --- a/xen/arch/x86/Kconfig
>>> +++ b/xen/arch/x86/Kconfig
>>> @@ -59,6 +59,20 @@ config BIGMEM
>>>  
>>>  	  If unsure, say N.
>>>  
>>> +config HVM_FEP
>>> +	bool "HVM Forced Emulation Prefix support"
>> And no "if EXPERT"?
> 
> Is that wise? Does that mean we get a default on option which can't be
> selected in menuconfig?

Oh, that's true. The default should be off in any event.

Jan


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Fri, Jun 17, 2016 at 08:28:34AM -0600, Jan Beulich wrote:
> >>> On 17.06.16 at 15:51, <andrew.cooper3@citrix.com> wrote:
> > On 17/06/16 12:34, Jan Beulich wrote:
> >>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
> >>> --- a/xen/arch/x86/Kconfig
> >>> +++ b/xen/arch/x86/Kconfig
> >>> @@ -59,6 +59,20 @@ config BIGMEM
> >>>  
> >>>  	  If unsure, say N.
> >>>  
> >>> +config HVM_FEP
> >>> +	bool "HVM Forced Emulation Prefix support"
> >> And no "if EXPERT"?
> > 
> > Is that wise? Does that mean we get a default on option which can't be
> > selected in menuconfig?
> 
> Oh, that's true. The default should be off in any event.
> 

So it depends on config EXPERT, defaults to off.

Fine by me in any event, just need a final decision and I will make the
change.

Wei.

> Jan
> 

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 17.06.16 at 16:32, <wei.liu2@citrix.com> wrote:
> On Fri, Jun 17, 2016 at 08:28:34AM -0600, Jan Beulich wrote:
>> >>> On 17.06.16 at 15:51, <andrew.cooper3@citrix.com> wrote:
>> > On 17/06/16 12:34, Jan Beulich wrote:
>> >>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
>> >>> --- a/xen/arch/x86/Kconfig
>> >>> +++ b/xen/arch/x86/Kconfig
>> >>> @@ -59,6 +59,20 @@ config BIGMEM
>> >>>  
>> >>>  	  If unsure, say N.
>> >>>  
>> >>> +config HVM_FEP
>> >>> +	bool "HVM Forced Emulation Prefix support"
>> >> And no "if EXPERT"?
>> > 
>> > Is that wise? Does that mean we get a default on option which can't be
>> > selected in menuconfig?
>> 
>> Oh, that's true. The default should be off in any event.
>> 
> 
> So it depends on config EXPERT, defaults to off.
> 
> Fine by me in any event, just need a final decision and I will make the
> change.

One more adjustment: I guess it should default to DEBUG, to retain
current behavior.

Jan


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Fri, Jun 17, 2016 at 08:40:40AM -0600, Jan Beulich wrote:
> >>> On 17.06.16 at 16:32, <wei.liu2@citrix.com> wrote:
> > On Fri, Jun 17, 2016 at 08:28:34AM -0600, Jan Beulich wrote:
> >> >>> On 17.06.16 at 15:51, <andrew.cooper3@citrix.com> wrote:
> >> > On 17/06/16 12:34, Jan Beulich wrote:
> >> >>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
> >> >>> --- a/xen/arch/x86/Kconfig
> >> >>> +++ b/xen/arch/x86/Kconfig
> >> >>> @@ -59,6 +59,20 @@ config BIGMEM
> >> >>>  
> >> >>>  	  If unsure, say N.
> >> >>>  
> >> >>> +config HVM_FEP
> >> >>> +	bool "HVM Forced Emulation Prefix support"
> >> >> And no "if EXPERT"?
> >> > 
> >> > Is that wise? Does that mean we get a default on option which can't be
> >> > selected in menuconfig?
> >> 
> >> Oh, that's true. The default should be off in any event.
> >> 
> > 
> > So it depends on config EXPERT, defaults to off.
> > 
> > Fine by me in any event, just need a final decision and I will make the
> > change.
> 
> One more adjustment: I guess it should default to DEBUG, to retain
> current behavior.
> 

I think I wrote this series to make this feature available to non-debug
builds.  I don't really get the rationale behind this request. Did you
change your mind during the last few days?

Wei.

> Jan
> 

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 17.06.16 at 16:44, <wei.liu2@citrix.com> wrote:
> On Fri, Jun 17, 2016 at 08:40:40AM -0600, Jan Beulich wrote:
>> >>> On 17.06.16 at 16:32, <wei.liu2@citrix.com> wrote:
>> > On Fri, Jun 17, 2016 at 08:28:34AM -0600, Jan Beulich wrote:
>> >> >>> On 17.06.16 at 15:51, <andrew.cooper3@citrix.com> wrote:
>> >> > On 17/06/16 12:34, Jan Beulich wrote:
>> >> >>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
>> >> >>> --- a/xen/arch/x86/Kconfig
>> >> >>> +++ b/xen/arch/x86/Kconfig
>> >> >>> @@ -59,6 +59,20 @@ config BIGMEM
>> >> >>>  
>> >> >>>  	  If unsure, say N.
>> >> >>>  
>> >> >>> +config HVM_FEP
>> >> >>> +	bool "HVM Forced Emulation Prefix support"
>> >> >> And no "if EXPERT"?
>> >> > 
>> >> > Is that wise? Does that mean we get a default on option which can't be
>> >> > selected in menuconfig?
>> >> 
>> >> Oh, that's true. The default should be off in any event.
>> >> 
>> > 
>> > So it depends on config EXPERT, defaults to off.
>> > 
>> > Fine by me in any event, just need a final decision and I will make the
>> > change.
>> 
>> One more adjustment: I guess it should default to DEBUG, to retain
>> current behavior.
>> 
> 
> I think I wrote this series to make this feature available to non-debug
> builds.  I don't really get the rationale behind this request. Did you
> change your mind during the last few days?

Making it available doesn't mean enable it by default for everyone.
People wanting it in non-debug builds should be able to get it, but
unaware people shouldn't be caught by surprise. (And btw.,
defaulting it to DEBUG is a relaxation over defaulting it to off.)

Jan


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Fri, Jun 17, 2016 at 08:55:04AM -0600, Jan Beulich wrote:
> >>> On 17.06.16 at 16:44, <wei.liu2@citrix.com> wrote:
> > On Fri, Jun 17, 2016 at 08:40:40AM -0600, Jan Beulich wrote:
> >> >>> On 17.06.16 at 16:32, <wei.liu2@citrix.com> wrote:
> >> > On Fri, Jun 17, 2016 at 08:28:34AM -0600, Jan Beulich wrote:
> >> >> >>> On 17.06.16 at 15:51, <andrew.cooper3@citrix.com> wrote:
> >> >> > On 17/06/16 12:34, Jan Beulich wrote:
> >> >> >>>>> On 17.06.16 at 13:05, <wei.liu2@citrix.com> wrote:
> >> >> >>> --- a/xen/arch/x86/Kconfig
> >> >> >>> +++ b/xen/arch/x86/Kconfig
> >> >> >>> @@ -59,6 +59,20 @@ config BIGMEM
> >> >> >>>  
> >> >> >>>  	  If unsure, say N.
> >> >> >>>  
> >> >> >>> +config HVM_FEP
> >> >> >>> +	bool "HVM Forced Emulation Prefix support"
> >> >> >> And no "if EXPERT"?
> >> >> > 
> >> >> > Is that wise? Does that mean we get a default on option which can't be
> >> >> > selected in menuconfig?
> >> >> 
> >> >> Oh, that's true. The default should be off in any event.
> >> >> 
> >> > 
> >> > So it depends on config EXPERT, defaults to off.
> >> > 
> >> > Fine by me in any event, just need a final decision and I will make the
> >> > change.
> >> 
> >> One more adjustment: I guess it should default to DEBUG, to retain
> >> current behavior.
> >> 
> > 
> > I think I wrote this series to make this feature available to non-debug
> > builds.  I don't really get the rationale behind this request. Did you
> > change your mind during the last few days?
> 
> Making it available doesn't mean enable it by default for everyone.
> People wanting it in non-debug builds should be able to get it, but
> unaware people shouldn't be caught by surprise. (And btw.,
> defaulting it to DEBUG is a relaxation over defaulting it to off.)
> 

I see. I misread as "it should depend on DEBUG". Sorry about that.

I will make the change as requested.

Wei.

> Jan
> 

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Andrew Cooper]

On 17/06/16 12:05, Wei Liu wrote:
> Originally hvm_fep was guarded by NDEBUG, which means it was only
> available to debug builds.
>
> However there is value to have it for non-debug builds as well. User can
> use that to run tests in setup that replicates production setup.
>
> Make it clear with a sync_console style warning that this option can't
> be used in production setup. Update command line documentation
> accordingly. Finally mark Xen as tainted when this option is enabled.
>
> Add a kconfig option under x86 to configure hvm_fep.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Doug Goldstein <cardoe@cardoe.com>
>
> v2:
> 1. unsigned -> unsigned int
> 2. %d -> %u
> 3. Add spaces around "-"
> 4. Update warning message
> 5. Only taint hv when fep is used
> 6. Add kconfig option
> ---
>  docs/misc/xen-command-line.markdown |  8 ++++++--
>  xen/arch/x86/Kconfig                | 14 ++++++++++++++
>  xen/arch/x86/hvm/hvm.c              | 28 ++++++++++++++++++++++++++--
>  xen/common/kernel.c                 |  6 ++++--
>  xen/include/asm-x86/hvm/hvm.h       |  2 +-
>  xen/include/xen/lib.h               |  1 +
>  6 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index fed732c..dc53e24 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -878,8 +878,12 @@ Recognized in debug builds of the hypervisor only.
>  Allow use of the Forced Emulation Prefix in HVM guests, to allow emulation of
>  arbitrary instructions.
>  
> -This option is intended for development purposes, and is only available in
> -debug builds of the hypervisor.
> +This option is intended for development and testing purposes.
> +
> +*Warning*
> +As this feature opens up the instruction emulator to HVM guest, don't

"to arbitrary instruction from an HVM guest,".  It is an important
distinction, because all guest can enter the emulator in other ways as
part of normal operation.

> +use this in production system. No security support is provided when
> +this flag is set.
>  
>  ### hvm\_port80
>  > `= <boolean>`
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 73f79cc..5e3b04a 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -59,6 +59,20 @@ config BIGMEM
>  
>  	  If unsure, say N.
>  
> +config HVM_FEP
> +	bool "HVM Forced Emulation Prefix support"
> +	default y
> +	---help---
> +
> +	  Compiles in a feature that allows HVM guest to enter
> +	  instruction emulator with forced emulation prefix.

"allows HVM guests to arbitrarily exercise the instruction emulator." 
There are other ways in which guests can enter the instruction emulator,
but they are specifically limited in nature.

I would also have a separate paragraph stating:

"This is strictly for testing purposes, and not appropriate for use in
production."

> +
> +	  This feature can only be enabled during boot time with
> +	  appropriate hypervisor command line option. Please read
> +	  hypervisor command line documentation before trying to use
> +	  this feature.
> +
> +	  If unsure, say Y.
>  endmenu
>  
>  source "common/Kconfig"
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 78db903..373b78e 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -37,6 +37,7 @@
>  #include <xen/mem_access.h>
>  #include <xen/rangeset.h>
>  #include <xen/vm_event.h>
> +#include <xen/delay.h>
>  #include <asm/shadow.h>
>  #include <asm/hap.h>
>  #include <asm/current.h>
> @@ -95,9 +96,9 @@ unsigned long __section(".bss.page_aligned")
>  static bool_t __initdata opt_hap_enabled = 1;
>  boolean_param("hap", opt_hap_enabled);
>  
> -#ifndef opt_hvm_fep
> +#if CONFIG_HVM_FEP
>  /* Permit use of the Forced Emulation Prefix in HVM guests */
> -bool_t opt_hvm_fep;
> +bool_t __read_mostly opt_hvm_fep;
>  boolean_param("hvm_fep", opt_hvm_fep);
>  #endif
>  
> @@ -182,6 +183,28 @@ static int __init hvm_enable(void)
>      if ( !opt_altp2m_enabled )
>          hvm_funcs.altp2m_supported = 0;
>  
> +    if ( opt_hvm_fep )
> +    {
> +        unsigned int i, j;
> +
> +        printk("**********************************************\n");
> +        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS AVAILABLE\n");
> +        printk("******* This option is *ONLY* intended to aid testing of Xen.\n");
> +        printk("******* It has implications on the security of the system.\n");
> +        printk("******* Please *DO NOT* use this in production.\n");
> +        printk("**********************************************\n");
> +        for ( i = 0; i < 3; i++ )
> +        {
> +            printk("%u... ", 3 - i);
> +            for ( j = 0; j < 100; j++ )
> +            {
> +                process_pending_softirqs();
> +                mdelay(10);
> +            }
> +        }

I would drop this 3s wait, and I plan to drop it from sync_console as
well.  It isn't of any practical use, even if you are watching the
serial console on boot, and just leads to an unnecessary delay.  Worse,
the two delays are cumulative.

~Andrew

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 17.06.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
> On 17/06/16 12:05, Wei Liu wrote:
>> @@ -182,6 +183,28 @@ static int __init hvm_enable(void)
>>      if ( !opt_altp2m_enabled )
>>          hvm_funcs.altp2m_supported = 0;
>>  
>> +    if ( opt_hvm_fep )
>> +    {
>> +        unsigned int i, j;
>> +
>> +        printk("**********************************************\n");
>> +        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS AVAILABLE\n");
>> +        printk("******* This option is *ONLY* intended to aid testing of Xen.\n");
>> +        printk("******* It has implications on the security of the system.\n");
>> +        printk("******* Please *DO NOT* use this in production.\n");
>> +        printk("**********************************************\n");
>> +        for ( i = 0; i < 3; i++ )
>> +        {
>> +            printk("%u... ", 3 - i);
>> +            for ( j = 0; j < 100; j++ )
>> +            {
>> +                process_pending_softirqs();
>> +                mdelay(10);
>> +            }
>> +        }
> 
> I would drop this 3s wait, and I plan to drop it from sync_console as
> well.  It isn't of any practical use, even if you are watching the
> serial console on boot, and just leads to an unnecessary delay.  Worse,
> the two delays are cumulative.

I think the delay serves a purpose (for the messages to not scroll by
unnoticed), but I do appreciate that having two such delays is not
really desirable. Considering that I would also like these messages to
go into .init.rodata (also those for the sync_console warning), perhaps
worth a little bit of abstraction?

Jan


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Fri, Jun 17, 2016 at 08:45:39AM -0600, Jan Beulich wrote:
> >>> On 17.06.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
> > On 17/06/16 12:05, Wei Liu wrote:
> >> @@ -182,6 +183,28 @@ static int __init hvm_enable(void)
> >>      if ( !opt_altp2m_enabled )
> >>          hvm_funcs.altp2m_supported = 0;
> >>  
> >> +    if ( opt_hvm_fep )
> >> +    {
> >> +        unsigned int i, j;
> >> +
> >> +        printk("**********************************************\n");
> >> +        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS AVAILABLE\n");
> >> +        printk("******* This option is *ONLY* intended to aid testing of Xen.\n");
> >> +        printk("******* It has implications on the security of the system.\n");
> >> +        printk("******* Please *DO NOT* use this in production.\n");
> >> +        printk("**********************************************\n");
> >> +        for ( i = 0; i < 3; i++ )
> >> +        {
> >> +            printk("%u... ", 3 - i);
> >> +            for ( j = 0; j < 100; j++ )
> >> +            {
> >> +                process_pending_softirqs();
> >> +                mdelay(10);
> >> +            }
> >> +        }
> > 
> > I would drop this 3s wait, and I plan to drop it from sync_console as
> > well.  It isn't of any practical use, even if you are watching the
> > serial console on boot, and just leads to an unnecessary delay.  Worse,
> > the two delays are cumulative.
> 
> I think the delay serves a purpose (for the messages to not scroll by
> unnoticed), but I do appreciate that having two such delays is not
> really desirable. Considering that I would also like these messages to
> go into .init.rodata (also those for the sync_console warning), perhaps
> worth a little bit of abstraction?
> 

What is the plan here?

Should I keep or remove the delay? Do you want me to refactor things?

Wei.

> Jan
> 

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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 20.06.16 at 11:09, <wei.liu2@citrix.com> wrote:
> On Fri, Jun 17, 2016 at 08:45:39AM -0600, Jan Beulich wrote:
>> >>> On 17.06.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
>> > On 17/06/16 12:05, Wei Liu wrote:
>> >> @@ -182,6 +183,28 @@ static int __init hvm_enable(void)
>> >>      if ( !opt_altp2m_enabled )
>> >>          hvm_funcs.altp2m_supported = 0;
>> >>  
>> >> +    if ( opt_hvm_fep )
>> >> +    {
>> >> +        unsigned int i, j;
>> >> +
>> >> +        printk("**********************************************\n");
>> >> +        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS 
> AVAILABLE\n");
>> >> +        printk("******* This option is *ONLY* intended to aid testing of 
> Xen.\n");
>> >> +        printk("******* It has implications on the security of the 
> system.\n");
>> >> +        printk("******* Please *DO NOT* use this in production.\n");
>> >> +        printk("**********************************************\n");
>> >> +        for ( i = 0; i < 3; i++ )
>> >> +        {
>> >> +            printk("%u... ", 3 - i);
>> >> +            for ( j = 0; j < 100; j++ )
>> >> +            {
>> >> +                process_pending_softirqs();
>> >> +                mdelay(10);
>> >> +            }
>> >> +        }
>> > 
>> > I would drop this 3s wait, and I plan to drop it from sync_console as
>> > well.  It isn't of any practical use, even if you are watching the
>> > serial console on boot, and just leads to an unnecessary delay.  Worse,
>> > the two delays are cumulative.
>> 
>> I think the delay serves a purpose (for the messages to not scroll by
>> unnoticed), but I do appreciate that having two such delays is not
>> really desirable. Considering that I would also like these messages to
>> go into .init.rodata (also those for the sync_console warning), perhaps
>> worth a little bit of abstraction?
> 
> What is the plan here?

I didn't see a response from Andrew yet; I continue to think some
delay ought to be there.

> Should I keep or remove the delay? Do you want me to refactor things?

If you're up to some refactoring, that'd be appreciated.

Jan


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

Re: [PATCH v2 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Mon, Jun 20, 2016 at 04:11:31AM -0600, Jan Beulich wrote:
> >>> On 20.06.16 at 11:09, <wei.liu2@citrix.com> wrote:
> > On Fri, Jun 17, 2016 at 08:45:39AM -0600, Jan Beulich wrote:
> >> >>> On 17.06.16 at 15:50, <andrew.cooper3@citrix.com> wrote:
> >> > On 17/06/16 12:05, Wei Liu wrote:
> >> >> @@ -182,6 +183,28 @@ static int __init hvm_enable(void)
> >> >>      if ( !opt_altp2m_enabled )
> >> >>          hvm_funcs.altp2m_supported = 0;
> >> >>  
> >> >> +    if ( opt_hvm_fep )
> >> >> +    {
> >> >> +        unsigned int i, j;
> >> >> +
> >> >> +        printk("**********************************************\n");
> >> >> +        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS 
> > AVAILABLE\n");
> >> >> +        printk("******* This option is *ONLY* intended to aid testing of 
> > Xen.\n");
> >> >> +        printk("******* It has implications on the security of the 
> > system.\n");
> >> >> +        printk("******* Please *DO NOT* use this in production.\n");
> >> >> +        printk("**********************************************\n");
> >> >> +        for ( i = 0; i < 3; i++ )
> >> >> +        {
> >> >> +            printk("%u... ", 3 - i);
> >> >> +            for ( j = 0; j < 100; j++ )
> >> >> +            {
> >> >> +                process_pending_softirqs();
> >> >> +                mdelay(10);
> >> >> +            }
> >> >> +        }
> >> > 
> >> > I would drop this 3s wait, and I plan to drop it from sync_console as
> >> > well.  It isn't of any practical use, even if you are watching the
> >> > serial console on boot, and just leads to an unnecessary delay.  Worse,
> >> > the two delays are cumulative.
> >> 
> >> I think the delay serves a purpose (for the messages to not scroll by
> >> unnoticed), but I do appreciate that having two such delays is not
> >> really desirable. Considering that I would also like these messages to
> >> go into .init.rodata (also those for the sync_console warning), perhaps
> >> worth a little bit of abstraction?
> > 
> > What is the plan here?
> 
> I didn't see a response from Andrew yet; I continue to think some
> delay ought to be there.
> 

OK. I will give Andrew a chance to reply.

> > Should I keep or remove the delay? Do you want me to refactor things?
> 
> If you're up to some refactoring, that'd be appreciated.
> 

Sure. I think I can factor out a "warning" infrastructure (just an array
to keep track of all warning texts,  a function to add text to the array
and a function to print out all the added warnings).

Wei.

> Jan
> 

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