* [PATCH v3 00/13] paravirtual IOMMU interface
@ 2018-07-17 13:38 Paul Durrant
2018-07-17 13:38 ` [PATCH v3 01/13] grant_table: use term 'mfn' for machine frame numbers Paul Durrant
` (12 more replies)
0 siblings, 13 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
Andrew Cooper, Ian Jackson, George Dunlap, Tim Deegan,
Julien Grall, Paul Durrant, Jan Beulich, Daniel De Graaf,
Suravee Suthikulpanit
The idea of a paravirtual IOMMU interface was last discussed on xen-devel
several years ago and narrowed down on a draft specification [1].
There was also an RFC patch series posted with an implementation, however
this was never followed through.
In this patch series I have tried to simplify the interface and therefore
have moved away from the draft specification. There is not yet any
new specification but hopefully the interface in the introduced iommu_op
header file will be understandable without such a specification.
[1] https://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01428.html
Paul Durrant (13):
grant_table: use term 'mfn' for machine frame numbers...
iommu: introduce the concept of BFN...
iommu: make use of type-safe BFN and MFN in exported functions
iommu: push use of type-safe BFN and MFN into iommu_ops
iommu: don't domain_crash() inside iommu_map/unmap_page()
public / x86: introduce __HYPERCALL_iommu_op
iommu: track reserved ranges using a rangeset
x86: add iommu_op to query reserved ranges
vtd: add lookup_page method to iommu_ops
x86: add iommu_op to enable modification of IOMMU mappings
memory: add get_paged_gfn() as a wrapper...
x86: add iommu_ops to modify and flush IOMMU mappings
x86: extend the map and unmap iommu_ops to support grant references
tools/flask/policy/modules/xen.if | 1 +
xen/arch/arm/p2m.c | 7 +-
xen/arch/x86/Makefile | 1 +
xen/arch/x86/hvm/emulate.c | 32 +-
xen/arch/x86/hvm/hvm.c | 16 +-
xen/arch/x86/hvm/hypercall.c | 1 +
xen/arch/x86/hypercall.c | 1 +
xen/arch/x86/iommu_op.c | 555 ++++++++++++++++++++++++++
xen/arch/x86/mm.c | 13 +-
xen/arch/x86/mm/p2m-ept.c | 13 +-
xen/arch/x86/mm/p2m-pt.c | 48 ++-
xen/arch/x86/mm/p2m.c | 34 +-
xen/arch/x86/pv/hypercall.c | 1 +
xen/arch/x86/x86_64/mm.c | 5 +-
xen/common/grant_table.c | 331 ++++++++++-----
xen/common/memory.c | 63 ++-
xen/drivers/passthrough/amd/iommu_cmd.c | 18 +-
xen/drivers/passthrough/amd/iommu_map.c | 86 ++--
xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +-
xen/drivers/passthrough/arm/smmu.c | 20 +-
xen/drivers/passthrough/iommu.c | 65 +--
xen/drivers/passthrough/vtd/iommu.c | 90 +++--
xen/drivers/passthrough/vtd/iommu.h | 3 +
xen/drivers/passthrough/vtd/x86/vtd.c | 23 +-
xen/drivers/passthrough/x86/iommu.c | 6 +-
xen/include/Makefile | 2 +
xen/include/asm-arm/p2m.h | 3 +
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 +-
xen/include/asm-x86/p2m.h | 2 +
xen/include/public/iommu_op.h | 171 ++++++++
xen/include/public/xen.h | 1 +
xen/include/xen/grant_table.h | 7 +
xen/include/xen/hypercall.h | 12 +
xen/include/xen/iommu.h | 66 ++-
xen/include/xen/mm.h | 10 +-
xen/include/xlat.lst | 6 +
xen/include/xsm/dummy.h | 6 +
xen/include/xsm/xsm.h | 6 +
xen/xsm/dummy.c | 1 +
xen/xsm/flask/hooks.c | 6 +
xen/xsm/flask/policy/access_vectors | 2 +
41 files changed, 1429 insertions(+), 317 deletions(-)
create mode 100644 xen/arch/x86/iommu_op.c
create mode 100644 xen/include/public/iommu_op.h
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 01/13] grant_table: use term 'mfn' for machine frame numbers...
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-17 13:38 ` [PATCH v3 02/13] iommu: introduce the concept of BFN Paul Durrant
` (11 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich
...rather than more ambiguous term 'frame'.
There are many places in the grant table code that use a variable or
field name '.*frame' to refer to a quantity that is strictly an MFN, and
even has type mfn_t.
This patch is a purely cosmetic patch that substitutes 'frame' with 'mfn'
in those places to make the purpose of the variable or field name more
obvious.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: George Dunlap <George.Dunlap@eu.citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v2:
- New in v2.
---
xen/common/grant_table.c | 142 ++++++++++++++++++++++++-----------------------
1 file changed, 72 insertions(+), 70 deletions(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 51e1f5ca4c..d2610e320c 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -167,7 +167,7 @@ struct gnttab_unmap_common {
/* Shared state beteen *_unmap and *_unmap_complete */
uint16_t done;
- mfn_t frame;
+ mfn_t mfn;
struct domain *rd;
grant_ref_t ref;
};
@@ -266,7 +266,7 @@ struct active_grant_entry {
grant. */
grant_ref_t trans_gref;
struct domain *trans_domain;
- mfn_t frame; /* Frame being granted. */
+ mfn_t mfn; /* Machine frame being granted. */
#ifndef NDEBUG
gfn_t gfn; /* Guest's idea of the frame being granted. */
#endif
@@ -370,15 +370,15 @@ static inline unsigned int grant_to_status_frames(unsigned int grant_frames)
/* Check if the page has been paged out, or needs unsharing.
If rc == GNTST_okay, *page contains the page struct with a ref taken.
Caller must do put_page(*page).
- If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */
-static int get_paged_frame(unsigned long gfn, mfn_t *frame,
+ If any error, *page = NULL, *mfn = INVALID_MFN, no ref taken. */
+static int get_paged_frame(unsigned long gfn, mfn_t *mfn,
struct page_info **page, bool readonly,
struct domain *rd)
{
int rc = GNTST_okay;
p2m_type_t p2mt;
- *frame = INVALID_MFN;
+ *mfn = INVALID_MFN;
*page = get_page_from_gfn(rd, gfn, &p2mt,
readonly ? P2M_ALLOC : P2M_UNSHARE);
if ( !*page )
@@ -405,7 +405,7 @@ static int get_paged_frame(unsigned long gfn, mfn_t *frame,
return GNTST_bad_page;
}
- *frame = page_to_mfn(*page);
+ *mfn = page_to_mfn(*page);
return rc;
}
@@ -843,7 +843,7 @@ static struct active_grant_entry *grant_map_exists(const struct domain *ld,
struct active_grant_entry *act = active_entry_acquire(rgt, ref);
if ( act->pin && act->domid == ld->domain_id &&
- mfn_eq(act->frame, mfn) )
+ mfn_eq(act->mfn, mfn) )
return act;
active_entry_release(act);
}
@@ -885,7 +885,7 @@ static unsigned int mapkind(
if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) ||
map->domid != rd->domain_id )
continue;
- if ( mfn_eq(_active_entry(rd->grant_table, map->ref).frame, mfn) )
+ if ( mfn_eq(_active_entry(rd->grant_table, map->ref).mfn, mfn) )
kind |= map->flags & GNTMAP_readonly ?
MAPKIND_READ : MAPKIND_WRITE;
}
@@ -908,7 +908,7 @@ map_grant_ref(
struct grant_table *lgt, *rgt;
struct vcpu *led;
grant_handle_t handle;
- mfn_t frame;
+ mfn_t mfn;
struct page_info *pg = NULL;
int rc = GNTST_okay;
u32 old_pin;
@@ -1001,13 +1001,13 @@ map_grant_ref(
shared_entry_v1(rgt, op->ref).frame :
shared_entry_v2(rgt, op->ref).full_page.frame;
- rc = get_paged_frame(gfn, &frame, &pg,
+ rc = get_paged_frame(gfn, &mfn, &pg,
op->flags & GNTMAP_readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out_clear;
act_set_gfn(act, _gfn(gfn));
act->domid = ld->domain_id;
- act->frame = frame;
+ act->mfn = mfn;
act->start = 0;
act->length = PAGE_SIZE;
act->is_sub_page = false;
@@ -1024,7 +1024,7 @@ map_grant_ref(
act->pin += (op->flags & GNTMAP_readonly) ?
GNTPIN_hstr_inc : GNTPIN_hstw_inc;
- frame = act->frame;
+ mfn = act->mfn;
act_pin = act->pin;
cache_flags = (shah->flags & (GTF_PAT | GTF_PWT | GTF_PCD) );
@@ -1035,7 +1035,7 @@ map_grant_ref(
/* pg may be set, with a refcount included, from get_paged_frame(). */
if ( !pg )
{
- pg = mfn_valid(frame) ? mfn_to_page(frame) : NULL;
+ pg = mfn_valid(mfn) ? mfn_to_page(mfn) : NULL;
if ( pg )
owner = page_get_owner_and_reference(pg);
}
@@ -1061,18 +1061,18 @@ map_grant_ref(
goto undo_out;
}
- if ( !iomem_access_permitted(rd, mfn_x(frame), mfn_x(frame)) )
+ if ( !iomem_access_permitted(rd, mfn_x(mfn), mfn_x(mfn)) )
{
gdprintk(XENLOG_WARNING,
"Iomem mapping not permitted %#"PRI_mfn" (domain %d)\n",
- mfn_x(frame), rd->domain_id);
+ mfn_x(mfn), rd->domain_id);
rc = GNTST_general_error;
goto undo_out;
}
if ( op->flags & GNTMAP_host_map )
{
- rc = create_grant_host_mapping(op->host_addr, frame, op->flags,
+ rc = create_grant_host_mapping(op->host_addr, mfn, op->flags,
cache_flags);
if ( rc != GNTST_okay )
goto undo_out;
@@ -1112,7 +1112,7 @@ map_grant_ref(
typecnt++;
}
- rc = create_grant_host_mapping(op->host_addr, frame, op->flags, 0);
+ rc = create_grant_host_mapping(op->host_addr, mfn, op->flags, 0);
if ( rc != GNTST_okay )
goto undo_out;
@@ -1124,7 +1124,7 @@ map_grant_ref(
could_not_pin:
if ( !rd->is_dying )
gdprintk(XENLOG_WARNING, "Could not pin grant frame %#"PRI_mfn"\n",
- mfn_x(frame));
+ mfn_x(mfn));
rc = GNTST_general_error;
goto undo_out;
}
@@ -1139,18 +1139,18 @@ map_grant_ref(
/* We're not translated, so we know that gmfns and mfns are
the same things, so the IOMMU entry is always 1-to-1. */
- kind = mapkind(lgt, rd, frame);
+ kind = mapkind(lgt, rd, mfn);
if ( (act_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) &&
!(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
{
if ( !(kind & MAPKIND_WRITE) )
- err = iommu_map_page(ld, mfn_x(frame), mfn_x(frame),
+ err = iommu_map_page(ld, mfn_x(mfn), mfn_x(mfn),
IOMMUF_readable|IOMMUF_writable);
}
else if ( act_pin && !old_pin )
{
if ( !kind )
- err = iommu_map_page(ld, mfn_x(frame), mfn_x(frame),
+ err = iommu_map_page(ld, mfn_x(mfn), mfn_x(mfn),
IOMMUF_readable);
}
if ( err )
@@ -1180,7 +1180,7 @@ map_grant_ref(
if ( need_iommu )
double_gt_unlock(lgt, rgt);
- op->dev_bus_addr = mfn_to_maddr(frame);
+ op->dev_bus_addr = mfn_to_maddr(mfn);
op->handle = handle;
op->status = GNTST_okay;
@@ -1190,7 +1190,7 @@ map_grant_ref(
undo_out:
if ( host_map_created )
{
- replace_grant_host_mapping(op->host_addr, frame, 0, op->flags);
+ replace_grant_host_mapping(op->host_addr, mfn, 0, op->flags);
gnttab_flush_tlb(ld);
}
@@ -1365,18 +1365,18 @@ unmap_common(
goto act_release_out;
}
- op->frame = act->frame;
+ op->mfn = act->mfn;
if ( op->dev_bus_addr &&
- unlikely(op->dev_bus_addr != mfn_to_maddr(act->frame)) )
+ unlikely(op->dev_bus_addr != mfn_to_maddr(act->mfn)) )
PIN_FAIL(act_release_out, GNTST_general_error,
"Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n",
- op->dev_bus_addr, mfn_to_maddr(act->frame));
+ op->dev_bus_addr, mfn_to_maddr(act->mfn));
if ( op->host_addr && (flags & GNTMAP_host_map) )
{
if ( (rc = replace_grant_host_mapping(op->host_addr,
- op->frame, op->new_addr,
+ op->mfn, op->new_addr,
flags)) < 0 )
goto act_release_out;
@@ -1411,12 +1411,12 @@ unmap_common(
double_gt_lock(lgt, rgt);
- kind = mapkind(lgt, rd, op->frame);
+ kind = mapkind(lgt, rd, op->mfn);
if ( !kind )
- err = iommu_unmap_page(ld, mfn_x(op->frame));
+ err = iommu_unmap_page(ld, mfn_x(op->mfn));
else if ( !(kind & MAPKIND_WRITE) )
- err = iommu_map_page(ld, mfn_x(op->frame),
- mfn_x(op->frame), IOMMUF_readable);
+ err = iommu_map_page(ld, mfn_x(op->mfn),
+ mfn_x(op->mfn), IOMMUF_readable);
double_gt_unlock(lgt, rgt);
@@ -1426,7 +1426,7 @@ unmap_common(
/* If just unmapped a writable mapping, mark as dirtied */
if ( rc == GNTST_okay && !(flags & GNTMAP_readonly) )
- gnttab_mark_dirty(rd, op->frame);
+ gnttab_mark_dirty(rd, op->mfn);
op->status = rc;
rcu_unlock_domain(rd);
@@ -1463,11 +1463,11 @@ unmap_common_complete(struct gnttab_unmap_common *op)
else
status = &status_entry(rgt, op->ref);
- pg = mfn_to_page(op->frame);
+ pg = mfn_to_page(op->mfn);
if ( op->done & GNTMAP_device_map )
{
- if ( !is_iomem_page(act->frame) )
+ if ( !is_iomem_page(act->mfn) )
{
if ( op->done & GNTMAP_readonly )
put_page(pg);
@@ -1484,7 +1484,7 @@ unmap_common_complete(struct gnttab_unmap_common *op)
if ( op->done & GNTMAP_host_map )
{
- if ( !is_iomem_page(op->frame) )
+ if ( !is_iomem_page(op->mfn) )
{
if ( gnttab_host_mapping_get_page_type(op->done & GNTMAP_readonly,
ld, rd) )
@@ -1525,7 +1525,7 @@ unmap_grant_ref(
common->done = 0;
common->new_addr = 0;
common->rd = NULL;
- common->frame = INVALID_MFN;
+ common->mfn = INVALID_MFN;
unmap_common(common);
op->status = common->status;
@@ -1591,7 +1591,7 @@ unmap_and_replace(
common->done = 0;
common->dev_bus_addr = 0;
common->rd = NULL;
- common->frame = INVALID_MFN;
+ common->mfn = INVALID_MFN;
unmap_common(common);
op->status = common->status;
@@ -2320,7 +2320,7 @@ release_grant_for_copy(
struct grant_table *rgt = rd->grant_table;
grant_entry_header_t *sha;
struct active_grant_entry *act;
- mfn_t r_frame;
+ mfn_t mfn;
uint16_t *status;
grant_ref_t trans_gref;
struct domain *td;
@@ -2329,7 +2329,7 @@ release_grant_for_copy(
act = active_entry_acquire(rgt, gref);
sha = shared_entry_header(rgt, gref);
- r_frame = act->frame;
+ mfn = act->mfn;
if ( rgt->gt_version == 1 )
{
@@ -2350,7 +2350,7 @@ release_grant_for_copy(
}
else
{
- gnttab_mark_dirty(rd, r_frame);
+ gnttab_mark_dirty(rd, mfn);
act->pin -= GNTPIN_hstw_inc;
if ( !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) )
@@ -2390,15 +2390,17 @@ static void fixup_status_for_copy_pin(const struct active_grant_entry *act,
gnttab_clear_flag(_GTF_reading, status);
}
-/* Grab a frame number from a grant entry and update the flags and pin
- count as appropriate. If rc == GNTST_okay, note that this *does*
- take one ref count on the target page, stored in *page.
- If there is any error, *page = NULL, no ref taken. */
+/*
+ * Grab a machine frame number from a grant entry and update the flags
+ * and pin count as appropriate. If rc == GNTST_okay, note that this *does*
+ * take one ref count on the target page, stored in *page.
+ * If there is any error, *page = NULL, no ref taken.
+ */
static int
acquire_grant_for_copy(
struct domain *rd, grant_ref_t gref, domid_t ldom, bool readonly,
- mfn_t *frame, struct page_info **page,
- uint16_t *page_off, uint16_t *length, bool allow_transitive)
+ mfn_t *mfn, struct page_info **page, uint16_t *page_off,
+ uint16_t *length, bool allow_transitive)
{
struct grant_table *rgt = rd->grant_table;
grant_entry_v2_t *sha2;
@@ -2409,7 +2411,7 @@ acquire_grant_for_copy(
domid_t trans_domid;
grant_ref_t trans_gref;
struct domain *td;
- mfn_t grant_frame;
+ mfn_t grant_mfn;
uint16_t trans_page_off;
uint16_t trans_length;
bool is_sub_page;
@@ -2487,7 +2489,7 @@ acquire_grant_for_copy(
grant_read_unlock(rgt);
rc = acquire_grant_for_copy(td, trans_gref, rd->domain_id,
- readonly, &grant_frame, page,
+ readonly, &grant_mfn, page,
&trans_page_off, &trans_length,
false);
@@ -2511,7 +2513,7 @@ acquire_grant_for_copy(
if ( rgt->gt_version != 2 ||
act->pin != old_pin ||
(old_pin && (act->domid != ldom ||
- !mfn_eq(act->frame, grant_frame) ||
+ !mfn_eq(act->mfn, grant_mfn) ||
act->start != trans_page_off ||
act->length != trans_length ||
act->trans_domain != td ||
@@ -2535,7 +2537,7 @@ acquire_grant_for_copy(
act->length = trans_length;
act->trans_domain = td;
act->trans_gref = trans_gref;
- act->frame = grant_frame;
+ act->mfn = grant_mfn;
act_set_gfn(act, INVALID_GFN);
/*
* The actual remote remote grant may or may not be a sub-page,
@@ -2559,7 +2561,7 @@ acquire_grant_for_copy(
{
unsigned long gfn = shared_entry_v1(rgt, gref).frame;
- rc = get_paged_frame(gfn, &grant_frame, page, readonly, rd);
+ rc = get_paged_frame(gfn, &grant_mfn, page, readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out_clear;
act_set_gfn(act, _gfn(gfn));
@@ -2569,7 +2571,7 @@ acquire_grant_for_copy(
}
else if ( !(sha2->hdr.flags & GTF_sub_page) )
{
- rc = get_paged_frame(sha2->full_page.frame, &grant_frame, page,
+ rc = get_paged_frame(sha2->full_page.frame, &grant_mfn, page,
readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out_clear;
@@ -2580,7 +2582,7 @@ acquire_grant_for_copy(
}
else
{
- rc = get_paged_frame(sha2->sub_page.frame, &grant_frame, page,
+ rc = get_paged_frame(sha2->sub_page.frame, &grant_mfn, page,
readonly, rd);
if ( rc != GNTST_okay )
goto unlock_out_clear;
@@ -2598,13 +2600,13 @@ acquire_grant_for_copy(
act->length = trans_length;
act->trans_domain = td;
act->trans_gref = trans_gref;
- act->frame = grant_frame;
+ act->mfn = grant_mfn;
}
}
else
{
- ASSERT(mfn_valid(act->frame));
- *page = mfn_to_page(act->frame);
+ ASSERT(mfn_valid(act->mfn));
+ *page = mfn_to_page(act->mfn);
td = page_get_owner_and_reference(*page);
/*
* act->pin being non-zero should guarantee the page to have a
@@ -2628,7 +2630,7 @@ acquire_grant_for_copy(
*page_off = act->start;
*length = act->length;
- *frame = act->frame;
+ *mfn = act->mfn;
active_entry_release(act);
grant_read_unlock(rgt);
@@ -2658,7 +2660,7 @@ struct gnttab_copy_buf {
/* Mapped etc. */
struct domain *domain;
- mfn_t frame;
+ mfn_t mfn;
struct page_info *page;
void *virt;
bool_t read_only;
@@ -2764,7 +2766,7 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy *op,
rc = acquire_grant_for_copy(buf->domain, ptr->u.ref,
current->domain->domain_id,
buf->read_only,
- &buf->frame, &buf->page,
+ &buf->mfn, &buf->page,
&buf->ptr.offset, &buf->len,
opt_transitive_grants);
if ( rc != GNTST_okay )
@@ -2774,7 +2776,7 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy *op,
}
else
{
- rc = get_paged_frame(ptr->u.gmfn, &buf->frame, &buf->page,
+ rc = get_paged_frame(ptr->u.gmfn, &buf->mfn, &buf->page,
buf->read_only, buf->domain);
if ( rc != GNTST_okay )
PIN_FAIL(out, rc,
@@ -2792,14 +2794,14 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy *op,
if ( !buf->domain->is_dying )
gdprintk(XENLOG_WARNING,
"Could not get writable frame %#"PRI_mfn"\n",
- mfn_x(buf->frame));
+ mfn_x(buf->mfn));
rc = GNTST_general_error;
goto out;
}
buf->have_type = 1;
}
- buf->virt = map_domain_page(buf->frame);
+ buf->virt = map_domain_page(buf->mfn);
rc = GNTST_okay;
out:
@@ -2843,7 +2845,7 @@ static int gnttab_copy_buf(const struct gnttab_copy *op,
memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset,
op->len);
- gnttab_mark_dirty(dest->domain, dest->frame);
+ gnttab_mark_dirty(dest->domain, dest->mfn);
rc = GNTST_okay;
out:
return rc;
@@ -3661,7 +3663,7 @@ gnttab_release_mappings(
else
status = &status_entry(rgt, ref);
- pg = mfn_to_page(act->frame);
+ pg = mfn_to_page(act->mfn);
if ( map->flags & GNTMAP_readonly )
{
@@ -3669,7 +3671,7 @@ gnttab_release_mappings(
{
BUG_ON(!(act->pin & GNTPIN_devr_mask));
act->pin -= GNTPIN_devr_inc;
- if ( !is_iomem_page(act->frame) )
+ if ( !is_iomem_page(act->mfn) )
put_page(pg);
}
@@ -3678,7 +3680,7 @@ gnttab_release_mappings(
BUG_ON(!(act->pin & GNTPIN_hstr_mask));
act->pin -= GNTPIN_hstr_inc;
if ( gnttab_release_host_mappings(d) &&
- !is_iomem_page(act->frame) )
+ !is_iomem_page(act->mfn) )
put_page(pg);
}
}
@@ -3688,7 +3690,7 @@ gnttab_release_mappings(
{
BUG_ON(!(act->pin & GNTPIN_devw_mask));
act->pin -= GNTPIN_devw_inc;
- if ( !is_iomem_page(act->frame) )
+ if ( !is_iomem_page(act->mfn) )
put_page_and_type(pg);
}
@@ -3697,7 +3699,7 @@ gnttab_release_mappings(
BUG_ON(!(act->pin & GNTPIN_hstw_mask));
act->pin -= GNTPIN_hstw_inc;
if ( gnttab_release_host_mappings(d) &&
- !is_iomem_page(act->frame) )
+ !is_iomem_page(act->mfn) )
{
if ( gnttab_host_mapping_get_page_type((map->flags &
GNTMAP_readonly),
@@ -3754,7 +3756,7 @@ void grant_table_warn_active_grants(struct domain *d)
#ifndef NDEBUG
gfn_x(act->gfn),
#endif
- mfn_x(act->frame));
+ mfn_x(act->mfn));
active_entry_release(act);
}
@@ -3963,7 +3965,7 @@ static void gnttab_usage_print(struct domain *rd)
/* [0xXXX] ddddd 0xXXXXX 0xXXXXXXXX ddddd 0xXXXXXX 0xXX */
printk("[0x%03x] %5d 0x%"PRI_mfn" 0x%08x %5d 0x%06"PRIx64" 0x%02x\n",
- ref, act->domid, mfn_x(act->frame), act->pin,
+ ref, act->domid, mfn_x(act->mfn), act->pin,
sha->domid, frame, status);
active_entry_release(act);
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 02/13] iommu: introduce the concept of BFN...
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
2018-07-17 13:38 ` [PATCH v3 01/13] grant_table: use term 'mfn' for machine frame numbers Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-19 8:08 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 03/13] iommu: make use of type-safe BFN and MFN in exported functions Paul Durrant
` (10 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Suravee Suthikulpanit,
Julien Grall, Paul Durrant, Jan Beulich
...meaning 'bus frame number' i.e. a frame number mapped in the IOMMU
rather than the MMU.
This patch is a largely cosmetic change that substitutes the terms 'gfn'
and 'gaddr' for 'bfn' and 'baddr' in all the places where the frame number
or address relate to the IOMMU rather than the MMU.
The parts that are not purely cosmetic are:
- the introduction of a type-safe declaration of bfn_t and definition of
INVALID_BFN to make the substitution of gfn_x(INVALID_GFN) mechanical.
- the introduction of __bfn_to_baddr and __baddr_to_bfn (and type-safe
variants without the leading __) with some use of the former.
Subsequent patches will convert code to make use of type-safe BFNs.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Kevin Tian <kevin.tian@intel.com>
v3:
- Get rid of intermediate 'frame' variables again.
v2:
- Addressed comments from Jan.
---
xen/drivers/passthrough/amd/iommu_cmd.c | 18 +++----
xen/drivers/passthrough/amd/iommu_map.c | 76 ++++++++++++++---------------
xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 16 +++---
xen/drivers/passthrough/iommu.c | 28 +++++------
xen/drivers/passthrough/vtd/iommu.c | 30 ++++++------
xen/include/xen/iommu.h | 38 ++++++++++++---
xen/include/xen/mm.h | 10 +++-
8 files changed, 125 insertions(+), 93 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
index 08247fa354..f93becd6e1 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -284,7 +284,7 @@ void invalidate_iommu_all(struct amd_iommu *iommu)
}
void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
- uint64_t gaddr, unsigned int order)
+ baddr_t baddr, unsigned int order)
{
unsigned long flags;
struct amd_iommu *iommu;
@@ -315,12 +315,12 @@ void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
/* send INVALIDATE_IOTLB_PAGES command */
spin_lock_irqsave(&iommu->lock, flags);
- invalidate_iotlb_pages(iommu, maxpend, 0, queueid, gaddr, req_id, order);
+ invalidate_iotlb_pages(iommu, maxpend, 0, queueid, baddr, req_id, order);
flush_command_buffer(iommu);
spin_unlock_irqrestore(&iommu->lock, flags);
}
-static void amd_iommu_flush_all_iotlbs(struct domain *d, uint64_t gaddr,
+static void amd_iommu_flush_all_iotlbs(struct domain *d, baddr_t baddr,
unsigned int order)
{
struct pci_dev *pdev;
@@ -333,7 +333,7 @@ static void amd_iommu_flush_all_iotlbs(struct domain *d, uint64_t gaddr,
u8 devfn = pdev->devfn;
do {
- amd_iommu_flush_iotlb(devfn, pdev, gaddr, order);
+ amd_iommu_flush_iotlb(devfn, pdev, baddr, order);
devfn += pdev->phantom_stride;
} while ( devfn != pdev->devfn &&
PCI_SLOT(devfn) == PCI_SLOT(pdev->devfn) );
@@ -342,7 +342,7 @@ static void amd_iommu_flush_all_iotlbs(struct domain *d, uint64_t gaddr,
/* Flush iommu cache after p2m changes. */
static void _amd_iommu_flush_pages(struct domain *d,
- uint64_t gaddr, unsigned int order)
+ baddr_t baddr, unsigned int order)
{
unsigned long flags;
struct amd_iommu *iommu;
@@ -352,13 +352,13 @@ static void _amd_iommu_flush_pages(struct domain *d,
for_each_amd_iommu ( iommu )
{
spin_lock_irqsave(&iommu->lock, flags);
- invalidate_iommu_pages(iommu, gaddr, dom_id, order);
+ invalidate_iommu_pages(iommu, baddr, dom_id, order);
flush_command_buffer(iommu);
spin_unlock_irqrestore(&iommu->lock, flags);
}
if ( ats_enabled )
- amd_iommu_flush_all_iotlbs(d, gaddr, order);
+ amd_iommu_flush_all_iotlbs(d, baddr, order);
}
void amd_iommu_flush_all_pages(struct domain *d)
@@ -367,9 +367,9 @@ void amd_iommu_flush_all_pages(struct domain *d)
}
void amd_iommu_flush_pages(struct domain *d,
- unsigned long gfn, unsigned int order)
+ unsigned long bfn, unsigned int order)
{
- _amd_iommu_flush_pages(d, (uint64_t) gfn << PAGE_SHIFT, order);
+ _amd_iommu_flush_pages(d, __bfn_to_baddr(bfn), order);
}
void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index 70b4345b37..4deab9cd2f 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -35,12 +35,12 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn, unsigned int level)
return idx;
}
-void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long gfn)
+void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long bfn)
{
u64 *table, *pte;
table = map_domain_page(_mfn(l1_mfn));
- pte = table + pfn_to_pde_idx(gfn, IOMMU_PAGING_MODE_LEVEL_1);
+ pte = table + pfn_to_pde_idx(bfn, IOMMU_PAGING_MODE_LEVEL_1);
*pte = 0;
unmap_domain_page(table);
}
@@ -104,7 +104,7 @@ static bool_t set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
return need_flush;
}
-static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long gfn,
+static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long bfn,
unsigned long next_mfn, int pde_level,
bool_t iw, bool_t ir)
{
@@ -114,7 +114,7 @@ static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long gfn,
table = map_domain_page(_mfn(pt_mfn));
- pde = (u32*)(table + pfn_to_pde_idx(gfn, pde_level));
+ pde = (u32*)(table + pfn_to_pde_idx(bfn, pde_level));
need_flush = set_iommu_pde_present(pde, next_mfn,
IOMMU_PAGING_MODE_LEVEL_0, iw, ir);
@@ -331,7 +331,7 @@ static void set_pde_count(u64 *pde, unsigned int count)
* otherwise increase pde count if mfn is contigous with mfn - 1
*/
static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
- unsigned long gfn, unsigned long mfn,
+ unsigned long bfn, unsigned long mfn,
unsigned int merge_level)
{
unsigned int pde_count, next_level;
@@ -347,7 +347,7 @@ static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
/* get pde at merge level */
table = map_domain_page(_mfn(pt_mfn));
- pde = table + pfn_to_pde_idx(gfn, merge_level);
+ pde = table + pfn_to_pde_idx(bfn, merge_level);
/* get page table of next level */
ntable_maddr = amd_iommu_get_next_table_from_pte((u32*)pde);
@@ -362,7 +362,7 @@ static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
mask = (1ULL<< (PTE_PER_TABLE_SHIFT * next_level)) - 1;
if ( ((first_mfn & mask) == 0) &&
- (((gfn & mask) | first_mfn) == mfn) )
+ (((bfn & mask) | first_mfn) == mfn) )
{
pde_count = get_pde_count(*pde);
@@ -387,7 +387,7 @@ out:
}
static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
- unsigned long gfn, unsigned int flags,
+ unsigned long bfn, unsigned int flags,
unsigned int merge_level)
{
u64 *table, *pde, *ntable;
@@ -398,7 +398,7 @@ static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
ASSERT( spin_is_locked(&hd->arch.mapping_lock) && pt_mfn );
table = map_domain_page(_mfn(pt_mfn));
- pde = table + pfn_to_pde_idx(gfn, merge_level);
+ pde = table + pfn_to_pde_idx(bfn, merge_level);
/* get first mfn */
ntable_mfn = amd_iommu_get_next_table_from_pte((u32*)pde) >> PAGE_SHIFT;
@@ -436,7 +436,7 @@ static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
* {Re, un}mapping super page frames causes re-allocation of io
* page tables.
*/
-static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
+static int iommu_pde_from_bfn(struct domain *d, unsigned long pfn,
unsigned long pt_mfn[])
{
u64 *pde, *next_table_vaddr;
@@ -477,11 +477,11 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
next_table_mfn != 0 )
{
int i;
- unsigned long mfn, gfn;
+ unsigned long mfn, bfn;
unsigned int page_sz;
page_sz = 1 << (PTE_PER_TABLE_SHIFT * (next_level - 1));
- gfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
+ bfn = pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
mfn = next_table_mfn;
/* allocate lower level page table */
@@ -499,10 +499,10 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
for ( i = 0; i < PTE_PER_TABLE_SIZE; i++ )
{
- set_iommu_pte_present(next_table_mfn, gfn, mfn, next_level,
+ set_iommu_pte_present(next_table_mfn, bfn, mfn, next_level,
!!IOMMUF_writable, !!IOMMUF_readable);
mfn += page_sz;
- gfn += page_sz;
+ bfn += page_sz;
}
amd_iommu_flush_all_pages(d);
@@ -540,7 +540,7 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
return 0;
}
-static int update_paging_mode(struct domain *d, unsigned long gfn)
+static int update_paging_mode(struct domain *d, unsigned long bfn)
{
u16 bdf;
void *device_entry;
@@ -554,13 +554,13 @@ static int update_paging_mode(struct domain *d, unsigned long gfn)
unsigned long old_root_mfn;
struct domain_iommu *hd = dom_iommu(d);
- if ( gfn == gfn_x(INVALID_GFN) )
+ if ( bfn == bfn_x(INVALID_BFN) )
return -EADDRNOTAVAIL;
- ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
+ ASSERT(!(bfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
level = hd->arch.paging_mode;
old_root = hd->arch.root_table;
- offset = gfn >> (PTE_PER_TABLE_SHIFT * (level - 1));
+ offset = bfn >> (PTE_PER_TABLE_SHIFT * (level - 1));
ASSERT(spin_is_locked(&hd->arch.mapping_lock) && is_hvm_domain(d));
@@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d, unsigned long gfn)
return 0;
}
-int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
+int amd_iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
unsigned int flags)
{
bool_t need_flush = 0;
@@ -651,34 +651,34 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Root table alloc failed, gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Root table alloc failed, bfn = %lx\n", bfn);
domain_crash(d);
return rc;
}
/* Since HVM domain is initialized with 2 level IO page table,
- * we might need a deeper page table for lager gfn now */
+ * we might need a deeper page table for wider bfn now */
if ( is_hvm_domain(d) )
{
- if ( update_paging_mode(d, gfn) )
+ if ( update_paging_mode(d, bfn) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Update page mode failed bfn = %lx\n", bfn);
domain_crash(d);
return -EFAULT;
}
}
- if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_bfn(d, bfn, pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry bfn = %lx\n", bfn);
domain_crash(d);
return -EFAULT;
}
/* Install 4k mapping first */
- need_flush = set_iommu_pte_present(pt_mfn[1], gfn, mfn,
+ need_flush = set_iommu_pte_present(pt_mfn[1], bfn, mfn,
IOMMU_PAGING_MODE_LEVEL_1,
!!(flags & IOMMUF_writable),
!!(flags & IOMMUF_readable));
@@ -690,7 +690,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
/* 4K mapping for PV guests never changes,
* no need to flush if we trust non-present bits */
if ( is_hvm_domain(d) )
- amd_iommu_flush_pages(d, gfn, 0);
+ amd_iommu_flush_pages(d, bfn, 0);
for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
merge_level <= hd->arch.paging_mode; merge_level++ )
@@ -698,15 +698,15 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
if ( pt_mfn[merge_level] == 0 )
break;
if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
- gfn, mfn, merge_level) )
+ bfn, mfn, merge_level) )
break;
- if ( iommu_merge_pages(d, pt_mfn[merge_level], gfn,
+ if ( iommu_merge_pages(d, pt_mfn[merge_level], bfn,
flags, merge_level) )
{
spin_unlock(&hd->arch.mapping_lock);
AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
- "gfn = %lx mfn = %lx\n", merge_level, gfn, mfn);
+ "bfn = %lx mfn = %lx\n", merge_level, bfn, mfn);
domain_crash(d);
return -EFAULT;
}
@@ -720,7 +720,7 @@ out:
return 0;
}
-int amd_iommu_unmap_page(struct domain *d, unsigned long gfn)
+int amd_iommu_unmap_page(struct domain *d, unsigned long bfn)
{
unsigned long pt_mfn[7];
struct domain_iommu *hd = dom_iommu(d);
@@ -739,34 +739,34 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long gfn)
}
/* Since HVM domain is initialized with 2 level IO page table,
- * we might need a deeper page table for lager gfn now */
+ * we might need a deeper page table for lager bfn now */
if ( is_hvm_domain(d) )
{
- int rc = update_paging_mode(d, gfn);
+ int rc = update_paging_mode(d, bfn);
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Update page mode failed bfn = %lx\n", bfn);
if ( rc != -EADDRNOTAVAIL )
domain_crash(d);
return rc;
}
}
- if ( iommu_pde_from_gfn(d, gfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_bfn(d, bfn, pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry gfn = %lx\n", gfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry bfn = %lx\n", bfn);
domain_crash(d);
return -EFAULT;
}
/* mark PTE as 'page not present' */
- clear_iommu_pte_present(pt_mfn[1], gfn);
+ clear_iommu_pte_present(pt_mfn[1], bfn);
spin_unlock(&hd->arch.mapping_lock);
- amd_iommu_flush_pages(d, gfn, 0);
+ amd_iommu_flush_pages(d, bfn, 0);
return 0;
}
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 12d2695b89..d608631e6e 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -578,7 +578,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, int level,
maddr_to_page(next_table_maddr), next_level,
address, indent + 1);
else
- printk("%*sgfn: %08lx mfn: %08lx\n",
+ printk("%*sbfn: %08lx mfn: %08lx\n",
indent, "",
(unsigned long)PFN_DOWN(address),
(unsigned long)PFN_DOWN(next_table_maddr));
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 74c09b0991..1e4d561b47 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2551,7 +2551,7 @@ static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
}
static int __must_check arm_smmu_iotlb_flush(struct domain *d,
- unsigned long gfn,
+ unsigned long bfn,
unsigned int page_count)
{
/* ARM SMMU v1 doesn't have flush by VMA and VMID */
@@ -2737,7 +2737,7 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
xfree(xen_domain);
}
-static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
+static int __must_check arm_smmu_map_page(struct domain *d, unsigned long bfn,
unsigned long mfn, unsigned int flags)
{
p2m_type_t t;
@@ -2748,10 +2748,10 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
* protected by an IOMMU, Xen needs to add a 1:1 mapping in the domain
* p2m to allow DMA request to work.
* This is only valid when the domain is directed mapped. Hence this
- * function should only be used by gnttab code with gfn == mfn.
+ * function should only be used by gnttab code with gfn == mfn == bfn.
*/
BUG_ON(!is_domain_direct_mapped(d));
- BUG_ON(mfn != gfn);
+ BUG_ON(mfn != bfn);
/* We only support readable and writable flags */
if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
@@ -2763,19 +2763,19 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
* The function guest_physmap_add_entry replaces the current mapping
* if there is already one...
*/
- return guest_physmap_add_entry(d, _gfn(gfn), _mfn(mfn), 0, t);
+ return guest_physmap_add_entry(d, _gfn(bfn), _mfn(bfn), 0, t);
}
-static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
+static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long bfn)
{
/*
* This function should only be used by gnttab code when the domain
- * is direct mapped
+ * is direct mapped (i.e. gfn == mfn == bfn).
*/
if ( !is_domain_direct_mapped(d) )
return -EINVAL;
- return guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
+ return guest_physmap_remove_page(d, _gfn(bfn), _mfn(bfn), 0);
}
static const struct iommu_ops arm_smmu_iommu_ops = {
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 2c44fabf99..46db9d0d04 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -185,7 +185,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
page_list_for_each ( page, &d->page_list )
{
unsigned long mfn = mfn_x(page_to_mfn(page));
- unsigned long gfn = mfn_to_gmfn(d, mfn);
+ unsigned long bfn = mfn_to_gmfn(d, mfn);
unsigned int mapping = IOMMUF_readable;
int ret;
@@ -194,7 +194,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
== PGT_writable_page) )
mapping |= IOMMUF_writable;
- ret = hd->platform_ops->map_page(d, gfn, mfn, mapping);
+ ret = hd->platform_ops->map_page(d, bfn, mfn, mapping);
if ( !rc )
rc = ret;
@@ -255,7 +255,7 @@ void iommu_domain_destroy(struct domain *d)
arch_iommu_domain_destroy(d);
}
-int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
+int iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
unsigned int flags)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -264,13 +264,13 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->map_page(d, gfn, mfn, flags);
+ rc = hd->platform_ops->map_page(d, bfn, mfn, flags);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU mapping gfn %#lx to mfn %#lx failed: %d\n",
- d->domain_id, gfn, mfn, rc);
+ "d%d: IOMMU mapping bfn %#lx to mfn %#lx failed: %d\n",
+ d->domain_id, bfn, mfn, rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -279,7 +279,7 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
return rc;
}
-int iommu_unmap_page(struct domain *d, unsigned long gfn)
+int iommu_unmap_page(struct domain *d, unsigned long bfn)
{
const struct domain_iommu *hd = dom_iommu(d);
int rc;
@@ -287,13 +287,13 @@ int iommu_unmap_page(struct domain *d, unsigned long gfn)
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->unmap_page(d, gfn);
+ rc = hd->platform_ops->unmap_page(d, bfn);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU unmapping gfn %#lx failed: %d\n",
- d->domain_id, gfn, rc);
+ "d%d: IOMMU unmapping bfn %#lx failed: %d\n",
+ d->domain_id, bfn, rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -319,7 +319,7 @@ static void iommu_free_pagetables(unsigned long unused)
cpumask_cycle(smp_processor_id(), &cpu_online_map));
}
-int iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+int iommu_iotlb_flush(struct domain *d, unsigned long bfn,
unsigned int page_count)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -328,13 +328,13 @@ int iommu_iotlb_flush(struct domain *d, unsigned long gfn,
if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
return 0;
- rc = hd->platform_ops->iotlb_flush(d, gfn, page_count);
+ rc = hd->platform_ops->iotlb_flush(d, bfn, page_count);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU IOTLB flush failed: %d, gfn %#lx, page count %u\n",
- d->domain_id, rc, gfn, page_count);
+ "d%d: IOMMU IOTLB flush failed: %d, bfn %#lx, page count %u\n",
+ d->domain_id, rc, bfn, page_count);
if ( !is_hardware_domain(d) )
domain_crash(d);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 1710256823..48f62e0e8d 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -585,7 +585,7 @@ static int __must_check iommu_flush_all(void)
}
static int __must_check iommu_flush_iotlb(struct domain *d,
- unsigned long gfn,
+ unsigned long bfn,
bool_t dma_old_pte_present,
unsigned int page_count)
{
@@ -612,12 +612,12 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
if ( iommu_domid == -1 )
continue;
- if ( page_count != 1 || gfn == gfn_x(INVALID_GFN) )
+ if ( page_count != 1 || bfn == bfn_x(INVALID_BFN) )
rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
0, flush_dev_iotlb);
else
rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
- (paddr_t)gfn << PAGE_SHIFT_4K,
+ __bfn_to_baddr(bfn),
PAGE_ORDER_4K,
!dma_old_pte_present,
flush_dev_iotlb);
@@ -633,15 +633,15 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
}
static int __must_check iommu_flush_iotlb_pages(struct domain *d,
- unsigned long gfn,
+ unsigned long bfn,
unsigned int page_count)
{
- return iommu_flush_iotlb(d, gfn, 1, page_count);
+ return iommu_flush_iotlb(d, bfn, 1, page_count);
}
static int __must_check iommu_flush_iotlb_all(struct domain *d)
{
- return iommu_flush_iotlb(d, gfn_x(INVALID_GFN), 0, 0);
+ return iommu_flush_iotlb(d, bfn_x(INVALID_BFN), 0, 0);
}
/* clear one page's page table */
@@ -1767,7 +1767,7 @@ static void iommu_domain_teardown(struct domain *d)
}
static int __must_check intel_iommu_map_page(struct domain *d,
- unsigned long gfn,
+ unsigned long bfn,
unsigned long mfn,
unsigned int flags)
{
@@ -1786,14 +1786,14 @@ static int __must_check intel_iommu_map_page(struct domain *d,
spin_lock(&hd->arch.mapping_lock);
- pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
+ pg_maddr = addr_to_dma_page_maddr(d, __bfn_to_baddr(bfn), 1);
if ( pg_maddr == 0 )
{
spin_unlock(&hd->arch.mapping_lock);
return -ENOMEM;
}
page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
- pte = page + (gfn & LEVEL_MASK);
+ pte = page + (bfn & LEVEL_MASK);
old = *pte;
dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
dma_set_pte_prot(new,
@@ -1817,22 +1817,22 @@ static int __must_check intel_iommu_map_page(struct domain *d,
unmap_vtd_domain_page(page);
if ( !this_cpu(iommu_dont_flush_iotlb) )
- rc = iommu_flush_iotlb(d, gfn, dma_pte_present(old), 1);
+ rc = iommu_flush_iotlb(d, bfn, dma_pte_present(old), 1);
return rc;
}
static int __must_check intel_iommu_unmap_page(struct domain *d,
- unsigned long gfn)
+ unsigned long bfn)
{
/* Do nothing if hardware domain and iommu supports pass thru. */
if ( iommu_passthrough && is_hardware_domain(d) )
return 0;
- return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
+ return dma_pte_clear_one(d, __bfn_to_baddr(bfn));
}
-int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
+int iommu_pte_flush(struct domain *d, uint64_t bfn, uint64_t *pte,
int order, int present)
{
struct acpi_drhd_unit *drhd;
@@ -1856,7 +1856,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
continue;
rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
- (paddr_t)gfn << PAGE_SHIFT_4K,
+ __bfn_to_baddr(bfn),
order, !present, flush_dev_iotlb);
if ( rc > 0 )
{
@@ -2626,7 +2626,7 @@ static void vtd_dump_p2m_table_level(paddr_t pt_maddr, int level, paddr_t gpa,
vtd_dump_p2m_table_level(dma_pte_addr(*pte), next_level,
address, indent + 1);
else
- printk("%*sgfn: %08lx mfn: %08lx\n",
+ printk("%*sbfn: %08lx mfn: %08lx\n",
indent, "",
(unsigned long)(address >> PAGE_SHIFT_4K),
(unsigned long)(dma_pte_addr(*pte) >> PAGE_SHIFT_4K));
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 6b42e3b876..14ada0c14e 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -23,11 +23,37 @@
#include <xen/page-defs.h>
#include <xen/spinlock.h>
#include <xen/pci.h>
+#include <xen/typesafe.h>
#include <public/hvm/ioreq.h>
#include <public/domctl.h>
#include <asm/device.h>
#include <asm/iommu.h>
+TYPE_SAFE(uint64_t, bfn);
+#define PRI_bfn PRIx64
+#define INVALID_BFN _bfn(~0UL)
+
+#ifndef bfn_t
+#define bfn_t /* Grep fodder: bfn_t, _bfn() and bfn_x() are defined above */
+#define _bfn
+#define bfn_x
+#undef bfn_t
+#undef _bfn
+#undef bfn_x
+#endif
+
+#define IOMMU_PAGE_SHIFT 12
+#define IOMMU_PAGE_SIZE (_AC(1,L) << IOMMU_PAGE_SHIFT)
+#define IOMMU_PAGE_MASK (~(IOMMU_PAGE_SIZE - 1))
+
+typedef uint64_t baddr_t;
+
+#define __bfn_to_baddr(bfn) ((baddr_t)(bfn) << IOMMU_PAGE_SHIFT)
+#define __baddr_to_bfn(baddr) ((uint64_t)(baddr >> IOMMU_PAGE_SHIFT))
+
+#define bfn_to_baddr(bfn) __bfn_to_baddr(bfn_x(bfn))
+#define baddr_to_bfn(baddr) _bfn(__baddr_to_bfn(baddr))
+
extern bool_t iommu_enable, iommu_enabled;
extern bool_t force_iommu, iommu_dom0_strict, iommu_verbose;
extern bool_t iommu_workaround_bios_bug, iommu_igfx, iommu_passthrough;
@@ -60,9 +86,9 @@ void iommu_teardown(struct domain *d);
#define IOMMUF_readable (1u<<_IOMMUF_readable)
#define _IOMMUF_writable 1
#define IOMMUF_writable (1u<<_IOMMUF_writable)
-int __must_check iommu_map_page(struct domain *d, unsigned long gfn,
+int __must_check iommu_map_page(struct domain *d, unsigned long bfn,
unsigned long mfn, unsigned int flags);
-int __must_check iommu_unmap_page(struct domain *d, unsigned long gfn);
+int __must_check iommu_unmap_page(struct domain *d, unsigned long bfn);
enum iommu_feature
{
@@ -152,9 +178,9 @@ struct iommu_ops {
#endif /* HAS_PCI */
void (*teardown)(struct domain *d);
- int __must_check (*map_page)(struct domain *d, unsigned long gfn,
+ int __must_check (*map_page)(struct domain *d, unsigned long bfn,
unsigned long mfn, unsigned int flags);
- int __must_check (*unmap_page)(struct domain *d, unsigned long gfn);
+ int __must_check (*unmap_page)(struct domain *d, unsigned long bfn);
void (*free_page_table)(struct page_info *);
#ifdef CONFIG_X86
void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value);
@@ -165,7 +191,7 @@ struct iommu_ops {
void (*resume)(void);
void (*share_p2m)(struct domain *d);
void (*crash_shutdown)(void);
- int __must_check (*iotlb_flush)(struct domain *d, unsigned long gfn,
+ int __must_check (*iotlb_flush)(struct domain *d, unsigned long bfn,
unsigned int page_count);
int __must_check (*iotlb_flush_all)(struct domain *d);
int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
@@ -187,7 +213,7 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d,
int iommu_do_domctl(struct xen_domctl *, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
-int __must_check iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+int __must_check iommu_iotlb_flush(struct domain *d, unsigned long bfn,
unsigned int page_count);
int __must_check iommu_iotlb_flush_all(struct domain *d);
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index e928551c91..e0387ff175 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -14,8 +14,9 @@
* should be adhered to.
*
* mfn: Machine Frame Number
- * The values Xen puts into its own pagetables. This is the host physical
- * memory address space with RAM, MMIO etc.
+ * The values Xen puts into its own pagetables, 2nd stage pagetables (where
+ * hardware assisted 2nd stage translation is used) or IOMMU page tables.
+ * This is the host physical memory address space with RAM, MMIO etc.
*
* gfn: Guest Frame Number
* The values a guest puts in its own pagetables. For an auto-translated
@@ -26,6 +27,11 @@
* A linear idea of a guest physical address space. For an auto-translated
* guest, pfn == gfn while for a non-translated guest, pfn != gfn.
*
+ * bfn: Bus Frame Number (definitions in include/xen/iommu.h)
+ * The linear frame numbers of IOMMU address space. All initiators for a
+ * given domain share a single IOMMU address space and, by default, Xen will
+ * ensure bfn == pfn.
+ *
* WARNING: Some of these terms have changed over time while others have been
* used inconsistently, meaning that a lot of existing code does not match the
* definitions above. New code should use these terms as described here, and
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 03/13] iommu: make use of type-safe BFN and MFN in exported functions
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
2018-07-17 13:38 ` [PATCH v3 01/13] grant_table: use term 'mfn' for machine frame numbers Paul Durrant
2018-07-17 13:38 ` [PATCH v3 02/13] iommu: introduce the concept of BFN Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-19 8:08 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 04/13] iommu: push use of type-safe BFN and MFN into iommu_ops Paul Durrant
` (9 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Paul Durrant, Jan Beulich
This patch modifies the declaration of the entry points to the IOMMU
sub-system to use bfn_t and mfn_t in place of unsigned long. A subsequent
patch will similarly modify the methods in the iommu_ops structure.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.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: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
v3:
- Removed most of the uses of an intermediate 'frame' variable.
v2:
- Addressed comments from Jan.
- Use intermediate 'frame' variable to avoid directly encapsulating
mfn or gfn values as bfns.
---
xen/arch/arm/p2m.c | 3 ++-
xen/arch/x86/mm.c | 10 ++++----
xen/arch/x86/mm/p2m-ept.c | 10 +++++---
xen/arch/x86/mm/p2m-pt.c | 45 ++++++++++++++++++++---------------
xen/arch/x86/mm/p2m.c | 16 ++++++++-----
xen/arch/x86/x86_64/mm.c | 5 ++--
xen/common/grant_table.c | 12 +++++-----
xen/common/memory.c | 4 ++--
xen/drivers/passthrough/iommu.c | 25 ++++++++++---------
xen/drivers/passthrough/vtd/x86/vtd.c | 3 ++-
xen/include/xen/iommu.h | 14 +++++++----
11 files changed, 85 insertions(+), 62 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 14791388ad..d719db1e30 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -953,7 +953,8 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
if ( need_iommu(p2m->domain) &&
(lpae_valid(orig_pte) || lpae_valid(*entry)) )
- rc = iommu_iotlb_flush(p2m->domain, gfn_x(sgfn), 1UL << page_order);
+ rc = iommu_iotlb_flush(p2m->domain, _bfn(gfn_x(sgfn)),
+ 1UL << page_order);
else
rc = 0;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index a1a1f5f3c3..99070f916d 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2729,14 +2729,14 @@ static int _get_page_type(struct page_info *page, unsigned long type,
struct domain *d = page_get_owner(page);
if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
{
- gfn_t gfn = _gfn(mfn_to_gmfn(d, mfn_x(page_to_mfn(page))));
+ mfn_t mfn = page_to_mfn(page);
if ( (x & PGT_type_mask) == PGT_writable_page )
- iommu_ret = iommu_unmap_page(d, gfn_x(gfn));
+ iommu_ret = iommu_unmap_page(d, _bfn(mfn_x(mfn)));
else if ( type == PGT_writable_page )
- iommu_ret = iommu_map_page(d, gfn_x(gfn),
- mfn_x(page_to_mfn(page)),
- IOMMUF_readable|IOMMUF_writable);
+ iommu_ret = iommu_map_page(d, _bfn(mfn_x(mfn)), mfn,
+ IOMMUF_readable |
+ IOMMUF_writable);
}
}
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 14b593923b..2089b5232d 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -870,15 +870,19 @@ out:
rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order, vtd_pte_present);
else
{
+ bfn_t bfn = _bfn(gfn);
+
if ( iommu_flags )
for ( i = 0; i < (1 << order); i++ )
{
- rc = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, iommu_flags);
+ rc = iommu_map_page(d, bfn_add(bfn, i),
+ mfn_add(mfn, i), iommu_flags);
if ( unlikely(rc) )
{
while ( i-- )
/* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(p2m->domain, gfn + i) )
+ if ( iommu_unmap_page(p2m->domain,
+ bfn_add(bfn, i)) )
continue;
break;
@@ -887,7 +891,7 @@ out:
else
for ( i = 0; i < (1 << order); i++ )
{
- ret = iommu_unmap_page(d, gfn + i);
+ ret = iommu_unmap_page(d, bfn_add(bfn, i));
if ( !rc )
rc = ret;
}
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index b8c5d2ed26..a441af388a 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -687,29 +687,36 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
if ( iommu_old_flags )
amd_iommu_flush_pages(p2m->domain, gfn, page_order);
}
- else if ( iommu_pte_flags )
- for ( i = 0; i < (1UL << page_order); i++ )
- {
- rc = iommu_map_page(p2m->domain, gfn + i, mfn_x(mfn) + i,
- iommu_pte_flags);
- if ( unlikely(rc) )
+ else
+ {
+ bfn_t bfn = _bfn(gfn);
+
+ if ( iommu_pte_flags )
+ for ( i = 0; i < (1UL << page_order); i++ )
{
- while ( i-- )
- /* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(p2m->domain, gfn + i) )
- continue;
+ rc = iommu_map_page(p2m->domain, bfn_add(bfn, i),
+ mfn_add(mfn, i), iommu_pte_flags);
+ if ( unlikely(rc) )
+ {
+ while ( i-- )
+ /* If statement to satisfy __must_check. */
+ if ( iommu_unmap_page(p2m->domain,
+ bfn_add(bfn, i)) )
+ continue;
- break;
+ break;
+ }
}
- }
- else
- for ( i = 0; i < (1UL << page_order); i++ )
- {
- int ret = iommu_unmap_page(p2m->domain, gfn + i);
+ else
+ for ( i = 0; i < (1UL << page_order); i++ )
+ {
+ int ret = iommu_unmap_page(p2m->domain,
+ bfn_add(bfn, i));
- if ( !rc )
- rc = ret;
- }
+ if ( !rc )
+ rc = ret;
+ }
+ }
}
/*
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index c53cab44d9..e8a3c04d09 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -714,9 +714,11 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
if ( need_iommu(p2m->domain) )
{
+ bfn_t bfn = _bfn(mfn);
+
for ( i = 0; i < (1 << page_order); i++ )
{
- int ret = iommu_unmap_page(p2m->domain, mfn + i);
+ int ret = iommu_unmap_page(p2m->domain, bfn_add(bfn, i));
if ( !rc )
rc = ret;
@@ -773,16 +775,17 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
{
if ( need_iommu(d) && t == p2m_ram_rw )
{
+ bfn_t bfn = _bfn(mfn_x(mfn));
+
for ( i = 0; i < (1 << page_order); i++ )
{
- rc = iommu_map_page(d, mfn_x(mfn_add(mfn, i)),
- mfn_x(mfn_add(mfn, i)),
+ rc = iommu_map_page(d, bfn_add(bfn, i), mfn_add(mfn, i),
IOMMUF_readable|IOMMUF_writable);
if ( rc != 0 )
{
while ( i-- > 0 )
/* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(d, mfn_x(mfn_add(mfn, i))) )
+ if ( iommu_unmap_page(d, bfn_add(bfn, i)) )
continue;
return rc;
@@ -1157,7 +1160,8 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l,
{
if ( !need_iommu(d) )
return 0;
- return iommu_map_page(d, gfn_l, gfn_l, IOMMUF_readable|IOMMUF_writable);
+ return iommu_map_page(d, _bfn(gfn_l), _mfn(gfn_l),
+ IOMMUF_readable | IOMMUF_writable);
}
gfn_lock(p2m, gfn, 0);
@@ -1247,7 +1251,7 @@ int clear_identity_p2m_entry(struct domain *d, unsigned long gfn_l)
{
if ( !need_iommu(d) )
return 0;
- return iommu_unmap_page(d, gfn_l);
+ return iommu_unmap_page(d, _bfn(gfn_l));
}
gfn_lock(p2m, gfn, 0);
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cca4ae926e..cc58e4cef4 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1429,13 +1429,14 @@ int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm)
if ( iommu_enabled && !iommu_passthrough && !need_iommu(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
- if ( iommu_map_page(hardware_domain, i, i, IOMMUF_readable|IOMMUF_writable) )
+ if ( iommu_map_page(hardware_domain, _bfn(i), _mfn(i),
+ IOMMUF_readable | IOMMUF_writable) )
break;
if ( i != epfn )
{
while (i-- > old_max)
/* If statement to satisfy __must_check. */
- if ( iommu_unmap_page(hardware_domain, i) )
+ if ( iommu_unmap_page(hardware_domain, _bfn(i)) )
continue;
goto destroy_m2p;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index d2610e320c..261e3b17a0 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1144,13 +1144,13 @@ map_grant_ref(
!(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
{
if ( !(kind & MAPKIND_WRITE) )
- err = iommu_map_page(ld, mfn_x(mfn), mfn_x(mfn),
- IOMMUF_readable|IOMMUF_writable);
+ err = iommu_map_page(ld, _bfn(mfn_x(mfn)), mfn,
+ IOMMUF_readable | IOMMUF_writable);
}
else if ( act_pin && !old_pin )
{
if ( !kind )
- err = iommu_map_page(ld, mfn_x(mfn), mfn_x(mfn),
+ err = iommu_map_page(ld, _bfn(mfn_x(mfn)), mfn,
IOMMUF_readable);
}
if ( err )
@@ -1413,10 +1413,10 @@ unmap_common(
kind = mapkind(lgt, rd, op->mfn);
if ( !kind )
- err = iommu_unmap_page(ld, mfn_x(op->mfn));
+ err = iommu_unmap_page(ld, _bfn(mfn_x(op->mfn)));
else if ( !(kind & MAPKIND_WRITE) )
- err = iommu_map_page(ld, mfn_x(op->mfn),
- mfn_x(op->mfn), IOMMUF_readable);
+ err = iommu_map_page(ld, _bfn(mfn_x(op->mfn)), op->mfn,
+ IOMMUF_readable);
double_gt_unlock(lgt, rgt);
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 75010b78a5..e1616eda32 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -833,11 +833,11 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
this_cpu(iommu_dont_flush_iotlb) = 0;
- ret = iommu_iotlb_flush(d, xatp->idx - done, done);
+ ret = iommu_iotlb_flush(d, _bfn(xatp->idx - done), done);
if ( unlikely(ret) && rc >= 0 )
rc = ret;
- ret = iommu_iotlb_flush(d, xatp->gpfn - done, done);
+ ret = iommu_iotlb_flush(d, _bfn(xatp->gpfn - done), done);
if ( unlikely(ret) && rc >= 0 )
rc = ret;
}
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 46db9d0d04..44797f9e92 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -255,7 +255,7 @@ void iommu_domain_destroy(struct domain *d)
arch_iommu_domain_destroy(d);
}
-int iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
+int iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
unsigned int flags)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -264,13 +264,13 @@ int iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->map_page(d, bfn, mfn, flags);
+ rc = hd->platform_ops->map_page(d, bfn_x(bfn), mfn_x(mfn), flags);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU mapping bfn %#lx to mfn %#lx failed: %d\n",
- d->domain_id, bfn, mfn, rc);
+ "d%d: IOMMU mapping bfn %"PRI_bfn" to mfn %"PRI_mfn" failed: %d\n",
+ d->domain_id, bfn_x(bfn), mfn_x(mfn), rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -279,7 +279,7 @@ int iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
return rc;
}
-int iommu_unmap_page(struct domain *d, unsigned long bfn)
+int iommu_unmap_page(struct domain *d, bfn_t bfn)
{
const struct domain_iommu *hd = dom_iommu(d);
int rc;
@@ -287,13 +287,13 @@ int iommu_unmap_page(struct domain *d, unsigned long bfn)
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->unmap_page(d, bfn);
+ rc = hd->platform_ops->unmap_page(d, bfn_x(bfn));
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU unmapping bfn %#lx failed: %d\n",
- d->domain_id, bfn, rc);
+ "d%d: IOMMU unmapping bfn %"PRI_bfn" failed: %d\n",
+ d->domain_id, bfn_x(bfn), rc);
if ( !is_hardware_domain(d) )
domain_crash(d);
@@ -319,8 +319,7 @@ static void iommu_free_pagetables(unsigned long unused)
cpumask_cycle(smp_processor_id(), &cpu_online_map));
}
-int iommu_iotlb_flush(struct domain *d, unsigned long bfn,
- unsigned int page_count)
+int iommu_iotlb_flush(struct domain *d, bfn_t bfn, unsigned int page_count)
{
const struct domain_iommu *hd = dom_iommu(d);
int rc;
@@ -328,13 +327,13 @@ int iommu_iotlb_flush(struct domain *d, unsigned long bfn,
if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
return 0;
- rc = hd->platform_ops->iotlb_flush(d, bfn, page_count);
+ rc = hd->platform_ops->iotlb_flush(d, bfn_x(bfn), page_count);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
printk(XENLOG_ERR
- "d%d: IOMMU IOTLB flush failed: %d, bfn %#lx, page count %u\n",
- d->domain_id, rc, bfn, page_count);
+ "d%d: IOMMU IOTLB flush failed: %d, bfn %"PRI_bfn", page count %u\n",
+ d->domain_id, rc, bfn_x(bfn), page_count);
if ( !is_hardware_domain(d) )
domain_crash(d);
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index cc2bfea162..dc37dce4b6 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -155,7 +155,8 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
for ( j = 0; j < tmp; j++ )
{
- int ret = iommu_map_page(d, pfn * tmp + j, pfn * tmp + j,
+ int ret = iommu_map_page(d, _bfn(pfn * tmp + j),
+ _mfn(pfn * tmp + j),
IOMMUF_readable|IOMMUF_writable);
if ( !rc )
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 14ada0c14e..a3c36c1148 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -24,6 +24,7 @@
#include <xen/spinlock.h>
#include <xen/pci.h>
#include <xen/typesafe.h>
+#include <xen/mm.h>
#include <public/hvm/ioreq.h>
#include <public/domctl.h>
#include <asm/device.h>
@@ -42,6 +43,11 @@ TYPE_SAFE(uint64_t, bfn);
#undef bfn_x
#endif
+static inline bfn_t bfn_add(bfn_t bfn, unsigned long i)
+{
+ return _bfn(bfn_x(bfn) + i);
+}
+
#define IOMMU_PAGE_SHIFT 12
#define IOMMU_PAGE_SIZE (_AC(1,L) << IOMMU_PAGE_SHIFT)
#define IOMMU_PAGE_MASK (~(IOMMU_PAGE_SIZE - 1))
@@ -86,9 +92,9 @@ void iommu_teardown(struct domain *d);
#define IOMMUF_readable (1u<<_IOMMUF_readable)
#define _IOMMUF_writable 1
#define IOMMUF_writable (1u<<_IOMMUF_writable)
-int __must_check iommu_map_page(struct domain *d, unsigned long bfn,
- unsigned long mfn, unsigned int flags);
-int __must_check iommu_unmap_page(struct domain *d, unsigned long bfn);
+int __must_check iommu_map_page(struct domain *d, bfn_t bfn,
+ mfn_t mfn, unsigned int flags);
+int __must_check iommu_unmap_page(struct domain *d, bfn_t bfn);
enum iommu_feature
{
@@ -213,7 +219,7 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d,
int iommu_do_domctl(struct xen_domctl *, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
-int __must_check iommu_iotlb_flush(struct domain *d, unsigned long bfn,
+int __must_check iommu_iotlb_flush(struct domain *d, bfn_t bfn,
unsigned int page_count);
int __must_check iommu_iotlb_flush_all(struct domain *d);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 04/13] iommu: push use of type-safe BFN and MFN into iommu_ops
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (2 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 03/13] iommu: make use of type-safe BFN and MFN in exported functions Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-19 8:10 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 05/13] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
` (8 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Suravee Suthikulpanit, Andrew Cooper, George Dunlap,
Paul Durrant, Jan Beulich
This patch modifies the methods in struct iommu_ops to use type-safe BFN
and MFN. This follows on from the prior patch that modified the functions
exported in xen/iommu.h.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
v3:
- Remove some use of intermediate 'frame' variables.
v2:
- Addressed comments from Jan.
- Extend use of intermediate 'frame' variable to avoid directly
encapsulating gfn values as bfns or vice versa.
---
xen/drivers/passthrough/amd/iommu_map.c | 46 ++++++++++++++++-----------
xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 16 +++++-----
xen/drivers/passthrough/iommu.c | 9 +++---
xen/drivers/passthrough/vtd/iommu.c | 26 +++++++--------
xen/drivers/passthrough/vtd/x86/vtd.c | 7 ++--
xen/drivers/passthrough/x86/iommu.c | 2 +-
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 ++---
xen/include/xen/iommu.h | 13 +++++---
9 files changed, 72 insertions(+), 57 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index 4deab9cd2f..5a9a0af320 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -631,7 +631,7 @@ static int update_paging_mode(struct domain *d, unsigned long bfn)
return 0;
}
-int amd_iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
+int amd_iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
unsigned int flags)
{
bool_t need_flush = 0;
@@ -651,7 +651,8 @@ int amd_iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Root table alloc failed, bfn = %lx\n", bfn);
+ AMD_IOMMU_DEBUG("Root table alloc failed, bfn = %"PRI_bfn"\n",
+ bfn_x(bfn));
domain_crash(d);
return rc;
}
@@ -660,25 +661,27 @@ int amd_iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
* we might need a deeper page table for wider bfn now */
if ( is_hvm_domain(d) )
{
- if ( update_paging_mode(d, bfn) )
+ if ( update_paging_mode(d, bfn_x(bfn)) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed bfn = %lx\n", bfn);
+ AMD_IOMMU_DEBUG("Update page mode failed bfn = %"PRI_bfn"\n",
+ bfn_x(bfn));
domain_crash(d);
return -EFAULT;
}
}
- if ( iommu_pde_from_bfn(d, bfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_bfn(d, bfn_x(bfn), pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry bfn = %lx\n", bfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry bfn = %"PRI_bfn"\n",
+ bfn_x(bfn));
domain_crash(d);
return -EFAULT;
}
/* Install 4k mapping first */
- need_flush = set_iommu_pte_present(pt_mfn[1], bfn, mfn,
+ need_flush = set_iommu_pte_present(pt_mfn[1], bfn_x(bfn), mfn_x(mfn),
IOMMU_PAGING_MODE_LEVEL_1,
!!(flags & IOMMUF_writable),
!!(flags & IOMMUF_readable));
@@ -690,7 +693,7 @@ int amd_iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
/* 4K mapping for PV guests never changes,
* no need to flush if we trust non-present bits */
if ( is_hvm_domain(d) )
- amd_iommu_flush_pages(d, bfn, 0);
+ amd_iommu_flush_pages(d, bfn_x(bfn), 0);
for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
merge_level <= hd->arch.paging_mode; merge_level++ )
@@ -698,15 +701,16 @@ int amd_iommu_map_page(struct domain *d, unsigned long bfn, unsigned long mfn,
if ( pt_mfn[merge_level] == 0 )
break;
if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
- bfn, mfn, merge_level) )
+ bfn_x(bfn), mfn_x(mfn), merge_level) )
break;
- if ( iommu_merge_pages(d, pt_mfn[merge_level], bfn,
+ if ( iommu_merge_pages(d, pt_mfn[merge_level], bfn_x(bfn),
flags, merge_level) )
{
spin_unlock(&hd->arch.mapping_lock);
AMD_IOMMU_DEBUG("Merge iommu page failed at level %d, "
- "bfn = %lx mfn = %lx\n", merge_level, bfn, mfn);
+ "bfn = %"PRI_bfn" mfn = %"PRI_mfn"\n",
+ merge_level, bfn_x(bfn), mfn_x(mfn));
domain_crash(d);
return -EFAULT;
}
@@ -720,7 +724,7 @@ out:
return 0;
}
-int amd_iommu_unmap_page(struct domain *d, unsigned long bfn)
+int amd_iommu_unmap_page(struct domain *d, bfn_t bfn)
{
unsigned long pt_mfn[7];
struct domain_iommu *hd = dom_iommu(d);
@@ -742,31 +746,33 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long bfn)
* we might need a deeper page table for lager bfn now */
if ( is_hvm_domain(d) )
{
- int rc = update_paging_mode(d, bfn);
+ int rc = update_paging_mode(d, bfn_x(bfn));
if ( rc )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Update page mode failed bfn = %lx\n", bfn);
+ AMD_IOMMU_DEBUG("Update page mode failed bfn = %"PRI_bfn"\n",
+ bfn_x(bfn));
if ( rc != -EADDRNOTAVAIL )
domain_crash(d);
return rc;
}
}
- if ( iommu_pde_from_bfn(d, bfn, pt_mfn) || (pt_mfn[1] == 0) )
+ if ( iommu_pde_from_bfn(d, bfn_x(bfn), pt_mfn) || (pt_mfn[1] == 0) )
{
spin_unlock(&hd->arch.mapping_lock);
- AMD_IOMMU_DEBUG("Invalid IO pagetable entry bfn = %lx\n", bfn);
+ AMD_IOMMU_DEBUG("Invalid IO pagetable entry bfn = %"PRI_bfn"\n",
+ bfn_x(bfn));
domain_crash(d);
return -EFAULT;
}
/* mark PTE as 'page not present' */
- clear_iommu_pte_present(pt_mfn[1], bfn);
+ clear_iommu_pte_present(pt_mfn[1], bfn_x(bfn));
spin_unlock(&hd->arch.mapping_lock);
- amd_iommu_flush_pages(d, bfn, 0);
+ amd_iommu_flush_pages(d, bfn_x(bfn), 0);
return 0;
}
@@ -787,7 +793,9 @@ int amd_iommu_reserve_domain_unity_map(struct domain *domain,
gfn = phys_addr >> PAGE_SHIFT;
for ( i = 0; i < npages; i++ )
{
- rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags);
+ unsigned long frame = gfn + i;
+
+ rt = amd_iommu_map_page(domain, _bfn(frame), _mfn(frame), flags);
if ( rt != 0 )
return rt;
}
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index d608631e6e..eea22c3d0d 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -271,7 +271,7 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain *d)
*/
if ( mfn_valid(_mfn(pfn)) )
{
- int ret = amd_iommu_map_page(d, pfn, pfn,
+ int ret = amd_iommu_map_page(d, _bfn(pfn), _mfn(pfn),
IOMMUF_readable|IOMMUF_writable);
if ( !rc )
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 1e4d561b47..221b62a59c 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2550,8 +2550,7 @@ static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
return 0;
}
-static int __must_check arm_smmu_iotlb_flush(struct domain *d,
- unsigned long bfn,
+static int __must_check arm_smmu_iotlb_flush(struct domain *d, bfn_t bfn,
unsigned int page_count)
{
/* ARM SMMU v1 doesn't have flush by VMA and VMID */
@@ -2737,8 +2736,8 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
xfree(xen_domain);
}
-static int __must_check arm_smmu_map_page(struct domain *d, unsigned long bfn,
- unsigned long mfn, unsigned int flags)
+static int __must_check arm_smmu_map_page(struct domain *d, bfn_t bfn,
+ mfn_t mfn, unsigned int flags)
{
p2m_type_t t;
@@ -2751,7 +2750,7 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long bfn,
* function should only be used by gnttab code with gfn == mfn == bfn.
*/
BUG_ON(!is_domain_direct_mapped(d));
- BUG_ON(mfn != bfn);
+ BUG_ON(mfn_x(mfn) != bfn_x(bfn));
/* We only support readable and writable flags */
if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
@@ -2763,10 +2762,11 @@ static int __must_check arm_smmu_map_page(struct domain *d, unsigned long bfn,
* The function guest_physmap_add_entry replaces the current mapping
* if there is already one...
*/
- return guest_physmap_add_entry(d, _gfn(bfn), _mfn(bfn), 0, t);
+ return guest_physmap_add_entry(d, _gfn(bfn_x(bfn)), _mfn(bfn_x(bfn)),
+ 0, t);
}
-static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long bfn)
+static int __must_check arm_smmu_unmap_page(struct domain *d, bfn_t bfn)
{
/*
* This function should only be used by gnttab code when the domain
@@ -2775,7 +2775,7 @@ static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long bfn)
if ( !is_domain_direct_mapped(d) )
return -EINVAL;
- return guest_physmap_remove_page(d, _gfn(bfn), _mfn(bfn), 0);
+ return guest_physmap_remove_page(d, _gfn(bfn_x(bfn)), _mfn(bfn_x(bfn)), 0);
}
static const struct iommu_ops arm_smmu_iommu_ops = {
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 44797f9e92..cf70a5c43c 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -194,7 +194,8 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
== PGT_writable_page) )
mapping |= IOMMUF_writable;
- ret = hd->platform_ops->map_page(d, bfn, mfn, mapping);
+ ret = hd->platform_ops->map_page(d, _bfn(bfn), _mfn(mfn),
+ mapping);
if ( !rc )
rc = ret;
@@ -264,7 +265,7 @@ int iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->map_page(d, bfn_x(bfn), mfn_x(mfn), flags);
+ rc = hd->platform_ops->map_page(d, bfn, mfn, flags);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
@@ -287,7 +288,7 @@ int iommu_unmap_page(struct domain *d, bfn_t bfn)
if ( !iommu_enabled || !hd->platform_ops )
return 0;
- rc = hd->platform_ops->unmap_page(d, bfn_x(bfn));
+ rc = hd->platform_ops->unmap_page(d, bfn);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
@@ -327,7 +328,7 @@ int iommu_iotlb_flush(struct domain *d, bfn_t bfn, unsigned int page_count)
if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
return 0;
- rc = hd->platform_ops->iotlb_flush(d, bfn_x(bfn), page_count);
+ rc = hd->platform_ops->iotlb_flush(d, bfn, page_count);
if ( unlikely(rc) )
{
if ( !d->is_shutting_down && printk_ratelimit() )
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 48f62e0e8d..c9f50f04ad 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -584,8 +584,7 @@ static int __must_check iommu_flush_all(void)
return rc;
}
-static int __must_check iommu_flush_iotlb(struct domain *d,
- unsigned long bfn,
+static int __must_check iommu_flush_iotlb(struct domain *d, bfn_t bfn,
bool_t dma_old_pte_present,
unsigned int page_count)
{
@@ -612,12 +611,12 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
if ( iommu_domid == -1 )
continue;
- if ( page_count != 1 || bfn == bfn_x(INVALID_BFN) )
+ if ( page_count != 1 || bfn_eq(bfn, INVALID_BFN) )
rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
0, flush_dev_iotlb);
else
rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
- __bfn_to_baddr(bfn),
+ bfn_to_baddr(bfn),
PAGE_ORDER_4K,
!dma_old_pte_present,
flush_dev_iotlb);
@@ -633,7 +632,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
}
static int __must_check iommu_flush_iotlb_pages(struct domain *d,
- unsigned long bfn,
+ bfn_t bfn,
unsigned int page_count)
{
return iommu_flush_iotlb(d, bfn, 1, page_count);
@@ -641,7 +640,7 @@ static int __must_check iommu_flush_iotlb_pages(struct domain *d,
static int __must_check iommu_flush_iotlb_all(struct domain *d)
{
- return iommu_flush_iotlb(d, bfn_x(INVALID_BFN), 0, 0);
+ return iommu_flush_iotlb(d, INVALID_BFN, 0, 0);
}
/* clear one page's page table */
@@ -676,7 +675,7 @@ static int __must_check dma_pte_clear_one(struct domain *domain, u64 addr)
iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
if ( !this_cpu(iommu_dont_flush_iotlb) )
- rc = iommu_flush_iotlb_pages(domain, addr >> PAGE_SHIFT_4K, 1);
+ rc = iommu_flush_iotlb_pages(domain, baddr_to_bfn(addr), 1);
unmap_vtd_domain_page(page);
@@ -1767,8 +1766,7 @@ static void iommu_domain_teardown(struct domain *d)
}
static int __must_check intel_iommu_map_page(struct domain *d,
- unsigned long bfn,
- unsigned long mfn,
+ bfn_t bfn, mfn_t mfn,
unsigned int flags)
{
struct domain_iommu *hd = dom_iommu(d);
@@ -1786,16 +1784,16 @@ static int __must_check intel_iommu_map_page(struct domain *d,
spin_lock(&hd->arch.mapping_lock);
- pg_maddr = addr_to_dma_page_maddr(d, __bfn_to_baddr(bfn), 1);
+ pg_maddr = addr_to_dma_page_maddr(d, bfn_to_baddr(bfn), 1);
if ( pg_maddr == 0 )
{
spin_unlock(&hd->arch.mapping_lock);
return -ENOMEM;
}
page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
- pte = page + (bfn & LEVEL_MASK);
+ pte = page + (bfn_x(bfn) & LEVEL_MASK);
old = *pte;
- dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
+ dma_set_pte_addr(new, mfn_to_maddr(mfn));
dma_set_pte_prot(new,
((flags & IOMMUF_readable) ? DMA_PTE_READ : 0) |
((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
@@ -1823,13 +1821,13 @@ static int __must_check intel_iommu_map_page(struct domain *d,
}
static int __must_check intel_iommu_unmap_page(struct domain *d,
- unsigned long bfn)
+ bfn_t bfn)
{
/* Do nothing if hardware domain and iommu supports pass thru. */
if ( iommu_passthrough && is_hardware_domain(d) )
return 0;
- return dma_pte_clear_one(d, __bfn_to_baddr(bfn));
+ return dma_pte_clear_one(d, bfn_to_baddr(bfn));
}
int iommu_pte_flush(struct domain *d, uint64_t bfn, uint64_t *pte,
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index dc37dce4b6..6fed4a92cb 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -121,6 +121,8 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
{
unsigned long pfn = pdx_to_pfn(i);
bool map;
+ bfn_t bfn;
+ mfn_t mfn;
int rc = 0;
/*
@@ -153,10 +155,11 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
continue;
tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
+ bfn = _bfn(pfn * tmp);
+ mfn = _mfn(pfn * tmp);
for ( j = 0; j < tmp; j++ )
{
- int ret = iommu_map_page(d, _bfn(pfn * tmp + j),
- _mfn(pfn * tmp + j),
+ int ret = iommu_map_page(d, bfn_add(bfn, j), mfn_add(mfn, j),
IOMMUF_readable|IOMMUF_writable);
if ( !rc )
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 68182afd91..379882c690 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -65,7 +65,7 @@ int arch_iommu_populate_page_table(struct domain *d)
{
ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
BUG_ON(SHARED_M2P(gfn));
- rc = hd->platform_ops->map_page(d, gfn, mfn,
+ rc = hd->platform_ops->map_page(d, _bfn(gfn), _mfn(mfn),
IOMMUF_readable |
IOMMUF_writable);
}
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index 99bc21c7b3..dce9ed6b83 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -52,9 +52,9 @@ int amd_iommu_init(void);
int amd_iommu_update_ivrs_mapping_acpi(void);
/* mapping functions */
-int __must_check amd_iommu_map_page(struct domain *d, unsigned long gfn,
- unsigned long mfn, unsigned int flags);
-int __must_check amd_iommu_unmap_page(struct domain *d, unsigned long gfn);
+int __must_check amd_iommu_map_page(struct domain *d, bfn_t bfn,
+ mfn_t mfn, unsigned int flags);
+int __must_check amd_iommu_unmap_page(struct domain *d, bfn_t bfn);
u64 amd_iommu_get_next_table_from_pte(u32 *entry);
int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
int amd_iommu_reserve_domain_unity_map(struct domain *domain,
@@ -77,7 +77,7 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64 gcr3,
/* send cmd to iommu */
void amd_iommu_flush_all_pages(struct domain *d);
-void amd_iommu_flush_pages(struct domain *d, unsigned long gfn,
+void amd_iommu_flush_pages(struct domain *d, unsigned long bfn,
unsigned int order);
void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
uint64_t gaddr, unsigned int order);
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index a3c36c1148..624784fec8 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -48,6 +48,11 @@ static inline bfn_t bfn_add(bfn_t bfn, unsigned long i)
return _bfn(bfn_x(bfn) + i);
}
+static inline bool_t bfn_eq(bfn_t x, bfn_t y)
+{
+ return bfn_x(x) == bfn_x(y);
+}
+
#define IOMMU_PAGE_SHIFT 12
#define IOMMU_PAGE_SIZE (_AC(1,L) << IOMMU_PAGE_SHIFT)
#define IOMMU_PAGE_MASK (~(IOMMU_PAGE_SIZE - 1))
@@ -184,9 +189,9 @@ struct iommu_ops {
#endif /* HAS_PCI */
void (*teardown)(struct domain *d);
- int __must_check (*map_page)(struct domain *d, unsigned long bfn,
- unsigned long mfn, unsigned int flags);
- int __must_check (*unmap_page)(struct domain *d, unsigned long bfn);
+ int __must_check (*map_page)(struct domain *d, bfn_t bfn, mfn_t mfn,
+ unsigned int flags);
+ int __must_check (*unmap_page)(struct domain *d, bfn_t bfn);
void (*free_page_table)(struct page_info *);
#ifdef CONFIG_X86
void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value);
@@ -197,7 +202,7 @@ struct iommu_ops {
void (*resume)(void);
void (*share_p2m)(struct domain *d);
void (*crash_shutdown)(void);
- int __must_check (*iotlb_flush)(struct domain *d, unsigned long bfn,
+ int __must_check (*iotlb_flush)(struct domain *d, bfn_t bfn,
unsigned int page_count);
int __must_check (*iotlb_flush_all)(struct domain *d);
int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 05/13] iommu: don't domain_crash() inside iommu_map/unmap_page()
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (3 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 04/13] iommu: push use of type-safe BFN and MFN into iommu_ops Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-17 13:38 ` [PATCH v3 06/13] public / x86: introduce __HYPERCALL_iommu_op Paul Durrant
` (7 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, Jun Nakajima, George Dunlap,
Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
Paul Durrant, Jan Beulich
Turn iommu_map/unmap_page() into straightforward wrappers that check the
existence of the relevant iommu_op and call through to it. This makes them
usable by PV IOMMU code to be delivered in future patches.
Leave the decision on whether to invoke domain_crash() up to the caller.
This has the added benefit that the (module/line number) message that
domain_crash() spits out will be more indicative of where the problem lies.
NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry()
replacing use of p2m->domain with the domain pointer passed into the
function.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: George Dunlap <George.Dunlap@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Andrew Cooper <andrew.cooper3@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: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
v2:
- New in v2.
---
xen/arch/arm/p2m.c | 4 ++++
xen/arch/x86/mm.c | 3 +++
xen/arch/x86/mm/p2m-ept.c | 3 +++
xen/arch/x86/mm/p2m-pt.c | 3 +++
xen/arch/x86/mm/p2m.c | 24 ++++++++++++++++++++----
xen/common/grant_table.c | 8 ++++++++
xen/common/memory.c | 3 +++
xen/drivers/passthrough/iommu.c | 12 ------------
xen/drivers/passthrough/x86/iommu.c | 4 ++++
9 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d719db1e30..eb39861b73 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -953,8 +953,12 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
if ( need_iommu(p2m->domain) &&
(lpae_valid(orig_pte) || lpae_valid(*entry)) )
+ {
rc = iommu_iotlb_flush(p2m->domain, _bfn(gfn_x(sgfn)),
1UL << page_order);
+ if ( unlikely(rc) && !is_hardware_domain(p2m->domain) )
+ domain_crash(p2m->domain);
+ }
else
rc = 0;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 99070f916d..08878574f3 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2737,6 +2737,9 @@ static int _get_page_type(struct page_info *page, unsigned long type,
iommu_ret = iommu_map_page(d, _bfn(mfn_x(mfn)), mfn,
IOMMUF_readable |
IOMMUF_writable);
+
+ if ( unlikely(iommu_ret) && !is_hardware_domain(d) )
+ domain_crash(d);
}
}
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 2089b5232d..33e77903d6 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -895,6 +895,9 @@ out:
if ( !rc )
rc = ret;
}
+
+ if ( unlikely(rc) && !is_hardware_domain(d) )
+ domain_crash(d);
}
}
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index a441af388a..9ff0b3fe7a 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -717,6 +717,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
rc = ret;
}
}
+
+ if ( unlikely(rc) && !is_hardware_domain(p2m->domain) )
+ domain_crash(p2m->domain);
}
/*
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e8a3c04d09..fffa7aa32c 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -723,6 +723,9 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
if ( !rc )
rc = ret;
}
+
+ if ( unlikely(rc) && !is_hardware_domain(p2m->domain) )
+ domain_crash(p2m->domain);
}
return rc;
@@ -788,6 +791,9 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
if ( iommu_unmap_page(d, bfn_add(bfn, i)) )
continue;
+ if ( !is_hardware_domain(d) )
+ domain_crash(d);
+
return rc;
}
}
@@ -1156,12 +1162,17 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l,
struct p2m_domain *p2m = p2m_get_hostp2m(d);
int ret;
- if ( !paging_mode_translate(p2m->domain) )
+ if ( !paging_mode_translate(d) )
{
if ( !need_iommu(d) )
return 0;
- return iommu_map_page(d, _bfn(gfn_l), _mfn(gfn_l),
- IOMMUF_readable | IOMMUF_writable);
+
+ ret = iommu_map_page(d, _bfn(gfn_l), _mfn(gfn_l),
+ IOMMUF_readable | IOMMUF_writable);
+ if ( unlikely(ret) && !is_hardware_domain(d) )
+ domain_crash(d);
+
+ return ret;
}
gfn_lock(p2m, gfn, 0);
@@ -1251,7 +1262,12 @@ int clear_identity_p2m_entry(struct domain *d, unsigned long gfn_l)
{
if ( !need_iommu(d) )
return 0;
- return iommu_unmap_page(d, _bfn(gfn_l));
+
+ ret = iommu_unmap_page(d, _bfn(gfn_l));
+ if ( unlikely(ret) && !is_hardware_domain(d) )
+ domain_crash(d);
+
+ return ret;
}
gfn_lock(p2m, gfn, 0);
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 261e3b17a0..a179c9cf2a 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1155,6 +1155,9 @@ map_grant_ref(
}
if ( err )
{
+ if ( !is_hardware_domain(ld) )
+ domain_crash(ld);
+
double_gt_unlock(lgt, rgt);
rc = GNTST_general_error;
goto undo_out;
@@ -1421,7 +1424,12 @@ unmap_common(
double_gt_unlock(lgt, rgt);
if ( err )
+ {
+ if ( !is_hardware_domain(ld) )
+ domain_crash(ld);
+
rc = GNTST_general_error;
+ }
}
/* If just unmapped a writable mapping, mark as dirtied */
diff --git a/xen/common/memory.c b/xen/common/memory.c
index e1616eda32..9012b83aae 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -840,6 +840,9 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
ret = iommu_iotlb_flush(d, _bfn(xatp->gpfn - done), done);
if ( unlikely(ret) && rc >= 0 )
rc = ret;
+
+ if ( unlikely(rc < 0) && !is_hardware_domain(d) )
+ domain_crash(d);
}
#endif
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index cf70a5c43c..9d4ed6d275 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -272,9 +272,6 @@ int iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
printk(XENLOG_ERR
"d%d: IOMMU mapping bfn %"PRI_bfn" to mfn %"PRI_mfn" failed: %d\n",
d->domain_id, bfn_x(bfn), mfn_x(mfn), rc);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
@@ -295,9 +292,6 @@ int iommu_unmap_page(struct domain *d, bfn_t bfn)
printk(XENLOG_ERR
"d%d: IOMMU unmapping bfn %"PRI_bfn" failed: %d\n",
d->domain_id, bfn_x(bfn), rc);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
@@ -335,9 +329,6 @@ int iommu_iotlb_flush(struct domain *d, bfn_t bfn, unsigned int page_count)
printk(XENLOG_ERR
"d%d: IOMMU IOTLB flush failed: %d, bfn %"PRI_bfn", page count %u\n",
d->domain_id, rc, bfn_x(bfn), page_count);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
@@ -358,9 +349,6 @@ int iommu_iotlb_flush_all(struct domain *d)
printk(XENLOG_ERR
"d%d: IOMMU IOTLB flush all failed: %d\n",
d->domain_id, rc);
-
- if ( !is_hardware_domain(d) )
- domain_crash(d);
}
return rc;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 379882c690..09573722bd 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -104,7 +104,11 @@ int arch_iommu_populate_page_table(struct domain *d)
this_cpu(iommu_dont_flush_iotlb) = 0;
if ( !rc )
+ {
rc = iommu_iotlb_flush_all(d);
+ if ( unlikely(rc) )
+ domain_crash(d);
+ }
if ( rc && rc != -ERESTART )
iommu_teardown(d);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 06/13] public / x86: introduce __HYPERCALL_iommu_op
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (4 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 05/13] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-17 13:38 ` [PATCH v3 07/13] iommu: track reserved ranges using a rangeset Paul Durrant
` (6 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Paul Durrant, Jan Beulich,
Daniel De Graaf
This patch introduces the boilerplate for a new hypercall to allow a
domain to control IOMMU mappings for its own pages.
Whilst there is duplication of code between the native and compat entry
points which appears ripe for some form of combination, I think it is
better to maintain the separation as-is because the compat entry point
will necessarily gain complexity in subsequent patches.
NOTE: This hypercall is only implemented for x86 and is currently
restricted by XSM to dom0. Its scope can be expanded in future
if need be.
Signed-off-by: Paul Durrant <paul.durrant@citrix.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: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
v3:
- Push op code into XSM check.
v2:
- Get rid of the can_control_iommu() function, leaving this patch as pure
boilerplate.
- Re-structure the hypercall to use a buffer array, similar to that used
by __HYPERCALL_dm_op, to allow for future expansion of op structure
without affecting binary compatibility.
- Drop use of __ in public header guard.
---
tools/flask/policy/modules/xen.if | 1 +
xen/arch/x86/Makefile | 1 +
xen/arch/x86/hvm/hypercall.c | 1 +
xen/arch/x86/hypercall.c | 1 +
xen/arch/x86/iommu_op.c | 184 ++++++++++++++++++++++++++++++++++++
xen/arch/x86/pv/hypercall.c | 1 +
xen/include/Makefile | 2 +
xen/include/public/iommu_op.h | 64 +++++++++++++
xen/include/public/xen.h | 1 +
xen/include/xen/hypercall.h | 12 +++
xen/include/xlat.lst | 2 +
xen/include/xsm/dummy.h | 6 ++
xen/include/xsm/xsm.h | 6 ++
xen/xsm/dummy.c | 1 +
xen/xsm/flask/hooks.c | 6 ++
xen/xsm/flask/policy/access_vectors | 2 +
16 files changed, 291 insertions(+)
create mode 100644 xen/arch/x86/iommu_op.c
create mode 100644 xen/include/public/iommu_op.h
diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
index 7aefd0061e..5161611471 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -60,6 +60,7 @@ define(`create_domain_common', `
allow $1 $2:grant setup;
allow $1 $2:hvm { getparam hvmctl sethvmc
setparam nested altp2mhvm altp2mhvm_op dm };
+ allow $1 $2:resource control_iommu;
')
# create_domain(priv, target)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5563c813dd..c3cf364439 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
obj-y += hypercall.o
obj-y += i387.o
obj-y += i8259.o
+obj-y += iommu_op.o
obj-y += io_apic.o
obj-$(CONFIG_LIVEPATCH) += alternative.o livepatch.o
obj-y += msi.o
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 85eacd7d33..3574966827 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -137,6 +137,7 @@ static const hypercall_table_t hvm_hypercall_table[] = {
COMPAT_CALL(mmuext_op),
HYPERCALL(xenpmu_op),
COMPAT_CALL(dm_op),
+ COMPAT_CALL(iommu_op),
HYPERCALL(arch_1)
};
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 90e88c1d2c..045753e702 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -68,6 +68,7 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] =
ARGS(xenpmu_op, 2),
ARGS(dm_op, 3),
ARGS(mca, 1),
+ ARGS(iommu_op, 2),
ARGS(arch_1, 1),
};
diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
new file mode 100644
index 0000000000..744c0fce27
--- /dev/null
+++ b/xen/arch/x86/iommu_op.c
@@ -0,0 +1,184 @@
+/******************************************************************************
+ * x86/iommu_op.c
+ *
+ * Paravirtualised IOMMU functionality
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2018 Citrix Systems Inc
+ */
+
+#include <xen/event.h>
+#include <xen/guest_access.h>
+#include <xen/hypercall.h>
+
+static void iommu_op(xen_iommu_op_t *op)
+{
+ switch ( op->op )
+ {
+ default:
+ op->status = -EOPNOTSUPP;
+ break;
+ }
+}
+
+int do_one_iommu_op(xen_iommu_op_buf_t *buf)
+{
+ xen_iommu_op_t op;
+ int rc;
+
+ if ( buf->size < sizeof(op) )
+ return -EFAULT;
+
+ if ( copy_from_guest((void *)&op, buf->h, sizeof(op)) )
+ return -EFAULT;
+
+ if ( op.pad )
+ return -EINVAL;
+
+ rc = xsm_iommu_op(XSM_PRIV, current->domain, op.op);
+ if ( rc )
+ return rc;
+
+ iommu_op(&op);
+
+ if ( __copy_field_to_guest(guest_handle_cast(buf->h, xen_iommu_op_t),
+ &op, status) )
+ return -EFAULT;
+
+ return 0;
+}
+
+long do_iommu_op(unsigned int nr_bufs,
+ XEN_GUEST_HANDLE_PARAM(xen_iommu_op_buf_t) bufs)
+{
+ unsigned int i;
+ long rc = 0;
+
+ for ( i = 0; i < nr_bufs; i++ )
+ {
+ xen_iommu_op_buf_t buf;
+
+ if ( ((i & 0xff) == 0xff) && hypercall_preempt_check() )
+ {
+ rc = i;
+ break;
+ }
+
+ if ( copy_from_guest_offset(&buf, bufs, i, 1) )
+ {
+ rc = -EFAULT;
+ break;
+ }
+
+ rc = do_one_iommu_op(&buf);
+ if ( rc )
+ break;
+ }
+
+ if ( rc > 0 )
+ {
+ ASSERT(rc < nr_bufs);
+ nr_bufs -= rc;
+ guest_handle_add_offset(bufs, rc);
+
+ rc = hypercall_create_continuation(__HYPERVISOR_iommu_op,
+ "ih", nr_bufs, bufs);
+ }
+
+ return rc;
+}
+
+int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
+{
+ compat_iommu_op_t cmp;
+ xen_iommu_op_t nat;
+ int rc;
+
+ if ( buf->size < sizeof(cmp) )
+ return -EFAULT;
+
+ if ( copy_from_compat((void *)&cmp, buf->h, sizeof(cmp)) )
+ return -EFAULT;
+
+ if ( cmp.pad )
+ return -EINVAL;
+
+ rc = xsm_iommu_op(XSM_PRIV, current->domain, cmp.op);
+ if ( rc )
+ return rc;
+
+ XLAT_iommu_op(&nat, &cmp);
+
+ iommu_op(&nat);
+
+ XLAT_iommu_op(&cmp, &nat);
+
+ if ( __copy_field_to_compat(compat_handle_cast(buf->h,
+ compat_iommu_op_t),
+ &cmp, status) )
+ return -EFAULT;
+
+ return 0;
+}
+
+int compat_iommu_op(unsigned int nr_bufs,
+ XEN_GUEST_HANDLE_PARAM(compat_iommu_op_buf_t) bufs)
+{
+ unsigned int i;
+ long rc = 0;
+
+ for ( i = 0; i < nr_bufs; i++ )
+ {
+ compat_iommu_op_buf_t buf;
+
+ if ( ((i & 0xff) == 0xff) && hypercall_preempt_check() )
+ {
+ rc = i;
+ break;
+ }
+
+ if ( copy_from_guest_offset(&buf, bufs, i, 1) )
+ {
+ rc = -EFAULT;
+ break;
+ }
+
+ rc = compat_one_iommu_op(&buf);
+ if ( rc )
+ break;
+ }
+
+ if ( rc > 0 )
+ {
+ ASSERT(rc < nr_bufs);
+ nr_bufs -= rc;
+ guest_handle_add_offset(bufs, rc);
+
+ rc = hypercall_create_continuation(__HYPERVISOR_iommu_op,
+ "ih", nr_bufs, bufs);
+ }
+
+ return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c
index bbc3011d1a..d23f9af42f 100644
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -80,6 +80,7 @@ const hypercall_table_t pv_hypercall_table[] = {
HYPERCALL(xenpmu_op),
COMPAT_CALL(dm_op),
HYPERCALL(mca),
+ COMPAT_CALL(iommu_op),
HYPERCALL(arch_1),
};
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 7c5034e6e0..3e6421c4b4 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -11,6 +11,7 @@ headers-y := \
compat/features.h \
compat/grant_table.h \
compat/kexec.h \
+ compat/iommu_op.h \
compat/memory.h \
compat/nmi.h \
compat/physdev.h \
@@ -29,6 +30,7 @@ headers-$(CONFIG_X86) += compat/arch-x86/xen-$(compat-arch-y).h
headers-$(CONFIG_X86) += compat/hvm/dm_op.h
headers-$(CONFIG_X86) += compat/hvm/hvm_op.h
headers-$(CONFIG_X86) += compat/hvm/hvm_vcpu.h
+headers-$(CONFIG_X86) += compat/iommu_op.h
headers-y += compat/arch-$(compat-arch-y).h compat/pmu.h compat/xlat.h
headers-$(CONFIG_FLASK) += compat/xsm/flask_op.h
diff --git a/xen/include/public/iommu_op.h b/xen/include/public/iommu_op.h
new file mode 100644
index 0000000000..c3b68f665a
--- /dev/null
+++ b/xen/include/public/iommu_op.h
@@ -0,0 +1,64 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (C) 2018 Citrix Systems Inc
+ */
+
+#ifndef XEN_PUBLIC_IOMMU_OP_H
+#define XEN_PUBLIC_IOMMU_OP_H
+
+#include "xen.h"
+
+struct xen_iommu_op {
+ uint16_t op; /* op type */
+ uint16_t pad;
+ int32_t status; /* op completion status: */
+ /* 0 for success otherwise, negative errno */
+};
+typedef struct xen_iommu_op xen_iommu_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_iommu_op_t);
+
+struct xen_iommu_op_buf {
+ XEN_GUEST_HANDLE(void) h;
+ xen_ulong_t size;
+};
+typedef struct xen_iommu_op_buf xen_iommu_op_buf_t;
+DEFINE_XEN_GUEST_HANDLE(xen_iommu_op_buf_t);
+
+/* ` enum neg_errnoval
+ * ` HYPERVISOR_iommu_op(unsigned int nr_bufs,
+ * ` xen_iommu_op_buf_t bufs[])
+ * `
+ *
+ * @nr_bufs is the number of buffers in the @bufs array.
+ * @bufs points to an array of buffers where each contains a struct
+ * xen_iommu_op.
+ */
+
+#endif /* XEN_PUBLIC_IOMMU_OP_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index fb1df8f293..68b0968e7d 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -121,6 +121,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
#define __HYPERVISOR_xc_reserved_op 39 /* reserved for XenClient */
#define __HYPERVISOR_xenpmu_op 40
#define __HYPERVISOR_dm_op 41
+#define __HYPERVISOR_iommu_op 42
/* Architecture-specific hypercall definitions. */
#define __HYPERVISOR_arch_0 48
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index cc99aea57d..2ebc999f4b 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -16,6 +16,7 @@
#include <public/version.h>
#include <public/pmu.h>
#include <public/hvm/dm_op.h>
+#include <public/iommu_op.h>
#include <asm/hypercall.h>
#include <xsm/xsm.h>
@@ -148,6 +149,10 @@ do_dm_op(
unsigned int nr_bufs,
XEN_GUEST_HANDLE_PARAM(xen_dm_op_buf_t) bufs);
+extern long
+do_iommu_op(unsigned int nr_bufs,
+ XEN_GUEST_HANDLE_PARAM(xen_iommu_op_buf_t) bufs);
+
#ifdef CONFIG_COMPAT
extern int
@@ -205,6 +210,13 @@ compat_dm_op(
unsigned int nr_bufs,
XEN_GUEST_HANDLE_PARAM(void) bufs);
+#include <compat/iommu_op.h>
+
+DEFINE_XEN_GUEST_HANDLE(compat_iommu_op_buf_t);
+extern int
+compat_iommu_op(unsigned int nr_bufs,
+ XEN_GUEST_HANDLE_PARAM(compat_iommu_op_buf_t) bufs);
+
#endif
void arch_get_xen_caps(xen_capabilities_info_t *info);
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 527332054a..3b15c18c4e 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -77,6 +77,8 @@
? vcpu_hvm_context hvm/hvm_vcpu.h
? vcpu_hvm_x86_32 hvm/hvm_vcpu.h
? vcpu_hvm_x86_64 hvm/hvm_vcpu.h
+! iommu_op iommu_op.h
+! iommu_op_buf iommu_op.h
? kexec_exec kexec.h
! kexec_image kexec.h
! kexec_range kexec.h
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index ff6b2dbf39..8891da0759 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -715,6 +715,12 @@ static XSM_INLINE int xsm_dm_op(XSM_DEFAULT_ARG struct domain *d)
return xsm_default_action(action, current->domain, d);
}
+static XSM_INLINE int xsm_iommu_op(XSM_DEFAULT_ARG struct domain *d, unsigned int op)
+{
+ XSM_ASSERT_ACTION(XSM_PRIV);
+ return xsm_default_action(action, current->domain, d);
+}
+
#endif /* CONFIG_X86 */
#include <public/version.h>
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index f0c6fc7e29..c47c1418eb 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -178,6 +178,7 @@ struct xsm_operations {
int (*ioport_mapping) (struct domain *d, uint32_t s, uint32_t e, uint8_t allow);
int (*pmu_op) (struct domain *d, unsigned int op);
int (*dm_op) (struct domain *d);
+ int (*iommu_op) (struct domain *d, unsigned int op);
#endif
int (*xen_version) (uint32_t cmd);
int (*domain_resource_map) (struct domain *d);
@@ -686,6 +687,11 @@ static inline int xsm_dm_op(xsm_default_t def, struct domain *d)
return xsm_ops->dm_op(d);
}
+static inline int xsm_iommu_op(xsm_default_t def, struct domain *d, unsigned int op)
+{
+ return xsm_ops->iommu_op(d, op);
+}
+
#endif /* CONFIG_X86 */
static inline int xsm_xen_version (xsm_default_t def, uint32_t op)
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 6e751199ee..02512a1566 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -155,6 +155,7 @@ void __init xsm_fixup_ops (struct xsm_operations *ops)
set_to_dummy_if_null(ops, ioport_mapping);
set_to_dummy_if_null(ops, pmu_op);
set_to_dummy_if_null(ops, dm_op);
+ set_to_dummy_if_null(ops, iommu_op);
#endif
set_to_dummy_if_null(ops, xen_version);
set_to_dummy_if_null(ops, domain_resource_map);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 78bc32602e..a04786106f 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1672,6 +1672,11 @@ static int flask_dm_op(struct domain *d)
return current_has_perm(d, SECCLASS_HVM, HVM__DM);
}
+static int flask_iommu_op(struct domain *d)
+{
+ return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__CONTROL_IOMMU);
+}
+
#endif /* CONFIG_X86 */
static int flask_xen_version (uint32_t op)
@@ -1850,6 +1855,7 @@ static struct xsm_operations flask_ops = {
.ioport_mapping = flask_ioport_mapping,
.pmu_op = flask_pmu_op,
.dm_op = flask_dm_op,
+ .iommu_op = flask_iommu_op,
#endif
.xen_version = flask_xen_version,
.domain_resource_map = flask_domain_resource_map,
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index c5d85486d0..0c894b733e 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -473,6 +473,8 @@ class resource
# checked for PHYSDEVOP_setup_gsi (target IRQ)
# checked for PHYSDEVOP_pci_mmcfg_reserved (target xen_t)
setup
+# checked for IOMMU_OP
+ control_iommu
}
# Class security describes the FLASK security server itself; these operations
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 07/13] iommu: track reserved ranges using a rangeset
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (5 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 06/13] public / x86: introduce __HYPERCALL_iommu_op Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-19 9:01 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 08/13] x86: add iommu_op to query reserved ranges Paul Durrant
` (5 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Paul Durrant, Jan Beulich
Ranges that should be considered reserved in the IOMMU are not necessarily
limited to RMRRs. If iommu_inclusive_mapping is set then any frame number
falling within an E820 reserved region should also be considered as
reserved in the IOMMU.
This patch adds a rangeset to the domain_iommu structure that is then used
to track all reserved ranges. This will be needed by a subsequent patch
to test whether it is safe to modify a particular IOMMU entry.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Kevin Tian <kevin.tian@intel.com>
v2:
- New in v2.
---
xen/drivers/passthrough/iommu.c | 10 +++++++++-
xen/drivers/passthrough/vtd/iommu.c | 20 +++++++++++++-------
xen/drivers/passthrough/vtd/x86/vtd.c | 17 ++++++++++++++++-
xen/include/xen/iommu.h | 6 ++++++
4 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 9d4ed6d275..2048342f3e 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -147,6 +147,10 @@ int iommu_domain_init(struct domain *d)
if ( !iommu_enabled )
return 0;
+ hd->reserved_ranges = rangeset_new(d, NULL, 0);
+ if ( !hd->reserved_ranges )
+ return -ENOMEM;
+
hd->platform_ops = iommu_get_ops();
return hd->platform_ops->init(d);
}
@@ -248,12 +252,16 @@ int iommu_construct(struct domain *d)
void iommu_domain_destroy(struct domain *d)
{
- if ( !iommu_enabled || !dom_iommu(d)->platform_ops )
+ const struct domain_iommu *hd = dom_iommu(d);
+
+ if ( !iommu_enabled || !hd->platform_ops )
return;
iommu_teardown(d);
arch_iommu_domain_destroy(d);
+
+ rangeset_destroy(hd->reserved_ranges);
}
int iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index c9f50f04ad..282e227414 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1910,6 +1910,7 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map,
unsigned long end_pfn = PAGE_ALIGN_4K(rmrr->end_address) >> PAGE_SHIFT_4K;
struct mapped_rmrr *mrmrr;
struct domain_iommu *hd = dom_iommu(d);
+ int err = 0;
ASSERT(pcidevs_locked());
ASSERT(rmrr->base_address < rmrr->end_address);
@@ -1923,8 +1924,6 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map,
if ( mrmrr->base == rmrr->base_address &&
mrmrr->end == rmrr->end_address )
{
- int ret = 0;
-
if ( map )
{
++mrmrr->count;
@@ -1934,28 +1933,35 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map,
if ( --mrmrr->count )
return 0;
- while ( base_pfn < end_pfn )
+ err = rangeset_remove_range(hd->reserved_ranges,
+ base_pfn, end_pfn);
+ while ( !err && base_pfn < end_pfn )
{
if ( clear_identity_p2m_entry(d, base_pfn) )
- ret = -ENXIO;
+ err = -ENXIO;
+
base_pfn++;
}
list_del(&mrmrr->list);
xfree(mrmrr);
- return ret;
+ return err;
}
}
if ( !map )
return -ENOENT;
+ err = rangeset_add_range(hd->reserved_ranges, base_pfn, end_pfn);
+ if ( err )
+ return err;
+
while ( base_pfn < end_pfn )
{
- int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
-
+ err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
if ( err )
return err;
+
base_pfn++;
}
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index 6fed4a92cb..032412b8c6 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -164,10 +164,25 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
if ( !rc )
rc = ret;
+
+ /*
+ * The only reason a reserved page would be mapped is that
+ * iommu_inclusive_mapping is set, in which case the BFN
+ * needs to be marked as reserved in the IOMMU.
+ */
+ if ( page_is_ram_type(pfn, RAM_TYPE_RESERVED) )
+ {
+ ASSERT(iommu_inclusive_mapping);
+
+ ret = rangeset_add_singleton(dom_iommu(d)->reserved_ranges,
+ bfn_x(bfn));
+ if ( !rc )
+ rc = ret;
+ }
}
if ( rc )
- printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n",
+ printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping/reservation failed: %d\n",
d->domain_id, rc);
if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K))))
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 624784fec8..cc0be81b4e 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -122,6 +122,12 @@ struct domain_iommu {
/* Features supported by the IOMMU */
DECLARE_BITMAP(features, IOMMU_FEAT_count);
+
+ /*
+ * BFN ranges that are reserved in the domain IOMMU mappings and
+ * must not be modified after initialization.
+ */
+ struct rangeset *reserved_ranges;
};
#define dom_iommu(d) (&(d)->iommu)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 08/13] x86: add iommu_op to query reserved ranges
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (6 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 07/13] iommu: track reserved ranges using a rangeset Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-19 9:37 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 09/13] vtd: add lookup_page method to iommu_ops Paul Durrant
` (4 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Paul Durrant, Jan Beulich
This patch adds an iommu_op to allow the domain IOMMU reserved ranges to be
queried by the guest.
NOTE: The number of reserved ranges is determined by system firmware, in
conjunction with Xen command line options, and is expected to be
small. Thus, to avoid over-complicating the code, there is no
pre-emption check within the operation.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.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: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v3:
- Avoid speculation beyond array bounds check.
v2:
- Re-implemented for v2 based on new rangeset.
---
xen/arch/x86/iommu_op.c | 170 ++++++++++++++++++++++++++++++++++++++++--
xen/include/public/iommu_op.h | 39 ++++++++++
xen/include/xlat.lst | 2 +
3 files changed, 205 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
index 744c0fce27..97811f8ced 100644
--- a/xen/arch/x86/iommu_op.c
+++ b/xen/arch/x86/iommu_op.c
@@ -22,11 +22,70 @@
#include <xen/event.h>
#include <xen/guest_access.h>
#include <xen/hypercall.h>
+#include <xen/nospec.h>
+
+struct get_reserved_ctxt {
+ unsigned int max_entries;
+ unsigned int nr_entries;
+ XEN_GUEST_HANDLE(xen_iommu_reserved_range_t) ranges;
+};
+
+static int get_reserved(unsigned long s, unsigned long e, void *arg)
+{
+ struct get_reserved_ctxt *ctxt = arg;
+
+ if ( ctxt->nr_entries < ctxt->max_entries )
+ {
+ xen_iommu_reserved_range_t range = {
+ .start_bfn = s,
+ .nr_frames = e - s,
+ };
+
+ if ( copy_to_guest_offset(ctxt->ranges, ctxt->nr_entries, &range,
+ 1) )
+ return -EFAULT;
+ }
+
+ ctxt->nr_entries++;
+ return 0;
+}
+
+static int iommu_op_query_reserved(struct xen_iommu_op_query_reserved *op)
+{
+ struct domain *currd = current->domain;
+ struct domain_iommu *iommu = dom_iommu(currd);
+ struct get_reserved_ctxt ctxt = {
+ .max_entries = op->nr_entries,
+ .ranges = op->ranges,
+ };
+ int rc;
+
+ if ( op->pad )
+ return -EINVAL;
+
+ rc = rangeset_report_ranges(iommu->reserved_ranges, 0, ~0ul,
+ get_reserved, &ctxt);
+ if ( rc )
+ return rc;
+
+ /* Pass back the actual number of reserved ranges */
+ op->nr_entries = ctxt.nr_entries;
+
+ if ( !guest_handle_is_null(ctxt.ranges) &&
+ ctxt.nr_entries > ctxt.max_entries )
+ return -ENOBUFS;
+
+ return 0;
+}
static void iommu_op(xen_iommu_op_t *op)
{
switch ( op->op )
{
+ case XEN_IOMMUOP_query_reserved:
+ op->status = iommu_op_query_reserved(&op->u.query_reserved);
+ break;
+
default:
op->status = -EOPNOTSUPP;
break;
@@ -35,13 +94,20 @@ static void iommu_op(xen_iommu_op_t *op)
int do_one_iommu_op(xen_iommu_op_buf_t *buf)
{
- xen_iommu_op_t op;
+ xen_iommu_op_t op = {};
+ size_t offset;
+ static const size_t op_size[] = {
+ [XEN_IOMMUOP_query_reserved] = sizeof(struct xen_iommu_op_query_reserved),
+ };
+ size_t size;
int rc;
- if ( buf->size < sizeof(op) )
+ offset = offsetof(struct xen_iommu_op, u);
+
+ if ( buf->size < offset )
return -EFAULT;
- if ( copy_from_guest((void *)&op, buf->h, sizeof(op)) )
+ if ( copy_from_guest((void *)&op, buf->h, offset) )
return -EFAULT;
if ( op.pad )
@@ -51,6 +117,16 @@ int do_one_iommu_op(xen_iommu_op_buf_t *buf)
if ( rc )
return rc;
+ if ( op.op >= ARRAY_SIZE(op_size) )
+ return -EOPNOTSUPP;
+
+ size = op_size[array_index_nospec(op.op, ARRAY_SIZE(op_size))];
+ if ( buf->size < offset + size )
+ return -EFAULT;
+
+ if ( copy_from_guest_offset((void *)&op.u, buf->h, offset, size) )
+ return -EFAULT;
+
iommu_op(&op);
if ( __copy_field_to_guest(guest_handle_cast(buf->h, xen_iommu_op_t),
@@ -102,14 +178,23 @@ long do_iommu_op(unsigned int nr_bufs,
int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
{
- compat_iommu_op_t cmp;
+ compat_iommu_op_t cmp = {};
+ size_t offset;
+ static const size_t op_size[] = {
+ [XEN_IOMMUOP_query_reserved] = sizeof(struct compat_iommu_op_query_reserved),
+ };
+ size_t size;
xen_iommu_op_t nat;
+ unsigned int u;
+ int32_t status;
int rc;
- if ( buf->size < sizeof(cmp) )
+ offset = offsetof(struct compat_iommu_op, u);
+
+ if ( buf->size < offset )
return -EFAULT;
- if ( copy_from_compat((void *)&cmp, buf->h, sizeof(cmp)) )
+ if ( copy_from_compat((void *)&cmp, buf->h, offset) )
return -EFAULT;
if ( cmp.pad )
@@ -119,12 +204,85 @@ int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
if ( rc )
return rc;
+ if ( cmp.op >= ARRAY_SIZE(op_size) )
+ return -EOPNOTSUPP;
+
+ size = op_size[array_index_nospec(cmp.op, ARRAY_SIZE(op_size))];
+ if ( buf->size < offset + size )
+ return -EFAULT;
+
+ if ( copy_from_compat_offset((void *)&cmp.u, buf->h, offset, size) )
+ return -EFAULT;
+
+ /*
+ * The xlat magic doesn't quite know how to handle the union so
+ * we need to fix things up here.
+ */
+#define XLAT_iommu_op_u_query_reserved XEN_IOMMUOP_query_reserved
+ u = cmp.op;
+
+#define XLAT_iommu_op_query_reserved_HNDL_ranges(_d_, _s_) \
+ do \
+ { \
+ if ( !compat_handle_is_null((_s_)->ranges) ) \
+ { \
+ unsigned int *nr_entries = COMPAT_ARG_XLAT_VIRT_BASE; \
+ xen_iommu_reserved_range_t *ranges = \
+ (void *)(nr_entries + 1); \
+ \
+ if ( sizeof(*nr_entries) + \
+ (sizeof(*ranges) * (_s_)->nr_entries) > \
+ COMPAT_ARG_XLAT_SIZE ) \
+ return -E2BIG; \
+ \
+ *nr_entries = (_s_)->nr_entries; \
+ set_xen_guest_handle((_d_)->ranges, ranges); \
+ } \
+ else \
+ set_xen_guest_handle((_d_)->ranges, NULL); \
+ } while (false)
+
XLAT_iommu_op(&nat, &cmp);
+#undef XLAT_iommu_op_query_reserved_HNDL_ranges
+
iommu_op(&nat);
+ status = nat.status;
+
+#define XLAT_iommu_op_query_reserved_HNDL_ranges(_d_, _s_) \
+ do \
+ { \
+ if ( !compat_handle_is_null((_d_)->ranges) ) \
+ { \
+ unsigned int *nr_entries = COMPAT_ARG_XLAT_VIRT_BASE; \
+ xen_iommu_reserved_range_t *ranges = \
+ (void *)(nr_entries + 1); \
+ unsigned int j; \
+ \
+ for ( j = 0; \
+ j < min_t(unsigned int, (_d_)->nr_entries, \
+ *nr_entries); \
+ j++ ) \
+ { \
+ compat_iommu_reserved_range_t range; \
+ \
+ XLAT_iommu_reserved_range(&range, &ranges[j]); \
+ \
+ if ( __copy_to_compat_offset((_d_)->ranges, j, \
+ &range, 1) ) \
+ status = -EFAULT; \
+ } \
+ } \
+ } while (false)
+
XLAT_iommu_op(&cmp, &nat);
+ /* status will have been modified if __copy_to_compat_offset() failed */
+ cmp.status = status;
+
+#undef XLAT_iommu_op_query_reserved_HNDL_ranges
+
if ( __copy_field_to_compat(compat_handle_cast(buf->h,
compat_iommu_op_t),
&cmp, status) )
diff --git a/xen/include/public/iommu_op.h b/xen/include/public/iommu_op.h
index c3b68f665a..02213c12a4 100644
--- a/xen/include/public/iommu_op.h
+++ b/xen/include/public/iommu_op.h
@@ -25,11 +25,50 @@
#include "xen.h"
+typedef unsigned long xen_bfn_t;
+
+/* Structure describing a single range reserved in the IOMMU */
+struct xen_iommu_reserved_range {
+ xen_bfn_t start_bfn;
+ unsigned int nr_frames;
+ unsigned int pad;
+};
+typedef struct xen_iommu_reserved_range xen_iommu_reserved_range_t;
+DEFINE_XEN_GUEST_HANDLE(xen_iommu_reserved_range_t);
+
+/*
+ * XEN_IOMMUOP_query_reserved: Query ranges reserved in the IOMMU.
+ */
+#define XEN_IOMMUOP_query_reserved 1
+
+struct xen_iommu_op_query_reserved {
+ /*
+ * IN/OUT - On entry this is the number of entries available
+ * in the ranges array below.
+ * On exit this is the actual number of reserved ranges.
+ */
+ unsigned int nr_entries;
+ unsigned int pad;
+ /*
+ * OUT - This array is populated with reserved ranges. If it is
+ * not sufficiently large then available entries are populated,
+ * but the op status code will be set to -ENOBUFS.
+ * It is permissable to set this to NULL if nr_entries is also
+ * set to zero. In this case, on exit, nr_entries will still be
+ * set to the actual number of reserved ranges but the status
+ * code will be set to zero.
+ */
+ XEN_GUEST_HANDLE(xen_iommu_reserved_range_t) ranges;
+};
+
struct xen_iommu_op {
uint16_t op; /* op type */
uint16_t pad;
int32_t status; /* op completion status: */
/* 0 for success otherwise, negative errno */
+ union {
+ struct xen_iommu_op_query_reserved query_reserved;
+ } u;
};
typedef struct xen_iommu_op xen_iommu_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_iommu_op_t);
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 3b15c18c4e..93bcf4b4d0 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -79,6 +79,8 @@
? vcpu_hvm_x86_64 hvm/hvm_vcpu.h
! iommu_op iommu_op.h
! iommu_op_buf iommu_op.h
+! iommu_op_query_reserved iommu_op.h
+! iommu_reserved_range iommu_op.h
? kexec_exec kexec.h
! kexec_image kexec.h
! kexec_range kexec.h
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 09/13] vtd: add lookup_page method to iommu_ops
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (7 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 08/13] x86: add iommu_op to query reserved ranges Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-19 9:55 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings Paul Durrant
` (3 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Paul Durrant, George Dunlap, Jan Beulich
This patch adds a new method to the VT-d IOMMU implementation to find the
MFN currently mapped by the specified BFN along with a wrapper function in
generic IOMMU code to call the implementation if it exists.
This functionality will be used by a subsequent patch.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: George Dunlap <george.dunlap@citrix.com>
v3:
- Addressed comments from George.
v2:
- Addressed some comments from Jan.
---
xen/drivers/passthrough/iommu.c | 11 +++++++++++
xen/drivers/passthrough/vtd/iommu.c | 34 ++++++++++++++++++++++++++++++++++
xen/drivers/passthrough/vtd/iommu.h | 3 +++
xen/include/xen/iommu.h | 4 ++++
4 files changed, 52 insertions(+)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 2048342f3e..fb9d0e1848 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -305,6 +305,17 @@ int iommu_unmap_page(struct domain *d, bfn_t bfn)
return rc;
}
+int iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
+ unsigned int *flags)
+{
+ const struct domain_iommu *hd = dom_iommu(d);
+
+ if ( !iommu_enabled || !hd->platform_ops )
+ return -EOPNOTSUPP;
+
+ return hd->platform_ops->lookup_page(d, bfn, mfn, flags);
+}
+
static void iommu_free_pagetables(unsigned long unused)
{
do {
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 282e227414..8cd3b59aa0 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1830,6 +1830,39 @@ static int __must_check intel_iommu_unmap_page(struct domain *d,
return dma_pte_clear_one(d, bfn_to_baddr(bfn));
}
+static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
+ unsigned int *flags)
+{
+ struct domain_iommu *hd = dom_iommu(d);
+ struct dma_pte *page = NULL, *pte = NULL, val;
+ u64 pg_maddr;
+
+ spin_lock(&hd->arch.mapping_lock);
+
+ pg_maddr = addr_to_dma_page_maddr(d, bfn_to_baddr(bfn), 0);
+ if ( pg_maddr == 0 )
+ {
+ spin_unlock(&hd->arch.mapping_lock);
+ return -ENOMEM;
+ }
+
+ page = map_vtd_domain_page(pg_maddr);
+ pte = page + (bfn_x(bfn) & LEVEL_MASK);
+ val = *pte;
+
+ unmap_vtd_domain_page(page);
+ spin_unlock(&hd->arch.mapping_lock);
+
+ if ( !dma_pte_present(val) )
+ return -ENOENT;
+
+ *mfn = maddr_to_mfn(dma_pte_addr(val));
+ *flags = dma_pte_read(val) ? IOMMUF_readable : 0;
+ *flags |= dma_pte_write(val) ? IOMMUF_writable : 0;
+
+ return 0;
+}
+
int iommu_pte_flush(struct domain *d, uint64_t bfn, uint64_t *pte,
int order, int present)
{
@@ -2661,6 +2694,7 @@ const struct iommu_ops intel_iommu_ops = {
.teardown = iommu_domain_teardown,
.map_page = intel_iommu_map_page,
.unmap_page = intel_iommu_unmap_page,
+ .lookup_page = intel_iommu_lookup_page,
.free_page_table = iommu_free_page_table,
.reassign_device = reassign_device_ownership,
.get_device_group_id = intel_iommu_group_id,
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index 72c1a2e3cd..47bdfcb5ea 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -272,6 +272,9 @@ struct dma_pte {
#define dma_set_pte_prot(p, prot) do { \
(p).val = ((p).val & ~DMA_PTE_PROT) | ((prot) & DMA_PTE_PROT); \
} while (0)
+#define dma_pte_prot(p) ((p).val & DMA_PTE_PROT)
+#define dma_pte_read(p) (dma_pte_prot(p) & DMA_PTE_READ)
+#define dma_pte_write(p) (dma_pte_prot(p) & DMA_PTE_WRITE)
#define dma_pte_addr(p) ((p).val & PADDR_MASK & PAGE_MASK_4K)
#define dma_set_pte_addr(p, addr) do {\
(p).val |= ((addr) & PAGE_MASK_4K); } while (0)
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index cc0be81b4e..7c5d46df81 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -100,6 +100,8 @@ void iommu_teardown(struct domain *d);
int __must_check iommu_map_page(struct domain *d, bfn_t bfn,
mfn_t mfn, unsigned int flags);
int __must_check iommu_unmap_page(struct domain *d, bfn_t bfn);
+int __must_check iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
+ unsigned int *flags);
enum iommu_feature
{
@@ -198,6 +200,8 @@ struct iommu_ops {
int __must_check (*map_page)(struct domain *d, bfn_t bfn, mfn_t mfn,
unsigned int flags);
int __must_check (*unmap_page)(struct domain *d, bfn_t bfn);
+ int __must_check (*lookup_page)(struct domain *d, bfn_t bfn, mfn_t *mfn,
+ unsigned int *flags);
void (*free_page_table)(struct page_info *);
#ifdef CONFIG_X86
void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value);
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (8 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 09/13] vtd: add lookup_page method to iommu_ops Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-20 8:59 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 11/13] memory: add get_paged_gfn() as a wrapper Paul Durrant
` (2 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich
This patch adds an iommu_op which checks whether it is possible or
safe for a domain to modify its own IOMMU mappings and, if so, creates
a rangeset to track modifications.
NOTE: The actual map and unmap operations are introduced by subsequent
patches.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.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: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v2:
- New in v2.
---
xen/arch/x86/iommu_op.c | 42 +++++++++++++++++++++++++++++++++++++++++
xen/drivers/passthrough/iommu.c | 2 +-
xen/include/public/iommu_op.h | 6 ++++++
xen/include/xen/iommu.h | 3 +++
4 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
index 97811f8ced..a70ce9f272 100644
--- a/xen/arch/x86/iommu_op.c
+++ b/xen/arch/x86/iommu_op.c
@@ -78,6 +78,42 @@ static int iommu_op_query_reserved(struct xen_iommu_op_query_reserved *op)
return 0;
}
+static int iommu_op_enable_modification(void)
+{
+ struct domain *currd = current->domain;
+ struct domain_iommu *iommu = dom_iommu(currd);
+ const struct iommu_ops *ops = iommu->platform_ops;
+
+ /* Has modification already been enabled? */
+ if ( iommu->iommu_op_ranges )
+ return 0;
+
+ /*
+ * The IOMMU mappings cannot be modified if:
+ * - the IOMMU is not enabled or,
+ * - the current domain is dom0 and tranlsation is disabled or,
+ * - HAP is enabled and the IOMMU shares the mappings.
+ */
+ if ( !iommu_enabled ||
+ (is_hardware_domain(currd) && iommu_passthrough) ||
+ iommu_use_hap_pt(currd) )
+ return -EACCES;
+
+ /*
+ * The IOMMU implementation must provide the lookup method if
+ * modification of the mappings is to be supported.
+ */
+ if ( !ops->lookup_page )
+ return -EOPNOTSUPP;
+
+ iommu->iommu_op_ranges = rangeset_new(currd, NULL, 0);
+ if ( !iommu->iommu_op_ranges )
+ return -ENOMEM;
+
+ currd->need_iommu = 0; /* Disable identity GFN mapping */
+ return 0;
+}
+
static void iommu_op(xen_iommu_op_t *op)
{
switch ( op->op )
@@ -86,6 +122,10 @@ static void iommu_op(xen_iommu_op_t *op)
op->status = iommu_op_query_reserved(&op->u.query_reserved);
break;
+ case XEN_IOMMUOP_enable_modification:
+ op->status = iommu_op_enable_modification();
+ break;
+
default:
op->status = -EOPNOTSUPP;
break;
@@ -98,6 +138,7 @@ int do_one_iommu_op(xen_iommu_op_buf_t *buf)
size_t offset;
static const size_t op_size[] = {
[XEN_IOMMUOP_query_reserved] = sizeof(struct xen_iommu_op_query_reserved),
+ [XEN_IOMMUOP_enable_modification] = 0,
};
size_t size;
int rc;
@@ -182,6 +223,7 @@ int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
size_t offset;
static const size_t op_size[] = {
[XEN_IOMMUOP_query_reserved] = sizeof(struct compat_iommu_op_query_reserved),
+ [XEN_IOMMUOP_enable_modification] = 0,
};
size_t size;
xen_iommu_op_t nat;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index fb9d0e1848..c517428621 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -26,7 +26,6 @@ static void iommu_dump_p2m_table(unsigned char key);
unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
integer_param("iommu_dev_iotlb_timeout", iommu_dev_iotlb_timeout);
-
/*
* The 'iommu' parameter enables the IOMMU. Optional comma separated
* value may contain:
@@ -262,6 +261,7 @@ void iommu_domain_destroy(struct domain *d)
arch_iommu_domain_destroy(d);
rangeset_destroy(hd->reserved_ranges);
+ rangeset_destroy(hd->iommu_op_ranges);
}
int iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
diff --git a/xen/include/public/iommu_op.h b/xen/include/public/iommu_op.h
index 02213c12a4..5a3148c247 100644
--- a/xen/include/public/iommu_op.h
+++ b/xen/include/public/iommu_op.h
@@ -61,6 +61,12 @@ struct xen_iommu_op_query_reserved {
XEN_GUEST_HANDLE(xen_iommu_reserved_range_t) ranges;
};
+/*
+ * XEN_IOMMUOP_enable_modification: Enable operations that modify IOMMU
+ * mappings.
+ */
+#define XEN_IOMMUOP_enable_modification 2
+
struct xen_iommu_op {
uint16_t op; /* op type */
uint16_t pad;
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 7c5d46df81..08b163cbcb 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -130,6 +130,9 @@ struct domain_iommu {
* must not be modified after initialization.
*/
struct rangeset *reserved_ranges;
+
+ /* Ranges under the control of iommu_op */
+ struct rangeset *iommu_op_ranges;
};
#define dom_iommu(d) (&(d)->iommu)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 11/13] memory: add get_paged_gfn() as a wrapper...
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (9 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-17 13:38 ` [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings Paul Durrant
2018-07-17 13:38 ` [PATCH v3 13/13] x86: extend the map and unmap iommu_ops to support grant references Paul Durrant
12 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich
...for some uses of get_page_from_gfn().
There are many occurences of the following pattern in the code:
q = <readonly look-up> ? P2M_ALLOC : P2M_UNSHARE;
page = get_page_from_gfn(d, gfn, &p2mt, q);
if ( p2m_is_paging(p2mt) )
{
if ( page )
put_page(page);
p2m_mem_paging_populate(d, gfn);
return <-EAGAIN or equivalent>;
}
if ( (q & P2M_UNSHARE) && p2m_is_shared(p2mt) )
{
if ( page )
put_page(page);
return <-EAGAIN or equivalent>;
}
if ( !page )
return <-EINVAL or equivalent>;
if ( !p2m_is_ram(p2mt) ||
(!<readonly look-up> && p2m_is_readonly(p2mt)) )
{
put_page(page);
return <-EINVAL or equivalent>;
}
There are some small differences between the exact way the occurrences are
coded but the desired semantic is the same.
This patch introduces a new common implementation of this code in
get_paged_gfn() and then converts the various open-coded patterns into
calls to this new function.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.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: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v3:
- Addressed comments from George.
v2:
- New in v2.
---
xen/arch/x86/hvm/emulate.c | 32 ++++++--------------------
xen/arch/x86/hvm/hvm.c | 16 ++-----------
xen/common/grant_table.c | 38 +++++++++----------------------
xen/common/memory.c | 56 +++++++++++++++++++++++++++++++++++++---------
xen/include/asm-arm/p2m.h | 3 +++
xen/include/asm-x86/p2m.h | 2 ++
6 files changed, 71 insertions(+), 76 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 8385c62145..c26281ea1c 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -332,34 +332,16 @@ static int hvmemul_do_io_buffer(
static int hvmemul_acquire_page(unsigned long gmfn, struct page_info **page)
{
- struct domain *curr_d = current->domain;
- p2m_type_t p2mt;
-
- *page = get_page_from_gfn(curr_d, gmfn, &p2mt, P2M_UNSHARE);
-
- if ( *page == NULL )
- return X86EMUL_UNHANDLEABLE;
-
- if ( p2m_is_paging(p2mt) )
- {
- put_page(*page);
- p2m_mem_paging_populate(curr_d, gmfn);
- return X86EMUL_RETRY;
- }
-
- if ( p2m_is_shared(p2mt) )
+ switch ( get_paged_gfn(current->domain, _gfn(gmfn), false, NULL, page) )
{
- put_page(*page);
+ case -EAGAIN:
return X86EMUL_RETRY;
- }
-
- /* This code should not be reached if the gmfn is not RAM */
- if ( p2m_is_mmio(p2mt) )
- {
- domain_crash(curr_d);
-
- put_page(*page);
+ case -EINVAL:
return X86EMUL_UNHANDLEABLE;
+ default:
+ ASSERT_UNREACHABLE();
+ case 0:
+ break;
}
return X86EMUL_OKAY;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e022f5ab0e..b7dba7168a 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2558,24 +2558,12 @@ static void *_hvm_map_guest_frame(unsigned long gfn, bool_t permanent,
bool_t *writable)
{
void *map;
- p2m_type_t p2mt;
struct page_info *page;
struct domain *d = current->domain;
+ p2m_type_t p2mt;
- page = get_page_from_gfn(d, gfn, &p2mt,
- writable ? P2M_UNSHARE : P2M_ALLOC);
- if ( (p2m_is_shared(p2mt) && writable) || !page )
- {
- if ( page )
- put_page(page);
- return NULL;
- }
- if ( p2m_is_paging(p2mt) )
- {
- put_page(page);
- p2m_mem_paging_populate(d, gfn);
+ if ( get_paged_gfn(d, _gfn(gfn), !writable, &p2mt, &page) )
return NULL;
- }
if ( writable )
{
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index a179c9cf2a..a492df2362 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -375,39 +375,23 @@ static int get_paged_frame(unsigned long gfn, mfn_t *mfn,
struct page_info **page, bool readonly,
struct domain *rd)
{
- int rc = GNTST_okay;
- p2m_type_t p2mt;
-
- *mfn = INVALID_MFN;
- *page = get_page_from_gfn(rd, gfn, &p2mt,
- readonly ? P2M_ALLOC : P2M_UNSHARE);
- if ( !*page )
- {
-#ifdef P2M_SHARED_TYPES
- if ( p2m_is_shared(p2mt) )
- return GNTST_eagain;
-#endif
-#ifdef P2M_PAGES_TYPES
- if ( p2m_is_paging(p2mt) )
- {
- p2m_mem_paging_populate(rd, gfn);
- return GNTST_eagain;
- }
-#endif
- return GNTST_bad_page;
- }
+ int rc;
- if ( p2m_is_foreign(p2mt) )
+ rc = get_paged_gfn(rd, _gfn(gfn), readonly, NULL, page);
+ switch ( rc )
{
- put_page(*page);
- *page = NULL;
-
+ case -EAGAIN:
+ return GNTST_eagain;
+ case -EINVAL:
return GNTST_bad_page;
+ default:
+ ASSERT_UNREACHABLE();
+ case 0:
+ break;
}
*mfn = page_to_mfn(*page);
-
- return rc;
+ return GNTST_okay;
}
static inline void
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 9012b83aae..46ee5ca372 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1571,37 +1571,73 @@ void destroy_ring_for_helper(
}
}
-int prepare_ring_for_helper(
- struct domain *d, unsigned long gmfn, struct page_info **_page,
- void **_va)
+/*
+ * Acquire a pointer to struct page_info for a specified doman and GFN,
+ * checking whether the page has been paged out, or needs unsharing.
+ * If the function succeeds then zero is returned and page_p is written
+ * with a pointer to the struct page_info with a reference taken. The
+ * caller is responsible for dropping the reference. If p2mt_p is non-NULL
+ * then it is also written with the P2M type of the page.
+ * If the function fails then an appropriate errno is returned and the
+ * values referenced by page_p and p2mt_p are undefined.
+ */
+int get_paged_gfn(struct domain *d, gfn_t gfn, bool readonly,
+ p2m_type_t *p2mt_p, struct page_info **page_p)
{
- struct page_info *page;
+ p2m_query_t q = readonly ? P2M_ALLOC : P2M_UNSHARE;
p2m_type_t p2mt;
- void *va;
+ struct page_info *page;
- page = get_page_from_gfn(d, gmfn, &p2mt, P2M_UNSHARE);
+ page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q);
#ifdef CONFIG_HAS_MEM_PAGING
if ( p2m_is_paging(p2mt) )
{
if ( page )
put_page(page);
- p2m_mem_paging_populate(d, gmfn);
- return -ENOENT;
+
+ p2m_mem_paging_populate(d, gfn_x(gfn));
+ return -EAGAIN;
}
#endif
#ifdef CONFIG_HAS_MEM_SHARING
- if ( p2m_is_shared(p2mt) )
+ if ( (q & P2M_UNSHARE) && p2m_is_shared(p2mt) )
{
if ( page )
put_page(page);
- return -ENOENT;
+
+ return -EAGAIN;
}
#endif
if ( !page )
return -EINVAL;
+ if ( !p2m_is_ram(p2mt) || (!readonly && p2m_is_readonly(p2mt)) )
+ {
+ put_page(page);
+ return -EINVAL;
+ }
+
+ if ( p2mt_p )
+ *p2mt_p = p2mt;
+
+ *page_p = page;
+ return 0;
+}
+
+int prepare_ring_for_helper(
+ struct domain *d, unsigned long gmfn, struct page_info **_page,
+ void **_va)
+{
+ struct page_info *page;
+ void *va;
+ int rc;
+
+ rc = get_paged_gfn(d, _gfn(gmfn), false, NULL, &page);
+ if ( rc )
+ return (rc == -EAGAIN) ? -ENOENT : rc;
+
if ( !get_page_type(page, PGT_writable_page) )
{
put_page(page);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 8823707c17..a39a4faabd 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -303,6 +303,9 @@ static inline struct page_info *get_page_from_gfn(
return page;
}
+int get_paged_gfn(struct domain *d, gfn_t gfn, bool readonly,
+ p2m_type_t *p2mt_p, struct page_info **page_p);
+
int get_page_type(struct page_info *page, unsigned long type);
bool is_iomem_page(mfn_t mfn);
static inline int get_page_and_type(struct page_info *page,
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index d4b3cfcb6e..e890bcd3e1 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -492,6 +492,8 @@ static inline struct page_info *get_page_from_gfn(
return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
}
+int get_paged_gfn(struct domain *d, gfn_t gfn, bool readonly,
+ p2m_type_t *p2mt_p, struct page_info **page_p);
/* General conversion function from mfn to gfn */
static inline unsigned long mfn_to_gfn(struct domain *d, mfn_t mfn)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (10 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 11/13] memory: add get_paged_gfn() as a wrapper Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
2018-07-23 13:35 ` Wei Liu
2018-07-23 14:26 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 13/13] x86: extend the map and unmap iommu_ops to support grant references Paul Durrant
12 siblings, 2 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich
This patch adds iommu_ops to add (map) or remove (unmap) frames in the
domain's IOMMU mappings, and an iommu_op to synchronize (flush) those
manipulations with the hardware.
Mappings added by the map operation are tracked and only those mappings
may be removed by a subsequent unmap operation. Frames are specified by the
owning domain and GFN. It is, of course, permissable for a domain to map
its own frames using DOMID_SELF.
NOTE: The owning domain and GFN must also be specified in the unmap
operation, as well as the BFN, so that they can be cross-checked
with the existent mapping.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.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: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v3:
- Add type pinning.
v2:
- Heavily re-worked in v2, including explicit tracking of mappings.
This avoids the need to clear non-reserved mappings from IOMMU
at start of day, which would be prohibitively slow on a large host.
---
xen/arch/x86/iommu_op.c | 141 ++++++++++++++++++++++++++++++++++++++++++
xen/include/public/iommu_op.h | 43 +++++++++++++
xen/include/xlat.lst | 2 +
3 files changed, 186 insertions(+)
diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
index a70ce9f272..02bf9c679e 100644
--- a/xen/arch/x86/iommu_op.c
+++ b/xen/arch/x86/iommu_op.c
@@ -114,6 +114,123 @@ static int iommu_op_enable_modification(void)
return 0;
}
+static int iommuop_map(struct xen_iommu_op_map *op)
+{
+ struct domain *d, *currd = current->domain;
+ struct domain_iommu *iommu = dom_iommu(currd);
+ bool readonly = op->flags & XEN_IOMMUOP_map_readonly;
+ bfn_t bfn = _bfn(op->bfn);
+ struct page_info *page;
+ unsigned int prot;
+ int rc, ignore;
+
+ if ( op->pad || (op->flags & ~XEN_IOMMUOP_map_readonly) )
+ return -EINVAL;
+
+ if ( !iommu->iommu_op_ranges )
+ return -EOPNOTSUPP;
+
+ /* Check whether the specified BFN falls in a reserved region */
+ if ( rangeset_contains_singleton(iommu->reserved_ranges, bfn_x(bfn)) )
+ return -EINVAL;
+
+ d = rcu_lock_domain_by_any_id(op->domid);
+ if ( !d )
+ return -ESRCH;
+
+ rc = get_paged_gfn(d, _gfn(op->gfn), readonly, NULL, &page);
+ if ( rc )
+ goto unlock;
+
+ rc = -EINVAL;
+ if ( !get_page_type(page, readonly ? PGT_none : PGT_writable_page) )
+ {
+ put_page(page);
+ goto unlock;
+ }
+
+ prot = IOMMUF_readable;
+ if ( !readonly )
+ prot |= IOMMUF_writable;
+
+ rc = -EIO;
+ if ( iommu_map_page(currd, bfn, page_to_mfn(page), prot) )
+ goto release;
+
+ rc = rangeset_add_singleton(iommu->iommu_op_ranges, bfn_x(bfn));
+ if ( rc )
+ goto unmap;
+
+ rc = 0;
+ goto unlock; /* retain mapping and references */
+
+ unmap:
+ ignore = iommu_unmap_page(currd, bfn);
+
+ release:
+ put_page_and_type(page);
+
+ unlock:
+ rcu_unlock_domain(d);
+ return rc;
+}
+
+static int iommuop_unmap(struct xen_iommu_op_unmap *op)
+{
+ struct domain *d, *currd = current->domain;
+ struct domain_iommu *iommu = dom_iommu(currd);
+ bfn_t bfn = _bfn(op->bfn);
+ mfn_t mfn;
+ unsigned int prot;
+ struct page_info *page;
+ int rc;
+
+ if ( op->pad0 || op->pad1 )
+ return -EINVAL;
+
+ if ( !iommu->iommu_op_ranges )
+ return -EOPNOTSUPP;
+
+ if ( !rangeset_contains_singleton(iommu->iommu_op_ranges, bfn_x(bfn)) ||
+ iommu_lookup_page(currd, bfn, &mfn, &prot) ||
+ !mfn_valid(mfn) )
+ return -ENOENT;
+
+ d = rcu_lock_domain_by_any_id(op->domid);
+ if ( !d )
+ return -ESRCH;
+
+ rc = get_paged_gfn(d, _gfn(op->gfn), !(prot & IOMMUF_writable), NULL,
+ &page);
+ if ( rc )
+ goto unlock;
+
+ put_page(page); /* release extra reference just taken */
+
+ rc = -EINVAL;
+ if ( !mfn_eq(page_to_mfn(page), mfn) )
+ goto unlock;
+
+ put_page_and_type(page); /* release references taken in map */
+
+ rc = rangeset_remove_singleton(iommu->iommu_op_ranges, bfn_x(bfn));
+ if ( rc )
+ goto unlock;
+
+ if ( !iommu_unmap_page(currd, bfn) )
+ rc = -EIO;
+
+ unlock:
+ rcu_unlock_domain(d);
+
+ return rc;
+}
+
+static int iommuop_flush(void)
+{
+ return !iommu_iotlb_flush_all(current->domain) ? 0 : -EIO;
+}
+
static void iommu_op(xen_iommu_op_t *op)
{
switch ( op->op )
@@ -126,6 +243,22 @@ static void iommu_op(xen_iommu_op_t *op)
op->status = iommu_op_enable_modification();
break;
+ case XEN_IOMMUOP_map:
+ this_cpu(iommu_dont_flush_iotlb) = 1;
+ op->status = iommuop_map(&op->u.map);
+ this_cpu(iommu_dont_flush_iotlb) = 0;
+ break;
+
+ case XEN_IOMMUOP_unmap:
+ this_cpu(iommu_dont_flush_iotlb) = 1;
+ op->status = iommuop_unmap(&op->u.unmap);
+ this_cpu(iommu_dont_flush_iotlb) = 0;
+ break;
+
+ case XEN_IOMMUOP_flush:
+ op->status = iommuop_flush();
+ break;
+
default:
op->status = -EOPNOTSUPP;
break;
@@ -139,6 +272,9 @@ int do_one_iommu_op(xen_iommu_op_buf_t *buf)
static const size_t op_size[] = {
[XEN_IOMMUOP_query_reserved] = sizeof(struct xen_iommu_op_query_reserved),
[XEN_IOMMUOP_enable_modification] = 0,
+ [XEN_IOMMUOP_map] = sizeof(struct xen_iommu_op_map),
+ [XEN_IOMMUOP_unmap] = sizeof(struct xen_iommu_op_unmap),
+ [XEN_IOMMUOP_flush] = 0,
};
size_t size;
int rc;
@@ -224,6 +360,9 @@ int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
static const size_t op_size[] = {
[XEN_IOMMUOP_query_reserved] = sizeof(struct compat_iommu_op_query_reserved),
[XEN_IOMMUOP_enable_modification] = 0,
+ [XEN_IOMMUOP_map] = sizeof(struct compat_iommu_op_map),
+ [XEN_IOMMUOP_unmap] = sizeof(struct compat_iommu_op_unmap),
+ [XEN_IOMMUOP_flush] = 0,
};
size_t size;
xen_iommu_op_t nat;
@@ -261,6 +400,8 @@ int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
* we need to fix things up here.
*/
#define XLAT_iommu_op_u_query_reserved XEN_IOMMUOP_query_reserved
+#define XLAT_iommu_op_u_map XEN_IOMMUOP_map
+#define XLAT_iommu_op_u_unmap XEN_IOMMUOP_unmap
u = cmp.op;
#define XLAT_iommu_op_query_reserved_HNDL_ranges(_d_, _s_) \
diff --git a/xen/include/public/iommu_op.h b/xen/include/public/iommu_op.h
index 5a3148c247..737e2c8cfe 100644
--- a/xen/include/public/iommu_op.h
+++ b/xen/include/public/iommu_op.h
@@ -67,6 +67,47 @@ struct xen_iommu_op_query_reserved {
*/
#define XEN_IOMMUOP_enable_modification 2
+/*
+ * XEN_IOMMUOP_map: Map a guest page in the IOMMU.
+ */
+#define XEN_IOMMUOP_map 3
+
+struct xen_iommu_op_map {
+ /* IN - The domid of the guest */
+ domid_t domid;
+ uint16_t flags;
+
+#define _XEN_IOMMUOP_map_readonly 0
+#define XEN_IOMMUOP_map_readonly (1 << (_XEN_IOMMUOP_map_readonly))
+
+ uint32_t pad;
+ /* IN - The IOMMU frame number which will hold the new mapping */
+ xen_bfn_t bfn;
+ /* IN - The guest frame number of the page to be mapped */
+ xen_pfn_t gfn;
+};
+
+/*
+ * XEN_IOMMUOP_unmap_gfn: Remove a mapping in the IOMMU.
+ */
+#define XEN_IOMMUOP_unmap 4
+
+struct xen_iommu_op_unmap {
+ /* IN - The domid of the guest */
+ domid_t domid;
+ uint16_t pad0;
+ uint32_t pad1;
+ /* IN - The IOMMU frame number which holds the mapping to be removed */
+ xen_bfn_t bfn;
+ /* IN - The guest frame number of the page that is mapped */
+ xen_pfn_t gfn;
+};
+
+/*
+ * XEN_IOMMUOP_flush: Flush the IOMMU TLB.
+ */
+#define XEN_IOMMUOP_flush 5
+
struct xen_iommu_op {
uint16_t op; /* op type */
uint16_t pad;
@@ -74,6 +115,8 @@ struct xen_iommu_op {
/* 0 for success otherwise, negative errno */
union {
struct xen_iommu_op_query_reserved query_reserved;
+ struct xen_iommu_op_map map;
+ struct xen_iommu_op_unmap unmap;
} u;
};
typedef struct xen_iommu_op xen_iommu_op_t;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 93bcf4b4d0..ed50216394 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -79,7 +79,9 @@
? vcpu_hvm_x86_64 hvm/hvm_vcpu.h
! iommu_op iommu_op.h
! iommu_op_buf iommu_op.h
+! iommu_op_map iommu_op.h
! iommu_op_query_reserved iommu_op.h
+! iommu_op_unmap iommu_op.h
! iommu_reserved_range iommu_op.h
? kexec_exec kexec.h
! kexec_image kexec.h
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 13/13] x86: extend the map and unmap iommu_ops to support grant references
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
` (11 preceding siblings ...)
2018-07-17 13:38 ` [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings Paul Durrant
@ 2018-07-17 13:38 ` Paul Durrant
12 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-17 13:38 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Paul Durrant, Jan Beulich
This patch allows a domain to add or remove foreign frames from its
IOMMU mappings by grant reference as well as GFN. This is necessary,
for example, to support a PV network backend that needs to construct a
packet buffer that can be directly accessed by a NIC.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.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: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
v2:
- New in v2.
---
xen/arch/x86/iommu_op.c | 76 +++++++++++++++-------
xen/common/grant_table.c | 143 ++++++++++++++++++++++++++++++++++++++++++
xen/include/public/iommu_op.h | 31 +++++++--
xen/include/xen/grant_table.h | 7 +++
4 files changed, 228 insertions(+), 29 deletions(-)
diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
index 02bf9c679e..b3de7bce25 100644
--- a/xen/arch/x86/iommu_op.c
+++ b/xen/arch/x86/iommu_op.c
@@ -23,6 +23,7 @@
#include <xen/guest_access.h>
#include <xen/hypercall.h>
#include <xen/nospec.h>
+#include <xen/grant_table.h>
struct get_reserved_ctxt {
unsigned int max_entries;
@@ -121,10 +122,12 @@ static int iommuop_map(struct xen_iommu_op_map *op)
bool readonly = op->flags & XEN_IOMMUOP_map_readonly;
bfn_t bfn = _bfn(op->bfn);
struct page_info *page;
+ mfn_t mfn;
unsigned int prot;
int rc, ignore;
- if ( op->pad || (op->flags & ~XEN_IOMMUOP_map_readonly) )
+ if ( op->pad || (op->flags & ~(XEN_IOMMUOP_map_readonly |
+ XEN_IOMMUOP_map_gref)) )
return -EINVAL;
if ( !iommu->iommu_op_ranges )
@@ -138,15 +141,28 @@ static int iommuop_map(struct xen_iommu_op_map *op)
if ( !d )
return -ESRCH;
- rc = get_paged_gfn(d, _gfn(op->gfn), readonly, NULL, &page);
- if ( rc )
- goto unlock;
+ if ( op->flags & XEN_IOMMUOP_map_gref )
+ {
+ rc = acquire_gref_for_iommu(d, op->u.gref, readonly, &mfn);
+ if ( rc )
+ goto unlock;
- rc = -EINVAL;
- if ( !get_page_type(page, readonly ? PGT_none : PGT_writable_page) )
+ page = mfn_to_page(mfn);
+ }
+ else
{
- put_page(page);
- goto unlock;
+ rc = get_paged_gfn(d, _gfn(op->u.gfn), readonly, NULL, &page);
+ if ( rc )
+ goto unlock;
+
+ rc = -EINVAL;
+ if ( !get_page_type(page, readonly ? PGT_none : PGT_writable_page) )
+ {
+ put_page(page);
+ goto unlock;
+ }
+
+ mfn = page_to_mfn(page);
}
prot = IOMMUF_readable;
@@ -154,7 +170,7 @@ static int iommuop_map(struct xen_iommu_op_map *op)
prot |= IOMMUF_writable;
rc = -EIO;
- if ( iommu_map_page(currd, bfn, page_to_mfn(page), prot) )
+ if ( iommu_map_page(currd, bfn, mfn, prot) )
goto release;
rc = rangeset_add_singleton(iommu->iommu_op_ranges, bfn_x(bfn));
@@ -168,7 +184,10 @@ static int iommuop_map(struct xen_iommu_op_map *op)
ignore = iommu_unmap_page(currd, bfn);
release:
- put_page_and_type(page);
+ if ( op->flags & XEN_IOMMUOP_map_gref )
+ release_gref_for_iommu(d, op->u.gref, readonly, mfn);
+ else
+ put_page_and_type(page);
unlock:
rcu_unlock_domain(d);
@@ -182,11 +201,10 @@ static int iommuop_unmap(struct xen_iommu_op_unmap *op)
bfn_t bfn = _bfn(op->bfn);
mfn_t mfn;
unsigned int prot;
- struct page_info *page;
+ bool readonly;
int rc;
- if ( op->pad0 || op->pad1 )
- return -EINVAL;
+ if ( op->pad || (op->flags & ~XEN_IOMMUOP_unmap_gref) )
if ( !iommu->iommu_op_ranges )
return -EOPNOTSUPP;
@@ -196,28 +214,40 @@ static int iommuop_unmap(struct xen_iommu_op_unmap *op)
!mfn_valid(mfn) )
return -ENOENT;
+ readonly = !(prot & IOMMUF_writable);
+
d = rcu_lock_domain_by_any_id(op->domid);
if ( !d )
return -ESRCH;
- rc = get_paged_gfn(d, _gfn(op->gfn), !(prot & IOMMUF_writable), NULL,
- &page);
- if ( rc )
- goto unlock;
+ if ( op->flags & XEN_IOMMUOP_unmap_gref )
+ {
+ rc = release_gref_for_iommu(d, op->u.gref, readonly, mfn);
+ if ( rc )
+ goto unlock;
+ }
+ else
+ {
+ struct page_info *page;
+
+ rc = get_paged_gfn(d, _gfn(op->u.gfn), readonly, NULL, &page);
+ if ( rc )
+ goto unlock;
- put_page(page); /* release extra reference just taken */
+ put_page(page); /* release extra reference just taken */
- rc = -EINVAL;
- if ( !mfn_eq(page_to_mfn(page), mfn) )
- goto unlock;
+ rc = -EINVAL;
+ if ( !mfn_eq(page_to_mfn(page), mfn) )
+ goto unlock;
- put_page_and_type(page); /* release references taken in map */
+ put_page_and_type(page); /* release references taken in map */
+ }
rc = rangeset_remove_singleton(iommu->iommu_op_ranges, bfn_x(bfn));
if ( rc )
goto unlock;
- if ( !iommu_unmap_page(currd, bfn) )
+ if ( iommu_unmap_page(currd, bfn) )
rc = -EIO;
unlock:
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index a492df2362..6d5bcca6df 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3909,6 +3909,149 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
return rc;
}
+int
+acquire_gref_for_iommu(struct domain *d, grant_ref_t gref,
+ bool readonly, mfn_t *mfn)
+{
+ struct domain *currd = current->domain;
+ struct grant_table *gt = d->grant_table;
+ grant_entry_header_t *shah;
+ struct active_grant_entry *act;
+ uint16_t *status;
+ int rc;
+
+ grant_read_lock(gt);
+
+ rc = -ENOENT;
+ if ( gref > nr_grant_entries(gt) )
+ goto unlock;
+
+ act = active_entry_acquire(gt, gref);
+ shah = shared_entry_header(gt, gref);
+ status = ( gt->gt_version == 2 ) ?
+ &status_entry(gt, gref) :
+ &shah->flags;
+
+ rc = -EACCES;
+ if ( (shah->flags & GTF_type_mask) != GTF_permit_access ||
+ (shah->flags & GTF_sub_page) )
+ goto release;
+
+ rc = -ERANGE;
+ if ( act->pin && ((act->domid != currd->domain_id) ||
+ (act->pin & 0x80808080U) != 0) )
+ goto release;
+
+ rc = -EINVAL;
+ if ( !act->pin ||
+ (!readonly && !(act->pin & GNTPIN_devw_mask)) ) {
+ if ( _set_status(gt->gt_version, currd->domain_id, readonly,
+ 0, shah, act, status) != GNTST_okay )
+ goto release;
+ }
+
+ if ( !act->pin )
+ {
+ gfn_t gfn = gt->gt_version == 1 ?
+ _gfn(shared_entry_v1(gt, gref).frame) :
+ _gfn(shared_entry_v2(gt, gref).full_page.frame);
+ struct page_info *page;
+
+ rc = get_paged_gfn(d, gfn, readonly, NULL, &page);
+ if ( rc )
+ goto clear;
+
+ act_set_gfn(act, gfn);
+ act->mfn = page_to_mfn(page);
+ act->domid = currd->domain_id;
+ act->start = 0;
+ act->length = PAGE_SIZE;
+ act->is_sub_page = false;
+ act->trans_domain = d;
+ act->trans_gref = gref;
+ }
+ else
+ {
+ ASSERT(mfn_valid(act->mfn));
+ if ( !get_page(mfn_to_page(act->mfn), d) )
+ goto clear;
+ }
+
+ rc = 0;
+ act->pin += readonly ? GNTPIN_devr_inc : GNTPIN_devw_inc;
+ *mfn = act->mfn;
+ goto release;
+
+ clear:
+ if ( !readonly && !(act->pin & GNTPIN_devw_mask) )
+ gnttab_clear_flag(_GTF_writing, status);
+
+ if ( !act->pin )
+ gnttab_clear_flag(_GTF_reading, status);
+
+ release:
+ active_entry_release(act);
+
+ unlock:
+ grant_read_unlock(gt);
+
+ return rc;
+}
+
+int
+release_gref_for_iommu(struct domain *d, grant_ref_t gref,
+ bool readonly, mfn_t mfn)
+{
+ struct domain *currd = current->domain;
+ struct grant_table *gt = d->grant_table;
+ grant_entry_header_t *shah;
+ struct active_grant_entry *act;
+ uint16_t *status;
+ int rc;
+
+ grant_read_lock(gt);
+
+ rc = -ENOENT;
+ if ( gref > nr_grant_entries(gt) )
+ goto unlock;
+
+ act = active_entry_acquire(gt, gref);
+ shah = shared_entry_header(gt, gref);
+ status = ( gt->gt_version == 2 ) ?
+ &status_entry(gt, gref) :
+ &shah->flags;
+
+ rc = -EINVAL;
+ if ( !act->pin || (act->domid != currd->domain_id) ||
+ !mfn_eq(act->mfn, mfn) )
+ goto release;
+
+ rc = 0;
+ if ( readonly )
+ act->pin -= GNTPIN_devr_inc;
+ else
+ {
+ gnttab_mark_dirty(d, mfn);
+
+ act->pin -= GNTPIN_devw_inc;
+ if ( !(act->pin & GNTPIN_devw_mask) )
+ gnttab_clear_flag(_GTF_writing, status);
+ }
+
+ if ( !act->pin )
+ gnttab_clear_flag(_GTF_reading, status);
+
+ put_page(mfn_to_page(mfn));
+
+ release:
+ active_entry_release(act);
+
+ unlock:
+ grant_read_unlock(gt);
+
+ return rc;
+}
+
static void gnttab_usage_print(struct domain *rd)
{
int first = 1;
diff --git a/xen/include/public/iommu_op.h b/xen/include/public/iommu_op.h
index 737e2c8cfe..485782a522 100644
--- a/xen/include/public/iommu_op.h
+++ b/xen/include/public/iommu_op.h
@@ -24,6 +24,7 @@
#define XEN_PUBLIC_IOMMU_OP_H
#include "xen.h"
+#include "grant_table.h"
typedef unsigned long xen_bfn_t;
@@ -79,12 +80,20 @@ struct xen_iommu_op_map {
#define _XEN_IOMMUOP_map_readonly 0
#define XEN_IOMMUOP_map_readonly (1 << (_XEN_IOMMUOP_map_readonly))
+#define _XEN_IOMMUOP_map_gref 1
+#define XEN_IOMMUOP_map_gref (1 << (_XEN_IOMMUOP_map_gref))
uint32_t pad;
/* IN - The IOMMU frame number which will hold the new mapping */
xen_bfn_t bfn;
- /* IN - The guest frame number of the page to be mapped */
- xen_pfn_t gfn;
+ /*
+ * IN - The guest frame number or grant reference of the page to
+ * be mapped.
+ */
+ union {
+ xen_pfn_t gfn;
+ grant_ref_t gref;
+ } u;
};
/*
@@ -95,12 +104,22 @@ struct xen_iommu_op_map {
struct xen_iommu_op_unmap {
/* IN - The domid of the guest */
domid_t domid;
- uint16_t pad0;
- uint32_t pad1;
+ uint16_t flags;
+
+#define _XEN_IOMMUOP_unmap_gref 0
+#define XEN_IOMMUOP_unmap_gref (1 << (_XEN_IOMMUOP_unmap_gref))
+
+ uint32_t pad;
/* IN - The IOMMU frame number which holds the mapping to be removed */
xen_bfn_t bfn;
- /* IN - The guest frame number of the page that is mapped */
- xen_pfn_t gfn;
+ /*
+ * IN - The guest frame number or grant reference of the page that
+ * is mapped.
+ */
+ union {
+ xen_pfn_t gfn;
+ grant_ref_t gref;
+ } u;
};
/*
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index b3a95fda58..22c80c2238 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -56,6 +56,13 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
mfn_t *mfn);
+int
+acquire_gref_for_iommu(struct domain *d, grant_ref_t gref,
+ bool readonly, mfn_t *mfn);
+int
+release_gref_for_iommu(struct domain *d, grant_ref_t gref,
+ bool readonly, mfn_t mfn);
+
unsigned int gnttab_dom0_frames(void);
#endif /* __XEN_GRANT_TABLE_H__ */
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3 03/13] iommu: make use of type-safe BFN and MFN in exported functions
2018-07-17 13:38 ` [PATCH v3 03/13] iommu: make use of type-safe BFN and MFN in exported functions Paul Durrant
@ 2018-07-19 8:08 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-19 8:08 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Jun Nakajima,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:06PM +0100, Paul Durrant wrote:
> This patch modifies the declaration of the entry points to the IOMMU
> sub-system to use bfn_t and mfn_t in place of unsigned long. A subsequent
> patch will similarly modify the methods in the iommu_ops structure.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 02/13] iommu: introduce the concept of BFN...
2018-07-17 13:38 ` [PATCH v3 02/13] iommu: introduce the concept of BFN Paul Durrant
@ 2018-07-19 8:08 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-19 8:08 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Suravee Suthikulpanit,
Julien Grall, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:05PM +0100, Paul Durrant wrote:
> ...meaning 'bus frame number' i.e. a frame number mapped in the IOMMU
> rather than the MMU.
>
> This patch is a largely cosmetic change that substitutes the terms 'gfn'
> and 'gaddr' for 'bfn' and 'baddr' in all the places where the frame number
> or address relate to the IOMMU rather than the MMU.
>
> The parts that are not purely cosmetic are:
>
> - the introduction of a type-safe declaration of bfn_t and definition of
> INVALID_BFN to make the substitution of gfn_x(INVALID_GFN) mechanical.
> - the introduction of __bfn_to_baddr and __baddr_to_bfn (and type-safe
> variants without the leading __) with some use of the former.
>
> Subsequent patches will convert code to make use of type-safe BFNs.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 04/13] iommu: push use of type-safe BFN and MFN into iommu_ops
2018-07-17 13:38 ` [PATCH v3 04/13] iommu: push use of type-safe BFN and MFN into iommu_ops Paul Durrant
@ 2018-07-19 8:10 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-19 8:10 UTC (permalink / raw)
To: Paul Durrant
Cc: Kevin Tian, Wei Liu, Suravee Suthikulpanit, Andrew Cooper,
George Dunlap, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:07PM +0100, Paul Durrant wrote:
> This patch modifies the methods in struct iommu_ops to use type-safe BFN
> and MFN. This follows on from the prior patch that modified the functions
> exported in xen/iommu.h.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 07/13] iommu: track reserved ranges using a rangeset
2018-07-17 13:38 ` [PATCH v3 07/13] iommu: track reserved ranges using a rangeset Paul Durrant
@ 2018-07-19 9:01 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-19 9:01 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel, Kevin Tian, Wei Liu, Jan Beulich
On Tue, Jul 17, 2018 at 02:38:10PM +0100, Paul Durrant wrote:
> Ranges that should be considered reserved in the IOMMU are not necessarily
> limited to RMRRs. If iommu_inclusive_mapping is set then any frame number
> falling within an E820 reserved region should also be considered as
> reserved in the IOMMU.
> This patch adds a rangeset to the domain_iommu structure that is then used
> to track all reserved ranges. This will be needed by a subsequent patch
> to test whether it is safe to modify a particular IOMMU entry.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 08/13] x86: add iommu_op to query reserved ranges
2018-07-17 13:38 ` [PATCH v3 08/13] x86: add iommu_op to query reserved ranges Paul Durrant
@ 2018-07-19 9:37 ` Wei Liu
2018-07-19 10:03 ` Paul Durrant
0 siblings, 1 reply; 30+ messages in thread
From: Wei Liu @ 2018-07-19 9:37 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:11PM +0100, Paul Durrant wrote:
[...]
> int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> {
> - compat_iommu_op_t cmp;
> + compat_iommu_op_t cmp = {};
> + size_t offset;
> + static const size_t op_size[] = {
> + [XEN_IOMMUOP_query_reserved] = sizeof(struct compat_iommu_op_query_reserved),
> + };
> + size_t size;
> xen_iommu_op_t nat;
> + unsigned int u;
> + int32_t status;
> int rc;
>
> - if ( buf->size < sizeof(cmp) )
> + offset = offsetof(struct compat_iommu_op, u);
> +
> + if ( buf->size < offset )
> return -EFAULT;
>
> - if ( copy_from_compat((void *)&cmp, buf->h, sizeof(cmp)) )
> + if ( copy_from_compat((void *)&cmp, buf->h, offset) )
> return -EFAULT;
>
> if ( cmp.pad )
> @@ -119,12 +204,85 @@ int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> if ( rc )
> return rc;
>
> + if ( cmp.op >= ARRAY_SIZE(op_size) )
> + return -EOPNOTSUPP;
> +
> + size = op_size[array_index_nospec(cmp.op, ARRAY_SIZE(op_size))];
> + if ( buf->size < offset + size )
> + return -EFAULT;
> +
> + if ( copy_from_compat_offset((void *)&cmp.u, buf->h, offset, size) )
> + return -EFAULT;
> +
> + /*
> + * The xlat magic doesn't quite know how to handle the union so
> + * we need to fix things up here.
> + */
> +#define XLAT_iommu_op_u_query_reserved XEN_IOMMUOP_query_reserved
Missing undef for this.
[...]
> diff --git a/xen/include/public/iommu_op.h b/xen/include/public/iommu_op.h
> index c3b68f665a..02213c12a4 100644
> --- a/xen/include/public/iommu_op.h
> +++ b/xen/include/public/iommu_op.h
> @@ -25,11 +25,50 @@
>
> #include "xen.h"
>
> +typedef unsigned long xen_bfn_t;
> +
This means xen_bfn_t will have different size on 32 bit and 64 bit
platform.
And obviously other unsigned ints in this commit and xen_pfn_t in later
patches will indeed make compat entry point necessary.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 09/13] vtd: add lookup_page method to iommu_ops
2018-07-17 13:38 ` [PATCH v3 09/13] vtd: add lookup_page method to iommu_ops Paul Durrant
@ 2018-07-19 9:55 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-19 9:55 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel, Kevin Tian, Wei Liu, George Dunlap, Jan Beulich
On Tue, Jul 17, 2018 at 02:38:12PM +0100, Paul Durrant wrote:
> This patch adds a new method to the VT-d IOMMU implementation to find the
> MFN currently mapped by the specified BFN along with a wrapper function in
> generic IOMMU code to call the implementation if it exists.
>
> This functionality will be used by a subsequent patch.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 08/13] x86: add iommu_op to query reserved ranges
2018-07-19 9:37 ` Wei Liu
@ 2018-07-19 10:03 ` Paul Durrant
0 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-19 10:03 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Jan Beulich, Ian Jackson, xen-devel
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 19 July 2018 10:37
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Wei Liu
> <wei.liu2@citrix.com>
> Subject: Re: [PATCH v3 08/13] x86: add iommu_op to query reserved ranges
>
> On Tue, Jul 17, 2018 at 02:38:11PM +0100, Paul Durrant wrote:
> [...]
> > int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> > {
> > - compat_iommu_op_t cmp;
> > + compat_iommu_op_t cmp = {};
> > + size_t offset;
> > + static const size_t op_size[] = {
> > + [XEN_IOMMUOP_query_reserved] = sizeof(struct
> compat_iommu_op_query_reserved),
> > + };
> > + size_t size;
> > xen_iommu_op_t nat;
> > + unsigned int u;
> > + int32_t status;
> > int rc;
> >
> > - if ( buf->size < sizeof(cmp) )
> > + offset = offsetof(struct compat_iommu_op, u);
> > +
> > + if ( buf->size < offset )
> > return -EFAULT;
> >
> > - if ( copy_from_compat((void *)&cmp, buf->h, sizeof(cmp)) )
> > + if ( copy_from_compat((void *)&cmp, buf->h, offset) )
> > return -EFAULT;
> >
> > if ( cmp.pad )
> > @@ -119,12 +204,85 @@ int
> compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> > if ( rc )
> > return rc;
> >
> > + if ( cmp.op >= ARRAY_SIZE(op_size) )
> > + return -EOPNOTSUPP;
> > +
> > + size = op_size[array_index_nospec(cmp.op, ARRAY_SIZE(op_size))];
> > + if ( buf->size < offset + size )
> > + return -EFAULT;
> > +
> > + if ( copy_from_compat_offset((void *)&cmp.u, buf->h, offset, size) )
> > + return -EFAULT;
> > +
> > + /*
> > + * The xlat magic doesn't quite know how to handle the union so
> > + * we need to fix things up here.
> > + */
> > +#define XLAT_iommu_op_u_query_reserved
> XEN_IOMMUOP_query_reserved
>
> Missing undef for this.
>
Yeah, not really needed but best to be tidy.
> [...]
> > diff --git a/xen/include/public/iommu_op.h
> b/xen/include/public/iommu_op.h
> > index c3b68f665a..02213c12a4 100644
> > --- a/xen/include/public/iommu_op.h
> > +++ b/xen/include/public/iommu_op.h
> > @@ -25,11 +25,50 @@
> >
> > #include "xen.h"
> >
> > +typedef unsigned long xen_bfn_t;
> > +
>
> This means xen_bfn_t will have different size on 32 bit and 64 bit
> platform.
>
Good point. I guess it doesn't need to be related to the OS native width, so uint64_t is probably better.
Paul
> And obviously other unsigned ints in this commit and xen_pfn_t in later
> patches will indeed make compat entry point necessary.
>
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings
2018-07-17 13:38 ` [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings Paul Durrant
@ 2018-07-20 8:59 ` Wei Liu
2018-07-20 9:05 ` Paul Durrant
0 siblings, 1 reply; 30+ messages in thread
From: Wei Liu @ 2018-07-20 8:59 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:13PM +0100, Paul Durrant wrote:
> +static int iommu_op_enable_modification(void)
> +{
> + struct domain *currd = current->domain;
> + struct domain_iommu *iommu = dom_iommu(currd);
> + const struct iommu_ops *ops = iommu->platform_ops;
> +
> + /* Has modification already been enabled? */
> + if ( iommu->iommu_op_ranges )
> + return 0;
> +
> + /*
> + * The IOMMU mappings cannot be modified if:
> + * - the IOMMU is not enabled or,
> + * - the current domain is dom0 and tranlsation is disabled or,
> + * - HAP is enabled and the IOMMU shares the mappings.
> + */
> + if ( !iommu_enabled ||
> + (is_hardware_domain(currd) && iommu_passthrough) ||
> + iommu_use_hap_pt(currd) )
> + return -EACCES;
> +
> + /*
> + * The IOMMU implementation must provide the lookup method if
> + * modification of the mappings is to be supported.
> + */
> + if ( !ops->lookup_page )
> + return -EOPNOTSUPP;
> +
> + iommu->iommu_op_ranges = rangeset_new(currd, NULL, 0);
> + if ( !iommu->iommu_op_ranges )
> + return -ENOMEM;
> +
> + currd->need_iommu = 0; /* Disable identity GFN mapping */
I think this flag currently serves two purposes: one is to indicate
whether there are resources allocated within xen, the other it indicate
whether it is in use (as suggested in the comment).
Suppose a domain gets all its devices deassigned after this call,
original iommu_teardown would have been called, but now it won't. The
iommu will still be torn down when it is destroyed, but this is a very
subtle change in behaviour.
IMHO we should clearly separate these two cases if possible.
> + return 0;
> +}
> +
[...]
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index fb9d0e1848..c517428621 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -26,7 +26,6 @@ static void iommu_dump_p2m_table(unsigned char key);
>
> unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
> integer_param("iommu_dev_iotlb_timeout", iommu_dev_iotlb_timeout);
> -
Please keep the blank line here.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings
2018-07-20 8:59 ` Wei Liu
@ 2018-07-20 9:05 ` Paul Durrant
2018-07-20 9:19 ` Paul Durrant
0 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-20 9:05 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, xen-devel
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 20 July 2018 09:59
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Julien
> Grall <julien.grall@arm.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH v3 10/13] x86: add iommu_op to enable modification of
> IOMMU mappings
>
> On Tue, Jul 17, 2018 at 02:38:13PM +0100, Paul Durrant wrote:
> > +static int iommu_op_enable_modification(void)
> > +{
> > + struct domain *currd = current->domain;
> > + struct domain_iommu *iommu = dom_iommu(currd);
> > + const struct iommu_ops *ops = iommu->platform_ops;
> > +
> > + /* Has modification already been enabled? */
> > + if ( iommu->iommu_op_ranges )
> > + return 0;
> > +
> > + /*
> > + * The IOMMU mappings cannot be modified if:
> > + * - the IOMMU is not enabled or,
> > + * - the current domain is dom0 and tranlsation is disabled or,
> > + * - HAP is enabled and the IOMMU shares the mappings.
> > + */
> > + if ( !iommu_enabled ||
> > + (is_hardware_domain(currd) && iommu_passthrough) ||
> > + iommu_use_hap_pt(currd) )
> > + return -EACCES;
> > +
> > + /*
> > + * The IOMMU implementation must provide the lookup method if
> > + * modification of the mappings is to be supported.
> > + */
> > + if ( !ops->lookup_page )
> > + return -EOPNOTSUPP;
> > +
> > + iommu->iommu_op_ranges = rangeset_new(currd, NULL, 0);
> > + if ( !iommu->iommu_op_ranges )
> > + return -ENOMEM;
> > +
> > + currd->need_iommu = 0; /* Disable identity GFN mapping */
>
> I think this flag currently serves two purposes: one is to indicate
> whether there are resources allocated within xen, the other it indicate
> whether it is in use (as suggested in the comment).
>
> Suppose a domain gets all its devices deassigned after this call,
> original iommu_teardown would have been called, but now it won't. The
> iommu will still be torn down when it is destroyed, but this is a very
> subtle change in behaviour.
That is pretty subtle.
>
> IMHO we should clearly separate these two cases if possible.
>
Agreed. I never liked the 'need_iommu' name anyway. I'll add a patch before this one to do the rename/split.
Paul
> > + return 0;
> > +}
> > +
> [...]
> > diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> > index fb9d0e1848..c517428621 100644
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -26,7 +26,6 @@ static void iommu_dump_p2m_table(unsigned char
> key);
> >
> > unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
> > integer_param("iommu_dev_iotlb_timeout", iommu_dev_iotlb_timeout);
> > -
>
> Please keep the blank line here.
>
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings
2018-07-20 9:05 ` Paul Durrant
@ 2018-07-20 9:19 ` Paul Durrant
2018-07-20 9:30 ` Wei Liu
0 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-20 9:19 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson
> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 20 July 2018 10:05
> To: Wei Liu <wei.liu2@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wei.liu2@citrix.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; George Dunlap <George.Dunlap@citrix.com>;
> Julien Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>; Ian
> Jackson <Ian.Jackson@citrix.com>; xen-devel@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH v3 10/13] x86: add iommu_op to enable
> modification of IOMMU mappings
>
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: 20 July 2018 09:59
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> > Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> > <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Julien
> > Grall <julien.grall@arm.com>; Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Tim
> > (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> > Subject: Re: [PATCH v3 10/13] x86: add iommu_op to enable modification
> of
> > IOMMU mappings
> >
> > On Tue, Jul 17, 2018 at 02:38:13PM +0100, Paul Durrant wrote:
> > > +static int iommu_op_enable_modification(void)
> > > +{
> > > + struct domain *currd = current->domain;
> > > + struct domain_iommu *iommu = dom_iommu(currd);
> > > + const struct iommu_ops *ops = iommu->platform_ops;
> > > +
> > > + /* Has modification already been enabled? */
> > > + if ( iommu->iommu_op_ranges )
> > > + return 0;
> > > +
> > > + /*
> > > + * The IOMMU mappings cannot be modified if:
> > > + * - the IOMMU is not enabled or,
> > > + * - the current domain is dom0 and tranlsation is disabled or,
> > > + * - HAP is enabled and the IOMMU shares the mappings.
> > > + */
> > > + if ( !iommu_enabled ||
> > > + (is_hardware_domain(currd) && iommu_passthrough) ||
> > > + iommu_use_hap_pt(currd) )
> > > + return -EACCES;
> > > +
> > > + /*
> > > + * The IOMMU implementation must provide the lookup method if
> > > + * modification of the mappings is to be supported.
> > > + */
> > > + if ( !ops->lookup_page )
> > > + return -EOPNOTSUPP;
> > > +
> > > + iommu->iommu_op_ranges = rangeset_new(currd, NULL, 0);
> > > + if ( !iommu->iommu_op_ranges )
> > > + return -ENOMEM;
> > > +
> > > + currd->need_iommu = 0; /* Disable identity GFN mapping */
> >
> > I think this flag currently serves two purposes: one is to indicate
> > whether there are resources allocated within xen, the other it indicate
> > whether it is in use (as suggested in the comment).
> >
> > Suppose a domain gets all its devices deassigned after this call,
> > original iommu_teardown would have been called, but now it won't. The
> > iommu will still be torn down when it is destroyed, but this is a very
> > subtle change in behaviour.
>
> That is pretty subtle.
Actually, thinking a bit more, that is the semantic I want. The domain is taking control of its own mappings. It doesn't want Xen to tear them down under its feet.
>
> >
> > IMHO we should clearly separate these two cases if possible.
> >
>
> Agreed. I never liked the 'need_iommu' name anyway. I'll add a patch before
> this one to do the rename/split.
>
I still think this renaming/clarification is worthwhile so I'll still add something into the series.
Paul
> Paul
>
> > > + return 0;
> > > +}
> > > +
> > [...]
> > > diff --git a/xen/drivers/passthrough/iommu.c
> > b/xen/drivers/passthrough/iommu.c
> > > index fb9d0e1848..c517428621 100644
> > > --- a/xen/drivers/passthrough/iommu.c
> > > +++ b/xen/drivers/passthrough/iommu.c
> > > @@ -26,7 +26,6 @@ static void iommu_dump_p2m_table(unsigned char
> > key);
> > >
> > > unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
> > > integer_param("iommu_dev_iotlb_timeout",
> iommu_dev_iotlb_timeout);
> > > -
> >
> > Please keep the blank line here.
> >
> > Wei.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings
2018-07-20 9:19 ` Paul Durrant
@ 2018-07-20 9:30 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-20 9:30 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, xen-devel, Ian Jackson
On Fri, Jul 20, 2018 at 10:19:44AM +0100, Paul Durrant wrote:
> > >
> > > I think this flag currently serves two purposes: one is to indicate
> > > whether there are resources allocated within xen, the other it indicate
> > > whether it is in use (as suggested in the comment).
> > >
> > > Suppose a domain gets all its devices deassigned after this call,
> > > original iommu_teardown would have been called, but now it won't. The
> > > iommu will still be torn down when it is destroyed, but this is a very
> > > subtle change in behaviour.
> >
> > That is pretty subtle.
>
> Actually, thinking a bit more, that is the semantic I want. The domain
> is taking control of its own mappings. It doesn't want Xen to tear
> them down under its feet.
Oh, this makes sense.
>
> >
> > >
> > > IMHO we should clearly separate these two cases if possible.
> > >
> >
> > Agreed. I never liked the 'need_iommu' name anyway. I'll add a patch before
> > this one to do the rename/split.
> >
>
> I still think this renaming/clarification is worthwhile so I'll still
> add something into the series.
>
Thanks.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings
2018-07-17 13:38 ` [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings Paul Durrant
@ 2018-07-23 13:35 ` Wei Liu
2018-07-23 13:40 ` Paul Durrant
2018-07-23 14:26 ` Wei Liu
1 sibling, 1 reply; 30+ messages in thread
From: Wei Liu @ 2018-07-23 13:35 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:15PM +0100, Paul Durrant wrote:
>
[...]
> +static int iommuop_map(struct xen_iommu_op_map *op)
> +{
> + struct domain *d, *currd = current->domain;
> + struct domain_iommu *iommu = dom_iommu(currd);
> + bool readonly = op->flags & XEN_IOMMUOP_map_readonly;
> + bfn_t bfn = _bfn(op->bfn);
> + struct page_info *page;
> + unsigned int prot;
> + int rc, ignore;
> +
> + if ( op->pad || (op->flags & ~XEN_IOMMUOP_map_readonly) )
> + return -EINVAL;
> +
> + if ( !iommu->iommu_op_ranges )
> + return -EOPNOTSUPP;
> +
> + /* Check whether the specified BFN falls in a reserved region */
> + if ( rangeset_contains_singleton(iommu->reserved_ranges, bfn_x(bfn)) )
> + return -EINVAL;
> +
> + d = rcu_lock_domain_by_any_id(op->domid);
I think this needs to be moved earlier before dereferencing assigning
iommu -- it depends on d being valid.
Same applies to the unmap function.
> +
> +static int iommuop_flush(void)
> +{
> + return !iommu_iotlb_flush_all(current->domain) ? 0 : -EIO;
I don't follow: why does this only flush current->domain? But
map/unmap use explicit argument to specify a domain?
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings
2018-07-23 13:35 ` Wei Liu
@ 2018-07-23 13:40 ` Paul Durrant
2018-07-23 13:49 ` Wei Liu
0 siblings, 1 reply; 30+ messages in thread
From: Paul Durrant @ 2018-07-23 13:40 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, xen-devel
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 23 July 2018 14:35
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Julien
> Grall <julien.grall@arm.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush
> IOMMU mappings
>
> On Tue, Jul 17, 2018 at 02:38:15PM +0100, Paul Durrant wrote:
> >
> [...]
> > +static int iommuop_map(struct xen_iommu_op_map *op)
> > +{
> > + struct domain *d, *currd = current->domain;
> > + struct domain_iommu *iommu = dom_iommu(currd);
> > + bool readonly = op->flags & XEN_IOMMUOP_map_readonly;
> > + bfn_t bfn = _bfn(op->bfn);
> > + struct page_info *page;
> > + unsigned int prot;
> > + int rc, ignore;
> > +
> > + if ( op->pad || (op->flags & ~XEN_IOMMUOP_map_readonly) )
> > + return -EINVAL;
> > +
> > + if ( !iommu->iommu_op_ranges )
> > + return -EOPNOTSUPP;
> > +
> > + /* Check whether the specified BFN falls in a reserved region */
> > + if ( rangeset_contains_singleton(iommu->reserved_ranges,
> bfn_x(bfn)) )
> > + return -EINVAL;
> > +
> > + d = rcu_lock_domain_by_any_id(op->domid);
>
> I think this needs to be moved earlier before dereferencing assigning
> iommu -- it depends on d being valid.
>
> Same applies to the unmap function.
>
Are you referring to the dom_iommu() macro? The code is uninterested in the IOMMU mappings of the target domain, only the current, so I can't see a problem here. Am I missing something?
> > +
> > +static int iommuop_flush(void)
> > +{
> > + return !iommu_iotlb_flush_all(current->domain) ? 0 : -EIO;
>
> I don't follow: why does this only flush current->domain? But
> map/unmap use explicit argument to specify a domain?
>
Because the page may come from another domain but the IOMMU mappings are only ever for the local domain.
Paul
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings
2018-07-23 13:40 ` Paul Durrant
@ 2018-07-23 13:49 ` Wei Liu
0 siblings, 0 replies; 30+ messages in thread
From: Wei Liu @ 2018-07-23 13:49 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, xen-devel
On Mon, Jul 23, 2018 at 02:40:05PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: 23 July 2018 14:35
> > To: Paul Durrant <Paul.Durrant@citrix.com>
> > Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> > Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> > <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Julien
> > Grall <julien.grall@arm.com>; Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>; Tim
> > (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> > Subject: Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush
> > IOMMU mappings
> >
> > On Tue, Jul 17, 2018 at 02:38:15PM +0100, Paul Durrant wrote:
> > >
> > [...]
> > > +static int iommuop_map(struct xen_iommu_op_map *op)
> > > +{
> > > + struct domain *d, *currd = current->domain;
> > > + struct domain_iommu *iommu = dom_iommu(currd);
> > > + bool readonly = op->flags & XEN_IOMMUOP_map_readonly;
> > > + bfn_t bfn = _bfn(op->bfn);
> > > + struct page_info *page;
> > > + unsigned int prot;
> > > + int rc, ignore;
> > > +
> > > + if ( op->pad || (op->flags & ~XEN_IOMMUOP_map_readonly) )
> > > + return -EINVAL;
> > > +
> > > + if ( !iommu->iommu_op_ranges )
> > > + return -EOPNOTSUPP;
> > > +
> > > + /* Check whether the specified BFN falls in a reserved region */
> > > + if ( rangeset_contains_singleton(iommu->reserved_ranges,
> > bfn_x(bfn)) )
> > > + return -EINVAL;
> > > +
> > > + d = rcu_lock_domain_by_any_id(op->domid);
> >
> > I think this needs to be moved earlier before dereferencing assigning
> > iommu -- it depends on d being valid.
> >
> > Same applies to the unmap function.
> >
>
> Are you referring to the dom_iommu() macro? The code is uninterested
> in the IOMMU mappings of the target domain, only the current, so I
> can't see a problem here. Am I missing something?
Oh, I misread. Sorry for the noise.
>
> > > +
> > > +static int iommuop_flush(void)
> > > +{
> > > + return !iommu_iotlb_flush_all(current->domain) ? 0 : -EIO;
> >
> > I don't follow: why does this only flush current->domain? But
> > map/unmap use explicit argument to specify a domain?
> >
>
> Because the page may come from another domain but the IOMMU mappings
> are only ever for the local domain.
I see. That's fine then.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings
2018-07-17 13:38 ` [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings Paul Durrant
2018-07-23 13:35 ` Wei Liu
@ 2018-07-23 14:26 ` Wei Liu
2018-07-23 14:27 ` Paul Durrant
1 sibling, 1 reply; 30+ messages in thread
From: Wei Liu @ 2018-07-23 14:26 UTC (permalink / raw)
To: Paul Durrant
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Julien Grall, Jan Beulich, xen-devel
On Tue, Jul 17, 2018 at 02:38:15PM +0100, Paul Durrant wrote:
[...]
> +static int iommuop_unmap(struct xen_iommu_op_unmap *op)
> +{
[...]
> +
> + if ( !iommu_unmap_page(currd, bfn) )
> + rc = -EIO;
So it is an error to unmap the page successfully? In fact this same line
is changed in the next patch. I suppose the code here is wrong.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings
2018-07-23 14:26 ` Wei Liu
@ 2018-07-23 14:27 ` Paul Durrant
0 siblings, 0 replies; 30+ messages in thread
From: Paul Durrant @ 2018-07-23 14:27 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Tim (Xen.org),
George Dunlap, Julien Grall, Jan Beulich, Ian Jackson, xen-devel
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 23 July 2018 15:26
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Julien
> Grall <julien.grall@arm.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH v3 12/13] x86: add iommu_ops to modify and flush
> IOMMU mappings
>
> On Tue, Jul 17, 2018 at 02:38:15PM +0100, Paul Durrant wrote:
> [...]
> > +static int iommuop_unmap(struct xen_iommu_op_unmap *op)
> > +{
> [...]
> > +
> > + if ( !iommu_unmap_page(currd, bfn) )
> > + rc = -EIO;
>
> So it is an error to unmap the page successfully? In fact this same line
> is changed in the next patch. I suppose the code here is wrong.
Yes, that looks wrong. I'll check.
Thanks,
Paul
>
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2018-07-23 14:27 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-17 13:38 [PATCH v3 00/13] paravirtual IOMMU interface Paul Durrant
2018-07-17 13:38 ` [PATCH v3 01/13] grant_table: use term 'mfn' for machine frame numbers Paul Durrant
2018-07-17 13:38 ` [PATCH v3 02/13] iommu: introduce the concept of BFN Paul Durrant
2018-07-19 8:08 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 03/13] iommu: make use of type-safe BFN and MFN in exported functions Paul Durrant
2018-07-19 8:08 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 04/13] iommu: push use of type-safe BFN and MFN into iommu_ops Paul Durrant
2018-07-19 8:10 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 05/13] iommu: don't domain_crash() inside iommu_map/unmap_page() Paul Durrant
2018-07-17 13:38 ` [PATCH v3 06/13] public / x86: introduce __HYPERCALL_iommu_op Paul Durrant
2018-07-17 13:38 ` [PATCH v3 07/13] iommu: track reserved ranges using a rangeset Paul Durrant
2018-07-19 9:01 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 08/13] x86: add iommu_op to query reserved ranges Paul Durrant
2018-07-19 9:37 ` Wei Liu
2018-07-19 10:03 ` Paul Durrant
2018-07-17 13:38 ` [PATCH v3 09/13] vtd: add lookup_page method to iommu_ops Paul Durrant
2018-07-19 9:55 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 10/13] x86: add iommu_op to enable modification of IOMMU mappings Paul Durrant
2018-07-20 8:59 ` Wei Liu
2018-07-20 9:05 ` Paul Durrant
2018-07-20 9:19 ` Paul Durrant
2018-07-20 9:30 ` Wei Liu
2018-07-17 13:38 ` [PATCH v3 11/13] memory: add get_paged_gfn() as a wrapper Paul Durrant
2018-07-17 13:38 ` [PATCH v3 12/13] x86: add iommu_ops to modify and flush IOMMU mappings Paul Durrant
2018-07-23 13:35 ` Wei Liu
2018-07-23 13:40 ` Paul Durrant
2018-07-23 13:49 ` Wei Liu
2018-07-23 14:26 ` Wei Liu
2018-07-23 14:27 ` Paul Durrant
2018-07-17 13:38 ` [PATCH v3 13/13] x86: extend the map and unmap iommu_ops to support grant references Paul Durrant
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.