| Submitter | Simon Glass |
|---|---|
| Date | Oct. 13, 2012, 12:26 a.m. |
| Message ID | <1350087972-1581-5-git-send-email-sjg@chromium.org> |
| Download | mbox | patch |
| Permalink | /patch/191271/ |
| State | Rejected, archived |
| Delegated to: | Tom Rini |
| Headers | show |
Comments
On 10/12/2012 06:26 PM, Simon Glass wrote: > From: Stefan Reinauer <reinauer@chromium.org> > > ChromeOS uses a GPT partition table to partition the disk. > However, Windows will refuse to install on a GPT partitioned > disk if there is no EFI available (Even if there is an MBR, too) > To hide the GPT partition table from Windows, we need to write > it with a header magic other than "EFI PART". To support old > and new systems, Check for the magic string "CHROMEOS" too. Surely if you wanted to install Windows on a disk containing ChromeOS, you would just wipe the disk and re-partition it? I suppose perhaps you're talking about dual-boot though? Either way, it doesn't see like a good idea to be using non-standard EFI signatures - especially if the idea is to hide the GPT from Windows, and presumably then have Windows use the MBR partitions, since that will end up with a decidedly non-standard partition setup; some partitions will only be represented in the MBR (those Windows creates) and some in GPT (presumably whatever ChromeOS created before).
On Mon, Oct 15, 2012 at 8:19 AM, Stephen Warren <swarren@wwwdotorg.org>wrote: > On 10/12/2012 06:26 PM, Simon Glass wrote: > > From: Stefan Reinauer <reinauer@chromium.org> > > > > ChromeOS uses a GPT partition table to partition the disk. > > However, Windows will refuse to install on a GPT partitioned > > disk if there is no EFI available (Even if there is an MBR, too) > > To hide the GPT partition table from Windows, we need to write > > it with a header magic other than "EFI PART". To support old > > and new systems, Check for the magic string "CHROMEOS" too. > > Surely if you wanted to install Windows on a disk containing ChromeOS, > you would just wipe the disk and re-partition it? I suppose perhaps > you're talking about dual-boot though? > Yes, this is only required if we're dual-booting on Windows and ChromeOS on the same disk. Either way, it doesn't see like a good idea to be using non-standard EFI > signatures - especially if the idea is to hide the GPT from Windows, and > presumably then have Windows use the MBR partitions, since that will end > up with a decidedly non-standard partition setup; some partitions will > only be represented in the MBR (those Windows creates) and some in GPT > (presumably whatever ChromeOS created before). > Yes, you will have to create a hybrid partition setup to make this work. It is unfortunate that Windows enforces this and there is no real way around it. This is a workaround specific to ChromeOS machines, and should be fixed differently in the long run (e.g. by using Tiano Core as a payload instead of SeaBIOS for booting Windows) Stefan
On 10/15/2012 11:17 AM, Stefan Reinauer wrote: > On Mon, Oct 15, 2012 at 8:19 AM, Stephen Warren <swarren@wwwdotorg.org > <mailto:swarren@wwwdotorg.org>> wrote: > > On 10/12/2012 06:26 PM, Simon Glass wrote: > > From: Stefan Reinauer <reinauer@chromium.org > <mailto:reinauer@chromium.org>> > > > > ChromeOS uses a GPT partition table to partition the disk. > > However, Windows will refuse to install on a GPT partitioned > > disk if there is no EFI available (Even if there is an MBR, too) > > To hide the GPT partition table from Windows, we need to write > > it with a header magic other than "EFI PART". To support old > > and new systems, Check for the magic string "CHROMEOS" too. > > Surely if you wanted to install Windows on a disk containing ChromeOS, > you would just wipe the disk and re-partition it? I suppose perhaps > you're talking about dual-boot though? > > > Yes, this is only required if we're dual-booting on Windows and ChromeOS > on the same disk. > > Either way, it doesn't see like a good idea to be using non-standard EFI > signatures - especially if the idea is to hide the GPT from Windows, and > presumably then have Windows use the MBR partitions, since that will end > up with a decidedly non-standard partition setup; some partitions will > only be represented in the MBR (those Windows creates) and some in GPT > (presumably whatever ChromeOS created before). > > > Yes, you will have to create a hybrid partition setup to make this work. > It is unfortunate that Windows enforces this and there is no real way > around it. Is this something common that someone using upstream U-Boot would care about, or is it something specific that should be contained in a ChromeOS U-Boot tree? I'm worried that applying this patch will just (a) support a situation that'll be very confusing to the user and (b) slightly de-stabilizes the U-Boot code in other situations by allowing non-standard (perhaps considered corrupt even) EFI partition tables. Still, I suppose I won't be impacted by case (a) so I probably shouldn't care much about it, and case (b) hopefully won't cause practical problems (famous last words), so I'm not opposed to this patch, it just feels slightly risky to me.
On Fri, Oct 12, 2012 at 05:26:10PM -0700, Simon Glass wrote: > From: Stefan Reinauer <reinauer@chromium.org> > > ChromeOS uses a GPT partition table to partition the disk. > However, Windows will refuse to install on a GPT partitioned > disk if there is no EFI available (Even if there is an MBR, too) > To hide the GPT partition table from Windows, we need to write > it with a header magic other than "EFI PART". To support old > and new systems, Check for the magic string "CHROMEOS" too. > > Signed-off-by: Stefan Reinauer <reinauer@chromium.org> > > Signed-off-by: Simon Glass <sjg@chromium.org> Stephen brings up a good point in that this is a ChromeOS specific work-around for a real issue. With my TI hat on, we've been pushing the idea that our releases will be mainline + patches, and those patches will be things that are either backports, works in progress or simply not mainlineable patches (enforce a CONFIG_BOOTDELAY of 3 for all boards, add mojo to just load uImages to their XIP address, etc). In my mind, this patch falls into the last grouping and I hope you're OK with just holding onto this as a not appropriate for mainline change. The rest of the series looks fine and no need to respin, I'll just skip this patch when I pick it up, assuming no further changes are needed elsewhere.
On Wed, Oct 17, 2012 at 4:36 PM, Tom Rini <trini@ti.com> wrote: > Stephen brings up a good point in that this is a ChromeOS specific > work-around for a real issue. With my TI hat on, we've been pushing > the idea that our releases will be mainline + patches, and those patches > will be things that are either backports, works in progress or simply > not mainlineable patches (enforce a CONFIG_BOOTDELAY of 3 for all > boards, add mojo to just load uImages to their XIP address, etc). In my > mind, this patch falls into the last grouping and I hope you're OK with > just holding onto this as a not appropriate for mainline change. > Yes, it's exactly that kind of change. > The rest of the series looks fine and no need to respin, I'll just skip > this patch when I pick it up, assuming no further changes are needed > elsewhere. > Great! Thanks a lot Stefan
Hi Tom, On Wed, Oct 17, 2012 at 4:39 PM, Stefan Reinauer <reinauer@google.com> wrote: > On Wed, Oct 17, 2012 at 4:36 PM, Tom Rini <trini@ti.com> wrote: >> >> Stephen brings up a good point in that this is a ChromeOS specific >> work-around for a real issue. With my TI hat on, we've been pushing >> the idea that our releases will be mainline + patches, and those patches >> will be things that are either backports, works in progress or simply >> not mainlineable patches (enforce a CONFIG_BOOTDELAY of 3 for all >> boards, add mojo to just load uImages to their XIP address, etc). In my >> mind, this patch falls into the last grouping and I hope you're OK with >> just holding onto this as a not appropriate for mainline change. > > > Yes, it's exactly that kind of change. > >> >> The rest of the series looks fine and no need to respin, I'll just skip >> this patch when I pick it up, assuming no further changes are needed >> elsewhere. > > > Great! Thanks a lot Good with me too, thanks Tom. > > Stefan > Regards, Simon
Patch
diff --git a/disk/part_efi.c b/disk/part_efi.c index 22a1dad..f2a2445 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -307,7 +307,8 @@ static int is_gpt_valid(block_dev_desc_t * dev_desc, unsigned long long lba, } /* Check the GPT header signature */ - if (le64_to_int(pgpt_head->signature) != GPT_HEADER_SIGNATURE) { + if ((le64_to_int(pgpt_head->signature) != GPT_HEADER_SIGNATURE) && + (le64_to_int(pgpt_head->signature) != GPT_HEADER_SIGNATURE2)) { printf("GUID Partition Table Header signature is wrong:" "0x%llX != 0x%llX\n", (unsigned long long)le64_to_int(pgpt_head->signature), diff --git a/disk/part_efi.h b/disk/part_efi.h index 5903e7c..07e590d 100644 --- a/disk/part_efi.h +++ b/disk/part_efi.h @@ -37,7 +37,9 @@ #define EFI_PMBR_OSTYPE_EFI_GPT 0xEE #define GPT_BLOCK_SIZE 512 -#define GPT_HEADER_SIGNATURE 0x5452415020494645ULL +#define GPT_HEADER_SIGNATURE 0x5452415020494645ULL +#define GPT_HEADER_SIGNATURE2 0x534f454d4f524843ULL + #define GPT_HEADER_REVISION_V1 0x00010000 #define GPT_PRIMARY_PARTITION_TABLE_LBA 1ULL #define GPT_ENTRY_NAME "gpt"