[PATCH RFC 01/13] xen: use the hardware e820 map on Dom0

[PATCH RFC 01/13] xen: use the hardware e820 map on Dom0

[Roger Pau Monne]

We need to do some tweaking of the hardware e820 map, since the memory
layout provided by Xen and the e820 map doesn't match.

This consists in clamping the e820 map so that regions above max_pfn
are marked as unusuable.
---
 sys/x86/xen/pv.c |   35 +++++++++++++++++++++++++++++++++--
 1 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
index 4f7415e..ab4afba 100644
--- a/sys/x86/xen/pv.c
+++ b/sys/x86/xen/pv.c
@@ -297,17 +297,48 @@ static void
 xen_pv_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx)
 {
 	struct xen_memory_map memmap;
+	unsigned long max_pfn = HYPERVISOR_start_info->nr_pages;
+	u_int64_t mem_end = ptoa(max_pfn);
 	u_int32_t size;
-	int rc;
+	int rc, mem_op, i;
 
 	/* Fetch the E820 map from Xen */
 	memmap.nr_entries = MAX_E820_ENTRIES;
 	set_xen_guest_handle(memmap.buffer, xen_smap);
-	rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
+	mem_op = xen_initial_domain() ?
+	                XENMEM_machine_memory_map :
+	                XENMEM_memory_map;
+	rc = HYPERVISOR_memory_op(mem_op, &memmap);
 	if (rc)
 		panic("unable to fetch Xen E820 memory map");
 	size = memmap.nr_entries * sizeof(xen_smap[0]);
 
+	/*
+	 * This should be improved, Xen provides us with a single
+	 * chunk of physical memory that goes from 0 to max_pfn,
+	 * and what we do here is clamp the HW memory map to make
+	 * sure regions above max_pfn are marked as reserved.
+	 *
+	 * TRTTD would be to move the memory not used because of
+	 * the holes in the HW memory map to the now clamped regions
+	 * using XENMEM_{decrease/increase}_reservation.
+	 */
+	for (i = 0; i < memmap.nr_entries; i++) {
+		u_int64_t end = xen_smap[i].base + xen_smap[i].length;
+		if (xen_smap[i].type == SMAP_TYPE_MEMORY) {
+			if (xen_smap[i].base > mem_end) {
+				/* Mark as reserved */
+				xen_smap[i].type = SMAP_TYPE_RESERVED;
+				continue;
+			}
+			if (end > mem_end) {
+				/* Truncate region */
+				xen_smap[i].length -= end - mem_end;
+			}
+		}
+	}
+
+
 	bios_add_smap_entries(xen_smap, size, physmap, physmap_idx);
 }
 
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 02/13] ioapic: introduce hooks for some ioapic ops

[Roger Pau Monne]

Create some hooks for IO APIC operations that will diverge from bare
metal when implemented for Xen Dom0.

This patch should not introduce any changes in functionality, it's a
preparatory patch for the implementation of the Xen IO APIC hooks.
---
 sys/amd64/include/apicvar.h |   13 ++++++++
 sys/i386/include/apicvar.h  |   13 ++++++++
 sys/x86/include/apicreg.h   |   12 ++++++++
 sys/x86/x86/io_apic.c       |   65 ++++++++++++++++++++++---------------------
 4 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/sys/amd64/include/apicvar.h b/sys/amd64/include/apicvar.h
index 84e01d4..a48a76b 100644
--- a/sys/amd64/include/apicvar.h
+++ b/sys/amd64/include/apicvar.h
@@ -161,6 +161,19 @@ struct apic_enumerator {
 	SLIST_ENTRY(apic_enumerator) apic_next;
 };
 
+struct ioapic_intsrc {
+	struct intsrc io_intsrc;
+	u_int io_irq;
+	u_int io_intpin:8;
+	u_int io_vector:8;
+	u_int io_cpu:8;
+	u_int io_activehi:1;
+	u_int io_edgetrigger:1;
+	u_int io_masked:1;
+	int io_bus:4;
+	uint32_t io_lowreg;
+};
+
 inthand_t
 	IDTVEC(apic_isr1), IDTVEC(apic_isr2), IDTVEC(apic_isr3),
 	IDTVEC(apic_isr4), IDTVEC(apic_isr5), IDTVEC(apic_isr6),
diff --git a/sys/i386/include/apicvar.h b/sys/i386/include/apicvar.h
index 24c99f0..c8ee9bc 100644
--- a/sys/i386/include/apicvar.h
+++ b/sys/i386/include/apicvar.h
@@ -160,6 +160,19 @@ struct apic_enumerator {
 	SLIST_ENTRY(apic_enumerator) apic_next;
 };
 
+struct ioapic_intsrc {
+	struct intsrc io_intsrc;
+	u_int io_irq;
+	u_int io_intpin:8;
+	u_int io_vector:8;
+	u_int io_cpu:8;
+	u_int io_activehi:1;
+	u_int io_edgetrigger:1;
+	u_int io_masked:1;
+	int io_bus:4;
+	uint32_t io_lowreg;
+};
+
 inthand_t
 	IDTVEC(apic_isr1), IDTVEC(apic_isr2), IDTVEC(apic_isr3),
 	IDTVEC(apic_isr4), IDTVEC(apic_isr5), IDTVEC(apic_isr6),
diff --git a/sys/x86/include/apicreg.h b/sys/x86/include/apicreg.h
index 283d50e..4629470 100644
--- a/sys/x86/include/apicreg.h
+++ b/sys/x86/include/apicreg.h
@@ -204,6 +204,18 @@ struct IOAPIC {
 
 typedef struct IOAPIC ioapic_t;
 
+struct ioapic_intsrc;
+/*
+ * Struct containing pointers to IO APIC management functions whose
+ * implementation is run time selectable.
+ */
+struct ioapic_ops {
+	u_int    (*read)(volatile ioapic_t *, int);
+	void     (*write)(volatile ioapic_t *, int, u_int);
+	void     (*register_intr)(struct ioapic_intsrc *);
+};
+extern struct ioapic_ops ioapic_ops;
+
 #undef PAD4
 #undef PAD3
 
diff --git a/sys/x86/x86/io_apic.c b/sys/x86/x86/io_apic.c
index 6d62b1e..125d06a 100644
--- a/sys/x86/x86/io_apic.c
+++ b/sys/x86/x86/io_apic.c
@@ -81,19 +81,6 @@ static MALLOC_DEFINE(M_IOAPIC, "io_apic", "I/O APIC structures");
  * ftp://download.intel.com/design/chipsets/datashts/29056601.pdf
  */
 
-struct ioapic_intsrc {
-	struct intsrc io_intsrc;
-	u_int io_irq;
-	u_int io_intpin:8;
-	u_int io_vector:8;
-	u_int io_cpu:8;
-	u_int io_activehi:1;
-	u_int io_edgetrigger:1;
-	u_int io_masked:1;
-	int io_bus:4;
-	uint32_t io_lowreg;
-};
-
 struct ioapic {
 	struct pic io_pic;
 	u_int io_id:8;			/* logical ID */
@@ -106,8 +93,9 @@ struct ioapic {
 	struct ioapic_intsrc io_pins[0];
 };
 
-static u_int	ioapic_read(volatile ioapic_t *apic, int reg);
-static void	ioapic_write(volatile ioapic_t *apic, int reg, u_int val);
+static u_int	native_ioapic_read(volatile ioapic_t *apic, int reg);
+static void	native_ioapic_write(volatile ioapic_t *apic, int reg, u_int val);
+static void	native_ioapic_register_intr(struct ioapic_intsrc *pin);
 static const char *ioapic_bus_string(int bus_type);
 static void	ioapic_print_irq(struct ioapic_intsrc *intpin);
 static void	ioapic_enable_source(struct intsrc *isrc);
@@ -139,6 +127,13 @@ SYSCTL_INT(_hw_apic, OID_AUTO, enable_extint, CTLFLAG_RDTUN, &enable_extint, 0,
     "Enable the ExtINT pin in the first I/O APIC");
 TUNABLE_INT("hw.apic.enable_extint", &enable_extint);
 
+/* Default ioapic_ops implementation. */
+struct ioapic_ops ioapic_ops = {
+	.read =			native_ioapic_read,
+	.write =		native_ioapic_write,
+	.register_intr =	native_ioapic_register_intr,
+};
+
 static __inline void
 _ioapic_eoi_source(struct intsrc *isrc)
 {
@@ -146,7 +141,7 @@ _ioapic_eoi_source(struct intsrc *isrc)
 }
 
 static u_int
-ioapic_read(volatile ioapic_t *apic, int reg)
+native_ioapic_read(volatile ioapic_t *apic, int reg)
 {
 
 	mtx_assert(&icu_lock, MA_OWNED);
@@ -155,7 +150,7 @@ ioapic_read(volatile ioapic_t *apic, int reg)
 }
 
 static void
-ioapic_write(volatile ioapic_t *apic, int reg, u_int val)
+native_ioapic_write(volatile ioapic_t *apic, int reg, u_int val)
 {
 
 	mtx_assert(&icu_lock, MA_OWNED);
@@ -163,6 +158,12 @@ ioapic_write(volatile ioapic_t *apic, int reg, u_int val)
 	apic->iowin = val;
 }
 
+static void
+native_ioapic_register_intr(struct ioapic_intsrc *pin)
+{
+	intr_register_source(&pin->io_intsrc);
+}
+
 static const char *
 ioapic_bus_string(int bus_type)
 {
@@ -212,7 +213,7 @@ ioapic_enable_source(struct intsrc *isrc)
 	mtx_lock_spin(&icu_lock);
 	if (intpin->io_masked) {
 		flags = intpin->io_lowreg & ~IOART_INTMASK;
-		ioapic_write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin),
+		ioapic_ops.write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin),
 		    flags);
 		intpin->io_masked = 0;
 	}
@@ -229,7 +230,7 @@ ioapic_disable_source(struct intsrc *isrc, int eoi)
 	mtx_lock_spin(&icu_lock);
 	if (!intpin->io_masked && !intpin->io_edgetrigger) {
 		flags = intpin->io_lowreg | IOART_INTMSET;
-		ioapic_write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin),
+		ioapic_ops.write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin),
 		    flags);
 		intpin->io_masked = 1;
 	}
@@ -264,10 +265,10 @@ ioapic_program_intpin(struct ioapic_intsrc *intpin)
 	mtx_assert(&icu_lock, MA_OWNED);
 	if (intpin->io_irq == IRQ_DISABLED || (intpin->io_irq < NUM_IO_INTS &&
 	    intpin->io_vector == 0)) {
-		low = ioapic_read(io->io_addr,
+		low = ioapic_ops.read(io->io_addr,
 		    IOAPIC_REDTBL_LO(intpin->io_intpin));
 		if ((low & IOART_INTMASK) == IOART_INTMCLR)
-			ioapic_write(io->io_addr,
+			ioapic_ops.write(io->io_addr,
 			    IOAPIC_REDTBL_LO(intpin->io_intpin),
 			    low | IOART_INTMSET);
 		return;
@@ -311,12 +312,12 @@ ioapic_program_intpin(struct ioapic_intsrc *intpin)
 	}
 
 	/* Write the values to the APIC. */
-	value = ioapic_read(io->io_addr, IOAPIC_REDTBL_HI(intpin->io_intpin));
+	value = ioapic_ops.read(io->io_addr, IOAPIC_REDTBL_HI(intpin->io_intpin));
 	value &= ~IOART_DEST;
 	value |= high;
-	ioapic_write(io->io_addr, IOAPIC_REDTBL_HI(intpin->io_intpin), value);
+	ioapic_ops.write(io->io_addr, IOAPIC_REDTBL_HI(intpin->io_intpin), value);
 	intpin->io_lowreg = low;
-	ioapic_write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin), low);
+	ioapic_ops.write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin), low);
 }
 
 static int
@@ -357,7 +358,7 @@ ioapic_assign_cpu(struct intsrc *isrc, u_int apic_id)
 	 */
 	mtx_lock_spin(&icu_lock);
 	if (!intpin->io_masked && !intpin->io_edgetrigger) {
-		ioapic_write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin),
+		ioapic_ops.write(io->io_addr, IOAPIC_REDTBL_LO(intpin->io_intpin),
 		    intpin->io_lowreg | IOART_INTMSET);
 		mtx_unlock_spin(&icu_lock);
 		DELAY(100);
@@ -512,7 +513,7 @@ ioapic_create(vm_paddr_t addr, int32_t apic_id, int intbase)
 	/* Map the register window so we can access the device. */
 	apic = pmap_mapdev(addr, IOAPIC_MEM_REGION);
 	mtx_lock_spin(&icu_lock);
-	value = ioapic_read(apic, IOAPIC_VER);
+	value = ioapic_ops.read(apic, IOAPIC_VER);
 	mtx_unlock_spin(&icu_lock);
 
 	/* If it's version register doesn't seem to work, punt. */
@@ -528,9 +529,9 @@ ioapic_create(vm_paddr_t addr, int32_t apic_id, int intbase)
 	io->io_pic = ioapic_template;
 	mtx_lock_spin(&icu_lock);
 	io->io_id = next_id++;
-	io->io_apic_id = ioapic_read(apic, IOAPIC_ID) >> APIC_ID_SHIFT;
+	io->io_apic_id = ioapic_ops.read(apic, IOAPIC_ID) >> APIC_ID_SHIFT;
 	if (apic_id != -1 && io->io_apic_id != apic_id) {
-		ioapic_write(apic, IOAPIC_ID, apic_id << APIC_ID_SHIFT);
+		ioapic_ops.write(apic, IOAPIC_ID, apic_id << APIC_ID_SHIFT);
 		mtx_unlock_spin(&icu_lock);
 		io->io_apic_id = apic_id;
 		printf("ioapic%u: Changing APIC ID to %d\n", io->io_id,
@@ -586,8 +587,8 @@ ioapic_create(vm_paddr_t addr, int32_t apic_id, int intbase)
 		 * be routed to other CPUs later after they are enabled.
 		 */
 		intpin->io_cpu = PCPU_GET(apic_id);
-		value = ioapic_read(apic, IOAPIC_REDTBL_LO(i));
-		ioapic_write(apic, IOAPIC_REDTBL_LO(i), value | IOART_INTMSET);
+		value = ioapic_ops.read(apic, IOAPIC_REDTBL_LO(i));
+		ioapic_ops.write(apic, IOAPIC_REDTBL_LO(i), value | IOART_INTMSET);
 	}
 	mtx_unlock_spin(&icu_lock);
 
@@ -788,7 +789,7 @@ ioapic_register(void *cookie)
 	io = (struct ioapic *)cookie;
 	apic = io->io_addr;
 	mtx_lock_spin(&icu_lock);
-	flags = ioapic_read(apic, IOAPIC_VER) & IOART_VER_VERSION;
+	flags = ioapic_ops.read(apic, IOAPIC_VER) & IOART_VER_VERSION;
 	STAILQ_INSERT_TAIL(&ioapic_list, io, io_next);
 	mtx_unlock_spin(&icu_lock);
 	printf("ioapic%u <Version %u.%u> irqs %u-%u on motherboard\n",
@@ -799,7 +800,7 @@ ioapic_register(void *cookie)
 	intr_register_pic(&io->io_pic);
 	for (i = 0, pin = io->io_pins; i < io->io_numintr; i++, pin++)
 		if (pin->io_irq < NUM_IO_INTS)
-			intr_register_source(&pin->io_intsrc);
+			ioapic_ops.register_intr(pin);
 }
 
 /* A simple new-bus driver to consume PCI I/O APIC devices. */
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 03/13] xen: mask event channels while changing affinity

[Roger Pau Monne]

Event channels should be masked while chaning affinity, or else we
might get spurious/lost interrupts.
---
 sys/x86/xen/xen_intr.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c
index fd36e68..bc0781e 100644
--- a/sys/x86/xen/xen_intr.c
+++ b/sys/x86/xen/xen_intr.c
@@ -797,7 +797,7 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
 	struct evtchn_bind_vcpu bind_vcpu;
 	struct xenisrc *isrc;
 	u_int to_cpu, vcpu_id;
-	int error;
+	int error, masked;
 
 #ifdef XENHVM
 	if (xen_vector_callback_enabled == 0)
@@ -815,6 +815,12 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
 		return (EINVAL);
 	}
 
