Messages in this thread | | | Subject | Re: [PATCH -next] iommu/arm-smmu-v3: Add suspend and resume support | From | Bixuan Cui <> | Date | Thu, 22 Jul 2021 14:34:32 +0800 |
| |
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. >
| |