From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH] iommu/arm-smmu-v3: Implement shutdown method Date: Thu, 29 Jun 2017 19:34:39 +0100 Message-ID: <20170629183438.GF5592@arm.com> References: <1498758015-7072-1-git-send-email-nwatters@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1498758015-7072-1-git-send-email-nwatters-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Nate Watterson Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: iommu@lists.linux-foundation.org On Thu, Jun 29, 2017 at 01:40:15PM -0400, Nate Watterson wrote: > The shutdown method disables the SMMU and its interrupts to avoid > potentially corrupting a new kernel started with kexec. > > Signed-off-by: Nate Watterson > --- > drivers/iommu/arm-smmu-v3.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) We should update arm-smmu.c as well. > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 380969a..907d576 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -2765,9 +2765,19 @@ static int arm_smmu_device_remove(struct platform_device *pdev) > struct arm_smmu_device *smmu = platform_get_drvdata(pdev); > > arm_smmu_device_disable(smmu); > + > + /* Disable IRQs */ > + arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL, > + ARM_SMMU_IRQ_CTRLACK); > + Can you justify the need for this? If we actually need to disable interrupts, then I'd like to understand why so that we can make sure we get the ordering right with respect to disabling the device. Also, do we need to clear the MSI registers too? My understanding is that kexec will mask irqs at the GIC, so there's not actually an issue here. Will From mboxrd@z Thu Jan 1 00:00:00 1970 From: will.deacon@arm.com (Will Deacon) Date: Thu, 29 Jun 2017 19:34:39 +0100 Subject: [PATCH] iommu/arm-smmu-v3: Implement shutdown method In-Reply-To: <1498758015-7072-1-git-send-email-nwatters@codeaurora.org> References: <1498758015-7072-1-git-send-email-nwatters@codeaurora.org> Message-ID: <20170629183438.GF5592@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Jun 29, 2017 at 01:40:15PM -0400, Nate Watterson wrote: > The shutdown method disables the SMMU and its interrupts to avoid > potentially corrupting a new kernel started with kexec. > > Signed-off-by: Nate Watterson > --- > drivers/iommu/arm-smmu-v3.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) We should update arm-smmu.c as well. > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 380969a..907d576 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -2765,9 +2765,19 @@ static int arm_smmu_device_remove(struct platform_device *pdev) > struct arm_smmu_device *smmu = platform_get_drvdata(pdev); > > arm_smmu_device_disable(smmu); > + > + /* Disable IRQs */ > + arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL, > + ARM_SMMU_IRQ_CTRLACK); > + Can you justify the need for this? If we actually need to disable interrupts, then I'd like to understand why so that we can make sure we get the ordering right with respect to disabling the device. Also, do we need to clear the MSI registers too? My understanding is that kexec will mask irqs at the GIC, so there's not actually an issue here. Will From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753309AbdF2Seq (ORCPT ); Thu, 29 Jun 2017 14:34:46 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:59676 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752601AbdF2Sej (ORCPT ); Thu, 29 Jun 2017 14:34:39 -0400 Date: Thu, 29 Jun 2017 19:34:39 +0100 From: Will Deacon To: Nate Watterson Cc: Robin Murphy , Joerg Roedel , linux-arm-kernel@lists.infradead.org, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] iommu/arm-smmu-v3: Implement shutdown method Message-ID: <20170629183438.GF5592@arm.com> References: <1498758015-7072-1-git-send-email-nwatters@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1498758015-7072-1-git-send-email-nwatters@codeaurora.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 29, 2017 at 01:40:15PM -0400, Nate Watterson wrote: > The shutdown method disables the SMMU and its interrupts to avoid > potentially corrupting a new kernel started with kexec. > > Signed-off-by: Nate Watterson > --- > drivers/iommu/arm-smmu-v3.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) We should update arm-smmu.c as well. > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 380969a..907d576 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -2765,9 +2765,19 @@ static int arm_smmu_device_remove(struct platform_device *pdev) > struct arm_smmu_device *smmu = platform_get_drvdata(pdev); > > arm_smmu_device_disable(smmu); > + > + /* Disable IRQs */ > + arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL, > + ARM_SMMU_IRQ_CTRLACK); > + Can you justify the need for this? If we actually need to disable interrupts, then I'd like to understand why so that we can make sure we get the ordering right with respect to disabling the device. Also, do we need to clear the MSI registers too? My understanding is that kexec will mask irqs at the GIC, so there's not actually an issue here. Will