On 2021/7/21 23:01, Marc Zyngier wrote:
> On Wed, 21 Jul 2021 14:59:47 +0100,
> Robin Murphy <robin.mur...@arm.com> wrote:
>>
>> On 2021-07-21 14:12, Marc Zyngier wrote:
>>> On Wed, 21 Jul 2021 12:42:14 +0100,
>>> Robin Murphy <robin.mur...@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 <cuibix...@huawei.com>
>>>>> Reviewed-by: Wei Yongjun <weiyongj...@huawei.com>
>>>>> Reviewed-by: Zhen Lei <thunder.leiz...@huawei.com>
>>>>> Reviewed-by: Ding Tianhong <dingtianh...@huawei.com>
>>>>> Reviewed-by: Hanjun Guo <guohan...@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.
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu