On 08/23/2017 03:09 PM, Andre Przywara wrote:
Hi,
Hi,
On 14/08/17 15:24, Julien Grall wrote:Currently, all the new mappings will be read-write non-executable. Allow the caller to use other permissions. Signed-off-by: Julien Grall <julien.gr...@arm.com> --- xen/arch/arm/mm.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index cd7bcf7aca..fe0646002e 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1022,6 +1022,14 @@ static int create_xen_entries(enum xenmap_operation op, 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); + if ( !pte.pt.ro && !pte.pt.xn )
I noticed I introduced a double-space here. I will fix.
+ { + printk("%s: Incorrect combination for addr=%lx\n", + __func__, addr); + return -EINVAL;I don't think this should be a handled runtime error, but rather a BUG_ON() or an ASSERT(). > I chased down the call chain for all create_xen_entries() invocations, and they all stem from some constant (combination of) hard coded flags. So ending up with an invalid combination here is clearly a bug in the code and should be treated as such.
Well, you could potentially call with your own flags. I don't see anything to restrict that and might be used for instance to setup early page table.
If we trust the caller will set the right permission, then a BUG_ON() would be fine here. If not, we should definitely return an error for at least non-debug build as the abort later on would be difficult to hunt down.
Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
