| Message ID | db8a9729-8408-c4a2-e28b-042c72bae186@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AMD/IOMMU: re-work mode updating | expand |
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan > Beulich > Sent: 25 November 2019 09:58 > To: xen-devel@lists.xenproject.org > Cc: Juergen Gross <jgross@suse.com>; Stefano Stabellini > <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Wei Liu > <wl@xen.org>; Konrad Wilk <konrad.wilk@oracle.com>; George Dunlap > <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>; > Sander Eikelenboom <linux@eikelenboom.it>; Ian Jackson > <ian.jackson@citrix.com>; Roger Pau Monné <roger.pau@citrix.com> > Subject: [Xen-devel] [PATCH v3 1/3] introduce GFN notification for > translated domains > > In order for individual IOMMU drivers (and from an abstract pov also > architectures) to be able to adjust, ahead of actual mapping requests, > their data structures when they might cover only a sub-range of all > possible GFNs, introduce a notification call used by various code paths > potentially installing a fresh mapping of a never used GFN (for a > particular domain). > > Note that before this patch, in gnttab_transfer(), once past > assign_pages(), further errors modifying the physmap are ignored > (presumably because it would be too complicated to try to roll back at > that point). This patch follows suit by ignoring failed notify_gfn()s or > races due to the need to intermediately drop locks, simply printing out > a warning that the gfn may not be accessible due to the failure. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > v3: Conditionalize upon CONFIG_IOMMU_FORCE_PT_SHARE, also covering the > share_p2m_table() functionality as appropriate. Un-comment the > GNTMAP_host_map check. > v2: Introduce arch_notify_gfn(), to invoke gfn_valid() on x86 (this > unfortunately means it and notify_gfn() now need to be macros, or > else include file dependencies get in the way, as gfn_valid() lives > in paging.h, which we shouldn't include from xen/sched.h). Improve > description. > > TBD: Does Arm actually have anything to check against in its > arch_notify_gfn()? > > --- a/xen/arch/x86/hvm/dom0_build.c > +++ b/xen/arch/x86/hvm/dom0_build.c > @@ -173,7 +173,8 @@ static int __init pvh_populate_memory_ra > continue; > } > > - rc = guest_physmap_add_page(d, _gfn(start), page_to_mfn(page), > + rc = notify_gfn(d, _gfn(start + (1UL << order) - 1)) ?: > + guest_physmap_add_page(d, _gfn(start), page_to_mfn(page), > order); > if ( rc != 0 ) > { > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4304,9 +4304,17 @@ static int hvmop_set_param( > if ( a.value > SHUTDOWN_MAX ) > rc = -EINVAL; > break; > + > case HVM_PARAM_IOREQ_SERVER_PFN: > - d->arch.hvm.ioreq_gfn.base = a.value; > + if ( d->arch.hvm.params[HVM_PARAM_NR_IOREQ_SERVER_PAGES] ) > + rc = notify_gfn( > + d, > + _gfn(a.value + d->arch.hvm.params > + [HVM_PARAM_NR_IOREQ_SERVER_PAGES] - > 1)); IIRC the PFN is typically set by the toolstack before the number of pages, so the notify will be for a.value - 1, i.e. the previous gfn. Is that a problem? Paul > + if ( !rc ) > + d->arch.hvm.ioreq_gfn.base = a.value; > break; > + > case HVM_PARAM_NR_IOREQ_SERVER_PAGES: > { > unsigned int i; > @@ -4317,6 +4325,9 @@ static int hvmop_set_param( > rc = -EINVAL; > break; > } > + rc = notify_gfn(d, _gfn(d->arch.hvm.ioreq_gfn.base + a.value - > 1)); > + if ( rc ) > + break; > for ( i = 0; i < a.value; i++ ) > set_bit(i, &d->arch.hvm.ioreq_gfn.mask); > > @@ -4330,7 +4341,11 @@ static int hvmop_set_param( > BUILD_BUG_ON(HVM_PARAM_BUFIOREQ_PFN > > sizeof(d->arch.hvm.ioreq_gfn.legacy_mask) * 8); > if ( a.value ) > - set_bit(a.index, &d->arch.hvm.ioreq_gfn.legacy_mask); > + { > + rc = notify_gfn(d, _gfn(a.value)); > + if ( !rc ) > + set_bit(a.index, &d->arch.hvm.ioreq_gfn.legacy_mask); > + } > break; > > case HVM_PARAM_X87_FIP_WIDTH: > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -946,6 +946,16 @@ map_grant_ref( > return; > } > > + if ( paging_mode_translate(ld) && (op->flags & GNTMAP_host_map) && > + (rc = notify_gfn(ld, gaddr_to_gfn(op->host_addr))) ) > + { > + gdprintk(XENLOG_INFO, "notify(%"PRI_gfn") -> %d\n", > + gfn_x(gaddr_to_gfn(op->host_addr)), rc); > + op->status = GNTST_general_error; > + return; > + BUILD_BUG_ON(GNTST_okay); > + } > + > if ( unlikely((rd = rcu_lock_domain_by_id(op->dom)) == NULL) ) > { > gdprintk(XENLOG_INFO, "Could not find domain %d\n", op->dom); > @@ -2123,6 +2133,7 @@ gnttab_transfer( > { > bool_t okay; > int rc; > + gfn_t gfn; > > if ( i && hypercall_preempt_check() ) > return i; > @@ -2300,21 +2311,52 @@ gnttab_transfer( > act = active_entry_acquire(e->grant_table, gop.ref); > > if ( evaluate_nospec(e->grant_table->gt_version == 1) ) > + gfn = _gfn(shared_entry_v1(e->grant_table, gop.ref).frame); > + else > + gfn = _gfn(shared_entry_v2(e->grant_table, > gop.ref).full_page.frame); > + > + if ( paging_mode_translate(e) ) > { > - grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, > gop.ref); > + gfn_t gfn2; > + > + active_entry_release(act); > + grant_read_unlock(e->grant_table); > + > + rc = notify_gfn(e, gfn); > + if ( rc ) > + printk(XENLOG_G_WARNING > + "%pd: gref %u: xfer GFN %"PRI_gfn" may be > inaccessible (%d)\n", > + e, gop.ref, gfn_x(gfn), rc); > + > + grant_read_lock(e->grant_table); > + act = active_entry_acquire(e->grant_table, gop.ref); > > - guest_physmap_add_page(e, _gfn(sha->frame), mfn, 0); > - if ( !paging_mode_translate(e) ) > - sha->frame = mfn_x(mfn); > + if ( evaluate_nospec(e->grant_table->gt_version == 1) ) > + gfn2 = _gfn(shared_entry_v1(e->grant_table, > gop.ref).frame); > + else > + gfn2 = _gfn(shared_entry_v2(e->grant_table, gop.ref). > + full_page.frame); > + > + if ( !gfn_eq(gfn, gfn2) ) > + { > + printk(XENLOG_G_WARNING > + "%pd: gref %u: xfer GFN went %"PRI_gfn" -> > %"PRI_gfn"\n", > + e, gop.ref, gfn_x(gfn), gfn_x(gfn2)); > + gfn = gfn2; > + } > } > - else > - { > - grant_entry_v2_t *sha = &shared_entry_v2(e->grant_table, > gop.ref); > > - guest_physmap_add_page(e, _gfn(sha->full_page.frame), mfn, > 0); > - if ( !paging_mode_translate(e) ) > - sha->full_page.frame = mfn_x(mfn); > + guest_physmap_add_page(e, gfn, mfn, 0); > + > + if ( !paging_mode_translate(e) ) > + { > + if ( evaluate_nospec(e->grant_table->gt_version == 1) ) > + shared_entry_v1(e->grant_table, gop.ref).frame = > mfn_x(mfn); > + else > + shared_entry_v2(e->grant_table, gop.ref).full_page.frame > = > + mfn_x(mfn); > } > + > smp_wmb(); > shared_entry_header(e->grant_table, gop.ref)->flags |= > GTF_transfer_completed; > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -203,6 +203,10 @@ static void populate_physmap(struct memo > if ( unlikely(__copy_from_guest_offset(&gpfn, a->extent_list, i, > 1)) ) > goto out; > > + if ( paging_mode_translate(d) && > + notify_gfn(d, _gfn(gpfn + (1U << a->extent_order) - 1)) ) > + goto out; > + > if ( a->memflags & MEMF_populate_on_demand ) > { > /* Disallow populating PoD pages on oneself. */ > @@ -745,6 +749,10 @@ static long memory_exchange(XEN_GUEST_HA > continue; > } > > + if ( paging_mode_translate(d) ) > + rc = notify_gfn(d, > + _gfn(gpfn + (1U << exch.out.extent_order) > - 1)); > + > mfn = page_to_mfn(page); > guest_physmap_add_page(d, _gfn(gpfn), mfn, > exch.out.extent_order); > @@ -813,12 +821,20 @@ int xenmem_add_to_physmap(struct domain > extra.foreign_domid = DOMID_INVALID; > > if ( xatp->space != XENMAPSPACE_gmfn_range ) > - return xenmem_add_to_physmap_one(d, xatp->space, extra, > + return notify_gfn(d, _gfn(xatp->gpfn)) ?: > + xenmem_add_to_physmap_one(d, xatp->space, extra, > xatp->idx, _gfn(xatp->gpfn)); > > if ( xatp->size < start ) > return -EILSEQ; > > + if ( !start && xatp->size ) > + { > + rc = notify_gfn(d, _gfn(xatp->gpfn + xatp->size - 1)); > + if ( rc ) > + return rc; > + } > + > xatp->idx += start; > xatp->gpfn += start; > xatp->size -= start; > @@ -891,7 +907,8 @@ static int xenmem_add_to_physmap_batch(s > extent, 1)) ) > return -EFAULT; > > - rc = xenmem_add_to_physmap_one(d, xatpb->space, > + rc = notify_gfn(d, _gfn(gpfn)) ?: > + xenmem_add_to_physmap_one(d, xatpb->space, > xatpb->u, > idx, _gfn(gpfn)); > > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -522,6 +522,7 @@ int iommu_do_domctl( > return ret; > } > > +#ifndef CONFIG_IOMMU_FORCE_PT_SHARE > void iommu_share_p2m_table(struct domain* d) > { > ASSERT(hap_enabled(d)); > @@ -530,6 +531,15 @@ void iommu_share_p2m_table(struct domain > iommu_get_ops()->share_p2m(d); > } > > +int iommu_notify_gfn(struct domain *d, gfn_t gfn) > +{ > + const struct iommu_ops *ops = dom_iommu(d)->platform_ops; > + > + return need_iommu_pt_sync(d) && ops->notify_dfn > + ? iommu_call(ops, notify_dfn, d, _dfn(gfn_x(gfn))) : 0; > +} > +#endif > + > void iommu_crash_shutdown(void) > { > if ( !iommu_crash_disable ) > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -272,6 +272,8 @@ static inline void free_vcpu_guest_conte > > static inline void arch_vcpu_block(struct vcpu *v) {} > > +#define arch_notify_gfn(d, gfn) ((void)(d), (void)(gfn), 0) > + > #endif /* __ASM_DOMAIN_H__ */ > > /* > --- a/xen/include/asm-x86/domain.h > +++ b/xen/include/asm-x86/domain.h > @@ -647,6 +647,8 @@ static inline void free_vcpu_guest_conte > > void arch_vcpu_regs_init(struct vcpu *v); > > +#define arch_notify_gfn(d, gfn) (gfn_valid(d, gfn) ? 0 : -EADDRNOTAVAIL) > + > struct vcpu_hvm_context; > int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context > *ctx); > > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -237,6 +237,11 @@ struct iommu_ops { > int __must_check (*lookup_page)(struct domain *d, dfn_t dfn, mfn_t > *mfn, > unsigned int *flags); > > +#ifndef CONFIG_IOMMU_FORCE_PT_SHARE > + void (*share_p2m)(struct domain *d); > + int __must_check (*notify_dfn)(struct domain *d, dfn_t dfn); > +#endif > + > void (*free_page_table)(struct page_info *); > > #ifdef CONFIG_X86 > @@ -253,7 +258,6 @@ struct iommu_ops { > > int __must_check (*suspend)(void); > void (*resume)(void); > - void (*share_p2m)(struct domain *d); > void (*crash_shutdown)(void); > int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn, > unsigned int page_count, > @@ -330,7 +334,15 @@ void iommu_resume(void); > void iommu_crash_shutdown(void); > int iommu_get_reserved_device_memory(iommu_grdm_t *, void *); > > +#ifndef CONFIG_IOMMU_FORCE_PT_SHARE > void iommu_share_p2m_table(struct domain *d); > +int __must_check iommu_notify_gfn(struct domain *d, gfn_t gfn); > +#else > +static inline int __must_check iommu_notify_gfn(struct domain *d, gfn_t > gfn) > +{ > + return 0; > +} > +#endif > > #ifdef CONFIG_HAS_PCI > int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d, > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -1039,6 +1039,8 @@ static always_inline bool is_iommu_enabl > return evaluate_nospec(d->options & XEN_DOMCTL_CDF_iommu); > } > > +#define notify_gfn(d, gfn) (arch_notify_gfn(d, gfn) ?: > iommu_notify_gfn(d, gfn)) > + > extern bool sched_smt_power_savings; > extern bool sched_disable_smt_switching; > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
On 25.11.2019 11:37, Durrant, Paul wrote: >> -----Original Message----- >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Jan >> Beulich >> Sent: 25 November 2019 09:58 >> >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -4304,9 +4304,17 @@ static int hvmop_set_param( >> if ( a.value > SHUTDOWN_MAX ) >> rc = -EINVAL; >> break; >> + >> case HVM_PARAM_IOREQ_SERVER_PFN: >> - d->arch.hvm.ioreq_gfn.base = a.value; >> + if ( d->arch.hvm.params[HVM_PARAM_NR_IOREQ_SERVER_PAGES] ) >> + rc = notify_gfn( >> + d, >> + _gfn(a.value + d->arch.hvm.params >> + [HVM_PARAM_NR_IOREQ_SERVER_PAGES] - >> 1)); > > IIRC the PFN is typically set by the toolstack before the number of > pages, so the notify will be for a.value - 1, i.e. the previous gfn. > Is that a problem? There's an if() around the invocation to avoid this situation, so I'm afraid I don't understand the question. Jan
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 25 November 2019 10:51 > To: Durrant, Paul <pdurrant@amazon.com> > Cc: xen-devel@lists.xenproject.org; Andrew Cooper > <andrew.cooper3@citrix.com>; Ian Jackson <ian.jackson@citrix.com>; Roger > Pau Monné <roger.pau@citrix.com>; Sander Eikelenboom > <linux@eikelenboom.it>; George Dunlap <George.Dunlap@eu.citrix.com>; > Stefano Stabellini <sstabellini@kernel.org>; Konrad Wilk > <konrad.wilk@oracle.com>; Juergen Gross <jgross@suse.com>; Julien Grall > <julien@xen.org>; Wei Liu <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH v3 1/3] introduce GFN notification for > translated domains > > On 25.11.2019 11:37, Durrant, Paul wrote: > >> -----Original Message----- > >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Jan > >> Beulich > >> Sent: 25 November 2019 09:58 > >> > >> --- a/xen/arch/x86/hvm/hvm.c > >> +++ b/xen/arch/x86/hvm/hvm.c > >> @@ -4304,9 +4304,17 @@ static int hvmop_set_param( > >> if ( a.value > SHUTDOWN_MAX ) > >> rc = -EINVAL; > >> break; > >> + > >> case HVM_PARAM_IOREQ_SERVER_PFN: > >> - d->arch.hvm.ioreq_gfn.base = a.value; > >> + if ( d->arch.hvm.params[HVM_PARAM_NR_IOREQ_SERVER_PAGES] ) > >> + rc = notify_gfn( > >> + d, > >> + _gfn(a.value + d->arch.hvm.params > >> + [HVM_PARAM_NR_IOREQ_SERVER_PAGES] > - > >> 1)); > > > > IIRC the PFN is typically set by the toolstack before the number of > > pages, so the notify will be for a.value - 1, i.e. the previous gfn. > > Is that a problem? > > There's an if() around the invocation to avoid this situation, so I'm > afraid I don't understand the question. D'oh... Missed it. Sorry for the noise. Paul > > Jan
--- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -173,7 +173,8 @@ static int __init pvh_populate_memory_ra continue; } - rc = guest_physmap_add_page(d, _gfn(start), page_to_mfn(page), + rc = notify_gfn(d, _gfn(start + (1UL << order) - 1)) ?: + guest_physmap_add_page(d, _gfn(start), page_to_mfn(page), order); if ( rc != 0 ) { --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -4304,9 +4304,17 @@ static int hvmop_set_param( if ( a.value > SHUTDOWN_MAX ) rc = -EINVAL; break; + case HVM_PARAM_IOREQ_SERVER_PFN: - d->arch.hvm.ioreq_gfn.base = a.value; + if ( d->arch.hvm.params[HVM_PARAM_NR_IOREQ_SERVER_PAGES] ) + rc = notify_gfn( + d, + _gfn(a.value + d->arch.hvm.params + [HVM_PARAM_NR_IOREQ_SERVER_PAGES] - 1)); + if ( !rc ) + d->arch.hvm.ioreq_gfn.base = a.value; break; + case HVM_PARAM_NR_IOREQ_SERVER_PAGES: { unsigned int i; @@ -4317,6 +4325,9 @@ static int hvmop_set_param( rc = -EINVAL; break; } + rc = notify_gfn(d, _gfn(d->arch.hvm.ioreq_gfn.base + a.value - 1)); + if ( rc ) + break; for ( i = 0; i < a.value; i++ ) set_bit(i, &d->arch.hvm.ioreq_gfn.mask); @@ -4330,7 +4341,11 @@ static int hvmop_set_param( BUILD_BUG_ON(HVM_PARAM_BUFIOREQ_PFN > sizeof(d->arch.hvm.ioreq_gfn.legacy_mask) * 8); if ( a.value ) - set_bit(a.index, &d->arch.hvm.ioreq_gfn.legacy_mask); + { + rc = notify_gfn(d, _gfn(a.value)); + if ( !rc ) + set_bit(a.index, &d->arch.hvm.ioreq_gfn.legacy_mask); + } break; case HVM_PARAM_X87_FIP_WIDTH: --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -946,6 +946,16 @@ map_grant_ref( return; } + if ( paging_mode_translate(ld) && (op->flags & GNTMAP_host_map) && + (rc = notify_gfn(ld, gaddr_to_gfn(op->host_addr))) ) + { + gdprintk(XENLOG_INFO, "notify(%"PRI_gfn") -> %d\n", + gfn_x(gaddr_to_gfn(op->host_addr)), rc); + op->status = GNTST_general_error; + return; + BUILD_BUG_ON(GNTST_okay); + } + if ( unlikely((rd = rcu_lock_domain_by_id(op->dom)) == NULL) ) { gdprintk(XENLOG_INFO, "Could not find domain %d\n", op->dom); @@ -2123,6 +2133,7 @@ gnttab_transfer( { bool_t okay; int rc; + gfn_t gfn; if ( i && hypercall_preempt_check() ) return i; @@ -2300,21 +2311,52 @@ gnttab_transfer( act = active_entry_acquire(e->grant_table, gop.ref); if ( evaluate_nospec(e->grant_table->gt_version == 1) ) + gfn = _gfn(shared_entry_v1(e->grant_table, gop.ref).frame); + else + gfn = _gfn(shared_entry_v2(e->grant_table, gop.ref).full_page.frame); + + if ( paging_mode_translate(e) ) { - grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, gop.ref); + gfn_t gfn2; + + active_entry_release(act); + grant_read_unlock(e->grant_table); + + rc = notify_gfn(e, gfn); + if ( rc ) + printk(XENLOG_G_WARNING + "%pd: gref %u: xfer GFN %"PRI_gfn" may be inaccessible (%d)\n", + e, gop.ref, gfn_x(gfn), rc); + + grant_read_lock(e->grant_table); + act = active_entry_acquire(e->grant_table, gop.ref); - guest_physmap_add_page(e, _gfn(sha->frame), mfn, 0); - if ( !paging_mode_translate(e) ) - sha->frame = mfn_x(mfn); + if ( evaluate_nospec(e->grant_table->gt_version == 1) ) + gfn2 = _gfn(shared_entry_v1(e->grant_table, gop.ref).frame); + else + gfn2 = _gfn(shared_entry_v2(e->grant_table, gop.ref). + full_page.frame); + + if ( !gfn_eq(gfn, gfn2) ) + { + printk(XENLOG_G_WARNING + "%pd: gref %u: xfer GFN went %"PRI_gfn" -> %"PRI_gfn"\n", + e, gop.ref, gfn_x(gfn), gfn_x(gfn2)); + gfn = gfn2; + } } - else - { - grant_entry_v2_t *sha = &shared_entry_v2(e->grant_table, gop.ref); - guest_physmap_add_page(e, _gfn(sha->full_page.frame), mfn, 0); - if ( !paging_mode_translate(e) ) - sha->full_page.frame = mfn_x(mfn); + guest_physmap_add_page(e, gfn, mfn, 0); + + if ( !paging_mode_translate(e) ) + { + if ( evaluate_nospec(e->grant_table->gt_version == 1) ) + shared_entry_v1(e->grant_table, gop.ref).frame = mfn_x(mfn); + else + shared_entry_v2(e->grant_table, gop.ref).full_page.frame = + mfn_x(mfn); } + smp_wmb(); shared_entry_header(e->grant_table, gop.ref)->flags |= GTF_transfer_completed; --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -203,6 +203,10 @@ static void populate_physmap(struct memo if ( unlikely(__copy_from_guest_offset(&gpfn, a->extent_list, i, 1)) ) goto out; + if ( paging_mode_translate(d) && + notify_gfn(d, _gfn(gpfn + (1U << a->extent_order) - 1)) ) + goto out; + if ( a->memflags & MEMF_populate_on_demand ) { /* Disallow populating PoD pages on oneself. */ @@ -745,6 +749,10 @@ static long memory_exchange(XEN_GUEST_HA continue; } + if ( paging_mode_translate(d) ) + rc = notify_gfn(d, + _gfn(gpfn + (1U << exch.out.extent_order) - 1)); + mfn = page_to_mfn(page); guest_physmap_add_page(d, _gfn(gpfn), mfn, exch.out.extent_order); @@ -813,12 +821,20 @@ int xenmem_add_to_physmap(struct domain extra.foreign_domid = DOMID_INVALID; if ( xatp->space != XENMAPSPACE_gmfn_range ) - return xenmem_add_to_physmap_one(d, xatp->space, extra, + return notify_gfn(d, _gfn(xatp->gpfn)) ?: + xenmem_add_to_physmap_one(d, xatp->space, extra, xatp->idx, _gfn(xatp->gpfn)); if ( xatp->size < start ) return -EILSEQ; + if ( !start && xatp->size ) + { + rc = notify_gfn(d, _gfn(xatp->gpfn + xatp->size - 1)); + if ( rc ) + return rc; + } + xatp->idx += start; xatp->gpfn += start; xatp->size -= start; @@ -891,7 +907,8 @@ static int xenmem_add_to_physmap_batch(s extent, 1)) ) return -EFAULT; - rc = xenmem_add_to_physmap_one(d, xatpb->space, + rc = notify_gfn(d, _gfn(gpfn)) ?: + xenmem_add_to_physmap_one(d, xatpb->space, xatpb->u, idx, _gfn(gpfn)); --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -522,6 +522,7 @@ int iommu_do_domctl( return ret; } +#ifndef CONFIG_IOMMU_FORCE_PT_SHARE void iommu_share_p2m_table(struct domain* d) { ASSERT(hap_enabled(d)); @@ -530,6 +531,15 @@ void iommu_share_p2m_table(struct domain iommu_get_ops()->share_p2m(d); } +int iommu_notify_gfn(struct domain *d, gfn_t gfn) +{ + const struct iommu_ops *ops = dom_iommu(d)->platform_ops; + + return need_iommu_pt_sync(d) && ops->notify_dfn + ? iommu_call(ops, notify_dfn, d, _dfn(gfn_x(gfn))) : 0; +} +#endif + void iommu_crash_shutdown(void) { if ( !iommu_crash_disable ) --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -272,6 +272,8 @@ static inline void free_vcpu_guest_conte static inline void arch_vcpu_block(struct vcpu *v) {} +#define arch_notify_gfn(d, gfn) ((void)(d), (void)(gfn), 0) + #endif /* __ASM_DOMAIN_H__ */ /* --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -647,6 +647,8 @@ static inline void free_vcpu_guest_conte void arch_vcpu_regs_init(struct vcpu *v); +#define arch_notify_gfn(d, gfn) (gfn_valid(d, gfn) ? 0 : -EADDRNOTAVAIL) + struct vcpu_hvm_context; int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context *ctx); --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -237,6 +237,11 @@ struct iommu_ops { int __must_check (*lookup_page)(struct domain *d, dfn_t dfn, mfn_t *mfn, unsigned int *flags); +#ifndef CONFIG_IOMMU_FORCE_PT_SHARE + void (*share_p2m)(struct domain *d); + int __must_check (*notify_dfn)(struct domain *d, dfn_t dfn); +#endif + void (*free_page_table)(struct page_info *); #ifdef CONFIG_X86 @@ -253,7 +258,6 @@ struct iommu_ops { int __must_check (*suspend)(void); void (*resume)(void); - void (*share_p2m)(struct domain *d); void (*crash_shutdown)(void); int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn, unsigned int page_count, @@ -330,7 +334,15 @@ void iommu_resume(void); void iommu_crash_shutdown(void); int iommu_get_reserved_device_memory(iommu_grdm_t *, void *); +#ifndef CONFIG_IOMMU_FORCE_PT_SHARE void iommu_share_p2m_table(struct domain *d); +int __must_check iommu_notify_gfn(struct domain *d, gfn_t gfn); +#else +static inline int __must_check iommu_notify_gfn(struct domain *d, gfn_t gfn) +{ + return 0; +} +#endif #ifdef CONFIG_HAS_PCI int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d, --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -1039,6 +1039,8 @@ static always_inline bool is_iommu_enabl return evaluate_nospec(d->options & XEN_DOMCTL_CDF_iommu); } +#define notify_gfn(d, gfn) (arch_notify_gfn(d, gfn) ?: iommu_notify_gfn(d, gfn)) + extern bool sched_smt_power_savings; extern bool sched_disable_smt_switching;
In order for individual IOMMU drivers (and from an abstract pov also architectures) to be able to adjust, ahead of actual mapping requests, their data structures when they might cover only a sub-range of all possible GFNs, introduce a notification call used by various code paths potentially installing a fresh mapping of a never used GFN (for a particular domain). Note that before this patch, in gnttab_transfer(), once past assign_pages(), further errors modifying the physmap are ignored (presumably because it would be too complicated to try to roll back at that point). This patch follows suit by ignoring failed notify_gfn()s or races due to the need to intermediately drop locks, simply printing out a warning that the gfn may not be accessible due to the failure. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v3: Conditionalize upon CONFIG_IOMMU_FORCE_PT_SHARE, also covering the share_p2m_table() functionality as appropriate. Un-comment the GNTMAP_host_map check. v2: Introduce arch_notify_gfn(), to invoke gfn_valid() on x86 (this unfortunately means it and notify_gfn() now need to be macros, or else include file dependencies get in the way, as gfn_valid() lives in paging.h, which we shouldn't include from xen/sched.h). Improve description. TBD: Does Arm actually have anything to check against in its arch_notify_gfn()?