Received-SPF: pass (google.com: domain of linux-kernel+bounces-106071-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <9ae4b453-4d5b-4537-b004-4db60183019a@os.amperecomputing.com>
Date: Mon, 18 Mar 2024 16:58:08 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH 12/28] KVM: arm64: Support timers in realm RECs
Content-Language: en-US
To: Steven Price <steven.price@arm.com>, kvm@vger.kernel.org,
 kvmarm@lists.linux.dev
Cc: Catalin Marinas <catalin.marinas@arm.com>, Marc Zyngier <maz@kernel.org>,
 Will Deacon <will@kernel.org>, James Morse <james.morse@arm.com>,
 Oliver Upton <oliver.upton@linux.dev>,
 Suzuki K Poulose <suzuki.poulose@arm.com>, Zenghui Yu
 <yuzenghui@huawei.com>, linux-arm-kernel@lists.infradead.org,
 linux-kernel@vger.kernel.org, Joey Gouly <joey.gouly@arm.com>,
 Alexandru Elisei <alexandru.elisei@arm.com>,
 Christoffer Dall <christoffer.dall@arm.com>, Fuad Tabba <tabba@google.com>,
 linux-coco@lists.linux.dev
References: <20230127112248.136810-1-suzuki.poulose@arm.com>
 <20230127112932.38045-1-steven.price@arm.com>
 <20230127112932.38045-13-steven.price@arm.com>
