[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] PCI/MSI: Re-add checks for skip masking MSI-X on Xen PV



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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.