+	/*
+	 * Mask the event channel port so we don't receive spurious events
+	 * while changing affinity.
+	 */
+	masked = evtchn_test_and_set_mask(isrc->xi_port);
+
 	if ((isrc->xi_type == EVTCHN_TYPE_VIRQ) ||
 		(isrc->xi_type == EVTCHN_TYPE_IPI)) {
 		/*
@@ -825,8 +831,7 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
 		evtchn_cpu_mask_port(isrc->xi_cpu, isrc->xi_port);
 		isrc->xi_cpu = to_cpu;
 		evtchn_cpu_unmask_port(isrc->xi_cpu, isrc->xi_port);
-		mtx_unlock(&xen_intr_isrc_lock);
-		return (0);
+		goto out;
 	}
 
 	bind_vcpu.port = isrc->xi_port;
@@ -848,6 +853,10 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
 			evtchn_cpu_mask_port(to_cpu, isrc->xi_port);
 		}
 	}
+
+out:
+	if (!masked)
+		evtchn_unmask_port(isrc->xi_port);
 	mtx_unlock(&xen_intr_isrc_lock);
 	return (0);
 #else
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 04/13] xen: implement basic PIRQ support for Dom0

[Roger Pau Monne]

This allows Dom0 to manage physical hardware, redirecting the
physical interrupts to event channels.
---
 sys/x86/xen/xen_intr.c |  190 +++++++++++++++++++++++++++++++++++++++++++++--
 sys/xen/xen_intr.h     |   11 +++
 2 files changed, 192 insertions(+), 9 deletions(-)

diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c
index bc0781e..340e5ed 100644
--- a/sys/x86/xen/xen_intr.c
+++ b/sys/x86/xen/xen_intr.c
@@ -104,6 +104,8 @@ DPCPU_DECLARE(struct vcpu_info *, vcpu_info);
 
 #define is_valid_evtchn(x)	((x) != 0)
 
+#define	EEXIST	17	/* Xen "already exists" error */
+
 struct xenisrc {
 	struct intsrc	xi_intsrc;
 	enum evtchn_type xi_type;
@@ -111,6 +113,9 @@ struct xenisrc {
 	int		xi_vector;	/* Global isrc vector number. */
 	evtchn_port_t	xi_port;
 	int		xi_pirq;
+	int 		xi_activehi:1;
+	int 		xi_edgetrigger:1;
+	int 		xi_configured:1;
 	int		xi_virq;
 	u_int		xi_close:1;	/* close on unbind? */
 	u_int		xi_needs_eoi:1;
@@ -136,6 +141,9 @@ static void	xen_intr_pirq_enable_source(struct intsrc *isrc);
 static void	xen_intr_pirq_disable_source(struct intsrc *isrc, int eoi);
 static void	xen_intr_pirq_eoi_source(struct intsrc *isrc);
 static void	xen_intr_pirq_enable_intr(struct intsrc *isrc);
+static void	xen_intr_pirq_disable_intr(struct intsrc *isrc);
+static int	xen_intr_pirq_config_intr(struct intsrc *isrc,
+		     enum intr_trigger trig, enum intr_polarity pol);
 
 /**
  * PIC interface for all event channel port types except physical IRQs.
@@ -163,12 +171,12 @@ struct pic xen_intr_pirq_pic = {
 	.pic_disable_source = xen_intr_pirq_disable_source,
 	.pic_eoi_source     = xen_intr_pirq_eoi_source,
 	.pic_enable_intr    = xen_intr_pirq_enable_intr,
-	.pic_disable_intr   = xen_intr_disable_intr,
+	.pic_disable_intr   = xen_intr_pirq_disable_intr,
 	.pic_vector         = xen_intr_vector,
 	.pic_source_pending = xen_intr_source_pending,
 	.pic_suspend        = xen_intr_suspend,
 	.pic_resume         = xen_intr_resume,
-	.pic_config_intr    = xen_intr_config_intr,
+	.pic_config_intr    = xen_intr_pirq_config_intr,
 	.pic_assign_cpu     = xen_intr_assign_cpu
 };
 
@@ -282,11 +290,10 @@ xen_intr_find_unused_isrc(enum evtchn_type type)
  *          object or NULL.
  */
 static struct xenisrc *
-xen_intr_alloc_isrc(enum evtchn_type type)
+xen_intr_alloc_isrc(enum evtchn_type type, int vector)
 {
 	static int warned;
 	struct xenisrc *isrc;
-	int vector;
 
 	KASSERT(mtx_owned(&xen_intr_isrc_lock), ("Evtchn alloc lock not held"));
 
@@ -297,12 +304,19 @@ xen_intr_alloc_isrc(enum evtchn_type type)
 		}
 		return (NULL);
 	}
-	vector = FIRST_EVTCHN_INT + xen_intr_isrc_count;
-	xen_intr_isrc_count++;
+
+	if (type != EVTCHN_TYPE_PIRQ) {
+		vector = FIRST_EVTCHN_INT + xen_intr_isrc_count;
+		xen_intr_isrc_count++;
+	}
+
+	KASSERT((intr_lookup_source(vector) == NULL),
+	        ("Trying to use an already allocated vector"));
 
 	mtx_unlock(&xen_intr_isrc_lock);
 	isrc = malloc(sizeof(*isrc), M_XENINTR, M_WAITOK | M_ZERO);
-	isrc->xi_intsrc.is_pic = &xen_intr_pic;
+	isrc->xi_intsrc.is_pic = (type == EVTCHN_TYPE_PIRQ) ?
+	                          &xen_intr_pirq_pic : &xen_intr_pic;
 	isrc->xi_vector = vector;
 	isrc->xi_type = type;
 	intr_register_source(&isrc->xi_intsrc);
@@ -388,7 +402,7 @@ xen_intr_bind_isrc(struct xenisrc **isrcp, evtchn_port_t local_port,
 	mtx_lock(&xen_intr_isrc_lock);
 	isrc = xen_intr_find_unused_isrc(type);
 	if (isrc == NULL) {
-		isrc = xen_intr_alloc_isrc(type);
+		isrc = xen_intr_alloc_isrc(type, 0);
 		if (isrc == NULL) {
 			mtx_unlock(&xen_intr_isrc_lock);
 			return (ENOSPC);
@@ -592,6 +606,10 @@ xen_intr_init(void *dummy __unused)
 	}
 
 	intr_register_pic(&xen_intr_pic);
+	intr_register_pic(&xen_intr_pirq_pic);
+
+	if (bootverbose)
+		printf("Xen interrupt system initialized\n");
 
 	return (0);
 }
@@ -925,6 +943,9 @@ xen_intr_pirq_disable_source(struct intsrc *base_isrc, int eoi)
 
 	isrc = (struct xenisrc *)base_isrc;
 	evtchn_mask_port(isrc->xi_port);
+
+	if (eoi == PIC_EOI)
+		xen_intr_pirq_eoi_source(base_isrc);
 }
 
 /*
@@ -966,8 +987,115 @@ xen_intr_pirq_eoi_source(struct intsrc *base_isrc)
  * \param isrc  The interrupt source to enable.
  */
 static void
-xen_intr_pirq_enable_intr(struct intsrc *isrc)
+xen_intr_pirq_enable_intr(struct intsrc *base_isrc)
+{
+	struct xenisrc *isrc;
+	struct evtchn_bind_pirq bind_pirq;
+	struct physdev_irq_status_query irq_status;
+	int error;
+
+	isrc = (struct xenisrc *)base_isrc;
+
+	if (!isrc->xi_configured) {
+		xen_intr_pirq_config_intr(base_isrc,
+                                isrc->xi_edgetrigger ? INTR_TRIGGER_EDGE :
+                                                       INTR_TRIGGER_LEVEL,
+                                isrc->xi_activehi ? INTR_POLARITY_HIGH :
+                                                    INTR_POLARITY_LOW);
+	}
+
+	irq_status.irq = isrc->xi_pirq;
+	error = HYPERVISOR_physdev_op(PHYSDEVOP_irq_status_query, &irq_status);
+	if (error)
+		panic("unable to get status of IRQ#%d", isrc->xi_pirq);
+
+	if (irq_status.flags & XENIRQSTAT_needs_eoi)
+		isrc->xi_needs_eoi = 1;
+
+	bind_pirq.pirq = isrc->xi_pirq;
+	bind_pirq.flags = isrc->xi_edgetrigger ? 0 : BIND_PIRQ__WILL_SHARE;
+	error = HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &bind_pirq);
+	if (error)
+		panic("unable to bind IRQ#%d", isrc->xi_pirq);
+
+	isrc->xi_port = bind_pirq.port;
+
+	mtx_lock(&xen_intr_isrc_lock);
+	KASSERT((xen_intr_port_to_isrc[bind_pirq.port] == NULL),
+		("trying to override an already setup event channel port"));
+	xen_intr_port_to_isrc[bind_pirq.port] = isrc;
+	mtx_unlock(&xen_intr_isrc_lock);
+
+	evtchn_unmask_port(isrc->xi_port);
+}
+
+/*
+ * Disable an interrupt source.
+ *
+ * \param isrc  The interrupt source to disable.
+ */
+static void
+xen_intr_pirq_disable_intr(struct intsrc *base_isrc)
+{
+	struct xenisrc *isrc;
+	struct evtchn_close close;
+	int error;
+
+	isrc = (struct xenisrc *)base_isrc;
+
+	close.port = isrc->xi_port;
+	error = HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
+	if (error)
+		panic("unable to close event channel %d IRQ#%d",
+		      isrc->xi_port, isrc->xi_pirq);
+
+	mtx_lock(&xen_intr_isrc_lock);
+	xen_intr_port_to_isrc[isrc->xi_port] = NULL;
+	mtx_unlock(&xen_intr_isrc_lock);
+
+	isrc->xi_port = 0;
+}
+
+/**
+ * Perform configuration of an interrupt source.
+ *
+ * \param isrc  The interrupt source to configure.
+ * \param trig  Edge or level.
+ * \param pol   Active high or low.
+ *
+ * \returns  0 if no events are pending, otherwise non-zero.
+ */
+static int
+xen_intr_pirq_config_intr(struct intsrc *base_isrc, enum intr_trigger trig,
+                          enum intr_polarity pol)
 {
+	struct xenisrc *isrc = (struct xenisrc *)base_isrc;
+	struct physdev_setup_gsi setup_gsi;
+	int error;
+
+	KASSERT(!(trig == INTR_TRIGGER_CONFORM || pol == INTR_POLARITY_CONFORM),
+	    ("%s: Conforming trigger or polarity\n", __func__));
+
+	setup_gsi.gsi = isrc->xi_pirq;
+	setup_gsi.triggering = trig == INTR_TRIGGER_EDGE ? 0 : 1;
+	setup_gsi.polarity = pol == INTR_POLARITY_HIGH ? 0 : 1;
+
+	error = HYPERVISOR_physdev_op(PHYSDEVOP_setup_gsi, &setup_gsi);
+	if (error == -EEXIST) {
+		if ((isrc->xi_edgetrigger && (trig != INTR_TRIGGER_EDGE)) ||
+		    (isrc->xi_activehi && (pol != INTR_POLARITY_HIGH)))
+			panic("unable to reconfigure interrupt IRQ#%d",
+			      isrc->xi_pirq);
+		error = 0;
+	}
+	if (error)
+		panic("unable to configure IRQ#%d\n", isrc->xi_pirq);
+
+	isrc->xi_configured = 1;
+	isrc->xi_activehi = pol == INTR_POLARITY_HIGH ? 1 : 0;
+	isrc->xi_edgetrigger = trig == INTR_POLARITY_HIGH ? 1 : 0;
+
+	return (0);
 }
 
 /*--------------------------- Public Functions -------------------------------*/
@@ -1190,6 +1318,50 @@ xen_intr_alloc_and_bind_ipi(device_t dev, u_int cpu,
 }
 
 int
+xen_register_pirq(int vector, int activehi, int edgetrigger)
+{
+	struct physdev_map_pirq map_pirq;
+	struct physdev_irq alloc_pirq;
+	struct xenisrc *isrc;
+	int error;
+
+	if (vector == 0)
+		return (EINVAL);
+
+	if (bootverbose)
+		printf("xen: register IRQ#%d\n", vector);
+
+	map_pirq.domid = DOMID_SELF;
+	map_pirq.type = MAP_PIRQ_TYPE_GSI;
+	map_pirq.index = vector;
+	map_pirq.pirq = vector;
+
+	error = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_pirq);
+	if (error) {
+		printf("xen: unable to map IRQ#%d\n", vector);
+		return (error);
+	}
+
+	alloc_pirq.irq = vector;
+	alloc_pirq.vector = 0;
+	error = HYPERVISOR_physdev_op(PHYSDEVOP_alloc_irq_vector, &alloc_pirq);
+	if (error) {
+		printf("xen: unable to alloc PIRQ for IRQ#%d\n", vector);
+		return (error);
+	}
+
+	mtx_lock(&xen_intr_isrc_lock);
+	isrc = xen_intr_alloc_isrc(EVTCHN_TYPE_PIRQ, vector);
+	mtx_unlock(&xen_intr_isrc_lock);
+	KASSERT((isrc != NULL), ("xen: unable to allocate isrc for interrupt"));
+	isrc->xi_pirq = vector;
+	isrc->xi_activehi = activehi;
+	isrc->xi_edgetrigger = edgetrigger;
+
+	return (0);
+}
+
+int
 xen_intr_describe(xen_intr_handle_t port_handle, const char *fmt, ...)
 {
 	char descr[MAXCOMLEN + 1];
diff --git a/sys/xen/xen_intr.h b/sys/xen/xen_intr.h
index 3b339a5..eda5fdf 100644
--- a/sys/xen/xen_intr.h
+++ b/sys/xen/xen_intr.h
@@ -159,6 +159,17 @@ int xen_intr_alloc_and_bind_ipi(device_t dev, u_int cpu,
 	xen_intr_handle_t *handlep);
 
 /**
+ * Register a physical interrupt vector and setup the interrupt source.
+ *
+ * \param vector        The global vector to use.
+ * \param activehi      Default polarity of the interrupt.
+ * \param edgetrigger   Default trigger method.
+ *
+ * \returns  0 on success, otherwise an errno.
+ */
+int xen_register_pirq(int vector, int activehi, int edgetrigger);
+
+/**
  * Unbind an interrupt handler from its interrupt source.
  *
  * \param handlep  A pointer to the opaque handle that was initialized
-- 
1.7.7.5 (Apple Git-26)

Re: [PATCH RFC 04/13] xen: implement basic PIRQ support for Dom0

[John Baldwin]

On Tuesday, December 24, 2013 6:20:53 am Roger Pau Monne wrote:
> This allows Dom0 to manage physical hardware, redirecting the
> physical interrupts to event channels.
> ---
>  sys/x86/xen/xen_intr.c |  190 
+++++++++++++++++++++++++++++++++++++++++++++--
>  sys/xen/xen_intr.h     |   11 +++
>  2 files changed, 192 insertions(+), 9 deletions(-)
> 
> diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c
> index bc0781e..340e5ed 100644
> --- a/sys/x86/xen/xen_intr.c
> +++ b/sys/x86/xen/xen_intr.c
> @@ -104,6 +104,8 @@ DPCPU_DECLARE(struct vcpu_info *, vcpu_info);
>  
>  #define is_valid_evtchn(x)	((x) != 0)
>  
> +#define	EEXIST	17	/* Xen "already exists" error */

Is there a xen_errno.h header?  Might be nice to have one and give these 
constants unique names (e.g. XEN_EEXIST or some such) to avoid 
confusion/conflicts with <sys/errno.h>.

Other than that I think this looks fine.

-- 
John Baldwin

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

[Roger Pau Monne]

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
@@ -49,6 +49,7 @@ __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>
@@ -58,6 +59,7 @@ __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>
 
@@ -73,6 +75,11 @@ 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;
@@ -92,6 +99,13 @@ 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;
@@ -342,6 +356,34 @@ 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);
+	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)
+{
+}
+
+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)
 {
@@ -349,4 +391,6 @@ 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)

