Re: [Xen-devel] [PATCH v4 08/10] vt-d/ept: propagate IOMMU Device-TLB flush error up to EPT update.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.05.16 at 16:58, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> On Tue, May 10, 2016 at 10:09 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 06.05.16 at 10:54, <quan.xu@xxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/p2m-ept.c
>>> +++ b/xen/arch/x86/mm/p2m-ept.c
>>> @@ -832,7 +832,8 @@ out:
>>>           need_modify_vtd_table )
>>>      {
>>>          if ( iommu_hap_pt_share )
>>> -            iommu_pte_flush(d, gfn, &ept_entry->epte, order, 
> vtd_pte_present);
>>> +            ret = iommu_pte_flush(d, gfn, &ept_entry->epte,
>>> +                                  order, vtd_pte_present);
>>>          else
>>>          {
>>>              if ( iommu_flags )
>>
>> Looking at this in conjunction with patch 3, I can't see where "ret"
>> would get consumed.
> 
> Hmm, and here I see where "rc == 0" might be a better option than
> "entry_written".
> 
> If we know rc is zero, we can just use rc here instead of 'ret', and I
> think everything falls out.
> 
> If rc is not zero, then we have to do this "if ( !rc ) rc = ret;"
> business, which seems a bit silly to do when we know it's zero and
> don't expect that to change.
> 
> On the other hand, using rc *without* actually checking that it's zero
> seems like asking for trouble.
> 
> So perhaps it would be better if we take your advice for patch 3, and
> then use 'rc' here?

Ah, yes, that's a good 2nd reason.

Jan


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