linux-kernel - Re: [PATCH 3/3] msix restore code optimization for dom0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 25 Jul 2013 15:17:29 +0800
From:	Zhenzhong Duan <zhenzhong.duan@...cle.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	linux-pci@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	xen-devel <xen-devel@...ts.xen.org>, bhelgaas@...gle.com,
	Feng Jin <joe.jin@...cle.com>,
	"x86@...nel.org >> \"x86@...nel.org\"" <x86@...nel.org>
Subject: Re: [PATCH 3/3] msix restore code optimization for dom0


On 2013-07-24 21:55, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 24, 2013 at 11:08:43AM +0800, Zhenzhong Duan wrote:
>> PHYSDEVOP_restore_msi is used to restore all msix entrys in one hypercall
>> in dom0. But it is called multi times in current code.
> Couldn't the restore_msi_irqs instead check whether it has already
> made the hypercall (perhaps by seeing if the MSI-X's are enabled?)
> and if so don't do the hypercall?
I think it's better to remove the for loop for dom0, also hard to know 
when to clear hypercall count.
>
> I think you are addressing the problem from a different viewpoint.
>
> The problem is not with the API (the x86_msi one). The problem
> is with the implementation (x86_msi.restore_msi_irq - specifically
> the Xen one) having an side effect.
We have x86_msi.restore_msi_irqs and no x86_msi.restore_msi_irq,if want to
add x86_msi.restore_msi_irq, same hypercall need to be added accordingly.
>
>> This patch split arch_restore_msi_irqs into two functions.
>> Use arch_restore_msi_irq deal with one entry and avoid call hypercall multi
>> times in __pci_restore_msix_state.
> But irregardless of how you address the problem, this in the MSI code
> is a bit odd:
>
> 	list_for_each_entry(entry, &dev->msi_list, list) {
> 		arch_restore_msi_irqs(dev, entry->irq);
> 	}
>
> and if you collapse that and make the 'arch_restore_msi_irqs' be responsible
> for doing all the heavy lifting.. That does seem an improvement on the API
> and will make it inline with 'teardown_msi_irqs'.
>
> So from that view I think it would be nicer?
Yes, This patch just did that. There is 
teardown_msi_irqs/teardown_msi_irq pair.
But in current code, arch_restore_msi_irqs is used for one msix entry, 
this is not consistent with
its naming tradiition. So I changed default_restore_msi_irqs to deal 
with all entrys and
default_restore_msi_irq to deal with one entry for baremetal. For dom0, 
we have
PHYSDEVOP_restore_msi  to restore all msix entrys.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/