On Wed, May 09, 2007 at 01:55:08PM +0100, Keir Fraser wrote:
>
> There's considerable code duplication in common/grant_table.c. Could we
> please somehow merge __gnttab_unmap() and __gnttab_unmap_and_replace(),
> because they only differ right now in the struct type they take, and in the
> function they call to do the actual unmap or unmap-and-replace work. Perhaps
> their wrappers could stuff a 'union structure' of some sort, with enough
> discrimination to ensure the correct underlying arch-specific function is
> called? We could even only supply unmap-and-replace functionality at the
> arch-specific interface and have new_addr==NULL/zero-pte mean we want the
> the old-style unmap semantics. Then the wrapper for unmap_op can stuff that
> field in the 'union structure' with zero to do the right thing.
OK, that was eazy enough to do. However, in doing so I found a
more serious problem with my second patch. It didn't keep track
of outstanding DMA requests so we could potentially put bogus
data on the wire.
Unfortunately the Linux DMA API doesn't help us in solving this
because on an DMA unmap we don't get the struct page that was
originally used so we'd have to keep a mapping of some sort
from the machine address to the guest-physical address in dom0
of a grant page.
IMHO it's too much of a hassle to do right now and we can live
with a delayed free in that case anyway. Right now we already
may delay certain grant entries in dom0 even with netloop. In
particular, any packets that don't go through netloop can be
delayed indefinitely.
If you disagree then I can put in the tracking needed to handle
this properly.
In the long term perhaps we could modify the Linux DMA API to
add the struct page to make this easier.
The first patch:
[XEN] gnttab: Add new op unmap_and_replace
The operation unmap_and_replace is an extension of unmap_grant_ref.
A new argument in the form of a virtual address (for PV) is given.
Instead of modifying the PTE for the mapped grant table entry to
null, we change it to the PTE for the new address. In turn we
point the new address to null.
As it stands grant table entries once mapped cannot be
remapped by the guest OS (it can however perform a new
mapping on the same entry but that is within our control).
Therefore it's safe to manipulate the mapped PTE entry to
redirect it to a normal page where we've copied the contents.
It's intended to be used as follows:
1) map_grant_ref to v1
2) ...
3) alloc page at v2
4) copy the page at v1 to v2
5) unmap_and_replace v1 with v2
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff -r 3ef0510e44d0 linux-2.6-xen-sparse/include/xen/gnttab.h
--- a/linux-2.6-xen-sparse/include/xen/gnttab.h Tue May 08 10:21:23 2007 +0100
+++ b/linux-2.6-xen-sparse/include/xen/gnttab.h Thu May 10 11:56:29 2007 +1000
@@ -135,4 +135,19 @@ gnttab_set_unmap_op(struct gnttab_unmap_
unmap->dev_bus_addr = 0;
}
+static inline void
+gnttab_set_replace_op(struct gnttab_unmap_and_replace *unmap, maddr_t addr,
+ maddr_t new_addr, grant_handle_t handle)
+{
+ if (xen_feature(XENFEAT_auto_translated_physmap)) {
+ unmap->host_addr = __pa(addr);
+ unmap->new_addr = __pa(new_addr);
+ } else {
+ unmap->host_addr = addr;
+ unmap->new_addr = new_addr;
+ }
+
+ unmap->handle = handle;
+}
+
#endif /* __ASM_GNTTAB_H__ */
diff -r 3ef0510e44d0 xen/arch/ia64/xen/mm.c
--- a/xen/arch/ia64/xen/mm.c Tue May 08 10:21:23 2007 +0100
+++ b/xen/arch/ia64/xen/mm.c Thu May 10 11:56:29 2007 +1000
@@ -63,7 +63,7 @@
* assign_domain_page_replace()
* - cmpxchg p2m entry
* assign_domain_page_cmpxchg_rel()
- * destroy_grant_host_mapping()
+ * replace_grant_host_mapping()
* steal_page()
* zap_domain_page_one()
* - read p2m entry
@@ -133,7 +133,7 @@
* - races between p2m entry update and tlb insert
* This is a race between reading/writing the p2m entry.
* reader: vcpu_itc_i(), vcpu_itc_d(), ia64_do_page_fault(), vcpu_fc()
- * writer: assign_domain_page_cmpxchg_rel(), destroy_grant_host_mapping(),
+ * writer: assign_domain_page_cmpxchg_rel(), replace_grant_host_mapping(),
* steal_page(), zap_domain_page_one()
*
* For example, vcpu_itc_i() is about to insert tlb by calling
@@ -151,7 +151,7 @@
* This is a race between reading/writing the p2m entry.
* reader: vcpu_get_domain_bundle(), vmx_get_domain_bundle(),
* efi_emulate_get_time()
- * writer: assign_domain_page_cmpxchg_rel(), destroy_grant_host_mapping(),
+ * writer: assign_domain_page_cmpxchg_rel(), replace_grant_host_mapping(),
* steal_page(), zap_domain_page_one()
*
* A page which assigned to a domain can be de-assigned by another vcpu.
@@ -1509,8 +1509,8 @@ create_grant_host_mapping(unsigned long
// grant table host unmapping
int
-destroy_grant_host_mapping(unsigned long gpaddr,
- unsigned long mfn, unsigned int flags)
+replace_grant_host_mapping(unsigned long gpaddr,
+ unsigned long mfn, unsigned long new_gpaddr, unsigned int flags)
{
struct domain* d = current->domain;
unsigned long gpfn = gpaddr >> PAGE_SHIFT;
@@ -1521,6 +1521,11 @@ destroy_grant_host_mapping(unsigned long
pte_t old_pte;
struct page_info* page = mfn_to_page(mfn);
+ if (new_gpaddr) {
+ gdprintk(XENLOG_INFO, "%s: new_gpaddr 0x%lx\n", __func__, new_gpaddr);
+ return GNTST_general_error;
+ }
+
if (flags & (GNTMAP_application_map | GNTMAP_contains_pte)) {
gdprintk(XENLOG_INFO, "%s: flags 0x%x\n", __func__, flags);
return GNTST_general_error;
@@ -1568,7 +1573,7 @@ destroy_grant_host_mapping(unsigned long
BUG_ON(pte_pgc_allocated(old_pte));
domain_page_flush_and_put(d, gpaddr, pte, old_pte, page);
- perfc_incr(destroy_grant_host_mapping);
+ perfc_incr(replace_grant_host_mapping);
return GNTST_okay;
}
diff -r 3ef0510e44d0 xen/arch/powerpc/mm.c
--- a/xen/arch/powerpc/mm.c Tue May 08 10:21:23 2007 +0100
+++ b/xen/arch/powerpc/mm.c Thu May 10 11:56:29 2007 +1000
@@ -183,9 +183,16 @@ int create_grant_host_mapping(
return create_grant_va_mapping(addr, frame, current);
}
-int destroy_grant_host_mapping(
- unsigned long addr, unsigned long frame, unsigned int flags)
-{
+int replace_grant_host_mapping(
+ unsigned long addr, unsigned long frame, unsigned long new_addr,
+ unsigned int flags)
+{
+ if (new_addr)
+ printk("%s: new_addr not supported\n", __func__);
+ BUG();
+ return GNTST_general_error;
+ }
+
if (flags & GNTMAP_contains_pte) {
printk("%s: GNTMAP_contains_pte not supported\n", __func__);
BUG();
diff -r 3ef0510e44d0 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c Tue May 08 10:21:23 2007 +0100
+++ b/xen/arch/x86/mm.c Thu May 10 11:56:29 2007 +1000
@@ -2619,8 +2619,8 @@ static int create_grant_va_mapping(
return GNTST_okay;
}
-static int destroy_grant_va_mapping(
- unsigned long addr, unsigned long frame, struct vcpu *v)
+static int replace_grant_va_mapping(
+ unsigned long addr, unsigned long frame, l1_pgentry_t nl1e, struct vcpu *v)
{
l1_pgentry_t *pl1e, ol1e;
unsigned long gl1mfn;
@@ -2644,7 +2644,7 @@ static int destroy_grant_va_mapping(
}
/* Delete pagetable entry. */
- if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, l1e_empty(), gl1mfn, v)) )
+ if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, v)) )
{
MEM_LOG("Cannot delete PTE entry at %p", (unsigned long *)pl1e);
rc = GNTST_general_error;
@@ -2654,6 +2654,12 @@ static int destroy_grant_va_mapping(
out:
guest_unmap_l1e(v, pl1e);
return rc;
+}
+
+static int destroy_grant_va_mapping(
+ unsigned long addr, unsigned long frame, struct vcpu *v)
+{
+ return replace_grant_va_mapping(addr, frame, l1e_empty(), v);
}
int create_grant_host_mapping(
@@ -2671,12 +2677,48 @@ int create_grant_host_mapping(
return create_grant_va_mapping(addr, pte, current);
}
-int destroy_grant_host_mapping(
- uint64_t addr, unsigned long frame, unsigned int flags)
-{
+int replace_grant_host_mapping(
+ uint64_t addr, unsigned long frame, uint64_t new_addr, unsigned int flags)
+{
+ l1_pgentry_t *pl1e, ol1e;
+ unsigned long gl1mfn;
+ int rc;
+
if ( flags & GNTMAP_contains_pte )
- return destroy_grant_pte_mapping(addr, frame, current->domain);
- return destroy_grant_va_mapping(addr, frame, current);
+ {
+ if (!new_addr)
+ return destroy_grant_pte_mapping(addr, frame, current->domain);
+
+ MEM_LOG("Unsupported grant table operation");
+ return GNTST_general_error;
+ }
+
+ if (!new_addr)
+ return destroy_grant_va_mapping(addr, frame, current);
+
+ pl1e = guest_map_l1e(current, new_addr, &gl1mfn);
+ if ( !pl1e )
+ {
+ MEM_LOG("Could not find L1 PTE for address %lx",
+ (unsigned long)new_addr);
+ return GNTST_general_error;
+ }
+ ol1e = *pl1e;
+
+ if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, l1e_empty(), gl1mfn, current))
)
+ {
+ MEM_LOG("Cannot delete PTE entry at %p", (unsigned long *)pl1e);
+ guest_unmap_l1e(current, pl1e);
+ return GNTST_general_error;
+ }
+
+ guest_unmap_l1e(current, pl1e);
+
+ rc = replace_grant_va_mapping(addr, frame, ol1e, current);
+ if ( rc && !paging_mode_refcounts(current->domain) )
+ put_page_from_l1e(ol1e, current->domain);
+
+ return rc;
}
int steal_page(
diff -r 3ef0510e44d0 xen/common/grant_table.c
--- a/xen/common/grant_table.c Tue May 08 10:21:23 2007 +0100
+++ b/xen/common/grant_table.c Thu May 10 11:56:29 2007 +1000
@@ -58,6 +58,16 @@ union grant_combo {
} shorts;
};
+/* Used to share code between unmap_grant_ref and unmap_and_replace. */
+struct gnttab_unmap_common {
+ uint64_t host_addr;
+ uint64_t dev_bus_addr;
+ uint64_t new_addr;
+ grant_handle_t handle;
+
+ int16_t status;
+};
+
#define PIN_FAIL(_lbl, _rc, _f, _a...) \
do { \
gdprintk(XENLOG_WARNING, _f, ## _a ); \
@@ -397,8 +407,8 @@ gnttab_map_grant_ref(
}
static void
-__gnttab_unmap_grant_ref(
- struct gnttab_unmap_grant_ref *op)
+__gnttab_unmap_common(
+ struct gnttab_unmap_common *op)
{
domid_t dom;
grant_ref_t ref;
@@ -477,8 +487,8 @@ __gnttab_unmap_grant_ref(
if ( (op->host_addr != 0) && (flags & GNTMAP_host_map) )
{
- if ( (rc = destroy_grant_host_mapping(op->host_addr,
- frame, flags)) < 0 )
+ if ( (rc = replace_grant_host_mapping(op->host_addr,
+ frame, op->new_addr, flags)) < 0
)
goto unmap_out;
ASSERT(act->pin & (GNTPIN_hstw_mask | GNTPIN_hstr_mask));
@@ -518,6 +528,20 @@ __gnttab_unmap_grant_ref(
rcu_unlock_domain(rd);
}
+static void
+__gnttab_unmap_grant_ref(
+ struct gnttab_unmap_grant_ref *op)
+{
+ struct gnttab_unmap_common common = {
+ .host_addr = op->host_addr,
+ .dev_bus_addr = op->dev_bus_addr,
+ .handle = op->handle,
+ };
+
+ __gnttab_unmap_common(&common);
+ op->status = common.status;
+}
+
static long
gnttab_unmap_grant_ref(
XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t) uop, unsigned int count)
@@ -530,6 +554,44 @@ gnttab_unmap_grant_ref(
if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
goto fault;
__gnttab_unmap_grant_ref(&op);
+ if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) )
+ goto fault;
+ }
+
+ flush_tlb_mask(current->domain->domain_dirty_cpumask);
+ return 0;
+
+fault:
+ flush_tlb_mask(current->domain->domain_dirty_cpumask);
+ return -EFAULT;
+}
+
+static void
+__gnttab_unmap_and_replace(
+ struct gnttab_unmap_and_replace *op)
+{
+ struct gnttab_unmap_common common = {
+ .host_addr = op->host_addr,
+ .new_addr = op->new_addr,
+ .handle = op->handle,
+ };
+
+ __gnttab_unmap_common(&common);
+ op->status = common.status;
+}
+
+static long
+gnttab_unmap_and_replace(
+ XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t) uop, unsigned int count)
+{
+ int i;
+ struct gnttab_unmap_and_replace op;
+
+ for ( i = 0; i < count; i++ )
+ {
+ if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
+ goto fault;
+ __gnttab_unmap_and_replace(&op);
if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) )
goto fault;
}
@@ -1203,6 +1265,21 @@ do_grant_table_op(
if ( unlikely(!grant_operation_permitted(d)) )
goto out;
rc = gnttab_unmap_grant_ref(unmap, count);
+ break;
+ }
+ case GNTTABOP_unmap_and_replace:
+ {
+ XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t) unmap =
+ guest_handle_cast(uop, gnttab_unmap_and_replace_t);
+ if ( unlikely(!guest_handle_okay(unmap, count)) )
+ goto out;
+ rc = -EPERM;
+ if ( unlikely(!grant_operation_permitted(d)) )
+ goto out;
+ rc = -ENOSYS;
+ if ( unlikely(!replace_grant_supported()) )
+ goto out;
+ rc = gnttab_unmap_and_replace(unmap, count);
break;
}
case GNTTABOP_setup_table:
diff -r 3ef0510e44d0 xen/include/asm-ia64/grant_table.h
--- a/xen/include/asm-ia64/grant_table.h Tue May 08 10:21:23 2007 +0100
+++ b/xen/include/asm-ia64/grant_table.h Thu May 10 11:56:29 2007 +1000
@@ -9,7 +9,7 @@
// for grant map/unmap
int create_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
unsigned int flags);
-int destroy_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
unsigned int flags);
+int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
unsigned long new_gpaddr, unsigned int flags);
// for grant transfer
void guest_physmap_add_page(struct domain *d, unsigned long gpfn, unsigned
long mfn);
@@ -67,4 +67,9 @@ static inline void gnttab_clear_flag(uns
#define gnttab_release_put_page(page) put_page((page))
#define gnttab_release_put_page_and_type(page) put_page_and_type((page))
+static inline int replace_grant_supported(void)
+{
+ return 0;
+}
+
#endif /* __ASM_GRANT_TABLE_H__ */
diff -r 3ef0510e44d0 xen/include/asm-powerpc/grant_table.h
--- a/xen/include/asm-powerpc/grant_table.h Tue May 08 10:21:23 2007 +0100
+++ b/xen/include/asm-powerpc/grant_table.h Thu May 10 11:56:29 2007 +1000
@@ -35,8 +35,9 @@ extern long pte_remove(ulong flags, ulon
int create_grant_host_mapping(
unsigned long addr, unsigned long frame, unsigned int flags);
-int destroy_grant_host_mapping(
- unsigned long addr, unsigned long frame, unsigned int flags);
+int replace_grant_host_mapping(
+ unsigned long addr, unsigned long frame, unsigned long new_addr,
+ unsigned int flags);
#define gnttab_create_shared_page(d, t, i) \
do { \
@@ -82,4 +83,8 @@ static inline uint cpu_foreign_map_order
#define gnttab_release_put_page_and_type(page) do { } while (0)
#endif
+static inline int replace_grant_supported(void)
+{
+ return 0;
+}
#endif /* __ASM_PPC_GRANT_TABLE_H__ */
diff -r 3ef0510e44d0 xen/include/asm-x86/grant_table.h
--- a/xen/include/asm-x86/grant_table.h Tue May 08 10:21:23 2007 +0100
+++ b/xen/include/asm-x86/grant_table.h Thu May 10 11:56:29 2007 +1000
@@ -15,8 +15,8 @@
*/
int create_grant_host_mapping(
uint64_t addr, unsigned long frame, unsigned int flags);
-int destroy_grant_host_mapping(
- uint64_t addr, unsigned long frame, unsigned int flags);
+int replace_grant_host_mapping(
+ uint64_t addr, unsigned long frame, uint64_t new_addr, unsigned int flags);
#define gnttab_create_shared_page(d, t, i) \
do { \
@@ -48,4 +48,9 @@ static inline void gnttab_clear_flag(uns
/* Done implicitly when page tables are destroyed. */ \
} while (0)
+static inline int replace_grant_supported(void)
+{
+ return 1;
+}
+
#endif /* __ASM_GRANT_TABLE_H__ */
diff -r 3ef0510e44d0 xen/include/public/grant_table.h
--- a/xen/include/public/grant_table.h Tue May 08 10:21:23 2007 +0100
+++ b/xen/include/public/grant_table.h Thu May 10 11:56:29 2007 +1000
@@ -327,6 +327,29 @@ struct gnttab_query_size {
};
typedef struct gnttab_query_size gnttab_query_size_t;
DEFINE_XEN_GUEST_HANDLE(gnttab_query_size_t);
+
+/*
+ * GNTTABOP_unmap_and_replace: Destroy one or more grant-reference mappings
+ * tracked by <handle> but atomically replace the page table entry with one
+ * pointing to the machine address under <new_addr>. <new_addr> will be
+ * redirected to the null entry.
+ * NOTES:
+ * 1. The call may fail in an undefined manner if either mapping is not
+ * tracked by <handle>.
+ * 2. After executing a batch of unmaps, it is guaranteed that no stale
+ * mappings will remain in the device or host TLBs.
+ */
+#define GNTTABOP_unmap_and_replace 7
+struct gnttab_unmap_and_replace {
+ /* IN parameters. */
+ uint64_t host_addr;
+ uint64_t new_addr;
+ grant_handle_t handle;
+ /* OUT parameters. */
+ int16_t status; /* GNTST_* */
+};
+typedef struct gnttab_unmap_and_replace gnttab_unmap_and_replace_t;
+DEFINE_XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t);
/*
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home