[PATCH v4 00/23] target/i386/sev: Housekeeping SEV + measured Linux SEV guest

[PATCH v4 00/23] target/i386/sev: Housekeeping SEV + measured Linux SEV guest

[Philippe Mathieu-Daudé]

Missing review:
  0005-target-i386-sev-Prefix-QMP-errors-with-SEV.patch
  0012-target-i386-sev-Use-g_autofree-in-sev_launch_get_mea.patch
  0014-target-i386-sev-Rename-sev_i386.h-sev.h.patch
  0016-target-i386-sev-Remove-stubs-by-using-code-elision.patch
  0023-MAINTAINERS-Cover-SEV-related-files-with-X86-KVM-sec.patch

Hi,

While testing James & Dov patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg810571.html
I wasted some time trying to figure out how OVMF was supposed to
behave until realizing the binary I was using was built without SEV
support... Then wrote this series to help other developers to not
hit the same problem.

Since v3:
- Rebased ('Measured Linux SEV guest' from Dov [1] merged)
- Addressed Paolo & David review comments

Since v2:
- Rebased on top of SGX
- Addressed review comments from Markus / David
- Included/rebased 'Measured Linux SEV guest' from Dov [1]
- Added orphean MAINTAINERS section

[1] https://lore.kernel.org/qemu-devel/20210825073538.959525-1-dovmurik@linux.ibm.com/

Supersedes: <20210616204328.2611406-1-philmd@redhat.com>

Dr. David Alan Gilbert (1):
  target/i386/sev: sev_get_attestation_report use g_autofree

Philippe Mathieu-Daudé (22):
  qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
  qapi/misc-target: Group SEV QAPI definitions
  target/i386/kvm: Introduce i386_softmmu_kvm Meson source set
  target/i386/kvm: Restrict SEV stubs to x86 architecture
  target/i386/sev: Prefix QMP errors with 'SEV'
  target/i386/monitor: Return QMP error when SEV is not enabled for
    guest
  target/i386/cpu: Add missing 'qapi/error.h' header
  target/i386/sev_i386.h: Remove unused headers
  target/i386/sev: Remove sev_get_me_mask()
  target/i386/sev: Mark unreachable code with g_assert_not_reached()
  target/i386/sev: Use g_autofree in sev_launch_get_measure()
  target/i386/sev: Restrict SEV to system emulation
  target/i386/sev: Rename sev_i386.h -> sev.h
  target/i386/sev: Declare system-specific functions in 'sev.h'
  target/i386/sev: Remove stubs by using code elision
  target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c
  target/i386/sev: Move qmp_sev_inject_launch_secret() to sev.c
  target/i386/sev: Move qmp_query_sev_capabilities() to sev.c
  target/i386/sev: Move qmp_query_sev_launch_measure() to sev.c
  target/i386/sev: Move qmp_query_sev() & hmp_info_sev() to sev.c
  monitor: Reduce hmp_info_sev() declaration
  MAINTAINERS: Cover SEV-related files with X86/KVM section

 qapi/misc-target.json                 |  77 ++++++------
 include/monitor/hmp-target.h          |   1 +
 include/monitor/hmp.h                 |   1 -
 include/sysemu/sev.h                  |  28 -----
 target/i386/{sev_i386.h => sev.h}     |  35 ++++--
 hw/i386/pc_sysfw.c                    |   2 +-
 hw/i386/x86.c                         |   2 +-
 target/i386/cpu.c                     |  16 +--
 target/i386/kvm/kvm.c                 |   3 +-
 {accel => target/i386}/kvm/sev-stub.c |   2 +-
 target/i386/monitor.c                 |  92 +--------------
 target/i386/sev-stub.c                |  88 --------------
 target/i386/sev-sysemu-stub.c         |  70 +++++++++++
 target/i386/sev.c                     | 164 +++++++++++++++++++-------
 MAINTAINERS                           |   2 +
 accel/kvm/meson.build                 |   1 -
 target/i386/kvm/meson.build           |   8 +-
 target/i386/meson.build               |   4 +-
 18 files changed, 279 insertions(+), 317 deletions(-)
 delete mode 100644 include/sysemu/sev.h
 rename target/i386/{sev_i386.h => sev.h} (62%)
 rename {accel => target/i386}/kvm/sev-stub.c (94%)
 delete mode 100644 target/i386/sev-stub.c
 create mode 100644 target/i386/sev-sysemu-stub.c

-- 
2.31.1

[PATCH v4 00/23] target/i386/sev: Housekeeping SEV + measured Linux SEV guest

[Philippe Mathieu-Daudé]

Missing review:
  0005-target-i386-sev-Prefix-QMP-errors-with-SEV.patch
  0012-target-i386-sev-Use-g_autofree-in-sev_launch_get_mea.patch
  0014-target-i386-sev-Rename-sev_i386.h-sev.h.patch
  0016-target-i386-sev-Remove-stubs-by-using-code-elision.patch
  0023-MAINTAINERS-Cover-SEV-related-files-with-X86-KVM-sec.patch

Hi,

While testing James & Dov patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg810571.html
I wasted some time trying to figure out how OVMF was supposed to
behave until realizing the binary I was using was built without SEV
support... Then wrote this series to help other developers to not
hit the same problem.

Since v3:
- Rebased ('Measured Linux SEV guest' from Dov [1] merged)
- Addressed Paolo & David review comments

Since v2:
- Rebased on top of SGX
- Addressed review comments from Markus / David
- Included/rebased 'Measured Linux SEV guest' from Dov [1]
- Added orphean MAINTAINERS section

[1] https://lore.kernel.org/qemu-devel/20210825073538.959525-1-dovmurik@linux.ibm.com/

Supersedes: <20210616204328.2611406-1-philmd@redhat.com>

Dr. David Alan Gilbert (1):
  target/i386/sev: sev_get_attestation_report use g_autofree

Philippe Mathieu-Daudé (22):
  qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
  qapi/misc-target: Group SEV QAPI definitions
  target/i386/kvm: Introduce i386_softmmu_kvm Meson source set
  target/i386/kvm: Restrict SEV stubs to x86 architecture
  target/i386/sev: Prefix QMP errors with 'SEV'
  target/i386/monitor: Return QMP error when SEV is not enabled for
    guest
  target/i386/cpu: Add missing 'qapi/error.h' header
  target/i386/sev_i386.h: Remove unused headers
  target/i386/sev: Remove sev_get_me_mask()
  target/i386/sev: Mark unreachable code with g_assert_not_reached()
  target/i386/sev: Use g_autofree in sev_launch_get_measure()
  target/i386/sev: Restrict SEV to system emulation
  target/i386/sev: Rename sev_i386.h -> sev.h
  target/i386/sev: Declare system-specific functions in 'sev.h'
  target/i386/sev: Remove stubs by using code elision
  target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c
  target/i386/sev: Move qmp_sev_inject_launch_secret() to sev.c
  target/i386/sev: Move qmp_query_sev_capabilities() to sev.c
  target/i386/sev: Move qmp_query_sev_launch_measure() to sev.c
  target/i386/sev: Move qmp_query_sev() & hmp_info_sev() to sev.c
  monitor: Reduce hmp_info_sev() declaration
  MAINTAINERS: Cover SEV-related files with X86/KVM section

 qapi/misc-target.json                 |  77 ++++++------
 include/monitor/hmp-target.h          |   1 +
 include/monitor/hmp.h                 |   1 -
 include/sysemu/sev.h                  |  28 -----
 target/i386/{sev_i386.h => sev.h}     |  35 ++++--
 hw/i386/pc_sysfw.c                    |   2 +-
 hw/i386/x86.c                         |   2 +-
 target/i386/cpu.c                     |  16 +--
 target/i386/kvm/kvm.c                 |   3 +-
 {accel => target/i386}/kvm/sev-stub.c |   2 +-
 target/i386/monitor.c                 |  92 +--------------
 target/i386/sev-stub.c                |  88 --------------
 target/i386/sev-sysemu-stub.c         |  70 +++++++++++
 target/i386/sev.c                     | 164 +++++++++++++++++++-------
 MAINTAINERS                           |   2 +
 accel/kvm/meson.build                 |   1 -
 target/i386/kvm/meson.build           |   8 +-
 target/i386/meson.build               |   4 +-
 18 files changed, 279 insertions(+), 317 deletions(-)
 delete mode 100644 include/sysemu/sev.h
 rename target/i386/{sev_i386.h => sev.h} (62%)
 rename {accel => target/i386}/kvm/sev-stub.c (94%)
 delete mode 100644 target/i386/sev-stub.c
 create mode 100644 target/i386/sev-sysemu-stub.c

-- 
2.31.1

[PATCH v4 01/23] qapi/misc-target: Wrap long 'SEV Attestation Report' long lines

[Philippe Mathieu-Daudé]

Wrap long lines before 70 characters for legibility.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/misc-target.json | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 594fbd1577f..ae5577e0390 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -300,8 +300,8 @@
 ##
 # @SevAttestationReport:
 #
-# The struct describes attestation report for a Secure Encrypted Virtualization
-# feature.
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
 #
 # @data:  guest attestation report (base64 encoded)
 #
@@ -315,10 +315,11 @@
 ##
 # @query-sev-attestation-report:
 #
-# This command is used to get the SEV attestation report, and is supported on AMD
-# X86 platforms only.
+# This command is used to get the SEV attestation report, and is
+# supported on AMD X86 platforms only.
 #
-# @mnonce: a random 16 bytes value encoded in base64 (it will be included in report)
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+#          included in report)
 #
 # Returns: SevAttestationReport objects.
 #
@@ -326,11 +327,13 @@
 #
 # Example:
 #
-# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
+# -> { "execute" : "query-sev-attestation-report",
+#                  "arguments": { "mnonce": "aaaaaaa" } }
 # <- { "return" : { "data": "aaaaaaaabbbddddd"} }
 #
 ##
-{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
+{ 'command': 'query-sev-attestation-report',
+  'data': { 'mnonce': 'str' },
   'returns': 'SevAttestationReport',
   'if': 'TARGET_I386' }
 
-- 
2.31.1

[PATCH v4 01/23] qapi/misc-target: Wrap long 'SEV Attestation Report' long lines

[Philippe Mathieu-Daudé]

Wrap long lines before 70 characters for legibility.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/misc-target.json | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 594fbd1577f..ae5577e0390 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -300,8 +300,8 @@
 ##
 # @SevAttestationReport:
 #
-# The struct describes attestation report for a Secure Encrypted Virtualization
-# feature.
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
 #
 # @data:  guest attestation report (base64 encoded)
 #
@@ -315,10 +315,11 @@
 ##
 # @query-sev-attestation-report:
 #
-# This command is used to get the SEV attestation report, and is supported on AMD
-# X86 platforms only.
+# This command is used to get the SEV attestation report, and is
+# supported on AMD X86 platforms only.
 #
-# @mnonce: a random 16 bytes value encoded in base64 (it will be included in report)
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+#          included in report)
 #
 # Returns: SevAttestationReport objects.
 #
@@ -326,11 +327,13 @@
 #
 # Example:
 #
-# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
+# -> { "execute" : "query-sev-attestation-report",
+#                  "arguments": { "mnonce": "aaaaaaa" } }
 # <- { "return" : { "data": "aaaaaaaabbbddddd"} }
 #
 ##
-{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
+{ 'command': 'query-sev-attestation-report',
+  'data': { 'mnonce': 'str' },
   'returns': 'SevAttestationReport',
   'if': 'TARGET_I386' }
 
-- 
2.31.1

[PATCH v4 02/23] qapi/misc-target: Group SEV QAPI definitions

[Philippe Mathieu-Daudé]

There is already a section with various SEV commands / types,
so move the SEV guest attestation together.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/misc-target.json | 80 +++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index ae5577e0390..5aa2b95b7d4 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -229,6 +229,46 @@
   'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
   'if': 'TARGET_I386' }
 
+##
+# @SevAttestationReport:
+#
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
+#
+# @data:  guest attestation report (base64 encoded)
+#
+#
+# Since: 6.1
+##
+{ 'struct': 'SevAttestationReport',
+  'data': { 'data': 'str'},
+  'if': 'TARGET_I386' }
+
+##
+# @query-sev-attestation-report:
+#
+# This command is used to get the SEV attestation report, and is
+# supported on AMD X86 platforms only.
+#
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+#          included in report)
+#
+# Returns: SevAttestationReport objects.
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute" : "query-sev-attestation-report",
+#                  "arguments": { "mnonce": "aaaaaaa" } }
+# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
+#
+##
+{ 'command': 'query-sev-attestation-report',
+  'data': { 'mnonce': 'str' },
+  'returns': 'SevAttestationReport',
+  'if': 'TARGET_I386' }
+
 ##
 # @dump-skeys:
 #
@@ -297,46 +337,6 @@
   'if': 'TARGET_ARM' }
 
 
-##
-# @SevAttestationReport:
-#
-# The struct describes attestation report for a Secure Encrypted
-# Virtualization feature.
-#
-# @data:  guest attestation report (base64 encoded)
-#
-#
-# Since: 6.1
-##
-{ 'struct': 'SevAttestationReport',
-  'data': { 'data': 'str'},
-  'if': 'TARGET_I386' }
-
-##
-# @query-sev-attestation-report:
-#
-# This command is used to get the SEV attestation report, and is
-# supported on AMD X86 platforms only.
-#
-# @mnonce: a random 16 bytes value encoded in base64 (it will be
-#          included in report)
-#
-# Returns: SevAttestationReport objects.
-#
-# Since: 6.1
-#
-# Example:
-#
-# -> { "execute" : "query-sev-attestation-report",
-#                  "arguments": { "mnonce": "aaaaaaa" } }
-# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
-#
-##
-{ 'command': 'query-sev-attestation-report',
-  'data': { 'mnonce': 'str' },
-  'returns': 'SevAttestationReport',
-  'if': 'TARGET_I386' }
-
 ##
 # @SGXInfo:
 #
-- 
2.31.1

[PATCH v4 02/23] qapi/misc-target: Group SEV QAPI definitions

[Philippe Mathieu-Daudé]

There is already a section with various SEV commands / types,
so move the SEV guest attestation together.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/misc-target.json | 80 +++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index ae5577e0390..5aa2b95b7d4 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -229,6 +229,46 @@
   'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
   'if': 'TARGET_I386' }
 
+##
+# @SevAttestationReport:
+#
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
+#
+# @data:  guest attestation report (base64 encoded)
+#
+#
+# Since: 6.1
+##
+{ 'struct': 'SevAttestationReport',
+  'data': { 'data': 'str'},
+  'if': 'TARGET_I386' }
+
+##
+# @query-sev-attestation-report:
+#
+# This command is used to get the SEV attestation report, and is
+# supported on AMD X86 platforms only.
+#
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+#          included in report)
+#
+# Returns: SevAttestationReport objects.
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute" : "query-sev-attestation-report",
+#                  "arguments": { "mnonce": "aaaaaaa" } }
+# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
+#
+##
+{ 'command': 'query-sev-attestation-report',
+  'data': { 'mnonce': 'str' },
+  'returns': 'SevAttestationReport',
+  'if': 'TARGET_I386' }
+
 ##
 # @dump-skeys:
 #
@@ -297,46 +337,6 @@
   'if': 'TARGET_ARM' }
 
 
-##
-# @SevAttestationReport:
-#
-# The struct describes attestation report for a Secure Encrypted
-# Virtualization feature.
-#
-# @data:  guest attestation report (base64 encoded)
-#
-#
-# Since: 6.1
-##
-{ 'struct': 'SevAttestationReport',
-  'data': { 'data': 'str'},
-  'if': 'TARGET_I386' }
-
-##
-# @query-sev-attestation-report:
-#
-# This command is used to get the SEV attestation report, and is
-# supported on AMD X86 platforms only.
-#
-# @mnonce: a random 16 bytes value encoded in base64 (it will be
-#          included in report)
-#
-# Returns: SevAttestationReport objects.
-#
-# Since: 6.1
-#
-# Example:
-#
-# -> { "execute" : "query-sev-attestation-report",
-#                  "arguments": { "mnonce": "aaaaaaa" } }
-# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
-#
-##
-{ 'command': 'query-sev-attestation-report',
-  'data': { 'mnonce': 'str' },
-  'returns': 'SevAttestationReport',
-  'if': 'TARGET_I386' }
-
 ##
 # @SGXInfo:
 #
-- 
2.31.1

[PATCH v4 03/23] target/i386/kvm: Introduce i386_softmmu_kvm Meson source set

[Philippe Mathieu-Daudé]

Introduce the i386_softmmu_kvm Meson source set to be able to
add features dependent on CONFIG_KVM.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/kvm/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index 0a533411cab..b1c76957c76 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -1,8 +1,12 @@
 i386_ss.add(when: 'CONFIG_KVM', if_false: files('kvm-stub.c'))
 
-i386_softmmu_ss.add(when: 'CONFIG_KVM', if_true: files(
+i386_softmmu_kvm_ss = ss.source_set()
+
+i386_softmmu_kvm_ss.add(files(
   'kvm.c',
   'kvm-cpu.c',
 ))
 
 i386_softmmu_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), if_false: files('hyperv-stub.c'))
+
+i386_softmmu_ss.add_all(when: 'CONFIG_KVM', if_true: i386_softmmu_kvm_ss)
-- 
2.31.1

[PATCH v4 03/23] target/i386/kvm: Introduce i386_softmmu_kvm Meson source set

[Philippe Mathieu-Daudé]

Introduce the i386_softmmu_kvm Meson source set to be able to
add features dependent on CONFIG_KVM.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/kvm/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index 0a533411cab..b1c76957c76 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -1,8 +1,12 @@
 i386_ss.add(when: 'CONFIG_KVM', if_false: files('kvm-stub.c'))
 
-i386_softmmu_ss.add(when: 'CONFIG_KVM', if_true: files(
+i386_softmmu_kvm_ss = ss.source_set()
+
+i386_softmmu_kvm_ss.add(files(
   'kvm.c',
   'kvm-cpu.c',
 ))
 
 i386_softmmu_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), if_false: files('hyperv-stub.c'))
+
+i386_softmmu_ss.add_all(when: 'CONFIG_KVM', if_true: i386_softmmu_kvm_ss)
-- 
2.31.1

[PATCH v4 04/23] target/i386/kvm: Restrict SEV stubs to x86 architecture

[Philippe Mathieu-Daudé]

SEV is x86-specific, no need to add its stub to other
architectures. Move the stub file to target/i386/kvm/.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 {accel => target/i386}/kvm/sev-stub.c | 0
 accel/kvm/meson.build                 | 1 -
 target/i386/kvm/meson.build           | 2 ++
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename {accel => target/i386}/kvm/sev-stub.c (100%)

diff --git a/accel/kvm/sev-stub.c b/target/i386/kvm/sev-stub.c
similarity index 100%
rename from accel/kvm/sev-stub.c
rename to target/i386/kvm/sev-stub.c
diff --git a/accel/kvm/meson.build b/accel/kvm/meson.build
index 8d219bea507..397a1fe1fd1 100644
--- a/accel/kvm/meson.build
+++ b/accel/kvm/meson.build
@@ -3,6 +3,5 @@
   'kvm-all.c',
   'kvm-accel-ops.c',
 ))
-kvm_ss.add(when: 'CONFIG_SEV', if_false: files('sev-stub.c'))
 
 specific_ss.add_all(when: 'CONFIG_KVM', if_true: kvm_ss)
diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index b1c76957c76..736df8b72e3 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -7,6 +7,8 @@
   'kvm-cpu.c',
 ))
 
+i386_softmmu_kvm_ss.add(when: 'CONFIG_SEV', if_false: files('sev-stub.c'))
+
 i386_softmmu_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), if_false: files('hyperv-stub.c'))
 
 i386_softmmu_ss.add_all(when: 'CONFIG_KVM', if_true: i386_softmmu_kvm_ss)
-- 
2.31.1

[PATCH v4 04/23] target/i386/kvm: Restrict SEV stubs to x86 architecture

[Philippe Mathieu-Daudé]

SEV is x86-specific, no need to add its stub to other
architectures. Move the stub file to target/i386/kvm/.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 {accel => target/i386}/kvm/sev-stub.c | 0
 accel/kvm/meson.build                 | 1 -
 target/i386/kvm/meson.build           | 2 ++
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename {accel => target/i386}/kvm/sev-stub.c (100%)

diff --git a/accel/kvm/sev-stub.c b/target/i386/kvm/sev-stub.c
similarity index 100%
rename from accel/kvm/sev-stub.c
rename to target/i386/kvm/sev-stub.c
diff --git a/accel/kvm/meson.build b/accel/kvm/meson.build
index 8d219bea507..397a1fe1fd1 100644
--- a/accel/kvm/meson.build
+++ b/accel/kvm/meson.build
@@ -3,6 +3,5 @@
   'kvm-all.c',
   'kvm-accel-ops.c',
 ))
-kvm_ss.add(when: 'CONFIG_SEV', if_false: files('sev-stub.c'))
 
 specific_ss.add_all(when: 'CONFIG_KVM', if_true: kvm_ss)
diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index b1c76957c76..736df8b72e3 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -7,6 +7,8 @@
   'kvm-cpu.c',
 ))
 
+i386_softmmu_kvm_ss.add(when: 'CONFIG_SEV', if_false: files('sev-stub.c'))
+
 i386_softmmu_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), if_false: files('hyperv-stub.c'))
 
 i386_softmmu_ss.add_all(when: 'CONFIG_KVM', if_true: i386_softmmu_kvm_ss)
-- 
2.31.1

[PATCH v4 05/23] target/i386/sev: Prefix QMP errors with 'SEV'

[Philippe Mathieu-Daudé]

Multiple errors might be reported to the monitor,
better to prefix the SEV ones so we can distinct them.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/monitor.c |  2 +-
 target/i386/sev.c     | 20 +++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 196c1c9e77f..eabbeb9be95 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -717,7 +717,7 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
 
     data = sev_get_launch_measurement();
     if (!data) {
-        error_setg(errp, "Measurement is not available");
+        error_setg(errp, "SEV launch measurement is not available");
         return NULL;
     }
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index bcd9260fa46..4f1952cd32f 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -440,7 +440,8 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
     r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
     if (r < 0) {
         if (err != SEV_RET_INVALID_LEN) {
-            error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
+            error_setg(errp, "SEV: Failed to export PDH cert"
+                             " ret=%d fw_err=%d (%s)",
                        r, err, fw_error_to_str(err));
             return 1;
         }
@@ -453,7 +454,7 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
 
     r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
     if (r < 0) {
-        error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
+        error_setg(errp, "SEV: Failed to export PDH cert ret=%d fw_err=%d (%s)",
                    r, err, fw_error_to_str(err));
         goto e_free;
     }
@@ -491,7 +492,7 @@ sev_get_capabilities(Error **errp)
 
     fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
     if (fd < 0) {
-        error_setg_errno(errp, errno, "Failed to open %s",
+        error_setg_errno(errp, errno, "SEV: Failed to open %s",
                          DEFAULT_SEV_DEVICE);
         return NULL;
     }
@@ -557,8 +558,9 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
             &input, &err);
     if (ret < 0) {
         if (err != SEV_RET_INVALID_LEN) {
-            error_setg(errp, "failed to query the attestation report length "
-                    "ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
+            error_setg(errp, "SEV: Failed to query the attestation report"
+                             " length ret=%d fw_err=%d (%s)",
+                       ret, err, fw_error_to_str(err));
             g_free(buf);
             return NULL;
         }
@@ -572,7 +574,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
             &input, &err);
     if (ret) {
-        error_setg_errno(errp, errno, "Failed to get attestation report"
+        error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
                 " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
         goto e_free_data;
     }
@@ -596,7 +598,7 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
     GError *error = NULL;
 
     if (!g_file_get_contents(filename, &base64, &sz, &error)) {
-        error_report("failed to read '%s' (%s)", filename, error->message);
+        error_report("SEV: Failed to read '%s' (%s)", filename, error->message);
         g_error_free(error);
         return -1;
     }
@@ -911,7 +913,7 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
     if (sev_check_state(sev_guest, SEV_STATE_LAUNCH_UPDATE)) {
         int ret = sev_launch_update_data(sev_guest, ptr, len);
         if (ret < 0) {
-            error_setg(errp, "failed to encrypt pflash rom");
+            error_setg(errp, "SEV: Failed to encrypt pflash rom");
             return ret;
         }
     }
@@ -930,7 +932,7 @@ int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
     MemoryRegion *mr = NULL;
 
     if (!sev_guest) {
-        error_setg(errp, "SEV: SEV not enabled.");
+        error_setg(errp, "SEV not enabled for guest");
         return 1;
     }
 
-- 
2.31.1

[PATCH v4 05/23] target/i386/sev: Prefix QMP errors with 'SEV'

[Philippe Mathieu-Daudé]

Multiple errors might be reported to the monitor,
better to prefix the SEV ones so we can distinct them.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/monitor.c |  2 +-
 target/i386/sev.c     | 20 +++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 196c1c9e77f..eabbeb9be95 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -717,7 +717,7 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
 
     data = sev_get_launch_measurement();
     if (!data) {
-        error_setg(errp, "Measurement is not available");
+        error_setg(errp, "SEV launch measurement is not available");
         return NULL;
     }
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index bcd9260fa46..4f1952cd32f 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -440,7 +440,8 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
     r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
     if (r < 0) {
         if (err != SEV_RET_INVALID_LEN) {
-            error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
+            error_setg(errp, "SEV: Failed to export PDH cert"
+                             " ret=%d fw_err=%d (%s)",
                        r, err, fw_error_to_str(err));
             return 1;
         }
@@ -453,7 +454,7 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
 
     r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
     if (r < 0) {
-        error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
+        error_setg(errp, "SEV: Failed to export PDH cert ret=%d fw_err=%d (%s)",
                    r, err, fw_error_to_str(err));
         goto e_free;
     }
@@ -491,7 +492,7 @@ sev_get_capabilities(Error **errp)
 
     fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
     if (fd < 0) {
-        error_setg_errno(errp, errno, "Failed to open %s",
+        error_setg_errno(errp, errno, "SEV: Failed to open %s",
                          DEFAULT_SEV_DEVICE);
         return NULL;
     }
@@ -557,8 +558,9 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
             &input, &err);
     if (ret < 0) {
         if (err != SEV_RET_INVALID_LEN) {
-            error_setg(errp, "failed to query the attestation report length "
-                    "ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
+            error_setg(errp, "SEV: Failed to query the attestation report"
+                             " length ret=%d fw_err=%d (%s)",
+                       ret, err, fw_error_to_str(err));
             g_free(buf);
             return NULL;
         }
@@ -572,7 +574,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
             &input, &err);
     if (ret) {
-        error_setg_errno(errp, errno, "Failed to get attestation report"
+        error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
                 " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
         goto e_free_data;
     }
@@ -596,7 +598,7 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
     GError *error = NULL;
 
     if (!g_file_get_contents(filename, &base64, &sz, &error)) {
-        error_report("failed to read '%s' (%s)", filename, error->message);
+        error_report("SEV: Failed to read '%s' (%s)", filename, error->message);
         g_error_free(error);
         return -1;
     }
@@ -911,7 +913,7 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
     if (sev_check_state(sev_guest, SEV_STATE_LAUNCH_UPDATE)) {
         int ret = sev_launch_update_data(sev_guest, ptr, len);
         if (ret < 0) {
-            error_setg(errp, "failed to encrypt pflash rom");
+            error_setg(errp, "SEV: Failed to encrypt pflash rom");
             return ret;
         }
     }
@@ -930,7 +932,7 @@ int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
     MemoryRegion *mr = NULL;
 
     if (!sev_guest) {
-        error_setg(errp, "SEV: SEV not enabled.");
+        error_setg(errp, "SEV not enabled for guest");
         return 1;
     }
 
-- 
2.31.1

[PATCH v4 06/23] target/i386/monitor: Return QMP error when SEV is not enabled for guest

[Philippe Mathieu-Daudé]

If the management layer tries to inject a secret, it gets an empty
response in case the guest doesn't have SEV enabled, or the binary
is built without SEV:

  { "execute": "sev-inject-launch-secret",
    "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "return": {
      }
  }

Make it clearer by returning an error:

  { "execute": "sev-inject-launch-secret",
    "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "error": {
          "class": "GenericError",
          "desc": "SEV not enabled for guest"
      }
  }

Note: we will remove the sev_inject_launch_secret() stub in few commits,
      so we don't bother to add error_setg() there.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/monitor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index eabbeb9be95..ea836678f51 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -28,6 +28,7 @@
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qerror.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sev.h"
 #include "qapi/error.h"
@@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
                                   bool has_gpa, uint64_t gpa,
                                   Error **errp)
 {
+    if (!sev_enabled()) {
+        error_setg(errp, "SEV not enabled for guest");
+        return;
+    }
     if (!has_gpa) {
         uint8_t *data;
         struct sev_secret_area *area;
-- 
2.31.1

[PATCH v4 06/23] target/i386/monitor: Return QMP error when SEV is not enabled for guest

[Philippe Mathieu-Daudé]

If the management layer tries to inject a secret, it gets an empty
response in case the guest doesn't have SEV enabled, or the binary
is built without SEV:

  { "execute": "sev-inject-launch-secret",
    "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "return": {
      }
  }

Make it clearer by returning an error:

  { "execute": "sev-inject-launch-secret",
    "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "error": {
          "class": "GenericError",
          "desc": "SEV not enabled for guest"
      }
  }

Note: we will remove the sev_inject_launch_secret() stub in few commits,
      so we don't bother to add error_setg() there.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/monitor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index eabbeb9be95..ea836678f51 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -28,6 +28,7 @@
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qerror.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sev.h"
 #include "qapi/error.h"
@@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
                                   bool has_gpa, uint64_t gpa,
                                   Error **errp)
 {
+    if (!sev_enabled()) {
+        error_setg(errp, "SEV not enabled for guest");
+        return;
+    }
     if (!has_gpa) {
         uint8_t *data;
         struct sev_secret_area *area;
-- 
2.31.1

[PATCH v4 07/23] target/i386/cpu: Add missing 'qapi/error.h' header

[Philippe Mathieu-Daudé]

Commit 00b81053244 ("target-i386: Remove assert_no_error usage")
forgot to add the "qapi/error.h" for &error_abort, add it now.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a7b1b6aa93a..b54b98551e9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -27,6 +27,7 @@
 #include "sysemu/hvf.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"
 #include "qapi/qapi-commands-machine-target.h"
-- 
2.31.1

[PATCH v4 07/23] target/i386/cpu: Add missing 'qapi/error.h' header

[Philippe Mathieu-Daudé]

Commit 00b81053244 ("target-i386: Remove assert_no_error usage")
forgot to add the "qapi/error.h" for &error_abort, add it now.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a7b1b6aa93a..b54b98551e9 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -27,6 +27,7 @@
 #include "sysemu/hvf.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"
 #include "qapi/qapi-commands-machine-target.h"
-- 
2.31.1

[PATCH v4 08/23] target/i386/sev_i386.h: Remove unused headers

[Philippe Mathieu-Daudé]

Declarations don't require these headers, remove them.

Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev_i386.h | 4 ----
 target/i386/sev-stub.c | 1 +
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 2afe1080690..9bf6cd18789 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -14,11 +14,7 @@
 #ifndef QEMU_SEV_I386_H
 #define QEMU_SEV_I386_H
 
-#include "qom/object.h"
-#include "qapi/error.h"
-#include "sysemu/kvm.h"
 #include "sysemu/sev.h"
-#include "qemu/error-report.h"
 #include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index d8e65831714..408441768dc 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "sev_i386.h"
 
 SevInfo *sev_get_info(void)
-- 
2.31.1

[PATCH v4 08/23] target/i386/sev_i386.h: Remove unused headers

[Philippe Mathieu-Daudé]

Declarations don't require these headers, remove them.

Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev_i386.h | 4 ----
 target/i386/sev-stub.c | 1 +
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 2afe1080690..9bf6cd18789 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -14,11 +14,7 @@
 #ifndef QEMU_SEV_I386_H
 #define QEMU_SEV_I386_H
 
-#include "qom/object.h"
-#include "qapi/error.h"
-#include "sysemu/kvm.h"
 #include "sysemu/sev.h"
-#include "qemu/error-report.h"
 #include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index d8e65831714..408441768dc 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "sev_i386.h"
 
 SevInfo *sev_get_info(void)
-- 
2.31.1

[PATCH v4 09/23] target/i386/sev: Remove sev_get_me_mask()

[Philippe Mathieu-Daudé]

Unused dead code makes review harder, so remove it.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev_i386.h | 1 -
 target/i386/sev-stub.c | 5 -----
 target/i386/sev.c      | 9 ---------
 3 files changed, 15 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 9bf6cd18789..d83428fa265 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -36,7 +36,6 @@ typedef struct SevKernelLoaderContext {
 } SevKernelLoaderContext;
 
 extern bool sev_es_enabled(void);
-extern uint64_t sev_get_me_mask(void);
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 408441768dc..20b1e18ec1b 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -25,11 +25,6 @@ bool sev_enabled(void)
     return false;
 }
 
-uint64_t sev_get_me_mask(void)
-{
-    return ~0;
-}
-
 uint32_t sev_get_cbit_position(void)
 {
     return 0;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 4f1952cd32f..9e3f2ec8dd3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -65,7 +65,6 @@ struct SevGuestState {
     uint8_t api_major;
     uint8_t api_minor;
     uint8_t build_id;
-    uint64_t me_mask;
     int sev_fd;
     SevState state;
     gchar *measurement;
@@ -389,12 +388,6 @@ sev_es_enabled(void)
     return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
 }
 
-uint64_t
-sev_get_me_mask(void)
-{
-    return sev_guest ? sev_guest->me_mask : ~0;
-}
-
 uint32_t
 sev_get_cbit_position(void)
 {
@@ -833,8 +826,6 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
         goto err;
     }
 
-    sev->me_mask = ~(1UL << sev->cbitpos);
-
     devname = object_property_get_str(OBJECT(sev), "sev-device", NULL);
     sev->sev_fd = open(devname, O_RDWR);
     if (sev->sev_fd < 0) {
-- 
2.31.1

[PATCH v4 09/23] target/i386/sev: Remove sev_get_me_mask()

[Philippe Mathieu-Daudé]

Unused dead code makes review harder, so remove it.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev_i386.h | 1 -
 target/i386/sev-stub.c | 5 -----
 target/i386/sev.c      | 9 ---------
 3 files changed, 15 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 9bf6cd18789..d83428fa265 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -36,7 +36,6 @@ typedef struct SevKernelLoaderContext {
 } SevKernelLoaderContext;
 
 extern bool sev_es_enabled(void);
-extern uint64_t sev_get_me_mask(void);
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 408441768dc..20b1e18ec1b 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -25,11 +25,6 @@ bool sev_enabled(void)
     return false;
 }
 
-uint64_t sev_get_me_mask(void)
-{
-    return ~0;
-}
-
 uint32_t sev_get_cbit_position(void)
 {
     return 0;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 4f1952cd32f..9e3f2ec8dd3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -65,7 +65,6 @@ struct SevGuestState {
     uint8_t api_major;
     uint8_t api_minor;
     uint8_t build_id;
-    uint64_t me_mask;
     int sev_fd;
     SevState state;
     gchar *measurement;
@@ -389,12 +388,6 @@ sev_es_enabled(void)
     return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
 }
 
-uint64_t
-sev_get_me_mask(void)
-{
-    return sev_guest ? sev_guest->me_mask : ~0;
-}
-
 uint32_t
 sev_get_cbit_position(void)
 {
@@ -833,8 +826,6 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
         goto err;
     }
 
-    sev->me_mask = ~(1UL << sev->cbitpos);
-
     devname = object_property_get_str(OBJECT(sev), "sev-device", NULL);
     sev->sev_fd = open(devname, O_RDWR);
     if (sev->sev_fd < 0) {
-- 
2.31.1

[PATCH v4 10/23] target/i386/sev: Mark unreachable code with g_assert_not_reached()

[Philippe Mathieu-Daudé]

The unique sev_encrypt_flash() invocation (in pc_system_flash_map)
is protected by the "if (sev_enabled())" check, so is not
reacheable.
Replace the abort() call in sev_es_save_reset_vector() by
g_assert_not_reached() which meaning is clearer.

Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev-stub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 20b1e18ec1b..55f1ec74196 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -54,7 +54,7 @@ int sev_inject_launch_secret(const char *hdr, const char *secret,
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
 {
-    return 0;
+    g_assert_not_reached();
 }
 
 bool sev_es_enabled(void)
@@ -68,7 +68,7 @@ void sev_es_set_reset_vector(CPUState *cpu)
 
 int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
 {
-    abort();
+    g_assert_not_reached();
 }
 
 SevAttestationReport *
-- 
2.31.1

[PATCH v4 10/23] target/i386/sev: Mark unreachable code with g_assert_not_reached()

[Philippe Mathieu-Daudé]

The unique sev_encrypt_flash() invocation (in pc_system_flash_map)
is protected by the "if (sev_enabled())" check, so is not
reacheable.
Replace the abort() call in sev_es_save_reset_vector() by
g_assert_not_reached() which meaning is clearer.

Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev-stub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 20b1e18ec1b..55f1ec74196 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -54,7 +54,7 @@ int sev_inject_launch_secret(const char *hdr, const char *secret,
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
 {
-    return 0;
+    g_assert_not_reached();
 }
 
 bool sev_es_enabled(void)
@@ -68,7 +68,7 @@ void sev_es_set_reset_vector(CPUState *cpu)
 
 int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
 {
-    abort();
+    g_assert_not_reached();
 }
 
 SevAttestationReport *
-- 
2.31.1

[PATCH v4 11/23] target/i386/sev: sev_get_attestation_report use g_autofree

[Philippe Mathieu-Daudé]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Removes a whole bunch of g_free's and a goto.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Message-Id: <20210603113017.34922-1-dgilbert@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 9e3f2ec8dd3..3a30ba6d94a 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -521,8 +521,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     struct kvm_sev_attestation_report input = {};
     SevAttestationReport *report = NULL;
     SevGuestState *sev = sev_guest;
-    guchar *data;
-    guchar *buf;
+    g_autofree guchar *data = NULL;
+    g_autofree guchar *buf = NULL;
     gsize len;
     int err = 0, ret;
 
@@ -542,7 +542,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     if (len != sizeof(input.mnonce)) {
         error_setg(errp, "SEV: mnonce must be %zu bytes (got %" G_GSIZE_FORMAT ")",
                 sizeof(input.mnonce), len);
-        g_free(buf);
         return NULL;
     }
 
@@ -554,7 +553,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
             error_setg(errp, "SEV: Failed to query the attestation report"
                              " length ret=%d fw_err=%d (%s)",
                        ret, err, fw_error_to_str(err));
-            g_free(buf);
             return NULL;
         }
     }
@@ -569,7 +567,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     if (ret) {
         error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
                 " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
-        goto e_free_data;
+        return NULL;
     }
 
     report = g_new0(SevAttestationReport, 1);
@@ -577,9 +575,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
 
     trace_kvm_sev_attestation_report(mnonce, report->data);
 
-e_free_data:
-    g_free(data);
-    g_free(buf);
     return report;
 }
 
-- 
2.31.1

[PATCH v4 11/23] target/i386/sev: sev_get_attestation_report use g_autofree

[Philippe Mathieu-Daudé]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Removes a whole bunch of g_free's and a goto.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Message-Id: <20210603113017.34922-1-dgilbert@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 9e3f2ec8dd3..3a30ba6d94a 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -521,8 +521,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     struct kvm_sev_attestation_report input = {};
     SevAttestationReport *report = NULL;
     SevGuestState *sev = sev_guest;
-    guchar *data;
-    guchar *buf;
+    g_autofree guchar *data = NULL;
+    g_autofree guchar *buf = NULL;
     gsize len;
     int err = 0, ret;
 
@@ -542,7 +542,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     if (len != sizeof(input.mnonce)) {
         error_setg(errp, "SEV: mnonce must be %zu bytes (got %" G_GSIZE_FORMAT ")",
                 sizeof(input.mnonce), len);
-        g_free(buf);
         return NULL;
     }
 
@@ -554,7 +553,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
             error_setg(errp, "SEV: Failed to query the attestation report"
                              " length ret=%d fw_err=%d (%s)",
                        ret, err, fw_error_to_str(err));
-            g_free(buf);
             return NULL;
         }
     }
@@ -569,7 +567,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     if (ret) {
         error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
                 " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
-        goto e_free_data;
+        return NULL;
     }
 
     report = g_new0(SevAttestationReport, 1);
@@ -577,9 +575,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
 
     trace_kvm_sev_attestation_report(mnonce, report->data);
 
-e_free_data:
-    g_free(data);
-    g_free(buf);
     return report;
 }
 
-- 
2.31.1

[PATCH v4 12/23] target/i386/sev: Use g_autofree in sev_launch_get_measure()

[Philippe Mathieu-Daudé]

Use g_autofree to remove a pair of g_free/goto.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 3a30ba6d94a..5cbbcf0bb93 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -685,8 +685,8 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
 {
     SevGuestState *sev = sev_guest;
     int ret, error;
-    guchar *data;
-    struct kvm_sev_launch_measure *measurement;
+    g_autofree guchar *data = NULL;
+    g_autofree struct kvm_sev_launch_measure *measurement = NULL;
 
     if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
         return;
@@ -708,7 +708,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     if (!measurement->len) {
         error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
                      __func__, ret, error, fw_error_to_str(errno));
-        goto free_measurement;
+        return;
     }
 
     data = g_new0(guchar, measurement->len);
@@ -720,7 +720,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     if (ret) {
         error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
                      __func__, ret, error, fw_error_to_str(errno));
-        goto free_data;
+        return;
     }
 
     sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
@@ -728,11 +728,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     /* encode the measurement value and emit the event */
     sev->measurement = g_base64_encode(data, measurement->len);
     trace_kvm_sev_launch_measurement(sev->measurement);
-
-free_data:
-    g_free(data);
-free_measurement:
-    g_free(measurement);
 }
 
 char *
-- 
2.31.1

[PATCH v4 12/23] target/i386/sev: Use g_autofree in sev_launch_get_measure()

[Philippe Mathieu-Daudé]

Use g_autofree to remove a pair of g_free/goto.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 3a30ba6d94a..5cbbcf0bb93 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -685,8 +685,8 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
 {
     SevGuestState *sev = sev_guest;
     int ret, error;
-    guchar *data;
-    struct kvm_sev_launch_measure *measurement;
+    g_autofree guchar *data = NULL;
+    g_autofree struct kvm_sev_launch_measure *measurement = NULL;
 
     if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
         return;
@@ -708,7 +708,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     if (!measurement->len) {
         error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
                      __func__, ret, error, fw_error_to_str(errno));
-        goto free_measurement;
+        return;
     }
 
     data = g_new0(guchar, measurement->len);
@@ -720,7 +720,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     if (ret) {
         error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
                      __func__, ret, error, fw_error_to_str(errno));
-        goto free_data;
+        return;
     }
 
     sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
@@ -728,11 +728,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     /* encode the measurement value and emit the event */
     sev->measurement = g_base64_encode(data, measurement->len);
     trace_kvm_sev_launch_measurement(sev->measurement);
-
-free_data:
-    g_free(data);
-free_measurement:
-    g_free(measurement);
 }
 
 char *
-- 
2.31.1

[PATCH v4 13/23] target/i386/sev: Restrict SEV to system emulation

[Philippe Mathieu-Daudé]

SEV is irrelevant on user emulation, so restrict it to sysemu.
Some stubs are still required because used in cpu.c by
x86_register_cpudef_types(), so move the sysemu specific stubs
to sev-sysemu-stub.c instead. This will allow us to simplify
monitor.c (which is not available in user emulation) in the
next commit.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev-stub.c        | 43 -------------------------
 target/i386/sev-sysemu-stub.c | 60 +++++++++++++++++++++++++++++++++++
 target/i386/meson.build       |  4 ++-
 3 files changed, 63 insertions(+), 44 deletions(-)
 create mode 100644 target/i386/sev-sysemu-stub.c

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 55f1ec74196..170e9f50fee 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -15,11 +15,6 @@
 #include "qapi/error.h"
 #include "sev_i386.h"
 
-SevInfo *sev_get_info(void)
-{
-    return NULL;
-}
-
 bool sev_enabled(void)
 {
     return false;
@@ -35,49 +30,11 @@ uint32_t sev_get_reduced_phys_bits(void)
     return 0;
 }
 
-char *sev_get_launch_measurement(void)
-{
-    return NULL;
-}
-
-SevCapability *sev_get_capabilities(Error **errp)
-{
-    error_setg(errp, "SEV is not available in this QEMU");
-    return NULL;
-}
-
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp)
-{
-    return 1;
-}
-
-int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
-{
-    g_assert_not_reached();
-}
-
 bool sev_es_enabled(void)
 {
     return false;
 }
 
-void sev_es_set_reset_vector(CPUState *cpu)
-{
-}
-
-int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
-{
-    g_assert_not_reached();
-}
-
-SevAttestationReport *
-sev_get_attestation_report(const char *mnonce, Error **errp)
-{
-    error_setg(errp, "SEV is not available in this QEMU");
-    return NULL;
-}
-
 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
 {
     g_assert_not_reached();
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
new file mode 100644
index 00000000000..d556b4f091f
--- /dev/null
+++ b/target/i386/sev-sysemu-stub.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU SEV system stub
+ *
+ * Copyright Advanced Micro Devices 2018
+ *
+ * Authors:
+ *      Brijesh Singh <brijesh.singh@amd.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/error.h"
+#include "sev_i386.h"
+
+SevInfo *sev_get_info(void)
+{
+    return NULL;
+}
+
+char *sev_get_launch_measurement(void)
+{
+    return NULL;
+}
+
+SevCapability *sev_get_capabilities(Error **errp)
+{
+    error_setg(errp, "SEV is not available in this QEMU");
+    return NULL;
+}
+
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+                             uint64_t gpa, Error **errp)
+{
+    return 1;
+}
+
+int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
+{
+    g_assert_not_reached();
+}
+
+void sev_es_set_reset_vector(CPUState *cpu)
+{
+}
+
+int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
+{
+    g_assert_not_reached();
+}
+
+SevAttestationReport *sev_get_attestation_report(const char *mnonce,
+                                                 Error **errp)
+{
+    error_setg(errp, "SEV is not available in this QEMU");
+    return NULL;
+}
diff --git a/target/i386/meson.build b/target/i386/meson.build
index dac19ec00d4..a4f45c3ec1d 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@
   'xsave_helper.c',
   'cpu-dump.c',
 ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c', 'sev.c'), if_false: files('sev-stub.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
 
 # x86 cpu type
 i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
@@ -20,6 +20,8 @@
   'monitor.c',
   'cpu-sysemu.c',
 ))
+i386_softmmu_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
+
 i386_user_ss = ss.source_set()
 
 subdir('kvm')
-- 
2.31.1

[PATCH v4 13/23] target/i386/sev: Restrict SEV to system emulation

[Philippe Mathieu-Daudé]

SEV is irrelevant on user emulation, so restrict it to sysemu.
Some stubs are still required because used in cpu.c by
x86_register_cpudef_types(), so move the sysemu specific stubs
to sev-sysemu-stub.c instead. This will allow us to simplify
monitor.c (which is not available in user emulation) in the
next commit.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev-stub.c        | 43 -------------------------
 target/i386/sev-sysemu-stub.c | 60 +++++++++++++++++++++++++++++++++++
 target/i386/meson.build       |  4 ++-
 3 files changed, 63 insertions(+), 44 deletions(-)
 create mode 100644 target/i386/sev-sysemu-stub.c

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 55f1ec74196..170e9f50fee 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -15,11 +15,6 @@
 #include "qapi/error.h"
 #include "sev_i386.h"
 
-SevInfo *sev_get_info(void)
-{
-    return NULL;
-}
-
 bool sev_enabled(void)
 {
     return false;
@@ -35,49 +30,11 @@ uint32_t sev_get_reduced_phys_bits(void)
     return 0;
 }
 
-char *sev_get_launch_measurement(void)
-{
-    return NULL;
-}
-
-SevCapability *sev_get_capabilities(Error **errp)
-{
-    error_setg(errp, "SEV is not available in this QEMU");
-    return NULL;
-}
-
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp)
-{
-    return 1;
-}
-
-int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
-{
-    g_assert_not_reached();
-}
-
 bool sev_es_enabled(void)
 {
     return false;
 }
 
-void sev_es_set_reset_vector(CPUState *cpu)
-{
-}
-
-int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
-{
-    g_assert_not_reached();
-}
-
-SevAttestationReport *
-sev_get_attestation_report(const char *mnonce, Error **errp)
-{
-    error_setg(errp, "SEV is not available in this QEMU");
-    return NULL;
-}
-
 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
 {
     g_assert_not_reached();
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
new file mode 100644
index 00000000000..d556b4f091f
--- /dev/null
+++ b/target/i386/sev-sysemu-stub.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU SEV system stub
+ *
+ * Copyright Advanced Micro Devices 2018
+ *
+ * Authors:
+ *      Brijesh Singh <brijesh.singh@amd.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/error.h"
+#include "sev_i386.h"
+
+SevInfo *sev_get_info(void)
+{
+    return NULL;
+}
+
+char *sev_get_launch_measurement(void)
+{
+    return NULL;
+}
+
+SevCapability *sev_get_capabilities(Error **errp)
+{
+    error_setg(errp, "SEV is not available in this QEMU");
+    return NULL;
+}
+
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+                             uint64_t gpa, Error **errp)
+{
+    return 1;
+}
+
+int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
+{
+    g_assert_not_reached();
+}
+
+void sev_es_set_reset_vector(CPUState *cpu)
+{
+}
+
+int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
+{
+    g_assert_not_reached();
+}
+
+SevAttestationReport *sev_get_attestation_report(const char *mnonce,
+                                                 Error **errp)
+{
+    error_setg(errp, "SEV is not available in this QEMU");
+    return NULL;
+}
diff --git a/target/i386/meson.build b/target/i386/meson.build
index dac19ec00d4..a4f45c3ec1d 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@
   'xsave_helper.c',
   'cpu-dump.c',
 ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c', 'sev.c'), if_false: files('sev-stub.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
 
 # x86 cpu type
 i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
@@ -20,6 +20,8 @@
   'monitor.c',
   'cpu-sysemu.c',
 ))
+i386_softmmu_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c'))
+
 i386_user_ss = ss.source_set()
 
 subdir('kvm')
-- 
2.31.1

[PATCH v4 14/23] target/i386/sev: Rename sev_i386.h -> sev.h

[Philippe Mathieu-Daudé]

SEV is a x86 specific feature, and the "sev_i386.h" header
is already in target/i386/. Rename it as "sev.h" to simplify.

Patch created mechanically using:

  $ git mv target/i386/sev_i386.h target/i386/sev.h
  $ sed -i s/sev_i386.h/sev.h/ $(git grep -l sev_i386.h)

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/{sev_i386.h => sev.h} | 0
 hw/i386/x86.c                     | 2 +-
 target/i386/cpu.c                 | 2 +-
 target/i386/kvm/kvm.c             | 2 +-
 target/i386/monitor.c             | 2 +-
 target/i386/sev-stub.c            | 2 +-
 target/i386/sev-sysemu-stub.c     | 2 +-
 target/i386/sev.c                 | 2 +-
 8 files changed, 7 insertions(+), 7 deletions(-)
 rename target/i386/{sev_i386.h => sev.h} (100%)

diff --git a/target/i386/sev_i386.h b/target/i386/sev.h
similarity index 100%
rename from target/i386/sev_i386.h
rename to target/i386/sev.h
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 0c7c054e3a0..76de7e2265e 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -47,7 +47,7 @@
 #include "hw/i386/fw_cfg.h"
 #include "hw/intc/i8259.h"
 #include "hw/rtc/mc146818rtc.h"
-#include "target/i386/sev_i386.h"
+#include "target/i386/sev.h"
 
 #include "hw/acpi/cpu_hotplug.h"
 #include "hw/irq.h"
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b54b98551e9..8289dc87bd5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -26,7 +26,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/hvf.h"
 #include "kvm/kvm_i386.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "qapi/error.h"
 #include "qapi/qapi-visit-machine.h"
 #include "qapi/qmp/qerror.h"
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index f25837f63f4..a5f6ff63c81 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -28,7 +28,7 @@
 #include "sysemu/kvm_int.h"
 #include "sysemu/runstate.h"
 #include "kvm_i386.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "hyperv.h"
 #include "hyperv-proto.h"
 
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index ea836678f51..109e4e61c0a 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -32,7 +32,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sev.h"
 #include "qapi/error.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qapi-commands-misc.h"
 #include "hw/i386/pc.h"
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 170e9f50fee..7e8b6f9a259 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "sev_i386.h"
+#include "sev.h"
 
 bool sev_enabled(void)
 {
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index d556b4f091f..8082781febf 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/error.h"
-#include "sev_i386.h"
+#include "sev.h"
 
 SevInfo *sev_get_info(void)
 {
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 5cbbcf0bb93..e43bbf3a17d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -25,7 +25,7 @@
 #include "qemu/uuid.h"
 #include "crypto/hash.h"
 #include "sysemu/kvm.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
 #include "trace.h"
-- 
2.31.1

[PATCH v4 14/23] target/i386/sev: Rename sev_i386.h -> sev.h

[Philippe Mathieu-Daudé]

SEV is a x86 specific feature, and the "sev_i386.h" header
is already in target/i386/. Rename it as "sev.h" to simplify.

Patch created mechanically using:

  $ git mv target/i386/sev_i386.h target/i386/sev.h
  $ sed -i s/sev_i386.h/sev.h/ $(git grep -l sev_i386.h)

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/{sev_i386.h => sev.h} | 0
 hw/i386/x86.c                     | 2 +-
 target/i386/cpu.c                 | 2 +-
 target/i386/kvm/kvm.c             | 2 +-
 target/i386/monitor.c             | 2 +-
 target/i386/sev-stub.c            | 2 +-
 target/i386/sev-sysemu-stub.c     | 2 +-
 target/i386/sev.c                 | 2 +-
 8 files changed, 7 insertions(+), 7 deletions(-)
 rename target/i386/{sev_i386.h => sev.h} (100%)

diff --git a/target/i386/sev_i386.h b/target/i386/sev.h
similarity index 100%
rename from target/i386/sev_i386.h
rename to target/i386/sev.h
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 0c7c054e3a0..76de7e2265e 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -47,7 +47,7 @@
 #include "hw/i386/fw_cfg.h"
 #include "hw/intc/i8259.h"
 #include "hw/rtc/mc146818rtc.h"
-#include "target/i386/sev_i386.h"
+#include "target/i386/sev.h"
 
 #include "hw/acpi/cpu_hotplug.h"
 #include "hw/irq.h"
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b54b98551e9..8289dc87bd5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -26,7 +26,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/hvf.h"
 #include "kvm/kvm_i386.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "qapi/error.h"
 #include "qapi/qapi-visit-machine.h"
 #include "qapi/qmp/qerror.h"
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index f25837f63f4..a5f6ff63c81 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -28,7 +28,7 @@
 #include "sysemu/kvm_int.h"
 #include "sysemu/runstate.h"
 #include "kvm_i386.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "hyperv.h"
 #include "hyperv-proto.h"
 
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index ea836678f51..109e4e61c0a 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -32,7 +32,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sev.h"
 #include "qapi/error.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qapi-commands-misc.h"
 #include "hw/i386/pc.h"
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 170e9f50fee..7e8b6f9a259 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "sev_i386.h"
+#include "sev.h"
 
 bool sev_enabled(void)
 {
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index d556b4f091f..8082781febf 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/error.h"
-#include "sev_i386.h"
+#include "sev.h"
 
 SevInfo *sev_get_info(void)
 {
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 5cbbcf0bb93..e43bbf3a17d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -25,7 +25,7 @@
 #include "qemu/uuid.h"
 #include "crypto/hash.h"
 #include "sysemu/kvm.h"
-#include "sev_i386.h"
+#include "sev.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
 #include "trace.h"
-- 
2.31.1

[PATCH v4 15/23] target/i386/sev: Declare system-specific functions in 'sev.h'

[Philippe Mathieu-Daudé]

"sysemu/sev.h" is only used from x86-specific files. Let's move it
to include/hw/i386, and merge it with target/i386/sev.h.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/sysemu/sev.h       | 28 ----------------------------
 target/i386/sev.h          | 12 +++++++++++-
 hw/i386/pc_sysfw.c         |  2 +-
 target/i386/kvm/kvm.c      |  1 -
 target/i386/kvm/sev-stub.c |  2 +-
 target/i386/monitor.c      |  1 -
 6 files changed, 13 insertions(+), 33 deletions(-)
 delete mode 100644 include/sysemu/sev.h

diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
deleted file mode 100644
index 94d821d737c..00000000000
--- a/include/sysemu/sev.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * QEMU Secure Encrypted Virutualization (SEV) support
- *
- * Copyright: Advanced Micro Devices, 2016-2018
- *
- * Authors:
- *  Brijesh Singh <brijesh.singh@amd.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_SEV_H
-#define QEMU_SEV_H
-
-#include "sysemu/kvm.h"
-
-bool sev_enabled(void);
-int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
-int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp);
-
-int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
-void sev_es_set_reset_vector(CPUState *cpu);
-
-#endif
diff --git a/target/i386/sev.h b/target/i386/sev.h
index d83428fa265..c96072bf78d 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -14,7 +14,7 @@
 #ifndef QEMU_SEV_I386_H
 #define QEMU_SEV_I386_H
 
-#include "sysemu/sev.h"
+#include "exec/confidential-guest-support.h"
 #include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
@@ -35,6 +35,7 @@ typedef struct SevKernelLoaderContext {
     size_t cmdline_size;
 } SevKernelLoaderContext;
 
+bool sev_enabled(void);
 extern bool sev_es_enabled(void);
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
@@ -45,4 +46,13 @@ extern SevAttestationReport *
 sev_get_attestation_report(const char *mnonce, Error **errp);
 extern bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
+int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+                             uint64_t gpa, Error **errp);
+
+int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
+void sev_es_set_reset_vector(CPUState *cpu);
+
+int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
+
 #endif
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 68d6b1f783e..c8b17af9535 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -37,7 +37,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/block/flash.h"
 #include "sysemu/kvm.h"
-#include "sysemu/sev.h"
+#include "sev.h"
 
 #define FLASH_SECTOR_SIZE 4096
 
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a5f6ff63c81..0eb7a0340cf 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -44,7 +44,6 @@
 #include "hw/i386/intel_iommu.h"
 #include "hw/i386/x86-iommu.h"
 #include "hw/i386/e820_memory_layout.h"
-#include "sysemu/sev.h"
 
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
diff --git a/target/i386/kvm/sev-stub.c b/target/i386/kvm/sev-stub.c
index 9587d1b2a31..6080c007a2e 100644
--- a/target/i386/kvm/sev-stub.c
+++ b/target/i386/kvm/sev-stub.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "sysemu/sev.h"
+#include "sev.h"
 
 int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 109e4e61c0a..935a8ee8ca4 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -30,7 +30,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/kvm.h"
-#include "sysemu/sev.h"
 #include "qapi/error.h"
 #include "sev.h"
 #include "qapi/qapi-commands-misc-target.h"
-- 
2.31.1

[PATCH v4 15/23] target/i386/sev: Declare system-specific functions in 'sev.h'

[Philippe Mathieu-Daudé]

"sysemu/sev.h" is only used from x86-specific files. Let's move it
to include/hw/i386, and merge it with target/i386/sev.h.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/sysemu/sev.h       | 28 ----------------------------
 target/i386/sev.h          | 12 +++++++++++-
 hw/i386/pc_sysfw.c         |  2 +-
 target/i386/kvm/kvm.c      |  1 -
 target/i386/kvm/sev-stub.c |  2 +-
 target/i386/monitor.c      |  1 -
 6 files changed, 13 insertions(+), 33 deletions(-)
 delete mode 100644 include/sysemu/sev.h

diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
deleted file mode 100644
index 94d821d737c..00000000000
--- a/include/sysemu/sev.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * QEMU Secure Encrypted Virutualization (SEV) support
- *
- * Copyright: Advanced Micro Devices, 2016-2018
- *
- * Authors:
- *  Brijesh Singh <brijesh.singh@amd.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_SEV_H
-#define QEMU_SEV_H
-
-#include "sysemu/kvm.h"
-
-bool sev_enabled(void);
-int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
-int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp);
-
-int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
-void sev_es_set_reset_vector(CPUState *cpu);
-
-#endif
diff --git a/target/i386/sev.h b/target/i386/sev.h
index d83428fa265..c96072bf78d 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -14,7 +14,7 @@
 #ifndef QEMU_SEV_I386_H
 #define QEMU_SEV_I386_H
 
-#include "sysemu/sev.h"
+#include "exec/confidential-guest-support.h"
 #include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
@@ -35,6 +35,7 @@ typedef struct SevKernelLoaderContext {
     size_t cmdline_size;
 } SevKernelLoaderContext;
 
+bool sev_enabled(void);
 extern bool sev_es_enabled(void);
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
@@ -45,4 +46,13 @@ extern SevAttestationReport *
 sev_get_attestation_report(const char *mnonce, Error **errp);
 extern bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
+int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+                             uint64_t gpa, Error **errp);
+
+int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
+void sev_es_set_reset_vector(CPUState *cpu);
+
+int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
+
 #endif
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 68d6b1f783e..c8b17af9535 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -37,7 +37,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/block/flash.h"
 #include "sysemu/kvm.h"
-#include "sysemu/sev.h"
+#include "sev.h"
 
 #define FLASH_SECTOR_SIZE 4096
 
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a5f6ff63c81..0eb7a0340cf 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -44,7 +44,6 @@
 #include "hw/i386/intel_iommu.h"
 #include "hw/i386/x86-iommu.h"
 #include "hw/i386/e820_memory_layout.h"
-#include "sysemu/sev.h"
 
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
diff --git a/target/i386/kvm/sev-stub.c b/target/i386/kvm/sev-stub.c
index 9587d1b2a31..6080c007a2e 100644
--- a/target/i386/kvm/sev-stub.c
+++ b/target/i386/kvm/sev-stub.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "sysemu/sev.h"
+#include "sev.h"
 
 int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
 {
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 109e4e61c0a..935a8ee8ca4 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -30,7 +30,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/kvm.h"
-#include "sysemu/sev.h"
 #include "qapi/error.h"
 #include "sev.h"
 #include "qapi/qapi-commands-misc-target.h"
-- 
2.31.1

[PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Philippe Mathieu-Daudé]

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@redhat.com>
---
 target/i386/sev.h       | 14 ++++++++++++--
 target/i386/cpu.c       | 13 +++++++------
 target/i386/sev-stub.c  | 41 -----------------------------------------
 target/i386/meson.build |  2 +-
 4 files changed, 20 insertions(+), 50 deletions(-)
 delete mode 100644 target/i386/sev-stub.c

diff --git a/target/i386/sev.h b/target/i386/sev.h
index c96072bf78d..d9548e3e642 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -14,6 +14,10 @@
 #ifndef QEMU_SEV_I386_H
 #define QEMU_SEV_I386_H
 
+#ifndef CONFIG_USER_ONLY
+#include CONFIG_DEVICES /* CONFIG_SEV */
+#endif
+
 #include "exec/confidential-guest-support.h"
 #include "qapi/qapi-types-misc-target.h"
 
@@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
     size_t cmdline_size;
 } SevKernelLoaderContext;
 
-bool sev_enabled(void);
-extern bool sev_es_enabled(void);
+#ifdef CONFIG_SEV
+ bool sev_enabled(void);
+bool sev_es_enabled(void);
+#else
+#define sev_enabled() 0
+#define sev_es_enabled() 0
+#endif
+
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8289dc87bd5..fc3ed80ef1e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5764,12 +5764,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;
     default:
         /* reserved values: zero */
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
deleted file mode 100644
index 7e8b6f9a259..00000000000
--- a/target/i386/sev-stub.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * QEMU SEV stub
- *
- * Copyright Advanced Micro Devices 2018
- *
- * Authors:
- *      Brijesh Singh <brijesh.singh@amd.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "qemu/osdep.h"
-#include "qapi/error.h"
-#include "sev.h"
-
-bool sev_enabled(void)
-{
-    return false;
-}
-
-uint32_t sev_get_cbit_position(void)
-{
-    return 0;
-}
-
-uint32_t sev_get_reduced_phys_bits(void)
-{
-    return 0;
-}
-
-bool sev_es_enabled(void)
-{
-    return false;
-}
-
-bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
-{
-    g_assert_not_reached();
-}
diff --git a/target/i386/meson.build b/target/i386/meson.build
index a4f45c3ec1d..ae38dc95635 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@
   'xsave_helper.c',
   'cpu-dump.c',
 ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
 
 # x86 cpu type
 i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
-- 
2.31.1

[PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Philippe Mathieu-Daudé]

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@redhat.com>
---
 target/i386/sev.h       | 14 ++++++++++++--
 target/i386/cpu.c       | 13 +++++++------
 target/i386/sev-stub.c  | 41 -----------------------------------------
 target/i386/meson.build |  2 +-
 4 files changed, 20 insertions(+), 50 deletions(-)
 delete mode 100644 target/i386/sev-stub.c

diff --git a/target/i386/sev.h b/target/i386/sev.h
index c96072bf78d..d9548e3e642 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -14,6 +14,10 @@
 #ifndef QEMU_SEV_I386_H
 #define QEMU_SEV_I386_H
 
+#ifndef CONFIG_USER_ONLY
+#include CONFIG_DEVICES /* CONFIG_SEV */
+#endif
+
 #include "exec/confidential-guest-support.h"
 #include "qapi/qapi-types-misc-target.h"
 
@@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
     size_t cmdline_size;
 } SevKernelLoaderContext;
 
-bool sev_enabled(void);
-extern bool sev_es_enabled(void);
+#ifdef CONFIG_SEV
+ bool sev_enabled(void);
+bool sev_es_enabled(void);
+#else
+#define sev_enabled() 0
+#define sev_es_enabled() 0
+#endif
+
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8289dc87bd5..fc3ed80ef1e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5764,12 +5764,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;
     default:
         /* reserved values: zero */
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
deleted file mode 100644
index 7e8b6f9a259..00000000000
--- a/target/i386/sev-stub.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * QEMU SEV stub
- *
- * Copyright Advanced Micro Devices 2018
- *
- * Authors:
- *      Brijesh Singh <brijesh.singh@amd.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "qemu/osdep.h"
-#include "qapi/error.h"
-#include "sev.h"
-
-bool sev_enabled(void)
-{
-    return false;
-}
-
-uint32_t sev_get_cbit_position(void)
-{
-    return 0;
-}
-
-uint32_t sev_get_reduced_phys_bits(void)
-{
-    return 0;
-}
-
-bool sev_es_enabled(void)
-{
-    return false;
-}
-
-bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
-{
-    g_assert_not_reached();
-}
diff --git a/target/i386/meson.build b/target/i386/meson.build
index a4f45c3ec1d..ae38dc95635 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@
   'xsave_helper.c',
   'cpu-dump.c',
 ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
 
 # x86 cpu type
 i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
-- 
2.31.1

[PATCH v4 17/23] target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev_attestation_report() from monitor.c to sev.c
and make sev_get_attestation_report() static. We don't need the
stub anymore, remove it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             |  2 --
 target/i386/monitor.c         |  6 ------
 target/i386/sev-sysemu-stub.c |  5 +++--
 target/i386/sev.c             | 12 ++++++++++--
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index d9548e3e642..2e90c05fc3f 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -52,8 +52,6 @@ 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 *
-sev_get_attestation_report(const char *mnonce, Error **errp);
 extern bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 935a8ee8ca4..cf4a8a61a02 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -763,12 +763,6 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
     sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
 }
 
-SevAttestationReport *
-qmp_query_sev_attestation_report(const char *mnonce, Error **errp)
-{
-    return sev_get_attestation_report(mnonce, errp);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 8082781febf..d5ec6b32e0a 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "sev.h"
 
@@ -52,8 +53,8 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
     g_assert_not_reached();
 }
 
-SevAttestationReport *sev_get_attestation_report(const char *mnonce,
-                                                 Error **errp)
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
+                                                       Error **errp)
 {
     error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index e43bbf3a17d..038fa560588 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,6 +32,8 @@
 #include "migration/blocker.h"
 #include "qom/object.h"
 #include "monitor/monitor.h"
+#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qmp/qerror.h"
 #include "exec/confidential-guest-support.h"
 #include "hw/i386/pc.h"
 
@@ -515,8 +517,8 @@ out:
     return cap;
 }
 
-SevAttestationReport *
-sev_get_attestation_report(const char *mnonce, Error **errp)
+static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
+                                                        Error **errp)
 {
     struct kvm_sev_attestation_report input = {};
     SevAttestationReport *report = NULL;
@@ -578,6 +580,12 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     return report;
 }
 
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
+                                                       Error **errp)
+{
+    return sev_get_attestation_report(mnonce, errp);
+}
+
 static int
 sev_read_file_base64(const char *filename, guchar **data, gsize *len)
 {
-- 
2.31.1

[PATCH v4 17/23] target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev_attestation_report() from monitor.c to sev.c
and make sev_get_attestation_report() static. We don't need the
stub anymore, remove it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             |  2 --
 target/i386/monitor.c         |  6 ------
 target/i386/sev-sysemu-stub.c |  5 +++--
 target/i386/sev.c             | 12 ++++++++++--
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index d9548e3e642..2e90c05fc3f 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -52,8 +52,6 @@ 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 *
-sev_get_attestation_report(const char *mnonce, Error **errp);
 extern bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 935a8ee8ca4..cf4a8a61a02 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -763,12 +763,6 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
     sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
 }
 
-SevAttestationReport *
-qmp_query_sev_attestation_report(const char *mnonce, Error **errp)
-{
-    return sev_get_attestation_report(mnonce, errp);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 8082781febf..d5ec6b32e0a 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "sev.h"
 
@@ -52,8 +53,8 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
     g_assert_not_reached();
 }
 
-SevAttestationReport *sev_get_attestation_report(const char *mnonce,
-                                                 Error **errp)
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
+                                                       Error **errp)
 {
     error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index e43bbf3a17d..038fa560588 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,6 +32,8 @@
 #include "migration/blocker.h"
 #include "qom/object.h"
 #include "monitor/monitor.h"
+#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qmp/qerror.h"
 #include "exec/confidential-guest-support.h"
 #include "hw/i386/pc.h"
 
@@ -515,8 +517,8 @@ out:
     return cap;
 }
 
-SevAttestationReport *
-sev_get_attestation_report(const char *mnonce, Error **errp)
+static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
+                                                        Error **errp)
 {
     struct kvm_sev_attestation_report input = {};
     SevAttestationReport *report = NULL;
@@ -578,6 +580,12 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     return report;
 }
 
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
+                                                       Error **errp)
+{
+    return sev_get_attestation_report(mnonce, errp);
+}
+
 static int
 sev_read_file_base64(const char *filename, guchar **data, gsize *len)
 {
-- 
2.31.1

[PATCH v4 18/23] target/i386/sev: Move qmp_sev_inject_launch_secret() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_sev_inject_launch_secret() from monitor.c to sev.c
and make sev_inject_launch_secret() static. We don't need the
stub anymore, remove it.

Previously with binaries built without SEV, management layer
was getting an empty response:

  { "execute": "sev-inject-launch-secret",
    "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "return": {
      }
  }

Now the response is explicit, mentioning the feature is disabled:

  { "execute": "sev-inject-launch-secret",
          "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "error": {
          "class": "GenericError",
          "desc": "this feature or command is not currently supported"
      }
  }

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/monitor.c         | 31 -------------------------------
 target/i386/sev-sysemu-stub.c |  6 +++---
 target/i386/sev.c             | 31 +++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index cf4a8a61a02..22883ef2ebb 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -732,37 +732,6 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
     return sev_get_capabilities(errp);
 }
 
-#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
-struct sev_secret_area {
-    uint32_t base;
-    uint32_t size;
-};
-
-void qmp_sev_inject_launch_secret(const char *packet_hdr,
-                                  const char *secret,
-                                  bool has_gpa, uint64_t gpa,
-                                  Error **errp)
-{
-    if (!sev_enabled()) {
-        error_setg(errp, "SEV not enabled for guest");
-        return;
-    }
-    if (!has_gpa) {
-        uint8_t *data;
-        struct sev_secret_area *area;
-
-        if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
-            error_setg(errp, "SEV: no secret area found in OVMF,"
-                       " gpa must be specified.");
-            return;
-        }
-        area = (struct sev_secret_area *)data;
-        gpa = area->base;
-    }
-
-    sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index d5ec6b32e0a..82c5ebb92fa 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -33,10 +33,10 @@ SevCapability *sev_get_capabilities(Error **errp)
     return NULL;
 }
 
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp)
+void qmp_sev_inject_launch_secret(const char *packet_header, const char *secret,
+                                  bool has_gpa, uint64_t gpa, Error **errp)
 {
-    return 1;
+    error_setg(errp, "SEV is not available in this QEMU");
 }
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 038fa560588..072bb6f0fd7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -973,6 +973,37 @@ int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
     return 0;
 }
 
+#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
+struct sev_secret_area {
+    uint32_t base;
+    uint32_t size;
+};
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+                                  const char *secret,
+                                  bool has_gpa, uint64_t gpa,
+                                  Error **errp)
+{
+    if (!sev_enabled()) {
+        error_setg(errp, "SEV not enabled for guest");
+        return;
+    }
+    if (!has_gpa) {
+        uint8_t *data;
+        struct sev_secret_area *area;
+
+        if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
+            error_setg(errp, "SEV: no secret area found in OVMF,"
+                       " gpa must be specified.");
+            return;
+        }
+        area = (struct sev_secret_area *)data;
+        gpa = area->base;
+    }
+
+    sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
+
 static int
 sev_es_parse_reset_block(SevInfoBlock *info, uint32_t *addr)
 {
-- 
2.31.1

[PATCH v4 18/23] target/i386/sev: Move qmp_sev_inject_launch_secret() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_sev_inject_launch_secret() from monitor.c to sev.c
and make sev_inject_launch_secret() static. We don't need the
stub anymore, remove it.

Previously with binaries built without SEV, management layer
was getting an empty response:

  { "execute": "sev-inject-launch-secret",
    "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "return": {
      }
  }

Now the response is explicit, mentioning the feature is disabled:

  { "execute": "sev-inject-launch-secret",
          "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 }
  }
  {
      "error": {
          "class": "GenericError",
          "desc": "this feature or command is not currently supported"
      }
  }

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/monitor.c         | 31 -------------------------------
 target/i386/sev-sysemu-stub.c |  6 +++---
 target/i386/sev.c             | 31 +++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index cf4a8a61a02..22883ef2ebb 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -732,37 +732,6 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
     return sev_get_capabilities(errp);
 }
 
-#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
-struct sev_secret_area {
-    uint32_t base;
-    uint32_t size;
-};
-
-void qmp_sev_inject_launch_secret(const char *packet_hdr,
-                                  const char *secret,
-                                  bool has_gpa, uint64_t gpa,
-                                  Error **errp)
-{
-    if (!sev_enabled()) {
-        error_setg(errp, "SEV not enabled for guest");
-        return;
-    }
-    if (!has_gpa) {
-        uint8_t *data;
-        struct sev_secret_area *area;
-
-        if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
-            error_setg(errp, "SEV: no secret area found in OVMF,"
-                       " gpa must be specified.");
-            return;
-        }
-        area = (struct sev_secret_area *)data;
-        gpa = area->base;
-    }
-
-    sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index d5ec6b32e0a..82c5ebb92fa 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -33,10 +33,10 @@ SevCapability *sev_get_capabilities(Error **errp)
     return NULL;
 }
 
-int sev_inject_launch_secret(const char *hdr, const char *secret,
-                             uint64_t gpa, Error **errp)
+void qmp_sev_inject_launch_secret(const char *packet_header, const char *secret,
+                                  bool has_gpa, uint64_t gpa, Error **errp)
 {
-    return 1;
+    error_setg(errp, "SEV is not available in this QEMU");
 }
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 038fa560588..072bb6f0fd7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -973,6 +973,37 @@ int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
     return 0;
 }
 
+#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
+struct sev_secret_area {
+    uint32_t base;
+    uint32_t size;
+};
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+                                  const char *secret,
+                                  bool has_gpa, uint64_t gpa,
+                                  Error **errp)
+{
+    if (!sev_enabled()) {
+        error_setg(errp, "SEV not enabled for guest");
+        return;
+    }
+    if (!has_gpa) {
+        uint8_t *data;
+        struct sev_secret_area *area;
+
+        if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
+            error_setg(errp, "SEV: no secret area found in OVMF,"
+                       " gpa must be specified.");
+            return;
+        }
+        area = (struct sev_secret_area *)data;
+        gpa = area->base;
+    }
+
+    sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
+
 static int
 sev_es_parse_reset_block(SevInfoBlock *info, uint32_t *addr)
 {
-- 
2.31.1

[PATCH v4 19/23] target/i386/sev: Move qmp_query_sev_capabilities() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev_capabilities() from monitor.c to sev.c
and make sev_get_capabilities() static. We don't need the
stub anymore, remove it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             | 1 -
 target/i386/monitor.c         | 5 -----
 target/i386/sev-sysemu-stub.c | 2 +-
 target/i386/sev.c             | 8 ++++++--
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 2e90c05fc3f..b70b7b56cb8 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -51,7 +51,6 @@ 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 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 22883ef2ebb..4c017b59b3a 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -727,11 +727,6 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
     return info;
 }
 
-SevCapability *qmp_query_sev_capabilities(Error **errp)
-{
-    return sev_get_capabilities(errp);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 82c5ebb92fa..3e8cab4c144 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -27,7 +27,7 @@ char *sev_get_launch_measurement(void)
     return NULL;
 }
 
-SevCapability *sev_get_capabilities(Error **errp)
+SevCapability *qmp_query_sev_capabilities(Error **errp)
 {
     error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 072bb6f0fd7..56e9e03accd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -466,8 +466,7 @@ e_free:
     return 1;
 }
 
-SevCapability *
-sev_get_capabilities(Error **errp)
+static SevCapability *sev_get_capabilities(Error **errp)
 {
     SevCapability *cap = NULL;
     guchar *pdh_data = NULL;
@@ -517,6 +516,11 @@ out:
     return cap;
 }
 
+SevCapability *qmp_query_sev_capabilities(Error **errp)
+{
+    return sev_get_capabilities(errp);
+}
+
 static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
                                                         Error **errp)
 {
-- 
2.31.1

[PATCH v4 19/23] target/i386/sev: Move qmp_query_sev_capabilities() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev_capabilities() from monitor.c to sev.c
and make sev_get_capabilities() static. We don't need the
stub anymore, remove it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             | 1 -
 target/i386/monitor.c         | 5 -----
 target/i386/sev-sysemu-stub.c | 2 +-
 target/i386/sev.c             | 8 ++++++--
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 2e90c05fc3f..b70b7b56cb8 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -51,7 +51,6 @@ 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 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 22883ef2ebb..4c017b59b3a 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -727,11 +727,6 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
     return info;
 }
 
-SevCapability *qmp_query_sev_capabilities(Error **errp)
-{
-    return sev_get_capabilities(errp);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 82c5ebb92fa..3e8cab4c144 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -27,7 +27,7 @@ char *sev_get_launch_measurement(void)
     return NULL;
 }
 
-SevCapability *sev_get_capabilities(Error **errp)
+SevCapability *qmp_query_sev_capabilities(Error **errp)
 {
     error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 072bb6f0fd7..56e9e03accd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -466,8 +466,7 @@ e_free:
     return 1;
 }
 
-SevCapability *
-sev_get_capabilities(Error **errp)
+static SevCapability *sev_get_capabilities(Error **errp)
 {
     SevCapability *cap = NULL;
     guchar *pdh_data = NULL;
@@ -517,6 +516,11 @@ out:
     return cap;
 }
 
+SevCapability *qmp_query_sev_capabilities(Error **errp)
+{
+    return sev_get_capabilities(errp);
+}
+
 static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
                                                         Error **errp)
 {
-- 
2.31.1

[PATCH v4 20/23] target/i386/sev: Move qmp_query_sev_launch_measure() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev_launch_measure() from monitor.c to sev.c
and make sev_get_launch_measurement() static. We don't need the
stub anymore, remove it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             |  1 -
 target/i386/monitor.c         | 17 -----------------
 target/i386/sev-sysemu-stub.c |  3 ++-
 target/i386/sev.c             | 20 ++++++++++++++++++--
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index b70b7b56cb8..dda350779f9 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -50,7 +50,6 @@ 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 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 4c017b59b3a..bd24d0d4737 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -710,23 +710,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict)
     qapi_free_SevInfo(info);
 }
 
-SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
-{
-    char *data;
-    SevLaunchMeasureInfo *info;
-
-    data = sev_get_launch_measurement();
-    if (!data) {
-        error_setg(errp, "SEV launch measurement is not available");
-        return NULL;
-    }
-
-    info = g_malloc0(sizeof(*info));
-    info->data = data;
-
-    return info;
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 3e8cab4c144..8d97d7c7e14 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -22,8 +22,9 @@ SevInfo *sev_get_info(void)
     return NULL;
 }
 
-char *sev_get_launch_measurement(void)
+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
 {
+    error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
 }
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 56e9e03accd..ec874b3df82 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -742,8 +742,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     trace_kvm_sev_launch_measurement(sev->measurement);
 }
 
-char *
-sev_get_launch_measurement(void)
+static char *sev_get_launch_measurement(void)
 {
     if (sev_guest &&
         sev_guest->state >= SEV_STATE_LAUNCH_SECRET) {
@@ -753,6 +752,23 @@ sev_get_launch_measurement(void)
     return NULL;
 }
 
+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
+{
+    char *data;
+    SevLaunchMeasureInfo *info;
+
+    data = sev_get_launch_measurement();
+    if (!data) {
+        error_setg(errp, "SEV launch measurement is not available");
+        return NULL;
+    }
+
+    info = g_malloc0(sizeof(*info));
+    info->data = data;
+
+    return info;
+}
+
 static Notifier sev_machine_done_notify = {
     .notify = sev_launch_get_measure,
 };
-- 
2.31.1

[PATCH v4 20/23] target/i386/sev: Move qmp_query_sev_launch_measure() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev_launch_measure() from monitor.c to sev.c
and make sev_get_launch_measurement() static. We don't need the
stub anymore, remove it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             |  1 -
 target/i386/monitor.c         | 17 -----------------
 target/i386/sev-sysemu-stub.c |  3 ++-
 target/i386/sev.c             | 20 ++++++++++++++++++--
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index b70b7b56cb8..dda350779f9 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -50,7 +50,6 @@ 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 bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
 
 int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 4c017b59b3a..bd24d0d4737 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -710,23 +710,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict)
     qapi_free_SevInfo(info);
 }
 
-SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
-{
-    char *data;
-    SevLaunchMeasureInfo *info;
-
-    data = sev_get_launch_measurement();
-    if (!data) {
-        error_setg(errp, "SEV launch measurement is not available");
-        return NULL;
-    }
-
-    info = g_malloc0(sizeof(*info));
-    info->data = data;
-
-    return info;
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 3e8cab4c144..8d97d7c7e14 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -22,8 +22,9 @@ SevInfo *sev_get_info(void)
     return NULL;
 }
 
-char *sev_get_launch_measurement(void)
+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
 {
+    error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
 }
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 56e9e03accd..ec874b3df82 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -742,8 +742,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
     trace_kvm_sev_launch_measurement(sev->measurement);
 }
 
-char *
-sev_get_launch_measurement(void)
+static char *sev_get_launch_measurement(void)
 {
     if (sev_guest &&
         sev_guest->state >= SEV_STATE_LAUNCH_SECRET) {
@@ -753,6 +752,23 @@ sev_get_launch_measurement(void)
     return NULL;
 }
 
+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
+{
+    char *data;
+    SevLaunchMeasureInfo *info;
+
+    data = sev_get_launch_measurement();
+    if (!data) {
+        error_setg(errp, "SEV launch measurement is not available");
+        return NULL;
+    }
+
+    info = g_malloc0(sizeof(*info));
+    info->data = data;
+
+    return info;
+}
+
 static Notifier sev_machine_done_notify = {
     .notify = sev_launch_get_measure,
 };
-- 
2.31.1

[PATCH v4 21/23] target/i386/sev: Move qmp_query_sev() & hmp_info_sev() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev() & hmp_info_sev()() from monitor.c to sev.c
and make sev_get_info() static. We don't need the stub anymore,
remove it. Add a stub for hmp_info_sev().

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             |  2 --
 target/i386/monitor.c         | 35 --------------------------------
 target/i386/sev-sysemu-stub.c | 10 ++++++++-
 target/i386/sev.c             | 38 +++++++++++++++++++++++++++++++++--
 4 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index dda350779f9..3fba1884a0d 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -19,7 +19,6 @@
 #endif
 
 #include "exec/confidential-guest-support.h"
-#include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
 #define SEV_POLICY_NOKS         0x2
@@ -47,7 +46,6 @@ bool sev_es_enabled(void);
 #define sev_es_enabled() 0
 #endif
 
-extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
 extern bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index bd24d0d4737..680d282591c 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -31,7 +31,6 @@
 #include "qapi/qmp/qerror.h"
 #include "sysemu/kvm.h"
 #include "qapi/error.h"
-#include "sev.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qapi-commands-misc.h"
 #include "hw/i386/pc.h"
@@ -676,40 +675,6 @@ void hmp_info_io_apic(Monitor *mon, const QDict *qdict)
                    "removed soon. Please use 'info pic' instead.\n");
 }
 
-SevInfo *qmp_query_sev(Error **errp)
-{
-    SevInfo *info;
-
-    info = sev_get_info();
-    if (!info) {
-        error_setg(errp, "SEV feature is not available");
-        return NULL;
-    }
-
-    return info;
-}
-
-void hmp_info_sev(Monitor *mon, const QDict *qdict)
-{
-    SevInfo *info = sev_get_info();
-
-    if (info && info->enabled) {
-        monitor_printf(mon, "handle: %d\n", info->handle);
-        monitor_printf(mon, "state: %s\n", SevState_str(info->state));
-        monitor_printf(mon, "build: %d\n", info->build_id);
-        monitor_printf(mon, "api version: %d.%d\n",
-                       info->api_major, info->api_minor);
-        monitor_printf(mon, "debug: %s\n",
-                       info->policy & SEV_POLICY_NODBG ? "off" : "on");
-        monitor_printf(mon, "key-sharing: %s\n",
-                       info->policy & SEV_POLICY_NOKS ? "off" : "on");
-    } else {
-        monitor_printf(mon, "SEV is not enabled\n");
-    }
-
-    qapi_free_SevInfo(info);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 8d97d7c7e14..68518fd3f9d 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -12,13 +12,16 @@
  */
 
 #include "qemu/osdep.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "sev.h"
 
-SevInfo *sev_get_info(void)
+SevInfo *qmp_query_sev(Error **errp)
 {
+    error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
 }
 
@@ -60,3 +63,8 @@ SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
     error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
 }
+
+void hmp_info_sev(Monitor *mon, const QDict *qdict)
+{
+    monitor_printf(mon, "SEV is not available in this QEMU\n");
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index ec874b3df82..19504796fb7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,6 +32,7 @@
 #include "migration/blocker.h"
 #include "qom/object.h"
 #include "monitor/monitor.h"
+#include "monitor/hmp.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "exec/confidential-guest-support.h"
@@ -402,8 +403,7 @@ sev_get_reduced_phys_bits(void)
     return sev_guest ? sev_guest->reduced_phys_bits : 0;
 }
 
-SevInfo *
-sev_get_info(void)
+static SevInfo *sev_get_info(void)
 {
     SevInfo *info;
 
@@ -422,6 +422,40 @@ sev_get_info(void)
     return info;
 }
 
+SevInfo *qmp_query_sev(Error **errp)
+{
+    SevInfo *info;
+
+    info = sev_get_info();
+    if (!info) {
+        error_setg(errp, "SEV feature is not available");
+        return NULL;
+    }
+
+    return info;
+}
+
+void hmp_info_sev(Monitor *mon, const QDict *qdict)
+{
+    SevInfo *info = sev_get_info();
+
+    if (info && info->enabled) {
+        monitor_printf(mon, "handle: %d\n", info->handle);
+        monitor_printf(mon, "state: %s\n", SevState_str(info->state));
+        monitor_printf(mon, "build: %d\n", info->build_id);
+        monitor_printf(mon, "api version: %d.%d\n",
+                       info->api_major, info->api_minor);
+        monitor_printf(mon, "debug: %s\n",
+                       info->policy & SEV_POLICY_NODBG ? "off" : "on");
+        monitor_printf(mon, "key-sharing: %s\n",
+                       info->policy & SEV_POLICY_NOKS ? "off" : "on");
+    } else {
+        monitor_printf(mon, "SEV is not enabled\n");
+    }
+
+    qapi_free_SevInfo(info);
+}
+
 static int
 sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
                  size_t *cert_chain_len, Error **errp)
-- 
2.31.1

[PATCH v4 21/23] target/i386/sev: Move qmp_query_sev() & hmp_info_sev() to sev.c

[Philippe Mathieu-Daudé]

Move qmp_query_sev() & hmp_info_sev()() from monitor.c to sev.c
and make sev_get_info() static. We don't need the stub anymore,
remove it. Add a stub for hmp_info_sev().

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev.h             |  2 --
 target/i386/monitor.c         | 35 --------------------------------
 target/i386/sev-sysemu-stub.c | 10 ++++++++-
 target/i386/sev.c             | 38 +++++++++++++++++++++++++++++++++--
 4 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index dda350779f9..3fba1884a0d 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -19,7 +19,6 @@
 #endif
 
 #include "exec/confidential-guest-support.h"
-#include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
 #define SEV_POLICY_NOKS         0x2
@@ -47,7 +46,6 @@ bool sev_es_enabled(void);
 #define sev_es_enabled() 0
 #endif
 
-extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
 extern bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index bd24d0d4737..680d282591c 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -31,7 +31,6 @@
 #include "qapi/qmp/qerror.h"
 #include "sysemu/kvm.h"
 #include "qapi/error.h"
-#include "sev.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qapi-commands-misc.h"
 #include "hw/i386/pc.h"
@@ -676,40 +675,6 @@ void hmp_info_io_apic(Monitor *mon, const QDict *qdict)
                    "removed soon. Please use 'info pic' instead.\n");
 }
 
-SevInfo *qmp_query_sev(Error **errp)
-{
-    SevInfo *info;
-
-    info = sev_get_info();
-    if (!info) {
-        error_setg(errp, "SEV feature is not available");
-        return NULL;
-    }
-
-    return info;
-}
-
-void hmp_info_sev(Monitor *mon, const QDict *qdict)
-{
-    SevInfo *info = sev_get_info();
-
-    if (info && info->enabled) {
-        monitor_printf(mon, "handle: %d\n", info->handle);
-        monitor_printf(mon, "state: %s\n", SevState_str(info->state));
-        monitor_printf(mon, "build: %d\n", info->build_id);
-        monitor_printf(mon, "api version: %d.%d\n",
-                       info->api_major, info->api_minor);
-        monitor_printf(mon, "debug: %s\n",
-                       info->policy & SEV_POLICY_NODBG ? "off" : "on");
-        monitor_printf(mon, "key-sharing: %s\n",
-                       info->policy & SEV_POLICY_NOKS ? "off" : "on");
-    } else {
-        monitor_printf(mon, "SEV is not enabled\n");
-    }
-
-    qapi_free_SevInfo(info);
-}
-
 SGXInfo *qmp_query_sgx(Error **errp)
 {
     return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 8d97d7c7e14..68518fd3f9d 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -12,13 +12,16 @@
  */
 
 #include "qemu/osdep.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "sev.h"
 
-SevInfo *sev_get_info(void)
+SevInfo *qmp_query_sev(Error **errp)
 {
+    error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
 }
 
@@ -60,3 +63,8 @@ SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
     error_setg(errp, "SEV is not available in this QEMU");
     return NULL;
 }
+
+void hmp_info_sev(Monitor *mon, const QDict *qdict)
+{
+    monitor_printf(mon, "SEV is not available in this QEMU\n");
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index ec874b3df82..19504796fb7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,6 +32,7 @@
 #include "migration/blocker.h"
 #include "qom/object.h"
 #include "monitor/monitor.h"
+#include "monitor/hmp.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "exec/confidential-guest-support.h"
@@ -402,8 +403,7 @@ sev_get_reduced_phys_bits(void)
     return sev_guest ? sev_guest->reduced_phys_bits : 0;
 }
 
-SevInfo *
-sev_get_info(void)
+static SevInfo *sev_get_info(void)
 {
     SevInfo *info;
 
@@ -422,6 +422,40 @@ sev_get_info(void)
     return info;
 }
 
+SevInfo *qmp_query_sev(Error **errp)
+{
+    SevInfo *info;
+
+    info = sev_get_info();
+    if (!info) {
+        error_setg(errp, "SEV feature is not available");
+        return NULL;
+    }
+
+    return info;
+}
+
+void hmp_info_sev(Monitor *mon, const QDict *qdict)
+{
+    SevInfo *info = sev_get_info();
+
+    if (info && info->enabled) {
+        monitor_printf(mon, "handle: %d\n", info->handle);
+        monitor_printf(mon, "state: %s\n", SevState_str(info->state));
+        monitor_printf(mon, "build: %d\n", info->build_id);
+        monitor_printf(mon, "api version: %d.%d\n",
+                       info->api_major, info->api_minor);
+        monitor_printf(mon, "debug: %s\n",
+                       info->policy & SEV_POLICY_NODBG ? "off" : "on");
+        monitor_printf(mon, "key-sharing: %s\n",
+                       info->policy & SEV_POLICY_NOKS ? "off" : "on");
+    } else {
+        monitor_printf(mon, "SEV is not enabled\n");
+    }
+
+    qapi_free_SevInfo(info);
+}
+
 static int
 sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
                  size_t *cert_chain_len, Error **errp)
-- 
2.31.1

[PATCH v4 22/23] monitor: Reduce hmp_info_sev() declaration

[Philippe Mathieu-Daudé]

While being conditionally used for TARGET_I386 in hmp-commands-info.hx,
hmp_info_sev() is declared for all targets. Reduce its declaration
to target including "monitor/hmp-target.h". This is a minor cleanup.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/monitor/hmp-target.h  | 1 +
 include/monitor/hmp.h         | 1 -
 target/i386/sev-sysemu-stub.c | 2 +-
 target/i386/sev.c             | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index dc53add7eef..96956d0fc41 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -49,6 +49,7 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict);
 void hmp_mce(Monitor *mon, const QDict *qdict);
 void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
 void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
+void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_sgx(Monitor *mon, const QDict *qdict);
 
 #endif /* MONITOR_HMP_TARGET_H */
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 3baa1058e2c..6bc27639e01 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -124,7 +124,6 @@ void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
-void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 68518fd3f9d..7a29295d1ed 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "monitor/monitor.h"
-#include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 19504796fb7..4c64c682442 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,7 +32,7 @@
 #include "migration/blocker.h"
 #include "qom/object.h"
 #include "monitor/monitor.h"
-#include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "exec/confidential-guest-support.h"
-- 
2.31.1

[PATCH v4 22/23] monitor: Reduce hmp_info_sev() declaration

[Philippe Mathieu-Daudé]

While being conditionally used for TARGET_I386 in hmp-commands-info.hx,
hmp_info_sev() is declared for all targets. Reduce its declaration
to target including "monitor/hmp-target.h". This is a minor cleanup.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/monitor/hmp-target.h  | 1 +
 include/monitor/hmp.h         | 1 -
 target/i386/sev-sysemu-stub.c | 2 +-
 target/i386/sev.c             | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index dc53add7eef..96956d0fc41 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -49,6 +49,7 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict);
 void hmp_mce(Monitor *mon, const QDict *qdict);
 void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
 void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
+void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_sgx(Monitor *mon, const QDict *qdict);
 
 #endif /* MONITOR_HMP_TARGET_H */
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 3baa1058e2c..6bc27639e01 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -124,7 +124,6 @@ void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
-void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 68518fd3f9d..7a29295d1ed 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "monitor/monitor.h"
-#include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 19504796fb7..4c64c682442 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,7 +32,7 @@
 #include "migration/blocker.h"
 #include "qom/object.h"
 #include "monitor/monitor.h"
-#include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qmp/qerror.h"
 #include "exec/confidential-guest-support.h"
-- 
2.31.1

[PATCH v4 23/23] MAINTAINERS: Cover SEV-related files with X86/KVM section

[Philippe Mathieu-Daudé]

Complete the x86/KVM section with SEV-related files.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 50435b8d2f5..a49555d94d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -417,7 +417,9 @@ M: Paolo Bonzini <pbonzini@redhat.com>
 M: Marcelo Tosatti <mtosatti@redhat.com>
 L: kvm@vger.kernel.org
 S: Supported
+F: docs/amd-memory-encryption.txt
 F: target/i386/kvm/
+F: target/i386/sev*
 F: scripts/kvm/vmxcap
 
 Guest CPU Cores (other accelerators)
-- 
2.31.1

[PATCH v4 23/23] MAINTAINERS: Cover SEV-related files with X86/KVM section

[Philippe Mathieu-Daudé]

Complete the x86/KVM section with SEV-related files.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 50435b8d2f5..a49555d94d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -417,7 +417,9 @@ M: Paolo Bonzini <pbonzini@redhat.com>
 M: Marcelo Tosatti <mtosatti@redhat.com>
 L: kvm@vger.kernel.org
 S: Supported
+F: docs/amd-memory-encryption.txt
 F: target/i386/kvm/
+F: target/i386/sev*
 F: scripts/kvm/vmxcap
 
 Guest CPU Cores (other accelerators)
-- 
2.31.1

Re: [PATCH v4 23/23] MAINTAINERS: Cover SEV-related files with X86/KVM section

[Philippe Mathieu-Daudé]

On 10/7/21 18:17, Philippe Mathieu-Daudé wrote:
> Complete the x86/KVM section with SEV-related files.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50435b8d2f5..a49555d94d5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -417,7 +417,9 @@ M: Paolo Bonzini <pbonzini@redhat.com>
>  M: Marcelo Tosatti <mtosatti@redhat.com>
>  L: kvm@vger.kernel.org
>  S: Supported
> +F: docs/amd-memory-encryption.txt

BTW maybe this one should be renamed docs/system/i386/sev.txt?

>  F: target/i386/kvm/
> +F: target/i386/sev*
>  F: scripts/kvm/vmxcap
>  
>  Guest CPU Cores (other accelerators)
> 

Re: [PATCH v4 23/23] MAINTAINERS: Cover SEV-related files with X86/KVM section

[Philippe Mathieu-Daudé]

On 10/7/21 18:17, Philippe Mathieu-Daudé wrote:
> Complete the x86/KVM section with SEV-related files.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50435b8d2f5..a49555d94d5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -417,7 +417,9 @@ M: Paolo Bonzini <pbonzini@redhat.com>
>  M: Marcelo Tosatti <mtosatti@redhat.com>
>  L: kvm@vger.kernel.org
>  S: Supported
> +F: docs/amd-memory-encryption.txt

BTW maybe this one should be renamed docs/system/i386/sev.txt?

>  F: target/i386/kvm/
> +F: target/i386/sev*
>  F: scripts/kvm/vmxcap
>  
>  Guest CPU Cores (other accelerators)
> 

Re: [PATCH v4 05/23] target/i386/sev: Prefix QMP errors with 'SEV'

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Multiple errors might be reported to the monitor,
> better to prefix the SEV ones so we can distinct them.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  target/i386/monitor.c |  2 +-
>  target/i386/sev.c     | 20 +++++++++++---------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 196c1c9e77f..eabbeb9be95 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -717,7 +717,7 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
>  
>      data = sev_get_launch_measurement();
>      if (!data) {
> -        error_setg(errp, "Measurement is not available");
> +        error_setg(errp, "SEV launch measurement is not available");
>          return NULL;
>      }
>  
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index bcd9260fa46..4f1952cd32f 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -440,7 +440,8 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
>      r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
>      if (r < 0) {
>          if (err != SEV_RET_INVALID_LEN) {
> -            error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
> +            error_setg(errp, "SEV: Failed to export PDH cert"
> +                             " ret=%d fw_err=%d (%s)",
>                         r, err, fw_error_to_str(err));
>              return 1;
>          }
> @@ -453,7 +454,7 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
>  
>      r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
>      if (r < 0) {
> -        error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
> +        error_setg(errp, "SEV: Failed to export PDH cert ret=%d fw_err=%d (%s)",
>                     r, err, fw_error_to_str(err));
>          goto e_free;
>      }
> @@ -491,7 +492,7 @@ sev_get_capabilities(Error **errp)
>  
>      fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
>      if (fd < 0) {
> -        error_setg_errno(errp, errno, "Failed to open %s",
> +        error_setg_errno(errp, errno, "SEV: Failed to open %s",
>                           DEFAULT_SEV_DEVICE);
>          return NULL;
>      }
> @@ -557,8 +558,9 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
>              &input, &err);
>      if (ret < 0) {
>          if (err != SEV_RET_INVALID_LEN) {
> -            error_setg(errp, "failed to query the attestation report length "
> -                    "ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
> +            error_setg(errp, "SEV: Failed to query the attestation report"
> +                             " length ret=%d fw_err=%d (%s)",
> +                       ret, err, fw_error_to_str(err));
>              g_free(buf);
>              return NULL;
>          }
> @@ -572,7 +574,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
>      ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
>              &input, &err);
>      if (ret) {
> -        error_setg_errno(errp, errno, "Failed to get attestation report"
> +        error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
>                  " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
>          goto e_free_data;
>      }
> @@ -596,7 +598,7 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
>      GError *error = NULL;
>  
>      if (!g_file_get_contents(filename, &base64, &sz, &error)) {
> -        error_report("failed to read '%s' (%s)", filename, error->message);
> +        error_report("SEV: Failed to read '%s' (%s)", filename, error->message);
>          g_error_free(error);
>          return -1;
>      }
> @@ -911,7 +913,7 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
>      if (sev_check_state(sev_guest, SEV_STATE_LAUNCH_UPDATE)) {
>          int ret = sev_launch_update_data(sev_guest, ptr, len);
>          if (ret < 0) {
> -            error_setg(errp, "failed to encrypt pflash rom");
> +            error_setg(errp, "SEV: Failed to encrypt pflash rom");
>              return ret;
>          }
>      }
> @@ -930,7 +932,7 @@ int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
>      MemoryRegion *mr = NULL;
>  
>      if (!sev_guest) {
> -        error_setg(errp, "SEV: SEV not enabled.");
> +        error_setg(errp, "SEV not enabled for guest");
>          return 1;
>      }
>  
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 05/23] target/i386/sev: Prefix QMP errors with 'SEV'

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Multiple errors might be reported to the monitor,
> better to prefix the SEV ones so we can distinct them.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  target/i386/monitor.c |  2 +-
>  target/i386/sev.c     | 20 +++++++++++---------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 196c1c9e77f..eabbeb9be95 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -717,7 +717,7 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
>  
>      data = sev_get_launch_measurement();
>      if (!data) {
> -        error_setg(errp, "Measurement is not available");
> +        error_setg(errp, "SEV launch measurement is not available");
>          return NULL;
>      }
>  
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index bcd9260fa46..4f1952cd32f 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -440,7 +440,8 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
>      r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
>      if (r < 0) {
>          if (err != SEV_RET_INVALID_LEN) {
> -            error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
> +            error_setg(errp, "SEV: Failed to export PDH cert"
> +                             " ret=%d fw_err=%d (%s)",
>                         r, err, fw_error_to_str(err));
>              return 1;
>          }
> @@ -453,7 +454,7 @@ sev_get_pdh_info(int fd, guchar **pdh, size_t *pdh_len, guchar **cert_chain,
>  
>      r = sev_platform_ioctl(fd, SEV_PDH_CERT_EXPORT, &export, &err);
>      if (r < 0) {
> -        error_setg(errp, "failed to export PDH cert ret=%d fw_err=%d (%s)",
> +        error_setg(errp, "SEV: Failed to export PDH cert ret=%d fw_err=%d (%s)",
>                     r, err, fw_error_to_str(err));
>          goto e_free;
>      }
> @@ -491,7 +492,7 @@ sev_get_capabilities(Error **errp)
>  
>      fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
>      if (fd < 0) {
> -        error_setg_errno(errp, errno, "Failed to open %s",
> +        error_setg_errno(errp, errno, "SEV: Failed to open %s",
>                           DEFAULT_SEV_DEVICE);
>          return NULL;
>      }
> @@ -557,8 +558,9 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
>              &input, &err);
>      if (ret < 0) {
>          if (err != SEV_RET_INVALID_LEN) {
> -            error_setg(errp, "failed to query the attestation report length "
> -                    "ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
> +            error_setg(errp, "SEV: Failed to query the attestation report"
> +                             " length ret=%d fw_err=%d (%s)",
> +                       ret, err, fw_error_to_str(err));
>              g_free(buf);
>              return NULL;
>          }
> @@ -572,7 +574,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
>      ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
>              &input, &err);
>      if (ret) {
> -        error_setg_errno(errp, errno, "Failed to get attestation report"
> +        error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
>                  " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
>          goto e_free_data;
>      }
> @@ -596,7 +598,7 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
>      GError *error = NULL;
>  
>      if (!g_file_get_contents(filename, &base64, &sz, &error)) {
> -        error_report("failed to read '%s' (%s)", filename, error->message);
> +        error_report("SEV: Failed to read '%s' (%s)", filename, error->message);
>          g_error_free(error);
>          return -1;
>      }
> @@ -911,7 +913,7 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
>      if (sev_check_state(sev_guest, SEV_STATE_LAUNCH_UPDATE)) {
>          int ret = sev_launch_update_data(sev_guest, ptr, len);
>          if (ret < 0) {
> -            error_setg(errp, "failed to encrypt pflash rom");
> +            error_setg(errp, "SEV: Failed to encrypt pflash rom");
>              return ret;
>          }
>      }
> @@ -930,7 +932,7 @@ int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
>      MemoryRegion *mr = NULL;
>  
>      if (!sev_guest) {
> -        error_setg(errp, "SEV: SEV not enabled.");
> +        error_setg(errp, "SEV not enabled for guest");
>          return 1;
>      }
>  
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 12/23] target/i386/sev: Use g_autofree in sev_launch_get_measure()

[Dov Murik]

On 07/10/2021 19:17, Philippe Mathieu-Daudé wrote:
> Use g_autofree to remove a pair of g_free/goto.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  target/i386/sev.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 3a30ba6d94a..5cbbcf0bb93 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -685,8 +685,8 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>  {
>      SevGuestState *sev = sev_guest;
>      int ret, error;
> -    guchar *data;
> -    struct kvm_sev_launch_measure *measurement;
> +    g_autofree guchar *data = NULL;
> +    g_autofree struct kvm_sev_launch_measure *measurement = NULL;

Sorry for joining so late, but why do we allocate struct
kvm_sev_launch_measure on the heap (even before this patch)? It's only
12 bytes. Might be simpler to have a local (stack) variable and not care
about memory management.

-Dov


>  
>      if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
>          return;
> @@ -708,7 +708,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>      if (!measurement->len) {
>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>                       __func__, ret, error, fw_error_to_str(errno));
> -        goto free_measurement;
> +        return;
>      }
>  
>      data = g_new0(guchar, measurement->len);
> @@ -720,7 +720,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>      if (ret) {
>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>                       __func__, ret, error, fw_error_to_str(errno));
> -        goto free_data;
> +        return;
>      }
>  
>      sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
> @@ -728,11 +728,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>      /* encode the measurement value and emit the event */
>      sev->measurement = g_base64_encode(data, measurement->len);
>      trace_kvm_sev_launch_measurement(sev->measurement);
> -
> -free_data:
> -    g_free(data);
> -free_measurement:
> -    g_free(measurement);
>  }
>  
>  char *
> 

Re: [PATCH v4 12/23] target/i386/sev: Use g_autofree in sev_launch_get_measure()

[Dov Murik]

On 07/10/2021 19:17, Philippe Mathieu-Daudé wrote:
> Use g_autofree to remove a pair of g_free/goto.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  target/i386/sev.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 3a30ba6d94a..5cbbcf0bb93 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -685,8 +685,8 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>  {
>      SevGuestState *sev = sev_guest;
>      int ret, error;
> -    guchar *data;
> -    struct kvm_sev_launch_measure *measurement;
> +    g_autofree guchar *data = NULL;
> +    g_autofree struct kvm_sev_launch_measure *measurement = NULL;

Sorry for joining so late, but why do we allocate struct
kvm_sev_launch_measure on the heap (even before this patch)? It's only
12 bytes. Might be simpler to have a local (stack) variable and not care
about memory management.

-Dov


>  
>      if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
>          return;
> @@ -708,7 +708,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>      if (!measurement->len) {
>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>                       __func__, ret, error, fw_error_to_str(errno));
> -        goto free_measurement;
> +        return;
>      }
>  
>      data = g_new0(guchar, measurement->len);
> @@ -720,7 +720,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>      if (ret) {
>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>                       __func__, ret, error, fw_error_to_str(errno));
> -        goto free_data;
> +        return;
>      }
>  
>      sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
> @@ -728,11 +728,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>      /* encode the measurement value and emit the event */
>      sev->measurement = g_base64_encode(data, measurement->len);
>      trace_kvm_sev_launch_measurement(sev->measurement);
> -
> -free_data:
> -    g_free(data);
> -free_measurement:
> -    g_free(measurement);
>  }
>  
>  char *
> 

Re: [PATCH v4 14/23] target/i386/sev: Rename sev_i386.h -> sev.h

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> SEV is a x86 specific feature, and the "sev_i386.h" header
> is already in target/i386/. Rename it as "sev.h" to simplify.
> 
> Patch created mechanically using:
> 
>   $ git mv target/i386/sev_i386.h target/i386/sev.h
>   $ sed -i s/sev_i386.h/sev.h/ $(git grep -l sev_i386.h)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  target/i386/{sev_i386.h => sev.h} | 0
>  hw/i386/x86.c                     | 2 +-
>  target/i386/cpu.c                 | 2 +-
>  target/i386/kvm/kvm.c             | 2 +-
>  target/i386/monitor.c             | 2 +-
>  target/i386/sev-stub.c            | 2 +-
>  target/i386/sev-sysemu-stub.c     | 2 +-
>  target/i386/sev.c                 | 2 +-
>  8 files changed, 7 insertions(+), 7 deletions(-)
>  rename target/i386/{sev_i386.h => sev.h} (100%)
> 
> diff --git a/target/i386/sev_i386.h b/target/i386/sev.h
> similarity index 100%
> rename from target/i386/sev_i386.h
> rename to target/i386/sev.h
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 0c7c054e3a0..76de7e2265e 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -47,7 +47,7 @@
>  #include "hw/i386/fw_cfg.h"
>  #include "hw/intc/i8259.h"
>  #include "hw/rtc/mc146818rtc.h"
> -#include "target/i386/sev_i386.h"
> +#include "target/i386/sev.h"
>  
>  #include "hw/acpi/cpu_hotplug.h"
>  #include "hw/irq.h"
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b54b98551e9..8289dc87bd5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -26,7 +26,7 @@
>  #include "sysemu/reset.h"
>  #include "sysemu/hvf.h"
>  #include "kvm/kvm_i386.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-visit-machine.h"
>  #include "qapi/qmp/qerror.h"
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index f25837f63f4..a5f6ff63c81 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -28,7 +28,7 @@
>  #include "sysemu/kvm_int.h"
>  #include "sysemu/runstate.h"
>  #include "kvm_i386.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "hyperv.h"
>  #include "hyperv-proto.h"
>  
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index ea836678f51..109e4e61c0a 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -32,7 +32,7 @@
>  #include "sysemu/kvm.h"
>  #include "sysemu/sev.h"
>  #include "qapi/error.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "qapi/qapi-commands-misc-target.h"
>  #include "qapi/qapi-commands-misc.h"
>  #include "hw/i386/pc.h"
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index 170e9f50fee..7e8b6f9a259 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -13,7 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  
>  bool sev_enabled(void)
>  {
> diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
> index d556b4f091f..8082781febf 100644
> --- a/target/i386/sev-sysemu-stub.c
> +++ b/target/i386/sev-sysemu-stub.c
> @@ -14,7 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/qapi-commands-misc-target.h"
>  #include "qapi/error.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  
>  SevInfo *sev_get_info(void)
>  {
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 5cbbcf0bb93..e43bbf3a17d 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -25,7 +25,7 @@
>  #include "qemu/uuid.h"
>  #include "crypto/hash.h"
>  #include "sysemu/kvm.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/runstate.h"
>  #include "trace.h"
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 14/23] target/i386/sev: Rename sev_i386.h -> sev.h

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> SEV is a x86 specific feature, and the "sev_i386.h" header
> is already in target/i386/. Rename it as "sev.h" to simplify.
> 
> Patch created mechanically using:
> 
>   $ git mv target/i386/sev_i386.h target/i386/sev.h
>   $ sed -i s/sev_i386.h/sev.h/ $(git grep -l sev_i386.h)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  target/i386/{sev_i386.h => sev.h} | 0
>  hw/i386/x86.c                     | 2 +-
>  target/i386/cpu.c                 | 2 +-
>  target/i386/kvm/kvm.c             | 2 +-
>  target/i386/monitor.c             | 2 +-
>  target/i386/sev-stub.c            | 2 +-
>  target/i386/sev-sysemu-stub.c     | 2 +-
>  target/i386/sev.c                 | 2 +-
>  8 files changed, 7 insertions(+), 7 deletions(-)
>  rename target/i386/{sev_i386.h => sev.h} (100%)
> 
> diff --git a/target/i386/sev_i386.h b/target/i386/sev.h
> similarity index 100%
> rename from target/i386/sev_i386.h
> rename to target/i386/sev.h
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 0c7c054e3a0..76de7e2265e 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -47,7 +47,7 @@
>  #include "hw/i386/fw_cfg.h"
>  #include "hw/intc/i8259.h"
>  #include "hw/rtc/mc146818rtc.h"
> -#include "target/i386/sev_i386.h"
> +#include "target/i386/sev.h"
>  
>  #include "hw/acpi/cpu_hotplug.h"
>  #include "hw/irq.h"
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b54b98551e9..8289dc87bd5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -26,7 +26,7 @@
>  #include "sysemu/reset.h"
>  #include "sysemu/hvf.h"
>  #include "kvm/kvm_i386.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-visit-machine.h"
>  #include "qapi/qmp/qerror.h"
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index f25837f63f4..a5f6ff63c81 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -28,7 +28,7 @@
>  #include "sysemu/kvm_int.h"
>  #include "sysemu/runstate.h"
>  #include "kvm_i386.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "hyperv.h"
>  #include "hyperv-proto.h"
>  
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index ea836678f51..109e4e61c0a 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -32,7 +32,7 @@
>  #include "sysemu/kvm.h"
>  #include "sysemu/sev.h"
>  #include "qapi/error.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "qapi/qapi-commands-misc-target.h"
>  #include "qapi/qapi-commands-misc.h"
>  #include "hw/i386/pc.h"
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index 170e9f50fee..7e8b6f9a259 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -13,7 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  
>  bool sev_enabled(void)
>  {
> diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
> index d556b4f091f..8082781febf 100644
> --- a/target/i386/sev-sysemu-stub.c
> +++ b/target/i386/sev-sysemu-stub.c
> @@ -14,7 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/qapi-commands-misc-target.h"
>  #include "qapi/error.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  
>  SevInfo *sev_get_info(void)
>  {
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 5cbbcf0bb93..e43bbf3a17d 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -25,7 +25,7 @@
>  #include "qemu/uuid.h"
>  #include "crypto/hash.h"
>  #include "sysemu/kvm.h"
> -#include "sev_i386.h"
> +#include "sev.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/runstate.h"
>  #include "trace.h"
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>

What makes it allowed to *rely* on the compiler eliding calls?

Dave

> ---
>  target/i386/sev.h       | 14 ++++++++++++--
>  target/i386/cpu.c       | 13 +++++++------
>  target/i386/sev-stub.c  | 41 -----------------------------------------
>  target/i386/meson.build |  2 +-
>  4 files changed, 20 insertions(+), 50 deletions(-)
>  delete mode 100644 target/i386/sev-stub.c
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index c96072bf78d..d9548e3e642 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -14,6 +14,10 @@
>  #ifndef QEMU_SEV_I386_H
>  #define QEMU_SEV_I386_H
>  
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES /* CONFIG_SEV */
> +#endif
> +
>  #include "exec/confidential-guest-support.h"
>  #include "qapi/qapi-types-misc-target.h"
>  
> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
>      size_t cmdline_size;
>  } SevKernelLoaderContext;
>  
> -bool sev_enabled(void);
> -extern bool sev_es_enabled(void);
> +#ifdef CONFIG_SEV
> + bool sev_enabled(void);
> +bool sev_es_enabled(void);
> +#else
> +#define sev_enabled() 0
> +#define sev_es_enabled() 0
> +#endif
> +
>  extern SevInfo *sev_get_info(void);
>  extern uint32_t sev_get_cbit_position(void);
>  extern uint32_t sev_get_reduced_phys_bits(void);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8289dc87bd5..fc3ed80ef1e 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5764,12 +5764,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;
>      default:
>          /* reserved values: zero */
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> deleted file mode 100644
> index 7e8b6f9a259..00000000000
> --- a/target/i386/sev-stub.c
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/*
> - * QEMU SEV stub
> - *
> - * Copyright Advanced Micro Devices 2018
> - *
> - * Authors:
> - *      Brijesh Singh <brijesh.singh@amd.com>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> - * See the COPYING file in the top-level directory.
> - *
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/error.h"
> -#include "sev.h"
> -
> -bool sev_enabled(void)
> -{
> -    return false;
> -}
> -
> -uint32_t sev_get_cbit_position(void)
> -{
> -    return 0;
> -}
> -
> -uint32_t sev_get_reduced_phys_bits(void)
> -{
> -    return 0;
> -}
> -
> -bool sev_es_enabled(void)
> -{
> -    return false;
> -}
> -
> -bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
> -{
> -    g_assert_not_reached();
> -}
> diff --git a/target/i386/meson.build b/target/i386/meson.build
> index a4f45c3ec1d..ae38dc95635 100644
> --- a/target/i386/meson.build
> +++ b/target/i386/meson.build
> @@ -6,7 +6,7 @@
>    'xsave_helper.c',
>    'cpu-dump.c',
>  ))
> -i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
> +i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
>  
>  # x86 cpu type
>  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>

What makes it allowed to *rely* on the compiler eliding calls?

Dave

> ---
>  target/i386/sev.h       | 14 ++++++++++++--
>  target/i386/cpu.c       | 13 +++++++------
>  target/i386/sev-stub.c  | 41 -----------------------------------------
>  target/i386/meson.build |  2 +-
>  4 files changed, 20 insertions(+), 50 deletions(-)
>  delete mode 100644 target/i386/sev-stub.c
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index c96072bf78d..d9548e3e642 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -14,6 +14,10 @@
>  #ifndef QEMU_SEV_I386_H
>  #define QEMU_SEV_I386_H
>  
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES /* CONFIG_SEV */
> +#endif
> +
>  #include "exec/confidential-guest-support.h"
>  #include "qapi/qapi-types-misc-target.h"
>  
> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
>      size_t cmdline_size;
>  } SevKernelLoaderContext;
>  
> -bool sev_enabled(void);
> -extern bool sev_es_enabled(void);
> +#ifdef CONFIG_SEV
> + bool sev_enabled(void);
> +bool sev_es_enabled(void);
> +#else
> +#define sev_enabled() 0
> +#define sev_es_enabled() 0
> +#endif
> +
>  extern SevInfo *sev_get_info(void);
>  extern uint32_t sev_get_cbit_position(void);
>  extern uint32_t sev_get_reduced_phys_bits(void);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8289dc87bd5..fc3ed80ef1e 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5764,12 +5764,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;
>      default:
>          /* reserved values: zero */
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> deleted file mode 100644
> index 7e8b6f9a259..00000000000
> --- a/target/i386/sev-stub.c
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/*
> - * QEMU SEV stub
> - *
> - * Copyright Advanced Micro Devices 2018
> - *
> - * Authors:
> - *      Brijesh Singh <brijesh.singh@amd.com>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> - * See the COPYING file in the top-level directory.
> - *
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/error.h"
> -#include "sev.h"
> -
> -bool sev_enabled(void)
> -{
> -    return false;
> -}
> -
> -uint32_t sev_get_cbit_position(void)
> -{
> -    return 0;
> -}
> -
> -uint32_t sev_get_reduced_phys_bits(void)
> -{
> -    return 0;
> -}
> -
> -bool sev_es_enabled(void)
> -{
> -    return false;
> -}
> -
> -bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
> -{
> -    g_assert_not_reached();
> -}
> diff --git a/target/i386/meson.build b/target/i386/meson.build
> index a4f45c3ec1d..ae38dc95635 100644
> --- a/target/i386/meson.build
> +++ b/target/i386/meson.build
> @@ -6,7 +6,7 @@
>    'xsave_helper.c',
>    'cpu-dump.c',
>  ))
> -i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
> +i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
>  
>  # x86 cpu type
>  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Philippe Mathieu-Daudé]

On 10/7/21 19:07, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>
> 
> What makes it allowed to *rely* on the compiler eliding calls?

I am not aware of a particular requirement on the compiler for code
elision, however we already use this syntax:

$ git grep -A4 'ifdef CONFIG_' include/sysemu/
...
include/sysemu/tcg.h:11:#ifdef CONFIG_TCG
include/sysemu/tcg.h-12-extern bool tcg_allowed;
include/sysemu/tcg.h-13-#define tcg_enabled() (tcg_allowed)
include/sysemu/tcg.h-14-#else
include/sysemu/tcg.h-15-#define tcg_enabled() 0
...

Cc'ing Richard/Eric/Daniel who have more experience with compiler
features in case they can enlighten me here.

>> ---
>>  target/i386/sev.h       | 14 ++++++++++++--
>>  target/i386/cpu.c       | 13 +++++++------
>>  target/i386/sev-stub.c  | 41 -----------------------------------------
>>  target/i386/meson.build |  2 +-
>>  4 files changed, 20 insertions(+), 50 deletions(-)
>>  delete mode 100644 target/i386/sev-stub.c
>>
>> diff --git a/target/i386/sev.h b/target/i386/sev.h
>> index c96072bf78d..d9548e3e642 100644
>> --- a/target/i386/sev.h
>> +++ b/target/i386/sev.h
>> @@ -14,6 +14,10 @@
>>  #ifndef QEMU_SEV_I386_H
>>  #define QEMU_SEV_I386_H
>>  
>> +#ifndef CONFIG_USER_ONLY
>> +#include CONFIG_DEVICES /* CONFIG_SEV */
>> +#endif
>> +
>>  #include "exec/confidential-guest-support.h"
>>  #include "qapi/qapi-types-misc-target.h"
>>  
>> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
>>      size_t cmdline_size;
>>  } SevKernelLoaderContext;
>>  
>> -bool sev_enabled(void);
>> -extern bool sev_es_enabled(void);
>> +#ifdef CONFIG_SEV
>> + bool sev_enabled(void);
>> +bool sev_es_enabled(void);
>> +#else
>> +#define sev_enabled() 0
>> +#define sev_es_enabled() 0
>> +#endif
>> +
>>  extern SevInfo *sev_get_info(void);
>>  extern uint32_t sev_get_cbit_position(void);
>>  extern uint32_t sev_get_reduced_phys_bits(void);
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 8289dc87bd5..fc3ed80ef1e 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -5764,12 +5764,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;
>>      default:
>>          /* reserved values: zero */
>> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
>> deleted file mode 100644
>> index 7e8b6f9a259..00000000000
>> --- a/target/i386/sev-stub.c
>> +++ /dev/null
>> @@ -1,41 +0,0 @@
>> -/*
>> - * QEMU SEV stub
>> - *
>> - * Copyright Advanced Micro Devices 2018
>> - *
>> - * Authors:
>> - *      Brijesh Singh <brijesh.singh@amd.com>
>> - *
>> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> - * See the COPYING file in the top-level directory.
>> - *
>> - */
>> -
>> -#include "qemu/osdep.h"
>> -#include "qapi/error.h"
>> -#include "sev.h"
>> -
>> -bool sev_enabled(void)
>> -{
>> -    return false;
>> -}
>> -
>> -uint32_t sev_get_cbit_position(void)
>> -{
>> -    return 0;
>> -}
>> -
>> -uint32_t sev_get_reduced_phys_bits(void)
>> -{
>> -    return 0;
>> -}
>> -
>> -bool sev_es_enabled(void)
>> -{
>> -    return false;
>> -}
>> -
>> -bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>> -{
>> -    g_assert_not_reached();
>> -}
>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>> index a4f45c3ec1d..ae38dc95635 100644
>> --- a/target/i386/meson.build
>> +++ b/target/i386/meson.build
>> @@ -6,7 +6,7 @@
>>    'xsave_helper.c',
>>    'cpu-dump.c',
>>  ))
>> -i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
>> +i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
>>  
>>  # x86 cpu type
>>  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
>> -- 
>> 2.31.1
>>

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Philippe Mathieu-Daudé]

On 10/7/21 19:07, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>
> 
> What makes it allowed to *rely* on the compiler eliding calls?

I am not aware of a particular requirement on the compiler for code
elision, however we already use this syntax:

$ git grep -A4 'ifdef CONFIG_' include/sysemu/
...
include/sysemu/tcg.h:11:#ifdef CONFIG_TCG
include/sysemu/tcg.h-12-extern bool tcg_allowed;
include/sysemu/tcg.h-13-#define tcg_enabled() (tcg_allowed)
include/sysemu/tcg.h-14-#else
include/sysemu/tcg.h-15-#define tcg_enabled() 0
...

Cc'ing Richard/Eric/Daniel who have more experience with compiler
features in case they can enlighten me here.

>> ---
>>  target/i386/sev.h       | 14 ++++++++++++--
>>  target/i386/cpu.c       | 13 +++++++------
>>  target/i386/sev-stub.c  | 41 -----------------------------------------
>>  target/i386/meson.build |  2 +-
>>  4 files changed, 20 insertions(+), 50 deletions(-)
>>  delete mode 100644 target/i386/sev-stub.c
>>
>> diff --git a/target/i386/sev.h b/target/i386/sev.h
>> index c96072bf78d..d9548e3e642 100644
>> --- a/target/i386/sev.h
>> +++ b/target/i386/sev.h
>> @@ -14,6 +14,10 @@
>>  #ifndef QEMU_SEV_I386_H
>>  #define QEMU_SEV_I386_H
>>  
>> +#ifndef CONFIG_USER_ONLY
>> +#include CONFIG_DEVICES /* CONFIG_SEV */
>> +#endif
>> +
>>  #include "exec/confidential-guest-support.h"
>>  #include "qapi/qapi-types-misc-target.h"
>>  
>> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
>>      size_t cmdline_size;
>>  } SevKernelLoaderContext;
>>  
>> -bool sev_enabled(void);
>> -extern bool sev_es_enabled(void);
>> +#ifdef CONFIG_SEV
>> + bool sev_enabled(void);
>> +bool sev_es_enabled(void);
>> +#else
>> +#define sev_enabled() 0
>> +#define sev_es_enabled() 0
>> +#endif
>> +
>>  extern SevInfo *sev_get_info(void);
>>  extern uint32_t sev_get_cbit_position(void);
>>  extern uint32_t sev_get_reduced_phys_bits(void);
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 8289dc87bd5..fc3ed80ef1e 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -5764,12 +5764,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;
>>      default:
>>          /* reserved values: zero */
>> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
>> deleted file mode 100644
>> index 7e8b6f9a259..00000000000
>> --- a/target/i386/sev-stub.c
>> +++ /dev/null
>> @@ -1,41 +0,0 @@
>> -/*
>> - * QEMU SEV stub
>> - *
>> - * Copyright Advanced Micro Devices 2018
>> - *
>> - * Authors:
>> - *      Brijesh Singh <brijesh.singh@amd.com>
>> - *
>> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> - * See the COPYING file in the top-level directory.
>> - *
>> - */
>> -
>> -#include "qemu/osdep.h"
>> -#include "qapi/error.h"
>> -#include "sev.h"
>> -
>> -bool sev_enabled(void)
>> -{
>> -    return false;
>> -}
>> -
>> -uint32_t sev_get_cbit_position(void)
>> -{
>> -    return 0;
>> -}
>> -
>> -uint32_t sev_get_reduced_phys_bits(void)
>> -{
>> -    return 0;
>> -}
>> -
>> -bool sev_es_enabled(void)
>> -{
>> -    return false;
>> -}
>> -
>> -bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>> -{
>> -    g_assert_not_reached();
>> -}
>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>> index a4f45c3ec1d..ae38dc95635 100644
>> --- a/target/i386/meson.build
>> +++ b/target/i386/meson.build
>> @@ -6,7 +6,7 @@
>>    'xsave_helper.c',
>>    'cpu-dump.c',
>>  ))
>> -i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
>> +i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
>>  
>>  # x86 cpu type
>>  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
>> -- 
>> 2.31.1
>>

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 10/7/21 19:07, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>
> > 
> > What makes it allowed to *rely* on the compiler eliding calls?
> 
> I am not aware of a particular requirement on the compiler for code
> elision, however we already use this syntax:
> 
> $ git grep -A4 'ifdef CONFIG_' include/sysemu/
> ...
> include/sysemu/tcg.h:11:#ifdef CONFIG_TCG
> include/sysemu/tcg.h-12-extern bool tcg_allowed;
> include/sysemu/tcg.h-13-#define tcg_enabled() (tcg_allowed)
> include/sysemu/tcg.h-14-#else
> include/sysemu/tcg.h-15-#define tcg_enabled() 0

So that I'm fine with, the bit I'm more worried about is the bit where
inside the if () we call functions (like sev_get_cbit_position )  which
we know the compiler will elide; I'm sure any sane compiler will,
but.....

Looking at your example, in cpu.c there's still places that ifdef around
areas with tcg_enabled.

Dave

