I'm not on this = mailing list, so please CC me on any replies. Thanks.

I ran qemu with valgrind l= ast night and found an error in the pci emulation code, which may= ,

or may not, be biting us. So fa= r the effects seem benign, although there exists the possibility

of trashing random memory.

In the function pci_change_irq_level() the= argument irq_num is passed in as 0-3, and used

as an index to change bus->irq_count[4]= . The problem is that in the loop above where

bus->irq_count[irq_num] is set, the value if irq_num is changed:

irq_nu= m =3D bus->map_irq(pci_dev, irq_num);

and I've seen the value of irq_num go as h= igh as 24. Since irq_count has only 4 elements,

the code is running off the end of the arr= ay. Luckily, so far, change has always been 0,

so doing the +=3D change has had no effect= , but it is clearly wrong. Here's = the original code:

static void pci_change_irq_lev= el(PCIDevice *pci_dev, int irq_num, int change)
{
    = PCIBus *bus;
    for (;;) {
    &n= bsp;   bus =3D pci_dev->bus;
     =    irq_num =3D bus->map_irq(pci_dev, irq_num);
  =       if (bus->set_irq)
   &n= bsp;        break;
   =      pci_dev =3D bus->parent_dev;
  &nb= sp; }
    bus->irq_count[irq_num] +=3D change;
&nbs= p;   bus->set_irq(bus->irq_opaque, irq_num, bus->irq_cou= nt[irq_num] !=3D 0);
}

I believe that the code should look like this inste= ad:

static void pci_cha= nge_irq_level(PCIDevice *pci_dev, int irq_num, int change)
{
&n= bsp; PCIBus *bus;

    int bus_irq_num;
    for (;;) {
     &nbs= p; bus =3D pci_dev->bus;
      &n= bsp; bus_irq_num = =3D bus->map_irq(pci_dev, irq_num);
     &nb= sp; if (bus->set_irq)
       = ;     break;
      &nb= sp; pci_dev =3D bus->parent_dev;
    }
    assert(irq_num >= =3D 0);
    assert(irq_num < bus->nirq);
=     bus->irq_count[irq_num] +=3D change;
  &= nbsp; bus->set_irq(bus->irq_opaque, bus_irq_num= , bus->irq_count[irq_num] !=3D 0);
}

Although this version may actually be correct if th= e irq needs to be remapped for each

cascaded bus:

static void pci_cha= nge_irq_level(PCIDevice *pci_dev, int irq_num, int change)
{
&n= bsp; PCIBus *bus;

int bus_irq_num =3D irq_num;

    = for (;;) {
        bus =3D pci_dev-&g= t;bus;
        = bus_irq_num =3D bus->map_irq(pci_dev, bus_i= rq_num);
        if (bus->s= et_irq)
          &nbs= p; break;
        pci_dev =3D bus->= ;parent_dev;
    }

=     assert(irq_num >=3D 0);
&= nbsp; assert(irq_num < bus->nirq);
&nbs= p;   bus->irq_count[irq_num] +=3D change;
   = ; bus->set_irq(bus->irq_opaque, bus_irq_num, bus->irq_count[irq_num] !=3D 0);
}

Thanks,

Alan Amaral

Virtual Computer

alan.amaral@virtualcomputer.com

From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56730) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R65VY-0003Dr-La for qemu-devel@nongnu.org; Tue, 20 Sep 2011 14:57:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R65VX-0003vY-FP for qemu-devel@nongnu.org; Tue, 20 Sep 2011 14:57:28 -0400 Received: from mail-ww0-f53.google.com ([74.125.82.53]:38811) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R65VX-0003vP-B5 for qemu-devel@nongnu.org; Tue, 20 Sep 2011 14:57:27 -0400 Received: by wwg14 with SMTP id 14so1043608wwg.10 for ; Tue, 20 Sep 2011 11:57:26 -0700 (PDT) Sender: Richard Henderson Message-ID: <4E78E207.5070308@twiddle.net> Date: Tue, 20 Sep 2011 11:57:11 -0700 From: Richard Henderson MIME-Version: 1.0 References: <00218408-4F7E-47E8-9A3A-7515E5472C40@mimectl> In-Reply-To: <00218408-4F7E-47E8-9A3A-7515E5472C40@mimectl> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alan Amaral Cc: "qemu-devel@nongnu.org" On 09/20/2011 10:24 AM, Alan Amaral wrote: > I'm not on this mailing list, so please CC me on any replies. Thanks. > > I ran qemu with valgrind last night and found an error in the pci emulation code, which may, > or may not, be biting us. So far the effects seem benign, although there exists the possibility > of trashing random memory. > > In the function pci_change_irq_level() the argument irq_num is passed in as 0-3, and used > as an index to change bus->irq_count[4]. I don't know what version of qemu you're using, but this is int *irq_count; in current sources. There's certainly no hard-coded "4". > assert(irq_num >= 0); > assert(irq_num < bus->nirq); > bus->irq_count[irq_num] += change; > bus->set_irq(bus->irq_opaque, bus_irq_num, bus->irq_count[irq_num] != 0); This version with the asserts, though, could be done. The site that created the bus ought to match up nirq with the map function. r~ From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54501) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R65qh-0001Ut-Id for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:19:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R65qg-0008NW-Bx for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:19:19 -0400 Received: from server514d.exghost.com ([72.32.253.69]:3198 helo=server514.appriver.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R65qg-0008NQ-45 for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:19:18 -0400 MIME-Version: 1.0 From: Alan Amaral In-Reply-To: <4E78E207.5070308@twiddle.net> References: <00218408-4F7E-47E8-9A3A-7515E5472C40@mimectl>, <4E78E207.5070308@twiddle.net> Content-Type: multipart/alternative; boundary="_09D111DF-7446-4110-90DF-17FCF756B087_" Message-ID: Date: Tue, 20 Sep 2011 15:19:18 -0400 Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: "qemu-devel@nongnu.org" --_09D111DF-7446-4110-90DF-17FCF756B087_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard You are correct, it's not hardcoded to 4. However, when it's allocated the= number of elements IS 4. Also, there's a comment just above pci_set_irq which says: /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */ static void pci_set_irq(void *opaque, int irq_num, int level) so, that implies to me that it's probably always 4... Sorry for the confus= ion. From: Richard Henderson Sent: Tue 9/20/2011 2:57 PM To: Alan Amaral Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] pci_change_irq_level is broken... On 09/20/2011 10:24 AM, Alan Amaral wrote: > I'm not on this mailing list, so please CC me on any replies. Thanks. > =20 > I ran qemu with valgrind last night and found an error in the pci emulati= on code, which may, > or may not, be biting us. So far the effects seem benign, although there= exists the possibility > of trashing random memory. > =20 > In the function pci_change_irq_level() the argument irq_num is passed in = as 0-3, and used > as an index to change bus->irq_count[4]. I don't know what version of qemu you're using, but this is int *irq_count; in current sources. There's certainly no hard-coded "4". > assert(irq_num >=3D 0); > assert(irq_num < bus->nirq); > bus->irq_count[irq_num] +=3D change; > bus->set_irq(bus->irq_opaque, bus_irq_num, bus->irq_count[irq_num] != =3D 0); This version with the asserts, though, could be done. The site that created the bus ought to match up nirq with the map function. r~ --_09D111DF-7446-4110-90DF-17FCF756B087_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

