Re: [Xen-devel] [PVH]: Help: msi.c

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.12.12 at 13:42, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Fri, 14 Dec 2012, Mukesh Rathor wrote:
>> On Thu, 13 Dec 2012 14:25:16 +0000
>> Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> 
>> > On Thu, 13 Dec 2012, Jan Beulich wrote:
>> > > >>> On 13.12.12 at 13:19, Stefano Stabellini
>> > > >>> <stefano.stabellini@xxxxxxxxxxxxx>
>> > > wrote:
>> > > > On Thu, 13 Dec 2012, Jan Beulich wrote:
>> > > >> >>> On 13.12.12 at 02:43, Mukesh Rathor
>> > > >> >>> <mukesh.rathor@xxxxxxxxxx> wrote:
>> > 
>> > Actually I think that you might be right: just looking at the code it
>> > seems that the mask bits get written to the table once as part of the
>> > initialization process:
>> > 
>> > pci_enable_msix -> msix_capability_init -> msix_program_entries
>> > 
>> > Unfortunately msix_program_entries is called few lines after
>> > arch_setup_msi_irqs, where we call PHYSDEVOP_map_pirq to map the MSI
>> > as a pirq.
>> > However after that is done, all the masking/unmask is done via
>> > irq_mask that we handle properly masking/unmasking the corresponding
>> > event channels.
>> > 
>> > 
>> > Possible solutions on top of my head:
>> > 
>> > - in msix_program_entries instead of writing to the table directly
>> > (__msix_mask_irq), call desc->irq_data.chip->irq_mask(); 
>> > 
>> > - replace msix_program_entries with arch_msix_program_entries, but it
>> > would probably be unpopular.
>> 
>> 
>> Can you or Jan or somebody please take that over? I can focus on other
>> PVH things then and try to get a patch in asap.
> 
> The following patch moves the MSI-X masking before arch_setup_msi_irqs,
> that is when Linux calls PHYSDEVOP_map_pirq that is the hypercall that
> causes Xen to execute msix_capability_init.

And in what way would that help?

Jan

> I don't have access to a machine with an MSI-X device right now so I
> have only tested the appended patch with MSI.
> 
> ---
> 
> 
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index a825d78..ef73e80 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -652,11 +652,22 @@ static void msix_program_entries(struct pci_dev *dev,
>       int i = 0;
>  
>       list_for_each_entry(entry, &dev->msi_list, list) {
> +             entries[i].vector = entry->irq;
> +             irq_set_msi_desc(entry->irq, entry);
> +             i++;
> +     }
> +}
> +
> +static void msix_mask_entries(struct pci_dev *dev,
> +                                     struct msix_entry *entries)
> +{
> +     struct msi_desc *entry;
> +     int i = 0;
> +
> +     list_for_each_entry(entry, &dev->msi_list, list) {
>               int offset = entries[i].entry * PCI_MSIX_ENTRY_SIZE +
>                                               PCI_MSIX_ENTRY_VECTOR_CTRL;
>  
> -             entries[i].vector = entry->irq;
> -             irq_set_msi_desc(entry->irq, entry);
>               entry->masked = readl(entry->mask_base + offset);
>               msix_mask_irq(entry, 1);
>               i++;
> @@ -696,6 +707,8 @@ static int msix_capability_init(struct pci_dev *dev,
>       if (ret)
>               return ret;
>  
> +     msix_mask_entries(dev, entries);
> +
>       ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
>       if (ret)
>               goto error;




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel