From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48613) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgZkR-0004vU-NQ for qemu-devel@nongnu.org; Fri, 10 Apr 2015 10:17:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YgZkP-0007C8-Hp for qemu-devel@nongnu.org; Fri, 10 Apr 2015 10:17:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45395) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YgZkP-0007Bw-7p for qemu-devel@nongnu.org; Fri, 10 Apr 2015 10:17:29 -0400 From: Gerd Hoffmann Date: Fri, 10 Apr 2015 16:17:08 +0200 Message-Id: <1428675432-31433-4-git-send-email-kraxel@redhat.com> In-Reply-To: <1428675432-31433-1-git-send-email-kraxel@redhat.com> References: <1428675432-31433-1-git-send-email-kraxel@redhat.com> Subject: [Qemu-devel] [PATCH 3/7] ipxe: add local patches List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: ipxe-devel@ipxe.org, Gerd Hoffmann There are two ipxe patches needed to make efi pxe boots work. They didn't made it upstream yet, and I don't want to wait any longer with updating qemu. So add them here, with some logic to apply them before building ipxe. /me still hopes I can revert that patch some day. Signed-off-by: Gerd Hoffmann --- roms/Makefile | 12 +- ...rove-compliance-with-the-EFI_SIMPLE_NETWO.patch | 160 +++++++++++++++++++++ ...0002-efi-make-load-file-protocol-optional.patch | 102 +++++++++++++ 3 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 roms/ipxe-patches/0001-efi_snp-improve-compliance-with-the-EFI_SIMPLE_NETWO.patch create mode 100644 roms/ipxe-patches/0002-efi-make-load-file-protocol-optional.patch diff --git a/roms/Makefile b/roms/Makefile index 461cb49..ab4532c 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -115,12 +115,12 @@ efi-rom-%: build-pxe-roms build-efi-roms -ec ipxe/src/bin-x86_64-efi/$(VID)$(DID).efidrv \ -o ../pc-bios/efi-$*.rom -build-pxe-roms: ipxe/src/config/local/general.h +build-pxe-roms: ipxe/qemu-patches ipxe/src/config/local/general.h $(MAKE) -C ipxe/src GITVERSION="" \ CROSS_COMPILE=$(x86_64_cross_prefix) \ $(patsubst %,bin/%.rom,$(pxerom_targets)) -build-efi-roms: build-pxe-roms ipxe/src/config/local/general.h +build-efi-roms: ipxe/qemu-patches build-pxe-roms ipxe/src/config/local/general.h $(MAKE) -C ipxe/src GITVERSION="" \ CROSS_COMPILE=$(x86_64_cross_prefix) \ $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \ @@ -129,6 +129,12 @@ build-efi-roms: build-pxe-roms ipxe/src/config/local/general.h ipxe/src/config/local/%: config.ipxe.% cp $< $@ +ipxe/qemu-patches: + for patch in ipxe-patches/*; do \ + echo "# applying $$patch"; \ + cat $$patch | (cd ipxe; patch -p1); \ + done + touch $@ slof: $(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu @@ -148,6 +154,6 @@ clean: $(MAKE) -C sgabios clean rm -f sgabios/.depend $(MAKE) -C ipxe/src veryclean - (cd ipxe; rm -f src/config/local/*.h) + (cd ipxe; git reset --hard; rm -f qemu-patches src/config/local/*.h) $(MAKE) -C SLOF clean rm -rf u-boot/build.e500 diff --git a/roms/ipxe-patches/0001-efi_snp-improve-compliance-with-the-EFI_SIMPLE_NETWO.patch b/roms/ipxe-patches/0001-efi_snp-improve-compliance-with-the-EFI_SIMPLE_NETWO.patch new file mode 100644 index 0000000..7f6febf --- /dev/null +++ b/roms/ipxe-patches/0001-efi_snp-improve-compliance-with-the-EFI_SIMPLE_NETWO.patch @@ -0,0 +1,160 @@ +From 9e870d92035ec7ca946e702236bfe104e964f8c6 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Thu, 22 Jan 2015 22:05:35 +0100 +Subject: [PATCH 1/2] efi_snp: improve compliance with the + EFI_SIMPLE_NETWORK_PROTOCOL spec + +The efi_snp interface dates back to 2008, when the GetStatus() interface +must have been seriously under-specified. The UEFI Specification (2.4) +specifies EFI_SIMPLE_NETWORK_PROTOCOL in detail however. In short: + +- the Transmit() interface is assumed to link (not copy) the SNP client's + buffer and return at once (without blocking), taking ownership of the + buffer temporarily; + +- the GetStatus() interface releases one of the completed (transmitted or + internally copied) buffers back to the caller. If there are several + completed buffers, it is unspecified which one is returned. + +The EFI build of the grub boot loader actually verifies the buffer address +returned by GetStatus(), therefore in efi_snp we must at least fake the +queueing of client buffers. This patch doesn't track client buffers +together with the internally queued io_buffer structures, we consider a +client buffer recyclable as soon as we make a deep copy of it and queue +the copy internally. + +Signed-off-by: Laszlo Ersek +Signed-off-by: Gerd Hoffmann +--- + src/include/ipxe/efi/efi_snp.h | 6 +++++ + src/interface/efi/efi_snp.c | 54 ++++++++++++++++++++++++------------------ + 2 files changed, 37 insertions(+), 23 deletions(-) + +diff --git a/src/include/ipxe/efi/efi_snp.h b/src/include/ipxe/efi/efi_snp.h +index a18bced..863a81a 100644 +--- a/src/include/ipxe/efi/efi_snp.h ++++ b/src/include/ipxe/efi/efi_snp.h +@@ -18,6 +18,8 @@ + #include + #include + ++#define MAX_RECYCLED_TXBUFS 64 ++ + /** An SNP device */ + struct efi_snp_device { + /** List of SNP devices */ +@@ -44,6 +46,10 @@ struct efi_snp_device { + * Used in order to generate TX completions. + */ + unsigned int tx_count_txbufs; ++ /** Holds the addresses of recycled SNP client buffers; a ring. */ ++ void *tx_recycled_txbufs[MAX_RECYCLED_TXBUFS]; ++ /** The index of the first buffer to return to the SNP client. */ ++ unsigned tx_first_txbuf; + /** Outstanding RX packet count (via "interrupt status") */ + unsigned int rx_count_interrupts; + /** Outstanding RX packet count (via WaitForPacket event) */ +diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c +index 67fba34..c21af33 100644 +--- a/src/interface/efi/efi_snp.c ++++ b/src/interface/efi/efi_snp.c +@@ -68,6 +68,14 @@ static void efi_snp_set_state ( struct efi_snp_device *snpdev ) { + */ + mode->State = EfiSimpleNetworkInitialized; + } ++ ++ if (mode->State != EfiSimpleNetworkInitialized) { ++ /* Zero the number of recycled buffers when moving to any other ++ * state than Initialized. Transmit() and GetStatus() are only ++ * valid in Initialized. ++ */ ++ snpdev->tx_count_txbufs = 0; ++ } + } + + /** +@@ -446,12 +454,12 @@ efi_snp_nvdata ( EFI_SIMPLE_NETWORK_PROTOCOL *snp, BOOLEAN read, + * + * @v snp SNP interface + * @v interrupts Interrupt status, or NULL +- * @v txbufs Recycled transmit buffer address, or NULL ++ * @v txbuf Recycled transmit buffer address, or NULL + * @ret efirc EFI status code + */ + static EFI_STATUS EFIAPI + efi_snp_get_status ( EFI_SIMPLE_NETWORK_PROTOCOL *snp, +- UINT32 *interrupts, VOID **txbufs ) { ++ UINT32 *interrupts, VOID **txbuf ) { + struct efi_snp_device *snpdev = + container_of ( snp, struct efi_snp_device, snp ); + +@@ -485,30 +493,22 @@ efi_snp_get_status ( EFI_SIMPLE_NETWORK_PROTOCOL *snp, + DBGC2 ( snpdev, " INTS:%02x", *interrupts ); + } + +- /* TX completions. It would be possible to design a more +- * idiotic scheme for this, but it would be a challenge. +- * According to the UEFI header file, txbufs will be filled in +- * with a list of "recycled transmit buffers" (i.e. completed +- * TX buffers). Observant readers may care to note that +- * *txbufs is a void pointer. Precisely how a list of +- * completed transmit buffers is meant to be represented as an +- * array of voids is left as an exercise for the reader. +- * +- * The only users of this interface (MnpDxe/MnpIo.c and +- * PxeBcDxe/Bc.c within the EFI dev kit) both just poll until +- * seeing a non-NULL result return in txbufs. This is valid +- * provided that they do not ever attempt to transmit more +- * than one packet concurrently (and that TX never times out). ++ /* In efi_snp_transmit() we enqueue packets by copying them (not by ++ * linking them), hence we can recycle them immediately to the SNP ++ * client. + */ +- if ( txbufs ) { +- if ( snpdev->tx_count_txbufs && +- list_empty ( &snpdev->netdev->tx_queue ) ) { +- *txbufs = "Which idiot designed this API?"; ++ if ( txbuf ) { ++ if ( snpdev->tx_count_txbufs ) { ++ unsigned first; ++ ++ first = snpdev->tx_first_txbuf++; ++ snpdev->tx_first_txbuf %= MAX_RECYCLED_TXBUFS; ++ *txbuf = snpdev->tx_recycled_txbufs[first]; + snpdev->tx_count_txbufs--; + } else { +- *txbufs = NULL; ++ *txbuf = NULL; + } +- DBGC2 ( snpdev, " TX:%s", ( *txbufs ? "some" : "none" ) ); ++ DBGC2 ( snpdev, " TX:%p", *txbuf ); + } + + DBGC2 ( snpdev, "\n" ); +@@ -560,6 +560,12 @@ efi_snp_transmit ( EFI_SIMPLE_NETWORK_PROTOCOL *snp, + if ( efi_snp_claimed ) + return EFI_NOT_READY; + ++ assert ( snpdev->tx_count_txbufs <= MAX_RECYCLED_TXBUFS ); ++ if ( snpdev->tx_count_txbufs == MAX_RECYCLED_TXBUFS ) { ++ /* No room to recycle another buffer. */ ++ return EFI_NOT_READY; ++ } ++ + /* Sanity checks */ + if ( ll_header_len ) { + if ( ll_header_len != ll_protocol->ll_header_len ) { +@@ -626,7 +632,9 @@ efi_snp_transmit ( EFI_SIMPLE_NETWORK_PROTOCOL *snp, + + /* Record transmission as outstanding */ + snpdev->tx_count_interrupts++; +- snpdev->tx_count_txbufs++; ++ snpdev->tx_recycled_txbufs[(snpdev->tx_first_txbuf + ++ snpdev->tx_count_txbufs++ ++ ) % MAX_RECYCLED_TXBUFS] = data; + + return 0; + +-- +1.8.3.1 + diff --git a/roms/ipxe-patches/0002-efi-make-load-file-protocol-optional.patch b/roms/ipxe-patches/0002-efi-make-load-file-protocol-optional.patch new file mode 100644 index 0000000..f921a3b --- /dev/null +++ b/roms/ipxe-patches/0002-efi-make-load-file-protocol-optional.patch @@ -0,0 +1,102 @@ +From 2daea2b8dd2c504a4f76a6b0b67bd3c4a2957fc7 Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Tue, 10 Feb 2015 14:28:09 +0100 +Subject: [PATCH 2/2] [efi] make load file protocol optional + +The load file implementation added by commit +c7c3d839fc9120aee28de9aabe452dc85ad91502 doesn't support loading +arbitrary files from the tftp server, so efi applications trying +to do exactly that fail to boot: + + iPXE 1.0.0+ (17ace) -- Open Source Network Boot Firmware -- http://ipxe.org + Features: HTTP DNS TFTP EFI Menu + + net0: 52:54:00:47:d3:07 using virtio-net on PCI00:09.0 (open) + [Link:up, TX:0 TXE:0 RX:13 RXE:2] + [RXE: 2 x "Operation not supported (http://ipxe.org/3c086083)"] + Configuring (net0 52:54:00:47:d3:07)...... ok + net0: 192.168.132.93/255.255.255.0 gw 192.168.132.1 + Next server: 192.168.132.1 + Filename: shim.efi + tftp://192.168.132.1/shim.efi... ok + Failed to open grubx64.efi - Not Found + Failed to load image grubx64.efi: Not Found + Failed to open MokManager.efi - Not Found + Failed to load image MokManager.efi: Not Found + Could not boot image: Error 0x7f04828e (http://ipxe.org/7f04828e) + + Boot Failed. EFI Network + +This is not acceptable for qemu. efi pxe configurations which work +just fine with real hardware must work with qemu virtual machines too. + +This patch adds a config option for the load file protocol +implementation, to allow it being disabled, so we can turn it off +for the pxe roms shipped with qemu. + +The default for the new option maintains current behavior, i.e. +load file is enabled unless you override it in config/local/general.h + +Suggested-by: Laszlo Ersek + +See discussion here: + http://lists.ipxe.org/pipermail/ipxe-devel/2015-February/003979.html + +Signed-off-by: Gerd Hoffmann +--- + src/config/general.h | 6 ++++++ + src/interface/efi/efi_snp.c | 5 +++++ + 2 files changed, 11 insertions(+) + +diff --git a/src/config/general.h b/src/config/general.h +index 65c1f85..8c91601 100644 +--- a/src/config/general.h ++++ b/src/config/general.h +@@ -142,6 +142,12 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + #undef NONPNP_HOOK_INT19 /* Hook INT19 on non-PnP BIOSes */ + + /* ++ * EFI specific options ++ * ++ */ ++#define EFI_PROTO_LOAD_FILE /* register LOAD_FILE protocol */ ++ ++/* + * Error message tables to include + * + */ +diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c +index c21af33..85f4fa0 100644 +--- a/src/interface/efi/efi_snp.c ++++ b/src/interface/efi/efi_snp.c +@@ -34,6 +34,7 @@ FILE_LICENCE ( GPL2_OR_LATER ); + #include + #include + #include ++#include + + /** List of SNP devices */ + static LIST_HEAD ( efi_snp_devices ); +@@ -1033,7 +1034,9 @@ static int efi_snp_probe ( struct net_device *netdev ) { + &efi_nii_protocol_guid, &snpdev->nii, + &efi_nii31_protocol_guid, &snpdev->nii, + &efi_component_name2_protocol_guid, &snpdev->name2, ++#ifdef EFI_PROTO_LOAD_FILE + &efi_load_file_protocol_guid, &snpdev->load_file, ++#endif + NULL ) ) != 0 ) { + rc = -EEFI ( efirc ); + DBGC ( snpdev, "SNPDEV %p could not install protocols: " +@@ -1082,7 +1085,9 @@ static int efi_snp_probe ( struct net_device *netdev ) { + &efi_nii_protocol_guid, &snpdev->nii, + &efi_nii31_protocol_guid, &snpdev->nii, + &efi_component_name2_protocol_guid, &snpdev->name2, ++#ifdef EFI_PROTO_LOAD_FILE + &efi_load_file_protocol_guid, &snpdev->load_file, ++#endif + NULL ); + err_install_protocol_interface: + free ( snpdev->path ); +-- +1.8.3.1 + -- 1.8.3.1