diff mbox

[3/6] e1000e: fix for migration compatibility

Message ID 1471444747-6277-4-git-send-email-caoj.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

Cao jin Aug. 17, 2016, 2:39 p.m. UTC
commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
is used by intr_state which exists in vmstate. Restore it for migration
to older QEMU versions

CC: Dmitry Fleytman <dmitry@daynix.com>
CC: Jason Wang <jasowang@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/net/e1000e.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Dmitry Fleytman Aug. 18, 2016, 5:25 a.m. UTC | #1
Acked-by: Dmitry Fleytman <dmitry@daynix.com>

> On 17 Aug 2016, at 17:39, Cao jin <caoj.fnst@cn.fujitsu.com> wrote:
> 
> commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
> is used by intr_state which exists in vmstate. Restore it for migration
> to older QEMU versions
> 
> CC: Dmitry Fleytman <dmitry@daynix.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
> hw/net/e1000e.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 82a7be1..ba37fe9 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -89,7 +89,8 @@ typedef struct E1000EState {
> #define E1000E_MSIX_TABLE   (0x0000)
> #define E1000E_MSIX_PBA     (0x2000)
> 
> -#define E1000E_USE_MSIX    BIT(0)
> +#define E1000E_USE_MSI     BIT(0)
> +#define E1000E_USE_MSIX    BIT(1)
> 
> static uint64_t
> e1000e_mmio_read(void *opaque, hwaddr addr, unsigned size)
> @@ -470,6 +471,8 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp)
>     ret = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL);
>     if (ret) {
>         trace_e1000e_msi_init_fail(ret);
> +    } else {
> +        s->intr_state |= E1000E_USE_MSI;
>     }
> 
>     if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset,
> -- 
> 2.1.0
> 
> 
>
Paolo Bonzini Aug. 18, 2016, 10:47 a.m. UTC | #2
On 17/08/2016 16:39, Cao jin wrote:
> commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
> is used by intr_state which exists in vmstate. Restore it for migration
> to older QEMU versions
> 
> CC: Dmitry Fleytman <dmitry@daynix.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>

Not necessary.  No released version of QEMU had e1000e and lacked commit
66bf7d58.

Paolo

>  hw/net/e1000e.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 82a7be1..ba37fe9 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -89,7 +89,8 @@ typedef struct E1000EState {
>  #define E1000E_MSIX_TABLE   (0x0000)
>  #define E1000E_MSIX_PBA     (0x2000)
>  
> -#define E1000E_USE_MSIX    BIT(0)
> +#define E1000E_USE_MSI     BIT(0)
> +#define E1000E_USE_MSIX    BIT(1)
>  
>  static uint64_t
>  e1000e_mmio_read(void *opaque, hwaddr addr, unsigned size)
> @@ -470,6 +471,8 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp)
>      ret = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL);
>      if (ret) {
>          trace_e1000e_msi_init_fail(ret);
> +    } else {
> +        s->intr_state |= E1000E_USE_MSI;
>      }
>  
>      if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset,
>
Paolo Bonzini Aug. 18, 2016, 1:04 p.m. UTC | #3
On 18/08/2016 15:11, Cao jin wrote:
> 
> 
> On 08/18/2016 06:47 PM, Paolo Bonzini wrote:
>>
>>
>> On 17/08/2016 16:39, Cao jin wrote:
>>> commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
>>> is used by intr_state which exists in vmstate. Restore it for migration
>>> to older QEMU versions
>>>
>>> CC: Dmitry Fleytman <dmitry@daynix.com>
>>> CC: Jason Wang <jasowang@redhat.com>
>>> CC: Markus Armbruster <armbru@redhat.com>
>>> CC: Marcel Apfelbaum <marcel@redhat.com>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>>
>> Not necessary.  No released version of QEMU had e1000e and lacked commit
>> 66bf7d58.
>>
>> Paolo
>>
> 
> Ok, then I will make this patch as separated one and send it out asap,
> so maybe it goes in 2.7

It's not necessary at all; why would it be useful in 2.7?

Paolo
Cao jin Aug. 18, 2016, 1:11 p.m. UTC | #4
On 08/18/2016 06:47 PM, Paolo Bonzini wrote:
>
>
> On 17/08/2016 16:39, Cao jin wrote:
>> commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
>> is used by intr_state which exists in vmstate. Restore it for migration
>> to older QEMU versions
>>
>> CC: Dmitry Fleytman <dmitry@daynix.com>
>> CC: Jason Wang <jasowang@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: Marcel Apfelbaum <marcel@redhat.com>
>> CC: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>
> Not necessary.  No released version of QEMU had e1000e and lacked commit
> 66bf7d58.
>
> Paolo
>

Ok, then I will make this patch as separated one and send it out asap, 
so maybe it goes in 2.7
Paolo Bonzini Aug. 18, 2016, 1:22 p.m. UTC | #5
On 18/08/2016 15:25, Cao jin wrote:
> 
> 
> On 08/18/2016 09:04 PM, Paolo Bonzini wrote:
>>
>>
>> On 18/08/2016 15:11, Cao jin wrote:
>>>
>>>
>>> On 08/18/2016 06:47 PM, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 17/08/2016 16:39, Cao jin wrote:
>>>>> commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
>>>>> is used by intr_state which exists in vmstate. Restore it for
>>>>> migration
>>>>> to older QEMU versions
>>>>>
>>>>> CC: Dmitry Fleytman <dmitry@daynix.com>
>>>>> CC: Jason Wang <jasowang@redhat.com>
>>>>> CC: Markus Armbruster <armbru@redhat.com>
>>>>> CC: Marcel Apfelbaum <marcel@redhat.com>
>>>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>>>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>>>>
>>>> Not necessary.  No released version of QEMU had e1000e and lacked
>>>> commit
>>>> 66bf7d58.
>>>>
>>>> Paolo
>>>>
>>>
>>> Ok, then I will make this patch as separated one and send it out asap,
>>> so maybe it goes in 2.7
>>
>> It's not necessary at all; why would it be useful in 2.7?
>>
>> Paolo
>>
> 
> commit 66bf7d58I already removed E1000E_USE_MSI, so I think maybe I can
> send a patch to remove E1000E_USE_MSIX & intr_state, so there will be no
> migration compatibility issue

Please do, worst case it won't be accepted.

Paolo
Cao jin Aug. 18, 2016, 1:25 p.m. UTC | #6
On 08/18/2016 09:04 PM, Paolo Bonzini wrote:
>
>
> On 18/08/2016 15:11, Cao jin wrote:
>>
>>
>> On 08/18/2016 06:47 PM, Paolo Bonzini wrote:
>>>
>>>
>>> On 17/08/2016 16:39, Cao jin wrote:
>>>> commit 66bf7d58 removed internal msi state flag E1000E_USE_MSI, but it
>>>> is used by intr_state which exists in vmstate. Restore it for migration
>>>> to older QEMU versions
>>>>
>>>> CC: Dmitry Fleytman <dmitry@daynix.com>
>>>> CC: Jason Wang <jasowang@redhat.com>
>>>> CC: Markus Armbruster <armbru@redhat.com>
>>>> CC: Marcel Apfelbaum <marcel@redhat.com>
>>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>>>
>>> Not necessary.  No released version of QEMU had e1000e and lacked commit
>>> 66bf7d58.
>>>
>>> Paolo
>>>
>>
>> Ok, then I will make this patch as separated one and send it out asap,
>> so maybe it goes in 2.7
>
> It's not necessary at all; why would it be useful in 2.7?
>
> Paolo
>

commit 66bf7d58I already removed E1000E_USE_MSI, so I think maybe I can 
send a patch to remove E1000E_USE_MSIX & intr_state, so there will be no 
migration compatibility issue
diff mbox

Patch

diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 82a7be1..ba37fe9 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -89,7 +89,8 @@  typedef struct E1000EState {
 #define E1000E_MSIX_TABLE   (0x0000)
 #define E1000E_MSIX_PBA     (0x2000)
 
-#define E1000E_USE_MSIX    BIT(0)
+#define E1000E_USE_MSI     BIT(0)
+#define E1000E_USE_MSIX    BIT(1)
 
 static uint64_t
 e1000e_mmio_read(void *opaque, hwaddr addr, unsigned size)
@@ -470,6 +471,8 @@  static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp)
     ret = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL);
     if (ret) {
         trace_e1000e_msi_init_fail(ret);
+    } else {
+        s->intr_state |= E1000E_USE_MSI;
     }
 
     if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset,