Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
References: <cover.de78568883814904b78add6317c263bf5bc20234.1689768831.git-series.apopple@nvidia.com>
 <8f293bb51a423afa71ddc3ba46e9f323ee9ffbc7.1689768831.git-series.apopple@nvidia.com>
 <87y1j4y7w8.fsf@mail.lhotse>
User-agent: mu4e 1.8.13; emacs 28.2
From:   Alistair Popple <apopple@nvidia.com>
To:     Michael Ellerman <mpe@ellerman.id.au>
Cc:     akpm@linux-foundation.org, ajd@linux.ibm.com,
        catalin.marinas@arm.com, fbarrat@linux.ibm.com,
        iommu@lists.linux.dev, jgg@ziepe.ca, jhubbard@nvidia.com,
        kevin.tian@intel.com, kvm@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,
        nicolinc@nvidia.com, npiggin@gmail.com, robin.murphy@arm.com,
        seanjc@google.com, will@kernel.org, x86@kernel.org,
        zhi.wang.linux@gmail.com
Subject: Re: [PATCH v2 3/5] mmu_notifiers: Call invalidate_range() when
 invalidating TLBs
Date:   Tue, 25 Jul 2023 15:51:45 +1000
In-reply-to: <87y1j4y7w8.fsf@mail.lhotse>
Message-ID: <871qgwzgfa.fsf@nvdebian.thelocal>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?umweCAJ7PqN6IM+z9GCVFDt2lQL1rTjIWH3TFF18HB9hlyw13wmZLuySyzP2?=
 =?us-ascii?Q?WD3vot6u5vg/tbdw+wmhMPNG/zvv5UlJbYRPFPagLE+OA8m8ncL1pn9MCvBw?=
 =?us-ascii?Q?zkN6iFOJUHDXESsiRo79cVpWkKVKSv9mB9UuP1bQgfFl0/H9hAjISdZS4cdG?=
 =?us-ascii?Q?cgrc6hAtqGaEGoHzsCGFzf0LyPqWbi3DBHLx7DDdyhQA/7cNJxIPBnec7+N/?=
 =?us-ascii?Q?KmD5MdaW839YsBVDUbcS4hgYWlQd6opFjby1vrMqXhYg/22qcA6fZzxcH5QN?=
 =?us-ascii?Q?Ze6pQ1ajErAlrnSNwZ1bxNt7jRYbx5SDQynxLTPa/MSAuxJQhoaribfGbgly?=
 =?us-ascii?Q?ugpmN+5e16W0AFfYaeNh5v1TuPuH33tvq9H/7yfh+4ABMQNGRrD7HYgXTB4L?=
 =?us-ascii?Q?iLmeplwLZTOPgx3713arCQ6w4MIMoJ5YqATklXgzGcMevrMQgL0liYBIXd72?=
 =?us-ascii?Q?VvVevTVC+uMJ+DqTB2Zk2qYLLFO2eRdBmLVPeQGBFoEvkKdBdNgNJtX2dUNK?=
 =?us-ascii?Q?W2QmEyQutNG4z0rbNf56Buip6rbP0y68V4jecjkR9ENjtjgR7LLJTOLzggBf?=
 =?us-ascii?Q?LsWKk8v5xGsSXGXCqWvvinKGot+ipXBheifVZV47uK94N4WIHld6RsriII+1?=
 =?us-ascii?Q?aDQ3sJfT9X317dkcjQ/ovEU3+gJoJkulKIp/MfMy7QRhChBdV9glfG8YxTS0?=
 =?us-ascii?Q?ICGoGtOSeoi2X8Yydf/8RY7wQfY+fec6I5IgD4yzv2XkCoDrrt7EtuKI81FE?=
 =?us-ascii?Q?nQGTnVThtXNmYGskIzBDP7k7W38nJMcseznOaDBMyuISakUNilEN0rSmVkIS?=
 =?us-ascii?Q?QMxoFvUsa223Zu63f9aDemtDA+WIWfvDbkeCmK0Jh9MFQl5D/YBYNO5gPzhI?=
 =?us-ascii?Q?xqi0pBqVwmO02y+uFLGZPYAip0g6mGy70MtQRBc9JWxQpg++ZDGv90ph5CJy?=
 =?us-ascii?Q?KT1PWeAxglNnUf4WW3KlhVrJ5nBt4b6so0rkJmDiJiUQWS/N3hGgadsuedeq?=
 =?us-ascii?Q?XG0dcOcczDIpGCrriiPdtGITmiPTkINNHoyMA0/DqAZZe1uSmxIzpVvhohgp?=
 =?us-ascii?Q?ajlyfb4vpHDX+mlsbAGylfSlO9pDXFv+VUPeVDmrq6rTXWHVavNjc5EE4F6l?=
 =?us-ascii?Q?jtfgCC0FRMIjw5ZwrP1GiU6XWEmr49GSqF60ChOPcxLMLRmvuUoBtfyizlsy?=
 =?us-ascii?Q?NTAArwjq+2ytgafh/T3PudMmrMoisdB9+/0vWLNT7Ui8FIqLA31MEdPfE7MO?=
 =?us-ascii?Q?OyCCl3kviAJ5iiJ+fa5puoFJO7wwUPaBXacvup++71x2EHRfXXKVaFCsHokr?=
 =?us-ascii?Q?f3NEflSTjIybw8baBxqn5Gtneg1wS6WwinizZYDOExcyCHxV3qOwExlu9WBY?=
 =?us-ascii?Q?pxykdscqwbxGPcv7ilkcJqPv28VSEM/yTakJhAAAo7rRYOaVcZieZHsO/qn7?=
 =?us-ascii?Q?8yK/kNOqIbI16fKARD9rI97Klv7bn1H/6+i6UtbyZDJcDkwg24LAdtCIupIa?=
 =?us-ascii?Q?8W6Y1Jq80vdqKj+WNsJVuP2Hne5Cja4Od/LVH487AQkORTqZIjUPcSCjivCS?=
 =?us-ascii?Q?WpqdL21itfhKd5pl58/LRaFb+lkGTTm+nhXFce8p?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6872f7de-13c4-45dc-8cf4-08db8cd35683
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jul 2023 05:52:32.2949
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: B56+fTPImOKdKhpcWTtAhSNZX5zycA+alLFiSTYno1+gVWV5tDK6TSqLkYuH0XPZYLNzAphcpk93en6+ETFpDw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB8553
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Michael Ellerman <mpe@ellerman.id.au> writes:

