target/arm: fix PAuth in user mode

[Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size

Posted by Rémi Denis-Courmont 6 years, 9 months ago

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

Re: [Qemu-devel] [PATCH 1/3] target/arm: fix AArch64 virtual address space size

Posted by Richard Henderson 6 years, 9 months ago

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~

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] target/arm: fix AArch64 virtual address space size

Posted by Rémi Denis-Courmont 6 years, 9 months ago

	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/

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] target/arm: fix AArch64 virtual address space size

Posted by Rémi Denis-Courmont 6 years, 9 months ago

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/

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] target/arm: fix AArch64 virtual address space size

Posted by Richard Henderson 6 years, 9 months ago

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~