diff mbox

[RFC,v1,6/6] spapr: Fix migration of Radix guests

Message ID 1494992962-6929-7-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bharata B Rao May 17, 2017, 3:49 a.m. UTC
Fix migration of radix guests by ensuring that we issue
KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.

Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c         | 15 +++++++++++++++
 hw/ppc/spapr_hcall.c   |  1 +
 include/hw/ppc/spapr.h |  1 +
 3 files changed, 17 insertions(+)

Comments

David Gibson May 17, 2017, 7 a.m. UTC | #1
On Wed, May 17, 2017 at 09:19:22AM +0530, Bharata B Rao wrote:
> Fix migration of radix guests by ensuring that we issue
> KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.
> 
> Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr.c         | 15 +++++++++++++++
>  hw/ppc/spapr_hcall.c   |  1 +
>  include/hw/ppc/spapr.h |  1 +
>  3 files changed, 17 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 05abfc1..dd1d687 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1443,6 +1443,20 @@ static int spapr_post_load(void *opaque, int version_id)
>          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
>      }
>  
> +    if (spapr->patb_entry) {
> +        if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
> +            err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu),
> +                                          spapr->patb_flags &
> +                                          SPAPR_PROC_TABLE_RADIX,
> +                                          spapr->patb_flags &
> +                                          SPAPR_PROC_TABLE_GTSE,

You should be able to work out the things you need here from
patb_entry without adding the new patb_flags field.