[PATCH RFC 06/13] xen: Dom0 console fixes

[Roger Pau Monne]

Minor fixes and workarounds to make the Xen Dom0 console work.
---
 sys/dev/xen/console/xencons_ring.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/sys/dev/xen/console/xencons_ring.c b/sys/dev/xen/console/xencons_ring.c
index d826363..ea97f7b 100644
--- a/sys/dev/xen/console/xencons_ring.c
+++ b/sys/dev/xen/console/xencons_ring.c
@@ -48,6 +48,9 @@ xencons_has_input(void)
 {
 	struct xencons_interface *intf; 
 
+	if (xen_initial_domain())
+		return 1;
+
 	intf = xencons_interface();		
 
 	return (intf->in_cons != intf->in_prod);
@@ -97,6 +100,19 @@ xencons_handle_input(void *unused)
 	XENCONS_RING_IDX cons, prod;
 
 	CN_LOCK(cn_mtx);
+
+	if (xen_initial_domain()) {
+		/* XXX: read from console */
+		static char rbuf[16];
+		int         l;
+
+		while ((l = HYPERVISOR_console_io(CONSOLEIO_read, 16, rbuf)) > 0)
+			xencons_rx(rbuf, l);
+
+		CN_UNLOCK(cn_mtx);
+		return;
+	}
+
 	intf = xencons_interface();
 
 	cons = intf->in_cons;
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 07/13] xen: implement IO APIC support in Xen mptable parser

[Roger Pau Monne]

Use madt_setup_io (from madt.c) on Xen apic_enumerator, in order to
parse the interrupt sources from the IO APIC.

