Re: [PATCH v2 4/8] xen/arm: Remove support for MSI on SMMUv3

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hello Julien,

> On 2 Dec 2020, at 2:11 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Rahul,
> 
> On 02/12/2020 13:12, Rahul Singh wrote:
>> Hello Stefano,
>>> On 2 Dec 2020, at 12:40 am, Stefano Stabellini <sstabellini@xxxxxxxxxx> 
>>> wrote:
>>> 
>>> On Tue, 1 Dec 2020, Stefano Stabellini wrote:
>>>> On Thu, 26 Nov 2020, Rahul Singh wrote:
>>>>> XEN does not support MSI on ARM platforms, therefore remove the MSI
>>>>> support from SMMUv3 driver.
>>>>> 
>>>>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>>>> 
>>>> I wonder if it makes sense to #ifdef CONFIG_MSI this code instead of
>>>> removing it completely.
>>> 
>>> One more thought: could this patch be achieved by reverting
>>> 166bdbd23161160f2abcea70621adba179050bee ? If this patch could be done
>>> by a couple of revert, it would be great to say it in the commit
>>> message.
>>> 
>>  Ok will add in next version.
>>> 
>>>> In the past, we tried to keep the entire file as textually similar to
>>>> the original Linux driver as possible to make it easier to backport
>>>> features and fixes. So, in this case we would probably not even used an
>>>> #ifdef but maybe something like:
>>>> 
>>>>  if (/* msi_enabled */ 0)
>>>>      return;
>>>> 
>>>> at the beginning of arm_smmu_setup_msis.
>>>> 
>>>> 
>>>> However, that strategy didn't actually work very well because backports
>>>> have proven difficult to do anyway. So at that point we might as well at
>>>> least have clean code in Xen and do the changes properly.
> 
> It was difficult because Linux decided to rework how IOMMU drivers works. I 
> agree the risk is still there and therefore clean code would be better with 
> some caveats (see below).
> 
>> Main reason to remove the feature/code that is not usable in XEN is to have 
>> a clean code.
> 
> I agree that short term this feature will not be usable. However, I think we 
> need to think about {medium, long}-term plan to avoid extra effort in the 
> future because the driver evolve in a way making the revert of revert 
> impossible.
> 
> Therefore I would prefer to keep both the MSI and PCI ATS present as they are 
> going to be useful/necessary on some platforms. It doesn't matter that they 
> don't work because the driver will be in tech preview.

Ok. As Stefano also agreed the same I will keep the PC ATS and MSI 
functionality in next version.

Regards,
Rahul

> 
> Cheers,
> 
> -- 
> Julien Grall