Laszlo, Let's see if we can close on the timeline for the .S/.asm RFC this week.
I am concerned about making them UINT8 from C code because future maintainer may think that the patch value type is UINT8. Labels in assembly that are defined to be a function that is callable from C code does not have a storage type. Why can't we make these labels the same way? Mike > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Monday, February 5, 2018 2:28 AM > To: Kinney, Michael D <[email protected]>; edk2- > devel-01 <[email protected]> > Cc: Ard Biesheuvel <[email protected]>; Dong, > Eric <[email protected]>; Yao, Jiewen > <[email protected]>; Leif Lindholm > <[email protected]>; Gao, Liming > <[email protected]>; Ni, Ruiyu <[email protected]> > Subject: Re: [PATCH 00/14] rid PiSmmCpuDxeSmm of DB- > encoded instructions > > 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
