diff mbox series

[LEDE-DEV,3/3] intel-microcode: create early load microcode image

Message ID 20180403131337.3094-4-tomek_n@o2.pl
State Superseded
Delegated to: Zoltan HERPAI
Headers show
Series intel-microcode: load as early as possible | expand

Commit Message

Tomasz Maciej Nowak April 3, 2018, 1:13 p.m. UTC 
Create initrd image with packed microcode. This'll allow to load it at
early boot stage. Unfortunately the package can't install files directly
to /boot directory, therefore additional installation hooks are placed
for standalone package and when building directly into target image.

Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
---
 package/firmware/intel-microcode/Makefile | 32 +++++++++++++++++++++++++------
 target/linux/x86/image/Makefile           |  6 ++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

Comments

Philip Prindeville April 9, 2018, 7:05 p.m. UTC | #1 
Inline

> On Apr 3, 2018, at 7:13 AM, Tomasz Maciej Nowak <tomek_n@o2.pl> wrote:
> 
> Create initrd image with packed microcode. This'll allow to load it at
> early boot stage. Unfortunately the package can't install files directly
> to /boot directory, therefore additional installation hooks are placed
> for standalone package and when building directly into target image.
> 
> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
> ---
> package/firmware/intel-microcode/Makefile | 32 +++++++++++++++++++++++++------
> target/linux/x86/image/Makefile           |  6 ++++++
> 2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/package/firmware/intel-microcode/Makefile b/package/firmware/intel-microcode/Makefile
> index d2663bb901..9970f8f072 100644
> --- a/package/firmware/intel-microcode/Makefile
> +++ b/package/firmware/intel-microcode/Makefile
> @@ -35,15 +35,35 @@ define Package/intel-microcode
> endef
> 
> define Build/Compile
> -	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool $(MAKE) -C $(PKG_BUILD_DIR)
> -	mkdir $(PKG_BUILD_DIR)/intel-ucode
> -	$(STAGING_DIR)/../host/bin/iucode_tool -q \
> -		--write-firmware=$(PKG_BUILD_DIR)/intel-ucode $(PKG_BUILD_DIR)/$(MICROCODE).bin
> +	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool \
> +		$(MAKE) -C $(PKG_BUILD_DIR)
> +	$(STAGING_DIR)/../host/bin/iucode_tool -q --mini-earlyfw \
> +		--write-earlyfw=$(PKG_BUILD_DIR)/intel-ucode.cpio \
> +		$(PKG_BUILD_DIR)/$(MICROCODE).bin
> endef
> 
> define Package/intel-microcode/install
> -	$(INSTALL_DIR) $(1)/lib/firmware/intel-ucode
> -	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode/* $(1)/lib/firmware/intel-ucode
> +	$(INSTALL_DIR) $(1)/lib/firmware
> +	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode.cpio \
> +		$(1)/lib/firmware/intel-ucode.img
> +endef
> +
> +ifeq ($(CONFIG_PACKAGE_intel-microcode),m)
> +define Package/intel-microcode/postinst
> +#!/bin/sh
> +
> +mount /boot -o remount,rw,noatime
> +cp -f /lib/firmware/intel-ucode.img /boot/

Can we preserve the timestamp (-p) on the microcode file, too?


> +mount /boot -o remount,ro,noatime
> +endef
> +endif
> +
> +define Package/intel-microcode/prerm
> +#!/bin/sh
> +
> +mount /boot -o remount,rw,noatime
> +rm /boot/intel-ucode.img


“rm -f” so that if the uninstall fails it’s idempotent and doesn’t leave things in a weird state.


> +mount /boot -o remount,ro,noatime
> endef
> 
> $(eval $(call BuildPackage,intel-microcode))
> diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
> index a05f4babd9..4d6a3016d2 100644
> --- a/target/linux/x86/image/Makefile
> +++ b/target/linux/x86/image/Makefile
> @@ -83,6 +83,9 @@ ifneq ($(CONFIG_GRUB_IMAGES),)
> 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
> 		-e 's#@ROOT@#$(GRUB_ROOT)#g' \
> 		./grub.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
> +    ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
> +    endif


Do we need this?  Won’t the postinst happen during “make world”?


> 	PADDING="$(CONFIG_TARGET_IMAGES_PAD)" SIGNATURE="$(SIGNATURE)" PATH="$(TARGET_PATH)" $(SCRIPT_DIR)/gen_image_generic.sh \
> 		$(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img \
> 		$(CONFIG_TARGET_KERNEL_PARTSIZE) $(KDIR)/root.grub \
> @@ -120,6 +123,9 @@ define Image/Build/iso
> 		-e 's#@CMDLINE@#root=/dev/sr0 rootfstype=iso9660 rootwait $(strip $(call Image/cmdline/$(1)) $(BOOTOPTS) $(GRUB_CONSOLE_CMDLINE))#g' \
> 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
> 		./grub-iso.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
> +  ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
> +  endif


Ditto.


> 	mkisofs -R -b boot/grub/eltorito.img -no-emul-boot -boot-info-table \
> 		-o $(KDIR)/root.iso $(KDIR)/root.grub $(TARGET_DIR)
> endef
> -- 
> 2.16.3
Tomasz Maciej Nowak April 9, 2018, 8:11 p.m. UTC | #2 
W dniu 09.04.2018 o 21:05, Philip Prindeville pisze:
> Inline
> 
>> On Apr 3, 2018, at 7:13 AM, Tomasz Maciej Nowak <tomek_n@o2.pl> wrote:
>>
>> Create initrd image with packed microcode. This'll allow to load it at
>> early boot stage. Unfortunately the package can't install files directly
>> to /boot directory, therefore additional installation hooks are placed
>> for standalone package and when building directly into target image.
>>
>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
>> ---
>> package/firmware/intel-microcode/Makefile | 32 +++++++++++++++++++++++++------
>> target/linux/x86/image/Makefile           |  6 ++++++
>> 2 files changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/package/firmware/intel-microcode/Makefile b/package/firmware/intel-microcode/Makefile
>> index d2663bb901..9970f8f072 100644
>> --- a/package/firmware/intel-microcode/Makefile
>> +++ b/package/firmware/intel-microcode/Makefile
>> @@ -35,15 +35,35 @@ define Package/intel-microcode
>> endef
>>
>> define Build/Compile
>> -	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool $(MAKE) -C $(PKG_BUILD_DIR)
>> -	mkdir $(PKG_BUILD_DIR)/intel-ucode
>> -	$(STAGING_DIR)/../host/bin/iucode_tool -q \
>> -		--write-firmware=$(PKG_BUILD_DIR)/intel-ucode $(PKG_BUILD_DIR)/$(MICROCODE).bin
>> +	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool \
>> +		$(MAKE) -C $(PKG_BUILD_DIR)
>> +	$(STAGING_DIR)/../host/bin/iucode_tool -q --mini-earlyfw \
>> +		--write-earlyfw=$(PKG_BUILD_DIR)/intel-ucode.cpio \
>> +		$(PKG_BUILD_DIR)/$(MICROCODE).bin
>> endef
>>
>> define Package/intel-microcode/install
>> -	$(INSTALL_DIR) $(1)/lib/firmware/intel-ucode
>> -	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode/* $(1)/lib/firmware/intel-ucode
>> +	$(INSTALL_DIR) $(1)/lib/firmware
>> +	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode.cpio \
>> +		$(1)/lib/firmware/intel-ucode.img
>> +endef
>> +
>> +ifeq ($(CONFIG_PACKAGE_intel-microcode),m)
>> +define Package/intel-microcode/postinst
>> +#!/bin/sh
>> +
>> +mount /boot -o remount,rw,noatime
>> +cp -f /lib/firmware/intel-ucode.img /boot/
> 
> Can we preserve the timestamp (-p) on the microcode file, too?

Will add in v2.

> 
> 
>> +mount /boot -o remount,ro,noatime
>> +endef
>> +endif
>> +
>> +define Package/intel-microcode/prerm
>> +#!/bin/sh
>> +
>> +mount /boot -o remount,rw,noatime
>> +rm /boot/intel-ucode.img
> 
> 
> “rm -f” so that if the uninstall fails it’s idempotent and doesn’t leave things in a weird state.

Good catch, had this locally but sent the wrong version.

> 
> 
>> +mount /boot -o remount,ro,noatime
>> endef
>>
>> $(eval $(call BuildPackage,intel-microcode))
>> diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
>> index a05f4babd9..4d6a3016d2 100644
>> --- a/target/linux/x86/image/Makefile
>> +++ b/target/linux/x86/image/Makefile
>> @@ -83,6 +83,9 @@ ifneq ($(CONFIG_GRUB_IMAGES),)
>> 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>> 		-e 's#@ROOT@#$(GRUB_ROOT)#g' \
>> 		./grub.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
>> +    ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
>> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
>> +    endif
> 
> 
> Do we need this?  Won’t the postinst happen during “make world”?

Yes it's needed, the package doesn't install files directly to /boot, 
and when executing postinst it'll naturally fail trying to mount host 
file system and stop the whole build process.

What can be changed is handling of this command, instead conditional, 
mark it as non-fatal.

> 
> 
>> 	PADDING="$(CONFIG_TARGET_IMAGES_PAD)" SIGNATURE="$(SIGNATURE)" PATH="$(TARGET_PATH)" $(SCRIPT_DIR)/gen_image_generic.sh \
>> 		$(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img \
>> 		$(CONFIG_TARGET_KERNEL_PARTSIZE) $(KDIR)/root.grub \
>> @@ -120,6 +123,9 @@ define Image/Build/iso
>> 		-e 's#@CMDLINE@#root=/dev/sr0 rootfstype=iso9660 rootwait $(strip $(call Image/cmdline/$(1)) $(BOOTOPTS) $(GRUB_CONSOLE_CMDLINE))#g' \
>> 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>> 		./grub-iso.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
>> +  ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
>> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
>> +  endif
> 
> 
> Ditto.
> >
>> 	mkisofs -R -b boot/grub/eltorito.img -no-emul-boot -boot-info-table \
>> 		-o $(KDIR)/root.iso $(KDIR)/root.grub $(TARGET_DIR)
>> endef
>> -- 
>> 2.16.3
>
Philip Prindeville April 9, 2018, 8:32 p.m. UTC | #3 
Inline

> On Apr 9, 2018, at 2:11 PM, Tomasz Maciej Nowak <tmn505@gmail.com> wrote:
> 
> W dniu 09.04.2018 o 21:05, Philip Prindeville pisze:
>> Inline
>>> On Apr 3, 2018, at 7:13 AM, Tomasz Maciej Nowak <tomek_n@o2.pl> wrote:
>>> 
>>> Create initrd image with packed microcode. This'll allow to load it at
>>> early boot stage. Unfortunately the package can't install files directly
>>> to /boot directory, therefore additional installation hooks are placed
>>> for standalone package and when building directly into target image.
>>> 
>>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
>>> ---
>>> package/firmware/intel-microcode/Makefile | 32 +++++++++++++++++++++++++------
>>> target/linux/x86/image/Makefile           |  6 ++++++
>>> 2 files changed, 32 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/package/firmware/intel-microcode/Makefile b/package/firmware/intel-microcode/Makefile
>>> index d2663bb901..9970f8f072 100644
>>> --- a/package/firmware/intel-microcode/Makefile
>>> +++ b/package/firmware/intel-microcode/Makefile
>>> @@ -35,15 +35,35 @@ define Package/intel-microcode
>>> endef
>>> 
>>> define Build/Compile
>>> -	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool $(MAKE) -C $(PKG_BUILD_DIR)
>>> -	mkdir $(PKG_BUILD_DIR)/intel-ucode
>>> -	$(STAGING_DIR)/../host/bin/iucode_tool -q \
>>> -		--write-firmware=$(PKG_BUILD_DIR)/intel-ucode $(PKG_BUILD_DIR)/$(MICROCODE).bin
>>> +	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool \
>>> +		$(MAKE) -C $(PKG_BUILD_DIR)
>>> +	$(STAGING_DIR)/../host/bin/iucode_tool -q --mini-earlyfw \
>>> +		--write-earlyfw=$(PKG_BUILD_DIR)/intel-ucode.cpio \
>>> +		$(PKG_BUILD_DIR)/$(MICROCODE).bin
>>> endef
>>> 
>>> define Package/intel-microcode/install
>>> -	$(INSTALL_DIR) $(1)/lib/firmware/intel-ucode
>>> -	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode/* $(1)/lib/firmware/intel-ucode
>>> +	$(INSTALL_DIR) $(1)/lib/firmware
>>> +	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode.cpio \
>>> +		$(1)/lib/firmware/intel-ucode.img
>>> +endef
>>> +
>>> +ifeq ($(CONFIG_PACKAGE_intel-microcode),m)
>>> +define Package/intel-microcode/postinst
>>> +#!/bin/sh
>>> +
>>> +mount /boot -o remount,rw,noatime
>>> +cp -f /lib/firmware/intel-ucode.img /boot/
>> Can we preserve the timestamp (-p) on the microcode file, too?
> 
> Will add in v2.
> 
>>> +mount /boot -o remount,ro,noatime
>>> +endef
>>> +endif
>>> +
>>> +define Package/intel-microcode/prerm
>>> +#!/bin/sh
>>> +
>>> +mount /boot -o remount,rw,noatime
>>> +rm /boot/intel-ucode.img
>> “rm -f” so that if the uninstall fails it’s idempotent and doesn’t leave things in a weird state.
> 
> Good catch, had this locally but sent the wrong version.
> 
>>> +mount /boot -o remount,ro,noatime
>>> endef
>>> 
>>> $(eval $(call BuildPackage,intel-microcode))
>>> diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
>>> index a05f4babd9..4d6a3016d2 100644
>>> --- a/target/linux/x86/image/Makefile
>>> +++ b/target/linux/x86/image/Makefile
>>> @@ -83,6 +83,9 @@ ifneq ($(CONFIG_GRUB_IMAGES),)
>>> 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>>> 		-e 's#@ROOT@#$(GRUB_ROOT)#g' \
>>> 		./grub.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
>>> +    ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
>>> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
>>> +    endif
>> Do we need this?  Won’t the postinst happen during “make world”?
> 
> Yes it's needed, the package doesn't install files directly to /boot, and when executing postinst it'll naturally fail trying to mount host file system and stop the whole build process.
> 
> What can be changed is handling of this command, instead conditional, mark it as non-fatal.


Okay, I assumed that mkimage (or whatever) ran in a chroot with /boot mounted…


> 
>>> 	PADDING="$(CONFIG_TARGET_IMAGES_PAD)" SIGNATURE="$(SIGNATURE)" PATH="$(TARGET_PATH)" $(SCRIPT_DIR)/gen_image_generic.sh \
>>> 		$(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img \
>>> 		$(CONFIG_TARGET_KERNEL_PARTSIZE) $(KDIR)/root.grub \
>>> @@ -120,6 +123,9 @@ define Image/Build/iso
>>> 		-e 's#@CMDLINE@#root=/dev/sr0 rootfstype=iso9660 rootwait $(strip $(call Image/cmdline/$(1)) $(BOOTOPTS) $(GRUB_CONSOLE_CMDLINE))#g' \
>>> 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>>> 		./grub-iso.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
>>> +  ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
>>> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
>>> +  endif
>> Ditto.
>> >
>>> 	mkisofs -R -b boot/grub/eltorito.img -no-emul-boot -boot-info-table \
>>> 		-o $(KDIR)/root.iso $(KDIR)/root.grub $(TARGET_DIR)
>>> endef
>>> -- 
>>> 2.16.3
Matthias Schiffer April 11, 2018, 7:26 a.m. UTC | #4 
On 04/03/2018 03:13 PM, Tomasz Maciej Nowak wrote:
> Create initrd image with packed microcode. This'll allow to load it at
> early boot stage. Unfortunately the package can't install files directly
> to /boot directory, therefore additional installation hooks are placed
> for standalone package and when building directly into target image.


> 
> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
> ---
>  package/firmware/intel-microcode/Makefile | 32 +++++++++++++++++++++++++------
>  target/linux/x86/image/Makefile           |  6 ++++++
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/package/firmware/intel-microcode/Makefile b/package/firmware/intel-microcode/Makefile
> index d2663bb901..9970f8f072 100644
> --- a/package/firmware/intel-microcode/Makefile
> +++ b/package/firmware/intel-microcode/Makefile
> @@ -35,15 +35,35 @@ define Package/intel-microcode
>  endef
>  
>  define Build/Compile
> -	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool $(MAKE) -C $(PKG_BUILD_DIR)
> -	mkdir $(PKG_BUILD_DIR)/intel-ucode
> -	$(STAGING_DIR)/../host/bin/iucode_tool -q \
> -		--write-firmware=$(PKG_BUILD_DIR)/intel-ucode $(PKG_BUILD_DIR)/$(MICROCODE).bin
> +	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool \
> +		$(MAKE) -C $(PKG_BUILD_DIR)
> +	$(STAGING_DIR)/../host/bin/iucode_tool -q --mini-earlyfw \
> +		--write-earlyfw=$(PKG_BUILD_DIR)/intel-ucode.cpio \
> +		$(PKG_BUILD_DIR)/$(MICROCODE).bin
>  endef
>  
>  define Package/intel-microcode/install
> -	$(INSTALL_DIR) $(1)/lib/firmware/intel-ucode
> -	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode/* $(1)/lib/firmware/intel-ucode
> +	$(INSTALL_DIR) $(1)/lib/firmware
> +	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode.cpio \
> +		$(1)/lib/firmware/intel-ucode.img
> +endef
> +
> +ifeq ($(CONFIG_PACKAGE_intel-microcode),m)

This condition is problematic: Even when the package is built into the
image, the generated ipk package should work on other systems as well; this
means the contents of the package may not depend on the m/y setting in any way.

Normally, we would deal with such cases by checking $IPKG_INSTROOT inside
the postinst script. But in this case, I'd prefer to change /boot always to
be mounted rw, so we can just get rid of the postinst/prerm scripts and
special /boot handling.

> +define Package/intel-microcode/postinst
> +#!/bin/sh
> +
> +mount /boot -o remount,rw,noatime
> +cp -f /lib/firmware/intel-ucode.img /boot/
> +mount /boot -o remount,ro,noatime
> +endef
> +endif
> +
> +define Package/intel-microcode/prerm
> +#!/bin/sh
> +
> +mount /boot -o remount,rw,noatime
> +rm /boot/intel-ucode.img
> +mount /boot -o remount,ro,noatime
>  endef
>  
>  $(eval $(call BuildPackage,intel-microcode))
> diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
> index a05f4babd9..4d6a3016d2 100644
> --- a/target/linux/x86/image/Makefile
> +++ b/target/linux/x86/image/Makefile
> @@ -83,6 +83,9 @@ ifneq ($(CONFIG_GRUB_IMAGES),)
>  		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>  		-e 's#@ROOT@#$(GRUB_ROOT)#g' \
>  		./grub.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
> +    ifeq ($(CONFIG_PACKAGE_intel-microcode),y)

We should get rid of this hack too, and move everything that is found in
/boot of the rootfs into the boot partition rather than dealing with
individual packages here.

> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
> +    endif
>  	PADDING="$(CONFIG_TARGET_IMAGES_PAD)" SIGNATURE="$(SIGNATURE)" PATH="$(TARGET_PATH)" $(SCRIPT_DIR)/gen_image_generic.sh \
>  		$(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img \
>  		$(CONFIG_TARGET_KERNEL_PARTSIZE) $(KDIR)/root.grub \
> @@ -120,6 +123,9 @@ define Image/Build/iso
>  		-e 's#@CMDLINE@#root=/dev/sr0 rootfstype=iso9660 rootwait $(strip $(call Image/cmdline/$(1)) $(BOOTOPTS) $(GRUB_CONSOLE_CMDLINE))#g' \
>  		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>  		./grub-iso.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
> +  ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
> +  endif
>  	mkisofs -R -b boot/grub/eltorito.img -no-emul-boot -boot-info-table \
>  		-o $(KDIR)/root.iso $(KDIR)/root.grub $(TARGET_DIR)
>  endef
> 

Regards,
Matthias
Tomasz Maciej Nowak April 11, 2018, 1:40 p.m. UTC | #5 
W dniu 11.04.2018 o 09:26, Matthias Schiffer pisze:
> On 04/03/2018 03:13 PM, Tomasz Maciej Nowak wrote:
>> Create initrd image with packed microcode. This'll allow to load it at
>> early boot stage. Unfortunately the package can't install files directly
>> to /boot directory, therefore additional installation hooks are placed
>> for standalone package and when building directly into target image.
> 
> 
>>
>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
>> ---
>>   package/firmware/intel-microcode/Makefile | 32 +++++++++++++++++++++++++------
>>   target/linux/x86/image/Makefile           |  6 ++++++
>>   2 files changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/package/firmware/intel-microcode/Makefile b/package/firmware/intel-microcode/Makefile
>> index d2663bb901..9970f8f072 100644
>> --- a/package/firmware/intel-microcode/Makefile
>> +++ b/package/firmware/intel-microcode/Makefile
>> @@ -35,15 +35,35 @@ define Package/intel-microcode
>>   endef
>>   
>>   define Build/Compile
>> -	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool $(MAKE) -C $(PKG_BUILD_DIR)
>> -	mkdir $(PKG_BUILD_DIR)/intel-ucode
>> -	$(STAGING_DIR)/../host/bin/iucode_tool -q \
>> -		--write-firmware=$(PKG_BUILD_DIR)/intel-ucode $(PKG_BUILD_DIR)/$(MICROCODE).bin
>> +	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool \
>> +		$(MAKE) -C $(PKG_BUILD_DIR)
>> +	$(STAGING_DIR)/../host/bin/iucode_tool -q --mini-earlyfw \
>> +		--write-earlyfw=$(PKG_BUILD_DIR)/intel-ucode.cpio \
>> +		$(PKG_BUILD_DIR)/$(MICROCODE).bin
>>   endef
>>   
>>   define Package/intel-microcode/install
>> -	$(INSTALL_DIR) $(1)/lib/firmware/intel-ucode
>> -	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode/* $(1)/lib/firmware/intel-ucode
>> +	$(INSTALL_DIR) $(1)/lib/firmware
>> +	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode.cpio \
>> +		$(1)/lib/firmware/intel-ucode.img
>> +endef
>> +
>> +ifeq ($(CONFIG_PACKAGE_intel-microcode),m)
> 
> This condition is problematic: Even when the package is built into the
> image, the generated ipk package should work on other systems as well; this
> means the contents of the package may not depend on the m/y setting in any way.

That's a major oversight on my part, thanks.

> 
> Normally, we would deal with such cases by checking $IPKG_INSTROOT inside
> the postinst script. But in this case, I'd prefer to change /boot always to
> be mounted rw, so we can just get rid of the postinst/prerm scripts and
> special /boot handling.

That will make things a lot less complicated.

> 
>> +define Package/intel-microcode/postinst
>> +#!/bin/sh
>> +
>> +mount /boot -o remount,rw,noatime
>> +cp -f /lib/firmware/intel-ucode.img /boot/
>> +mount /boot -o remount,ro,noatime
>> +endef
>> +endif
>> +
>> +define Package/intel-microcode/prerm
>> +#!/bin/sh
>> +
>> +mount /boot -o remount,rw,noatime
>> +rm /boot/intel-ucode.img
>> +mount /boot -o remount,ro,noatime
>>   endef
>>   
>>   $(eval $(call BuildPackage,intel-microcode))
>> diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
>> index a05f4babd9..4d6a3016d2 100644
>> --- a/target/linux/x86/image/Makefile
>> +++ b/target/linux/x86/image/Makefile
>> @@ -83,6 +83,9 @@ ifneq ($(CONFIG_GRUB_IMAGES),)
>>   		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>>   		-e 's#@ROOT@#$(GRUB_ROOT)#g' \
>>   		./grub.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
>> +    ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
> 
> We should get rid of this hack too, and move everything that is found in
> /boot of the rootfs into the boot partition rather than dealing with
> individual packages here.

Will address all of the concerns in next version.

> 
>> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
>> +    endif
>>   	PADDING="$(CONFIG_TARGET_IMAGES_PAD)" SIGNATURE="$(SIGNATURE)" PATH="$(TARGET_PATH)" $(SCRIPT_DIR)/gen_image_generic.sh \
>>   		$(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img \
>>   		$(CONFIG_TARGET_KERNEL_PARTSIZE) $(KDIR)/root.grub \
>> @@ -120,6 +123,9 @@ define Image/Build/iso
>>   		-e 's#@CMDLINE@#root=/dev/sr0 rootfstype=iso9660 rootwait $(strip $(call Image/cmdline/$(1)) $(BOOTOPTS) $(GRUB_CONSOLE_CMDLINE))#g' \
>>   		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
>>   		./grub-iso.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
>> +  ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
>> +	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
>> +  endif
>>   	mkisofs -R -b boot/grub/eltorito.img -no-emul-boot -boot-info-table \
>>   		-o $(KDIR)/root.iso $(KDIR)/root.grub $(TARGET_DIR)
>>   endef
>>
> 
> Regards,
> Matthias
> 

Thanks for the review.
diff mbox series

Patch

diff --git a/package/firmware/intel-microcode/Makefile b/package/firmware/intel-microcode/Makefile
index d2663bb901..9970f8f072 100644
--- a/package/firmware/intel-microcode/Makefile
+++ b/package/firmware/intel-microcode/Makefile
@@ -35,15 +35,35 @@  define Package/intel-microcode
 endef
 
 define Build/Compile
-	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool $(MAKE) -C $(PKG_BUILD_DIR)
-	mkdir $(PKG_BUILD_DIR)/intel-ucode
-	$(STAGING_DIR)/../host/bin/iucode_tool -q \
-		--write-firmware=$(PKG_BUILD_DIR)/intel-ucode $(PKG_BUILD_DIR)/$(MICROCODE).bin
+	IUCODE_TOOL=$(STAGING_DIR)/../host/bin/iucode_tool \
+		$(MAKE) -C $(PKG_BUILD_DIR)
+	$(STAGING_DIR)/../host/bin/iucode_tool -q --mini-earlyfw \
+		--write-earlyfw=$(PKG_BUILD_DIR)/intel-ucode.cpio \
+		$(PKG_BUILD_DIR)/$(MICROCODE).bin
 endef
 
 define Package/intel-microcode/install
-	$(INSTALL_DIR) $(1)/lib/firmware/intel-ucode
-	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode/* $(1)/lib/firmware/intel-ucode
+	$(INSTALL_DIR) $(1)/lib/firmware
+	$(INSTALL_DATA) $(PKG_BUILD_DIR)/intel-ucode.cpio \
+		$(1)/lib/firmware/intel-ucode.img
+endef
+
+ifeq ($(CONFIG_PACKAGE_intel-microcode),m)
+define Package/intel-microcode/postinst
+#!/bin/sh
+
+mount /boot -o remount,rw,noatime
+cp -f /lib/firmware/intel-ucode.img /boot/
+mount /boot -o remount,ro,noatime
+endef
+endif
+
+define Package/intel-microcode/prerm
+#!/bin/sh
+
+mount /boot -o remount,rw,noatime
+rm /boot/intel-ucode.img
+mount /boot -o remount,ro,noatime
 endef
 
 $(eval $(call BuildPackage,intel-microcode))
diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
index a05f4babd9..4d6a3016d2 100644
--- a/target/linux/x86/image/Makefile
+++ b/target/linux/x86/image/Makefile
@@ -83,6 +83,9 @@  ifneq ($(CONFIG_GRUB_IMAGES),)
 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
 		-e 's#@ROOT@#$(GRUB_ROOT)#g' \
 		./grub.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
+    ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
+	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
+    endif
 	PADDING="$(CONFIG_TARGET_IMAGES_PAD)" SIGNATURE="$(SIGNATURE)" PATH="$(TARGET_PATH)" $(SCRIPT_DIR)/gen_image_generic.sh \
 		$(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img \
 		$(CONFIG_TARGET_KERNEL_PARTSIZE) $(KDIR)/root.grub \
@@ -120,6 +123,9 @@  define Image/Build/iso
 		-e 's#@CMDLINE@#root=/dev/sr0 rootfstype=iso9660 rootwait $(strip $(call Image/cmdline/$(1)) $(BOOTOPTS) $(GRUB_CONSOLE_CMDLINE))#g' \
 		-e 's#@TIMEOUT@#$(GRUB_TIMEOUT)#g' \
 		./grub-iso.cfg > $(KDIR)/root.grub/boot/grub/grub.cfg
+  ifeq ($(CONFIG_PACKAGE_intel-microcode),y)
+	$(CP) $(STAGING_DIR)/root-x86/lib/firmware/intel-ucode.img $(KDIR)/root.grub/boot/
+  endif
 	mkisofs -R -b boot/grub/eltorito.img -no-emul-boot -boot-info-table \
 		-o $(KDIR)/root.iso $(KDIR)/root.grub $(TARGET_DIR)
 endef