Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 24 Jul 2023 12:25:03 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     Zeng Guang <guang.zeng@intel.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        H Peter Anvin <hpa@zytor.com>, <kvm@vger.kernel.org>,
        <x86@kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 5/8] KVM: emulator: Add emulation of LASS violation
 checks on linear address
Message-ID: <ZL39H3MnSgcJMmCV@chao-email>
References: <20230718131844.5706-1-guang.zeng@intel.com>
 <20230718131844.5706-6-guang.zeng@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230718131844.5706-6-guang.zeng@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?yswij0D2XWqlRsInSF+gKewnVfeF4HICuWpBJG1vVeeTKupXY8KQzEff7eKH?=
 =?us-ascii?Q?Ab7iy9cC2k+5gEw1uGhGPOQ2KPy3RWmO0zAAm+tr/UKrn/wRe+7UrKch489N?=
 =?us-ascii?Q?v+La1FkPAbDXro6eyqNXFlbkxFSDpRkrEDki1Db7ki2jLmrAT4eQcULAfAUt?=
 =?us-ascii?Q?tZtgZObLYvV4CR03Ra5pfL9qUpZVWwhekN8pLyMZKw6SQbKa0NHvl8IqYFev?=
 =?us-ascii?Q?Q+4Qlpp3iF808i+Mk3ZWoBW33LvAtTG/3HThYI2xdhNO51Rww3zn0pZbJR0z?=
 =?us-ascii?Q?40TuGCSv2XQh1nzP4GeoV+4RuM2wq5+rcP7bP5lbA7k5eY6yKHCG/3zuBfjy?=
 =?us-ascii?Q?friAsI7svSQR1AX+b4y2peRC5s5TYxs/fK2xdXlqdg1Ft3pUov/dHLB7fSb6?=
 =?us-ascii?Q?eIIr/GDcODn9JfdmuhmcdFgzqeOH2DTQrYFpHwx7/hhLFOvSVAt5d/+tjWNa?=
 =?us-ascii?Q?ogXH/oYul1py1V0RZ7lHzpeDrjS7IpL/E7RkpdZIwmoBJMrzLb0JcSjtKmtg?=
 =?us-ascii?Q?Cy4uAxxNKmg5z/eT0ZFr8qj+aJv1RMhSWAdfFhZR5tBGSQDPjISYsOlq36/l?=
 =?us-ascii?Q?8pdYpLXi+ZJXl+mWpRbPEzrP7s4wXNw7rjDTJErW1rnAh8pTi7ktuTI7AGP1?=
 =?us-ascii?Q?7PZ1G6MXCrwtIPJ0VjUovVruDmL5FZ7Wf1XHyrYDK35KsO/+Eio9icc92C1q?=
 =?us-ascii?Q?CJEpjmW3uOJYC55GYrqE8504BV7iM5Rk/6zdMygg7LR65tqMJcg/1BHbAq4F?=
 =?us-ascii?Q?oxYi0qjPU90lbQOa8A1N3TQX+9M7oxTYPAw+lwfDD5emyKHJojEJvJJQh0PR?=
 =?us-ascii?Q?Il7qMZuPD+uYH4CqkbuaMbJBEKJGQ6UvchfZ8XRhiQ/2QaHc3Pxb62FjEJSj?=
 =?us-ascii?Q?7YNWN7lNP0GWnT7ZkWBht2upPypzvEbOhPBS20tbCFSQXAzk/RO7AlcH61cq?=
 =?us-ascii?Q?xXTQGgXGBhMo6lTZVsweLJ5yxdUn8nze52J92giSZmfun/odG/GZfU4Wjo1G?=
 =?us-ascii?Q?MtlSjExFf5XFTxBYaTcPHNa6cz2qXsxM8DAkE2X7yfGF5KCzEn/ffxOVilJ6?=
 =?us-ascii?Q?vzHCdT6z5peAi/Kb8wtmZWHPnfv416+WE4g2yVBR0ZzRJPGcwWNKvcwf639c?=
 =?us-ascii?Q?RYooFQEwXcPQ0ZHy8LY7erGkdw22KuCeOhQiB5DU6Dj6to/VWXLeSX77EVmq?=
 =?us-ascii?Q?lZN4i8vYqlD80KlFFxiPwGPKEToRuA0jBhRcQdzCJp9RSZkbTsKf0Eq1VqVW?=
 =?us-ascii?Q?8BvkGYaqdKItI284JG5pfrtrVR4/r5fVODjWn5u92KZ1PYwINpsWwQlm6kZ5?=
 =?us-ascii?Q?htuHx68NzmRVruVSopSlwikJwGxl6hFPi3N1Fxr0GBpQXgbv3yRwx6Lzy9Zw?=
 =?us-ascii?Q?tateSAfhXaSwZKM/MCj9P+YJbhj9aLkZtYHPSKeVI7eDNxQFB/rh7Plzcu5R?=
 =?us-ascii?Q?L76hWX/hZ/9j8AiqgfmVyPrXFNHWjsZZlKBwlfjwWw/B49727hb2yEHYkoLg?=
 =?us-ascii?Q?o5wXuAYE8LxNrcmymzfG7lnojnu4bcbviei6a5g/e+DRKEQyZbJWY5wSCg8h?=
 =?us-ascii?Q?JmLyOPg6SDTuhyDioAh1XQqjxrWjPuKY+/zE3ipS?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5e2317c3-0144-4857-8f2e-08db8bfdfa81
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB6780.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jul 2023 04:25:15.8809
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8474PM3VvFR1jh2hnEeLbHRFqUxLPxJTItH91GLjw6pcLGwNPKafohUscRGRFYrn5ERrdkm+zUP23WMXjeSgOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB6940
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jul 18, 2023 at 09:18:41PM +0800, Zeng Guang wrote:
>When enabled Intel CPU feature Linear Address Space Separation (LASS),
>KVM emulator will take LASS violation check on every access to guest
>memory by a linear address.

