LinuxLists.cc - [PATCH 1/2]MSI(X) save/restore for suspend/resume

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Tue, 2005-12-27 at 15:55, Arjan van de Ven wrote:
> > diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
> > --- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
> > +++ linux-2.6.15-rc5-root/include/linux/pci.h 2005-12-22 09:23:16.000000000 +0800
> > @@ -135,6 +135,7 @@ struct pci_dev {
> > unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
> >
> > u32 saved_config_space[16]; /* config space saved at suspend time */
> > + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> > struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> > int rom_attr_enabled; /* has display of the rom attribute been enabled? */
>
>
> void feels like sort of the wrong thing here....
So what is good to you :)?

Thanks,
Shaohua

2005-12-28 08:25:38

by Arjan van de Ven

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, 2005-12-28 at 09:24 +0800, Shaohua Li wrote:
> On Tue, 2005-12-27 at 15:55, Arjan van de Ven wrote:
> > > diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
> > > --- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
> > > +++ linux-2.6.15-rc5-root/include/linux/pci.h 2005-12-22 09:23:16.000000000 +0800
> > > @@ -135,6 +135,7 @@ struct pci_dev {
> > > unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
> > >
> > > u32 saved_config_space[16]; /* config space saved at suspend time */
> > > + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> > > struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> > > int rom_attr_enabled; /* has display of the rom attribute been enabled? */
> >
> >
> > void feels like sort of the wrong thing here....
> So what is good to you :)?

doesn't it contain u16's ?

2005-12-28 08:35:24

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, 2005-12-28 at 16:25, Arjan van de Ven wrote:
> On Wed, 2005-12-28 at 09:24 +0800, Shaohua Li wrote:
> > On Tue, 2005-12-27 at 15:55, Arjan van de Ven wrote:
> > > > diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
> > > > --- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
> > > > +++ linux-2.6.15-rc5-root/include/linux/pci.h 2005-12-22 09:23:16.000000000 +0800
> > > > @@ -135,6 +135,7 @@ struct pci_dev {
> > > > unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
> > > >
> > > > u32 saved_config_space[16]; /* config space saved at suspend time */
> > > > + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> > > > struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> > > > int rom_attr_enabled; /* has display of the rom attribute been enabled? */
> > >
> > >
> > > void feels like sort of the wrong thing here....
> > So what is good to you :)?
>
> doesn't it contain u16's ?
It might contain u16 or u32 or anything else. And depends on the
specific capability.

Thanks,
Shaohua

2006-01-04 07:13:41

by Andrew Morton

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

Shaohua Li <[email protected]> wrote:
>
> Add MSI(X) configure space save/restore in generic PCI helper.

This adds a bunch of code which isn't needed if !CONFIG_PM.

2006-01-05 01:06:59

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, 2006-01-04 at 15:13, Andrew Morton wrote:
> Shaohua Li <[email protected]> wrote:
> >
> > Add MSI(X) configure space save/restore in generic PCI helper.
>
> This adds a bunch of code which isn't needed if !CONFIG_PM.
Ok, the updated one fixes this.

Add MSI(X) configure sapce save/restore in generic PCI helper.

---

linux-2.6.15-rc5-root/drivers/pci/msi.c | 213 +++++++++++++++++++++----
linux-2.6.15-rc5-root/drivers/pci/pci.c | 6
linux-2.6.15-rc5-root/drivers/pci/pci.h | 11 +
linux-2.6.15-rc5-root/include/linux/pci.h | 1
linux-2.6.15-rc5-root/include/linux/pci_regs.h | 1
5 files changed, 203 insertions(+), 29 deletions(-)

diff -puN drivers/pci/msi.c~msi_save_restore drivers/pci/msi.c
--- linux-2.6.15-rc5/drivers/pci/msi.c~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/msi.c 2006-01-05 08:50:11.000000000 +0800
@@ -499,6 +499,187 @@ void pci_scan_msi_device(struct pci_dev
nr_reserved_vectors++;
}

+#ifdef CONFIG_PM
+int pci_save_msi_state(struct pci_dev *dev)
+{
+ int pos, i = 0;
+ u16 control;
+ u32 *cap;
+
+ if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSI)) <= 0 ||
+ dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSI_FLAGS_ENABLE))
+ return 0;
+
+ cap = kzalloc(sizeof(u32) * 5, GFP_KERNEL);
+ if (!cap) {
+ printk(KERN_ERR "Out of memory in pci_save_msi_state\n");
+ return -ENOMEM;
+ }
+
+ pci_read_config_dword(dev, pos, &cap[i++]);
+ control = cap[0] >> 16;
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, &cap[i++]);
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_64, &cap[i++]);
+ } else
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_32, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_read_config_dword(dev, pos + PCI_MSI_MASK_BIT, &cap[i++]);
+ dev->saved_cap_space[PCI_CAP_ID_MSI] = cap;
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ return 0;
+}
+
+void pci_restore_msi_state(struct pci_dev *dev)
+{
+ int i = 0, pos;
+ u16 control;
+ u32 *cap = dev->saved_cap_space[PCI_CAP_ID_MSI];
+
+ if (!cap || (pos = pci_find_capability(dev, PCI_CAP_ID_MSI)) <= 0)
+ return;
+
+ control = cap[i++] >> 16;
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, cap[i++]);
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_64, cap[i++]);
+ } else
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_32, cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_write_config_dword(dev, pos + PCI_MSI_MASK_BIT, cap[i++]);
+ pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ dev->saved_cap_space[PCI_CAP_ID_MSI] = NULL;
+ kfree(cap);
+}
+
+int pci_save_msix_state(struct pci_dev *dev)
+{
+ int pos;
+ u16 control;
+ u16 *save;
+
+ if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0 ||
+ dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSIX_FLAGS_ENABLE))
+ return 0;
+ save = kmalloc(sizeof(u16), GFP_KERNEL);
+ if (!save) {
+ printk(KERN_ERR "Out of memory in pci_save_msix_state\n");
+ return -ENOMEM;
+ }
+ *save = control;
+ dev->saved_cap_space[PCI_CAP_ID_MSIX] = save;
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+ return 0;
+}
+
+void pci_restore_msix_state(struct pci_dev *dev)
+{
+ u16 *control = dev->saved_cap_space[PCI_CAP_ID_MSIX];
+ u16 save;
+ int pos;
+ int vector, head, tail = 0;
+ void __iomem *base;
+ int j;
+ struct msg_address address;
+ struct msg_data data;
+ struct msi_desc *entry;
+ int temp;
+
+ if (control == NULL)
+ return;
+ save = *control;
+ dev->saved_cap_space[PCI_CAP_ID_MSIX] = NULL;
+ kfree(control);
+ if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0)
+ return;
+
+ /* route the table */
+ temp = dev->irq;
+ if (msi_lookup_vector(dev, PCI_CAP_ID_MSIX))
+ return;
+ vector = head = dev->irq;
+ while (head != tail) {
+ entry = msi_desc[vector];
+ base = entry->mask_base;
+ j = entry->msi_attrib.entry_nr;
+
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+
+ address.lo_address.value &= MSI_ADDRESS_DEST_ID_MASK;
+ address.lo_address.value |= entry->msi_attrib.current_cpu <<
+ MSI_TARGET_CPU_SHIFT;
+
+ writel(address.lo_address.value,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET);
+ writel(address.hi_address,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET);
+ writel(*(u32*)&data,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_DATA_OFFSET);
+
+ tail = msi_desc[vector]->link.tail;
+ vector = tail;
+ }
+ dev->irq = temp;
+
+ pci_write_config_word(dev, msi_control_reg(pos), save);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+}
+#endif
+
+static void msi_register_init(struct pci_dev *dev, struct msi_desc *entry)
+{
+ struct msg_address address;
+ struct msg_data data;
+ int pos, vector = dev->irq;
+ u16 control;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ /* Configure MSI capability structure */
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+ entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
+ MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
+ pci_write_config_dword(dev, msi_lower_address_reg(pos),
+ address.lo_address.value);
+ if (is_64bit_address(control)) {
+ pci_write_config_dword(dev,
+ msi_upper_address_reg(pos), address.hi_address);
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 1), *((u32*)&data));
+ } else
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 0), *((u32*)&data));
+ if (entry->msi_attrib.maskbit) {
+ unsigned int maskbits, temp;
+ /* All MSIs are unmasked by default, Mask them all */
+ pci_read_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ &maskbits);
+ temp = (1 << multi_msi_capable(control));
+ temp = ((temp - 1) & ~temp);
+ maskbits |= temp;
+ pci_write_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ maskbits);
+ }
+}
+
/**
* msi_capability_init - configure device's MSI capability structure
* @dev: pointer to the pci_dev data structure of MSI device function
@@ -511,8 +692,6 @@ void pci_scan_msi_device(struct pci_dev
static int msi_capability_init(struct pci_dev *dev)
{
struct msi_desc *entry;
- struct msg_address address;
- struct msg_data data;
int pos, vector;
u16 control;

@@ -542,33 +721,8 @@ static int msi_capability_init(struct pc
/* Replace with MSI handler */
irq_handler_init(PCI_CAP_ID_MSI, vector, entry->msi_attrib.maskbit);
/* Configure MSI capability structure */
- msi_address_init(&address);
- msi_data_init(&data, vector);
- entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
- MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
- pci_write_config_dword(dev, msi_lower_address_reg(pos),
- address.lo_address.value);
- if (is_64bit_address(control)) {
- pci_write_config_dword(dev,
- msi_upper_address_reg(pos), address.hi_address);
- pci_write_config_word(dev,
- msi_data_reg(pos, 1), *((u32*)&data));
- } else
- pci_write_config_word(dev,
- msi_data_reg(pos, 0), *((u32*)&data));
- if (entry->msi_attrib.maskbit) {
- unsigned int maskbits, temp;
- /* All MSIs are unmasked by default, Mask them all */
- pci_read_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- &maskbits);
- temp = (1 << multi_msi_capable(control));
- temp = ((temp - 1) & ~temp);
- maskbits |= temp;
- pci_write_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- maskbits);
- }
+ msi_register_init(dev, entry);
+
attach_msi_entry(entry, vector);
/* Set MSI enabled bits */
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
@@ -717,6 +871,7 @@ int pci_enable_msi(struct pci_dev* dev)
vector_irq[dev->irq] = -1;
nr_released_vectors--;
spin_unlock_irqrestore(&msi_lock, flags);
+ msi_register_init(dev, msi_desc[dev->irq]);
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
return 0;
}
diff -puN drivers/pci/pci.c~msi_save_restore drivers/pci/pci.c
--- linux-2.6.15-rc5/drivers/pci/pci.c~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.c 2005-12-27 08:42:09.000000000 +0800
@@ -438,6 +438,10 @@ pci_save_state(struct pci_dev *dev)
/* XXX: 100% dword access ok here? */
for (i = 0; i < 16; i++)
pci_read_config_dword(dev, i * 4,&dev->saved_config_space[i]);
+ if ((i = pci_save_msi_state(dev)) != 0)
+ return i;
+ if ((i = pci_save_msix_state(dev)) != 0)
+ return i;
return 0;
}

