Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-02-02 Thread Laszlo Ersek
On 02/02/18 14:28, Leif Lindholm wrote: > On Fri, Feb 02, 2018 at 10:06:07AM +, Ard Biesheuvel wrote: >> On 31 January 2018 at 10:40, Laszlo Ersek wrote: >>> On 01/30/18 23:25, Kinney, Michael D wrote: Laszlo, I agree that the function is better than a macro. I thought

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-02-02 Thread Leif Lindholm
On Fri, Feb 02, 2018 at 10:06:07AM +, Ard Biesheuvel wrote: > On 31 January 2018 at 10:40, Laszlo Ersek wrote: > > On 01/30/18 23:25, Kinney, Michael D wrote: > >> Laszlo, > >> > >> I agree that the function is better than a macro. > >> > >> I thought of the alignment issues as well. CopyMem(

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-02-02 Thread Laszlo Ersek
> >> - If Ard and Leif say the API is only useful on x86, then I suggest that >> we implement the API separately for all arches (still in BaseLib): >> >> - On x86, we should simply open-code the unaligned accesses (like you >> originall suggested). The poin

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-02-02 Thread Ard Biesheuvel
;> PatchAssembly (&gAsmSmmCr4, AsmReadCr4 (), 4); >>> ... >>> } >>> >>> (I think it's fine to open-code the last argument as "4", >>> rather than >>> "sizeof (UINT32)", because for patching, we must have >&

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-02-01 Thread Laszlo Ersek
On 01/31/18 23:11, Kinney, Michael D wrote: > Laszlo, > > I agree the Unaligned functions have issues. > We should see if we could change the param type. > It should be a backwards compatible change to > go from a type specific pointer to VOID *. But > need to check with all supported compilers.

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-31 Thread Kinney, Michael D
gt;> ... > >> { > >> PatchAssembly (&gAsmSmmCr0, AsmReadCr0 (), 4); > >> PatchAssembly (&gAsmSmmCr3, AsmReadCr3 (), 4); > >> PatchAssembly (&gAsmSmmCr4, AsmReadCr4 (), 4); > >> ... > >> } > >> > >>

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-31 Thread Laszlo Ersek
On 01/31/18 06:54, Ni, Ruiyu wrote: > Laszlo, Mike, > Considering this patch doesn't make the code worse, > actually improved a tiny bit, can we firstly check in the three patches? I agree; the PatchAssembly() discussion is taking quite a bit of thought, meanwhile IA32 SMM is broken on KVM -- and

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-31 Thread Laszlo Ersek
e 8: >> WriteUnaligned64 ((UINT64 *)(BufferEnd) - 1, PatchValue)); >> break; >>    default: >> ASSERT (FALSE); >>    } >> } >> >> Mike >> >>> -Original Message- >>> From: Laszlo Ersek [mailto:ler...@redhat.co

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-31 Thread Laszlo Ersek
t; >> - the size is spelled out once per patching >> - the function name and the variable names make it clear >> we are patching >> separately compiled assembly code that was linked into >> the same >> module. >> >> What do you think? >> >

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Ni, Ruiyu
ao, Jiewen ; Dong, Eric Subject: Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup() On 01/30/18 21:31, Kinney, Michael D wrote: Laszlo, We have already used this technique in other NASM files to remove DBs. OK. Let us know if you have suggestions on how to mak

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Ni, Ruiyu
Ersek Sent: Tuesday, January 30, 2018 10:17 AM To: Kinney, Michael D ; edk2- devel-01 Cc: Ni, Ruiyu ; Paolo Bonzini ; Yao, Jiewen ; Dong, Eric Subject: Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup() On 01/30/18 18:22, Kinney, Michael D wrote: Laszlo, Th

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Kinney, Michael D
Michael D ; edk2- > devel-01 > Cc: Ni, Ruiyu ; Paolo Bonzini > ; Yao, Jiewen > ; Dong, Eric > Subject: Re: [edk2] [PATCH 1/3] > UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 > SmmStartup() > > On 01/30/18 21:31, Kinney, Michael D wrote: > > Laszlo, > >

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Laszlo Ersek
t; ; Kinney, Michael D >> >> Cc: Ni, Ruiyu ; Paolo Bonzini >> ; Yao, Jiewen >> ; Dong, Eric >> Subject: RE: [edk2] [PATCH 1/3] >> UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 >> SmmStartup() >> >> Laszlo, >> >> We have alre

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Laszlo Ersek
10:17 AM >> To: Kinney, Michael D ; edk2- >> devel-01 >> Cc: Ni, Ruiyu ; Paolo Bonzini >> ; Yao, Jiewen >> ; Dong, Eric >> Subject: Re: [edk2] [PATCH 1/3] >> UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 >> SmmStartup() >> >> On 01/3

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Kinney, Michael D
> To: Laszlo Ersek ; edk2-devel-01 > ; Kinney, Michael D > > Cc: Ni, Ruiyu ; Paolo Bonzini > ; Yao, Jiewen > ; Dong, Eric > Subject: RE: [edk2] [PATCH 1/3] > UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 > SmmStartup() > > Laszlo, > > We have already use

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Kinney, Michael D
01.org] > On Behalf Of Laszlo Ersek > Sent: Tuesday, January 30, 2018 10:17 AM > To: Kinney, Michael D ; edk2- > devel-01 > Cc: Ni, Ruiyu ; Paolo Bonzini > ; Yao, Jiewen > ; Dong, Eric > Subject: Re: [edk2] [PATCH 1/3] > UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 >

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Laszlo Ersek
On 01/30/18 18:22, Kinney, Michael D wrote: > Laszlo, > > The DBs can be removed if the label is moved after > the instruction and the patch is done to the label > minus the size of the patch value. Indeed I haven't thought of this. If I understand correctly, it means extern UINT8 gSmmCr0;

Re: [edk2] [PATCH 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: update comments in IA32 SmmStartup()

2018-01-30 Thread Kinney, Michael D
Laszlo, The DBs can be removed if the label is moved after the instruction and the patch is done to the label minus the size of the patch value. Mike > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] > On Behalf Of Laszlo Ersek > Sent: Tuesday, January 30,