Hello Happy New Year! and thanks for your patience waiting for the review of this series.
On Mon, Dec 9, 2024 at 2:54 PM Sandeep Gundlupet Raju via lists.yoctoproject.org <sandeep.gundlupet-raju= [email protected]> wrote: > Add a new bbclass for xen launch config variables. This bbclass provides > common xen launch config variables which can be inherited by any > vendor specific u-boot boot script and non-u-boot recipes. Also these > variable are configurable from recipes, global and machine configuration > files. > Once this series has completed all reviews and is acceptable for this layer, do you think it would make sense to send corresponding patches to meta-raspberrypi and meta-xilinx to set default values for these new variables in the appropriate machine configuration files? (Raspberry Pi 4 64-bit, zynqmp and versal machines) > Variable nomenclature is aligned with xen documentation. > https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html > > Currently it supports Xen Dom0 boot and can be extended for DomU > or Dom0less boot. > > Signed-off-by: Sandeep Gundlupet Raju <[email protected]> > --- > classes-recipe/xen-launch-config.bbclass | 43 ++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > create mode 100644 classes-recipe/xen-launch-config.bbclass > > diff --git a/classes-recipe/xen-launch-config.bbclass > b/classes-recipe/xen-launch-config.bbclass > new file mode 100644 > index 00000000..df8287e1 > --- /dev/null > +++ b/classes-recipe/xen-launch-config.bbclass > @@ -0,0 +1,43 @@ > +# Copyright (C) 2024, Advanced Micro Devices, Inc. All rights reserved. > +# > +# SPDX-License-Identifier: MIT > +# > +# This bbclass defines u-boot script variables required for xen boot > which can be > +# inherited u-boot boot scripts recipes and also allows to configure > these variables > +# from recipes, global and machine configurations files. > + +# Variable nomenclature is aligned with > +# https://xenbits.xen.org/docs/unstable/misc/xen-command-line.html > + > Suggest: just retain the SPDX line from above? > +# Image Load Address for Xen Dom0 boot > +KERNEL_LOAD_ADDRESS ??= "0x00400000" > +XEN_LOAD_ADDRESS ??= "0x00200000" > +DEVICETREE_LOAD_ADDRESS ??= "0xC000000" > +RAMDISK_LOAD_ADDRESS ??= "0x2600000" + > +# Xen boot image types. > +# KERNEL_IMAGETYPE: Specifies DomU kernel image file to be loaded by > u-boot. > +# XEN_IMAGETYPE: Specifies xen hypervisor binary to be loaded by u-boot. > Please could the u-boot references be removed from the above comment lines? Next, in the Yocto documentation, KERNEL_IMAGETYPE is defined to set the type of kernel binary that is _built_ by the kernel recipe, and then KERNEL_IMAGETYPE is also used elsewhere to identify the kernel file to be installed and to load at boot. Accordingly, a new XEN_IMAGETYPE variable should behave predictably for anyone familiar with using KERNEL_IMAGETYPE. However, it is not being defined here to determine any output of the Xen kernel build process -- it doesn't affect the Xen build at all. It is being defined to determine the name of the binary loaded by the boot process. I think it's important to consider this carefully, as there genuinely are multiple Xen image types built in x86 builds. That said, I don't think there is a problem, because it doesn't prevent the Xen build being adjusted to use it if necessary, and if there is a need to add a XEN_IMAGETYPES (plural) variable later, I still think this looks fine. In patch 3, there is a variable: XEN_IMAGE_NAME - what is the function of the XEN_IMAGE_NAME variable? ie. What is it used for? - is there a reason that XEN_IMAGETYPE is not being used wherever XEN_IMAGE_NAME is used? > +# Example: xen or xen.efi or xen.gz > +# DOM0_RAMDISK_IMAGETYPE: Specifies DOM0 ramdisk to be used, Example: > cpio.gz > +XEN_IMAGETYPE ??= "xen" > +DOM0_RAMDISK_IMAGETYPE ??= "rootfs.cpio.gz" > + > +# Set the amount of memory for dom0 depending on total available memory > size(DDR). > It could be helpful if the comment indicated what behaviour to expect if this variable is empty/unset. > +DOM0_MEM ??= "256M" Could passing the dom0_mem= command line argument be skipped if the variable does not have a value set? (in patch 2) If so, could the default value of this variable be set to empty here and then set it differently elsewhere in machine configurations. > + > +# Specify which UART console Xen should use. You can sepecify the > devicetree > +# alias or full path to a node in the devicetree > +# XEN_SERIAL_CONSOLES = "/soc/serial@7e215040" or > +# XEN_SERIAL_CONSOLES = "serial0" or > +# XEN_SERIAL_CONSOLES = "/axi/serial@ff000000" > +XEN_SERIAL_CONSOLES ??= "/soc/serial@7e215040" > It would also be a useful improvement to make the boot script avoid passing any dtuart= command line argument (in patch 2) when the value for this is empty or unset. Is there a way to enable configuring multiple consoles if there are multiple values passed in the variable? > + > +# Specify additional command line arguments used for Xen and this will be > appended > +# to xen-bootargs cariable. This can also be used for passing debug cmd > line arguments. > +# Examples: XEN_CMDLINE_APPEND ?= "sched=credit loglvl=all > guest_loglvl=debug" > +XEN_CMDLINE_APPEND ??= "sync_console bootscrub=0" > + > +# Specify the max number of vcpus for dom0 > +# Example usage: DOM0_MAX_VCPUS = "2" or DOM0_MAX_VCPUS = "2-4" > +DOM0_MAX_VCPUS ??= "1" > Another variable where having good behaviour, ie. avoiding passing a command line argument, when it is empty would be useful and then empty would then also be a sensible default. thanks, Christopher > -- > 2.34.1 > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#9071): https://lists.yoctoproject.org/g/meta-virtualization/message/9071 Mute This Topic: https://lists.yoctoproject.org/mt/110006796/21656 Group Owner: [email protected] Unsubscribe: https://lists.yoctoproject.org/g/meta-virtualization/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
