RE: [PATCH V2 08/10] iommu/ipmmu-vmsa: Set IPMMU bit IMSCTLR_USE_SECGRP to 0

[Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>]

Hello Oleksandr-san,

> From: Oleksandr Tyshchenko, Sent: Tuesday, December 21, 2021 6:16 AM
> 
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> 
> Based on the following commits from the Renesas BSP:
> 8fba83d97cca709a05139c38e29408e81ed4cf62
> a8d93bc07da89a7fcf4d85f34d119a030310efa5
> located at:
<snip>
> 
> Original commit messages:
>  commit 8fba83d97cca709a05139c38e29408e81ed4cf62
>  Author: Nam Nguyen <nam.nguyen.yh@xxxxxxxxxxx>
>  Date:   Wed Apr 28 18:54:44 2021 +0700
> 
>   iommu/ipmmu-vmsa: Set IPMMU bit IMSCTLR_USE_SECGRP to 0
> 
>   Need to set bit IMSCTLR_USE_SECGRP to 0
>   because H/W initial value is unknown, without this
>   dma-transfer cannot be done due to address translation doesn't work.
> 
>   Signed-off-by: Nam Nguyen <nam.nguyen.yh@xxxxxxxxxxx>
> 
>  commit a8d93bc07da89a7fcf4d85f34d119a030310efa5
>  Author: Nam Nguyen <nam.nguyen.yh@xxxxxxxxxxx>
>  Date:   Tue Sep 7 14:46:12 2021 +0700
> 
>   iommu/ipmmu-vmsa: Update IMSCTLR register offset address for R-Car S4
> 
>   Update IMSCTLR register offset address to align with R-Car S4 H/W UM.
> 
>   Signed-off-by: Nam Nguyen <nam.nguyen.yh@xxxxxxxxxxx>
> 
> **********
> 
> It is still a question whether this really needs to be done in Xen,
> rather in firmware, but better to be on the safe side. After all,
> if firmware already takes care of clearing this bit, nothing bad
> will happen.
> 
> Please note the following:
> 1. I decided to squash both commits since the first commit adds clearing
> code and only the second one makes it functional on S4. Moreover, this is
> not a direct port. So it would be better to introduce complete solution
> by a single patch.
> 2. Although patch indeed does what it claims in the subject,
> the implementation is different in comparison with original changes.
> On Linux the clearing is done at runtime in ipmmu_domain_setup_context().
> On Xen the clearing is done at boot time in ipmmu_probe().
> The IMSCTLR is not a MMU "context" register at all, so I think there is
> no point in performing the clearing each time we initialize the context,
> instead perform the clearing at once during initialization. Also do not
> abuse ctx_offset_stride_adj field for the register's offset calculation,
> instead use recently added control_offset_base field.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>

Best regards,
Yoshihiro Shimoda