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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

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);
}

> 
>>      }
>>      
>>      pdev = pci_get_pdev_by_domain(hardware_domain, seg, bus,
>> devfn);
>> --
>> 1.9.1


Best regards,
Yang



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