[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH] Re-evaluate SVE vector length everyt

From: Alex Bennée
Subject: Re: [Qemu-arm] [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:

  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;
      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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]