xen/pt: allow QEMU to request MSI unmasking at bind time

PATCH RESEND Applied Andra Paraschiv 3 participants 7 messages Oct 10, 2018 – Nov 5, 2018

Share & tools

Andra Paraschiv

author

2018-10-10 18:53 UTC #0 lore raw

PATCH RESEND 1/1 xen/pt: allow QEMU to request MSI unmasking at bind time

When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.

This causes problems with guests that first configure interrupts and clean the per-entry MSIX table mask bit and afterwards enable MSIX globally. In such scenario the Xen internal msixtbl handlers would not detect the unmasking of MSIX entries because vectors are not yet registered since MSIX is not enabled, and vectors would be left masked.

Introduce a new flag in the gflags field to signal Xen whether a MSI interrupt should be unmasked after being bound.

This also requires to track the mask register for MSI interrupts, so QEMU can also notify to Xen whether the MSI interrupt should be bound masked or unmasked

Signed-off-by Roger Pau Monné <[email protected]>

Reviewed-by Jan Beulich <[email protected]>

Reported-by Andreas Kinzler <[email protected]>

Reviewed-by Stefano Stabellini <[email protected]>

Signed-off-by Stefano Stabellini <[email protected]>

[Backported from commit a8036336609d2e184fc3543a4c439c0ba7d7f3a2
https://git.qemu.org/?p=qemu.git;a=commit;h=a8036336609d2e184fc3543a4c439c0ba7d7f3a2
https://xenbits.xen.org/gitweb/?p=qemu-xen.git;a=commit;h=a8036336609d2e184fc3543a4c439c0ba7d7f3a2]

Signed-off-by Andra Paraschiv <[email protected]>

---
 hw/pass-through.c | 23 +++++++++++++++++++++--
 hw/pass-through.h |  1 +
 hw/pt-msi.c       | 25 +++++++++++++++++++++----
 hw/pt-msi.h       |  9 +++++++++
 4 files changed, 52 insertions(+), 6 deletions(-)

hw/pass-through.c +21 -2

index 0b765858..c6cb60e2 100644
@@ -188,6 +188,9 @@ static int pt_word_reg_write(struct pt_dev *ptdev,
 static int pt_long_reg_write(struct pt_dev *ptdev,
     struct pt_reg_tbl *cfg_entry,
     uint32_t *value, uint32_t dev_value, uint32_t valid_mask);
+static int pt_mask_reg_write(struct pt_dev *ptdev,
+    struct pt_reg_tbl *cfg_entry,
+    uint32_t *value, uint32_t dev_value, uint32_t valid_mask);
 static int pt_cmd_reg_write(struct pt_dev *ptdev,
     struct pt_reg_tbl *cfg_entry,
     uint16_t *value, uint16_t dev_value, uint16_t valid_mask);

@@ -755,7 +758,7 @@ static struct pt_reg_info_tbl pt_emu_reg_msi_tbl[] = {
         .emu_mask   = 0xFFFFFFFF,
         .init       = pt_mask_reg_init,
         .u.dw.read  = pt_long_reg_read,
-        .u.dw.write = pt_long_reg_write,
+        .u.dw.write = pt_mask_reg_write,
     },
     /* Mask reg (if PCI_MSI_FLAGS_MASK_BIT set, for 64-bit devices) */
     {

@@ -766,7 +769,7 @@ static struct pt_reg_info_tbl pt_emu_reg_msi_tbl[] = {
         .emu_mask   = 0xFFFFFFFF,
         .init       = pt_mask_reg_init,
         .u.dw.read  = pt_long_reg_read,
-        .u.dw.write = pt_long_reg_write,
+        .u.dw.write = pt_mask_reg_write,
     },
     /* Pending reg (if PCI_MSI_FLAGS_MASK_BIT set, for 32-bit devices) */
     {

@@ -3572,6 +3575,22 @@ static int pt_long_reg_write(struct pt_dev *ptdev,
     return 0;
 }
 
+/* write guest mask bits */
+static int pt_mask_reg_write(struct pt_dev *ptdev,
+        struct pt_reg_tbl *cfg_entry,
+        uint32_t *value, uint32_t dev_value, uint32_t valid_mask)
+{
+    int rc;
+
+    rc = pt_long_reg_write(ptdev, cfg_entry, value, dev_value, valid_mask);
+    if (rc)
+        return rc;
+
+    ptdev->msi->mask = *value;
+
+    return 0;
+}
+
 /* write Command register */
 static int pt_cmd_reg_write(struct pt_dev *ptdev,
         struct pt_reg_tbl *cfg_entry,

hw/pass-through.h +1 -0

index bb6ddce0..0b582241 100644
@@ -191,6 +191,7 @@ struct pt_region {
 struct pt_msi_info {
     uint32_t flags;
     uint32_t ctrl_offset; /* saved control offset */
+    uint32_t mask;     /* guest mask bits */
     int pirq;          /* guest pirq corresponding */
     uint32_t addr_lo;  /* guest message address */
     uint32_t addr_hi;  /* guest message upper address */

hw/pt-msi.c +21 -4

index c6baea94..8b06450f 100644
@@ -138,6 +138,9 @@ int pt_msi_update(struct pt_dev *d)
     addr = (uint64_t)d->msi->addr_hi << 32 | d->msi->addr_lo;
     gflags = __get_msi_gflags(d->msi->data, addr);
 
+    /* Current MSI emulation in QEMU only supports 1 vector */
+    gflags |= (d->msi->mask & 1) ? 0 : (1u << GLFAGS_SHIFT_UNMASKED);
+
     PT_LOG("Update msi with pirq %x gvec %x gflags %x\n",
            d->msi->pirq, gvec, gflags);

@@ -275,7 +278,8 @@ void pt_disable_msi_translate(struct pt_dev *dev)
     }
 }
 
-static int pt_msix_update_one(struct pt_dev *dev, int entry_nr)
+static int pt_msix_update_one(struct pt_dev *dev, int entry_nr,
+                              uint32_t vec_ctrl)
 {
     struct msix_entry_info *entry = &dev->msix->msix_entry[entry_nr];
     int pirq = entry->pirq;

@@ -316,6 +320,9 @@ static int pt_msix_update_one(struct pt_dev *dev, int entry_nr)
         entry->pirq = pirq;
     }
 
+    gflags |= (vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT) ? 0 :
+        (1u << GLFAGS_SHIFT_UNMASKED);
+
     PT_LOG("Update msix entry %x with pirq %x gvec %x\n",
             entry_nr, pirq, gvec);

@@ -343,7 +350,7 @@ int pt_msix_update(struct pt_dev *dev)
 
     for ( i = 0; i < msix->total_entries; i++ )
     {
-        pt_msix_update_one(dev, i);
+        pt_msix_update_one(dev, i, msix->msix_entry[i].io_mem[3]);
     }
 
     return 0;

@@ -479,8 +486,18 @@ static void pci_msix_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
 
     if ( offset == 3 )
     {
-        if ( msix->enabled && !(val & 0x1) )
-            pt_msix_update_one(dev, entry_nr);
+        if ( msix->enabled && !(val & 0x1) ) {
+            const volatile uint32_t *vec_ctrl;
+
+            /*
+             * If Xen intercepts the mask bit access, io_mem[3] may not be
+             * up-to-date. Read from hardware directly.
+             */
+            vec_ctrl = dev->msix->phys_iomem_base +
+                PCI_MSIX_ENTRY_SIZE * entry_nr + PCI_MSIX_ENTRY_VECTOR_CTRL;
+
+            pt_msix_update_one(dev, entry_nr, *vec_ctrl);
+        }
     }
 }

hw/pt-msi.h +9 -0

index 94e0d35f..a80d76e8 100644
@@ -33,6 +33,14 @@
 #define PCI_MSIX_PBA        8
 #define  PCI_MSIX_BIR       0x7
 
+/* MSI-X Table entry format */
+#define PCI_MSIX_ENTRY_SIZE             16
+#define  PCI_MSIX_ENTRY_LOWER_ADDR      0
+#define  PCI_MSIX_ENTRY_UPPER_ADDR      4
+#define  PCI_MSIX_ENTRY_DATA            8
+#define  PCI_MSIX_ENTRY_VECTOR_CTRL     12
+#define   PCI_MSIX_ENTRY_CTRL_MASKBIT   1
+
 #define MSI_FLAG_UNINIT 0x1000
 #define PT_MSI_MAPPED   0x2000

@@ -77,6 +85,7 @@
 #define GFLAGS_SHIFT_DM             9
 #define GLFAGS_SHIFT_DELIV_MODE     12
 #define GLFAGS_SHIFT_TRG_MODE       15
+#define GLFAGS_SHIFT_UNMASKED       16
 
 void
 msi_set_enable(struct pt_dev *dev, int en);

Signature · 10 lines

2.15.2.AMZN




Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Ian Jackson

Applied

2018-10-24 15:20 UTC #1 lore raw

Andra Paraschiv writes ("[PATCH RESEND qemu-xen-traditional] xen/pt: allow QEMU to request MSI unmasking at bind time"):

Quoted 3 lines

When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.

Applied, and pushed the corresponding update to QEMU_TAG in qemu-xen-unstable.

Does this patch need to be backported to earlier Xen releases ? It wasn't clear to me whether there is new Xen code in this area without which the problem (which the patch solves) does not arise; or whether, conversely, simply running a new guest is sufficient.

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Pasi Kärkkäinen

2018-10-29 14:58 UTC #2 lore raw

Hi,

Quoted 5 lines · Ian Jackson

Andra Paraschiv writes ("[PATCH RESEND qemu-xen-traditional] xen/pt: allow QEMU to request MSI unmasking at bind time"):
Quoted 3 lines
When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.

Quoted 8 lines

Applied, and pushed the corresponding update to QEMU_TAG in qemu-xen-unstable.
Does this patch need to be backported to earlier Xen releases ? It wasn't clear to me whether there is new Xen code in this area without which the problem (which the patch solves) does not arise; or whether, conversely, simply running a new guest is sufficient.

Good question. In the earlier thread about this patch it was mentioned the bug happens with latest stable version of Xen, which sounds like this patch should be backported to stable Xen versions. Is that correct?

Thanks,

-- Pasi

Quoted 3 lines

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Paraschiv, Andra-Irina

author

2018-10-29 20:55 UTC #3 lore raw

Quoted 17 lines · Pasi Kärkkäinen

Hi,
Quoted 5 lines · Ian Jackson
Andra Paraschiv writes ("[PATCH RESEND qemu-xen-traditional] xen/pt: allow QEMU to request MSI unmasking at bind time"):
Quoted 3 lines
When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.
Quoted 8 lines
Applied, and pushed the corresponding update to QEMU_TAG in qemu-xen-unstable.
Does this patch need to be backported to earlier Xen releases ? It wasn't clear to me whether there is new Xen code in this area without which the problem (which the patch solves) does not arise; or whether, conversely, simply running a new guest is sufficient.

Quoted 3 lines

Good question. In the earlier thread about this patch it was mentioned the bug happens with latest stable version of Xen, which sounds like this patch should be backported to stable Xen versions. Is that correct?

Replied later than expected, but I'm back now.

Ian, Pasi,

Right. We have this mail thread where the issue was originally reported:
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00915.html

And the follow-up with the patches for Xen and QEMU (not traditional):
https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg01238.html

"Hm, I think I might have fixed this issue, see:

https://git.qemu.org/?p=qemu.git;a=commit;h=a8036336609d2e184fc3543a4c439c0ba7d7f3a2

And the Xen side:

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=69d99d1b223fc5082400374698ddd7486e5ea953"

The Xen patch above is present in the tree from 4.10.0-rc1 tag forward.

Thanks for feedback. Anything I can help further, we can discuss about it.

Andra

Quoted 7 lines

Thanks,
-- Pasi
Quoted 3 lines
Regards,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Pasi Kärkkäinen

2018-10-30 17:33 UTC #4 lore raw

Quoted 36 lines · Andra-Irina

Quoted 17 lines · Pasi Kärkkäinen
Hi,
Quoted 5 lines · Ian Jackson
Andra Paraschiv writes ("[PATCH RESEND qemu-xen-traditional] xen/pt: allow QEMU to request MSI unmasking at bind time"):
Quoted 3 lines
When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.
Quoted 8 lines
Applied, and pushed the corresponding update to QEMU_TAG in qemu-xen-unstable.
Does this patch need to be backported to earlier Xen releases ? It wasn't clear to me whether there is new Xen code in this area without which the problem (which the patch solves) does not arise; or whether, conversely, simply running a new guest is sufficient.
Quoted 3 lines
Good question. In the earlier thread about this patch it was mentioned the bug happens with latest stable version of Xen, which sounds like this patch should be backported to stable Xen versions. Is that correct?
Replied later than expected, but I'm back now.
Ian, Pasi,
Right. We have this mail thread where the issue was originally reported:
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00915.html
And the follow-up with the patches for Xen and QEMU (not traditional):
https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg01238.html
"Hm, I think I might have fixed this issue, see:

Quoted 8 lines

https://git.qemu.org/?p=qemu.git;a=commit;h=a8036336609d2e184fc3543a4c439c0ba7d7f3a2
And the Xen side:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=69d99d1b223fc5082400374698ddd7486e5ea953"
The Xen patch above is present in the tree from 4.10.0-rc1 tag forward.

Thanks! So it looks like the patch should be applied/backported to qemu-traditional trees for Xen 4.10 and 4.11.

-- Pasi

Quoted 11 lines

Thanks for feedback. Anything I can help further, we can discuss about it.
Andra
Quoted 7 lines
Thanks,
-- Pasi
Quoted 3 lines
Regards,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Pasi Kärkkäinen

2018-11-04 20:49 UTC #5 lore raw

In reply to Paraschiv, Andra-Irina

Hello Ian,

Quoted 36 lines · Andra-Irina

Quoted 17 lines · Pasi Kärkkäinen
Hi,
Quoted 5 lines · Ian Jackson
Andra Paraschiv writes ("[PATCH RESEND qemu-xen-traditional] xen/pt: allow QEMU to request MSI unmasking at bind time"):
Quoted 3 lines
When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.
Quoted 8 lines
Applied, and pushed the corresponding update to QEMU_TAG in qemu-xen-unstable.
Does this patch need to be backported to earlier Xen releases ? It wasn't clear to me whether there is new Xen code in this area without which the problem (which the patch solves) does not arise; or whether, conversely, simply running a new guest is sufficient.
Quoted 3 lines
Good question. In the earlier thread about this patch it was mentioned the bug happens with latest stable version of Xen, which sounds like this patch should be backported to stable Xen versions. Is that correct?
Replied later than expected, but I'm back now.
Ian, Pasi,
Right. We have this mail thread where the issue was originally reported:
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00915.html
And the follow-up with the patches for Xen and QEMU (not traditional):
https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg01238.html
"Hm, I think I might have fixed this issue, see:

Quoted 8 lines

https://git.qemu.org/?p=qemu.git;a=commit;h=a8036336609d2e184fc3543a4c439c0ba7d7f3a2
And the Xen side:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=69d99d1b223fc5082400374698ddd7486e5ea953"
The Xen patch above is present in the tree from 4.10.0-rc1 tag forward.

This sounds like an important fix to have for PCI passthrough users, and it seems the backport of this patch needs to go to Xen qemu-traditional to both 4.10 and 4.11 branches, because the Xen hypervisor side patch is already in those branches.

Thanks,

-- Pasi

Quoted 12 lines

Thanks for feedback. Anything I can help further, we can discuss about it.
Andra
Quoted 7 lines
Thanks,
-- Pasi
Quoted 3 lines
Regards,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Paraschiv, Andra-Irina

author

2018-11-05 09:58 UTC #6 lore raw

Quoted 44 lines · Pasi Kärkkäinen

Hello Ian,
Quoted 40 lines · Andra-Irina
Quoted 17 lines · Pasi Kärkkäinen
Hi,
Quoted 5 lines · Ian Jackson
Andra Paraschiv writes ("[PATCH RESEND qemu-xen-traditional] xen/pt: allow QEMU to request MSI unmasking at bind time"):
Quoted 3 lines
When a MSI interrupt is bound to a guest using xc_domain_update_msi_irq (XEN_DOMCTL_bind_pt_irq) the interrupt is left masked by default.
Quoted 8 lines
Applied, and pushed the corresponding update to QEMU_TAG in qemu-xen-unstable.
Does this patch need to be backported to earlier Xen releases ? It wasn't clear to me whether there is new Xen code in this area without which the problem (which the patch solves) does not arise; or whether, conversely, simply running a new guest is sufficient.
Quoted 3 lines
Good question. In the earlier thread about this patch it was mentioned the bug happens with latest stable version of Xen, which sounds like this patch should be backported to stable Xen versions. Is that correct?
Replied later than expected, but I'm back now.
Ian, Pasi,
Right. We have this mail thread where the issue was originally reported:
https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00915.html
And the follow-up with the patches for Xen and QEMU (not traditional):
https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg01238.html
"Hm, I think I might have fixed this issue, see:
https://git.qemu.org/?p=qemu.git;a=commit;h=a8036336609d2e184fc3543a4c439c0ba7d7f3a2
And the Xen side:

Quoted 7 lines

Quoted 4 lines
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=69d99d1b223fc5082400374698ddd7486e5ea953"
The Xen patch above is present in the tree from 4.10.0-rc1 tag forward.
This sounds like an important fix to have for PCI passthrough users, and it seems the back port of this patch needs to go to Xen qemu-traditional to both 4.10 and 4.11 branches,

because the Xen hypervisor side patch is already in those branches.

As a follow-up, tested and the fix applied cleanly over the patch [0] that corresponds to xen-4.10.0 and xen-4.11.0 tags.

Thanks, Ian, Pasi.

Andra

[0] https://xenbits.xen.org/gitweb/?p=qemu-xen-traditional.git;a=commit;h=c8ea0457495342c417c3dc033bba25148b279f60

Quoted 17 lines

Thanks,
-- Pasi
Quoted 11 lines
Thanks for feedback. Anything I can help further, we can discuss about it.
Andra
Quoted 7 lines
Thanks,
-- Pasi
Quoted 3 lines
Regards,
Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel