From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: PCI MSI questions Date: Thu, 24 Jul 2008 09:22:54 +0100 Message-ID: <488857FE.76E4.0078.0@novell.com> References: <4888452A.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <4888452A.76E4.0078.0@novell.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org >>> "Jan Beulich" 24.07.08 09:02 >>> >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? > >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? > >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? 5) Shouldn't most of the MSI/MSI-X capability structures be write- protected even in permissive mode (which at once would suppress the warning I'd expect from pciback_config_write() for HVM guests which get an MSI capable device assigned)? Or shouldn't perhaps writes of incorrect control/address/data values even render the device non-functional? And shouldn't then reads return the values written rather than the ones in hardware? Jan