lkml.org 
[lkml]   [2021]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v9 14/26] x86/arch_prctl: Create ARCH_SET_STATE_ENABLE/ARCH_GET_STATE_ENABLE
    Date
    On Friday, 30 July 2021 07:59:45 PDT Chang S. Bae wrote:
    > + for_each_thread(tsk, t) {
    > + t->thread.fpu.dynamic_state_perm |= req_dynstate_perm;
    > + nr_threads++;
    > + }
    > +
    > + if (nr_threads != tsk->signal->nr_threads) {
    > + for_each_thread(tsk, t)
    > + t->thread.fpu.dynamic_state_perm =
    > old_dynstate_perm;
    > + pr_err("x86/fpu: ARCH_XSTATE_PERM failed
    > as thread number mismatched.\n");
    > + return -EBUSY;
    > + }
    > + return 0;
    > +}

    Hello all

    As I was trying to write the matching userspace code, I think the solution
    above had two problems.

    First the simpler one: that EBUSY. It must go and you can do that with a lock.
    Library code cannot ensure that it is running in single-threaded state and
    that no other threads are started or exit while they make the system call.
    There's nothing the library in question can do if it got an EBUSY. Do you want
    me to try again? What if it fails again? What's the state of the dynamically
    permitted states after an EBUSY? It's probably inconsistent. Moreover, there's
    an ABA problem there: what happens if a thread starts and another exits while
    this system call is running? And what happens if two threads are making this
    system call?
    (also, shouldn't tsk->signal->nr_threads be an atomic read?)

    The second and bigger problem is the consequence of not issuing the
    ARCH_SET_STATE_ENABLE call: a SIGILL. Up until now, this hasn't happened, so I
    expect this to be a surprise to people, in the worst possible way. The Intel
    Software Developer Manual and every single tutorial out there says that the
    sequence of actions is:
    1) check that OSXSAVE is enabled
    2) check that the AVX, AVX512 or AMX instructions are supported with CPUID
    3) execute XGETBV EAX=0
    4) disable any instructions whose matching state is not enabled by the OS

    This is what software developers will write for AMX and any new future state,
    until they learn better. This is also all that other OSes will require to run.
    Moreover, until developers can actually run their software on CPUs with AMX
    support, they will not notice any missed system calls (the Software
    Development Emulator tool will execute the instructions whether you've issued
    the syscall or not).

    As a consequence, there's a large chance that a test escape like that will
    cause software to start crashing when run on AMX-capable CPUs when those start
    showing up and get enabled in public clouds.

    So I have to insist that the XGETBV instruction's result match exactly what is
    permitted to run. That means we either enable AMX unconditionally with no need
    for system calls (with or without XFD trapping to dynamically allocate more
    state), or that the XCR0 register be set without the AMX bits by default,
    until the system call is issued.

    --
    Thiago Macieira - thiago.macieira (AT) intel.com
    Software Architect - Intel DPG Cloud Engineering



    \
     
     \ /
      Last update: 2021-08-06 18:47    [W:4.308 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site