You are correct,= it's not hardcoded to 4. However, when it's allocated the number of = elements IS 4. Also,

there's a comment just above pci= _set_irq which says:

/* 0 <=3D irq_num <=3D 3. level m= ust be 0 or 1 */
static void pci_set_irq(void *opaque, int irq_num, int = level)

so, that implies= to me that it's probably always 4... Sorry for the confusion.

From: Richard Henderson
Sent:= Tue 9/20/2011 2:57 PM
To: Alan Amaral
Cc: qemu-devel@n= ongnu.org
Subject: Re: [Qemu-devel] pci_change_irq_level is broke= n...

On 09/20/2011 10:24 AM, Alan Amar=
al wrote:
> I'm not on this mailing list, so please CC me on any replies.  Thanks.
> =20
> I ran qemu with valgrind last night and found an error in the pci emul=
ation code, which may,
> or may not, be biting us.  So far the effects seem benign, although th=
ere exists the possibility
> of trashing random memory.
> =20
> In the function pci_change_irq_level() the argument irq_num is passed =
in as 0-3, and used
> as an index to change bus->irq_count[4].

I don't know what version of qemu you're using, but this is

      int *irq_count;

in current sources.  There's certainly no hard-coded "4".

>     assert(irq_num >=3D 0);
>     assert(irq_num < bus->nirq);
>     bus->irq_count[irq_num] +=3D change;
>     bus->set_irq(bus->irq_opaque, bus_irq_num, bus->irq_count=
[irq_num] !=3D 0);

This version with the asserts, though, could be done.  The site
that created the bus ought to match up nirq with the map function.


r~