> +                                          spapr->patb_entry);
> +        } else {
> +            error_report("Radix guest is unsupported by the host");
> +            return -EINVAL;
> +        }
> +    }
> +
>      return err;
>  }
>  
> @@ -1527,6 +1541,7 @@ static const VMStateDescription vmstate_spapr_patb_entry = {
>      .needed = spapr_patb_entry_needed,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(patb_entry, sPAPRMachineState),
> +        VMSTATE_UINT64(patb_flags, sPAPRMachineState),
>          VMSTATE_END_OF_LIST()
>      },
>  };
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 768aa57..b002fae 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -986,6 +986,7 @@ static target_ulong h_register_process_table(PowerPCCPU *cpu,
>      spapr_check_setup_free_hpt(spapr, spapr->patb_entry, cproc);
>  
>      spapr->patb_entry = cproc; /* Save new process table */
> +    spapr->patb_flags = flags; /* Save the flags */
>  
>      /* Update the UPRT and GTSE bits in the LPCR for all cpus */
>      CPU_FOREACH(cs) {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5b39a26..c25a32e 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -75,6 +75,7 @@ struct sPAPRMachineState {
>      void *htab;
>      uint32_t htab_shift;
>      uint64_t patb_entry; /* Process tbl registed in H_REGISTER_PROCESS_TABLE */
> +    uint64_t patb_flags;
>      hwaddr rma_size;
>      int vrma_adjust;
>      ssize_t rtas_size;
Bharata B Rao May 17, 2017, 7:15 a.m. UTC | #2
On Wed, May 17, 2017 at 05:00:49PM +1000, David Gibson wrote:
> On Wed, May 17, 2017 at 09:19:22AM +0530, Bharata B Rao wrote:
> > Fix migration of radix guests by ensuring that we issue
> > KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.
> > 
> > Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c         | 15 +++++++++++++++
> >  hw/ppc/spapr_hcall.c   |  1 +
> >  include/hw/ppc/spapr.h |  1 +
> >  3 files changed, 17 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 05abfc1..dd1d687 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1443,6 +1443,20 @@ static int spapr_post_load(void *opaque, int version_id)
> >          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
> >      }
> >  
> > +    if (spapr->patb_entry) {
> > +        if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
> > +            err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu),
> > +                                          spapr->patb_flags &
> > +                                          SPAPR_PROC_TABLE_RADIX,
> > +                                          spapr->patb_flags &
> > +                                          SPAPR_PROC_TABLE_GTSE,
> 
> You should be able to work out the things you need here from
> patb_entry without adding the new patb_flags field.

kvmppc_configure_v3_mmu() needs two bools: radix and gtse. The radix
bit can be implied from patb_entry, I needed patb_flags to get the
gtse value. Not immediately obvious of how to get gtse bit from patb_entry,
but let me take a relook.

Regards,
Bharata.
David Gibson May 17, 2017, 7:20 a.m. UTC | #3
On Wed, May 17, 2017 at 12:45:39PM +0530, Bharata B Rao wrote:
> On Wed, May 17, 2017 at 05:00:49PM +1000, David Gibson wrote:
> > On Wed, May 17, 2017 at 09:19:22AM +0530, Bharata B Rao wrote:
> > > Fix migration of radix guests by ensuring that we issue
> > > KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.
> > > 
> > > Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
> > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > ---
> > >  hw/ppc/spapr.c         | 15 +++++++++++++++
> > >  hw/ppc/spapr_hcall.c   |  1 +
> > >  include/hw/ppc/spapr.h |  1 +
> > >  3 files changed, 17 insertions(+)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 05abfc1..dd1d687 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1443,6 +1443,20 @@ static int spapr_post_load(void *opaque, int version_id)
> > >          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
> > >      }
> > >  
> > > +    if (spapr->patb_entry) {
> > > +        if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
> > > +            err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu),
> > > +                                          spapr->patb_flags &
> > > +                                          SPAPR_PROC_TABLE_RADIX,
> > > +                                          spapr->patb_flags &
> > > +                                          SPAPR_PROC_TABLE_GTSE,
> > 
> > You should be able to work out the things you need here from
> > patb_entry without adding the new patb_flags field.
> 
> kvmppc_configure_v3_mmu() needs two bools: radix and gtse. The radix
> bit can be implied from patb_entry, I needed patb_flags to get the
> gtse value. Not immediately obvious of how to get gtse bit from patb_entry,
> but let me take a relook.

Oh, sorry, you can't get GTSE from the patb_entry, but you can get it
from the LPCR.
Bharata B Rao May 18, 2017, 5:03 a.m. UTC | #4
On Wed, May 17, 2017 at 05:20:31PM +1000, David Gibson wrote:
> On Wed, May 17, 2017 at 12:45:39PM +0530, Bharata B Rao wrote:
> > On Wed, May 17, 2017 at 05:00:49PM +1000, David Gibson wrote:
> > > On Wed, May 17, 2017 at 09:19:22AM +0530, Bharata B Rao wrote:
> > > > Fix migration of radix guests by ensuring that we issue
> > > > KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.
> > > > 
> > > > Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
> > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > ---
> > > >  hw/ppc/spapr.c         | 15 +++++++++++++++
> > > >  hw/ppc/spapr_hcall.c   |  1 +
> > > >  include/hw/ppc/spapr.h |  1 +
> > > >  3 files changed, 17 insertions(+)
> > > > 
> > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > index 05abfc1..dd1d687 100644
> > > > --- a/hw/ppc/spapr.c
> > > > +++ b/hw/ppc/spapr.c
> > > > @@ -1443,6 +1443,20 @@ static int spapr_post_load(void *opaque, int version_id)
> > > >          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
> > > >      }
> > > >  
> > > > +    if (spapr->patb_entry) {
> > > > +        if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
> > > > +            err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu),
> > > > +                                          spapr->patb_flags &
> > > > +                                          SPAPR_PROC_TABLE_RADIX,
> > > > +                                          spapr->patb_flags &
> > > > +                                          SPAPR_PROC_TABLE_GTSE,
> > > 
> > > You should be able to work out the things you need here from
> > > patb_entry without adding the new patb_flags field.
> > 
> > kvmppc_configure_v3_mmu() needs two bools: radix and gtse. The radix
> > bit can be implied from patb_entry, I needed patb_flags to get the
> > gtse value. Not immediately obvious of how to get gtse bit from patb_entry,
> > but let me take a relook.
> 
> Oh, sorry, you can't get GTSE from the patb_entry, but you can get it
> from the LPCR.

So I need to get GTSE from LPCR at the target after the LPCR has been updated
from the incoming stream (via vmstate_ppc_cpu vmsd). However we are also
in the middle of processing the incoming stream here (via spapr_post_load).
Can we be sure that spapr_post_load() processing happens after all
SPRs (and hence LPCR) have been updated via vmstate_ppc_cpu vmsd handlers ?

Also, in addition to issuing KVM_PPC_CONFIGURE_V3_MMU, should we be
walking all the CPUs and setting the LPCR_UPRT and LPCR_GTSE bits like how
H_REGISTER_PROC_TBL does ?

