From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bojku-0007jC-RG for qemu-devel@nongnu.org; Tue, 27 Sep 2016 00:12:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bojkq-0001C0-Gi for qemu-devel@nongnu.org; Tue, 27 Sep 2016 00:12:31 -0400 Received: from ozlabs.org ([103.22.144.67]:37605) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bojkp-0001Bd-U6 for qemu-devel@nongnu.org; Tue, 27 Sep 2016 00:12:28 -0400 Date: Tue, 27 Sep 2016 14:11:38 +1000 From: David Gibson Message-ID: <20160927041138.GF30322@umbus.fritz.box> References: <1474921408-24710-1-git-send-email-hpoussin@reactos.org> <1474921408-24710-3-git-send-email-hpoussin@reactos.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KJY2Ze80yH5MUxol" Content-Disposition: inline In-Reply-To: <1474921408-24710-3-git-send-email-hpoussin@reactos.org> Subject: Re: [Qemu-devel] [PATCH v2 2/6] intc/i8259: implement InterruptStatsProvider interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Herv=E9?= Poussineau Cc: qemu-devel@nongnu.org, Paolo Bonzini , "Michael S. Tsirkin" , Luiz Capitulino --KJY2Ze80yH5MUxol Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 26, 2016 at 10:23:24PM +0200, Herv=E9 Poussineau wrote: > Signed-off-by: Herv=E9 Poussineau > --- > hw/intc/i8259.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) >=20 > diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c > index c2607a5..75c8d22 100644 > --- a/hw/intc/i8259.c > +++ b/hw/intc/i8259.c > @@ -29,6 +29,7 @@ > #include "qemu/timer.h" > #include "qemu/log.h" > #include "hw/isa/i8259_internal.h" > +#include "hw/intc/intc.h" > =20 > /* debug PIC */ > //#define DEBUG_PIC > @@ -251,6 +252,35 @@ static void pic_reset(DeviceState *dev) > pic_init_reset(s); > } > =20 > +static bool pic_get_statistics(InterruptStatsProvider *obj, > + uint64_t **irq_counts, unsigned int *nb_i= rqs) > +{ > + PICCommonState *s =3D PIC_COMMON(obj); > + > + if (s->master) { > +#ifdef DEBUG_IRQ_COUNT > + *irq_counts =3D irq_count; So, the irq_counts return parameter is set to point at an internal structure of the intc, in this and the other implementations. Is that safe, without some contract about how long the array pointer is valid and/or correct? Could it be a problem if in future we tried to implement this for an intc that doesn't keep irq stats as a simple array (e.g. kept the count in a structure also containing other information for each irq)? I'm wondering if a safer interface might be to actually copy out a snapshot of the counts, which the caller is responsible for freeing. > + *nb_irqs =3D ARRAY_SIZE(irq_count); > +#else > + return false; > +#endif > + } else { > + *irq_counts =3D NULL; > + *nb_irqs =3D 0; > + } > + return true; > +} > + > +static void pic_print_info(InterruptStatsProvider *obj, Monitor *mon) > +{ > + PICCommonState *s =3D PIC_COMMON(obj); > + monitor_printf(mon, "pic%d: irr=3D%02x imr=3D%02x isr=3D%02x hprio= =3D%d " > + "irq_base=3D%02x rr_sel=3D%d elcr=3D%02x fnm=3D%d\n", > + s->master ? 0 : 1, s->irr, s->imr, s->isr, s->priorit= y_add, > + s->irq_base, s->read_reg_select, s->elcr, > + s->special_fully_nested_mode); > +} > + > static void pic_ioport_write(void *opaque, hwaddr addr64, > uint64_t val64, unsigned size) > { > @@ -503,10 +533,13 @@ static void i8259_class_init(ObjectClass *klass, vo= id *data) > { > PICClass *k =3D PIC_CLASS(klass); > DeviceClass *dc =3D DEVICE_CLASS(klass); > + InterruptStatsProviderClass *ic =3D INTERRUPT_STATS_PROVIDER_CLASS(k= lass); > =20 > k->parent_realize =3D dc->realize; > dc->realize =3D pic_realize; > dc->reset =3D pic_reset; > + ic->get_statistics =3D pic_get_statistics; > + ic->print_info =3D pic_print_info; > } > =20 > static const TypeInfo i8259_info =3D { > @@ -515,6 +548,10 @@ static const TypeInfo i8259_info =3D { > .parent =3D TYPE_PIC_COMMON, > .class_init =3D i8259_class_init, > .class_size =3D sizeof(PICClass), > + .interfaces =3D (InterfaceInfo[]) { > + { TYPE_INTERRUPT_STATS_PROVIDER }, > + { } > + }, > }; > =20 > static void pic_register_types(void) --=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 --KJY2Ze80yH5MUxol Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX6fF6AAoJEGw4ysog2bOScaIP/jAkEp8ia9NrBisHKASsRyUE GsMvbr28NPEo9AB31ty6aUgNeiHxAOmQj8sTLwLjF5KHtL97QRhDbN+i25j0NK+w 69eXWXcHX909DGl1XaDtrH21MUx/EncU+PIu7wwJY9kXbU062CgHO/IW/p+QTT48 ve06lONsFwJecOfYWjD6HMJQJ+Lkp1CvqpLNbaMu8V0un/okdQjbh0lLZ4umr8P0 qf/DLCkEUW900D6LrtkShWNYp3orJz49PiTYXaspxoeZVtvsCbnw8J//5Ck/1zpI mW1iOcLLfnW3KtUpj4IYJ8GJf0/J4yiYBQ0BRQxz/3nrmVqeW5kb2kHlAo+Ez/tj RYzaFH7D2QrTmc/9Vt2By3ACH5fzWb2OlDn7CNb0DdPUi1+O2XcjwRy1wnuG2D+X xH0Iwuk8/2bkNGhDP0hZaopw7z0HdHB4BxMa8bo4zSzl8tCY3MkBC7myT+qXKQKg 0pBsyfgl/F9z+8P5Bx+lxZhbtBqpGfHWmJU48NoN/+0Lq+Yem1kl7u3XK/AmbYEb u9MXKzp0motMDg+b6/W7W8B+q7PUChfMqO1O6Zz+Cznhz5P2WEkDsj6F+EyuBsF9 po11VhuUz/9tr6gwx1maU8Y2S6KNTkeIfB4uQjF1rteJVaviFga4DmK8HYVaTgbz Ei+v3z4xu8eOOcHFqU0u =Hq8a -----END PGP SIGNATURE----- --KJY2Ze80yH5MUxol--