[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Julien Grall <julien@xxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Fri, 23 Oct 2020 11:41:02 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4UglLBGLZ5V1tj8IgCmnb7EYeOzzLeZk/KUGgNgTsFo=; b=ALng+dWjKzGdoxRogD5QiikW0h6xpoF7Gt++TDWrqcCpGKRyJlQnXR5ARzItCR7YqNrWUm1u0TNzRuC31Aq1RSN+XYv0WsOpgRmU5R6kL1F7X6hK7d4tQ2kE/w5o5YsqoYjN/xQh7V7hLY45ff21VQQ58g62DqZO0mqPCtowfniiEHjfgej841VmWO2Mi0056RR25ubNvIolKXTP84OsA/ODfOdmqvKmVlBYxEysO3czMfxBlG2/DIZZEZ7ls8Q3ISr7CEBUan8HvFpBPe3OlBUs1Ph0/hmBMnSqbTb1/YNmfLmehuddQ7MBtZAQ45qPoK4NA4EHdSMiurG8gQIsww==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DZNTaWxiMoPo1n3mmEo6nCRh+rZBCok/28zROfpdLfAGgCgba/iea3pvXqWuuI/xaAdwTpYGfzMzH7AQ4OuF7eWqiDQp1kCgjrUbt3Ucsl6VyJJmnTpOe8ri7e9mR/vpjCJyFa4JwNn3hs9/Z+p/f+cwuPqcLA7XMwDqSoywSj8NK23lkeXrvP1zWZyZoYxtPIbzcf7bKDowO3Eyfe/fyjPgv32kVmMvZPy4fIwg9aEF8xXj5Yw9KC4GMf8o7spOkka61c/YShFiKpRHz14zGtQ6Hwglrq1KEnhk6rpYTL2I2dUhPXXnxWeb1RD5Cxsb+COr1n/icwPooexJlC/ssQ==
  • Authentication-results-original: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Fri, 23 Oct 2020 11:41:40 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHWoVvrkmJwOYERdUOadvid1OghFamgw0AAgAEzsQCAAWIugIABxu0A
  • Thread-topic: [XEN PATCH v1] xen/arm : Add support for SMMUv3 driver

Hello Julien,

> On 22 Oct 2020, at 9:32 am, Julien Grall <julien@xxxxxxx> wrote:
> On 21/10/2020 12:25, Rahul Singh wrote:
>> Hello Julien,
> Hi Rahul,
>>> On 20 Oct 2020, at 6:03 pm, Julien Grall <julien@xxxxxxx> 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.singh@xxxxxxx>
>>>> ---
>>>>  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




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.