On 10/4/21 10:19, Paolo Bonzini wrote:
> On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:
>> Only declare sev_enabled() and sev_es_enabled() when CONFIG_SEV is
>> set, to allow the compiler to elide unused code. Remove unnecessary
>> stubs.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
>> ---
>> include/sysemu/sev.h | 14 +++++++++++++-
>> target/i386/sev_i386.h | 3 ---
>> target/i386/cpu.c | 16 +++++++++-------
>> target/i386/sev-stub.c | 36 ------------------------------------
>> target/i386/meson.build | 2 +-
>> 5 files changed, 23 insertions(+), 48 deletions(-)
>> delete mode 100644 target/i386/sev-stub.c
>>
>> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
>> index a329ed75c1c..f5c625bb3b3 100644
>> --- a/include/sysemu/sev.h
>> +++ b/include/sysemu/sev.h
>> @@ -14,9 +14,21 @@
>> #ifndef QEMU_SEV_H
>> #define QEMU_SEV_H
>> -#include "sysemu/kvm.h"
>> +#ifndef CONFIG_USER_ONLY
>> +#include CONFIG_DEVICES /* CONFIG_SEV */
>> +#endif
>> +#ifdef CONFIG_SEV
>> bool sev_enabled(void);
>> +bool sev_es_enabled(void);
>> +#else
>> +#define sev_enabled() 0
>> +#define sev_es_enabled() 0
>> +#endif
>
> This means that sev.h can only be included from target-specific files.
>
> An alternative could be:
>
> #ifdef NEED_CPU_H
> # include CONFIG_DEVICES
<command-line>: fatal error: x86_64-linux-user-config-devices.h: No such
file or directory
> #endif
>
> #if defined NEED_CPU_H && !defined CONFIG_SEV
> # define sev_enabled() 0
> # define sev_es_enabled() 0
> #else
> bool sev_enabled(void);
> bool sev_es_enabled(void);
> #endif
>
> ... but in fact sysemu/sev.h _is_ only used from x86-specific files. So
> should it be moved to include/hw/i386, and even merged with
> target/i386/sev_i386.h? Do we need two files?
No clue, I don't think we need. Brijesh?
>
> Thanks,
>
> Paolo
>
>> +uint32_t sev_get_cbit_position(void);
>> +uint32_t sev_get_reduced_phys_bits(void);
>> +
>> int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
>> #endif
>> diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
>> index 0798ab3519a..2d9a1a0112e 100644
>> --- a/target/i386/sev_i386.h
>> +++ b/target/i386/sev_i386.h
>> @@ -24,10 +24,7 @@
>> #define SEV_POLICY_DOMAIN 0x10
>> #define SEV_POLICY_SEV 0x20
>> -extern bool sev_es_enabled(void);
>> extern SevInfo *sev_get_info(void);
>> -extern uint32_t sev_get_cbit_position(void);
>> -extern uint32_t sev_get_reduced_phys_bits(void);
>> extern char *sev_get_launch_measurement(void);
>> extern SevCapability *sev_get_capabilities(Error **errp);
>> extern SevAttestationReport *
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index e169a01713d..27992bdc9f8 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -25,8 +25,8 @@
>> #include "tcg/helper-tcg.h"
>> #include "sysemu/reset.h"
>> #include "sysemu/hvf.h"
>> +#include "sysemu/sev.h"
>> #include "kvm/kvm_i386.h"
>> -#include "sev_i386.h"
>> #include "qapi/error.h"
>> #include "qapi/qapi-visit-machine.h"
>> #include "qapi/qmp/qerror.h"
>> @@ -38,6 +38,7 @@
>> #include "exec/address-spaces.h"
>> #include "hw/boards.h"
>> #include "hw/i386/sgx-epc.h"
>> +#include "sev_i386.h"
>> #endif
>> #include "disas/capstone.h"
>> @@ -5764,12 +5765,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>> index, uint32_t count,
>> *edx = 0;
>> break;
>> case 0x8000001F:
>> - *eax = sev_enabled() ? 0x2 : 0;
>> - *eax |= sev_es_enabled() ? 0x8 : 0;
>> - *ebx = sev_get_cbit_position();
>> - *ebx |= sev_get_reduced_phys_bits() << 6;
>> - *ecx = 0;
>> - *edx = 0;
>> + *eax = *ebx = *ecx = *edx = 0;
>> + if (sev_enabled()) {
>> + *eax = 0x2;
>> + *eax |= sev_es_enabled() ? 0x8 : 0;
>> + *ebx = sev_get_cbit_position();
>> + *ebx |= sev_get_reduced_phys_bits() << 6;
>> + }
>> break;
