linux-kernel - Re: [PATCH] xen: reuse the same pirq allocated when driver load first time

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]

Message-ID: <20130510185339.GD2562@phenom.dumpdata.com>
Date:	Fri, 10 May 2013 14:53:39 -0400
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	Zhenzhong Duan <zhenzhong.duan@...cle.com>,
	Stefano Stabellini <stefano.stabellini@...citrix.com>
Cc:	xen-devel@...ts.xensource.com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Yuval Shaia <yuval.shaia@...cle.com>,
	Feng Jin <joe.jin@...cle.com>, Chien Yen <chien.yen@...cle.com>
Subject: Re: [PATCH] xen: reuse the same pirq allocated when driver load
 first time

On Wed, May 08, 2013 at 04:18:24PM +0800, Zhenzhong Duan wrote:
> When driver load and unload in a loop, pirq will exhaust finally.
> Try to use the same pirq which was already mapped and binded at first time

So what happens if I unload and reload two drivers in random order?

> when driver loaded.
> 
> Read pirq from msix entry and test if data is XEN_PIRQ_MSI_DATA
> xen_irq_from_pirq(pirq) < 0 checking is wrong as irq will be freed
> when driver unload, it's always true in second load.

If my understanding is right the issue at hand is that the caching
information about the pirq disappears once the driver has been
unloaded b/c the event's irq-info is removed (as the driver is
unloaded and free_irq is called).

Stefano,
Is there a specific write to the MSI structure that would cause the
hypervisor to drop the PIRQ? Or a nice hypercall to "free" an
PIRQ in usage?

I presume that on the first load the msg.data value is
ALWAYS != XEN_PIRQ_MSI_DATA right? Is that somethign we can ALWAYS
depend? Or is there a possibility that it could be that when the
PCI device is unplugged from one guest and plugged in another?

Could we use PHYSDEVOP_irq_status_query to figure out if the PIRQ
has been already allocated? Ah, does not look like it.

I am a bit worried about depending on just msg.data to tells us
whether the device had been initialized before.

Perhaps a different solution could be to utilize the
'xen_device_domain_owner' code. See attached patch (not
compile tested) to retain whether we had initialized this
device in the past and we can skip the hypercall.

On a second thought it could also use some extra checks
to make sure that the pirq that would be used on the second
run is the same as on the first one.

> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@...cle.com>
> Tested-by: Yuval Shaia <yuval.shaia@...cle.com>
> ---
>  arch/x86/pci/xen.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 94e7662..4289eef 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -230,8 +230,7 @@ static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>  		__read_msi_msg(msidesc, &msg);
>  		pirq = MSI_ADDR_EXT_DEST_ID(msg.address_hi) |
>  			((msg.address_lo >> MSI_ADDR_DEST_ID_SHIFT) & 0xff);
> -		if (msg.data != XEN_PIRQ_MSI_DATA ||
> -		    xen_irq_from_pirq(pirq) < 0) {
> +		if (msg.data != XEN_PIRQ_MSI_DATA) {

>  			if (pirq < 0) {
>  				irq = -ENODEV;
> -- 
> 1.7.3
> 

View attachment "use-xen-device-domain-owner.patch" of type "text/plain" (1858 bytes)