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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.12.12 at 15:16, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Mon, 17 Dec 2012, Jan Beulich wrote:
>> >>> 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?
> 
> I was working under the assumption that before the call to
> msix_capability_init (in particular before
> rangeset_add_range(mmio_ro_ranges, dev->msix_table...) in Xen, the table
> is actually writeable by the guest.
> 
> If that is the case, then this scheme should work.
> If it is not the case, this patch is wrong.

The question is not if or when the table is writable - the Dom0
kernel should _never_ try to write to the mask bits.

Jan


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