I would like to get opinions, but I think we should rename and move
madt_setup_io to io_apic.c.
---
 sys/amd64/include/apicvar.h |    1 +
 sys/i386/include/apicvar.h  |    1 +
 sys/x86/acpica/madt.c       |    5 ++---
 sys/x86/xen/mptable.c       |   24 +++++++++++++++++++++++-
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/sys/amd64/include/apicvar.h b/sys/amd64/include/apicvar.h
index a48a76b..3974067 100644
--- a/sys/amd64/include/apicvar.h
+++ b/sys/amd64/include/apicvar.h
@@ -233,6 +233,7 @@ int	lapic_set_lvt_triggermode(u_int apic_id, u_int lvt,
 void	lapic_set_tpr(u_int vector);
 void	lapic_setup(int boot);
 void	xen_intr_handle_upcall(struct trapframe *frame);
+int	madt_setup_io(void);
 
 #endif /* !LOCORE */
 #endif /* _MACHINE_APICVAR_H_ */
diff --git a/sys/i386/include/apicvar.h b/sys/i386/include/apicvar.h
index c8ee9bc..05ec013 100644
--- a/sys/i386/include/apicvar.h
+++ b/sys/i386/include/apicvar.h
@@ -232,6 +232,7 @@ int	lapic_set_lvt_triggermode(u_int apic_id, u_int lvt,
 void	lapic_set_tpr(u_int vector);
 void	lapic_setup(int boot);
 void	xen_intr_handle_upcall(struct trapframe *frame);
+int	madt_setup_io(void);
 
 #endif /* !LOCORE */
 #endif /* _MACHINE_APICVAR_H_ */
diff --git a/sys/x86/acpica/madt.c b/sys/x86/acpica/madt.c
index 5929fde..6f3b591 100644
--- a/sys/x86/acpica/madt.c
+++ b/sys/x86/acpica/madt.c
@@ -61,7 +61,7 @@ static struct lapic_info {
 } lapics[MAX_APIC_ID + 1];
 
 static int madt_found_sci_override;
-static ACPI_TABLE_MADT *madt;
+ACPI_TABLE_MADT *madt;
 static vm_paddr_t madt_physaddr;
 static vm_offset_t madt_length;
 
@@ -84,7 +84,6 @@ static void	madt_probe_cpus_handler(ACPI_SUBTABLE_HEADER *entry,
 		    void *arg __unused);
 static void	madt_register(void *dummy);
 static int	madt_setup_local(void);
-static int	madt_setup_io(void);
 static void	madt_walk_table(acpi_subtable_handler *handler, void *arg);
 
 static struct apic_enumerator madt_enumerator = {
@@ -147,7 +146,7 @@ madt_setup_local(void)
 /*
  * Enumerate I/O APICs and setup interrupt sources.
  */
-static int
+int
 madt_setup_io(void)
 {
 	void *ioapic;
diff --git a/sys/x86/xen/mptable.c b/sys/x86/xen/mptable.c
index 0384886..46b03f3 100644
--- a/sys/x86/xen/mptable.c
+++ b/sys/x86/xen/mptable.c
@@ -43,6 +43,9 @@ __FBSDID("$FreeBSD$");
 #include <machine/intr_machdep.h>
 #include <machine/apicvar.h>
 
+#include <contrib/dev/acpica/include/acpi.h>
+#include <contrib/dev/acpica/include/actables.h>
+
 #include <machine/cpu.h>
 #include <machine/smp.h>
 
@@ -51,6 +54,9 @@ __FBSDID("$FreeBSD$");
 
 #include <xen/interface/vcpu.h>
 
+/* From madt.c */
+extern ACPI_TABLE_MADT *madt;
+
 static int xenpv_probe(void);
 static int xenpv_probe_cpus(void);
 static int xenpv_setup_local(void);
@@ -107,7 +113,23 @@ xenpv_setup_local(void)
 static int
 xenpv_setup_io(void)
 {
-	return (0);
+	vm_paddr_t madt_physaddr;
+	vm_offset_t madt_length;
+
+	if (!xen_initial_domain())
+		return (0);
+
+	madt_physaddr = acpi_find_table(ACPI_SIG_MADT);
+	if (madt_physaddr == 0)
+		panic("could not find MADT table");
+	madt = acpi_map_table(madt_physaddr, ACPI_SIG_MADT);
+	if (madt == NULL)
+		panic("unable to map MADT");
+	madt_length = madt->Header.Length;
+	acpi_unmap_table(madt);
+	madt = pmap_mapbios(madt_physaddr, madt_length);
+
+	return (madt_setup_io());
 }
 
 static void
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 08/13] xen: change order of Xen intr init and IO APIC registration

[Roger Pau Monne]

Change order of some of the services in the SI_SUB_INTR stage, so
that it follows the order:

- System intr initialization
- Xen intr initalization
- IO APIC source registration
---
 sys/x86/x86/local_apic.c |    2 +-
 sys/x86/xen/xen_intr.c   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sys/x86/x86/local_apic.c b/sys/x86/x86/local_apic.c
index 85736c8..c206281 100644
--- a/sys/x86/x86/local_apic.c
+++ b/sys/x86/x86/local_apic.c
@@ -1383,7 +1383,7 @@ apic_setup_io(void *dummy __unused)
 	/* Enable the MSI "pic". */
 	msi_init();
 }
-SYSINIT(apic_setup_io, SI_SUB_INTR, SI_ORDER_SECOND, apic_setup_io, NULL);
+SYSINIT(apic_setup_io, SI_SUB_INTR, SI_ORDER_THIRD, apic_setup_io, NULL);
 
 #ifdef SMP
 /*
diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c
index 340e5ed..700fd22 100644
--- a/sys/x86/xen/xen_intr.c
+++ b/sys/x86/xen/xen_intr.c
@@ -613,7 +613,7 @@ xen_intr_init(void *dummy __unused)
 
 	return (0);
 }
-SYSINIT(xen_intr_init, SI_SUB_INTR, SI_ORDER_MIDDLE, xen_intr_init, NULL);
+SYSINIT(xen_intr_init, SI_SUB_INTR, SI_ORDER_SECOND, xen_intr_init, NULL);
 
 /*--------------------------- Common PIC Functions ---------------------------*/
 /**
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 09/13] xen: change quality of the MADT ACPI enumerator

[Roger Pau Monne]

Lower the quality of the MADT ACPI enumerator, so on Xen Dom0 we can
force the usage of the Xen mptable enumerator even when ACPI is
detected.
---
 sys/x86/acpica/madt.c |    2 +-
 sys/x86/xen/mptable.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sys/x86/acpica/madt.c b/sys/x86/acpica/madt.c
index 6f3b591..897189c 100644
--- a/sys/x86/acpica/madt.c
+++ b/sys/x86/acpica/madt.c
@@ -104,7 +104,7 @@ madt_probe(void)
 	madt_physaddr = acpi_find_table(ACPI_SIG_MADT);
 	if (madt_physaddr == 0)
 		return (ENXIO);
-	return (0);
+	return (-50);
 }
 
 /*
diff --git a/sys/x86/xen/mptable.c b/sys/x86/xen/mptable.c
index 46b03f3..a9704ab 100644
--- a/sys/x86/xen/mptable.c
+++ b/sys/x86/xen/mptable.c
@@ -76,7 +76,7 @@ static struct apic_enumerator xenpv_enumerator = {
 static int
 xenpv_probe(void)
 {
-	return (-100);
+	return (0);
 }
 
 /*
-- 
1.7.7.5 (Apple Git-26)

Re: [PATCH RFC 09/13] xen: change quality of the MADT ACPI enumerator

[John Baldwin]

On Tuesday, December 24, 2013 12:20:58 PM Roger Pau Monne wrote:
> Lower the quality of the MADT ACPI enumerator, so on Xen Dom0 we can
> force the usage of the Xen mptable enumerator even when ACPI is
> detected.

Hmm, so I think one question is why does the existing MADT parser
not work with the MADT table provided by Xen?  This may very well
be correct, but if it's only a small change to make the existing
MADT parser work with Xen's MADT table, that route might be
preferable.

-- 
John Baldwin

Re: [PATCH RFC 09/13] xen: change quality of the MADT ACPI enumerator

[Andrew Cooper]

On 08/02/2014 21:42, John Baldwin wrote:
> On Tuesday, December 24, 2013 12:20:58 PM Roger Pau Monne wrote:
>> Lower the quality of the MADT ACPI enumerator, so on Xen Dom0 we can
>> force the usage of the Xen mptable enumerator even when ACPI is
>> detected.
> Hmm, so I think one question is why does the existing MADT parser
> not work with the MADT table provided by Xen?  This may very well
> be correct, but if it's only a small change to make the existing
> MADT parser work with Xen's MADT table, that route might be
> preferable.
>

For dom0, the MADT seen is the system MADT, which does not bear any
reality to dom0's topology.  For PV domU, no MADT will be found.  For
HVM domU, the MADT seen ought to represent (virtual) reality.

~Andrew

Re: [PATCH RFC 09/13] xen: change quality of the MADT ACPI enumerator

[John Baldwin]

On Thursday, February 13, 2014 8:49:24 pm Andrew Cooper wrote:
> On 08/02/2014 21:42, John Baldwin wrote:
> > On Tuesday, December 24, 2013 12:20:58 PM Roger Pau Monne wrote:
> >> Lower the quality of the MADT ACPI enumerator, so on Xen Dom0 we can
> >> force the usage of the Xen mptable enumerator even when ACPI is
> >> detected.
> > Hmm, so I think one question is why does the existing MADT parser
> > not work with the MADT table provided by Xen?  This may very well
> > be correct, but if it's only a small change to make the existing
> > MADT parser work with Xen's MADT table, that route might be
> > preferable.
> >
> 
> For dom0, the MADT seen is the system MADT, which does not bear any
> reality to dom0's topology.  For PV domU, no MADT will be found.  For
> HVM domU, the MADT seen ought to represent (virtual) reality.

Hmm, the other changes suggested that you do want to use the I/O APIC
entries and interrupt overrides from the system MADT for dom0?  Just
not the CPU entries.  Is that correct?

-- 
John Baldwin

Re: [PATCH RFC 09/13] xen: change quality of the MADT ACPI enumerator

[Roger Pau Monné]

On 14/02/14 18:51, John Baldwin wrote:
> On Thursday, February 13, 2014 8:49:24 pm Andrew Cooper wrote:
>> On 08/02/2014 21:42, John Baldwin wrote:
>>> On Tuesday, December 24, 2013 12:20:58 PM Roger Pau Monne wrote:
>>>> Lower the quality of the MADT ACPI enumerator, so on Xen Dom0 we can
>>>> force the usage of the Xen mptable enumerator even when ACPI is
>>>> detected.
>>> Hmm, so I think one question is why does the existing MADT parser
>>> not work with the MADT table provided by Xen?  This may very well
>>> be correct, but if it's only a small change to make the existing
>>> MADT parser work with Xen's MADT table, that route might be
>>> preferable.
>>>
>>
>> For dom0, the MADT seen is the system MADT, which does not bear any
>> reality to dom0's topology.  For PV domU, no MADT will be found.  For
>> HVM domU, the MADT seen ought to represent (virtual) reality.
> 
> Hmm, the other changes suggested that you do want to use the I/O APIC
> entries and interrupt overrides from the system MADT for dom0?  Just
> not the CPU entries.  Is that correct?

Yes, we need the interrupt entries in order to interact with the
underlying hardware, but not the CPU entries/topology.

Roger.

[PATCH RFC 10/13] xen: add ACPI bus to xen_nexus when running as Dom0

[Roger Pau Monne]

Also disable a couple of ACPI devices that are not usable under Dom0.
---
 sys/x86/xen/xen_nexus.c |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/sys/x86/xen/xen_nexus.c b/sys/x86/xen/xen_nexus.c
index 288e6b6..823b3bc 100644
--- a/sys/x86/xen/xen_nexus.c
+++ b/sys/x86/xen/xen_nexus.c
@@ -35,6 +35,10 @@ __FBSDID("$FreeBSD$");
 #include <sys/systm.h>
 #include <sys/smp.h>
 
+#include <contrib/dev/acpica/include/acpi.h>
+
+#include <dev/acpica/acpivar.h>
+
 #include <machine/nexusvar.h>
 
 #include <xen/xen-os.h>
@@ -44,7 +48,6 @@ static const char *xen_devices[] =
 	"xenstore",		/* XenStore bus */
 	"xen_et",		/* Xen PV timer (provides: tc, et, clk) */
 	"xc",			/* Xen PV console */
-	"isa",			/* Dummy ISA bus for sc to attach */
 };
 
 /*
@@ -56,13 +59,14 @@ nexus_xen_probe(device_t dev)
 	if (!xen_pv_domain())
 		return (ENXIO);
 
-	return (BUS_PROBE_DEFAULT);
+	return (BUS_PROBE_SPECIFIC);
 }
 
 static int
 nexus_xen_attach(device_t dev)
 {
 	int i, error = 0;
+	device_t acpi_dev;
 
 	nexus_init_resources();
 	bus_generic_probe(dev);
@@ -79,8 +83,22 @@ nexus_xen_attach(device_t dev)
 		if (BUS_ADD_CHILD(dev, 0, xen_devices[i], 0) == NULL)
 			panic("%s: could not add", xen_devices[i]);
 	}
+	if (xen_initial_domain()) {
+		/* Disable some ACPI devices that are not usable by Dom0 */
+		setenv("debug.acpi.disabled", "cpu hpet timer");
+
+		acpi_dev = BUS_ADD_CHILD(dev, 10, "acpi", 0);
+		if (acpi_dev == NULL)
+			panic("Unable to add ACPI bus to Xen Dom0");
+	} else {
+		/* Dummy ISA bus for sc to attach */
+		if (BUS_ADD_CHILD(dev, 0, "isa", 0) == NULL)
+			panic("isa: could not add");
+	}
 
