From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-il1-f176.google.com (mail-il1-f176.google.com [209.85.166.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5C77319CC28 for ; Fri, 17 Jan 2025 16:31:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.176 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737131472; cv=none; b=H844Qg7a1JWesPqxPcHrYKJt2NjDn2gURLZPpAoaVfV+0+aHEf/lkOJ5buV/F5F/e4Htnjo4GXVFjtuIKZY0CzWzNbupLKq9SwZCCD0WqCmCRky2thnauctTvYbUtQUH0dUPx6hk1OHVAvjVOeq+QiHbPt+TebVvcvaPJU98VkA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737131472; c=relaxed/simple; bh=mxqFj7Cbt7mFEh8WMnbue8M2/6bNKGAQaEo8QmVgDjM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=sCG+ZxgMYIS3XFFnWNaVLNKNXvzhd2EWhxdLBf58TUWk2tWlj8iMzN8Uqi/mc/gpESPWFx2mBDCKVgUXqapGH1EFQimbfT9Wxjcl84upqZySbZ5NseoIi2PBzh83/hjg2+zFCsQFVjZVCg26lGHMqQJdoyaLbL40Os/FpO5G2A0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=sifive.com; spf=pass smtp.mailfrom=sifive.com; dkim=pass (2048-bit key) header.d=sifive.com header.i=@sifive.com header.b=WYU9o+Wy; arc=none smtp.client-ip=209.85.166.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=sifive.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sifive.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sifive.com header.i=@sifive.com header.b="WYU9o+Wy" Received: by mail-il1-f176.google.com with SMTP id e9e14a558f8ab-3ce8c9a656dso18103025ab.3 for ; Fri, 17 Jan 2025 08:31:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1737131469; x=1737736269; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=2LgnsHnmS3fh5mDGhqIRf5OGV7HMImqUQ/i0e8Ovvcw=; b=WYU9o+WyKze7Qv+kqWOhSKa1Z6veT/IJgs2nZLMgqPsT4nLn4GKuZrP8Pmf2yWNgVL tviLW83q3ZHgVj2DCC8/0p2/hYDe+fwmQ42qnHeP9EEE0E5xPN0uH5anV4hk9aCknfF0 7hutgy41kH4yj4waJLKh7isE+GA/G1ZxVdNsCPBp6wkCRNfnWr0lJm/xNzwz/J/oXyoN aBPIzix/b7I/4QQBrJmD/lgeAr6cK9KiUDUAEpqLR36Gsnt5w4lSnnj4+1YAFLR/gmP/ IDv0CF0ggqfq6y8xc2CU9Do102skyRJXtmzA6jq2/7Dsy2ylQGZ8syCY77mWcKt9YTjq UOYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737131469; x=1737736269; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=2LgnsHnmS3fh5mDGhqIRf5OGV7HMImqUQ/i0e8Ovvcw=; b=q3Xo8Tnbr7CnviehNE5X0l7gLRP0FsyJPuaYBB/HDy9pcQQnkPE7EWQAbi2jkmpsXb d4B5oUflvaHmnFyurPT59tBeOZU3D53t95VtUy1Jxz9f4Mjdq6cZUOoiOSrg6wkY4ec0 bag9K879fFQ3nyex9jQ7BILu1KtPqkpNS6wBu1n8znT8XhhoXafFp5cv/bwS6wTnHuKc xc+EdKRi7/Ltfrs8x6xxxuH6MyBBG4KCghcV0khOLwPrH3HhszbZu3Fd3ftVrWglW7fV tHV/elXuAZVMjKY/P5d87sCNLDxGRg303HpIFA89NXc/5vYnpEjVUp4oPZBIJdwDR4gX VSqw== X-Forwarded-Encrypted: i=1; AJvYcCWA8lz0Ruj851Sl2vusMMUHej7kZkms3TZ2f1po000bOJqZGSElMTvHqE5VnQ0kH80qufzvKWXd+WV2xV8=@vger.kernel.org X-Gm-Message-State: AOJu0YwX0XLAG46bLrhiOCGpDsuPqFK72nrucwu6nBd9FvcUNtvQ2WUW 4RDlam+6l8Pju7F6e2kHjx8Au9SVRNgbx18jTSvCp5P3lHtARIjN137MxeUCfR8= X-Gm-Gg: ASbGncs2sA00gut/q2fAuOgQOSHEt7+fBsrceyplTIlyRXUq02iCOHgFTjtqrrJxfxC yVLDe5UuoLuIcDr2k9aJdxNS/3yGlgRqGfL2Ghx6uf2Nk+FebJdFAPbkjBri/xzbDvg+X8XOozS xOZzfun8EWloVWv9/mI+QUf2Hl8BP3V7xIXcfP0A2Co5H5IlgITHrPdf76Q4zLU4dyAa6kwSB3t SJ2SK9pRiaZ9Zf6W/KD/Zo2tiZcYMMeVmVWJPinxxiUgGbxUNQaXqir3dZQMuU9jOWEdWZYsYis HY97 X-Google-Smtp-Source: AGHT+IFnKYDZSdNxxP6fjbNh6IOeT1lfl9g/Gyt6iEwmsb06hqBZJxurWC5Tjc8A5f2CPYlVAQFeaA== X-Received: by 2002:a05:6e02:214d:b0:3a7:e7bd:9f09 with SMTP id e9e14a558f8ab-3cf743b447dmr25305575ab.5.1737131469361; Fri, 17 Jan 2025 08:31:09 -0800 (PST) Received: from [100.64.0.1] ([165.188.116.9]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-4ea7566e4a0sm695782173.121.2025.01.17.08.31.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 17 Jan 2025 08:31:08 -0800 (PST) Message-ID: Date: Fri, 17 Jan 2025 10:31:08 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 6/6] RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG To: =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= Cc: Paul Walmsley , Palmer Dabbelt , Anup Patel , Atish Patra , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kvm-riscv@lists.infradead.org References: <20250106154847.1100344-1-cleger@rivosinc.com> <20250106154847.1100344-7-cleger@rivosinc.com> From: Samuel Holland Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Clément, On 2025-01-17 10:05 AM, Clément Léger wrote: > On 11/01/2025 00:55, Samuel Holland wrote: >> On 2025-01-06 9:48 AM, Clément Léger wrote: >>> SBI_FWFT_MISALIGNED_DELEG needs hedeleg to be modified to delegate >>> misaligned load/store exceptions. Save and restore it during CPU >>> load/put. >>> >>> Signed-off-by: Clément Léger >>> --- >>> arch/riscv/kvm/vcpu.c | 3 +++ >>> arch/riscv/kvm/vcpu_sbi_fwft.c | 39 ++++++++++++++++++++++++++++++++++ >>> 2 files changed, 42 insertions(+) >>> >>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c >>> index 3420a4a62c94..bb6f788d46f5 100644 >>> --- a/arch/riscv/kvm/vcpu.c >>> +++ b/arch/riscv/kvm/vcpu.c >>> @@ -641,6 +641,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) >>> { >>> void *nsh; >>> struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr; >>> + struct kvm_vcpu_config *cfg = &vcpu->arch.cfg; >>> >>> vcpu->cpu = -1; >>> >>> @@ -666,6 +667,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) >>> csr->vstval = nacl_csr_read(nsh, CSR_VSTVAL); >>> csr->hvip = nacl_csr_read(nsh, CSR_HVIP); >>> csr->vsatp = nacl_csr_read(nsh, CSR_VSATP); >>> + cfg->hedeleg = nacl_csr_read(nsh, CSR_HEDELEG); >>> } else { >>> csr->vsstatus = csr_read(CSR_VSSTATUS); >>> csr->vsie = csr_read(CSR_VSIE); >>> @@ -676,6 +678,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) >>> csr->vstval = csr_read(CSR_VSTVAL); >>> csr->hvip = csr_read(CSR_HVIP); >>> csr->vsatp = csr_read(CSR_VSATP); >>> + cfg->hedeleg = csr_read(CSR_HEDELEG); >>> } >>> } >>> >>> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c >>> index 55433e805baa..1e85ff6666af 100644 >>> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c >>> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c >>> @@ -14,6 +14,8 @@ >>> #include >>> #include >>> >>> +#define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED) >>> + >>> static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = { >>> SBI_FWFT_MISALIGNED_EXC_DELEG, >>> SBI_FWFT_LANDING_PAD, >>> @@ -35,7 +37,44 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature) >>> return false; >>> } >>> >>> +static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu) >>> +{ >>> + if (!unaligned_ctl_available()) >> >> This seems like the wrong condition. Patch 2 requests delegation regardless of >> what probing detects. For MISALIGNED_SCALAR_FAST, the delegation likely doesn't> change any actual behavior, because the hardware likely never raises the >> exception. But it does ensure M-mode never emulates anything, so if the >> exception were to occur, the kernel has the choice whether to handle it. And >> this lets us provide the same guarantee to KVM guests. So I think this feature >> should also be supported if we successfully delegated the exception on the host >> side. > > Not sure to completely follow you here but patch 2 actually does the > reverse of what you said. We request delegation from SBI *before* > probing so that allows probing to see if we (kernel) receives misaligned > accesses traps and thus set MISALIGNED_SCALAR_EMULATED. Ah, right, that makes sense. > But if I understood correctly, you mean that guest delegation should > also be available to guest in case misaligned access were delegated by > the SBI to the host which I agree. I think this condition should be Yes, your understanding is correct. > reworked to report the delegation status itself and not the misaligned > access speed that was detected Agreed, with the nuance that delegation may have been enabled by a FWFT call, or it may have been pre-existing, as detected by MISALIGNED_SCALAR_EMULATED or MISALIGNED_VECTOR_EMULATED. For example, I would consider a platform where the hardware supports misaligned scalar accesses (MISALIGNED_SCALAR_FAST) but not vector accesses, and M-mode doesn't emulate them (MISALIGNED_VECTOR_EMULATED) to be delegated for the purposes of this check, even if M-mode doesn't implement the FWFT extension. Regards, Samuel >>> + return false; >>> + >>> + return true; >>> +} >>> + >>> +static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu, >>> + struct kvm_sbi_fwft_config *conf, >>> + unsigned long value) >>> +{ >>> + if (value == 1) >>> + csr_set(CSR_HEDELEG, MIS_DELEG); >>> + else if (value == 0) >>> + csr_clear(CSR_HEDELEG, MIS_DELEG); >>> + else >>> + return SBI_ERR_INVALID_PARAM; >>> + >>> + return SBI_SUCCESS; >>> +} >>> + >>> +static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu, >>> + struct kvm_sbi_fwft_config *conf, >>> + unsigned long *value) >>> +{ >>> + *value = (csr_read(CSR_HEDELEG) & MIS_DELEG) != 0; >>> + >>> + return SBI_SUCCESS; >>> +} >>> + >>> static const struct kvm_sbi_fwft_feature features[] = { >>> + { >>> + .id = SBI_FWFT_MISALIGNED_EXC_DELEG, >>> + .supported = kvm_sbi_fwft_misaligned_delegation_supported, >>> + .set = kvm_sbi_fwft_set_misaligned_delegation, >>> + .get = kvm_sbi_fwft_get_misaligned_delegation, >>> + } >> >> nit: Please add a trailing comma here as future patches will extend the array. >> >> Regards, >> Samuel >> >>> }; >>> >>> static struct kvm_sbi_fwft_config * >> >