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.
> 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?
> +#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).
> + 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.
> 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();
Thanks,
Alex
--
Alex Williamson HP Open Source & Linux Org.
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home