| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BYAPR21MB16885D6652BEEA882D96C97CD7D09@BYAPR21MB1688.namprd21.prod.outlook.com>
Date: Tue, 31 Jan 2023 17:55:18 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: Tianyu Lan <ltykernel@...il.com>,
"luto@...nel.org" <luto@...nel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"seanjc@...gle.com" <seanjc@...gle.com>,
"pbonzini@...hat.com" <pbonzini@...hat.com>,
"jgross@...e.com" <jgross@...e.com>,
Tianyu Lan <Tianyu.Lan@...rosoft.com>,
"kirill@...temov.name" <kirill@...temov.name>,
"jiangshan.ljs@...group.com" <jiangshan.ljs@...group.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"ashish.kalra@....com" <ashish.kalra@....com>,
"srutherford@...gle.com" <srutherford@...gle.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"anshuman.khandual@....com" <anshuman.khandual@....com>,
"pawan.kumar.gupta@...ux.intel.com"
<pawan.kumar.gupta@...ux.intel.com>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>,
"daniel.sneddon@...ux.intel.com" <daniel.sneddon@...ux.intel.com>,
"alexander.shishkin@...ux.intel.com"
<alexander.shishkin@...ux.intel.com>,
"sandipan.das@....com" <sandipan.das@....com>,
"ray.huang@....com" <ray.huang@....com>,
"brijesh.singh@....com" <brijesh.singh@....com>,
"michael.roth@....com" <michael.roth@....com>,
"thomas.lendacky@....com" <thomas.lendacky@....com>,
"venu.busireddy@...cle.com" <venu.busireddy@...cle.com>,
"sterritt@...gle.com" <sterritt@...gle.com>,
"tony.luck@...el.com" <tony.luck@...el.com>,
"samitolvanen@...gle.com" <samitolvanen@...gle.com>,
"fenghua.yu@...el.com" <fenghua.yu@...el.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>
Subject: RE: [RFC PATCH V3 03/16] x86/hyperv: Set Virtual Trust Level in vmbus
init message
From: Tianyu Lan <ltykernel@...il.com> Sent: Saturday, January 21, 2023 6:46 PM
>
> sev-snp guest provides vtl(Virtual Trust Level) and
> get it from hyperv hvcall via HVCALL_GET_VP_REGISTERS.
> Set target vtl in the vmbus init message.
I'm still wondering why this is necessary in an SNP VM, vs.
just assuming VTL 0.
Also, I had several comments on v2 of this patch that don't appear
to have been taken into account. I strongly think the code should
use the standard helper functions for checking hypercall results.
Some of my other code comments are more nit-picky and could
perhaps be ignored. :-)
>
> Signed-off-by: Tianyu Lan <tiala@...rosoft.com>
> ---
> Change since RFC v2:
> * Rename get_current_vtl() to get_vtl()
> * Fix some coding style issues
> ---
> arch/x86/hyperv/hv_init.c | 37 ++++++++++++++++++++++++++++++
> arch/x86/include/asm/hyperv-tlfs.h | 4 ++++
> drivers/hv/connection.c | 1 +
> include/asm-generic/mshyperv.h | 2 ++
> include/linux/hyperv.h | 4 ++--
> 5 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 24154c1ee12b..9e9757049915 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -384,6 +384,40 @@ static void __init hv_get_partition_id(void)
> local_irq_restore(flags);
> }
>
> +static u8 __init get_vtl(void)
> +{
> + u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS;
> + struct hv_get_vp_registers_input *input = NULL;
> + struct hv_get_vp_registers_output *output = NULL;
> + u64 vtl = 0;
> + int ret;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + input = *(struct hv_get_vp_registers_input **)this_cpu_ptr(hyperv_pcpu_input_arg);
> + output = (struct hv_get_vp_registers_output *)input;
> + if (!input || !output) {
> + local_irq_restore(flags);
> + goto done;
> + }
> +
> + memset(input, 0, sizeof(*input) + sizeof(input->element[0]));
> + input->header.partitionid = HV_PARTITION_ID_SELF;
> + input->header.vpindex = HV_VP_INDEX_SELF;
> + input->header.inputvtl = 0;
> + input->element[0].name0 = HV_X64_REGISTER_VSM_VP_STATUS;
> +
> + ret = hv_do_hypercall(control, input, output);
> + if (ret == 0)
> + vtl = output->as64.low & HV_X64_VTL_MASK;
> + else
> + pr_err("Hyper-V: failed to get VTL!");
> + local_irq_restore(flags);
> +
> +done:
> + return vtl;
> +}
> +
> /*
> * This function is to be invoked early in the boot sequence after the
> * hypervisor has been detected.
> @@ -512,6 +546,9 @@ void __init hyperv_init(void)
> /* Query the VMs extended capability once, so that it can be cached. */
> hv_query_ext_cap(0);
>
> + /* Find the VTL */
> + ms_hyperv.vtl = get_vtl();
> +
> return;
>
> clean_guest_os_id:
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index db2202d985bd..6dcbb21aac2b 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -36,6 +36,10 @@
> #define HYPERV_CPUID_MIN 0x40000005
> #define HYPERV_CPUID_MAX 0x4000ffff
>
> +/* Support for HVCALL_GET_VP_REGISTERS hvcall */
The above comment isn't really right, in that these definitions
aren't for the hypercall. They are for the specific synthetic register.
> +#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003
> +#define HV_X64_VTL_MASK GENMASK(3, 0)
Hyper-V synthetic registers have two different numbering schemes.
For registers that have synthetic MSR equivalents, there's a full list
starting with HV_X64_MSR_GUEST_OS_ID, which defines the MSR
address. But these registers also have register numbers that are
not the same as the MSR address. These register numbers
aren't defined anywhere in x86 Linux code because we don't access
them using the register number. (The register numbers *are*
defined in ARM64 code since ARM64 doesn't have MSRs.) But this
register is an exception on x86. There's no MSR equivalent so we
must use a hypercall to fetch the value.
I'd suggest starting a separate list after the definition of
HV_X64_MSR_REFERENCE_TSC and make clear in a comment
about the list that this is a list of register numbers, not MSR addresses.
> +
> /*
> * Group D Features. The bit assignments are custom to each architecture.
> * On x86/x64 these are HYPERV_CPUID_FEATURES.EDX bits.
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index f670cfd2e056..e4c39f4016ad 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -98,6 +98,7 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo
> *msginfo, u32 version)
> */
> if (version >= VERSION_WIN10_V5) {
> msg->msg_sint = VMBUS_MESSAGE_SINT;
> + msg->msg_vtl = ms_hyperv.vtl;
> vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4;
> } else {
> msg->interrupt_page = virt_to_phys(vmbus_connection.int_page);
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index f2c0856f1797..44e56777fea7 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -48,6 +48,7 @@ struct ms_hyperv_info {
> };
> };
> u64 shared_gpa_boundary;
> + u8 vtl;
> };
> extern struct ms_hyperv_info ms_hyperv;
>
> @@ -57,6 +58,7 @@ extern void * __percpu *hyperv_pcpu_output_arg;
> extern u64 hv_do_hypercall(u64 control, void *inputaddr, void *outputaddr);
> extern u64 hv_do_fast_hypercall8(u16 control, u64 input8);
> extern bool hv_isolation_type_snp(void);
> +extern bool hv_isolation_type_en_snp(void);
>
> /* Helper functions that provide a consistent pattern for checking Hyper-V hypercall
> status. */
> static inline int hv_result(u64 status)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 85f7c5a63aa6..65121b21b0af 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -665,8 +665,8 @@ struct vmbus_channel_initiate_contact {
> u64 interrupt_page;
> struct {
> u8 msg_sint;
> - u8 padding1[3];
> - u32 padding2;
> + u8 msg_vtl;
> + u8 reserved[6];
> };
> };
> u64 monitor_page1;
> --
> 2.25.1
Powered by blists - more mailing lists