Re: [Xen-devel] [PATCH] Avoid needless flush cache when guest change MTRR

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Zhang, Yang Z
> Sent: Wednesday, February 04, 2015 1:54 PM
> 
> Tian, Kevin wrote on 2015-02-04:
> >> From: Li, Liang Z
> >> Sent: Wednesday, February 04, 2015 11:28 AM
> >>
> >> Flushing cache is needed only when guest has IOMMU device, using
> >> need_iommu(d) can minimize the impact to guest with device assigned,
> >> since a guest may be hot plugged with a device thus there may be
> >> dirty cache lines before need_iommu(d) becoming true, force the
> >> flush_all when the first device is assigned to guest to amend this issue.
> >>
> >> Signed-off-by: Liang Li <liang.z.li@xxxxxxxxx>
> >> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> >> ---
> >>  xen/arch/x86/hvm/mtrr.c       | 2 +-
> >>  xen/drivers/passthrough/pci.c | 5 +++++
> >>  2 files changed, 6 insertions(+), 1 deletion(-)
> >> diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index
> >> ee18553..858fb7e 100644 --- a/xen/arch/x86/hvm/mtrr.c +++
> >> b/xen/arch/x86/hvm/mtrr.c @@ -791,7 +791,7 @@
> >> HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr,
> hvm_load_mtrr_msr,
> >>
> >>  void memory_type_changed(struct domain *d)  {
> >> -    if ( iommu_enabled && d->vcpu && d->vcpu[0] )
> >> +    if ( need_iommu(d) && d->vcpu && d->vcpu[0] )
> >>      {
> >>          p2m_memory_type_changed(d);
> >>          flush_all(FLUSH_CACHE);
> >> diff --git a/xen/drivers/passthrough/pci.c
> >> b/xen/drivers/passthrough/pci.c index 78c6977..a023d1d 100644
> >> --- a/xen/drivers/passthrough/pci.c
> >> +++ b/xen/drivers/passthrough/pci.c
> >> @@ -1365,6 +1365,11 @@ static int assign_device(struct domain *d,
> >> u16 seg,
> >> u8 bus, u8 devfn)
> >>              }
> >>          }
> >>          d->need_iommu = 1;
> >> +        /* There may be dirty cache lines when a device is assigned
> >> +         * and before need_iommu(d) becoming true, this will cause
> >> +         * memory_type_changed lose effect if memory type changes.
> >> +         * Call memory_type_changed here to amend this. */
> >> +        memory_type_changed(d);
> >
> > could we relax this force flush only on the 1st assigned device which
> > actually changes need_iommu(d) from false to true?
> 
> It is already covered by current code when updating need_iommu, but the
> patch doesn't show it:
> 
> if ( need_iommu(d) <= 0 )
> {
>     .....
>     d->need_iommu = 1;
>     memory_type_changed(d);
> }
> 

You're right. :-)

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>

Thanks
Kevin

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