--_09D111DF-7446-4110-90DF-17FCF756B087_-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59516) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R66CZ-0004RR-Nu for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:41:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R66CY-0004f2-GQ for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:41:55 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:42231) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R66CY-0004ew-3U for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:41:54 -0400 Message-ID: <4E78EC7B.9030808@web.de> Date: Tue, 20 Sep 2011 21:41:47 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <00218408-4F7E-47E8-9A3A-7515E5472C40@mimectl>, <4E78E207.5070308@twiddle.net> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig78D49CCFFFEDEFBC228B4E78" Sender: jan.kiszka@web.de Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alan Amaral Cc: "qemu-devel@nongnu.org" , Richard Henderson This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig78D49CCFFFEDEFBC228B4E78 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2011-09-20 21:19, Alan Amaral wrote: > QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard (That's an ambitious development version.) >=20 > You are correct, it's not hardcoded to 4. However, when it's allocated= the number of elements IS 4. Also, > there's a comment just above pci_set_irq which says: >=20 > /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */ > static void pci_set_irq(void *opaque, int irq_num, int level) >=20 > so, that implies to me that it's probably always 4... Sorry for the co= nfusion. Assuming you look at PIIX3: Yes, it allocates 4 IRQs - but only returns 0..3 via pci_slot_get_pirq. Xen uses some more, but also looks safe. Can you provide a backtrace where irq_num gets larger than 3 and writes beyond the end of irq_count? Do you have private patches in your tree? Jan --------------enig78D49CCFFFEDEFBC228B4E78 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk547H4ACgkQitSsb3rl5xR5sACg4XSIuZ9UZper4JHrLjwSldIa BhsAoMbdewqCjyEDAtFuRTmN4MhgkH8O =/M+S -----END PGP SIGNATURE----- --------------enig78D49CCFFFEDEFBC228B4E78-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40314) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R66RB-0006sj-Or for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:57:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R66RA-00085j-QS for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:57:01 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:35988) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R66RA-00085d-Ln for qemu-devel@nongnu.org; Tue, 20 Sep 2011 15:57:00 -0400 Received: by fxh13 with SMTP id 13so948701fxh.4 for ; Tue, 20 Sep 2011 12:56:59 -0700 (PDT) Sender: Richard Henderson Message-ID: <4E78F006.8070908@twiddle.net> Date: Tue, 20 Sep 2011 12:56:54 -0700 From: Richard Henderson MIME-Version: 1.0 References: <00218408-4F7E-47E8-9A3A-7515E5472C40@mimectl>, <4E78E207.5070308@twiddle.net> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alan Amaral Cc: "qemu-devel@nongnu.org" On 09/20/2011 12:19 PM, Alan Amaral wrote: > QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard > You are correct, it's not hardcoded to 4. However, when it's allocated the number of elements IS 4. Also, > there's a comment just above pci_set_irq which says: > > /* 0 <= irq_num <= 3. level must be 0 or 1 */ > static void pci_set_irq(void *opaque, int irq_num, int level) > so, that implies to me that it's probably always 4... The first use I examined was apb_pci.c: d->bus = pci_register_bus(&d->busdev.qdev, "pci", pci_apb_set_irq, pci_pbm_map_irq, d, &d->pci_mmio, get_system_io(), 0, 32); where the last argument indicates that we allocate 32 irqs, and the pci_pbm_map_irq function returns a value in the set { 0, 1, 2, 3, 16, 17, 18, 19 }. r~ From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39742) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6Pd8-0008N7-Lq for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:26:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R6Pd6-00084S-NK for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:26:38 -0400 Received: from server514c.exghost.com ([72.32.253.76]:2511 helo=server514.appriver.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6Pd6-00084I-B3 for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:26:36 -0400 MIME-Version: 1.0 content-class: urn:content-classes:message From: Alan Amaral Content-Type: multipart/alternative; boundary="_F091F466-ECE6-4648-91ED-454E96FDC5DA_" Message-ID: <179EEE32-724C-4349-B568-AF8A8B85721A@mimectl> Date: Wed, 21 Sep 2011 12:26:38 -0400 Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "jan.kiszka@web.de" Cc: "qemu-devel@nongnu.org" , "rth@twiddle.net" --_F091F466-ECE6-4648-91ED-454E96FDC5DA_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable =20 From: Jan Kiszka Sent: Tue 9/20/2011 3:41 PM To: Alan Amaral Cc: Richard Henderson; qemu-devel@nongnu.org Subject: Re: pci_change_irq_level is broken... > On 2011-09-20 21:19, Alan Amaral wrote: > > QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard >=20 > (That's an ambitious development version.) It's what we're using. > >=20 > > You are correct, it's not hardcoded to 4. However, when it's allocated= the number of elements IS 4. Also, > > there's a comment just above pci_set_irq which says: > >=20 > > /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */ > > static void pci_set_irq(void *opaque, int irq_num, int level) > >=20 > > so, that implies to me that it's probably always 4... Sorry for the co= nfusion. >=20 > Assuming you look at PIIX3: Yes, it allocates 4 IRQs - but only returns > 0..3 via pci_slot_get_pirq. Xen uses some more, but also looks safe. We are running under Xen and in this case it is using PIIX_NUM_PIRQS, which is 4, as the last arg to pci_bus_irqs(). =20 PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *pic, ram_addr_t ram_size) { PCIBus *b; b =3D i440fx_common_init("i440FX-xen", pi440fx_state, piix3_devfn, pic,= ram_size); pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq, (*pi440fx_state)->piix3, PIIX_NUM_PIRQS); return b; } Also, since we are using xen, xen_pci_slot_get_pirq is being used, which al= ways returns something >=3D 4 (at least when pci_dev->devfn > 0). Here's the code: int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) { return irq_num + ((pci_dev->devfn >> 3) << 2); } In the case I'm seeing devfn =3D=3D 9. I'm in gdb now, and at a breakpoint= at the error condition. The call is: Breakpoint 1, pci_change_irq_level (pci_dev=3D0x1c3a730, irq_num=3D0, chang= e=3D0) (gdb) p *pci_dev $1 =3D {qdev =3D {id =3D 0x0, state =3D DEV_STATE_INITIALIZED, opts =3D 0x0= ,=20 hotplugged =3D 0, info =3D 0xa08c00, parent_bus =3D 0x1af5700, num_gpio= _out =3D 0,=20 gpio_out =3D 0x0, num_gpio_in =3D 0, gpio_in =3D 0x0, child_bus =3D { lh_first =3D 0x1c3b0c8}, num_child_bus =3D 2, sibling =3D { le_next =3D 0x1c37440, le_prev =3D 0x1c9dcb0}, instance_id_alias =3D = -1,=20 alias_required_for_version =3D 0}, config =3D 0x1c3b8e0 "\206\200\020p"= ,=20 cmask =3D 0x1c3b9f0 "\377\377\377\377", wmask =3D 0x1c3bb00 "",=20 w1cmask =3D 0x1c3bc10 "", used =3D 0x1c3bd20 "", bus =3D 0x1af5700, devfn= =3D 9,=20 name =3D "piix3-ide", '\000' , io_regions =3D {{addr = =3D 0,=20 size =3D 0, filtered_size =3D 0, type =3D 0 '\000', map_func =3D 0,=20 ram_addr =3D 0}, {addr =3D 0, size =3D 0, filtered_size =3D 0, type = =3D 0 '\000',=20 map_func =3D 0, ram_addr =3D 0}, {addr =3D 0, size =3D 0, filtered_si= ze =3D 0,=20 type =3D 0 '\000', map_func =3D 0, ram_addr =3D 0}, {addr =3D 0, size= =3D 0,=20 filtered_size =3D 0, type =3D 0 '\000', map_func =3D 0, ram_addr =3D = 0}, { addr =3D 18446744073709551615, size =3D 16, filtered_size =3D 16,=20 type =3D 1 '\001', map_func =3D 0x60095c , ram_addr =3D 16= }, { addr =3D 0, size =3D 0, filtered_size =3D 0, type =3D 0 '\000', map_f= unc =3D 0,=20 ram_addr =3D 0}, {addr =3D 0, size =3D 0, filtered_size =3D 0, type = =3D 0 '\000',=20 map_func =3D 0, ram_addr =3D 0}},=20 config_read =3D 0x5be0c9 ,=20 config_write =3D 0x5be192 , irq =3D 0x1c3be30,= =20 irq_state =3D 0 '\000', cap_present =3D 16, msix_cap =3D 0 '\000',=20 msix_entries_nr =3D 0, msix_table_page =3D 0x0, msix_mmio_index =3D 0,=20 msix_entry_used =3D 0x0, msix_bar_size =3D 0, version_id =3D 2,=20 msi_cap =3D 0 '\000', exp =3D {exp_cap =3D 0 '\000', hpev_intx =3D 0,=20 hpev_notified =3D false, aer_cap =3D 0, aer_log =3D {log_num =3D 0, log= _max =3D 0,=20 log =3D 0x0}, aer_intx =3D 0}, romfile =3D 0x0, rom_offset =3D 0, rom= _bar =3D 1} (gdb) p *bus $2 =3D {qbus =3D {parent =3D 0x1af41b0, info =3D 0x9e4e60, name =3D 0x17369= 10 "pci.0",=20 allow_hotplug =3D 1, qdev_allocated =3D 1, children =3D {lh_first =3D 0= x1cd3520},=20 sibling =3D {le_next =3D 0x0, le_prev =3D 0x1af4200}}, devfn_min =3D 0 = '\000',=20 set_irq =3D 0x62e600 ,=20 map_irq =3D 0x62e5b2 ,=20 hotplug =3D 0x5d71b8 , hotplug_qdev =3D 0x1cd1600,= =20 irq_opaque =3D 0x1af6fd0, devices =3D {0x1af6150, 0x0, 0x0, 0x0, 0x0, 0x0= , 0x0,=20 0x0, 0x1af6fd0, 0x1c3a730, 0x1cd09c0, 0x1cd1600, 0x0, 0x0, 0x0, 0x0,=20 0x1af7be0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1c37440, 0x0, 0x0, 0x0,= =20 0x0, 0x0, 0x0, 0x0, 0x1cbe010, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,=20 0x1c9dc50, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1cd3520,=20 0x0 }, parent_dev =3D 0x0, mem_base =3D 0, child =3D= { lh_first =3D 0x0}, sibling =3D {le_next =3D 0x0, le_prev =3D 0x0}, nirq= =3D 4,=20 irq_count =3D 0x1af7ab0} (gdb) p bus->nirq =20 $3 =3D 4 (gdb) p pci_dev->devfn $4 =3D 9 (gdb) p bus->map_irq $5 =3D (pci_map_irq_fn) 0x62e5b2 After the first call to map_irq irq_num is changed to 4: (gdb) n 119 bus =3D pci_dev->bus; (gdb)=20 120 irq_num =3D bus->map_irq(pci_dev, irq_num); (gdb)=20 121 if (bus->set_irq) (gdb) p irq_num $6 =3D 4 (gdb) p bus->set_irq=20 $7 =3D (pci_set_irq_fn) 0x62e600 (gdb) n 125 bus->irq_count[irq_num] +=3D change; (gdb) p irq_num $8 =3D 4 (gdb) p bus->nirq=20 $9 =3D 4 (gdb) whatis bus->irq_count[0]=20 type =3D int (gdb) p sizeof(bus->irq_count[0]) $10 =3D 4 (gdb)=20 This all clearly shows that the irq_count array has 4 elements and the code is trying to read and write irq_count[4], which is outside of the malloc'd = block. > Can you provide a backtrace where irq_num gets larger than 3 and writes > beyond the end of irq_count? Do you have private patches in your tree? Backtrace is below. Oh, yes, we're using private patches, but I don't beli= eve that this code has been patched at all. > Jan >=20 Here's a stack trace from valgrind. You'll note that the allocated block s= ize is 16, which is 4 ints, and I verified above in gdb that bus->irq_nirq was 4. =3D=3D1901=3D=3D=3D=3D1901=3D=3D 52 errors in context 17 of 125:=3D=3D1901= =3D=3D Thread 1:=3D=3D1901=3D=3D Invalid read of size 4=3D=3D1901=3D=3D = at 0x5BB7FB: pci_change_irq_level (pci.c:126)=3D=3D1901=3D=3D by 0x5BE02= 1: pci_update_irq_disabled (pci.c:1083)=3D=3D1901=3D=3D by 0x5BE303: pci= _default_write_config (pci.c:1116)=3D=3D1901=3D=3D by 0x5C4724: pci_data= _write (pci_host.c:60)=3D=3D1901=3D=3D by 0x5C4923: pci_host_data_write = (pci_host.c:109)=3D=3D1901=3D=3D by 0x428B3B: ioport_simple_writew (rwha= ndler.c:50)=3D=3D1901=3D=3D by 0x481EF0: ioport_write (ioport.c:81)=3D= =3D1901=3D=3D by 0x48285B: cpu_outw (ioport.c:273)=3D=3D1901=3D=3D by= 0x62ED59: do_outp (xen-all.c:307)=3D=3D1901=3D=3D by 0x62EEDA: cpu_iore= q_pio (xen-all.c:335)=3D=3D1901=3D=3D by 0x62F28E: handle_ioreq (xen-all= .c:396)=3D=3D1901=3D=3D by 0x62F5A6: cpu_handle_ioreq (xen-all.c:464)=3D= =3D1901=3D=3D by 0x4BD078: qemu_iohandler_poll (iohandler.c:120)=3D=3D19= 01=3D=3D by 0x5AE4F2: main_loop_wait (vl.c:1359)=3D=3D1901=3D=3D by 0= x5AE5F7: main_loop (vl.c:1404)=3D=3D1901=3D=3D by 0x5B2FBC: main (vl.c:3= 436)=3D=3D1901=3D=3D Address 0x2b7f9d30 is 0 bytes after a block of size 1= 6 alloc'd=3D=3D1901=3D=3D at 0x4C279FC: calloc (vg_replace_malloc.c:467)= =3D=3D1901=3D=3D by 0x42A6B5: qemu_mallocz (qemu-malloc.c:71)=3D=3D1901= =3D=3D by 0x5BC01E: pci_bus_irqs (pci.c:296)=3D=3D1901=3D=3D by 0x66D= 6B7: i440fx_xen_init (piix_pci.c:304)=3D=3D1901=3D=3D by 0x670D88: pc_in= it1 (pc_piix.c:135)=3D=3D1901=3D=3D by 0x671279: pc_init_pci_no_kvmclock= (pc_piix.c:236)=3D=3D1901=3D=3D by 0x671385: pc_xen_hvm_init (pc_piix.c= :266)=3D=3D1901=3D=3D by 0x5B2C01: main (vl.c:3281)=3D=3D1901=3D=3D=3D= =3D1901=3D=3D 52 errors in context 18 of 125:=3D=3D1901=3D=3D Invalid write= of size 4=3D=3D1901=3D=3D at 0x5BB7D9: pci_change_irq_level (pci.c:125)= =3D=3D1901=3D=3D by 0x5BE021: pci_update_irq_disabled (pci.c:1083)=3D=3D= 1901=3D=3D by 0x5BE303: pci_default_write_config (pci.c:1116)=3D=3D1901= =3D=3D by 0x5C4724: pci_data_write (pci_host.c:60)=3D=3D1901=3D=3D by= 0x5C4923: pci_host_data_write (pci_host.c:109)=3D=3D1901=3D=3D by 0x428= B3B: ioport_simple_writew (rwhandler.c:50)=3D=3D1901=3D=3D by 0x481EF0: = ioport_write (ioport.c:81)=3D=3D1901=3D=3D by 0x48285B: cpu_outw (ioport= .c:273)=3D=3D1901=3D=3D by 0x62ED59: do_outp (xen-all.c:307)=3D=3D1901= =3D=3D by 0x62EEDA: cpu_ioreq_pio (xen-all.c:335)=3D=3D1901=3D=3D by = 0x62F28E: handle_ioreq (xen-all.c:396)=3D=3D1901=3D=3D by 0x62F5A6: cpu_= handle_ioreq (xen-all.c:464)=3D=3D1901=3D=3D by 0x4BD078: qemu_iohandler= _poll (iohandler.c:120)=3D=3D1901=3D=3D by 0x5AE4F2: main_loop_wait (vl.= c:1359)=3D=3D1901=3D=3D by 0x5AE5F7: main_loop (vl.c:1404)=3D=3D1901=3D= =3D by 0x5B2FBC: main (vl.c:3436)=3D=3D1901=3D=3D Address 0x2b7f9d30 is= 0 bytes after a block of size 16 alloc'd=3D=3D1901=3D=3D at 0x4C279FC: = calloc (vg_replace_malloc.c:467)=3D=3D1901=3D=3D by 0x42A6B5: qemu_mallo= cz (qemu-malloc.c:71)=3D=3D1901=3D=3D by 0x5BC01E: pci_bus_irqs (pci.c:2= 96)=3D=3D1901=3D=3D by 0x66D6B7: i440fx_xen_init (piix_pci.c:304)=3D=3D1= 901=3D=3D by 0x670D88: pc_init1 (pc_piix.c:135)=3D=3D1901=3D=3D by 0x= 671279: pc_init_pci_no_kvmclock (pc_piix.c:236)=3D=3D1901=3D=3D by 0x671= 385: pc_xen_hvm_init (pc_piix.c:266)=3D=3D1901=3D=3D by 0x5B2C01: main (= vl.c:3281)=3D=3D1901=3D=3D --_F091F466-ECE6-4648-91ED-454E96FDC5DA_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

