@@ -92,7 +92,7 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
s->count_shift = (v + 1) & 7;
s->initial_count_load_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- apic_next_timer(s, s->initial_count_load_time);
+ apic_next_timer(s, s->initial_count_load_time, false);
}
static void kvm_apic_set_base(APICCommonState *s, uint64_t val)
@@ -619,9 +619,10 @@ int apic_accept_pic_intr(DeviceState *dev)
return 0;
}
-static void apic_timer_update(APICCommonState *s, int64_t current_time)
+static void apic_timer_update(APICCommonState *s, int64_t current_time,
+ bool switch_to_periodic)
{
- if (apic_next_timer(s, current_time)) {
+ if (apic_next_timer(s, current_time, switch_to_periodic)) {
timer_mod(s->timer, s->next_time);
} else {
timer_del(s->timer);
@@ -633,7 +634,7 @@ static void apic_timer(void *opaque)
APICCommonState *s = opaque;
apic_local_deliver(s, APIC_LVT_TIMER);
- apic_timer_update(s, s->next_time);
+ apic_timer_update(s, s->next_time, false);
}
static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
@@ -814,18 +815,38 @@ static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
case 0x32 ... 0x37:
{
int n = index - 0x32;
- s->lvt[n] = val;
if (n == APIC_LVT_TIMER) {
- apic_timer_update(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
- } else if (n == APIC_LVT_LINT0 && apic_check_pic(s)) {
- apic_update_irq(s);
+ uint32_t old_val = s->lvt[n];
+
+ /* Check if we switch from one-shot to periodic mode */
+ if (!(old_val & APIC_LVT_TIMER_PERIODIC) &&
+ (val & APIC_LVT_TIMER_PERIODIC)) {
+ apic_timer_update(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+ true);
+ } else {
+ apic_timer_update(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+ false);
+ }
+ /*
+ * Set the lvt timer entry after we handle the switch mode
+ * so that a concurrent apic_timer_update triggered via
+ * apic_timer does not see the new value before we handle
+ * the switch properly.
+ */
+ s->lvt[n] = val;
+ } else {
+ s->lvt[n] = val;
+ if (n == APIC_LVT_LINT0 && apic_check_pic(s)) {
+ apic_update_irq(s);
+ }
}
}
break;
case 0x38:
s->initial_count = val;
s->initial_count_load_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- apic_timer_update(s, s->initial_count_load_time);
+ s->timer_stop = false;
+ apic_timer_update(s, s->initial_count_load_time, false);
break;
case 0x39:
break;
@@ -130,7 +130,8 @@ void apic_deliver_nmi(DeviceState *dev)
info->external_nmi(s);
}
-bool apic_next_timer(APICCommonState *s, int64_t current_time)
+bool apic_next_timer(APICCommonState *s, int64_t current_time,
+ bool switch_to_periodic)
{
int64_t d;
@@ -146,10 +147,24 @@ bool apic_next_timer(APICCommonState *s, int64_t current_time)
d = (current_time - s->initial_count_load_time) >> s->count_shift;
+ /*
+ * The switch_to_periodic is true only when we write to the
+ * timer lvt entry to change timer mode from one-shot mode
+ * to periodic mode. In that case, we need to check if the
+ * timer current count reaches 0. If so, don't start the new
+ * timer, only start the time when there is a write to
+ * initial count.
+ */
+ if (switch_to_periodic && d >= s->initial_count) {
+ s->timer_stop = true;
+ return false;
+ }
+
if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
if (!s->initial_count) {
return false;
}
+
d = ((d / ((uint64_t)s->initial_count + 1)) + 1) *
((uint64_t)s->initial_count + 1);
} else {
@@ -167,6 +182,11 @@ uint32_t apic_get_current_count(APICCommonState *s)
{
int64_t d;
uint32_t val;
+
+ if (s->timer_stop) {
+ return 0;
+ }
+
d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->initial_count_load_time) >>
s->count_shift;
if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
@@ -282,6 +302,7 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
if (s->legacy_instance_id) {
instance_id = VMSTATE_INSTANCE_ID_ANY;
}
+ s->timer_stop = false;
vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
s, -1, 0, NULL);
}
@@ -309,6 +330,7 @@ static int apic_pre_load(void *opaque)
* absent.
*/
s->wait_for_sipi = 0;
+ s->timer_stop = false;
return 0;
}
@@ -353,6 +375,22 @@ static const VMStateDescription vmstate_apic_common_sipi = {
}
};
+static bool apic_common_timer_stop_needed(void *opaque)
+{
+ APICCommonState *s = APIC_COMMON(opaque);
+ return s->timer_stop != 0;
+}
+
+static const VMStateDescription vmstate_apic_common_timer_stop = {
+ .name = "apic_timer_stop",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = apic_common_timer_stop_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_BOOL(timer_stop, APICCommonState),
+ VMSTATE_END_OF_LIST()
+ }
+};
static const VMStateDescription vmstate_apic_common = {
.name = "apic",
.version_id = 3,
@@ -385,6 +423,7 @@ static const VMStateDescription vmstate_apic_common = {
},
.subsections = (const VMStateDescription*[]) {
&vmstate_apic_common_sipi,
+ &vmstate_apic_common_timer_stop,
NULL
}
};
@@ -187,6 +187,8 @@ struct APICCommonState {
DeviceState *vapic;
hwaddr vapic_paddr; /* note: persistence via kvmvapic */
bool legacy_instance_id;
+
+ bool timer_stop;
};
typedef struct VAPICState {
@@ -199,7 +201,8 @@ typedef struct VAPICState {
extern bool apic_report_tpr_access;
-bool apic_next_timer(APICCommonState *s, int64_t current_time);
+bool apic_next_timer(APICCommonState *s, int64_t current_time,
+ bool switch_to_periodic);
void apic_enable_tpr_access_reporting(DeviceState *d, bool enable);
void apic_enable_vapic(DeviceState *d, hwaddr paddr);
When running kvm-unit-tests[1] on current APIC[2], we get a failed test case related to APIC timer env QEMU=build/qemu-system-x86_64 ACCEL=tcg ./run_tests.sh -g apic FAIL: TMCCT should stay at zero The test case sets the timer mode to oneshot and sets the intial count, it waits until the current count reaches 0 then change the mode to periodic. It is expected that the timer does not start again, the current count must stay at 0. However, in the current implementation, the write to lvt timer entry to change to periodic mode triggers the new periodic timer. This commit adds an additional check when the write to lvt timer entry to change from oneshot to periodic mode happens. This check verifies if the current count reaches 0 in oneshot mode already, then it does not start a new timer and sets timer_stop bool flag to true. The apic_get_current_count uses this bool flag to report the correct current count. [1]: This patch is applied to kvm-unit-tests before running test diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c index 1734afb..f56fe1c 100644 --- a/lib/x86/fwcfg.c +++ b/lib/x86/fwcfg.c @@ -27,6 +27,7 @@ static void read_cfg_override(void) if ((str = getenv("TEST_DEVICE"))) no_test_device = !atol(str); + no_test_device = true; if ((str = getenv("MEMLIMIT"))) fw_override[FW_CFG_MAX_RAM] = atol(str) * 1024 * 1024; [2]: We need to disable the APIC disable before running kvm-unit-tests because the test try to disable and re-enable APIC which does not follow the xAPIC disable rule in Intel SDM Section 11.4.3. Enabling or Disabling the local APIC "When IA32_APIC_BASE[11] is set to 0, processor APICs based on the 3-wire APIC bus cannot be generally re-enabled until a system hardware reset" So the current implementation does not support disable and re-enable local APIC in xAPIC. We need to apply this patch to run the test diff --git a/hw/intc/apic.c b/hw/intc/apic.c index ec0a20da59..5c4e0ee3bd 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -293,11 +293,13 @@ static void apic_set_base(APICCommonState *s, uint64_t val) s->apicbase = (val & 0xfffff000) | (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE)); /* if disabled, cannot be enabled again */ + /* if (!(val & MSR_IA32_APICBASE_ENABLE)) { s->apicbase &= ~MSR_IA32_APICBASE_ENABLE; cpu_clear_apic_feature(&s->cpu->env); s->spurious_vec &= ~APIC_SV_ENABLE; } + */ } Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com> --- hw/i386/kvm/apic.c | 2 +- hw/intc/apic.c | 37 ++++++++++++++++++++++------- hw/intc/apic_common.c | 41 ++++++++++++++++++++++++++++++++- include/hw/i386/apic_internal.h | 5 +++- 4 files changed, 74 insertions(+), 11 deletions(-)