[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime ADDVL i
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime ADDVL is executed |
Date: |
Thu, 14 Mar 2019 10:51:00 +0000 |
User-agent: |
mu4e 1.1.0; emacs 26.1 |
Amir CHARIF <address@hidden> writes:
> Hello,
> Thanks for your answer.
>
> The wrong size was definitely being stored in the TB, and, it only affected
> ADDVL/RDVL/ADDPL (i.e. not all instructions are wrong). Here is what I think
> was happening:
>
> - The kernel disables SVE in EL0 (ZEN= 01).
> - When the user space application is entered, the TB containing ADDVL has its
> length set to 0 (16 bytes), as we are in EL0 (so sve_exception_el!=0), and FP
> is enabled.
> - ADDVL is executed (without trapping) on the basis of the current
> length (16). (Nested function calls in the same context will cause a
> ton of ADDVL instructions to be executed with a vecsize of 16.)
So this looks like the error. Certainly the pseudo code says:
CheckSVEEnabled();
bits(64) operand1 = if n == 31 then SP[] else X[n];
bits(64) result = operand1 + (imm * (VL DIV 8));
if d == 31 then
SP[] = result;
else
X[d] = result;
so we should trap to the kernel and we won't without sve_access_check()
> - The next different SVE instruction traps as it should and the size is
> adjusted for the rest of the execution (but the stack space that was
> initially allocated is wrong so execution fails).
>
> So another fix would be to allow these instructions to trap by calling
> sve_access_check(), as is done with the rest of SVE instructions.
>
> I'm not that familiar with the instruction set, so could you please let me
> know if it is correct to call sve_access_check() in these instructions ?
>
> Thanks in advance,
>
> Best regards,
> Amir
>
> -----Message d'origine-----
> De : Richard Henderson <address@hidden>
> Envoyé : mercredi 13 mars 2019 17:35
> À : Amir CHARIF <address@hidden>; address@hidden
> Cc : address@hidden
> Objet : Re: [Qemu-devel] [PATCH] Re-evaluate SVE vector length everytime
> ADDVL is executed
>
> On 3/13/19 7:41 AM, Amir Charif wrote:
>> In system emulation mode, the kernel may internally use 16-byte vectors.
>> If this size is saved in the DisasContext before entering a userspace
>> app that uses higher SVE sizes, the wrong size may be allocated on the
>> stack resulting in corruption (segfaults in user space).
>> This fix evaluates the vector size at runtime (as opposed to
>> translation time) to always allocate the correct size on the stack (when
>> ADDVL is used).
>
> This is wrong.
>
> In particular, if the computation of VL is wrong for ADDVL, it is wrong for
> every other SVE instruction as well. Most of which cannot have VL computed
> at runtime like this.
>
> That is why we break the TB at every change to VL.
>
> Where do we "enter a userspace app" without breaking the TB and recomputing?
> As far as I know this must have executed an ERET to return from EL1 to EL0,
> which most definitely happens between TBs, or else no system calls would work
> at all.
>
> Do you have an example that provokes this failure?
>
>
> r~
--
Alex Bennée