Re: [Xen-devel] [PATCH v6 11/18] vt-d: Add API to update IRTE when VT-d PI is used

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, September 04, 2015 11:11 PM
> To: Wu, Feng
> Cc: Andrew Cooper; Tian, Kevin; Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; Keir
> Fraser
> Subject: Re: [PATCH v6 11/18] vt-d: Add API to update IRTE when VT-d PI is 
> used
> 
> >>> On 25.08.15 at 03:57, <feng.wu@xxxxxxxxx> wrote:
> > This patch adds an API which is used to update the IRTE
> > for posted-interrupt when guest changes MSI/MSI-X information.
> >
> > CC: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> > CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> > CC: Keir Fraser <keir@xxxxxxx>
> > CC: Jan Beulich <jbeulich@xxxxxxxx>
> > CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> > ---
> > v6:
> > - In some error cases, the desc->lock will be unlocked twice, fix it.
> 
> Bug fixes invalidate prior acks.
> 
> > --- a/xen/drivers/passthrough/vtd/intremap.c
> > +++ b/xen/drivers/passthrough/vtd/intremap.c
> > @@ -899,3 +899,110 @@ void iommu_disable_x2apic_IR(void)
> >      for_each_drhd_unit ( drhd )
> >          disable_qinval(drhd->iommu);
> >  }
> > +
> > +static void setup_posted_irte(struct iremap_entry *new_ire,
> > +    const struct pi_desc *pi_desc, const uint8_t gvec)
> > +{
> > +    new_ire->post.urg = 0;
> > +    new_ire->post.vector = gvec;
> > +    new_ire->post.pda_l = virt_to_maddr(pi_desc) >> (32 - PDA_LOW_BIT);
> > +    new_ire->post.pda_h = virt_to_maddr(pi_desc) >> 32;
> > +
> > +    new_ire->post.res_1 = 0;
> > +    new_ire->post.res_2 = 0;
> > +    new_ire->post.res_3 = 0;
> > +    new_ire->post.res_4 = 0;
> > +    new_ire->post.res_5 = 0;
> > +
> > +    new_ire->post.im = 1;
> > +}
> 
> I think it would be better to just clear out the structure first. This
> may then also allow for no longer naming all of the bitfield res_*
> member, making it even more obvious that they're reserved. Or
> wait - looking at the use below you seem to be updating the
> descriptor. Why would you need to zero out reserved fields in
> that case?

Here we first get the IRTE from hardware, which may be in
remapped format, we still need some of the information in it
after updating it to posted format, such as, fpd, sid, etc. So
I cannot clear it here. Besides that, there are some fields which
are needed in remapped format are defined as reserved in posted
format, I need to clear the reserved field one by one if we get a
remapped format from the hardware, here I just simply clear it
for all the cases and it is not a frequent operations, I think it is
not a big deal.

Thanks,
Feng

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