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

diff --git a/sys/amd64/vmm/io/vlapic.c b/sys/amd64/vmm/io/vlapic.c
index b3aa824c955d00b1b904931100bfaddc57898be9..74e6cd9673963ad7c2efe21f38bb16f40a47ad8d 100644 (file)
--- a/sys/amd64/vmm/io/vlapic.c
+++ b/sys/amd64/vmm/io/vlapic.c
@@ -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 vector, bool level)
        }
 
        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)
-               return (0);
+       reg = atomic_load_acq_32(&vlapic->lvt_last[lvt]);
 
-       vec = lvt & APIC_LVT_VECTOR;
-       mode = lvt & APIC_LVT_DM;
+       if (reg & APIC_LVT_M)
+               return (0);
+       vec = reg & APIC_LVT_VECTOR;
+       mode = reg & 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 generated by vlapic");
 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_INDEX + 1,
 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 *retu)
        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);
        }

diff --git a/sys/amd64/vmm/io/vlapic.h b/sys/amd64/vmm/io/vlapic.h
index b2d5db7318f7e5e2c074ae5c3b3dc9908cde78ef..2a5f54003253479cada3625c35a4879734557eb5 100644 (file)
--- a/sys/amd64/vmm/io/vlapic.h
+++ b/sys/amd64/vmm/io/vlapic.h
@@ -71,7 +71,6 @@ int vlapic_set_intr_ready(struct vlapic *vlapic, int vector, bool level);
  */
 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);
 

diff --git a/sys/amd64/vmm/io/vlapic_priv.h b/sys/amd64/vmm/io/vlapic_priv.h
index 43abd20a0a90c2ed160cbaa6a82e046b620071de..fe7965cb65d724451ad1cebd459127d55449f395 100644 (file)
--- a/sys/amd64/vmm/io/vlapic_priv.h
+++ b/sys/amd64/vmm/io/vlapic_priv.h
@@ -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 */