Thanks Laszlo. I agree with you that we can support Base = 0, Length = MAX_ADDR+1. The last page is MAX_ADDR+1-PAGE_SIZE, which is a valid value.
I do not think it is necessary to support Base=MAX_ADDR+1, Length=0. It does not match any entry in the page table. Thank you Yao Jiewen From: Laszlo Ersek [mailto:[email protected]] Sent: Sunday, July 9, 2017 3:50 AM To: Yao, Jiewen <[email protected]> Cc: edk2-devel-01 <[email protected]> Subject: Re: memory protection crash in PiSmmCpuDxeSmm, Ia32 build with RAM above 4GB On 07/08/17 15:38, Yao, Jiewen wrote: > Thanks Laszlo. I think this is a special case we did not test before. And it > is a bug we need fix. > > Unfortunately, I am out of office these days with limited email access. I > just saw the email today. > > > I have a quick look at the code. Thank you for your help, and I'm sorry about disturbing you while you are out of office. > I believe we need add below check in > UefiCpuPkg\PiSmmCpuDxeSmm\SmmCpuMemoryManagement.c, > ConvertMemoryPageAttributes() > > ========================== > if (BaseAddress > MAX_ADDRESS) { > return RETURN_UNSUPPORTED; > } Mathematically speaking (not in C expressions), I think BaseAddress:=(MAX_ADDRESS+1) with Length:=0 is valid as well. (In theory anyway.) So I would replace this check as follows: if ((BaseAddress > 0) && ((BaseAddress - 1) > MAX_ADDRESS)) { return RETURN_UNSUPPORTED; } > if (Length > MAX_ADDRESS) { > return RETURN_UNSUPPORTED; > } Mathematically speaking (not in C expressions), I think BaseAddress:=0 with Length:=(MAX_ADDRESS+1) is valid too. (In theory anyway.) So, I would replace this check as follows: if ((Length > 0) && ((Length - 1) > MAX_ADDRESS)) { return RETURN_UNSUPPORTED; } > if ((Length != 0) && (BaseAddress > MAX_ADDRESS - (Length - 1))) { > return RETURN_UNSUPPORTED; > } Yes, this looks good. And, it would work correctly with the above two modifications as well; it will accept both of the mentioned "corner cases". (Anyway I haven't looked at the source, and the difference is purely theoretical.) > ========================== > to filter invalid address in IA32. > > > (Well, it is valid for OS, because OS may use PAE to match to lower. But it > is invalid for UEFI, because UEFI uses identical address) > > > Would you please file an HSD for that? What does HSD stand for? :) Either way, I've filed: https://bugzilla.tianocore.org/show_bug.cgi?id=624 Thank you! Laszlo >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Saturday, July 8, 2017 10:12 AM >> To: Yao, Jiewen <[email protected]<mailto:[email protected]>> >> Cc: edk2-devel-01 <[email protected]<mailto:[email protected]>> >> Subject: memory protection crash in PiSmmCpuDxeSmm, Ia32 build with RAM >> above 4GB >> >> Hi Jiewen, >> >> I just noticed that building OvmfIa32.dsc with -D SMM_REQUIRE, and then >> running the 32-bit guest with 4G RAM (of which 2GB are placed in 64-bit >> address pace), the guest crashes when PiSmmCpuDxeSmm tries to protect >> the memory range at 4GB. Please find the log attached (it ends with the >> crash). >> >> Is this expected to work? >> >> Thanks >> Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
