lkml.org 
[lkml]   [2022]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] iommu/arm-smmu-v3: fixed check process for disable_bypass module parameter
Date
Hi, Robin,
Thank you for your comments.

> > /* Enable the SMMU interface, or ensure bypass */
> > - if (!bypass || disable_bypass) {
> > + if (!bypass && disable_bypass) {
>
> This change looks obviously wrong - if bypass is false here then we definitely
> want to enable the SMMU, so disable_bypass is irrelevant. It shouldn't even be
> possible to get here with bypass==true under ACPI, since
> arm_smmu_device_acpi_probe() cannot fail :/

Sorry, my understanding of the meaning of the disable_bypass module parameter
and the process of setting GBPA_ABORT was insufficient.

I misunderstood that the disable_bypass module parameter is used to simply
bypass (disable) SMMU (SMMU_CR0.SMMUEN == 0 and SMMU_GBPA.ABORT == 0).
Forget about the fixes in this patch.

Although our understanding was lacking,
we thought it would be a good idea to have a module parameter that simply disables SMMU,
so we were considering a fix.

Best regards,
Shuuichirou.

> -----Original Message-----
> From: Robin Murphy <robin.murphy@arm.com>
> Sent: Wednesday, August 3, 2022 6:26 PM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>;
> will@kernel.org; joro@8bytes.org; thunder.leizhen@huawei.com; jgg@ziepe.ca;
> tglx@linutronix.de; chenxiang66@hisilicon.com; christophe.jaillet@wanadoo.fr;
> john.garry@huawei.com; baolu.lu@linux.intel.com;
> linux-arm-kernel@lists.infradead.org; iommu@lists.linux.dev;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] iommu/arm-smmu-v3: fixed check process for
> disable_bypass module parameter
>
> On 2022-08-03 00:42, Shuuichirou Ishii wrote:
> > The current process does not enable the bypass setting regardless of
> > the value of the disable_bypass module parameter when ACPI is enabled,
> > so the value of the disable_bypass module parameter has been corrected
> > so that it is handled correctly.
> >
> > Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> > ---
> > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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 88817a3376ef..256d7b2a83a7 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -3396,7 +3396,7 @@ static int arm_smmu_device_reset(struct
> arm_smmu_device *smmu, bool bypass)
> > enables &= ~(CR0_EVTQEN | CR0_PRIQEN);
> >
> > /* Enable the SMMU interface, or ensure bypass */
> > - if (!bypass || disable_bypass) {
> > + if (!bypass && disable_bypass) {
>
> This change looks obviously wrong - if bypass is false here then we definitely
> want to enable the SMMU, so disable_bypass is irrelevant. It shouldn't even be
> possible to get here with bypass==true under ACPI, since
> arm_smmu_device_acpi_probe() cannot fail :/
> Robin.
>
> > enables |= CR0_SMMUEN;
> > } else {
> > ret = arm_smmu_update_gbpa(smmu, 0, GBPA_ABORT);
\
 
 \ /
  Last update: 2022-08-03 14:47    [W:0.118 / U:2.688 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site