Skip to contentSkip to conversation

arch/x86/include/asm/sev.h:222:93: warning: 'struct snp_guest_request_ioctl' declared inside parameter list will not be visible outside of this definition or declaration

PATCH Applied kernel test robot 2 participants 2 messages Mar 6, 2023 – Mar 21, 2023

Subsystems (2)
lore
Share & tools
lore

kernel test robot

author build bot
#0 lore raw
tree:   https://github.com/ammarfaizi2/linux-block bp/bp/tip-x86-sev
head:   9bda6a75b659d5b34bff06c22afee2b9b21d1146
commit: 9bda6a75b659d5b34bff06c22afee2b9b21d1146 [11/11] x86/sev: Change snp_guest_issue_request()'s fw_err argument
config: i386-tinyconfig (https://download.01.org/0day-ci/archive/20230307/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

reproduce (this is a W=1 build):
# https://github.com/ammarfaizi2/linux-block/commit/9bda6a75b659d5b34bff06c22afee2b9b21d1146
git remote add ammarfaizi2-block https://github.com/ammarfaizi2/linux-block
git fetch --no-tags ammarfaizi2-block bp/bp/tip-x86-sev
git checkout 9bda6a75b659d5b34bff06c22afee2b9b21d1146
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash arch/x86/realmode/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from arch/x86/realmode/init.c:12:

>> arch/x86/include/asm/sev.h:222:93: warning: 'struct snp_guest_request_ioctl' declared inside parameter list will not be visible outside of this definition or declaration

222 | static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)
| ^~~~~~~~~~~~~~~~~~~~~~~

vim +222 arch/x86/include/asm/sev.h

190
191 void setup_ghcb(void);
192 void __init early_snp_set_memory_private(unsigned long vaddr, unsigned long paddr,
193 unsigned int npages);
194 void __init early_snp_set_memory_shared(unsigned long vaddr, unsigned long paddr,
195 unsigned int npages);
196 void __init snp_prep_memory(unsigned long paddr, unsigned int sz, enum psc_op op);
197 void snp_set_memory_shared(unsigned long vaddr, unsigned int npages);
198 void snp_set_memory_private(unsigned long vaddr, unsigned int npages);
199 void snp_set_wakeup_secondary_cpu(void);
200 bool snp_init(struct boot_params *bp);
201 void __init __noreturn snp_abort(void);
202 int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
203 #else
204 static inline void sev_es_ist_enter(struct pt_regs *regs) { }
205 static inline void sev_es_ist_exit(void) { }
206 static inline int sev_es_setup_ap_jump_table(struct real_mode_header *rmh) { return 0; }
207 static inline void sev_es_nmi_complete(void) { }
208 static inline int sev_es_efi_map_ghcbs(pgd_t *pgd) { return 0; }
209 static inline int pvalidate(unsigned long vaddr, bool rmp_psize, bool validate) { return 0; }
210 static inline int rmpadjust(unsigned long vaddr, bool rmp_psize, unsigned long attrs) { return 0; }
211 static inline void setup_ghcb(void) { }
212 static inline void __init
213 early_snp_set_memory_private(unsigned long vaddr, unsigned long paddr, unsigned int npages) { }
214 static inline void __init
215 early_snp_set_memory_shared(unsigned long vaddr, unsigned long paddr, unsigned int npages) { }
216 static inline void __init snp_prep_memory(unsigned long paddr, unsigned int sz, enum psc_op op) { }
217 static inline void snp_set_memory_shared(unsigned long vaddr, unsigned int npages) { }
218 static inline void snp_set_memory_private(unsigned long vaddr, unsigned int npages) { }
219 static inline void snp_set_wakeup_secondary_cpu(void) { }
220 static inline bool snp_init(struct boot_params *bp) { return false; }
221 static inline void snp_abort(void) { }

 > 222	static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)

223 {
224 return -ENOTTY;
225 }
226 #endif
227

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
merge bot Applied
#1 lore raw

[tip: x86/sev] x86/sev: Change snp_guest_issue_request()'s fw_err argument

The following commit has been merged into the x86/sev branch of tip:

The following commit has been merged into the x86/sev branch of tip:

Commit-ID:     0144e3b85d7b42e8a4cda991c0e81f131897457a
Gitweb:        https://git.kernel.org/tip/0144e3b85d7b42e8a4cda991c0e81f131897457a
Author:        Dionna Glaze <[email protected]>
AuthorDate:    Tue, 07 Mar 2023 20:24:49 +01:00
Committer:     Borislav Petkov (AMD) <[email protected]>
CommitterDate: Tue, 21 Mar 2023 15:43:19 +01:00

x86/sev: Change snp_guest_issue_request()'s fw_err argument

The GHCB specification declares that the firmware error value for a guest request will be stored in the lower 32 bits of EXIT_INFO_2. The upper 32 bits are for the VMM's own error code. The fw_err argument to snp_guest_issue_request() is thus a misnomer, and callers will need access to all 64 bits.

The type of unsigned long also causes problems, since sw_exit_info2 is u64 (unsigned long long) vs the argument's unsigned long*. Change this type for issuing the guest request. Pass the ioctl command struct's error field directly instead of in a local variable, since an incomplete guest request may not set the error code, and uninitialized stack memory would be written back to user space.

The firmware might not even be called, so bookend the call with the no firmware call error and clear the error.

Since the "fw_err" field is really exitinfo2 split into the upper bits' vmm error code and lower bits' firmware error code, convert the 64 bit value to a union.

[ bp:
- Massage commit message
- adjust code
- Fix a build issue as

- print exitinfo2 in hex
Tom:
- Correct -EIO exit case. ]

---
 Documentation/virt/coco/sev-guest.rst   | 20 ++++---
 arch/x86/include/asm/sev-common.h       |  4 +-
 arch/x86/include/asm/sev.h              | 10 ++-
 arch/x86/kernel/sev.c                   | 15 ++---
 drivers/virt/coco/sev-guest/sev-guest.c | 72 +++++++++++++-----------
 include/uapi/linux/sev-guest.h          | 18 +++++-
 6 files changed, 83 insertions(+), 56 deletions(-)
Documentation/virt/coco/sev-guest.rst +13 -7
index aa3e4c6..68b0d23 100644
@@ -37,11 +37,11 @@ along with a description:
       the return value.  General error numbers (-ENOMEM, -EINVAL)
       are not detailed, but errors with specific meanings are.
 
-The guest ioctl should be issued on a file descriptor of the /dev/sev-guest device.
-The ioctl accepts struct snp_user_guest_request. The input and output structure is
-specified through the req_data and resp_data field respectively. If the ioctl fails
-to execute due to a firmware error, then fw_err code will be set. Otherwise, fw_err
-will be set to 0x00000000ffffffff, i.e., the lower 32-bits are -1.
+The guest ioctl should be issued on a file descriptor of the /dev/sev-guest
+device.  The ioctl accepts struct snp_user_guest_request. The input and
+output structure is specified through the req_data and resp_data field
+respectively. If the ioctl fails to execute due to a firmware error, then
+the fw_error code will be set, otherwise fw_error will be set to -1.
 
 The firmware checks that the message sequence counter is one greater than
 the guests message sequence counter. If guest driver fails to increment message
@@ -57,8 +57,14 @@ counter (e.g. counter overflow), then -EIO will be returned.
                 __u64 req_data;
                 __u64 resp_data;
 
-                /* firmware error code on failure (see psp-sev.h) */
-                __u64 fw_err;
+                /* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */
+                union {
+                        __u64 exitinfo2;
+                        struct {
+                                __u32 fw_error;
+                                __u32 vmm_error;
+                        };
+                };
         };
 
 2.1 SNP_GET_REPORT
arch/x86/include/asm/sev-common.h +0 -4
index b63be69..0759af9 100644
@@ -128,10 +128,6 @@ struct snp_psc_desc {
 	struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY];
 } __packed;
 
-/* Guest message request error codes */
-#define SNP_GUEST_REQ_INVALID_LEN	BIT_ULL(32)
-#define SNP_GUEST_REQ_ERR_BUSY		BIT_ULL(33)
-
 #define GHCB_MSR_TERM_REQ		0x100
 #define GHCB_MSR_TERM_REASON_SET_POS	12
 #define GHCB_MSR_TERM_REASON_SET_MASK	0xf
arch/x86/include/asm/sev.h +7 -3
index ebc271b..13dc2a9 100644
@@ -9,6 +9,8 @@
 #define __ASM_ENCRYPTED_STATE_H
 
 #include <linux/types.h>
+#include <linux/sev-guest.h>
+
 #include <asm/insn.h>
 #include <asm/sev-common.h>
 #include <asm/bootparam.h>
@@ -185,6 +187,9 @@ static inline int pvalidate(unsigned long vaddr, bool rmp_psize, bool validate)
 
 	return rc;
 }
