Received-SPF: pass (google.com: domain of linux-kernel+bounces-188066-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <d18611c7-9108-46f7-a5a5-6c8e0069de9b@os.amperecomputing.com>
Date: Thu, 23 May 2024 15:13:23 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [v2 PATCH] arm64: mm: force write fault for atomic RMW
 instructions
To: Catalin Marinas <catalin.marinas@arm.com>,
 "Christoph Lameter (Ampere)" <cl@linux.com>
Cc: will@kernel.org, anshuman.khandual@arm.com, scott@os.amperecomputing.com,
 linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20240520165636.802268-1-yang@os.amperecomputing.com>
 <Zk93vBqAD3LgmbGb@arm.com> <640f8606-2757-4e82-721f-9625d48ded65@gentwo.org>
 <Zk-SNVyEHT1UsxqD@arm.com> <ad87bb77-a9a5-2c0d-b4b2-13db09615d7c@linux.com>
 <Zk-2a7s2pvkVsm2C@arm.com>
Content-Language: en-US
From: Yang Shi <yang@os.amperecomputing.com>
In-Reply-To: <Zk-2a7s2pvkVsm2C@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?c1daM3dFRUcxTlVaT1VXcENlTE9kWFk5TWNaNFlCKzJZamZFVWxGSHNVVjJz?=
 =?utf-8?B?SDBRSDBVZ2hZS0tDMUJMUnlCaHRiQWtreTdxZkNSUmRaM1RkOWF6aGR5alhD?=
 =?utf-8?B?UUVONFU4QlloUU1rSlhhc0tOWVV1VEhoVDhzVFprbkkzTTJqL0VLOTdZUGZ2?=
 =?utf-8?B?Z0k1azg3QXVPSE14WlhRV0dXN0NYM3NScDJmMjcxazJSUnlZNE9aQkxQbE9t?=
 =?utf-8?B?dkQyNVQ1QVNHZndtYlltWHNtcm9Odm1LcVVIKzMyU1ZsRmNseXlRVFNORFJE?=
 =?utf-8?B?K3gzU1o2SEJyejhQZFEvMC9GUVBqZ0dCLzVOM1UxVWpHVHE0MjFURnAzOHpN?=
 =?utf-8?B?SzdIRnk2MHFhcDRYdFlzNFZLS0xpVkY5VFozV3pIeFlpOGVUazJ5ZkdwQTJx?=
 =?utf-8?B?eUYzWUNQdkthbzVyZ1QyNHZRRzNIbEdHYUdNUWRRcUZMN3pWaXh2ZTRWOVk3?=
 =?utf-8?B?cmZKeWJPdGovVDZ0RTNGT1ZOSVArK1FLQTBtT2tjSzBpVklZZFB6Ky81TFh4?=
 =?utf-8?B?dXNFU2greUh6bkRVcGZTcmE2Ym9MTTZzSktoTkxrSGx4MWpIQTFScVVmZFov?=
 =?utf-8?B?WnJRU1lGUjBVZHhHQ2dWNExKanMxUmd5WitZenJaRk1mV0ZHZHVpU0dSazQ2?=
 =?utf-8?B?OUJtYXNjRys0ditsTlNVV0NPYUJrWHg3UCtobUpOM3p0dnlUOTF0NDU4SjJI?=
 =?utf-8?B?L2ZXdGVJZHppQVpSYVNMbHlKN21Nb1ptMVFzTGYwNThXMTdzcmpCMG1melp1?=
 =?utf-8?B?ZXREeVA5NDg0WGtzZjJGTXlWTllJalVlSzVEZEU5cFJ3MlEwTVhSUEhwUVpX?=
 =?utf-8?B?N3NXQ1h5Sy84cStQTERmODFudkV6aUdNa2pOZmlMWWluMVJQTUg2eHdPVmhY?=
 =?utf-8?B?bys3TkNkT2JzTW5WZFQxaDlUVm1DRHhhUXM0ajh1b0xBMjZTNlF1UDZmUk51?=
 =?utf-8?B?eGQ1WGxkZkl6R1Rzd1d1c2t6Z3MyYlBiRDNDQnBpdThPa0ZQY0xUcFBJWEQ3?=
 =?utf-8?B?cUpnS3N3a1lKS3UwcklsZEE4a2VzM2VEQzNrMytUUStXVER4UlYwTWhKeDNs?=
 =?utf-8?B?WXMrb2N4TnlwZFN1SFdFL3lGRFVEZ0lBTW9RVmdDMys4L3JPYWlDUkQzRHJL?=
 =?utf-8?B?THBoL2dSSXJUcTFFbXU4QTZqa2lHWVNkVVJkWjdZR1BMSjhwSVJid0pzanJP?=
 =?utf-8?B?SkhaK0ZtVFovYXhobzd3VDgwSEltS05LK245dGtjNUJFRVhXejBSZ1pnR0ps?=
 =?utf-8?B?b2FsQ3ljRWFrWWJ3UDMwd3FhWjByODhRSGt6eTJiQUovMEN2cXRibXFQZWlE?=
 =?utf-8?B?S3F4dGhTaWI1SStyWXkrUEs4SnY1NjAyems2UjVkTGlwL0FwUHF5NE9uWXJH?=
 =?utf-8?B?RVZ2cGs3eGRFOGEyNUdha0pGeW9MTTdWdlpMTTA2OHQzT0VBVTF2THFKZ3kz?=
 =?utf-8?B?Nkc4cDhVZ0QvQi9YWGVIU3dRU2xKQ0pEQmpjdGJRMEo1cHRXcDhoS0R3aEdJ?=
 =?utf-8?B?ZGsvWXlwSGxyd01rSHhYeVhoVXYzL3VaRVU0c2p4Z0RZUVZuS2ZrWlZGV3BH?=
 =?utf-8?B?K2VKVWRlRW1zZ1ZJSHllOExaeFd6dFZLWW9PbHZBYXdzUTdSUVlrbTE5VGc3?=
 =?utf-8?B?RkQyc1VNZDUzS3dzMjJ4d1VvbTVtRS9JM2NYakswV1l1bUhJbjNScFZSRDl6?=
 =?utf-8?B?N2Mvb2RUU3hGRG13cFJMSGFZQnpWY25telVJS0ROQW45eStmS0RLVE9mSkJn?=
 =?utf-8?B?d25xVC84SXJ2b0FWd01MZkRZbmlnYnk0cDJ3ekMyNWFDZ3UxQWgwaFM3c3pQ?=
 =?utf-8?B?TWg1VHpubnd5bDFKNmM1Q1E0VEpBNUpLVC9MSlhvb29aMkF0TXl5aElEQTRF?=
 =?utf-8?B?T3B6dFcyZnFsZDVRV0E0QVhuQWNQbGNjRUNiUUlwcTI0YnRFRktpdHEvUDMz?=
 =?utf-8?B?bUxIY1VMQXZCbGRoQVgzSzh4dnQ1RHBuK2xNSk91SXVaMHBwOWUwQlhlTTlS?=
 =?utf-8?B?NVNYZ2VsSEZwazBkSjMvQXNudU0zek9SNC9hTC95NWlhVEMzSjNqZk96cTU1?=
 =?utf-8?B?cHdiNzc1ZTlsc2kvT0xFckJ5dFlPN0pTNHZVQnZCcTJ1dTYybGd4MHpEVUVi?=
 =?utf-8?B?N3JUNzdUdmswYnowWlVIQ3FHa2crNE12cmRmenFSTVB0Z0dBUUFvRDA0eEF5?=
 =?utf-8?Q?RWB3U1nS1ngp3w5gUQFOOVw=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ae1baf25-9dbe-481f-c171-08dc7b75925b
X-MS-Exchange-CrossTenant-AuthSource: BYAPR01MB5463.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 22:13:27.7687
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: YPTeY9YHIJDKfXR5r9o9jAbRP9yNYAJmBcGvTkoeSU5uZOa0LtXb0C+TA9DrDvYe/l+g+YVD7EIhA/q3j37mX+qkrLrZztcSykidcM63dZM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR01MB8502


On 5/23/24 2:34 PM, Catalin Marinas wrote:
> On Thu, May 23, 2024 at 12:43:34PM -0700, Christoph Lameter (Ampere) wrote:
>> On Thu, 23 May 2024, Catalin Marinas wrote:
>>>>> While this class includes all atomics that currently require write
>>>>> permission, there's some unallocated space in this range and we don't
>>>>> know what future architecture versions may introduce. Unfortunately we
>>>>> need to check each individual atomic op in this class (not sure what the
>>>>> overhead will be).
>>>> Can you tell us which bits or pattern is not allocated? Maybe we can exclude
>>>> that from the pattern.
>>> Yes, it may be easier to exclude those patterns. See the Arm ARM K.a
>>> section C4.1.94.29 (page 791).
>> Hmmm. We could consult an exception table once the pattern matches to reduce
>> the overhead.
> Yeah, check the atomic class first and then go into the finer-grained
> details. I think this would reduce the overhead for non-atomic
> instructions.

If I read the instruction encoding correctly, the unallocated 
instructions are decided by the below fields:

   - size
   - VAR
   - o3
   - opc

To exclude them I think we can do something like:

if atomic instructions {
     if V == 1
         return false;
     if o3 opc == 111x
         return false;
     switch VAR {
         000
             check o3 and opc
         001
             check 03 and opc
         010
             check o3 and opc
         011
             check o3 and opc
         default
             if size != 11
                 check o3 and opc
     }
}

So it may take 4 + the possible unallocated combos of o3 and opc 
branches for the worst case. I saw 5 different combos for o3 and opc, so 
9 branches for worst cases.

>
>> However, the harm done I think is acceptable even if we leave things as is.
>> In the worst case we create unnecesssary write fault processing for an
>> "atomic op" that does not need write access. Also: Why would it need to be
>> atomic if it does not write???
> I'm thinking of some conditional instruction that states no write if
> condition fails. But it could be even worse if the architects decide to
> reuse that unallocated space for some instructions that have nothing to
> do with the atomic accesses.

Even though the condition fails, forcing write fault still seems fine 
IIUC. I'm supposed the read fault will happen regardless of the 
condition. Then a page with all 0 content is installed. This is 
guaranteed. We just end up having write permission instead of read-only 
permission. We will also be in this state transiently with current 
supported atomic instructions.

But if they will be allocated to non-atomic instructions, we have to do 
fine-grained decoding, but it may be easier since we can just filter out 
those non-atomic instructions? Anyway it depends on how they will be 
used. Hopefully this won't happen.

>
> It's something we need to clarify with them but I'm about to go on
> holiday for a week, so I won't be able to check.

Have a good holiday.

>
>> The ultimate solution would be to change the spec so that arm processors can
>> skip useless read faults.
> I raised this already, waiting for feedback from the architects.

Thank you so much.

>