diff mbox

[RFC,1/2] target-ppc: migrate interrupt vectors address for spapr VM

Message ID 56F0268A.9030509@fr.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cédric Le Goater March 21, 2016, 4:51 p.m. UTC 
On 03/21/2016 05:18 PM, Thomas Huth wrote:
> On 21.03.2016 15:02, Cédric Le Goater wrote:
>> This address is changed by the linux kernel using the H_SET_MODE hcall
>> and needs to be migrated in order to restart a spapr VM running in
>> TCG. Other platforms should not be affected.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>>  target-ppc/machine.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>> index 692121e98319..a418d463db83 100644
>> --- a/target-ppc/machine.c
>> +++ b/target-ppc/machine.c
>> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu = {
>>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>>          /* FIXME: access_type? */
>>  
>> +        /* Effective Address of interrupt vectors */
>> +        VMSTATE_UINTTL(env.excp_prefix, PowerPCCPU),
>> +
>>          /* Sanity checking */
>>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> 
> I'm really no expert with all this migration stuff, but don't you have
> to bump the version_id when you add new fields to the vmstate?
> ... and/or use VMSTATE_UINTTL_V() so that migration from older versions
> of QEMU to the current one also still works with KVM? For example, is it
> still possible to migrate from QEMU 2.5 to QEMU 2.6 in KVM if you only
> use VMSTATE_UINTTL without the _V suffix?

Yes. You are right. I think we need something like below.

Thanks,

C.


target-ppc: migrate interrupt vectors address for spapr VM

This address is changed by the linux kernel using the H_SET_MODE hcall
and needs to be migrated in order to restart a spapr VM running in
TCG. Other platforms should not be affected.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 target-ppc/machine.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)



 
>  Thomas
>

Comments

Greg Kurz March 21, 2016, 5:04 p.m. UTC | #1 
On Mon, 21 Mar 2016 17:51:22 +0100
Cédric Le Goater <clg@fr.ibm.com> wrote:

> On 03/21/2016 05:18 PM, Thomas Huth wrote:
> > On 21.03.2016 15:02, Cédric Le Goater wrote:  
> >> This address is changed by the linux kernel using the H_SET_MODE hcall
> >> and needs to be migrated in order to restart a spapr VM running in
> >> TCG. Other platforms should not be affected.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> >> ---
> >>  target-ppc/machine.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> >> index 692121e98319..a418d463db83 100644
> >> --- a/target-ppc/machine.c
> >> +++ b/target-ppc/machine.c
> >> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu = {
> >>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
> >>          /* FIXME: access_type? */
> >>  
> >> +        /* Effective Address of interrupt vectors */
> >> +        VMSTATE_UINTTL(env.excp_prefix, PowerPCCPU),
> >> +
> >>          /* Sanity checking */
> >>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
> >>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),  
> > 
> > I'm really no expert with all this migration stuff, but don't you have
> > to bump the version_id when you add new fields to the vmstate?
> > ... and/or use VMSTATE_UINTTL_V() so that migration from older versions
> > of QEMU to the current one also still works with KVM? For example, is it
> > still possible to migrate from QEMU 2.5 to QEMU 2.6 in KVM if you only
> > use VMSTATE_UINTTL without the _V suffix?  
> 
> Yes. You are right. I think we need something like below.
> 

It looks much better !

And just to be sure: this only affects TCG because excp_prefix isn't accessed by
QEMU when running with KVM, right ?

If the answer is as simple as yes, then:

Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

