Re: [Xen-devel] [PATCH v5 5/7] VT-d: No need to set irq affinity for posted format IRTE

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Wednesday, October 12, 2016 9:56 PM
> To: Wu, Feng <feng.wu@xxxxxxxxx>
> Cc: andrew.cooper3@xxxxxxxxxx; dario.faggioli@xxxxxxxxxx;
> george.dunlap@xxxxxxxxxxxxx; Tian, Kevin <kevin.tian@xxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxx
> Subject: Re: [PATCH v5 5/7] VT-d: No need to set irq affinity for posted 
> format
> IRTE
> 
> >>> On 11.10.16 at 02:57, <feng.wu@xxxxxxxxx> wrote:
> > --- a/xen/drivers/passthrough/vtd/intremap.c
> > +++ b/xen/drivers/passthrough/vtd/intremap.c
> > @@ -547,6 +547,49 @@ static int remap_entry_to_msi_msg(
> >      return 0;
> >  }
> >
> > +static bool_t pi_can_suppress_irte_update(struct iremap_entry *new,
> 
> bool (and true/false respectively) please.
> 
> And then the function name suggests that no modification gets done
> here (and hence the first parameter could be const too), yet the
> implementation does otherwise (and I don't understand why).

The idea here is that if the old IRTE is in posted format and fields like
'fpd', 'sid', 'sq', or 'svt' is going to be changed , we need to use these
new values for the new_ire, while we still need to use the old values
of other fields in IRTE, so this function returns the new irte in its first
 parameter it we cannot suppress the update. I try to do it in this
function.

> 
> > +    const struct iremap_entry *old)
> > +{
> > +    bool_t ret = 1;
> > +    u16 fpd, sid, sq, svt;
> > +
> > +    if ( !old->remap.p || !old->remap.im )
> > +        return 0;
> > +
> > +    fpd = new->post.fpd;
> > +    sid = new->post.sid;
> > +    sq = new->post.sq;
> > +    svt = new->post.svt;
> > +
> > +    *new = *old;
> > +
> > +    if ( fpd != old->post.fpd )
> > +    {
> > +        new->post.fpd = fpd;
> > +        ret = 0;
> > +    }
> > +
> > +    if ( sid != old->post.sid )
> > +    {
> > +        new->post.sid = sid;
> > +        ret = 0;
> > +    }
> > +
> > +    if ( sq != old->post.sq )
> > +    {
> > +        new->post.sq = sq;
> > +        ret = 0;
> > +    }
> > +
> > +    if ( svt != old->post.svt )
> > +    {
> > +        new->post.svt = svt;
> > +        ret = 0;
> > +    }
> 
> What's the selection of the fields based on? Namely, what about
> vector, pda_l, and pda_h?

These filed are the common field between posted format and remapped format.
'vector' field has different meaning in the two formant, pda_l and pda_h is only
for posted format. As mentioned above, the purpose of this function is to find
whether use need to update this common field in posted format, if it is, we need
to use them and reuse the old value of other fields (pda_l, pda_h, vector, 
etc.).
since we need to suppress affinity related update for posted format.

Thanks,
Feng

> 
> Jan


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