On 06/13/19 10:34, David Woodhouse wrote: > On Thu, 2019-06-13 at 06:28 +0000, Wu, Hao A wrote: >> Hello Ray, >> >> Do you have comment on this? >> >> Some inline comments below: >> >>> -----Original Message----- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >>> David Woodhouse >>> Sent: Thursday, June 13, 2019 5:43 AM >>> To: Wu, Hao A >>> Cc: devel@edk2.groups.io; Ni, Ray; Justen, Jordan L; Laszlo Ersek; Ard >>> Biesheuvel >>> Subject: [edk2-devel] [PATCH 11/10] OvmfPkg/Csm/LegacyBiosDxe: Correct >>> Legacy16GetTableAddress call for E820 data >>> >>> The DX register is supposed to contain the required alignment for the >>> allocation. It was zero, and SeaBIOS doesn't (well, didn't) cope well >>> with that. Set it appropriately, and set BX to indicate the regions >>> it's OK to allocate in too. That was already OK but let's make sure >>> it's initialised properly and not just working by chance. >>> >>> Also actually return an error if the allocation fails. Instead of going >>> all the way through into the CSM and just letting it have a bogus >>> pointer to the E82o data. >>> >>> Signed-off-by: David Woodhouse <dw...@infradead.org> >>> --- >>> I made SeaBIOS cope with the zero too: >>> https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/thread/4PH >>> W3O3Y3HJFENODFV5INBGDLZMXA5KE/ >>> >>> OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c | 3 +++ >> >> >> Not sure if it is the issue of my mail client, the new lines added are >> with LF line ending on my local machine after applying the patch. > > Hm, that's normal, isn't it? > > We still haven't fixed the repository to store LF line endings > internally and then let all the tools automatically do the right thing > by checking out to LF or CRLF as appropriate for the system you're > checking out on. > > I vaguely recall that Laszlo has some scripts which mangle an email and > make it apply with CRLF? But only vaguely... which is why I asked for a > git tree instead of trying to run the gauntlet of trying to apply > patches 1-10 of your series myself. > > I've pushed v2 with the E82o typo fixed, and using BX==0, to > http://git.infradead.org/users/dwmw2/edk2.git/shortlog/refs/heads/csm > git://http://git.infradead.org/users/dwmw2/edk2.git csm > > I'll try sending that with git-send-email directly rather than with via > my mailer, but IIRC that doesn't make any difference. CRLF conversions > are baked into every level — even SMTP converts to send CRLF on the > wire and often back again at the receive side.
You may be remembering this article: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers I think Hao may have missed "--keep-cr" with git-am (or "git config am.keepcr true" for a permanent setting). Leif recently implemented a python script that configures one's git clone the way we suggest: 1 5b3e695d8ac5 BaseTools: add centralized location for git config files 2 4eb0acb1e2be BaseTools: add script to configure local git options The conversion of the repo's internal representation to LF has not been forgotten (nor abandoned); it's just that I've personally learned to cope with CRLF, and everyone's got to pick their fights. I support the conversion but I have no capacity for working on it. Thanks, Laszlo >> >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c >>> b/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c >>> index 211750c012..e7766eb2b1 100644 >>> --- a/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c >>> +++ b/OvmfPkg/Csm/LegacyBiosDxe/LegacyBootSupport.c >>> @@ -928,7 +928,9 @@ GenericLegacyBoot ( >>> if (CopySize > Private->Legacy16Table->E820Length) { >>> ZeroMem (&Regs, sizeof (EFI_IA32_REGISTER_SET)); >>> Regs.X.AX = Legacy16GetTableAddress; >>> + Regs.X.BX = (UINT16) 0x3; // Region >> >> >> According to the spec: >> >> ''' >> BX = Allocation region >> 00 = Allocate from either 0xE0000 or 0xF0000 64 KB blocks. >> Bit 0 = 1 Allocate from 0xF0000 64 KB block >> Bit 1 = 1 Allocate from 0xE0000 64 KB block >> ''' >> >> I think the value 0 for BX is fine which indicates the allocation can >> happen in either ranges. Not sure if setting BX to 0x11 is a valid >> operation. > > Note, I may have lied in my reply to Jordan when I said that 0x11 is > what was already happening. The way SeaBIOS copes with zero is by > setting it to 3... *before* the debug print showing what it was set to. > >> With the above comments handled: >> Reviewed-by: Hao A Wu <hao.a...@intel.com> > > Thanks. > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42360): https://edk2.groups.io/g/devel/message/42360 Mute This Topic: https://groups.io/mt/32045939/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
