Message ID | 20110328111956.GF16639@valinux.co.jp |
---|---|
State | New |
Headers | show |
On Mon, Mar 28, 2011 at 08:19:56PM +0900, Isaku Yamahata wrote: > On Sun, Mar 27, 2011 at 04:56:29PM +0200, Michael S. Tsirkin wrote: > > On Wed, Mar 23, 2011 at 11:17:19AM +0900, Isaku Yamahata wrote: > > > v4 has minor typo. I sent it too early. Here's fixed one. > > > > > > v3 -> v4 Main changes are > > > - use pirq, pci_intx instead of irq_num in piix_pci.c > > > - patch 4/4 cleans the code a bit > > > > With this applied e1000 fails to work for me. > > Command line: > > > > qemu-system-x86_64 -enable-kvm -m 1G -drive > > if=virtio,file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net > > nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir > > tcp:8022::22 -net nic,model=e1000,netdev=foo,macaddr=52:54:00:12:34:56 > > -netdev > > tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no > > -nographic > > > > Could you try that please? > > Does the following patch help? Seems to help, but I have to ask - how did you test v5? > If so, I'll prepare v6. > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > index c019793..5f0d92f 100644 > --- a/hw/piix_pci.c > +++ b/hw/piix_pci.c > @@ -277,7 +277,8 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) > { > qemu_set_irq(piix3->pic[pic_irq], > !!(piix3->pic_levels & > - ((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS)))); > + (((1UL << PIIX_NUM_PIRQS) - 1) << > + (pic_irq * PIIX_NUM_PIRQS)))); I think we should just make it ~0ULL << (pic_irq * PIIX_NUM_PIRQS). Didn't try this though. > } > > static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) > > -- > yamahata
On Mon, Mar 28, 2011 at 01:34:02PM +0200, Michael S. Tsirkin wrote: > On Mon, Mar 28, 2011 at 08:19:56PM +0900, Isaku Yamahata wrote: > > On Sun, Mar 27, 2011 at 04:56:29PM +0200, Michael S. Tsirkin wrote: > > > On Wed, Mar 23, 2011 at 11:17:19AM +0900, Isaku Yamahata wrote: > > > > v4 has minor typo. I sent it too early. Here's fixed one. > > > > > > > > v3 -> v4 Main changes are > > > > - use pirq, pci_intx instead of irq_num in piix_pci.c > > > > - patch 4/4 cleans the code a bit > > > > > > With this applied e1000 fails to work for me. > > > Command line: > > > > > > qemu-system-x86_64 -enable-kvm -m 1G -drive > > > if=virtio,file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net > > > nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir > > > tcp:8022::22 -net nic,model=e1000,netdev=foo,macaddr=52:54:00:12:34:56 > > > -netdev > > > tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no > > > -nographic > > > > > > Could you try that please? > > > > Does the following patch help? > > Seems to help, but I have to ask - how did you test v5? I booted a guest to see login prompt. My setting happened to use only PIRQ A and B. I found this fix by code reviewing. > > If so, I'll prepare v6. > > > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > > index c019793..5f0d92f 100644 > > --- a/hw/piix_pci.c > > +++ b/hw/piix_pci.c > > @@ -277,7 +277,8 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) > > { > > qemu_set_irq(piix3->pic[pic_irq], > > !!(piix3->pic_levels & > > - ((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS)))); > > + (((1UL << PIIX_NUM_PIRQS) - 1) << > > + (pic_irq * PIIX_NUM_PIRQS)))); > > I think we should just make it ~0ULL << (pic_irq * PIIX_NUM_PIRQS). > Didn't try this though. To get if pic_irq is raised/lowered, only 4bits are checked. bit 63 7 4 3 0 pic_irq | 15| 14|... |... | 1| 0| PIRQ |DCBA|DCBA|DCBA|... |DCBA|DCBA| 1111 = (1UL << PIIX_NUM_PIRQS) - 1 1111 << (pic_irq * PIIX_NUM_PIRQS) thanks, > > > } > > > > static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) > > > > -- > > yamahata >
diff --git a/hw/piix_pci.c b/hw/piix_pci.c index c019793..5f0d92f 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -277,7 +277,8 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { qemu_set_irq(piix3->pic[pic_irq], !!(piix3->pic_levels & - ((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS)))); + (((1UL << PIIX_NUM_PIRQS) - 1) << + (pic_irq * PIIX_NUM_PIRQS)))); } static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)