=20

From: Jan Kiszka
Sent: = Tue 9/20/2011 3:41 PM
To: Alan Amaral
Cc: Richard Hende= rson; qemu-devel@nongnu.org
Subject: Re: pci_change_irq_level is = broken...

> On 2011-09-20 21:19, Alan Amaral wrote:
>= ; > QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bella= rd
>
> (That's an ambitious development version.)

It's what we're using.

> >
> > You ar= e correct, it's not hardcoded to 4. However, when it's allocated the = number of elements IS 4. Also,
> > there's a comment just ab= ove pci_set_irq which says:
> >
> > /* 0 <=3D irq_num= <=3D 3. level must be 0 or 1 */
> > static void pci_set_irq(vo= id *opaque, int irq_num, int level)
> >
> > so, that imp= lies to me that it's probably always 4... Sorry for the confusion.>
> Assuming you look at PIIX3: Yes, it allocates 4 IRQs - but o= nly returns
> 0..3 via pci_slot_get_pirq. Xen uses some more, but als= o looks safe.

We are running under Xen and in this case it is using PIIX_N= UM_PIRQS,

which is 4, as the last arg to pci_bus_irqs().

PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, = int *piix3_devfn,
= &nb= sp; qemu_irq *pic, ram_addr_t ram_size)
{
PC= IBus *b;

    b =3D i440fx_common_init("i440FX-xen", pi= 440fx_state, piix3_devfn, pic, ram_size);
    pci_bus_irq= s(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,=
            = ;     (*pi440fx_state)->piix3, PIIX_NUM_PIRQS);

return b;
}

