* [PATCH 00/12] xen/arm: Provide a generic function to update Xen PT
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Hi all,
This is the third part of the boot/memory rework for Xen on Arm. At the
moment, the update to Xen PT is scattered all around mm.c. This makes
difficult to rework Xen memory layout or even ensuring we are following the
Arm Arm properly (and we are not so far!).
This part contains code to provide a generic function to update Xen PT.
While I could have started from scratch, I decided to base the new function
on create_xen_entries() (now renamed xen_pt_update()). This makes slightly
easier to follow the changes.
In this series, the new generic function will only support 3rd-level update
and cannot be used in early boot (i.e because xenheap is initialized).
This will be extended in follow-up patch to allow more use within mm.c.
There are probably some optimization possible around the TLBs flush. I haven't
looked at it so far.
The last two patches of this series is to show how existing callers can be
converted. There are more conversion to come in follow-up series.
Cheers,
Julien Grall (12):
xen/arm: lpae: Add a macro to generate offsets from an address
xen/arm: mm: Rename create_xen_entries() to xen_pt_update()
xen/arm: mm: Move out of xen_pt_update() the logic to update an entry
xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
xen/arm: mm: Only increment mfn when valid in xen_pt_update
xen/arm: mm: Sanity check any update of Xen page tables
xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
xen/arm: mm: Remove enum xenmap_operation
xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
xen/arm: mm: Rework Xen page-tables walk during update
xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
xen/arm: mm: Remove set_pte_flags_on_range()
xen/arch/arm/mm.c | 421 ++++++++++++++++++++++++++++++---------------
xen/arch/arm/p2m.c | 23 +--
xen/include/asm-arm/lpae.h | 9 +
xen/include/asm-arm/page.h | 9 +-
4 files changed, 305 insertions(+), 157 deletions(-)
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 00/12] xen/arm: Provide a generic function to update Xen PT
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Hi all,
This is the third part of the boot/memory rework for Xen on Arm. At the
moment, the update to Xen PT is scattered all around mm.c. This makes
difficult to rework Xen memory layout or even ensuring we are following the
Arm Arm properly (and we are not so far!).
This part contains code to provide a generic function to update Xen PT.
While I could have started from scratch, I decided to base the new function
on create_xen_entries() (now renamed xen_pt_update()). This makes slightly
easier to follow the changes.
In this series, the new generic function will only support 3rd-level update
and cannot be used in early boot (i.e because xenheap is initialized).
This will be extended in follow-up patch to allow more use within mm.c.
There are probably some optimization possible around the TLBs flush. I haven't
looked at it so far.
The last two patches of this series is to show how existing callers can be
converted. There are more conversion to come in follow-up series.
Cheers,
Julien Grall (12):
xen/arm: lpae: Add a macro to generate offsets from an address
xen/arm: mm: Rename create_xen_entries() to xen_pt_update()
xen/arm: mm: Move out of xen_pt_update() the logic to update an entry
xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
xen/arm: mm: Only increment mfn when valid in xen_pt_update
xen/arm: mm: Sanity check any update of Xen page tables
xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
xen/arm: mm: Remove enum xenmap_operation
xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
xen/arm: mm: Rework Xen page-tables walk during update
xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
xen/arm: mm: Remove set_pte_flags_on_range()
xen/arch/arm/mm.c | 421 ++++++++++++++++++++++++++++++---------------
xen/arch/arm/p2m.c | 23 +--
xen/include/asm-arm/lpae.h | 9 +
xen/include/asm-arm/page.h | 9 +-
4 files changed, 305 insertions(+), 157 deletions(-)
--
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] 54+ messages in thread
* [PATCH 01/12] xen/arm: lpae: Add a macro to generate offsets from an address
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
There are few places requiring to generate offsets from an address.
Rather than open-coding everywhere, we can introduce a macro to do the
job for us.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/p2m.c | 23 +++--------------------
xen/include/asm-arm/lpae.h | 9 +++++++++
2 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 92c2413f20..06fa342a8f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -368,12 +368,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
p2m_type_t _t;
/* Convenience aliases */
- const unsigned int offsets[4] = {
- zeroeth_table_offset(addr),
- first_table_offset(addr),
- second_table_offset(addr),
- third_table_offset(addr)
- };
+ DECLARE_OFFSETS(offsets, addr);
ASSERT(p2m_is_locked(p2m));
BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
@@ -888,7 +883,6 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
p2m_type_t t,
p2m_access_t a)
{
- paddr_t addr = gfn_to_gaddr(sgfn);
unsigned int level = 0;
unsigned int target = 3 - (page_order / LPAE_SHIFT);
lpae_t *entry, *table, orig_pte;
@@ -897,12 +891,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
bool removing_mapping = mfn_eq(smfn, INVALID_MFN);
/* Convenience aliases */
- const unsigned int offsets[4] = {
- zeroeth_table_offset(addr),
- first_table_offset(addr),
- second_table_offset(addr),
- third_table_offset(addr)
- };
+ DECLARE_OFFSETS(offsets, gfn_to_gaddr(sgfn));
ASSERT(p2m_is_write_locked(p2m));
@@ -1199,15 +1188,9 @@ bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn)
unsigned int level = 0;
bool resolved = false;
lpae_t entry, *table;
- paddr_t addr = gfn_to_gaddr(gfn);
/* Convenience aliases */
- const unsigned int offsets[4] = {
- zeroeth_table_offset(addr),
- first_table_offset(addr),
- second_table_offset(addr),
- third_table_offset(addr)
- };
+ DECLARE_OFFSETS(offsets, gfn_to_gaddr(gfn));
p2m_write_lock(p2m);
diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index 545b0c8f24..c22780f8f3 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -218,6 +218,15 @@ TABLE_OFFSET_HELPERS(64);
#undef TABLE_OFFSET
#undef TABLE_OFFSET_HELPERS
+/* Generate an array @var containing the offset for each level from @addr */
+#define DECLARE_OFFSETS(var, addr) \
+ const unsigned int var[4] = { \
+ zeroeth_table_offset(addr), \
+ first_table_offset(addr), \
+ second_table_offset(addr), \
+ third_table_offset(addr) \
+ }
+
#endif /* __ASSEMBLY__ */
/*
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 01/12] xen/arm: lpae: Add a macro to generate offsets from an address
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
There are few places requiring to generate offsets from an address.
Rather than open-coding everywhere, we can introduce a macro to do the
job for us.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/p2m.c | 23 +++--------------------
xen/include/asm-arm/lpae.h | 9 +++++++++
2 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 92c2413f20..06fa342a8f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -368,12 +368,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
p2m_type_t _t;
/* Convenience aliases */
- const unsigned int offsets[4] = {
- zeroeth_table_offset(addr),
- first_table_offset(addr),
- second_table_offset(addr),
- third_table_offset(addr)
- };
+ DECLARE_OFFSETS(offsets, addr);
ASSERT(p2m_is_locked(p2m));
BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
@@ -888,7 +883,6 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
p2m_type_t t,
p2m_access_t a)
{
- paddr_t addr = gfn_to_gaddr(sgfn);
unsigned int level = 0;
unsigned int target = 3 - (page_order / LPAE_SHIFT);
lpae_t *entry, *table, orig_pte;
@@ -897,12 +891,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
bool removing_mapping = mfn_eq(smfn, INVALID_MFN);
/* Convenience aliases */
- const unsigned int offsets[4] = {
- zeroeth_table_offset(addr),
- first_table_offset(addr),
- second_table_offset(addr),
- third_table_offset(addr)
- };
+ DECLARE_OFFSETS(offsets, gfn_to_gaddr(sgfn));
ASSERT(p2m_is_write_locked(p2m));
@@ -1199,15 +1188,9 @@ bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn)
unsigned int level = 0;
bool resolved = false;
lpae_t entry, *table;
- paddr_t addr = gfn_to_gaddr(gfn);
/* Convenience aliases */
- const unsigned int offsets[4] = {
- zeroeth_table_offset(addr),
- first_table_offset(addr),
- second_table_offset(addr),
- third_table_offset(addr)
- };
+ DECLARE_OFFSETS(offsets, gfn_to_gaddr(gfn));
p2m_write_lock(p2m);
diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index 545b0c8f24..c22780f8f3 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -218,6 +218,15 @@ TABLE_OFFSET_HELPERS(64);
#undef TABLE_OFFSET
#undef TABLE_OFFSET_HELPERS
+/* Generate an array @var containing the offset for each level from @addr */
+#define DECLARE_OFFSETS(var, addr) \
+ const unsigned int var[4] = { \
+ zeroeth_table_offset(addr), \
+ first_table_offset(addr), \
+ second_table_offset(addr), \
+ third_table_offset(addr) \
+ }
+
#endif /* __ASSEMBLY__ */
/*
--
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] 54+ messages in thread
* [PATCH 02/12] xen/arm: mm: Rename create_xen_entries() to xen_pt_update()
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
create_xen_entries() is doing more than creating entries. It can also
modify and remove entries.
Rename the function to make clear what the function is actually doing.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d6157c35d6..df2ec3a36b 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -965,11 +965,11 @@ enum xenmap_operation {
static DEFINE_SPINLOCK(xen_pt_lock);
-static int create_xen_entries(enum xenmap_operation op,
- unsigned long virt,
- mfn_t mfn,
- unsigned long nr_mfns,
- unsigned int flags)
+static int xen_pt_update(enum xenmap_operation op,
+ unsigned long virt,
+ mfn_t mfn,
+ unsigned long nr_mfns,
+ unsigned int flags)
{
int rc = 0;
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
@@ -1062,25 +1062,24 @@ int map_pages_to_xen(unsigned long virt,
unsigned long nr_mfns,
unsigned int flags)
{
- return create_xen_entries(INSERT, virt, mfn, nr_mfns, flags);
+ return xen_pt_update(INSERT, virt, mfn, nr_mfns, flags);
}
int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
{
- return create_xen_entries(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
+ return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
}
int destroy_xen_mappings(unsigned long v, unsigned long e)
{
ASSERT(v <= e);
- return create_xen_entries(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
+ return xen_pt_update(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
}
int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
{
ASSERT(s <= e);
- return create_xen_entries(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT,
- flags);
+ return xen_pt_update(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
}
enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 02/12] xen/arm: mm: Rename create_xen_entries() to xen_pt_update()
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
create_xen_entries() is doing more than creating entries. It can also
modify and remove entries.
Rename the function to make clear what the function is actually doing.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d6157c35d6..df2ec3a36b 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -965,11 +965,11 @@ enum xenmap_operation {
static DEFINE_SPINLOCK(xen_pt_lock);
-static int create_xen_entries(enum xenmap_operation op,
- unsigned long virt,
- mfn_t mfn,
- unsigned long nr_mfns,
- unsigned int flags)
+static int xen_pt_update(enum xenmap_operation op,
+ unsigned long virt,
+ mfn_t mfn,
+ unsigned long nr_mfns,
+ unsigned int flags)
{
int rc = 0;
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
@@ -1062,25 +1062,24 @@ int map_pages_to_xen(unsigned long virt,
unsigned long nr_mfns,
unsigned int flags)
{
- return create_xen_entries(INSERT, virt, mfn, nr_mfns, flags);
+ return xen_pt_update(INSERT, virt, mfn, nr_mfns, flags);
}
int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
{
- return create_xen_entries(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
+ return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
}
int destroy_xen_mappings(unsigned long v, unsigned long e)
{
ASSERT(v <= e);
- return create_xen_entries(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
+ return xen_pt_update(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
}
int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
{
ASSERT(s <= e);
- return create_xen_entries(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT,
- flags);
+ return xen_pt_update(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
}
enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
--
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] 54+ messages in thread
* [PATCH 03/12] xen/arm: mm: Move out of xen_pt_update() the logic to update an entry
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
In preparation of rework of the Xen PT, the logic to update an entry
in moved out in a separate function.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 140 +++++++++++++++++++++++++++++-------------------------
1 file changed, 74 insertions(+), 66 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index df2ec3a36b..6b1d41cfba 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -963,6 +963,76 @@ enum xenmap_operation {
RESERVE
};
+static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
+ mfn_t mfn, unsigned int flags)
+{
+ lpae_t pte, *entry;
+ lpae_t *third = NULL;
+
+ entry = &xen_second[second_linear_offset(addr)];
+ if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
+ {
+ int rc = create_xen_table(entry);
+ if ( rc < 0 ) {
+ printk("%s: L2 failed\n", __func__);
+ return rc;
+ }
+ }
+
+ BUG_ON(!lpae_is_valid(*entry));
+
+ third = mfn_to_virt(lpae_get_mfn(*entry));
+ entry = &third[third_table_offset(addr)];
+
+ switch ( op ) {
+ case INSERT:
+ case RESERVE:
+ if ( lpae_is_valid(*entry) )
+ {
+ printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
+ __func__, addr, mfn_x(mfn));
+ return -EINVAL;
+ }
+ if ( op == RESERVE )
+ break;
+ pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
+ pte.pt.ro = PAGE_RO_MASK(flags);
+ pte.pt.xn = PAGE_XN_MASK(flags);
+ BUG_ON(!pte.pt.ro && !pte.pt.xn);
+ pte.pt.table = 1;
+ write_pte(entry, pte);
+ break;
+ case MODIFY:
+ case REMOVE:
+ if ( !lpae_is_valid(*entry) )
+ {
+ printk("%s: trying to %s a non-existing mapping addr=%lx\n",
+ __func__, op == REMOVE ? "remove" : "modify", addr);
+ return -EINVAL;
+ }
+ if ( op == REMOVE )
+ pte.bits = 0;
+ else
+ {
+ pte = *entry;
+ pte.pt.ro = PAGE_RO_MASK(flags);
+ pte.pt.xn = PAGE_XN_MASK(flags);
+ if ( !pte.pt.ro && !pte.pt.xn )
+ {
+ printk("%s: Incorrect combination for addr=%lx\n",
+ __func__, addr);
+ return -EINVAL;
+ }
+ }
+ write_pte(entry, pte);
+ break;
+ default:
+ BUG();
+ }
+
+ return 0;
+}
+
static DEFINE_SPINLOCK(xen_pt_lock);
static int xen_pt_update(enum xenmap_operation op,
@@ -973,78 +1043,16 @@ static int xen_pt_update(enum xenmap_operation op,
{
int rc = 0;
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
- lpae_t pte, *entry;
- lpae_t *third = NULL;
spin_lock(&xen_pt_lock);
for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
{
- entry = &xen_second[second_linear_offset(addr)];
- if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
- {
- rc = create_xen_table(entry);
- if ( rc < 0 ) {
- printk("%s: L2 failed\n", __func__);
- goto out;
- }
- }
-
- BUG_ON(!lpae_is_valid(*entry));
-
- third = mfn_to_virt(lpae_get_mfn(*entry));
- entry = &third[third_table_offset(addr)];
-
- switch ( op ) {
- case INSERT:
- case RESERVE:
- if ( lpae_is_valid(*entry) )
- {
- printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
- __func__, addr, mfn_x(mfn));
- rc = -EINVAL;
- goto out;
- }
- if ( op == RESERVE )
- break;
- pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- BUG_ON(!pte.pt.ro && !pte.pt.xn);
- pte.pt.table = 1;
- write_pte(entry, pte);
- break;
- case MODIFY:
- case REMOVE:
- if ( !lpae_is_valid(*entry) )
- {
- printk("%s: trying to %s a non-existing mapping addr=%lx\n",
- __func__, op == REMOVE ? "remove" : "modify", addr);
- rc = -EINVAL;
- goto out;
- }
- if ( op == REMOVE )
- pte.bits = 0;
- else
- {
- pte = *entry;
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- if ( !pte.pt.ro && !pte.pt.xn )
- {
- printk("%s: Incorrect combination for addr=%lx\n",
- __func__, addr);
- rc = -EINVAL;
- goto out;
- }
- }
- write_pte(entry, pte);
- break;
- default:
- BUG();
- }
+ rc = xen_pt_update_entry(op, addr, mfn, flags);
+ if ( rc )
+ break;
}
-out:
+
/*
* Flush the TLBs even in case of failure because we may have
* partially modified the PT. This will prevent any unexpected
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 03/12] xen/arm: mm: Move out of xen_pt_update() the logic to update an entry
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
In preparation of rework of the Xen PT, the logic to update an entry
in moved out in a separate function.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 140 +++++++++++++++++++++++++++++-------------------------
1 file changed, 74 insertions(+), 66 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index df2ec3a36b..6b1d41cfba 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -963,6 +963,76 @@ enum xenmap_operation {
RESERVE
};
+static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
+ mfn_t mfn, unsigned int flags)
+{
+ lpae_t pte, *entry;
+ lpae_t *third = NULL;
+
+ entry = &xen_second[second_linear_offset(addr)];
+ if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
+ {
+ int rc = create_xen_table(entry);
+ if ( rc < 0 ) {
+ printk("%s: L2 failed\n", __func__);
+ return rc;
+ }
+ }
+
+ BUG_ON(!lpae_is_valid(*entry));
+
+ third = mfn_to_virt(lpae_get_mfn(*entry));
+ entry = &third[third_table_offset(addr)];
+
+ switch ( op ) {
+ case INSERT:
+ case RESERVE:
+ if ( lpae_is_valid(*entry) )
+ {
+ printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
+ __func__, addr, mfn_x(mfn));
+ return -EINVAL;
+ }
+ if ( op == RESERVE )
+ break;
+ pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
+ pte.pt.ro = PAGE_RO_MASK(flags);
+ pte.pt.xn = PAGE_XN_MASK(flags);
+ BUG_ON(!pte.pt.ro && !pte.pt.xn);
+ pte.pt.table = 1;
+ write_pte(entry, pte);
+ break;
+ case MODIFY:
+ case REMOVE:
+ if ( !lpae_is_valid(*entry) )
+ {
+ printk("%s: trying to %s a non-existing mapping addr=%lx\n",
+ __func__, op == REMOVE ? "remove" : "modify", addr);
+ return -EINVAL;
+ }
+ if ( op == REMOVE )
+ pte.bits = 0;
+ else
+ {
+ pte = *entry;
+ pte.pt.ro = PAGE_RO_MASK(flags);
+ pte.pt.xn = PAGE_XN_MASK(flags);
+ if ( !pte.pt.ro && !pte.pt.xn )
+ {
+ printk("%s: Incorrect combination for addr=%lx\n",
+ __func__, addr);
+ return -EINVAL;
+ }
+ }
+ write_pte(entry, pte);
+ break;
+ default:
+ BUG();
+ }
+
+ return 0;
+}
+
static DEFINE_SPINLOCK(xen_pt_lock);
static int xen_pt_update(enum xenmap_operation op,
@@ -973,78 +1043,16 @@ static int xen_pt_update(enum xenmap_operation op,
{
int rc = 0;
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
- lpae_t pte, *entry;
- lpae_t *third = NULL;
spin_lock(&xen_pt_lock);
for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
{
- entry = &xen_second[second_linear_offset(addr)];
- if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
- {
- rc = create_xen_table(entry);
- if ( rc < 0 ) {
- printk("%s: L2 failed\n", __func__);
- goto out;
- }
- }
-
- BUG_ON(!lpae_is_valid(*entry));
-
- third = mfn_to_virt(lpae_get_mfn(*entry));
- entry = &third[third_table_offset(addr)];
-
- switch ( op ) {
- case INSERT:
- case RESERVE:
- if ( lpae_is_valid(*entry) )
- {
- printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
- __func__, addr, mfn_x(mfn));
- rc = -EINVAL;
- goto out;
- }
- if ( op == RESERVE )
- break;
- pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- BUG_ON(!pte.pt.ro && !pte.pt.xn);
- pte.pt.table = 1;
- write_pte(entry, pte);
- break;
- case MODIFY:
- case REMOVE:
- if ( !lpae_is_valid(*entry) )
- {
- printk("%s: trying to %s a non-existing mapping addr=%lx\n",
- __func__, op == REMOVE ? "remove" : "modify", addr);
- rc = -EINVAL;
- goto out;
- }
- if ( op == REMOVE )
- pte.bits = 0;
- else
- {
- pte = *entry;
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- if ( !pte.pt.ro && !pte.pt.xn )
- {
- printk("%s: Incorrect combination for addr=%lx\n",
- __func__, addr);
- rc = -EINVAL;
- goto out;
- }
- }
- write_pte(entry, pte);
- break;
- default:
- BUG();
- }
+ rc = xen_pt_update_entry(op, addr, mfn, flags);
+ if ( rc )
+ break;
}
-out:
+
/*
* Flush the TLBs even in case of failure because we may have
* partially modified the PT. This will prevent any unexpected
--
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] 54+ messages in thread
* [PATCH 04/12] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
At the moment, the flags are not enough to describe what kind of update
will done on the VA range. They need to be used in conjunction with the
enum xenmap_operation.
It would be more convenient to have all the information for the update
in a single place.
Two new flags are added to remove the relience on xenmap_operation:
- _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
- _PAGE_POPULATE: Indicate whether we only populate page-tables
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 2 +-
xen/include/asm-arm/page.h | 9 +++++++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6b1d41cfba..b61217abd0 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1075,7 +1075,7 @@ int map_pages_to_xen(unsigned long virt,
int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
{
- return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
+ return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
}
int destroy_xen_mappings(unsigned long v, unsigned long e)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 2bcdb0f1a5..caf2fac1ff 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -76,6 +76,8 @@
*
* [0:2] Memory Attribute Index
* [3:4] Permission flags
+ * [5] Present bit
+ * [6] Populate page table
*/
#define PAGE_AI_MASK(x) ((x) & 0x7U)
@@ -86,12 +88,15 @@
#define PAGE_XN_MASK(x) (((x) >> _PAGE_XN_BIT) & 0x1U)
#define PAGE_RO_MASK(x) (((x) >> _PAGE_RO_BIT) & 0x1U)
+#define _PAGE_PRESENT (1U << 5)
+#define _PAGE_POPULATE (1U << 6)
+
/*
* _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
* meant to be used outside of this header.
*/
-#define _PAGE_DEVICE _PAGE_XN
-#define _PAGE_NORMAL MT_NORMAL
+#define _PAGE_DEVICE (_PAGE_XN|_PAGE_PRESENT)
+#define _PAGE_NORMAL (MT_NORMAL|_PAGE_PRESENT)
#define PAGE_HYPERVISOR_RO (_PAGE_NORMAL|_PAGE_RO|_PAGE_XN)
#define PAGE_HYPERVISOR_RX (_PAGE_NORMAL|_PAGE_RO)
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 04/12] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
At the moment, the flags are not enough to describe what kind of update
will done on the VA range. They need to be used in conjunction with the
enum xenmap_operation.
It would be more convenient to have all the information for the update
in a single place.
Two new flags are added to remove the relience on xenmap_operation:
- _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
- _PAGE_POPULATE: Indicate whether we only populate page-tables
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 2 +-
xen/include/asm-arm/page.h | 9 +++++++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6b1d41cfba..b61217abd0 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1075,7 +1075,7 @@ int map_pages_to_xen(unsigned long virt,
int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
{
- return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, 0);
+ return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
}
int destroy_xen_mappings(unsigned long v, unsigned long e)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 2bcdb0f1a5..caf2fac1ff 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -76,6 +76,8 @@
*
* [0:2] Memory Attribute Index
* [3:4] Permission flags
+ * [5] Present bit
+ * [6] Populate page table
*/
#define PAGE_AI_MASK(x) ((x) & 0x7U)
@@ -86,12 +88,15 @@
#define PAGE_XN_MASK(x) (((x) >> _PAGE_XN_BIT) & 0x1U)
#define PAGE_RO_MASK(x) (((x) >> _PAGE_RO_BIT) & 0x1U)
+#define _PAGE_PRESENT (1U << 5)
+#define _PAGE_POPULATE (1U << 6)
+
/*
* _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
* meant to be used outside of this header.
*/
-#define _PAGE_DEVICE _PAGE_XN
-#define _PAGE_NORMAL MT_NORMAL
+#define _PAGE_DEVICE (_PAGE_XN|_PAGE_PRESENT)
+#define _PAGE_NORMAL (MT_NORMAL|_PAGE_PRESENT)
#define PAGE_HYPERVISOR_RO (_PAGE_NORMAL|_PAGE_RO|_PAGE_XN)
#define PAGE_HYPERVISOR_RX (_PAGE_NORMAL|_PAGE_RO)
--
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] 54+ messages in thread
* [PATCH 05/12] xen/arm: mm: Only increment mfn when valid in xen_pt_update
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Currently, the MFN will be incremented even if it is invalid. This will
result to have a valid MFN after the first iteration.
While this is not a major problem today, this will be in the future if
the code expect an invalid MFN at every iteration.
Such behavior is prevented by avoiding to increment an invalid MFN.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b61217abd0..eee7122c88 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1046,11 +1046,14 @@ static int xen_pt_update(enum xenmap_operation op,
spin_lock(&xen_pt_lock);
- for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
+ for( ; addr < addr_end; addr += PAGE_SIZE )
{
rc = xen_pt_update_entry(op, addr, mfn, flags);
if ( rc )
break;
+
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ mfn = mfn_add(mfn, 1);
}
/*
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 05/12] xen/arm: mm: Only increment mfn when valid in xen_pt_update
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Currently, the MFN will be incremented even if it is invalid. This will
result to have a valid MFN after the first iteration.
While this is not a major problem today, this will be in the future if
the code expect an invalid MFN at every iteration.
Such behavior is prevented by avoiding to increment an invalid MFN.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b61217abd0..eee7122c88 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1046,11 +1046,14 @@ static int xen_pt_update(enum xenmap_operation op,
spin_lock(&xen_pt_lock);
- for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1))
+ for( ; addr < addr_end; addr += PAGE_SIZE )
{
rc = xen_pt_update_entry(op, addr, mfn, flags);
if ( rc )
break;
+
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ mfn = mfn_add(mfn, 1);
}
/*
--
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] 54+ messages in thread
* [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
The code handling Xen PT update has quite a few restrictions on what it
can do. This is not a bad thing as it keeps the code simple.
There are already a few checks scattered in current page table handling.
However they are not sufficient as they could still allow to
modify/remove entry with contiguous bit set.
The checks are divided in two sets:
- per entry check: They are gathered in a new function that will
check whether an update is valid based on the flags passed and the
current value of an entry.
- global check: They are sanity check on xen_pt_update() parameters.
Additionally to contiguous check, we also now check that the caller is
not trying to modify the memory attributes of an entry.
Lastly, it was probably a bit over the top to forbid removing an
invalid mapping. This could just be ignored. The new behavior will be
helpful in future changes.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 97 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index eee7122c88..5eb6f47d74 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -50,6 +50,19 @@ struct domain *dom_xen, *dom_io, *dom_cow;
#undef mfn_to_virt
#define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
+#ifdef NDEBUG
+static inline void
+__attribute__ ((__format__ (__printf__, 1, 2)))
+mm_printk(const char *fmt, ...) {}
+#else
+#define mm_printk(fmt, args...) \
+ do \
+ { \
+ dprintk(XENLOG_ERR, fmt, ## args); \
+ WARN(); \
+ } while (0);
+#endif
+
/* Static start-of-day pagetables that we use before the allocators
* are up. These are used by all CPUs during bringup before switching
* to the CPUs own pagetables.
@@ -963,12 +976,74 @@ enum xenmap_operation {
RESERVE
};
+/* Sanity check of the entry */
+static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
+{
+ /* Sanity check when modifying a page. */
+ if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
+ {
+ /* We don't allow changing memory attributes. */
+ if ( entry.pt.ai != PAGE_AI_MASK(flags) )
+ {
+ mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n",
+ entry.pt.ai, PAGE_AI_MASK(flags));
+ return false;
+ }
+
+ /* We don't allow modifying entry with contiguous bit set. */
+ if ( entry.pt.contig )
+ {
+ mm_printk("Modifying entry with contiguous bit set is not allowed.\n");
+ return false;
+ }
+ }
+ /* Sanity check when inserting a page */
+ else if ( flags & _PAGE_PRESENT )
+ {
+ /* We should be here with a valid MFN. */
+ ASSERT(!mfn_eq(mfn, INVALID_MFN));
+
+ /* We don't allow replacing any valid entry. */
+ if ( lpae_is_valid(entry) )
+ {
+ mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
+ mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+ return false;
+ }
+ }
+ /* Sanity check when removing a page. */
+ else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 )
+ {
+ /* We should be here with an invalid MFN. */
+ ASSERT(mfn_eq(mfn, INVALID_MFN));
+
+ /* We don't allow removing page with contiguous bit set. */
+ if ( entry.pt.contig )
+ {
+ mm_printk("Removing entry with contiguous bit set is not allowed.\n");
+ return false;
+ }
+ }
+ /* Sanity check when populating the page-table. No check so far. */
+ else
+ {
+ ASSERT(!(flags & _PAGE_POPULATE));
+ /* We should be here with an invalid MFN */
+ ASSERT(mfn_eq(mfn, INVALID_MFN));
+ }
+
+ return true;
+}
+
static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
mfn_t mfn, unsigned int flags)
{
lpae_t pte, *entry;
lpae_t *third = NULL;
+ /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
+ ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
+
entry = &xen_second[second_linear_offset(addr)];
if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
{
@@ -984,15 +1059,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
third = mfn_to_virt(lpae_get_mfn(*entry));
entry = &third[third_table_offset(addr)];
+ if ( !xen_pt_check_entry(*entry, mfn, flags) )
+ return -EINVAL;
+
switch ( op ) {
case INSERT:
case RESERVE:
- if ( lpae_is_valid(*entry) )
- {
- printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
- __func__, addr, mfn_x(mfn));
- return -EINVAL;
- }
if ( op == RESERVE )
break;
pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
@@ -1004,12 +1076,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
break;
case MODIFY:
case REMOVE:
- if ( !lpae_is_valid(*entry) )
- {
- printk("%s: trying to %s a non-existing mapping addr=%lx\n",
- __func__, op == REMOVE ? "remove" : "modify", addr);
- return -EINVAL;
- }
if ( op == REMOVE )
pte.bits = 0;
else
@@ -1017,12 +1083,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
pte = *entry;
pte.pt.ro = PAGE_RO_MASK(flags);
pte.pt.xn = PAGE_XN_MASK(flags);
- if ( !pte.pt.ro && !pte.pt.xn )
- {
- printk("%s: Incorrect combination for addr=%lx\n",
- __func__, addr);
- return -EINVAL;
- }
}
write_pte(entry, pte);
break;
@@ -1044,6 +1104,25 @@ static int xen_pt_update(enum xenmap_operation op,
int rc = 0;
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
+ /*
+ * The hardware was configured to forbid mapping both writeable and
+ * executable.
+ * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
+ * prevent any update if this happen.
+ */
+ if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) &&
+ !PAGE_XN_MASK(flags) )
+ {
+ mm_printk("Mappings should not be both Writeable and Executable.\n");
+ return -EINVAL;
+ }
+
+ if ( !IS_ALIGNED(virt, PAGE_SIZE) )
+ {
+ mm_printk("The virtual address is not aligned to the page-size.\n");
+ return -EINVAL;
+ }
+
spin_lock(&xen_pt_lock);
for( ; addr < addr_end; addr += PAGE_SIZE )
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
The code handling Xen PT update has quite a few restrictions on what it
can do. This is not a bad thing as it keeps the code simple.
There are already a few checks scattered in current page table handling.
However they are not sufficient as they could still allow to
modify/remove entry with contiguous bit set.
The checks are divided in two sets:
- per entry check: They are gathered in a new function that will
check whether an update is valid based on the flags passed and the
current value of an entry.
- global check: They are sanity check on xen_pt_update() parameters.
Additionally to contiguous check, we also now check that the caller is
not trying to modify the memory attributes of an entry.
Lastly, it was probably a bit over the top to forbid removing an
invalid mapping. This could just be ignored. The new behavior will be
helpful in future changes.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 97 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index eee7122c88..5eb6f47d74 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -50,6 +50,19 @@ struct domain *dom_xen, *dom_io, *dom_cow;
#undef mfn_to_virt
#define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
+#ifdef NDEBUG
+static inline void
+__attribute__ ((__format__ (__printf__, 1, 2)))
+mm_printk(const char *fmt, ...) {}
+#else
+#define mm_printk(fmt, args...) \
+ do \
+ { \
+ dprintk(XENLOG_ERR, fmt, ## args); \
+ WARN(); \
+ } while (0);
+#endif
+
/* Static start-of-day pagetables that we use before the allocators
* are up. These are used by all CPUs during bringup before switching
* to the CPUs own pagetables.
@@ -963,12 +976,74 @@ enum xenmap_operation {
RESERVE
};
+/* Sanity check of the entry */
+static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
+{
+ /* Sanity check when modifying a page. */
+ if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
+ {
+ /* We don't allow changing memory attributes. */
+ if ( entry.pt.ai != PAGE_AI_MASK(flags) )
+ {
+ mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n",
+ entry.pt.ai, PAGE_AI_MASK(flags));
+ return false;
+ }
+
+ /* We don't allow modifying entry with contiguous bit set. */
+ if ( entry.pt.contig )
+ {
+ mm_printk("Modifying entry with contiguous bit set is not allowed.\n");
+ return false;
+ }
+ }
+ /* Sanity check when inserting a page */
+ else if ( flags & _PAGE_PRESENT )
+ {
+ /* We should be here with a valid MFN. */
+ ASSERT(!mfn_eq(mfn, INVALID_MFN));
+
+ /* We don't allow replacing any valid entry. */
+ if ( lpae_is_valid(entry) )
+ {
+ mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
+ mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+ return false;
+ }
+ }
+ /* Sanity check when removing a page. */
+ else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 )
+ {
+ /* We should be here with an invalid MFN. */
+ ASSERT(mfn_eq(mfn, INVALID_MFN));
+
+ /* We don't allow removing page with contiguous bit set. */
+ if ( entry.pt.contig )
+ {
+ mm_printk("Removing entry with contiguous bit set is not allowed.\n");
+ return false;
+ }
+ }
+ /* Sanity check when populating the page-table. No check so far. */
+ else
+ {
+ ASSERT(!(flags & _PAGE_POPULATE));
+ /* We should be here with an invalid MFN */
+ ASSERT(mfn_eq(mfn, INVALID_MFN));
+ }
+
+ return true;
+}
+
static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
mfn_t mfn, unsigned int flags)
{
lpae_t pte, *entry;
lpae_t *third = NULL;
+ /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
+ ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
+
entry = &xen_second[second_linear_offset(addr)];
if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
{
@@ -984,15 +1059,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
third = mfn_to_virt(lpae_get_mfn(*entry));
entry = &third[third_table_offset(addr)];
+ if ( !xen_pt_check_entry(*entry, mfn, flags) )
+ return -EINVAL;
+
switch ( op ) {
case INSERT:
case RESERVE:
- if ( lpae_is_valid(*entry) )
- {
- printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
- __func__, addr, mfn_x(mfn));
- return -EINVAL;
- }
if ( op == RESERVE )
break;
pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
@@ -1004,12 +1076,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
break;
case MODIFY:
case REMOVE:
- if ( !lpae_is_valid(*entry) )
- {
- printk("%s: trying to %s a non-existing mapping addr=%lx\n",
- __func__, op == REMOVE ? "remove" : "modify", addr);
- return -EINVAL;
- }
if ( op == REMOVE )
pte.bits = 0;
else
@@ -1017,12 +1083,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
pte = *entry;
pte.pt.ro = PAGE_RO_MASK(flags);
pte.pt.xn = PAGE_XN_MASK(flags);
- if ( !pte.pt.ro && !pte.pt.xn )
- {
- printk("%s: Incorrect combination for addr=%lx\n",
- __func__, addr);
- return -EINVAL;
- }
}
write_pte(entry, pte);
break;
@@ -1044,6 +1104,25 @@ static int xen_pt_update(enum xenmap_operation op,
int rc = 0;
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
+ /*
+ * The hardware was configured to forbid mapping both writeable and
+ * executable.
+ * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
+ * prevent any update if this happen.
+ */
+ if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) &&
+ !PAGE_XN_MASK(flags) )
+ {
+ mm_printk("Mappings should not be both Writeable and Executable.\n");
+ return -EINVAL;
+ }
+
+ if ( !IS_ALIGNED(virt, PAGE_SIZE) )
+ {
+ mm_printk("The virtual address is not aligned to the page-size.\n");
+ return -EINVAL;
+ }
+
spin_lock(&xen_pt_lock);
for( ; addr < addr_end; addr += PAGE_SIZE )
--
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] 54+ messages in thread
* [PATCH 07/12] xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
With the newly introduced flags, it is now possible to know how the page
will be updated through the flags.
All the use of xenmap_operation are not replaced with the flags. At the
same time, validity check are now removed as they are gathered in
xen_pt_check_entry().
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 47 +++++++++++++++++++++++------------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5eb6f47d74..611ea53992 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1062,34 +1062,33 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
if ( !xen_pt_check_entry(*entry, mfn, flags) )
return -EINVAL;
- switch ( op ) {
- case INSERT:
- case RESERVE:
- if ( op == RESERVE )
- break;
+ /* If we are only populating page-table, then we are done. */
+ if ( flags & _PAGE_POPULATE )
+ return 0;
+
+ /* We are removing the page */
+ if ( !(flags & _PAGE_PRESENT) )
+ memset(&pte, 0x00, sizeof(pte));
+ else
+ {
+ /* We are inserting a mapping => Create new pte. */
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ {
pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- BUG_ON(!pte.pt.ro && !pte.pt.xn);
+
+ /* Third level entries set pte.pt.table = 1 */
pte.pt.table = 1;
- write_pte(entry, pte);
- break;
- case MODIFY:
- case REMOVE:
- if ( op == REMOVE )
- pte.bits = 0;
- else
- {
- pte = *entry;
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- }
- write_pte(entry, pte);
- break;
- default:
- BUG();
+ }
+ else /* We are updating the permission => Copy the current pte. */
+ pte = *entry;
+
+ /* Set permission */
+ pte.pt.ro = PAGE_RO_MASK(flags);
+ pte.pt.xn = PAGE_XN_MASK(flags);
}
+ write_pte(entry, pte);
+
return 0;
}
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 07/12] xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
With the newly introduced flags, it is now possible to know how the page
will be updated through the flags.
All the use of xenmap_operation are not replaced with the flags. At the
same time, validity check are now removed as they are gathered in
xen_pt_check_entry().
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 47 +++++++++++++++++++++++------------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5eb6f47d74..611ea53992 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1062,34 +1062,33 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
if ( !xen_pt_check_entry(*entry, mfn, flags) )
return -EINVAL;
- switch ( op ) {
- case INSERT:
- case RESERVE:
- if ( op == RESERVE )
- break;
+ /* If we are only populating page-table, then we are done. */
+ if ( flags & _PAGE_POPULATE )
+ return 0;
+
+ /* We are removing the page */
+ if ( !(flags & _PAGE_PRESENT) )
+ memset(&pte, 0x00, sizeof(pte));
+ else
+ {
+ /* We are inserting a mapping => Create new pte. */
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ {
pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- BUG_ON(!pte.pt.ro && !pte.pt.xn);
+
+ /* Third level entries set pte.pt.table = 1 */
pte.pt.table = 1;
- write_pte(entry, pte);
- break;
- case MODIFY:
- case REMOVE:
- if ( op == REMOVE )
- pte.bits = 0;
- else
- {
- pte = *entry;
- pte.pt.ro = PAGE_RO_MASK(flags);
- pte.pt.xn = PAGE_XN_MASK(flags);
- }
- write_pte(entry, pte);
- break;
- default:
- BUG();
+ }
+ else /* We are updating the permission => Copy the current pte. */
+ pte = *entry;
+
+ /* Set permission */
+ pte.pt.ro = PAGE_RO_MASK(flags);
+ pte.pt.xn = PAGE_XN_MASK(flags);
}
+ write_pte(entry, pte);
+
return 0;
}
--
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] 54+ messages in thread
* [PATCH 08/12] xen/arm: mm: Remove enum xenmap_operation
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
The enum xenmap_operation is not used anymore. So remove it.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 611ea53992..97e876d866 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -969,13 +969,6 @@ static int create_xen_table(lpae_t *entry)
return 0;
}
-enum xenmap_operation {
- INSERT,
- REMOVE,
- MODIFY,
- RESERVE
-};
-
/* Sanity check of the entry */
static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
{
@@ -1035,8 +1028,8 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
return true;
}
-static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
- mfn_t mfn, unsigned int flags)
+static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
+ unsigned int flags)
{
lpae_t pte, *entry;
lpae_t *third = NULL;
@@ -1094,8 +1087,7 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
static DEFINE_SPINLOCK(xen_pt_lock);
-static int xen_pt_update(enum xenmap_operation op,
- unsigned long virt,
+static int xen_pt_update(unsigned long virt,
mfn_t mfn,
unsigned long nr_mfns,
unsigned int flags)
@@ -1126,7 +1118,7 @@ static int xen_pt_update(enum xenmap_operation op,
for( ; addr < addr_end; addr += PAGE_SIZE )
{
- rc = xen_pt_update_entry(op, addr, mfn, flags);
+ rc = xen_pt_update_entry(addr, mfn, flags);
if ( rc )
break;
@@ -1151,24 +1143,24 @@ int map_pages_to_xen(unsigned long virt,
unsigned long nr_mfns,
unsigned int flags)
{
- return xen_pt_update(INSERT, virt, mfn, nr_mfns, flags);
+ return xen_pt_update(virt, mfn, nr_mfns, flags);
}
int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
{
- return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
+ return xen_pt_update(virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
}
int destroy_xen_mappings(unsigned long v, unsigned long e)
{
ASSERT(v <= e);
- return xen_pt_update(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
+ return xen_pt_update(v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
}
int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
{
ASSERT(s <= e);
- return xen_pt_update(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
+ return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
}
enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 08/12] xen/arm: mm: Remove enum xenmap_operation
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
The enum xenmap_operation is not used anymore. So remove it.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 611ea53992..97e876d866 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -969,13 +969,6 @@ static int create_xen_table(lpae_t *entry)
return 0;
}
-enum xenmap_operation {
- INSERT,
- REMOVE,
- MODIFY,
- RESERVE
-};
-
/* Sanity check of the entry */
static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
{
@@ -1035,8 +1028,8 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
return true;
}
-static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
- mfn_t mfn, unsigned int flags)
+static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
+ unsigned int flags)
{
lpae_t pte, *entry;
lpae_t *third = NULL;
@@ -1094,8 +1087,7 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
static DEFINE_SPINLOCK(xen_pt_lock);
-static int xen_pt_update(enum xenmap_operation op,
- unsigned long virt,
+static int xen_pt_update(unsigned long virt,
mfn_t mfn,
unsigned long nr_mfns,
unsigned int flags)
@@ -1126,7 +1118,7 @@ static int xen_pt_update(enum xenmap_operation op,
for( ; addr < addr_end; addr += PAGE_SIZE )
{
- rc = xen_pt_update_entry(op, addr, mfn, flags);
+ rc = xen_pt_update_entry(addr, mfn, flags);
if ( rc )
break;
@@ -1151,24 +1143,24 @@ int map_pages_to_xen(unsigned long virt,
unsigned long nr_mfns,
unsigned int flags)
{
- return xen_pt_update(INSERT, virt, mfn, nr_mfns, flags);
+ return xen_pt_update(virt, mfn, nr_mfns, flags);
}
int populate_pt_range(unsigned long virt, unsigned long nr_mfns)
{
- return xen_pt_update(RESERVE, virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
+ return xen_pt_update(virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
}
int destroy_xen_mappings(unsigned long v, unsigned long e)
{
ASSERT(v <= e);
- return xen_pt_update(REMOVE, v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
+ return xen_pt_update(v, INVALID_MFN, (e - v) >> PAGE_SHIFT, 0);
}
int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
{
ASSERT(s <= e);
- return xen_pt_update(MODIFY, s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
+ return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
}
enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
--
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] 54+ messages in thread
* [PATCH 09/12] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Currently, the virtual address of the 3rd level page-tables is obtained
using mfn_to_virt().
On Arm32, mfn_to_virt can only work on xenheap page. While in practice
all the page-tables updated will reside in xenheap, in practive the
page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
Furthermore, a follow-up change will update xen_pt_update_entry() to
walk all the levels and therefore be more generic. Some of the
page-tables will also part of Xen memory and therefore will not be
reachable using mfn_to_virt().
The easiest way to reach those pages is to use {, un}map_domain_page().
While on arm32 this means an extra mapping in the normal cases, this is not
very important as xen page-tables are not updated often.
In order to allow future change in the way Xen page-tables are mapped,
two new helpers are introduced to map/unmap the page-tables.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 97e876d866..115e8340f1 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -969,6 +969,16 @@ static int create_xen_table(lpae_t *entry)
return 0;
}
+static lpae_t *xen_map_table(mfn_t mfn)
+{
+ return map_domain_page(mfn);
+}
+
+static void xen_unmap_table(const lpae_t *table)
+{
+ unmap_domain_page(table);
+}
+
/* Sanity check of the entry */
static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
{
@@ -1031,6 +1041,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
unsigned int flags)
{
+ int rc;
lpae_t pte, *entry;
lpae_t *third = NULL;
@@ -1049,15 +1060,17 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
BUG_ON(!lpae_is_valid(*entry));
- third = mfn_to_virt(lpae_get_mfn(*entry));
+ third = xen_map_table(lpae_get_mfn(*entry));
entry = &third[third_table_offset(addr)];
+ rc = -EINVAL;
if ( !xen_pt_check_entry(*entry, mfn, flags) )
- return -EINVAL;
+ goto out;
/* If we are only populating page-table, then we are done. */
+ rc = 0;
if ( flags & _PAGE_POPULATE )
- return 0;
+ goto out;
/* We are removing the page */
if ( !(flags & _PAGE_PRESENT) )
@@ -1082,7 +1095,12 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
write_pte(entry, pte);
- return 0;
+ rc = 0;
+
+out:
+ xen_unmap_table(third);
+
+ return rc;
}
static DEFINE_SPINLOCK(xen_pt_lock);
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 09/12] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Currently, the virtual address of the 3rd level page-tables is obtained
using mfn_to_virt().
On Arm32, mfn_to_virt can only work on xenheap page. While in practice
all the page-tables updated will reside in xenheap, in practive the
page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
Furthermore, a follow-up change will update xen_pt_update_entry() to
walk all the levels and therefore be more generic. Some of the
page-tables will also part of Xen memory and therefore will not be
reachable using mfn_to_virt().
The easiest way to reach those pages is to use {, un}map_domain_page().
While on arm32 this means an extra mapping in the normal cases, this is not
very important as xen page-tables are not updated often.
In order to allow future change in the way Xen page-tables are mapped,
two new helpers are introduced to map/unmap the page-tables.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 97e876d866..115e8340f1 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -969,6 +969,16 @@ static int create_xen_table(lpae_t *entry)
return 0;
}
+static lpae_t *xen_map_table(mfn_t mfn)
+{
+ return map_domain_page(mfn);
+}
+
+static void xen_unmap_table(const lpae_t *table)
+{
+ unmap_domain_page(table);
+}
+
/* Sanity check of the entry */
static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
{
@@ -1031,6 +1041,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
unsigned int flags)
{
+ int rc;
lpae_t pte, *entry;
lpae_t *third = NULL;
@@ -1049,15 +1060,17 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
BUG_ON(!lpae_is_valid(*entry));
- third = mfn_to_virt(lpae_get_mfn(*entry));
+ third = xen_map_table(lpae_get_mfn(*entry));
entry = &third[third_table_offset(addr)];
+ rc = -EINVAL;
if ( !xen_pt_check_entry(*entry, mfn, flags) )
- return -EINVAL;
+ goto out;
/* If we are only populating page-table, then we are done. */
+ rc = 0;
if ( flags & _PAGE_POPULATE )
- return 0;
+ goto out;
/* We are removing the page */
if ( !(flags & _PAGE_PRESENT) )
@@ -1082,7 +1095,12 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
write_pte(entry, pte);
- return 0;
+ rc = 0;
+
+out:
+ xen_unmap_table(third);
+
+ return rc;
}
static DEFINE_SPINLOCK(xen_pt_lock);
--
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] 54+ messages in thread
* [PATCH 10/12] xen/arm: mm: Rework Xen page-tables walk during update
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Currently, xen_pt_update_entry() is only able to update the region covered
by xen_second (i.e 0 to 0x7fffffff).
Because of the restriction we end to have multiple functions in mm.c
modifying the page-tables differently.
Furthermore, we never walked the page-tables fully. This means that any
change in the layout may requires major rewrite of the page-tables code.
Lastly, we have been quite lucky that no one ever tried to pass an address
outside this range because it would have blown-up.
xen_pt_update_entry() is reworked to walk over the page-tables every
time. The logic has been borrowed from arch/arm/p2m.c and contain some
limitations for the time being:
- Superpage cannot be shattered
- Only level 3 (i.e 4KB) can be done
Note that the parameter 'addr' has been renamed to 'virt' to make clear
we are dealing with a virtual address.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 106 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 115e8340f1..022967ff9f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -979,6 +979,53 @@ static void xen_unmap_table(const lpae_t *table)
unmap_domain_page(table);
}
+#define XEN_TABLE_MAP_FAILED 0
+#define XEN_TABLE_SUPER_PAGE 1
+#define XEN_TABLE_NORMAL_PAGE 2
+
+/*
+ * Take the currently mapped table, find the corresponding entry,
+ * and map the next table, if available.
+ *
+ * The read_only parameters indicates whether intermediate tables should
+ * be allocated when not present.
+ *
+ * Return values:
+ * XEN_TABLE_MAP_FAILED: Either read_only was set and the entry
+ * was empty, or allocating a new page failed.
+ * XEN_TABLE_NORMAL_PAGE: next level mapped normally
+ * XEN_TABLE_SUPER_PAGE: The next entry points to a superpage.
+ */
+static int xen_pt_next_level(bool read_only, unsigned int level,
+ lpae_t **table, unsigned int offset)
+{
+ lpae_t *entry;
+ int ret;
+
+ entry = *table + offset;
+
+ if ( !lpae_is_valid(*entry) )
+ {
+ if ( read_only )
+ return XEN_TABLE_MAP_FAILED;
+
+ ret = create_xen_table(entry);
+ if ( ret )
+ return XEN_TABLE_MAP_FAILED;
+ }
+
+ ASSERT(lpae_is_valid(*entry));
+
+ /* The function xen_pt_next_level is never called at the 3rd level */
+ if ( lpae_is_mapping(*entry, level) )
+ return XEN_TABLE_SUPER_PAGE;
+
+ xen_unmap_table(*table);
+ *table = xen_map_table(lpae_get_mfn(*entry));
+
+ return XEN_TABLE_NORMAL_PAGE;
+}
+
/* Sanity check of the entry */
static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
{
@@ -1038,30 +1085,65 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
return true;
}
-static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
- unsigned int flags)
+static int xen_pt_update_entry(mfn_t root, unsigned long virt,
+ mfn_t mfn, unsigned int flags)
{
int rc;
+ unsigned int level;
+ /* We only support 4KB mapping (i.e level 3) for now */
+ unsigned int target = 3;
+ lpae_t *table;
+ /*
+ * The intermediate page tables are read-only when the MFN is not valid
+ * and we are not populating page table.
+ * This means we either modify permissions or remove an entry.
+ */
+ bool read_only = mfn_eq(mfn, INVALID_MFN) && !(flags & _PAGE_POPULATE);
lpae_t pte, *entry;
- lpae_t *third = NULL;
+
+ /* convenience aliases */
+ DECLARE_OFFSETS(offsets, (paddr_t)virt);
/* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
- entry = &xen_second[second_linear_offset(addr)];
- if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
+ table = xen_map_table(root);
+ for ( level = HYP_PT_ROOT_LEVEL; level < target; level++ )
{
- int rc = create_xen_table(entry);
- if ( rc < 0 ) {
- printk("%s: L2 failed\n", __func__);
- return rc;
+ rc = xen_pt_next_level(read_only, level, &table, offsets[level]);
+ if ( rc == XEN_TABLE_MAP_FAILED )
+ {
+ /*
+ * We are here because xen_pt_next_level has failed to map
+ * the intermediate page table (e.g the table does not exist
+ * and the pt is read-only). It is a valid case when
+ * removing a mapping as it may not exist in the page table.
+ * In this case, just ignore it.
+ */
+ if ( flags & (_PAGE_PRESENT|_PAGE_POPULATE) )
+ {
+ mm_printk("%s: Unable to map level %u\n", __func__, level);
+ rc = -ENOENT;
+ goto out;
+ }
+ else
+ {
+ rc = 0;
+ goto out;
+ }
}
+ else if ( rc != XEN_TABLE_NORMAL_PAGE )
+ break;
}
- BUG_ON(!lpae_is_valid(*entry));
+ if ( level != target )
+ {
+ mm_printk("%s: Shattering superpage is not supported\n", __func__);
+ rc = -EOPNOTSUPP;
+ goto out;
+ }
- third = xen_map_table(lpae_get_mfn(*entry));
- entry = &third[third_table_offset(addr)];
+ entry = table + offsets[level];
rc = -EINVAL;
if ( !xen_pt_check_entry(*entry, mfn, flags) )
@@ -1098,7 +1180,7 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
rc = 0;
out:
- xen_unmap_table(third);
+ xen_unmap_table(table);
return rc;
}
@@ -1114,6 +1196,15 @@ static int xen_pt_update(unsigned long virt,
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
/*
+ * For arm32, page-tables are different on each CPUs. Yet, they share
+ * some common mappings. It is assumed that only common mappings
+ * will be modified with this function.
+ *
+ * XXX: Add a check.
+ */
+ const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
+
+ /*
* The hardware was configured to forbid mapping both writeable and
* executable.
* When modifying/creating mapping (i.e _PAGE_PRESENT is set),
@@ -1134,9 +1225,9 @@ static int xen_pt_update(unsigned long virt,
spin_lock(&xen_pt_lock);
- for( ; addr < addr_end; addr += PAGE_SIZE )
+ for ( ; addr < addr_end; addr += PAGE_SIZE )
{
- rc = xen_pt_update_entry(addr, mfn, flags);
+ rc = xen_pt_update_entry(root, addr, mfn, flags);
if ( rc )
break;
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 10/12] xen/arm: mm: Rework Xen page-tables walk during update
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
Currently, xen_pt_update_entry() is only able to update the region covered
by xen_second (i.e 0 to 0x7fffffff).
Because of the restriction we end to have multiple functions in mm.c
modifying the page-tables differently.
Furthermore, we never walked the page-tables fully. This means that any
change in the layout may requires major rewrite of the page-tables code.
Lastly, we have been quite lucky that no one ever tried to pass an address
outside this range because it would have blown-up.
xen_pt_update_entry() is reworked to walk over the page-tables every
time. The logic has been borrowed from arch/arm/p2m.c and contain some
limitations for the time being:
- Superpage cannot be shattered
- Only level 3 (i.e 4KB) can be done
Note that the parameter 'addr' has been renamed to 'virt' to make clear
we are dealing with a virtual address.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 106 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 115e8340f1..022967ff9f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -979,6 +979,53 @@ static void xen_unmap_table(const lpae_t *table)
unmap_domain_page(table);
}
+#define XEN_TABLE_MAP_FAILED 0
+#define XEN_TABLE_SUPER_PAGE 1
+#define XEN_TABLE_NORMAL_PAGE 2
+
+/*
+ * Take the currently mapped table, find the corresponding entry,
+ * and map the next table, if available.
+ *
+ * The read_only parameters indicates whether intermediate tables should
+ * be allocated when not present.
+ *
+ * Return values:
+ * XEN_TABLE_MAP_FAILED: Either read_only was set and the entry
+ * was empty, or allocating a new page failed.
+ * XEN_TABLE_NORMAL_PAGE: next level mapped normally
+ * XEN_TABLE_SUPER_PAGE: The next entry points to a superpage.
+ */
+static int xen_pt_next_level(bool read_only, unsigned int level,
+ lpae_t **table, unsigned int offset)
+{
+ lpae_t *entry;
+ int ret;
+
+ entry = *table + offset;
+
+ if ( !lpae_is_valid(*entry) )
+ {
+ if ( read_only )
+ return XEN_TABLE_MAP_FAILED;
+
+ ret = create_xen_table(entry);
+ if ( ret )
+ return XEN_TABLE_MAP_FAILED;
+ }
+
+ ASSERT(lpae_is_valid(*entry));
+
+ /* The function xen_pt_next_level is never called at the 3rd level */
+ if ( lpae_is_mapping(*entry, level) )
+ return XEN_TABLE_SUPER_PAGE;
+
+ xen_unmap_table(*table);
+ *table = xen_map_table(lpae_get_mfn(*entry));
+
+ return XEN_TABLE_NORMAL_PAGE;
+}
+
/* Sanity check of the entry */
static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
{
@@ -1038,30 +1085,65 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
return true;
}
-static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
- unsigned int flags)
+static int xen_pt_update_entry(mfn_t root, unsigned long virt,
+ mfn_t mfn, unsigned int flags)
{
int rc;
+ unsigned int level;
+ /* We only support 4KB mapping (i.e level 3) for now */
+ unsigned int target = 3;
+ lpae_t *table;
+ /*
+ * The intermediate page tables are read-only when the MFN is not valid
+ * and we are not populating page table.
+ * This means we either modify permissions or remove an entry.
+ */
+ bool read_only = mfn_eq(mfn, INVALID_MFN) && !(flags & _PAGE_POPULATE);
lpae_t pte, *entry;
- lpae_t *third = NULL;
+
+ /* convenience aliases */
+ DECLARE_OFFSETS(offsets, (paddr_t)virt);
/* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
- entry = &xen_second[second_linear_offset(addr)];
- if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
+ table = xen_map_table(root);
+ for ( level = HYP_PT_ROOT_LEVEL; level < target; level++ )
{
- int rc = create_xen_table(entry);
- if ( rc < 0 ) {
- printk("%s: L2 failed\n", __func__);
- return rc;
+ rc = xen_pt_next_level(read_only, level, &table, offsets[level]);
+ if ( rc == XEN_TABLE_MAP_FAILED )
+ {
+ /*
+ * We are here because xen_pt_next_level has failed to map
+ * the intermediate page table (e.g the table does not exist
+ * and the pt is read-only). It is a valid case when
+ * removing a mapping as it may not exist in the page table.
+ * In this case, just ignore it.
+ */
+ if ( flags & (_PAGE_PRESENT|_PAGE_POPULATE) )
+ {
+ mm_printk("%s: Unable to map level %u\n", __func__, level);
+ rc = -ENOENT;
+ goto out;
+ }
+ else
+ {
+ rc = 0;
+ goto out;
+ }
}
+ else if ( rc != XEN_TABLE_NORMAL_PAGE )
+ break;
}
- BUG_ON(!lpae_is_valid(*entry));
+ if ( level != target )
+ {
+ mm_printk("%s: Shattering superpage is not supported\n", __func__);
+ rc = -EOPNOTSUPP;
+ goto out;
+ }
- third = xen_map_table(lpae_get_mfn(*entry));
- entry = &third[third_table_offset(addr)];
+ entry = table + offsets[level];
rc = -EINVAL;
if ( !xen_pt_check_entry(*entry, mfn, flags) )
@@ -1098,7 +1180,7 @@ static int xen_pt_update_entry(unsigned long addr, mfn_t mfn,
rc = 0;
out:
- xen_unmap_table(third);
+ xen_unmap_table(table);
return rc;
}
@@ -1114,6 +1196,15 @@ static int xen_pt_update(unsigned long virt,
unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
/*
+ * For arm32, page-tables are different on each CPUs. Yet, they share
+ * some common mappings. It is assumed that only common mappings
+ * will be modified with this function.
+ *
+ * XXX: Add a check.
+ */
+ const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
+
+ /*
* The hardware was configured to forbid mapping both writeable and
* executable.
* When modifying/creating mapping (i.e _PAGE_PRESENT is set),
@@ -1134,9 +1225,9 @@ static int xen_pt_update(unsigned long virt,
spin_lock(&xen_pt_lock);
- for( ; addr < addr_end; addr += PAGE_SIZE )
+ for ( ; addr < addr_end; addr += PAGE_SIZE )
{
- rc = xen_pt_update_entry(addr, mfn, flags);
+ rc = xen_pt_update_entry(root, addr, mfn, flags);
if ( rc )
break;
--
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] 54+ messages in thread
* [PATCH 11/12] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
{set, clear}_fixmap() are currently open-coding update to the Xen
page-tables. This can be avoided by using the generic helpers
map_pages_to_xen() and destroy_xen_mappings().
Both function are not meant to fail for fixmap, hence the BUG_ON()
checking the return.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 022967ff9f..7e6df9f877 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -343,19 +343,19 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
/* Map a 4k page in a fixmap entry */
void set_fixmap(unsigned map, mfn_t mfn, unsigned int flags)
{
- lpae_t pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
- pte.pt.table = 1; /* 4k mappings always have this bit set */
- pte.pt.xn = 1;
- write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
- flush_xen_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+ int res;
+
+ res = map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags);
+ BUG_ON(res != 0);
}
/* Remove a mapping from a fixmap entry */
void clear_fixmap(unsigned map)
{
- lpae_t pte = {0};
- write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
- flush_xen_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+ int res;
+
+ res = destroy_xen_mappings(FIXMAP_ADDR(map), FIXMAP_ADDR(map) + PAGE_SIZE);
+ BUG_ON(res != 0);
}
/* Create Xen's mappings of memory.
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 11/12] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
{set, clear}_fixmap() are currently open-coding update to the Xen
page-tables. This can be avoided by using the generic helpers
map_pages_to_xen() and destroy_xen_mappings().
Both function are not meant to fail for fixmap, hence the BUG_ON()
checking the return.
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 022967ff9f..7e6df9f877 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -343,19 +343,19 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr)
/* Map a 4k page in a fixmap entry */
void set_fixmap(unsigned map, mfn_t mfn, unsigned int flags)
{
- lpae_t pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
- pte.pt.table = 1; /* 4k mappings always have this bit set */
- pte.pt.xn = 1;
- write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
- flush_xen_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+ int res;
+
+ res = map_pages_to_xen(FIXMAP_ADDR(map), mfn, 1, flags);
+ BUG_ON(res != 0);
}
/* Remove a mapping from a fixmap entry */
void clear_fixmap(unsigned map)
{
- lpae_t pte = {0};
- write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
- flush_xen_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+ int res;
+
+ res = destroy_xen_mappings(FIXMAP_ADDR(map), FIXMAP_ADDR(map) + PAGE_SIZE);
+ BUG_ON(res != 0);
}
/* Create Xen's mappings of memory.
--
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] 54+ messages in thread
* [PATCH 12/12] xen/arm: mm: Remove set_pte_flags_on_range()
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
set_pte_flags_on_range() is yet another function that will open-code
update to a specific range in the Xen page-tables. It can be completely
dropped by using either modify_xen_mappings() or destroy_xen_mappings().
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 58 ++++++++++---------------------------------------------
1 file changed, 10 insertions(+), 48 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7e6df9f877..5810d569ee 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1272,52 +1272,6 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
}
-enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
-static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
-{
- lpae_t pte;
- int i;
-
- ASSERT(is_kernel(p) && is_kernel(p + l));
-
- /* Can only guard in page granularity */
- ASSERT(!((unsigned long) p & ~PAGE_MASK));
- ASSERT(!(l & ~PAGE_MASK));
-
- for ( i = (p - _start) / PAGE_SIZE;
- i < (p + l - _start) / PAGE_SIZE;
- i++ )
- {
- pte = xen_xenmap[i];
- switch ( mg )
- {
- case mg_clear:
- pte.pt.valid = 0;
- break;
- case mg_ro:
- pte.pt.valid = 1;
- pte.pt.pxn = 1;
- pte.pt.xn = 1;
- pte.pt.ro = 1;
- break;
- case mg_rw:
- pte.pt.valid = 1;
- pte.pt.pxn = 1;
- pte.pt.xn = 1;
- pte.pt.ro = 0;
- break;
- case mg_rx:
- pte.pt.valid = 1;
- pte.pt.pxn = 0;
- pte.pt.xn = 0;
- pte.pt.ro = 1;
- break;
- }
- write_pte(xen_xenmap + i, pte);
- }
- flush_xen_tlb_local();
-}
-
/* Release all __init and __initdata ranges to be reused */
void free_init_memory(void)
{
@@ -1326,8 +1280,12 @@ void free_init_memory(void)
uint32_t insn;
unsigned int i, nr = len / sizeof(insn);
uint32_t *p;
+ int rc;
- set_pte_flags_on_range(__init_begin, len, mg_rw);
+ rc = modify_xen_mappings((unsigned long)__init_begin,
+ (unsigned long)__init_end, PAGE_HYPERVISOR_RW);
+ if ( rc )
+ panic("Unable to map RW the init section (rc = %d)", rc);
/*
* From now on, init will not be used for execution anymore,
@@ -1345,7 +1303,11 @@ void free_init_memory(void)
for ( i = 0; i < nr; i++ )
*(p + i) = insn;
- set_pte_flags_on_range(__init_begin, len, mg_clear);
+ rc = destroy_xen_mappings((unsigned long)__init_begin,
+ (unsigned long)__init_end);
+ if ( rc )
+ panic("Unable to remove the init section (rc = %d)", rc);
+
init_domheap_pages(pa, pa + len);
printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
}
--
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] 54+ messages in thread
* [Xen-devel] [PATCH 12/12] xen/arm: mm: Remove set_pte_flags_on_range()
@ 2019-04-24 16:59 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-04-24 16:59 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr_Tyshchenko, Julien Grall, sstabellini, Andrii_Anisov
set_pte_flags_on_range() is yet another function that will open-code
update to a specific range in the Xen page-tables. It can be completely
dropped by using either modify_xen_mappings() or destroy_xen_mappings().
Signed-off-by: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/mm.c | 58 ++++++++++---------------------------------------------
1 file changed, 10 insertions(+), 48 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7e6df9f877..5810d569ee 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1272,52 +1272,6 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, flags);
}
-enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
-static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
-{
- lpae_t pte;
- int i;
-
- ASSERT(is_kernel(p) && is_kernel(p + l));
-
- /* Can only guard in page granularity */
- ASSERT(!((unsigned long) p & ~PAGE_MASK));
- ASSERT(!(l & ~PAGE_MASK));
-
- for ( i = (p - _start) / PAGE_SIZE;
- i < (p + l - _start) / PAGE_SIZE;
- i++ )
- {
- pte = xen_xenmap[i];
- switch ( mg )
- {
- case mg_clear:
- pte.pt.valid = 0;
- break;
- case mg_ro:
- pte.pt.valid = 1;
- pte.pt.pxn = 1;
- pte.pt.xn = 1;
- pte.pt.ro = 1;
- break;
- case mg_rw:
- pte.pt.valid = 1;
- pte.pt.pxn = 1;
- pte.pt.xn = 1;
- pte.pt.ro = 0;
- break;
- case mg_rx:
- pte.pt.valid = 1;
- pte.pt.pxn = 0;
- pte.pt.xn = 0;
- pte.pt.ro = 1;
- break;
- }
- write_pte(xen_xenmap + i, pte);
- }
- flush_xen_tlb_local();
-}
-
/* Release all __init and __initdata ranges to be reused */
void free_init_memory(void)
{
@@ -1326,8 +1280,12 @@ void free_init_memory(void)
uint32_t insn;
unsigned int i, nr = len / sizeof(insn);
uint32_t *p;
+ int rc;
- set_pte_flags_on_range(__init_begin, len, mg_rw);
+ rc = modify_xen_mappings((unsigned long)__init_begin,
+ (unsigned long)__init_end, PAGE_HYPERVISOR_RW);
+ if ( rc )
+ panic("Unable to map RW the init section (rc = %d)", rc);
/*
* From now on, init will not be used for execution anymore,
@@ -1345,7 +1303,11 @@ void free_init_memory(void)
for ( i = 0; i < nr; i++ )
*(p + i) = insn;
- set_pte_flags_on_range(__init_begin, len, mg_clear);
+ rc = destroy_xen_mappings((unsigned long)__init_begin,
+ (unsigned long)__init_end);
+ if ( rc )
+ panic("Unable to remove the init section (rc = %d)", rc);
+
init_domheap_pages(pa, pa + len);
printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
}
--
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] 54+ messages in thread
* Re: [PATCH 01/12] xen/arm: lpae: Add a macro to generate offsets from an address
@ 2019-05-06 12:47 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:47 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> There are few places requiring to generate offsets from an address.
> Rather than open-coding everywhere, we can introduce a macro to do the
> job for us.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/p2m.c | 23 +++--------------------
> xen/include/asm-arm/lpae.h | 9 +++++++++
> 2 files changed, 12 insertions(+), 20 deletions(-)
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 01/12] xen/arm: lpae: Add a macro to generate offsets from an address
@ 2019-05-06 12:47 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:47 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> There are few places requiring to generate offsets from an address.
> Rather than open-coding everywhere, we can introduce a macro to do the
> job for us.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/p2m.c | 23 +++--------------------
> xen/include/asm-arm/lpae.h | 9 +++++++++
> 2 files changed, 12 insertions(+), 20 deletions(-)
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 02/12] xen/arm: mm: Rename create_xen_entries() to xen_pt_update()
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> create_xen_entries() is doing more than creating entries. It can also
> modify and remove entries.
>
> Rename the function to make clear what the function is actually doing.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 02/12] xen/arm: mm: Rename create_xen_entries() to xen_pt_update()
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> create_xen_entries() is doing more than creating entries. It can also
> modify and remove entries.
>
> Rename the function to make clear what the function is actually doing.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 03/12] xen/arm: mm: Move out of xen_pt_update() the logic to update an entry
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> In preparation of rework of the Xen PT, the logic to update an entry
> in moved out in a separate function.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 03/12] xen/arm: mm: Move out of xen_pt_update() the logic to update an entry
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> In preparation of rework of the Xen PT, the logic to update an entry
> in moved out in a separate function.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 04/12] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> At the moment, the flags are not enough to describe what kind of update
> will done on the VA range. They need to be used in conjunction with the
> enum xenmap_operation.
>
> It would be more convenient to have all the information for the update
> in a single place.
>
> Two new flags are added to remove the relience on xenmap_operation:
> - _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
> - _PAGE_POPULATE: Indicate whether we only populate page-tables
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 04/12] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> At the moment, the flags are not enough to describe what kind of update
> will done on the VA range. They need to be used in conjunction with the
> enum xenmap_operation.
>
> It would be more convenient to have all the information for the update
> in a single place.
>
> Two new flags are added to remove the relience on xenmap_operation:
> - _PAGE_PRESENT: Indicate whether we are adding/removing the mapping
> - _PAGE_POPULATE: Indicate whether we only populate page-tables
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 05/12] xen/arm: mm: Only increment mfn when valid in xen_pt_update
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> Currently, the MFN will be incremented even if it is invalid. This will
> result to have a valid MFN after the first iteration.
>
> While this is not a major problem today, this will be in the future if
> the code expect an invalid MFN at every iteration.
>
> Such behavior is prevented by avoiding to increment an invalid MFN.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 05/12] xen/arm: mm: Only increment mfn when valid in xen_pt_update
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> Currently, the MFN will be incremented even if it is invalid. This will
> result to have a valid MFN after the first iteration.
>
> While this is not a major problem today, this will be in the future if
> the code expect an invalid MFN at every iteration.
>
> Such behavior is prevented by avoiding to increment an invalid MFN.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 07/12] xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> With the newly introduced flags, it is now possible to know how the page
> will be updated through the flags.
>
> All the use of xenmap_operation are not replaced with the flags. At the
I suppose it should be "are noW replaced with the flags".
> same time, validity check are now removed as they are gathered in
> xen_pt_check_entry().
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 07/12] xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> With the newly introduced flags, it is now possible to know how the page
> will be updated through the flags.
>
> All the use of xenmap_operation are not replaced with the flags. At the
I suppose it should be "are noW replaced with the flags".
> same time, validity check are now removed as they are gathered in
> xen_pt_check_entry().
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
Hello Julien,
On 24.04.19 19:59, Julien Grall wrote:
> The code handling Xen PT update has quite a few restrictions on what it
> can do. This is not a bad thing as it keeps the code simple.
>
> There are already a few checks scattered in current page table handling.
> However they are not sufficient as they could still allow to
> modify/remove entry with contiguous bit set.
>
> The checks are divided in two sets:
> - per entry check: They are gathered in a new function that will
> check whether an update is valid based on the flags passed and the
> current value of an entry.
> - global check: They are sanity check on xen_pt_update() parameters.
>
> Additionally to contiguous check, we also now check that the caller is
> not trying to modify the memory attributes of an entry.
>
> Lastly, it was probably a bit over the top to forbid removing an
> invalid mapping. This could just be ignored. The new behavior will be
> helpful in future changes.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/mm.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 97 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index eee7122c88..5eb6f47d74 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -50,6 +50,19 @@ struct domain *dom_xen, *dom_io, *dom_cow;
> #undef mfn_to_virt
> #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
>
> +#ifdef NDEBUG
> +static inline void
> +__attribute__ ((__format__ (__printf__, 1, 2)))
> +mm_printk(const char *fmt, ...) {}
> +#else
> +#define mm_printk(fmt, args...) \
> + do \
> + { \
> + dprintk(XENLOG_ERR, fmt, ## args); \
> + WARN(); \
> + } while (0);
> +#endif
> +
> /* Static start-of-day pagetables that we use before the allocators
> * are up. These are used by all CPUs during bringup before switching
> * to the CPUs own pagetables.
> @@ -963,12 +976,74 @@ enum xenmap_operation {
> RESERVE
> };
>
> +/* Sanity check of the entry */
> +static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
> +{
> + /* Sanity check when modifying a page. */
> + if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
> + {
> + /* We don't allow changing memory attributes. */
> + if ( entry.pt.ai != PAGE_AI_MASK(flags) )
> + {
> + mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n",
> + entry.pt.ai, PAGE_AI_MASK(flags));
> + return false;
> + }
> +
> + /* We don't allow modifying entry with contiguous bit set. */
> + if ( entry.pt.contig )
> + {
> + mm_printk("Modifying entry with contiguous bit set is not allowed.\n");
> + return false;
> + }
> + }
> + /* Sanity check when inserting a page */
> + else if ( flags & _PAGE_PRESENT )
> + {
> + /* We should be here with a valid MFN. */
> + ASSERT(!mfn_eq(mfn, INVALID_MFN));
> +
> + /* We don't allow replacing any valid entry. */
> + if ( lpae_is_valid(entry) )
> + {
> + mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
> + mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
> + return false;
> + }
> + }
> + /* Sanity check when removing a page. */
> + else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 )
Shouldn't it be `(flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0` ? As I understand from the patch 04, we do remove a page when we do unset both flags.
BTW, I would suggest patches reordering in this series. This patch should go right after the patch 04, where you introduce those flags. Because here you implement their usage. But somewhy you inserted an unrelated change in between.
> + {
> + /* We should be here with an invalid MFN. */
> + ASSERT(mfn_eq(mfn, INVALID_MFN));
> +
> + /* We don't allow removing page with contiguous bit set. */
> + if ( entry.pt.contig )
> + {
> + mm_printk("Removing entry with contiguous bit set is not allowed.\n");
> + return false;
> + }
> + }
> + /* Sanity check when populating the page-table. No check so far. */
> + else
> + {
> + ASSERT(!(flags & _PAGE_POPULATE));
> + /* We should be here with an invalid MFN */
> + ASSERT(mfn_eq(mfn, INVALID_MFN));
> + }
> +
> + return true;
> +}
> +
> static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> mfn_t mfn, unsigned int flags)
> {
> lpae_t pte, *entry;
> lpae_t *third = NULL;
>
> + /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
> + ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
> +
> entry = &xen_second[second_linear_offset(addr)];
> if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
> {
> @@ -984,15 +1059,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> third = mfn_to_virt(lpae_get_mfn(*entry));
> entry = &third[third_table_offset(addr)];
>
> + if ( !xen_pt_check_entry(*entry, mfn, flags) )
> + return -EINVAL;
> +
> switch ( op ) {
> case INSERT:
> case RESERVE:
> - if ( lpae_is_valid(*entry) )
> - {
> - printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
> - __func__, addr, mfn_x(mfn));
> - return -EINVAL;
> - }
> if ( op == RESERVE )
> break;
> pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
> @@ -1004,12 +1076,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> break;
> case MODIFY:
> case REMOVE:
> - if ( !lpae_is_valid(*entry) )
> - {
> - printk("%s: trying to %s a non-existing mapping addr=%lx\n",
> - __func__, op == REMOVE ? "remove" : "modify", addr);
> - return -EINVAL;
> - }
> if ( op == REMOVE )
> pte.bits = 0;
> else
> @@ -1017,12 +1083,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> pte = *entry;
> pte.pt.ro = PAGE_RO_MASK(flags);
> pte.pt.xn = PAGE_XN_MASK(flags);
> - if ( !pte.pt.ro && !pte.pt.xn )
> - {
> - printk("%s: Incorrect combination for addr=%lx\n",
> - __func__, addr);
> - return -EINVAL;
> - }
> }
> write_pte(entry, pte);
> break;
> @@ -1044,6 +1104,25 @@ static int xen_pt_update(enum xenmap_operation op,
> int rc = 0;
> unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
>
> + /*
> + * The hardware was configured to forbid mapping both writeable and
> + * executable.
> + * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
> + * prevent any update if this happen.
> + */
> + if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) &&
> + !PAGE_XN_MASK(flags) )
> + {
> + mm_printk("Mappings should not be both Writeable and Executable.\n");
> + return -EINVAL;
> + }
> +
> + if ( !IS_ALIGNED(virt, PAGE_SIZE) )
> + {
> + mm_printk("The virtual address is not aligned to the page-size.\n");
> + return -EINVAL;
> + }
> +
> spin_lock(&xen_pt_lock);
>
> for( ; addr < addr_end; addr += PAGE_SIZE )
>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
Hello Julien,
On 24.04.19 19:59, Julien Grall wrote:
> The code handling Xen PT update has quite a few restrictions on what it
> can do. This is not a bad thing as it keeps the code simple.
>
> There are already a few checks scattered in current page table handling.
> However they are not sufficient as they could still allow to
> modify/remove entry with contiguous bit set.
>
> The checks are divided in two sets:
> - per entry check: They are gathered in a new function that will
> check whether an update is valid based on the flags passed and the
> current value of an entry.
> - global check: They are sanity check on xen_pt_update() parameters.
>
> Additionally to contiguous check, we also now check that the caller is
> not trying to modify the memory attributes of an entry.
>
> Lastly, it was probably a bit over the top to forbid removing an
> invalid mapping. This could just be ignored. The new behavior will be
> helpful in future changes.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/mm.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 97 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index eee7122c88..5eb6f47d74 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -50,6 +50,19 @@ struct domain *dom_xen, *dom_io, *dom_cow;
> #undef mfn_to_virt
> #define mfn_to_virt(mfn) __mfn_to_virt(mfn_x(mfn))
>
> +#ifdef NDEBUG
> +static inline void
> +__attribute__ ((__format__ (__printf__, 1, 2)))
> +mm_printk(const char *fmt, ...) {}
> +#else
> +#define mm_printk(fmt, args...) \
> + do \
> + { \
> + dprintk(XENLOG_ERR, fmt, ## args); \
> + WARN(); \
> + } while (0);
> +#endif
> +
> /* Static start-of-day pagetables that we use before the allocators
> * are up. These are used by all CPUs during bringup before switching
> * to the CPUs own pagetables.
> @@ -963,12 +976,74 @@ enum xenmap_operation {
> RESERVE
> };
>
> +/* Sanity check of the entry */
> +static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
> +{
> + /* Sanity check when modifying a page. */
> + if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
> + {
> + /* We don't allow changing memory attributes. */
> + if ( entry.pt.ai != PAGE_AI_MASK(flags) )
> + {
> + mm_printk("Modifying memory attributes is not allowed (0x%x -> 0x%x).\n",
> + entry.pt.ai, PAGE_AI_MASK(flags));
> + return false;
> + }
> +
> + /* We don't allow modifying entry with contiguous bit set. */
> + if ( entry.pt.contig )
> + {
> + mm_printk("Modifying entry with contiguous bit set is not allowed.\n");
> + return false;
> + }
> + }
> + /* Sanity check when inserting a page */
> + else if ( flags & _PAGE_PRESENT )
> + {
> + /* We should be here with a valid MFN. */
> + ASSERT(!mfn_eq(mfn, INVALID_MFN));
> +
> + /* We don't allow replacing any valid entry. */
> + if ( lpae_is_valid(entry) )
> + {
> + mm_printk("Changing MFN for a valid entry is not allowed (%#"PRI_mfn" -> %#"PRI_mfn").\n",
> + mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
> + return false;
> + }
> + }
> + /* Sanity check when removing a page. */
> + else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 )
Shouldn't it be `(flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0` ? As I understand from the patch 04, we do remove a page when we do unset both flags.
BTW, I would suggest patches reordering in this series. This patch should go right after the patch 04, where you introduce those flags. Because here you implement their usage. But somewhy you inserted an unrelated change in between.
> + {
> + /* We should be here with an invalid MFN. */
> + ASSERT(mfn_eq(mfn, INVALID_MFN));
> +
> + /* We don't allow removing page with contiguous bit set. */
> + if ( entry.pt.contig )
> + {
> + mm_printk("Removing entry with contiguous bit set is not allowed.\n");
> + return false;
> + }
> + }
> + /* Sanity check when populating the page-table. No check so far. */
> + else
> + {
> + ASSERT(!(flags & _PAGE_POPULATE));
> + /* We should be here with an invalid MFN */
> + ASSERT(mfn_eq(mfn, INVALID_MFN));
> + }
> +
> + return true;
> +}
> +
> static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> mfn_t mfn, unsigned int flags)
> {
> lpae_t pte, *entry;
> lpae_t *third = NULL;
>
> + /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together. */
> + ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) != (_PAGE_POPULATE|_PAGE_PRESENT));
> +
> entry = &xen_second[second_linear_offset(addr)];
> if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) )
> {
> @@ -984,15 +1059,12 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> third = mfn_to_virt(lpae_get_mfn(*entry));
> entry = &third[third_table_offset(addr)];
>
> + if ( !xen_pt_check_entry(*entry, mfn, flags) )
> + return -EINVAL;
> +
> switch ( op ) {
> case INSERT:
> case RESERVE:
> - if ( lpae_is_valid(*entry) )
> - {
> - printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
> - __func__, addr, mfn_x(mfn));
> - return -EINVAL;
> - }
> if ( op == RESERVE )
> break;
> pte = mfn_to_xen_entry(mfn, PAGE_AI_MASK(flags));
> @@ -1004,12 +1076,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> break;
> case MODIFY:
> case REMOVE:
> - if ( !lpae_is_valid(*entry) )
> - {
> - printk("%s: trying to %s a non-existing mapping addr=%lx\n",
> - __func__, op == REMOVE ? "remove" : "modify", addr);
> - return -EINVAL;
> - }
> if ( op == REMOVE )
> pte.bits = 0;
> else
> @@ -1017,12 +1083,6 @@ static int xen_pt_update_entry(enum xenmap_operation op, unsigned long addr,
> pte = *entry;
> pte.pt.ro = PAGE_RO_MASK(flags);
> pte.pt.xn = PAGE_XN_MASK(flags);
> - if ( !pte.pt.ro && !pte.pt.xn )
> - {
> - printk("%s: Incorrect combination for addr=%lx\n",
> - __func__, addr);
> - return -EINVAL;
> - }
> }
> write_pte(entry, pte);
> break;
> @@ -1044,6 +1104,25 @@ static int xen_pt_update(enum xenmap_operation op,
> int rc = 0;
> unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
>
> + /*
> + * The hardware was configured to forbid mapping both writeable and
> + * executable.
> + * When modifying/creating mapping (i.e _PAGE_PRESENT is set),
> + * prevent any update if this happen.
> + */
> + if ( (flags & _PAGE_PRESENT) && !PAGE_RO_MASK(flags) &&
> + !PAGE_XN_MASK(flags) )
> + {
> + mm_printk("Mappings should not be both Writeable and Executable.\n");
> + return -EINVAL;
> + }
> +
> + if ( !IS_ALIGNED(virt, PAGE_SIZE) )
> + {
> + mm_printk("The virtual address is not aligned to the page-size.\n");
> + return -EINVAL;
> + }
> +
> spin_lock(&xen_pt_lock);
>
> for( ; addr < addr_end; addr += PAGE_SIZE )
>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 08/12] xen/arm: mm: Remove enum xenmap_operation
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> The enum xenmap_operation is not used anymore. So remove it.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 08/12] xen/arm: mm: Remove enum xenmap_operation
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> The enum xenmap_operation is not used anymore. So remove it.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 09/12] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> Currently, the virtual address of the 3rd level page-tables is obtained
> using mfn_to_virt().
>
> On Arm32, mfn_to_virt can only work on xenheap page. While in practice
> all the page-tables updated will reside in xenheap, in practive the
> page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
>
> Furthermore, a follow-up change will update xen_pt_update_entry() to
> walk all the levels and therefore be more generic. Some of the
> page-tables will also part of Xen memory and therefore will not be
> reachable using mfn_to_virt().
>
> The easiest way to reach those pages is to use {, un}map_domain_page().
> While on arm32 this means an extra mapping in the normal cases, this is not
> very important as xen page-tables are not updated often.
>
> In order to allow future change in the way Xen page-tables are mapped,
> two new helpers are introduced to map/unmap the page-tables.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 09/12] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables
@ 2019-05-06 12:48 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:48 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> Currently, the virtual address of the 3rd level page-tables is obtained
> using mfn_to_virt().
>
> On Arm32, mfn_to_virt can only work on xenheap page. While in practice
> all the page-tables updated will reside in xenheap, in practive the
> page-tables covering Xen memory (e.g xen_mapping) is part of Xen binary.
>
> Furthermore, a follow-up change will update xen_pt_update_entry() to
> walk all the levels and therefore be more generic. Some of the
> page-tables will also part of Xen memory and therefore will not be
> reachable using mfn_to_virt().
>
> The easiest way to reach those pages is to use {, un}map_domain_page().
> While on arm32 this means an extra mapping in the normal cases, this is not
> very important as xen page-tables are not updated often.
>
> In order to allow future change in the way Xen page-tables are mapped,
> two new helpers are introduced to map/unmap the page-tables.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 10/12] xen/arm: mm: Rework Xen page-tables walk during update
@ 2019-05-06 12:49 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:49 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> Currently, xen_pt_update_entry() is only able to update the region covered
> by xen_second (i.e 0 to 0x7fffffff).
>
> Because of the restriction we end to have multiple functions in mm.c
> modifying the page-tables differently.
>
> Furthermore, we never walked the page-tables fully. This means that any
> change in the layout may requires major rewrite of the page-tables code.
>
> Lastly, we have been quite lucky that no one ever tried to pass an address
> outside this range because it would have blown-up.
>
> xen_pt_update_entry() is reworked to walk over the page-tables every
> time. The logic has been borrowed from arch/arm/p2m.c and contain some
> limitations for the time being:
> - Superpage cannot be shattered
> - Only level 3 (i.e 4KB) can be done
>
> Note that the parameter 'addr' has been renamed to 'virt' to make clear
> we are dealing with a virtual address.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 10/12] xen/arm: mm: Rework Xen page-tables walk during update
@ 2019-05-06 12:49 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:49 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> Currently, xen_pt_update_entry() is only able to update the region covered
> by xen_second (i.e 0 to 0x7fffffff).
>
> Because of the restriction we end to have multiple functions in mm.c
> modifying the page-tables differently.
>
> Furthermore, we never walked the page-tables fully. This means that any
> change in the layout may requires major rewrite of the page-tables code.
>
> Lastly, we have been quite lucky that no one ever tried to pass an address
> outside this range because it would have blown-up.
>
> xen_pt_update_entry() is reworked to walk over the page-tables every
> time. The logic has been borrowed from arch/arm/p2m.c and contain some
> limitations for the time being:
> - Superpage cannot be shattered
> - Only level 3 (i.e 4KB) can be done
>
> Note that the parameter 'addr' has been renamed to 'virt' to make clear
> we are dealing with a virtual address.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 11/12] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
@ 2019-05-06 12:49 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:49 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> {set, clear}_fixmap() are currently open-coding update to the Xen
> page-tables. This can be avoided by using the generic helpers
> map_pages_to_xen() and destroy_xen_mappings().
>
> Both function are not meant to fail for fixmap, hence the BUG_ON()
> checking the return.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 11/12] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap()
@ 2019-05-06 12:49 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:49 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> {set, clear}_fixmap() are currently open-coding update to the Xen
> page-tables. This can be avoided by using the generic helpers
> map_pages_to_xen() and destroy_xen_mappings().
>
> Both function are not meant to fail for fixmap, hence the BUG_ON()
> checking the return.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 12/12] xen/arm: mm: Remove set_pte_flags_on_range()
@ 2019-05-06 12:49 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:49 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> set_pte_flags_on_range() is yet another function that will open-code
> update to a specific range in the Xen page-tables. It can be completely
> dropped by using either modify_xen_mappings() or destroy_xen_mappings().
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 12/12] xen/arm: mm: Remove set_pte_flags_on_range()
@ 2019-05-06 12:49 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-06 12:49 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
On 24.04.19 19:59, Julien Grall wrote:
> set_pte_flags_on_range() is yet another function that will open-code
> update to a specific range in the Xen page-tables. It can be completely
> dropped by using either modify_xen_mappings() or destroy_xen_mappings().
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-05-06 17:01 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-05-06 17:01 UTC (permalink / raw)
To: Andrii Anisov, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
Hi,
On 5/6/19 1:48 PM, Andrii Anisov wrote:
>> + /* Sanity check when removing a page. */
>> + else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 )
>
> Shouldn't it be `(flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0` ? As I
> understand from the patch 04, we do remove a page when we do unset both
> flags.
Hmmm, yes. I will update in the next version.
>
> BTW, I would suggest patches reordering in this series. This patch
> should go right after the patch 04, where you introduce those flags.
> Because here you implement their usage. But somewhy you inserted an
> unrelated change in between.
It is not unrelated, the patch is necessary to make this patch works.
Otherwise you will hit the ASSERT(mfn_eq(mfn, INVALID_MFN)) after one
iteration.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-05-06 17:01 ` Julien Grall
0 siblings, 0 replies; 54+ messages in thread
From: Julien Grall @ 2019-05-06 17:01 UTC (permalink / raw)
To: Andrii Anisov, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
Hi,
On 5/6/19 1:48 PM, Andrii Anisov wrote:
>> + /* Sanity check when removing a page. */
>> + else if ( (flags & (_PAGE_PRESENT|_PAGE_POPULATE)) != 0 )
>
> Shouldn't it be `(flags & (_PAGE_PRESENT|_PAGE_POPULATE)) == 0` ? As I
> understand from the patch 04, we do remove a page when we do unset both
> flags.
Hmmm, yes. I will update in the next version.
>
> BTW, I would suggest patches reordering in this series. This patch
> should go right after the patch 04, where you introduce those flags.
> Because here you implement their usage. But somewhy you inserted an
> unrelated change in between.
It is not unrelated, the patch is necessary to make this patch works.
Otherwise you will hit the ASSERT(mfn_eq(mfn, INVALID_MFN)) after one
iteration.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-05-07 7:38 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-07 7:38 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
Hello Julien,
On 06.05.19 20:01, Julien Grall wrote:
> It is not unrelated, the patch is necessary to make this patch works. Otherwise you will hit the ASSERT(mfn_eq(mfn, INVALID_MFN)) after one iteration.
I'm definitely not saying it is totally odd here. I'm just express my opinion that patches with flags introduction and implementation better go one after another.
But for sure this comment is a minor note, so it's up to you:)
BTW, with the fixed condition:
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [Xen-devel] [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables
@ 2019-05-07 7:38 ` Andrii Anisov
0 siblings, 0 replies; 54+ messages in thread
From: Andrii Anisov @ 2019-05-07 7:38 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Oleksandr_Tyshchenko, sstabellini, Andrii_Anisov
Hello Julien,
On 06.05.19 20:01, Julien Grall wrote:
> It is not unrelated, the patch is necessary to make this patch works. Otherwise you will hit the ASSERT(mfn_eq(mfn, INVALID_MFN)) after one iteration.
I'm definitely not saying it is totally odd here. I'm just express my opinion that patches with flags introduction and implementation better go one after another.
But for sure this comment is a minor note, so it's up to you:)
BTW, with the fixed condition:
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
end of thread, other threads:[~2019-05-07 7:39 UTC | newest]
Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 16:59 [PATCH 00/12] xen/arm: Provide a generic function to update Xen PT Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-04-24 16:59 ` [PATCH 01/12] xen/arm: lpae: Add a macro to generate offsets from an address Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:47 ` Andrii Anisov
2019-05-06 12:47 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 02/12] xen/arm: mm: Rename create_xen_entries() to xen_pt_update() Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 03/12] xen/arm: mm: Move out of xen_pt_update() the logic to update an entry Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 04/12] xen/arm: mm: Introduce _PAGE_PRESENT and _PAGE_POPULATE Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 05/12] xen/arm: mm: Only increment mfn when valid in xen_pt_update Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 06/12] xen/arm: mm: Sanity check any update of Xen page tables Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-05-06 17:01 ` Julien Grall
2019-05-06 17:01 ` [Xen-devel] " Julien Grall
2019-05-07 7:38 ` Andrii Anisov
2019-05-07 7:38 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 07/12] xen/arm: mm: Rework xen_pt_update_entry to avoid use xenmap_operation Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 08/12] xen/arm: mm: Remove enum xenmap_operation Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 09/12] xen/arm: mm: Use {, un}map_domain_page() to map/unmap Xen page-tables Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:48 ` Andrii Anisov
2019-05-06 12:48 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 10/12] xen/arm: mm: Rework Xen page-tables walk during update Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:49 ` Andrii Anisov
2019-05-06 12:49 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 11/12] xen/arm: mm: Don't open-code Xen PT update in {set, clear}_fixmap() Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:49 ` Andrii Anisov
2019-05-06 12:49 ` [Xen-devel] " Andrii Anisov
2019-04-24 16:59 ` [PATCH 12/12] xen/arm: mm: Remove set_pte_flags_on_range() Julien Grall
2019-04-24 16:59 ` [Xen-devel] " Julien Grall
2019-05-06 12:49 ` Andrii Anisov
2019-05-06 12:49 ` [Xen-devel] " Andrii Anisov
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.