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

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hello Julien,

> On 23 Oct 2020, at 2:00 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 23/10/2020 12:35, Rahul Singh wrote:
>> 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.
> 
> This depends on your tasklet. Some will run from the softirq context which is 
> usually (for Arm) on the return of an exception.
> 

Thanks for the info. I will check and will get better understanding of the 
tasklet how it will run in XEN.

>>> 
>>>>>>> 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.
> 
> Hmmm... I would not have expected a lot of effort required to add the 3 
> atomics operations above. Are you trying to also port the LSE support at the 
> same time?

There are other atomic operations used in the SMMUv3 code apart from the 3 
atomic operation I mention. I just mention 3 operation as an example. I tried 
to port at that time but when I start porting I realised that one atomic 
operation depend on another one so I decided not to proceed further.

> 
> Cheers,
> 
> -- 
> Julien Grall


Regards,
Rahul