Also, since we are using xen, xen_pci_slot_get_pirq is&= nbsp;being used, which always returns

something >=3D 4 (at lea= st when pci_dev->devfn > 0). Here's the code:

int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num){
return irq_num + ((pci_dev->devfn >> 3) = << 2);
}

In the case I'm seeing devfn =3D=3D 9. I'm in gdb now,= and at a breakpoint at the error condition.

The call is:

Breakpoint 1, pci_change_irq_level (pci_dev=3D0x1c3a730, irq= _num=3D0, change=3D0)

(gdb) p *pci_dev
$1 =3D {qdev =3D {id =3D 0x0, state =3D = DEV_STATE_INITIALIZED, opts =3D 0x0,
    hotplugged =3D = 0, info =3D 0xa08c00, parent_bus =3D 0x1af5700, num_gpio_out =3D 0,
&nb= sp;   gpio_out =3D 0x0, num_gpio_in =3D 0, gpio_in =3D 0x0, child= _bus =3D {
      lh_first =3D 0x1c3b0c8}, num_c= hild_bus =3D 2, sibling =3D {
      le_next =3D= 0x1c37440, le_prev =3D 0x1c9dcb0}, instance_id_alias =3D -1,
&nb= sp; alias_required_for_version =3D 0}, config =3D 0x1c3b8e0 "\206\200= \020p",
cmask =3D 0x1c3b9f0 "\377\377\377\377", wmask =3D 0x1c3b= b00 "",
w1cmask =3D 0x1c3bc10 "", used =3D 0x1c3bd20 "", bus =3D= 0x1af5700, devfn =3D 9,
name =3D "= piix3-ide", '\000' <repeats 54 times>, io_regions =3D {{addr =3D 0, <= BR>      size =3D 0, filtered_size =3D 0, type =3D= 0 '\000', map_func =3D 0,
      ram_addr =3D = 0}, {addr =3D 0, size =3D 0, filtered_size =3D 0, type =3D 0 '\000',
&n= bsp;     map_func =3D 0, ram_addr =3D 0}, {addr =3D 0, = size =3D 0, filtered_size =3D 0,
      type = =3D 0 '\000', map_func =3D 0, ram_addr =3D 0}, {addr =3D 0, size =3D 0,       filtered_size =3D 0, type =3D 0 '\000', map= _func =3D 0, ram_addr =3D 0}, {
      addr =3D = 18446744073709551615, size =3D 16, filtered_size =3D 16,
  &n= bsp;   type =3D 1 '\001', map_func =3D 0x60095c <bmdma_map>= , ram_addr =3D 16}, {
      addr =3D 0, size = =3D 0, filtered_size =3D 0, type =3D 0 '\000', map_func =3D 0,
&n= bsp;    ram_addr =3D 0}, {addr =3D 0, size =3D 0, filtered_s= ize =3D 0, type =3D 0 '\000',
      map_func = =3D 0, ram_addr =3D 0}},
config_read =3D 0x5be0c9 <pci_defaul= t_read_config>,
config_write =3D 0x5be192 <pci_default_wri= te_config>, irq =3D 0x1c3be30,
irq_state =3D 0 '\000', cap_pr= esent =3D 16, msix_cap =3D 0 '\000',
msix_entries_nr =3D 0, msix= _table_page =3D 0x0, msix_mmio_index =3D 0,
msix_entry_used= =3D 0x0, msix_bar_size =3D 0, version_id =3D 2,
msi_cap =3D 0 '= \000', exp =3D {exp_cap =3D 0 '\000', hpev_intx =3D 0,
  &nbs= p; hpev_notified =3D false, aer_cap =3D 0, aer_log =3D {log_num =3D 0, log_= max =3D 0,
      log =3D 0x0}, aer_intx =3D 0}= , romfile =3D 0x0, rom_offset =3D 0, rom_bar =3D 1}

(gdb) p *bus
$2 =3D {qbus =3D {parent =3D 0x1af41b0, info= =3D 0x9e4e60, name =3D 0x1736910 "pci.0",
    allow_hot= plug =3D 1, qdev_allocated =3D 1, children =3D {lh_first =3D 0x1cd3520},     sibling =3D {le_next =3D 0x0, le_prev =3D 0x1af4200}},= devfn_min =3D 0 '\000',
set_irq =3D 0x62e= 600 <xen_piix3_set_irq>,
map_irq =3D 0x62e5b2 <xen_pci_= slot_get_pirq>,
hotplug =3D 0x5d71b8 <piix4_device_= hotplug>, hotplug_qdev =3D 0x1cd1600,
irq_opaque =3D 0x1af6fd= 0, devices =3D {0x1af6150, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
  &n= bsp; 0x0, 0x1af6fd0, 0x1c3a730, 0x1cd09c0, 0x1cd1600, 0x0, 0x0, 0x0, 0x0, <= BR>    0x1af7be0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1c374= 40, 0x0, 0x0, 0x0,
    0x0, 0x0, 0x0, 0x0, 0x1cbe010, 0x= 0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
    0x1c9dc50, 0x0, 0x0= , 0x0, 0x0, 0x0, 0x0, 0x0, 0x1cd3520,
    0x0 <repeat= s 207 times>}, parent_dev =3D 0x0, mem_base =3D 0, child =3D {
=    lh_first =3D 0x0}, sibling =3D {le_next =3D 0x0, le_prev =3D 0= x0}, nirq =3D 4,
irq_count =3D 0x1a= f7ab0}

(gdb) p bus->nirq
$3 =3D 4(gdb) p pci_dev->devfn
$4 =3D 9
(gdb) p bus->map_irq
$5 =3D= (pci_map_irq_fn) 0x62e5b2 <xen_pci_slot_get_pirq>

After the first call to map_irq irq_num is changed to 4:

(gdb) n
119       &nbs= p; bus =3D pci_dev->bus;
(gdb)
120     &= nbsp;   irq_num =3D bus->map_irq(pci_dev, irq_num);
(gdb) <= BR>121         if (bus->set_irq)=
(gdb) p irq_num
$6 =3D 4
(gdb) p bus->set_irq
$7 =3D (pci_= set_irq_fn) 0x62e600 <xen_piix3_set_irq>
(gdb) n
125  = ;   bus->irq_count[irq_num] +=3D change;
(gdb) p irq_num$8 =3D 4
(gdb) p bus->nirq
$9 =3D 4
(gdb) whatis bus->irq_= count[0]
type =3D int
(gdb) p sizeof(bus->irq_count[0])
$10 = =3D 4
(gdb)

This all clearly shows that the irq_count array has 4 elemen= ts and the code

is trying to read and write irq_count[4], which is outside o= f the malloc'd block.

> Can you provide a backtrace where irq_num gets larg= er than 3 and writes
> beyond the end of irq_count? Do you have priva= te patches in your tree?

Backtrace is below. Oh, yes, we're using private patch= es, but I don't believe

that this code has been patched at all.

> Jan
>

Here's a stack trace from valgrind.&nb= sp; You'll note that the allocated block size is 16,

which is 4 ints, and I verified above in gdb that bus->ir= q_nirq was 4.

=3D=3D1901=3D=3D
=3D=3D1901=3D=3D= 52 errors in context 17 of 125:
=3D=3D1901=3D=3D Thread 1:
=3D=3D190= 1=3D=3D Invalid read of size 4
=3D=3D1901=3D=3D    at 0x5= BB7FB: pci_change_irq_level (pci.c:126)
=3D=3D1901=3D=3D  &nbs= p; by 0x5BE021: pci_update_irq_disabled (pci.c:1083)
=3D=3D1901=3D=3D&nb= sp;   by 0x5BE303: pci_default_write_config (pci.c:1116)
=3D= =3D1901=3D=3D    by 0x5C4724: pci_data_write (pci_host.c:60)=
=3D=3D1901=3D=3D    by 0x5C4923: pci_host_data_write (pc= i_host.c:109)
=3D=3D1901=3D=3D    by 0x428B3B: ioport_sim= ple_writew (rwhandler.c:50)
=3D=3D1901=3D=3D    by 0x481E= F0: ioport_write (ioport.c:81)
=3D=3D1901=3D=3D    by 0x4= 8285B: cpu_outw (ioport.c:273)
=3D=3D1901=3D=3D    by 0x6= 2ED59: do_outp (xen-all.c:307)
=3D=3D1901=3D=3D    by 0x6= 2EEDA: cpu_ioreq_pio (xen-all.c:335)
=3D=3D1901=3D=3D    = by 0x62F28E: handle_ioreq (xen-all.c:396)
=3D=3D1901=3D=3D  &n= bsp; by 0x62F5A6: cpu_handle_ioreq (xen-all.c:464)
=3D=3D1901=3D=3D = ;   by 0x4BD078: qemu_iohandler_poll (iohandler.c:120)
=3D=3D1= 901=3D=3D    by 0x5AE4F2: main_loop_wait (vl.c:1359)
=3D= =3D1901=3D=3D    by 0x5AE5F7: main_loop (vl.c:1404)
=3D= =3D1901=3D=3D    by 0x5B2FBC: main (vl.c:3436)
=3D=3D1901= =3D=3D Address 0x2b7f9d30 is 0 bytes after a block of size 16 alloc'd=
=3D=3D1901=3D=3D    at 0x4C279FC: calloc (vg_replace_mal= loc.c:467)
=3D=3D1901=3D=3D    by 0x42A6B5: qemu_mallocz = (qemu-malloc.c:71)
=3D=3D1901=3D=3D    by 0x5BC01E: pci_b= us_irqs (pci.c:296)
=3D=3D1901=3D=3D    by 0x66D6B7: i440= fx_xen_init (piix_pci.c:304)
=3D=3D1901=3D=3D    by 0x670= D88: pc_init1 (pc_piix.c:135)
=3D=3D1901=3D=3D    by 0x67= 1279: pc_init_pci_no_kvmclock (pc_piix.c:236)
=3D=3D1901=3D=3D &nbs= p; by 0x671385: pc_xen_hvm_init (pc_piix.c:266)
=3D=3D1901=3D=3D&n= bsp;   by 0x5B2C01: main (vl.c:3281)

=3D= =3D1901=3D=3D
=3D=3D1901=3D=3D 52 errors in context 18 of 125:
=3D=3D= 1901=3D=3D Invalid write of size 4
=3D=3D1901=3D=3D    at= 0x5BB7D9: pci_change_irq_level (pci.c:125)
=3D=3D1901=3D=3D  = by 0x5BE021: pci_update_irq_disabled (pci.c:1083)
=3D=3D1901=3D= =3D    by 0x5BE303: pci_default_write_config (pci.c:1116)=3D=3D1901=3D=3D    by 0x5C4724: pci_data_write (pci_host.c= :60)
=3D=3D1901=3D=3D    by 0x5C4923: pci_host_data_write= (pci_host.c:109)
=3D=3D1901=3D=3D    by 0x428B3B: ioport= _simple_writew (rwhandler.c:50)
=3D=3D1901=3D=3D    by 0x= 481EF0: ioport_write (ioport.c:81)
=3D=3D1901=3D=3D    by= 0x48285B: cpu_outw (ioport.c:273)
=3D=3D1901=3D=3D    by= 0x62ED59: do_outp (xen-all.c:307)
=3D=3D1901=3D=3D    by= 0x62EEDA: cpu_ioreq_pio (xen-all.c:335)
=3D=3D1901=3D=3D  &nb= sp; by 0x62F28E: handle_ioreq (xen-all.c:396)
=3D=3D1901=3D=3D &nbs= p; by 0x62F5A6: cpu_handle_ioreq (xen-all.c:464)
=3D=3D1901=3D=3D&= nbsp;   by 0x4BD078: qemu_iohandler_poll (iohandler.c:120)
=3D= =3D1901=3D=3D    by 0x5AE4F2: main_loop_wait (vl.c:1359)
= =3D=3D1901=3D=3D    by 0x5AE5F7: main_loop (vl.c:1404)
= =3D=3D1901=3D=3D    by 0x5B2FBC: main (vl.c:3436)
=3D=3D1= 901=3D=3D Address 0x2b7f9d30 is 0 bytes after a block of size 16 allo= c'd
=3D=3D1901=3D=3D    at 0x4C279FC: calloc (vg_replace_= malloc.c:467)
=3D=3D1901=3D=3D    by 0x42A6B5: qemu_mallo= cz (qemu-malloc.c:71)
=3D=3D1901=3D=3D    by 0x5BC01E: pc= i_bus_irqs (pci.c:296)
=3D=3D1901=3D=3D    by 0x66D6B7: i= 440fx_xen_init (piix_pci.c:304)
=3D=3D1901=3D=3D    by 0x= 670D88: pc_init1 (pc_piix.c:135)
=3D=3D1901=3D=3D    by 0= x671279: pc_init_pci_no_kvmclock (pc_piix.c:236)
=3D=3D1901=3D=3D &= nbsp; by 0x671385: pc_xen_hvm_init (pc_piix.c:266)
=3D=3D1901=3D= =3D    by 0x5B2C01: main (vl.c:3281)
=3D=3D1901=3D=3D

--_F091F466-ECE6-4648-91ED-454E96FDC5DA_-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6Pk6-0003Jl-Az for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:33:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R6Pk2-0001Jj-4W for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:33:50 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:44007) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6Pk1-0001JY-LJ for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:33:46 -0400 Message-ID: <4E7A11E3.4030609@web.de> Date: Wed, 21 Sep 2011 18:33:39 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <179EEE32-724C-4349-B568-AF8A8B85721A@mimectl> In-Reply-To: <179EEE32-724C-4349-B568-AF8A8B85721A@mimectl> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigB5EF92C8176F4A7CBF26189A" Sender: jan.kiszka@web.de Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alan Amaral Cc: "qemu-devel@nongnu.org" , "rth@twiddle.net" This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigB5EF92C8176F4A7CBF26189A Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2011-09-21 18:26, Alan Amaral wrote: > =20 >=20 >=20 >=20 > From: Jan Kiszka > Sent: Tue 9/20/2011 3:41 PM > To: Alan Amaral > Cc: Richard Henderson; qemu-devel@nongnu.org > Subject: Re: pci_change_irq_level is broken... >=20 >> On 2011-09-20 21:19, Alan Amaral wrote: >>> QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellar= d >> >> (That's an ambitious development version.) >=20 > It's what we're using. >=20 >>> >>> You are correct, it's not hardcoded to 4. However, when it's allocat= ed the number of elements IS 4. Also, >>> there's a comment just above pci_set_irq which says: >>> >>> /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */ >>> static void pci_set_irq(void *opaque, int irq_num, int level) >>> >>> so, that implies to me that it's probably always 4... Sorry for the = confusion. >> >> Assuming you look at PIIX3: Yes, it allocates 4 IRQs - but only return= s >> 0..3 via pci_slot_get_pirq. Xen uses some more, but also looks safe. >=20 > We are running under Xen and in this case it is using PIIX_NUM_PIRQS, > which is 4, as the last arg to pci_bus_irqs(). =20 >=20 > PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, int *piix3_devf= n, > qemu_irq *pic, ram_addr_t ram_size) > { > PCIBus *b; > b =3D i440fx_common_init("i440FX-xen", pi440fx_state, piix3_devfn, = pic, ram_size); > pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq, > (*pi440fx_state)->piix3, PIIX_NUM_PIRQS); > return b; > } bf09551a6b, aka "xen: fix interrupt routing". Jan --------------enigB5EF92C8176F4A7CBF26189A Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk56EeYACgkQitSsb3rl5xQJCQCgpYX75IB6WoANmY+vhEjZclKa 5wEAoLNKUyYx2xlAPfER8YriiBVGQZo0 =Hy2X -----END PGP SIGNATURE----- --------------enigB5EF92C8176F4A7CBF26189A-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6Pkd-00040s-Qz for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:34:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R6Pkc-0001Pe-Qk for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:34:23 -0400 Received: from server514c.exghost.com ([72.32.253.76]:4818 helo=server514.appriver.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6Pkc-0001PT-Lx for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:34:22 -0400 MIME-Version: 1.0 From: Alan Amaral In-Reply-To: <4E78F006.8070908@twiddle.net> References: <00218408-4F7E-47E8-9A3A-7515E5472C40@mimectl>, <4E78E207.5070308@twiddle.net> , <4E78F006.8070908@twiddle.net> Content-Type: multipart/alternative; boundary="_92BADF2C-4764-4A5C-B715-4270C309CDD9_" Message-ID: <979561B6-2F67-46E0-B71D-403B8353F71D@mimectl> Date: Wed, 21 Sep 2011 12:34:24 -0400 Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: "qemu-devel@nongnu.org" --_92BADF2C-4764-4A5C-B715-4270C309CDD9_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Ok. Cases to the contrary notwithstanding, what I'm seeing is the size of = the irq_count array is 4 and the code is clearly accessing entry 4 and beyond. We are using Xe= n (sorry if I didn't mention that earlier). #0 pci_change_irq_level (pci_dev=3D0x1c3a730, irq_num=3D4, change=3D0) at /home/aamaral/orc-next/orc-tree/orc-xen/xen-4.0/tools/ioemu-dir/hw/p= ci.c:125 125 bus->irq_count[irq_num] +=3D change; (gdb) p irq_num $23 =3D 4 <- It was zero on entry to this function but was c= hanged by xen_pci_slot_get_pirq (gdb) p bus->nirq=20 $24 =3D 4 (gdb) Please see my earlier reply to Jan Kiszka for full details. Thanks, Alan From: Richard Henderson Sent: Tue 9/20/2011 3:56 PM To: Alan Amaral Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] pci_change_irq_level is broken... On 09/20/2011 12:19 PM, Alan Amaral wrote: > QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard > You are correct, it's not hardcoded to 4. However, when it's allocated t= he number of elements IS 4. Also, > there's a comment just above pci_set_irq which says: > =20 > /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */ > static void pci_set_irq(void *opaque, int irq_num, int level) > so, that implies to me that it's probably always 4... The first use I examined was apb_pci.c: d->bus =3D pci_register_bus(&d->busdev.qdev, "pci", pci_apb_set_irq, pci_pbm_map_irq, d, &d->pci_mmio, get_system_io(), 0, 32); where the last argument indicates that we allocate 32 irqs,=20 and the pci_pbm_map_irq function returns a value in the set { 0, 1, 2, 3, 16, 17, 18, 19 }. r~ --_92BADF2C-4764-4A5C-B715-4270C309CDD9_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

