Re: [PATCH -next] iommu/arm-smmu-v3: Add suspend and resume support

On Wed, 21 Jul 2021 14:59:47 +0100,
Robin Murphy [off-list ref] wrote:

On 2021-07-21 14:12, Marc Zyngier wrote:

quoted

On Wed, 21 Jul 2021 12:42:14 +0100,
Robin Murphy [off-list ref] wrote:

quoted

[ +Marc for MSI bits ]

On 2021-07-21 02:33, Bixuan Cui wrote:

quoted

Add suspend and resume support for arm-smmu-v3 by low-power mode.

When the smmu is suspended, it is powered off and the registers are
cleared. So saves the msi_msg context during msi interrupt initialization
of smmu. When resume happens it calls arm_smmu_device_reset() to restore
the registers.

Signed-off-by: Bixuan Cui <redacted>
Reviewed-by: Wei Yongjun <redacted>
Reviewed-by: Zhen Lei <redacted>
Reviewed-by: Ding Tianhong <redacted>
Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
---

   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 72 ++++++++++++++++++---
   1 file changed, 64 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 235f9bdaeaf2..bf1163acbcb1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -40,6 +40,7 @@ MODULE_PARM_DESC(disable_bypass,
     static bool disable_msipolling;
   module_param(disable_msipolling, bool, 0444);
+static bool bypass;
   MODULE_PARM_DESC(disable_msipolling,
   	"Disable MSI-based polling for CMD_SYNC completion.");
   @@ -3129,11 +3130,37 @@ static void arm_smmu_write_msi_msg(struct

msi_desc *desc, struct msi_msg *msg)
   	doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
   	doorbell &= MSI_CFG0_ADDR_MASK;
   +	/* Saves the msg context for resume if desc->msg is empty */
+	if (desc->msg.address_lo == 0 && desc->msg.address_hi == 0) {
+		desc->msg.address_lo = msg->address_lo;
+		desc->msg.address_hi = msg->address_hi;
+		desc->msg.data = msg->data;
+	}

My gut feeling is that this is something a device driver maybe
shouldn't be poking into, but I'm not entirely familiar with the area
:/

Certainly not. If you rely on the message being stored into the
descriptors, then implement this in the core code, like we do for PCI.

Ah, so it would be an acceptable compromise to *read* desc->msg (and
thus avoid having to store our own copy of the message) if the core
was guaranteed to cache it? That's good to know, thanks.

Yeah, vfio, a couple of other weird drivers and (*surprise!*) ia64 are
using this kind of trick. I don't see a reason not to implement that
for platform-MSI (although level signalling may be interesting...), or
even to move it into the core MSI code.

quoted

quoted

quoted

+
   	writeq_relaxed(doorbell, smmu->base + cfg[0]);
   	writel_relaxed(msg->data, smmu->base + cfg[1]);
   	writel_relaxed(ARM_SMMU_MEMATTR_DEVICE_nGnRE, smmu->base + cfg[2]);
   }
   +static void arm_smmu_resume_msis(struct arm_smmu_device *smmu)
+{
+	struct msi_desc *desc;
+	struct device *dev = smmu->dev;
+
+	for_each_msi_entry(desc, dev) {
+		switch (desc->platform.msi_index) {
+		case EVTQ_MSI_INDEX:
+		case GERROR_MSI_INDEX:
+		case PRIQ_MSI_INDEX:
+			arm_smmu_write_msi_msg(desc, &(desc->msg));

Consider using get_cached_msi_msg() instead of using the internals of
the descriptor.

Oh, there's even a proper API for it, marvellous! I hadn't managed to
dig that far myself :)

It is a bit odd in the sense that it takes a copy of the message
instead of returning a pointer, but at least this solves lifetime
issues.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel