Hello Oleksandr-san, Thank you for the patch!
> From: Oleksandr Tyshchenko, Sent: Sunday, November 28, 2021 2:52 AM > > From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com> > > Based on the following commits from the Renesas BSP: > 8fba83d97cca709a05139c38e29408e81ed4cf62 > a8d93bc07da89a7fcf4d85f34d119a030310efa5 > located at: <snip> > > Original commit messages: > commit 8fba83d97cca709a05139c38e29408e81ed4cf62 > Author: Nam Nguyen <nam.nguyen...@renesas.com> > 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...@renesas.com> > > commit a8d93bc07da89a7fcf4d85f34d119a030310efa5 > Author: Nam Nguyen <nam.nguyen...@renesas.com> > 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...@renesas.com> > > ********** > > 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. IIUC, we need this for IPMMU-DS0. > 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. I agree. However, I realized IMSCTLR and IMSAUXCTLR registers' offset differs between Gen3 and Gen4. About BSP patch of 07/10, it seems to take care of the offset. But, Linux upstream based code doesn't take care of it. So, we have to add a new member (maybe "control_offset_base" is a good name?) to calculate the address. > 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. ipmmu_domain_setup_context() is called in probing and resuming. So, it's enough to clear in ipmmu_probe() I think. > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com> > --- > xen/drivers/passthrough/arm/ipmmu-vmsa.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c > b/xen/drivers/passthrough/arm/ipmmu-vmsa.c > index 8dfdae8..22dd84e 100644 > --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c > +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c > @@ -229,6 +229,9 @@ static DEFINE_SPINLOCK(ipmmu_devices_lock); > #define IMUASID0(n) (0x0308 + ((n) * 16)) > #define IMUASID32(n) (0x0608 + (((n) - 32) * 16)) > > +#define IMSCTLR 0x0500 > +#define IMSCTLR_USE_SECGRP (1 << 28) > + > #define IMSAUXCTLR 0x0504 > #define IMSAUXCTLR_S2PTE (1 << 3) As I mentioned above, we have to adjust these registers' offset for both Gen3 (+0) and Gen4 (+0x1000) somehow. > @@ -979,6 +982,10 @@ static int ipmmu_probe(struct dt_device_node *node) > set_bit(0, mmu->ctx); > } > > + /* Do not use security group function. */ > + reg = IMSCTLR + mmu->features->ctx_offset_stride_adj; > + ipmmu_write(mmu, reg, ipmmu_read(mmu, reg) & ~IMSCTLR_USE_SECGRP); If we modify the 07/10 patch, we cannot use ctx_offset_stride_adj. # But, using "ctx_offset" here seems to be abused though because # the register is not context. Best regards, Yoshihiro Shimoda
