Re: [Qemu-devel] [PATCH v7 3/4] piix_pci: optimize set irq path

2011-05-22 Thread Stefan Weil

Am 01.04.2011 13:43, schrieb Isaku Yamahata:

optimize irq routing in piix_pic.c which has been a TODO.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is 
addressed

by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

test:
- create VM with 4 e1000 nics in different pci slots
(i.e. fn=0 for each e1000)
Thus those e1000's INTA are connected to each PIRQ[A-D].
- run linux as guest and saw each devices triggers interrupt
by seeing /proc/interrupts. And then confirmed that each PIRQ[A-D]
surely asserted interrupts.
Because irq 10 and 11 are shared by 4 e1000's, it only one NIC is 
activated

with ifconfig ethN up/down when counting interrupts.

Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
Changes v4 - v5:
- fix piix_set_irq_pic()

Changes v3 - v4:
- replace irq_num with pirq or pci_intx

Changes v1 - v2:
- some minor clean ups
- commit log message
---
hw/piix_pci.c | 101 
+++-

1 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 35e420c..7ffb821 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c

...


Hi,

I got a regression report which was obviously caused by this commit:

$ git bisect bad
ab431c283e7055bcd6fb622f212bb29e84a6a134 is the first bad commit
commit ab431c283e7055bcd6fb622f212bb29e84a6a134
Author: Isaku Yamahata yamah...@valinux.co.jp
Date:   Fri Apr 1 20:43:23 2011 +0900

My test scenario:

i386-softmmu/qemu -L pc-bios -cdrom tinycore_3.5.1.iso -m 256 -boot d 
-net nic -net user -net dump


* Boot the default configuration (tinycore, or tinycore debug).
* Wait until X Windows is up.
* Check the size of qemu*.pcap. If it is only a few bytes, the test failed.
  Emulated networking then does not work, Tinycore does not get an IP 
address via DHCP.


The original report used tinycore_3.6.iso and the latest w32 binaries 
from qemu.weilnetz.de
with the same result. It also says that other Linux distributions show 
the same problem

(Fedora-14-i686-Live-Desktop.iso).

Regards
Stefan W.





Re: [Qemu-devel] [PATCH v7 3/4] piix_pci: optimize set irq path

2011-05-22 Thread TeLeMan
I did a patch for it.
http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg01239.html

On Sun, May 22, 2011 at 20:24, Stefan Weil w...@mail.berlios.de wrote:

 Hi,

 I got a regression report which was obviously caused by this commit:

 $ git bisect bad
 ab431c283e7055bcd6fb622f212bb29e84a6a134 is the first bad commit
 commit ab431c283e7055bcd6fb622f212bb29e84a6a134
 Author: Isaku Yamahata yamah...@valinux.co.jp
 Date:   Fri Apr 1 20:43:23 2011 +0900

 My test scenario:

 i386-softmmu/qemu -L pc-bios -cdrom tinycore_3.5.1.iso -m 256 -boot d -net
 nic -net user -net dump

 * Boot the default configuration (tinycore, or tinycore debug).
 * Wait until X Windows is up.
 * Check the size of qemu*.pcap. If it is only a few bytes, the test failed.
  Emulated networking then does not work, Tinycore does not get an IP address
 via DHCP.

 The original report used tinycore_3.6.iso and the latest w32 binaries from
 qemu.weilnetz.de
 with the same result. It also says that other Linux distributions show the
 same problem
 (Fedora-14-i686-Live-Desktop.iso).

 Regards
 Stefan W.







[Qemu-devel] [PATCH v7 3/4] piix_pci: optimize set irq path

2011-04-01 Thread Isaku Yamahata
optimize irq routing in piix_pic.c which has been a TODO.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is addressed
by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

test:
- create VM with 4 e1000 nics in different pci slots
  (i.e. fn=0 for each e1000)
  Thus those e1000's INTA are connected to each PIRQ[A-D].
- run linux as guest and saw each devices triggers interrupt
  by seeing /proc/interrupts. And then confirmed that each PIRQ[A-D]
  surely asserted interrupts.
  Because irq 10 and 11 are shared by 4 e1000's, it only one NIC is activated
  with ifconfig ethN up/down when counting interrupts.

Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
Changes v4 - v5:
- fix piix_set_irq_pic()

Changes v3 - v4:
- replace irq_num with pirq or pci_intx

Changes v1 - v2:
- some minor clean ups
- commit log message
---
 hw/piix_pci.c |  101 +++-
 1 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 35e420c..7ffb821 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -37,10 +37,27 @@
 
 typedef PCIHostState I440FXState;
 
+#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
 #define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
+#define PIIX_PIRQC  0x60
 
 typedef struct PIIX3State {
 PCIDevice dev;
+
+/*
+ * bitmap to track pic levels.
+ * The pic level is the logical OR of all the PCI irqs mapped to it
+ * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
+ *
+ * PIRQ is mapped to PIC pins, we track it by
+ * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+ * pic_irq * PIIX_NUM_PIRQS + pirq
+ */
+#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS  64
+#error unable to encode pic state in 64bit in pic_levels.
+#endif
+uint64_t pic_levels;
+
 qemu_irq *pic;
 
 /* This member isn't used. Just for save/load compatibility */
@@ -59,16 +76,16 @@ struct PCII440FXState {
 #define I440FX_PAM_SIZE 7
 #define I440FX_SMRAM0x72
 
-static void piix3_set_irq(void *opaque, int irq_num, int level);
+static void piix3_set_irq(void *opaque, int pirq, int level);
 
 /* return the global irq number corresponding to a given device irq
pin. We could also use the bus number to have a more precise
mapping. */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 {
 int slot_addend;
 slot_addend = (pci_dev-devfn  3) - 1;
-return (irq_num + slot_addend)  3;
+return (pci_intx + slot_addend)  3;
 }
 
 static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
@@ -256,25 +273,64 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
*piix3_devfn, qemu_irq *
 }
 
 /* PIIX3 PCI to ISA bridge */
+static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
+{
+qemu_set_irq(piix3-pic[pic_irq],
+ !!(piix3-pic_levels 
+(((1UL  PIIX_NUM_PIRQS) - 1) 
+ (pic_irq * PIIX_NUM_PIRQS;
+}
 
-static void piix3_set_irq(void *opaque, int irq_num, int level)
+static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level,
+bool propagate)
+{
+int pic_irq;
+uint64_t mask;
+
+pic_irq = piix3-dev.config[PIIX_PIRQC + pirq];
+if (pic_irq = PIIX_NUM_PIC_IRQS) {
+return;
+}
+
+mask = 1ULL  ((pic_irq * PIIX_NUM_PIRQS) + pirq);
+piix3-pic_levels = ~mask;
+piix3-pic_levels |= mask * !!level;
+
+if (propagate) {
+piix3_set_irq_pic(piix3, pic_irq);
+}
+}
+
+static void piix3_set_irq(void *opaque, int pirq, int level)
 {
-int i, pic_irq, pic_level;
 PIIX3State *piix3 = opaque;
+piix3_set_irq_level(piix3, pirq, level, true);
+}
 
-/* now we change the pic irq level according to the piix irq mappings */
-/* XXX: optimize */
-pic_irq = piix3-dev.config[0x60 + irq_num];
-if (pic_irq  16) {
-/* The pic level is the logical OR of all the PCI irqs mapped
-   to it */
-pic_level = 0;
-for (i = 0; i  4; i++) {
-if (pic_irq == piix3-dev.config[0x60 + i]) {
-pic_level |= pci_bus_get_irq_level(piix3-dev.bus, i);
-}
+/* irq routing is changed. so rebuild bitmap */
+static void piix3_update_irq_levels(PIIX3State *piix3)
+{
+int pirq;
+