On Fri, Mar 25, 2011 at 11:55:32PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> On 25.03.2011 23:13, Mario Limonciello wrote:
> > + * util/grub-setup.c: Conditionalize the partition map copy on
> > floppy
> > + support, not on whether the target contains partitions.
> > +
> > + Otherwise, the BIOS on Dell Latitude E series laptops will
> > freeze
> > + during POST if an invalid partition table is contained in the PBR
> > + of the active partition when GRUB is installed to a partition.
>
> It's wrong to assume that no floppies contain partition tables. Also
> --allow-floppy is usually used for USB sticks which sometimes appear as
> floppies on some BIOSes and they pretty much contain the partition
> table. Bottom line is: if you see a partition table: preserve it.
Reading from floppies requires the floppy_probe function in boot.S,
doesn't it, which collides with the partition table? But it makes sense
to keep dest_partmap in the condition for safety anyway (and it saves a
call to grub_util_biosdisk_is_floppy), so we'd end up with:
if (dest_partmap ||
(!allow_floppy && !grub_util_biosdisk_is_floppy (dest_dev->disk)))
memcpy (boot_img + GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
tmp_img + GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
GRUB_BOOT_MACHINE_PART_END - GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC);
?
> But it's ok to preserve this contents if floppy support is disabled
> even if no partition table is discovered.
In the case Mario discovered, there was indeed no partition table (all
zeroes). Writing the floppy-probing code into that area caused it to
fail.
> A small logic lecture:
>
> Floppy support is: allow_floppy || grub_util_biosdisk_is_floppy
> (dest_dev->disk)
> Negation of it is either !(allow_floppy || grub_util_biosdisk_is_floppy
> (dest_dev->disk)) or !allow_floppy && !grub_util_biosdisk_is_floppy
> (dest_dev->disk), not what you wrote.
Agreed. The latter form is found elsewhere in grub-setup.
> Please don't move around unrelated parts.
The parts Mario moved around weren't unrelated. Since the case at hand
is that of installing to a partition boot record, this if block will
run:
if (! dest_partmap)
{
grub_util_warn (_("Attempting to install GRUB to a partitionless disk
or to a partition. This is a BAD idea."));
goto unable_to_embed;
}
In order to preserve the partition table (or lack thereof) in this case,
it was necessary to move the memcpy up above that test.
Cheers,
--
Colin Watson [[email protected]]
_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
