Re: [Qemu-devel] [PATCH v3 1/5] ppc: spapr: Register and handle HCALL to receive updated RTAS region

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]

On Mon, Aug 21, 2017 at 03:12:49PM +0530, Aravinda Prasad wrote:
> 
> 
> On Thursday 17 August 2017 07:04 AM, David Gibson wrote:
> > On Wed, Aug 16, 2017 at 02:42:13PM +0530, Aravinda Prasad wrote:
> >> Receive updates from SLOF about the updated rtas-base.
> >> A separate patch for SLOF [1] adds functionality to invoke
> >> a private HCALL whenever OS issues instantiate-rtas with
> >> a new rtas-base.
> >>
> >> This is required as QEMU needs to know the updated rtas-base
> >> as it allocates error reporting structure in RTAS space upon
> >> a machine check exception.
> >>
> >> [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-August/120386.html
> >>
> >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
> >> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > Actually, I take back this R-b, see below.
> > 
> > In any case I'm not willing to apply the patches which depend on this
> > until the corresponding SLOF update is merged as well.
> 
> As Nikunj mentioned, SLOF updates are already merged.

In qemu as well as SLOF master, ok, good.  Commit message could do
with updating to reflect that.

> >>  hw/ppc/spapr_hcall.c   |    8 ++++++++
> >>  include/hw/ppc/spapr.h |    4 +++-
> >>  2 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 72ea5a8..e66c72e 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1062,6 +1062,13 @@ static target_ulong h_rtas(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >>                             nret, rtas_r3 + 12 + 4*nargs);
> >>  }
> >>  
> >> +static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >> +                                  target_ulong opcode, target_ulong *args)
> >> +{
> >> +    spapr->rtas_addr = args[0];
> >> +    return 0;
> >> +}
> >> +
> >>  static target_ulong h_logical_load(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >>                                     target_ulong opcode, target_ulong *args)
> >>  {
> >> @@ -1717,6 +1724,7 @@ static void hypercall_register_types(void)
> >>  
> >>      /* qemu/KVM-PPC specific hcalls */
> >>      spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
> >> +    spapr_register_hypercall(KVMPPC_H_RTAS_UPDATE, h_rtas_update);
> >>  
> >>      /* ibm,client-architecture-support support */
> >>      spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index 2a303a7..46012b3 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -90,6 +90,7 @@ struct sPAPRMachineState {
> >>  
> >>      hwaddr rma_size;
> >>      int vrma_adjust;
> >> +    hwaddr rtas_addr;
> > 
> > This can now change at runtime, which means it needs to be migrated -
> > that's not happening in your patches yet.
> 
> Yes. Need a bit of help in understanding the migration process.
> 
> As rtas_addr is updated by SLOF, I think we need to modify SLOF to issue
> KVMPPC_H_RTAS_UPDATE HCALL with the new rtas_addr during migration. But
> I am not sure if SLOF is notified of migrations.

Uh.. no.  By the time you're migrating chances are SLOF isn't even
running any more, and it wouldn't make sense for it to be aware of
migration anyway.

Instead we need to add the rtas_addr field to the vmstate information
for the spapr machine object.  However, we can't just add it plain,
because that would break backwards migration.  Instead we'll need to
add another sub-vmstate which will migrate rtas_addr if it differs
from the default value.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]