On Sun, Oct 24, 2021 at 2:55 PM Josef Johansson <josef@xxxxxxxxxxx> wrote:
> I ended up with this patch, I also masked pci_set_mask and
> pci_set_unmask, even though patching __pci_restore_msi_state and
> __pci_restore_msi_state solved this problem, I found that it did not
> properly make the system be able to survive flip_done timeout related
> problems during suspend/resume. Would this be something you had in mind
> Marc? I will make one more try with just patching
> __pci_restore_msi_state and __pci_restore_msix_state just to make sure.
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index
> 4b4792940e86..0b2225066778 100644 --- a/drivers/pci/msi.c +++
> b/drivers/pci/msi.c @@ -420,7 +420,8 @@ static void
> __pci_restore_msi_state(struct pci_dev *dev) arch_restore_msi_irqs(dev);
> pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control); -
> pci_msi_update_mask(entry, 0, 0); + if (!(pci_msi_ignore_mask ||
> entry->msi_attrib.is_virtual)) + pci_msi_update_mask(entry, 0, 0);
> control &= ~PCI_MSI_FLAGS_QSIZE; control |= (entry->msi_attrib.multiple
This patch was mangled.
> This makes sense the patch would be like so, I'm testing this out now
> hoping it will
>
> perform as good. Now the check is performed in four places
Close. I'll reply with my compiled, but untested patch of what I was thinking.
> That leaves me with a though, will this set masked, and should be checked as well?
>
> void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> {
> struct pci_dev *dev = msi_desc_to_pci_dev(entry);
>
> if (dev->current_state != PCI_D0 || pci_dev_is_disconnected(dev)) {
> /* Don't touch the hardware now */
> } else if (entry->msi_attrib.is_msix) {
> void __iomem *base = pci_msix_desc_addr(entry);
> u32 ctrl = entry->msix_ctrl;
> bool unmasked = !(ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT);
>
> if (entry->msi_attrib.is_virtual)
> goto skip;
>
> /*
> * The specification mandates that the entry is masked
> * when the message is modified:
> *
> * "If software changes the Address or Data value of an
> * entry while the entry is unmasked, the result is
> * undefined."
> */
> if (unmasked)
> >>> pci_msix_write_vector_ctrl(entry, ctrl | PCI_MSIX_ENTRY_CTRL_MASKBIT);
My patch adds a check in pci_msix_write_vector_ctrl(), but the comment
above means PV Xen's behavior may be incorrect if Linux is calling
this function and modifying the message.
Regards,
Jason
