--- drivers/xen/balloon.c | 26 +++++++++++++++++++++----- 1 files changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 31ab82f..57960a1 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -358,10 +358,18 @@ static enum bp_state increase_reservation(unsigned long nr_pages) BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && phys_to_machine_mapping_valid(pfn)); - set_phys_to_machine(pfn, frame_list[i]); - + if (!xen_pvh_domain()) { + set_phys_to_machine(pfn, frame_list[i]); + } else { + pte_t *ptep; + unsigned int level; + void *addr = __va(pfn << PAGE_SHIFT); + ptep = lookup_address((unsigned long)addr, &level); + set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL)); + } /* Link back into the page tables if not highmem. */ - if (xen_pv_domain() && !PageHighMem(page)) { + if (xen_pv_domain() && !PageHighMem(page) && + !xen_pvh_domain()) { int ret; ret = HYPERVISOR_update_va_mapping( (unsigned long)__va(pfn << PAGE_SHIFT), @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) scrub_page(page); - if (xen_pv_domain() && !PageHighMem(page)) { + if (xen_pvh_domain() && !PageHighMem(page)) { + unsigned int level; + pte_t *ptep; + void *addr = __va(pfn << PAGE_SHIFT); + ptep = lookup_address((unsigned long)addr, &level); + set_pte(ptep, __pte(0)); + + } else if (xen_pv_domain() && !PageHighMem(page)) { ret = HYPERVISOR_update_va_mapping( (unsigned long)__va(pfn << PAGE_SHIFT), __pte_ma(0), 0); @@ -433,7 +448,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) /* No more mappings: invalidate P2M and add to balloon. */ for (i = 0; i < nr_pages; i++) { pfn = mfn_to_pfn(frame_list[i]); - __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); + if (!xen_pvh_domain()) + __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); balloon_append(pfn_to_page(pfn)); } -- 1.7.2.3
And patches like this one are the reason why I don''t want xen_pvh_domain in non-x86 xen files: I am pretty sure that if you didn''t use xen_pvh_domain here but a combination of XENFEAT_auto_translated_physmap and xen_hvm_domain I would be able to reuse this work on ARM (that doesn''t have a working balloon driver yet) as is. On Thu, 16 Aug 2012, Mukesh Rathor wrote:> --- > drivers/xen/balloon.c | 26 +++++++++++++++++++++----- > 1 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 31ab82f..57960a1 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -358,10 +358,18 @@ static enum bp_state increase_reservation(unsigned long nr_pages) > BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && > phys_to_machine_mapping_valid(pfn)); > > - set_phys_to_machine(pfn, frame_list[i]); > - > + if (!xen_pvh_domain()) { > + set_phys_to_machine(pfn, frame_list[i]); > + } else { > + pte_t *ptep; > + unsigned int level; > + void *addr = __va(pfn << PAGE_SHIFT); > + ptep = lookup_address((unsigned long)addr, &level); > + set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL)); > + } > /* Link back into the page tables if not highmem. */ > - if (xen_pv_domain() && !PageHighMem(page)) { > + if (xen_pv_domain() && !PageHighMem(page) && > + !xen_pvh_domain()) { > int ret; > ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) > > scrub_page(page); > > - if (xen_pv_domain() && !PageHighMem(page)) { > + if (xen_pvh_domain() && !PageHighMem(page)) { > + unsigned int level; > + pte_t *ptep; > + void *addr = __va(pfn << PAGE_SHIFT); > + ptep = lookup_address((unsigned long)addr, &level); > + set_pte(ptep, __pte(0)); > + > + } else if (xen_pv_domain() && !PageHighMem(page)) { > ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > __pte_ma(0), 0); > @@ -433,7 +448,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) > /* No more mappings: invalidate P2M and add to balloon. */ > for (i = 0; i < nr_pages; i++) { > pfn = mfn_to_pfn(frame_list[i]); > - __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); > + if (!xen_pvh_domain()) > + __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); > balloon_append(pfn_to_page(pfn)); > } > > -- > 1.7.2.3 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
On Thu, 2012-08-16 at 02:05 +0100, Mukesh Rathor wrote:> --- > drivers/xen/balloon.c | 26 +++++++++++++++++++++----- > 1 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 31ab82f..57960a1 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -358,10 +358,18 @@ static enum bp_state increase_reservation(unsigned long nr_pages) > BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && > phys_to_machine_mapping_valid(pfn)); > > - set_phys_to_machine(pfn, frame_list[i]); > - > + if (!xen_pvh_domain()) {XENFEAT_auto_translated...?> + set_phys_to_machine(pfn, frame_list[i]); > + } else { > + pte_t *ptep; > + unsigned int level; > + void *addr = __va(pfn << PAGE_SHIFT); > + ptep = lookup_address((unsigned long)addr, &level); > + set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL)); > + } > /* Link back into the page tables if not highmem. */ > - if (xen_pv_domain() && !PageHighMem(page)) { > + if (xen_pv_domain() && !PageHighMem(page) && > + !xen_pvh_domain()) {It feels like this ought to be inside the !xen_pvh_domain above as well as just hte set_phys_to_machine. And is the else above not missing a !PageHighMem check?> int ret; > ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) > > scrub_page(page); > > - if (xen_pv_domain() && !PageHighMem(page)) { > + if (xen_pvh_domain() && !PageHighMem(page)) { > + unsigned int level; > + pte_t *ptep; > + void *addr = __va(pfn << PAGE_SHIFT); > + ptep = lookup_address((unsigned long)addr, &level); > + set_pte(ptep, __pte(0)); > + > + } else if (xen_pv_domain() && !PageHighMem(page)) { > ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > __pte_ma(0), 0);This pattern: if ( xen_pvh_... ) ... lookup_address / set_pte ... else ... HYOERVISOR_update_va_mapping Was present above too -- candidate for a helper? (I was a bit surprised there wasn''t already one...)> @@ -433,7 +448,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) > /* No more mappings: invalidate P2M and add to balloon. */ > for (i = 0; i < nr_pages; i++) { > pfn = mfn_to_pfn(frame_list[i]); > - __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); > + if (!xen_pvh_domain())XENFEAT_something?> + __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); > balloon_append(pfn_to_page(pfn)); > } >
On Thu, 2012-08-16 at 02:05 +0100, Mukesh Rathor wrote:> --- > drivers/xen/balloon.c | 26 +++++++++++++++++++++----- > 1 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 31ab82f..57960a1 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -358,10 +358,18 @@ static enum bp_state increase_reservation(unsigned long nr_pages) > BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && > phys_to_machine_mapping_valid(pfn)); > > - set_phys_to_machine(pfn, frame_list[i]); > - > + if (!xen_pvh_domain()) { > + set_phys_to_machine(pfn, frame_list[i]); > + } else { > + pte_t *ptep; > + unsigned int level; > + void *addr = __va(pfn << PAGE_SHIFT); > + ptep = lookup_address((unsigned long)addr, &level); > + set_pte(ptep, pfn_pte(pfn, PAGE_KERNEL));Another thing I just tripped over on ARM which might be pertinent on x86 PVH too is that lowmem mappings on ARM are super 2M mappings, so trying to clear the PTE here fails. I was a bit wary of leaving stage 1 mappings in place for pfn''s with no backing mfn but this 2M mapping issue has caused me to revise that opinion -- there''s no need to worry about the stage 1 mapping still being present as long as you guarantee you never touch the associated virtual address, which the balloon driver should be able to do. The other option is to shatter such mappings which is just too much pain to contemplate. Long story short I reckon you can drop this hunk (and associated similar changes) and rely on the XENFEAT_auto_translated_physmap check inside set_phys_to_machine to do the right thing. I guess PVH currently suppresses superpage mappings on x86 (probably inherited from PV) but undoing that might be something to investigate for the future? It''ll help perf I expect. Ian.> + } > /* Link back into the page tables if not highmem. */ > - if (xen_pv_domain() && !PageHighMem(page)) { > + if (xen_pv_domain() && !PageHighMem(page) && > + !xen_pvh_domain()) { > int ret; > ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > @@ -417,7 +425,14 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) > > scrub_page(page); > > - if (xen_pv_domain() && !PageHighMem(page)) { > + if (xen_pvh_domain() && !PageHighMem(page)) { > + unsigned int level; > + pte_t *ptep; > + void *addr = __va(pfn << PAGE_SHIFT); > + ptep = lookup_address((unsigned long)addr, &level); > + set_pte(ptep, __pte(0)); > + > + } else if (xen_pv_domain() && !PageHighMem(page)) { > ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > __pte_ma(0), 0); > @@ -433,7 +448,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) > /* No more mappings: invalidate P2M and add to balloon. */ > for (i = 0; i < nr_pages; i++) { > pfn = mfn_to_pfn(frame_list[i]); > - __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); > + if (!xen_pvh_domain()) > + __set_phys_to_machine(pfn, INVALID_P2M_ENTRY); > balloon_append(pfn_to_page(pfn)); > } >