From: Bixuan Cui <cuibixuan@huawei.com>
To: Marc Zyngier <maz@kernel.org>, Robin Murphy <robin.murphy@arm.com>
Cc: <iommu@lists.linux-foundation.org>,
<linux-kernel@vger.kernel.org>, <will@kernel.org>,
<weiyongjun1@huawei.com>, <john.wanghui@huawei.com>,
<dingtianhong@huawei.com>, <thunder.leizhen@huawei.com>,
<guohanjun@huawei.com>, <joro@8bytes.org>,
<jean-philippe@linaro.org>, <Jonathan.Cameron@huawei.com>,
<song.bao.hua@hisilicon.com>,
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH -next] iommu/arm-smmu-v3: Add suspend and resume support
Date: Thu, 22 Jul 2021 14:34:32 +0800 [thread overview]
Message-ID: <5054a5cd-579f-3fe9-1884-5219c8d13531@huawei.com> (raw)
In-Reply-To: <877dhj3e4b.wl-maz@kernel.org>
On 2021/7/21 23:01, Marc Zyngier wrote:
> On Wed, 21 Jul 2021 14:59:47 +0100,
> Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-07-21 14:12, Marc Zyngier wrote:
>>> On Wed, 21 Jul 2021 12:42:14 +0100,
>>> Robin Murphy <robin.murphy@arm.com> wrote:
>>>>
>>>> [ +Marc for MSI bits ]
>>>>
>>>> On 2021-07-21 02:33, Bixuan Cui wrote:
>>>>> 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 <cuibixuan@huawei.com>
>>>>> Reviewed-by: Wei Yongjun <weiyongjun1@huawei.com>
>>>>> Reviewed-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>>> Reviewed-by: Ding Tianhong <dingtianhong@huawei.com>
>>>>> 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.
Agree. If msg is saved to desc->msg in MSI core, the code here will not need.
During the initialization of the MSI interrupt of the SMMU, the desc->msg
is never used. So I save msg to desc->msg for resume use.
>>
>>>>> +
>>>>> 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.
The code of arm_smmu_write_msi_msg() is multiplexed to restore the register. Therefore,
the parameter must be supplemented. Generally, desc is sufficient as an input parameter..
:)
Thanks,
Bixuan Cui
>
> Thanks,
>
> M.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-07-22 6:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-21 1:33 [PATCH -next] iommu/arm-smmu-v3: Add suspend and resume support Bixuan Cui
2021-07-21 11:42 ` Robin Murphy
2021-07-21 13:12 ` Marc Zyngier
2021-07-21 13:59 ` Robin Murphy
2021-07-21 15:01 ` Marc Zyngier
2021-07-22 6:34 ` Bixuan Cui [this message]
2021-07-22 7:26 ` Bixuan Cui
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=5054a5cd-579f-3fe9-1884-5219c8d13531@huawei.com \
--to=cuibixuan@huawei.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=dingtianhong@huawei.com \
--cc=guohanjun@huawei.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=john.wanghui@huawei.com \
--cc=joro@8bytes.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=robin.murphy@arm.com \
--cc=song.bao.hua@hisilicon.com \
--cc=thunder.leizhen@huawei.com \
--cc=weiyongjun1@huawei.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 NNTP newsgroup(s).