| Message ID | 20190330005900.17282-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement ARMv8.5-BTI for linux-user | expand |
On Sat, 30 Mar 2019 at 00:59, Richard Henderson <richard.henderson@linaro.org> wrote: > > There is agreement that there will be a mmap/mprotect bit, > although no word yet on the value or the name. Invent a > name to make forward progress. > > The PAGE_TARGET_1 bit, is qemu internal, and allows the > target something to query from the guest page tables. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> This looks ok code-wise, but we'd need to hide it behind a defaults-to-off x-something property if we wanted to commit it before the kernel ABI is fixed. Do we also need to handle this in mprotect() ? thanks -- PMM
On 4/29/19 9:21 AM, Peter Maydell wrote: > This looks ok code-wise, but we'd need to hide it behind > a defaults-to-off x-something property if we wanted to > commit it before the kernel ABI is fixed. I'm not intending to change the user-level abi, only the internal abi within qemu, for handling of the elf notes. You think this should be done differently, so that there's zero possibility of a user-level setting the relevant bit? > Do we also need to handle this in mprotect() ? Not until there's a kernel abi. r~
On Mon, 29 Apr 2019 at 21:13, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 4/29/19 9:21 AM, Peter Maydell wrote: > > This looks ok code-wise, but we'd need to hide it behind > > a defaults-to-off x-something property if we wanted to > > commit it before the kernel ABI is fixed. > > I'm not intending to change the user-level abi, only the > internal abi within qemu, for handling of the elf notes. You're changing target_mmap(), which is used by the guest mmap syscall, though, right? > You think this should be done differently, so that there's > zero possibility of a user-level setting the relevant bit? I think that we shouldn't allow guest binaries written to the ad-hoc TARGET_PROT_BTI ABI to work without an explicit x-something command line argument to QEMU, to indicate that the user knows they're doing something odd and that these binaries won't always continue to work in future QEMU versions. thanks -- PMM
On 4/30/19 3:40 AM, Peter Maydell wrote: > On Mon, 29 Apr 2019 at 21:13, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> On 4/29/19 9:21 AM, Peter Maydell wrote: >>> This looks ok code-wise, but we'd need to hide it behind >>> a defaults-to-off x-something property if we wanted to >>> commit it before the kernel ABI is fixed. >> >> I'm not intending to change the user-level abi, only the >> internal abi within qemu, for handling of the elf notes. > > You're changing target_mmap(), which is used by the > guest mmap syscall, though, right? Yes, but it's also used by elfload.c to map the executable. > I think that we shouldn't allow guest binaries written > to the ad-hoc TARGET_PROT_BTI ABI to work without an > explicit x-something command line argument to QEMU... The guest binary is not written to an ad-hoc abi. It is written to the finalized ELF note abi, which I am trying to implement with a *private* QEMU abi. r~
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index b16c9ec513..fb38467ed1 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -242,13 +242,19 @@ extern intptr_t qemu_host_page_mask; /* original state of the write flag (used when tracking self-modifying code */ #define PAGE_WRITE_ORG 0x0010 -/* Invalidate the TLB entry immediately, helpful for s390x - * Low-Address-Protection. Used with PAGE_WRITE in tlb_set_page_with_attrs() */ -#define PAGE_WRITE_INV 0x0040 #if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY) /* FIXME: Code that sets/uses this is broken and needs to go away. */ #define PAGE_RESERVED 0x0020 #endif +/* + * Invalidate the TLB entry immediately, helpful for s390x + * Low-Address-Protection. Used with PAGE_WRITE in tlb_set_page_with_attrs() + */ +#define PAGE_WRITE_INV 0x0040 +/* + * Some target-specific bits that will be used via page_get_flags(). + */ +#define PAGE_TARGET_1 0x0080 #if defined(CONFIG_USER_ONLY) void page_dump(FILE *f); diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index 12c8407144..563f752081 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -1090,6 +1090,11 @@ struct target_winsize { #define TARGET_PROT_SEM 0x08 #endif +#ifdef TARGET_AARCH64 +/* FIXME: Placeholder while waiting on the official ABI. */ +#define TARGET_PROT_BTI 0x1000 +#endif + /* Common */ #define TARGET_MAP_SHARED 0x01 /* Share changes */ #define TARGET_MAP_PRIVATE 0x02 /* Changes are private */ diff --git a/linux-user/mmap.c b/linux-user/mmap.c index e0249efe4f..48b4d9ea02 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -362,6 +362,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, int flags, int fd, abi_ulong offset) { abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len; + int page_flags; mmap_lock(); #ifdef DEBUG_MMAP @@ -451,6 +452,16 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, } } + page_flags = (prot & PAGE_BITS) | PAGE_VALID; + +#ifdef TARGET_AARCH64 + /* Remember the BTI bit for page_get_flags, but don't pass to host. */ + if (prot & TARGET_PROT_BTI) { + page_flags |= PAGE_TARGET_1; + prot &= ~TARGET_PROT_BTI; + } +#endif + if (!(flags & MAP_FIXED)) { unsigned long host_start; void *p; @@ -562,7 +573,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, } } the_end1: - page_set_flags(start, start + len, prot | PAGE_VALID); + page_set_flags(start, start + len, page_flags); the_end: #ifdef DEBUG_MMAP printf("ret=0x" TARGET_ABI_FMT_lx "\n", start); diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index dcdeb80176..5b7bdc3926 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -14144,10 +14144,10 @@ static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn) */ static bool is_guarded_page(CPUARMState *env, DisasContext *s) { -#ifdef CONFIG_USER_ONLY - return false; /* FIXME */ -#else uint64_t addr = s->base.pc_first; +#ifdef CONFIG_USER_ONLY + return page_get_flags(addr) & PAGE_TARGET_1; +#else int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx); unsigned int index = tlb_index(env, mmu_idx, addr); CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
There is agreement that there will be a mmap/mprotect bit, although no word yet on the value or the name. Invent a name to make forward progress. The PAGE_TARGET_1 bit, is qemu internal, and allows the target something to query from the guest page tables. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/exec/cpu-all.h | 12 +++++++++--- linux-user/syscall_defs.h | 5 +++++ linux-user/mmap.c | 13 ++++++++++++- target/arm/translate-a64.c | 6 +++--- 4 files changed, 29 insertions(+), 7 deletions(-) -- 2.17.1