PERFORCE change 89485 for review

[John Baldwin]

On Tuesday 10 January 2006 10:47 pm, Kip Macy wrote:
> http://perforce.freebsd.org/chv.cgi?CH=89485
>
> Change 89485 by kmacy at kmacy:freebsd7_xen3 on 2006/01/11 03:47:18
>
> 	add dom0 io apic support
> 	determination of ioapic_read / ioapic_write functions is made at runtime
>
> Affected files ...
>
> .. //depot/projects/xen3/src/sys/i386-xen/conf/XENCONF#2 edit
> .. //depot/projects/xen3/src/sys/i386-xen/i386-xen/io_apic.c#1 add
> .. //depot/projects/xen3/src/sys/i386-xen/i386-xen/machdep.c#5 edit
> .. //depot/projects/xen3/src/sys/i386/i386/io_apic.c#3 edit
> .. //depot/projects/xen3/src/sys/i386/i386/machdep.c#2 edit
> .. //depot/projects/xen3/src/sys/i386/i386/mptable.c#2 edit
>
> Differences ...
>
> ==== //depot/projects/xen3/src/sys/i386/i386/io_apic.c#3 (text+ko) ====
>
> @@ -539,34 +508,42 @@
>  void *
>  ioapic_create(uintptr_t addr, int32_t apic_id, int intbase)
>  {
> -	struct ioapic *io;
> +	struct ioapic *io, tio;
>  	struct ioapic_intsrc *intpin;
> -	volatile ioapic_t *apic;
>  	u_int numintr, i;
>  	uint32_t value;
>
> +	io = &tio;
> +	/* XXX should xen just ignore this? */
>  	/* Map the register window so we can access the device. */
> -	apic = (ioapic_t *)pmap_mapdev(addr, IOAPIC_MEM_REGION);
> +	io->io_addr = (ioapic_t *)pmap_mapdev(addr, IOAPIC_MEM_REGION);
> +
> +	/* set the nominal apic_id for the initial read */
> +	io->io_apic_id = apic_id;
> +
>  	mtx_lock_spin(&icu_lock);
> -	value = ioapic_read(apic, IOAPIC_VER);
> +	/* fetch the APIC id in case we're running under xen */
> +	io->io_apic_id = access.apic_read(io, IOAPIC_ID) >> APIC_ID_SHIFT;
> +	value = access.apic_read(io, IOAPIC_VER);
>  	mtx_unlock_spin(&icu_lock);
>
>  	/* If it's version register doesn't seem to work, punt. */
>  	if (value == 0xffffffff) {
> -		pmap_unmapdev((vm_offset_t)apic, IOAPIC_MEM_REGION);
> +		pmap_unmapdev((vm_offset_t)io->io_addr, IOAPIC_MEM_REGION);
> +		free(io, M_IOAPIC);

You haven't malloced an io here, so you shouldn't free it.  The reason 
ioapic_read/ioapic_write currently take the ioapic_t addr directly is to 
avoid the need for the tio hack.  I think you can make Xen work if you think 
a bit more creatively.  First off, don't pass in the raw address and don't 
call pmap_mapdev() (since you aren't going to use the pointer anyway) for 
Xen.  Instead, pass the APIC ID in as the 'addr' variable and just use that 
as io_addr and as the argument you pass to your xen functions.  Thus, you end 
up with something like this:

xen_ioapic_read(addr, reg)
{

	u.apic.id = addr;
	u.apic.reg = reg;
	/* hypervisor call */
}

And in ioapic_create() you start off doing something like this:

	if (running_xen)
		apic = (void *)addr;
	if (!running_xen)
		apic = pmap_mapdev();

	/* read IOAPIC_VER */
	/* test for 0xffffffff */
	/* read numintr value */
	if (!running_xen)  {
		update_apic_id();
	}

This would imply that Xen needs its own APIC enumerator, but that is probably 
sensible and those are fairly easy to write.  This would also simplify the 
diffs here a bunch I think.

>  		return (NULL);
>  	}
>
> -	/* Determine the number of vectors and set the APIC ID. */

Comment is still valid.

>  	numintr = ((value & IOART_VER_MAXREDIR) >> MAXREDIRSHIFT) + 1;
>  	io = malloc(sizeof(struct ioapic) +
>  	    numintr * sizeof(struct ioapic_intsrc), M_IOAPIC, M_WAITOK);
>  	io->io_pic = ioapic_template;

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org