> ...
> 
> Cc'ing Richard/Eric/Daniel who have more experience with compiler
> features in case they can enlighten me here.
> 
> >> ---
> >>  target/i386/sev.h       | 14 ++++++++++++--
> >>  target/i386/cpu.c       | 13 +++++++------
> >>  target/i386/sev-stub.c  | 41 -----------------------------------------
> >>  target/i386/meson.build |  2 +-
> >>  4 files changed, 20 insertions(+), 50 deletions(-)
> >>  delete mode 100644 target/i386/sev-stub.c
> >>
> >> diff --git a/target/i386/sev.h b/target/i386/sev.h
> >> index c96072bf78d..d9548e3e642 100644
> >> --- a/target/i386/sev.h
> >> +++ b/target/i386/sev.h
> >> @@ -14,6 +14,10 @@
> >>  #ifndef QEMU_SEV_I386_H
> >>  #define QEMU_SEV_I386_H
> >>  
> >> +#ifndef CONFIG_USER_ONLY
> >> +#include CONFIG_DEVICES /* CONFIG_SEV */
> >> +#endif
> >> +
> >>  #include "exec/confidential-guest-support.h"
> >>  #include "qapi/qapi-types-misc-target.h"
> >>  
> >> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
> >>      size_t cmdline_size;
> >>  } SevKernelLoaderContext;
> >>  
> >> -bool sev_enabled(void);
> >> -extern bool sev_es_enabled(void);
> >> +#ifdef CONFIG_SEV
> >> + bool sev_enabled(void);
> >> +bool sev_es_enabled(void);
> >> +#else
> >> +#define sev_enabled() 0
> >> +#define sev_es_enabled() 0
> >> +#endif
> >> +
> >>  extern SevInfo *sev_get_info(void);
> >>  extern uint32_t sev_get_cbit_position(void);
> >>  extern uint32_t sev_get_reduced_phys_bits(void);
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index 8289dc87bd5..fc3ed80ef1e 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -5764,12 +5764,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;
> >>      default:
> >>          /* reserved values: zero */
> >> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> >> deleted file mode 100644
> >> index 7e8b6f9a259..00000000000
> >> --- a/target/i386/sev-stub.c
> >> +++ /dev/null
> >> @@ -1,41 +0,0 @@
> >> -/*
> >> - * QEMU SEV stub
> >> - *
> >> - * Copyright Advanced Micro Devices 2018
> >> - *
> >> - * Authors:
> >> - *      Brijesh Singh <brijesh.singh@amd.com>
> >> - *
> >> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >> - * See the COPYING file in the top-level directory.
> >> - *
> >> - */
> >> -
> >> -#include "qemu/osdep.h"
> >> -#include "qapi/error.h"
> >> -#include "sev.h"
> >> -
> >> -bool sev_enabled(void)
> >> -{
> >> -    return false;
> >> -}
> >> -
> >> -uint32_t sev_get_cbit_position(void)
> >> -{
> >> -    return 0;
> >> -}
> >> -
> >> -uint32_t sev_get_reduced_phys_bits(void)
> >> -{
> >> -    return 0;
> >> -}
> >> -
> >> -bool sev_es_enabled(void)
> >> -{
> >> -    return false;
> >> -}
> >> -
> >> -bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
> >> -{
> >> -    g_assert_not_reached();
> >> -}
> >> diff --git a/target/i386/meson.build b/target/i386/meson.build
> >> index a4f45c3ec1d..ae38dc95635 100644
> >> --- a/target/i386/meson.build
> >> +++ b/target/i386/meson.build
> >> @@ -6,7 +6,7 @@
> >>    'xsave_helper.c',
> >>    'cpu-dump.c',
> >>  ))
> >> -i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
> >> +i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
> >>  
> >>  # x86 cpu type
> >>  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
> >> -- 
> >> 2.31.1
> >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 10/7/21 19:07, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>
> > 
> > What makes it allowed to *rely* on the compiler eliding calls?
> 
> I am not aware of a particular requirement on the compiler for code
> elision, however we already use this syntax:
> 
> $ git grep -A4 'ifdef CONFIG_' include/sysemu/
> ...
> include/sysemu/tcg.h:11:#ifdef CONFIG_TCG
> include/sysemu/tcg.h-12-extern bool tcg_allowed;
> include/sysemu/tcg.h-13-#define tcg_enabled() (tcg_allowed)
> include/sysemu/tcg.h-14-#else
> include/sysemu/tcg.h-15-#define tcg_enabled() 0

So that I'm fine with, the bit I'm more worried about is the bit where
inside the if () we call functions (like sev_get_cbit_position )  which
we know the compiler will elide; I'm sure any sane compiler will,
but.....

Looking at your example, in cpu.c there's still places that ifdef around
areas with tcg_enabled.

Dave

> ...
> 
> Cc'ing Richard/Eric/Daniel who have more experience with compiler
> features in case they can enlighten me here.
> 
> >> ---
> >>  target/i386/sev.h       | 14 ++++++++++++--
> >>  target/i386/cpu.c       | 13 +++++++------
> >>  target/i386/sev-stub.c  | 41 -----------------------------------------
> >>  target/i386/meson.build |  2 +-
> >>  4 files changed, 20 insertions(+), 50 deletions(-)
> >>  delete mode 100644 target/i386/sev-stub.c
> >>
> >> diff --git a/target/i386/sev.h b/target/i386/sev.h
> >> index c96072bf78d..d9548e3e642 100644
> >> --- a/target/i386/sev.h
> >> +++ b/target/i386/sev.h
> >> @@ -14,6 +14,10 @@
> >>  #ifndef QEMU_SEV_I386_H
> >>  #define QEMU_SEV_I386_H
> >>  
> >> +#ifndef CONFIG_USER_ONLY
> >> +#include CONFIG_DEVICES /* CONFIG_SEV */
> >> +#endif
> >> +
> >>  #include "exec/confidential-guest-support.h"
> >>  #include "qapi/qapi-types-misc-target.h"
> >>  
> >> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
> >>      size_t cmdline_size;
> >>  } SevKernelLoaderContext;
> >>  
> >> -bool sev_enabled(void);
> >> -extern bool sev_es_enabled(void);
> >> +#ifdef CONFIG_SEV
> >> + bool sev_enabled(void);
> >> +bool sev_es_enabled(void);
> >> +#else
> >> +#define sev_enabled() 0
> >> +#define sev_es_enabled() 0
> >> +#endif
> >> +
> >>  extern SevInfo *sev_get_info(void);
> >>  extern uint32_t sev_get_cbit_position(void);
> >>  extern uint32_t sev_get_reduced_phys_bits(void);
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index 8289dc87bd5..fc3ed80ef1e 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -5764,12 +5764,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;
> >>      default:
> >>          /* reserved values: zero */
> >> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> >> deleted file mode 100644
> >> index 7e8b6f9a259..00000000000
> >> --- a/target/i386/sev-stub.c
> >> +++ /dev/null
> >> @@ -1,41 +0,0 @@
> >> -/*
> >> - * QEMU SEV stub
> >> - *
> >> - * Copyright Advanced Micro Devices 2018
> >> - *
> >> - * Authors:
> >> - *      Brijesh Singh <brijesh.singh@amd.com>
> >> - *
> >> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >> - * See the COPYING file in the top-level directory.
> >> - *
> >> - */
> >> -
> >> -#include "qemu/osdep.h"
> >> -#include "qapi/error.h"
> >> -#include "sev.h"
> >> -
> >> -bool sev_enabled(void)
> >> -{
> >> -    return false;
> >> -}
> >> -
> >> -uint32_t sev_get_cbit_position(void)
> >> -{
> >> -    return 0;
> >> -}
> >> -
> >> -uint32_t sev_get_reduced_phys_bits(void)
> >> -{
> >> -    return 0;
> >> -}
> >> -
> >> -bool sev_es_enabled(void)
> >> -{
> >> -    return false;
> >> -}
> >> -
> >> -bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
> >> -{
> >> -    g_assert_not_reached();
> >> -}
> >> diff --git a/target/i386/meson.build b/target/i386/meson.build
> >> index a4f45c3ec1d..ae38dc95635 100644
> >> --- a/target/i386/meson.build
> >> +++ b/target/i386/meson.build
> >> @@ -6,7 +6,7 @@
> >>    'xsave_helper.c',
> >>    'cpu-dump.c',
> >>  ))
> >> -i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: files('sev-stub.c'))
> >> +i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
> >>  
> >>  # x86 cpu type
> >>  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
> >> -- 
> >> 2.31.1
> >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Daniel P. Berrangé]

On Thu, Oct 07, 2021 at 07:18:04PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/7/21 19:07, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>
> > 
> > What makes it allowed to *rely* on the compiler eliding calls?
> 
> I am not aware of a particular requirement on the compiler for code
> elision, however we already use this syntax:

Maybe I'm mis-understanding David's question, but I'm not
sure it matters whether the compiler elides the code or
not.

IIUC, with the old code using stubs it is unlikely to be
elided at all. With the new code it will probably be
elided, but if it isn't, then it is no worse than the
code its replacing.

Or am I mis-understanding David's question ?

> $ git grep -A4 'ifdef CONFIG_' include/sysemu/
> ...
> include/sysemu/tcg.h:11:#ifdef CONFIG_TCG
> include/sysemu/tcg.h-12-extern bool tcg_allowed;
> include/sysemu/tcg.h-13-#define tcg_enabled() (tcg_allowed)
> include/sysemu/tcg.h-14-#else
> include/sysemu/tcg.h-15-#define tcg_enabled() 0
> ...
> 
> Cc'ing Richard/Eric/Daniel who have more experience with compiler
> features in case they can enlighten me here.

I'd say my general view is we are free to use features explicitly
supported by our designated compilers. We should avoid relying on
undefined compiler behaviour for funtional results in QEMU.

We can rely on our designated compilers to optimize certain code
patterns, as long as its purely for performance benefits, not
functional benefits, since optimizations are not guaranteed and
users can turn them off too.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Daniel P. Berrangé]

On Thu, Oct 07, 2021 at 07:18:04PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/7/21 19:07, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) 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@redhat.com>
> > 
> > What makes it allowed to *rely* on the compiler eliding calls?
> 
> I am not aware of a particular requirement on the compiler for code
> elision, however we already use this syntax:

Maybe I'm mis-understanding David's question, but I'm not
sure it matters whether the compiler elides the code or
not.

IIUC, with the old code using stubs it is unlikely to be
elided at all. With the new code it will probably be
elided, but if it isn't, then it is no worse than the
code its replacing.

Or am I mis-understanding David's question ?

> $ git grep -A4 'ifdef CONFIG_' include/sysemu/
> ...
> include/sysemu/tcg.h:11:#ifdef CONFIG_TCG
> include/sysemu/tcg.h-12-extern bool tcg_allowed;
> include/sysemu/tcg.h-13-#define tcg_enabled() (tcg_allowed)
> include/sysemu/tcg.h-14-#else
> include/sysemu/tcg.h-15-#define tcg_enabled() 0
> ...
> 
> Cc'ing Richard/Eric/Daniel who have more experience with compiler
> features in case they can enlighten me here.

I'd say my general view is we are free to use features explicitly
supported by our designated compilers. We should avoid relying on
undefined compiler behaviour for funtional results in QEMU.

We can rely on our designated compilers to optimize certain code
patterns, as long as its purely for performance benefits, not
functional benefits, since optimizations are not guaranteed and
users can turn them off too.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Eric Blake]

On Thu, Oct 07, 2021 at 06:17:09PM +0200, 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@redhat.com>
> ---
>  target/i386/sev.h       | 14 ++++++++++++--
>  target/i386/cpu.c       | 13 +++++++------
>  target/i386/sev-stub.c  | 41 -----------------------------------------
>  target/i386/meson.build |  2 +-
>  4 files changed, 20 insertions(+), 50 deletions(-)
>  delete mode 100644 target/i386/sev-stub.c
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index c96072bf78d..d9548e3e642 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -14,6 +14,10 @@
>  #ifndef QEMU_SEV_I386_H
>  #define QEMU_SEV_I386_H
>  
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES /* CONFIG_SEV */
> +#endif
> +
>  #include "exec/confidential-guest-support.h"
>  #include "qapi/qapi-types-misc-target.h"
>  
> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
>      size_t cmdline_size;
>  } SevKernelLoaderContext;
>  
> -bool sev_enabled(void);
> -extern bool sev_es_enabled(void);
> +#ifdef CONFIG_SEV
> + bool sev_enabled(void);
> +bool sev_es_enabled(void);
> +#else

Is that leading space on the sev_enabled() line intentional?

> +#define sev_enabled() 0
> +#define sev_es_enabled() 0
> +#endif
> +

This allows an optimizing compiler to elide code, but does not require
that the elision worked. The real test is whether there is a link
error when functions that are only called inside what we hope is
elided have no stub.

>  extern SevInfo *sev_get_info(void);
>  extern uint32_t sev_get_cbit_position(void);
>  extern uint32_t sev_get_reduced_phys_bits(void);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8289dc87bd5..fc3ed80ef1e 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5764,12 +5764,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;
> +        }

As long as this compiles in all of our configurations, then the
compiler really has elided the calls and we can get rid of the stub.
But that's merely because we're relying on our particular gcc or clang
compiler behavior, and NOT because it is standardized behavior.  On
the other hand, I doubt either compiler would break this assumption,
as it is probably used in lots of places, even if it is not portable.

Since you asked for my opinion, I'm okay giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Eric Blake]

On Thu, Oct 07, 2021 at 06:17:09PM +0200, 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@redhat.com>
> ---
>  target/i386/sev.h       | 14 ++++++++++++--
>  target/i386/cpu.c       | 13 +++++++------
>  target/i386/sev-stub.c  | 41 -----------------------------------------
>  target/i386/meson.build |  2 +-
>  4 files changed, 20 insertions(+), 50 deletions(-)
>  delete mode 100644 target/i386/sev-stub.c
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index c96072bf78d..d9548e3e642 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -14,6 +14,10 @@
>  #ifndef QEMU_SEV_I386_H
>  #define QEMU_SEV_I386_H
>  
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES /* CONFIG_SEV */
> +#endif
> +
>  #include "exec/confidential-guest-support.h"
>  #include "qapi/qapi-types-misc-target.h"
>  
> @@ -35,8 +39,14 @@ typedef struct SevKernelLoaderContext {
>      size_t cmdline_size;
>  } SevKernelLoaderContext;
>  
> -bool sev_enabled(void);
> -extern bool sev_es_enabled(void);
> +#ifdef CONFIG_SEV
> + bool sev_enabled(void);
> +bool sev_es_enabled(void);
> +#else

Is that leading space on the sev_enabled() line intentional?

> +#define sev_enabled() 0
> +#define sev_es_enabled() 0
> +#endif
> +

This allows an optimizing compiler to elide code, but does not require
that the elision worked. The real test is whether there is a link
error when functions that are only called inside what we hope is
elided have no stub.

>  extern SevInfo *sev_get_info(void);
>  extern uint32_t sev_get_cbit_position(void);
>  extern uint32_t sev_get_reduced_phys_bits(void);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8289dc87bd5..fc3ed80ef1e 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5764,12 +5764,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;
> +        }

As long as this compiles in all of our configurations, then the
compiler really has elided the calls and we can get rid of the stub.
But that's merely because we're relying on our particular gcc or clang
compiler behavior, and NOT because it is standardized behavior.  On
the other hand, I doubt either compiler would break this assumption,
as it is probably used in lots of places, even if it is not portable.

Since you asked for my opinion, I'm okay giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Paolo Bonzini]

On 07/10/21 19:22, Dr. David Alan Gilbert wrote:
> So that I'm fine with, the bit I'm more worried about is the bit where
> inside the if () we call functions (like sev_get_cbit_position )  which
> we know the compiler will elide; I'm sure any sane compiler will,
> but.....
> 
> Looking at your example, in cpu.c there's still places that ifdef around
> areas with tcg_enabled.

I think that's just because nobody tried changing it; it should work 
there as well.

Paolo

Re: [PATCH v4 16/23] target/i386/sev: Remove stubs by using code elision

[Paolo Bonzini]

On 07/10/21 19:22, Dr. David Alan Gilbert wrote:
> So that I'm fine with, the bit I'm more worried about is the bit where
> inside the if () we call functions (like sev_get_cbit_position )  which
> we know the compiler will elide; I'm sure any sane compiler will,
> but.....
> 
> Looking at your example, in cpu.c there's still places that ifdef around
> areas with tcg_enabled.

I think that's just because nobody tried changing it; it should work 
there as well.

Paolo

Re: [PATCH v4 12/23] target/i386/sev: Use g_autofree in sev_launch_get_measure()

[Dov Murik]

On 07/10/2021 19:28, Dov Murik wrote:
> 
> 
> On 07/10/2021 19:17, Philippe Mathieu-Daudé wrote:
>> Use g_autofree to remove a pair of g_free/goto.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  target/i386/sev.c | 13 ++++---------
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 3a30ba6d94a..5cbbcf0bb93 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -685,8 +685,8 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>  {
>>      SevGuestState *sev = sev_guest;
>>      int ret, error;
>> -    guchar *data;
>> -    struct kvm_sev_launch_measure *measurement;
>> +    g_autofree guchar *data = NULL;
>> +    g_autofree struct kvm_sev_launch_measure *measurement = NULL;
> 
> Sorry for joining so late, but why do we allocate struct
> kvm_sev_launch_measure on the heap (even before this patch)? It's only
> 12 bytes. Might be simpler to have a local (stack) variable and not care
> about memory management.
> 

I sent another series [1] with this fix (and a similar fix in
launch_start); it can be added as part of this housekeeping series.

[1] https://lore.kernel.org/qemu-devel/20211011173026.2454294-1-dovmurik@linux.ibm.com/

-Dov

> -Dov
> 
> 
>>  
>>      if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
>>          return;
>> @@ -708,7 +708,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>      if (!measurement->len) {
>>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>>                       __func__, ret, error, fw_error_to_str(errno));
>> -        goto free_measurement;
>> +        return;
>>      }
>>  
>>      data = g_new0(guchar, measurement->len);
>> @@ -720,7 +720,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>      if (ret) {
>>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>>                       __func__, ret, error, fw_error_to_str(errno));
>> -        goto free_data;
>> +        return;
>>      }
>>  
>>      sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
>> @@ -728,11 +728,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>      /* encode the measurement value and emit the event */
>>      sev->measurement = g_base64_encode(data, measurement->len);
>>      trace_kvm_sev_launch_measurement(sev->measurement);
>> -
>> -free_data:
>> -    g_free(data);
>> -free_measurement:
>> -    g_free(measurement);
>>  }
>>  
>>  char *
>>

Re: [PATCH v4 12/23] target/i386/sev: Use g_autofree in sev_launch_get_measure()

[Dov Murik]

On 07/10/2021 19:28, Dov Murik wrote:
> 
> 
> On 07/10/2021 19:17, Philippe Mathieu-Daudé wrote:
>> Use g_autofree to remove a pair of g_free/goto.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  target/i386/sev.c | 13 ++++---------
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 3a30ba6d94a..5cbbcf0bb93 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -685,8 +685,8 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>  {
>>      SevGuestState *sev = sev_guest;
>>      int ret, error;
>> -    guchar *data;
>> -    struct kvm_sev_launch_measure *measurement;
>> +    g_autofree guchar *data = NULL;
>> +    g_autofree struct kvm_sev_launch_measure *measurement = NULL;
> 
> Sorry for joining so late, but why do we allocate struct
> kvm_sev_launch_measure on the heap (even before this patch)? It's only
> 12 bytes. Might be simpler to have a local (stack) variable and not care
> about memory management.
> 

I sent another series [1] with this fix (and a similar fix in
launch_start); it can be added as part of this housekeeping series.

[1] https://lore.kernel.org/qemu-devel/20211011173026.2454294-1-dovmurik@linux.ibm.com/

-Dov

> -Dov
> 
> 
>>  
>>      if (!sev_check_state(sev, SEV_STATE_LAUNCH_UPDATE)) {
>>          return;
>> @@ -708,7 +708,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>      if (!measurement->len) {
>>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>>                       __func__, ret, error, fw_error_to_str(errno));
>> -        goto free_measurement;
>> +        return;
>>      }
>>  
>>      data = g_new0(guchar, measurement->len);
>> @@ -720,7 +720,7 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>      if (ret) {
>>          error_report("%s: LAUNCH_MEASURE ret=%d fw_error=%d '%s'",
>>                       __func__, ret, error, fw_error_to_str(errno));
>> -        goto free_data;
>> +        return;
>>      }
>>  
>>      sev_set_guest_state(sev, SEV_STATE_LAUNCH_SECRET);
>> @@ -728,11 +728,6 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
>>      /* encode the measurement value and emit the event */
>>      sev->measurement = g_base64_encode(data, measurement->len);
>>      trace_kvm_sev_launch_measurement(sev->measurement);
>> -
>> -free_data:
>> -    g_free(data);
>> -free_measurement:
>> -    g_free(measurement);
>>  }
>>  
>>  char *
>>