@@ -452,6 +456,8 @@ pci_restore_state(struct pci_dev *dev)

for (i = 0; i < 16; i++)
pci_write_config_dword(dev,i * 4, dev->saved_config_space[i]);
+ pci_restore_msi_state(dev);
+ pci_restore_msix_state(dev);
return 0;
}

diff -puN drivers/pci/pci.h~msi_save_restore drivers/pci/pci.h
--- linux-2.6.15-rc5/drivers/pci/pci.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.h 2006-01-05 08:51:51.000000000 +0800
@@ -58,6 +58,17 @@ void disable_msi_mode(struct pci_dev *de
#else
static inline void disable_msi_mode(struct pci_dev *dev, int pos, int type) { }
#endif
+#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PM)
+int pci_save_msi_state(struct pci_dev *dev);
+int pci_save_msix_state(struct pci_dev *dev);
+void pci_restore_msi_state(struct pci_dev *dev);
+void pci_restore_msix_state(struct pci_dev *dev);
+#else
+static inline void pci_save_msi_state(struct pci_dev *dev) {}
+static inline void pci_save_msix_state(struct pci_dev *dev) {}
+static inline void pci_restore_msi_state(struct pci_dev *dev) {}
+static inline void pci_restore_msix_state(struct pci_dev *dev) {}
+#endif

extern int pcie_mch_quirk;
extern struct device_attribute pci_dev_attrs[];
diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
--- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
+++ linux-2.6.15-rc5-root/include/linux/pci.h 2005-12-22 09:23:16.000000000 +0800
@@ -135,6 +135,7 @@ struct pci_dev {
unsigned int block_ucfg_access:1; /* userspace config space access is blocked */

u32 saved_config_space[16]; /* config space saved at suspend time */
+ void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
diff -puN include/linux/pci_regs.h~msi_save_restore include/linux/pci_regs.h
--- linux-2.6.15-rc5/include/linux/pci_regs.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
+++ linux-2.6.15-rc5-root/include/linux/pci_regs.h 2005-12-22 09:23:16.000000000 +0800
@@ -199,6 +199,7 @@
#define PCI_CAP_ID_SHPC 0x0C /* PCI Standard Hot-Plug Controller */
#define PCI_CAP_ID_EXP 0x10 /* PCI Express */
#define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
+#define PCI_CAP_ID_MAX PCI_CAP_ID_MSIX
#define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
#define PCI_CAP_FLAGS 2 /* Capability defined flags (16 bits) */
#define PCI_CAP_SIZEOF 4
_

2006-01-10 20:16:25

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Thu, Jan 05, 2006 at 08:58:00AM +0800, Shaohua Li wrote:
> --- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2005-12-22 09:23:16.000000000 +0800
> +++ linux-2.6.15-rc5-root/include/linux/pci.h 2005-12-22 09:23:16.000000000 +0800
> @@ -135,6 +135,7 @@ struct pci_dev {
> unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
>
> u32 saved_config_space[16]; /* config space saved at suspend time */
> + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> int rom_attr_enabled; /* has display of the rom attribute been enabled? */
> struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */

This should not be a void *, right? Why not make it the proper type?

thanks,

greg k-h

2006-01-10 20:28:43

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Thu, Jan 05, 2006 at 08:58:00AM +0800, Shaohua Li wrote:
> + if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0 ||
> + dev->no_msi)

No assignments within conditionals, please.

pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
if (pos <= 0 || dev->no_msi)

> u32 saved_config_space[16]; /* config space saved at suspend time */
> + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
...
> #define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
> +#define PCI_CAP_ID_MAX PCI_CAP_ID_MSIX
> #define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */

Rather than taking all this space in the pci_dev structure (even
without CONFIG_PM), how about:

struct pci_cap_saved_state {
struct pci_cap_saved_state *next;
char cap_nr;
char data[0];
}

and then just add:

struct pci_cap_saved_state *saved_cap_space;

to the struct pci_dev? One pointer, rather than (currently!) 12.
That's an 88 byte saving per PCI device on 64-bit machines!

2006-01-11 01:18:08

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Tue, 2006-01-10 at 13:28 -0700, Matthew Wilcox wrote:
> On Thu, Jan 05, 2006 at 08:58:00AM +0800, Shaohua Li wrote:
> > + if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0 ||
> > + dev->no_msi)
>
> No assignments within conditionals, please.
Ok.

> pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> if (pos <= 0 || dev->no_msi)
>
> > u32 saved_config_space[16]; /* config space saved at suspend time */
> > + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> > struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> ...
> > #define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
> > +#define PCI_CAP_ID_MAX PCI_CAP_ID_MSIX
> > #define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
>
> Rather than taking all this space in the pci_dev structure (even
> without CONFIG_PM), how about:
>
> struct pci_cap_saved_state {
> struct pci_cap_saved_state *next;
> char cap_nr;
> char data[0];
> }
>
> and then just add:
>
> struct pci_cap_saved_state *saved_cap_space;
>
> to the struct pci_dev? One pointer, rather than (currently!) 12.
> That's an 88 byte saving per PCI device on 64-bit machines!
It's not that big, right? This will make things a little complex. How
about just define saved_cap_space[] with CONFIG_PM configured? Anyway,
if you insist on less space, I'm happy to change it.

Thanks,
Shaohua

2006-01-11 04:14:32

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, Jan 11, 2006 at 09:17:19AM +0800, Shaohua Li wrote:
> On Tue, 2006-01-10 at 13:28 -0700, Matthew Wilcox wrote:
> > On Thu, Jan 05, 2006 at 08:58:00AM +0800, Shaohua Li wrote:
> > > + if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0 ||
> > > + dev->no_msi)
> >
> > No assignments within conditionals, please.
> Ok.
>
> > pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> > if (pos <= 0 || dev->no_msi)
> >
> > > u32 saved_config_space[16]; /* config space saved at suspend time */
> > > + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> > > struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> > ...
> > > #define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
> > > +#define PCI_CAP_ID_MAX PCI_CAP_ID_MSIX
> > > #define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
> >
> > Rather than taking all this space in the pci_dev structure (even
> > without CONFIG_PM), how about:
> >
> > struct pci_cap_saved_state {
> > struct pci_cap_saved_state *next;
> > char cap_nr;
> > char data[0];
> > }
> >
> > and then just add:
> >
> > struct pci_cap_saved_state *saved_cap_space;
> >
> > to the struct pci_dev? One pointer, rather than (currently!) 12.
> > That's an 88 byte saving per PCI device on 64-bit machines!
> It's not that big, right? This will make things a little complex. How
> about just define saved_cap_space[] with CONFIG_PM configured? Anyway,
> if you insist on less space, I'm happy to change it.

