[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-ia64-devel] [patch 01/14] kexec for xen
On Wed, Sep 12, 2007 at 12:21:19PM -0600, Alex Williamson wrote: > On Wed, 2007-09-12 at 17:08 +0900, Simon Horman wrote: > > > @@ -401,6 +406,14 @@ efi_get_pal_addr (void) > > > > #ifdef XEN > > void *pal_vaddr = 0; > > + > > +void * > > +efi_get_pal_addr(void) > > +{ > > + if (pal_vaddr) > > + return pal_vaddr; > > + return pal_vaddr = __efi_get_pal_addr(); > > +} > > #endif > > Coding style nit > > if (!pal_vaddr) > pal_vaddr = __efi_get_pal_addr(); > return pal_vaddr; > > Does the same thing in the same number of lines and is cleaner IMHO. Thanks, changed. > > > void > > @@ -408,9 +421,7 @@ efi_map_pal_code (void) > > { > > #ifdef XEN > > u64 psr; > > - if (!pal_vaddr) { > > - pal_vaddr = efi_get_pal_addr (); > > - } > > + efi_get_pal_addr(); > > (void)efi_get_pal_addr(); to make it obvious the return is > intentionally ignored? Also updated. > > > > +#ifdef XEN > > +/* find a block of memory aligned to 64M exclude reserved regions > > + * rsvd_regions are sorted > > + */ > > +unsigned long > > +kdump_find_rsvd_region(unsigned long size, struct rsvd_region *r, int n) > > +{ > > This all looks fine, but is there any reason this chunk of memory > needs to be at a low address? If not, maybe the memmap should be > searched in reverse order to avoid using up 32bit memory (should it > exist). No, there isn't. The basic reason the code is like it is is because thats how the Linux code is. I did previously work on a patch for upstream Linux to search in reverse order, but it turned out to be a little messier than I would have liked and I abandoned it. I can try again if you like. > > > + int i; > > + u64 start, end; > > + u64 alignment = 1UL << _PAGE_SIZE_64M; > > + void *efi_map_start, *efi_map_end, *p; > > + efi_memory_desc_t *md; > > + u64 efi_desc_size; > > + > > + efi_map_start = __va(ia64_boot_param->efi_memmap); > > + efi_map_end = efi_map_start + ia64_boot_param->efi_memmap_size; > > + efi_desc_size = ia64_boot_param->efi_memdesc_size; > > + > > + for (p = efi_map_start; p < efi_map_end; p += efi_desc_size) { > > + md = p; > > + if (!efi_wb(md)) > > + continue; > > + start = ALIGN(md->phys_addr, alignment); > > + end = efi_md_end(md); > > + for (i = 0; i < n; i++) { > > + if (__pa(r[i].start) >= start && __pa(r[i].end) < end) { > > + if (__pa(r[i].start) > start + size) > > + return start; > > + start = ALIGN(__pa(r[i].end), alignment); > > + if (i < n - 1 > > + && __pa(r[i + 1].start) < start + size) > > + continue; > > + else > > + break; > > + } > > + } > > + if (end > start + size) > > + return start; > > + } > > + > > + printk(KERN_WARNING > > + "Cannot reserve 0x%lx byte of memory for crashdump\n", size); > > + return ~0UL; > > +} > > +#endif > > There seems to be a mix of tab and space indenting in the > machine_kexec.c changes. Please pick one. Thanks, fixed. > > > Index: xen-unstable.hg/xen/arch/ia64/xen/machine_kexec.c > > =================================================================== > > --- xen-unstable.hg.orig/xen/arch/ia64/xen/machine_kexec.c 2007-07-11 > > 13:20:14.000000000 +0900 > > +++ xen-unstable.hg/xen/arch/ia64/xen/machine_kexec.c 2007-07-11 > > 13:20:16.000000000 +0900 > > > +static void ia64_machine_kexec(struct unw_frame_info *info, void *arg) > > +{ > > + xen_kexec_image_t *image = arg; > > + relocate_new_kernel_t rnk; > > + unsigned long code_addr = (unsigned > > long)__va(image->reboot_code_buffer); > > + unsigned long cpu_data_pa = (unsigned long) > > + __pa(cpu_data(smp_processor_id())); > > + unsigned long vector; > > + int ii; > > + > > + /* Interrupts aren't acceptable while we reboot */ > > + local_irq_disable(); > > + > > + /* Mask CMC and Performance Monitor interrupts */ > > + ia64_setreg(_IA64_REG_CR_PMV, 1 << 16); > > + ia64_setreg(_IA64_REG_CR_CMCV, 1 << 16); > > + > > + /* Mask ITV and Local Redirect Registers */ > > + ia64_set_itv(1 << 16); > > + ia64_set_lrr0(1 << 16); > > + ia64_set_lrr1(1 << 16); > > + > > + /* terminate possible nested in-service interrupts */ > > + for (ii = 0; ii < 16; ii++) > > + ia64_eoi(); > > + > > + /* unmask TPR and clear any pending interrupts */ > > + ia64_setreg(_IA64_REG_CR_TPR, 0); > > + ia64_srlz_d(); > > + vector = ia64_get_ivr(); > > + while (vector != IA64_SPURIOUS_INT_VECTOR) { > > + ia64_eoi(); > > + vector = ia64_get_ivr(); > > + } > > Is there a need for vector? It seems extraneous > > while (ia64_get_ivr() != IA64_SPURIOUS_INT_VECTOR) > ia64_eoi(); Agreed. I have removed vector. I'll send a patch to upstream Linux too. -- 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |