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: <20230522063725.284686-1-apopple@nvidia.com>
 <cf2d7d64-8872-2b22-0b2e-8db96135d3ef@nvidia.com>
User-agent: mu4e 1.8.10; emacs 28.2
From:   Alistair Popple <apopple@nvidia.com>
To:     John Hubbard <jhubbard@nvidia.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, robin.murphy@arm.com,
        will@kernel.org, nicolinc@nvidia.com,
        linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
        kvmarm@lists.cs.columbia.edu, jgg@nvidia.com
Subject: Re: [PATCH] mmu_notifiers: Notify on pte permission upgrades
Date:   Tue, 23 May 2023 11:12:50 +1000
In-reply-to: <cf2d7d64-8872-2b22-0b2e-8db96135d3ef@nvidia.com>
Message-ID: <87jzwz97cv.fsf@nvidia.com>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?C2Gsbiza1Vpk9lMBQIfOsAxuAeqiEoB/XztK+EbHMbnMXugEheT9RWWOGlk3?=
 =?us-ascii?Q?CjzbJGAQNaalC0eGiq7CM+50CJTaIPVdkQwpNvCz1pV8n8ipjBPGlsOX5Tng?=
 =?us-ascii?Q?xVlc53uKV0GJiApH0+3gK3xszQIdedV7YLWe39ETXeE9ibj61xDzXx4zysEc?=
 =?us-ascii?Q?TdO3yYcStva5q0LzdpcotjpNj+B55iQ28MjOY4P5cP2zA/LOImjVMHttW9GV?=
 =?us-ascii?Q?1Hnd0+Qx75ROMJ6AFTLcyQhIXjZLtsdIvV6HAbz1PkGlm5MJCIiVPnHv97ZZ?=
 =?us-ascii?Q?bcS34mdr3WSfHrCva1htIfq82guRaDvYN7DzA2GhFexHQfJRPtNvMUwmvmlI?=
 =?us-ascii?Q?NeMNzIs1kqNTtlGNRjCQiMjcTcETLIOTX7GDpHiY36BUxVwdTww+yCyD9Do3?=
 =?us-ascii?Q?iaiQMJHjzgirasgLO6EDiWrRf8JI91KSEaR/+AlewXMP00RISU9zFqobXnJM?=
 =?us-ascii?Q?yaCR+L2FW4V0NvRZiRhgkyvV45gOEC313neU7XdYvXRk0ywhNarJvsquRXK3?=
 =?us-ascii?Q?+z+miRXcMdS+V8j0+8y3W15mKwuF9XHmMlo03vB8Cqa2V6LMg1kFv3x+aQU7?=
 =?us-ascii?Q?oCJvbAmkFU9cLjqW+ZEq7R/+HP2V/cZK6rMJV6WOwun/PWIDgI23At6YZ46l?=
 =?us-ascii?Q?RZsIunmMXeEQsbPReXFZIVBnmEpflkE9hz2Hc+YtDQcVlAdzuOTmTC5Hgjwn?=
 =?us-ascii?Q?QMpYhTkxsbBfreg7wZ8sSfq2lsSx084nTWEkcXZpMFqJa8WUX8zwpldpL43Q?=
 =?us-ascii?Q?hO3RGqv/tj8/dES06U6T34lnf0neF1ZOUg3OtqEfoxEHqwKZ9dbP/Y9SuAMo?=
 =?us-ascii?Q?MMpnWzSM4BLdYExGzIAETXcJ2DcPmDCfFYfjzXGkuqxLLkO6OlEZVODRKcXE?=
 =?us-ascii?Q?We851HQzcjoxdYBHPpVitRckQlxARWdQEdGjB9OT/lmg1XBkEqVwOSJ9UkWo?=
 =?us-ascii?Q?EQQ3a3Oh/NuCaqKfW69xfcYB9zFsBtMvO+6xQI2R4ABvPejo1qkxHw7EUxHo?=
 =?us-ascii?Q?6uSaf4Xf9BFifs7OJDFWULRimFjwIrSDuIdTmKOK2vxjaGCipniYIOgI6q1T?=
 =?us-ascii?Q?moxH55kyXzZ5BAPqPyJ4kl53AXzRZL4dWy1zOQQSkOR58hq8Qmjxp4YX3xR9?=
 =?us-ascii?Q?AM1KXhUhCVrz0KeaFR//xtViuWsJls5k82smOWPr9biSeghlxXR9UVtjSKoo?=
 =?us-ascii?Q?xA/B+TuweSoORSmJWsmzzIQmsQlvddh3g1Wp7Ay5Cf/eeGrF2f7fD7XpfacD?=
 =?us-ascii?Q?3D6MUrOgocT9xjdO876z3MnOyr2BwnNSFg6/s8IHDWbRKnTc7GE0wL8XlQ45?=
 =?us-ascii?Q?GHCn2Up3RqiyXgKeO0PTXZb93mU5POlW1pSpRYWGP16IEif/9Hn2Z3TFhTUe?=
 =?us-ascii?Q?YVmzZ/pdrEs9yso4KlbbzLUxxq8xPyWNmfu2LTi8QSV0Bg7N1mAdJJjj9U34?=
 =?us-ascii?Q?XvuxfnwKuCxpvdZ6qJ5GaQwuZFPnifzTiWIT22QndNAEIaexXet5KeTZpjsQ?=
 =?us-ascii?Q?WtkpiNoVxO5D0y3QOgGeoFYyx8B3xH8BFtPkgPMOke2sM4dZ74aE7IW6JmNt?=
 =?us-ascii?Q?jzOSn1uEEDP+UkZbYZUjWydkeapM87bug2pId0GT?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0fb855b3-5bd3-40c0-05e2-08db5b2b175d
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2023 01:14:43.8785
 (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: BcPd0IaqjPtcZ43rlG1qoJDHk1IzI6KhnMnYQROBhpx/td8hIZBkdZWIty/iA0kttgVa3kyegTvLJOPj5mCMjw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5764
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_NONE,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


John Hubbard <jhubbard@nvidia.com> writes:

> On 5/21/23 23:37, Alistair Popple wrote:
>> Some architectures, specifically ARM and perhaps Sparc and IA64,
>> require TLB invalidates when upgrading pte permission from read-only
>> to read-write.
>> The current mmu_notifier implementation assumes that upgrades do not
>> need notifications. Typically though mmu_notifiers are used to
>> implement TLB invalidations for secondary MMUs that comply with the
>> main CPU architecture.
>> Therefore if the main CPU architecture requires an invalidation for
>> permission upgrade the secondary MMU will as well and an mmu_notifier
>> should be sent for the upgrade.
>> Currently CPU invalidations for permission upgrade occur in
>> ptep_set_access_flags(). Unfortunately MMU notifiers cannot be called
>> directly from this architecture specific code as the notifier
>> callbacks can sleep, and ptep_set_access_flags() is usually called
>> whilst holding the PTL spinlock. Therefore add the notifier calls
>> after the PTL is dropped and only if the PTE actually changed. This
>> will allow secondary MMUs to obtain an updated PTE with appropriate
>> permissions.
>> This problem was discovered during testing of an ARM SMMU
>> implementation that does not support broadcast TLB maintenance
>> (BTM). In this case the SMMU driver uses notifiers to issue TLB
>> invalidates. For read-only to read-write pte upgrades the SMMU
>> continually returned a read-only PTE to the device, even though the
>> CPU had a read-write PTE installed.
>> Sending a mmu notifier event to the SMMU driver fixes the problem by
>> flushing secondary TLB entries. A new notifier event type is added so
>> drivers may filter out these invalidations if not required. Note a
>> driver should never upgrade or install a PTE in response to this mmu
>> notifier event as it is not synchronised against other PTE operations.
>> Signed-off-by: Alistair Popple <apopple@nvidia.com>
>> ---
>>   include/linux/mmu_notifier.h |  6 +++++
>>   mm/hugetlb.c                 | 24 ++++++++++++++++++-
>>   mm/memory.c                  | 45 ++++++++++++++++++++++++++++++++++--
>>   3 files changed, 72 insertions(+), 3 deletions(-)
>
> Point of order:
>
> What is this based on? It would really help if you would either use --base
> with git-format-patch, or else just mention the base somewhere. Otherwise,
> actually applying the patch takes some hunting around...in particular for
> older stuff. This is from Feb, 2023, before hugetlb.c got converted to
> folios, right?

Yes, my bad. This is based on v6.2. I had meant to rebase it prior to
sending but forgot. Based on the other review comments though I will be
reworking this to put the notifier call in ptep_set_access_flags() on
Arm anyway.

> thanks,