Re: [Xen-devel] [PATCH] VT-d: make flush-all actually flush all

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.15 at 09:29, <feng.wu@xxxxxxxxx> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Thursday, December 10, 2015 3:39 PM
>> To: Wu, Feng <feng.wu@xxxxxxxxx>
>> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; xen-devel <xen-
>> devel@xxxxxxxxxxxxxxxxxxxx>
>> Subject: RE: [PATCH] VT-d: make flush-all actually flush all
>> 
>> >>> On 10.12.15 at 04:06, <feng.wu@xxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> Sent: Wednesday, December 9, 2015 10:53 PM
>> >> --- a/xen/drivers/passthrough/vtd/iommu.c
>> >> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> >> @@ -583,7 +583,7 @@ static void __intel_iommu_iotlb_flush(st
>> >>          if ( iommu_domid == -1 )
>> >>              continue;
>> >>
>> >> -        if ( page_count > 1 || gfn == -1 )
>> >> +        if ( page_count != 1 || gfn == INVALID_GFN )
>> >
>> > This patch looks good me, but I think using 'page_count'  to decide
>> > whether using dsi or psi is not a good idea, since psi should also support
>> > invalidate multiple pages from VT-d Spec. (Seems no support in Xen?)
>> 
>> I'm fine with this getting improved in a subsequent patch, but I
>> don't see this to be done here - what you propose is an
>> enhancement, while here I'm fixing a latent bug (which originally
>> got reported to security@, and we were able to discard security
>> concerns merely because the sole intel_iommu_iotlb_flush_all()
>> caller sits on a code path reachable only through an XSA-77
>> covered domctl). The more that there currently is no caller
>> passing in other than 0 or 1.
> 
> In intel_iommu_iotlb_flush_all(), 0 is passed in as the 'page_count',
> but intel_iommu_iotlb_flush() can pass in a value more than 1
> for 'page_count', right?

Ah, yes, it being the .iotlb_flush handler.

Jan


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