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

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

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?

Everything else looks good.

 -George

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