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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