+
+struct snp_guest_request_ioctl;
+
 void setup_ghcb(void);
 void __init early_snp_set_memory_private(unsigned long vaddr, unsigned long paddr,
 					 unsigned int npages);
@@ -196,7 +201,7 @@ void snp_set_memory_private(unsigned long vaddr, unsigned int npages);
 void snp_set_wakeup_secondary_cpu(void);
 bool snp_init(struct boot_params *bp);
 void __init __noreturn snp_abort(void);
-int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err);
+int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
 #else
 static inline void sev_es_ist_enter(struct pt_regs *regs) { }
 static inline void sev_es_ist_exit(void) { }
@@ -216,8 +221,7 @@ static inline void snp_set_memory_private(unsigned long vaddr, unsigned int npag
 static inline void snp_set_wakeup_secondary_cpu(void) { }
 static inline bool snp_init(struct boot_params *bp) { return false; }
 static inline void snp_abort(void) { }
-static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input,
-					  unsigned long *fw_err)
+static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)
 {
 	return -ENOTTY;
 }
arch/x86/kernel/sev.c +8 -7
index 3f664ab..b031244 100644
@@ -22,6 +22,8 @@
 #include <linux/efi.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
+#include <linux/psp-sev.h>
+#include <uapi/linux/sev-guest.h>
 
 #include <asm/cpu_entry_area.h>
 #include <asm/stacktrace.h>
@@ -2175,7 +2177,7 @@ static int __init init_sev_config(char *str)
 }
 __setup("sev=", init_sev_config);
 
-int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err)
+int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio)
 {
 	struct ghcb_state state;
 	struct es_em_ctxt ctxt;
@@ -2183,8 +2185,7 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned 
 	struct ghcb *ghcb;
 	int ret;
 
-	if (!fw_err)
-		return -EINVAL;
+	rio->exitinfo2 = SEV_RET_NO_FW_CALL;
 
 	/*
 	 * __sev_get_ghcb() needs to run with IRQs disabled because it is using
@@ -2209,16 +2210,16 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned 
 	if (ret)
 		goto e_put;
 
-	*fw_err = ghcb->save.sw_exit_info_2;
-	switch (*fw_err) {
+	rio->exitinfo2 = ghcb->save.sw_exit_info_2;
+	switch (rio->exitinfo2) {
 	case 0:
 		break;
 
-	case SNP_GUEST_REQ_ERR_BUSY:
+	case SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_BUSY):
 		ret = -EAGAIN;
 		break;
 
-	case SNP_GUEST_REQ_INVALID_LEN:
+	case SNP_GUEST_VMM_ERR(SNP_GUEST_VMM_ERR_INVALID_LEN):
 		/* Number of expected pages are returned in RBX */
 		if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) {
 			input->data_npages = ghcb_get_rbx(ghcb);
drivers/virt/coco/sev-guest/sev-guest.c +39 -33

161 lines not on this page · raw message

include/uapi/linux/sev-guest.h +16 -2
index 256aaef..2aa3911 100644
@@ -52,8 +52,14 @@ struct snp_guest_request_ioctl {
 	__u64 req_data;
 	__u64 resp_data;
 
-	/* firmware error code on failure (see psp-sev.h) */
-	__u64 fw_err;
+	/* bits[63:32]: VMM error code, bits[31:0] firmware error code (see psp-sev.h) */
+	union {
+		__u64 exitinfo2;
+		struct {
+			__u32 fw_error;
+			__u32 vmm_error;
+		};
+	};
 };
 
 struct snp_ext_report_req {
@@ -77,4 +83,12 @@ struct snp_ext_report_req {
 /* Get SNP extended report as defined in the GHCB specification version 2. */
 #define SNP_GET_EXT_REPORT _IOWR(SNP_GUEST_REQ_IOC_TYPE, 0x2, struct snp_guest_request_ioctl)
 
+/* Guest message request EXIT_INFO_2 constants */
+#define SNP_GUEST_FW_ERR_MASK		GENMASK_ULL(31, 0)
+#define SNP_GUEST_VMM_ERR_SHIFT		32
+#define SNP_GUEST_VMM_ERR(x)		(((u64)x) << SNP_GUEST_VMM_ERR_SHIFT)
+
+#define SNP_GUEST_VMM_ERR_INVALID_LEN	1
+#define SNP_GUEST_VMM_ERR_BUSY		2
+
 #endif /* __UAPI_LINUX_SEV_GUEST_H_ */