A pointer to the structure is much nicer, I'd recommend doing it that
way.

thanks,

greg k-h

2006-01-11 08:19:19

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Tue, 2006-01-10 at 17:26 -0800, Greg KH wrote:
> On Wed, Jan 11, 2006 at 09:17:19AM +0800, Shaohua Li wrote:
> > On Tue, 2006-01-10 at 13:28 -0700, Matthew Wilcox wrote:
> > > On Thu, Jan 05, 2006 at 08:58:00AM +0800, Shaohua Li wrote:
> > > > + if ((pos = pci_find_capability(dev, PCI_CAP_ID_MSIX)) <= 0 ||
> > > > + dev->no_msi)
> > >
> > > No assignments within conditionals, please.
> > Ok.
> >
> > > pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> > > if (pos <= 0 || dev->no_msi)
> > >
> > > > u32 saved_config_space[16]; /* config space saved at suspend time */
> > > > + void *saved_cap_space[PCI_CAP_ID_MAX + 1]; /* ext config space saved at suspend time */
> > > > struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
> > > ...
> > > > #define PCI_CAP_ID_MSIX 0x11 /* MSI-X */
> > > > +#define PCI_CAP_ID_MAX PCI_CAP_ID_MSIX
> > > > #define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
> > >
> > > Rather than taking all this space in the pci_dev structure (even
> > > without CONFIG_PM), how about:
> > >
> > > struct pci_cap_saved_state {
> > > struct pci_cap_saved_state *next;
> > > char cap_nr;
> > > char data[0];
> > > }
> > >
> > > and then just add:
> > >
> > > struct pci_cap_saved_state *saved_cap_space;
> > >
> > > to the struct pci_dev? One pointer, rather than (currently!) 12.
> > > That's an 88 byte saving per PCI device on 64-bit machines!
> > It's not that big, right? This will make things a little complex. How
> > about just define saved_cap_space[] with CONFIG_PM configured? Anyway,
> > if you insist on less space, I'm happy to change it.
>
> A pointer to the structure is much nicer, I'd recommend doing it that
> way.
Ok, here it is.

Add MSI(X) configure space save/restore in generic PCI helper.

Signed-off-by: Shaohua Li<[email protected]>
---

linux-2.6.15-rc5-root/drivers/pci/msi.c | 227 ++++++++++++++++++++++++++----
linux-2.6.15-rc5-root/drivers/pci/pci.c | 6
linux-2.6.15-rc5-root/drivers/pci/pci.h | 11 +
linux-2.6.15-rc5-root/include/linux/pci.h | 42 +++++
4 files changed, 257 insertions(+), 29 deletions(-)

diff -puN drivers/pci/msi.c~msi_save_restore drivers/pci/msi.c
--- linux-2.6.15-rc5/drivers/pci/msi.c~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/msi.c 2006-01-11 15:47:40.000000000 +0800
@@ -499,6 +499,201 @@ void pci_scan_msi_device(struct pci_dev
nr_reserved_vectors++;
}