-	bus_generic_attach(dev);
+	error = bus_generic_attach(dev);
+	if (xen_initial_domain() && (error == 0))
+		acpi_install_wakeup_handler(device_get_softc(acpi_dev));
 
 	return (error);
 }
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 11/13] pci: introduce a new event on PCI device detection

[Roger Pau Monne]

Add a new event that will fire each time a PCI device is added to the
system, and allows us to register the device with Xen.
---
 sys/dev/pci/pci.c       |    1 +
 sys/sys/eventhandler.h  |    5 +++++
 sys/x86/xen/pv.c        |   21 +++++++++++++++++++++
 sys/x86/xen/xen_nexus.c |    6 ++++++
 sys/xen/pv.h            |    1 +
 5 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c
index 4d8837f..2ee5093 100644
--- a/sys/dev/pci/pci.c
+++ b/sys/dev/pci/pci.c
@@ -3293,6 +3293,7 @@ pci_add_child(device_t bus, struct pci_devinfo *dinfo)
 	resource_list_init(&dinfo->resources);
 	pci_cfg_save(dinfo->cfg.dev, dinfo, 0);
 	pci_cfg_restore(dinfo->cfg.dev, dinfo);
+	EVENTHANDLER_INVOKE(pci_add, dinfo);
 	pci_print_verbose(dinfo);
 	pci_add_resources(bus, dinfo->cfg.dev, 0, 0);
 }
diff --git a/sys/sys/eventhandler.h b/sys/sys/eventhandler.h
index 111c21b..3201848 100644
--- a/sys/sys/eventhandler.h
+++ b/sys/sys/eventhandler.h
@@ -269,5 +269,10 @@ typedef void (*unregister_framebuffer_fn)(void *, struct fb_info *);
 EVENTHANDLER_DECLARE(register_framebuffer, register_framebuffer_fn);
 EVENTHANDLER_DECLARE(unregister_framebuffer, unregister_framebuffer_fn);
 
+/* PCI events */
+struct pci_devinfo;
+typedef void (*pci_add_fn)(void *, struct pci_devinfo *);
+EVENTHANDLER_DECLARE(pci_add, pci_add_fn);
+
 #endif /* _SYS_EVENTHANDLER_H_ */
 
diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
index e5ad200..a44f8ca 100644
--- a/sys/x86/xen/pv.c
+++ b/sys/x86/xen/pv.c
@@ -39,6 +39,9 @@ __FBSDID("$FreeBSD$");
 #include <sys/rwlock.h>
 #include <sys/mutex.h>
 #include <sys/smp.h>
+#include <sys/reboot.h>
+#include <sys/pciio.h>
+#include <sys/eventhandler.h>
 
 #include <vm/vm.h>
 #include <vm/vm_extern.h>
@@ -63,6 +66,8 @@ __FBSDID("$FreeBSD$");
 
 #include <xen/interface/vcpu.h>
 
+#include <dev/pci/pcivar.h>
+
 /* Native initial function */
 extern u_int64_t hammer_time(u_int64_t, u_int64_t);
 /* Xen initial function */
@@ -384,6 +389,22 @@ xen_pv_ioapic_register_intr(struct ioapic_intsrc *pin)
 	xen_register_pirq(pin->io_irq, pin->io_activehi, pin->io_edgetrigger);
 }
 
+void
+xen_pv_pci_device_add(void *arg, struct pci_devinfo *dinfo)
+{
+	struct physdev_pci_device_add add_pci;
+	int error;
+
+	bzero(&add_pci, sizeof(add_pci));
+	add_pci.seg = dinfo->cfg.domain;
+	add_pci.bus = dinfo->cfg.bus;
+	add_pci.devfn = (dinfo->cfg.slot << 3) | dinfo->cfg.func;
+	error = HYPERVISOR_physdev_op(PHYSDEVOP_pci_device_add, &add_pci);
+	if (error)
+		printf("unable to add device bus %u devfn %u error: %d\n",
+		       add_pci.bus, add_pci.devfn, error);
+}
+
 static void
 xen_pv_set_init_ops(void)
 {
diff --git a/sys/x86/xen/xen_nexus.c b/sys/x86/xen/xen_nexus.c
index 823b3bc..60c6c5d 100644
--- a/sys/x86/xen/xen_nexus.c
+++ b/sys/x86/xen/xen_nexus.c
@@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/sysctl.h>
 #include <sys/systm.h>
 #include <sys/smp.h>
+#include <sys/eventhandler.h>
 
 #include <contrib/dev/acpica/include/acpi.h>
 
@@ -42,6 +43,7 @@ __FBSDID("$FreeBSD$");
 #include <machine/nexusvar.h>
 
 #include <xen/xen-os.h>
+#include <xen/pv.h>
 
 static const char *xen_devices[] =
 {
@@ -87,6 +89,10 @@ nexus_xen_attach(device_t dev)
 		/* Disable some ACPI devices that are not usable by Dom0 */
 		setenv("debug.acpi.disabled", "cpu hpet timer");
 
+		/* Register PCI add hook */
+		EVENTHANDLER_REGISTER(pci_add, xen_pv_pci_device_add, NULL,
+		                      EVENTHANDLER_PRI_FIRST);
+
 		acpi_dev = BUS_ADD_CHILD(dev, 10, "acpi", 0);
 		if (acpi_dev == NULL)
 			panic("Unable to add ACPI bus to Xen Dom0");
diff --git a/sys/xen/pv.h b/sys/xen/pv.h
index a9d6eb0..ac737a7 100644
--- a/sys/xen/pv.h
+++ b/sys/xen/pv.h
@@ -24,5 +24,6 @@
 #define	__XEN_PV_H__
 
 int	xen_pv_start_all_aps(void);
+void	xen_pv_pci_device_add(void *, struct pci_devinfo *);
 
 #endif	/* __XEN_PV_H__ */
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 12/13] mca: disable cmc enable on Xen PV

[Roger Pau Monne]

Xen PV guests doesn't have a lapic, so disable the lapic call in mca
initialization.
---
 sys/x86/x86/mca.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sys/x86/x86/mca.c b/sys/x86/x86/mca.c
index f1369cd..e9d2c1d 100644
--- a/sys/x86/x86/mca.c
+++ b/sys/x86/x86/mca.c
@@ -897,7 +897,7 @@ _mca_init(int boot)
 		}
 
 #ifdef DEV_APIC
-		if (PCPU_GET(cmci_mask) != 0 && boot)
+		if (PCPU_GET(cmci_mask) != 0 && boot && !lapic_disabled)
 			lapic_enable_cmc();
 #endif
 	}
-- 
1.7.7.5 (Apple Git-26)

[PATCH RFC 13/13] xenstore: changes needed to boot in Dom0 mode

[Roger Pau Monne]

This patch includes changes to xenstore in order to boot as Dom0. This
is different from booting as a guest, since when booted as Dom0
xenstore is not available. This patch sets up a memory page, an
event channel for xenstore and disables xenbus device probing at boot.
It contains a workaround for xs_watch, that should be fixed when we
are able to start xenstored from Dom0.
---
 sys/xen/xenbus/xenbusb.c    |    6 ++++--
 sys/xen/xenstore/xenstore.c |   22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/sys/xen/xenbus/xenbusb.c b/sys/xen/xenbus/xenbusb.c
index 1f84795..11be0f5 100644
--- a/sys/xen/xenbus/xenbusb.c
+++ b/sys/xen/xenbus/xenbusb.c
@@ -760,8 +760,10 @@ xenbusb_attach(device_t dev, char *bus_node, u_int id_components)
 	 * bus when they are dynamically attached to us
 	 * by a Xen management action.
 	 */
-	(void)xenbusb_enumerate_bus(xbs);
-	xenbusb_probe_children(dev);
+	if (!xen_initial_domain()) {
+		(void)xenbusb_enumerate_bus(xbs);
+		xenbusb_probe_children(dev);
+	}
 
 	xbs->xbs_device_watch.node = bus_node;
 	xbs->xbs_device_watch.callback = xenbusb_devices_changed;
