From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33999) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLOP5-0002Yo-0i for qemu-devel@nongnu.org; Fri, 08 Jul 2016 01:32:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLOP3-0007Sq-8T for qemu-devel@nongnu.org; Fri, 08 Jul 2016 01:32:42 -0400 Date: Fri, 8 Jul 2016 15:32:56 +1000 From: David Gibson Message-ID: <20160708053256.GO14675@voom.fritz.box> References: <1467903025-13383-1-git-send-email-bharata@linux.vnet.ibm.com> <1467903025-13383-5-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pndui+VhQ7yNUqd0" Content-Disposition: inline In-Reply-To: <1467903025-13383-5-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v2 4/5] xics: Use stable_cpu_id instead of cpu_index in XICS code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, imammedo@redhat.com, groug@kaod.org, nikunj@linux.vnet.ibm.com, pbonzini@redhat.com --pndui+VhQ7yNUqd0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 07, 2016 at 08:20:24PM +0530, Bharata B Rao wrote: > xics maintains an array of ICPState structures which is indexed > by cpu_index. Optionally change this to index the ICPState array by > stable_cpu_id. When the use of stable_cpu_id is enabled from pseries-2.7 > onwards, this allows migration of guest to succeed when there are holes in > cpu_index range due to CPU core hot removal. You haven't changed the allocation path, so this will waste a bunch of space if the stable IDs aren't dense. They always are for now, but that might not be true for the upcoming powernv machine type. > Signed-off-by: Bharata B Rao > --- > hw/intc/xics.c | 21 +++++++++++++++++---- > hw/intc/xics_kvm.c | 10 ++++------ > hw/intc/xics_spapr.c | 29 +++++++++++++++++------------ > include/hw/ppc/xics.h | 1 + > 4 files changed, 39 insertions(+), 22 deletions(-) >=20 > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index cd48f42..97ff3c5 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -36,6 +36,17 @@ > #include "qemu/error-report.h" > #include "qapi/visitor.h" > =20 > +int xics_get_server(PowerPCCPU *cpu) > +{ > + CPUState *cs =3D CPU(cpu); > + > + if (cs->has_stable_cpu_id) { > + return cs->stable_cpu_id; > + } else { > + return cs->cpu_index; > + } > +} I really think we want a generic helper that gets our best guess at a stable id - the actual stable id if it's present, otherwise cpu_index. I think a bunch of things are going to want this. > + > int xics_get_cpu_index_by_dt_id(int cpu_dt_id) > { > PowerPCCPU *cpu =3D ppc_get_vcpu_by_dt_id(cpu_dt_id); > @@ -50,9 +61,10 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id) > void xics_cpu_destroy(XICSState *xics, PowerPCCPU *cpu) > { > CPUState *cs =3D CPU(cpu); > - ICPState *ss =3D &xics->ss[cs->cpu_index]; > + int server =3D xics_get_server(cpu); > + ICPState *ss =3D &xics->ss[server]; > =20 > - assert(cs->cpu_index < xics->nr_servers); > + assert(server < xics->nr_servers); > assert(cs =3D=3D ss->cs); > =20 > ss->output =3D NULL; > @@ -63,10 +75,11 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > { > CPUState *cs =3D CPU(cpu); > CPUPPCState *env =3D &cpu->env; > - ICPState *ss =3D &xics->ss[cs->cpu_index]; > + int server =3D xics_get_server(cpu); > + ICPState *ss =3D &xics->ss[server]; > XICSStateClass *info =3D XICS_COMMON_GET_CLASS(xics); > =20 > - assert(cs->cpu_index < xics->nr_servers); > + assert(server < xics->nr_servers); > =20 > ss->cs =3D cs; > =20 > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index edbd62f..f71b468 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -326,14 +326,12 @@ static const TypeInfo ics_kvm_info =3D { > */ > static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > { > - CPUState *cs; > - ICPState *ss; > + CPUState *cs =3D CPU(cpu); > KVMXICSState *xicskvm =3D XICS_SPAPR_KVM(xics); > + int server =3D xics_get_server(cpu); > + ICPState *ss =3D ss =3D &xics->ss[server]; > =20 > - cs =3D CPU(cpu); > - ss =3D &xics->ss[cs->cpu_index]; > - > - assert(cs->cpu_index < xics->nr_servers); > + assert(server < xics->nr_servers); > if (xicskvm->kernel_xics_fd =3D=3D -1) { > abort(); > } > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 618826d..5491f82 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -31,6 +31,7 @@ > #include "trace.h" > #include "qemu/timer.h" > #include "hw/ppc/spapr.h" > +#include "hw/ppc/spapr_cpu_core.h" > #include "hw/ppc/xics.h" > #include "qapi/visitor.h" > #include "qapi/error.h" > @@ -42,17 +43,19 @@ > static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr, > target_ulong opcode, target_ulong *args) > { > - CPUState *cs =3D CPU(cpu); > + int server =3D xics_get_server(cpu); > target_ulong cppr =3D args[0]; > =20 > - icp_set_cppr(spapr->xics, cs->cpu_index, cppr); > + icp_set_cppr(spapr->xics, server, cppr); > return H_SUCCESS; > } > =20 > static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr, > target_ulong opcode, target_ulong *args) > { > - target_ulong server =3D xics_get_cpu_index_by_dt_id(args[0]); > + CPUState *cs =3D CPU(cpu); > + target_ulong server =3D cs->has_stable_cpu_id ? args[0] : > + xics_get_cpu_index_by_dt_id(args[0]); > target_ulong mfrr =3D args[1]; > =20 > if (server >=3D spapr->xics->nr_servers) { > @@ -66,8 +69,8 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachine= State *spapr, > static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr, > target_ulong opcode, target_ulong *args) > { > - CPUState *cs =3D CPU(cpu); > - uint32_t xirr =3D icp_accept(spapr->xics->ss + cs->cpu_index); > + int server =3D xics_get_server(cpu); > + uint32_t xirr =3D icp_accept(spapr->xics->ss + server); > =20 > args[0] =3D xirr; > return H_SUCCESS; > @@ -76,8 +79,8 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachin= eState *spapr, > static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr, > target_ulong opcode, target_ulong *args) > { > - CPUState *cs =3D CPU(cpu); > - ICPState *ss =3D &spapr->xics->ss[cs->cpu_index]; > + int server =3D xics_get_server(cpu); > + ICPState *ss =3D &spapr->xics->ss[server]; > uint32_t xirr =3D icp_accept(ss); > =20 > args[0] =3D xirr; > @@ -88,19 +91,19 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMa= chineState *spapr, > static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr, > target_ulong opcode, target_ulong *args) > { > - CPUState *cs =3D CPU(cpu); > + int server =3D xics_get_server(cpu); > target_ulong xirr =3D args[0]; > =20 > - icp_eoi(spapr->xics, cs->cpu_index, xirr); > + icp_eoi(spapr->xics, server, xirr); > return H_SUCCESS; > } > =20 > static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr, > target_ulong opcode, target_ulong *args) > { > - CPUState *cs =3D CPU(cpu); > + int server =3D xics_get_server(cpu); > uint32_t mfrr; > - uint32_t xirr =3D icp_ipoll(spapr->xics->ss + cs->cpu_index, &mfrr); > + uint32_t xirr =3D icp_ipoll(spapr->xics->ss + server, &mfrr); > =20 > args[0] =3D xirr; > args[1] =3D mfrr; > @@ -113,6 +116,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachi= neState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > + CPUState *cs =3D CPU(cpu); > ICSState *ics =3D spapr->xics->ics; > uint32_t nr, server, priority; > =20 > @@ -122,7 +126,8 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachi= neState *spapr, > } > =20 > nr =3D rtas_ld(args, 0); > - server =3D xics_get_cpu_index_by_dt_id(rtas_ld(args, 1)); > + server =3D cs->has_stable_cpu_id ? rtas_ld(args, 1) : > + xics_get_cpu_index_by_dt_id(rtas_ld(args, 1)); > priority =3D rtas_ld(args, 2); > =20 > if (!ics_valid_irq(ics, nr) || (server >=3D ics->xics->nr_servers) > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 6189a3b..aea0678 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -195,5 +195,6 @@ void ics_write_xive(ICSState *ics, int nr, int server, > void ics_set_irq_type(ICSState *ics, int srcno, bool lsi); > =20 > int xics_find_source(XICSState *icp, int irq); > +int xics_get_server(PowerPCCPU *cpu); > =20 > #endif /* __XICS_H__ */ --=20 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 --pndui+VhQ7yNUqd0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXfzsIAAoJEGw4ysog2bOStmEP/RH72K3kktTcFLS61QEm13i1 JoSxrf09VZR35KikxEa9WRb1bfNlRvSdugIs2ip+6AJLxPH8CIanQnCH8Eian3T/ rJ+3zYTp2mxG/mFoyBrrRiTljhLIrUgYRmrCObQuUVJ3A+sucv7Tvxurp3+sx8sr Sz9atpqT7bjopwp6q1MSLo7nteWIGB1G/yGYN91vUFCJAuLEB4gpWhhrO2ZBla3M lfACkBgm7h+9W4rX04I6dX3qBnxdqymeKGW/gr7EbCthp2zGtS6mSyQOjXSovgrN Sv9Gqt4LgYfGeHSjZNVpZ41Hh7TsdclQJpo1DpLZD2BMwlQz+whAP4nRiUUsGmET lnzVyJeb+16rNzTBscD8by3d++On7QrB94jF3YqrQzP0V8jjFN78+2ahdFHa6A+M 9YKFpQ9e5eEJqHEBTD5TtGvHIa5DXPwUcUAEwjbt9WzqJbYiU2KlCnLmdlXYN1tD qGiDv4wzaRxoo87Vvt3ZzeCT+/4a732jKNSBc8ff9YFlINEqr5R7eHSqdaJltMJ2 wH5AlwkeXulJwvdTf5l7NxU+CKtOhLG4F5Z57retUWrO97IYJA22tj5hS8/7PtZI HI957xA/SmVIF3TY6MdECPWYTTeahaFvays1nzmMHvlgR0Y/GVwjmlfy/V8ZVr5R uGndJHLJ0D8/zc6bufWB =ctOD -----END PGP SIGNATURE----- --pndui+VhQ7yNUqd0--