When Intel Linear Address Space Separation (LASS) is enabled, the processor
applies a LASS violation check to every access to a linear address. To
align with hardware behavior, KVM needs to perform the same check in
instruction emulation.

>
>We defined a new function prototype in kvm_x86_ops for emulator to
>construct the interface to identify whether a LASS violation occurs.
>It can have further practical implementation according to vendor
>specific requirements.
>
>Emulator will use the passed (address, size) pair and instruction
>operation type (flags) to enforce LASS protection when KVM emulates
>instruction fetch, data access including implicit data access to a
>system data structure.

Define a new function in x86_emulator_ops to perform the LASS violation
check in KVM emulator. The function accepts an address and a size, which
delimit the memory access, and a flag, which provides extra information
about the access that is necessary for LASS violation checks, e.g., whether
the access is an instruction fetch or implicit access.

emulator_is_lass_violation() is just a placeholder. it will be wired up
to VMX/SVM implementation by a later patch.

(I think the commit message can also explain why the LASS violation
 check is added in the three functions, i.e., __linearize(),
 linear_read_system() and linear_write_system(), and why only in them)

>
>Signed-off-by: Zeng Guang <guang.zeng@intel.com>
>Tested-by: Xuelian Guo <xuelian.guo@intel.com>
>---
> arch/x86/include/asm/kvm-x86-ops.h |  3 ++-
> arch/x86/include/asm/kvm_host.h    |  3 +++
> arch/x86/kvm/emulate.c             | 11 +++++++++++
> arch/x86/kvm/kvm_emulate.h         |  2 ++
> arch/x86/kvm/x86.c                 | 10 ++++++++++
> 5 files changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
>index 13bc212cd4bc..a301f0a46381 100644
>--- a/arch/x86/include/asm/kvm-x86-ops.h
>+++ b/arch/x86/include/asm/kvm-x86-ops.h
>@@ -132,7 +132,8 @@ KVM_X86_OP_OPTIONAL(migrate_timers)
> KVM_X86_OP(msr_filter_changed)
> KVM_X86_OP(complete_emulated_msr)
> KVM_X86_OP(vcpu_deliver_sipi_vector)
>-KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
>+KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons)
>+KVM_X86_OP_OPTIONAL_RET0(is_lass_violation)

...

> 
> #undef KVM_X86_OP
> #undef KVM_X86_OP_OPTIONAL
>diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>index fb9d1f2d6136..791f0dd48cd9 100644
>--- a/arch/x86/include/asm/kvm_host.h
>+++ b/arch/x86/include/asm/kvm_host.h
>@@ -1731,6 +1731,9 @@ struct kvm_x86_ops {
> 	 * Returns vCPU specific APICv inhibit reasons
> 	 */
> 	unsigned long (*vcpu_get_apicv_inhibit_reasons)(struct kvm_vcpu *vcpu);
>+
>+	bool (*is_lass_violation)(struct kvm_vcpu *vcpu, unsigned long addr,
>+				  unsigned int size, unsigned int flags);

I may think we can just return false in emulator_is_lass_violation() and
fold this new kvm_x86_ops definition into its vmx implementation. This way
is more natural to me.

> };
> 
> struct kvm_x86_nested_ops {
>diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>index 9b4b3ce6d52a..2289a4ad21be 100644
>--- a/arch/x86/kvm/emulate.c
>+++ b/arch/x86/kvm/emulate.c
>@@ -742,6 +742,10 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
> 		}
> 		break;
> 	}
>+
>+	if (ctxt->ops->is_lass_violation(ctxt, *linear, size, flags))
>+		goto bad;
>+
> 	if (la & (insn_alignment(ctxt, size) - 1))
> 		return emulate_gp(ctxt, 0);
> 	return X86EMUL_CONTINUE;
>@@ -848,6 +852,9 @@ static inline int jmp_rel(struct x86_emulate_ctxt *ctxt, int rel)
> static int linear_read_system(struct x86_emulate_ctxt *ctxt, ulong linear,
> 			      void *data, unsigned size)
> {
>+	if (ctxt->ops->is_lass_violation(ctxt, linear, size, X86EMUL_F_IMPLICIT))
>+		return emulate_gp(ctxt, 0);
>+
> 	return ctxt->ops->read_std(ctxt, linear, data, size, &ctxt->exception, true);
> }
> 
>@@ -855,6 +862,10 @@ static int linear_write_system(struct x86_emulate_ctxt *ctxt,
> 			       ulong linear, void *data,
> 			       unsigned int size)
> {
>+	if (ctxt->ops->is_lass_violation(ctxt, linear, size,
>+					 X86EMUL_F_IMPLICIT | X86EMUL_F_FETCH))

s/X86EMUL_F_FETCH/X86EMUL_F_WRITE/