Re: [Xen-devel] [PATCH V13 4/7] xen/arm: Data abort exception (R/W) mem_events.

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 12/03/15 15:35, Ian Campbell wrote:
> On Thu, 2015-03-12 at 16:19 +0100, Tamas K Lengyel wrote:
>>
>>>  out:
>>                 > +    if ( flush )
>>                 > +    {
>>                 > +        flush_tlb_domain(d);
>>                 > +        iommu_iotlb_flush(d, sgfn, egfn - sgfn);
>>                 > +    }
>>                 
>>                 Is moving the flush out of the loop an independent bug
>>                 fix? If so please
>>                 do in a separate commit with a rationale in the commit
>>                 log. If it is
>>                 somehow related to the changes here then please
>>                 mention it in this
>>                 commit log, since it's a bit subtle.
>>         
>>         
>>         Right, it's not a bugfix and not required to be outside the
>>         loop, I think I just moved it because it made sense to me to
>>         flush it only once instead at every iteration. I'll place it
>>         back.
> 
>>
>> Sorry, the flush wasn't actually part of the loop to begin with. I
>> just moved it under the label out so that the TLB gets flushed when
>> the memaccess setting hypercall gets preempted. I will just set a
>> separate label for it before out so that the existing behavior is
>> preserved but the tlb is still flushed when memaccess is preempted.
> 
> I wonder why this isn't needed for the other uses of goto out, i.e. on
> the relinquish check.

relinquish is only done when the domain is destroyed. So the flush is
not strictly necessary :).


> I'm not convinced this isn't just a straight up bug. Anyone remember any
> reasoning why we don't flush on exit if any work has been done?

Actually the flush is necessary is 3 cases:
        - ALLOCATE
        - INSERT
        - REMOVE

I don't count RELINQUISH because the guest is not scheduled anymore when
it happens.

REMOVE can never fail. In the case of ALLOCATE/INSERT, we redo the
mapping (REMOVE) which will issue a flush.

So for now we are safe. But I think, in general, the flush would be
better after the out label.

It would avoid missing flushing if one day we decide to implement
preemption on more use-case (such as INSERT/ALLOCATE).

Regards,


-- 
Julien Grall

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