diff --git a/sys/xen/xenstore/xenstore.c b/sys/xen/xenstore/xenstore.c
index 2893c84..bde3f5d 100644
--- a/sys/xen/xenstore/xenstore.c
+++ b/sys/xen/xenstore/xenstore.c
@@ -1144,6 +1144,24 @@ xs_attach(device_t dev)
 		xs.gpfn = hvm_get_parameter(HVM_PARAM_STORE_PFN);
 		xen_store = pmap_mapdev(xs.gpfn * PAGE_SIZE, PAGE_SIZE);
 	} else if (xen_pv_domain()) {
+		if (!HYPERVISOR_start_info->store_evtchn) {
+			struct evtchn_alloc_unbound alloc_unbound;
+
+			/* Allocate a local event channel for xenstore */
+			alloc_unbound.dom        = DOMID_SELF;
+			alloc_unbound.remote_dom = DOMID_SELF;
+			error = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound,
+			                                    &alloc_unbound);
+			if (error)
+				panic("unable to alloc event channel for Dom0: %d",
+				      error);
+
+			HYPERVISOR_start_info->store_evtchn = alloc_unbound.port;
+
+			/* Allocate memory for the xs shared ring */
+			xen_store = malloc(PAGE_SIZE, M_XENSTORE,
+			                   M_WAITOK | M_ZERO);
+		}
 		xs.evtchn = HYPERVISOR_start_info->store_evtchn;
 	} else {
 		panic("Unknown domain type, cannot initialize xenstore\n");
@@ -1579,6 +1597,10 @@ xs_register_watch(struct xs_watch *watch)
 	char token[sizeof(watch) * 2 + 1];
 	int error;
 
+	/* XXX: this is a hack until we get xenstored working */
+	if (xen_initial_domain())
+		return (0);
+
 	sprintf(token, "%lX", (long)watch);
 
 	sx_slock(&xs.suspend_mutex);
-- 
1.7.7.5 (Apple Git-26)

Re: [PATCH RFC 03/13] xen: mask event channels while changing affinity

[Konrad Rzeszutek Wilk]

On Tue, Dec 24, 2013 at 12:20:52PM +0100, Roger Pau Monne wrote:
> Event channels should be masked while chaning affinity, or else we
changing

> might get spurious/lost interrupts.
> ---
>  sys/x86/xen/xen_intr.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c
> index fd36e68..bc0781e 100644
> --- a/sys/x86/xen/xen_intr.c
> +++ b/sys/x86/xen/xen_intr.c
> @@ -797,7 +797,7 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
>  	struct evtchn_bind_vcpu bind_vcpu;
>  	struct xenisrc *isrc;
>  	u_int to_cpu, vcpu_id;
> -	int error;
> +	int error, masked;
>  
>  #ifdef XENHVM
>  	if (xen_vector_callback_enabled == 0)
> @@ -815,6 +815,12 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
>  		return (EINVAL);
>  	}
>  
> +	/*
> +	 * Mask the event channel port so we don't receive spurious events
> +	 * while changing affinity.
> +	 */
> +	masked = evtchn_test_and_set_mask(isrc->xi_port);
> +
>  	if ((isrc->xi_type == EVTCHN_TYPE_VIRQ) ||
>  		(isrc->xi_type == EVTCHN_TYPE_IPI)) {
>  		/*
> @@ -825,8 +831,7 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
>  		evtchn_cpu_mask_port(isrc->xi_cpu, isrc->xi_port);
>  		isrc->xi_cpu = to_cpu;
>  		evtchn_cpu_unmask_port(isrc->xi_cpu, isrc->xi_port);
> -		mtx_unlock(&xen_intr_isrc_lock);
> -		return (0);
> +		goto out;
>  	}
>  
>  	bind_vcpu.port = isrc->xi_port;
> @@ -848,6 +853,10 @@ xen_intr_assign_cpu(struct intsrc *base_isrc, u_int apic_id)
>  			evtchn_cpu_mask_port(to_cpu, isrc->xi_port);
>  		}
>  	}
> +
> +out:
> +	if (!masked)
> +		evtchn_unmask_port(isrc->xi_port);
>  	mtx_unlock(&xen_intr_isrc_lock);
>  	return (0);
>  #else
> -- 
> 1.7.7.5 (Apple Git-26)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH RFC 01/13] xen: use the hardware e820 map on Dom0

[John Baldwin]

On Tuesday, December 24, 2013 6:20:50 am Roger Pau Monne wrote:
> We need to do some tweaking of the hardware e820 map, since the memory
> layout provided by Xen and the e820 map doesn't match.
> 
> This consists in clamping the e820 map so that regions above max_pfn
> are marked as unusuable.
> ---
>  sys/x86/xen/pv.c |   35 +++++++++++++++++++++++++++++++++--
>  1 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
> index 4f7415e..ab4afba 100644
> --- a/sys/x86/xen/pv.c
> +++ b/sys/x86/xen/pv.c
> @@ -297,17 +297,48 @@ static void
>  xen_pv_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx)
>  {
>  	struct xen_memory_map memmap;
> +	unsigned long max_pfn = HYPERVISOR_start_info->nr_pages;
> +	u_int64_t mem_end = ptoa(max_pfn);
>  	u_int32_t size;
> -	int rc;
> +	int rc, mem_op, i;

One minor nit is that it is preferred to not initalize variables in 
declarations style-wise.  Aside from that, this looks fine to me.

>  	/* Fetch the E820 map from Xen */
>  	memmap.nr_entries = MAX_E820_ENTRIES;
>  	set_xen_guest_handle(memmap.buffer, xen_smap);
> -	rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
> +	mem_op = xen_initial_domain() ?
> +	                XENMEM_machine_memory_map :
> +	                XENMEM_memory_map;
> +	rc = HYPERVISOR_memory_op(mem_op, &memmap);
>  	if (rc)
>  		panic("unable to fetch Xen E820 memory map");
>  	size = memmap.nr_entries * sizeof(xen_smap[0]);
>  
> +	/*
> +	 * This should be improved, Xen provides us with a single
> +	 * chunk of physical memory that goes from 0 to max_pfn,
> +	 * and what we do here is clamp the HW memory map to make
> +	 * sure regions above max_pfn are marked as reserved.
> +	 *
> +	 * TRTTD would be to move the memory not used because of
> +	 * the holes in the HW memory map to the now clamped regions
> +	 * using XENMEM_{decrease/increase}_reservation.
> +	 */
> +	for (i = 0; i < memmap.nr_entries; i++) {
> +		u_int64_t end = xen_smap[i].base + xen_smap[i].length;
> +		if (xen_smap[i].type == SMAP_TYPE_MEMORY) {
> +			if (xen_smap[i].base > mem_end) {
> +				/* Mark as reserved */
> +				xen_smap[i].type = SMAP_TYPE_RESERVED;
> +				continue;
> +			}
> +			if (end > mem_end) {
> +				/* Truncate region */
> +				xen_smap[i].length -= end - mem_end;
> +			}
> +		}
> +	}
> +
> +
>  	bios_add_smap_entries(xen_smap, size, physmap, physmap_idx);
>  }
>  
> -- 
> 1.7.7.5 (Apple Git-26)
> 
> 

-- 
John Baldwin

Re: [PATCH RFC 02/13] ioapic: introduce hooks for some ioapic ops

[John Baldwin]

On Tuesday, December 24, 2013 6:20:51 am Roger Pau Monne wrote:
> Create some hooks for IO APIC operations that will diverge from bare
> metal when implemented for Xen Dom0.
> 
> This patch should not introduce any changes in functionality, it's a
> preparatory patch for the implementation of the Xen IO APIC hooks.

I think this is fine.  I should really create a sys/x86/include/apicvar.h
as there is a lot shared between those two headers.

-- 
John Baldwin

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

