From: Thomas Gleixner <tglx@linutronix.de>
To: Jason Gunthorpe <jgg@ziepe.ca>, "Peng Fan (OSS)" <peng.fan@oss.nxp.com>
Cc: Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
Joerg Roedel <joro@8bytes.org>, Marc Zyngier <maz@kernel.org>,
Bixuan Cui <cuibixuan@huawei.com>,
linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com>
Subject: Re: [PATCH 3/3] iommu/arm-smmu-v3: support suspend/resume
Date: Tue, 26 Mar 2024 21:11:03 +0100 [thread overview]
Message-ID: <87msqkeotk.ffs@tglx> (raw)
In-Reply-To: <87bk72fcf4.ffs@tglx>
On Mon, Mar 25 2024 at 18:29, Thomas Gleixner wrote:
> On Mon, Mar 25 2024 at 10:48, Jason Gunthorpe wrote:
>> On Sun, Mar 24, 2024 at 08:29:00PM +0800, Peng Fan (OSS) wrote:
>>> + if (smmu->features & ARM_SMMU_FEAT_PRI) {
>>> + desc = irq_get_msi_desc(smmu->priq.q.irq);
>>> + if (desc) {
>>> + get_cached_msi_msg(smmu->priq.q.irq, &msg);
>>> + arm_smmu_write_msi_msg(desc, &msg);
>>> + }
>>> + }
>>> +}
>>
>> I wonder if this should be done instead by converting the driver away
>> from platform MSI to the new MSI mechanism?
>
> There is work in progress for that. Should come around in the next
> weeks.
But that won't solve it. The above is a horrible hack and I think this
should be solved completely differently.
On suspend the core interrupt code disables all interrupts which are not
marked as wakeup interrupts. On resume it reenables them including
restoring the affinity setting.
So this just should work, but it does not because the MSI message is
only written once, when the interrupt is activated. Further affinity
changes affect only the ITS table and do not result in a message write.
The most trivial way w/o changing any core code for it, would be to free
the interrupts on suspend and request them on resume again, because that
would deactivate and reactivate it.
Though that does not work if the IOMMU is resumed in the early resume
path. But we can enforce writing the message in the resume path.
See the untested below.
Thanks,
tglx
---
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -223,6 +223,7 @@ struct irq_data {
* irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
* IRQD_RESEND_WHEN_IN_PROGRESS - Interrupt may fire when already in progress in which
* case it must be resent at the next available opportunity.
+ * IRQD_RESUMING - Interrupt is resumed after suspend
*/
enum {
IRQD_TRIGGER_MASK = 0xf,
@@ -249,6 +250,7 @@ enum {
IRQD_AFFINITY_ON_ACTIVATE = BIT(28),
IRQD_IRQ_ENABLED_ON_SUSPEND = BIT(29),
IRQD_RESEND_WHEN_IN_PROGRESS = BIT(30),
+ IRQD_RESUMING = BIT(31),
};
#define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
@@ -443,6 +445,11 @@ static inline bool irqd_needs_resend_whe
return __irqd_to_state(d) & IRQD_RESEND_WHEN_IN_PROGRESS;
}
+static inline bool irqd_resuming(struct irq_data *d)
+{
+ return __irqd_to_state(d) & IRQD_RESUMING;
+}
+
#undef __irqd_to_state
static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -617,6 +617,7 @@ static unsigned int msi_domain_get_hwsiz
static inline void irq_chip_write_msi_msg(struct irq_data *data,
struct msi_msg *msg)
{
+ irq_data_get_msi_desc(data)->msg = *msg;
data->chip->irq_write_msi_msg(data, msg);
}
@@ -652,7 +653,10 @@ int msi_domain_set_affinity(struct irq_d
int ret;
ret = parent->chip->irq_set_affinity(parent, mask, force);
- if (ret >= 0 && ret != IRQ_SET_MASK_OK_DONE) {
+ if (ret < 0)
+ return ret;
+
+ if (ret != IRQ_SET_MASK_OK_DONE || irqd_resuming(irq_data)) {
BUG_ON(irq_chip_compose_msi_msg(irq_data, msg));
msi_check_level(irq_data->domain, msg);
irq_chip_write_msi_msg(irq_data, msg);
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -177,7 +177,13 @@ static void resume_irq(struct irq_desc *
irq_state_set_masked(desc);
resume:
desc->istate &= ~IRQS_SUSPENDED;
+ /*
+ * Ensure that MSI messages get rewritten on resume. The device
+ * might have lost it due to power disabling.
+ */
+ irqd_set(&desc->irq_data, IRQD_RESUMING);
__enable_irq(desc);
+ irqd_clear(&desc->irq_data, IRQD_RESUMING);
}
static void resume_irqs(bool want_early)
prev parent reply other threads:[~2024-03-26 20:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-24 12:28 [PATCH 0/3] iommu/smmu-v3: support suspend/resume Peng Fan (OSS)
2024-03-24 12:28 ` [PATCH 1/3] iommu/arm-smmu-v3: save bypass in smmu device structure Peng Fan (OSS)
2024-03-24 12:28 ` [PATCH 2/3] genirq/msi: cache the last msi msg Peng Fan (OSS)
2024-03-25 17:40 ` Thomas Gleixner
2024-03-26 1:02 ` Peng Fan
2024-03-24 12:29 ` [PATCH 3/3] iommu/arm-smmu-v3: support suspend/resume Peng Fan (OSS)
2024-03-25 13:48 ` Jason Gunthorpe
2024-03-25 17:29 ` Thomas Gleixner
2024-03-26 1:26 ` Peng Fan
2024-03-26 20:11 ` Thomas Gleixner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87msqkeotk.ffs@tglx \
--to=tglx@linutronix.de \
--cc=cuibixuan@huawei.com \
--cc=iommu@lists.linux.dev \
--cc=jgg@ziepe.ca \
--cc=joro@8bytes.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=peng.fan@nxp.com \
--cc=peng.fan@oss.nxp.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).