| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 07 Jul 2008 12:05:24 +1000
From: Michael Ellerman <michael@...erman.id.au>
To: Matthew Wilcox <matthew@....cx>
Cc: linux-pci@...r.kernel.org, kaneshige.kenji@...fujitsu.com,
mingo@...e.hu, tglx@...utronix.de, davem@...emloft.net,
dan.j.williams@...el.com, Martine.Silbermann@...com,
benh@...nel.crashing.org, linux-kernel@...r.kernel.org,
Matthew Wilcox <willy@...ux.intel.com>
Subject: Re: [PATCH 1/4] PCI MSI: Store the number of messages in the
msi_desc
On Sat, 2008-07-05 at 09:34 -0400, Matthew Wilcox wrote:
> This first part simply changes the msi_attrib data structure to store
> how many vectors have been allocated. In order to do this, I shrink the
> 'type' from 5 bits to 2 and rename it to _type to catch any unsuspecting
> users.
Please don't, it significantly uglifies the code IMHO. Just add a new
field for the size, I'd rather call it qsize to match the register.
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.
cheers
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 8c61304..92992a8 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -106,11 +106,11 @@ static void msix_flush_writes(unsigned int irq)
>
> entry = get_irq_msi(irq);
> BUG_ON(!entry || !entry->dev);
> - switch (entry->msi_attrib.type) {
> - case PCI_CAP_ID_MSI:
> + switch (entry->msi_attrib._type) {
> + case MSI_ATTRIB:
> /* nothing to do */
> break;
> - case PCI_CAP_ID_MSIX:
> + case MSIX_ATTRIB:
> {
> int offset = entry->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET;
> @@ -129,8 +129,8 @@ static void msi_set_mask_bits(unsigned int irq, u32 mask, u32 flag)
>
> entry = get_irq_msi(irq);
> BUG_ON(!entry || !entry->dev);
> - switch (entry->msi_attrib.type) {
> - case PCI_CAP_ID_MSI:
> + switch (entry->msi_attrib._type) {
> + case MSI_ATTRIB:
> if (entry->msi_attrib.maskbit) {
> int pos;
> u32 mask_bits;
> @@ -144,7 +144,7 @@ static void msi_set_mask_bits(unsigned int irq, u32 mask, u32 flag)
> msi_set_enable(entry->dev, !flag);
> }
> break;
> - case PCI_CAP_ID_MSIX:
> + case MSIX_ATTRIB:
> {
> int offset = entry->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET;
> @@ -162,8 +162,8 @@ static void msi_set_mask_bits(unsigned int irq, u32 mask, u32 flag)
> void read_msi_msg(unsigned int irq, struct msi_msg *msg)
> {
> struct msi_desc *entry = get_irq_msi(irq);
> - switch(entry->msi_attrib.type) {
> - case PCI_CAP_ID_MSI:
> + switch(entry->msi_attrib._type) {
> + case MSI_ATTRIB:
> {
> struct pci_dev *dev = entry->dev;
> int pos = entry->msi_attrib.pos;
> @@ -182,7 +182,7 @@ void read_msi_msg(unsigned int irq, struct msi_msg *msg)
> msg->data = data;
> break;
> }
> - case PCI_CAP_ID_MSIX:
> + case MSIX_ATTRIB:
> {
> void __iomem *base;
> base = entry->mask_base +
> @@ -201,11 +201,17 @@ void read_msi_msg(unsigned int irq, struct msi_msg *msg)
> void write_msi_msg(unsigned int irq, struct msi_msg *msg)
> {
> struct msi_desc *entry = get_irq_msi(irq);
> - switch (entry->msi_attrib.type) {
> - case PCI_CAP_ID_MSI:
> + switch (entry->msi_attrib._type) {
> + case MSI_ATTRIB:
> {
> 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);
>
> pci_write_config_dword(dev, msi_lower_address_reg(pos),
> msg->address_lo);
> @@ -220,7 +226,7 @@ void write_msi_msg(unsigned int irq, struct msi_msg *msg)
> }
> break;
> }
> - case PCI_CAP_ID_MSIX:
> + case MSIX_ATTRIB:
> {
> void __iomem *base;
> base = entry->mask_base +
> @@ -359,7 +365,7 @@ static int msi_capability_init(struct pci_dev *dev)
> if (!entry)
> return -ENOMEM;
>
> - entry->msi_attrib.type = PCI_CAP_ID_MSI;
> + entry->msi_attrib._type = MSI_ATTRIB;
> entry->msi_attrib.is_64 = is_64bit_address(control);
> entry->msi_attrib.entry_nr = 0;
> entry->msi_attrib.maskbit = is_mask_bit_support(control);
> @@ -446,7 +452,7 @@ static int msix_capability_init(struct pci_dev *dev,
> break;
>
> j = entries[i].entry;
> - entry->msi_attrib.type = PCI_CAP_ID_MSIX;
> + entry->msi_attrib._type = MSIX_ATTRIB;
> entry->msi_attrib.is_64 = 1;
> entry->msi_attrib.entry_nr = j;
> entry->msi_attrib.maskbit = 1;
> @@ -589,12 +595,13 @@ void pci_msi_shutdown(struct pci_dev* dev)
> u32 mask = entry->msi_attrib.maskbits_mask;
> msi_set_mask_bits(dev->irq, mask, ~mask);
> }
> - if (!entry->dev || entry->msi_attrib.type != PCI_CAP_ID_MSI)
> + if (!entry->dev || entry->msi_attrib._type != MSI_ATTRIB)
> return;
>
> /* Restore dev->irq to its default pin-assertion irq */
> dev->irq = entry->msi_attrib.default_irq;
> }
> +
> void pci_disable_msi(struct pci_dev* dev)
> {
> struct msi_desc *entry;
> @@ -605,7 +612,7 @@ void pci_disable_msi(struct pci_dev* dev)
> pci_msi_shutdown(dev);
>
> entry = list_entry(dev->msi_list.next, struct msi_desc, list);
> - if (!entry->dev || entry->msi_attrib.type != PCI_CAP_ID_MSI)
> + if (!entry->dev || entry->msi_attrib._type != MSI_ATTRIB)
> return;
>
> msi_free_irqs(dev);
> @@ -624,7 +631,7 @@ static int msi_free_irqs(struct pci_dev* dev)
> arch_teardown_msi_irqs(dev);
>
> list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
> - if (entry->msi_attrib.type == PCI_CAP_ID_MSIX) {
> + if (entry->msi_attrib._type == MSIX_ATTRIB) {
> writel(1, entry->mask_base + entry->msi_attrib.entry_nr
> * PCI_MSIX_ENTRY_SIZE
> + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
> index 3898f52..b72e0bd 100644
> --- a/drivers/pci/msi.h
> +++ b/drivers/pci/msi.h
> @@ -22,12 +22,8 @@
> #define msi_disable(control) control &= ~PCI_MSI_FLAGS_ENABLE
> #define multi_msi_capable(control) \
> (1 << ((control & PCI_MSI_FLAGS_QMASK) >> 1))
> -#define multi_msi_enable(control, num) \
> - control |= (((num >> 1) << 4) & PCI_MSI_FLAGS_QSIZE);
> #define is_64bit_address(control) (!!(control & PCI_MSI_FLAGS_64BIT))
> #define is_mask_bit_support(control) (!!(control & PCI_MSI_FLAGS_MASKBIT))
> -#define msi_enable(control, num) multi_msi_enable(control, num); \
> - control |= PCI_MSI_FLAGS_ENABLE
>
> #define msix_table_offset_reg(base) (base + 0x04)
> #define msix_pba_offset_reg(base) (base + 0x08)
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 8f29392..d322148 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -15,9 +15,13 @@ extern void unmask_msi_irq(unsigned int irq);
> extern void read_msi_msg(unsigned int irq, struct msi_msg *msg);
> extern void write_msi_msg(unsigned int irq, struct msi_msg *msg);
>
> +#define MSI_ATTRIB 1
> +#define MSIX_ATTRIB 2
> +
> struct msi_desc {
> struct {
> - __u8 type : 5; /* {0: unused, 5h:MSI, 11h:MSI-X} */
> + __u8 _type : 2; /* {0: unused, 1:MSI, 2:MSI-X} */
> + __u8 multiple: 3; /* log2 number of messages */
> __u8 maskbit : 1; /* mask-pending bit supported ? */
> __u8 masked : 1;
> __u8 is_64 : 1; /* Address size: 0=32bit 1=64bit */
--
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
Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)
Powered by blists - more mailing lists