> Thanks,
> 
> C.
> 
> 
> target-ppc: migrate interrupt vectors address for spapr VM
> 
> This address is changed by the linux kernel using the H_SET_MODE hcall
> and needs to be migrated in order to restart a spapr VM running in
> TCG. Other platforms should not be affected.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
>  target-ppc/machine.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c
> ===================================================================
> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c
> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c
> @@ -522,7 +522,7 @@ static const VMStateDescription vmstate_
> 
>  const VMStateDescription vmstate_ppc_cpu = {
>      .name = "cpu",
> -    .version_id = 5,
> +    .version_id = 6,
>      .minimum_version_id = 5,
>      .minimum_version_id_old = 4,
>      .load_state_old = cpu_load_old,
> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu
>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>          /* FIXME: access_type? */
> 
> +        /* Effective Address of interrupt vectors */
> +        VMSTATE_UINTTL_V(env.excp_prefix, PowerPCCPU, 6),
> +
>          /* Sanity checking */
>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> 
> 
> 
> >  Thomas
> >   
> 
>
David Gibson March 22, 2016, 12:15 a.m. UTC | #2 
On Mon, Mar 21, 2016 at 05:51:22PM +0100, Cédric Le Goater wrote:
> On 03/21/2016 05:18 PM, Thomas Huth wrote:
> > On 21.03.2016 15:02, Cédric Le Goater wrote:
> >> This address is changed by the linux kernel using the H_SET_MODE hcall
> >> and needs to be migrated in order to restart a spapr VM running in
> >> TCG. Other platforms should not be affected.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> >> ---
> >>  target-ppc/machine.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> >> index 692121e98319..a418d463db83 100644
> >> --- a/target-ppc/machine.c
> >> +++ b/target-ppc/machine.c
> >> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu = {
> >>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
> >>          /* FIXME: access_type? */
> >>  
> >> +        /* Effective Address of interrupt vectors */
> >> +        VMSTATE_UINTTL(env.excp_prefix, PowerPCCPU),
> >> +
> >>          /* Sanity checking */
> >>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
> >>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> > 
> > I'm really no expert with all this migration stuff, but don't you have
> > to bump the version_id when you add new fields to the vmstate?
> > ... and/or use VMSTATE_UINTTL_V() so that migration from older versions
> > of QEMU to the current one also still works with KVM? For example, is it
> > still possible to migrate from QEMU 2.5 to QEMU 2.6 in KVM if you only
> > use VMSTATE_UINTTL without the _V suffix?
> 
> Yes. You are right. I think we need something like below.
> 
> Thanks,
> 
> C.
> 
> 
> target-ppc: migrate interrupt vectors address for spapr VM
> 
> This address is changed by the linux kernel using the H_SET_MODE hcall
> and needs to be migrated in order to restart a spapr VM running in
> TCG. Other platforms should not be affected.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
>  target-ppc/machine.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c
> ===================================================================
> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c
> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c
> @@ -522,7 +522,7 @@ static const VMStateDescription vmstate_
>  
>  const VMStateDescription vmstate_ppc_cpu = {
>      .name = "cpu",
> -    .version_id = 5,
> +    .version_id = 6,
>      .minimum_version_id = 5,
>      .minimum_version_id_old = 4,
>      .load_state_old = cpu_load_old,
> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu
>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>          /* FIXME: access_type? */
>  
> +        /* Effective Address of interrupt vectors */
> +        VMSTATE_UINTTL_V(env.excp_prefix, PowerPCCPU, 6),


So, I dislike putting what's essentially emulator internal state (as
opposed to architected state) into the migration stream if we can
possibly avoid it.

I think recalculating excp_prefix from the MSR on incoming migration
is the correct approach here - I see that there are bugs with that in
the other patch, but so far I'm not seeing a reason to migrate
excp_prefix itself.

>          /* Sanity checking */
>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> 
> 
>  
> >  Thomas
> > 
>
Cédric Le Goater March 22, 2016, 7:13 a.m. UTC | #3 
On 03/22/2016 01:15 AM, David Gibson wrote:
> On Mon, Mar 21, 2016 at 05:51:22PM +0100, Cédric Le Goater wrote:
>> On 03/21/2016 05:18 PM, Thomas Huth wrote:
>>> On 21.03.2016 15:02, Cédric Le Goater wrote:
>>>> This address is changed by the linux kernel using the H_SET_MODE hcall
>>>> and needs to be migrated in order to restart a spapr VM running in
>>>> TCG. Other platforms should not be affected.
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>>>> ---
>>>>  target-ppc/machine.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>>>> index 692121e98319..a418d463db83 100644
>>>> --- a/target-ppc/machine.c
>>>> +++ b/target-ppc/machine.c
>>>> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu = {
>>>>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>>>>          /* FIXME: access_type? */
>>>>  
>>>> +        /* Effective Address of interrupt vectors */
>>>> +        VMSTATE_UINTTL(env.excp_prefix, PowerPCCPU),
>>>> +
>>>>          /* Sanity checking */
>>>>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>>>>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
>>>
>>> I'm really no expert with all this migration stuff, but don't you have
>>> to bump the version_id when you add new fields to the vmstate?
>>> ... and/or use VMSTATE_UINTTL_V() so that migration from older versions
>>> of QEMU to the current one also still works with KVM? For example, is it
>>> still possible to migrate from QEMU 2.5 to QEMU 2.6 in KVM if you only
>>> use VMSTATE_UINTTL without the _V suffix?
>>
>> Yes. You are right. I think we need something like below.
>>
>> Thanks,
>>
>> C.
>>
>>
>> target-ppc: migrate interrupt vectors address for spapr VM
>>
>> This address is changed by the linux kernel using the H_SET_MODE hcall
>> and needs to be migrated in order to restart a spapr VM running in
>> TCG. Other platforms should not be affected.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>>  target-ppc/machine.c |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c
>> ===================================================================
>> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c
>> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c
>> @@ -522,7 +522,7 @@ static const VMStateDescription vmstate_
>>  
>>  const VMStateDescription vmstate_ppc_cpu = {
>>      .name = "cpu",
>> -    .version_id = 5,
>> +    .version_id = 6,
>>      .minimum_version_id = 5,
>>      .minimum_version_id_old = 4,
>>      .load_state_old = cpu_load_old,
>> @@ -553,6 +553,9 @@ const VMStateDescription vmstate_ppc_cpu
>>          VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>>          /* FIXME: access_type? */
>>  
>> +        /* Effective Address of interrupt vectors */
>> +        VMSTATE_UINTTL_V(env.excp_prefix, PowerPCCPU, 6),
> 
> 
> So, I dislike putting what's essentially emulator internal state (as
> opposed to architected state) into the migration stream if we can
> possibly avoid it.
> 
> I think recalculating excp_prefix from the MSR on incoming migration
> is the correct approach here - I see that there are bugs with that in
> the other patch, but so far I'm not seeing a reason to migrate
> excp_prefix itself.

OK. It seems quite feasible as we can compute the value from the LPCR_AIL 
bits in SPR_LPCR. I will give it a try.

Thanks,

C.



>>          /* Sanity checking */
>>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>>          VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
>>
>>
>>  
>>>  Thomas
>>>
>>
>
diff mbox

Patch

Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c
===================================================================
--- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c
+++ qemu-dgibson-for-2.6.git/target-ppc/machine.c
@@ -522,7 +522,7 @@  static const VMStateDescription vmstate_
 
 const VMStateDescription vmstate_ppc_cpu = {
     .name = "cpu",
-    .version_id = 5,
+    .version_id = 6,
     .minimum_version_id = 5,
     .minimum_version_id_old = 4,
     .load_state_old = cpu_load_old,
@@ -553,6 +553,9 @@  const VMStateDescription vmstate_ppc_cpu
         VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
         /* FIXME: access_type? */
 
+        /* Effective Address of interrupt vectors */
+        VMSTATE_UINTTL_V(env.excp_prefix, PowerPCCPU, 6),
+
         /* Sanity checking */
         VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
         VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),