Re: [PATCH RFC 05/13] xen: implement Xen IO APIC ops

On Tuesday, December 24, 2013 6:20:54 am Roger Pau Monne wrote:
> Implement a different set of hooks for IO APIC to use when running
> under Xen Dom0.
> ---
>  sys/x86/xen/pv.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
> index ab4afba..e5ad200 100644
> --- a/sys/x86/xen/pv.c
> +++ b/sys/x86/xen/pv.c
> _at__at_ -49,6 +49,7 _at__at_ __FBSDID("$FreeBSD$");
>  #include <vm/vm_pager.h>
>  #include <vm/vm_param.h>
>  
> +#include <x86/apicreg.h>
>  #include <machine/sysarch.h>
>  #include <machine/clock.h>
>  #include <machine/pc/bios.h>
> _at__at_ -58,6 +59,7 _at__at_ __FBSDID("$FreeBSD$");
>  #include <xen/xen-os.h>
>  #include <xen/hypervisor.h>
>  #include <xen/pv.h>
> +#include <xen/xen_intr.h>
>  
>  #include <xen/interface/vcpu.h>
>  
> _at__at_ -73,6 +75,11 _at__at_ static caddr_t xen_pv_parse_preload_data(u_int64_t);
>  static void xen_pv_parse_memmap(caddr_t, vm_paddr_t *, int *);
>  
>  static void xen_pv_set_init_ops(void);
> +
> +static u_int xen_pv_ioapic_read(volatile ioapic_t *, int);
> +static void xen_pv_ioapic_write(volatile ioapic_t *, int, u_int);
> +static void xen_pv_ioapic_register_intr(struct ioapic_intsrc *);
> +
>  /*---------------------------- Extern Declarations 
---------------------------*/
>  /* Variables used by amd64 mp_machdep to start APs */
>  extern struct mtx ap_boot_mtx;
> _at__at_ -92,6 +99,13 _at__at_ struct init_ops xen_init_ops = {
>  	.parse_memmap =		xen_pv_parse_memmap,
>  };
>  
> +/* Xen ioapic_ops implementation */
> +struct ioapic_ops xen_ioapic_ops = {
> +	.read =			xen_pv_ioapic_read,
> +	.write =		xen_pv_ioapic_write,
> +	.register_intr =	xen_pv_ioapic_register_intr,
> +};
> +
>  static struct
>  {
>  	const char	*ev;
> _at__at_ -342,6 +356,34 _at__at_ xen_pv_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, 
int *physmap_idx)
>  	bios_add_smap_entries(xen_smap, size, physmap, physmap_idx);
>  }
>  
> +static u_int
> +xen_pv_ioapic_read(volatile ioapic_t *apic, int reg)
> +{
> +	struct physdev_apic apic_op;
> +	int rc;
> +
> +	mtx_assert(&icu_lock, MA_OWNED);
> +
> +	apic_op.apic_physbase = pmap_kextract((vm_offset_t) apic);

Seems a shame to have to do this.  I wouldn't mind if you changed the 
read/write callbacks to take 'struct ioapic *' instead and then use the 
'io_paddr' member.  I do think that would be cleaner.

> +	apic_op.reg = reg;
> +	rc = HYPERVISOR_physdev_op(PHYSDEVOP_apic_read, &apic_op);
> +	if (rc)
> +		panic("apic_read operation failed");
> +
> +	return (apic_op.value);
> +}
> +
> +static void
> +xen_pv_ioapic_write(volatile ioapic_t *apic, int reg, u_int val)
> +{
> +}

I guess not allowing writes is on purpose?

> +
> +static void
> +xen_pv_ioapic_register_intr(struct ioapic_intsrc *pin)
> +{
> +	xen_register_pirq(pin->io_irq, pin->io_activehi, pin->io_edgetrigger);
> +}
> +
>  static void
>  xen_pv_set_init_ops(void)
>  {
> _at__at_ -349,4 +391,6 _at__at_ xen_pv_set_init_ops(void)
>  	init_ops = xen_init_ops;
>  	/* Disable lapic */
>  	lapic_disabled = true;
> +	/* IOAPIC ops for Xen PV */
> +	ioapic_ops = xen_ioapic_ops;
>  }
> -- 
> 1.7.7.5 (Apple Git-26)
> 
> 

-- 
John Baldwin