Hello Julien, > On 22 Oct 2020, at 9:32 am, Julien Grall <jul...@xen.org> wrote: > > > > On 21/10/2020 12:25, Rahul Singh wrote: >> Hello Julien, > > Hi Rahul, > >>> On 20 Oct 2020, at 6:03 pm, Julien Grall <jul...@xen.org> wrote: >>> >>> Hi Rahul, >>> >>> Thank you for the contribution. Lets make sure this attempt to SMMUv3 >>> support in Xen will be more successful than the other one :). >> Yes sure. >>> >>> I haven't reviewed the code yet, but I wanted to provide feedback on the >>> commit message. >>> >>> 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.
Yes I agree. > >> 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. Let me try to use the timer and will share my 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? > >>> >>>> Atomic functions used by the commands queue access functions is not >>>> implemented in XEN therefore we decided to port the earlier version >>>> of the code. Once the proper atomic operations will be available in XEN >>>> the driver can be updated. >>>> Signed-off-by: Rahul Singh <rahul.si...@arm.com> >>>> --- >>>> xen/drivers/passthrough/Kconfig | 10 + >>>> xen/drivers/passthrough/arm/Makefile | 1 + >>>> xen/drivers/passthrough/arm/smmu-v3.c | 2847 +++++++++++++++++++++++++ >>>> 3 files changed, 2858 insertions(+) >>> >>> This is quite significant patch to review. Is there any way to get it split >>> (maybe a verbatim Linux copy + Xen modification)? >> Yes, I understand this is a quite significant patch to review let me think >> to get it split. If it is ok for you to review this patch and provide your >> comments then it will great for us. > I will try to have a look next week. Thanks in advance ☺️ > > Cheers, > > -- > Julien Grall Regards, Rahul
