All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Improved EFI boot support
@ 2013-09-12 17:19 Jason Wessel
  2013-09-12 17:19 ` [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function Jason Wessel
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 17:19 UTC (permalink / raw)
  To: Openembedded-core; +Cc: dvhart

The goal of this patch set is to put the EFI ISO boot method on par
with the ISO PCBIOS boot method.  This also allows for the creation of
a single ISO image which can boot in "legacy boot" mode or EFI boot
mode.

In order to make this possible the cdrtools needed an update to the
very latest and this brings in an license change.  Given that change
it was not entirely clear to me if we needed to leave the older
package around.  I opted to remove the older package recipe since
there was no clean way to fail in the case you wanted to build EFI
boot media.

The bbnote that was previously in the bootimg.bbclass about EFI only
media being untested was a bit of an understatement, as the images
would not even build without errors due to the call to isohybrid.  I
removed any such warnings because that path has been fully vetted now.

Cheers,
Jason.


----------------------------------------------------------------
Jason Wessel (5):
      bootimage.bbclass: Move fat image creation into a function
      cdrtools-native: Update from 3.00 to 3.01a17
      grub-efi-native: Add support for EFI ISO images
      bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support
      grub-efi.bbclass: Add serial and graphics menu options

 meta/classes/bootimg.bbclass                       |  147 ++++++++++++--------
 meta/classes/grub-efi.bbclass                      |   41 ++++--
 meta/conf/machine/qemux86-64.conf                  |    2 +-
 meta/conf/machine/qemux86.conf                     |    2 +
 meta/recipes-bsp/grub/grub-efi-native_2.00.bb      |    8 +-
 ...s-native_3.00.bb => cdrtools-native_3.01a17.bb} |   13 +-
 6 files changed, 135 insertions(+), 78 deletions(-)
 rename meta/recipes-devtools/cdrtools/{cdrtools-native_3.00.bb => cdrtools-native_3.01a17.bb} (64%)


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function
  2013-09-12 17:19 [PATCH 0/5] Improved EFI boot support Jason Wessel
@ 2013-09-12 17:19 ` Jason Wessel
  2013-09-12 17:38   ` Darren Hart
  2013-09-12 17:19 ` [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17 Jason Wessel
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 17:19 UTC (permalink / raw)
  To: Openembedded-core; +Cc: dvhart

In order to call the fat image creation multiple times it needs to be
in its own function.  A future commit will make use of the new
function to additionally create EFI image files for use with an ISO.

[YOCTO #4100]

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 meta/classes/bootimg.bbclass |  110 ++++++++++++++++++++++--------------------
 1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
index b312e0d..2ed7017 100644
--- a/meta/classes/bootimg.bbclass
+++ b/meta/classes/bootimg.bbclass
@@ -128,6 +128,63 @@ build_iso() {
 	ln -s ${IMAGE_NAME}.iso ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.iso
 }
 
+build_fat_img() {
+	FATSOURCEDIR=$1
+	FATIMG=$2
+
+	# Calculate the size required for the final image including the
+	# data and filesystem overhead.
+	# Sectors: 512 bytes
+	#  Blocks: 1024 bytes
+
+	# Determine the sector count just for the data
+	SECTORS=$(expr $(du --apparent-size -ks ${FATSOURCEDIR} | cut -f 1) \* 2)
+
+	# Account for the filesystem overhead. This includes directory
+	# entries in the clusters as well as the FAT itself.
+	# Assumptions:
+	#   FAT32 (12 or 16 may be selected by mkdosfs, but the extra
+	#   padding will be minimal on those smaller images and not
+	#   worth the logic here to caclulate the smaller FAT sizes)
+	#   < 16 entries per directory
+	#   8.3 filenames only
+
+	# 32 bytes per dir entry
+	DIR_BYTES=$(expr $(find ${FATSOURCEDIR} | tail -n +2 | wc -l) \* 32)
+	# 32 bytes for every end-of-directory dir entry
+	DIR_BYTES=$(expr $DIR_BYTES + $(expr $(find ${FATSOURCEDIR} -type d | tail -n +2 | wc -l) \* 32))
+	# 4 bytes per FAT entry per sector of data
+	FAT_BYTES=$(expr $SECTORS \* 4)
+	# 4 bytes per FAT entry per end-of-cluster list
+	FAT_BYTES=$(expr $FAT_BYTES + $(expr $(find ${FATSOURCEDIR} -type d | tail -n +2 | wc -l) \* 4))
+
+	# Use a ceiling function to determine FS overhead in sectors
+	DIR_SECTORS=$(expr $(expr $DIR_BYTES + 511) / 512)
+	# There are two FATs on the image
+	FAT_SECTORS=$(expr $(expr $(expr $FAT_BYTES + 511) / 512) \* 2)
+	SECTORS=$(expr $SECTORS + $(expr $DIR_SECTORS + $FAT_SECTORS))
+
+	# Determine the final size in blocks accounting for some padding
+	BLOCKS=$(expr $(expr $SECTORS / 2) + ${BOOTIMG_EXTRA_SPACE})
+
+	# Ensure total sectors is an integral number of sectors per
+	# track or mcopy will complain. Sectors are 512 bytes, and we
+	# generate images with 32 sectors per track. This calculation is
+	# done in blocks, thus the mod by 16 instead of 32.
+	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
+
+	# mkdosfs will sometimes use FAT16 when it is not appropriate,
+	# resulting in a boot failure from SYSLINUX. Use FAT32 for
+	# images larger than 512MB, otherwise let mkdosfs decide.
+	if [ $(expr $BLOCKS / 1024) -gt 512 ]; then
+		FATSIZE="-F 32"
+	fi
+
+	mkdosfs ${FATSIZE} -n ${BOOTIMG_VOLUME_ID} -S 512 -C ${FATIMG} ${BLOCKS}
+	# Copy FATSOURCEDIR recursively into the image file directly
+	mcopy -i ${FATIMG} -s ${FATSOURCEDIR}/* ::/
+}
+
 build_hddimg() {
 	# Create an HDD image
 	if [ "${NOHDD}" != "1" ] ; then
@@ -140,58 +197,7 @@ build_hddimg() {
 			grubefi_hddimg_populate
 		fi
 
-		# Calculate the size required for the final image including the
-		# data and filesystem overhead.
-		# Sectors: 512 bytes
-		#  Blocks: 1024 bytes
-
-		# Determine the sector count just for the data
-		SECTORS=$(expr $(du --apparent-size -ks ${HDDDIR} | cut -f 1) \* 2)
-
-		# Account for the filesystem overhead. This includes directory
-		# entries in the clusters as well as the FAT itself.
-		# Assumptions:
-		#   FAT32 (12 or 16 may be selected by mkdosfs, but the extra
-		#   padding will be minimal on those smaller images and not
-		#   worth the logic here to caclulate the smaller FAT sizes)
-		#   < 16 entries per directory
-		#   8.3 filenames only
-
-		# 32 bytes per dir entry
-		DIR_BYTES=$(expr $(find ${HDDDIR} | tail -n +2 | wc -l) \* 32)
-		# 32 bytes for every end-of-directory dir entry
-		DIR_BYTES=$(expr $DIR_BYTES + $(expr $(find ${HDDDIR} -type d | tail -n +2 | wc -l) \* 32))
-		# 4 bytes per FAT entry per sector of data
-		FAT_BYTES=$(expr $SECTORS \* 4)
-		# 4 bytes per FAT entry per end-of-cluster list
-		FAT_BYTES=$(expr $FAT_BYTES + $(expr $(find ${HDDDIR} -type d | tail -n +2 | wc -l) \* 4))
-
-		# Use a ceiling function to determine FS overhead in sectors
-		DIR_SECTORS=$(expr $(expr $DIR_BYTES + 511) / 512)
-		# There are two FATs on the image
-		FAT_SECTORS=$(expr $(expr $(expr $FAT_BYTES + 511) / 512) \* 2)
-		SECTORS=$(expr $SECTORS + $(expr $DIR_SECTORS + $FAT_SECTORS))
-
-		# Determine the final size in blocks accounting for some padding
-		BLOCKS=$(expr $(expr $SECTORS / 2) + ${BOOTIMG_EXTRA_SPACE})
-
-		# Ensure total sectors is an integral number of sectors per
-		# track or mcopy will complain. Sectors are 512 bytes, and we
-		# generate images with 32 sectors per track. This calculation is
-		# done in blocks, thus the mod by 16 instead of 32.
-		BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
-
-		# mkdosfs will sometimes use FAT16 when it is not appropriate,
-		# resulting in a boot failure from SYSLINUX. Use FAT32 for
-		# images larger than 512MB, otherwise let mkdosfs decide.
-		if [ $(expr $BLOCKS / 1024) -gt 512 ]; then
-			FATSIZE="-F 32"
-		fi
-
-		IMG=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hddimg
-		mkdosfs ${FATSIZE} -n ${BOOTIMG_VOLUME_ID} -S 512 -C ${IMG} ${BLOCKS}
-		# Copy HDDDIR recursively into the image file directly
-		mcopy -i ${IMG} -s ${HDDDIR}/* ::/
+		build_fat_img ${HDDDIR} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hddimg
 
 		if [ "${PCBIOS}" = "1" ]; then
 			syslinux_hddimg_install
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17
  2013-09-12 17:19 [PATCH 0/5] Improved EFI boot support Jason Wessel
  2013-09-12 17:19 ` [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function Jason Wessel
@ 2013-09-12 17:19 ` Jason Wessel
  2013-09-12 17:40   ` Darren Hart
  2013-09-12 17:40   ` Saul Wold
  2013-09-12 17:19 ` [PATCH 3/5] grub-efi-native: Add support for EFI ISO images Jason Wessel
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 17:19 UTC (permalink / raw)
  To: Openembedded-core; +Cc: dvhart

The update is needed to support generation of EFI boot images that
work with optical media.  Specifically the "-eltorito-platform efi"
capability for mkisofs is needed.

[YOCTO #4100]

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 .../cdrtools/cdrtools-native_3.00.bb               |   30 -------------------
 .../cdrtools/cdrtools-native_3.01a17.bb            |   31 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 30 deletions(-)
 delete mode 100644 meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
 create mode 100644 meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb

diff --git a/meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb b/meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
deleted file mode 100644
index 7e4c381..0000000
--- a/meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
+++ /dev/null
@@ -1,30 +0,0 @@
-# cdrtools-native OE build file
-# Copyright (C) 2004-2006, Advanced Micro Devices, Inc.  All Rights Reserved
-# Released under the MIT license (see packages/COPYING)
-SUMMARY = "A set of tools for CD recording, including cdrecord"
-DESCRIPTION = "A set of tools for CD recording, including cdrecord"
-HOMEPAGE = "http://cdrecord.berlios.de/private/cdrecord.html"
-SECTION = "console/utils"
-LICENSE = "GPLv2"
-LIC_FILES_CHKSUM = "file://COPYING;md5=8d16123ffd39e649a5e4a6bc1de60e6d"
-PR = "r0"
-
-SRC_URI = "ftp://ftp.berlios.de/pub/cdrecord/cdrtools-${PV}.tar.bz2 \
-           file://no_usr_src.patch"
-
-SRC_URI[md5sum] = "f9fbab08fbd458b0d2312976d8c5f558"
-SRC_URI[sha256sum] = "7f9cb64820055573b880f77b2f16662a512518336ba95ab49228a1617973423d"
-
-inherit native
-
-STAGE_TEMP = "${WORKDIR}/image-temp"
-
-FILESPATH = "${FILE_DIRNAME}/cdrtools-native/"
-
-do_install() {
-	install -d ${STAGE_TEMP}
-	make install INS_BASE=${STAGE_TEMP}
-
-	install -d ${D}${bindir}/
-	install ${STAGE_TEMP}/bin/* ${D}${bindir}/
-}
diff --git a/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb b/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
new file mode 100644
index 0000000..4a7a160
--- /dev/null
+++ b/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
@@ -0,0 +1,31 @@
+# cdrtools-native OE build file
+# Copyright (C) 2004-2006, Advanced Micro Devices, Inc.  All Rights Reserved
+# Released under the MIT license (see packages/COPYING)
+SUMMARY = "A set of tools for CD recording, including cdrecord"
+DESCRIPTION = "A set of tools for CD recording, including cdrecord"
+HOMEPAGE = "http://cdrecord.berlios.de/private/cdrecord.html"
+SECTION = "console/utils"
+LICENSE = "GPLv2 & CDDL-1.0 & LGPLv2.1+"
+LIC_FILES_CHKSUM = "file://COPYING;md5=32f68170be424c2cd64804337726b312"
+PR = "r0"
+
+SRC_URI = "ftp://ftp.berlios.de/pub/cdrecord/alpha/cdrtools-${PV}.tar.bz2"
+
+SRC_URI[md5sum] = "4cef9db0cf15a770c52d65b00bbee2db"
+SRC_URI[sha256sum] = "3d613965b213ad83e4be0ba2535e784901839ea4d11a20a2beb6765f0eb76dfa"
+
+S = "${WORKDIR}/cdrtools-3.01"
+
+inherit native
+
+STAGE_TEMP = "${WORKDIR}/image-temp"
+
+FILESPATH = "${FILE_DIRNAME}/cdrtools-native/"
+
+do_install() {
+	install -d ${STAGE_TEMP}
+	make install INS_BASE=${STAGE_TEMP}
+
+	install -d ${D}${bindir}/
+	install ${STAGE_TEMP}/bin/* ${D}${bindir}/
+}
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/5] grub-efi-native: Add support for EFI ISO images
  2013-09-12 17:19 [PATCH 0/5] Improved EFI boot support Jason Wessel
  2013-09-12 17:19 ` [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function Jason Wessel
  2013-09-12 17:19 ` [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17 Jason Wessel
@ 2013-09-12 17:19 ` Jason Wessel
  2013-09-12 17:48   ` Darren Hart
  2013-09-12 17:19 ` [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support Jason Wessel
  2013-09-12 17:19 ` [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options Jason Wessel
  4 siblings, 1 reply; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 17:19 UTC (permalink / raw)
  To: Openembedded-core; +Cc: dvhart

The iso9660 file system support needs to be added to grub in order to
be able to correctly find the grub.cfg.  The grub commands to locate
the grub.cfg also needs to be encoded into grub's default
configuration.

This change allows the resulting grub binary to work both in the hard
drive / USB boot case or the optical media boot case.

[YOCTO #4100]

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 meta/recipes-bsp/grub/grub-efi-native_2.00.bb |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-bsp/grub/grub-efi-native_2.00.bb b/meta/recipes-bsp/grub/grub-efi-native_2.00.bb
index 2d3d68a..3cff838 100644
--- a/meta/recipes-bsp/grub/grub-efi-native_2.00.bb
+++ b/meta/recipes-bsp/grub/grub-efi-native_2.00.bb
@@ -66,9 +66,13 @@ EXTRA_OECONF = "--with-platform=efi --disable-grub-mkfont \
                 --enable-liblzma=no --enable-device-mapper=no --enable-libzfs=no"
 
 do_mkimage() {
-	./grub-mkimage -p /EFI/BOOT -d ./grub-core/ \
+	# Search for the grub.cfg on the local boot media with built in config file
+	echo 'search.file /EFI/BOOT/grub.cfg root' > cfg
+	echo 'set prefix=($root)/EFI/BOOT' >> cfg
+
+	./grub-mkimage -c cfg -p /EFI/BOOT -d ./grub-core/ \
 	               -O ${GRUB_TARGET}-efi -o ./${GRUB_IMAGE} \
-	               boot linux ext2 fat serial part_msdos part_gpt normal efi_gop
+	               boot linux ext2 fat serial part_msdos part_gpt normal efi_gop iso9660 search
 }
 addtask mkimage after do_compile before do_install
 
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support
  2013-09-12 17:19 [PATCH 0/5] Improved EFI boot support Jason Wessel
                   ` (2 preceding siblings ...)
  2013-09-12 17:19 ` [PATCH 3/5] grub-efi-native: Add support for EFI ISO images Jason Wessel
@ 2013-09-12 17:19 ` Jason Wessel
  2013-09-12 18:09   ` Darren Hart
  2013-09-12 17:19 ` [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options Jason Wessel
  4 siblings, 1 reply; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 17:19 UTC (permalink / raw)
  To: Openembedded-core; +Cc: dvhart

Using the latest mkisofs it is possible to generate 3 different types
of ISO images, which can be used in various scenarios.

1) PCBIOS Only ISO
   - This option remains unchanged by this commit
   - Uses syslinux menus
   - Can be directly copied with dd to a USB device
   - Can be burned to optical media

2) EFI Only ISO
   - Uses grub 2 menus
   - Can be burned to optical media
   - If you want to use this image on a USB device
     extra steps must be taken in order to format the USB
     device with fat32, and copy an EFI loader which will
     in turn load the iso image

3) PCBIOS / EFI ISO
   - This is a hybrid image ISO that will work for case 1 or 2
     as above with the same restrictions and boot menu types
     depending on what type of firmware is installed on
     the hardware or depending on if EFI or "Legacy Boot" is
     enabled on some UEFI firmwares.

[YOCTO #4100]

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 meta/classes/bootimg.bbclass |   37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
index 2ed7017..b4301e8 100644
--- a/meta/classes/bootimg.bbclass
+++ b/meta/classes/bootimg.bbclass
@@ -35,6 +35,7 @@ EXCLUDE_FROM_WORLD = "1"
 
 HDDDIR = "${S}/hddimg"
 ISODIR = "${S}/iso"
+EFIIMGDIR = "${S}/efi_img"
 COMPACT_ISODIR = "${S}/iso.z"
 
 BOOTIMG_VOLUME_ID   ?= "boot"
@@ -109,19 +110,49 @@ build_iso() {
 		mkisofs_opts="-R -z -D -l"
 	fi
 
-	if [ "${PCBIOS}" = "1" ]; then
+	if [ "${EFI}" = "1" ] ; then
+		# Build a EFI directory to create efi.img
+		mkdir -p ${EFIIMGDIR}/${EFIDIR}
+		cp ${ISODIR}/${EFIDIR}/* ${EFIIMGDIR}${EFIDIR}
+		cp ${ISODIR}/vmlinuz ${EFIIMGDIR}
+		GRUB_IMAGE="bootia32.efi"
+		if [ "${TARGET_ARCH}" = "x86_64" ]; then
+			GRUB_IMAGE="bootx64.efi"
+		fi
+		echo "EFI\\BOOT\\${GRUB_IMAGE}" > ${EFIIMGDIR}/startup.nsh
+		if [ -f "${ISODIR}/initrd" ] ; then
+			cp ${ISODIR}/initrd ${EFIIMGDIR}
+		fi
+		build_fat_img ${EFIIMGDIR} ${ISODIR}/efi.img
+	fi
+
+	# Three ways to build the media
+	if [ "${PCBIOS}" = "1" -a "${EFI}" = "1" ]; then
+		# 1) Build EFI + PCBIOS hybrid
+		mkisofs -A ${BOOTIMG_VOLUME_ID} -V ${BOOTIMG_VOLUME_ID} \
+		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
+			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
+			${MKISOFS_OPTIONS} \
+			-eltorito-alt-boot -eltorito-platform efi \
+			-b efi.img -no-emul-boot \
+			${ISODIR}
+	elif [ "${PCBIOS}" = "1" ] ; then
+		# 2) Build PCBIOS only media
 		mkisofs -V ${BOOTIMG_VOLUME_ID} \
 		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
 			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
 			${MKISOFS_OPTIONS} ${ISODIR}
 	else
-		bbnote "EFI-only ISO images are untested, please provide feedback."
 		mkisofs -V ${BOOTIMG_VOLUME_ID} \
 		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
+			-eltorito-alt-boot -eltorito-platform efi \
+			-b efi.img -no-emul-boot \
 			-r ${ISODIR}
 	fi
 
-	isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
+	if [ "${PCBIOS}" = "1" ]; then
+		isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
+	fi
 
 	cd ${DEPLOY_DIR_IMAGE}
 	rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.iso
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 17:19 [PATCH 0/5] Improved EFI boot support Jason Wessel
                   ` (3 preceding siblings ...)
  2013-09-12 17:19 ` [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support Jason Wessel
@ 2013-09-12 17:19 ` Jason Wessel
  2013-09-12 18:01   ` Saul Wold
  2013-09-12 18:16   ` Darren Hart
  4 siblings, 2 replies; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 17:19 UTC (permalink / raw)
  To: Openembedded-core; +Cc: dvhart

The syslinux.bbclass already has support for automatically generated
serial and graphics menu choices.  This patch adds the same concept to
the grub-efi menu.  That makes it possible to generate a single image
which can boot on a PCBIOS or EFI firmware with consistent looking
boot options.

[YOCTO #4100]

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
 meta/conf/machine/qemux86-64.conf |    2 +-
 meta/conf/machine/qemux86.conf    |    2 ++
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
index c6f5d4e..c07e4a1 100644
--- a/meta/classes/grub-efi.bbclass
+++ b/meta/classes/grub-efi.bbclass
@@ -9,6 +9,7 @@
 # External variables
 # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
 # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
+# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
 # ${LABELS} - a list of targets for the automatic config
 # ${APPEND} - an override list of append strings for each label
 # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
@@ -16,6 +17,7 @@
 
 do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
 
+GRUB_SERIAL ?= "console=ttyS0,115200"
 GRUBCFG = "${S}/grub.cfg"
 GRUB_TIMEOUT ?= "10"
 #FIXME: build this from the machine config
@@ -55,6 +57,8 @@ python build_grub_cfg() {
         bb.error("WORKDIR not defined, unable to package")
         return
 
+    gfxserial = d.getVar('GRUB_GFXSERIAL', True) or ""
+
     labels = d.getVar('LABELS', True)
     if not labels:
         bb.debug(1, "LABELS not defined, nothing to do")
@@ -88,6 +92,12 @@ python build_grub_cfg() {
     else:
         cfgfile.write('timeout=50\n')
 
+    if gfxserial == "1":
+        btypes = [ [ " graphics console", "console=tty0" ],
+            [ " serial console", d.getVar('GRUB_SERIAL', True) or "" ] ]
+    else:
+        btypes = [ [ "", "" ] ]
+
     for label in labels.split():
         localdata = d.createCopy()
 
@@ -95,24 +105,27 @@ python build_grub_cfg() {
         if not overrides:
             raise bb.build.FuncFailed('OVERRIDES not defined')
 
-        localdata.setVar('OVERRIDES', label + ':' + overrides)
-        bb.data.update_data(localdata)
+        for btype in btypes:
+            localdata.setVar('OVERRIDES', label + ':' + overrides)
+            bb.data.update_data(localdata)
 
-        cfgfile.write('\nmenuentry \'%s\'{\n' % (label))
-        if label == "install":
-            label = "install-efi"
-        cfgfile.write('linux /vmlinuz LABEL=%s' % (label))
+            cfgfile.write('\nmenuentry \'%s%s\'{\n' % (label, btype[0]))
+            lb = label
+            if label == "install":
+                lb = "install-efi"
+            cfgfile.write('linux /vmlinuz LABEL=%s' % (lb))
 
-        append = localdata.getVar('APPEND', True)
-        initrd = localdata.getVar('INITRD', True)
+            append = localdata.getVar('APPEND', True)
+            initrd = localdata.getVar('INITRD', True)
 
-        if append:
-            cfgfile.write('%s' % (append))
-        cfgfile.write('\n')
+            if append:
+                cfgfile.write('%s' % (append))
+            cfgfile.write(' %s' % btype[1])
+            cfgfile.write('\n')
 
-        if initrd:
-            cfgfile.write('initrd /initrd')
-        cfgfile.write('\n}\n')
+            if initrd:
+                cfgfile.write('initrd /initrd')
+            cfgfile.write('\n}\n')
 
     cfgfile.close()
 }
diff --git a/meta/conf/machine/qemux86-64.conf b/meta/conf/machine/qemux86-64.conf
index c572225..6f68410 100644
--- a/meta/conf/machine/qemux86-64.conf
+++ b/meta/conf/machine/qemux86-64.conf
@@ -21,6 +21,6 @@ XSERVER = "xserver-xorg \
            xf86-input-evdev \
            xf86-video-vmware"
 
-MACHINE_FEATURES += "x86"
+MACHINE_FEATURES += "x86 efi"
 
 MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
diff --git a/meta/conf/machine/qemux86.conf b/meta/conf/machine/qemux86.conf
index 94ee573..57a9a50 100644
--- a/meta/conf/machine/qemux86.conf
+++ b/meta/conf/machine/qemux86.conf
@@ -22,5 +22,7 @@ XSERVER = "xserver-xorg \
            xf86-video-vmware"
 
 MACHINE_FEATURES += "x86"
+MACHINE_FEATURES += "efi"
+#MACHINE_FEATURES += "pcbios"
 
 MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function
  2013-09-12 17:19 ` [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function Jason Wessel
@ 2013-09-12 17:38   ` Darren Hart
  0 siblings, 0 replies; 22+ messages in thread
From: Darren Hart @ 2013-09-12 17:38 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> In order to call the fat image creation multiple times it needs to be
> in its own function.  A future commit will make use of the new
> function to additionally create EFI image files for use with an ISO.
> 
> [YOCTO #4100]
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

Thanks Jason!

Also, see https://bugzilla.yoctoproject.org/show_bug.cgi?id=1913. I
think you can add [YOCTO #1913] to your commit message. This isn't in a
new bbclass, but I think that's fine. The abstraction was the important
thing.

Acked-by: Darren Hart <dvhart@linux.intel.com>


> ---
>  meta/classes/bootimg.bbclass |  110 ++++++++++++++++++++++--------------------
>  1 file changed, 58 insertions(+), 52 deletions(-)
> 
> diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
> index b312e0d..2ed7017 100644
> --- a/meta/classes/bootimg.bbclass
> +++ b/meta/classes/bootimg.bbclass
> @@ -128,6 +128,63 @@ build_iso() {
>  	ln -s ${IMAGE_NAME}.iso ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.iso
>  }
>  
> +build_fat_img() {
> +	FATSOURCEDIR=$1
> +	FATIMG=$2
> +
> +	# Calculate the size required for the final image including the
> +	# data and filesystem overhead.
> +	# Sectors: 512 bytes
> +	#  Blocks: 1024 bytes
> +
> +	# Determine the sector count just for the data
> +	SECTORS=$(expr $(du --apparent-size -ks ${FATSOURCEDIR} | cut -f 1) \* 2)
> +
> +	# Account for the filesystem overhead. This includes directory
> +	# entries in the clusters as well as the FAT itself.
> +	# Assumptions:
> +	#   FAT32 (12 or 16 may be selected by mkdosfs, but the extra
> +	#   padding will be minimal on those smaller images and not
> +	#   worth the logic here to caclulate the smaller FAT sizes)
> +	#   < 16 entries per directory
> +	#   8.3 filenames only
> +
> +	# 32 bytes per dir entry
> +	DIR_BYTES=$(expr $(find ${FATSOURCEDIR} | tail -n +2 | wc -l) \* 32)
> +	# 32 bytes for every end-of-directory dir entry
> +	DIR_BYTES=$(expr $DIR_BYTES + $(expr $(find ${FATSOURCEDIR} -type d | tail -n +2 | wc -l) \* 32))
> +	# 4 bytes per FAT entry per sector of data
> +	FAT_BYTES=$(expr $SECTORS \* 4)
> +	# 4 bytes per FAT entry per end-of-cluster list
> +	FAT_BYTES=$(expr $FAT_BYTES + $(expr $(find ${FATSOURCEDIR} -type d | tail -n +2 | wc -l) \* 4))
> +
> +	# Use a ceiling function to determine FS overhead in sectors
> +	DIR_SECTORS=$(expr $(expr $DIR_BYTES + 511) / 512)
> +	# There are two FATs on the image
> +	FAT_SECTORS=$(expr $(expr $(expr $FAT_BYTES + 511) / 512) \* 2)
> +	SECTORS=$(expr $SECTORS + $(expr $DIR_SECTORS + $FAT_SECTORS))
> +
> +	# Determine the final size in blocks accounting for some padding
> +	BLOCKS=$(expr $(expr $SECTORS / 2) + ${BOOTIMG_EXTRA_SPACE})
> +
> +	# Ensure total sectors is an integral number of sectors per
> +	# track or mcopy will complain. Sectors are 512 bytes, and we
> +	# generate images with 32 sectors per track. This calculation is
> +	# done in blocks, thus the mod by 16 instead of 32.
> +	BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
> +
> +	# mkdosfs will sometimes use FAT16 when it is not appropriate,
> +	# resulting in a boot failure from SYSLINUX. Use FAT32 for
> +	# images larger than 512MB, otherwise let mkdosfs decide.
> +	if [ $(expr $BLOCKS / 1024) -gt 512 ]; then
> +		FATSIZE="-F 32"
> +	fi
> +
> +	mkdosfs ${FATSIZE} -n ${BOOTIMG_VOLUME_ID} -S 512 -C ${FATIMG} ${BLOCKS}
> +	# Copy FATSOURCEDIR recursively into the image file directly
> +	mcopy -i ${FATIMG} -s ${FATSOURCEDIR}/* ::/
> +}
> +
>  build_hddimg() {
>  	# Create an HDD image
>  	if [ "${NOHDD}" != "1" ] ; then
> @@ -140,58 +197,7 @@ build_hddimg() {
>  			grubefi_hddimg_populate
>  		fi
>  
> -		# Calculate the size required for the final image including the
> -		# data and filesystem overhead.
> -		# Sectors: 512 bytes
> -		#  Blocks: 1024 bytes
> -
> -		# Determine the sector count just for the data
> -		SECTORS=$(expr $(du --apparent-size -ks ${HDDDIR} | cut -f 1) \* 2)
> -
> -		# Account for the filesystem overhead. This includes directory
> -		# entries in the clusters as well as the FAT itself.
> -		# Assumptions:
> -		#   FAT32 (12 or 16 may be selected by mkdosfs, but the extra
> -		#   padding will be minimal on those smaller images and not
> -		#   worth the logic here to caclulate the smaller FAT sizes)
> -		#   < 16 entries per directory
> -		#   8.3 filenames only
> -
> -		# 32 bytes per dir entry
> -		DIR_BYTES=$(expr $(find ${HDDDIR} | tail -n +2 | wc -l) \* 32)
> -		# 32 bytes for every end-of-directory dir entry
> -		DIR_BYTES=$(expr $DIR_BYTES + $(expr $(find ${HDDDIR} -type d | tail -n +2 | wc -l) \* 32))
> -		# 4 bytes per FAT entry per sector of data
> -		FAT_BYTES=$(expr $SECTORS \* 4)
> -		# 4 bytes per FAT entry per end-of-cluster list
> -		FAT_BYTES=$(expr $FAT_BYTES + $(expr $(find ${HDDDIR} -type d | tail -n +2 | wc -l) \* 4))
> -
> -		# Use a ceiling function to determine FS overhead in sectors
> -		DIR_SECTORS=$(expr $(expr $DIR_BYTES + 511) / 512)
> -		# There are two FATs on the image
> -		FAT_SECTORS=$(expr $(expr $(expr $FAT_BYTES + 511) / 512) \* 2)
> -		SECTORS=$(expr $SECTORS + $(expr $DIR_SECTORS + $FAT_SECTORS))
> -
> -		# Determine the final size in blocks accounting for some padding
> -		BLOCKS=$(expr $(expr $SECTORS / 2) + ${BOOTIMG_EXTRA_SPACE})
> -
> -		# Ensure total sectors is an integral number of sectors per
> -		# track or mcopy will complain. Sectors are 512 bytes, and we
> -		# generate images with 32 sectors per track. This calculation is
> -		# done in blocks, thus the mod by 16 instead of 32.
> -		BLOCKS=$(expr $BLOCKS + $(expr 16 - $(expr $BLOCKS % 16)))
> -
> -		# mkdosfs will sometimes use FAT16 when it is not appropriate,
> -		# resulting in a boot failure from SYSLINUX. Use FAT32 for
> -		# images larger than 512MB, otherwise let mkdosfs decide.
> -		if [ $(expr $BLOCKS / 1024) -gt 512 ]; then
> -			FATSIZE="-F 32"
> -		fi
> -
> -		IMG=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hddimg
> -		mkdosfs ${FATSIZE} -n ${BOOTIMG_VOLUME_ID} -S 512 -C ${IMG} ${BLOCKS}
> -		# Copy HDDDIR recursively into the image file directly
> -		mcopy -i ${IMG} -s ${HDDDIR}/* ::/
> +		build_fat_img ${HDDDIR} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hddimg
>  
>  		if [ "${PCBIOS}" = "1" ]; then
>  			syslinux_hddimg_install

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17
  2013-09-12 17:19 ` [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17 Jason Wessel
@ 2013-09-12 17:40   ` Darren Hart
  2013-09-12 17:40   ` Saul Wold
  1 sibling, 0 replies; 22+ messages in thread
From: Darren Hart @ 2013-09-12 17:40 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> The update is needed to support generation of EFI boot images that
> work with optical media.  Specifically the "-eltorito-platform efi"
> capability for mkisofs is needed.
> 
> [YOCTO #4100]
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  .../cdrtools/cdrtools-native_3.00.bb               |   30 -------------------
>  .../cdrtools/cdrtools-native_3.01a17.bb            |   31 ++++++++++++++++++++
>  2 files changed, 31 insertions(+), 30 deletions(-)
>  delete mode 100644 meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
>  create mode 100644 meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
> 

...

> diff --git a/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb b/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
> new file mode 100644
> index 0000000..4a7a160
> --- /dev/null
> +++ b/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
> @@ -0,0 +1,31 @@
> +# cdrtools-native OE build file
> +# Copyright (C) 2004-2006, Advanced Micro Devices, Inc.  All Rights Reserved
> +# Released under the MIT license (see packages/COPYING)
> +SUMMARY = "A set of tools for CD recording, including cdrecord"
> +DESCRIPTION = "A set of tools for CD recording, including cdrecord"
> +HOMEPAGE = "http://cdrecord.berlios.de/private/cdrecord.html"
> +SECTION = "console/utils"
> +LICENSE = "GPLv2 & CDDL-1.0 & LGPLv2.1+"
> +LIC_FILES_CHKSUM = "file://COPYING;md5=32f68170be424c2cd64804337726b312"
> +PR = "r0"

Recent consensus is that we drop the PR, especially if set to "r0".
Otherwise, looks good.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17
  2013-09-12 17:19 ` [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17 Jason Wessel
  2013-09-12 17:40   ` Darren Hart
@ 2013-09-12 17:40   ` Saul Wold
  1 sibling, 0 replies; 22+ messages in thread
From: Saul Wold @ 2013-09-12 17:40 UTC (permalink / raw)
  To: Jason Wessel; +Cc: dvhart, Openembedded-core

On 09/12/2013 10:19 AM, Jason Wessel wrote:
> The update is needed to support generation of EFI boot images that
> work with optical media.  Specifically the "-eltorito-platform efi"
> capability for mkisofs is needed.
>
> [YOCTO #4100]
>
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>   .../cdrtools/cdrtools-native_3.00.bb               |   30 -------------------
>   .../cdrtools/cdrtools-native_3.01a17.bb            |   31 ++++++++++++++++++++
>   2 files changed, 31 insertions(+), 30 deletions(-)
>   delete mode 100644 meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
>   create mode 100644 meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
>
> diff --git a/meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb b/meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
> deleted file mode 100644
> index 7e4c381..0000000
> --- a/meta/recipes-devtools/cdrtools/cdrtools-native_3.00.bb
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -# cdrtools-native OE build file
> -# Copyright (C) 2004-2006, Advanced Micro Devices, Inc.  All Rights Reserved
> -# Released under the MIT license (see packages/COPYING)
> -SUMMARY = "A set of tools for CD recording, including cdrecord"
> -DESCRIPTION = "A set of tools for CD recording, including cdrecord"
> -HOMEPAGE = "http://cdrecord.berlios.de/private/cdrecord.html"
> -SECTION = "console/utils"
> -LICENSE = "GPLv2"
> -LIC_FILES_CHKSUM = "file://COPYING;md5=8d16123ffd39e649a5e4a6bc1de60e6d"
> -PR = "r0"
> -
> -SRC_URI = "ftp://ftp.berlios.de/pub/cdrecord/cdrtools-${PV}.tar.bz2 \
> -           file://no_usr_src.patch"
> -
> -SRC_URI[md5sum] = "f9fbab08fbd458b0d2312976d8c5f558"
> -SRC_URI[sha256sum] = "7f9cb64820055573b880f77b2f16662a512518336ba95ab49228a1617973423d"
> -
> -inherit native
> -
> -STAGE_TEMP = "${WORKDIR}/image-temp"
> -
> -FILESPATH = "${FILE_DIRNAME}/cdrtools-native/"
> -
> -do_install() {
> -	install -d ${STAGE_TEMP}
> -	make install INS_BASE=${STAGE_TEMP}
> -
> -	install -d ${D}${bindir}/
> -	install ${STAGE_TEMP}/bin/* ${D}${bindir}/
> -}
> diff --git a/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb b/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
> new file mode 100644
> index 0000000..4a7a160
> --- /dev/null
> +++ b/meta/recipes-devtools/cdrtools/cdrtools-native_3.01a17.bb
> @@ -0,0 +1,31 @@
> +# cdrtools-native OE build file
> +# Copyright (C) 2004-2006, Advanced Micro Devices, Inc.  All Rights Reserved
> +# Released under the MIT license (see packages/COPYING)
> +SUMMARY = "A set of tools for CD recording, including cdrecord"
> +DESCRIPTION = "A set of tools for CD recording, including cdrecord"
> +HOMEPAGE = "http://cdrecord.berlios.de/private/cdrecord.html"
> +SECTION = "console/utils"
> +LICENSE = "GPLv2 & CDDL-1.0 & LGPLv2.1+"
> +LIC_FILES_CHKSUM = "file://COPYING;md5=32f68170be424c2cd64804337726b312"
> +PR = "r0"
You can drop the PR completely.

> +
> +SRC_URI = "ftp://ftp.berlios.de/pub/cdrecord/alpha/cdrtools-${PV}.tar.bz2"
> +
> +SRC_URI[md5sum] = "4cef9db0cf15a770c52d65b00bbee2db"
> +SRC_URI[sha256sum] = "3d613965b213ad83e4be0ba2535e784901839ea4d11a20a2beb6765f0eb76dfa"
> +
> +S = "${WORKDIR}/cdrtools-3.01"
> +
> +inherit native
> +
> +STAGE_TEMP = "${WORKDIR}/image-temp"
> +
> +FILESPATH = "${FILE_DIRNAME}/cdrtools-native/"
> +
> +do_install() {
> +	install -d ${STAGE_TEMP}
> +	make install INS_BASE=${STAGE_TEMP}
> +
> +	install -d ${D}${bindir}/
> +	install ${STAGE_TEMP}/bin/* ${D}${bindir}/
> +}
>
I was working on this update also and was able to simplify this 
do_install to the following:

EXTRA_OEMAKE += "GMAKE_NOWARN=true INS_BASE=${prefix_native} DESTDIR=${D}

do_install() {
	oe_runmake install
}

This allowed me to get rid of the STAGE_TEMP

Sau!



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] grub-efi-native: Add support for EFI ISO images
  2013-09-12 17:19 ` [PATCH 3/5] grub-efi-native: Add support for EFI ISO images Jason Wessel
@ 2013-09-12 17:48   ` Darren Hart
  0 siblings, 0 replies; 22+ messages in thread
From: Darren Hart @ 2013-09-12 17:48 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> The iso9660 file system support needs to be added to grub in order to
> be able to correctly find the grub.cfg.  The grub commands to locate
> the grub.cfg also needs to be encoded into grub's default
> configuration.
> 
> This change allows the resulting grub binary to work both in the hard
> drive / USB boot case or the optical media boot case.
> 



> [YOCTO #4100]
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  meta/recipes-bsp/grub/grub-efi-native_2.00.bb |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/recipes-bsp/grub/grub-efi-native_2.00.bb b/meta/recipes-bsp/grub/grub-efi-native_2.00.bb
> index 2d3d68a..3cff838 100644
> --- a/meta/recipes-bsp/grub/grub-efi-native_2.00.bb
> +++ b/meta/recipes-bsp/grub/grub-efi-native_2.00.bb
> @@ -66,9 +66,13 @@ EXTRA_OECONF = "--with-platform=efi --disable-grub-mkfont \
>                  --enable-liblzma=no --enable-device-mapper=no --enable-libzfs=no"
>  
>  do_mkimage() {
> -	./grub-mkimage -p /EFI/BOOT -d ./grub-core/ \
> +	# Search for the grub.cfg on the local boot media with built in config file
> +	echo 'search.file /EFI/BOOT/grub.cfg root' > cfg\
> +	echo 'set prefix=($root)/EFI/BOOT' >> cfg

Hrm, I think adding a cfg as a file via the SRC_URI would be preferable
to dynamically creating it every time. This would make it easier to
modify the config in the future.

Otherwise, looks good. 

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 17:19 ` [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options Jason Wessel
@ 2013-09-12 18:01   ` Saul Wold
  2013-09-12 18:06     ` Jason Wessel
  2013-09-12 18:11     ` Darren Hart
  2013-09-12 18:16   ` Darren Hart
  1 sibling, 2 replies; 22+ messages in thread
From: Saul Wold @ 2013-09-12 18:01 UTC (permalink / raw)
  To: Jason Wessel; +Cc: dvhart, Openembedded-core

On 09/12/2013 10:19 AM, Jason Wessel wrote:
> The syslinux.bbclass already has support for automatically generated
> serial and graphics menu choices.  This patch adds the same concept to
> the grub-efi menu.  That makes it possible to generate a single image
> which can boot on a PCBIOS or EFI firmware with consistent looking
> boot options.
>
> [YOCTO #4100]
>
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>   meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
>   meta/conf/machine/qemux86-64.conf |    2 +-
>   meta/conf/machine/qemux86.conf    |    2 ++
>   3 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
> index c6f5d4e..c07e4a1 100644
> --- a/meta/classes/grub-efi.bbclass
> +++ b/meta/classes/grub-efi.bbclass
> @@ -9,6 +9,7 @@
>   # External variables
>   # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
>   # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
> +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
>   # ${LABELS} - a list of targets for the automatic config
>   # ${APPEND} - an override list of append strings for each label
>   # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
> @@ -16,6 +17,7 @@
>
>   do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
>
> +GRUB_SERIAL ?= "console=ttyS0,115200"
>   GRUBCFG = "${S}/grub.cfg"
>   GRUB_TIMEOUT ?= "10"
>   #FIXME: build this from the machine config
> @@ -55,6 +57,8 @@ python build_grub_cfg() {
>           bb.error("WORKDIR not defined, unable to package")
>           return
>
> +    gfxserial = d.getVar('GRUB_GFXSERIAL', True) or ""
> +
>       labels = d.getVar('LABELS', True)
>       if not labels:
>           bb.debug(1, "LABELS not defined, nothing to do")
> @@ -88,6 +92,12 @@ python build_grub_cfg() {
>       else:
>           cfgfile.write('timeout=50\n')
>
> +    if gfxserial == "1":
> +        btypes = [ [ " graphics console", "console=tty0" ],
> +            [ " serial console", d.getVar('GRUB_SERIAL', True) or "" ] ]
> +    else:
> +        btypes = [ [ "", "" ] ]
> +
>       for label in labels.split():
>           localdata = d.createCopy()
>
> @@ -95,24 +105,27 @@ python build_grub_cfg() {
>           if not overrides:
>               raise bb.build.FuncFailed('OVERRIDES not defined')
>
> -        localdata.setVar('OVERRIDES', label + ':' + overrides)
> -        bb.data.update_data(localdata)
> +        for btype in btypes:
> +            localdata.setVar('OVERRIDES', label + ':' + overrides)
> +            bb.data.update_data(localdata)
>
> -        cfgfile.write('\nmenuentry \'%s\'{\n' % (label))
> -        if label == "install":
> -            label = "install-efi"
> -        cfgfile.write('linux /vmlinuz LABEL=%s' % (label))
> +            cfgfile.write('\nmenuentry \'%s%s\'{\n' % (label, btype[0]))
> +            lb = label
> +            if label == "install":
> +                lb = "install-efi"
> +            cfgfile.write('linux /vmlinuz LABEL=%s' % (lb))
>
> -        append = localdata.getVar('APPEND', True)
> -        initrd = localdata.getVar('INITRD', True)
> +            append = localdata.getVar('APPEND', True)
> +            initrd = localdata.getVar('INITRD', True)
>
> -        if append:
> -            cfgfile.write('%s' % (append))
> -        cfgfile.write('\n')
> +            if append:
> +                cfgfile.write('%s' % (append))
> +            cfgfile.write(' %s' % btype[1])
> +            cfgfile.write('\n')
>
> -        if initrd:
> -            cfgfile.write('initrd /initrd')
> -        cfgfile.write('\n}\n')
> +            if initrd:
> +                cfgfile.write('initrd /initrd')
> +            cfgfile.write('\n}\n')
>
>       cfgfile.close()
>   }
> diff --git a/meta/conf/machine/qemux86-64.conf b/meta/conf/machine/qemux86-64.conf
> index c572225..6f68410 100644
> --- a/meta/conf/machine/qemux86-64.conf
> +++ b/meta/conf/machine/qemux86-64.conf
> @@ -21,6 +21,6 @@ XSERVER = "xserver-xorg \
>              xf86-input-evdev \
>              xf86-video-vmware"
>
> -MACHINE_FEATURES += "x86"
> +MACHINE_FEATURES += "x86 efi"
>
>   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
> diff --git a/meta/conf/machine/qemux86.conf b/meta/conf/machine/qemux86.conf
> index 94ee573..57a9a50 100644
> --- a/meta/conf/machine/qemux86.conf
> +++ b/meta/conf/machine/qemux86.conf
> @@ -22,5 +22,7 @@ XSERVER = "xserver-xorg \
>              xf86-video-vmware"
>
>   MACHINE_FEATURES += "x86"
> +MACHINE_FEATURES += "efi"
> +#MACHINE_FEATURES += "pcbios"
>
Did you intend to keep the commented out like vs what you did above in 
x86-64?

>   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
>

Will this affect the genericx86* also?

Sau!



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 18:01   ` Saul Wold
@ 2013-09-12 18:06     ` Jason Wessel
  2013-09-12 18:11     ` Darren Hart
  1 sibling, 0 replies; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 18:06 UTC (permalink / raw)
  To: Saul Wold; +Cc: dvhart, Openembedded-core

On 09/12/2013 01:01 PM, Saul Wold wrote:
> On 09/12/2013 10:19 AM, Jason Wessel wrote:
>> The syslinux.bbclass already has support for automatically generated
>> serial and graphics menu choices.  This patch adds the same concept to
>> the grub-efi menu.  That makes it possible to generate a single image
>> which can boot on a PCBIOS or EFI firmware with consistent looking
>> boot options.
>>
>> [YOCTO #4100]
>>
>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>> ---
>>   meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
>>   meta/conf/machine/qemux86-64.conf |    2 +-
>>   meta/conf/machine/qemux86.conf    |    2 ++
>>   3 files changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
>> index c6f5d4e..c07e4a1 100644
>> --- a/meta/classes/grub-efi.bbclass
>> +++ b/meta/classes/grub-efi.bbclass
>> @@ -9,6 +9,7 @@
>>   # External variables
>>   # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
>>   # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
>> +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
>>   # ${LABELS} - a list of targets for the automatic config
>>   # ${APPEND} - an override list of append strings for each label
>>   # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
>> @@ -16,6 +17,7 @@
>>
>>   do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
>>
>> +GRUB_SERIAL ?= "console=ttyS0,115200"
>>   GRUBCFG = "${S}/grub.cfg"
>>   GRUB_TIMEOUT ?= "10"
>>   #FIXME: build this from the machine config
>> @@ -55,6 +57,8 @@ python build_grub_cfg() {
>>           bb.error("WORKDIR not defined, unable to package")
>>           return
>>
>> +    gfxserial = d.getVar('GRUB_GFXSERIAL', True) or ""
>> +
>>       labels = d.getVar('LABELS', True)
>>       if not labels:
>>           bb.debug(1, "LABELS not defined, nothing to do")
>> @@ -88,6 +92,12 @@ python build_grub_cfg() {
>>       else:
>>           cfgfile.write('timeout=50\n')
>>
>> +    if gfxserial == "1":
>> +        btypes = [ [ " graphics console", "console=tty0" ],
>> +            [ " serial console", d.getVar('GRUB_SERIAL', True) or "" ] ]
>> +    else:
>> +        btypes = [ [ "", "" ] ]
>> +
>>       for label in labels.split():
>>           localdata = d.createCopy()
>>
>> @@ -95,24 +105,27 @@ python build_grub_cfg() {
>>           if not overrides:
>>               raise bb.build.FuncFailed('OVERRIDES not defined')
>>
>> -        localdata.setVar('OVERRIDES', label + ':' + overrides)
>> -        bb.data.update_data(localdata)
>> +        for btype in btypes:
>> +            localdata.setVar('OVERRIDES', label + ':' + overrides)
>> +            bb.data.update_data(localdata)
>>
>> -        cfgfile.write('\nmenuentry \'%s\'{\n' % (label))
>> -        if label == "install":
>> -            label = "install-efi"
>> -        cfgfile.write('linux /vmlinuz LABEL=%s' % (label))
>> +            cfgfile.write('\nmenuentry \'%s%s\'{\n' % (label, btype[0]))
>> +            lb = label
>> +            if label == "install":
>> +                lb = "install-efi"
>> +            cfgfile.write('linux /vmlinuz LABEL=%s' % (lb))
>>
>> -        append = localdata.getVar('APPEND', True)
>> -        initrd = localdata.getVar('INITRD', True)
>> +            append = localdata.getVar('APPEND', True)
>> +            initrd = localdata.getVar('INITRD', True)
>>
>> -        if append:
>> -            cfgfile.write('%s' % (append))
>> -        cfgfile.write('\n')
>> +            if append:
>> +                cfgfile.write('%s' % (append))
>> +            cfgfile.write(' %s' % btype[1])
>> +            cfgfile.write('\n')
>>
>> -        if initrd:
>> -            cfgfile.write('initrd /initrd')
>> -        cfgfile.write('\n}\n')
>> +            if initrd:
>> +                cfgfile.write('initrd /initrd')
>> +            cfgfile.write('\n}\n')
>>
>>       cfgfile.close()
>>   }
>> diff --git a/meta/conf/machine/qemux86-64.conf b/meta/conf/machine/qemux86-64.conf
>> index c572225..6f68410 100644
>> --- a/meta/conf/machine/qemux86-64.conf
>> +++ b/meta/conf/machine/qemux86-64.conf
>> @@ -21,6 +21,6 @@ XSERVER = "xserver-xorg \
>>              xf86-input-evdev \
>>              xf86-video-vmware"
>>
>> -MACHINE_FEATURES += "x86"
>> +MACHINE_FEATURES += "x86 efi"
>>
>>   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
>> diff --git a/meta/conf/machine/qemux86.conf b/meta/conf/machine/qemux86.conf
>> index 94ee573..57a9a50 100644
>> --- a/meta/conf/machine/qemux86.conf
>> +++ b/meta/conf/machine/qemux86.conf
>> @@ -22,5 +22,7 @@ XSERVER = "xserver-xorg \
>>              xf86-video-vmware"
>>
>>   MACHINE_FEATURES += "x86"
>> +MACHINE_FEATURES += "efi"
>> +#MACHINE_FEATURES += "pcbios"
>>
> Did you intend to keep the commented out like vs what you did above in 
> x86-64?
>
>>   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
>>
> Will this affect the genericx86* also?


Doh!  That isn't even supposed to be in there.   I was just using the qemu targets to build some generic images and then run them on qemu + some real hw + Tiano UEFI core images.   It is possible that we can just start building pcbios and efi on the qemu BSPs, but I don't think that is the intent since we do not currently have Tiano core roms, tboot or anything of the like for use with runqemu.

I'll fix this up along with your other comments and Darren's in a v2 series.

Jason.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support
  2013-09-12 17:19 ` [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support Jason Wessel
@ 2013-09-12 18:09   ` Darren Hart
  2013-09-12 20:14     ` Jason Wessel
  0 siblings, 1 reply; 22+ messages in thread
From: Darren Hart @ 2013-09-12 18:09 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> Using the latest mkisofs it is possible to generate 3 different types
> of ISO images, which can be used in various scenarios.
> 
> 1) PCBIOS Only ISO
>    - This option remains unchanged by this commit
>    - Uses syslinux menus
>    - Can be directly copied with dd to a USB device
>    - Can be burned to optical media
> 
> 2) EFI Only ISO
>    - Uses grub 2 menus
>    - Can be burned to optical media
>    - If you want to use this image on a USB device
>      extra steps must be taken in order to format the USB
>      device with fat32, and copy an EFI loader which will
>      in turn load the iso image
> 
> 3) PCBIOS / EFI ISO
>    - This is a hybrid image ISO that will work for case 1 or 2
>      as above with the same restrictions and boot menu types
>      depending on what type of firmware is installed on
>      the hardware or depending on if EFI or "Legacy Boot" is
>      enabled on some UEFI firmwares.
> 
> [YOCTO #4100]
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  meta/classes/bootimg.bbclass |   37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
> index 2ed7017..b4301e8 100644
> --- a/meta/classes/bootimg.bbclass
> +++ b/meta/classes/bootimg.bbclass
> @@ -35,6 +35,7 @@ EXCLUDE_FROM_WORLD = "1"
>  
>  HDDDIR = "${S}/hddimg"
>  ISODIR = "${S}/iso"
> +EFIIMGDIR = "${S}/efi_img"
>  COMPACT_ISODIR = "${S}/iso.z"
>  
>  BOOTIMG_VOLUME_ID   ?= "boot"
> @@ -109,19 +110,49 @@ build_iso() {
>  		mkisofs_opts="-R -z -D -l"
>  	fi
>  

Am I missing a patch? I don't have this mkisofs_opts in my
bootimg.bbclass....

How are $mkisofs_opts and ${MKISOFS_OPTIONS} related? Do we need both?

> -	if [ "${PCBIOS}" = "1" ]; then
> +	if [ "${EFI}" = "1" ] ; then
> +		# Build a EFI directory to create efi.img
> +		mkdir -p ${EFIIMGDIR}/${EFIDIR}
> +		cp ${ISODIR}/${EFIDIR}/* ${EFIIMGDIR}${EFIDIR}
> +		cp ${ISODIR}/vmlinuz ${EFIIMGDIR}
> +		GRUB_IMAGE="bootia32.efi"
> +		if [ "${TARGET_ARCH}" = "x86_64" ]; then
> +			GRUB_IMAGE="bootx64.efi"
> +		fi
> +		echo "EFI\\BOOT\\${GRUB_IMAGE}" > ${EFIIMGDIR}/startup.nsh
> +		if [ -f "${ISODIR}/initrd" ] ; then
> +			cp ${ISODIR}/initrd ${EFIIMGDIR}
> +		fi
> +		build_fat_img ${EFIIMGDIR} ${ISODIR}/efi.img
> +	fi
> +
> +	# Three ways to build the media
> +	if [ "${PCBIOS}" = "1" -a "${EFI}" = "1" ]; then

Per the Dash as BinSh wiki, the -a syntax is discouraged, best practices
seem to be the use of && and || instead:

if ([ "${PCBIOS}" = "1" ] && [ "${EFI}" = "1" ]); then


> +		# 1) Build EFI + PCBIOS hybrid
> +		mkisofs -A ${BOOTIMG_VOLUME_ID} -V ${BOOTIMG_VOLUME_ID} \
> +		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
> +			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
> +			${MKISOFS_OPTIONS} \
> +			-eltorito-alt-boot -eltorito-platform efi \
> +			-b efi.img -no-emul-boot \
> +			${ISODIR}

Has this been verified on any 32 bit PCBIOS platforms?
It was my understanding the many 32 bit PCBIOS systems had very buggy
el-torito support and didn't deal well with multiple el-torito images on
the same disk. Although... looking at the command above, I'm not sure
you're doing this with mutliple images. Do you have the syslinux and the
efi image combined in the same efi.img?

Has this been verified to work on USB media?
It was explained to me that, at least for some implementations, if the
boot device selection code will not look for ISO9660 on anything that
doesn't have a block size of 2048, making it give up on a USB device
before even attempting to find the ISO9660 FS.



> +	elif [ "${PCBIOS}" = "1" ] ; then
> +		# 2) Build PCBIOS only media
>  		mkisofs -V ${BOOTIMG_VOLUME_ID} \
>  		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
>  			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
>  			${MKISOFS_OPTIONS} ${ISODIR}
>  	else

This should also verify that we are building for EFI. !PCBIOS != EFI. If
none of these options eval to true, an error should be printed

> -		bbnote "EFI-only ISO images are untested, please provide feedback."
>  		mkisofs -V ${BOOTIMG_VOLUME_ID} \
>  		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
> +			-eltorito-alt-boot -eltorito-platform efi \
> +			-b efi.img -no-emul-boot \
>  			-r ${ISODIR}
>  	fi
>  
> -	isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
> +	if [ "${PCBIOS}" = "1" ]; then
> +		isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
> +	fi

A comment would be good around the isohybrid block describing what it is
doing and why it only applies to PCBIOS.

>  
>  	cd ${DEPLOY_DIR_IMAGE}
>  	rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.iso


Thanks Jason, this is long overdue. Your effort here is very much
appreciated.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 18:01   ` Saul Wold
  2013-09-12 18:06     ` Jason Wessel
@ 2013-09-12 18:11     ` Darren Hart
  1 sibling, 0 replies; 22+ messages in thread
From: Darren Hart @ 2013-09-12 18:11 UTC (permalink / raw)
  To: Saul Wold; +Cc: Openembedded-core

On Thu, 2013-09-12 at 11:01 -0700, Saul Wold wrote:
> On 09/12/2013 10:19 AM, Jason Wessel wrote:
> > The syslinux.bbclass already has support for automatically generated
> > serial and graphics menu choices.  This patch adds the same concept to
> > the grub-efi menu.  That makes it possible to generate a single image
> > which can boot on a PCBIOS or EFI firmware with consistent looking
> > boot options.
> >
> > [YOCTO #4100]
> >
> > Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> > ---
> >   meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
> >   meta/conf/machine/qemux86-64.conf |    2 +-
> >   meta/conf/machine/qemux86.conf    |    2 ++
> >   3 files changed, 30 insertions(+), 15 deletions(-)
> >
> > diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
> > index c6f5d4e..c07e4a1 100644
> > --- a/meta/classes/grub-efi.bbclass
> > +++ b/meta/classes/grub-efi.bbclass
> > @@ -9,6 +9,7 @@
> >   # External variables
> >   # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
> >   # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
> > +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
> >   # ${LABELS} - a list of targets for the automatic config
> >   # ${APPEND} - an override list of append strings for each label
> >   # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
> > @@ -16,6 +17,7 @@
> >
> >   do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
> >
> > +GRUB_SERIAL ?= "console=ttyS0,115200"
> >   GRUBCFG = "${S}/grub.cfg"
> >   GRUB_TIMEOUT ?= "10"
> >   #FIXME: build this from the machine config
> > @@ -55,6 +57,8 @@ python build_grub_cfg() {
> >           bb.error("WORKDIR not defined, unable to package")
> >           return
> >
> > +    gfxserial = d.getVar('GRUB_GFXSERIAL', True) or ""
> > +
> >       labels = d.getVar('LABELS', True)
> >       if not labels:
> >           bb.debug(1, "LABELS not defined, nothing to do")
> > @@ -88,6 +92,12 @@ python build_grub_cfg() {
> >       else:
> >           cfgfile.write('timeout=50\n')
> >
> > +    if gfxserial == "1":
> > +        btypes = [ [ " graphics console", "console=tty0" ],
> > +            [ " serial console", d.getVar('GRUB_SERIAL', True) or "" ] ]
> > +    else:
> > +        btypes = [ [ "", "" ] ]
> > +
> >       for label in labels.split():
> >           localdata = d.createCopy()
> >
> > @@ -95,24 +105,27 @@ python build_grub_cfg() {
> >           if not overrides:
> >               raise bb.build.FuncFailed('OVERRIDES not defined')
> >
> > -        localdata.setVar('OVERRIDES', label + ':' + overrides)
> > -        bb.data.update_data(localdata)
> > +        for btype in btypes:
> > +            localdata.setVar('OVERRIDES', label + ':' + overrides)
> > +            bb.data.update_data(localdata)
> >
> > -        cfgfile.write('\nmenuentry \'%s\'{\n' % (label))
> > -        if label == "install":
> > -            label = "install-efi"
> > -        cfgfile.write('linux /vmlinuz LABEL=%s' % (label))
> > +            cfgfile.write('\nmenuentry \'%s%s\'{\n' % (label, btype[0]))
> > +            lb = label
> > +            if label == "install":
> > +                lb = "install-efi"
> > +            cfgfile.write('linux /vmlinuz LABEL=%s' % (lb))
> >
> > -        append = localdata.getVar('APPEND', True)
> > -        initrd = localdata.getVar('INITRD', True)
> > +            append = localdata.getVar('APPEND', True)
> > +            initrd = localdata.getVar('INITRD', True)
> >
> > -        if append:
> > -            cfgfile.write('%s' % (append))
> > -        cfgfile.write('\n')
> > +            if append:
> > +                cfgfile.write('%s' % (append))
> > +            cfgfile.write(' %s' % btype[1])
> > +            cfgfile.write('\n')
> >
> > -        if initrd:
> > -            cfgfile.write('initrd /initrd')
> > -        cfgfile.write('\n}\n')
> > +            if initrd:
> > +                cfgfile.write('initrd /initrd')
> > +            cfgfile.write('\n}\n')
> >
> >       cfgfile.close()
> >   }
> > diff --git a/meta/conf/machine/qemux86-64.conf b/meta/conf/machine/qemux86-64.conf
> > index c572225..6f68410 100644
> > --- a/meta/conf/machine/qemux86-64.conf
> > +++ b/meta/conf/machine/qemux86-64.conf
> > @@ -21,6 +21,6 @@ XSERVER = "xserver-xorg \
> >              xf86-input-evdev \
> >              xf86-video-vmware"
> >
> > -MACHINE_FEATURES += "x86"
> > +MACHINE_FEATURES += "x86 efi"
> >
> >   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
> > diff --git a/meta/conf/machine/qemux86.conf b/meta/conf/machine/qemux86.conf
> > index 94ee573..57a9a50 100644
> > --- a/meta/conf/machine/qemux86.conf
> > +++ b/meta/conf/machine/qemux86.conf
> > @@ -22,5 +22,7 @@ XSERVER = "xserver-xorg \
> >              xf86-video-vmware"
> >
> >   MACHINE_FEATURES += "x86"
> > +MACHINE_FEATURES += "efi"
> > +#MACHINE_FEATURES += "pcbios"
> >

This can just be one line:

MACHINE_FEATURES += "x86 pcbios efi"

> Did you intend to keep the commented out like vs what you did above in 
> x86-64?
> 
> >   MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
> >
> 
> Will this affect the genericx86* also?

No, both genericx86 machines have efi and pcbios in their machine
features.

> 
> Sau!
> 

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 17:19 ` [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options Jason Wessel
  2013-09-12 18:01   ` Saul Wold
@ 2013-09-12 18:16   ` Darren Hart
  2013-09-12 19:52     ` Jason Wessel
  1 sibling, 1 reply; 22+ messages in thread
From: Darren Hart @ 2013-09-12 18:16 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> The syslinux.bbclass already has support for automatically generated
> serial and graphics menu choices.  This patch adds the same concept to
> the grub-efi menu.  That makes it possible to generate a single image
> which can boot on a PCBIOS or EFI firmware with consistent looking
> boot options.
> 
> [YOCTO #4100]
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
>  meta/conf/machine/qemux86-64.conf |    2 +-
>  meta/conf/machine/qemux86.conf    |    2 ++
>  3 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
> index c6f5d4e..c07e4a1 100644
> --- a/meta/classes/grub-efi.bbclass
> +++ b/meta/classes/grub-efi.bbclass
> @@ -9,6 +9,7 @@
>  # External variables
>  # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
>  # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
> +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
>  # ${LABELS} - a list of targets for the automatic config
>  # ${APPEND} - an override list of append strings for each label
>  # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
> @@ -16,6 +17,7 @@
>  
>  do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
>  
> +GRUB_SERIAL ?= "console=ttyS0,115200"
>  GRUBCFG = "${S}/grub.cfg"
>  GRUB_TIMEOUT ?= "10"
>  #FIXME: build this from the machine config
> @@ -55,6 +57,8 @@ python build_grub_cfg() {
>          bb.error("WORKDIR not defined, unable to package")
>          return
>  
> +    gfxserial = d.getVar('GRUB_GFXSERIAL', True) or ""
> +
>      labels = d.getVar('LABELS', True)
>      if not labels:
>          bb.debug(1, "LABELS not defined, nothing to do")
> @@ -88,6 +92,12 @@ python build_grub_cfg() {
>      else:
>          cfgfile.write('timeout=50\n')
>  
> +    if gfxserial == "1":
> +        btypes = [ [ " graphics console", "console=tty0" ],
> +            [ " serial console", d.getVar('GRUB_SERIAL', True) or "" ] ]
> +    else:
> +        btypes = [ [ "", "" ] ]
> +
>      for label in labels.split():
>          localdata = d.createCopy()
>  
> @@ -95,24 +105,27 @@ python build_grub_cfg() {
>          if not overrides:
>              raise bb.build.FuncFailed('OVERRIDES not defined')
>  
> -        localdata.setVar('OVERRIDES', label + ':' + overrides)
> -        bb.data.update_data(localdata)
> +        for btype in btypes:
> +            localdata.setVar('OVERRIDES', label + ':' + overrides)
> +            bb.data.update_data(localdata)
>  
> -        cfgfile.write('\nmenuentry \'%s\'{\n' % (label))
> -        if label == "install":
> -            label = "install-efi"
> -        cfgfile.write('linux /vmlinuz LABEL=%s' % (label))
> +            cfgfile.write('\nmenuentry \'%s%s\'{\n' % (label, btype[0]))
> +            lb = label
> +            if label == "install":
> +                lb = "install-efi"
> +            cfgfile.write('linux /vmlinuz LABEL=%s' % (lb))
>  
> -        append = localdata.getVar('APPEND', True)
> -        initrd = localdata.getVar('INITRD', True)
> +            append = localdata.getVar('APPEND', True)
> +            initrd = localdata.getVar('INITRD', True)
>  
> -        if append:
> -            cfgfile.write('%s' % (append))
> -        cfgfile.write('\n')
> +            if append:
> +                cfgfile.write('%s' % (append))
> +            cfgfile.write(' %s' % btype[1])
> +            cfgfile.write('\n')
>  
> -        if initrd:
> -            cfgfile.write('initrd /initrd')
> -        cfgfile.write('\n}\n')
> +            if initrd:
> +                cfgfile.write('initrd /initrd')
> +            cfgfile.write('\n}\n')
>  
>      cfgfile.close()
>  }

I'm not very familiar with the cfgfile for menus and such, so I don't
have much to add. The one thing that catches me by surprise is the need
for the serial device. On EFI systems, grub here uses the EFI console
service, so if that uses the serial port you get it for free, no need
for GRUB to try and use it directly. In fact.... does the above not
cause some kind of conflict between the EFI console service and grub
serial?

Both of the following should be in a separate patch. In fact, they
should probably have a qemux86-common.inc which took care of most of
this (as was done recently for genericx86-common.inc).

> diff --git a/meta/conf/machine/qemux86-64.conf b/meta/conf/machine/qemux86-64.conf
> index c572225..6f68410 100644
> --- a/meta/conf/machine/qemux86-64.conf
> +++ b/meta/conf/machine/qemux86-64.conf
> @@ -21,6 +21,6 @@ XSERVER = "xserver-xorg \
>             xf86-input-evdev \
>             xf86-video-vmware"
>  
> -MACHINE_FEATURES += "x86"
> +MACHINE_FEATURES += "x86 efi"
>  
>  MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
> diff --git a/meta/conf/machine/qemux86.conf b/meta/conf/machine/qemux86.conf
> index 94ee573..57a9a50 100644
> --- a/meta/conf/machine/qemux86.conf
> +++ b/meta/conf/machine/qemux86.conf
> @@ -22,5 +22,7 @@ XSERVER = "xserver-xorg \
>             xf86-video-vmware"
>  
>  MACHINE_FEATURES += "x86"
> +MACHINE_FEATURES += "efi"
> +#MACHINE_FEATURES += "pcbios"
>  
>  MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 18:16   ` Darren Hart
@ 2013-09-12 19:52     ` Jason Wessel
  2013-09-12 20:09       ` Darren Hart
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 19:52 UTC (permalink / raw)
  To: Darren Hart; +Cc: Openembedded-core

On 09/12/2013 01:16 PM, Darren Hart wrote:
> On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
>> The syslinux.bbclass already has support for automatically generated
>> serial and graphics menu choices.  This patch adds the same concept to
>> the grub-efi menu.  That makes it possible to generate a single image
>> which can boot on a PCBIOS or EFI firmware with consistent looking
>> boot options.
>>
>> [YOCTO #4100]
>>
>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>> ---
>>  meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
>>  meta/conf/machine/qemux86-64.conf |    2 +-
>>  meta/conf/machine/qemux86.conf    |    2 ++
>>  3 files changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
>> index c6f5d4e..c07e4a1 100644
>> --- a/meta/classes/grub-efi.bbclass
>> +++ b/meta/classes/grub-efi.bbclass
>> @@ -9,6 +9,7 @@
>>  # External variables
>>  # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
>>  # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
>> +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
>>  # ${LABELS} - a list of targets for the automatic config
>>  # ${APPEND} - an override list of append strings for each label
>>  # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
>> @@ -16,6 +17,7 @@
>>  
>>  do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
>>  
>> +GRUB_SERIAL ?= "console=ttyS0,115200"
>>  GRUBCFG = "${S}/grub.cfg"
>>  GRUB_TIMEOUT ?= "10"
>>  #FIXME: build this from the machine config
>> @@ -55,6 +57,8 @@ python build_grub_cfg() {
>>          bb.error("WORKDIR not defined, unable to package")
>>          return
>>  
>> +    gfxserial = d.getVar('GRUB_GFXSERIAL', True) or ""
>> +
>>      labels = d.getVar('LABELS', True)
>>      if not labels:
>>          bb.debug(1, "LABELS not defined, nothing to do")
>> @@ -88,6 +92,12 @@ python build_grub_cfg() {
>>      else:
>>          cfgfile.write('timeout=50\n')
>>  
>> +    if gfxserial == "1":
>> +        btypes = [ [ " graphics console", "console=tty0" ],
>> +            [ " serial console", d.getVar('GRUB_SERIAL', True) or "" ] ]
>> +    else:
>> +        btypes = [ [ "", "" ] ]
>> +
>>      for label in labels.split():
>>          localdata = d.createCopy()
>>  
>> @@ -95,24 +105,27 @@ python build_grub_cfg() {
>>          if not overrides:
>>              raise bb.build.FuncFailed('OVERRIDES not defined')
>>  
>> -        localdata.setVar('OVERRIDES', label + ':' + overrides)
>> -        bb.data.update_data(localdata)
>> +        for btype in btypes:
>> +            localdata.setVar('OVERRIDES', label + ':' + overrides)
>> +            bb.data.update_data(localdata)
>>  
>> -        cfgfile.write('\nmenuentry \'%s\'{\n' % (label))
>> -        if label == "install":
>> -            label = "install-efi"
>> -        cfgfile.write('linux /vmlinuz LABEL=%s' % (label))
>> +            cfgfile.write('\nmenuentry \'%s%s\'{\n' % (label, btype[0]))
>> +            lb = label
>> +            if label == "install":
>> +                lb = "install-efi"
>> +            cfgfile.write('linux /vmlinuz LABEL=%s' % (lb))
>>  
>> -        append = localdata.getVar('APPEND', True)
>> -        initrd = localdata.getVar('INITRD', True)
>> +            append = localdata.getVar('APPEND', True)
>> +            initrd = localdata.getVar('INITRD', True)
>>  
>> -        if append:
>> -            cfgfile.write('%s' % (append))
>> -        cfgfile.write('\n')
>> +            if append:
>> +                cfgfile.write('%s' % (append))
>> +            cfgfile.write(' %s' % btype[1])
>> +            cfgfile.write('\n')
>>  
>> -        if initrd:
>> -            cfgfile.write('initrd /initrd')
>> -        cfgfile.write('\n}\n')
>> +            if initrd:
>> +                cfgfile.write('initrd /initrd')
>> +            cfgfile.write('\n}\n')
>>  
>>      cfgfile.close()
>>  }
> 
> I'm not very familiar with the cfgfile for menus and such, so I don't
> have much to add. The one thing that catches me by surprise is the need
> for the serial device. On EFI systems, grub here uses the EFI console
> service, so if that uses the serial port you get it for free, no need
> for GRUB to try and use it directly. In fact.... does the above not
> cause some kind of conflict between the EFI console service and grub
> serial?
> 

In part that is why it is optional.   With respect to the serial bits, these are only the kernel boot arguments we are talking about.  It doesn't seem that there is a "primary" display interface for the HCDP in the EFI firmware I have.   Additionally, the kernel throws the EFI serial console under the bus at ACPI probe time, while this certainly could also be a bug in the firmware I have on my test board, the only way to keep the serial port alive for a login and the kernel boot information was to specify console=ttyS0...

I have yet another system I need to try this on which has a much newer UEFI and a serial port, but I thought it would be best to get something out there that covers the "buggy firmwares" as well which can be built optionally. 


> Both of the following should be in a separate patch. In fact, they
> should probably have a qemux86-common.inc which took care of most of
> this (as was done recently for genericx86-common.inc).


So I am not sure that we want to patch up the qemux86* conf files at all.  Do we want to enable EFI + PCBIOS all the time now that we have a way to generate images (noting these images will work with runqemu)?  I figured I would just drop those modifications entirely.  I would expect something like a "all encompassing" white box BSP to select both, but for the qemux86*, I don't think it makes sense.

Cheers,
Jason.


> 
>> diff --git a/meta/conf/machine/qemux86-64.conf b/meta/conf/machine/qemux86-64.conf
>> index c572225..6f68410 100644
>> --- a/meta/conf/machine/qemux86-64.conf
>> +++ b/meta/conf/machine/qemux86-64.conf
>> @@ -21,6 +21,6 @@ XSERVER = "xserver-xorg \
>>             xf86-input-evdev \
>>             xf86-video-vmware"
>>  
>> -MACHINE_FEATURES += "x86"
>> +MACHINE_FEATURES += "x86 efi"
>>  
>>  MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
>> diff --git a/meta/conf/machine/qemux86.conf b/meta/conf/machine/qemux86.conf
>> index 94ee573..57a9a50 100644
>> --- a/meta/conf/machine/qemux86.conf
>> +++ b/meta/conf/machine/qemux86.conf
>> @@ -22,5 +22,7 @@ XSERVER = "xserver-xorg \
>>             xf86-video-vmware"
>>  
>>  MACHINE_FEATURES += "x86"
>> +MACHINE_FEATURES += "efi"
>> +#MACHINE_FEATURES += "pcbios"
>>  
>>  MACHINE_ESSENTIAL_EXTRA_RDEPENDS += "v86d"
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 19:52     ` Jason Wessel
@ 2013-09-12 20:09       ` Darren Hart
  2013-09-13 21:58         ` Jason Wessel
  0 siblings, 1 reply; 22+ messages in thread
From: Darren Hart @ 2013-09-12 20:09 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 14:52 -0500, Jason Wessel wrote:
> On 09/12/2013 01:16 PM, Darren Hart wrote:
> > On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> >> The syslinux.bbclass already has support for automatically generated
> >> serial and graphics menu choices.  This patch adds the same concept to
> >> the grub-efi menu.  That makes it possible to generate a single image
> >> which can boot on a PCBIOS or EFI firmware with consistent looking
> >> boot options.
> >>
> >> [YOCTO #4100]
> >>
> >> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> >> ---
> >>  meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
> >>  meta/conf/machine/qemux86-64.conf |    2 +-
> >>  meta/conf/machine/qemux86.conf    |    2 ++
> >>  3 files changed, 30 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
> >> index c6f5d4e..c07e4a1 100644
> >> --- a/meta/classes/grub-efi.bbclass
> >> +++ b/meta/classes/grub-efi.bbclass
> >> @@ -9,6 +9,7 @@
> >>  # External variables
> >>  # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
> >>  # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
> >> +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
> >>  # ${LABELS} - a list of targets for the automatic config
> >>  # ${APPEND} - an override list of append strings for each label
> >>  # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
> >> @@ -16,6 +17,7 @@
> >>  
> >>  do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
> >>  
> >> +GRUB_SERIAL ?= "console=ttyS0,115200"

...

> > I'm not very familiar with the cfgfile for menus and such, so I don't
> > have much to add. The one thing that catches me by surprise is the need
> > for the serial device. On EFI systems, grub here uses the EFI console
> > service, so if that uses the serial port you get it for free, no need
> > for GRUB to try and use it directly. In fact.... does the above not
> > cause some kind of conflict between the EFI console service and grub
> > serial?
> > 
> 
> In part that is why it is optional.   With respect to the serial bits,
> these are only the kernel boot arguments we are talking about.  It


Hrm.... this should be handled with APPEND parameter from the machine
configs, not a new GRUB_SERIAL statement....


>  doesn't seem that there is a "primary" display interface for the HCDP
> in the EFI firmware I have.   Additionally, the kernel throws the EFI
> serial console under the bus at ACPI probe time, while this certainly
> could also be a bug in the firmware I have on my test board, the only
> way to keep the serial port alive for a login and the kernel boot
> information was to specify console=ttyS0...

Hrm.... interesting. I guess we'll just need to test more broadly.
Indeed the kernel should be using it's own console= parameter, but
again, that should come from the APPEND_machine variable.


> I have yet another system I need to try this on which has a much newer
> UEFI and a serial port, but I thought it would be best to get
> something out there that covers the "buggy firmwares" as well which
> can be built optionally.

Agreed, so long as it doesn't break the common case.


> > Both of the following should be in a separate patch. In fact, they
> > should probably have a qemux86-common.inc which took care of most of
> > this (as was done recently for genericx86-common.inc).
> 
> 
> So I am not sure that we want to patch up the qemux86* conf files at
> all.  Do we want to enable EFI + PCBIOS all the time now that we have
> a way to generate images (noting these images will work with runqemu)?
> I figured I would just drop those modifications entirely.  I would
> expect something like a "all encompassing" white box BSP to select
> both, but for the qemux86*, I don't think it makes sense.
> 

At some point we want qemux86* to boot with OVMF, but we can add "efi"
to MACHINE_FEATURES when/if that becomes doable with oe-core. So yeah,
go ahead and just drop these entirely for now.

...

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support
  2013-09-12 18:09   ` Darren Hart
@ 2013-09-12 20:14     ` Jason Wessel
  2013-09-12 20:28       ` Darren Hart
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wessel @ 2013-09-12 20:14 UTC (permalink / raw)
  To: Darren Hart; +Cc: Openembedded-core

On 09/12/2013 01:09 PM, Darren Hart wrote:
> On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
>> Using the latest mkisofs it is possible to generate 3 different types
>> of ISO images, which can be used in various scenarios.
>>
>> 1) PCBIOS Only ISO
>>    - This option remains unchanged by this commit
>>    - Uses syslinux menus
>>    - Can be directly copied with dd to a USB device
>>    - Can be burned to optical media
>>
>> 2) EFI Only ISO
>>    - Uses grub 2 menus
>>    - Can be burned to optical media
>>    - If you want to use this image on a USB device
>>      extra steps must be taken in order to format the USB
>>      device with fat32, and copy an EFI loader which will
>>      in turn load the iso image
>>
>> 3) PCBIOS / EFI ISO
>>    - This is a hybrid image ISO that will work for case 1 or 2
>>      as above with the same restrictions and boot menu types
>>      depending on what type of firmware is installed on
>>      the hardware or depending on if EFI or "Legacy Boot" is
>>      enabled on some UEFI firmwares.
>>
>> [YOCTO #4100]
>>
>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>> ---
>>  meta/classes/bootimg.bbclass |   37 ++++++++++++++++++++++++++++++++++---
>>  1 file changed, 34 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
>> index 2ed7017..b4301e8 100644
>> --- a/meta/classes/bootimg.bbclass
>> +++ b/meta/classes/bootimg.bbclass
>> @@ -35,6 +35,7 @@ EXCLUDE_FROM_WORLD = "1"
>>  
>>  HDDDIR = "${S}/hddimg"
>>  ISODIR = "${S}/iso"
>> +EFIIMGDIR = "${S}/efi_img"
>>  COMPACT_ISODIR = "${S}/iso.z"
>>  
>>  BOOTIMG_VOLUME_ID   ?= "boot"
>> @@ -109,19 +110,49 @@ build_iso() {
>>  		mkisofs_opts="-R -z -D -l"
>>  	fi
>>  
> 
> Am I missing a patch? I don't have this mkisofs_opts in my
> bootimg.bbclass....
> 

Yes


> How are $mkisofs_opts and ${MKISOFS_OPTIONS} related? Do we need both?
> 

Yes.

I built this patch set on top of the zisofs support I posted a day or so ago.  I'll include that patch in the series next time, else it would create merge conflicts.


The piece from the other patch is:

 +       if [ "${COMPRESSISO}" = "1" ] ; then
 +               # create compact directory, compress iso
 +               mkdir -p ${COMPACT_ISODIR}
 +               mkzftree -z 9 -p 4 -F ${ISODIR}/rootfs.img ${COMPACT_ISODIR}/rootfs.img
 +
 +               # move compact iso to iso, then remove compact directory
 +               mv ${COMPACT_ISODIR}/rootfs.img ${ISODIR}/rootfs.img
 +               rm -Rf ${COMPACT_ISODIR}
 +               mkisofs_opts="-r"
 +       else
 +               mkisofs_opts="-R -z -D -l"
 +       fi
 +


The zisofs is a big win for some folks because it will cut their image size in 1/2 so they can use single layer media instead of dual layer media for example. 



>> -	if [ "${PCBIOS}" = "1" ]; then
>> +	if [ "${EFI}" = "1" ] ; then
>> +		# Build a EFI directory to create efi.img
>> +		mkdir -p ${EFIIMGDIR}/${EFIDIR}
>> +		cp ${ISODIR}/${EFIDIR}/* ${EFIIMGDIR}${EFIDIR}
>> +		cp ${ISODIR}/vmlinuz ${EFIIMGDIR}
>> +		GRUB_IMAGE="bootia32.efi"
>> +		if [ "${TARGET_ARCH}" = "x86_64" ]; then
>> +			GRUB_IMAGE="bootx64.efi"
>> +		fi
>> +		echo "EFI\\BOOT\\${GRUB_IMAGE}" > ${EFIIMGDIR}/startup.nsh
>> +		if [ -f "${ISODIR}/initrd" ] ; then
>> +			cp ${ISODIR}/initrd ${EFIIMGDIR}
>> +		fi
>> +		build_fat_img ${EFIIMGDIR} ${ISODIR}/efi.img
>> +	fi
>> +
>> +	# Three ways to build the media
>> +	if [ "${PCBIOS}" = "1" -a "${EFI}" = "1" ]; then
> 
> Per the Dash as BinSh wiki, the -a syntax is discouraged, best practices
> seem to be the use of && and || instead:
> 
> if ([ "${PCBIOS}" = "1" ] && [ "${EFI}" = "1" ]); then
> 


Noted for the future :-)


> 
>> +		# 1) Build EFI + PCBIOS hybrid
>> +		mkisofs -A ${BOOTIMG_VOLUME_ID} -V ${BOOTIMG_VOLUME_ID} \
>> +		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
>> +			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
>> +			${MKISOFS_OPTIONS} \
>> +			-eltorito-alt-boot -eltorito-platform efi \
>> +			-b efi.img -no-emul-boot \
>> +			${ISODIR}
> 
> Has this been verified on any 32 bit PCBIOS platforms?


Yes, on tunnel creek based hardware. 


> It was my understanding the many 32 bit PCBIOS systems had very buggy
> el-torito support and didn't deal well with multiple el-torito images on
> the same disk. Although... looking at the command above, I'm not sure
> you're doing this with mutliple images. Do you have the syslinux and the
> efi image combined in the same efi.img?


My testing showed it worked for both EFI only and EFI+PCBIOS.  In the case of the EFI+PCBIOS image there are two eltorito images, but in the case of the EFI only, there is only one, so at least you have two options.  The Fedora install media uses the EFI+PCBIOS style image, so if that booted on the hardware you had problems with, this will too. 


> 
> Has this been verified to work on USB media?


The image only works on USB media with the PCBIOS. 


> It was explained to me that, at least for some implementations, if the
> boot device selection code will not look for ISO9660 on anything that
> doesn't have a block size of 2048, making it give up on a USB device
> before even attempting to find the ISO9660 FS.


Is there some magic argument to mkisofs that would cause this to change the block size?  Given that the ISO's I created with EFI would not work directly as a USB image I suspect I have one of the of those implementations.  Of course it can be worked around by copying the ISO onto a partition and using a different loader to deal with it. 

I guess my point here is that some further work is probably needed down the road, but the first pass works for the important cases (like real optical media). 


> 
> 
> 
>> +	elif [ "${PCBIOS}" = "1" ] ; then
>> +		# 2) Build PCBIOS only media
>>  		mkisofs -V ${BOOTIMG_VOLUME_ID} \
>>  		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
>>  			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
>>  			${MKISOFS_OPTIONS} ${ISODIR}
>>  	else
> 
> This should also verify that we are building for EFI. !PCBIOS != EFI. If
> none of these options eval to true, an error should be printed



This case cannot actually happen, thanks to you and Richard (which I got from git blame)

# Include legacy boot if MACHINE_FEATURES includes "pcbios" or if it does not
# contain "efi". This way legacy is supported by default if neither is
# specified, maintaining the original behavior.
def pcbios(d):
    pcbios = base_contains("MACHINE_FEATURES", "pcbios", "1", "0", d)
    if pcbios == "0":
        pcbios = base_contains("MACHINE_FEATURES", "efi", "0", "1", d)
    return pcbios

One or the other will always be set.

> 
>> -		bbnote "EFI-only ISO images are untested, please provide feedback."
>>  		mkisofs -V ${BOOTIMG_VOLUME_ID} \
>>  		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
>> +			-eltorito-alt-boot -eltorito-platform efi \
>> +			-b efi.img -no-emul-boot \
>>  			-r ${ISODIR}
>>  	fi
>>  
>> -	isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
>> +	if [ "${PCBIOS}" = "1" ]; then
>> +		isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
>> +	fi
> 
> A comment would be good around the isohybrid block describing what it is
> doing and why it only applies to PCBIOS.


The isohybrid is what allows you to boot the via a USB with a PCBIOS by populating the MBR.  The EFI firmware skips it entirely, but sure I'll add a comment. 


Cheers,
Jason.

> 
>>  
>>  	cd ${DEPLOY_DIR_IMAGE}
>>  	rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.iso
> 
> 
> Thanks Jason, this is long overdue. Your effort here is very much
> appreciated.
> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support
  2013-09-12 20:14     ` Jason Wessel
@ 2013-09-12 20:28       ` Darren Hart
  0 siblings, 0 replies; 22+ messages in thread
From: Darren Hart @ 2013-09-12 20:28 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Thu, 2013-09-12 at 15:14 -0500, Jason Wessel wrote:
> On 09/12/2013 01:09 PM, Darren Hart wrote:
> > On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> >> Using the latest mkisofs it is possible to generate 3 different types
> >> of ISO images, which can be used in various scenarios.
> >>
> >> 1) PCBIOS Only ISO
> >>    - This option remains unchanged by this commit
> >>    - Uses syslinux menus
> >>    - Can be directly copied with dd to a USB device
> >>    - Can be burned to optical media
> >>
> >> 2) EFI Only ISO
> >>    - Uses grub 2 menus
> >>    - Can be burned to optical media
> >>    - If you want to use this image on a USB device
> >>      extra steps must be taken in order to format the USB
> >>      device with fat32, and copy an EFI loader which will
> >>      in turn load the iso image
> >>
> >> 3) PCBIOS / EFI ISO
> >>    - This is a hybrid image ISO that will work for case 1 or 2
> >>      as above with the same restrictions and boot menu types
> >>      depending on what type of firmware is installed on
> >>      the hardware or depending on if EFI or "Legacy Boot" is
> >>      enabled on some UEFI firmwares.
> >>
> >> [YOCTO #4100]
> >>
> >> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> >> ---
> >>  meta/classes/bootimg.bbclass |   37 ++++++++++++++++++++++++++++++++++---
> >>  1 file changed, 34 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
> >> index 2ed7017..b4301e8 100644
> >> --- a/meta/classes/bootimg.bbclass
> >> +++ b/meta/classes/bootimg.bbclass
> >> @@ -35,6 +35,7 @@ EXCLUDE_FROM_WORLD = "1"
> >>  
> >>  HDDDIR = "${S}/hddimg"
> >>  ISODIR = "${S}/iso"
> >> +EFIIMGDIR = "${S}/efi_img"
> >>  COMPACT_ISODIR = "${S}/iso.z"
> >>  
> >>  BOOTIMG_VOLUME_ID   ?= "boot"
> >> @@ -109,19 +110,49 @@ build_iso() {
> >>  		mkisofs_opts="-R -z -D -l"
> >>  	fi
> >>  
> > 
> > Am I missing a patch? I don't have this mkisofs_opts in my
> > bootimg.bbclass....
> > 
> 
> Yes
> 
> 
> > How are $mkisofs_opts and ${MKISOFS_OPTIONS} related? Do we need both?
> > 
> 
> Yes.
> 
> I built this patch set on top of the zisofs support I posted a day or
> so ago.  I'll include that patch in the series next time, else it
> would create merge conflicts.

OK, thanks.

> 
> 
> The piece from the other patch is:
> 
>  +       if [ "${COMPRESSISO}" = "1" ] ; then
>  +               # create compact directory, compress iso
>  +               mkdir -p ${COMPACT_ISODIR}
>  +               mkzftree -z 9 -p 4 -F ${ISODIR}/rootfs.img ${COMPACT_ISODIR}/rootfs.img
>  +
>  +               # move compact iso to iso, then remove compact directory
>  +               mv ${COMPACT_ISODIR}/rootfs.img ${ISODIR}/rootfs.img
>  +               rm -Rf ${COMPACT_ISODIR}
>  +               mkisofs_opts="-r"
>  +       else
>  +               mkisofs_opts="-R -z -D -l"
>  +       fi
>  +
> 

So my objection here is the two variables are VERY similar in name and
there is no way to distinguish between them. Would it make sense to
either append to MKISOFS_OPTIONS above or to rename mkisofs_opts to
something like mkisofs_compression_opts to distinguish them.

> 
> The zisofs is a big win for some folks because it will cut their image
> size in 1/2 so they can use single layer media instead of dual layer
> media for example. 

Nice.

> 
> 
> 
> >> -	if [ "${PCBIOS}" = "1" ]; then
> >> +	if [ "${EFI}" = "1" ] ; then
> >> +		# Build a EFI directory to create efi.img
> >> +		mkdir -p ${EFIIMGDIR}/${EFIDIR}
> >> +		cp ${ISODIR}/${EFIDIR}/* ${EFIIMGDIR}${EFIDIR}
> >> +		cp ${ISODIR}/vmlinuz ${EFIIMGDIR}
> >> +		GRUB_IMAGE="bootia32.efi"
> >> +		if [ "${TARGET_ARCH}" = "x86_64" ]; then
> >> +			GRUB_IMAGE="bootx64.efi"
> >> +		fi
> >> +		echo "EFI\\BOOT\\${GRUB_IMAGE}" > ${EFIIMGDIR}/startup.nsh
> >> +		if [ -f "${ISODIR}/initrd" ] ; then
> >> +			cp ${ISODIR}/initrd ${EFIIMGDIR}
> >> +		fi
> >> +		build_fat_img ${EFIIMGDIR} ${ISODIR}/efi.img
> >> +	fi
> >> +
> >> +	# Three ways to build the media
> >> +	if [ "${PCBIOS}" = "1" -a "${EFI}" = "1" ]; then
> > 
> > Per the Dash as BinSh wiki, the -a syntax is discouraged, best practices
> > seem to be the use of && and || instead:
> > 
> > if ([ "${PCBIOS}" = "1" ] && [ "${EFI}" = "1" ]); then
> > 
> 
> 
> Noted for the future :-)


POSIX shell compatibility can be a bit of a hot-button with recipes.


> > 
> >> +		# 1) Build EFI + PCBIOS hybrid
> >> +		mkisofs -A ${BOOTIMG_VOLUME_ID} -V ${BOOTIMG_VOLUME_ID} \
> >> +		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
> >> +			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
> >> +			${MKISOFS_OPTIONS} \
> >> +			-eltorito-alt-boot -eltorito-platform efi \
> >> +			-b efi.img -no-emul-boot \
> >> +			${ISODIR}
> > 
> > Has this been verified on any 32 bit PCBIOS platforms?
> 
> 
> Yes, on tunnel creek based hardware. 
> 
> 
> > It was my understanding the many 32 bit PCBIOS systems had very buggy
> > el-torito support and didn't deal well with multiple el-torito images on
> > the same disk. Although... looking at the command above, I'm not sure
> > you're doing this with mutliple images. Do you have the syslinux and the
> > efi image combined in the same efi.img?
> 
> 
> My testing showed it worked for both EFI only and EFI+PCBIOS.  In the
> case of the EFI+PCBIOS image there are two eltorito images, but in the
> case of the EFI only, there is only one, so at least you have two
> options.  The Fedora install media uses the EFI+PCBIOS style image, so
> if that booted on the hardware you had problems with, this will too. 
> 
> 
> > 
> > Has this been verified to work on USB media?
> 
> 
> The image only works on USB media with the PCBIOS. 
> 
> 
> > It was explained to me that, at least for some implementations, if the
> > boot device selection code will not look for ISO9660 on anything that
> > doesn't have a block size of 2048, making it give up on a USB device
> > before even attempting to find the ISO9660 FS.
> 
> 
> Is there some magic argument to mkisofs that would cause this to
> change the block size?  Given that the ISO's I created with EFI would


I don't think so. This was something Saul and I crashed into during our
efforts. Maybe there is something, but I haven't seen it. I was thinking
this block size was device dependent.... can it be changed in software?


>  not work directly as a USB image I suspect I have one of the of those
> implementations.  Of course it can be worked around by copying the ISO
> onto a partition and using a different loader to deal with it. 
> 
> I guess my point here is that some further work is probably needed
> down the road, but the first pass works for the important cases (like
> real optical media). 


Agreed.

 
> > 
> > 
> >> +	elif [ "${PCBIOS}" = "1" ] ; then
> >> +		# 2) Build PCBIOS only media
> >>  		mkisofs -V ${BOOTIMG_VOLUME_ID} \
> >>  		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
> >>  			-b ${ISO_BOOTIMG} -c ${ISO_BOOTCAT} $mkisofs_opts \
> >>  			${MKISOFS_OPTIONS} ${ISODIR}
> >>  	else
> > 
> > This should also verify that we are building for EFI. !PCBIOS != EFI. If
> > none of these options eval to true, an error should be printed
> 
> 
> 
> This case cannot actually happen, thanks to you and Richard (which I got from git blame)
> 
> # Include legacy boot if MACHINE_FEATURES includes "pcbios" or if it does not
> # contain "efi". This way legacy is supported by default if neither is
> # specified, maintaining the original behavior.
> def pcbios(d):
>     pcbios = base_contains("MACHINE_FEATURES", "pcbios", "1", "0", d)
>     if pcbios == "0":
>         pcbios = base_contains("MACHINE_FEATURES", "efi", "0", "1", d)
>     return pcbios

Hah, well OK then ;-) Still, these sort of implicit things often cause
issues during bug analysis, but this one isn't a big deal. So, fine.


> One or the other will always be set.
> 
> > 
> >> -		bbnote "EFI-only ISO images are untested, please provide feedback."
> >>  		mkisofs -V ${BOOTIMG_VOLUME_ID} \
> >>  		        -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso \
> >> +			-eltorito-alt-boot -eltorito-platform efi \
> >> +			-b efi.img -no-emul-boot \
> >>  			-r ${ISODIR}
> >>  	fi
> >>  
> >> -	isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
> >> +	if [ "${PCBIOS}" = "1" ]; then
> >> +		isohybrid ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.iso
> >> +	fi
> > 
> > A comment would be good around the isohybrid block describing what it is
> > doing and why it only applies to PCBIOS.
> 
> 
> The isohybrid is what allows you to boot the via a USB with a PCBIOS
> by populating the MBR.  The EFI firmware skips it entirely, but sure
> I'll add a comment. 

Right, you and I know that, but someone that hasn't spent much time on
this might appreciate the heads-up.

Thanks Jason.

--
Darren


> 
> 
> Cheers,
> Jason.
> 
> > 
> >>  
> >>  	cd ${DEPLOY_DIR_IMAGE}
> >>  	rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.iso
> > 
> > 
> > Thanks Jason, this is long overdue. Your effort here is very much
> > appreciated.
> > 
> 

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-12 20:09       ` Darren Hart
@ 2013-09-13 21:58         ` Jason Wessel
  2013-09-16 17:49           ` Darren Hart
  0 siblings, 1 reply; 22+ messages in thread
From: Jason Wessel @ 2013-09-13 21:58 UTC (permalink / raw)
  To: Darren Hart; +Cc: Openembedded-core

On 09/12/2013 03:09 PM, Darren Hart wrote:
> On Thu, 2013-09-12 at 14:52 -0500, Jason Wessel wrote:
>> On 09/12/2013 01:16 PM, Darren Hart wrote:
>>> On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
>>>> The syslinux.bbclass already has support for automatically generated
>>>> serial and graphics menu choices.  This patch adds the same concept to
>>>> the grub-efi menu.  That makes it possible to generate a single image
>>>> which can boot on a PCBIOS or EFI firmware with consistent looking
>>>> boot options.
>>>>
>>>> [YOCTO #4100]
>>>>
>>>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>>>> ---
>>>>  meta/classes/grub-efi.bbclass     |   41 ++++++++++++++++++++++++-------------
>>>>  meta/conf/machine/qemux86-64.conf |    2 +-
>>>>  meta/conf/machine/qemux86.conf    |    2 ++
>>>>  3 files changed, 30 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/meta/classes/grub-efi.bbclass b/meta/classes/grub-efi.bbclass
>>>> index c6f5d4e..c07e4a1 100644
>>>> --- a/meta/classes/grub-efi.bbclass
>>>> +++ b/meta/classes/grub-efi.bbclass
>>>> @@ -9,6 +9,7 @@
>>>>  # External variables
>>>>  # ${INITRD} - indicates a filesystem image to use as an initrd (optional)
>>>>  # ${ROOTFS} - indicates a filesystem image to include as the root filesystem (optional)
>>>> +# ${GRUB_GFXSERIAL} - set this to 1 to have graphics and serial in the boot menu
>>>>  # ${LABELS} - a list of targets for the automatic config
>>>>  # ${APPEND} - an override list of append strings for each label
>>>>  # ${GRUB_OPTS} - additional options to add to the config, ';' delimited # (optional)
>>>> @@ -16,6 +17,7 @@
>>>>  
>>>>  do_bootimg[depends] += "grub-efi-${TRANSLATED_TARGET_ARCH}-native:do_deploy"
>>>>  
>>>> +GRUB_SERIAL ?= "console=ttyS0,115200"
> 
> ...
> 
>>> I'm not very familiar with the cfgfile for menus and such, so I don't
>>> have much to add. The one thing that catches me by surprise is the need
>>> for the serial device. On EFI systems, grub here uses the EFI console
>>> service, so if that uses the serial port you get it for free, no need
>>> for GRUB to try and use it directly. In fact.... does the above not
>>> cause some kind of conflict between the EFI console service and grub
>>> serial?
>>>
>>
>> In part that is why it is optional.   With respect to the serial bits,
>> these are only the kernel boot arguments we are talking about.  It
> 
> 
> Hrm.... this should be handled with APPEND parameter from the machine
> configs, not a new GRUB_SERIAL statement....
> 


Well there is a problem with that.  You only have 1 APPEND, but I need 2 options.  The whole point is the ability to add SERIAL or Graphics console access for early boot all the way through user space hand off and seamlessly picking things up later in user space with mingetty etc...

This option is mainly used for the installer media and to provide a consistent look and feel regardless if you come from EFI or a PCIBIOS + syslinux.


> 
>>  doesn't seem that there is a "primary" display interface for the HCDP
>> in the EFI firmware I have.   Additionally, the kernel throws the EFI
>> serial console under the bus at ACPI probe time, while this certainly
>> could also be a bug in the firmware I have on my test board, the only
>> way to keep the serial port alive for a login and the kernel boot
>> information was to specify console=ttyS0...
> 
> Hrm.... interesting. I guess we'll just need to test more broadly.
> Indeed the kernel should be using it's own console= parameter, but
> again, that should come from the APPEND_machine variable.
> 
> 
>> I have yet another system I need to try this on which has a much newer
>> UEFI and a serial port, but I thought it would be best to get
>> something out there that covers the "buggy firmwares" as well which
>> can be built optionally.
> 
> Agreed, so long as it doesn't break the common case.


The common case is not broken.  This is something you have to turn on in the image or local.conf. 

The new patches will available soon for the whole series as soon as all the test hardware has booted in legacy and efi mode.

Cheers,
Jason.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-13 21:58         ` Jason Wessel
@ 2013-09-16 17:49           ` Darren Hart
  2013-09-17 12:19             ` Jason Wessel
  0 siblings, 1 reply; 22+ messages in thread
From: Darren Hart @ 2013-09-16 17:49 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Openembedded-core

On Fri, 2013-09-13 at 16:58 -0500, Jason Wessel wrote:
> On 09/12/2013 03:09 PM, Darren Hart wrote:
> > On Thu, 2013-09-12 at 14:52 -0500, Jason Wessel wrote:
> >> On 09/12/2013 01:16 PM, Darren Hart wrote:
> >>> On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
...

 
> >>>> +GRUB_SERIAL ?= "console=ttyS0,115200"
> > 
> > ...
> > 
> >>> I'm not very familiar with the cfgfile for menus and such, so I don't
> >>> have much to add. The one thing that catches me by surprise is the need
> >>> for the serial device. On EFI systems, grub here uses the EFI console
> >>> service, so if that uses the serial port you get it for free, no need
> >>> for GRUB to try and use it directly. In fact.... does the above not
> >>> cause some kind of conflict between the EFI console service and grub
> >>> serial?
> >>>
> >>
> >> In part that is why it is optional.   With respect to the serial bits,
> >> these are only the kernel boot arguments we are talking about.  It
> > 
> > 
> > Hrm.... this should be handled with APPEND parameter from the machine
> > configs, not a new GRUB_SERIAL statement....
> > 
> 
> 
> Well there is a problem with that.  You only have 1 APPEND, but I need
> 2 options.  The whole point is the ability to add SERIAL or Graphics


I don't follow. Do you mean you need both serial and VGA? That is still
handled by the single variable. Take the FRI2 for example:

APPEND += "console=ttyPCH1,115200 console=tty0"

With what you have here, it seems to me this will be ignored and GRUB
will instead pass console=ttyS0,115200 - which will not work on the
FRI2.

>  console access for early boot all the way through user space hand off
> and seamlessly picking things up later in user space with mingetty
> etc...
> 
> This option is mainly used for the installer media and to provide a
> consistent look and feel regardless if you come from EFI or a PCIBIOS
> + syslinux.

SYSLINUX also has some recipe-specific console options, but those are
due to the syntax differences and are used to specify how syslinux talks
to the device, while this appears to be Linux kernel syntax
("console=ttyS0") rather than grub syntax ("serial --unit=0
--speed=115200")... and yet I don't see the "console=ttyS0" being passed
to the kernel.... so I am confused. Where does the GRUB_SERIAL content
actually get used? I mean besides the creation of the btypes array....

--
Darren

> 
> 
> > 
> >>  doesn't seem that there is a "primary" display interface for the HCDP
> >> in the EFI firmware I have.   Additionally, the kernel throws the EFI
> >> serial console under the bus at ACPI probe time, while this certainly
> >> could also be a bug in the firmware I have on my test board, the only
> >> way to keep the serial port alive for a login and the kernel boot
> >> information was to specify console=ttyS0...
> > 
> > Hrm.... interesting. I guess we'll just need to test more broadly.
> > Indeed the kernel should be using it's own console= parameter, but
> > again, that should come from the APPEND_machine variable.
> > 
> > 
> >> I have yet another system I need to try this on which has a much newer
> >> UEFI and a serial port, but I thought it would be best to get
> >> something out there that covers the "buggy firmwares" as well which
> >> can be built optionally.
> > 
> > Agreed, so long as it doesn't break the common case.
> 
> 
> The common case is not broken.  This is something you have to turn on
> in the image or local.conf. 
> 
> The new patches will available soon for the whole series as soon as
> all the test hardware has booted in legacy and efi mode.
> 
> Cheers,
> Jason.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options
  2013-09-16 17:49           ` Darren Hart
@ 2013-09-17 12:19             ` Jason Wessel
  0 siblings, 0 replies; 22+ messages in thread
From: Jason Wessel @ 2013-09-17 12:19 UTC (permalink / raw)
  To: Darren Hart; +Cc: Openembedded-core

On 09/16/2013 12:49 PM, Darren Hart wrote:
> On Fri, 2013-09-13 at 16:58 -0500, Jason Wessel wrote:
>> On 09/12/2013 03:09 PM, Darren Hart wrote:
>>> On Thu, 2013-09-12 at 14:52 -0500, Jason Wessel wrote:
>>>> On 09/12/2013 01:16 PM, Darren Hart wrote:
>>>>> On Thu, 2013-09-12 at 12:19 -0500, Jason Wessel wrote:
> ...
>
>  
>>>>>> +GRUB_SERIAL ?= "console=ttyS0,115200"
>>> ...
>>>
>>>>> I'm not very familiar with the cfgfile for menus and such, so I don't
>>>>> have much to add. The one thing that catches me by surprise is the need
>>>>> for the serial device. On EFI systems, grub here uses the EFI console
>>>>> service, so if that uses the serial port you get it for free, no need
>>>>> for GRUB to try and use it directly. In fact.... does the above not
>>>>> cause some kind of conflict between the EFI console service and grub
>>>>> serial?
>>>>>
>>>> In part that is why it is optional.   With respect to the serial bits,
>>>> these are only the kernel boot arguments we are talking about.  It
>>>
>>> Hrm.... this should be handled with APPEND parameter from the machine
>>> configs, not a new GRUB_SERIAL statement....
>>>
>>
>> Well there is a problem with that.  You only have 1 APPEND, but I need
>> 2 options.  The whole point is the ability to add SERIAL or Graphics
>
> I don't follow. Do you mean you need both serial and VGA? That is still
> handled by the single variable. Take the FRI2 for example:
>
> APPEND += "console=ttyPCH1,115200 console=tty0"
>
> With what you have here, it seems to me this will be ignored and GRUB
> will instead pass console=ttyS0,115200 - which will not work on the
> FRI2.


This is fixed in the new series. 

The auto menu is intended to offer the end user the choice of which is their default console.  Obviously you can emit text to both, but you only get one selected as the default input device.

It might not be obvious but we use mingetty and selectively use the console.   The whole alternate boot thing is completely optional.  Now if you set the SERIAL stuff to be "".

It might even be better to point this out more directly by changing the variable names to something like ALT_CONSOLE...


>
>>  console access for early boot all the way through user space hand off
>> and seamlessly picking things up later in user space with mingetty
>> etc...
>>
>> This option is mainly used for the installer media and to provide a
>> consistent look and feel regardless if you come from EFI or a PCIBIOS
>> + syslinux.
> SYSLINUX also has some recipe-specific console options, but those are
> due to the syntax differences and are used to specify how syslinux talks
> to the device, while this appears to be Linux kernel syntax
> ("console=ttyS0") rather than grub syntax ("serial --unit=0
> --speed=115200")... and yet I don't see the "console=ttyS0" being passed
> to the kernel.... so I am confused. Where does the GRUB_SERIAL content
> actually get used? I mean besides the creation of the btypes array....


This is addressed to, but we may yet go another round to clean up the understanding of things.  A v3 to arrive soon.

Cheers,
Jason.



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2013-09-17 12:19 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-12 17:19 [PATCH 0/5] Improved EFI boot support Jason Wessel
2013-09-12 17:19 ` [PATCH 1/5] bootimage.bbclass: Move fat image creation into a function Jason Wessel
2013-09-12 17:38   ` Darren Hart
2013-09-12 17:19 ` [PATCH 2/5] cdrtools-native: Update from 3.00 to 3.01a17 Jason Wessel
2013-09-12 17:40   ` Darren Hart
2013-09-12 17:40   ` Saul Wold
2013-09-12 17:19 ` [PATCH 3/5] grub-efi-native: Add support for EFI ISO images Jason Wessel
2013-09-12 17:48   ` Darren Hart
2013-09-12 17:19 ` [PATCH 4/5] bootimage.bbclass: Improve EFI & PCBIOS+EFI ISO support Jason Wessel
2013-09-12 18:09   ` Darren Hart
2013-09-12 20:14     ` Jason Wessel
2013-09-12 20:28       ` Darren Hart
2013-09-12 17:19 ` [PATCH 5/5] grub-efi.bbclass: Add serial and graphics menu options Jason Wessel
2013-09-12 18:01   ` Saul Wold
2013-09-12 18:06     ` Jason Wessel
2013-09-12 18:11     ` Darren Hart
2013-09-12 18:16   ` Darren Hart
2013-09-12 19:52     ` Jason Wessel
2013-09-12 20:09       ` Darren Hart
2013-09-13 21:58         ` Jason Wessel
2013-09-16 17:49           ` Darren Hart
2013-09-17 12:19             ` Jason Wessel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.