Re: [Xen-devel] [PATCH XEN v2] x86/pt: add a MSI unmask flag to XEN_DOMCTL_bind_pt_irq

[Roger Pau Monne]

On Thu, Aug 24, 2017 at 07:04:55AM -0600, Jan Beulich wrote:
> >>> On 24.08.17 at 14:19,  wrote:
> > @@ -438,6 +439,20 @@ int pt_irq_create_bind(
> >  pi_update_irte(vcpu ? >arch.hvm_vmx.pi_desc : NULL,
> > info, pirq_dpci->gmsi.gvec);
> >  
> > +if ( pt_irq_bind->u.msi.gflags & VMSI_UNMASKED )
> > +{
> > +struct irq_desc *desc = pirq_spin_lock_irq_desc(info, NULL);
> > +
> > +if ( !desc )
> > +{
> > +pt_irq_destroy_bind(d, pt_irq_bind);
> > +return -EINVAL;
> > +}
> > +
> > +guest_mask_msi_irq(desc, false);
> > +spin_unlock_irq(>lock);
> > +}
> 
> In v1 you've used spin_unlock_irqrestore() here - any reason
> you went to the less safe variant? I do understand it is correct,
> but I'd still prefer spin_{,un}lock_irq() to be avoided as much as
> possible.

OK, I saw it was not really needed and switched to the less safe one.
Will switch back, I've also need to resend the QEMU patch to fix a
coding style mistake.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH XEN v2] x86/pt: add a MSI unmask flag to XEN_DOMCTL_bind_pt_irq

[Jan Beulich]

>>> On 24.08.17 at 14:19,  wrote:
> @@ -438,6 +439,20 @@ int pt_irq_create_bind(
>  pi_update_irte(vcpu ? >arch.hvm_vmx.pi_desc : NULL,
> info, pirq_dpci->gmsi.gvec);
>  
> +if ( pt_irq_bind->u.msi.gflags & VMSI_UNMASKED )
> +{
> +struct irq_desc *desc = pirq_spin_lock_irq_desc(info, NULL);
> +
> +if ( !desc )
> +{
> +pt_irq_destroy_bind(d, pt_irq_bind);
> +return -EINVAL;
> +}
> +
> +guest_mask_msi_irq(desc, false);
> +spin_unlock_irq(>lock);
> +}

In v1 you've used spin_unlock_irqrestore() here - any reason
you went to the less safe variant? I do understand it is correct,
but I'd still prefer spin_{,un}lock_irq() to be avoided as much as
possible.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH XEN v2] x86/pt: add a MSI unmask flag to XEN_DOMCTL_bind_pt_irq

[Roger Pau Monne]

The flag is part of the gflags, and should be used to request the
unmask of a MSI interrupt once it's bound.

This is required for the device model in order to be capable of
binding MSIX interrupts that have the entry mask bit already unset at
bind time. Without this fix the interrupts would be left masked.

Note that this commit introduces a change to the domctl, which
requires a bump of the interface version. This is done done here
because the interface version has already been bumped in this release
cycle.

Signed-off-by: Roger Pau Monné 
Reported by: Andreas Kinzler 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
Changes since v1:
 - Use pirq_spin_lock_irq_desc.
---
 xen/drivers/passthrough/io.c  | 21 ++---
 xen/include/asm-x86/hvm/irq.h |  1 +
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index 19a21bf85a..e204b044d0 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -342,13 +342,14 @@ int pt_irq_create_bind(
 uint8_t dest, dest_mode, delivery_mode;
 int dest_vcpu_id;
 const struct vcpu *vcpu;
+uint32_t gflags = pt_irq_bind->u.msi.gflags & ~VMSI_UNMASKED;
 
 if ( !(pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) )
 {
 pirq_dpci->flags = HVM_IRQ_DPCI_MAPPED | HVM_IRQ_DPCI_MACH_MSI |
HVM_IRQ_DPCI_GUEST_MSI;
 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
-pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags;
+pirq_dpci->gmsi.gflags = gflags;
 /*
  * 'pt_irq_create_bind' can be called after 'pt_irq_destroy_bind'.
  * The 'pirq_cleanup_check' which would free the structure is only
@@ -401,13 +402,13 @@ int pt_irq_create_bind(
 
 /* If pirq is already mapped as vmsi, update guest data/addr. */
 if ( pirq_dpci->gmsi.gvec != pt_irq_bind->u.msi.gvec ||
- pirq_dpci->gmsi.gflags != pt_irq_bind->u.msi.gflags )
+ pirq_dpci->gmsi.gflags != gflags )
 {
 /* Directly clear pending EOIs before enabling new MSI info. */
 pirq_guest_eoi(info);
 
 pirq_dpci->gmsi.gvec = pt_irq_bind->u.msi.gvec;
-pirq_dpci->gmsi.gflags = pt_irq_bind->u.msi.gflags;
+pirq_dpci->gmsi.gflags = gflags;
 }
 }
 /* Calculate dest_vcpu_id for MSI-type pirq migration. */
@@ -438,6 +439,20 @@ int pt_irq_create_bind(
 pi_update_irte(vcpu ? >arch.hvm_vmx.pi_desc : NULL,
info, pirq_dpci->gmsi.gvec);
 
+if ( pt_irq_bind->u.msi.gflags & VMSI_UNMASKED )
+{
+struct irq_desc *desc = pirq_spin_lock_irq_desc(info, NULL);
+
+if ( !desc )
+{
+pt_irq_destroy_bind(d, pt_irq_bind);
+return -EINVAL;
+}
+
+guest_mask_msi_irq(desc, false);
+spin_unlock_irq(>lock);
+}
+
 break;
 }
 
diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
index 106dc19613..9546c24879 100644
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -136,6 +136,7 @@ struct dev_intx_gsi_link {
 #define VMSI_DM_MASK  0x200
 #define VMSI_DELIV_MASK   0x7000
 #define VMSI_TRIG_MODE0x8000
+#define VMSI_UNMASKED 0x1
 
 #define GFLAGS_SHIFT_RH 8
 #define GFLAGS_SHIFT_DELIV_MODE 12
-- 
2.11.0 (Apple Git-81)


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel