Re: [Xen-devel] [PATCH] pci: clear host_maskall field on assign

From: Chao Gao <chao.gao@intel.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: xen-devel@lists.xenproject.org,
	Stanislav Spassov <stanspas@amazon.de>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH] pci: clear host_maskall field on assign
Date: Tue, 8 Oct 2019 14:18:19 +0800	[thread overview]
Message-ID: <20191008061817.GA25749@gao-cwp> (raw)
In-Reply-To: <e31ebe3d-3aa8-dafd-1104-54fcaf1a4896@suse.com>

On Mon, Oct 07, 2019 at 09:38:48AM +0200, Jan Beulich wrote:
>On 05.10.2019 01:58, Chao Gao wrote:
>> On Wed, Oct 02, 2019 at 12:49:35PM +0200, Roger Pau Monne wrote:
>>> The current implementation of host_maskall makes it sticky across
>>> assign and deassign calls, which means that once a guest forces Xen to
>>> set host_maskall the maskall bit is not going to be cleared until a
>>> call to PHYSDEVOP_prepare_msix is performed. Such call however
>>> shouldn't be part of the normal flow when doing PCI passthrough, and
>>> hence the flag needs to be cleared when assigning in order to prevent
>>> host_maskall being carried over from previous assignations.
>>>
>>> Note that other mask fields, like guest_masked or the entry maskbit
>>> are already reset when the msix capability is initialized. Also note
>>> that doing the reset of host_maskall there would allow the guest to
>>> reset such field by enabling and disabling MSIX, which is not
>>> intended.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>> Cc: Chao Gao <chao.gao@intel.com>
>>> Cc: "Spassov, Stanislav" <stanspas@amazon.de>
>>> Cc: Pasi Kärkkäinen <pasik@iki.fi>
>>> ---
>>> Chao, Stanislav, can you please check if this patch fixes your
>>> issues?
>> 
>> I am glad to. For your testing, you can just kill qemu and destroy the
>> guest. Then maskall bit of a pass-thru device will be set. And in this
>> case, try to recreate the guest and check whether the maskall bit is
>> cleared in guest.
>> 
>> The solution is similar to my v1 [1]. One question IMO (IIRC, it is why
>> I changed to another approach) is: why not do such reset at deivce
>> deassignment such that dom0 can use a clean device. Otherwise, the
>> device won't work after being unbound from pciback. But I am not so
>> sure, I can check it next Tuesday.
>
>I too did think about this, but aiui pciback needs to issue
>PHYSDEVOP_release_msix anyway, and Dom0 would then re-setup MSI-X
>"from scratch", i.e. we'd clear the flag anyway in
>msix_capability_init() due to msix->used_entries being zero at
>the first (of possibly several) invocation(s).

Yes. I just checked it on my machine and found you are right.

Thanks
Chao

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel