| Message ID | 1453142404-8819-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1/18/16 12:40 PM, Andrew Cooper wrote: > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Doug Goldstein <cardoe@cardoe.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Doug Goldstein <cardoe@cardoe.com> > --- > xen/arch/x86/Kconfig | 14 ++++++++++++++ > xen/arch/x86/Rules.mk | 4 ---- > xen/arch/x86/mm/shadow/Makefile | 2 +- > 3 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig > index 4781b34..9869630 100644 > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -27,6 +27,20 @@ menu "Architecture Features" > > source "arch/Kconfig" > > +config SHADOW_PAGING > + bool "Shadow Paging" > + default y > + ---help--- > + Shadow paging is a software alternative to hardware paging support > + (Intel EPT, AMD NPT) for use with HVM guests. > + > + It is required to run HVM guests for first-generation hardware > + virtualisation (Intel VT-x, AMD SVM) which did not include hardware > + paging support. Under a small number of specific workloads, shadow > + paging may also be deliberately used as a performance improvement. > + > + If unsure, say Y. > + > config BIGMEM > bool "big memory support" > default n > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > index a108d24..a1cdae0 100644 > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -22,13 +22,9 @@ $(call as-insn-check,CFLAGS,CC,".equ \"x\"$$(comma)1", \ > -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \ > '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@') > > -shadow-paging ?= y > - > CFLAGS += -mno-red-zone -mno-sse -fpic > CFLAGS += -fno-asynchronous-unwind-tables > # -fvisibility=hidden reduces -fpic cost, if it's available > ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n) > CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE > endif > - > -CFLAGS-$(shadow-paging) += -DCONFIG_SHADOW_PAGING > diff --git a/xen/arch/x86/mm/shadow/Makefile b/xen/arch/x86/mm/shadow/Makefile > index a07bc0c..df194ad 100644 > --- a/xen/arch/x86/mm/shadow/Makefile > +++ b/xen/arch/x86/mm/shadow/Makefile > @@ -1,4 +1,4 @@ > -ifeq ($(shadow-paging),y) > +ifdef CONFIG_SHADOW_PAGING > obj-y += common.o guest_2.o guest_3.o guest_4.o > else > obj-y += none.o >
On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Does this have any impact on migration of either PV or HVM guests? What about nested virt? Are things which are defined in xen/arch/*/Rules.mk in this way overrideable from the old top-level .config or does one need to dive deeper to modify them? If it's not configurable from top-level .config today then I think it either needs a "depends EXPERT" or for the changelog to make a convincing argument why this should be made user selectable. Lastly, Tim is maintainer of the shadow code and should have been CC-d, also George as maintainer of the mm stuff might have an interest. Both CC-s added. > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Doug Goldstein <cardoe@cardoe.com> > --- > xen/arch/x86/Kconfig | 14 ++++++++++++++ > xen/arch/x86/Rules.mk | 4 ---- > xen/arch/x86/mm/shadow/Makefile | 2 +- > 3 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig > index 4781b34..9869630 100644 > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -27,6 +27,20 @@ menu "Architecture Features" > > source "arch/Kconfig" > > +config SHADOW_PAGING > + bool "Shadow Paging" > + default y > + ---help--- > + Shadow paging is a software alternative to hardware paging > support > + (Intel EPT, AMD NPT) for use with HVM guests. > + > + It is required to run HVM guests for first-generation hardware > + virtualisation (Intel VT-x, AMD SVM) which did not include > hardware > + paging support. Under a small number of specific workloads, > shadow > + paging may also be deliberately used as a performance > improvement. > + > + If unsure, say Y. > + > config BIGMEM > bool "big memory support" > default n > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > index a108d24..a1cdae0 100644 > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -22,13 +22,9 @@ $(call as-insn-check,CFLAGS,CC,".equ \"x\"$$(comma)1", > \ > -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \ > '-D__OBJECT_LABEL__=$(subst > $(BASEDIR)/,,$(CURDIR))/$$@') > > -shadow-paging ?= y > - > CFLAGS += -mno-red-zone -mno-sse -fpic > CFLAGS += -fno-asynchronous-unwind-tables > # -fvisibility=hidden reduces -fpic cost, if it's available > ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n) > CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE > endif > - > -CFLAGS-$(shadow-paging) += -DCONFIG_SHADOW_PAGING > diff --git a/xen/arch/x86/mm/shadow/Makefile > b/xen/arch/x86/mm/shadow/Makefile > index a07bc0c..df194ad 100644 > --- a/xen/arch/x86/mm/shadow/Makefile > +++ b/xen/arch/x86/mm/shadow/Makefile > @@ -1,4 +1,4 @@ > -ifeq ($(shadow-paging),y) > +ifdef CONFIG_SHADOW_PAGING > obj-y += common.o guest_2.o guest_3.o guest_4.o > else > obj-y += none.o
>>> On 19.01.16 at 10:46, <ian.campbell@citrix.com> wrote: > On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Does this have any impact on migration of either PV or HVM guests? What > about nested virt? At least PV guests won't be migratable anymore when there's no shadow mode. > Are things which are defined in xen/arch/*/Rules.mk in this way > overrideable from the old top-level .config or does one need to dive deeper > to modify them? Overriding these in the old top-level .config is how I have been regularly build testing this and the bigmem features since their introduction. Jan
>>> On 18.01.16 at 19:40, <andrew.cooper3@citrix.com> wrote: > --- a/xen/arch/x86/mm/shadow/Makefile > +++ b/xen/arch/x86/mm/shadow/Makefile > @@ -1,4 +1,4 @@ > -ifeq ($(shadow-paging),y) > +ifdef CONFIG_SHADOW_PAGING > obj-y += common.o guest_2.o guest_3.o guest_4.o > else > obj-y += none.o Why didn't you retain the ifeq? The way it's now, accidental definition of the macro elsewhere would alter behavior. In fact I think the better thing here would be to change this to purely list model: obj-y := none.o obj-$(CONFIG_SHADOW_PAGING) := common.o guest_2.o guest_3.o guest_4.o Not sure why I didn't do that right when making this configurable... Jan
On 19/01/16 13:23, Jan Beulich wrote: >>>> On 19.01.16 at 10:46, <ian.campbell@citrix.com> wrote: >> On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Does this have any impact on migration of either PV or HVM guests? What >> about nested virt? > At least PV guests won't be migratable anymore when there's no > shadow mode. What? Shadow mode (or lack thereof) has no impact whatsoever on migration. > >> Are things which are defined in xen/arch/*/Rules.mk in this way >> overrideable from the old top-level .config or does one need to dive deeper >> to modify them? > Overriding these in the old top-level .config is how I have been > regularly build testing this and the bigmem features since their > introduction. This patch matches the bigmem patch in its behaviour. ~Andrew
On 19/01/16 13:27, Jan Beulich wrote: >>>> On 18.01.16 at 19:40, <andrew.cooper3@citrix.com> wrote: >> --- a/xen/arch/x86/mm/shadow/Makefile >> +++ b/xen/arch/x86/mm/shadow/Makefile >> @@ -1,4 +1,4 @@ >> -ifeq ($(shadow-paging),y) >> +ifdef CONFIG_SHADOW_PAGING >> obj-y += common.o guest_2.o guest_3.o guest_4.o >> else >> obj-y += none.o > Why didn't you retain the ifeq? The way it's now, accidental > definition of the macro elsewhere would alter behavior. In fact > I think the better thing here would be to change this to purely > list model: > > obj-y := none.o > obj-$(CONFIG_SHADOW_PAGING) := common.o guest_2.o guest_3.o guest_4.o > > Not sure why I didn't do that right when making this configurable... none.o must not be linked if !CONFIG_SHADOW_PAGING. It was cleanest to retain the existing structure. ~Andrew
>>> On 19.01.16 at 14:30, <andrew.cooper3@citrix.com> wrote: > On 19/01/16 13:23, Jan Beulich wrote: >>>>> On 19.01.16 at 10:46, <ian.campbell@citrix.com> wrote: >>> On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: >>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Does this have any impact on migration of either PV or HVM guests? What >>> about nested virt? >> At least PV guests won't be migratable anymore when there's no >> shadow mode. > > What? Shadow mode (or lack thereof) has no impact whatsoever on migration. So how would log-dirty mode work for a PV guest without shadow code? Jan
On 19/01/16 09:46, Ian Campbell wrote: > On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Does this have any impact on migration of either PV or HVM guests? Not in the slightest. > What about nested virt? Nested virt is completely broken with respect to migration. At no point has anyone considered wiring architectural nesting state into the migration stream. Currently, the VM resumes on the far side with all of its nested setup missing. > > Are things which are defined in xen/arch/*/Rules.mk in this way > overrideable from the old top-level .config or does one need to dive deeper > to modify them? Absolutely everything anywhere in the build system is/was configurable from the top .config > If it's not configurable from top-level .config today then > I think it either needs a "depends EXPERT" or for the changelog to make a > convincing argument why this should be made user selectable. This patch matches the bigmem patch which has already been accepted (c/s 165f36d). These two options should match, whatever the outcome of this discussion is. > Lastly, Tim is maintainer of the shadow code and should have been CC-d, > also George as maintainer of the mm stuff might have an interest. Both CC-s > added. Oops yes, although this is an entirely mechanical alteration to an option which already existed. ~Andrew
>>> On 19.01.16 at 14:33, <andrew.cooper3@citrix.com> wrote: > On 19/01/16 13:27, Jan Beulich wrote: >>>>> On 18.01.16 at 19:40, <andrew.cooper3@citrix.com> wrote: >>> --- a/xen/arch/x86/mm/shadow/Makefile >>> +++ b/xen/arch/x86/mm/shadow/Makefile >>> @@ -1,4 +1,4 @@ >>> -ifeq ($(shadow-paging),y) >>> +ifdef CONFIG_SHADOW_PAGING >>> obj-y += common.o guest_2.o guest_3.o guest_4.o >>> else >>> obj-y += none.o >> Why didn't you retain the ifeq? The way it's now, accidental >> definition of the macro elsewhere would alter behavior. In fact >> I think the better thing here would be to change this to purely >> list model: >> >> obj-y := none.o >> obj-$(CONFIG_SHADOW_PAGING) := common.o guest_2.o guest_3.o guest_4.o >> >> Not sure why I didn't do that right when making this configurable... > > none.o must not be linked if !CONFIG_SHADOW_PAGING. And it wouldn't be, due to using ":=" (instead of "+="). Jan
At 09:46 +0000 on 19 Jan (1453196796), Ian Campbell wrote: > On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Does this have any impact on migration of either PV or HVM guests? What > about nested virt? Without shadow paging you can't do live migration of PV guests. That should go in the kconfig description. AFAICT nested virt depends on HAP rather than shadow. > Are things which are defined in xen/arch/*/Rules.mk in this way > overrideable from the old top-level .config or does one need to dive deeper > to modify them? If it's not configurable from top-level .config today then > I think it either needs a "depends EXPERT" or for the changelog to make a > convincing argument why this should be made user selectable. I'm all in favour of making this selectable -- people who don't need the feature can avoid 7k LOC that way. :) If it's not going to be 'EXPERT' maybe we could have some console logging, either in the none.c stubs or just at boot time, to make it more obvious why the user's live migration/non-HAP guest doesn't work. Cheers, Tim.
On 19/01/16 13:38, Jan Beulich wrote: >>>> On 19.01.16 at 14:30, <andrew.cooper3@citrix.com> wrote: >> On 19/01/16 13:23, Jan Beulich wrote: >>>>>> On 19.01.16 at 10:46, <ian.campbell@citrix.com> wrote: >>>> On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: >>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >>>> Does this have any impact on migration of either PV or HVM guests? What >>>> about nested virt? >>> At least PV guests won't be migratable anymore when there's no >>> shadow mode. >> What? Shadow mode (or lack thereof) has no impact whatsoever on migration. > So how would log-dirty mode work for a PV guest without shadow > code? Oops yes. I am confusing the fact that logdirty and vram tracking are independent, rather than logdiry and shadow. (we have far too many modes) I should update the text to talk about migration. ~Andrew
On Tue, 2016-01-19 at 13:39 +0000, Andrew Cooper wrote: > On 19/01/16 09:46, Ian Campbell wrote: > > On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Does this have any impact on migration of either PV or HVM guests? > > Not in the slightest. > > > What about nested virt? > > Nested virt is completely broken with respect to migration. Sorry, I meant impact (generally) on nested, not WRT migration. AIUI shadow is required for either L1 or L2 (not sure which). > > Are things which are defined in xen/arch/*/Rules.mk in this way > > overrideable from the old top-level .config or does one need to dive > > deeper > > to modify them? > > Absolutely everything anywhere in the build system is/was configurable > from the top .config OK, good, so no change here, that's fine then. > > Lastly, Tim is maintainer of the shadow code and should have been CC-d, > > also George as maintainer of the mm stuff might have an interest. Both > > CC-s > > added. > > Oops yes, although this is an entirely mechanical alteration to an > option which already existed. > > ~Andrew
On 19/01/16 13:46, Tim Deegan wrote: > At 09:46 +0000 on 19 Jan (1453196796), Ian Campbell wrote: >> On Mon, 2016-01-18 at 18:40 +0000, Andrew Cooper wrote: >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Does this have any impact on migration of either PV or HVM guests? What >> about nested virt? > Without shadow paging you can't do live migration of PV guests. That > should go in the kconfig description. > > AFAICT nested virt depends on HAP rather than shadow. Nothing currently enforces this. Xen dies particularly quickly with spinlock contention if a shadow HVM guest tries to use nested-virt. (Just another item on the long list of tasks requires before nested-virt becomes anything other than experimental.) ~Andrew
On 19/01/16 13:50, Ian Campbell wrote: > >>> What about nested virt? >> Nested virt is completely broken with respect to migration. > Sorry, I meant impact (generally) on nested, not WRT migration. AIUI shadow > is required for either L1 or L2 (not sure which). There is no requirement one way or another. It function in any combination you care to pick. However, if you don't have HAP all the way down, Xen falls over very quickly with spinlock contention if a guest actually starts doing some real work. ~Andrew
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 4781b34..9869630 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -27,6 +27,20 @@ menu "Architecture Features" source "arch/Kconfig" +config SHADOW_PAGING + bool "Shadow Paging" + default y + ---help--- + Shadow paging is a software alternative to hardware paging support + (Intel EPT, AMD NPT) for use with HVM guests. + + It is required to run HVM guests for first-generation hardware + virtualisation (Intel VT-x, AMD SVM) which did not include hardware + paging support. Under a small number of specific workloads, shadow + paging may also be deliberately used as a performance improvement. + + If unsure, say Y. + config BIGMEM bool "big memory support" default n diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk index a108d24..a1cdae0 100644 --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -22,13 +22,9 @@ $(call as-insn-check,CFLAGS,CC,".equ \"x\"$$(comma)1", \ -U__OBJECT_LABEL__ -DHAVE_GAS_QUOTED_SYM \ '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@') -shadow-paging ?= y - CFLAGS += -mno-red-zone -mno-sse -fpic CFLAGS += -fno-asynchronous-unwind-tables # -fvisibility=hidden reduces -fpic cost, if it's available ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n) CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE endif - -CFLAGS-$(shadow-paging) += -DCONFIG_SHADOW_PAGING diff --git a/xen/arch/x86/mm/shadow/Makefile b/xen/arch/x86/mm/shadow/Makefile index a07bc0c..df194ad 100644 --- a/xen/arch/x86/mm/shadow/Makefile +++ b/xen/arch/x86/mm/shadow/Makefile @@ -1,4 +1,4 @@ -ifeq ($(shadow-paging),y) +ifdef CONFIG_SHADOW_PAGING obj-y += common.o guest_2.o guest_3.o guest_4.o else obj-y += none.o
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Doug Goldstein <cardoe@cardoe.com> --- xen/arch/x86/Kconfig | 14 ++++++++++++++ xen/arch/x86/Rules.mk | 4 ---- xen/arch/x86/mm/shadow/Makefile | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-)