Re: [PATCH v2 3/6] OvmfPkg: convert ES Reset Block structure to be guided
On 11/20/20 19:45, James Bottomley wrote:
Convert the current ES reset block structure to an extensible guid(1) Please update the subject line to:
OvmfPkg/ResetVector: convert SEV-ES Reset Block structure to be GUIDed
- edk2 prefers including module names too in the patch subjects
- "ES" is harder to understand than "SEV-ES"
- "GUIDed" is harder to misread as "guided"
- subject length is still OK (70 chars)
(2) This will insert 32 zero bytes if the size is already aligned to 32
bytes (because 32-0 = 32). In other words, the above produces 1 to 32
zero bytes, dependent on table size.
The variant I proposed in point (5) at
takes this into account, and only prepends 0 to 31 bytes (inclusive):
TIMES (31 - (guidedStructureEnd - guidedStructureStart + 31) % 32) DB 0
- This variant subtracts 1 inside the remainder operation (which is
expressed as adding 31).
- For compensation, it adds 1 just outside of the remainder operation.
This addition in effect increases the subtrahend for the leftmost 32.
Therefore this (-1) addend is ultimately folded into the leftmost 32,
yielding 31 on the leftmost side.
TIMES (32 - ((guidedStructureEnd - guidedStructureStart - 1) % 32 + 1)) DB 0
| in the
slide down residue
class modulo 32
TIMES (32 - ((guidedStructureEnd - guidedStructureStart + 31) % 32) - 1) DB 0
addition, to avoid
using % on a negative
integer (in case size
TIMES (31 - ((guidedStructureEnd - guidedStructureStart + 31) % 32)) DB 0
fold previous (-1) addend into leftmost constant
- This juggling of 1 results in no changes for residue classes 1 through
31, but wraps the outermost result (the padding size) for residue
class 0, from 32 to 0.
+(3) suggest "table footer GUID" (the GUID follows the data, in address
+; (96b582de-1fb2-45f7-baea-a366c55a082d) at 0xffffffd0. If that(4) can we make the whole table size field 32-bit? I don't have a
particular use case in mind, it just looks more extensible than 16-bit.
We can still keep the individual structs we have in mind 16-bit sized.
+;(5) This is hard to interpret, as "data" precedes "guid" in address
space (guid is a footer, not a header).
I suggest "length from start of data to end of GUID"
+; guid (16 bytes)(6) suggest "from the footer"
+; either find the guid you're looking for or run off the end of the(7) suggest "run off the beginning of the table"
... I realize "start" and "end" can be interpreted temporally and
spatially. In a forward traversal they are the same, but now they
aren't. I suggest we use the spatial (address space order)
+;(8) Did I understand from the v1 discussion that the corresponding QEMU
parser is not upstream yet? (Or at least not released?)
(9) The 16-bit size field of the SEV-ES reset block structure is not
;(10) I suggest "table footer" (twice)
+; guid: 96b582de-1fb2-45f7-baea-a366c55a082dThanks!