[John Baldwin]

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
> @@ -49,6 +49,7 @@ __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>
> @@ -58,6 +59,7 @@ __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>
>  
> @@ -73,6 +75,11 @@ 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;
> @@ -92,6 +99,13 @@ 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;
> @@ -342,6 +356,34 @@ 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)
>  {
> @@ -349,4 +391,6 @@ 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

Re: [PATCH RFC 06/13] xen: Dom0 console fixes

[John Baldwin]

On Tuesday, December 24, 2013 6:20:55 am Roger Pau Monne wrote:
> Minor fixes and workarounds to make the Xen Dom0 console work.

Looks ok to me.

-- 
John Baldwin

Re: [PATCH RFC 07/13] xen: implement IO APIC support in Xen mptable parser

[John Baldwin]

On Tuesday, December 24, 2013 12:20:56 PM Roger Pau Monne wrote:
> Use madt_setup_io (from madt.c) on Xen apic_enumerator, in order to
> parse the interrupt sources from the IO APIC.
> 
> I would like to get opinions, but I think we should rename and move
> madt_setup_io to io_apic.c.

It wouldn't be appropriate for io_apic.c as it isn't generic to I/O
APICs but is specific to ACPI.  However, mptable.c is really not a
great name for this file in sys/x86/xen.  I wonder if it should be
xen_apic.c instead?  Also, if Xen PV has an MADT table, why do you
need a custom APIC enumerator at all?  That is, what is preventing
the code in madt.c from just working?  Do you just not have
'device acpi' in the kernel config you are using?

-- 
John Baldwin

Re: [PATCH RFC 08/13] xen: change order of Xen intr init and IO APIC registration

[John Baldwin]

On Tuesday, December 24, 2013 12:20:57 PM Roger Pau Monne wrote:
> Change order of some of the services in the SI_SUB_INTR stage, so
> that it follows the order:
> 
> - System intr initialization
> - Xen intr initalization
> - IO APIC source registration

This is ok.

-- 
John Baldwin

Re: [PATCH RFC 10/13] xen: add ACPI bus to xen_nexus when running as Dom0

[John Baldwin]

On Tuesday, December 24, 2013 12:20:59 PM Roger Pau Monne wrote:
> Also disable a couple of ACPI devices that are not usable under Dom0.

Hmm, setting debug.acpi.disabled in this way is a bit hacky.  It might
be fine however if there's no way for the user to set it before booting
the kernel (as opposed to haing the relevant drivers explicitly disable
themselves under Xen which I think would be cleaner, but would also
make your patch larger)

-- 
John Baldwin

Re: [PATCH RFC 10/13] xen: add ACPI bus to xen_nexus when running as Dom0

[Roger Pau Monné]

On 08/02/14 22:50, John Baldwin wrote:
> On Tuesday, December 24, 2013 12:20:59 PM Roger Pau Monne wrote:
>> Also disable a couple of ACPI devices that are not usable under Dom0.
> 
> Hmm, setting debug.acpi.disabled in this way is a bit hacky.  It might
> be fine however if there's no way for the user to set it before booting
> the kernel (as opposed to haing the relevant drivers explicitly disable
> themselves under Xen which I think would be cleaner, but would also
> make your patch larger)

Thanks for the review, the user can pass parameters to FreeBSD when
booted as Dom0, I just find it uncomfortable to force the user into
always setting something on the command line in order to boot.

What do you mean with "haing the relevant drivers explicitly disable
themselves under Xen"? Adding a gate on every one of those devices like
"if (xen_pv_domain()) return (ENXIO);" in the identify/probe routine
seems even worse.

Roger.

Re: [PATCH RFC 10/13] xen: add ACPI bus to xen_nexus when running as Dom0

[John Baldwin]

On Friday, February 14, 2014 5:38:15 am Roger Pau Monné wrote:
> On 08/02/14 22:50, John Baldwin wrote:
> > On Tuesday, December 24, 2013 12:20:59 PM Roger Pau Monne wrote:
> >> Also disable a couple of ACPI devices that are not usable under Dom0.
> > 
> > Hmm, setting debug.acpi.disabled in this way is a bit hacky.  It might
> > be fine however if there's no way for the user to set it before booting
> > the kernel (as opposed to haing the relevant drivers explicitly disable
> > themselves under Xen which I think would be cleaner, but would also
> > make your patch larger)
> 
> Thanks for the review, the user can pass parameters to FreeBSD when
> booted as Dom0, I just find it uncomfortable to force the user into
> always setting something on the command line in order to boot.

Can the user set debug.acpi.disabled?  If so, you are overriding their
setting which would be bad.

> What do you mean with "haing the relevant drivers explicitly disable
> themselves under Xen"? Adding a gate on every one of those devices like
> "if (xen_pv_domain()) return (ENXIO);" in the identify/probe routine
> seems even worse.

A check like this in probe() is what I had in mind, though I agree it's
not perfect.

-- 
John Baldwin

Re: [PATCH RFC 11/13] pci: introduce a new event on PCI device detection

[John Baldwin]

On Tuesday, December 24, 2013 12:21:00 PM Roger Pau Monne wrote:
> Add a new event that will fire each time a PCI device is added to the
> system, and allows us to register the device with Xen.

It's really hackish to make this PCI specific.  OTOH, I can't think of a
good place to have a more generic new-bus callback.  You could make the
eventhandler pass the 'device_t' instead of the dinfo.  The dinfo isn't
really a public structure, and since the device_t's ivars are already set
you can use things like 'pci_get_domain()' and 'pci_get_bus()' of the
passed in device in your callback function.

> ---
>  sys/dev/pci/pci.c       |    1 +
>  sys/sys/eventhandler.h  |    5 +++++
>  sys/x86/xen/pv.c        |   21 +++++++++++++++++++++
>  sys/x86/xen/xen_nexus.c |    6 ++++++
>  sys/xen/pv.h            |    1 +
>  5 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c
> index 4d8837f..2ee5093 100644
> --- a/sys/dev/pci/pci.c
> +++ b/sys/dev/pci/pci.c
> @@ -3293,6 +3293,7 @@ pci_add_child(device_t bus, struct pci_devinfo *dinfo)
> resource_list_init(&dinfo->resources);
>  	pci_cfg_save(dinfo->cfg.dev, dinfo, 0);
>  	pci_cfg_restore(dinfo->cfg.dev, dinfo);
> +	EVENTHANDLER_INVOKE(pci_add, dinfo);
>  	pci_print_verbose(dinfo);
>  	pci_add_resources(bus, dinfo->cfg.dev, 0, 0);
>  }
> diff --git a/sys/sys/eventhandler.h b/sys/sys/eventhandler.h
> index 111c21b..3201848 100644
> --- a/sys/sys/eventhandler.h
> +++ b/sys/sys/eventhandler.h
> @@ -269,5 +269,10 @@ typedef void (*unregister_framebuffer_fn)(void *,
> struct fb_info *); EVENTHANDLER_DECLARE(register_framebuffer,
> register_framebuffer_fn); EVENTHANDLER_DECLARE(unregister_framebuffer,
> unregister_framebuffer_fn);
> 
> +/* PCI events */
> +struct pci_devinfo;
> +typedef void (*pci_add_fn)(void *, struct pci_devinfo *);
> +EVENTHANDLER_DECLARE(pci_add, pci_add_fn);
> +
>  #endif /* _SYS_EVENTHANDLER_H_ */
> 
> diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
> index e5ad200..a44f8ca 100644
> --- a/sys/x86/xen/pv.c
> +++ b/sys/x86/xen/pv.c
> @@ -39,6 +39,9 @@ __FBSDID("$FreeBSD$");
>  #include <sys/rwlock.h>
>  #include <sys/mutex.h>
>  #include <sys/smp.h>
> +#include <sys/reboot.h>
> +#include <sys/pciio.h>
> +#include <sys/eventhandler.h>
> 
>  #include <vm/vm.h>
>  #include <vm/vm_extern.h>
> @@ -63,6 +66,8 @@ __FBSDID("$FreeBSD$");
> 
>  #include <xen/interface/vcpu.h>
> 
> +#include <dev/pci/pcivar.h>
> +
>  /* Native initial function */
>  extern u_int64_t hammer_time(u_int64_t, u_int64_t);
>  /* Xen initial function */
> @@ -384,6 +389,22 @@ xen_pv_ioapic_register_intr(struct ioapic_intsrc *pin)
>  	xen_register_pirq(pin->io_irq, pin->io_activehi, pin->io_edgetrigger);
>  }
> 
> +void
> +xen_pv_pci_device_add(void *arg, struct pci_devinfo *dinfo)
> +{
> +	struct physdev_pci_device_add add_pci;
> +	int error;
> +
> +	bzero(&add_pci, sizeof(add_pci));
> +	add_pci.seg = dinfo->cfg.domain;
> +	add_pci.bus = dinfo->cfg.bus;
> +	add_pci.devfn = (dinfo->cfg.slot << 3) | dinfo->cfg.func;
> +	error = HYPERVISOR_physdev_op(PHYSDEVOP_pci_device_add, &add_pci);
> +	if (error)
> +		printf("unable to add device bus %u devfn %u error: %d\n",
> +		       add_pci.bus, add_pci.devfn, error);
> +}
> +
>  static void
>  xen_pv_set_init_ops(void)
>  {
> diff --git a/sys/x86/xen/xen_nexus.c b/sys/x86/xen/xen_nexus.c
> index 823b3bc..60c6c5d 100644
> --- a/sys/x86/xen/xen_nexus.c
> +++ b/sys/x86/xen/xen_nexus.c
> @@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$");
>  #include <sys/sysctl.h>
>  #include <sys/systm.h>
>  #include <sys/smp.h>
> +#include <sys/eventhandler.h>
> 
>  #include <contrib/dev/acpica/include/acpi.h>
> 
> @@ -42,6 +43,7 @@ __FBSDID("$FreeBSD$");
>  #include <machine/nexusvar.h>
> 
>  #include <xen/xen-os.h>
> +#include <xen/pv.h>
> 
>  static const char *xen_devices[] =
>  {
> @@ -87,6 +89,10 @@ nexus_xen_attach(device_t dev)
>  		/* Disable some ACPI devices that are not usable by Dom0 */
>  		setenv("debug.acpi.disabled", "cpu hpet timer");
> 
> +		/* Register PCI add hook */
> +		EVENTHANDLER_REGISTER(pci_add, xen_pv_pci_device_add, NULL,
> +		                      EVENTHANDLER_PRI_FIRST);
> +
>  		acpi_dev = BUS_ADD_CHILD(dev, 10, "acpi", 0);
>  		if (acpi_dev == NULL)
>  			panic("Unable to add ACPI bus to Xen Dom0");
> diff --git a/sys/xen/pv.h b/sys/xen/pv.h
> index a9d6eb0..ac737a7 100644
> --- a/sys/xen/pv.h
> +++ b/sys/xen/pv.h
> @@ -24,5 +24,6 @@
>  #define	__XEN_PV_H__
> 
>  int	xen_pv_start_all_aps(void);
> +void	xen_pv_pci_device_add(void *, struct pci_devinfo *);
> 
>  #endif	/* __XEN_PV_H__ */

-- 
John Baldwin

Re: [PATCH RFC 12/13] mca: disable cmc enable on Xen PV

[John Baldwin]

On Tuesday, December 24, 2013 12:21:01 PM Roger Pau Monne wrote:
> Xen PV guests doesn't have a lapic, so disable the lapic call in mca
> initialization.

I think this is fine, but I wonder if it wouldn't be cleaner to have 
lapic_enable_cmc() do the check instead.  Where else do you check 
lapic_disabled?

> ---
>  sys/x86/x86/mca.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sys/x86/x86/mca.c b/sys/x86/x86/mca.c
> index f1369cd..e9d2c1d 100644
> --- a/sys/x86/x86/mca.c
> +++ b/sys/x86/x86/mca.c
> @@ -897,7 +897,7 @@ _mca_init(int boot)
>  		}
> 
>  #ifdef DEV_APIC
> -		if (PCPU_GET(cmci_mask) != 0 && boot)
> +		if (PCPU_GET(cmci_mask) != 0 && boot && !lapic_disabled)
>  			lapic_enable_cmc();
>  #endif
>  	}

-- 
John Baldwin