On 02/16/16 17:36, Jeremy Linton wrote: > > (trimming) > > |Not wishing to influence the discussion, just out of curiosity: Jeremy > |mentions "numerous other BlockIo protocol providers in edk2 bounce IO > |operations rather than simply allowing them to fail" -- can we see some > |examples? I wonder if, upon seeing that code, we could use "git blame" > |to find out *why* those workarounds had been introduced. > > It's the same case, grub2 fails to honor the alignment requirements. > > OvmfPkg/XenPvBlkDxe/BlockIo.c > > I thought when I originally posted that I had found a couple more > cases, but then when talking to Leif about it a while back, that was > the one case I found.
// // Grub2 does not appear to respect IoAlign of 512, so reallocate the // buffer here. // That's not an example that makes me especially happy. In fact, now that I know about it, I think I'll only tolerate it in OvmfPkg because it's related to Xen. (And because it's already there.) I'm actually pretty disappointed. The above XenPvBlkDxe code was committed to edk2 in October 2014 (5de8a35c62406), but grub2 has the bug to *this day*. If you grep the grub2 code at 25492a0f047c for "io_align", the only hit is in the structure definition, in "include/grub/efi/api.h". Apparently, noone has bothered in the past ~1.5 years to report and fix the bug in grub2, despite the bugfix looking quite manageable. Also, I don't think there is a recent change in edk2 that can be pinpointed as "breaking" grub2. So this example sort of makes me want to influence the discussion, after all. > Either way, the problem will eventually get solved in grub, but that > doesn't solve the problem of trying to boot existing OS's. Can you please explain the expected lifecycle of the firmware side bugfix? Assuming the patch gets applied, edk2 becomes able to work around the grub2 bug. How are users, for whom the OS installer (or the prebuilt image) fails to boot on their hardware, due to the grub2 bug, benefit from the edk2 bugfix? Are they expected to rebuild, and/or reflash, their firmware? Anyway, what worries me most is, once we add this kludge to edk2, we'll never know when it's safe to remove it. We'll be stuck with it forever. I think that's a bad prospect for the project that is supposed to be the reference implementation for UEFI. (Side note: I hope my argument resembles Ard's argument against the properties table closely enough! ;)) edk2 does include workarounds -- for hardware bugs. OS installers and prebuilt images are not hardware though. They can be refreshed. I think grub_efidisk_readwrite() in grub2 genuinely needs a fix, and that fix should be backported as far as a given distro cares. Beyond that, I think this patch should go into downstreams of edk2 that care about supporting very old distributions whose install media (or prebuilt images) cannot be refreshed with a fixed grub2. (Side note: I'm acutely aware of the fact that OVMF depends on some quirks in KVM, namely the MTRR emulation. That's a different story however: (a) we *did* try to fix it -- but failed, because it is rooted in physical hardware, and extremely hard to debug and reason about --, and (b) there was a very localizable, bisectable *change* in KVM that ultimately broke the user experience with OVMF, and had to be quirked for OVMF's sake. Neither of these traits seem to hold for the grub2 bug. Compat kludges have not been created equal.) Can you carry this patch in your downstream edk2 project? Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
