From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59433) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c5DDB-00057L-B0 for qemu-devel@nongnu.org; Fri, 11 Nov 2016 09:53:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c5DD6-0004b2-Qf for qemu-devel@nongnu.org; Fri, 11 Nov 2016 09:53:49 -0500 Received: from mail-wm0-x22c.google.com ([2a00:1450:400c:c09::22c]:36377) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c5DD6-0004ah-Ju for qemu-devel@nongnu.org; Fri, 11 Nov 2016 09:53:44 -0500 Received: by mail-wm0-x22c.google.com with SMTP id g23so102796659wme.1 for ; Fri, 11 Nov 2016 06:53:44 -0800 (PST) References: <1478798481-25030-1-git-send-email-drjones@redhat.com> <1478798481-25030-11-git-send-email-drjones@redhat.com> <874m3f8621.fsf@linaro.org> <20161110203713.4idzgs5u2xm7ms4o@hawk.localdomain> <871syi8hbg.fsf@linaro.org> <20161111135421.5oxdkfqynkgipgom@hawk.localdomain> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <20161111135421.5oxdkfqynkgipgom@hawk.localdomain> Date: Fri, 11 Nov 2016 14:53:42 +0000 Message-ID: <87twbe6pah.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [kvm-unit-tests PATCH v5 10/11] arm/arm64: gicv3: add an IPI test List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Jones Cc: peter.maydell@linaro.org, kvm@vger.kernel.org, marc.zyngier@arm.com, andre.przywara@arm.com, qemu-devel@nongnu.org, eric.auger@redhat.com, qemu-arm@nongnu.org, pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org Andrew Jones writes: > On Fri, Nov 11, 2016 at 10:02:59AM +0000, Alex Bennée wrote: >> >> Andrew Jones writes: >> >> > On Thu, Nov 10, 2016 at 07:53:58PM +0000, Alex Bennée wrote: >> > [...] >> >> > +struct gic gicv2 = { >> >> > + .ipi = { >> >> > + .enable = gicv2_enable_defaults, >> >> > + .send_self = gicv2_ipi_send_self, >> >> > + .send_tlist = gicv2_ipi_send_tlist, >> >> > + .send_broadcast = gicv2_ipi_send_broadcast, >> >> > + }, >> >> > + .read_iar = gicv2_read_iar, >> >> > + .irqnr = gicv2_irqnr, >> >> > + .write_eoi = gicv2_write_eoi, >> >> > +}; >> >> > + >> >> > +struct gic gicv3 = { >> >> > + .ipi = { >> >> > + .enable = gicv3_enable_defaults, >> >> > + .send_self = gicv3_ipi_send_self, >> >> > + .send_tlist = gicv3_ipi_send_tlist, >> >> > + .send_broadcast = gicv3_ipi_send_broadcast, >> >> > + }, >> >> > + .read_iar = gicv3_read_iar, >> >> > + .irqnr = gicv3_irqnr, >> >> > + .write_eoi = gicv3_write_eoir, >> >> > +}; >> >> > + >> >> >> >> So I was re-basing my kvm-unit-tests against your GIC rework and found >> >> myself copy and pasting a bunch of this into my tests that fire IRQs. >> >> That makes me think the abstraction should be in the library code so >> >> other tests can fiddle with sending IRQs. >> >> >> >> What do you think? >> >> >> > >> > I guess you mean moving the above two structs and their corresponding >> > functions (all which aren't already common) to lib/arm/ ? Or do you >> > just mean the one non-trivial function gicv3_ipi_send_tlist? I think >> > agree with gicv3_ipi_send_tlist getting shared, but the others are >> > mostly one-liners, so I'm not sure. I guess I'd have to see how you're >> > using them first. >> >> So it looked like there were some functions in the common code for one >> GIC which had local test defined functions for the other. They should at >> least be consistent. > > gicv3_read_iar and gicv3_write_eoir being common already is a product of > being sysreg wrappers, allowing for both arm32 and arm64 to use functions > of the same names, not because I wanted gicv3 to be inconsistent with > gicv2 (which uses MMIO and thus doesn't need wrappers) > >> >> For my use case I could do with a common: >> >> gic_enable > > OK, I can extend gic_init() to initialize a 'struct gic_common_ops' that > includes an enable -> *_enable_defaults(void), ipi_send(int cpu), > read_iar(void), iar_irqnr(u32 iar), and write_eoi(u32 irqstat). And also > provide the wrappers gic_enable, gic_ipi_send(cpu), ... > >> gic_send_spi(cpu, irq) > > I'll let you add this one to the new common ops struct :-) > >> gic_irq_ack() which returns the iar. > > This one will be called read_iar. > > Would that work for you, Alex? Sounds good to me :-) > > Andre, > > Would this also satisfy your needs for more common code? > > Thanks, > drew > >> >> See: >> >> https://github.com/stsquad/kvm-unit-tests/blob/mttcg/current-tests-v6/arm/tcg-test.c#L113 >> >> > >> > Thanks, >> > drew >> >> >> -- >> Alex Bennée >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- Alex Bennée