From: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>
In-Reply-To: <20230127112932.38045-13-steven.price@arm.com>
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?UnYvNWZHYzBuS1FKcHd3UTk4RnB4OUdoeW1Hdk9VNzQxM3NBaU1aSGt0QXZi?=
 =?utf-8?B?Ulp3TTNmazIzWjlaaVhnVVVpVmRuZkNua0hDTG5DQU04NHJ3MnZRZGIzbk42?=
 =?utf-8?B?dnpCMUNHL2Y1QnlPYkxkOXlGdVFrYllDcG5WcGp2TnFselhuSjU4Z0poVzRW?=
 =?utf-8?B?TDZkR0xMVmhDUGUrQ3FyQ2dJcXUraDNXbjE3dWs5VC9aVFlRSGhkb2F0TjQy?=
 =?utf-8?B?bnRRZThPRWR0UFp4RkgraS9Xc1I3RWFCcEdudklLRTY0cmhTdG1GUGFCT2pq?=
 =?utf-8?B?bnUzSTZEMVBXajJBeC9maHRacjQxV1pJWlFXb1pNWHRoSjRGSTVkVXRHT2Fr?=
 =?utf-8?B?MkEyZWJtK0NIc2QrZnhJWGRrMXpJaHpSTnBUZkdYdTZsVG9pU2tqTjNwZDRX?=
 =?utf-8?B?V0tTVGlrOWV0VFpRdDFTNXlKREZUSzNiNG1PaldPU29JOFRyMDltajR0clZ0?=
 =?utf-8?B?blBwWHZpaTR5MWxxNFd4bVVhS2NGeDdWeTdSZ3NnZkNhSEE3ZkljYUxFQ2ZD?=
 =?utf-8?B?YWRRbG9IYnAybk5EUWFkamtEUzBDYXErTnJVS2ZweU1HZFpsYkVMN0s1QVla?=
 =?utf-8?B?ZTJxdit4U0JlVjU3eElqWE10WlZ3NVdrWnZQVkhlYkJDcHZLZmV2Y1RtWlpM?=
 =?utf-8?B?VkZ4Ty9RRjQrb3ZFTjZpVmFFOVJvTmsyd21CTkVQRDh0RDhnYzRlYlR0Zm1z?=
 =?utf-8?B?MFZOVWhVUnloM2JyRHpqRTlqN1lMeVZmMTZaenVyVWRBRDhrbk9ZMjBjVFRV?=
 =?utf-8?B?WEErRk90MkNnMk9TNEppUGZUYXVRckN6YldPVDQvR05oblRrenZwM2FKakox?=
 =?utf-8?B?dU56ejhPbThlNVVEYldya1lOalAxZE9VcFRDM0RMMGg5bUNEVWpJVTNDaEtU?=
 =?utf-8?B?bjNvSng0bENEeGNuaHV4eVBFOXAvaElYU3BxbEUvMEVSTzRFVEJ5c0Y1WXpu?=
 =?utf-8?B?WGF2ejdhR1VORjFBSU5HMWNNd2hkV2YvRDErczhiYlF4aG5BckVwYWFZR1pq?=
 =?utf-8?B?VFNiU25xTy9wQ1puaWh6RTh0QTA3emRXNnYwYUdHeGxQKzh2Zk5menNDVFlI?=
 =?utf-8?B?S2VIZ2YxYngvYjZSOGkybFdtWmpJNGNGVTJ4cTE2TG9VczYyVDZHNW53cDkv?=
 =?utf-8?B?MmxGdGQ0aFVMYXZIcmlxMm44czdSSHJ4OXozMlYzd1pKQ2M2QWtXbmdTeCty?=
 =?utf-8?B?N3RpSlNMMFd3bjVlN05wcEtucXUvVUlqT1FVWURhYys1YXlQWVBQR3AxN0pU?=
 =?utf-8?B?bGtkVTJibVBzaHdPcnVaYnJ5UWhRWTM2VU5vbWNJbHZQejdjS2JHRmVjOWNa?=
 =?utf-8?B?VXEwV1VrMUU2eU8wR1p3SGw1VUxsQ2duUkdyRlJuWURDR3VWMlIrc2U0eitQ?=
 =?utf-8?B?YmtVZGpaUm85RnV5R0k0cnhyajhmV1hPZ2ZUVFYxTTRUK29Ebld1cDNobTlJ?=
 =?utf-8?B?ZG1taDNMcXVFZTBTbUI1ZlN2QXozV3h5YTQ2R1NMQW5BMy80eFdkbTB5N3Ri?=
 =?utf-8?B?OHZKQlJ0MDVJUmp4OU5aS0JKQm9FeGpyMzJqRHAyN2ZVRnZrK2JhYjJIbzRY?=
 =?utf-8?B?R1JPalRqOXlEeDhKTm5Ka2VDRkJMbkwwR3ZhdXBXVnA2Ujc0eFI4bTBZMm9M?=
 =?utf-8?B?aTc5UUU1b2tkemJ3TWNqdFpubU9xQUVQeFpTbXFLR3pLWXQ2c1dUNHFxdlJk?=
 =?utf-8?B?NDFMamNTTHNaRFFnMUozRVBOajNGYjBkSno2bytoTVNWNDZFT3Q0elM2U1ZN?=
 =?utf-8?B?b1o5MnNReFE0VDVCWThQMHd2bjZtZGpIR04yTTJadm9JR3Y1L0tjSjg2VzRr?=
 =?utf-8?B?alJuMW1XakNmaGdwNUdHNjdCVHlwNnhBUzY3V0p0MUtnSkk4azEySkt3S0Vv?=
 =?utf-8?B?L09uZlk3VFRic0Uva1F3d0pJVTIwVjBMZWRNakhpbFpUSXVjMEpwT1JjY0Vp?=
 =?utf-8?B?QjY1a1JheGRCNmdZY003RFZ6WGpPOERhakk3RFV1MWJZYkgxZzlGZU42UExK?=
 =?utf-8?B?UElrV0EvTDZmWmRianZhZnEvZlVkeVJHbnMzRUNxVGVIRHVHTWx3eG5xMzBU?=
 =?utf-8?B?dHJSU1huY0ZjbHo3WDBlbG1QNHpDcitTbnRIYUhKcllzTWNaSXZBRVRObGxM?=
 =?utf-8?B?dVBlcmJyMGRMb0hRbG5EbitPV3phSDlYZnRzenBWd1l0WG1MY3hOT2VIVU9w?=
 =?utf-8?Q?8K9T5Xlz/+/YEVRwLUjen1qgTjqzqVnA8PfMWLw9ho1a?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d5f05b23-3df8-422e-e782-08dc473e8403
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR01MB8101.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Mar 2024 11:28:20.9345
 (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: RpeOGHg8Ouv+iQARbxdsgkrj6qu3DJBR8tssUtgiUB41uHmAh1no8DNqYnSWdzEYOg8SOLBh9szqWInQ+IoxE3Vry8z7fK8l0HxmmhXE4LhLM2yxi3K0GdMhYpQVcfsW
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR01MB8063


On 27-01-2023 04:59 pm, Steven Price wrote:
> The RMM keeps track of the timer while the realm REC is running, but on
> exit to the normal world KVM is responsible for handling the timers.
> 
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
>   arch/arm64/kvm/arch_timer.c  | 53 ++++++++++++++++++++++++++++++++----
>   include/kvm/arm_arch_timer.h |  2 ++
>   2 files changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index bb24a76b4224..d4af9ee58550 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -130,6 +130,11 @@ static void timer_set_offset(struct arch_timer_context *ctxt, u64 offset)
>   {
>   	struct kvm_vcpu *vcpu = ctxt->vcpu;
>   
> +	if (kvm_is_realm(vcpu->kvm)) {
> +		WARN_ON(offset);
> +		return;
> +	}
> +
>   	switch(arch_timer_ctx_index(ctxt)) {
>   	case TIMER_VTIMER:
>   		__vcpu_sys_reg(vcpu, CNTVOFF_EL2) = offset;
> @@ -411,6 +416,21 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
>   	}
>   }
>   
> +void kvm_realm_timers_update(struct kvm_vcpu *vcpu)
> +{
> +	struct arch_timer_cpu *arch_timer = &vcpu->arch.timer_cpu;
> +	int i;
> +
> +	for (i = 0; i < NR_KVM_TIMERS; i++) {

Do we required to check for all timers, is realm/rmm uses hyp timers?

> +		struct arch_timer_context *timer = &arch_timer->timers[i];
> +		bool status = timer_get_ctl(timer) & ARCH_TIMER_CTRL_IT_STAT;
> +		bool level = kvm_timer_irq_can_fire(timer) && status;
> +
> +		if (level != timer->irq.level)
> +			kvm_timer_update_irq(vcpu, level, timer);
> +	}
> +}
> +
>   /* Only called for a fully emulated timer */
>   static void timer_emulate(struct arch_timer_context *ctx)
>   {
> @@ -621,6 +641,11 @@ void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
>   	if (unlikely(!timer->enabled))
>   		return;
>   
> +	kvm_timer_unblocking(vcpu);
> +
> +	if (vcpu_is_rec(vcpu))
> +		return;
> +

For realm, timer->enabled is not set, load returns before this check.

>   	get_timer_map(vcpu, &map);
>   
>   	if (static_branch_likely(&has_gic_active_state)) {
> @@ -633,8 +658,6 @@ void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)
>   
>   	set_cntvoff(timer_get_offset(map.direct_vtimer));
>   
> -	kvm_timer_unblocking(vcpu);
> -
>   	timer_restore_state(map.direct_vtimer);
>   	if (map.direct_ptimer)
>   		timer_restore_state(map.direct_ptimer);
> @@ -668,6 +691,9 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
>   	if (unlikely(!timer->enabled))
>   		return;
>   
> +	if (vcpu_is_rec(vcpu))
> +		goto out;
> +
>   	get_timer_map(vcpu, &map);
>   
>   	timer_save_state(map.direct_vtimer);
> @@ -686,9 +712,6 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
>   	if (map.emul_ptimer)
>   		soft_timer_cancel(&map.emul_ptimer->hrtimer);
>   
> -	if (kvm_vcpu_is_blocking(vcpu))
> -		kvm_timer_blocking(vcpu);
> -
>   	/*
>   	 * The kernel may decide to run userspace after calling vcpu_put, so
>   	 * we reset cntvoff to 0 to ensure a consistent read between user
> @@ -697,6 +720,11 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
>   	 * virtual offset of zero, so no need to zero CNTVOFF_EL2 register.
>   	 */
>   	set_cntvoff(0);
> +
> +out:
> +	if (kvm_vcpu_is_blocking(vcpu))
> +		kvm_timer_blocking(vcpu);
> +
>   }
>   
>   /*
> @@ -785,12 +813,18 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
>   	struct arch_timer_cpu *timer = vcpu_timer(vcpu);
>   	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>   	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
> +	u64 cntvoff;
>   
>   	vtimer->vcpu = vcpu;
>   	ptimer->vcpu = vcpu;
>   
> +	if (kvm_is_realm(vcpu->kvm))
> +		cntvoff = 0;
> +	else
> +		cntvoff = kvm_phys_timer_read();
> +
>   	/* Synchronize cntvoff across all vtimers of a VM. */
> -	update_vtimer_cntvoff(vcpu, kvm_phys_timer_read());
> +	update_vtimer_cntvoff(vcpu, cntvoff);
>   	timer_set_offset(ptimer, 0);
>   
>   	hrtimer_init(&timer->bg_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS_HARD);
> @@ -1265,6 +1299,13 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
>   		return -EINVAL;
>   	}
>   
> +	/*
> +	 * We don't use mapped IRQs for Realms because the RMI doesn't allow
> +	 * us setting the LR.HW bit in the VGIC.
> +	 */
> +	if (vcpu_is_rec(vcpu))
> +		return 0;
> +
>   	get_timer_map(vcpu, &map);
>   
>   	ret = kvm_vgic_map_phys_irq(vcpu,
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index cd6d8f260eab..158280e15a33 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -76,6 +76,8 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
>   int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
>   int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr);
>   
> +void kvm_realm_timers_update(struct kvm_vcpu *vcpu);
> +
>   u64 kvm_phys_timer_read(void);
>   
>   void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu);

Thanks,
Ganapat