Ok. Cases = to the contrary notwithstanding, what I'm seeing is the size of the irq_cou= nt array

is 4 and the cod= e is clearly accessing entry 4 and beyon= d. We are using Xen (sorry if I didn't

mention that earlier).

#0 pci_change_irq_level (pci_dev=3D0x1c3a730, irq_num= =3D4, change=3D0)
    at /home/aamaral/orc-next/orc-tree/= orc-xen/xen-4.0/tools/ioemu-dir/hw/pci.c:125
125    = bus->irq_count[irq_num] +=3D change;
(gdb) p irq_num
$23 =3D 4&nb= sp;            =    <- It was zero on entry to this function but was chang= ed by xen_pci_slot_get_pirq
(gdb) p bus->nirq
$24 =3D 4
(gdb)<= /DIV>

Please see my earlier reply to J= an Kiszka for full details.

Thanks,

Alan

From: Richard Henderson
Sent:= Tue 9/20/2011 3:56 PM
To: Alan Amaral
Cc: qemu-devel@n= ongnu.org
Subject: Re: [Qemu-devel] pci_change_irq_level is broke= n...

On 09/20/2011 12:19 PM, Alan Amar=
al wrote:
> QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard
> You are correct, it's not hardcoded to 4.  However, when it's allocate=
d the number of elements IS 4.  Also,
> there's a comment just above pci_set_irq which says:
> =20
> /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */
> static void pci_set_irq(void *opaque, int irq_num, int level)
> so, that implies to me that it's probably always 4...

The first use I examined was apb_pci.c:

    d->bus =3D pci_register_bus(&d->busdev.qdev, "pci",
                              pci_apb_set_irq, pci_pbm_map_irq, d,
                              &d->pci_mmio,
                              get_system_io(),
                              0, 32);

where the last argument indicates that we allocate 32 irqs,=20
and the pci_pbm_map_irq function returns a value in the set
{ 0, 1, 2, 3, 16, 17, 18, 19 }.


r~

--_92BADF2C-4764-4A5C-B715-4270C309CDD9_-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40046) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6PoS-0006vz-JW for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:38:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R6PoR-0002cp-N4 for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:38:20 -0400 Received: from server514d.exghost.com ([72.32.253.69]:1714 helo=server514.appriver.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R6PoR-0002cg-IT for qemu-devel@nongnu.org; Wed, 21 Sep 2011 12:38:19 -0400 MIME-Version: 1.0 Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable From: Alan Amaral In-Reply-To: <179EEE32-724C-4349-B568-AF8A8B85721A@mimectl> References: <179EEE32-724C-4349-B568-AF8A8B85721A@mimectl> Message-ID: <97C0D929-2EF8-431F-978E-7AA8678BADA1@mimectl> Date: Wed, 21 Sep 2011 12:38:21 -0400 Subject: Re: [Qemu-devel] pci_change_irq_level is broken... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alan Amaral , jan.kiszka@web.de Cc: qemu-devel@nongnu.org, rth@twiddle.net

Excellent! Thanks...

Alan

From: Jan Kiszka
S= ent: Wed 9/21/2011 12:33 PM
To: Alan Amaral
Cc: qem= u-devel@nongnu.org; rth@twiddle.net
Subject: Re: pci_change_irq_l= evel is broken...

On 2011-09-21 18:26, Alan Amaral =
wrote:
>  =20
>=20
>=20
>=20
> From: Jan Kiszka
> Sent: Tue 9/20/2011 3:41 PM
> To: Alan Amaral
> Cc: Richard Henderson; qemu-devel@nongnu.org
> Subject: Re: pci_change_irq_level is broken...
>=20
>> On 2011-09-20 21:19, Alan Amaral wrote:
>>> QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice=
 Bellard
>>
>> (That's an ambitious development version.)
>=20
> It's what we're using.
>=20
>>>
>>> You are correct, it's not hardcoded to 4.  However, when it's =
allocated the number of elements IS 4.  Also,
>>> there's a comment just above pci_set_irq which says:
>>>
>>> /* 0 <=3D irq_num <=3D 3. level must be 0 or 1 */
>>> static void pci_set_irq(void *opaque, int irq_num, int level)
>>>
>>> so, that implies to me that it's probably always 4...  Sorry f=
or the confusion.
>>
>> Assuming you look at PIIX3: Yes, it allocates 4 IRQs - but only re=
turns
>> 0..3 via pci_slot_get_pirq. Xen uses some more, but also looks saf=
e.
>=20
> We are running under Xen and in this case it is using PIIX_NUM_PIRQS,
> which is 4, as the last arg to pci_bus_irqs(). =20
>=20
> PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, int *piix3_dev=
fn,
>                         qemu_irq *pic, ram_addr_t ram_size)
> {
>     PCIBus *b;
>     b =3D i440fx_common_init("i440FX-xen", pi440fx_state, piix3_devfn,=
 pic, ram_size);
>     pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
>                  (*pi440fx_state)->piix3, PIIX_NUM_PIRQS);
>     return b;
> }

bf09551a6b, aka "xen: fix interrupt routing".

Jan