[Qemu-devel] Re: [PATCH v5 0/4] piix_pci: optimize irq data path

[Isaku 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
> 

-- 
yamahata