Received-SPF: pass (google.com: domain of linux-kernel+bounces-54948-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <252d6094-b2d6-496d-b28f-93507a193ede@nvidia.com>
Date: Tue, 6 Feb 2024 12:28:27 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [Patch] memory: tegra: Skip SID override from Guest VM
Content-Language: en-US
To: Marc Zyngier <maz@kernel.org>, Sumit Gupta <sumitg@nvidia.com>
Cc: treding@nvidia.com, krzysztof.kozlowski@linaro.org, mark.rutland@arm.com,
 linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org,
 amhetre@nvidia.com, bbasu@nvidia.com
References: <20240206114852.8472-1-sumitg@nvidia.com>
 <86wmrh6b2n.wl-maz@kernel.org>
From: Jon Hunter <jonathanh@nvidia.com>
In-Reply-To: <86wmrh6b2n.wl-maz@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?SW1FZmY0dGoxdTZjc2FxdndmRUdCV2FOWFVlZ2xJWXVKYUJEWXRKbm42LzJv?=
 =?utf-8?B?SWZoeG9oNXRLTDhPZTd0MHN6b1EvNGJJYzNRVm5mR0JsM0VvUEh5WGFKTkh2?=
 =?utf-8?B?bkV3b1hIYURjMnd1ZHdkYTllYmxxbXVGK2JTWEhFNE5EUC9sZWIvWVNYVHp0?=
 =?utf-8?B?OEorUDAveFdPUlA4ZU1aYjhZWmU0SkxGWWozYXc0b1FCMFpIQmlJS3dNT011?=
 =?utf-8?B?SnpwQlBDNEE2RzlRbnh4cFNyY0ErdUhwdVFnbGk1cnZqZ1lNQ0NGdDZMbitB?=
 =?utf-8?B?Rkc3SE01TXcrTFdLU012Ry9vK1hlUERoODdISGE5RFlPZFBLQTlhUGN5MFU0?=
 =?utf-8?B?WmgvZVlqSHRSN1J1VXN6aSsvOVN6alJzWUxRQ1NjSjkzZy8rdGM0ZWR3bW5P?=
 =?utf-8?B?SnY4ZnFKRVpEZHV0YWVFNkJYRmdTQ0FzY2xzaGFTdG1wTXFxOGFwWHY0dkp0?=
 =?utf-8?B?QTE1YlVoTlZGYnZlVVVSSnpCRmdvUFV6K05QYWNtbTZLOXNhVXh6OTBySHlu?=
 =?utf-8?B?RXlOcFhDa0VWcC96OVhhMTc3bnhLbFB1aGFIdytkMk8vaUhVekt5ak9JWjBM?=
 =?utf-8?B?V3VMRWtENGtxUUxMODlvaUJBbWpSTFFMWFdmZ1BZMGxDVk1McHF5SlFjdnBj?=
 =?utf-8?B?T1VvbjJLWU96Z0NGNE1tT1luNWlYNnRWZGpIYTV5S3dYczZuUWMwV0phVGpm?=
 =?utf-8?B?ZTlGL0d6K1VRRzRWbVE4K3laRDQ3QWlQbXN0bUpVWTIrZ2FJbUZpMkloK09K?=
 =?utf-8?B?L25JZXk5amxTSmZnamxLaVkxS2RpdWNGeEhpMDBXSXM5SnpranB2ZkpvYThz?=
 =?utf-8?B?aUxTNG44ejE2QmlLS1B6VVlDZnpZTkE2R2hJSjZoaGpraXlnWEZYQ3U3c2J3?=
 =?utf-8?B?dFA3TkI5OUVwWnduU0hMOUdwZHM5Q1drWVV1WDUvWXlHNU03WXZ4Vkc3MDdv?=
 =?utf-8?B?WU9EWHVORkxZclMzcmVwVFpjUkljOWI4ckY2aWt2b01WKzdPTEh3NDN4bGNB?=
 =?utf-8?B?WHRVd0NHMHg3U042UGxkV08zcFlMYkw2UHFDNkljM21kYXNvUGpKanJEcmgy?=
 =?utf-8?B?NldTcENjUHJvODNuZ2pocVlyd3ovOUQzcEdNc00rSW5NZW04bDhoTFB3MWNX?=
 =?utf-8?B?K0ZXRnp0ekVKYnBDQlp4aGVSRXlJd1FyeXVYTURWYU0vZUJkOGRjdzZRVTdG?=
 =?utf-8?B?aHdPREdyRzNQeUhWMHh0Q3p2OXV0RTU4T0krMFNRdGlnbEtBWGU1cDZVY2V2?=
 =?utf-8?B?eGpWKzBGZm0xTXVHQXB6K29kUGJjRnVnQnJnSDhMczlEOHBqdGhqc1NvSElI?=
 =?utf-8?B?cUFDMkJKQXVSakZZTG9FTlAydnMxY3htNlpkbmZ0QUhBMUZuTDZOcXBqdjY4?=
 =?utf-8?B?R1pIaW1kTVZuVnJmU21RK1FMM2xNMEdSOG81c2JTSnZpb1NXbngvRHE1OVdx?=
 =?utf-8?B?aVlpcktiMmVyRFl6djgxR3lWWERkbUlJMnlpdXlaRHRseEdncEFhNG5ia01U?=
 =?utf-8?B?T1RXRE1NOWxUYTFlNVE3YXRZNXlNaDdxamtGT0ZhTWFTSWlHZkh5ZUJhdnpO?=
 =?utf-8?B?anZnODU1blBiajFVcUF6aFVuc29LRis2UUN0Vm80RHJTdGhOM2p5VTJjUXpy?=
 =?utf-8?B?RzVSVWIwRkZZSkk3MTlNVWdOL2R2TEVxSEkzWm5XWElzUVNHNWZUTERUWHVX?=
 =?utf-8?B?YWpJY01ZRHJSVk5KN3hud3VxUVNVTXJrWWhXa2ZBeVFBbm5OOFoveUR1MUpL?=
 =?utf-8?B?VEdLM1lPWTN4RTBFVVQyVjdjbnc2UVZFK1RLTFlLQU51OFgyalVQRDNmc2F1?=
 =?utf-8?B?U04rUHRPcHJGNHl4R3FRY01kM2JjUUVaU3RDRFhPL256cHJPSURGOGZ2czZl?=
 =?utf-8?B?MUlja2d4cld2RUtHQnE4UFBROGlDMTNMend0TlhsK2x2dit0ME5iVlFMSFF1?=
 =?utf-8?B?ZHpKQ2s4UEdVTDVXcFMrT3VldWswV1VxbjBsMzF1bys1T0c0RVNEemI0bjlU?=
 =?utf-8?B?UDEvY0paak9KNXRxSiszZDFBQkpYcmZQeGpMNi9oSWQ1czAxdCtiMUlBUzg0?=
 =?utf-8?B?bnZCU3lBYmViYlpLcTJaSUJrSnNqWW5uL0xra2l3OEJYVllmblkzU3IzQmNz?=
 =?utf-8?Q?ies3CICjWQPytwJ9cqv4TcdLG?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ea6ddf0e-4417-49dc-2a79-08dc270f21cd
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5444.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Feb 2024 12:28:32.5901
 (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: T6z4upQ1aon0Bz5vQy4NJSPzjJcr56bHdQgIoOYx+U0U1rDxIEwFROJYjtgTlWChBcWbBArFTulIosB2N2DRWA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR12MB9437

Hi Marc,

On 06/02/2024 12:17, Marc Zyngier wrote:
> Hi Sumit,
> 
> On Tue, 06 Feb 2024 11:48:52 +0000,
> Sumit Gupta <sumitg@nvidia.com> wrote:
>>
>> MC SID register access is restricted for Guest VM.
>> So, skip the SID override programming from the Guest VM.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>   drivers/memory/tegra/tegra186.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>> index 1b3183951bfe..df441896b69d 100644
>> --- a/drivers/memory/tegra/tegra186.c
>> +++ b/drivers/memory/tegra/tegra186.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/of.h>
>>   #include <linux/of_platform.h>
>>   #include <linux/platform_device.h>
>> +#include <asm/virt.h>
>>   
>>   #include <soc/tegra/mc.h>
>>   
>> @@ -118,6 +119,11 @@ static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
>>   	unsigned int i, index = 0;
>>   	u32 sid;
>>   
>> +	if (!is_kernel_in_hyp_mode()) {
>> +		dev_dbg(mc->dev, "Register access not allowed\n");
>> +		return 0;
>> +	}
>> +
>>   	if (!tegra_dev_iommu_get_stream_id(dev, &sid))
>>   		return 0;
>>   
>> @@ -146,6 +152,11 @@ static int tegra186_mc_resume(struct tegra_mc *mc)
>>   #if IS_ENABLED(CONFIG_IOMMU_API)
>>   	unsigned int i;
>>   
>> +	if (!is_kernel_in_hyp_mode()) {
>> +		dev_dbg(mc->dev, "Register access not allowed\n");
>> +		return 0;
>> +	}
>> +
>>   	for (i = 0; i < mc->soc->num_clients; i++) {
>>   		const struct tegra_mc_client *client = &mc->soc->clients[i];
>>   
> 
> This doesn't look right. Multiple reasons:
> 
> - This helper really has nothing to do in a driver. This is
>    architectural stuff that is not intended for use outside of arch
>    core code.

We see a few other drivers using this ...

drivers/perf/arm_pmuv3.c
drivers/clocksource/arm_arch_timer.c
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
drivers/hwtracing/coresight/coresight-etm4x-core.c
drivers/hwtracing/coresight/coresight-etm4x-core.c
drivers/irqchip/irq-apple-aic.c

We were looking for a way to determine if the OS is a guest OS or not. 
However, I can see that this is a ARM64 specific API and so probably the 
above are only compiled for ARM64. Interestingly, the AMD driver 
implements the following ...

static inline bool is_virtual_machine(void)
{
#if defined(CONFIG_X86)
         return boot_cpu_has(X86_FEATURE_HYPERVISOR);
#elif defined(CONFIG_ARM64)
         return !is_kernel_in_hyp_mode();
#else
         return false;
#endif
}

> - My own tegra186 HW doesn't have VHE, since it is ARMv8.0, and this
>    helper will always return 'false'. How could this result in
>    something that still works? Can I get a free CPU upgrade?

I thought this API just checks to see if we are in EL2?

> - If you assign this device to a VM and that the hypervisor doesn't
>    correctly virtualise it, then it is a different device and you
>    should simply advertise it something else. Or even better, fix your
>    hypervisor.

Sumit can add some more details on why we don't completely disable the 
device for guest OSs.

Jon

-- 
nvpublic