On Mon, 2008-07-07 at 06:04 -0600, Matthew Wilcox wrote: > On Mon, Jul 07, 2008 at 01:48:32PM +1000, Michael Ellerman wrote: > > Yeah seriously :) The _ is part of it, but MSI_ATTRIB is uglier than > > PCI_CAP_ID_MSI exactly because it's not PCI_CAP_ID_MSI, which exists and > > is well defined and is used in the rest of the code. > > Here's an improvement over both the status quo and my patch -- simply > use a single bit called is_msix. That is cleaner, you get to fix it when they create MSIXX though ;) > > I didn't say it was a queue, but a Q ;) But I agree it's not a good > > name, the spec calls it "multiple message enable", nvec would match the > > existing code best, or log_nvec. > > I don't see what's wrong with 'multiple'. log_nvec is clunky, and > 'multiple' works well as a boolean (since 0 means 1 interrupt). For me 'multiple' only makes sense as a boolean, but whatever. > > > > If you're worried about bloating msi_desc, there's several fields in > > > > there that are per-device not per-desc, so we could do another patch to > > > > move them into pci_dev or something hanging off it, eg. > > > > pci_dev->msi_info rather than storing them in every desc. > > Ouch. I just used pahole and discovered we were using 72 bytes on > 64-bit. A swift rearrangement of a u16 gets us back down to 64. pahole is awesome, nice find. > Here's the replacement patch: Perhaps I'm pedantic, but I'd rather it was two patches, one to change type to is_msix and one to add the multiple flag. > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index 8c61304..8f7e483 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -180,32 +167,31 @@ void read_msi_msg(unsigned int irq, struct msi_msg *msg) ... > struct pci_dev *dev = entry->dev; > int pos = entry->msi_attrib.pos; > + u16 msgctl; > + > + pci_read_config_word(dev, msi_control_reg(pos), &msgctl); > + msgctl &= ~PCI_MSI_FLAGS_QSIZE; > + msgctl |= entry->msi_attrib.multiple << 4; > + pci_write_config_word(dev, msi_control_reg(pos), msgctl); A #define for "<< 4" would be nice. And should we be paranoid about potentially writing 0b110 or 0b111 which are reserved? cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Attachment:
signature.asc
Description: This is a digitally signed message part
