On Wed, Sep 12, 2007 at 03:38:19PM -0600, Alex Williamson wrote:
> On Wed, 2007-09-12 at 17:28 +0900, Simon Horman wrote:
> > plain text document attachment (ia64-kexec.patch)
>
> > Changes and updates.
> >
> > 1. Remove fake rendz path and related code according to discuss with Khalid
> > Aziz.
> > 2. fc.i offset fix in relocate_kernel.S.
> > 3. iospic shutdown code eoi and mask race fix from Fujitsu.
> > 4. Warm boot hook in machine_kexec to SN SAL code from Jack Steiner.
> > 5. Send slave to SAL slave loop patch from Jay Lan.
> > 6. Kdump on non-recoverable MCA event patch from Jay Lan
> > 7. Use CTL_UNNUMBERED in kdump_on_init sysctl.
> >
>
> Couple general issues:
>
> * This would be easier to consume as two patches. One as a port
> of the upstream patch, the second with the xen-ification.
> * Tabs and spaces are mixed in this patch. As a linux patch, this
> should use only tab indenting. kdump_find_rsvd_region() has a
> particularly odd style.
I believe that most of the bizzare indentation just reflects the
state of the code in Linux, but I'll double check to make sure
that I'm not adding additional weirdness.
I'll try splitting the patch as you suggest. This should make it obvious
what is coming straight from Linux, and what crazy things have been
added by my hand.
>
> And a specific one below...
>
> > +#ifdef CONFIG_KEXEC
> > + /* crashkernel=size@offset specifies the size to reserve for a crash
> > + * kernel.(offset is ingored for keep compatibility with other archs)
> > + * By reserving this memory we guarantee that linux never set's it
> > + * up as a DMA target.Useful for holding code to do something
> > + * appropriate after a kernel panic.
> > + */
> > + {
> > + char *from = strstr(saved_command_line, "crashkernel=");
> > +#ifndef CONFIG_XEN
> > + unsigned long base, size;
> > + if (from) {
> > + size = memparse(from + 12, &from);
> > + if (size) {
> > + sort_regions(rsvd_region, n);
> > + base = kdump_find_rsvd_region(size,
> > + rsvd_region, n);
> > + if (base != ~0UL) {
> > + rsvd_region[n].start =
> > + (unsigned long)__va(base);
> > + rsvd_region[n].end =
> > + (unsigned long)__va(base +
> > size);
> > + n++;
> > + crashk_res.start = base;
> > + crashk_res.end = base + size - 1;
> > + }
> > + }
> > + }
> > +#else /* CONFIG_XEN */
> > + if (from)
> > + printk("Ignoring crashkernel command line, "
> > + "parameter will be supplied by xen\n");
> > + xen_machine_kexec_setup_resources();
> > +#endif /* CONFIG_XEN */
>
> This doesn't look transparent paravirtualization safe. I'll note
> this in a few other places too, but I'm sure I've missed some. A
> CONFIG_XEN kernel should be able to boot on bare metal too. Thanks,
I wasn't aware of that, I'll try and fix it up and also look for other
problem areas elsewhere.
--
Horms
H: http://www.vergenet.net/~horms/
W: http://www.valinux.co.jp/en/
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home