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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Thu, Aug 24, 2017 at 04:17:32AM -0600, Jan Beulich wrote:
>>>> On 24.08.17 at 12:12, <roger.pau@xxxxxxxxxx> wrote:
>> On Thu, Aug 24, 2017 at 04:07:40AM -0600, Jan Beulich wrote:
>>> >>> On 24.08.17 at 11:47, <roger.pau@xxxxxxxxxx> wrote:
>>> > @@ -438,6 +439,22 @@ int pt_irq_create_bind(
>>> >              pi_update_irte(vcpu ? &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 = irq_to_desc(info->arch.irq);
>>> > +            unsigned long flags;
>>> > +
>>> > +            if ( !desc )
>>> > +            {
>>> > +                pt_irq_destroy_bind(d, pt_irq_bind);
>>> > +                return -EINVAL;
>>> > +            }
>>> > +
>>> > +            spin_lock_irqsave(&desc->lock, flags);
>>> > +            guest_mask_msi_irq(desc, false);
>>> > +            spin_unlock_irqrestore(&desc->lock, flags);
>>> > +        }
>>> > +
>>> >          break;
>>> >      }
>>> 
>>> I think you would better use pirq_spin_lock_irq_desc() here. And
>>> wouldn't the addition better be moved up a little (perhaps right
>>> after the dropping of the domain's event lock)?
>> 
>> Shouldn't the unmask happen after the posted interrupt is setup? Or it
>> doesn't really matter?
>> 
>> I though it was safer to unmask once the bind process was finished.
>
>Yeah, I'm not entirely certain either, hence I've put it as a question.
>Kevin, Chao?
>

Hi, Jan and Roger.

pi_update_irte() right above the piece of code is to set IRTE properly
according to the request. Unmasking the msi without updating IRTE, I
think may leads to inject an interrupt whose vector or destination is
out of date.

Thanks
Chao

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