I'm not sure if the number labels work as expected. They can be avoided.

View Change

8 comments:

• File src/soc/intel/common/block/cpu/car/cache_as_ram.S:

  • Patch Set #4, Line 27:

     * macro: find_free_mtrr
    	 * Clobbers %eax, %ebx, %ecx, %edx.
    	 * If not found %ebx is 0.
    	 * If found MTRR_BASE is at %ecx.
    

    Is this macro returning a tuple as (%ebx, %ecx) ? The comment is not very clear.

  • Patch Set #4, Line 32: .macro find_free_mtrr

    nit: Maybe don't indent this?

  • Patch Set #4, Line 43: 2f

    What does this refer to? Maybe call it "mtrr_found" instead ?

  • Patch Set #4, Line 46: 1b

    Is this a loop?

  • Patch Set #4, Line 48: decl

    why decl here?

  • Patch Set #4, Line 122: cmp $0, %ebx

    I'd use the idiomatic way:

     test	%ebx, %ebx

  • Patch Set #4, Line 123: jne 1f

    je . […]

    Or even jz .halt_forever

  • Patch Set #4, Line 133: inc %ecx

    This looks wrong. Why is the mask the result of incrementing ecx?

To view, visit change 37190. To unsubscribe, or for help writing mail filters, visit settings.