+#ifdef CONFIG_PM
+int pci_save_msi_state(struct pci_dev *dev)
+{
+ int pos, i = 0;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ if (pos <= 0 || dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSI_FLAGS_ENABLE))
+ return 0;
+
+ save_state = kzalloc(sizeof(struct pci_cap_saved_state) + sizeof(u32) * 5,
+ GFP_KERNEL);
+ if (!save_state) {
+ printk(KERN_ERR "Out of memory in pci_save_msi_state\n");
+ return -ENOMEM;
+ }
+ cap = &save_state->data[0];
+
+ pci_read_config_dword(dev, pos, &cap[i++]);
+ control = cap[0] >> 16;
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, &cap[i++]);
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_64, &cap[i++]);
+ } else
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_32, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_read_config_dword(dev, pos + PCI_MSI_MASK_BIT, &cap[i++]);
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ save_state->cap_nr = PCI_CAP_ID_MSI;
+ pci_add_saved_cap(dev, save_state);
+ return 0;
+}
+
+void pci_restore_msi_state(struct pci_dev *dev)
+{
+ int i = 0, pos;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSI);
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ if (!save_state || pos <= 0)
+ return;
+ cap = &save_state->data[0];
+
+ control = cap[i++] >> 16;
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, cap[i++]);
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_64, cap[i++]);
+ } else
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_32, cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_write_config_dword(dev, pos + PCI_MSI_MASK_BIT, cap[i++]);
+ pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ pci_remove_saved_cap(dev, save_state);
+ kfree(save_state);
+}
+
+int pci_save_msix_state(struct pci_dev *dev)
+{
+ int pos;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+ if (pos <= 0 || dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSIX_FLAGS_ENABLE))
+ return 0;
+ save_state = kzalloc(sizeof(struct pci_cap_saved_state) + sizeof(u16),
+ GFP_KERNEL);
+ if (!save_state) {
+ printk(KERN_ERR "Out of memory in pci_save_msix_state\n");
+ return -ENOMEM;
+ }
+ *((u16 *)&save_state->data[0]) = control;
+
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+ save_state->cap_nr = PCI_CAP_ID_MSIX;
+ pci_add_saved_cap(dev, save_state);
+ return 0;
+}
+
+void pci_restore_msix_state(struct pci_dev *dev)
+{
+ u16 save;
+ int pos;
+ int vector, head, tail = 0;
+ void __iomem *base;
+ int j;
+ struct msg_address address;
+ struct msg_data data;
+ struct msi_desc *entry;
+ int temp;
+ struct pci_cap_saved_state *save_state;
+
+ save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSIX);
+ if (!save_state)
+ return;
+ save = *((u16 *)&save_state->data[0]);
+ pci_remove_saved_cap(dev, save_state);
+ kfree(save_state);
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+ if (pos <= 0)
+ return;
+
+ /* route the table */
+ temp = dev->irq;
+ if (msi_lookup_vector(dev, PCI_CAP_ID_MSIX))
+ return;
+ vector = head = dev->irq;
+ while (head != tail) {
+ entry = msi_desc[vector];
+ base = entry->mask_base;
+ j = entry->msi_attrib.entry_nr;
+
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+
+ address.lo_address.value &= MSI_ADDRESS_DEST_ID_MASK;
+ address.lo_address.value |= entry->msi_attrib.current_cpu <<
+ MSI_TARGET_CPU_SHIFT;
+
+ writel(address.lo_address.value,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET);
+ writel(address.hi_address,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET);
+ writel(*(u32*)&data,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_DATA_OFFSET);
+
+ tail = msi_desc[vector]->link.tail;
+ vector = tail;
+ }
+ dev->irq = temp;
+
+ pci_write_config_word(dev, msi_control_reg(pos), save);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+}
+#endif
+
+static void msi_register_init(struct pci_dev *dev, struct msi_desc *entry)
+{
+ struct msg_address address;
+ struct msg_data data;
+ int pos, vector = dev->irq;
+ u16 control;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ /* Configure MSI capability structure */
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+ entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
+ MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
+ pci_write_config_dword(dev, msi_lower_address_reg(pos),
+ address.lo_address.value);
+ if (is_64bit_address(control)) {
+ pci_write_config_dword(dev,
+ msi_upper_address_reg(pos), address.hi_address);
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 1), *((u32*)&data));
+ } else
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 0), *((u32*)&data));
+ if (entry->msi_attrib.maskbit) {
+ unsigned int maskbits, temp;
+ /* All MSIs are unmasked by default, Mask them all */
+ pci_read_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ &maskbits);
+ temp = (1 << multi_msi_capable(control));
+ temp = ((temp - 1) & ~temp);
+ maskbits |= temp;
+ pci_write_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ maskbits);
+ }
+}
+
/**
* msi_capability_init - configure device's MSI capability structure
* @dev: pointer to the pci_dev data structure of MSI device function
@@ -511,8 +706,6 @@ void pci_scan_msi_device(struct pci_dev
static int msi_capability_init(struct pci_dev *dev)
{
struct msi_desc *entry;
- struct msg_address address;
- struct msg_data data;
int pos, vector;
u16 control;

@@ -542,33 +735,8 @@ static int msi_capability_init(struct pc
/* Replace with MSI handler */
irq_handler_init(PCI_CAP_ID_MSI, vector, entry->msi_attrib.maskbit);
/* Configure MSI capability structure */
- msi_address_init(&address);
- msi_data_init(&data, vector);
- entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
- MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
- pci_write_config_dword(dev, msi_lower_address_reg(pos),
- address.lo_address.value);
- if (is_64bit_address(control)) {
- pci_write_config_dword(dev,
- msi_upper_address_reg(pos), address.hi_address);
- pci_write_config_word(dev,
- msi_data_reg(pos, 1), *((u32*)&data));
- } else
- pci_write_config_word(dev,
- msi_data_reg(pos, 0), *((u32*)&data));
- if (entry->msi_attrib.maskbit) {
- unsigned int maskbits, temp;
- /* All MSIs are unmasked by default, Mask them all */
- pci_read_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- &maskbits);
- temp = (1 << multi_msi_capable(control));
- temp = ((temp - 1) & ~temp);
- maskbits |= temp;
- pci_write_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- maskbits);
- }
+ msi_register_init(dev, entry);
+
attach_msi_entry(entry, vector);
/* Set MSI enabled bits */
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
@@ -717,6 +885,7 @@ int pci_enable_msi(struct pci_dev* dev)
vector_irq[dev->irq] = -1;
nr_released_vectors--;
spin_unlock_irqrestore(&msi_lock, flags);
+ msi_register_init(dev, msi_desc[dev->irq]);
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
return 0;
}
diff -puN drivers/pci/pci.c~msi_save_restore drivers/pci/pci.c
--- linux-2.6.15-rc5/drivers/pci/pci.c~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.c 2006-01-11 13:33:33.000000000 +0800
@@ -438,6 +438,10 @@ pci_save_state(struct pci_dev *dev)
/* XXX: 100% dword access ok here? */
for (i = 0; i < 16; i++)
pci_read_config_dword(dev, i * 4,&dev->saved_config_space[i]);
+ if ((i = pci_save_msi_state(dev)) != 0)
+ return i;
+ if ((i = pci_save_msix_state(dev)) != 0)
+ return i;
return 0;
}

@@ -452,6 +456,8 @@ pci_restore_state(struct pci_dev *dev)

for (i = 0; i < 16; i++)
pci_write_config_dword(dev,i * 4, dev->saved_config_space[i]);
+ pci_restore_msi_state(dev);
+ pci_restore_msix_state(dev);
return 0;
}

