Received-SPF: pass (google.com: domain of linux-kernel+bounces-178906-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 14 May 2024 09:05:35 -0700
From: Deepak Gupta <debug@rivosinc.com>
To: =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= <cleger@rivosinc.com>
Cc: Conor Dooley <conor@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	kvm-riscv@lists.infradead.org, Ved Shanbhogue <ved@rivosinc.com>
Subject: Re: [RFC PATCH 6/7] riscv: kvm: add SBI FWFT support for
 SBI_FWFT_DOUBLE_TRAP_ENABLE
Message-ID: <ZkOLz0ANkESSv5qr@debug.ba.rivosinc.com>
References: <20240418142701.1493091-1-cleger@rivosinc.com>
 <20240418142701.1493091-7-cleger@rivosinc.com>
 <ZixSFLZYZaf8BKHP@debug.ba.rivosinc.com>
 <c8e4c44d-8125-417e-8c61-a1f6d438815e@rivosinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1; format=flowed
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c8e4c44d-8125-417e-8c61-a1f6d438815e@rivosinc.com>

On Tue, May 14, 2024 at 11:43:15AM +0200, Cl?ment L?ger wrote:
>
>
>On 27/04/2024 03:17, Deepak Gupta wrote:
>> On Thu, Apr 18, 2024 at 04:26:45PM +0200, Cl?ment L?ger wrote:
>>> Add support in KVM SBI FWFT extension to allow VS-mode to request double
>>> trap enabling. Double traps can then be generated by VS-mode, allowing
>>> M-mode to redirect them to S-mode.
>>>
>>
>>> +
>>> +??? if (value)
>>> +??????? csr_set(CSR_HENVCFG_DBLTRP, DBLTRP_DTE);
>>> +??? else
>>> +??????? csr_clear(CSR_HENVCFG_DBLTRP, DBLTRP_DTE);
>>
>> I think vcpu->arch.cfg has `henvcfg` field. Can we reflect it there as
>> well so that current
>> `henvcfg` copy in vcpu arch specifci config is consistent? Otherwise
>> it'll be lost when vCPU
>> is scheduled out and later scheduled back in (on vcpu load)
>
>henvcfg is restored when loading the vpcu (kvm_arch_vcpu_load()) and
>saved when the CPU is put (kvm_arch_vcpu_put()). But I just saw that
>this change is included in the next patch. Should have been this one ,
>I'll fix that.
>
>
>>
>> Furthermore, lets not do feature specific alias names for CSR.
>>
>> Instead let's keep consistent 64bit image of henvcfg in vcpu->arch.cfg.
>>
>> And whenever it's time to pick up the setting, pick up logic either
>> perform the writes in
>> henvcfg. And if required it'll perform henvcfgh too (as
>> `kvm_arch_vcpu_load` already does)
>
>I don't have a strong opinion on that point so if you think it really is
>better, I'll switch to that.

Thanks.

>
>Thanks,
>
>Cl?ment
>
>>
>>> +
>>> +??? return SBI_SUCCESS;
>>> +}
>>> +
>>> +static int kvm_sbi_fwft_get_double_trap(struct kvm_vcpu *vcpu,
>>> +??????????????????? struct kvm_sbi_fwft_config *conf,
>>> +??????????????????? unsigned long *value)
>>> +{
>>> +??? if (!riscv_double_trap_enabled())
>>> +??????? return SBI_ERR_NOT_SUPPORTED;
>>> +
>>> +??? *value = (csr_read(CSR_HENVCFG_DBLTRP) & DBLTRP_DTE) != 0;
>>> +
>>> +??? return SBI_SUCCESS;
>>> +}
>>> +
>>> static struct kvm_sbi_fwft_config *
>>> kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t
>>> feature)
>>> {
>>> @@ -111,6 +147,11 @@ static const struct kvm_sbi_fwft_feature
>>> features[] = {
>>> ??????? .id = SBI_FWFT_MISALIGNED_DELEG,
>>> ??????? .set = kvm_sbi_fwft_set_misaligned_delegation,
>>> ??????? .get = kvm_sbi_fwft_get_misaligned_delegation,
>>> +??? },
>>> +??? {
>>> +??????? .id = SBI_FWFT_DOUBLE_TRAP_ENABLE,
>>> +??????? .set = kvm_sbi_fwft_set_double_trap,
>>> +??????? .get = kvm_sbi_fwft_get_double_trap,
>>> ????}
>>> };
>>>
>>> --?
>>> 2.43.0
>>>
>>>