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

From: Julien Grall <julien@xen.org>
To: Rahul Singh <Rahul.Singh@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	Jan Beulich <jbeulich@suse.com>, Paul Durrant <paul@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [XEN PATCH v1] xen/arm : Add support for SMMUv3 driver
Date: Fri, 23 Oct 2020 14:00:00 +0100	[thread overview]
Message-ID: <7314936f-6c1e-5ca6-a33b-973c8e61ba3b@xen.org> (raw)
In-Reply-To: <D8EF4B06-B64D-4264-8C86-DA1B5A1146D2@arm.com>

On 23/10/2020 12:35, Rahul Singh wrote:
> Hello,
> 
>> On 23 Oct 2020, at 1:02 am, Stefano Stabellini <sstabellini@kernel.org> 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.

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

Cheers,

-- 
Julien Grall