Re: [XEN PATCH v1] xen/arm : Add support for SMMUv3 driver

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hello,

> On 23 Oct 2020, at 1:02 am, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> On Thu, 22 Oct 2020, Julien Grall wrote:
>>>> On 20/10/2020 16:25, Rahul Singh wrote:
>>>>> Add support for ARM architected SMMUv3 implementations. It is based on
>>>>> the Linux SMMUv3 driver.
>>>>> Major differences between the Linux driver are as follows:
>>>>> 1. Only Stage-2 translation is supported as compared to the Linux driver
>>>>>    that supports both Stage-1 and Stage-2 translations.
>>>>> 2. Use P2M  page table instead of creating one as SMMUv3 has the
>>>>>    capability to share the page tables with the CPU.
>>>>> 3. Tasklets is used in place of threaded IRQ's in Linux for event queue
>>>>>    and priority queue IRQ handling.
>>>> 
>>>> Tasklets are not a replacement for threaded IRQ. In particular, they will
>>>> have priority over anything else (IOW nothing will run on the pCPU until
>>>> they are done).
>>>> 
>>>> Do you know why Linux is using thread. Is it because of long running
>>>> operations?
>>> 
>>> Yes you are right because of long running operations Linux is using the
>>> threaded IRQs.
>>> 
>>> SMMUv3 reports fault/events bases on memory-based circular buffer queues not
>>> based on the register. As per my understanding, it is time-consuming to
>>> process the memory based queues in interrupt context because of that Linux
>>> is using threaded IRQ to process the faults/events from SMMU.
>>> 
>>> I didn’t find any other solution in XEN in place of tasklet to defer the
>>> work, that’s why I used tasklet in XEN in replacement of threaded IRQs. If
>>> we do all work in interrupt context we will make XEN less responsive.
>> 
>> So we need to make sure that Xen continue to receives interrupts, but we also
>> need to make sure that a vCPU bound to the pCPU is also responsive.
>> 
>>> 
>>> If you know another solution in XEN that will be used to defer the work in
>>> the interrupt please let me know I will try to use that.
>> 
>> One of my work colleague encountered a similar problem recently. He had a 
>> long
>> running tasklet and wanted to be broken down in smaller chunk.
>> 
>> We decided to use a timer to reschedule the taslket in the future. This 
>> allows
>> the scheduler to run other loads (e.g. vCPU) for some time.
>> 
>> This is pretty hackish but I couldn't find a better solution as tasklet have
>> high priority.
>> 
>> Maybe the other will have a better idea.
> 
> Julien's suggestion is a good one.
> 
> But I think tasklets can be configured to be called from the idle_loop,
> in which case they are not run in interrupt context?
> 

 Yes you are right tasklet will be scheduled from the idle_loop that is not 
interrupt conext.

> Still, tasklets run until completion in Xen, which could take too long.
> The code has to voluntarily release control of the execution flow once
> it realizes it has been running for too long. The rescheduling via a
> timer works.
> 
> 
> Now, to brainstorm other possible alternatives, for hypercalls we have
> been using hypercall continuations.  Continuations is a way to break a
> hypercall implementation that takes too long into multiple execution
> chunks. It works by calling into itself again: making the same hypercall
> again with updated arguments, so that the scheduler has a chance to do
> other operations in between, including running other tasklets and
> softirqs.
> 
> That works well because  the source of the work is a guest request,
> specifically a hypercall. However, in the case of the SMMU driver, there
> is no hypercall. The Xen driver has to do work in response to an
> interrupt and the work is not tied to one particular domain.
> 
> So I don't think the hypercall continuation model could work here. The
> timer seems to be the best option.
> 

Yes, I agree with you as the source of the work is not a guest request in the 
case of SMMU I think we can not use the hyper call continuation.

As suggested I will try to use the timer to schedule the work and will share 
the findings.
> 
> 
>>>>> 4. Latest version of the Linux SMMUv3 code implements the commands queue
>>>>>    access functions based on atomic operations implemented in Linux.
>>>> 
>>>> Can you provide more details?
>>> 
>>> I tried to port the latest version of the SMMUv3 code than I observed that
>>> in order to port that code I have to also port atomic operation implemented
>>> in Linux to XEN. As latest Linux code uses atomic operation to process the
>>> command queues (atomic_cond_read_relaxed(),atomic_long_cond_read_relaxed() ,
>>> atomic_fetch_andnot_relaxed()) .
>> 
>> Thank you for the explanation. I think it would be best to import the atomic
>> helpers and use the latest code.
>> 
>> This will ensure that we don't re-introduce bugs and also buy us some time
>> before the Linux and Xen driver diverge again too much.
>> 
>> Stefano, what do you think?
> 
> I think you are right.

Yes, I agree with you to have XEN code in sync with Linux code that's why I 
started with to port the Linux atomic operations to XEN  then I realised that 
it is not straightforward to port atomic operations and it requires lots of 
effort and testing. Therefore I decided to port the code before the atomic 
operation is introduced in Linux.


Regards,
Rahul