Re: [Xen-devel] [PATCH v3 5/6] 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: Thursday, September 1, 2016 4:39 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 v3 5/6] VT-d: No need to set irq affinity for posted 
> format
> IRTE
> 
> >>> On 31.08.16 at 05:56, <feng.wu@xxxxxxxxx> wrote:
> > We don't set the affinity for posted format IRTE, since the
> > destination of these interrupts is vCPU and the vCPU affinity
> > is set during vCPU scheduling.
> 
> So is this based on the assumption that after initial setup the function
> would only ever get called for affinity changes? I'm not even sure
> this is the case today, but I don't think we should build in such a
> dependency.

I don't think we have such dependency, as you mentioned below, the
purpose of this patch if to void changing the affinity related bit when
the IRTE is in posted mode.

> 
> Assuming that the main motivation is to avoid ...
> 
> > @@ -637,9 +640,12 @@ static int msi_msg_to_remap_entry(
> >      remap_rte->address_hi = 0;
> >      remap_rte->data = index - i;
> >
> > -    memcpy(iremap_entry, &new_ire, sizeof(struct iremap_entry));
> > -    iommu_flush_cache_entry(iremap_entry, sizeof(struct iremap_entry));
> > -    iommu_flush_iec_index(iommu, 0, index);
> > +    if ( !iremap_entry->remap.p || !iremap_entry->remap.im )
> > +    {
> > +        memcpy(iremap_entry, &new_ire, sizeof(struct iremap_entry));
> > +        iommu_flush_cache_entry(iremap_entry, sizeof(struct iremap_entry));
> > +        iommu_flush_iec_index(iommu, 0, index);
> > +    }
> 
> ... the actual updating here, may I suggest that you keep the
> construction of new_ire and modify the if() here to check whether
> nothing except affinity related bits changed? That would also take
> care of certain (older) compiler versions likely warning about new_ire
> potentially being used uninitialized.

Sure, maybe I can keep the construction of new_ire and only add this
if() part, since if we find the IRTE is in posted mode ('IM' is set), we don't
need to copy new_ire to iremap_entry.

Thanks,
Feng 

> 
> Jan


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