qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode
@ 2019-01-25 21:48 Rémi Denis-Courmont
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-25 21:48 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

	Hello,

As things stand, ARMv8.3-PAuth does not work in user mode. These tiny patches 
attempt to fix that.

Br,

----------------------------------------------------------------
Remi Denis-Courmont (3):
      target/arm: fix AArch64 virtual address space size
      target/arm: actually enable PAuth in user mode
      target/arm: fix decoding of B{,L}RA{A,B}

 target/arm/cpu.h           | 2 +-
 target/arm/cpu64.c         | 4 ++--
 target/arm/translate-a64.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
Rémi Denis-Courmont
http://www.remlab.net/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
@ 2019-01-25 21:49 ` Rémi Denis-Courmont
  2019-01-25 23:29   ` Richard Henderson
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-25 21:49 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>

Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
extension (yet), the VA address space is signed 48-bits. User mode can
only handle the positive half of the address space, so that makes a
limit of 47 bits.

(With LVA, it would be 52 and 51 bits respectively.)

The incorrectly large address space conflicts with PAuth instructions,
which bits 48-54 and 56-63 for the pointer authentication code. This
also conflicts with (as yet unsupported by QEMU) data tagging and with
the ARMv8.5-MTE extension.

Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ff81db420d..2ccd04b8f7 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2503,7 +2503,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 
 #if defined(TARGET_AARCH64)
 #  define TARGET_PHYS_ADDR_SPACE_BITS 48
-#  define TARGET_VIRT_ADDR_SPACE_BITS 64
+#  define TARGET_VIRT_ADDR_SPACE_BITS 47
 #else
 #  define TARGET_PHYS_ADDR_SPACE_BITS 40
 #  define TARGET_VIRT_ADDR_SPACE_BITS 32
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 2/3] target/arm: actually enable PAuth in user mode
  2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
@ 2019-01-25 21:49 ` Rémi Denis-Courmont
  2019-01-25 23:34   ` Richard Henderson
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{,L}RA{A,B} Rémi Denis-Courmont
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-25 21:49 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>

This always enables IA, IB, DA and DB keys in user mode on the maximum
CPU, in a manner that is consistent with the other CPUs. That is to say
redefining the reset value of SCTLR_ELx registers.

Without this patch, the PAC* and AUT* instructions have no effects
(except PACGA of course).

Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index e9bc461c36..148c103ca4 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -413,8 +413,8 @@ static void aarch64_max_initfn(Object *obj)
                                 (void *)&apdb_bit, &error_fatal);
 
             /* Enable all PAC keys by default.  */
-            cpu->env.cp15.sctlr_el[1] |= SCTLR_EnIA | SCTLR_EnIB;
-            cpu->env.cp15.sctlr_el[1] |= SCTLR_EnDA | SCTLR_EnDB;
+            cpu->reset_sctlr |= SCTLR_EnIA | SCTLR_EnIB;
+            cpu->reset_sctlr |= SCTLR_EnDA | SCTLR_EnDB;
         }
 #endif
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{,L}RA{A,B}
  2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
