* [PATCH 1/5] tools/firmware: Build ipxe as a standalone ROM
2018-03-15 17:31 Make iPXE a standalone ROM Anoob Soman
@ 2018-03-15 17:31 ` Anoob Soman
2018-03-16 11:18 ` Jan Beulich
2018-03-15 17:31 ` [PATCH 2/5] tools/firmware: #define IPXE_PATH Anoob Soman
` (3 subsequent siblings)
4 siblings, 1 reply; 25+ messages in thread
From: Anoob Soman @ 2018-03-15 17:31 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Anoob Soman, ian.jackson, jbeulich, andrew.cooper3
This patches doesn't get rid of etherboot[] from roms.inc. Instead,
makes a standalone iPXE rom, which will later be used by hvmloader (when
all the plubming to use standalone iPXE rom are in place)
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
---
tools/firmware/Makefile | 3 +++
tools/firmware/hvmloader/Makefile | 8 +++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 5a7cf77..20cab38 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -59,6 +59,9 @@ ifeq ($(CONFIG_PV_SHIM),y)
$(INSTALL_DATA) xen-dir/xen-shim $(INST_DIR)/xen-shim
$(INSTALL_DATA) xen-dir/xen-shim-syms $(DEBG_DIR)/xen-shim-syms
endif
+ifeq ($(CONFIG_ROMBIOS),y)
+ $(INSTALL_DATA) etherboot/ipxe/src/bin/ipxe.bin $(INST_DIR)/ipxe.bin
+endif
.PHONY: uninstall
uninstall:
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index a5b4c32..087b41d 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -52,6 +52,7 @@ else
CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin
endif
ETHERBOOT_ROMS := $(addprefix ../etherboot/ipxe/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS)))
+ETHERBOOT_ROM := ../etherboot/ipxe/src/bin/ipxe.bin
endif
ROMS :=
@@ -71,7 +72,7 @@ all: acpi subdirs-all
acpi:
$(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) DSDT_FILES="$(DSDT_FILES)"
-rombios.o: roms.inc
+rombios.o: $(ETHERBOOT_ROM) roms.inc
smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
ACPI_PATH = ../../libacpi
@@ -113,6 +114,11 @@ endif
mv $@.new $@
+ifneq ($(ETHERBOOT_ROMS),)
+$(ETHERBOOT_ROM): $(ETHERBOOT_ROMS)
+ cat $^ > $@
+endif
+
.PHONY: clean
clean: subdirs-clean
rm -f roms.inc roms.inc.new acpi.h
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 1/5] tools/firmware: Build ipxe as a standalone ROM
2018-03-15 17:31 ` [PATCH 1/5] tools/firmware: Build ipxe as " Anoob Soman
@ 2018-03-16 11:18 ` Jan Beulich
2018-03-16 11:21 ` Andrew Cooper
2018-03-18 1:30 ` Doug Goldstein
0 siblings, 2 replies; 25+ messages in thread
From: Jan Beulich @ 2018-03-16 11:18 UTC (permalink / raw)
To: Anoob Soman; +Cc: andrew.cooper3, wei.liu2, ian.jackson, xen-devel
>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
> @@ -71,7 +72,7 @@ all: acpi subdirs-all
> acpi:
> $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) DSDT_FILES="$(DSDT_FILES)"
>
> -rombios.o: roms.inc
> +rombios.o: $(ETHERBOOT_ROM) roms.inc
Please don't introduce dead dependencies: If a need for this arises
in a later patch, add the dependency there.
> @@ -113,6 +114,11 @@ endif
>
> mv $@.new $@
>
> +ifneq ($(ETHERBOOT_ROMS),)
> +$(ETHERBOOT_ROM): $(ETHERBOOT_ROMS)
> + cat $^ > $@
> +endif
I don't understand this: How can the bare concatenation of multiple
binary blobs produce anything usable? How will the consumer know
where the boundaries are, and which one is which?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/5] tools/firmware: Build ipxe as a standalone ROM
2018-03-16 11:18 ` Jan Beulich
@ 2018-03-16 11:21 ` Andrew Cooper
2018-03-18 1:30 ` Doug Goldstein
1 sibling, 0 replies; 25+ messages in thread
From: Andrew Cooper @ 2018-03-16 11:21 UTC (permalink / raw)
To: Jan Beulich, Anoob Soman; +Cc: ian.jackson, wei.liu2, xen-devel
On 16/03/18 11:18, Jan Beulich wrote:
>
>> @@ -113,6 +114,11 @@ endif
>>
>> mv $@.new $@
>>
>> +ifneq ($(ETHERBOOT_ROMS),)
>> +$(ETHERBOOT_ROM): $(ETHERBOOT_ROMS)
>> + cat $^ > $@
>> +endif
> I don't understand this: How can the bare concatenation of multiple
> binary blobs produce anything usable? How will the consumer know
> where the boundaries are, and which one is which?
These details are all covered in the option ROM/PnP metadata. See
scan_etherboot_nic()/scan_option_rom() in hvmloader.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/5] tools/firmware: Build ipxe as a standalone ROM
2018-03-16 11:18 ` Jan Beulich
2018-03-16 11:21 ` Andrew Cooper
@ 2018-03-18 1:30 ` Doug Goldstein
2018-03-19 7:48 ` Jan Beulich
1 sibling, 1 reply; 25+ messages in thread
From: Doug Goldstein @ 2018-03-18 1:30 UTC (permalink / raw)
To: Jan Beulich, Anoob Soman; +Cc: andrew.cooper3, wei.liu2, ian.jackson, xen-devel
On 3/16/18 6:18 AM, Jan Beulich wrote:
>>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
>> @@ -71,7 +72,7 @@ all: acpi subdirs-all
>> acpi:
>> $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) DSDT_FILES="$(DSDT_FILES)"
>>
>> -rombios.o: roms.inc
>> +rombios.o: $(ETHERBOOT_ROM) roms.inc
>
> Please don't introduce dead dependencies: If a need for this arises
> in a later patch, add the dependency there.
Well this is what's creating the ipxe.bin that's being installed in the
section you snipped out.
But I kind of agree. I feels like patch 1 & 2 belong together maybe?
--
Doug Goldstein
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/5] tools/firmware: Build ipxe as a standalone ROM
2018-03-18 1:30 ` Doug Goldstein
@ 2018-03-19 7:48 ` Jan Beulich
2018-03-19 13:52 ` Doug Goldstein
0 siblings, 1 reply; 25+ messages in thread
From: Jan Beulich @ 2018-03-19 7:48 UTC (permalink / raw)
To: Doug Goldstein
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, Anoob Soman
>>> On 18.03.18 at 02:30, <cardoe@cardoe.com> wrote:
> On 3/16/18 6:18 AM, Jan Beulich wrote:
>>>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
>>> @@ -71,7 +72,7 @@ all: acpi subdirs-all
>>> acpi:
>>> $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) DSDT_FILES="$(DSDT_FILES)"
>>>
>>> -rombios.o: roms.inc
>>> +rombios.o: $(ETHERBOOT_ROM) roms.inc
>>
>> Please don't introduce dead dependencies: If a need for this arises
>> in a later patch, add the dependency there.
>
> Well this is what's creating the ipxe.bin that's being installed in the
> section you snipped out.
I don't understand: The question isn't what is being generated, but
whether rombios.o really depends on that binary blob, and nothing
in the patch here suggests it does. If the goal is simply to have a
dependency triggering the creation of the blob, then this should be
done via e.g. TARGET, and the whole logic should rather sit in
firmware/Makefile (matching the installation of it done there).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/5] tools/firmware: Build ipxe as a standalone ROM
2018-03-19 7:48 ` Jan Beulich
@ 2018-03-19 13:52 ` Doug Goldstein
0 siblings, 0 replies; 25+ messages in thread
From: Doug Goldstein @ 2018-03-19 13:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, Anoob Soman
On 3/19/18 2:48 AM, Jan Beulich wrote:
>>>> On 18.03.18 at 02:30, <cardoe@cardoe.com> wrote:
>> On 3/16/18 6:18 AM, Jan Beulich wrote:
>>>>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
>>>> @@ -71,7 +72,7 @@ all: acpi subdirs-all
>>>> acpi:
>>>> $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) DSDT_FILES="$(DSDT_FILES)"
>>>>
>>>> -rombios.o: roms.inc
>>>> +rombios.o: $(ETHERBOOT_ROM) roms.inc
>>>
>>> Please don't introduce dead dependencies: If a need for this arises
>>> in a later patch, add the dependency there.
>>
>> Well this is what's creating the ipxe.bin that's being installed in the
>> section you snipped out.
>
> I don't understand: The question isn't what is being generated, but
> whether rombios.o really depends on that binary blob, and nothing
> in the patch here suggests it does. If the goal is simply to have a
> dependency triggering the creation of the blob, then this should be
> done via e.g. TARGET, and the whole logic should rather sit in
> firmware/Makefile (matching the installation of it done there).
>
> Jan
>
Oh, 100% agreement there.
--
Doug Goldstein
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/5] tools/firmware: #define IPXE_PATH
2018-03-15 17:31 Make iPXE a standalone ROM Anoob Soman
2018-03-15 17:31 ` [PATCH 1/5] tools/firmware: Build ipxe as " Anoob Soman
@ 2018-03-15 17:31 ` Anoob Soman
2018-03-21 15:18 ` Wei Liu
2018-03-15 17:31 ` [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest Anoob Soman
` (2 subsequent siblings)
4 siblings, 1 reply; 25+ messages in thread
From: Anoob Soman @ 2018-03-15 17:31 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Anoob Soman, ian.jackson, jbeulich, andrew.cooper3
--with-system-ipxe allows the user to specify ipxe rom. If this option
is given, use system supplied ipxe instead of building and installing
our own version
Plumbing for using iPXE roms, specified with --with-system-ipxe, doesn't
exist and will be added in future commits.
Re-run of autoconf is needed.
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
---
config/Tools.mk.in | 2 ++
tools/configure.ac | 18 ++++++++++++++++++
tools/firmware/Makefile | 4 ++--
tools/firmware/hvmloader/Makefile | 3 +++
4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 0f79f4e..dc6eae8 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -24,6 +24,7 @@ AWK := @AWK@
FETCHER := @FETCHER@
SEABIOS_PATH := @seabios_path@
OVMF_PATH := @ovmf_path@
+IPXE_PATH := @ipxe_path@
# Extra folder for libs/includes
PREPEND_INCLUDES := @PREPEND_INCLUDES@
@@ -59,6 +60,7 @@ CONFIG_QEMU_XEN := @qemu_xen@
CONFIG_BLKTAP2 := @blktap2@
CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
CONFIG_LIBNL := @libnl@
+CONFIG_IPXE := @ipxe@
CONFIG_SYSTEMD := @systemd@
SYSTEMD_CFLAGS := @SYSTEMD_CFLAGS@
diff --git a/tools/configure.ac b/tools/configure.ac
index 06eb16d..022a2f9 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -84,6 +84,7 @@ AX_ARG_DEFAULT_ENABLE([ocamltools], [Disable Ocaml tools])
AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disable XSM policy compilation])
AX_ARG_DEFAULT_DISABLE([ovmf], [Enable OVMF])
AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS])
+AX_ARG_DEFAULT_ENABLE([ipxe], [Disable iPXE])
AC_ARG_WITH([linux-backend-modules],
AS_HELP_STRING([--with-linux-backend-modules="mod1 mod2"],
@@ -241,6 +242,23 @@ AS_IF([test "x$ovmf" = "xy" -o -n "$ovmf_path" ], [
[OVMF path])
])
+AC_ARG_WITH([system-ipxe],
+ AS_HELP_STRING([--with-system-ipxe@<:@=PATH@:>@],
+ [Use system supplied ipxe PATH instead of building and installing
+ our own version]),[
+ # Disable compilation of iPXE.
+ ipxe=n
+ case $withval in
+ no) ipxe_path= ;;
+ *) ipxe_path=$withval ;;
+ esac
+],[])
+AS_IF([test "x$ipxe" = "xy" -o -n "$ipxe_path" ], [
+ AC_DEFINE_UNQUOTED([IPXE_PATH],
+ ["${ipxe_path:-$XENFIRMWAREDIR/ipxe.bin}"],
+ [IPXE path])
+])
+
AC_ARG_WITH([extra-qemuu-configure-args],
AS_HELP_STRING([--with-extra-qemuu-configure-args@<:@="--ARG1 ..."@:>@],
[List of additional configure options for upstream qemu]),[
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile
index 20cab38..ca333a9 100644
--- a/tools/firmware/Makefile
+++ b/tools/firmware/Makefile
@@ -15,7 +15,7 @@ SUBDIRS-$(CONFIG_OVMF) += ovmf-dir
SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir
SUBDIRS-$(CONFIG_ROMBIOS) += rombios
SUBDIRS-$(CONFIG_ROMBIOS) += vgabios
-SUBDIRS-$(CONFIG_ROMBIOS) += etherboot
+SUBDIRS-$(CONFIG_IPXE) += etherboot
SUBDIRS-$(CONFIG_PV_SHIM) += xen-dir
SUBDIRS-y += hvmloader
@@ -59,7 +59,7 @@ ifeq ($(CONFIG_PV_SHIM),y)
$(INSTALL_DATA) xen-dir/xen-shim $(INST_DIR)/xen-shim
$(INSTALL_DATA) xen-dir/xen-shim-syms $(DEBG_DIR)/xen-shim-syms
endif
-ifeq ($(CONFIG_ROMBIOS),y)
+ifeq ($(CONFIG_IPXE),y)
$(INSTALL_DATA) etherboot/ipxe/src/bin/ipxe.bin $(INST_DIR)/ipxe.bin
endif
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 087b41d..fa1bf30 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -51,6 +51,9 @@ CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
else
CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.bin
endif
+endif
+
+ifeq ($(CONFIG_IPXE),y)
ETHERBOOT_ROMS := $(addprefix ../etherboot/ipxe/src/bin/, $(addsuffix .rom, $(ETHERBOOT_NICS)))
ETHERBOOT_ROM := ../etherboot/ipxe/src/bin/ipxe.bin
endif
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 2/5] tools/firmware: #define IPXE_PATH
2018-03-15 17:31 ` [PATCH 2/5] tools/firmware: #define IPXE_PATH Anoob Soman
@ 2018-03-21 15:18 ` Wei Liu
2018-03-26 15:53 ` Anoob Soman
0 siblings, 1 reply; 25+ messages in thread
From: Wei Liu @ 2018-03-21 15:18 UTC (permalink / raw)
To: Anoob Soman; +Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, xen-devel
On Thu, Mar 15, 2018 at 05:31:50PM +0000, Anoob Soman wrote:
> --with-system-ipxe allows the user to specify ipxe rom. If this option
> is given, use system supplied ipxe instead of building and installing
> our own version
>
> Plumbing for using iPXE roms, specified with --with-system-ipxe, doesn't
> exist and will be added in future commits.
>
> Re-run of autoconf is needed.
>
> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
This looks fine. But it should probably be squashed to one of the other
patches.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/5] tools/firmware: #define IPXE_PATH
2018-03-21 15:18 ` Wei Liu
@ 2018-03-26 15:53 ` Anoob Soman
0 siblings, 0 replies; 25+ messages in thread
From: Anoob Soman @ 2018-03-26 15:53 UTC (permalink / raw)
To: Wei Liu; +Cc: andrew.cooper3, ian.jackson, jbeulich, xen-devel
On 21/03/18 15:18, Wei Liu wrote:
> On Thu, Mar 15, 2018 at 05:31:50PM +0000, Anoob Soman wrote:
>> --with-system-ipxe allows the user to specify ipxe rom. If this option
>> is given, use system supplied ipxe instead of building and installing
>> our own version
>>
>> Plumbing for using iPXE roms, specified with --with-system-ipxe, doesn't
>> exist and will be added in future commits.
>>
>> Re-run of autoconf is needed.
>>
>> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
> This looks fine. But it should probably be squashed to one of the other
Sure, let me see what I can do.
> patches.
>
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest
2018-03-15 17:31 Make iPXE a standalone ROM Anoob Soman
2018-03-15 17:31 ` [PATCH 1/5] tools/firmware: Build ipxe as " Anoob Soman
2018-03-15 17:31 ` [PATCH 2/5] tools/firmware: #define IPXE_PATH Anoob Soman
@ 2018-03-15 17:31 ` Anoob Soman
2018-03-18 1:32 ` Doug Goldstein
2018-03-21 15:17 ` Wei Liu
2018-03-15 17:31 ` [PATCH 4/5] libxl: Load iPXE ROM from a file Anoob Soman
2018-03-15 17:31 ` [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file Anoob Soman
4 siblings, 2 replies; 25+ messages in thread
From: Anoob Soman @ 2018-03-15 17:31 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Anoob Soman, ian.jackson, jbeulich, andrew.cooper3
This allows to load iPXE rom as a firmware module, instead of requiring
it to be embedded into hvmloader.
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
---
tools/libxc/xc_dom_x86.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 0b65dab..be06d43 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1723,6 +1723,19 @@ static int bootlate_hvm(struct xc_dom_image *dom)
{
add_module_to_list(dom, &dom->system_firmware_module, "firmware",
modlist, start_info);
+ for ( i = 0; i < dom->num_modules; i++ )
+ {
+ struct xc_hvm_firmware_module mod;
+
+ DOMPRINTF("Adding module %u", i);
+ mod.guest_addr_out =
+ dom->modules[i].seg.vstart;
+ mod.length =
+ dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
+
+ add_module_to_list(dom, &mod, dom->modules[i].cmdline,
+ modlist, start_info);
+ }
}
if ( start_info->nr_modules )
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest
2018-03-15 17:31 ` [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest Anoob Soman
@ 2018-03-18 1:32 ` Doug Goldstein
2018-03-19 14:31 ` Anoob Soman
2018-03-21 15:17 ` Wei Liu
1 sibling, 1 reply; 25+ messages in thread
From: Doug Goldstein @ 2018-03-18 1:32 UTC (permalink / raw)
To: Anoob Soman, xen-devel; +Cc: ian.jackson, wei.liu2, jbeulich, andrew.cooper3
On 3/15/18 12:31 PM, Anoob Soman wrote:
> This allows to load iPXE rom as a firmware module, instead of requiring
> it to be embedded into hvmloader.
>
> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
> ---
> tools/libxc/xc_dom_x86.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 0b65dab..be06d43 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1723,6 +1723,19 @@ static int bootlate_hvm(struct xc_dom_image *dom)
> {
> add_module_to_list(dom, &dom->system_firmware_module, "firmware",
> modlist, start_info);
> + for ( i = 0; i < dom->num_modules; i++ )
> + {
> + struct xc_hvm_firmware_module mod;
> +
> + DOMPRINTF("Adding module %u", i);
nothing more helpful for debugging than the for loop's number available?
--
Doug Goldstein
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest
2018-03-18 1:32 ` Doug Goldstein
@ 2018-03-19 14:31 ` Anoob Soman
0 siblings, 0 replies; 25+ messages in thread
From: Anoob Soman @ 2018-03-19 14:31 UTC (permalink / raw)
To: Doug Goldstein, xen-devel; +Cc: ian.jackson, wei.liu2, jbeulich, andrew.cooper3
On 18/03/18 01:32, Doug Goldstein wrote:
> On 3/15/18 12:31 PM, Anoob Soman wrote:
>> This allows to load iPXE rom as a firmware module, instead of requiring
>> it to be embedded into hvmloader.
>>
>> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
>> ---
>> tools/libxc/xc_dom_x86.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
>> index 0b65dab..be06d43 100644
>> --- a/tools/libxc/xc_dom_x86.c
>> +++ b/tools/libxc/xc_dom_x86.c
>> @@ -1723,6 +1723,19 @@ static int bootlate_hvm(struct xc_dom_image *dom)
>> {
>> add_module_to_list(dom, &dom->system_firmware_module, "firmware",
>> modlist, start_info);
>> + for ( i = 0; i < dom->num_modules; i++ )
>> + {
>> + struct xc_hvm_firmware_module mod;
>> +
>> + DOMPRINTF("Adding module %u", i);
> nothing more helpful for debugging than the for loop's number available?
>
I will add guest_addr_out, cmdline and length to DOMPRINTF.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest
2018-03-15 17:31 ` [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest Anoob Soman
2018-03-18 1:32 ` Doug Goldstein
@ 2018-03-21 15:17 ` Wei Liu
2018-03-26 15:51 ` Anoob Soman
1 sibling, 1 reply; 25+ messages in thread
From: Wei Liu @ 2018-03-21 15:17 UTC (permalink / raw)
To: Anoob Soman; +Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, xen-devel
On Thu, Mar 15, 2018 at 05:31:51PM +0000, Anoob Soman wrote:
> This allows to load iPXE rom as a firmware module, instead of requiring
> it to be embedded into hvmloader.
>
> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
> ---
> tools/libxc/xc_dom_x86.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 0b65dab..be06d43 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1723,6 +1723,19 @@ static int bootlate_hvm(struct xc_dom_image *dom)
> {
> add_module_to_list(dom, &dom->system_firmware_module, "firmware",
> modlist, start_info);
> + for ( i = 0; i < dom->num_modules; i++ )
> + {
> + struct xc_hvm_firmware_module mod;
> +
> + DOMPRINTF("Adding module %u", i);
> + mod.guest_addr_out =
> + dom->modules[i].seg.vstart;
Shouldn't this be dom->modules[i].seg.vstart - dom->parms.virt_base?
> + mod.length =
> + dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
> +
> + add_module_to_list(dom, &mod, dom->modules[i].cmdline,
> + modlist, start_info);
> + }
Now that both paths of the if ... else ... structure contain the same
code it should be lifted outside of the loop.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest
2018-03-21 15:17 ` Wei Liu
@ 2018-03-26 15:51 ` Anoob Soman
0 siblings, 0 replies; 25+ messages in thread
From: Anoob Soman @ 2018-03-26 15:51 UTC (permalink / raw)
To: Wei Liu; +Cc: andrew.cooper3, ian.jackson, jbeulich, xen-devel
On 21/03/18 15:17, Wei Liu wrote:
> On Thu, Mar 15, 2018 at 05:31:51PM +0000, Anoob Soman wrote:
>> modlist, start_info);
>> + for ( i = 0; i < dom->num_modules; i++ )
>> + {
>> + struct xc_hvm_firmware_module mod;
>> +
>> + DOMPRINTF("Adding module %u", i);
>> + mod.guest_addr_out =
>> + dom->modules[i].seg.vstart;
> Shouldn't this be dom->modules[i].seg.vstart - dom->parms.virt_base?
It didn't work when guest_addr_out = seg.vstart - virt_base. I will try
to figure out why.
>
>> + mod.length =
>> + dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
>> +
>> + add_module_to_list(dom, &mod, dom->modules[i].cmdline,
>> + modlist, start_info);
>> + }
> Now that both paths of the if ... else ... structure contain the same
> code it should be lifted outside of the loop.
Sure, I can do that.
-Anoob
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/5] libxl: Load iPXE ROM from a file
2018-03-15 17:31 Make iPXE a standalone ROM Anoob Soman
` (2 preceding siblings ...)
2018-03-15 17:31 ` [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest Anoob Soman
@ 2018-03-15 17:31 ` Anoob Soman
2018-03-18 1:34 ` Doug Goldstein
2018-03-21 15:25 ` Wei Liu
2018-03-15 17:31 ` [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file Anoob Soman
4 siblings, 2 replies; 25+ messages in thread
From: Anoob Soman @ 2018-03-15 17:31 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Anoob Soman, ian.jackson, jbeulich, andrew.cooper3
Load iPXE ROM from a file pointed to by IPXE_PATH. If --with-system-ipxe
is not specified default Xen firmware directory is picked up as
IPXE_PATH
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
---
tools/libxl/libxl_dom.c | 12 ++++++++++++
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_paths.c | 9 +++++++++
3 files changed, 22 insertions(+)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 2e29b52..104d6a0 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1003,6 +1003,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
int datalen = 0;
void *data;
const char *bios_filename = NULL;
+ const char *ipxe_filename = NULL;
if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
if (info->u.hvm.firmware) {
@@ -1094,6 +1095,17 @@ static int libxl__domain_firmware(libxl__gc *gc,
assert(info->type == LIBXL_DOMAIN_TYPE_HVM);
rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
libxl__xenfirmwaredir_path()));
+ if (rc) {
+ LOGE(ERROR, "xc_dom_kernel_file failed");
+ goto out;
+ }
+ if ((ipxe_filename = libxl__ipxe_path())) {
+ rc = xc_dom_module_file(dom, ipxe_filename, "ipxe");
+ if (rc) {
+ LOGE(ERROR, "xc_dom_ipxe_module_file failed");
+ goto out;
+ }
+ }
}
if (rc != 0) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 506687f..f649696 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2359,6 +2359,7 @@ _hidden const char *libxl__lock_dir_path(void);
_hidden const char *libxl__run_dir_path(void);
_hidden const char *libxl__seabios_path(void);
_hidden const char *libxl__ovmf_path(void);
+_hidden const char *libxl__ipxe_path(void);
/*----- subprocess execution with timeout -----*/
diff --git a/tools/libxl/libxl_paths.c b/tools/libxl/libxl_paths.c
index 0643c1b..3f6a336 100644
--- a/tools/libxl/libxl_paths.c
+++ b/tools/libxl/libxl_paths.c
@@ -53,6 +53,15 @@ const char *libxl__ovmf_path(void)
#endif
}
+const char *libxl__ipxe_path(void)
+{
+#ifdef IPXE_PATH
+ return IPXE_PATH;
+#else
+ return NULL;
+#endif
+}
+
/*
* Local variables:
* mode: C
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 4/5] libxl: Load iPXE ROM from a file
2018-03-15 17:31 ` [PATCH 4/5] libxl: Load iPXE ROM from a file Anoob Soman
@ 2018-03-18 1:34 ` Doug Goldstein
2018-03-21 15:25 ` Wei Liu
1 sibling, 0 replies; 25+ messages in thread
From: Doug Goldstein @ 2018-03-18 1:34 UTC (permalink / raw)
To: Anoob Soman, xen-devel; +Cc: ian.jackson, wei.liu2, jbeulich, andrew.cooper3
On 3/15/18 12:31 PM, Anoob Soman wrote:
> Load iPXE ROM from a file pointed to by IPXE_PATH. If --with-system-ipxe
> is not specified default Xen firmware directory is picked up as
> IPXE_PATH
>
> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/5] libxl: Load iPXE ROM from a file
2018-03-15 17:31 ` [PATCH 4/5] libxl: Load iPXE ROM from a file Anoob Soman
2018-03-18 1:34 ` Doug Goldstein
@ 2018-03-21 15:25 ` Wei Liu
2018-03-26 15:51 ` Anoob Soman
1 sibling, 1 reply; 25+ messages in thread
From: Wei Liu @ 2018-03-21 15:25 UTC (permalink / raw)
To: Anoob Soman; +Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, xen-devel
On Thu, Mar 15, 2018 at 05:31:52PM +0000, Anoob Soman wrote:
> Load iPXE ROM from a file pointed to by IPXE_PATH. If --with-system-ipxe
> is not specified default Xen firmware directory is picked up as
> IPXE_PATH
>
> Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
> ---
> tools/libxl/libxl_dom.c | 12 ++++++++++++
> tools/libxl/libxl_internal.h | 1 +
> tools/libxl/libxl_paths.c | 9 +++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 2e29b52..104d6a0 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1003,6 +1003,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
> int datalen = 0;
> void *data;
> const char *bios_filename = NULL;
> + const char *ipxe_filename = NULL;
>
> if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
> if (info->u.hvm.firmware) {
> @@ -1094,6 +1095,17 @@ static int libxl__domain_firmware(libxl__gc *gc,
> assert(info->type == LIBXL_DOMAIN_TYPE_HVM);
> rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
> libxl__xenfirmwaredir_path()));
> + if (rc) {
> + LOGE(ERROR, "xc_dom_kernel_file failed");
> + goto out;
> + }
> + if ((ipxe_filename = libxl__ipxe_path())) {
> + rc = xc_dom_module_file(dom, ipxe_filename, "ipxe");
> + if (rc) {
> + LOGE(ERROR, "xc_dom_ipxe_module_file failed");
> + goto out;
This is the wrong place. Being an HVM guest doesn't mean ipxe should be
loaded. You probably need to look a few lines down and add code in
appropriate places like when ROMBIOS is picked.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 4/5] libxl: Load iPXE ROM from a file
2018-03-21 15:25 ` Wei Liu
@ 2018-03-26 15:51 ` Anoob Soman
0 siblings, 0 replies; 25+ messages in thread
From: Anoob Soman @ 2018-03-26 15:51 UTC (permalink / raw)
To: Wei Liu; +Cc: andrew.cooper3, ian.jackson, jbeulich, xen-devel
On 21/03/18 15:25, Wei Liu wrote:
>
>> + LOGE(ERROR, "xc_dom_kernel_file failed");
>> + goto out;
>> + }
>> + if ((ipxe_filename = libxl__ipxe_path())) {
>> + rc = xc_dom_module_file(dom, ipxe_filename, "ipxe");
>> + if (rc) {
>> + LOGE(ERROR, "xc_dom_ipxe_module_file failed");
>> + goto out;
> This is the wrong place. Being an HVM guest doesn't mean ipxe should be
> loaded. You probably need to look a few lines down and add code in
> appropriate places like when ROMBIOS is picked.
Sure, I will fix it up.
>
> Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file
2018-03-15 17:31 Make iPXE a standalone ROM Anoob Soman
` (3 preceding siblings ...)
2018-03-15 17:31 ` [PATCH 4/5] libxl: Load iPXE ROM from a file Anoob Soman
@ 2018-03-15 17:31 ` Anoob Soman
2018-03-16 11:26 ` Jan Beulich
4 siblings, 1 reply; 25+ messages in thread
From: Anoob Soman @ 2018-03-15 17:31 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Anoob Soman, ian.jackson, jbeulich, andrew.cooper3
splatering of mkhex-ed etherboot inside hvmloader/rombios is removed,
instead hvmloader/rombios now relies on iPXE ROM to be added,loaded as
a module.
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
---
tools/firmware/hvmloader/Makefile | 7 +------
tools/firmware/hvmloader/config.h | 5 +++++
tools/firmware/hvmloader/hvmloader.c | 9 ++++++++-
tools/firmware/hvmloader/rombios.c | 13 +++++++++----
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index fa1bf30..f347cb4 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -64,7 +64,7 @@ ifeq ($(CONFIG_ROMBIOS),y)
OBJS += optionroms.o 32bitbios_support.o rombios.o
CFLAGS += -DENABLE_ROMBIOS
ROMBIOS_ROM := $(ROMBIOS_DIR)/BIOS-bochs-latest
-ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM) $(ETHERBOOT_ROMS)
+ROMS += $(ROMBIOS_ROM) $(STDVGA_ROM) $(CIRRUSVGA_ROM)
endif
.PHONY: all
@@ -109,11 +109,6 @@ ifneq ($(CIRRUSVGA_ROM),)
sh ../../misc/mkhex vgabios_cirrusvga $(CIRRUSVGA_ROM) >> $@.new
echo "#endif" >> $@.new
endif
-ifneq ($(ETHERBOOT_ROMS),)
- echo "#ifdef ROM_INCLUDE_ETHERBOOT" >> $@.new
- sh ../../misc/mkhex etherboot $(ETHERBOOT_ROMS) >> $@.new
- echo "#endif" >> $@.new
-endif
mv $@.new $@
diff --git a/tools/firmware/hvmloader/config.h b/tools/firmware/hvmloader/config.h
index 6e00413..8f5d1040 100644
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -33,6 +33,11 @@ struct bios_config {
void (*create_mp_tables)(void);
void (*create_smbios_tables)(void);
void (*create_pir_tables)(void);
+
+ /* Physical address of iPXE ROM, loaded by domain builder
+ * when using ROMBIOS
+ */
+ unsigned int *ipxe_rom_addresss;
};
extern struct bios_config rombios_config;
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index f603f68..731cd81 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -323,6 +323,8 @@ int main(void)
const struct bios_config *bios;
int acpi_enabled;
const struct hvm_modlist_entry *bios_module;
+ const struct hvm_modlist_entry *ipxe_module = NULL;
+ unsigned int ipxe_rom_addresss = 0;
/* Initialise hypercall stubs with RET, rendering them no-ops. */
memset((void *)HYPERCALL_PHYSICAL_ADDRESS, 0xc3 /* RET */, PAGE_SIZE);
@@ -368,7 +370,12 @@ int main(void)
#ifdef ENABLE_ROMBIOS
else if ( bios == &rombios_config )
{
- bios->bios_load(bios, NULL, 0);
+ ipxe_module = get_module_entry(hvm_start_info, "ipxe");
+
+ if ( ipxe_module )
+ ipxe_rom_addresss = ipxe_module->paddr;
+
+ bios->bios_load(bios, (void *)ipxe_rom_addresss, 0);
}
#endif
else
diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c
index c736fd9..c0e04cb 100644
--- a/tools/firmware/hvmloader/rombios.c
+++ b/tools/firmware/hvmloader/rombios.c
@@ -95,9 +95,12 @@ static void rombios_load_roms(void)
etherboot_phys_addr = VGABIOS_PHYSICAL_ADDRESS + vgabios_sz;
if ( etherboot_phys_addr < OPTIONROM_PHYSICAL_ADDRESS )
etherboot_phys_addr = OPTIONROM_PHYSICAL_ADDRESS;
- etherboot_sz = scan_etherboot_nic(OPTIONROM_PHYSICAL_END,
- etherboot_phys_addr,
- etherboot);
+
+ if ( rombios_config.ipxe_rom_addresss )
+ etherboot_sz = scan_etherboot_nic(OPTIONROM_PHYSICAL_END,
+ etherboot_phys_addr,
+ rombios_config.ipxe_rom_addresss);
+
option_rom_phys_addr = etherboot_phys_addr + etherboot_sz;
option_rom_sz = pci_load_option_roms(OPTIONROM_PHYSICAL_END,
@@ -119,7 +122,7 @@ static void rombios_load_roms(void)
}
static void rombios_load(const struct bios_config *config,
- void *unused_addr, uint32_t unused_size)
+ void *ipxe_rom_addr, uint32_t unused_size)
{
uint32_t bioshigh;
struct rombios_info *info;
@@ -133,6 +136,8 @@ static void rombios_load(const struct bios_config *config,
info = (struct rombios_info *)BIOS_INFO_PHYSICAL_ADDRESS;
info->bios32_entry = bioshigh;
+
+ rombios_config.ipxe_rom_addresss = ipxe_rom_addr;
}
/*
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file
2018-03-15 17:31 ` [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file Anoob Soman
@ 2018-03-16 11:26 ` Jan Beulich
2018-03-16 11:44 ` Andrew Cooper
2018-03-19 14:24 ` Anoob Soman
0 siblings, 2 replies; 25+ messages in thread
From: Jan Beulich @ 2018-03-16 11:26 UTC (permalink / raw)
To: Anoob Soman; +Cc: andrew.cooper3, wei.liu2, ian.jackson, xen-devel
>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
> --- a/tools/firmware/hvmloader/config.h
> +++ b/tools/firmware/hvmloader/config.h
> @@ -33,6 +33,11 @@ struct bios_config {
> void (*create_mp_tables)(void);
> void (*create_smbios_tables)(void);
> void (*create_pir_tables)(void);
> +
> + /* Physical address of iPXE ROM, loaded by domain builder
> + * when using ROMBIOS
> + */
> + unsigned int *ipxe_rom_addresss;
Comment style. And can the pointer be to const?
> @@ -368,7 +370,12 @@ int main(void)
> #ifdef ENABLE_ROMBIOS
> else if ( bios == &rombios_config )
> {
> - bios->bios_load(bios, NULL, 0);
> + ipxe_module = get_module_entry(hvm_start_info, "ipxe");
> +
> + if ( ipxe_module )
> + ipxe_rom_addresss = ipxe_module->paddr;
> +
> + bios->bios_load(bios, (void *)ipxe_rom_addresss, 0);
> }
> #endif
Considering the #ifdef around here - is this whole series an
enhancement for qemu-trad only? I don't think we mean to make
any such enhancements anymore.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file
2018-03-16 11:26 ` Jan Beulich
@ 2018-03-16 11:44 ` Andrew Cooper
2018-03-17 16:54 ` Doug Goldstein
2018-03-19 14:24 ` Anoob Soman
1 sibling, 1 reply; 25+ messages in thread
From: Andrew Cooper @ 2018-03-16 11:44 UTC (permalink / raw)
To: Jan Beulich, Anoob Soman; +Cc: ian.jackson, wei.liu2, xen-devel
On 16/03/18 11:26, Jan Beulich wrote:
>>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
>> --- a/tools/firmware/hvmloader/config.h
>> +++ b/tools/firmware/hvmloader/config.h
>> @@ -33,6 +33,11 @@ struct bios_config {
>> void (*create_mp_tables)(void);
>> void (*create_smbios_tables)(void);
>> void (*create_pir_tables)(void);
>> +
>> + /* Physical address of iPXE ROM, loaded by domain builder
>> + * when using ROMBIOS
>> + */
>> + unsigned int *ipxe_rom_addresss;
> Comment style. And can the pointer be to const?
>
>> @@ -368,7 +370,12 @@ int main(void)
>> #ifdef ENABLE_ROMBIOS
>> else if ( bios == &rombios_config )
>> {
>> - bios->bios_load(bios, NULL, 0);
>> + ipxe_module = get_module_entry(hvm_start_info, "ipxe");
>> +
>> + if ( ipxe_module )
>> + ipxe_rom_addresss = ipxe_module->paddr;
>> +
>> + bios->bios_load(bios, (void *)ipxe_rom_addresss, 0);
>> }
>> #endif
> Considering the #ifdef around here - is this whole series an
> enhancement for qemu-trad only? I don't think we mean to make
> any such enhancements anymore.
It is for several important reasons, including allowing distros to
remove their qemu-trad packages completely.
The main purpose is to have iPXE as a ROM not embedded in hvmloader, and
finishes some incomplete earlier work by Anthony to split the SeaBIOS
and OVMF roms out of HVMLoader (and then later some work which Boris
offered to do, but time hasn't materialised yet).
This means that distros don't need to rebuild and redeploy their Xen
packages every time they want to change their iPXE package and/or don't
need to maintain multiple iPXE packages.
It also removes the final non-stubdom bit of the build which downloads a
random tarball from the web, which is something downstreams have been
crying out for for years.
As for the Qemu side of things, RomBIOS functions perfectly well with
qemu-upstream, and is the only viable transition mechanism available for
downstreams wishing to purge themselves of qemu-trad entirely, while
keeping older VMs working.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file
2018-03-16 11:44 ` Andrew Cooper
@ 2018-03-17 16:54 ` Doug Goldstein
0 siblings, 0 replies; 25+ messages in thread
From: Doug Goldstein @ 2018-03-17 16:54 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich, Anoob Soman; +Cc: wei.liu2, ian.jackson, xen-devel
On 3/16/18 6:44 AM, Andrew Cooper wrote:
> On 16/03/18 11:26, Jan Beulich wrote:
>>>>> On 15.03.18 at 18:31, <anoob.soman@citrix.com> wrote:
>>> --- a/tools/firmware/hvmloader/config.h
>>> +++ b/tools/firmware/hvmloader/config.h
>>> @@ -33,6 +33,11 @@ struct bios_config {
>>> void (*create_mp_tables)(void);
>>> void (*create_smbios_tables)(void);
>>> void (*create_pir_tables)(void);
>>> +
>>> + /* Physical address of iPXE ROM, loaded by domain builder
>>> + * when using ROMBIOS
>>> + */
>>> + unsigned int *ipxe_rom_addresss;
>> Comment style. And can the pointer be to const?
>>
>>> @@ -368,7 +370,12 @@ int main(void)
>>> #ifdef ENABLE_ROMBIOS
>>> else if ( bios == &rombios_config )
>>> {
>>> - bios->bios_load(bios, NULL, 0);
>>> + ipxe_module = get_module_entry(hvm_start_info, "ipxe");
>>> +
>>> + if ( ipxe_module )
>>> + ipxe_rom_addresss = ipxe_module->paddr;
>>> +
>>> + bios->bios_load(bios, (void *)ipxe_rom_addresss, 0);
>>> }
>>> #endif
>> Considering the #ifdef around here - is this whole series an
>> enhancement for qemu-trad only? I don't think we mean to make
>> any such enhancements anymore.
>
> It is for several important reasons, including allowing distros to
> remove their qemu-trad packages completely.
>
>
> The main purpose is to have iPXE as a ROM not embedded in hvmloader, and
> finishes some incomplete earlier work by Anthony to split the SeaBIOS
> and OVMF roms out of HVMLoader (and then later some work which Boris
> offered to do, but time hasn't materialised yet).
>
> This means that distros don't need to rebuild and redeploy their Xen
> packages every time they want to change their iPXE package and/or don't
> need to maintain multiple iPXE packages.>
> It also removes the final non-stubdom bit of the build which downloads a
> random tarball from the web, which is something downstreams have been
> crying out for for years.
I was originally asking if this was the case on the cover letter. I'll
lend a +1 to the overall series here. As someone that packages Xen in
two source based distros whose package managers run the build in a
separate phase from the fetch stage this is a welcome change. I've been
hacking around this and am glad to drop that.
--
Doug Goldstein
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file
2018-03-16 11:26 ` Jan Beulich
2018-03-16 11:44 ` Andrew Cooper
@ 2018-03-19 14:24 ` Anoob Soman
2018-03-19 15:21 ` Jan Beulich
1 sibling, 1 reply; 25+ messages in thread
From: Anoob Soman @ 2018-03-19 14:24 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, wei.liu2, ian.jackson, xen-devel
On 16/03/18 11:26, Jan Beulich wrote:
>>
>> + /* Physical address of iPXE ROM, loaded by domain builder
>> + * when using ROMBIOS
>> + */
>> + unsigned int *ipxe_rom_addresss;
> Comment style. And can the pointer be to const?
I will fixup the comment style and but making ipxe_rom_address a pointer
to const will require codes changes in hvmloader/rombios.c,
hvmloader/optionroms.c to make all function that use ipxe_rom_address
const as well. Are you fine with changing these files/functions.
-Anoob
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/5] hvmloader: Use iPXE ROM loaded from a standalone file
2018-03-19 14:24 ` Anoob Soman
@ 2018-03-19 15:21 ` Jan Beulich
0 siblings, 0 replies; 25+ messages in thread
From: Jan Beulich @ 2018-03-19 15:21 UTC (permalink / raw)
To: Anoob Soman; +Cc: andrew.cooper3, wei.liu2, ian.jackson, xen-devel
>>> On 19.03.18 at 15:24, <anoob.soman@citrix.com> wrote:
> On 16/03/18 11:26, Jan Beulich wrote:
>>>
>>> + /* Physical address of iPXE ROM, loaded by domain builder
>>> + * when using ROMBIOS
>>> + */
>>> + unsigned int *ipxe_rom_addresss;
>> Comment style. And can the pointer be to const?
>
> I will fixup the comment style and but making ipxe_rom_address a pointer
> to const will require codes changes in hvmloader/rombios.c,
> hvmloader/optionroms.c to make all function that use ipxe_rom_address
> const as well. Are you fine with changing these files/functions.
In a separate patch, yes. If you don't want to create a separate
patch, I'm also fine with your explanation of why it can't be const
for now.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 25+ messages in thread