diff mbox

Revert "apic: save apic_delivered flag"

Message ID 20170322121344.5484-1-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini March 22, 2017, 12:13 p.m. UTC
This reverts commit 07bfa354772f2de67008dc66c201b627acff0106.
The global variable is only read as part of a

            apic_reset_irq_delivered();
            qemu_irq_raise(s->irq);
            if (!apic_get_irq_delivered()) {

sequence, so the value never matters at migration time.

Reported-by: Dr. David Alan Gilbert <dglibert@redhat.com>
Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic_common.c           | 33 ---------------------------------
 include/hw/i386/apic_internal.h |  2 --
 2 files changed, 35 deletions(-)

Comments

Peter Xu March 23, 2017, 7:26 a.m. UTC | #1
On Wed, Mar 22, 2017 at 01:13:44PM +0100, Paolo Bonzini wrote:
> This reverts commit 07bfa354772f2de67008dc66c201b627acff0106.
> The global variable is only read as part of a
> 
>             apic_reset_irq_delivered();
>             qemu_irq_raise(s->irq);
>             if (!apic_get_irq_delivered()) {
> 
> sequence, so the value never matters at migration time.
> 
> Reported-by: Dr. David Alan Gilbert <dglibert@redhat.com>
> Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- peterx
Pavel Dovgalyuk March 23, 2017, 7:34 a.m. UTC | #2
This value is used by mc146818rtc.
Therefore it affects the vitrual machine state.
I've encountered the cases when replay was broken without migrating of this variable.

⁣Отправлено с помощью BlueMail ​

На 22 Мар 2017 г., 15:13, в 15:13, Paolo Bonzini <pbonzini@redhat.com> написал:>This reverts commit 07bfa354772f2de67008dc66c201b627acff0106.
>The global variable is only read as part of a
>
>            apic_reset_irq_delivered();
>            qemu_irq_raise(s->irq);
>            if (!apic_get_irq_delivered()) {
>
>sequence, so the value never matters at migration time.
>
>Reported-by: Dr. David Alan Gilbert <dglibert@redhat.com>
>Cc: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> hw/intc/apic_common.c           | 33 ---------------------------------
> include/hw/i386/apic_internal.h |  2 --
> 2 files changed, 35 deletions(-)
>
>diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>index 7a6e771..c3829e3 100644
>--- a/hw/intc/apic_common.c
>+++ b/hw/intc/apic_common.c
>@@ -387,25 +387,6 @@ static bool apic_common_sipi_needed(void *opaque)
>     return s->wait_for_sipi != 0;
> }
> 
>-static bool apic_irq_delivered_needed(void *opaque)
>-{
>-    APICCommonState *s = APIC_COMMON(opaque);
>-    return s->cpu == X86_CPU(first_cpu) && apic_irq_delivered != 0;
>-}
>-
>-static void apic_irq_delivered_pre_save(void *opaque)
>-{
>-    APICCommonState *s = APIC_COMMON(opaque);
>-    s->apic_irq_delivered = apic_irq_delivered;
>-}
>-
>-static int apic_irq_delivered_post_load(void *opaque, int version_id)
>-{
>-    APICCommonState *s = APIC_COMMON(opaque);
>-    apic_irq_delivered = s->apic_irq_delivered;
>-    return 0;
>-}
>-
> static const VMStateDescription vmstate_apic_common_sipi = {
>     .name = "apic_sipi",
>     .version_id = 1,
>@@ -418,19 +399,6 @@ static const VMStateDescription
>vmstate_apic_common_sipi = {
>     }
> };
> 
>-static const VMStateDescription vmstate_apic_irq_delivered = {
>-    .name = "apic_irq_delivered",
>-    .version_id = 1,
>-    .minimum_version_id = 1,
>-    .needed = apic_irq_delivered_needed,
>-    .pre_save = apic_irq_delivered_pre_save,
>-    .post_load = apic_irq_delivered_post_load,
>-    .fields = (VMStateField[]) {
>-        VMSTATE_INT32(apic_irq_delivered, APICCommonState),
>-        VMSTATE_END_OF_LIST()
>-    }
>-};
>-
> static const VMStateDescription vmstate_apic_common = {
>     .name = "apic",
>     .version_id = 3,
>@@ -465,7 +433,6 @@ static const VMStateDescription vmstate_apic_common
>= {
>     },
>     .subsections = (const VMStateDescription*[]) {
>         &vmstate_apic_common_sipi,
>-        &vmstate_apic_irq_delivered,
>         NULL
>     }
> };
>diff --git a/include/hw/i386/apic_internal.h
>b/include/hw/i386/apic_internal.h
>index 20ad28c..1209eb4 100644
>--- a/include/hw/i386/apic_internal.h
>+++ b/include/hw/i386/apic_internal.h
>@@ -189,8 +189,6 @@ struct APICCommonState {
>     DeviceState *vapic;
>     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
>     bool legacy_instance_id;
>-
>-    int apic_irq_delivered; /* for saving static variable */
> };
> 
> typedef struct VAPICState {
>-- 
>2.9.3
diff mbox

Patch

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 7a6e771..c3829e3 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -387,25 +387,6 @@  static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
 
-static bool apic_irq_delivered_needed(void *opaque)
-{
-    APICCommonState *s = APIC_COMMON(opaque);
-    return s->cpu == X86_CPU(first_cpu) && apic_irq_delivered != 0;
-}
-
-static void apic_irq_delivered_pre_save(void *opaque)
-{
-    APICCommonState *s = APIC_COMMON(opaque);
-    s->apic_irq_delivered = apic_irq_delivered;
-}
-
-static int apic_irq_delivered_post_load(void *opaque, int version_id)
-{
-    APICCommonState *s = APIC_COMMON(opaque);
-    apic_irq_delivered = s->apic_irq_delivered;
-    return 0;
-}
-
 static const VMStateDescription vmstate_apic_common_sipi = {
     .name = "apic_sipi",
     .version_id = 1,
@@ -418,19 +399,6 @@  static const VMStateDescription vmstate_apic_common_sipi = {
     }
 };
 
-static const VMStateDescription vmstate_apic_irq_delivered = {
-    .name = "apic_irq_delivered",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .needed = apic_irq_delivered_needed,
-    .pre_save = apic_irq_delivered_pre_save,
-    .post_load = apic_irq_delivered_post_load,
-    .fields = (VMStateField[]) {
-        VMSTATE_INT32(apic_irq_delivered, APICCommonState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static const VMStateDescription vmstate_apic_common = {
     .name = "apic",
     .version_id = 3,
@@ -465,7 +433,6 @@  static const VMStateDescription vmstate_apic_common = {
     },
     .subsections = (const VMStateDescription*[]) {
         &vmstate_apic_common_sipi,
-        &vmstate_apic_irq_delivered,
         NULL
     }
 };
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 20ad28c..1209eb4 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -189,8 +189,6 @@  struct APICCommonState {
     DeviceState *vapic;
     hwaddr vapic_paddr; /* note: persistence via kvmvapic */
     bool legacy_instance_id;
-
-    int apic_irq_delivered; /* for saving static variable */
 };
 
 typedef struct VAPICState {