On Fri, Sep 13, 2013 at 12:26:39PM -0700, Davidlohr Bueso wrote: > On Fri, 2013-09-13 at 14:33 -0400, Matt Porter wrote: > > On 09/13/2013 02:29 PM, Davidlohr Bueso wrote: > > > On Fri, 2013-09-13 at 20:17 +0200, Karel Zak wrote: > > >> On Fri, Sep 13, 2013 at 02:09:55PM -0400, Matt Porter wrote: > > >>>> Come to think of it, we do have a long existing workaround: the > > >>>> force_gpt option. Setting it will bypass any MBR checking > > >>>> (is_pmbr_valid(), specifically). > > >>> > > >>> Yes, that's what I used at first after seeing what the problem was. But > > >>> then > > >>> I opted to fix my PMBR. > > >>> > > >>> I felt like it was a regression since it required a new option passed > > >>> on the > > >>> cmdline. > > >> > > >> Yep, it is *regression* and I guess Davidlohr is going to fix it asap > > >> :-) > > > > > > I was doing a git revert, but what would you guys think of keeping the > > > check but making it more flexible? Instead of enforcing the minimum, > > > just make sure that the size_in_lba is either the whole disk or 2 TiB, > > > that should take care of Matt's issue. > > > > That seems to be the way to go given the departure from the spec. > > Matt, could you please verify that this patch fixes your problem? > > Thanks! > > 8<------------------------------------------------- > > From: Davidlohr Bueso <davidl...@hp.com> > Subject: [PATCH] partitions/efi: loosen pmbr size in lba check > > Matt found that commit 27a7c64 (partitions/efi: account for pmbr size in lba) > caused his GPT formatted eMMC device not to boot. The reason is that this > commit > enforced Linux to always check the lesser of the whole disk or 2Tib for the > pMBR size in LBA. While most disk partitioning tools out there create a pMBR > with > these characteristics, Microsoft does not, as it always sets the entry to the > maximum 32-bit limitation - even though a drive may be smaller than that[1]. > > Loosen this check and only verify that the size is either the whole disk or > 0xFFFFFFFF. No tool in its right mind would set it to any value other than > these. > > [1] http://thestarman.pcministry.com/asm/mbr/GPT.htm#GPTPT > > Reported-by: Matt Porter <matt.por...@linaro.org> > Signed-off-by: Davidlohr Bueso <davidl...@hp.com> > --- > block/partitions/efi.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/block/partitions/efi.c b/block/partitions/efi.c > index 1a5ec9a..9bae49c 100644 > --- a/block/partitions/efi.c > +++ b/block/partitions/efi.c > @@ -186,6 +186,7 @@ invalid: > */ > static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors) > { > + uint32_t sz = 0; > int i, part = 0, ret = 0; /* invalid by default */ > > if (!mbr || le16_to_cpu(mbr->signature) != MSDOS_MBR_SIGNATURE) > @@ -216,12 +217,15 @@ check_hybrid: > /* > * Protective MBRs take up the lesser of the whole disk > * or 2 TiB (32bit LBA), ignoring the rest of the disk. > + * Some partitioning programs, nonetheless, choose to set > + * the size to the maximum 32-bit limitation, disregarding > + * the disk size. > * > * Hybrid MBRs do not necessarily comply with this. > */ > if (ret == GPT_MBR_PROTECTIVE) { > - if (le32_to_cpu(mbr->partition_record[part].size_in_lba) != > - min((uint32_t) total_sectors - 1, 0xFFFFFFFF)) > + sz = le32_to_cpu(mbr->partition_record[part].size_in_lba); > + if (sz != (uint32_t) total_sectors - 1 || sz != 0xFFFFFFFF)
Almost! You'll want to get that brown paper bag out cause I've worn it for this same type of bug before. Add this fix in and you can have my Tested-by: Matt Porter <matt.por...@linaro.org> diff --git a/block/partitions/efi.c b/block/partitions/efi.c index 9bae49c..1eb09ee 100644 --- a/block/partitions/efi.c +++ b/block/partitions/efi.c @@ -225,7 +225,7 @@ check_hybrid: */ if (ret == GPT_MBR_PROTECTIVE) { sz = le32_to_cpu(mbr->partition_record[part].size_in_lba); - if (sz != (uint32_t) total_sectors - 1 || sz != 0xFFFFFFFF) + if (sz != (uint32_t) total_sectors - 1 && sz != 0xFFFFFFFF) ret = 0; } done: Thanks, Matt -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/