@ 2019-01-25 21:49 ` Rémi Denis-Courmont
  2019-01-25 23:40   ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{, L}RA{A, B} Richard Henderson
  2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-25 21:49 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>

A flawed test lead to the instructions always being treated as
unallocated encodings.

Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 4d28a27c3b..c550a3f013 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -2036,7 +2036,7 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
         if (!dc_isar_feature(aa64_pauth, s)) {
             goto do_unallocated;
         }
-        if (op3 != 2 || op3 != 3) {
+        if ((op3 & ~1) != 2) {
             goto do_unallocated;
         }
         if (s->pauth_active) {
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
@ 2019-01-25 23:29   ` Richard Henderson
  2019-01-26  6:36     ` [Qemu-devel] [Qemu-arm] " Rémi Denis-Courmont
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2019-01-25 23:29 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 1:49 PM, Rémi Denis-Courmont wrote:
> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
> extension (yet), the VA address space is signed 48-bits. User mode can
> only handle the positive half of the address space, so that makes a
> limit of 47 bits.
> 
> (With LVA, it would be 52 and 51 bits respectively.)
> 
> The incorrectly large address space conflicts with PAuth instructions,
> which bits 48-54 and 56-63 for the pointer authentication code. This
> also conflicts with (as yet unsupported by QEMU) data tagging and with
> the ARMv8.5-MTE extension.
> 
> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index ff81db420d..2ccd04b8f7 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2503,7 +2503,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>  
>  #if defined(TARGET_AARCH64)
>  #  define TARGET_PHYS_ADDR_SPACE_BITS 48
> -#  define TARGET_VIRT_ADDR_SPACE_BITS 64
> +#  define TARGET_VIRT_ADDR_SPACE_BITS 47
>  #else
>  #  define TARGET_PHYS_ADDR_SPACE_BITS 40
>  #  define TARGET_VIRT_ADDR_SPACE_BITS 32
> 


This doesn't really conflict, as this doesn't affect much besides the sizing of
the PageDesc for page_find.

It's true adjusting this is worthwhile.  The current setting requires 7 levels
(6 * 10 bits + 1 * 4 bits), whereas a 48-bit address space only requires 5
levels (4 * 10 bits + 8 bits).  Even for LVA it will be (4 * 10 + 1 * 12 bits).

This will both save memory and reduce the time required for TranlationBlock
maintenance.

Your choice of 47 is wrong.  This value is also used for system mode, and the
kernel does use the negative half of the address space, so we need to have all
48 bits here.


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] target/arm: actually enable PAuth in user mode
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
@ 2019-01-25 23:34   ` Richard Henderson
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-01-25 23:34 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 1:49 PM, Rémi Denis-Courmont wrote:
> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This always enables IA, IB, DA and DB keys in user mode on the maximum
> CPU, in a manner that is consistent with the other CPUs. That is to say
> redefining the reset value of SCTLR_ELx registers.
> 
> Without this patch, the PAC* and AUT* instructions have no effects
> (except PACGA of course).
> 
> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu64.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index e9bc461c36..148c103ca4 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -413,8 +413,8 @@ static void aarch64_max_initfn(Object *obj)
>                                  (void *)&apdb_bit, &error_fatal);
>  
>              /* Enable all PAC keys by default.  */
> -            cpu->env.cp15.sctlr_el[1] |= SCTLR_EnIA | SCTLR_EnIB;
> -            cpu->env.cp15.sctlr_el[1] |= SCTLR_EnDA | SCTLR_EnDB;
> +            cpu->reset_sctlr |= SCTLR_EnIA | SCTLR_EnIB;
> +            cpu->reset_sctlr |= SCTLR_EnDA | SCTLR_EnDB;

I just sent another patch for this:
http://lists.nongnu.org/archive/html/qemu-devel/2019-01/msg06737.html

This way is valid as well, but would also need to adjust the property callbacks
to modify reset_sctlr as well.

Peter, do you have a preference?


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{, L}RA{A, B}
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{,L}RA{A,B} Rémi Denis-Courmont
@ 2019-01-25 23:40   ` Richard Henderson
  2019-01-27 19:06     ` Richard Henderson
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2019-01-25 23:40 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 1:49 PM, Rémi Denis-Courmont wrote:
> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> A flawed test lead to the instructions always being treated as
> unallocated encodings.
> 
> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/translate-a64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-25 23:29   ` Richard Henderson
@ 2019-01-26  6:36     ` Rémi Denis-Courmont
  2019-01-26  6:46       ` Rémi Denis-Courmont
  2019-01-26 20:00       ` Richard Henderson
  0 siblings, 2 replies; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-26  6:36 UTC (permalink / raw)
  To: qemu-arm; +Cc: Richard Henderson, qemu-devel

	Hi,

Le lauantaina 26. tammikuuta 2019, 1.29.27 EET Richard Henderson a écrit :
> On 1/25/19 1:49 PM, Rémi Denis-Courmont wrote:
> > From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> > 
> > Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
> > extension (yet), the VA address space is signed 48-bits. User mode can
> > only handle the positive half of the address space, so that makes a
> > limit of 47 bits.
> > 
> > (With LVA, it would be 52 and 51 bits respectively.)
> > 
> > The incorrectly large address space conflicts with PAuth instructions,
> > which bits 48-54 and 56-63 for the pointer authentication code. This
> > also conflicts with (as yet unsupported by QEMU) data tagging and with
> > the ARMv8.5-MTE extension.
> > 
> > Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> > ---
> > 
> >  target/arm/cpu.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index ff81db420d..2ccd04b8f7 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -2503,7 +2503,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
> > 
> >  #if defined(TARGET_AARCH64)
> >  #  define TARGET_PHYS_ADDR_SPACE_BITS 48
> > 
> > -#  define TARGET_VIRT_ADDR_SPACE_BITS 64
> > +#  define TARGET_VIRT_ADDR_SPACE_BITS 47
> > 
> >  #else
> >  #  define TARGET_PHYS_ADDR_SPACE_BITS 40
> >  #  define TARGET_VIRT_ADDR_SPACE_BITS 32
> 
> This doesn't really conflict, as this doesn't affect much besides the sizing
> of the PageDesc for page_find.

Uh, that value controls the range of target virtual addresses user mode maps 
to host virtual addresses. Without this patch, AFAICT, there are no 
guarantees, only chance, that emulated system calls such as mmap() will not 
allocate architecturally invalid addresses.

> It's true adjusting this is worthwhile.  The current setting requires 7
> levels (6 * 10 bits + 1 * 4 bits), whereas a 48-bit address space only
> requires 5 levels (4 * 10 bits + 8 bits).  Even for LVA it will be (4 * 10
> + 1 * 12 bits).

LVA only works with 64 KiB page size and user mode forces 4 KiB page size at 
the moment. And I somewhat doubt that somebody would ever need LVA in QEMU 
user mode. So that's highly hypothetical.

> This will both save memory and reduce the time required for TranlationBlock
> maintenance.
> 
> Your choice of 47 is wrong.  This value is also used for system mode,

No, it is not. System mode uses the other value, TARGET_PHYS_ADDR_SPACE_BITS, 
which this patch leaves untouched.

> and
> the kernel does use the negative half of the address space, so we need to
> have all 48 bits here.

I believe that TARGET_PHYS_ADDR_SPACE_BITS could be reduced to 56 bits. 
Anything less would presumably break handling of the address "sign" bit 55.

However, that would merely constitute an optimization, as there are no risk of 
generating architecturally invalid addresses at run-time in system mode.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-26  6:36     ` [Qemu-devel] [Qemu-arm] " Rémi Denis-Courmont
@ 2019-01-26  6:46       ` Rémi Denis-Courmont
  2019-01-26 20:00       ` Richard Henderson
  1 sibling, 0 replies; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-26  6:46 UTC (permalink / raw)
  To: qemu-arm, qemu-devel

Le lauantaina 26. tammikuuta 2019, 8.36.00 EET Rémi Denis-Courmont a écrit :
> I believe that TARGET_PHYS_ADDR_SPACE_BITS could be reduced to 56 bits.
> Anything less would presumably break handling of the address "sign" bit 55.

Err, sorry. That is already 48 bits and that is the correct value of course. 
There is no sign bit in physical address space.

-- 
レミ・デニ-クールモン
http://www.remlab.net/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCHv2 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
                   ` (2 preceding siblings ...)
  2019-01-25 21:49 ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{,L}RA{A,B} Rémi Denis-Courmont
@ 2019-01-26  6:52 ` Rémi Denis-Courmont
  2019-01-27 18:47   ` Richard Henderson
  2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
  2019-02-01 15:27 ` [Qemu-devel] [Qemu-arm] [PATCH 0/3] target/arm: fix " Peter Maydell
  5 siblings, 1 reply; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-26  6:52 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>

Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
extension (yet), the VA address space is 48-bits plus a sign bit. User
mode can only handle the positive half of the address space, so that
makes a limit of 48 bits.

(With LVA, it would be 53 and 52 bits respectively.)

The incorrectly large address space conflicts with PAuth instructions,
which bits 48-54 and 56-63 for the pointer authentication code. This
also conflicts with (as yet unsupported by QEMU) data tagging and with
the ARMv8.5-MTE extension.

Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ff81db420d..a3781600ba 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2503,7 +2503,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 
 #if defined(TARGET_AARCH64)
 #  define TARGET_PHYS_ADDR_SPACE_BITS 48
-#  define TARGET_VIRT_ADDR_SPACE_BITS 64
+#  define TARGET_VIRT_ADDR_SPACE_BITS 48
 #else
 #  define TARGET_PHYS_ADDR_SPACE_BITS 40
 #  define TARGET_VIRT_ADDR_SPACE_BITS 32
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCHv2 2/3] target/arm: actually enable PAuth in user mode
  2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
                   ` (3 preceding siblings ...)
  2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
@ 2019-01-26  6:52 ` Rémi Denis-Courmont
  2019-01-27 18:48   ` Richard Henderson
  2019-02-01 15:27 ` [Qemu-devel] [Qemu-arm] [PATCH 0/3] target/arm: fix " Peter Maydell
  5 siblings, 1 reply; 16+ messages in thread
From: Rémi Denis-Courmont @ 2019-01-26  6:52 UTC (permalink / raw)
  To: qemu-arm; +Cc: qemu-devel

From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>

This always enables IA, IB, DA and DB keys in user mode on the maximum
CPU, in a manner that is consistent with the other CPUs. That is to say
redefining the reset value of SCTLR_ELx registers.

Without this patch, the PAC* and AUT* instructions have no effects
(except PACGA of course).

Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 target/arm/cpu64.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index e9bc461c36..c8ed943c65 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -287,7 +287,7 @@ static void cpu_max_get_packey(Object *obj, Visitor *v, const char *name,
 {
     ARMCPU *cpu = ARM_CPU(obj);
     const uint64_t *bit = opaque;
-    bool enabled = (cpu->env.cp15.sctlr_el[1] & *bit) != 0;
+    bool enabled = (cpu->reset_sctlr & *bit) != 0;
 
     visit_type_bool(v, name, &enabled, errp);
 }
@@ -304,9 +304,9 @@ static void cpu_max_set_packey(Object *obj, Visitor *v, const char *name,
 
     if (!err) {
         if (enabled) {
-            cpu->env.cp15.sctlr_el[1] |= *bit;
+            cpu->reset_sctlr |= *bit;
         } else {
-            cpu->env.cp15.sctlr_el[1] &= ~*bit;
+            cpu->reset_sctlr &= ~*bit;
         }
     }
     error_propagate(errp, err);
@@ -413,8 +413,8 @@ static void aarch64_max_initfn(Object *obj)
                                 (void *)&apdb_bit, &error_fatal);
 
             /* Enable all PAC keys by default.  */
-            cpu->env.cp15.sctlr_el[1] |= SCTLR_EnIA | SCTLR_EnIB;
-            cpu->env.cp15.sctlr_el[1] |= SCTLR_EnDA | SCTLR_EnDB;
+            cpu->reset_sctlr |= SCTLR_EnIA | SCTLR_EnIB;
+            cpu->reset_sctlr |= SCTLR_EnDA | SCTLR_EnDB;
         }
 #endif
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-26  6:36     ` [Qemu-devel] [Qemu-arm] " Rémi Denis-Courmont
  2019-01-26  6:46       ` Rémi Denis-Courmont
@ 2019-01-26 20:00       ` Richard Henderson
  1 sibling, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-01-26 20:00 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 10:36 PM, Rémi Denis-Courmont wrote:
> 	Hi,
> 
> Le lauantaina 26. tammikuuta 2019, 1.29.27 EET Richard Henderson a écrit :
>> On 1/25/19 1:49 PM, Rémi Denis-Courmont wrote:
>>> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
>>>
>>> Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
>>> extension (yet), the VA address space is signed 48-bits. User mode can
>>> only handle the positive half of the address space, so that makes a
>>> limit of 47 bits.
>>>
>>> (With LVA, it would be 52 and 51 bits respectively.)
>>>
>>> The incorrectly large address space conflicts with PAuth instructions,
>>> which bits 48-54 and 56-63 for the pointer authentication code. This
>>> also conflicts with (as yet unsupported by QEMU) data tagging and with
>>> the ARMv8.5-MTE extension.
>>>
>>> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
>>> ---
>>>
>>>  target/arm/cpu.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>>> index ff81db420d..2ccd04b8f7 100644
>>> --- a/target/arm/cpu.h
>>> +++ b/target/arm/cpu.h
>>> @@ -2503,7 +2503,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>>>
>>>  #if defined(TARGET_AARCH64)
>>>  #  define TARGET_PHYS_ADDR_SPACE_BITS 48
>>>
>>> -#  define TARGET_VIRT_ADDR_SPACE_BITS 64
>>> +#  define TARGET_VIRT_ADDR_SPACE_BITS 47
>>>
>>>  #else
>>>  #  define TARGET_PHYS_ADDR_SPACE_BITS 40
>>>  #  define TARGET_VIRT_ADDR_SPACE_BITS 32
>>
>> This doesn't really conflict, as this doesn't affect much besides the sizing
>> of the PageDesc for page_find.
> 
> Uh, that value controls the range of target virtual addresses user mode maps 
> to host virtual addresses. Without this patch, AFAICT, there are no 
> guarantees, only chance, that emulated system calls such as mmap() will not 
> allocate architecturally invalid addresses.

Well, that's the theory, yes.

On the plus side, the host platform that virtually everyone uses, x86_64, also
uses a 48-bit virtual address space, so there's no chance of such an invalid
address.

On the minus side, the code within target_mmap is not sufficient to ensure that
mmap will not produce invalid addresses.

See 76393642ae6, where I extended target/alpha TARGET_VIRT_ADDR_SPACE_BITS to
63 to work around exactly this problem.  To wit: target_mmap loops forever
asking the kernel for an address below 2**40, which the kernel refused to provide.

So changing this value for target/arm runs the risk of causing
aarch64-linux-user to fail on a ppc64/s390x/sparc64 host.  For all of the tens
of people around the world that care about such a configuration.


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCHv2 1/3] target/arm: fix AArch64 virtual address space size
  2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
