From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhenzhong Duan Subject: Re: [PATCH 1/2] xen, libxc: init msix addr/data with value from qemu via hypercall Date: Fri, 10 May 2013 15:39:29 +0800 Message-ID: <518CA431.30800@oracle.com> References: <518A0A15.60301@oracle.com> <518A397F02000078000D44AE@nat28.tlf.novell.com> <518A2237.9060709@oracle.com> <518A5B4A02000078000D4668@nat28.tlf.novell.com> <518B11CD.1050308@oracle.com> <518C019C020000780009A295@nat28.tlf.novell.com> <518C604B.7010206@oracle.com> <518CB1C502000078000D4F61@nat28.tlf.novell.com> Reply-To: zhenzhong.duan@oracle.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <518CB1C502000078000D4F61@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: chien.yen@oracle.com, konrad.wilk@oracle.com, joe.jin@oracle.com, yuval.shaia@oracle.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 2013-05-10 14:37, Jan Beulich wrote: >>>> On 10.05.13 at 04:49, Zhenzhong Duan wrote: >> On 2013-05-10 03:05, Jan Beulich wrote: >>>>>> Zhenzhong Duan 05/09/13 5:02 AM >>> >>>> On 2013/5/8 20:03, Jan Beulich wrote: >>>>> But of course I still don't really understand why all of the sudden >>>>> this needs to be passed in rather than being under the full control >>>>> of the hypervisor at all times. Perhaps this is related to me not >>>>> understanding why the kernel would read these values at all: >>>>> There's no other place in the kernel where the message would >>>>> be read before first getting written (in fact, apart from the >>>>> use of __read_msi_msg() by the Xen code, there's only one >>>>> other user under arch/powerpc/, and there - according to the >>>>> accompanying comment - this is just to save away the data for >>>>> later use during resume). >>>> There is a bug if msi_ad is not passed in. >>>> >>>> when driver first load, >>>> >>>> kernel.__read_msi_msg() >>>> (got all zero) >>> But you don't even comment on the apparently bogus use of the function here. >> This pattern is used only when hvm_pirq is enabled. kernel need to check >> XEN_PIRQ_MSI_DATA. >> It's not a issue if data is 0 at first driver load, kernel will call >> __write_msi_msg with pirq and XEN_PIRQ_MSI_DATA set. > But this doesn't make the use of __read_msi_msg() less bogus. It's > not clear on what basis this mechanism got invented in the first > place. It's there since hvm_irq introduced. But it works indeed. > >>>> kernel.__write_msi_msg(pirq) >>>> (ioreq passed to qemu as no msixtbl_entry established yet) >>>> qemu.pt_msi_update_one() >>>> xc_domain_update_msi_irq() >>>> (msixtbl_entry dynamicly allocated with msi_ad all zero) >>>> >>>> then driver unload, >>>> ... >>>> driver load again, >>>> >>>> kernel.__read_msi_msg() >>>> (got all zero from xen as accelerated entry just established with all zero) >>> If all zeroes get returned, why would the flow here be different then above? >> Because pirq and related mapping and binding are not freed between >> driver load-unload-load. They are freed when device detach. >> We should try to use the last pirq. > But then you need to solve the problem generically, i.e. not just > for the driver reload case, but also for e.g. the kexec one (where > __read_msi_msg() returning other than all zeros wouldn't help you > as xen_irq_from_pirq() would then return -1, and you'd be back to > the same problem. No, not only kexec ones, it's driver unload that makes xen_irq_from_pirq return -1. So there is also a bug in kernel side. I have sent a patch about kernel. I think you miss it. http://www.gossamer-threads.com/lists/xen/devel/281498 > IOW I think the prior IRQ needs to be freed > anyway rather than an attempt be made to reuse it. I have ever thought about this idea, but when to free the pirq is a problem. When driver unload? qemu has no idea of if driver unloaded. When msix entry masked? kernel mask and unmask msix entry intermittently, especially when irqbalance enabled. So based on above, I think it's better to reuse same pirq, only free it when device detached. Regards zduan