Okay, I think I understand it better now.
Patch set 4:Code-Review +1
5 comments:
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
* macro: find_free_mtrr
* Clobbers %eax, %ebx, %ecx, %edx.
* If not found %ebx is 0.
* If found MTRR_BASE is at %ecx.
Yes, how could it be more clear?
I would describe both return values, one is the free MTRR count and the other one is the MTRR base.
macro: find_free_mtrr
Note: the MTRR MSRs are contiguous, and alternating between BASE and MASK:
MTRR_PHYS_MASK(0) = MTRR_PHYS_BASE(0) + 1
MTRR_PHYS_BASE(n) = MTRR_PHYS_BASE(0) + 2 * n
MTRR_PHYS_MASK(n) = MTRR_PHYS_MASK(0) + 2 * n
Clobbers %eax, %ebx, %ecx, %edx.
Returns:
%ebx: Number of free MTRRs. If zero, there are no free MTRRs left.
%ecx: If %ebx is not zero, the MTRR_BASE of the first available MTRR.
2f means 2 in forward direction, i.e. […]
right. Personally, I'd prefer names so that code self-documents
1 backwards, line 40
Ack. I'd also use a self-documenting name
MTRRs come in pairs, one BASE followed by one MASK reg. We've checked a bit […]
Right. I would add a comment (suggestions welcome):
Decrement %ecx to obtain the MTRR base from the MTRR mask.
Patch Set #4, Line 133: inc %ecx
See above, returned %ecx is the BASE reg, +1 for the MASK reg.
Right. I wonder why "inc" is used here, but "incl" is used twice on the codepath below.
To view, visit change 37190. To unsubscribe, or for help writing mail filters, visit settings.