Regards,
Bharata.
David Gibson May 18, 2017, 5:50 a.m. UTC | #5
On Thu, May 18, 2017 at 10:33:05AM +0530, Bharata B Rao wrote:
> On Wed, May 17, 2017 at 05:20:31PM +1000, David Gibson wrote:
> > On Wed, May 17, 2017 at 12:45:39PM +0530, Bharata B Rao wrote:
> > > On Wed, May 17, 2017 at 05:00:49PM +1000, David Gibson wrote:
> > > > On Wed, May 17, 2017 at 09:19:22AM +0530, Bharata B Rao wrote:
> > > > > Fix migration of radix guests by ensuring that we issue
> > > > > KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.
> > > > > 
> > > > > Reported-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
> > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > > > > ---
> > > > >  hw/ppc/spapr.c         | 15 +++++++++++++++
> > > > >  hw/ppc/spapr_hcall.c   |  1 +
> > > > >  include/hw/ppc/spapr.h |  1 +
> > > > >  3 files changed, 17 insertions(+)
> > > > > 
> > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > > index 05abfc1..dd1d687 100644
> > > > > --- a/hw/ppc/spapr.c
> > > > > +++ b/hw/ppc/spapr.c
> > > > > @@ -1443,6 +1443,20 @@ static int spapr_post_load(void *opaque, int version_id)
> > > > >          err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
> > > > >      }
> > > > >  
> > > > > +    if (spapr->patb_entry) {
> > > > > +        if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
> > > > > +            err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu),
> > > > > +                                          spapr->patb_flags &
> > > > > +                                          SPAPR_PROC_TABLE_RADIX,
> > > > > +                                          spapr->patb_flags &
> > > > > +                                          SPAPR_PROC_TABLE_GTSE,
> > > > 
> > > > You should be able to work out the things you need here from
> > > > patb_entry without adding the new patb_flags field.
> > > 
> > > kvmppc_configure_v3_mmu() needs two bools: radix and gtse. The radix
> > > bit can be implied from patb_entry, I needed patb_flags to get the
> > > gtse value. Not immediately obvious of how to get gtse bit from patb_entry,
> > > but let me take a relook.
> > 
> > Oh, sorry, you can't get GTSE from the patb_entry, but you can get it
> > from the LPCR.
> 
> So I need to get GTSE from LPCR at the target after the LPCR has been updated
> from the incoming stream (via vmstate_ppc_cpu vmsd). However we are also
> in the middle of processing the incoming stream here (via spapr_post_load).
> Can we be sure that spapr_post_load() processing happens after all
> SPRs (and hence LPCR) have been updated via vmstate_ppc_cpu vmsd handlers ?

Yes, you can.  An object's post_load is called after all that object's
state - and that of all objects below it in the QOM composition tree -
is transferred.  Since the CPUs are below the machine in the
composition tree, you can rely on the LPCR being correct in the
machine post_load handler.

> Also, in addition to issuing KVM_PPC_CONFIGURE_V3_MMU, should we be
> walking all the CPUs and setting the LPCR_UPRT and LPCR_GTSE bits like how
> H_REGISTER_PROC_TBL does ?

Shouldn't be necessary - the LPCR state should already be transferred,
along with the rest of the SPRs.
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 05abfc1..dd1d687 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1443,6 +1443,20 @@  static int spapr_post_load(void *opaque, int version_id)
         err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
     }
 
+    if (spapr->patb_entry) {
+        if (kvmppc_has_cap_mmu_radix() && kvm_enabled()) {
+            err = kvmppc_configure_v3_mmu(POWERPC_CPU(first_cpu),
+                                          spapr->patb_flags &
+                                          SPAPR_PROC_TABLE_RADIX,
+                                          spapr->patb_flags &
+                                          SPAPR_PROC_TABLE_GTSE,
+                                          spapr->patb_entry);
+        } else {
+            error_report("Radix guest is unsupported by the host");
+            return -EINVAL;
+        }
+    }
+
     return err;
 }
 
@@ -1527,6 +1541,7 @@  static const VMStateDescription vmstate_spapr_patb_entry = {
     .needed = spapr_patb_entry_needed,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(patb_entry, sPAPRMachineState),
+        VMSTATE_UINT64(patb_flags, sPAPRMachineState),
         VMSTATE_END_OF_LIST()
     },
 };
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 768aa57..b002fae 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -986,6 +986,7 @@  static target_ulong h_register_process_table(PowerPCCPU *cpu,
     spapr_check_setup_free_hpt(spapr, spapr->patb_entry, cproc);
 
     spapr->patb_entry = cproc; /* Save new process table */
+    spapr->patb_flags = flags; /* Save the flags */
 
     /* Update the UPRT and GTSE bits in the LPCR for all cpus */
     CPU_FOREACH(cs) {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5b39a26..c25a32e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -75,6 +75,7 @@  struct sPAPRMachineState {
     void *htab;
     uint32_t htab_shift;
     uint64_t patb_entry; /* Process tbl registed in H_REGISTER_PROCESS_TABLE */
+    uint64_t patb_flags;
     hwaddr rma_size;
     int vrma_adjust;
     ssize_t rtas_size;