[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH QEMU v4] xen/pt: allow QEMU to request MSI unmas
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH QEMU v4] xen/pt: allow QEMU to request MSI unmasking at bind time |
Date: |
Thu, 24 Aug 2017 18:55:01 -0700 (PDT) |
User-agent: |
Alpine 2.10 (DEB 1266 2009-07-14) |
On Thu, 24 Aug 2017, Roger Pau Monne wrote:
> 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é <address@hidden>
> Reviewed-by: Jan Beulich <address@hidden>
> Reported-by: Andreas Kinzler <address@hidden>
Acked-by: Stefano Stabellini <address@hidden>
I'll queue it up.
> ---
> Cc: Stefano Stabellini <address@hidden>
> Cc: Anthony Perard <address@hidden>
> Cc: Jan Beulich <address@hidden>
> Cc: address@hidden
> ---
> Changes since v2:
> - Fix coding style.
>
> Changes since v1:
> - Track MSI mask bits.
> - Notify Xen of whether MSI interrupts should be unmasked after
> binding, instead of hardcoding it.
> ---
> hw/xen/xen_pt.h | 1 +
> hw/xen/xen_pt_config_init.c | 20 ++++++++++++++++++--
> hw/xen/xen_pt_msi.c | 13 ++++++++++---
> 3 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 191d9caea1..aa39a9aa5f 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -180,6 +180,7 @@ typedef struct XenPTMSI {
> uint32_t addr_hi; /* guest message upper address */
> uint16_t data; /* guest message data */
> uint32_t ctrl_offset; /* saved control offset */
> + uint32_t mask; /* guest mask bits */
> int pirq; /* guest pirq corresponding */
> bool initialized; /* when guest MSI is initialized */
> bool mapped; /* when pirq is mapped */
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index 1f04ec5eec..a3ce33e78b 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -1315,6 +1315,22 @@ static int
> xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
> return 0;
> }
>
> +static int xen_pt_mask_reg_write(XenPCIPassthroughState *s, XenPTReg
> *cfg_entry,
> + uint32_t *val, uint32_t dev_value,
> + uint32_t valid_mask)
> +{
> + int rc;
> +
> + rc = xen_pt_long_reg_write(s, cfg_entry, val, dev_value, valid_mask);
> + if (rc) {
> + return rc;
> + }
> +
> + s->msi->mask = *val;
> +
> + return 0;
> +}
> +
> /* MSI Capability Structure reg static information table */
> static XenPTRegInfo xen_pt_emu_reg_msi[] = {
> /* Next Pointer reg */
> @@ -1393,7 +1409,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
> .emu_mask = 0xFFFFFFFF,
> .init = xen_pt_mask_reg_init,
> .u.dw.read = xen_pt_long_reg_read,
> - .u.dw.write = xen_pt_long_reg_write,
> + .u.dw.write = xen_pt_mask_reg_write,
> },
> /* Mask reg (if PCI_MSI_FLAGS_MASKBIT set, for 64-bit devices) */
> {
> @@ -1404,7 +1420,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
> .emu_mask = 0xFFFFFFFF,
> .init = xen_pt_mask_reg_init,
> .u.dw.read = xen_pt_long_reg_read,
> - .u.dw.write = xen_pt_long_reg_write,
> + .u.dw.write = xen_pt_mask_reg_write,
> },
> /* Pending reg (if PCI_MSI_FLAGS_MASKBIT set, for 32-bit devices) */
> {
> diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
> index ff9a79f5d2..6d1e3bdeb4 100644
> --- a/hw/xen/xen_pt_msi.c
> +++ b/hw/xen/xen_pt_msi.c
> @@ -24,6 +24,7 @@
> #define XEN_PT_GFLAGS_SHIFT_DM 9
> #define XEN_PT_GFLAGSSHIFT_DELIV_MODE 12
> #define XEN_PT_GFLAGSSHIFT_TRG_MODE 15
> +#define XEN_PT_GFLAGSSHIFT_UNMASKED 16
>
> #define latch(fld) latch[PCI_MSIX_ENTRY_##fld / sizeof(uint32_t)]
>
> @@ -155,7 +156,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
> int pirq,
> bool is_msix,
> int msix_entry,
> - int *old_pirq)
> + int *old_pirq,
> + bool masked)
> {
> PCIDevice *d = &s->dev;
> uint8_t gvec = msi_vector(data);
> @@ -171,6 +173,8 @@ static int msi_msix_update(XenPCIPassthroughState *s,
> table_addr = s->msix->mmio_base_addr;
> }
>
> + gflags |= masked ? 0 : (1u << XEN_PT_GFLAGSSHIFT_UNMASKED);
> +
> rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
> pirq, gflags, table_addr);
>
> @@ -273,8 +277,10 @@ int xen_pt_msi_setup(XenPCIPassthroughState *s)
> int xen_pt_msi_update(XenPCIPassthroughState *s)
> {
> XenPTMSI *msi = s->msi;
> +
> + /* Current MSI emulation in QEMU only supports 1 vector */
> return msi_msix_update(s, msi_addr64(msi), msi->data, msi->pirq,
> - false, 0, &msi->pirq);
> + false, 0, &msi->pirq, msi->mask & 1);
> }
>
> void xen_pt_msi_disable(XenPCIPassthroughState *s)
> @@ -355,7 +361,8 @@ static int xen_pt_msix_update_one(XenPCIPassthroughState
> *s, int entry_nr,
> }
>
> rc = msi_msix_update(s, entry->addr, entry->data, pirq, true,
> - entry_nr, &entry->pirq);
> + entry_nr, &entry->pirq,
> + vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT);
>
> if (!rc) {
> entry->updated = false;
> --
> 2.11.0 (Apple Git-81)
>