From: Tianyu Lan <[email protected]>
*** BLURB HERE ***
Hyper-V provides two modes for running SEV-SNP VMs:
1) In vTOM mode with a paravisor (see Section 15.36.8 of [1])
2) In "fully enlightened" mode with normal "C" bit control
over page encryption, and no paravisor
For #1, the paravisor runs in VMPL 0, while Linux runs in VMPL 2
(see Section 15.36.7 of [1]). The paravisor is typically provided
by Hyper-V and handles most of the SNP-related functionality. As
such, most of the SNP functionality in the Linux guest is bypassed.
The guest operates in vTOM mode, where encryption is enabled by default.
The guest must still request page transitions between private and shared,
but there is relatively less SNP machinery required in the guest. Support
for this mode of operation first went upstream in the 5.15 kernel.
For #2, this patch set provides the initial support. The existing
SEV-SNP machinery in the kernel is fully used, but Hyper-V specific
updates are required to properly share Hyper-V communication pages
between the guest and host and to start APs at boot time.
In either mode, Hyper-V requires that the guest implement the SEV-SNP
Restricted Interrupt Injection feature (see Section 15.36.16 of [1],
and Section 5 of [2]). Without this feature, the guest is subject to
attack by a compromised hypervisor that can inject any exception at
any time, such as injecting an interrupt while the guest has interrupts
disabled. In vTOM mode, Restricted Interrupt Injection is implemented
by the paravisor, so no Linux guest changes are required. But in fully
enlightened mode, the Linux guest must provide the implementation.
This patch set is derived from an earlier patch set that includes both
the Hyper-V specific changes and Restricted Interrupt Injection support.[3]
But it is now limited to only the Hyper-V specific changes. The Restricted
Interrupt Injection support will come later in a separate patch set.
[1] https://www.amd.com/system/files/TechDocs/24593.pdf
[2] https://www.amd.com/system/files/TechDocs/56421-guest-hypervisor-communication-block-standardization.pdf
[3] https://lore.kernel.org/lkml/[email protected]/
Change since v4:
* Fix compile error on the ARM or other platform.
Change since v3:
* Fix fossil comment
Change since v2:
* Update Change log.
* Rework Hyper-V hypercall implementation.
Change since v1:
* vTOM case uses paravisor_present flag and
HV_ISOLATION_TYPE_SNP type.
* Rework some patches' change log
* Fix some comments in the patches
Tianyu Lan (8):
x86/hyperv: Add sev-snp enlightened guest static key
x86/hyperv: Set Virtual Trust Level in VMBus init message
x86/hyperv: Mark Hyper-V vp assist page unencrypted in SEV-SNP
enlightened guest
drivers: hv: Mark percpu hvcall input arg page unencrypted in SEV-SNP
enlightened guest
x86/hyperv: Use vmmcall to implement Hyper-V hypercall in sev-snp
enlightened guest
clocksource: hyper-v: Mark hyperv tsc page unencrypted in sev-snp
enlightened guest
x86/hyperv: Add smp support for SEV-SNP guest
x86/hyperv: Add hyperv-specific handling for VMMCALL under SEV-ES
arch/x86/hyperv/hv_init.c | 55 +++++++++++++-
arch/x86/hyperv/ivm.c | 111 +++++++++++++++++++++++++++++
arch/x86/include/asm/hyperv-tlfs.h | 7 ++
arch/x86/include/asm/mshyperv.h | 43 +++++++----
arch/x86/kernel/cpu/mshyperv.c | 43 +++++++++--
drivers/clocksource/hyperv_timer.c | 2 +-
drivers/hv/connection.c | 1 +
drivers/hv/hv.c | 57 ++++++++++++++-
drivers/hv/hv_common.c | 19 +++++
include/asm-generic/hyperv-tlfs.h | 1 +
include/asm-generic/mshyperv.h | 14 +++-
include/linux/hyperv.h | 4 +-
12 files changed, 330 insertions(+), 27 deletions(-)
--
2.25.1
From: Tianyu Lan <[email protected]>
Introduce static key isolation_type_en_snp for enlightened
sev-snp guest check.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Tianyu Lan <[email protected]>
---
arch/x86/hyperv/ivm.c | 11 +++++++++++
arch/x86/include/asm/mshyperv.h | 2 ++
arch/x86/kernel/cpu/mshyperv.c | 9 +++++++--
drivers/hv/hv_common.c | 6 ++++++
include/asm-generic/mshyperv.h | 13 ++++++++++---
5 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 14f46ad2ca64..b2b5cb19fac9 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -413,3 +413,14 @@ bool hv_isolation_type_snp(void)
{
return static_branch_unlikely(&isolation_type_snp);
}
+
+DEFINE_STATIC_KEY_FALSE(isolation_type_en_snp);
+/*
+ * hv_isolation_type_en_snp - Check system runs in the AMD SEV-SNP based
+ * isolation enlightened VM.
+ */
+bool hv_isolation_type_en_snp(void)
+{
+ return static_branch_unlikely(&isolation_type_en_snp);
+}
+
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 88d9ef98e087..9f11f0495950 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -26,6 +26,7 @@
union hv_ghcb;
DECLARE_STATIC_KEY_FALSE(isolation_type_snp);
+DECLARE_STATIC_KEY_FALSE(isolation_type_en_snp);
typedef int (*hyperv_fill_flush_list_func)(
struct hv_guest_mapping_flush_list *flush,
@@ -239,6 +240,7 @@ static inline void hv_vtom_init(void) {}
#endif
extern bool hv_isolation_type_snp(void);
+extern bool hv_isolation_type_en_snp(void);
static inline bool hv_is_synic_reg(unsigned int reg)
{
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index c7969e806c64..5398fb2f4d39 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -402,8 +402,12 @@ static void __init ms_hyperv_init_platform(void)
pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",
ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
- if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
+
+ if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
+ static_branch_enable(&isolation_type_en_snp);
+ } else if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP) {
static_branch_enable(&isolation_type_snp);
+ }
}
if (hv_max_functions_eax >= HYPERV_CPUID_NESTED_FEATURES) {
@@ -473,7 +477,8 @@ static void __init ms_hyperv_init_platform(void)
#if IS_ENABLED(CONFIG_HYPERV)
if ((hv_get_isolation_type() == HV_ISOLATION_TYPE_VBS) ||
- (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP))
+ ((hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP) &&
+ ms_hyperv.paravisor_present))
hv_vtom_init();
/*
* Setup the hook to get control post apic initialization.
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 542a1d53b303..4b4aa53c34c2 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -502,6 +502,12 @@ bool __weak hv_isolation_type_snp(void)
}
EXPORT_SYMBOL_GPL(hv_isolation_type_snp);
+bool __weak hv_isolation_type_en_snp(void)
+{
+ return false;
+}
+EXPORT_SYMBOL_GPL(hv_isolation_type_en_snp);
+
void __weak hv_setup_vmbus_handler(void (*handler)(void))
{
}
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 402a8c1c202d..580c766958de 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -36,15 +36,21 @@ struct ms_hyperv_info {
u32 nested_features;
u32 max_vp_index;
u32 max_lp_index;
- u32 isolation_config_a;
+ union {
+ u32 isolation_config_a;
+ struct {
+ u32 paravisor_present : 1;
+ u32 reserved_a1 : 31;
+ };
+ };
union {
u32 isolation_config_b;
struct {
u32 cvm_type : 4;
- u32 reserved1 : 1;
+ u32 reserved_b1 : 1;
u32 shared_gpa_boundary_active : 1;
u32 shared_gpa_boundary_bits : 6;
- u32 reserved2 : 20;
+ u32 reserved_b2 : 20;
};
};
u64 shared_gpa_boundary;
@@ -58,6 +64,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)
--
2.25.1
From: Tianyu Lan <[email protected]>
SEV-SNP guests on Hyper-V can run at multiple Virtual Trust
Levels (VTL). During boot, get the VTL at which we're running
using the GET_VP_REGISTERs hypercall, and save the value
for future use. Then during VMBus initialization, set the VTL
with the saved value as required in the VMBus init message.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Tianyu Lan <[email protected]>
---
* Change since v3:
Call get_vtl() when SEV-SNP is available and set vtl to 0
by default if fail to get VTL from Hyper-V.
* Change since v2:
Update the change log.
---
arch/x86/hyperv/hv_init.c | 39 ++++++++++++++++++++++++++++++
arch/x86/include/asm/hyperv-tlfs.h | 7 ++++++
drivers/hv/connection.c | 1 +
include/asm-generic/mshyperv.h | 1 +
include/linux/hyperv.h | 4 +--
5 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 6c04b52f139b..0fd0f82c4f07 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -378,6 +378,41 @@ 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;
+ struct hv_get_vp_registers_output *output;
+ unsigned long flags;
+ u64 ret;
+
+ local_irq_save(flags);
+ input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ output = (struct hv_get_vp_registers_output *)input;
+ if (!input) {
+ local_irq_restore(flags);
+ goto done;
+ }
+
+ memset(input, 0, struct_size(input, element, 1));
+ 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 (hv_result_success(ret)) {
+ ret = output->as64.low & HV_X64_VTL_MASK;
+ } else {
+ pr_err("Failed to get VTL and set VTL to zero by default.\n");
+ ret = 0;
+ }
+
+ local_irq_restore(flags);
+done:
+ return ret;
+}
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -506,6 +541,10 @@ 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 */
+ if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
+ 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 cea95dcd27c2..4bf0b315b0ce 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -301,6 +301,13 @@ enum hv_isolation_type {
#define HV_X64_MSR_TIME_REF_COUNT HV_REGISTER_TIME_REF_COUNT
#define HV_X64_MSR_REFERENCE_TSC HV_REGISTER_REFERENCE_TSC
+/*
+ * Registers are only accessible via HVCALL_GET_VP_REGISTERS hvcall and
+ * there is not associated MSR address.
+ */
+#define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003
+#define HV_X64_VTL_MASK GENMASK(3, 0)
+
/* Hyper-V memory host visibility */
enum hv_mem_host_visibility {
VMBUS_PAGE_NOT_VISIBLE = 0,
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 5978e9dbc286..02b54f85dc60 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 580c766958de..efd0d2aedad3 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -54,6 +54,7 @@ struct ms_hyperv_info {
};
};
u64 shared_gpa_boundary;
+ u8 vtl;
};
extern struct ms_hyperv_info ms_hyperv;
extern bool hv_nested;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index bfbc37ce223b..1f2bfec4abde 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
From: Tianyu Lan <[email protected]>
Hypervisor needs to access input arg, VMBus synic event and
message pages. Mark these pages unencrypted in the SEV-SNP
guest and free them only if they have been marked encrypted
successfully.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Tianyu Lan <[email protected]>
---
drivers/hv/hv.c | 57 +++++++++++++++++++++++++++++++++++++++---
drivers/hv/hv_common.c | 13 ++++++++++
2 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index de6708dbe0df..ec6e35a0d9bf 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -20,6 +20,7 @@
#include <linux/interrupt.h>
#include <clocksource/hyperv_timer.h>
#include <asm/mshyperv.h>
+#include <linux/set_memory.h>
#include "hyperv_vmbus.h"
/* The one and only */
@@ -78,7 +79,7 @@ int hv_post_message(union hv_connection_id connection_id,
int hv_synic_alloc(void)
{
- int cpu;
+ int cpu, ret = -ENOMEM;
struct hv_per_cpu_context *hv_cpu;
/*
@@ -123,26 +124,76 @@ int hv_synic_alloc(void)
goto err;
}
}
+
+ if (hv_isolation_type_en_snp()) {
+ ret = set_memory_decrypted((unsigned long)
+ hv_cpu->synic_message_page, 1);
+ if (ret) {
+ pr_err("Failed to decrypt SYNIC msg page: %d\n", ret);
+ hv_cpu->synic_message_page = NULL;
+
+ /*
+ * Free the event page here so that hv_synic_free()
+ * won't later try to re-encrypt it.
+ */
+ free_page((unsigned long)hv_cpu->synic_event_page);
+ hv_cpu->synic_event_page = NULL;
+ goto err;
+ }
+
+ ret = set_memory_decrypted((unsigned long)
+ hv_cpu->synic_event_page, 1);
+ if (ret) {
+ pr_err("Failed to decrypt SYNIC event page: %d\n", ret);
+ hv_cpu->synic_event_page = NULL;
+ goto err;
+ }
+
+ memset(hv_cpu->synic_message_page, 0, PAGE_SIZE);
+ memset(hv_cpu->synic_event_page, 0, PAGE_SIZE);
+ }
}
return 0;
+
err:
/*
* Any memory allocations that succeeded will be freed when
* the caller cleans up by calling hv_synic_free()
*/
- return -ENOMEM;
+ return ret;
}
void hv_synic_free(void)
{
- int cpu;
+ int cpu, ret;
for_each_present_cpu(cpu) {
struct hv_per_cpu_context *hv_cpu
= per_cpu_ptr(hv_context.cpu_context, cpu);
+ /* It's better to leak the page if the encryption fails. */
+ if (hv_isolation_type_en_snp()) {
+ if (hv_cpu->synic_message_page) {
+ ret = set_memory_encrypted((unsigned long)
+ hv_cpu->synic_message_page, 1);
+ if (ret) {
+ pr_err("Failed to encrypt SYNIC msg page: %d\n", ret);
+ hv_cpu->synic_message_page = NULL;
+ }
+ }
+
+ if (hv_cpu->synic_event_page) {
+ ret = set_memory_encrypted((unsigned long)
+ hv_cpu->synic_event_page, 1);
+ if (ret) {
+ pr_err("Failed to encrypt SYNIC event page: %d\n", ret);
+ hv_cpu->synic_event_page = NULL;
+ }
+ }
+ }
+
free_page((unsigned long)hv_cpu->synic_event_page);
free_page((unsigned long)hv_cpu->synic_message_page);
}
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 4b4aa53c34c2..2d43ba2bc925 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -24,6 +24,7 @@
#include <linux/kmsg_dump.h>
#include <linux/slab.h>
#include <linux/dma-map-ops.h>
+#include <linux/set_memory.h>
#include <asm/hyperv-tlfs.h>
#include <asm/mshyperv.h>
@@ -359,6 +360,7 @@ int hv_common_cpu_init(unsigned int cpu)
u64 msr_vp_index;
gfp_t flags;
int pgcount = hv_root_partition ? 2 : 1;
+ int ret;
/* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
flags = irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL;
@@ -378,6 +380,17 @@ int hv_common_cpu_init(unsigned int cpu)
outputarg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
*outputarg = (char *)(*inputarg) + HV_HYP_PAGE_SIZE;
}
+
+ if (hv_isolation_type_en_snp()) {
+ ret = set_memory_decrypted((unsigned long)*inputarg, pgcount);
+ if (ret) {
+ kfree(*inputarg);
+ *inputarg = NULL;
+ return ret;
+ }
+
+ memset(*inputarg, 0x00, pgcount * PAGE_SIZE);
+ }
}
msr_vp_index = hv_get_register(HV_REGISTER_VP_INDEX);
--
2.25.1
From: Tianyu Lan <[email protected]>
hv vp assist page needs to be shared between SEV-SNP guest and Hyper-V.
So mark the page unencrypted in the SEV-SNP guest.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Tianyu Lan <[email protected]>
---
arch/x86/hyperv/hv_init.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 0fd0f82c4f07..547ebf6a03bc 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -18,6 +18,7 @@
#include <asm/hyperv-tlfs.h>
#include <asm/mshyperv.h>
#include <asm/idtentry.h>
+#include <asm/set_memory.h>
#include <linux/kexec.h>
#include <linux/version.h>
#include <linux/vmalloc.h>
@@ -106,8 +107,21 @@ static int hv_cpu_init(unsigned int cpu)
* in hv_cpu_die(), otherwise a CPU may not be stopped in the
* case of CPU offlining and the VM will hang.
*/
- if (!*hvp)
+ if (!*hvp) {
*hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL | __GFP_ZERO);
+
+ /*
+ * Hyper-V should never specify a VM that is a Confidential
+ * VM and also running in the root partition. Root partition
+ * is blocked to run in Confidential VM. So only decrypt assist
+ * page in non-root partition here.
+ */
+ if (*hvp && hv_isolation_type_en_snp()) {
+ WARN_ON_ONCE(set_memory_decrypted((unsigned long)(*hvp), 1));
+ memset(*hvp, 0, PAGE_SIZE);
+ }
+ }
+
if (*hvp)
msr.pfn = vmalloc_to_pfn(*hvp);
--
2.25.1
From: Tianyu Lan <[email protected]>
Add Hyperv-specific handling for faults caused by VMMCALL
instructions.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Tianyu Lan <[email protected]>
---
arch/x86/kernel/cpu/mshyperv.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index c2ccb49b49c2..b7d73f3107c6 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -32,6 +32,7 @@
#include <asm/nmi.h>
#include <clocksource/hyperv_timer.h>
#include <asm/numa.h>
+#include <asm/svm.h>
/* Is Linux running as the root partition? */
bool hv_root_partition;
@@ -574,6 +575,22 @@ static bool __init ms_hyperv_msi_ext_dest_id(void)
return eax & HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE;
}
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+static void hv_sev_es_hcall_prepare(struct ghcb *ghcb, struct pt_regs *regs)
+{
+ /* RAX and CPL are already in the GHCB */
+ ghcb_set_rcx(ghcb, regs->cx);
+ ghcb_set_rdx(ghcb, regs->dx);
+ ghcb_set_r8(ghcb, regs->r8);
+}
+
+static bool hv_sev_es_hcall_finish(struct ghcb *ghcb, struct pt_regs *regs)
+{
+ /* No checking of the return state needed */
+ return true;
+}
+#endif
+
const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.name = "Microsoft Hyper-V",
.detect = ms_hyperv_platform,
@@ -581,4 +598,8 @@ const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.init.x2apic_available = ms_hyperv_x2apic_available,
.init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
.init.init_platform = ms_hyperv_init_platform,
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+ .runtime.sev_es_hcall_prepare = hv_sev_es_hcall_prepare,
+ .runtime.sev_es_hcall_finish = hv_sev_es_hcall_finish,
+#endif
};
--
2.25.1
From: Tianyu Lan <[email protected]>
In the AMD SEV-SNP guest, AP needs to be started up via sev es
save area and Hyper-V requires to call HVCALL_START_VP hypercall
to pass the gpa of sev es save area with AP's vp index and VTL(Virtual
trust level) parameters. Override wakeup_secondary_cpu_64 callback
with hv_snp_boot_ap.
Reviewed-by: Michael Kelley <[email protected]>
Signed-off-by: Tianyu Lan <[email protected]>
---
arch/x86/hyperv/ivm.c | 100 ++++++++++++++++++++++++++++++
arch/x86/include/asm/mshyperv.h | 14 +++++
arch/x86/kernel/cpu/mshyperv.c | 13 +++-
include/asm-generic/hyperv-tlfs.h | 1 +
4 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index b2b5cb19fac9..ee08a0cd6da3 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -18,11 +18,20 @@
#include <asm/mshyperv.h>
#include <asm/hypervisor.h>
#include <asm/mtrr.h>
+#include <asm/coco.h>
+#include <asm/io_apic.h>
+#include <asm/sev.h>
+#include <asm/realmode.h>
+#include <asm/e820/api.h>
+#include <asm/desc.h>
#ifdef CONFIG_AMD_MEM_ENCRYPT
#define GHCB_USAGE_HYPERV_CALL 1
+static u8 ap_start_input_arg[PAGE_SIZE] __bss_decrypted __aligned(PAGE_SIZE);
+static u8 ap_start_stack[PAGE_SIZE] __aligned(PAGE_SIZE);
+
union hv_ghcb {
struct ghcb ghcb;
struct {
@@ -357,6 +366,97 @@ static bool hv_is_private_mmio(u64 addr)
return false;
}
+#define hv_populate_vmcb_seg(seg, gdtr_base) \
+do { \
+ if (seg.selector) { \
+ seg.base = 0; \
+ seg.limit = HV_AP_SEGMENT_LIMIT; \
+ seg.attrib = *(u16 *)(gdtr_base + seg.selector + 5); \
+ seg.attrib = (seg.attrib & 0xFF) | ((seg.attrib >> 4) & 0xF00); \
+ } \
+} while (0) \
+
+int hv_snp_boot_ap(int cpu, unsigned long start_ip)
+{
+ struct sev_es_save_area *vmsa = (struct sev_es_save_area *)
+ __get_free_page(GFP_KERNEL | __GFP_ZERO);
+ struct desc_ptr gdtr;
+ u64 ret, rmp_adjust, retry = 5;
+ struct hv_enable_vp_vtl *start_vp_input;
+ unsigned long flags;
+
+ native_store_gdt(&gdtr);
+
+ vmsa->gdtr.base = gdtr.address;
+ vmsa->gdtr.limit = gdtr.size;
+
+ asm volatile("movl %%es, %%eax;" : "=a" (vmsa->es.selector));
+ hv_populate_vmcb_seg(vmsa->es, vmsa->gdtr.base);
+
+ asm volatile("movl %%cs, %%eax;" : "=a" (vmsa->cs.selector));
+ hv_populate_vmcb_seg(vmsa->cs, vmsa->gdtr.base);
+
+ asm volatile("movl %%ss, %%eax;" : "=a" (vmsa->ss.selector));
+ hv_populate_vmcb_seg(vmsa->ss, vmsa->gdtr.base);
+
+ asm volatile("movl %%ds, %%eax;" : "=a" (vmsa->ds.selector));
+ hv_populate_vmcb_seg(vmsa->ds, vmsa->gdtr.base);
+
+ vmsa->efer = native_read_msr(MSR_EFER);
+
+ asm volatile("movq %%cr4, %%rax;" : "=a" (vmsa->cr4));
+ asm volatile("movq %%cr3, %%rax;" : "=a" (vmsa->cr3));
+ asm volatile("movq %%cr0, %%rax;" : "=a" (vmsa->cr0));
+
+ vmsa->xcr0 = 1;
+ vmsa->g_pat = HV_AP_INIT_GPAT_DEFAULT;
+ vmsa->rip = (u64)secondary_startup_64_no_verify;
+ vmsa->rsp = (u64)&ap_start_stack[PAGE_SIZE];
+
+ /*
+ * Set the SNP-specific fields for this VMSA:
+ * VMPL level
+ * SEV_FEATURES (matches the SEV STATUS MSR right shifted 2 bits)
+ */
+ vmsa->vmpl = 0;
+ vmsa->sev_features = sev_status >> 2;
+
+ /*
+ * Running at VMPL0 allows the kernel to change the VMSA bit for a page
+ * using the RMPADJUST instruction. However, for the instruction to
+ * succeed it must target the permissions of a lesser privileged
+ * (higher numbered) VMPL level, so use VMPL1 (refer to the RMPADJUST
+ * instruction in the AMD64 APM Volume 3).
+ */
+ rmp_adjust = RMPADJUST_VMSA_PAGE_BIT | 1;
+ ret = rmpadjust((unsigned long)vmsa, RMP_PG_SIZE_4K,
+ rmp_adjust);
+ if (ret != 0) {
+ pr_err("RMPADJUST(%llx) failed: %llx\n", (u64)vmsa, ret);
+ return ret;
+ }
+
+ local_irq_save(flags);
+ start_vp_input =
+ (struct hv_enable_vp_vtl *)ap_start_input_arg;
+ memset(start_vp_input, 0, sizeof(*start_vp_input));
+ start_vp_input->partition_id = -1;
+ start_vp_input->vp_index = cpu;
+ start_vp_input->target_vtl.target_vtl = ms_hyperv.vtl;
+ *(u64 *)&start_vp_input->vp_context = __pa(vmsa) | 1;
+
+ do {
+ ret = hv_do_hypercall(HVCALL_START_VP,
+ start_vp_input, NULL);
+ } while (hv_result(ret) == HV_STATUS_TIME_OUT && retry--);
+
+ local_irq_restore(flags);
+
+ if (!hv_result_success(ret))
+ pr_err("HvCallStartVirtualProcessor failed: %llx\n", ret);
+ return ret;
+}
+
void __init hv_vtom_init(void)
{
/*
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 07cad6c2af56..8dce3c8ce038 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -48,6 +48,13 @@ extern u64 hv_current_partition_id;
extern union hv_ghcb * __percpu *hv_ghcb_pg;
+/*
+ * DEFAULT INIT GPAT and SEGMENT LIMIT value in struct VMSA
+ * to start AP in enlightened SEV guest.
+ */
+#define HV_AP_INIT_GPAT_DEFAULT 0x0007040600070406ULL
+#define HV_AP_SEGMENT_LIMIT 0xffffffff
+
int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
@@ -232,12 +239,19 @@ void hv_ghcb_msr_read(u64 msr, u64 *value);
bool hv_ghcb_negotiate_protocol(void);
void __noreturn hv_ghcb_terminate(unsigned int set, unsigned int reason);
void hv_vtom_init(void);
+int hv_snp_boot_ap(int cpu, unsigned long start_ip);
#else
static inline void hv_ghcb_msr_write(u64 msr, u64 value) {}
static inline void hv_ghcb_msr_read(u64 msr, u64 *value) {}
static inline bool hv_ghcb_negotiate_protocol(void) { return false; }
static inline void hv_ghcb_terminate(unsigned int set, unsigned int reason) {}
static inline void hv_vtom_init(void) {}
+<<<<<<< ours
+static int hv_snp_boot_ap(int cpu, unsigned long start_ip) {}
+=======
+static int hv_snp_boot_ap(int cpu, unsigned long start_ip) { return 0; }
+static inline void hv_sev_init_mem_and_cpu(void) {}
+>>>>>>> theirs
#endif
extern bool hv_isolation_type_snp(void);
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 5398fb2f4d39..c2ccb49b49c2 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -295,6 +295,16 @@ static void __init hv_smp_prepare_cpus(unsigned int max_cpus)
native_smp_prepare_cpus(max_cpus);
+ /*
+ * Override wakeup_secondary_cpu_64 callback for SEV-SNP
+ * enlightened guest.
+ */
+ if (hv_isolation_type_en_snp())
+ apic->wakeup_secondary_cpu_64 = hv_snp_boot_ap;
+
+ if (!hv_root_partition)
+ return;
+
#ifdef CONFIG_X86_64
for_each_present_cpu(i) {
if (i == 0)
@@ -502,8 +512,7 @@ static void __init ms_hyperv_init_platform(void)
# ifdef CONFIG_SMP
smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
- if (hv_root_partition)
- smp_ops.smp_prepare_cpus = hv_smp_prepare_cpus;
+ smp_ops.smp_prepare_cpus = hv_smp_prepare_cpus;
# endif
/*
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index f4e4cc4f965f..fdac4a1714ec 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -223,6 +223,7 @@ enum HV_GENERIC_SET_FORMAT {
#define HV_STATUS_INVALID_PORT_ID 17
#define HV_STATUS_INVALID_CONNECTION_ID 18
#define HV_STATUS_INSUFFICIENT_BUFFERS 19
+#define HV_STATUS_TIME_OUT 120
#define HV_STATUS_VTL_ALREADY_ENABLED 134
/*
--
2.25.1
From: Tianyu Lan <[email protected]>
In sev-snp enlightened guest, Hyper-V hypercall needs
to use vmmcall to trigger vmexit and notify hypervisor
to handle hypercall request.
Signed-off-by: Tianyu Lan <[email protected]>
---
arch/x86/include/asm/mshyperv.h | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 9f11f0495950..07cad6c2af56 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -62,12 +62,12 @@ static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
if (!hv_hypercall_pg)
return U64_MAX;
- __asm__ __volatile__("mov %4, %%r8\n"
- CALL_NOSPEC
+ __asm__ __volatile__("mov %[output], %%r8\n"
+ ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES)
: "=a" (hv_status), ASM_CALL_CONSTRAINT,
- "+c" (control), "+d" (input_address)
- : "r" (output_address),
- THUNK_TARGET(hv_hypercall_pg)
+ "+c" (control), "+d" (input_address)
+ : [output] "r" (output_address),
+ THUNK_TARGET(hv_hypercall_pg)
: "cc", "memory", "r8", "r9", "r10", "r11");
#else
u32 input_address_hi = upper_32_bits(input_address);
@@ -103,7 +103,8 @@ static inline u64 _hv_do_fast_hypercall8(u64 control, u64 input1)
#ifdef CONFIG_X86_64
{
- __asm__ __volatile__(CALL_NOSPEC
+ __asm__ __volatile__("mov %[thunk_target], %%r8\n"
+ ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES)
: "=a" (hv_status), ASM_CALL_CONSTRAINT,
"+c" (control), "+d" (input1)
: THUNK_TARGET(hv_hypercall_pg)
@@ -148,13 +149,13 @@ static inline u64 _hv_do_fast_hypercall16(u64 control, u64 input1, u64 input2)
#ifdef CONFIG_X86_64
{
- __asm__ __volatile__("mov %4, %%r8\n"
- CALL_NOSPEC
- : "=a" (hv_status), ASM_CALL_CONSTRAINT,
- "+c" (control), "+d" (input1)
- : "r" (input2),
- THUNK_TARGET(hv_hypercall_pg)
- : "cc", "r8", "r9", "r10", "r11");
+ __asm__ __volatile__("mov %[output], %%r8\n"
+ ALTERNATIVE(CALL_NOSPEC, "vmmcall", X86_FEATURE_SEV_ES)
+ : "=a" (hv_status), ASM_CALL_CONSTRAINT,
+ "+c" (control), "+d" (input1)
+ : [output] "r" (input2),
+ THUNK_TARGET(hv_hypercall_pg)
+ : "cc", "r8", "r9", "r10", "r11");
}
#else
{
--
2.25.1
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> Introduce static key isolation_type_en_snp for enlightened
> sev-snp guest check.
>
> Reviewed-by: Michael Kelley <[email protected]>
> Signed-off-by: Tianyu Lan <[email protected]>
Reviewed-by: Dexuan Cui <[email protected]>
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> @@ -378,6 +378,41 @@ 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;
> + struct hv_get_vp_registers_output *output;
> + unsigned long flags;
> + u64 ret;
This should be
u64 ret = 0;
> + local_irq_save(flags);
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + output = (struct hv_get_vp_registers_output *)input;
> + if (!input) {
> + local_irq_restore(flags);
> + goto done;
Here the uninitialized 'ret' is returned.
If we move the "done:" label one line earlier, we won't need the
the above " local_irq_restore(flags);"
Maybe we should add a WARN_ON_ONCE(1) before "goto done"?
> + }
> +
> + memset(input, 0, struct_size(input, element, 1));
> + 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 (hv_result_success(ret)) {
> + ret = output->as64.low & HV_X64_VTL_MASK;
> + } else {
> + pr_err("Failed to get VTL and set VTL to zero by default.\n");
> + ret = 0;
If we have "u64 ret = 0;", we won't need the "ret = 0;" here, and we
won't need the curly braces.
> + }
> +
> + local_irq_restore(flags);
> +done:
> + return ret;
> +}
After the above minor issues are fixed, feel free to add
Reviewed-by: Dexuan Cui <[email protected]>
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> Hypervisor needs to access input arg, VMBus synic event and
> message pages. Mark these pages unencrypted in the SEV-SNP
> guest and free them only if they have been marked encrypted
> successfully.
>
> Reviewed-by: Michael Kelley <[email protected]>
> Signed-off-by: Tianyu Lan <[email protected]>
Reviewed-by: Dexuan Cui <[email protected]>
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> hv vp assist page needs to be shared between SEV-SNP guest and Hyper-V.
> So mark the page unencrypted in the SEV-SNP guest.
>
> Reviewed-by: Michael Kelley <[email protected]>
> Signed-off-by: Tianyu Lan <[email protected]>
Reviewed-by: Dexuan Cui <[email protected]>
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> @@ -103,7 +103,8 @@ static inline u64 _hv_do_fast_hypercall8(u64 control,
> u64 input1)
>
> #ifdef CONFIG_X86_64
> {
> - __asm__ __volatile__(CALL_NOSPEC
> + __asm__ __volatile__("mov %[thunk_target], %%r8\n"
The "mov %[thunk_target], %%r8\n" is dubious.
I removed it and the kernel still worked fine for my regular VM (on an AMD host)
and for my SNP VM (with HCL).
I suspect a fully enlightened SNP VM also doesn't need it as this hypercall
doesn't really need an output param.
I noticed your
[PATCH V5 8/8] x86/hyperv: Add hyperv-specific handling for VMMCALL under SEV-ES
exposes r8 to the hypervisor:
+static void hv_sev_es_hcall_prepare(struct ghcb *ghcb, struct pt_regs *regs)
+{
+ /* RAX and CPL are already in the GHCB */
+ ghcb_set_rcx(ghcb, regs->cx);
+ ghcb_set_rdx(ghcb, regs->dx);
+ ghcb_set_r8(ghcb, regs->r8);
+}
I guess the intent here is that we want to pass a deterministic value in R8 (rather
a random value) to the hypervisor for security's purpose. If so, can we just set
R8 to 0 rather than %[thunk_target]?
Please add a comment.
Sorry, I was not in the earlier discussion, so I may be missing something.
> + ALTERNATIVE(CALL_NOSPEC, "vmmcall",
> X86_FEATURE_SEV_ES)
> : "=a" (hv_status),
> ASM_CALL_CONSTRAINT,
> "+c" (control), "+d" (input1)
> : THUNK_TARGET(hv_hypercall_pg)
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> --- a/arch/x86/hyperv/ivm.c
> +++ b/arch/x86/hyperv/ivm.c
> [...]
> +static u8 ap_start_input_arg[PAGE_SIZE] __bss_decrypted
> __aligned(PAGE_SIZE);
> +static u8 ap_start_stack[PAGE_SIZE] __aligned(PAGE_SIZE);
> +
> union hv_ghcb {
> struct ghcb ghcb;
> struct {
> @@ -357,6 +366,97 @@ static bool hv_is_private_mmio(u64 addr)
> return false;
> }
>
> +#define hv_populate_vmcb_seg(seg, gdtr_base) \
> +do { \
> + if (seg.selector) { \
> + seg.base = 0; \
> + seg.limit = HV_AP_SEGMENT_LIMIT; \
> + seg.attrib = *(u16 *)(gdtr_base + seg.selector + 5); \
> + seg.attrib = (seg.attrib & 0xFF) | ((seg.attrib >> 4) & 0xF00); \
> + } \
> +} while (0) \
> +
> +int hv_snp_boot_ap(int cpu, unsigned long start_ip)
> +{
> + struct sev_es_save_area *vmsa = (struct sev_es_save_area *)
> + __get_free_page(GFP_KERNEL | __GFP_ZERO);
Should we check against -ENOMEM?
> + rmp_adjust = RMPADJUST_VMSA_PAGE_BIT | 1;
> + ret = rmpadjust((unsigned long)vmsa, RMP_PG_SIZE_4K,
> + rmp_adjust);
> + if (ret != 0) {
> + pr_err("RMPADJUST(%llx) failed: %llx\n", (u64)vmsa, ret);
Need to free the 'vmsa' page before returning?
> + return ret;
> + }
> +
> + local_irq_save(flags);
> + start_vp_input =
> + (struct hv_enable_vp_vtl *)ap_start_input_arg;
The above 2 lines can be merged into one line.
> + memset(start_vp_input, 0, sizeof(*start_vp_input));
> + start_vp_input->partition_id = -1;
> + start_vp_input->vp_index = cpu;
> + start_vp_input->target_vtl.target_vtl = ms_hyperv.vtl;
> + *(u64 *)&start_vp_input->vp_context = __pa(vmsa) | 1;
Is CPU online/offline supported for a fully enlightened SNP VM?
If yes, then it looks like we need to reuse the same "vmsa" page?
Currently hv_snp_boot_ap() always creates a new page and it looks
like the page is never released
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> [...]
> +<<<<<<< ours
> +static int hv_snp_boot_ap(int cpu, unsigned long start_ip) {}
> +=======
Remove the 3 lines.
> +static int hv_snp_boot_ap(int cpu, unsigned long start_ip) { return 0; }
> +static inline void hv_sev_init_mem_and_cpu(void) {}
> +>>>>>>> theirs
Remove the line
> #endif
>
> extern bool hv_isolation_type_snp(void);
> diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> index 5398fb2f4d39..c2ccb49b49c2 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -295,6 +295,16 @@ static void __init hv_smp_prepare_cpus(unsigned
> int max_cpus)
>
> native_smp_prepare_cpus(max_cpus);
>
> + /*
> + * Override wakeup_secondary_cpu_64 callback for SEV-SNP
> + * enlightened guest.
> + */
> + if (hv_isolation_type_en_snp())
> + apic->wakeup_secondary_cpu_64 = hv_snp_boot_ap;
I suspect the global page "ap_start_stack" in arch/x86/hyperv/ivm.c
can be used simultaneously by different APs? If so, IMO we should
add x86_cpuinit.parallel_bringup = false;
Thanks,
Dexuan
> From: Tianyu Lan <[email protected]>
> Sent: Thursday, August 10, 2023 9:04 AM
> [...]
> Add Hyperv-specific handling for faults caused by VMMCALL
> instructions.
>
> Reviewed-by: Michael Kelley <[email protected]>
> Signed-off-by: Tianyu Lan <[email protected]>
Reviewed-by: Dexuan Cui <[email protected]>
On Thu, Aug 10, 2023 at 10:59:35PM +0000, Dexuan Cui wrote:
> > From: Tianyu Lan <[email protected]>
> > Sent: Thursday, August 10, 2023 9:04 AM
> > [...]
> > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > @@ -378,6 +378,41 @@ 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;
> > + struct hv_get_vp_registers_output *output;
> > + unsigned long flags;
> > + u64 ret;
>
> This should be
> u64 ret = 0;
>
> > + local_irq_save(flags);
> > + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > + output = (struct hv_get_vp_registers_output *)input;
> > + if (!input) {
> > + local_irq_restore(flags);
> > + goto done;
>
> Here the uninitialized 'ret' is returned.
>
> If we move the "done:" label one line earlier, we won't need the
> the above " local_irq_restore(flags);"
> Maybe we should add a WARN_ON_ONCE(1) before "goto done"?
Out of interest why will input be NULL here?
Thanks,
Wei.
> From: Wei Liu <[email protected]>
> Sent: Friday, August 11, 2023 2:15 PM
> To: Dexuan Cui <[email protected]>
> > [...]
> > > + local_irq_save(flags);
> > > + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > > + output = (struct hv_get_vp_registers_output *)input;
> > > + if (!input) {
> > > + local_irq_restore(flags);
> > > + goto done;
> >
> > Here the uninitialized 'ret' is returned.
> >
> > If we move the "done:" label one line earlier, we won't need the
> > the above " local_irq_restore(flags);"
> > Maybe we should add a WARN_ON_ONCE(1) before "goto done"?
>
> Out of interest why will input be NULL here?
>
> Thanks,
> Wei.
I don't think 'input' could be NULL here.
IMO we can use it without checking against NULL.