Re: [Xen-devel] [PATCH V2 6/6] iommu/arm: Add Renesas IPMMU-VMSA support

[Julien Grall <julien.grall@xxxxxxx>]

On 07/08/2019 21:28, Oleksandr Tyshchenko wrote:
> Hi, Julien.
Hi,

> 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?
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel