RE: [Xen-devel] PCI MSI questions

["Shan, Haitao" <haitao.shan@xxxxxxxxx>]

Jan Beulich wrote:
> Looking at the MSI implementation I have a couple of questions:
> 
> 1) There currently seems to be a hidden requirement of NR_PIRQS in the
> kernel needing to be no smaller than NR_IRQS in the hypervisor.
> Otherwise, the pirq returned from PHYSDEVOP_map_pirq may collide
> with the dynamic IRQs in the kernel or even be out of range
> altogether. Therefore I think that NR_PIRQS has to become a variable
> defaulting 
> to 256 but getting initialized from a hypervisor reported value
> (perhaps in start_info, or else from a new (sub-)hypercall).
> 
> 2) While pci_restore_msi_state() properly checks the success of
> msi_map_pirq_to_vector(), pci_restore_msix_state() doesn't. Is this
> for a reason, or just because the code would get more complex if the
> error needs to be handled?
Yes. I do not know what is the proper action. If one of the MSI-X pirq
failed, should we return? Or unmap those already mapped and return? Or
continue processing other MSI-X entries?
Any comments on this? Jan.

> 
> 3) The type parameter of xc_physdev_map_pirq{,_msi}() seems
> superfluous, or is there any reason why these could be called with the
> respectively reversed types?
Yes. The type is not useful in current code.
I am not quite sure about the reason. I think at the beginning of
submitting the patches, we do not have two seperate wrap functions for
this hypercall (only xc_physdev_map_pirq). That's where the "type"
parameter comes. Later, with MSI capabilities owned by Xen, we need pass
down more information to Xen via this hypercall. Thus the second one was
born.
Agree that this may need to be cleaned up.

> 
> 4) The hypervisor option "msi_irq_enable" seems to be named pretty
> oddly - both the "irq" and the "enable" in the name are more or less
> redundant. So unless there's a reason for this long a name for an
> option that generally I would expect most people want to set (at
> least on bigger systems), I'd like to change it into "msi" or, if
> that's considered prone for ambiguity, "pci-msi". Also, are there any
> plans when to make have default be on rather than off?
> 
> Thanks, Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel