diff mbox

[v8,3/9] apic: save apic_delivered flag

Message ID 20170126123429.5412.94368.stgit@PASHA-ISP (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Dovgalyuk Jan. 26, 2017, 12:34 p.m. UTC 
This patch implements saving/restoring of static apic_delivered variable.

v8: saving static variable only for one of the APICs

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 hw/intc/apic_common.c           |   37 +++++++++++++++++++++++++++++++++++++
 include/hw/i386/apic_internal.h |    2 ++
 2 files changed, 39 insertions(+)

Comments

Paolo Bonzini Jan. 26, 2017, 12:49 p.m. UTC | #1 
On 26/01/2017 13:34, Pavel Dovgalyuk wrote:
> This patch implements saving/restoring of static apic_delivered variable.
> 
> v8: saving static variable only for one of the APICs
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  hw/intc/apic_common.c           |   37 +++++++++++++++++++++++++++++++++++++
>  include/hw/i386/apic_internal.h |    2 ++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index d78c885..edacb16 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -384,6 +384,29 @@ static bool apic_common_sipi_needed(void *opaque)
>      return s->wait_for_sipi != 0;
>  }
>  
> +static bool apic_irq_delivered_needed(void *opaque)
> +{
> +    static APICCommonState *first_apic;
> +    APICCommonState *s = APIC_COMMON(opaque);
> +    if (!first_apic) {
> +        first_apic = s;
> +    }
> +    return s == first_apic;

Should also check " && apic_irq_delivered != 0".

Paolo

> +}
> +
> +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,
> @@ -396,6 +419,19 @@ 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,
> @@ -430,6 +466,7 @@ 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 1209eb4..20ad28c 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -189,6 +189,8 @@ 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 {
>
Pavel Dovgalyuk Jan. 26, 2017, 1:03 p.m. UTC | #2 
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 26/01/2017 13:34, Pavel Dovgalyuk wrote:
> > This patch implements saving/restoring of static apic_delivered variable.
> >
> > v8: saving static variable only for one of the APICs
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> >  hw/intc/apic_common.c           |   37 +++++++++++++++++++++++++++++++++++++
> >  include/hw/i386/apic_internal.h |    2 ++
> >  2 files changed, 39 insertions(+)
> >
> > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> > index d78c885..edacb16 100644
> > --- a/hw/intc/apic_common.c
> > +++ b/hw/intc/apic_common.c
> > @@ -384,6 +384,29 @@ static bool apic_common_sipi_needed(void *opaque)
> >      return s->wait_for_sipi != 0;
> >  }
> >
> > +static bool apic_irq_delivered_needed(void *opaque)
> > +{
> > +    static APICCommonState *first_apic;
> > +    APICCommonState *s = APIC_COMMON(opaque);
> > +    if (!first_apic) {
> > +        first_apic = s;
> > +    }
> > +    return s == first_apic;
> 
> Should also check " && apic_irq_delivered != 0".

Reset of this variable when machine reboots is also forgotten.

Pavel Dovgalyuk
Paolo Bonzini Jan. 26, 2017, 1:06 p.m. UTC | #3 
On 26/01/2017 14:03, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> On 26/01/2017 13:34, Pavel Dovgalyuk wrote:
>>> This patch implements saving/restoring of static apic_delivered variable.
>>>
>>> v8: saving static variable only for one of the APICs
>>>
>>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>>> ---
>>>  hw/intc/apic_common.c           |   37 +++++++++++++++++++++++++++++++++++++
>>>  include/hw/i386/apic_internal.h |    2 ++
>>>  2 files changed, 39 insertions(+)
>>>
>>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>>> index d78c885..edacb16 100644
>>> --- a/hw/intc/apic_common.c
>>> +++ b/hw/intc/apic_common.c
>>> @@ -384,6 +384,29 @@ static bool apic_common_sipi_needed(void *opaque)
>>>      return s->wait_for_sipi != 0;
>>>  }
>>>
>>> +static bool apic_irq_delivered_needed(void *opaque)
>>> +{
>>> +    static APICCommonState *first_apic;
>>> +    APICCommonState *s = APIC_COMMON(opaque);
>>> +    if (!first_apic) {
>>> +        first_apic = s;
>>> +    }
>>> +    return s == first_apic;
>>
>> Should also check " && apic_irq_delivered != 0".
> 
> Reset of this variable when machine reboots is also forgotten.

Ok, I'll queue this patch with the condition changed, can you send one
for reset?

Paolo
Pavel Dovgalyuk Jan. 26, 2017, 1:07 p.m. UTC | #4 
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 26/01/2017 14:03, Pavel Dovgalyuk wrote:
> >> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> >> On 26/01/2017 13:34, Pavel Dovgalyuk wrote:
> >>> This patch implements saving/restoring of static apic_delivered variable.
> >>>
> >>> v8: saving static variable only for one of the APICs
> >>>
> >>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> >>> ---
> >>>  hw/intc/apic_common.c           |   37 +++++++++++++++++++++++++++++++++++++
> >>>  include/hw/i386/apic_internal.h |    2 ++
> >>>  2 files changed, 39 insertions(+)
> >>>
> >>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> >>> index d78c885..edacb16 100644
> >>> --- a/hw/intc/apic_common.c
> >>> +++ b/hw/intc/apic_common.c
> >>> @@ -384,6 +384,29 @@ static bool apic_common_sipi_needed(void *opaque)
> >>>      return s->wait_for_sipi != 0;
> >>>  }
> >>>
> >>> +static bool apic_irq_delivered_needed(void *opaque)
> >>> +{
> >>> +    static APICCommonState *first_apic;
> >>> +    APICCommonState *s = APIC_COMMON(opaque);
> >>> +    if (!first_apic) {
> >>> +        first_apic = s;
> >>> +    }
> >>> +    return s == first_apic;
> >>
> >> Should also check " && apic_irq_delivered != 0".
> >
> > Reset of this variable when machine reboots is also forgotten.
> 
> Ok, I'll queue this patch with the condition changed, can you send one
> for reset?

Ok, I'll send it with the next version of the series.


Pavel Dovgalyuk
diff mbox

Patch

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index d78c885..edacb16 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -384,6 +384,29 @@  static bool apic_common_sipi_needed(void *opaque)
     return s->wait_for_sipi != 0;
 }
 
+static bool apic_irq_delivered_needed(void *opaque)
+{
+    static APICCommonState *first_apic;
+    APICCommonState *s = APIC_COMMON(opaque);
+    if (!first_apic) {
+        first_apic = s;
+    }
+    return s == first_apic;
+}
+
+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,
@@ -396,6 +419,19 @@  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,
@@ -430,6 +466,7 @@  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 1209eb4..20ad28c 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -189,6 +189,8 @@  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 {