diff -puN drivers/pci/pci.h~msi_save_restore drivers/pci/pci.h
--- linux-2.6.15-rc5/drivers/pci/pci.h~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.h 2006-01-11 13:33:33.000000000 +0800
@@ -58,6 +58,17 @@ void disable_msi_mode(struct pci_dev *de
#else
static inline void disable_msi_mode(struct pci_dev *dev, int pos, int type) { }
#endif
+#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PM)
+int pci_save_msi_state(struct pci_dev *dev);
+int pci_save_msix_state(struct pci_dev *dev);
+void pci_restore_msi_state(struct pci_dev *dev);
+void pci_restore_msix_state(struct pci_dev *dev);
+#else
+static inline void pci_save_msi_state(struct pci_dev *dev) {}
+static inline void pci_save_msix_state(struct pci_dev *dev) {}
+static inline void pci_restore_msi_state(struct pci_dev *dev) {}
+static inline void pci_restore_msix_state(struct pci_dev *dev) {}
+#endif

extern int pcie_mch_quirk;
extern struct device_attribute pci_dev_attrs[];
diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
--- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/include/linux/pci.h 2006-01-11 15:45:59.000000000 +0800
@@ -78,6 +78,12 @@ typedef int __bitwise pci_power_t;
#define PCI_UNKNOWN ((pci_power_t __force) 5)
#define PCI_POWER_ERROR ((pci_power_t __force) -1)

+struct pci_cap_saved_state {
+ struct pci_cap_saved_state *next;
+ char cap_nr;
+ u32 data[0];
+};
+
/*
* The pci_dev structure is used to describe PCI devices.
*/
@@ -135,6 +141,7 @@ struct pci_dev {
unsigned int block_ucfg_access:1; /* userspace config space access is blocked */

u32 saved_config_space[16]; /* config space saved at suspend time */
+ struct pci_cap_saved_state *saved_cap_space; /* extend config space saved at suspend */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
@@ -145,6 +152,41 @@ struct pci_dev {
#define to_pci_dev(n) container_of(n, struct pci_dev, dev)
#define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)

+static inline struct pci_cap_saved_state *pci_find_saved_cap(
+ struct pci_dev *pci_dev,char cap)
+{
+ struct pci_cap_saved_state *tmp;
+ tmp = pci_dev->saved_cap_space;
+ while (tmp && tmp->cap_nr != cap)
+ tmp = tmp->next;
+ return tmp;
+}
+
+static inline void pci_add_saved_cap(struct pci_dev *pci_dev,
+ struct pci_cap_saved_state *new_cap)
+{
+ new_cap->next = pci_dev->saved_cap_space;
+ pci_dev->saved_cap_space = new_cap;
+}
+
+static inline void pci_remove_saved_cap(struct pci_dev *pci_dev,
+ struct pci_cap_saved_state *cap)
+{
+ struct pci_cap_saved_state *last;
+ last = pci_dev->saved_cap_space;
+ if (!last)
+ return;
+
+ if (last == cap) {
+ pci_dev->saved_cap_space = last->next;
+ return;
+ }
+ while (last->next && last->next != cap)
+ last = last->next;
+ if (last->next)
+ last->next = last->next->next;
+}
+
/*
* For PCI devices, the region numbers are assigned this way:
*
_

2006-01-11 15:20:51

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, Jan 11, 2006 at 04:18:22PM +0800, Shaohua Li wrote:
> +static inline void pci_remove_saved_cap(struct pci_dev *pci_dev,
> + struct pci_cap_saved_state *cap)
> +{
> + struct pci_cap_saved_state *last;
> + last = pci_dev->saved_cap_space;
> + if (!last)
> + return;
> +
> + if (last == cap) {
> + pci_dev->saved_cap_space = last->next;
> + return;
> + }
> + while (last->next && last->next != cap)
> + last = last->next;
> + if (last->next)
> + last->next = last->next->next;
> +}

I believe the more standard way of doing a singly-linked-list
delete looks like this:

{
struct pci_cap_saved_state **lastp = &pci_dev->saved_cap_space;

while (*lastp && *lastp != cap)
lastp = &(*lastp)->next;
if (*lastp)
*lastp = (*lastp)->next;
}

untested, of course.

2006-01-11 17:57:27

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, Jan 11, 2006 at 04:18:22PM +0800, Shaohua Li wrote:
> +struct pci_cap_saved_state {
> + struct pci_cap_saved_state *next;
> + char cap_nr;
> + u32 data[0];
> +};

Use the in-kernel list functions instead of creating your own logic for
a linked list.

thanks,

greg k-h

2006-01-11 18:39:48

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, Jan 11, 2006 at 07:51:42AM -0800, Greg KH wrote:
> On Wed, Jan 11, 2006 at 04:18:22PM +0800, Shaohua Li wrote:
> > +struct pci_cap_saved_state {
> > + struct pci_cap_saved_state *next;
> > + char cap_nr;
> > + u32 data[0];
> > +};
>
> Use the in-kernel list functions instead of creating your own logic for
> a linked list.

BTW, please use the hlist functions, not list_head for this case.

2006-01-12 02:31:56

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, 2006-01-11 at 07:51 -0800, Greg KH wrote:
> On Wed, Jan 11, 2006 at 04:18:22PM +0800, Shaohua Li wrote:
> > +struct pci_cap_saved_state {
> > + struct pci_cap_saved_state *next;
> > + char cap_nr;
> > + u32 data[0];
> > +};
>
> Use the in-kernel list functions instead of creating your own logic for
> a linked list.

Add MSI(X) configure space save/restore in generic PCI helper.

Signed-off-by: Shaohua Li<[email protected]>
---

linux-2.6.15-rc5-root/drivers/pci/msi.c | 227 ++++++++++++++++++++++++++----
linux-2.6.15-rc5-root/drivers/pci/pci.c | 6
linux-2.6.15-rc5-root/drivers/pci/pci.h | 11 +
linux-2.6.15-rc5-root/drivers/pci/probe.c | 1
linux-2.6.15-rc5-root/include/linux/pci.h | 30 +++
5 files changed, 246 insertions(+), 29 deletions(-)

diff -puN drivers/pci/msi.c~msi_save_restore drivers/pci/msi.c
--- linux-2.6.15-rc5/drivers/pci/msi.c~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/msi.c 2006-01-12 10:13:41.000000000 +0800
@@ -499,6 +499,201 @@ void pci_scan_msi_device(struct pci_dev
nr_reserved_vectors++;
}

+#ifdef CONFIG_PM
+int pci_save_msi_state(struct pci_dev *dev)
+{
+ int pos, i = 0;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ if (pos <= 0 || dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSI_FLAGS_ENABLE))
+ return 0;
+
+ save_state = kzalloc(sizeof(struct pci_cap_saved_state) + sizeof(u32) * 5,
+ GFP_KERNEL);
+ if (!save_state) {
+ printk(KERN_ERR "Out of memory in pci_save_msi_state\n");
+ return -ENOMEM;
+ }
+ cap = &save_state->data[0];
+
+ pci_read_config_dword(dev, pos, &cap[i++]);
+ control = cap[0] >> 16;
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, &cap[i++]);
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_64, &cap[i++]);
+ } else
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_32, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_read_config_dword(dev, pos + PCI_MSI_MASK_BIT, &cap[i++]);
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ save_state->cap_nr = PCI_CAP_ID_MSI;
+ pci_add_saved_cap(dev, save_state);
+ return 0;
+}
+
+void pci_restore_msi_state(struct pci_dev *dev)
+{
+ int i = 0, pos;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSI);
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ if (!save_state || pos <= 0)
+ return;
+ cap = &save_state->data[0];
+
+ control = cap[i++] >> 16;
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, cap[i++]);
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_64, cap[i++]);
+ } else
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_32, cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_write_config_dword(dev, pos + PCI_MSI_MASK_BIT, cap[i++]);
+ pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ pci_remove_saved_cap(save_state);
+ kfree(save_state);
+}
+
+int pci_save_msix_state(struct pci_dev *dev)
+{
+ int pos;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+ if (pos <= 0 || dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSIX_FLAGS_ENABLE))
+ return 0;
+ save_state = kzalloc(sizeof(struct pci_cap_saved_state) + sizeof(u16),
+ GFP_KERNEL);
+ if (!save_state) {
+ printk(KERN_ERR "Out of memory in pci_save_msix_state\n");
+ return -ENOMEM;
+ }
+ *((u16 *)&save_state->data[0]) = control;
+
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+ save_state->cap_nr = PCI_CAP_ID_MSIX;
+ pci_add_saved_cap(dev, save_state);
+ return 0;
+}
+
+void pci_restore_msix_state(struct pci_dev *dev)
+{
+ u16 save;
+ int pos;
+ int vector, head, tail = 0;
+ void __iomem *base;
+ int j;
+ struct msg_address address;
+ struct msg_data data;
+ struct msi_desc *entry;
+ int temp;
+ struct pci_cap_saved_state *save_state;
+
+ save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSIX);
+ if (!save_state)
+ return;
+ save = *((u16 *)&save_state->data[0]);
+ pci_remove_saved_cap(save_state);
+ kfree(save_state);
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+ if (pos <= 0)
+ return;
+
+ /* route the table */
+ temp = dev->irq;
+ if (msi_lookup_vector(dev, PCI_CAP_ID_MSIX))
+ return;
+ vector = head = dev->irq;
+ while (head != tail) {
+ entry = msi_desc[vector];
+ base = entry->mask_base;
+ j = entry->msi_attrib.entry_nr;
+
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+
+ address.lo_address.value &= MSI_ADDRESS_DEST_ID_MASK;
+ address.lo_address.value |= entry->msi_attrib.current_cpu <<
+ MSI_TARGET_CPU_SHIFT;
+
+ writel(address.lo_address.value,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET);
+ writel(address.hi_address,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET);
+ writel(*(u32*)&data,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_DATA_OFFSET);
+
+ tail = msi_desc[vector]->link.tail;
+ vector = tail;
+ }
+ dev->irq = temp;
+
+ pci_write_config_word(dev, msi_control_reg(pos), save);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+}
+#endif
+
+static void msi_register_init(struct pci_dev *dev, struct msi_desc *entry)
+{
+ struct msg_address address;
+ struct msg_data data;
+ int pos, vector = dev->irq;
+ u16 control;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ /* Configure MSI capability structure */
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+ entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
+ MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
+ pci_write_config_dword(dev, msi_lower_address_reg(pos),
+ address.lo_address.value);
+ if (is_64bit_address(control)) {
+ pci_write_config_dword(dev,
+ msi_upper_address_reg(pos), address.hi_address);
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 1), *((u32*)&data));
+ } else
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 0), *((u32*)&data));
+ if (entry->msi_attrib.maskbit) {
+ unsigned int maskbits, temp;
+ /* All MSIs are unmasked by default, Mask them all */
+ pci_read_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ &maskbits);
+ temp = (1 << multi_msi_capable(control));
+ temp = ((temp - 1) & ~temp);
+ maskbits |= temp;
+ pci_write_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ maskbits);
+ }
+}
+
/**
* msi_capability_init - configure device's MSI capability structure
* @dev: pointer to the pci_dev data structure of MSI device function
@@ -511,8 +706,6 @@ void pci_scan_msi_device(struct pci_dev
static int msi_capability_init(struct pci_dev *dev)
{
struct msi_desc *entry;
- struct msg_address address;
- struct msg_data data;
int pos, vector;
u16 control;

@@ -542,33 +735,8 @@ static int msi_capability_init(struct pc
/* Replace with MSI handler */
irq_handler_init(PCI_CAP_ID_MSI, vector, entry->msi_attrib.maskbit);
/* Configure MSI capability structure */
- msi_address_init(&address);
- msi_data_init(&data, vector);
- entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
- MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
- pci_write_config_dword(dev, msi_lower_address_reg(pos),
- address.lo_address.value);
- if (is_64bit_address(control)) {
- pci_write_config_dword(dev,
- msi_upper_address_reg(pos), address.hi_address);
- pci_write_config_word(dev,
- msi_data_reg(pos, 1), *((u32*)&data));
- } else
- pci_write_config_word(dev,
- msi_data_reg(pos, 0), *((u32*)&data));
- if (entry->msi_attrib.maskbit) {
- unsigned int maskbits, temp;
- /* All MSIs are unmasked by default, Mask them all */
- pci_read_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- &maskbits);
- temp = (1 << multi_msi_capable(control));
- temp = ((temp - 1) & ~temp);
- maskbits |= temp;
- pci_write_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- maskbits);
- }
+ msi_register_init(dev, entry);
+
attach_msi_entry(entry, vector);
/* Set MSI enabled bits */
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
@@ -717,6 +885,7 @@ int pci_enable_msi(struct pci_dev* dev)
vector_irq[dev->irq] = -1;
nr_released_vectors--;
spin_unlock_irqrestore(&msi_lock, flags);
+ msi_register_init(dev, msi_desc[dev->irq]);
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
return 0;
}
diff -puN drivers/pci/pci.c~msi_save_restore drivers/pci/pci.c
--- linux-2.6.15-rc5/drivers/pci/pci.c~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.c 2006-01-11 13:33:33.000000000 +0800
@@ -438,6 +438,10 @@ pci_save_state(struct pci_dev *dev)
/* XXX: 100% dword access ok here? */
for (i = 0; i < 16; i++)
pci_read_config_dword(dev, i * 4,&dev->saved_config_space[i]);
+ if ((i = pci_save_msi_state(dev)) != 0)
+ return i;
+ if ((i = pci_save_msix_state(dev)) != 0)
+ return i;
return 0;
}

@@ -452,6 +456,8 @@ pci_restore_state(struct pci_dev *dev)

for (i = 0; i < 16; i++)
pci_write_config_dword(dev,i * 4, dev->saved_config_space[i]);
+ pci_restore_msi_state(dev);
+ pci_restore_msix_state(dev);
return 0;
}

diff -puN drivers/pci/pci.h~msi_save_restore drivers/pci/pci.h
--- linux-2.6.15-rc5/drivers/pci/pci.h~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.h 2006-01-12 10:24:48.000000000 +0800
@@ -58,6 +58,17 @@ void disable_msi_mode(struct pci_dev *de
#else
static inline void disable_msi_mode(struct pci_dev *dev, int pos, int type) { }
#endif
+#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PM)
+int pci_save_msi_state(struct pci_dev *dev);
+int pci_save_msix_state(struct pci_dev *dev);
+void pci_restore_msi_state(struct pci_dev *dev);
+void pci_restore_msix_state(struct pci_dev *dev);
+#else
+static inline int pci_save_msi_state(struct pci_dev *dev) { return 0; }
+static inline int pci_save_msix_state(struct pci_dev *dev) { return 0; }
+static inline void pci_restore_msi_state(struct pci_dev *dev) {}
+static inline void pci_restore_msix_state(struct pci_dev *dev) {}
+#endif

extern int pcie_mch_quirk;
extern struct device_attribute pci_dev_attrs[];
diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
--- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/include/linux/pci.h 2006-01-12 10:09:12.000000000 +0800
@@ -78,6 +78,12 @@ typedef int __bitwise pci_power_t;
#define PCI_UNKNOWN ((pci_power_t __force) 5)
#define PCI_POWER_ERROR ((pci_power_t __force) -1)

+struct pci_cap_saved_state {
+ struct list_head next;
+ char cap_nr;
+ u32 data[0];
+};
+
/*
* The pci_dev structure is used to describe PCI devices.
*/
@@ -135,6 +141,7 @@ struct pci_dev {
unsigned int block_ucfg_access:1; /* userspace config space access is blocked */

u32 saved_config_space[16]; /* config space saved at suspend time */
+ struct list_head saved_cap_space; /* extend config space saved at suspend */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
@@ -145,6 +152,29 @@ struct pci_dev {
#define to_pci_dev(n) container_of(n, struct pci_dev, dev)
#define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)

+static inline struct pci_cap_saved_state *pci_find_saved_cap(
+ struct pci_dev *pci_dev,char cap)
+{
+ struct pci_cap_saved_state *tmp;
+
+ list_for_each_entry(tmp, &pci_dev->saved_cap_space, next) {
+ if (tmp->cap_nr == cap)
+ return tmp;
+ }
+ return NULL;
+}
+
+static inline void pci_add_saved_cap(struct pci_dev *pci_dev,
+ struct pci_cap_saved_state *new_cap)
+{
+ list_add(&new_cap->next, &pci_dev->saved_cap_space);
+}
+
+static inline void pci_remove_saved_cap(struct pci_cap_saved_state *cap)
+{
+ list_del(&cap->next);
+}
+
/*
* For PCI devices, the region numbers are assigned this way:
*
diff -puN drivers/pci/probe.c~msi_save_restore drivers/pci/probe.c
--- linux-2.6.15-rc5/drivers/pci/probe.c~msi_save_restore 2006-01-12 10:12:00.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/probe.c 2006-01-12 10:12:55.000000000 +0800
@@ -788,6 +788,7 @@ void __devinit pci_device_add(struct pci
/* Fix up broken headers */
pci_fixup_device(pci_fixup_header, dev);

+ INIT_LIST_HEAD(&dev->saved_cap_space);
/*
* Add the device to our list of discovered devices
* and the bus list for fixup functions, etc.
_

2006-01-12 02:47:33

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Thu, Jan 12, 2006 at 10:30:59AM +0800, Shaohua Li wrote:

Did you not understand my comment about using the hlist functions?

> +struct pci_cap_saved_state {
> + struct list_head next;

struct hlist_node list;

> + char cap_nr;
> + u32 data[0];
> +};

> u32 saved_config_space[16]; /* config space saved at suspend time */
> + struct list_head saved_cap_space; /* extend config space saved at suspend */

struct hlist_head saved_cap_space;

BTW, the comment is confusing and wrong. Extended config space is
0x100-0xfff. How about simply:

/* Capabilities saved on suspend */

Or even omit it. I think it's pretty obvious what "saved_cap_space" is.

> +static inline struct pci_cap_saved_state *pci_find_saved_cap(
> + struct pci_dev *pci_dev,char cap)
> +{
> + struct pci_cap_saved_state *tmp;
> +
> + list_for_each_entry(tmp, &pci_dev->saved_cap_space, next) {

hlist_for_each_entry

> + if (tmp->cap_nr == cap)
> + return tmp;
> + }
> + return NULL;
> +}
> +
> +static inline void pci_add_saved_cap(struct pci_dev *pci_dev,
> + struct pci_cap_saved_state *new_cap)
> +{
> + list_add(&new_cap->next, &pci_dev->saved_cap_space);

hlist_add_head

> +}
> +
> +static inline void pci_remove_saved_cap(struct pci_cap_saved_state *cap)
> +{
> + list_del(&cap->next);

hlist_del

> +}
> +
> /*
> * For PCI devices, the region numbers are assigned this way:
> *
> diff -puN drivers/pci/probe.c~msi_save_restore drivers/pci/probe.c
> --- linux-2.6.15-rc5/drivers/pci/probe.c~msi_save_restore 2006-01-12 10:12:00.000000000 +0800
> +++ linux-2.6.15-rc5-root/drivers/pci/probe.c 2006-01-12 10:12:55.000000000 +0800
> @@ -788,6 +788,7 @@ void __devinit pci_device_add(struct pci
> /* Fix up broken headers */
> pci_fixup_device(pci_fixup_header, dev);
>
> + INIT_LIST_HEAD(&dev->saved_cap_space);

and while you could use INIT_HLIST_HEAD, you shouldn't need to since an
empty hlist is NULL.

2006-01-12 03:18:36

[permalink] [raw]

Subject: Re: [PATCH 1/2]MSI(X) save/restore for suspend/resume

On Wed, 2006-01-11 at 19:47 -0700, Matthew Wilcox wrote:
> On Thu, Jan 12, 2006 at 10:30:59AM +0800, Shaohua Li wrote:
>
> Did you not understand my comment about using the hlist functions?
>
> > +struct pci_cap_saved_state {
> > + struct list_head next;
>
> struct hlist_node list;

Add MSI(X) configure space save/restore in generic PCI helper.

Signed-off-by: Shaohua Li <[email protected]>
---

linux-2.6.15-rc5-root/drivers/pci/msi.c | 227 ++++++++++++++++++++++++++----
linux-2.6.15-rc5-root/drivers/pci/pci.c | 6
linux-2.6.15-rc5-root/drivers/pci/pci.h | 11 +
linux-2.6.15-rc5-root/include/linux/pci.h | 31 ++++
5 files changed, 246 insertions(+), 29 deletions(-)

diff -puN drivers/pci/msi.c~msi_save_restore drivers/pci/msi.c
--- linux-2.6.15-rc5/drivers/pci/msi.c~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/msi.c 2006-01-12 10:13:41.000000000 +0800
@@ -499,6 +499,201 @@ void pci_scan_msi_device(struct pci_dev
nr_reserved_vectors++;
}

+#ifdef CONFIG_PM
+int pci_save_msi_state(struct pci_dev *dev)
+{
+ int pos, i = 0;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ if (pos <= 0 || dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSI_FLAGS_ENABLE))
+ return 0;
+
+ save_state = kzalloc(sizeof(struct pci_cap_saved_state) + sizeof(u32) * 5,
+ GFP_KERNEL);
+ if (!save_state) {
+ printk(KERN_ERR "Out of memory in pci_save_msi_state\n");
+ return -ENOMEM;
+ }
+ cap = &save_state->data[0];
+
+ pci_read_config_dword(dev, pos, &cap[i++]);
+ control = cap[0] >> 16;
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, &cap[i++]);
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_64, &cap[i++]);
+ } else
+ pci_read_config_dword(dev, pos + PCI_MSI_DATA_32, &cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_read_config_dword(dev, pos + PCI_MSI_MASK_BIT, &cap[i++]);
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ save_state->cap_nr = PCI_CAP_ID_MSI;
+ pci_add_saved_cap(dev, save_state);
+ return 0;
+}
+
+void pci_restore_msi_state(struct pci_dev *dev)
+{
+ int i = 0, pos;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+ u32 *cap;
+
+ save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSI);
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ if (!save_state || pos <= 0)
+ return;
+ cap = &save_state->data[0];
+
+ control = cap[i++] >> 16;
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_LO, cap[i++]);
+ if (control & PCI_MSI_FLAGS_64BIT) {
+ pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_HI, cap[i++]);
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_64, cap[i++]);
+ } else
+ pci_write_config_dword(dev, pos + PCI_MSI_DATA_32, cap[i++]);
+ if (control & PCI_MSI_FLAGS_MASKBIT)
+ pci_write_config_dword(dev, pos + PCI_MSI_MASK_BIT, cap[i++]);
+ pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
+ pci_remove_saved_cap(save_state);
+ kfree(save_state);
+}
+
+int pci_save_msix_state(struct pci_dev *dev)
+{
+ int pos;
+ u16 control;
+ struct pci_cap_saved_state *save_state;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+ if (pos <= 0 || dev->no_msi)
+ return 0;
+
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ if (!(control & PCI_MSIX_FLAGS_ENABLE))
+ return 0;
+ save_state = kzalloc(sizeof(struct pci_cap_saved_state) + sizeof(u16),
+ GFP_KERNEL);
+ if (!save_state) {
+ printk(KERN_ERR "Out of memory in pci_save_msix_state\n");
+ return -ENOMEM;
+ }
+ *((u16 *)&save_state->data[0]) = control;
+
+ disable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+ save_state->cap_nr = PCI_CAP_ID_MSIX;
+ pci_add_saved_cap(dev, save_state);
+ return 0;
+}
+
+void pci_restore_msix_state(struct pci_dev *dev)
+{
+ u16 save;
+ int pos;
+ int vector, head, tail = 0;
+ void __iomem *base;
+ int j;
+ struct msg_address address;
+ struct msg_data data;
+ struct msi_desc *entry;
+ int temp;
+ struct pci_cap_saved_state *save_state;
+
+ save_state = pci_find_saved_cap(dev, PCI_CAP_ID_MSIX);
+ if (!save_state)
+ return;
+ save = *((u16 *)&save_state->data[0]);
+ pci_remove_saved_cap(save_state);
+ kfree(save_state);
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+ if (pos <= 0)
+ return;
+
+ /* route the table */
+ temp = dev->irq;
+ if (msi_lookup_vector(dev, PCI_CAP_ID_MSIX))
+ return;
+ vector = head = dev->irq;
+ while (head != tail) {
+ entry = msi_desc[vector];
+ base = entry->mask_base;
+ j = entry->msi_attrib.entry_nr;
+
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+
+ address.lo_address.value &= MSI_ADDRESS_DEST_ID_MASK;
+ address.lo_address.value |= entry->msi_attrib.current_cpu <<
+ MSI_TARGET_CPU_SHIFT;
+
+ writel(address.lo_address.value,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET);
+ writel(address.hi_address,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET);
+ writel(*(u32*)&data,
+ base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_DATA_OFFSET);
+
+ tail = msi_desc[vector]->link.tail;
+ vector = tail;
+ }
+ dev->irq = temp;
+
+ pci_write_config_word(dev, msi_control_reg(pos), save);
+ enable_msi_mode(dev, pos, PCI_CAP_ID_MSIX);
+}
+#endif
+
+static void msi_register_init(struct pci_dev *dev, struct msi_desc *entry)
+{
+ struct msg_address address;
+ struct msg_data data;
+ int pos, vector = dev->irq;
+ u16 control;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
+ pci_read_config_word(dev, msi_control_reg(pos), &control);
+ /* Configure MSI capability structure */
+ msi_address_init(&address);
+ msi_data_init(&data, vector);
+ entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
+ MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
+ pci_write_config_dword(dev, msi_lower_address_reg(pos),
+ address.lo_address.value);
+ if (is_64bit_address(control)) {
+ pci_write_config_dword(dev,
+ msi_upper_address_reg(pos), address.hi_address);
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 1), *((u32*)&data));
+ } else
+ pci_write_config_word(dev,
+ msi_data_reg(pos, 0), *((u32*)&data));
+ if (entry->msi_attrib.maskbit) {
+ unsigned int maskbits, temp;
+ /* All MSIs are unmasked by default, Mask them all */
+ pci_read_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ &maskbits);
+ temp = (1 << multi_msi_capable(control));
+ temp = ((temp - 1) & ~temp);
+ maskbits |= temp;
+ pci_write_config_dword(dev,
+ msi_mask_bits_reg(pos, is_64bit_address(control)),
+ maskbits);
+ }
+}
+
/**
* msi_capability_init - configure device's MSI capability structure
* @dev: pointer to the pci_dev data structure of MSI device function
@@ -511,8 +706,6 @@ void pci_scan_msi_device(struct pci_dev
static int msi_capability_init(struct pci_dev *dev)
{
struct msi_desc *entry;
- struct msg_address address;
- struct msg_data data;
int pos, vector;
u16 control;

@@ -542,33 +735,8 @@ static int msi_capability_init(struct pc
/* Replace with MSI handler */
irq_handler_init(PCI_CAP_ID_MSI, vector, entry->msi_attrib.maskbit);
/* Configure MSI capability structure */
- msi_address_init(&address);
- msi_data_init(&data, vector);
- entry->msi_attrib.current_cpu = ((address.lo_address.u.dest_id >>
- MSI_TARGET_CPU_SHIFT) & MSI_TARGET_CPU_MASK);
- pci_write_config_dword(dev, msi_lower_address_reg(pos),
- address.lo_address.value);
- if (is_64bit_address(control)) {
- pci_write_config_dword(dev,
- msi_upper_address_reg(pos), address.hi_address);
- pci_write_config_word(dev,
- msi_data_reg(pos, 1), *((u32*)&data));
- } else
- pci_write_config_word(dev,
- msi_data_reg(pos, 0), *((u32*)&data));
- if (entry->msi_attrib.maskbit) {
- unsigned int maskbits, temp;
- /* All MSIs are unmasked by default, Mask them all */
- pci_read_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- &maskbits);
- temp = (1 << multi_msi_capable(control));
- temp = ((temp - 1) & ~temp);
- maskbits |= temp;
- pci_write_config_dword(dev,
- msi_mask_bits_reg(pos, is_64bit_address(control)),
- maskbits);
- }
+ msi_register_init(dev, entry);
+
attach_msi_entry(entry, vector);
/* Set MSI enabled bits */
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
@@ -717,6 +885,7 @@ int pci_enable_msi(struct pci_dev* dev)
vector_irq[dev->irq] = -1;
nr_released_vectors--;
spin_unlock_irqrestore(&msi_lock, flags);
+ msi_register_init(dev, msi_desc[dev->irq]);
enable_msi_mode(dev, pos, PCI_CAP_ID_MSI);
return 0;
}
diff -puN drivers/pci/pci.c~msi_save_restore drivers/pci/pci.c
--- linux-2.6.15-rc5/drivers/pci/pci.c~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.c 2006-01-11 13:33:33.000000000 +0800
@@ -438,6 +438,10 @@ pci_save_state(struct pci_dev *dev)
/* XXX: 100% dword access ok here? */
for (i = 0; i < 16; i++)
pci_read_config_dword(dev, i * 4,&dev->saved_config_space[i]);
+ if ((i = pci_save_msi_state(dev)) != 0)
+ return i;
+ if ((i = pci_save_msix_state(dev)) != 0)
+ return i;
return 0;
}

