Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 29 Aug 2019 18:23:38 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r351609 - head/sys/amd64/vmm/io
Message-ID:  <201908291823.x7TINcJC054017@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jhb
Date: Thu Aug 29 18:23:38 2019
New Revision: 351609
URL: https://svnweb.freebsd.org/changeset/base/351609

Log:
  Simplify bhyve vlapic ESR logic.
  
  The bhyve virtual local APIC uses an instance-global flag to indicate
  when an error LVT is being delivered to prevent infinite recursion.
  Use a function argument instead to reduce the amount of instance-global
  state.
  
  This was inspired by reviewing the bhyve save/restore work, which
  saves a copy of the instance-global state for each vlapic.
  
  Smart OS bug:	https://smartos.org/bugview/OS-7777
  Submitted by:	Patrick Mooney
  Reviewed by:	markj, rgrimes
  Obtained from:	SmartOS / Joyent
  Differential Revision:	https://reviews.freebsd.org/D20365

Modified:
  head/sys/amd64/vmm/io/vlapic.c
  head/sys/amd64/vmm/io/vlapic.h
  head/sys/amd64/vmm/io/vlapic_priv.h

Modified: head/sys/amd64/vmm/io/vlapic.c
==============================================================================
--- head/sys/amd64/vmm/io/vlapic.c	Thu Aug 29 17:25:50 2019	(r351608)
+++ head/sys/amd64/vmm/io/vlapic.c	Thu Aug 29 18:23:38 2019	(r351609)
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2011 NetApp, Inc.
  * All rights reserved.
+ * Copyright (c) 2019 Joyent, Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -78,6 +79,8 @@ __FBSDID("$FreeBSD$");
  */
 #define VLAPIC_BUS_FREQ		(128 * 1024 * 1024)
 
+static void vlapic_set_error(struct vlapic *, uint32_t, bool);
+
 static __inline uint32_t
 vlapic_get_id(struct vlapic *vlapic)
 {
@@ -275,7 +278,8 @@ vlapic_set_intr_ready(struct vlapic *vlapic, int vecto
 	}
 
 	if (vector < 16) {
-		vlapic_set_error(vlapic, APIC_ESR_RECEIVE_ILLEGAL_VECTOR);
+		vlapic_set_error(vlapic, APIC_ESR_RECEIVE_ILLEGAL_VECTOR,
+		    false);
 		VLAPIC_CTR1(vlapic, "vlapic ignoring interrupt to vector %d",
 		    vector);
 		return (1);
@@ -432,20 +436,22 @@ vlapic_mask_lvts(struct vlapic *vlapic)
 }
 
 static int
-vlapic_fire_lvt(struct vlapic *vlapic, uint32_t lvt)
+vlapic_fire_lvt(struct vlapic *vlapic, u_int lvt)
 {
-	uint32_t vec, mode;
+	uint32_t mode, reg, vec;
 
-	if (lvt & APIC_LVT_M)
+	reg = atomic_load_acq_32(&vlapic->lvt_last[lvt]);
+
+	if (reg & APIC_LVT_M)
 		return (0);
+	vec = reg & APIC_LVT_VECTOR;
+	mode = reg & APIC_LVT_DM;
 
-	vec = lvt & APIC_LVT_VECTOR;
-	mode = lvt & APIC_LVT_DM;
-
 	switch (mode) {
 	case APIC_LVT_DM_FIXED:
 		if (vec < 16) {
-			vlapic_set_error(vlapic, APIC_ESR_SEND_ILLEGAL_VECTOR);
+			vlapic_set_error(vlapic, APIC_ESR_SEND_ILLEGAL_VECTOR,
+			    lvt == APIC_LVT_ERROR);
 			return (0);
 		}
 		if (vlapic_set_intr_ready(vlapic, vec, false))
@@ -606,22 +612,22 @@ vlapic_periodic_timer(struct vlapic *vlapic)
 
 static VMM_STAT(VLAPIC_INTR_ERROR, "error interrupts generated by vlapic");
 
-void
-vlapic_set_error(struct vlapic *vlapic, uint32_t mask)
+static void
+vlapic_set_error(struct vlapic *vlapic, uint32_t mask, bool lvt_error)
 {
-	uint32_t lvt;
 
 	vlapic->esr_pending |= mask;
-	if (vlapic->esr_firing)
+
+	/*
+	 * Avoid infinite recursion if the error LVT itself is configured with
+	 * an illegal vector.
+	 */
+	if (lvt_error)
 		return;
-	vlapic->esr_firing = 1;
 
-	// The error LVT always uses the fixed delivery mode.
-	lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_ERROR_LVT);
-	if (vlapic_fire_lvt(vlapic, lvt | APIC_LVT_DM_FIXED)) {
+	if (vlapic_fire_lvt(vlapic, APIC_LVT_ERROR)) {
 		vmm_stat_incr(vlapic->vm, vlapic->vcpuid, VLAPIC_INTR_ERROR, 1);
 	}
-	vlapic->esr_firing = 0;
 }
 
 static VMM_STAT(VLAPIC_INTR_TIMER, "timer interrupts generated by vlapic");
@@ -629,13 +635,10 @@ static VMM_STAT(VLAPIC_INTR_TIMER, "timer interrupts g
 static void
 vlapic_fire_timer(struct vlapic *vlapic)
 {
-	uint32_t lvt;
 
 	KASSERT(VLAPIC_TIMER_LOCKED(vlapic), ("vlapic_fire_timer not locked"));
-	
-	// The timer LVT always uses the fixed delivery mode.
-	lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_TIMER_LVT);
-	if (vlapic_fire_lvt(vlapic, lvt | APIC_LVT_DM_FIXED)) {
+
+	if (vlapic_fire_lvt(vlapic, APIC_LVT_TIMER)) {
 		VLAPIC_CTR0(vlapic, "vlapic timer fired");
 		vmm_stat_incr(vlapic->vm, vlapic->vcpuid, VLAPIC_INTR_TIMER, 1);
 	}
@@ -647,10 +650,8 @@ static VMM_STAT(VLAPIC_INTR_CMC,
 void
 vlapic_fire_cmci(struct vlapic *vlapic)
 {
-	uint32_t lvt;
 
-	lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_CMCI_LVT);
-	if (vlapic_fire_lvt(vlapic, lvt)) {
+	if (vlapic_fire_lvt(vlapic, APIC_LVT_CMCI)) {
 		vmm_stat_incr(vlapic->vm, vlapic->vcpuid, VLAPIC_INTR_CMC, 1);
 	}
 }
@@ -661,7 +662,6 @@ static VMM_STAT_ARRAY(LVTS_TRIGGERRED, VLAPIC_MAXLVT_I
 int
 vlapic_trigger_lvt(struct vlapic *vlapic, int vector)
 {
-	uint32_t lvt;
 
 	if (vlapic_enabled(vlapic) == false) {
 		/*
@@ -684,35 +684,20 @@ vlapic_trigger_lvt(struct vlapic *vlapic, int vector)
 
 	switch (vector) {
 	case APIC_LVT_LINT0:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_LINT0_LVT);
-		break;
 	case APIC_LVT_LINT1:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_LINT1_LVT);
-		break;
 	case APIC_LVT_TIMER:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_TIMER_LVT);
-		lvt |= APIC_LVT_DM_FIXED;
-		break;
 	case APIC_LVT_ERROR:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_ERROR_LVT);
-		lvt |= APIC_LVT_DM_FIXED;
-		break;
 	case APIC_LVT_PMC:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_PERF_LVT);
-		break;
 	case APIC_LVT_THERMAL:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_THERM_LVT);
-		break;
 	case APIC_LVT_CMCI:
-		lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_CMCI_LVT);
+		if (vlapic_fire_lvt(vlapic, vector)) {
+			vmm_stat_array_incr(vlapic->vm, vlapic->vcpuid,
+			    LVTS_TRIGGERRED, vector, 1);
+		}
 		break;
 	default:
 		return (EINVAL);
 	}
-	if (vlapic_fire_lvt(vlapic, lvt)) {
-		vmm_stat_array_incr(vlapic->vm, vlapic->vcpuid,
-		    LVTS_TRIGGERRED, vector, 1);
-	}
 	return (0);
 }
 
@@ -980,7 +965,7 @@ vlapic_icrlo_write_handler(struct vlapic *vlapic, bool
 	mode = icrval & APIC_DELMODE_MASK;
 
 	if (mode == APIC_DELMODE_FIXED && vec < 16) {
-		vlapic_set_error(vlapic, APIC_ESR_SEND_ILLEGAL_VECTOR);
+		vlapic_set_error(vlapic, APIC_ESR_SEND_ILLEGAL_VECTOR, false);
 		VLAPIC_CTR1(vlapic, "Ignoring invalid IPI %d", vec);
 		return (0);
 	}

Modified: head/sys/amd64/vmm/io/vlapic.h
==============================================================================
--- head/sys/amd64/vmm/io/vlapic.h	Thu Aug 29 17:25:50 2019	(r351608)
+++ head/sys/amd64/vmm/io/vlapic.h	Thu Aug 29 18:23:38 2019	(r351609)
@@ -71,7 +71,6 @@ int vlapic_set_intr_ready(struct vlapic *vlapic, int v
  */
 void vlapic_post_intr(struct vlapic *vlapic, int hostcpu, int ipinum);
 
-void vlapic_set_error(struct vlapic *vlapic, uint32_t mask);
 void vlapic_fire_cmci(struct vlapic *vlapic);
 int vlapic_trigger_lvt(struct vlapic *vlapic, int vector);
 

Modified: head/sys/amd64/vmm/io/vlapic_priv.h
==============================================================================
--- head/sys/amd64/vmm/io/vlapic_priv.h	Thu Aug 29 17:25:50 2019	(r351608)
+++ head/sys/amd64/vmm/io/vlapic_priv.h	Thu Aug 29 18:23:38 2019	(r351609)
@@ -156,7 +156,6 @@ struct vlapic {
 	struct vlapic_ops	ops;
 
 	uint32_t		esr_pending;
-	int			esr_firing;
 
 	struct callout	callout;	/* vlapic timer */
 	struct bintime	timer_fire_bt;	/* callout expiry time */

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201908291823.x7TINcJC054017>