Andrew Jones <drjones@xxxxxxxxxx> 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. For my use case I could do with a common: gic_enable gic_send_spi(cpu, irq) gic_irq_ack() which returns the iar. 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html