Re: [Xen-devel] [Patch RFC 03/13] vt-d: Track the Device-TLB invalidation status in an invalidation table.

["Xu, Quan" <quan.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@xxxxxxxxxx]
> Sent: Wednesday, September 16, 2015 5:34 PM
> To: Xu, Quan; andrew.cooper3@xxxxxxxxxx; Dong, Eddie; ian.campbell@xxxxxxxxxx;
> ian.jackson@xxxxxxxxxxxxx; jbeulich@xxxxxxxx; Nakajima, Jun; keir@xxxxxxx;
> Tian, Kevin; tim@xxxxxxx; Zhang, Yang Z; george.dunlap@xxxxxxxxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [Patch RFC 03/13] vt-d: Track the Device-TLB 
> invalidation
> status in an invalidation table.
> 
> Hi Quan,
> 
> The time of the mail is in a future. Can you configure your mail to report the
> correct time?


Yes, I should set the time. Thanks for your quick response.

> 
> On 16/09/2015 14:23, Quan Xu wrote:
> > diff --git a/xen/include/xen/hvm/iommu.h b/xen/include/xen/hvm/iommu.h
> > index 106e08f..28e7fc3 100644
> > --- a/xen/include/xen/hvm/iommu.h
> > +++ b/xen/include/xen/hvm/iommu.h
> > @@ -23,6 +23,21 @@
> >   #include <xen/list.h>
> >   #include <asm/hvm/iommu.h>
> >
> > +/*
> > + * Status Address and Data: Status address and data is used by
> > +hardware to perform
> > + * wait descriptor completion status write when the Status Write(SW) field 
> > is
> Set.
> > + *
> > + * Track the Device-TLB invalidation status in an invalidation table.
> > +Update
> > + * invalidation table's count of in-flight Device-TLB invalidation
> > +request and
> > + * assign the address of global polling parameter per domain in the
> > +Status Address
> > + * of each invalidation wait descriptor, when submit Device-TLB
> > +invalidation
> > + * requests.
> > + */
> > +struct qi_talbe {
> 
> Did you want to say table rather than talbe?

Yes, It is 'table'. Thanks. I will correct it in next version.

> 
> > +    u64 qi_table_poll_slot;
> > +    u32 qi_table_status_data;
> > +};
> > +
> >   struct hvm_iommu {
> >       struct arch_hvm_iommu arch;
> >
> > @@ -34,6 +49,9 @@ struct hvm_iommu {
> >       struct list_head dt_devices;
> >   #endif
> >
> > +    /* IOMMU Queued Invalidation(QI) */
> > +    struct qi_talbe talbe;
> > +
> 
> This header is should contain any common code between ARM and x86.
> Although, this feature seems to be vtd only (i.e x86).
> 
> So this should be moved in arch_hvm_iommu defined in asm-x86/hvm/iommu.h.
> 
> You would then be able to access the data using
> domain_hvm_iommu(d)->arch.field
> 

I think it doesn't look good. let me redefine it.

> 
> >       /* Features supported by the IOMMU */
> >       DECLARE_BITMAP(features, IOMMU_FEAT_count);
> >   };
> > @@ -41,4 +59,9 @@ struct hvm_iommu {
> >   #define iommu_set_feature(d, f)   set_bit((f),
> domain_hvm_iommu(d)->features)
> >   #define iommu_clear_feature(d, f) clear_bit((f),
> domain_hvm_iommu(d)->features)
> >
> > +#define qi_table_data(d) \
> > +    (d->arch.hvm_domain.hvm_iommu.talbe.qi_table_status_data)
> > +#define qi_table_pollslot(d) \
> > +    (d->arch.hvm_domain.hvm_iommu.talbe.qi_table_poll_slot)
> 
> The way to access the iommu data on ARM and x86 are different. Please
> use domain_hvm_iommu(d)->field if you keep these fields in common code.
>

Ditto.

Julien, thanks for your review.


Quan
 
> > +
> >   #endif /* __XEN_HVM_IOMMU_H__ */
> >
> 
> Regards,
> 
> --
> Julien Grall

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