| Message ID | 20181214115855.6713-6-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Implement Set/Way operations | expand |
On Fri, 14 Dec 2018, Julien Grall wrote: > At the moment, the implementation of Set/Way operations will go through > all the entries of the guest P2M and flush them. However, this is very > expensive and may render unusable a guest OS using them. > > For instance, Linux 32-bit will use Set/Way operations during secondary > CPU bring-up. As the implementation is really expensive, it may be possible > to hit the CPU bring-up timeout. > > To limit the Set/Way impact, we track what pages has been of the guest > has been accessed between batch of Set/Way operations. This is done > using bit[0] (aka valid bit) of the P2M entry. > > This patch adds a new per-arch helper is introduced to perform actions just > before the guest is first unpaused. This will be used to invalidate the > P2M to track access from the start of the guest. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > > While we can spread d->creation_finished all over the code, the per-arch > helper to perform actions just before the guest is first unpaused can > bring a lot of benefit for both architecture. For instance, on Arm, the > flush to the instruction cache could be delayed until the domain is > first run. This would improve greatly the performance of creating guest. > > I am still doing the benchmark whether having a command line option is > worth it. I will provide numbers as soon as I have them. > > Changes in v3: > - Add Jan reviewed-by for non-ARM pieces > > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.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: Tim Deegan <tim@xen.org> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > xen/arch/arm/domain.c | 14 ++++++++++++++ > xen/arch/arm/p2m.c | 29 +++++++++++++++++++++++++++-- > xen/arch/x86/domain.c | 4 ++++ > xen/common/domain.c | 5 ++++- > xen/include/asm-arm/p2m.h | 2 ++ > xen/include/xen/domain.h | 2 ++ > 6 files changed, 53 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 1d926dcb29..41f101746e 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -767,6 +767,20 @@ int arch_domain_soft_reset(struct domain *d) > return -ENOSYS; > } > > +void arch_domain_creation_finished(struct domain *d) > +{ > + /* > + * To avoid flushing the whole guest RAM on the first Set/Way, we > + * invalidate the P2M to track what has been accessed. > + * > + * This is only turned when IOMMU is not used or the page-table are > + * not shared because bit[0] (e.g valid bit) unset will result > + * IOMMU fault that could be not fixed-up. > + */ > + if ( !iommu_use_hap_pt(d) ) > + p2m_invalidate_root(p2m_get_hostp2m(d)); > +} > + > static int is_guest_pv32_psr(uint32_t psr) > { > switch (psr & PSR_MODE_MASK) > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 125d858d02..347028c325 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1079,6 +1079,22 @@ static void p2m_invalidate_table(struct p2m_domain *p2m, mfn_t mfn) > } > > /* > + * Invalidate all entries in the root page-tables. This is > + * useful to get fault on entry and do an action. > + */ > +void p2m_invalidate_root(struct p2m_domain *p2m) > +{ > + unsigned int i; > + > + p2m_write_lock(p2m); > + > + for ( i = 0; i < P2M_ROOT_LEVEL; i++ ) > + p2m_invalidate_table(p2m, page_to_mfn(p2m->root + i)); > + > + p2m_write_unlock(p2m); > +} > + > +/* > * Resolve any translation fault due to change in the p2m. This > * includes break-before-make and valid bit cleared. > */ > @@ -1587,10 +1603,12 @@ int p2m_cache_flush_range(struct domain *d, gfn_t *pstart, gfn_t end) > */ > if ( gfn_eq(start, next_block_gfn) ) > { > - mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL); > + bool valid; > + > + mfn = p2m_get_entry(p2m, start, &t, NULL, &order, &valid); > next_block_gfn = gfn_next_boundary(start, order); > > - if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) ) > + if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) || !valid ) > { > count++; > start = next_block_gfn; > @@ -1624,6 +1642,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t *pstart, gfn_t end) > */ > void p2m_flush_vm(struct vcpu *v) > { > + struct p2m_domain *p2m = p2m_get_hostp2m(v->domain); > int rc; > gfn_t start = _gfn(0); > > @@ -1643,6 +1662,12 @@ void p2m_flush_vm(struct vcpu *v) > "P2M has not been correctly cleaned (rc = %d)\n", > rc); > > + /* > + * Invalidate the p2m to track which page was modified by the guest > + * between call of p2m_flush_vm(). > + */ > + p2m_invalidate_root(p2m); > + > v->arch.need_flush_to_ram = false; > } > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index f0e0cdbb0e..3729887d00 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -762,6 +762,10 @@ int arch_domain_soft_reset(struct domain *d) > return ret; > } > > +void arch_domain_creation_finished(struct domain *d) > +{ > +} > + > /* > * These are the masks of CR4 bits (subject to hardware availability) which a > * PV guest may not legitimiately attempt to modify. > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 78cc5249e8..c623daec56 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -1116,8 +1116,11 @@ int domain_unpause_by_systemcontroller(struct domain *d) > * Creation is considered finished when the controller reference count > * first drops to 0. > */ > - if ( new == 0 ) > + if ( new == 0 && !d->creation_finished ) > + { > d->creation_finished = true; > + arch_domain_creation_finished(d); > + } > > domain_unpause(d); > > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index 79abcb5a63..01cd3ee4b5 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -231,6 +231,8 @@ int p2m_set_entry(struct p2m_domain *p2m, > > bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn); > > +void p2m_invalidate_root(struct p2m_domain *p2m); > + > /* > * Clean & invalidate caches corresponding to a region [start,end) of guest > * address space. > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h > index 33e41486cb..d1bfc82f57 100644 > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -70,6 +70,8 @@ void arch_domain_unpause(struct domain *d); > > int arch_domain_soft_reset(struct domain *d); > > +void arch_domain_creation_finished(struct domain *d); > + > void arch_p2m_set_access_required(struct domain *d, bool access_required); > > int arch_set_info_guest(struct vcpu *, vcpu_guest_context_u); > -- > 2.11.0 >
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 1d926dcb29..41f101746e 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -767,6 +767,20 @@ int arch_domain_soft_reset(struct domain *d) return -ENOSYS; } +void arch_domain_creation_finished(struct domain *d) +{ + /* + * To avoid flushing the whole guest RAM on the first Set/Way, we + * invalidate the P2M to track what has been accessed. + * + * This is only turned when IOMMU is not used or the page-table are + * not shared because bit[0] (e.g valid bit) unset will result + * IOMMU fault that could be not fixed-up. + */ + if ( !iommu_use_hap_pt(d) ) + p2m_invalidate_root(p2m_get_hostp2m(d)); +} + static int is_guest_pv32_psr(uint32_t psr) { switch (psr & PSR_MODE_MASK) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 125d858d02..347028c325 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1079,6 +1079,22 @@ static void p2m_invalidate_table(struct p2m_domain *p2m, mfn_t mfn) } /* + * Invalidate all entries in the root page-tables. This is + * useful to get fault on entry and do an action. + */ +void p2m_invalidate_root(struct p2m_domain *p2m) +{ + unsigned int i; + + p2m_write_lock(p2m); + + for ( i = 0; i < P2M_ROOT_LEVEL; i++ ) + p2m_invalidate_table(p2m, page_to_mfn(p2m->root + i)); + + p2m_write_unlock(p2m); +} + +/* * Resolve any translation fault due to change in the p2m. This * includes break-before-make and valid bit cleared. */ @@ -1587,10 +1603,12 @@ int p2m_cache_flush_range(struct domain *d, gfn_t *pstart, gfn_t end) */ if ( gfn_eq(start, next_block_gfn) ) { - mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL); + bool valid; + + mfn = p2m_get_entry(p2m, start, &t, NULL, &order, &valid); next_block_gfn = gfn_next_boundary(start, order); - if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) ) + if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) || !valid ) { count++; start = next_block_gfn; @@ -1624,6 +1642,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t *pstart, gfn_t end) */ void p2m_flush_vm(struct vcpu *v) { + struct p2m_domain *p2m = p2m_get_hostp2m(v->domain); int rc; gfn_t start = _gfn(0); @@ -1643,6 +1662,12 @@ void p2m_flush_vm(struct vcpu *v) "P2M has not been correctly cleaned (rc = %d)\n", rc); + /* + * Invalidate the p2m to track which page was modified by the guest + * between call of p2m_flush_vm(). + */ + p2m_invalidate_root(p2m); + v->arch.need_flush_to_ram = false; } diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index f0e0cdbb0e..3729887d00 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -762,6 +762,10 @@ int arch_domain_soft_reset(struct domain *d) return ret; } +void arch_domain_creation_finished(struct domain *d) +{ +} + /* * These are the masks of CR4 bits (subject to hardware availability) which a * PV guest may not legitimiately attempt to modify. diff --git a/xen/common/domain.c b/xen/common/domain.c index 78cc5249e8..c623daec56 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -1116,8 +1116,11 @@ int domain_unpause_by_systemcontroller(struct domain *d) * Creation is considered finished when the controller reference count * first drops to 0. */ - if ( new == 0 ) + if ( new == 0 && !d->creation_finished ) + { d->creation_finished = true; + arch_domain_creation_finished(d); + } domain_unpause(d); diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index 79abcb5a63..01cd3ee4b5 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -231,6 +231,8 @@ int p2m_set_entry(struct p2m_domain *p2m, bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn); +void p2m_invalidate_root(struct p2m_domain *p2m); + /* * Clean & invalidate caches corresponding to a region [start,end) of guest * address space. diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index 33e41486cb..d1bfc82f57 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -70,6 +70,8 @@ void arch_domain_unpause(struct domain *d); int arch_domain_soft_reset(struct domain *d); +void arch_domain_creation_finished(struct domain *d); + void arch_p2m_set_access_required(struct domain *d, bool access_required); int arch_set_info_guest(struct vcpu *, vcpu_guest_context_u);