From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E4D25C7EE25 for ; Fri, 9 Jun 2023 08:01:01 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1q7X33-0000CH-JI; Fri, 09 Jun 2023 04:00:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1q7X2s-00009d-Lw; Fri, 09 Jun 2023 04:00:31 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1q7X2p-0001z8-Vx; Fri, 09 Jun 2023 04:00:30 -0400 Received: from pps.filterd (m0353724.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3597qFd3010654; Fri, 9 Jun 2023 08:00:23 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=jYIPamXaUFBd0RD/6Ljs2dR/UC/6LWRW+uSQlB1qGcI=; b=hkzm1Y+3mnsloPsKQCHEzDgsZTZPXha+vI4oxJbJ+GsgcMRrEtuqQxL+S2IqeGrNgU6Q Knd2hOJtTZriD7uV8k4ZyyxypuiPf0BzydUXYHVWboyAtELVs4gyqA55BP//tmwSiqB4 W1e2GVvWfsmTjuhwKbSFylj/dAsk+CY2LpdQV3tDWEjIfbZElsKMJk8qV63/zJ/7ndhS RypYDjkeyf3rCF+FFJA6CmDkYcFk9UR6dv4e+G+6+IqxOtf7ARGd2UAYaf3KGz/013Yw OM4De5MImWODLrby/ClBGlWjHFXmJVRFHZbBGyd2zm/wqgr9rTuGsRdWzaWmlBQhV8bc jQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3r402n04mu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 09 Jun 2023 08:00:23 +0000 Received: from m0353724.ppops.net (m0353724.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3597sWrH015604; Fri, 9 Jun 2023 08:00:22 GMT Received: from ppma01dal.us.ibm.com (83.d6.3fa9.ip4.static.sl-reverse.com [169.63.214.131]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3r402n04mg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 09 Jun 2023 08:00:22 +0000 Received: from pps.filterd (ppma01dal.us.ibm.com [127.0.0.1]) by ppma01dal.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 3597HJH5008966; Fri, 9 Jun 2023 08:00:22 GMT Received: from smtprelay03.wdc07v.mail.ibm.com ([9.208.129.113]) by ppma01dal.us.ibm.com (PPS) with ESMTPS id 3r2a77kmkp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 09 Jun 2023 08:00:22 +0000 Received: from smtpav01.dal12v.mail.ibm.com (smtpav01.dal12v.mail.ibm.com [10.241.53.100]) by smtprelay03.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 35980KCR64291302 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 9 Jun 2023 08:00:20 GMT Received: from smtpav01.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 102915805D; Fri, 9 Jun 2023 08:00:20 +0000 (GMT) Received: from smtpav01.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7929E58062; Fri, 9 Jun 2023 08:00:16 +0000 (GMT) Received: from [9.171.48.82] (unknown [9.171.48.82]) by smtpav01.dal12v.mail.ibm.com (Postfix) with ESMTP; Fri, 9 Jun 2023 08:00:16 +0000 (GMT) Message-ID: <90f62b35-aba5-df79-a50d-29f78a79c7b3@linux.ibm.com> Date: Fri, 9 Jun 2023 13:30:14 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH 3/4] ppc/spapr: load and store l2 state with helper functions Content-Language: en-US To: Nicholas Piggin , qemu-ppc@nongnu.org Cc: qemu-devel@nongnu.org, Daniel Henrique Barboza References: <20230608091344.88685-1-npiggin@gmail.com> <20230608091344.88685-4-npiggin@gmail.com> From: Harsh Prateek Bora In-Reply-To: <20230608091344.88685-4-npiggin@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: AQp84XAJfLN3NDjP2kNS17V_E3yp5Bf1 X-Proofpoint-ORIG-GUID: 7D0kIt8HWWXjMMIArGL0gz7__ZEJyajE X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-09_04,2023-06-08_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 spamscore=0 impostorscore=0 suspectscore=0 phishscore=0 clxscore=1015 priorityscore=1501 adultscore=0 bulkscore=0 malwarescore=0 mlxlogscore=999 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306090066 Received-SPF: pass client-ip=148.163.158.5; envelope-from=harshpb@linux.ibm.com; helo=mx0b-001b2d01.pphosted.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.091, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On 6/8/23 14:43, Nicholas Piggin wrote: > Arguably this is just shuffling around register accesses, but one nice > thing it does is allow the exit to save away the L2 state then switch > the environment to the L1 before copying L2 data back to the L1, which > logically flows more naturally and simplifies the error paths. > > Signed-off-by: Nicholas Piggin > --- > hw/ppc/spapr_hcall.c | 164 ++++++++++++++++++++++--------------------- > 1 file changed, 85 insertions(+), 79 deletions(-) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index d5b8d54692..da6440f235 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1663,9 +1663,9 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu, > target_ulong *args) > { > PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); > - CPUState *cs = CPU(cpu); > CPUPPCState *env = &cpu->env; > SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); > + struct nested_ppc_state l2_state; > target_ulong hv_ptr = args[0]; > target_ulong regs_ptr = args[1]; > target_ulong hdec, now = cpu_ppc_load_tbl(env); > @@ -1699,6 +1699,10 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu, > return H_PARAMETER; > } > > + if (hv_state.lpid == 0) { > + return H_PARAMETER; > + } > + > spapr_cpu->nested_host_state = g_try_new(struct nested_ppc_state, 1); > if (!spapr_cpu->nested_host_state) { > return H_NO_MEM; > @@ -1717,46 +1721,49 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu, > return H_P2; > } > > - len = sizeof(env->gpr); > + len = sizeof(l2_state.gpr); > assert(len == sizeof(regs->gpr)); > - memcpy(env->gpr, regs->gpr, len); > + memcpy(l2_state.gpr, regs->gpr, len); > > - env->lr = regs->link; > - env->ctr = regs->ctr; > - cpu_write_xer(env, regs->xer); > - ppc_set_cr(env, regs->ccr); > - > - env->msr = regs->msr; > - env->nip = regs->nip; > + l2_state.lr = regs->link; > + l2_state.ctr = regs->ctr; > + l2_state.xer = regs->xer; > + l2_state.cr = regs->ccr; > + l2_state.msr = regs->msr; > + l2_state.nip = regs->nip; > > address_space_unmap(CPU(cpu)->as, regs, len, len, false); > > - env->cfar = hv_state.cfar; > - > - assert(env->spr[SPR_LPIDR] == 0); > - env->spr[SPR_LPIDR] = hv_state.lpid; > + l2_state.cfar = hv_state.cfar; > + l2_state.lpidr = hv_state.lpid; > > lpcr_mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_MER; > lpcr = (env->spr[SPR_LPCR] & ~lpcr_mask) | (hv_state.lpcr & lpcr_mask); > lpcr |= LPCR_HR | LPCR_UPRT | LPCR_GTSE | LPCR_HVICE | LPCR_HDICE; > lpcr &= ~LPCR_LPES0; > - env->spr[SPR_LPCR] = lpcr & pcc->lpcr_mask; > + l2_state.lpcr = lpcr & pcc->lpcr_mask; > > - env->spr[SPR_PCR] = hv_state.pcr; > + l2_state.pcr = hv_state.pcr; > /* hv_state.amor is not used */ > - env->spr[SPR_DPDES] = hv_state.dpdes; > - env->spr[SPR_HFSCR] = hv_state.hfscr; > - hdec = hv_state.hdec_expiry - now; > + l2_state.dpdes = hv_state.dpdes; > + l2_state.hfscr = hv_state.hfscr; > /* TCG does not implement DAWR*, CIABR, PURR, SPURR, IC, VTB, HEIR SPRs*/ > - env->spr[SPR_SRR0] = hv_state.srr0; > - env->spr[SPR_SRR1] = hv_state.srr1; > - env->spr[SPR_SPRG0] = hv_state.sprg[0]; > - env->spr[SPR_SPRG1] = hv_state.sprg[1]; > - env->spr[SPR_SPRG2] = hv_state.sprg[2]; > - env->spr[SPR_SPRG3] = hv_state.sprg[3]; > - env->spr[SPR_BOOKS_PID] = hv_state.pidr; > - env->spr[SPR_PPR] = hv_state.ppr; > + l2_state.srr0 = hv_state.srr0; > + l2_state.srr1 = hv_state.srr1; > + l2_state.sprg0 = hv_state.sprg[0]; > + l2_state.sprg1 = hv_state.sprg[1]; > + l2_state.sprg2 = hv_state.sprg[2]; > + l2_state.sprg3 = hv_state.sprg[3]; > + l2_state.pidr = hv_state.pidr; > + l2_state.ppr = hv_state.ppr; > + l2_state.tb_offset = env->tb_env->tb_offset + hv_state.tb_offset; > + > + /* > + * Switch to the nested guest environment and start the "hdec" timer. > + */ > + nested_load_state(cpu, &l2_state); > > + hdec = hv_state.hdec_expiry - now; > cpu_ppc_hdecr_init(env); > cpu_ppc_store_hdecr(env, hdec); > > @@ -1772,14 +1779,8 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu, > * and it's not obviously worth a new data structure to do it. > */ > > - env->tb_env->tb_offset += hv_state.tb_offset; > spapr_cpu->in_nested = true; > > - hreg_compute_hflags(env); > - ppc_maybe_interrupt(env); > - tlb_flush(cs); > - env->reserve_addr = -1; /* Reset the reservation */ > - > /* > * The spapr hcall helper sets env->gpr[3] to the return value, but at > * this point the L1 is not returning from the hcall but rather we > @@ -1793,49 +1794,69 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp) > { > CPUPPCState *env = &cpu->env; > SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); > - target_ulong r3_return = env->excp_vectors[excp]; /* hcall return value */ > + struct nested_ppc_state l2_state; > target_ulong hv_ptr = spapr_cpu->nested_host_state->gpr[4]; > target_ulong regs_ptr = spapr_cpu->nested_host_state->gpr[5]; > + target_ulong hsrr0, hsrr1, hdar, asdr, hdsisr; > struct kvmppc_hv_guest_state *hvstate; > struct kvmppc_pt_regs *regs; > hwaddr len; > > assert(spapr_cpu->in_nested); > > + nested_save_state(&l2_state, cpu); > + hsrr0 = env->spr[SPR_HSRR0]; > + hsrr1 = env->spr[SPR_HSRR1]; > + hdar = env->spr[SPR_HDAR]; > + hdsisr = env->spr[SPR_HDSISR]; > + asdr = env->spr[SPR_ASDR]; > + > + /* > + * Switch back to the host environment (including for any error). > + */ > + assert(env->spr[SPR_LPIDR] != 0); > + nested_load_state(cpu, spapr_cpu->nested_host_state); > + env->gpr[3] = env->excp_vectors[excp]; /* hcall return value */ > + > cpu_ppc_hdecr_exit(env); > > + spapr_cpu->in_nested = false; > + > + g_free(spapr_cpu->nested_host_state); > + spapr_cpu->nested_host_state = NULL; > + > len = sizeof(*hvstate); > hvstate = address_space_map(CPU(cpu)->as, hv_ptr, &len, true, > MEMTXATTRS_UNSPECIFIED); > if (len != sizeof(*hvstate)) { > address_space_unmap(CPU(cpu)->as, hvstate, len, 0, true); > - r3_return = H_PARAMETER; > - goto out_restore_l1; > + env->gpr[3] = H_PARAMETER; > + return; > } > > - hvstate->cfar = env->cfar; > - hvstate->lpcr = env->spr[SPR_LPCR]; > - hvstate->pcr = env->spr[SPR_PCR]; > - hvstate->dpdes = env->spr[SPR_DPDES]; > - hvstate->hfscr = env->spr[SPR_HFSCR]; > + hvstate->cfar = l2_state.cfar; > + hvstate->lpcr = l2_state.lpcr; > + hvstate->pcr = l2_state.pcr; > + hvstate->dpdes = l2_state.dpdes; > + hvstate->hfscr = l2_state.hfscr; > > if (excp == POWERPC_EXCP_HDSI) { > - hvstate->hdar = env->spr[SPR_HDAR]; > - hvstate->hdsisr = env->spr[SPR_HDSISR]; > - hvstate->asdr = env->spr[SPR_ASDR]; > + hvstate->hdar = hdar; > + hvstate->hdsisr = hdsisr; > + hvstate->asdr = asdr; > } else if (excp == POWERPC_EXCP_HISI) { > - hvstate->asdr = env->spr[SPR_ASDR]; > + hvstate->asdr = asdr; > } > > /* HEIR should be implemented for HV mode and saved here. */ > - hvstate->srr0 = env->spr[SPR_SRR0]; > - hvstate->srr1 = env->spr[SPR_SRR1]; > - hvstate->sprg[0] = env->spr[SPR_SPRG0]; > - hvstate->sprg[1] = env->spr[SPR_SPRG1]; > - hvstate->sprg[2] = env->spr[SPR_SPRG2]; > - hvstate->sprg[3] = env->spr[SPR_SPRG3]; > - hvstate->pidr = env->spr[SPR_BOOKS_PID]; > - hvstate->ppr = env->spr[SPR_PPR]; > + hvstate->srr0 = l2_state.srr0; > + hvstate->srr1 = l2_state.srr1; > + hvstate->sprg[0] = l2_state.sprg0; > + hvstate->sprg[1] = l2_state.sprg1; > + hvstate->sprg[2] = l2_state.sprg2; > + hvstate->sprg[3] = l2_state.sprg3; > + hvstate->pidr = l2_state.pidr; > + hvstate->ppr = l2_state.ppr; > > /* Is it okay to specify write length larger than actual data written? */ > address_space_unmap(CPU(cpu)->as, hvstate, len, len, true); > @@ -1845,46 +1866,31 @@ void spapr_exit_nested(PowerPCCPU *cpu, int excp) > MEMTXATTRS_UNSPECIFIED); > if (!regs || len != sizeof(*regs)) { > address_space_unmap(CPU(cpu)->as, regs, len, 0, true); > - r3_return = H_P2; > - goto out_restore_l1; > + env->gpr[3] = H_P2; > + return; > } > > len = sizeof(env->gpr); > assert(len == sizeof(regs->gpr)); > - memcpy(regs->gpr, env->gpr, len); > + memcpy(regs->gpr, l2_state.gpr, len); > > - regs->link = env->lr; > - regs->ctr = env->ctr; > - regs->xer = cpu_read_xer(env); > - regs->ccr = ppc_get_cr(env); > + regs->link = l2_state.lr; > + regs->ctr = l2_state.ctr; > + regs->xer = l2_state.xer; > + regs->ccr = l2_state.cr; > > if (excp == POWERPC_EXCP_MCHECK || > excp == POWERPC_EXCP_RESET || > excp == POWERPC_EXCP_SYSCALL) { > - regs->nip = env->spr[SPR_SRR0]; > - regs->msr = env->spr[SPR_SRR1] & env->msr_mask; > + regs->nip = l2_state.srr0; > + regs->msr = l2_state.srr1 & env->msr_mask; > } else { > - regs->nip = env->spr[SPR_HSRR0]; > - regs->msr = env->spr[SPR_HSRR1] & env->msr_mask; > + regs->nip = hsrr0; > + regs->msr = hsrr1 & env->msr_mask; > } > Can the copying of L2 data to L1 be abstraced with an inline helper routine as well ? Then the spapr_exit_nested will look more neat: - nested_save_state(l2) - nested_load_state(host) - copy_l2_state_to_l1() <=== something like this? regards, Harsh > /* Is it okay to specify write length larger than actual data written? */ > address_space_unmap(CPU(cpu)->as, regs, len, len, true); > - > -out_restore_l1: > - assert(env->spr[SPR_LPIDR] != 0); > - nested_load_state(cpu, spapr_cpu->nested_host_state); > - > - /* > - * Return the interrupt vector address from H_ENTER_NESTED to the L1 > - * (or error code). > - */ > - env->gpr[3] = r3_return; > - > - spapr_cpu->in_nested = false; > - > - g_free(spapr_cpu->nested_host_state); > - spapr_cpu->nested_host_state = NULL; > } > > static void hypercall_register_nested(void)