>>> On 20.10.17 at 19:55, <andrew.coop...@citrix.com> wrote: > On 20/10/17 08:12, Jan Beulich wrote: >>>>> On 19.10.17 at 18:22, <andrew.coop...@citrix.com> wrote: >>> DMA-ing to the stack is generally considered bad practice. In this case, >>> if > a >>> timeout occurs because of a sluggish device which is processing the request, >>> the completion notification will corrupt the stack of a subsequent deeper > call >>> tree. >>> >>> Place the poll_slot in a percpu area and DMA to that instead. >>> >>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> Please could you extend the commit message to state the issue >> remaining with using a single per-CPU slot? With that >> Reviewed-by: Jan Beulich <jbeul...@suse.com> > > How about this? > > Note: This change does not address other issues with the current > implementation, such as once a timeout has been suffered, subsequent > completions can't be correlated with their requests.
Sounds good. >> albeit ... >> >>> @@ -167,7 +169,7 @@ static int __must_check queue_invalidate_wait(struct >>> iommu *iommu, >>> qinval_entry->q.inv_wait_dsc.lo.res_1 = 0; >>> qinval_entry->q.inv_wait_dsc.lo.sdata = QINVAL_STAT_DONE; >>> qinval_entry->q.inv_wait_dsc.hi.res_1 = 0; >>> - qinval_entry->q.inv_wait_dsc.hi.saddr = virt_to_maddr(&poll_slot) >> 2; >>> + qinval_entry->q.inv_wait_dsc.hi.saddr = virt_to_maddr(this_poll_slot) >>> >> 2; >> ... this one is still a literal number rather than something allowing >> to associate back where that value is coming from (but since you're >> not introducing it here, I also won't insist on you changing it in this >> patch). > > I don't understand. What is still a literal number? There's still that literal 2 there as the shift count. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
