xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Doug Goldstein <cardoe@cardoe.com>
To: Wei Liu <wei.liu2@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Jan Beulich <jbeulich@suse.com>
Subject: Re: [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well
Date: Wed, 15 Jun 2016 09:39:24 -0500	[thread overview]
Message-ID: <146f27e5-c8d1-b727-a40c-8f17fd873209@cardoe.com> (raw)
In-Reply-To: <1466001104-22240-3-git-send-email-wei.liu2@citrix.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 6329 bytes --]

On 6/15/16 9:31 AM, 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.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
>  docs/misc/xen-command-line.markdown |  8 ++++++--
>  xen/arch/x86/hvm/hvm.c              | 31 ++++++++++++++++++++++++++++---
>  xen/common/kernel.c                 |  6 ++++--
>  xen/include/asm-x86/hvm/hvm.h       |  4 ----
>  xen/include/xen/lib.h               |  1 +
>  5 files changed, 39 insertions(+), 11 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/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 78db903..5bafaef 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,11 +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
>  /* 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
>  
>  /* Xen command-line option to enable altp2m */
>  static bool_t __initdata opt_altp2m_enabled = 0;
> @@ -182,6 +181,32 @@ static int __init hvm_enable(void)
>      if ( !opt_altp2m_enabled )
>          hvm_funcs.altp2m_supported = 0;
>  
> +    if ( opt_hvm_fep )
> +    {
> +        unsigned i, j;
> +
> +        printk("**********************************************\n");
> +        printk("******* WARNING: HVM FORCED EMULATION PREFIX IS PERMITTED\n");
> +        printk("******* This option is *ONLY* intended to aid debugging "
> +               "and testing of Xen\n");
> +        printk("******* that HVM guest can enter instruction emulator "
> +               "with UD instruction.\n");
> +        printk("******* It has implication on the security of the system.\n");
> +        printk("******* Please *DO NOT* use this in production.\n");
> +        printk("**********************************************\n");
> +        add_taint(TAINT_HVM_FEP);
> +        for ( i = 0; i < 3; i++ )
> +        {
> +            printk("%d... ", 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).
> 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..217112d 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -27,12 +27,8 @@
>  #include <public/hvm/save.h>
>  #include <xen/mm.h>
>  
> -#ifndef NDEBUG
>  /* Permit use of the Forced Emulation Prefix in HVM guests */
>  extern bool_t opt_hvm_fep;
> -#else
> -#define opt_hvm_fep 0
> -#endif

Please instead add this as a Kconfig option and you can default it to
enabled.

>  
>  /* Interrupt acknowledgement sources. */
>  enum hvm_intsrc {
> 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);
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

  reply	other threads:[~2016-06-15 14:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-15 14:31 [PATCH RFC 0/2] Make hvm_fep available to non-debug builds Wei Liu
2016-06-15 14:31 ` [PATCH RFC 1/2] xen/kernel: document 'C' in print_tainted Wei Liu
2016-06-15 14:53   ` Jan Beulich
2016-06-15 14:31 ` [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well Wei Liu
2016-06-15 14:39   ` Doug Goldstein [this message]
2016-06-15 14:47     ` Wei Liu
2016-06-15 16:12       ` Doug Goldstein
2016-06-15 16:14         ` Wei Liu
2016-06-16 11:37   ` Jan Beulich
2016-06-16 11:52     ` Wei Liu
2016-06-16 12:12       ` Andrew Cooper
2016-06-16 12:20         ` Wei Liu
2016-06-16 12:31       ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=146f27e5-c8d1-b727-a40c-8f17fd873209@cardoe.com \
    --to=cardoe@cardoe.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).