From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57563) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkBF7-00006V-4e for qemu-devel@nongnu.org; Thu, 08 Oct 2015 09:28:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkBEw-0001Fe-7U for qemu-devel@nongnu.org; Thu, 08 Oct 2015 09:28:20 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:63242) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkBEw-0001E3-0N for qemu-devel@nongnu.org; Thu, 08 Oct 2015 09:28:10 -0400 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NVW00JSCLESIA80@mailout4.w1.samsung.com> for qemu-devel@nongnu.org; Thu, 08 Oct 2015 14:28:04 +0100 (BST) From: Pavel Fedin References: <1444247430-14808-1-git-send-email-christoffer.dall@linaro.org> <003a01d10199$58e96590$0abc30b0$@samsung.com> <20151008082302.GE14315@cbox> <005e01d101a9$31ee80f0$95cb82d0$@samsung.com> <20151008092858.GA16100@cbox> <561641F5.4060202@arm.com> <009501d101c4$dde8e2e0$99baa8a0$@samsung.com> <20151008123630.GC20936@cbox> <009901d101c7$3331b3b0$99951b10$@samsung.com> In-reply-to: Date: Thu, 08 Oct 2015 16:28:03 +0300 Message-id: <00a801d101cd$29570fb0$7c052f10$@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: quoted-printable Content-language: ru Subject: Re: [Qemu-devel] [PATCH] KVM: arm/arm64: Add VGICv3 save/restore API documentation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Peter Maydell' , 'QEMU' Cc: 'Eric Auger' , 'Marc Zyngier' , 'Andre Przywara' , 'Shlomo Pongratz' , 'Shlomo Pongratz' , 'Christoffer Dall' Hello! Since we are discussing qemu here, i removed kernel mailing lists and = added qemu-devel. > A quick look at your patch suggests you still have data > structures like >=20 > +struct gicv3_irq_state { > + /* The enable bits are only banked for per-cpu interrupts. */ > + unsigned long *enabled; > + unsigned long *pending; > + unsigned long *active; > + unsigned long *level; > + unsigned long *group; > + bool edge_trigger; /* true: edge-triggered, false: = level-triggered */ > + uint32_t mask_size; /* Size of bitfields in long's, for migration = */ > +}; >=20 > I think it's probably going to be better to have an array > of redistributor structures (one per CPU) Already done. See struct GICv3CPUState - this is per-CPU data. During = realize, i allocate an array of these structures and put the pointer = into s->cpu. GICv3CPUState holds both redistributor and CPU interface data, for = simplicity, because they come in pairs. > and then keep > the state that in hardware is in each redistributor inside > those sub-structures. I can do it easily, but here i left the original layout by Shlomo = Pongratz, because i thought that it will be easier for his SW emulation = code to handle it. I remember that he performs some bitwise logic on = these bitmasks based on CPU masks in order to determine what to do on = which CPUs, so i thought that it will be better to leave them as they = are. These bitfields have one bit per CPU, just SPI bits are always = set/cleared altogether (i have set_all_cpus() and clear_all_cpus() = inlines for this purpose). I had even better idea of splitting up gicv3_irq_state, and storing = only per-CPU IRQs in the GICv3CPUState, and SPIs in GICv3State. In this = case bitfields would be of a fixed size, and one bit would represent one = interrupt. This would decrease memory usage, because we would not have = to duplicate SPI bits for every CPU. But will this be good for SW = emulation? OTOH, i have rechecked current SW emulation code, and i see that it = uses macros like GIC_TEST_xxx, and cpu mask is always (1 << ncpu). So, = can we safely replace mask with just CPU number in these macros? It = would solve the problem. Shlomo, your word? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia