* [PATCH v6 01/10] xen/arm: Introduce steps in domain_relinquish_resource
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 02/10] xen/arm: move mfn_to_p2m_entry in arch/arm/p2m.c Julien Grall
` (9 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
In a later patch, a new step will be added. It will avoid to check every step
when the function was preempted.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v2:
- Introduce the patch
---
xen/arch/arm/domain.c | 37 ++++++++++++++++++++++++++++++-------
xen/include/asm-arm/domain.h | 8 ++++++++
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 67c65c3..1590708 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -497,6 +497,8 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
{
int rc;
+ d->arch.relmem = RELMEM_not_started;
+
/* Idle domains do not need this setup */
if ( is_idle_domain(d) )
return 0;
@@ -696,15 +698,36 @@ int domain_relinquish_resources(struct domain *d)
{
int ret = 0;
- ret = relinquish_memory(d, &d->xenpage_list);
- if ( ret )
- return ret;
+ switch ( d->arch.relmem )
+ {
+ case RELMEM_not_started:
+ d->arch.relmem = RELMEM_xen;
+ /* Falltrough */
- ret = relinquish_memory(d, &d->page_list);
- if ( ret )
- return ret;
+ case RELMEM_xen:
+ ret = relinquish_memory(d, &d->xenpage_list);
+ if ( ret )
+ return ret;
- return ret;
+ d->arch.relmem = RELMEM_page;
+ /* Fallthrough */
+
+ case RELMEM_page:
+ ret = relinquish_memory(d, &d->page_list);
+ if ( ret )
+ return ret;
+
+ d->arch.relmem = RELMEM_done;
+ /* Fallthrough */
+
+ case RELMEM_done:
+ break;
+
+ default:
+ BUG();
+ }
+
+ return 0;
}
void arch_dump_domain_info(struct domain *d)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 0733c5e..53c9895 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -107,6 +107,14 @@ struct arch_domain
struct hvm_domain hvm_domain;
xen_pfn_t *grant_table_gpfn;
+ /* Continuable domain_relinquish_resources(). */
+ enum {
+ RELMEM_not_started,
+ RELMEM_xen,
+ RELMEM_page,
+ RELMEM_done,
+ } relmem;
+
/* Virtual CPUID */
uint32_t vpidr;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 02/10] xen/arm: move mfn_to_p2m_entry in arch/arm/p2m.c
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
2013-12-17 16:27 ` [PATCH v6 01/10] xen/arm: Introduce steps in domain_relinquish_resource Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 03/10] xen/arm: Implement p2m_type_t as an enum Julien Grall
` (8 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
The function mfn_to_p2m_entry will be extended in a following patch to handle
p2m_type_t. It will break compilation because p2m_type_t is not defined
(interdependence between includes).
It's easier to move the function in arch/arm/p2m.c and it's not harmful as the
function is only used in this file.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/p2m.c | 22 ++++++++++++++++++++++
xen/include/asm-arm/page.h | 22 ----------------------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 083f8bf..74636df 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -128,6 +128,28 @@ int p2m_pod_decrease_reservation(struct domain *d,
return -ENOSYS;
}
+static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr)
+{
+ paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
+ lpae_t e = (lpae_t) {
+ .p2m.xn = 0,
+ .p2m.af = 1,
+ .p2m.sh = LPAE_SH_OUTER,
+ .p2m.read = 1,
+ .p2m.write = 1,
+ .p2m.mattr = mattr,
+ .p2m.table = 1,
+ .p2m.valid = 1,
+ };
+
+ ASSERT(!(pa & ~PAGE_MASK));
+ ASSERT(!(pa & ~PADDR_MASK));
+
+ e.bits |= pa;
+
+ return e;
+}
+
/* Allocate a new page table page and hook it in via the given entry */
static int p2m_create_table(struct domain *d,
lpae_t *entry)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index d468418..0625464 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -213,28 +213,6 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn)
return e;
}
-static inline lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr)
-{
- paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
- lpae_t e = (lpae_t) {
- .p2m.xn = 0,
- .p2m.af = 1,
- .p2m.sh = LPAE_SH_OUTER,
- .p2m.write = 1,
- .p2m.read = 1,
- .p2m.mattr = mattr,
- .p2m.table = 1,
- .p2m.valid = 1,
- };
-
- ASSERT(!(pa & ~PAGE_MASK));
- ASSERT(!(pa & ~PADDR_MASK));
-
- e.bits |= pa;
-
- return e;
-}
-
#if defined(CONFIG_ARM_32)
# include <asm/arm32/page.h>
#elif defined(CONFIG_ARM_64)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 03/10] xen/arm: Implement p2m_type_t as an enum
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
2013-12-17 16:27 ` [PATCH v6 01/10] xen/arm: Introduce steps in domain_relinquish_resource Julien Grall
2013-12-17 16:27 ` [PATCH v6 02/10] xen/arm: move mfn_to_p2m_entry in arch/arm/p2m.c Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 04/10] xen/arm: Store p2m type in each page of the guest Julien Grall
` (7 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Until now, Xen doesn't know the type of the page (ram, foreign page, mmio,...).
Introduce p2m_type_t with basic types:
- p2m_invalid: Nothing is mapped here
- p2m_ram_rw: Normal read/write guest RAM
- p2m_ram_ro: Read-only guest RAM
- p2m_mmio_direct: Read/write mapping of device memory
- p2m_map_foreign: RAM page from foreign guest
- p2m_grant_map_rw: Read/write grant mapping
- p2m_grant_map_ro: Read-only grant mapping
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v3:
- s/ / /
- Replace p2m by either pte or p2m entry
- Fix compilation (unitialized value)
- Add BUILD_BUG_ON (from patch #4) and fix it
Changes in v2:
- Add comment for future improvement
- Add p2m_max_real_type. Will be use later to check the size of
the enum
- Let the compiler choose the value for each name of the enum
- Add grant mapping type
---
xen/arch/arm/p2m.c | 2 ++
xen/include/asm-arm/p2m.h | 20 +++++++++++++++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 74636df..691cdfa 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -142,6 +142,8 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr)
.p2m.valid = 1,
};
+ BUILD_BUG_ON(p2m_max_real_type > (1 << 4));
+
ASSERT(!(pa & ~PAGE_MASK));
ASSERT(!(pa & ~PADDR_MASK));
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c660820..bc86a49 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -20,6 +20,25 @@ struct p2m_domain {
uint8_t vmid;
};
+/* List of possible type for each page in the p2m entry.
+ * The number of available bit per page in the pte for this purpose is 4 bits.
+ * So it's possible to only have 16 fields. If we run out of value in the
+ * future, it's possible to use higher value for pseudo-type and don't store
+ * them in the p2m entry.
+ */
+typedef enum {
+ p2m_invalid = 0, /* Nothing mapped here */
+ p2m_ram_rw, /* Normal read/write guest RAM */
+ p2m_ram_ro, /* Read-only; writes are silently dropped */
+ p2m_mmio_direct, /* Read/write mapping of genuine MMIO area */
+ p2m_map_foreign, /* Ram pages from foreign domain */
+ p2m_grant_map_rw, /* Read/write grant mapping */
+ p2m_grant_map_ro, /* Read-only grant mapping */
+ p2m_max_real_type, /* Types after this won't be store in the p2m */
+} p2m_type_t;
+
+#define p2m_is_foreign(_t) ((_t) == p2m_map_foreign)
+
/* Initialise vmid allocator */
void p2m_vmid_allocator_init(void);
@@ -72,7 +91,6 @@ p2m_pod_decrease_reservation(struct domain *d,
unsigned int order);
/* Look up a GFN and take a reference count on the backing page. */
-typedef int p2m_type_t;
typedef unsigned int p2m_query_t;
#define P2M_ALLOC (1u<<0) /* Populate PoD and paged-out entries */
#define P2M_UNSHARE (1u<<1) /* Break CoW sharing */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 04/10] xen/arm: Store p2m type in each page of the guest
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (2 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 03/10] xen/arm: Implement p2m_type_t as an enum Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-18 14:18 ` Ian Campbell
2013-12-17 16:27 ` [PATCH v6 05/10] xen/arm: p2m: Extend p2m_lookup parameters to retrieve the p2m type Julien Grall
` (6 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Use the field 'avail' to store the type of the page. Rename it to 'type' for
convenience.
The information stored in this field will be retrieved in a future patch to
change the behaviour when the page is removed.
Also introduce guest_physmap_add_entry to map and set a specific p2m type for
a page.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Changes in v6:
- Refactor switch
- Invalid mapping and MMIO mapping doesn't need to have
execution right
- Remove default case in the switch
Changes in v5:
- Foreign mapping doesn't need to have execution right
Changes in v3:
- Typo in the commit message
- Rename 'p2mt' field to 'type'
- Remove default in switch to let the compiler warns
- Move BUILD_BUG_ON to patch #3
Changes in v2:
- Rename 'avail' field to 'p2mt' in the p2m structure
- Add BUILD_BUG_ON to check if the enum value will fit in the field
- Implement grant mapping type
---
xen/arch/arm/p2m.c | 68 +++++++++++++++++++++++++++++++++-----------
xen/include/asm-arm/p2m.h | 18 +++++++++---
xen/include/asm-arm/page.h | 2 +-
3 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 691cdfa..3dda5c2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -128,22 +128,53 @@ int p2m_pod_decrease_reservation(struct domain *d,
return -ENOSYS;
}
-static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr)
+static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr,
+ p2m_type_t t)
{
paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
+ /* xn and write bit will be defined in the switch */
lpae_t e = (lpae_t) {
- .p2m.xn = 0,
.p2m.af = 1,
.p2m.sh = LPAE_SH_OUTER,
.p2m.read = 1,
- .p2m.write = 1,
.p2m.mattr = mattr,
.p2m.table = 1,
.p2m.valid = 1,
+ .p2m.type = t,
};
BUILD_BUG_ON(p2m_max_real_type > (1 << 4));
+ switch (t)
+ {
+ case p2m_ram_rw:
+ e.p2m.xn = 0;
+ e.p2m.write = 1;
+ break;
+
+ case p2m_ram_ro:
+ e.p2m.xn = 0;
+ e.p2m.write = 0;
+ break;
+
+ case p2m_map_foreign:
+ case p2m_grant_map_rw:
+ case p2m_mmio_direct:
+ e.p2m.xn = 1;
+ e.p2m.write = 1;
+ break;
+
+ case p2m_grant_map_ro:
+ case p2m_invalid:
+ e.p2m.xn = 1;
+ e.p2m.write = 0;
+ break;
+
+ case p2m_max_real_type:
+ BUG();
+ break;
+ }
+
ASSERT(!(pa & ~PAGE_MASK));
ASSERT(!(pa & ~PADDR_MASK));
@@ -173,7 +204,7 @@ static int p2m_create_table(struct domain *d,
clear_page(p);
unmap_domain_page(p);
- pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM);
+ pte = mfn_to_p2m_entry(page_to_mfn(page), MATTR_MEM, p2m_invalid);
write_pte(entry, pte);
@@ -191,7 +222,8 @@ static int create_p2m_entries(struct domain *d,
paddr_t start_gpaddr,
paddr_t end_gpaddr,
paddr_t maddr,
- int mattr)
+ int mattr,
+ p2m_type_t t)
{
int rc, flush;
struct p2m_domain *p2m = &d->arch.p2m;
@@ -271,14 +303,15 @@ static int create_p2m_entries(struct domain *d,
goto out;
}
- pte = mfn_to_p2m_entry(page_to_mfn(page), mattr);
+ pte = mfn_to_p2m_entry(page_to_mfn(page), mattr, t);
write_pte(&third[third_table_offset(addr)], pte);
}
break;
case INSERT:
{
- lpae_t pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT, mattr);
+ lpae_t pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT,
+ mattr, t);
write_pte(&third[third_table_offset(addr)], pte);
maddr += PAGE_SIZE;
}
@@ -313,7 +346,8 @@ int p2m_populate_ram(struct domain *d,
paddr_t start,
paddr_t end)
{
- return create_p2m_entries(d, ALLOCATE, start, end, 0, MATTR_MEM);
+ return create_p2m_entries(d, ALLOCATE, start, end,
+ 0, MATTR_MEM, p2m_ram_rw);
}
int map_mmio_regions(struct domain *d,
@@ -321,18 +355,20 @@ int map_mmio_regions(struct domain *d,
paddr_t end_gaddr,
paddr_t maddr)
{
- return create_p2m_entries(d, INSERT, start_gaddr, end_gaddr, maddr, MATTR_DEV);
+ return create_p2m_entries(d, INSERT, start_gaddr, end_gaddr,
+ maddr, MATTR_DEV, p2m_mmio_direct);
}
-int guest_physmap_add_page(struct domain *d,
- unsigned long gpfn,
- unsigned long mfn,
- unsigned int page_order)
+int guest_physmap_add_entry(struct domain *d,
+ unsigned long gpfn,
+ unsigned long mfn,
+ unsigned long page_order,
+ p2m_type_t t)
{
return create_p2m_entries(d, INSERT,
pfn_to_paddr(gpfn),
- pfn_to_paddr(gpfn + (1<<page_order)),
- pfn_to_paddr(mfn), MATTR_MEM);
+ pfn_to_paddr(gpfn + (1 << page_order)),
+ pfn_to_paddr(mfn), MATTR_MEM, t);
}
void guest_physmap_remove_page(struct domain *d,
@@ -342,7 +378,7 @@ void guest_physmap_remove_page(struct domain *d,
create_p2m_entries(d, REMOVE,
pfn_to_paddr(gpfn),
pfn_to_paddr(gpfn + (1<<page_order)),
- pfn_to_paddr(mfn), MATTR_MEM);
+ pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid);
}
int p2m_alloc_table(struct domain *d)
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index bc86a49..5742bbc 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -68,11 +68,21 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
int map_mmio_regions(struct domain *d, paddr_t start_gaddr,
paddr_t end_gaddr, paddr_t maddr);
+int guest_physmap_add_entry(struct domain *d,
+ unsigned long gfn,
+ unsigned long mfn,
+ unsigned long page_order,
+ p2m_type_t t);
+
/* Untyped version for RAM only, for compatibility */
-int guest_physmap_add_page(struct domain *d,
- unsigned long gfn,
- unsigned long mfn,
- unsigned int page_order);
+static inline int guest_physmap_add_page(struct domain *d,
+ unsigned long gfn,
+ unsigned long mfn,
+ unsigned int page_order)
+{
+ return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
+}
+
void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order);
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 0625464..670d4e7 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -153,7 +153,7 @@ typedef struct {
unsigned long contig:1; /* In a block of 16 contiguous entries */
unsigned long sbz2:1;
unsigned long xn:1; /* eXecute-Never */
- unsigned long avail:4; /* Ignored by hardware */
+ unsigned long type:4; /* Ignore by hardware. Used to store p2m types */
unsigned long sbz1:5;
} __attribute__((__packed__)) lpae_p2m_t;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v6 04/10] xen/arm: Store p2m type in each page of the guest
2013-12-17 16:27 ` [PATCH v6 04/10] xen/arm: Store p2m type in each page of the guest Julien Grall
@ 2013-12-18 14:18 ` Ian Campbell
0 siblings, 0 replies; 23+ messages in thread
From: Ian Campbell @ 2013-12-18 14:18 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, patches
On Tue, 2013-12-17 at 16:27 +0000, Julien Grall wrote:
> Use the field 'avail' to store the type of the page. Rename it to 'type' for
> convenience.
> The information stored in this field will be retrieved in a future patch to
> change the behaviour when the page is removed.
>
> Also introduce guest_physmap_add_entry to map and set a specific p2m type for
> a page.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 05/10] xen/arm: p2m: Extend p2m_lookup parameters to retrieve the p2m type
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (3 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 04/10] xen/arm: Store p2m type in each page of the guest Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 06/10] xen/arm: Retrieve p2m type in get_page_from_gfn Julien Grall
` (5 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v6:
- Add an ASSERT to check if p2mt != p2m_invalid
Changes in v3:
- 'p2mt' field was renamed in 'type'
- missing one p2m_lookup
Changes in v2:
- This patch was "Add p2m_get_entry" on the previous version
- Don't add a new function but only extend p2m_lookup
---
xen/arch/arm/mm.c | 2 +-
xen/arch/arm/p2m.c | 14 ++++++++++++--
xen/arch/arm/traps.c | 6 +++---
xen/include/asm-arm/p2m.h | 2 +-
4 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 654281a..b1222be 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1032,7 +1032,7 @@ static int xenmem_add_to_physmap_one(
return rc;
}
- maddr = p2m_lookup(od, pfn_to_paddr(idx));
+ maddr = p2m_lookup(od, pfn_to_paddr(idx), NULL);
if ( maddr == INVALID_PADDR )
{
dump_p2m_lookup(od, pfn_to_paddr(idx));
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 3dda5c2..bc32935 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -72,11 +72,17 @@ static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
* There are no processor functions to do a stage 2 only lookup therefore we
* do a a software walk.
*/
-paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
+paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
{
struct p2m_domain *p2m = &d->arch.p2m;
lpae_t pte, *first = NULL, *second = NULL, *third = NULL;
paddr_t maddr = INVALID_PADDR;
+ p2m_type_t _t;
+
+ /* Allow t to be NULL */
+ t = t ?: &_t;
+
+ *t = p2m_invalid;
spin_lock(&p2m->lock);
@@ -102,7 +108,11 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
done:
if ( pte.p2m.valid )
+ {
+ ASSERT(pte.p2m.type != p2m_invalid);
maddr = (pte.bits & PADDR_MASK & PAGE_MASK) | (paddr & ~PAGE_MASK);
+ *t = pte.p2m.type;
+ }
if (third) unmap_domain_page(third);
if (second) unmap_domain_page(second);
@@ -521,7 +531,7 @@ err:
unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
{
- paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn));
+ paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn), NULL);
return p >> PAGE_SHIFT;
}
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index e01045e..7c5ab19 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1421,7 +1421,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
printk(" TTBCR: 0x%08"PRIregister"\n", ttbcr);
printk(" TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n",
- ttbr0, p2m_lookup(d, ttbr0 & PAGE_MASK));
+ ttbr0, p2m_lookup(d, ttbr0 & PAGE_MASK, NULL));
if ( ttbcr & TTBCR_EAE )
{
@@ -1434,7 +1434,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
return;
}
- paddr = p2m_lookup(d, ttbr0 & PAGE_MASK);
+ paddr = p2m_lookup(d, ttbr0 & PAGE_MASK, NULL);
if ( paddr == INVALID_PADDR )
{
printk("Failed TTBR0 maddr lookup\n");
@@ -1449,7 +1449,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
!(first[offset] & 0x2) )
goto done;
- paddr = p2m_lookup(d, first[offset] & PAGE_MASK);
+ paddr = p2m_lookup(d, first[offset] & PAGE_MASK, NULL);
if ( paddr == INVALID_PADDR )
{
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 5742bbc..9d0d1e7 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -58,7 +58,7 @@ int p2m_alloc_table(struct domain *d);
void p2m_load_VTTBR(struct domain *d);
/* Look up the MFN corresponding to a domain's PFN. */
-paddr_t p2m_lookup(struct domain *d, paddr_t gpfn);
+paddr_t p2m_lookup(struct domain *d, paddr_t gpfn, p2m_type_t *t);
/* Setup p2m RAM mapping for domain d from start-end. */
int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 06/10] xen/arm: Retrieve p2m type in get_page_from_gfn
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (4 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 05/10] xen/arm: p2m: Extend p2m_lookup parameters to retrieve the p2m type Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 07/10] xen/arm: Handle remove foreign mapping Julien Grall
` (4 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v3:
- Return NULL when p2m type is invalid or mmio
Changes in v2:
- Use p2m_lookup as p2m_get_entry was removed
---
xen/include/asm-arm/p2m.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 9d0d1e7..0eb07a8 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -109,11 +109,17 @@ static inline struct page_info *get_page_from_gfn(
struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
{
struct page_info *page;
- unsigned long mfn = gmfn_to_mfn(d, gfn);
+ p2m_type_t p2mt;
+ paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), &p2mt);
+ unsigned long mfn = maddr >> PAGE_SHIFT;
- ASSERT(t == NULL);
+ if (t)
+ *t = p2mt;
- if (!mfn_valid(mfn))
+ if ( p2mt == p2m_invalid || p2mt == p2m_mmio_direct )
+ return NULL;
+
+ if ( !mfn_valid(mfn) )
return NULL;
page = mfn_to_page(mfn);
if ( !get_page(page, d) )
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 07/10] xen/arm: Handle remove foreign mapping
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (5 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 06/10] xen/arm: Retrieve p2m type in get_page_from_gfn Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page Julien Grall
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Modify get_page_from_gfn to take reference on foreign mapping. This will avoid
specific handling in the common code.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v6:
- Use pte.p2m.base instead of arithmetics
- We don't need to clean pte where valid == 0 (save time)
Changes in v4.2:
- get_page_from_gfn: move foreign checking before get_page
- add assert fdom != dom
Changes in v4.1:
- Remove specific p2m handling in common code
- Handle foreign mapping in get_page_from_gfn
Changes in v4:
- Split patch #6 from dom0 pvh series v6.2 to retrieve only common
code.
- Rework commit title
- Rename xen_rem_foreign_from_p2m in p2m_remove_foreign
- Get the mfn from the pte. We are not sure that maddr given in
parameters is valid
Changes in v3:
- Move put_page in create_p2m_entries
- Move xenmem_rem_foreign_from_p2m in arch/arm/p2m.c
Changes in v2:
- Introduce the patch
---
xen/arch/arm/p2m.c | 15 +++++++++++++--
xen/include/asm-arm/p2m.h | 12 ++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index bc32935..410acb6 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -328,10 +328,21 @@ static int create_p2m_entries(struct domain *d,
break;
case REMOVE:
{
- lpae_t pte;
+ lpae_t pte = third[third_table_offset(addr)];
+ unsigned long mfn = pte.p2m.base;
+
+ if ( !pte.p2m.valid )
+ break;
+
+ /* TODO: Handle other p2m type */
+ if ( p2m_is_foreign(pte.p2m.type) )
+ {
+ ASSERT(mfn_valid(mfn));
+ put_page(mfn_to_page(mfn));
+ }
+
memset(&pte, 0x00, sizeof(pte));
write_pte(&third[third_table_offset(addr)], pte);
- maddr += PAGE_SIZE;
}
break;
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 0eb07a8..5ccfa7f 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -122,6 +122,18 @@ static inline struct page_info *get_page_from_gfn(
if ( !mfn_valid(mfn) )
return NULL;
page = mfn_to_page(mfn);
+
+ /* get_page won't work on foreign mapping because the page doesn't
+ * belong to the current domain.
+ */
+ if ( p2mt == p2m_map_foreign )
+ {
+ struct domain *fdom = page_get_owner_and_reference(page);
+ ASSERT(fdom != NULL);
+ ASSERT(fdom != d);
+ return page;
+ }
+
if ( !get_page(page, d) )
return NULL;
return page;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (6 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 07/10] xen/arm: Handle remove foreign mapping Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:36 ` Ian Campbell
` (2 more replies)
2013-12-17 16:27 ` [PATCH v6 09/10] xen/arm: Set foreign page type to p2m_map_foreign Julien Grall
` (2 subsequent siblings)
10 siblings, 3 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
This function will be called when the domain relinquishes its memory.
It removes refcount on every mapped page to a valid MFN.
Currently, Xen doesn't take reference on every new mapping but only for foreign
mapping. Restrict the function only on foreign mapping.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Changes in v6:
- Typoes
- Rework preempt
- Clean up if ( p2mt == ... || p2mt == ... )
Changes in v4:
- Use LPAE_ENTRIES instead of hardcoded value
Changes in v3:
- Rework title
- Reuse create_p2m_entries to remove reference
- Don't forget to set relmem!
- Fix compilation (missing include)
Changes in v2:
- Introduce the patch
---
xen/arch/arm/domain.c | 8 +++++++
xen/arch/arm/p2m.c | 49 +++++++++++++++++++++++++++++++++++++++++-
xen/include/asm-arm/domain.h | 1 +
xen/include/asm-arm/p2m.h | 15 +++++++++++++
4 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 1590708..4099e88 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -717,6 +717,14 @@ int domain_relinquish_resources(struct domain *d)
if ( ret )
return ret;
+ d->arch.relmem = RELMEM_mapping;
+ /* Fallthrough */
+
+ case RELMEM_mapping:
+ ret = relinquish_p2m_mapping(d);
+ if ( ret )
+ return ret;
+
d->arch.relmem = RELMEM_done;
/* Fallthrough */
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 410acb6..3f4ab37 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -6,6 +6,8 @@
#include <xen/bitops.h>
#include <asm/flushtlb.h>
#include <asm/gic.h>
+#include <asm/event.h>
+#include <asm/hardirq.h>
/* First level P2M is 2 consecutive pages */
#define P2M_FIRST_ORDER 1
@@ -224,7 +226,8 @@ static int p2m_create_table(struct domain *d,
enum p2m_operation {
INSERT,
ALLOCATE,
- REMOVE
+ REMOVE,
+ RELINQUISH,
};
static int create_p2m_entries(struct domain *d,
@@ -242,6 +245,7 @@ static int create_p2m_entries(struct domain *d,
unsigned long cur_first_page = ~0,
cur_first_offset = ~0,
cur_second_offset = ~0;
+ unsigned long count = 0;
spin_lock(&p2m->lock);
@@ -326,13 +330,19 @@ static int create_p2m_entries(struct domain *d,
maddr += PAGE_SIZE;
}
break;
+ case RELINQUISH:
case REMOVE:
{
lpae_t pte = third[third_table_offset(addr)];
unsigned long mfn = pte.p2m.base;
if ( !pte.p2m.valid )
+ {
+ count++;
break;
+ }
+
+ count += 0x10;
/* TODO: Handle other p2m type */
if ( p2m_is_foreign(pte.p2m.type) )
@@ -343,12 +353,35 @@ static int create_p2m_entries(struct domain *d,
memset(&pte, 0x00, sizeof(pte));
write_pte(&third[third_table_offset(addr)], pte);
+ count++;
}
break;
}
if ( flush )
flush_tlb_all_local();
+
+ /* Preempt every 2MiB (mapped) or 32 MiB (unmapped) - arbitrary */
+ if ( op == RELINQUISH && count >= 0x2000 )
+ {
+ if ( hypercall_preempt_check() )
+ {
+ p2m->next_gfn_to_relinquish = maddr >> PAGE_SHIFT;
+ rc = -EAGAIN;
+ goto out;
+ }
+ count = 0;
+ }
+ }
+
+ if ( op == ALLOCATE || op == INSERT )
+ {
+ unsigned long sgfn = paddr_to_pfn(start_gpaddr);
+ unsigned long egfn = paddr_to_pfn(end_gpaddr);
+
+ p2m->max_mapped_gfn = MAX(p2m->max_mapped_gfn, egfn);
+ /* Use next_gfn_to_relinquish to store the lowest gfn mapped */
+ p2m->next_gfn_to_relinquish = MIN(p2m->next_gfn_to_relinquish, sgfn);
}
rc = 0;
@@ -534,12 +567,26 @@ int p2m_init(struct domain *d)
p2m->first_level = NULL;
+ p2m->max_mapped_gfn = 0;
+ p2m->next_gfn_to_relinquish = ULONG_MAX;
+
err:
spin_unlock(&p2m->lock);
return rc;
}
+int relinquish_p2m_mapping(struct domain *d)
+{
+ struct p2m_domain *p2m = &d->arch.p2m;
+
+ return create_p2m_entries(d, RELINQUISH,
+ pfn_to_paddr(p2m->next_gfn_to_relinquish),
+ pfn_to_paddr(p2m->max_mapped_gfn),
+ pfn_to_paddr(INVALID_MFN),
+ MATTR_MEM, p2m_invalid);
+}
+
unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
{
paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn), NULL);
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 53c9895..28d39a0 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -112,6 +112,7 @@ struct arch_domain
RELMEM_not_started,
RELMEM_xen,
RELMEM_page,
+ RELMEM_mapping,
RELMEM_done,
} relmem;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 5ccfa7f..8b7b6d0 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -18,6 +18,15 @@ struct p2m_domain {
/* Current VMID in use */
uint8_t vmid;
+
+ /* Highest guest frame that's ever been mapped in the p2m
+ * Only takes into account ram and foreign mapping
+ */
+ unsigned long max_mapped_gfn;
+
+ /* When releasing mapped gfn's in a preemptible manner, recall where
+ * to resume the search */
+ unsigned long next_gfn_to_relinquish;
};
/* List of possible type for each page in the p2m entry.
@@ -48,6 +57,12 @@ int p2m_init(struct domain *d);
/* Return all the p2m resources to Xen. */
void p2m_teardown(struct domain *d);
+/* Remove mapping refcount on each mapping page in the p2m
+ *
+ * TODO: For the moment only foreign mappings are handled
+ */
+int relinquish_p2m_mapping(struct domain *d);
+
/* Allocate a new p2m table for a domain.
*
* Returns 0 for success or -errno.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 16:27 ` [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page Julien Grall
@ 2013-12-17 16:36 ` Ian Campbell
2013-12-17 16:58 ` Julien Grall
2013-12-18 14:20 ` Ian Campbell
2013-12-18 16:48 ` Julien Grall
2 siblings, 1 reply; 23+ messages in thread
From: Ian Campbell @ 2013-12-17 16:36 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, patches
On Tue, 2013-12-17 at 16:27 +0000, Julien Grall wrote:
> This function will be called when the domain relinquishes its memory.
> It removes refcount on every mapped page to a valid MFN.
>
> Currently, Xen doesn't take reference on every new mapping but only for foreign
> mapping. Restrict the function only on foreign mapping.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>
> ---
> Changes in v6:
> - Typoes
> - Rework preempt
> - Clean up if ( p2mt == ... || p2mt == ... )
You don't seem to have addressed the issue I pointed out about
create_p2m_entries exiting on any holes which it finds at the first or
second level.
I think you missed my second reply to 08/10 on v5 see:
<1387279904.27441.47.camel@kazak.uk.xensource.com>
Ian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 16:36 ` Ian Campbell
@ 2013-12-17 16:58 ` Julien Grall
2013-12-17 17:02 ` Ian Campbell
0 siblings, 1 reply; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:58 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, ian.jackson, patches
On 12/17/2013 04:36 PM, Ian Campbell wrote:
> On Tue, 2013-12-17 at 16:27 +0000, Julien Grall wrote:
>> This function will be called when the domain relinquishes its memory.
>> It removes refcount on every mapped page to a valid MFN.
>>
>> Currently, Xen doesn't take reference on every new mapping but only for foreign
>> mapping. Restrict the function only on foreign mapping.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>> Changes in v6:
>> - Typoes
>> - Rework preempt
>> - Clean up if ( p2mt == ... || p2mt == ... )
>
> You don't seem to have addressed the issue I pointed out about
> create_p2m_entries exiting on any holes which it finds at the first or
> second level.
>
> I think you missed my second reply to 08/10 on v5 see:
> <1387279904.27441.47.camel@kazak.uk.xensource.com>
Sorry I forgot to address your comment (see the copy of your email here):
> If this function finds any non-present first or second level PTE then it
> will stop and exit (goto out), meaning it will miss any mappings which
> are higher up after the hole.
>
> e.g. if you have a guest p2m with RAM at 0-2M and 4-6M then
> relinquishing 0-6M will only actually free 0-2M, then abort on 4-6M.
>
> Perhaps this could be fixed by making relinquish_p2m_mapping loop over
> the address space relinquishing 2M chunks as it goes? This would
remove
> the need for the if ( op == RELINQUISH && .. && prempt() ) stuff,
> because you could add the preempt in that loop.
I think you are wrong:
- every first pte exists. We return NULL in case of the address is very
high.
- if the second pte doesn't exist, we will create it (if it
fails we go out).
For the last item, I think it's a bit stupid to create table if we are
removing/relinquish mapping. But I think it's an improvement for later.
There are lots of improvement to do in this function (eg: flushing).
--
Julien Grall
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 16:58 ` Julien Grall
@ 2013-12-17 17:02 ` Ian Campbell
2013-12-18 13:21 ` Ian Campbell
0 siblings, 1 reply; 23+ messages in thread
From: Ian Campbell @ 2013-12-17 17:02 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, patches
On Tue, 2013-12-17 at 16:58 +0000, Julien Grall wrote:
> > If this function finds any non-present first or second level PTE then it
> > will stop and exit (goto out), meaning it will miss any mappings which
> > are higher up after the hole.
> >
> > e.g. if you have a guest p2m with RAM at 0-2M and 4-6M then
> > relinquishing 0-6M will only actually free 0-2M, then abort on 4-6M.
> >
> > Perhaps this could be fixed by making relinquish_p2m_mapping loop over
> > the address space relinquishing 2M chunks as it goes? This would
> remove
> > the need for the if ( op == RELINQUISH && .. && prempt() ) stuff,
> > because you could add the preempt in that loop.
>
> I think you are wrong:
> - every first pte exists. We return NULL in case of the address is very
> high.
> - if the second pte doesn't exist, we will create it (if it
> fails we go out).
Oops, I misread the code, sorry. I think you are correct.
> For the last item, I think it's a bit stupid to create table if we are
> removing/relinquish mapping. But I think it's an improvement for later.
> There are lots of improvement to do in this function (eg: flushing).
Agreed.
Ian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 17:02 ` Ian Campbell
@ 2013-12-18 13:21 ` Ian Campbell
2013-12-18 13:30 ` Julien Grall
2013-12-18 14:06 ` Julien Grall
0 siblings, 2 replies; 23+ messages in thread
From: Ian Campbell @ 2013-12-18 13:21 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Tim Deegan, ian.jackson, Stefano Stabellini, patches
(the cc line here is a bit odd, why Ian J? But not Tim or Stefano? I've
added those two) On Tue, 2013-12-17 at 17:02 +0000, Ian Campbell wrote:
> > For the last item, I think it's a bit stupid to create table if we are
> > removing/relinquish mapping. But I think it's an improvement for later.
> > There are lots of improvement to do in this function (eg: flushing).
>
> Agreed.
Actually, there is an efficiency concern here.
If we were to skip non-present first and second levels then we would
skip vast swathes of the address space very quickly. As it stands we
actively spend time filling it in just so we can recurse over it.
This effectively turns this back into a loop over the entire gfn space,
which is what we wanted to avoid.
The use of next_gfn_to_relinquish..max_mapped_gfn mitigates this
somewhat, but max_mapped_gfn is guest controlled and can trivially be
made huge by the guest.
How hard would it be to skip missing entries for remove and relinquish
walks? Should just be roughtly:
int populate = (op == INSERT || op == ALLOCATE)
for ( addr = ... ; addr < ... ; addr ... )
{
[...]
if ( !first[...].valid )
{
if ( !populate ) {
addr += skip size of a first supermapping,
continue;
}
rc = p2m_create(...)
[..error..]
[same for second etc]
}
}
"size of a first" needs care vs the += PAGE_SIZE in the for loop, I'd be
inclined to turn this into a while loop and move the += PAGE_SIZE to the
end.
Ian.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-18 13:21 ` Ian Campbell
@ 2013-12-18 13:30 ` Julien Grall
2013-12-18 14:06 ` Julien Grall
1 sibling, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-18 13:30 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, Tim Deegan, ian.jackson, Stefano Stabellini, patches
On 12/18/2013 01:21 PM, Ian Campbell wrote:
> (the cc line here is a bit odd, why Ian J? But not Tim or Stefano? I've
> added those two) On Tue, 2013-12-17 at 17:02 +0000, Ian Campbell wrote:
>>> For the last item, I think it's a bit stupid to create table if we are
>>> removing/relinquish mapping. But I think it's an improvement for later.
>>> There are lots of improvement to do in this function (eg: flushing).
>>
>> Agreed.
>
> Actually, there is an efficiency concern here.
>
> If we were to skip non-present first and second levels then we would
> skip vast swathes of the address space very quickly. As it stands we
> actively spend time filling it in just so we can recurse over it.
>
> This effectively turns this back into a loop over the entire gfn space,
> which is what we wanted to avoid.
>
> The use of next_gfn_to_relinquish..max_mapped_gfn mitigates this
> somewhat, but max_mapped_gfn is guest controlled and can trivially be
> made huge by the guest.
>
> How hard would it be to skip missing entries for remove and relinquish
> walks? Should just be roughtly:
>
> int populate = (op == INSERT || op == ALLOCATE)
>
> for ( addr = ... ; addr < ... ; addr ... )
> {
> [...]
>
> if ( !first[...].valid )
> {
> if ( !populate ) {
> addr += skip size of a first supermapping,
> continue;
> }
> rc = p2m_create(...)
> [..error..]
>
> [same for second etc]
> }
> }
>
> "size of a first" needs care vs the += PAGE_SIZE in the for loop, I'd be
> inclined to turn this into a while loop and move the += PAGE_SIZE to the
> end.
Your solution should be fine. I will add a patch in the series.
--
Julien Grall
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-18 13:21 ` Ian Campbell
2013-12-18 13:30 ` Julien Grall
@ 2013-12-18 14:06 ` Julien Grall
2013-12-18 14:12 ` Ian Campbell
1 sibling, 1 reply; 23+ messages in thread
From: Julien Grall @ 2013-12-18 14:06 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, Tim Deegan, ian.jackson, Stefano Stabellini, patches
On 12/18/2013 01:21 PM, Ian Campbell wrote:
> (the cc line here is a bit odd, why Ian J? But not Tim or Stefano? I've
> added those two) On Tue, 2013-12-17 at 17:02 +0000, Ian Campbell wrote:
>>> For the last item, I think it's a bit stupid to create table if we are
>>> removing/relinquish mapping. But I think it's an improvement for later.
>>> There are lots of improvement to do in this function (eg: flushing).
>>
>> Agreed.
>
> Actually, there is an efficiency concern here.
>
> If we were to skip non-present first and second levels then we would
> skip vast swathes of the address space very quickly. As it stands we
> actively spend time filling it in just so we can recurse over it.
>
> This effectively turns this back into a loop over the entire gfn space,
> which is what we wanted to avoid.
>
> The use of next_gfn_to_relinquish..max_mapped_gfn mitigates this
> somewhat, but max_mapped_gfn is guest controlled and can trivially be
> made huge by the guest.
>
> How hard would it be to skip missing entries for remove and relinquish
> walks? Should just be roughtly:
>
> int populate = (op == INSERT || op == ALLOCATE)
>
> for ( addr = ... ; addr < ... ; addr ... )
> {
> [...]
>
> if ( !first[...].valid )
> {
> if ( !populate ) {
> addr += skip size of a first supermapping,
> continue;
> }
> rc = p2m_create(...)
> [..error..]
>
> [same for second etc]
> }
> }
>
> "size of a first" needs care vs the += PAGE_SIZE in the for loop, I'd be
> inclined to turn this into a while loop and move the += PAGE_SIZE to the
> end.
>
> Ian.
>
Ian, is it fine for you if I send a patch on top of this patch series?
So we can go ahead with the current series.
--
Julien Grall
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-18 14:06 ` Julien Grall
@ 2013-12-18 14:12 ` Ian Campbell
0 siblings, 0 replies; 23+ messages in thread
From: Ian Campbell @ 2013-12-18 14:12 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Tim Deegan, ian.jackson, Stefano Stabellini, patches
On Wed, 2013-12-18 at 14:06 +0000, Julien Grall wrote:
>
> On 12/18/2013 01:21 PM, Ian Campbell wrote:
> > (the cc line here is a bit odd, why Ian J? But not Tim or Stefano? I've
> > added those two) On Tue, 2013-12-17 at 17:02 +0000, Ian Campbell wrote:
> >>> For the last item, I think it's a bit stupid to create table if we are
> >>> removing/relinquish mapping. But I think it's an improvement for later.
> >>> There are lots of improvement to do in this function (eg: flushing).
> >>
> >> Agreed.
> >
> > Actually, there is an efficiency concern here.
> >
> > If we were to skip non-present first and second levels then we would
> > skip vast swathes of the address space very quickly. As it stands we
> > actively spend time filling it in just so we can recurse over it.
> >
> > This effectively turns this back into a loop over the entire gfn space,
> > which is what we wanted to avoid.
> >
> > The use of next_gfn_to_relinquish..max_mapped_gfn mitigates this
> > somewhat, but max_mapped_gfn is guest controlled and can trivially be
> > made huge by the guest.
> >
> > How hard would it be to skip missing entries for remove and relinquish
> > walks? Should just be roughtly:
> >
> > int populate = (op == INSERT || op == ALLOCATE)
> >
> > for ( addr = ... ; addr < ... ; addr ... )
> > {
> > [...]
> >
> > if ( !first[...].valid )
> > {
> > if ( !populate ) {
> > addr += skip size of a first supermapping,
> > continue;
> > }
> > rc = p2m_create(...)
> > [..error..]
> >
> > [same for second etc]
> > }
> > }
> >
> > "size of a first" needs care vs the += PAGE_SIZE in the for loop, I'd be
> > inclined to turn this into a while loop and move the += PAGE_SIZE to the
> > end.
> >
> > Ian.
> >
>
> Ian, is it fine for you if I send a patch on top of this patch series?
> So we can go ahead with the current series.
Yeah, lets do that.
Ian.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 16:27 ` [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page Julien Grall
2013-12-17 16:36 ` Ian Campbell
@ 2013-12-18 14:20 ` Ian Campbell
2013-12-18 16:48 ` Julien Grall
2 siblings, 0 replies; 23+ messages in thread
From: Ian Campbell @ 2013-12-18 14:20 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, patches
On Tue, 2013-12-17 at 16:27 +0000, Julien Grall wrote:
> This function will be called when the domain relinquishes its memory.
> It removes refcount on every mapped page to a valid MFN.
>
> Currently, Xen doesn't take reference on every new mapping but only for foreign
> mapping. Restrict the function only on foreign mapping.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
With the proviso that this will be made to properly skip non-present
entries, instead of allocating an empty page to walk, ASAP and certainly
before the release.
>
> ---
> Changes in v6:
> - Typoes
> - Rework preempt
> - Clean up if ( p2mt == ... || p2mt == ... )
> Changes in v4:
> - Use LPAE_ENTRIES instead of hardcoded value
> Changes in v3:
> - Rework title
> - Reuse create_p2m_entries to remove reference
> - Don't forget to set relmem!
> - Fix compilation (missing include)
> Changes in v2:
> - Introduce the patch
> ---
> xen/arch/arm/domain.c | 8 +++++++
> xen/arch/arm/p2m.c | 49 +++++++++++++++++++++++++++++++++++++++++-
> xen/include/asm-arm/domain.h | 1 +
> xen/include/asm-arm/p2m.h | 15 +++++++++++++
> 4 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 1590708..4099e88 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -717,6 +717,14 @@ int domain_relinquish_resources(struct domain *d)
> if ( ret )
> return ret;
>
> + d->arch.relmem = RELMEM_mapping;
> + /* Fallthrough */
> +
> + case RELMEM_mapping:
> + ret = relinquish_p2m_mapping(d);
> + if ( ret )
> + return ret;
> +
> d->arch.relmem = RELMEM_done;
> /* Fallthrough */
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 410acb6..3f4ab37 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -6,6 +6,8 @@
> #include <xen/bitops.h>
> #include <asm/flushtlb.h>
> #include <asm/gic.h>
> +#include <asm/event.h>
> +#include <asm/hardirq.h>
>
> /* First level P2M is 2 consecutive pages */
> #define P2M_FIRST_ORDER 1
> @@ -224,7 +226,8 @@ static int p2m_create_table(struct domain *d,
> enum p2m_operation {
> INSERT,
> ALLOCATE,
> - REMOVE
> + REMOVE,
> + RELINQUISH,
> };
>
> static int create_p2m_entries(struct domain *d,
> @@ -242,6 +245,7 @@ static int create_p2m_entries(struct domain *d,
> unsigned long cur_first_page = ~0,
> cur_first_offset = ~0,
> cur_second_offset = ~0;
> + unsigned long count = 0;
>
> spin_lock(&p2m->lock);
>
> @@ -326,13 +330,19 @@ static int create_p2m_entries(struct domain *d,
> maddr += PAGE_SIZE;
> }
> break;
> + case RELINQUISH:
> case REMOVE:
> {
> lpae_t pte = third[third_table_offset(addr)];
> unsigned long mfn = pte.p2m.base;
>
> if ( !pte.p2m.valid )
> + {
> + count++;
> break;
> + }
> +
> + count += 0x10;
>
> /* TODO: Handle other p2m type */
> if ( p2m_is_foreign(pte.p2m.type) )
> @@ -343,12 +353,35 @@ static int create_p2m_entries(struct domain *d,
>
> memset(&pte, 0x00, sizeof(pte));
> write_pte(&third[third_table_offset(addr)], pte);
> + count++;
> }
> break;
> }
>
> if ( flush )
> flush_tlb_all_local();
> +
> + /* Preempt every 2MiB (mapped) or 32 MiB (unmapped) - arbitrary */
> + if ( op == RELINQUISH && count >= 0x2000 )
> + {
> + if ( hypercall_preempt_check() )
> + {
> + p2m->next_gfn_to_relinquish = maddr >> PAGE_SHIFT;
> + rc = -EAGAIN;
> + goto out;
> + }
> + count = 0;
> + }
> + }
> +
> + if ( op == ALLOCATE || op == INSERT )
> + {
> + unsigned long sgfn = paddr_to_pfn(start_gpaddr);
> + unsigned long egfn = paddr_to_pfn(end_gpaddr);
> +
> + p2m->max_mapped_gfn = MAX(p2m->max_mapped_gfn, egfn);
> + /* Use next_gfn_to_relinquish to store the lowest gfn mapped */
> + p2m->next_gfn_to_relinquish = MIN(p2m->next_gfn_to_relinquish, sgfn);
> }
>
> rc = 0;
> @@ -534,12 +567,26 @@ int p2m_init(struct domain *d)
>
> p2m->first_level = NULL;
>
> + p2m->max_mapped_gfn = 0;
> + p2m->next_gfn_to_relinquish = ULONG_MAX;
> +
> err:
> spin_unlock(&p2m->lock);
>
> return rc;
> }
>
> +int relinquish_p2m_mapping(struct domain *d)
> +{
> + struct p2m_domain *p2m = &d->arch.p2m;
> +
> + return create_p2m_entries(d, RELINQUISH,
> + pfn_to_paddr(p2m->next_gfn_to_relinquish),
> + pfn_to_paddr(p2m->max_mapped_gfn),
> + pfn_to_paddr(INVALID_MFN),
> + MATTR_MEM, p2m_invalid);
> +}
> +
> unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
> {
> paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn), NULL);
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 53c9895..28d39a0 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -112,6 +112,7 @@ struct arch_domain
> RELMEM_not_started,
> RELMEM_xen,
> RELMEM_page,
> + RELMEM_mapping,
> RELMEM_done,
> } relmem;
>
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 5ccfa7f..8b7b6d0 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -18,6 +18,15 @@ struct p2m_domain {
>
> /* Current VMID in use */
> uint8_t vmid;
> +
> + /* Highest guest frame that's ever been mapped in the p2m
> + * Only takes into account ram and foreign mapping
> + */
> + unsigned long max_mapped_gfn;
> +
> + /* When releasing mapped gfn's in a preemptible manner, recall where
> + * to resume the search */
> + unsigned long next_gfn_to_relinquish;
> };
>
> /* List of possible type for each page in the p2m entry.
> @@ -48,6 +57,12 @@ int p2m_init(struct domain *d);
> /* Return all the p2m resources to Xen. */
> void p2m_teardown(struct domain *d);
>
> +/* Remove mapping refcount on each mapping page in the p2m
> + *
> + * TODO: For the moment only foreign mappings are handled
> + */
> +int relinquish_p2m_mapping(struct domain *d);
> +
> /* Allocate a new p2m table for a domain.
> *
> * Returns 0 for success or -errno.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-17 16:27 ` [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page Julien Grall
2013-12-17 16:36 ` Ian Campbell
2013-12-18 14:20 ` Ian Campbell
@ 2013-12-18 16:48 ` Julien Grall
2013-12-18 16:50 ` Ian Campbell
2 siblings, 1 reply; 23+ messages in thread
From: Julien Grall @ 2013-12-18 16:48 UTC (permalink / raw)
To: ian.campbell; +Cc: xen-devel, Julien Grall, ian.jackson, patches
On 12/17/2013 04:27 PM, Julien Grall wrote:
> @@ -343,12 +353,35 @@ static int create_p2m_entries(struct domain *d,
>
> memset(&pte, 0x00, sizeof(pte));
> write_pte(&third[third_table_offset(addr)], pte);
> + count++;
> }
> break;
> }
>
> if ( flush )
> flush_tlb_all_local();
> +
> + /* Preempt every 2MiB (mapped) or 32 MiB (unmapped) - arbitrary */
> + if ( op == RELINQUISH && count >= 0x2000 )
> + {
> + if ( hypercall_preempt_check() )
> + {
> + p2m->next_gfn_to_relinquish = maddr >> PAGE_SHIFT;
Oops, I made a mistake in this patch.
It should be addr instead of maddr.
As it's already in staging, I will send a fix right now.
--
Julien Grall
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page
2013-12-18 16:48 ` Julien Grall
@ 2013-12-18 16:50 ` Ian Campbell
0 siblings, 0 replies; 23+ messages in thread
From: Ian Campbell @ 2013-12-18 16:50 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, patches
On Wed, 2013-12-18 at 16:48 +0000, Julien Grall wrote:
> On 12/17/2013 04:27 PM, Julien Grall wrote:
> > @@ -343,12 +353,35 @@ static int create_p2m_entries(struct domain *d,
> >
> > memset(&pte, 0x00, sizeof(pte));
> > write_pte(&third[third_table_offset(addr)], pte);
> > + count++;
> > }
> > break;
> > }
> >
> > if ( flush )
> > flush_tlb_all_local();
> > +
> > + /* Preempt every 2MiB (mapped) or 32 MiB (unmapped) - arbitrary */
> > + if ( op == RELINQUISH && count >= 0x2000 )
> > + {
> > + if ( hypercall_preempt_check() )
> > + {
> > + p2m->next_gfn_to_relinquish = maddr >> PAGE_SHIFT;
>
> Oops, I made a mistake in this patch.
>
> It should be addr instead of maddr.
>
> As it's already in staging, I will send a fix right now.
Yes please!
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 09/10] xen/arm: Set foreign page type to p2m_map_foreign
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (7 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-17 16:27 ` [PATCH v6 10/10] xen/arm: grant-table: Support read-only mapping Julien Grall
2013-12-18 14:41 ` [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Ian Campbell
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Xen needs to know that the current page belongs to another domain. Also take
a reference to this page.
The current process to add a foreign page is:
1) get the page from the foreign p2m
2) take a reference on the page with the foreign domain in parameters
3) add the page to the current domain p2m
If the foreign domain drops the page:
- before 2), get_page will return NULL because the page doesn't
belong anymore to the domain
- after 2), the current domain already have a reference. Write will
occur to an old page which is not yet released. It can corrupt the foreign
domain.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v5:
- Missing rcu_unlock_domain in one path
- Check if the foreign page is RAM
Changes in v4:
- Typo s/release/released/
- Improve commit message
Changes in v3:
- Typoes
- Check if the foreign domain is different from the current domain
Changes in v2:
- Even if gcc is buggy (see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501)
define p2m type per mapspace to let the compiler warns about unitialized
values.
---
xen/arch/arm/mm.c | 38 ++++++++++++++++++++++++++++++--------
xen/include/asm-arm/p2m.h | 1 +
2 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b1222be..6829822 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -977,6 +977,7 @@ static int xenmem_add_to_physmap_one(
{
unsigned long mfn = 0;
int rc;
+ p2m_type_t t;
switch ( space )
{
@@ -1009,22 +1010,33 @@ static int xenmem_add_to_physmap_one(
d->arch.grant_table_gpfn[idx] = gpfn;
+ t = p2m_ram_rw;
+
spin_unlock(&d->grant_table->lock);
break;
case XENMAPSPACE_shared_info:
- if ( idx == 0 )
- mfn = virt_to_mfn(d->shared_info);
- else
+ if ( idx != 0 )
return -EINVAL;
+
+ mfn = virt_to_mfn(d->shared_info);
+ t = p2m_ram_rw;
+
break;
case XENMAPSPACE_gmfn_foreign:
{
- paddr_t maddr;
struct domain *od;
+ struct page_info *page;
+ p2m_type_t p2mt;
od = rcu_lock_domain_by_any_id(foreign_domid);
if ( od == NULL )
return -ESRCH;
+ if ( od == d )
+ {
+ rcu_unlock_domain(od);
+ return -EINVAL;
+ }
+
rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
if ( rc )
{
@@ -1032,15 +1044,25 @@ static int xenmem_add_to_physmap_one(
return rc;
}
- maddr = p2m_lookup(od, pfn_to_paddr(idx), NULL);
- if ( maddr == INVALID_PADDR )
+ /* Take reference to the foreign domain page.
+ * Reference will be released in XENMEM_remove_from_physmap */
+ page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC);
+ if ( !page )
{
dump_p2m_lookup(od, pfn_to_paddr(idx));
rcu_unlock_domain(od);
return -EINVAL;
}
- mfn = maddr >> PAGE_SHIFT;
+ if ( !p2m_is_ram(p2mt) )
+ {
+ put_page(page);
+ rcu_unlock_domain(od);
+ return -EINVAL;
+ }
+
+ mfn = page_to_mfn(page);
+ t = p2m_map_foreign;
rcu_unlock_domain(od);
break;
@@ -1051,7 +1073,7 @@ static int xenmem_add_to_physmap_one(
}
/* Map at new location. */
- rc = guest_physmap_add_page(d, gpfn, mfn, 0);
+ rc = guest_physmap_add_entry(d, gpfn, mfn, 0, t);
return rc;
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 8b7b6d0..53b3266 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -47,6 +47,7 @@ typedef enum {
} p2m_type_t;
#define p2m_is_foreign(_t) ((_t) == p2m_map_foreign)
+#define p2m_is_ram(_t) ((_t) == p2m_ram_rw || (_t) == p2m_ram_ro)
/* Initialise vmid allocator */
void p2m_vmid_allocator_init(void);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v6 10/10] xen/arm: grant-table: Support read-only mapping
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (8 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 09/10] xen/arm: Set foreign page type to p2m_map_foreign Julien Grall
@ 2013-12-17 16:27 ` Julien Grall
2013-12-18 14:41 ` [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Ian Campbell
10 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2013-12-17 16:27 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, patches
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v2:
- Use p2m grant type to map grant-table mapping
---
xen/arch/arm/mm.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6829822..b1d3c60 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1307,19 +1307,17 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame,
unsigned int flags, unsigned int cache_flags)
{
int rc;
+ p2m_type_t t = p2m_grant_map_rw;
if ( cache_flags || (flags & ~GNTMAP_readonly) != GNTMAP_host_map )
return GNTST_general_error;
- /* XXX: read only mappings */
if ( flags & GNTMAP_readonly )
- {
- gdprintk(XENLOG_WARNING, "read only mappings not implemented yet\n");
- return GNTST_general_error;
- }
+ t = p2m_grant_map_ro;
+
+ rc = guest_physmap_add_entry(current->domain, addr >> PAGE_SHIFT,
+ frame, 0, t);
- rc = guest_physmap_add_page(current->domain,
- addr >> PAGE_SHIFT, frame, 0);
if ( rc )
return GNTST_general_error;
else
--
1.7.10.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping
2013-12-17 16:27 [PATCH v6 00/10] xen/arm: Handle correctly foreign mapping Julien Grall
` (9 preceding siblings ...)
2013-12-17 16:27 ` [PATCH v6 10/10] xen/arm: grant-table: Support read-only mapping Julien Grall
@ 2013-12-18 14:41 ` Ian Campbell
10 siblings, 0 replies; 23+ messages in thread
From: Ian Campbell @ 2013-12-18 14:41 UTC (permalink / raw)
To: Julien Grall; +Cc: George Dunlap, xen-devel, ian.jackson, patches
On Tue, 2013-12-17 at 16:27 +0000, Julien Grall wrote:
> Hello,
>
> This patch series aims to fix "Failed to unmap" message in dom0 when a guest is
> creating. Without this patch series, dom0 will leak memory each time a domain
> is created. It should be considered as a blocker for Xen 4.4 release.
>
> - Patch #1-2: prepare work for the others patches
> - Patch #3-6: add support for p2m type
> - Patch #7-9: handle correctly foreign mapping.
> - Patch #10: it's not really part of this series. It adds support
> for read-only grant-mapping
>
> For all the changes, see in each patch.
>
> Sincerely yours,
> Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
Applied.
Please can you fixup the relinquish loop ASAP
^ permalink raw reply [flat|nested] 23+ messages in thread