@ 2019-01-27 18:47   ` Richard Henderson
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-01-27 18:47 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 10:52 PM, Rémi Denis-Courmont wrote:
> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
> extension (yet), the VA address space is 48-bits plus a sign bit. User
> mode can only handle the positive half of the address space, so that
> makes a limit of 48 bits.
> 
> (With LVA, it would be 53 and 52 bits respectively.)
> 
> The incorrectly large address space conflicts with PAuth instructions,
> which bits 48-54 and 56-63 for the pointer authentication code. This
> also conflicts with (as yet unsupported by QEMU) data tagging and with
> the ARMv8.5-MTE extension.
> 
> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCHv2 2/3] target/arm: actually enable PAuth in user mode
  2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
@ 2019-01-27 18:48   ` Richard Henderson
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-01-27 18:48 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 10:52 PM, Rémi Denis-Courmont wrote:
> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> This always enables IA, IB, DA and DB keys in user mode on the maximum
> CPU, in a manner that is consistent with the other CPUs. That is to say
> redefining the reset value of SCTLR_ELx registers.
> 
> Without this patch, the PAC* and AUT* instructions have no effects
> (except PACGA of course).
> 
> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> ---
>  target/arm/cpu64.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{, L}RA{A, B}
  2019-01-25 23:40   ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{, L}RA{A, B} Richard Henderson
@ 2019-01-27 19:06     ` Richard Henderson
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-01-27 19:06 UTC (permalink / raw)
  To: Rémi Denis-Courmont, qemu-arm; +Cc: qemu-devel

On 1/25/19 3:40 PM, Richard Henderson wrote:
> On 1/25/19 1:49 PM, Rémi Denis-Courmont wrote:
>> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
>>
>> A flawed test lead to the instructions always being treated as
>> unallocated encodings.
>>
>> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
>> ---
>>  target/arm/translate-a64.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Fixes: https://bugs.launchpad.net/bugs/1813460


r~

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH 0/3] target/arm: fix PAuth in user mode
  2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
                   ` (4 preceding siblings ...)
  2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
@ 2019-02-01 15:27 ` Peter Maydell
  5 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2019-02-01 15:27 UTC (permalink / raw)
  To: Rémi Denis-Courmont; +Cc: qemu-arm, qemu-devel

On Fri, 25 Jan 2019 at 23:02, Rémi Denis-Courmont <remi@remlab.net> wrote:
>
>         Hello,
>
> As things stand, ARMv8.3-PAuth does not work in user mode. These tiny patches
> attempt to fix that.
>
> Br,
>
> ----------------------------------------------------------------
> Remi Denis-Courmont (3):
>       target/arm: fix AArch64 virtual address space size
>       target/arm: actually enable PAuth in user mode
>       target/arm: fix decoding of B{,L}RA{A,B}

Thanks for this patchset. I've applied patches 1 (v2) and 3
to my target-arm.next tree. (For patch 2 I prefer the approach
in Richard's patchset which deletes the cpu properties entirely.)

PS: for future patchsets, if you do a v2 of a patch it's better
to then resend the whole patchset as a fresh email thread (not a
followup. That makes our automated patch email handling tooling
happier as it can keep the v1 and v2 versions distinct.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-02-01 15:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 21:48 [Qemu-devel] [PATCH 0/3] target/arm: fix PAuth in user mode Rémi Denis-Courmont
2019-01-25 21:49 ` [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
2019-01-25 23:29   ` Richard Henderson
2019-01-26  6:36     ` [Qemu-devel] [Qemu-arm] " Rémi Denis-Courmont
2019-01-26  6:46       ` Rémi Denis-Courmont
2019-01-26 20:00       ` Richard Henderson
2019-01-25 21:49 ` [Qemu-devel] [PATCH 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
2019-01-25 23:34   ` Richard Henderson
2019-01-25 21:49 ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{,L}RA{A,B} Rémi Denis-Courmont
2019-01-25 23:40   ` [Qemu-devel] [PATCH 3/3] target/arm: fix decoding of B{, L}RA{A, B} Richard Henderson
2019-01-27 19:06     ` Richard Henderson
2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 1/3] target/arm: fix AArch64 virtual address space size Rémi Denis-Courmont
2019-01-27 18:47   ` Richard Henderson
2019-01-26  6:52 ` [Qemu-devel] [PATCHv2 2/3] target/arm: actually enable PAuth in user mode Rémi Denis-Courmont
2019-01-27 18:48   ` Richard Henderson
2019-02-01 15:27 ` [Qemu-devel] [Qemu-arm] [PATCH 0/3] target/arm: fix " Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).