Re: [PATCH v3 0/8] xen/arm: Add support for SMMUv3 driver

From: Rahul Singh <Rahul.Singh@arm.com>
To: Julien Grall <julien@xen.org>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Jan Beulich <jbeulich@suse.com>,
	Wei Liu <wl@xen.org>, Paul Durrant <paul@xen.org>
Subject: Re: [PATCH v3 0/8] xen/arm: Add support for SMMUv3 driver
Date: Mon, 14 Dec 2020 20:01:07 +0000	[thread overview]
Message-ID: <8ED5EAAF-48B0-4289-BCB0-232F70001134@arm.com> (raw)
In-Reply-To: <ea121c23-4deb-c566-4d1d-bb9dd4959015@xen.org>

Hello Julien,Stefano

> On 11 Dec 2020, at 2:29 pm, Julien Grall <julien@xen.org> wrote:
> 
> Hi Rahul,
> 
> On 10/12/2020 16:56, Rahul Singh wrote:
>> This patch series is v3 of the work to add support for the SMMUv3 driver.
>> Approach taken is to first merge the Linux copy of the SMMUv3 driver
>> (tag v5.8.18) and then modify the driver to build on XEN.
>> MSI and PCI ATS functionality are not supported. Code is not tested and
>> compiled. Code is guarded by the flag CONFIG_PCI_ATS and CONFIG_MSI to compile
>> the driver.
>> Code specific to Linux is removed from the driver to avoid dead code.
>> Driver is currently supported as tech preview.
>> Following functionality should be supported before driver is out for tech
>> preview
>> 1. Investigate the timing analysis of using spin lock in place of mutex when
>>    attaching a  device to SMMU.
>> 2. Merged the latest Linux SMMUv3 driver code once atomic operation is
>>    available in XEN.
>> 3. PCI ATS and MSI interrupts should be supported.
>> 4. Investigate side-effect of using tasklet in place of threaded IRQ and fix
>>    if any.
> In your last e-mail, you wrote that you would investigate and then come back to use. It wasn't clear that this meant you will not deal with it in this series.
> 

Yes, I will investigate the side-effect of using tasklet but not part of this patch series. It will be great if we proceed on this patch series as it is (using tasklet in place of threaded-IRQ).

>> 5. fallthorugh keyword should be supported.
> 
> This one should really be done now... It is not a complicated one...

Ok. I will implement in next version.

> 
>> 6. Implement the ffsll function in bitops.h file.

While implementing the code I found out that Linux is using the built-in function “__builtin_ffsll() “ for ffsll and there is no implementation available in Linux for ffsll.
If we implement the ffsll in XEN we will diverge from Linux. I am thinking of adding the below code in "xen/include/asm-arm/bitops.h”. 
Please suggest if it is okay?

static always_inline int ffsll(long long x)                                           
{                                                                               
    return __builtin_ffsll(x);                                                    
}

Regards,
Rahul

> 
> ... same here.
> 
> Cheers,
> 
> -- 
> Julien Grall