Re: [PATCH] kexec on ia64

[Andrew Morton <akpm@osdl.org>]

Khalid Aziz <khalid_aziz@hp.com> wrote:
>
> Add kexec support on ia64.
> 

Neat.  How well does it work?

> +#ifdef CONFIG_PCI
> +void machine_shutdown(void)
> +{
> +	struct pci_dev *dev;
> +	irq_desc_t *idesc;
> +	cpumask_t mask = CPU_MASK_NONE;
> +
> +	/* Disable all PCI devices */
> +	list_for_each_entry(dev, &pci_devices, global_list) {
> +		if (!(dev->is_enabled))
> +			continue;
> +		idesc = irq_descp(dev->irq);
> +		if (!idesc)
> +			continue;
> +		cpu_set(0, mask);
> +		disable_irq_nosync(dev->irq);
> +		idesc->handler->end(dev->irq);
> +		idesc->handler->set_affinity(dev->irq, mask);
> +		idesc->action = NULL;
> +		pci_disable_device(dev);
> +		pci_set_power_state(dev, 0);
> +	}
> +}
> +#endif

Ahem.

  /* Do NOT directly access these two variables, unless you are arch specific pci
   * code, or pci core code. */
  extern struct list_head pci_root_buses;	/* list of all known PCI buses */
  extern struct list_head pci_devices;	/* list of all devices */

I think it would be kinder to the API to use pci_find_device(PCI_ANY_ID,
PCI_ANY_ID, ...) here.

> +/*
> + * Do not allocate memory (or fail in any way) in machine_kexec().
> + * We are past the point of no return, committed to rebooting now. 
> + */
> +void machine_kexec(struct kimage *image)
> +{
> +	unsigned long indirection_page;
> +	relocate_new_kernel_t rnk;
> +	unsigned long pta, impl_va_bits;
> +	void *pal_addr = efi_get_pal_addr();
> +	unsigned long code_addr = (unsigned long)page_address(image->control_code_page);
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		if (cpu != smp_processor_id())
> +			cpu_down(cpu);
> +	}
> +#elif CONFIG_SMP

This will generate a CPP warning if CONFIG_SMP is not defined.

	#elif defined(CONFIG_SMP)

would be preferred.

> --- linux-2.6.16/kernel/irq/manage.c	2006-03-19 22:53:29.000000000 -0700
> +++ linux-2.6.16-kexec/kernel/irq/manage.c	2006-03-27 17:02:08.000000000 -0700
> @@ -377,3 +377,22 @@ int request_irq(unsigned int irq,
>  
>  EXPORT_SYMBOL(request_irq);
>  
> +/*
> + * Terminate any outstanding interrupts
> + */
> +void terminate_irqs(void)
> +{
> +	struct irqaction * action;
> +	irq_desc_t *idesc;
> +	int i;
> +
> +	for (i=0; i<NR_IRQS; i++) {

	for (i = 0; i < NR_IRQS; i++) {

> +		idesc = irq_descp(i);
> +		action = idesc->action;
> +		if (!action)
> +			continue;
> +		if (idesc->handler->end)
> +			idesc->handler->end(i);
> +	}
> +}

Could we have a bit more description of what this function does, and why we
need it?

Should other kexec-using architectures be using this?  If not, why does
ia64 need it?

Thanks.