Re: [PATCH 00/14] rid PiSmmCpuDxeSmm of DB-encoded instructions

[Laszlo Ersek <lersek@redhat.com>]

On 02/03/18 01:45, Kinney, Michael D wrote:
> Laszlo,
> 
> Thanks for all the work on this series and the very
> detailed commit messages.
> 
> Liming's email on removing the .S and .asm files is an
> RFC.  We need to see this RFC approved before we can
> commit changes to remove .S and .asm files.  This should
> be a separate activity.

Sure, I can drop that patch, but then the PiSmmCpuDxeSmm changes in the
other patches will divert the NASM files from the .S and .asm files. Is
that (temporary) non-uniformity better than removing the .S and .asm files?

> One odd thing I see in this series is that the instruction
> patch label in the .nasm file is just a label and does not
> have any storage associated with it.

No, this is not correct; the storage that is associated with each of
these "patch labels" is the one byte (UINT8) directly following the
label -- whatever that byte might be. It is generally part of a totally
unrelated instruction.

In case we had to patch an immediate operand that happened to comprise
the very last byte(s) of a NASM source file, *then* we'd have to add one
dummy DB at the end, just so there was something that the label directly
refered to.

This is why UINT8 is a good type here, because it requires us to add the
least amount of padding.

> But in the C code
> the type UINT8 is used with the label which implies some
> storage.  Can we make the globals in C code be a pointer
> (maybe VOID *) instead of UINT8?

I don't think so. For building the addresses, we rely on the linker, and
the linker needs definitions (allocations) of objects. Your above
observation is correct (i.e. that storage is required), my addition to
that is that storage is *already* allocated (one UINT8 per patch label /
symbol).

Thanks!
Laszlo