@@ -452,6 +456,8 @@ pci_restore_state(struct pci_dev *dev)

for (i = 0; i < 16; i++)
pci_write_config_dword(dev,i * 4, dev->saved_config_space[i]);
+ pci_restore_msi_state(dev);
+ pci_restore_msix_state(dev);
return 0;
}

diff -puN drivers/pci/pci.h~msi_save_restore drivers/pci/pci.h
--- linux-2.6.15-rc5/drivers/pci/pci.h~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/drivers/pci/pci.h 2006-01-12 10:24:48.000000000 +0800
@@ -58,6 +58,17 @@ void disable_msi_mode(struct pci_dev *de
#else
static inline void disable_msi_mode(struct pci_dev *dev, int pos, int type) { }
#endif
+#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PM)
+int pci_save_msi_state(struct pci_dev *dev);
+int pci_save_msix_state(struct pci_dev *dev);
+void pci_restore_msi_state(struct pci_dev *dev);
+void pci_restore_msix_state(struct pci_dev *dev);
+#else
+static inline int pci_save_msi_state(struct pci_dev *dev) { return 0; }
+static inline int pci_save_msix_state(struct pci_dev *dev) { return 0; }
+static inline void pci_restore_msi_state(struct pci_dev *dev) {}
+static inline void pci_restore_msix_state(struct pci_dev *dev) {}
+#endif

extern int pcie_mch_quirk;
extern struct device_attribute pci_dev_attrs[];
diff -puN include/linux/pci.h~msi_save_restore include/linux/pci.h
--- linux-2.6.15-rc5/include/linux/pci.h~msi_save_restore 2006-01-11 13:33:33.000000000 +0800
+++ linux-2.6.15-rc5-root/include/linux/pci.h 2006-01-12 11:01:39.000000000 +0800
@@ -78,6 +78,12 @@ typedef int __bitwise pci_power_t;
#define PCI_UNKNOWN ((pci_power_t __force) 5)
#define PCI_POWER_ERROR ((pci_power_t __force) -1)

+struct pci_cap_saved_state {
+ struct hlist_node next;
+ char cap_nr;
+ u32 data[0];
+};
+
/*
* The pci_dev structure is used to describe PCI devices.
*/
@@ -135,6 +141,7 @@ struct pci_dev {
unsigned int block_ucfg_access:1; /* userspace config space access is blocked */

u32 saved_config_space[16]; /* config space saved at suspend time */
+ struct hlist_head saved_cap_space;
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
@@ -145,6 +152,30 @@ struct pci_dev {
#define to_pci_dev(n) container_of(n, struct pci_dev, dev)
#define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)

+static inline struct pci_cap_saved_state *pci_find_saved_cap(
+ struct pci_dev *pci_dev,char cap)
+{
+ struct pci_cap_saved_state *tmp;
+ struct hlist_node *pos;
+
+ hlist_for_each_entry(tmp, pos, &pci_dev->saved_cap_space, next) {
+ if (tmp->cap_nr == cap)
+ return tmp;
+ }
+ return NULL;
+}
+
+static inline void pci_add_saved_cap(struct pci_dev *pci_dev,
+ struct pci_cap_saved_state *new_cap)
+{
+ hlist_add_head(&new_cap->next, &pci_dev->saved_cap_space);
+}
+
+static inline void pci_remove_saved_cap(struct pci_cap_saved_state *cap)
+{
+ hlist_del(&cap->next);
+}
+
/*
* For PCI devices, the region numbers are assigned this way:
*
_

2006-01-12 07:16:36