Message ID | 1466161540-2159-3-git-send-email-wei.liu2@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> 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
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 >
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
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
>>> 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
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 >
>>> 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
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 >
>>> 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
>>> 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
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 >
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 >
>>> 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
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 >
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);
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(-)