| Message ID | 20250408160802.49870-12-agarciav@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Hyperlaunch device tree for dom0 | expand |
On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarciav@amd.com> wrote: > > > From: "Daniel P. Smith" dpsmith@apertussolutions.com > > > Look for a subnode of type `multiboot,ramdisk` within a domain node and > parse via the fdt_read_multiboot_module() helper. After a successful > helper call, the module index is returned and the module is guaranteed > to be in the module list. > > Fix unused typo in adjacent comment. > > Signed-off-by: Daniel P. Smith dpsmith@apertussolutions.com > > Signed-off-by: Jason Andryuk jason.andryuk@amd.com > > Signed-off-by: Alejandro Vallejo agarciav@amd.com > > --- > v3: > * Reworded commit message to state the helper postconditions. > * Wrapped long line > * Fix ramdisk -> module rename > > * Move ramdisk parsing from later patch > * Remove initrdidx indent > --- > xen/arch/x86/domain-builder/fdt.c | 29 +++++++++++++++++++++++++++++ > xen/arch/x86/setup.c | 4 ++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/domain-builder/fdt.c b/xen/arch/x86/domain-builder/fdt.c > index bc9903a9de..0f5fd01557 100644 > --- a/xen/arch/x86/domain-builder/fdt.c > +++ b/xen/arch/x86/domain-builder/fdt.c > @@ -195,6 +195,35 @@ static int __init process_domain_node( > !((char *)__va(bd->kernel->cmdline_pa))[0] ) > > bd->kernel->fdt_cmdline = fdt_get_prop_offset( > > fdt, node, "bootargs", &bd->kernel->cmdline_pa); > > + > + continue; > + } > + else if ( fdt_node_check_compatible(fdt, node, > + "multiboot,ramdisk") == 0 ) > + { > + int idx; > + > + if ( bd->module ) > > + { > + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n", I would start the message with lower case so it is consistent with the other one. > + name); > + continue; > + } > + > + idx = fdt_read_multiboot_module(fdt, node, address_cells, > + size_cells,bi); > + if ( idx < 0 ) > + { > + printk(" failed processing ramdisk module for domain %s\n", > + name); Prepend the log message with XENLOG_ERR ? > + return -EINVAL; > + } > + > + printk(" ramdisk: boot module %d\n", idx); > + bi->mods[idx].type = BOOTMOD_RAMDISK; > > + bd->module = &bi->mods[idx]; > > + > + continue; > } > } > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index ca4415d020..3dfa81b48c 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -2149,11 +2149,11 @@ void asmlinkage __init noreturn __start_xen(void) > * At this point all capabilities that consume boot modules should have > * claimed their boot modules. Find the first unclaimed boot module and > * claim it as the initrd ramdisk. Do a second search to see if there are > - * any remaining unclaimed boot modules, and report them as unusued initrd > + * any remaining unclaimed boot modules, and report them as unused initrd > * candidates. > */ > initrdidx = first_boot_module_index(bi, BOOTMOD_UNKNOWN); > - if ( initrdidx < MAX_NR_BOOTMODS ) > + if ( !bi->hyperlaunch_enabled && initrdidx < MAX_NR_BOOTMODS ) > > { > bi->mods[initrdidx].type = BOOTMOD_RAMDISK; > > bi->domains[0].module = &bi->mods[initrdidx]; > > -- > 2.43.0
On 08.04.2025 18:07, Alejandro Vallejo wrote: > --- a/xen/arch/x86/domain-builder/fdt.c > +++ b/xen/arch/x86/domain-builder/fdt.c > @@ -195,6 +195,35 @@ static int __init process_domain_node( > !((char *)__va(bd->kernel->cmdline_pa))[0] ) > bd->kernel->fdt_cmdline = fdt_get_prop_offset( > fdt, node, "bootargs", &bd->kernel->cmdline_pa); > + > + continue; With this ... > + } > + else if ( fdt_node_check_compatible(fdt, node, ... no need for "else" here? > + "multiboot,ramdisk") == 0 ) > + { > + int idx; > + > + if ( bd->module ) > + { > + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n", Stray ')' in the string literal. > + name); > + continue; > + } > + > + idx = fdt_read_multiboot_module(fdt, node, address_cells, > + size_cells,bi); > + if ( idx < 0 ) > + { > + printk(" failed processing ramdisk module for domain %s\n", > + name); > + return -EINVAL; > + } Along the lines of what Denis has said - please be consistent about log messages: XENLOG_* or not, preferably no capital at the start, initial blank padding. May apply elsewhere in the series as well. > + printk(" ramdisk: boot module %d\n", idx); > + bi->mods[idx].type = BOOTMOD_RAMDISK; > + bd->module = &bi->mods[idx]; The field's named "module" now, but that now ends up inconsistent with naming used elsewhere, as is pretty noticeable here. > + continue; This isn't strictly needed, is it, ... > } > } ... considering we're at the bottom of the loop? Jan
On Wed Apr 9, 2025 at 11:07 PM BST, Denis Mukhin wrote: > On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarciav@amd.com> wrote: > >> >> >> From: "Daniel P. Smith" dpsmith@apertussolutions.com >> >> >> Look for a subnode of type `multiboot,ramdisk` within a domain node and >> parse via the fdt_read_multiboot_module() helper. After a successful >> helper call, the module index is returned and the module is guaranteed >> to be in the module list. >> >> Fix unused typo in adjacent comment. >> >> Signed-off-by: Daniel P. Smith dpsmith@apertussolutions.com >> >> Signed-off-by: Jason Andryuk jason.andryuk@amd.com >> >> Signed-off-by: Alejandro Vallejo agarciav@amd.com >> >> --- >> v3: >> * Reworded commit message to state the helper postconditions. >> * Wrapped long line >> * Fix ramdisk -> module rename >> >> * Move ramdisk parsing from later patch >> * Remove initrdidx indent >> --- >> xen/arch/x86/domain-builder/fdt.c | 29 +++++++++++++++++++++++++++++ >> xen/arch/x86/setup.c | 4 ++-- >> 2 files changed, 31 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/x86/domain-builder/fdt.c b/xen/arch/x86/domain-builder/fdt.c >> index bc9903a9de..0f5fd01557 100644 >> --- a/xen/arch/x86/domain-builder/fdt.c >> +++ b/xen/arch/x86/domain-builder/fdt.c >> @@ -195,6 +195,35 @@ static int __init process_domain_node( >> !((char *)__va(bd->kernel->cmdline_pa))[0] ) >> >> bd->kernel->fdt_cmdline = fdt_get_prop_offset( >> >> fdt, node, "bootargs", &bd->kernel->cmdline_pa); >> >> + >> + continue; >> + } >> + else if ( fdt_node_check_compatible(fdt, node, >> + "multiboot,ramdisk") == 0 ) >> + { >> + int idx; >> + >> + if ( bd->module ) >> >> + { >> + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n", > > I would start the message with lower case so it is consistent with the other one. As mentioned before, this is due to how it's meant to be rendered. This is a standalone message, hence the uppercase (consistent with the duplicate kernel). Will change the XENLOG_ERR into XENLOG_WARNING though. > >> + name); >> + continue; >> + } >> + >> + idx = fdt_read_multiboot_module(fdt, node, address_cells, >> + size_cells,bi); >> + if ( idx < 0 ) >> + { >> + printk(" failed processing ramdisk module for domain %s\n", >> + name); > > Prepend the log message with XENLOG_ERR ? Indeed. > >> + return -EINVAL; >> + } >> + >> + printk(" ramdisk: boot module %d\n", idx); >> + bi->mods[idx].type = BOOTMOD_RAMDISK; >> >> + bd->module = &bi->mods[idx]; >> >> + >> + continue; >> } >> } >> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index ca4415d020..3dfa81b48c 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -2149,11 +2149,11 @@ void asmlinkage __init noreturn __start_xen(void) >> * At this point all capabilities that consume boot modules should have >> * claimed their boot modules. Find the first unclaimed boot module and >> * claim it as the initrd ramdisk. Do a second search to see if there are >> - * any remaining unclaimed boot modules, and report them as unusued initrd >> + * any remaining unclaimed boot modules, and report them as unused initrd >> * candidates. >> */ >> initrdidx = first_boot_module_index(bi, BOOTMOD_UNKNOWN); >> - if ( initrdidx < MAX_NR_BOOTMODS ) >> + if ( !bi->hyperlaunch_enabled && initrdidx < MAX_NR_BOOTMODS ) >> >> { >> bi->mods[initrdidx].type = BOOTMOD_RAMDISK; >> >> bi->domains[0].module = &bi->mods[initrdidx]; >> >> -- >> 2.43.0 Cheers, Alejandro
On Thu Apr 10, 2025 at 12:34 PM BST, Jan Beulich wrote: > On 08.04.2025 18:07, Alejandro Vallejo wrote: >> --- a/xen/arch/x86/domain-builder/fdt.c >> +++ b/xen/arch/x86/domain-builder/fdt.c >> @@ -195,6 +195,35 @@ static int __init process_domain_node( >> !((char *)__va(bd->kernel->cmdline_pa))[0] ) >> bd->kernel->fdt_cmdline = fdt_get_prop_offset( >> fdt, node, "bootargs", &bd->kernel->cmdline_pa); >> + >> + continue; > > With this ... > >> + } >> + else if ( fdt_node_check_compatible(fdt, node, > > ... no need for "else" here? Sure > >> + "multiboot,ramdisk") == 0 ) >> + { >> + int idx; >> + >> + if ( bd->module ) >> + { >> + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n", > > Stray ')' in the string literal. Ack. > >> + name); >> + continue; >> + } >> + >> + idx = fdt_read_multiboot_module(fdt, node, address_cells, >> + size_cells,bi); >> + if ( idx < 0 ) >> + { >> + printk(" failed processing ramdisk module for domain %s\n", >> + name); >> + return -EINVAL; >> + } > > Along the lines of what Denis has said - please be consistent about log > messages: XENLOG_* or not, preferably no capital at the start, initial > blank padding. May apply elsewhere in the series as well. I don't mind dropping that and making everything flat (uppercase + no padding), but there is some consistency. Albeit, it is true the rationale is somewhat obscure. ATM the consistency is: "padding spaces + lowercase" when giving extra information on hyperlaunch. It ends up creating a hyperlaunch block in `dmesg` with a "Hyperlaunch detected" line on top so it's easier to know what lines are hyperlaunch related and which ones aren't. Do you have a preference for a specific reporting style? > >> + printk(" ramdisk: boot module %d\n", idx); >> + bi->mods[idx].type = BOOTMOD_RAMDISK; >> + bd->module = &bi->mods[idx]; > > The field's named "module" now, but that now ends up inconsistent with > naming used elsewhere, as is pretty noticeable here. Well, yes. It is confusing. Also, the DTB is called multiboot,ramdisk, because multiboot,module is already used to detect what nodes are expressed as multiboot,modules. I'm considering going back and calling them ramdisk again. If anything, to avoid the ambiguity between domain modules and multiboot modules. e.g: a kernel is a multiboot module, but not a domain module. > >> + continue; > > This isn't strictly needed, is it, ... > >> } >> } > > ... considering we're at the bottom of the loop? Indeed > > Jan Cheers, Alejandro
On Mon Apr 14, 2025 at 6:06 PM BST, Alejandro Vallejo wrote: > On Thu Apr 10, 2025 at 12:34 PM BST, Jan Beulich wrote: >> On 08.04.2025 18:07, Alejandro Vallejo wrote: >> >>> + printk(" ramdisk: boot module %d\n", idx); >>> + bi->mods[idx].type = BOOTMOD_RAMDISK; >>> + bd->module = &bi->mods[idx]; >> >> The field's named "module" now, but that now ends up inconsistent with >> naming used elsewhere, as is pretty noticeable here. > > Well, yes. It is confusing. Also, the DTB is called multiboot,ramdisk, > because multiboot,module is already used to detect what nodes are > expressed as multiboot,modules. I'm considering going back and calling > them ramdisk again. If anything, to avoid the ambiguity between > domain modules and multiboot modules. e.g: a kernel is a multiboot > module, but not a domain module. Particularly when misc/arm/device-tree/booting.txt already states that the initrd for dom0 ought to be provided with the "multiboot,ramdisk" string in the "compatible" prop. Deviating from that is just going to make it far more annoying to unify arm and x86 in the future. And calling those ramdisks anything but ramdisk internally is just plain confusing (as evidenced in the current series). So... how frontally opposed would you be to restoring the ramdisk nomenclature? Also, for ease of rebasing future patches it'd be far nicer to go back to ramdisk rather than reinventing some new name. I'm for the time being leaving things as they are (because it is a pain to change these things) until we settle on something. Cheers, Alejandro
On 14.04.2025 19:06, Alejandro Vallejo wrote: > On Thu Apr 10, 2025 at 12:34 PM BST, Jan Beulich wrote: >> On 08.04.2025 18:07, Alejandro Vallejo wrote: >>> --- a/xen/arch/x86/domain-builder/fdt.c >>> +++ b/xen/arch/x86/domain-builder/fdt.c >>> @@ -195,6 +195,35 @@ static int __init process_domain_node( >>> !((char *)__va(bd->kernel->cmdline_pa))[0] ) >>> bd->kernel->fdt_cmdline = fdt_get_prop_offset( >>> fdt, node, "bootargs", &bd->kernel->cmdline_pa); >>> + >>> + continue; >> >> With this ... >> >>> + } >>> + else if ( fdt_node_check_compatible(fdt, node, >> >> ... no need for "else" here? > > Sure > >> >>> + "multiboot,ramdisk") == 0 ) >>> + { >>> + int idx; >>> + >>> + if ( bd->module ) >>> + { >>> + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n", For context below, note this message. >>> + name); >>> + continue; >>> + } >>> + >>> + idx = fdt_read_multiboot_module(fdt, node, address_cells, >>> + size_cells,bi); >>> + if ( idx < 0 ) >>> + { >>> + printk(" failed processing ramdisk module for domain %s\n", >>> + name); >>> + return -EINVAL; >>> + } >> >> Along the lines of what Denis has said - please be consistent about log >> messages: XENLOG_* or not, preferably no capital at the start, initial >> blank padding. May apply elsewhere in the series as well. > > I don't mind dropping that and making everything flat (uppercase + no > padding), but there is some consistency. Albeit, it is true the > rationale is somewhat obscure. Obscure or not - it might be fine if stated that way sufficiently prominently, such that future additions here then will adhere to that model. > ATM the consistency is: "padding spaces + lowercase" when giving extra > information on hyperlaunch. It ends up creating a hyperlaunch block in > `dmesg` with a "Hyperlaunch detected" line on top so it's easier to > know what lines are hyperlaunch related and which ones aren't. > > Do you have a preference for a specific reporting style? XENLOG_* or not want to be consistent, no matter what. Generally I think that log messages shouldn't start with capitals, unless it's e.g. an acronym. The padding to help grouping would be fine with me if, as said, properly spelled as the scheme to use. Jan
On 14.04.2025 19:27, Alejandro Vallejo wrote: > On Mon Apr 14, 2025 at 6:06 PM BST, Alejandro Vallejo wrote: >> On Thu Apr 10, 2025 at 12:34 PM BST, Jan Beulich wrote: >>> On 08.04.2025 18:07, Alejandro Vallejo wrote: >>> >>>> + printk(" ramdisk: boot module %d\n", idx); >>>> + bi->mods[idx].type = BOOTMOD_RAMDISK; >>>> + bd->module = &bi->mods[idx]; >>> >>> The field's named "module" now, but that now ends up inconsistent with >>> naming used elsewhere, as is pretty noticeable here. >> >> Well, yes. It is confusing. Also, the DTB is called multiboot,ramdisk, >> because multiboot,module is already used to detect what nodes are >> expressed as multiboot,modules. I'm considering going back and calling >> them ramdisk again. If anything, to avoid the ambiguity between >> domain modules and multiboot modules. e.g: a kernel is a multiboot >> module, but not a domain module. > > Particularly when misc/arm/device-tree/booting.txt already states that > the initrd for dom0 ought to be provided with the "multiboot,ramdisk" > string in the "compatible" prop. Deviating from that is just going to > make it far more annoying to unify arm and x86 in the future. And > calling those ramdisks anything but ramdisk internally is just plain > confusing (as evidenced in the current series). Yet the limitation of this is quite obvious: How would you express multiple such items? Have many "ramdisk"s? Even if some of them serve an entirely different purpose? See how Linux has gone to tuck together multiple CPIOs, as they can have only one thing called "ramdisk" (which, aiui, now no longer truly is). > So... how frontally opposed would you be to restoring the ramdisk > nomenclature? Also, for ease of rebasing future patches it'd be far > nicer to go back to ramdisk rather than reinventing some new name. Well, I fear I wouldn't ack such a patch. If everyone else agrees that "ramdisk" is the best of all names (or at least getting close), I'd perhaps mumble over, but let it go in. (Only partly as a joke: If we dislike "module", how about "blob" or some such?) Jan
On Tue Apr 15, 2025 at 7:17 AM BST, Jan Beulich wrote: > On 14.04.2025 19:27, Alejandro Vallejo wrote: >> On Mon Apr 14, 2025 at 6:06 PM BST, Alejandro Vallejo wrote: >>> On Thu Apr 10, 2025 at 12:34 PM BST, Jan Beulich wrote: >>>> On 08.04.2025 18:07, Alejandro Vallejo wrote: >>>> >>>>> + printk(" ramdisk: boot module %d\n", idx); >>>>> + bi->mods[idx].type = BOOTMOD_RAMDISK; >>>>> + bd->module = &bi->mods[idx]; >>>> >>>> The field's named "module" now, but that now ends up inconsistent with >>>> naming used elsewhere, as is pretty noticeable here. >>> >>> Well, yes. It is confusing. Also, the DTB is called multiboot,ramdisk, >>> because multiboot,module is already used to detect what nodes are >>> expressed as multiboot,modules. I'm considering going back and calling >>> them ramdisk again. If anything, to avoid the ambiguity between >>> domain modules and multiboot modules. e.g: a kernel is a multiboot >>> module, but not a domain module. >> >> Particularly when misc/arm/device-tree/booting.txt already states that >> the initrd for dom0 ought to be provided with the "multiboot,ramdisk" >> string in the "compatible" prop. Deviating from that is just going to >> make it far more annoying to unify arm and x86 in the future. And >> calling those ramdisks anything but ramdisk internally is just plain >> confusing (as evidenced in the current series). > > Yet the limitation of this is quite obvious: How would you express > multiple such items? With multiple such nodes. initrd1 { compatible = "multiboot,ramdisk", "multiboot,module" module-idx = <2> }; initrd2 { compatible = "multiboot,ramdisk", "multiboot,module" module-idx = <3> }; as is done in dom0less. This is not a hypothetical, it's already comitted. > Have many "ramdisk"s? As many as I require. If I was booting Xen(dom0+initrd) on Xen(hyperlaunch), that'd be 2 (with Xen passed as the kernel). > Even if some of them serve an entirely different purpose? The purpose of a ramdisk/initrd is of no concern to the hypervisor. They are specially crafted blobs consumed by kernels for purposes of bootstrap. > See how Linux has gone to tuck together multiple CPIOs, as they can > have only one thing called "ramdisk" (which, aiui, now no longer truly > is). That's an internal Linux matter. If they need 1, we'll pass 1. We do need several such blobs, because some OSs do need them. Namely, Xen. And AFAICS we call them modules mostly because (a) Xen does not have the module infrastructure that Linux has, which would causes ambiguities and (b) is typically loaded via multiboot, which does call each blob a module. > >> So... how frontally opposed would you be to restoring the ramdisk >> nomenclature? Also, for ease of rebasing future patches it'd be far >> nicer to go back to ramdisk rather than reinventing some new name. > > Well, I fear I wouldn't ack such a patch. If everyone else agrees > that "ramdisk" is the best of all names (or at least getting close), > I'd perhaps mumble over, but let it go in. Ok... When I send v4 I'll do so keeping the "module" rename. Meanwhile, I'll try to think of some options. Calling Xen's modules and the booted kernel modules the same way is just way too confusing. I take it you have the same dislike for initrd as you do for ramdisk? > > (Only partly as a joke: If we dislike "module", how about "blob" or > some such?) Better not. Blob makes "kernel" sound like an adjective: Kernel blob. There might be some other suitable word. I'll think about it. Cheers, Alejandro
On 15.04.2025 13:59, Alejandro Vallejo wrote: > On Tue Apr 15, 2025 at 7:17 AM BST, Jan Beulich wrote: >> On 14.04.2025 19:27, Alejandro Vallejo wrote: >>> So... how frontally opposed would you be to restoring the ramdisk >>> nomenclature? Also, for ease of rebasing future patches it'd be far >>> nicer to go back to ramdisk rather than reinventing some new name. >> >> Well, I fear I wouldn't ack such a patch. If everyone else agrees >> that "ramdisk" is the best of all names (or at least getting close), >> I'd perhaps mumble over, but let it go in. > > Ok... When I send v4 I'll do so keeping the "module" rename. Meanwhile, > I'll try to think of some options. Calling Xen's modules and the booted > kernel modules the same way is just way too confusing. > > I take it you have the same dislike for initrd as you do for ramdisk? Indeed. Jan
On 4/15/25 07:59, Alejandro Vallejo wrote: > On Tue Apr 15, 2025 at 7:17 AM BST, Jan Beulich wrote: >> On 14.04.2025 19:27, Alejandro Vallejo wrote: >>> On Mon Apr 14, 2025 at 6:06 PM BST, Alejandro Vallejo wrote: >>>> On Thu Apr 10, 2025 at 12:34 PM BST, Jan Beulich wrote: >>>>> On 08.04.2025 18:07, Alejandro Vallejo wrote: >>>>> >>>>>> + printk(" ramdisk: boot module %d\n", idx); >>>>>> + bi->mods[idx].type = BOOTMOD_RAMDISK; >>>>>> + bd->module = &bi->mods[idx]; >>>>> >>>>> The field's named "module" now, but that now ends up inconsistent with >>>>> naming used elsewhere, as is pretty noticeable here. >>>> >>>> Well, yes. It is confusing. Also, the DTB is called multiboot,ramdisk, >>>> because multiboot,module is already used to detect what nodes are >>>> expressed as multiboot,modules. I'm considering going back and calling >>>> them ramdisk again. If anything, to avoid the ambiguity between >>>> domain modules and multiboot modules. e.g: a kernel is a multiboot >>>> module, but not a domain module. >>> >>> Particularly when misc/arm/device-tree/booting.txt already states that >>> the initrd for dom0 ought to be provided with the "multiboot,ramdisk" >>> string in the "compatible" prop. Deviating from that is just going to >>> make it far more annoying to unify arm and x86 in the future. And >>> calling those ramdisks anything but ramdisk internally is just plain >>> confusing (as evidenced in the current series). >> >> Yet the limitation of this is quite obvious: How would you express >> multiple such items? > > With multiple such nodes. > > initrd1 { > compatible = "multiboot,ramdisk", "multiboot,module" > module-idx = <2> > }; > initrd2 { > compatible = "multiboot,ramdisk", "multiboot,module" > module-idx = <3> > }; > > as is done in dom0less. This is not a hypothetical, it's already > comitted. > >> Have many "ramdisk"s? > > As many as I require. If I was booting Xen(dom0+initrd) on > Xen(hyperlaunch), that'd be 2 (with Xen passed as the kernel). > >> Even if some of them serve an entirely different purpose? > > The purpose of a ramdisk/initrd is of no concern to the hypervisor. They are > specially crafted blobs consumed by kernels for purposes of bootstrap. > >> See how Linux has gone to tuck together multiple CPIOs, as they can >> have only one thing called "ramdisk" (which, aiui, now no longer truly >> is). > > That's an internal Linux matter. If they need 1, we'll pass 1. We do > need several such blobs, because some OSs do need them. Namely, Xen. And > AFAICS we call them modules mostly because (a) Xen does not have the > module infrastructure that Linux has, which would causes ambiguities and > (b) is typically loaded via multiboot, which does call each blob a > module. > >> >>> So... how frontally opposed would you be to restoring the ramdisk >>> nomenclature? Also, for ease of rebasing future patches it'd be far >>> nicer to go back to ramdisk rather than reinventing some new name. >> >> Well, I fear I wouldn't ack such a patch. If everyone else agrees >> that "ramdisk" is the best of all names (or at least getting close), >> I'd perhaps mumble over, but let it go in. > > Ok... When I send v4 I'll do so keeping the "module" rename. Meanwhile, > I'll try to think of some options. Calling Xen's modules and the booted > kernel modules the same way is just way too confusing. > > I take it you have the same dislike for initrd as you do for ramdisk? > >> >> (Only partly as a joke: If we dislike "module", how about "blob" or >> some such?) > > Better not. Blob makes "kernel" sound like an adjective: Kernel blob. > > There might be some other suitable word. I'll think about it. And all this confusion and mess would have been avoided if left as the abstract ramdisk name. v/r, dps
diff --git a/xen/arch/x86/domain-builder/fdt.c b/xen/arch/x86/domain-builder/fdt.c index bc9903a9de..0f5fd01557 100644 --- a/xen/arch/x86/domain-builder/fdt.c +++ b/xen/arch/x86/domain-builder/fdt.c @@ -195,6 +195,35 @@ static int __init process_domain_node( !((char *)__va(bd->kernel->cmdline_pa))[0] ) bd->kernel->fdt_cmdline = fdt_get_prop_offset( fdt, node, "bootargs", &bd->kernel->cmdline_pa); + + continue; + } + else if ( fdt_node_check_compatible(fdt, node, + "multiboot,ramdisk") == 0 ) + { + int idx; + + if ( bd->module ) + { + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n", + name); + continue; + } + + idx = fdt_read_multiboot_module(fdt, node, address_cells, + size_cells,bi); + if ( idx < 0 ) + { + printk(" failed processing ramdisk module for domain %s\n", + name); + return -EINVAL; + } + + printk(" ramdisk: boot module %d\n", idx); + bi->mods[idx].type = BOOTMOD_RAMDISK; + bd->module = &bi->mods[idx]; + + continue; } } diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index ca4415d020..3dfa81b48c 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -2149,11 +2149,11 @@ void asmlinkage __init noreturn __start_xen(void) * At this point all capabilities that consume boot modules should have * claimed their boot modules. Find the first unclaimed boot module and * claim it as the initrd ramdisk. Do a second search to see if there are - * any remaining unclaimed boot modules, and report them as unusued initrd + * any remaining unclaimed boot modules, and report them as unused initrd * candidates. */ initrdidx = first_boot_module_index(bi, BOOTMOD_UNKNOWN); - if ( initrdidx < MAX_NR_BOOTMODS ) + if ( !bi->hyperlaunch_enabled && initrdidx < MAX_NR_BOOTMODS ) { bi->mods[initrdidx].type = BOOTMOD_RAMDISK; bi->domains[0].module = &bi->mods[initrdidx];