WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] PCI MSI questions

To: "Jan Beulich" <jbeulich@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] PCI MSI questions
From: "Shan, Haitao" <haitao.shan@xxxxxxxxx>
Date: Thu, 24 Jul 2008 16:39:12 +0800
Cc:
Delivery-date: Thu, 24 Jul 2008 01:42:10 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4888452A.76E4.0078.0@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4888452A.76E4.0078.0@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcjtW0Z4TRTbzNYyS4W4njVtIcgzCwAC3i6w
Thread-topic: [Xen-devel] PCI MSI questions
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

<Prev in Thread] Current Thread [Next in Thread>