Hi Oleksandr,
On 8/8/19 8:29 PM, Oleksandr wrote:
Sorry for the possible format issues.
> No, it is not disabled. But, ipmmu_irq() uses another
mmu->lock. So, I
> think, there won't be a deadlock.
>
> Or I really missed something?
>
> If we worry about ipmmu_tlb_invalidate() which is called
here (to
> perform a flush by request from P2M code, which manages a
page table)
> and from the irq handler (to perform a flush to resume
address
> translation), I could use a tasklet to schedule
ipmmu_tlb_invalidate()
> from the irq handler then. This way we would get this
serialized. What
> do you think?
I am afraid a tasklet is not an option. You need to perform
the TLB
flush when requested otherwise you are introducing a security
issue.
This is because as soon as a region is unmapped in the page
table, we
remove the drop the reference on any page backing that
region. When the
reference is dropped to zero, the page can be reallocated to
another
domain or even Xen. If the TLB flush happen after, then the
guest may
still be able to access the page for a short time if the
translation has
been cached by the any TLB (IOMMU, Processor).
I understand this. I am not proposing to delay a requested by P2M
code TLB flush in any case. I just propose to issue TLB flush
(which we have to perform in case of page faults, to resolve
error condition and resume translations) from a tasklet rather
than from interrupt handler directly. This is the TLB flush I am
speaking about:
https://github.com/otyshchenko1/xen/blob/ipmmu_upstream2/xen/drivers/passthrough/arm/ipmmu-vmsa.c#L598
Sorry if I was unclear.
My mistake, I misread what you wrote.
I found the flush in the renesas-bsp and not Linux upstream but it
is not clear why this is actually required. You are not fixing any
translation error. So what this flush will do?
Regarding the placement of the flush, then if you execute in a
tasklet it will likely be done later on when the IRQ has been
acknowledge. What's the implication to delay it?
Looks like, there is no need to put this flush into a tasklet. As I
understand from Shimoda-san's answer it is OK to call flush here.
So, my worry about calling ipmmu_tlb_invalidate() directly from the
interrupt handler is not actual anymore.
----------
This is my understanding regarding the flush purpose here. This
code, just follows the TRM, no more no less,
which mentions about a need to flush TLB after clearing error
status register and updating a page table entries (which, I assume,
means to resolve a reason why translation/page fault error actually
have happened) to resume address translation request.
Well, I don't have the TRM... so my point of reference is Linux. Why
does upstream not do the TLB flush?
I have no idea regarding that. >
I have been told this is an errata on the IPMMU. Is it related to
the series posted on linux-iommu [1]?
I don't think, the TLB flush we are speaking about, is related to
that series [1] somehow. This TLB flush, I think, is just the last
step in a sequence of actions which should be performed when the
error occurs, no more no less. This is how I understand this.
If you have to flush the TLBs in the IRQ context then something has
gone really wrong.
I don't deny that Break-Before-Make is an issue. However, if it is
handled correctly in the P2M code. You should only be there because
there are no mapping in the TLBs for the address accessed. So flushing
the TLBs should be unnecessary, unless your TLB is also caching
invalid entry?
Sorry, I don't quite understand why we need to worry about this flush
too much for a case which won't occur in normal condition (if everything
is correct). Why we can't just consider this flush as a required action,
A translation error can be easy to reach. For instance if the guest does
not program the Device correctly and request to access an address that
is not mapped.
which needed to exit from the error state and resume stopped address
translation request. The same required action as "clearing error status
flags" before. We are not trying to understand, why is it so necessary
to clear error flags when error happens, or can we end up without
clearing it, for example. We just follow what described in document. The
same, I think, we have for that flush, if described, then should be
followed. Looks like this flush acts as a trigger to unblock stopped
transaction in that particular case.
What will actually happen if the transaction fail again? For instance,
if the IOVA was not mapped. Will you receive the interrupt again?
If so, are you going to make the flush again and again until the guest
is killed?
Different H/W could have different restoring sequences. Some H/W
requires just clearing error status, other H/W requires full
re-initialization in a specific order to recover from the error state.
Please correct me if I am wrong.
I am not confident to accept any code that I don't understand or I don't
find sensible. As I pointed out in my previous e-mail, this hasn't
reached upstream so something looks quite fishy here.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
