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

[Chao Gao <chao.gao@xxxxxxxxx>]

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.

[1]: https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg00863.html

Thanks
Chao

>---
> xen/drivers/passthrough/pci.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
>index 7deef2f12b..b4f1ac2dd9 100644
>--- a/xen/drivers/passthrough/pci.c
>+++ b/xen/drivers/passthrough/pci.c
>@@ -1504,7 +1504,10 @@ static int assign_device(struct domain *d, u16 seg, u8 
>bus, u8 devfn, u32 flag)
>     }
> 
>     if ( pdev->msix )
>+    {
>         msixtbl_init(d);
>+        pdev->msix->host_maskall = false;
>+    }

It is similar to my v1 patch here.
[1]: https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg00863.html

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