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

[Chao Gao <chao.gao@xxxxxxxxx>]

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@xxxxxxxxxx>
>>> ---
>>> Cc: Chao Gao <chao.gao@xxxxxxxxx>
>>> Cc: "Spassov, Stanislav" <stanspas@xxxxxxxxx>
>>> Cc: Pasi Kärkkäinen <pasik@xxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel