lkml.org 
[lkml]   [2022]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] powerpc: Fix irq_soft_mask_set() and irq_soft_mask_return() with sanitizer
    Hi!

    On Tue, Aug 30, 2022 at 09:10:02AM +0000, Christophe Leroy wrote:
    > Le 30/08/2022 à 11:01, Nicholas Piggin a écrit :
    > > On Tue Aug 30, 2022 at 3:24 PM AEST, Christophe Leroy wrote:
    > >>> This is still slightly concerning to me. Is there any guarantee that the
    > >>> compiler would not use a different sequence for the address here?
    > >>>
    > >>> Maybe explicit r13 is required.
    > >>>
    > >>
    > >> local_paca is defined as:
    > >>
    > >> register struct paca_struct *local_paca asm("r13");

    And this is in global scope, making it a global register variable.

    > >> Why would the compiler use another register ?
    > >
    > > Hopefully it doesn't. Is it guaranteed that it won't?

    Yes, this is guaranteed.

    For a local register variable this is guaranteed only for operands to an
    extended inline asm; any other access to the variable does not have to
    put it in the specified register.

    But this is a global register variable. The only thing that would make
    this crash and burn is if *any* translation unit did not see this
    declaration: it could then use r13 (if that was allowed by the ABI in
    effect, heh).

    > > I'm sure Segher will be delighted with the creative asm in __do_IRQ
    > > and call_do_irq :) *Grabs popcorn*

    All that %% is blinding, yes.

    Inline tabs are bad taste.

    Operand names instead of numbers are great for obfuscation, and nothing
    else -- unless you have more than four or five operands, in which case
    you have bigger problems already.

    Oh, this function is a good example of proper use of local register asm,
    btw.

    Comments like "// Inputs" are just harmful. As is the "creative"
    indentation here. Both harm readability and do not help understanding
    in any other way either.

    The thing about inline asm is the smallest details change meaning of the
    whole, it is a very harsh environment, you are programming both in C and
    directly assembler code as well, and things have to be valid for both,
    although on the other hand there is almost no error checking. Keeping
    it small, simple, readable is paramount.

    The rules for using inline asm:

    0) Do no use inline asm.
    1) Use extended asm, unless you know all differences with basic asm, and
    you know you want that. And if you answer "yes I do" to the latter,
    you answered wrong to the former.
    2) Do not use toplevel asm.
    3) Do no use inline asm.
    4) Do no use inline asm.
    5) Do no use inline asm.

    Inline asm is a very powerful escape hatch. Like all emergency exits,
    you should not use them if you do not need them! :-)

    But, you are talking about the function calling and the frame change I
    bet :-) Both of these are only okay because everything is back as it
    was when this (single!) asm is done, and the state created is perfectly
    fine (this is very dependent on exact ABI used, etc.)

    I would have used real assembler code here (in a .s file). But there
    probably are reasons to do things this way, performance probably?


    Segher

    \
     
     \ /
      Last update: 2022-09-01 00:50    [W:7.049 / U:0.644 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site