qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] target/i386/sev: Reduce system specific declarations


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 2/2] target/i386/sev: Reduce system specific declarations
Date: Wed, 18 Dec 2024 17:22:17 +0100
User-agent: Mozilla Thunderbird

On 18/12/24 17:17, Daniel P. Berrangé wrote:
On Wed, Dec 18, 2024 at 04:59:13PM +0100, Philippe Mathieu-Daudé wrote:
"system/confidential-guest-support.h" is not needed,
remove it. Reorder #ifdef'ry to reduce declarations
exposed on user emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
  target/i386/sev.h  | 29 ++++++++++++++++-------------
  hw/i386/pc_sysfw.c |  2 +-
  2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 2664c0b1b6c..373669eaace 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -18,7 +18,17 @@
  #include CONFIG_DEVICES /* CONFIG_SEV */
  #endif
-#include "system/confidential-guest-support.h"
+#if !defined(CONFIG_SEV) || defined(CONFIG_USER_ONLY)
+#define sev_enabled() 0
+#define sev_es_enabled() 0
+#define sev_snp_enabled() 0
+#else
+bool sev_enabled(void);
+bool sev_es_enabled(void);
+bool sev_snp_enabled(void);
+#endif
+
+#if !defined(CONFIG_USER_ONLY)

I'm surprised any of this header file is relevant to
user mode. If something is mistakely calling sev_ functions
from user mode compiled code, I'd be inclined to fix the
caller such that its #include ".../sev.h" can be wrapped
by !CONFIG_USER_ONLY

I forgot to mention and just replied in another post:

  The motivation is to reduce system-specific definitions
  exposed to user-mode in target/i386/cpu.c, like hwaddr &co,
  but I'm not there yet and have too many local patches so
  starting to send what's ready.

WRT SEV what is bugging me is in cpu_x86_cpuid():

target/i386/cpu.c-7137-    case 0x8000001F:
target/i386/cpu.c-7138-        *eax = *ebx = *ecx = *edx = 0;
target/i386/cpu.c:7139:        if (sev_enabled()) {
target/i386/cpu.c-7140-            *eax = 0x2;
target/i386/cpu.c-7141-            *eax |= sev_es_enabled() ? 0x8 : 0;
target/i386/cpu.c-7142-            *eax |= sev_snp_enabled() ? 0x10 : 0;
target/i386/cpu.c-7143- *ebx = sev_get_cbit_position() & 0x3f; /* EBX[5:0] */ target/i386/cpu.c-7144- *ebx |= (sev_get_reduced_phys_bits() & 0x3f) << 6; /* EBX[11:6] */
target/i386/cpu.c-7145-        }
target/i386/cpu.c-7146-        break;

but maybe I can use #ifdef'ry around CONFIG_USER_ONLY like
with SGX:

    case 0x12:
#ifndef CONFIG_USER_ONLY
        if (count > 1) {
            uint64_t epc_addr, epc_size;

            if (sgx_epc_get_section(count - 2, &epc_addr, &epc_size)) {
                *eax = *ebx = *ecx = *edx = 0;
                break;
            }
            ...
#endif
        break;


#define TYPE_SEV_COMMON "sev-common"
  #define TYPE_SEV_GUEST "sev-guest"
@@ -45,18 +55,6 @@ typedef struct SevKernelLoaderContext {
      size_t cmdline_size;
  } SevKernelLoaderContext;
-#ifdef CONFIG_SEV
-bool sev_enabled(void);
-bool sev_es_enabled(void);
-bool sev_snp_enabled(void);
-#else
-#define sev_enabled() 0
-#define sev_es_enabled() 0
-#define sev_snp_enabled() 0
-#endif
-
-uint32_t sev_get_cbit_position(void);
-uint32_t sev_get_reduced_phys_bits(void);
  bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
int sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp);




reply via email to

[Prev in Thread] Current Thread [Next in Thread]