| Message ID | 20210425201318.15447-6-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: mm: Remove open-coding mappings | expand |
On Sun, 25 Apr 2021, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Currently, the function xen_pt_update() will flush the TLBs even when > the mappings are inserted. This is a bit wasteful because we don't > allow mapping replacement. Even if we were, the flush would need to > happen earlier because mapping replacement should use Break-Before-Make > when updating the entry. > > A single call to xen_pt_update() can perform a single action. IOW, it > is not possible to, for instance, mix inserting and removing mappings. > Therefore, we can use `flags` to determine what action is performed. > > This change will be particularly help to limit the impact of switching > boot time mapping to use xen_pt_update(). > > Signed-off-by: Julien Grall <jgrall@amazon.com> Nice improvement! Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > > Changes in v2: > - New patch > --- > xen/arch/arm/mm.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 8ebb36899314..1fe52b3af722 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1101,7 +1101,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level, > /* We should be here with a valid MFN. */ > ASSERT(!mfn_eq(mfn, INVALID_MFN)); > > - /* We don't allow replacing any valid entry. */ > + /* > + * We don't allow replacing any valid entry. > + * > + * Note that the function xen_pt_update() relies on this > + * assumption and will skip the TLB flush. The function will need > + * to be updated if the check is relaxed. > + */ > if ( lpae_is_valid(entry) ) > { > if ( lpae_is_mapping(entry, level) ) > @@ -1333,11 +1339,16 @@ static int xen_pt_update(unsigned long virt, > } > > /* > - * Flush the TLBs even in case of failure because we may have > + * The TLBs flush can be safely skipped when a mapping is inserted > + * as we don't allow mapping replacement (see xen_pt_check_entry()). > + * > + * For all the other cases, the TLBs will be flushed unconditionally > + * even if the mapping are failed. This is because we may have > * partially modified the PT. This will prevent any unexpected > * behavior afterwards. > */ > - flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns); > + if ( !(flags & _PAGE_PRESENT) || mfn_eq(mfn, INVALID_MFN) ) > + flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns); > > spin_unlock(&xen_pt_lock); > > -- > 2.17.1 >
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 8ebb36899314..1fe52b3af722 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1101,7 +1101,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level, /* We should be here with a valid MFN. */ ASSERT(!mfn_eq(mfn, INVALID_MFN)); - /* We don't allow replacing any valid entry. */ + /* + * We don't allow replacing any valid entry. + * + * Note that the function xen_pt_update() relies on this + * assumption and will skip the TLB flush. The function will need + * to be updated if the check is relaxed. + */ if ( lpae_is_valid(entry) ) { if ( lpae_is_mapping(entry, level) ) @@ -1333,11 +1339,16 @@ static int xen_pt_update(unsigned long virt, } /* - * Flush the TLBs even in case of failure because we may have + * The TLBs flush can be safely skipped when a mapping is inserted + * as we don't allow mapping replacement (see xen_pt_check_entry()). + * + * For all the other cases, the TLBs will be flushed unconditionally + * even if the mapping are failed. This is because we may have * partially modified the PT. This will prevent any unexpected * behavior afterwards. */ - flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns); + if ( !(flags & _PAGE_PRESENT) || mfn_eq(mfn, INVALID_MFN) ) + flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns); spin_unlock(&xen_pt_lock);