> Alistair Popple <apopple@nvidia.com> writes:
>> The invalidate_range() is going to become an architecture specific mmu
>> notifier used to keep the TLB of secondary MMUs such as an IOMMU in
>> sync with the CPU page tables. Currently it is called from separate
>> code paths to the main CPU TLB invalidations. This can lead to a
>> secondary TLB not getting invalidated when required and makes it hard
>> to reason about when exactly the secondary TLB is invalidated.
>>
>> To fix this move the notifier call to the architecture specific TLB
>> maintenance functions for architectures that have secondary MMUs
>> requiring explicit software invalidations.
>>
>> This fixes a SMMU bug on ARM64. On ARM64 PTE permission upgrades
>> require a TLB invalidation. This invalidation is done by the
>> arahitecutre specific ptep_set_access_flags() which calls
>   ^
>   architecture

Oh. I'd forgotten to apt install codespell ;-)
  
>> flush_tlb_page() if required. However this doesn't call the notifier
>> resulting in infinite faults being generated by devices using the SMMU
>> if it has previously cached a read-only PTE in it's TLB.
>>
>> Moving the invalidations into the TLB invalidation functions ensures
>> all invalidations happen at the same time as the CPU invalidation. The
>> architecture specific flush_tlb_all() routines do not call the
>> notifier as none of the IOMMUs require this.
>>
>> Signed-off-by: Alistair Popple <apopple@nvidia.com>
>> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
>> 
> ...
>
>> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
>> index 0bd4866..9724b26 100644
>> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
>> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
>> @@ -752,6 +752,8 @@ void radix__local_flush_tlb_page(struct vm_area_struct *vma, unsigned long vmadd
>>  		return radix__local_flush_hugetlb_page(vma, vmaddr);
>>  #endif
>>  	radix__local_flush_tlb_page_psize(vma->vm_mm, vmaddr, mmu_virtual_psize);
>> +	mmu_notifier_invalidate_range(vma->vm_mm, vmaddr,
>> +						vmaddr + mmu_virtual_psize);
>>  }
>>  EXPORT_SYMBOL(radix__local_flush_tlb_page);
>
> I think we can skip calling the notifier there? It's explicitly a local flush.

I suspect you're correct. It's been a while since I last worked on PPC
TLB invalidation code though and it's changed a fair bit since then so
was being conservative and appreciate any comments there. Was worried I
may have missed some clever optimisation that detects a local flush is
all that's needed, but I see OCXL calls mm_context_add_copro() though so
that should be ok. Will respin and drop it.

> cheers