[PATCH v2 00/23] target/i386/sev: Housekeeping (OVMF + SEV-disabled binaries)

[PATCH v2 00/23] target/i386/sev: Housekeeping (OVMF + SEV-disabled binaries)

[Philippe Mathieu-Daudé]

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.

Some SEV patches I was following have been queued on Eduardo's
'x86-next' tree, so I used his tree as base, and included David and
Connor patches to reduce merge conflicts.

Patches 1-16 are SEV related,
patches 17-23 are OVMF related.

Special comment for Laszlo: Please don't review this version,
wait for the respin (hoping the SEV cleanup patch get accepted,
the respin will be focused on OVMF).

Based-on: https://gitlab.com/ehabkost/qemu/-/commits/x86-next/
Supersedes: <20210610064556.1421620-1-philmd@redhat.com>

Connor Kuehl (1):
  MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV

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

Philippe Mathieu-Daudé (21):
  qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
  qapi/misc-target: Group SEV QAPI definitions
  target/i386/monitor: Return QMP error when SEV is disabled in build
  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: Restrict SEV to system emulation
  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: Restrict 'info sev' to x86 targets
  hw/i386/acpi-common: Remove unused includes
  hw/i386: Rename acpi-build.c -> acpi-pc.c
  hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c
  hw/acpi: Do not restrict ACPI core routines to x86 architecture
  hw/i386: Introduce X86_FW_OVMF Kconfig symbol
  hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)
  hw/i386/Kconfig: Add missing Kconfig dependency (runtime error)

 qapi/misc-target.json               |  78 ++++++++--------
 include/hw/i386/pc.h                |   1 +
 include/monitor/hmp-target.h        |   1 +
 include/monitor/hmp.h               |   1 -
 target/i386/sev_i386.h              |  11 ---
 hw/i386/acpi-common.c               |  45 ----------
 hw/i386/{acpi-build.c => acpi-pc.c} |  39 ++++++++
 hw/i386/pc_sysfw.c                  | 107 ----------------------
 hw/i386/pc_sysfw_ovmf-stubs.c       |  26 ++++++
 hw/i386/pc_sysfw_ovmf.c             | 135 ++++++++++++++++++++++++++++
 target/i386/cpu.c                   |   1 +
 target/i386/monitor.c               |  91 -------------------
 target/i386/sev-stub.c              |  49 +---------
 target/i386/sev-sysemu-stub.c       |  69 ++++++++++++++
 target/i386/sev.c                   | 130 +++++++++++++++++++++------
 MAINTAINERS                         |   8 ++
 hw/acpi/Kconfig                     |   4 +
 hw/acpi/meson.build                 |   3 +-
 hw/i386/Kconfig                     |   5 ++
 hw/i386/meson.build                 |   4 +-
 target/i386/meson.build             |   4 +-
 21 files changed, 443 insertions(+), 369 deletions(-)
 rename hw/i386/{acpi-build.c => acpi-pc.c} (98%)
 create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
 create mode 100644 hw/i386/pc_sysfw_ovmf.c
 create mode 100644 target/i386/sev-sysemu-stub.c

-- 
2.31.1

[PATCH v2 01/23] MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV

[Philippe Mathieu-Daudé]

From: Connor Kuehl <ckuehl@redhat.com>

It may not be appropriate for me to take over as a maintainer at this time,
but I would consider myself familiar with AMD SEV and what this code is
meant to be doing as part of a VMM for launching SEV-protected guests.

To that end, I would be happy to volunteer as a reviewer for SEV-related
changes so that I am CC'd on them and can help share the review burden with
whoever does maintain this code.

Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608192537.103584-1-ckuehl@redhat.com>
[PMD: Cover more files]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ccdb81c9f62..fbfaaa54721 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2937,6 +2937,13 @@ F: hw/core/clock-vmstate.c
 F: hw/core/qdev-clock.c
 F: docs/devel/clocks.rst
 
+AMD Secure Encrypted Virtualization (SEV)
+R: Connor Kuehl <ckuehl@redhat.com>
+F: docs/amd-memory-encryption.txt
+F: accel/kvm/sev-stub.c
+F: target/i386/sev*
+F: include/sysemu/sev.h
+
 Usermode Emulation
 ------------------
 Overall usermode emulation
-- 
2.31.1

Re: [PATCH v2 01/23] MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV

[Philippe Mathieu-Daudé]

On 6/16/21 10:43 PM, Philippe Mathieu-Daudé wrote:
> From: Connor Kuehl <ckuehl@redhat.com>
> 
> It may not be appropriate for me to take over as a maintainer at this time,
> but I would consider myself familiar with AMD SEV and what this code is
> meant to be doing as part of a VMM for launching SEV-protected guests.
> 
> To that end, I would be happy to volunteer as a reviewer for SEV-related
> changes so that I am CC'd on them and can help share the review burden with
> whoever does maintain this code.
> 
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Message-Id: <20210608192537.103584-1-ckuehl@redhat.com>
> [PMD: Cover more files]
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ccdb81c9f62..fbfaaa54721 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2937,6 +2937,13 @@ F: hw/core/clock-vmstate.c
>  F: hw/core/qdev-clock.c
>  F: docs/devel/clocks.rst
>  
> +AMD Secure Encrypted Virtualization (SEV)
> +R: Connor Kuehl <ckuehl@redhat.com>

Is this patch still valid?

> +F: docs/amd-memory-encryption.txt
> +F: accel/kvm/sev-stub.c
> +F: target/i386/sev*
> +F: include/sysemu/sev.h
> +
>  Usermode Emulation
>  ------------------
>  Overall usermode emulation
> 

Re: [PATCH v2 01/23] MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV

[Connor Kuehl]

On 8/30/21 9:18 AM, Philippe Mathieu-Daudé wrote:
>>  
>> +AMD Secure Encrypted Virtualization (SEV)
>> +R: Connor Kuehl <ckuehl@redhat.com>
> 
> Is this patch still valid?

Thank you for championing it, but due to recent changes, no, it is
no longer valid.

Thank you,

Connor

[PATCH v2 02/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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Note: it would be nice if checkpatch enforce 70 char for json
      (or at least QAPI json), that would save future
      developer modifying QAPI definitions reformating time.
---
 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 5573dcf8f08..81646126267 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -290,8 +290,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)
 #
@@ -305,10 +305,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.
 #
@@ -316,10 +317,12 @@
 #
 # 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': 'defined(TARGET_I386)' }
-- 
2.31.1

Re: [PATCH v2 02/23] qapi/misc-target: Wrap long 'SEV Attestation Report' long lines

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Wrap long lines before 70 characters for legibility.
> 
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Note: it would be nice if checkpatch enforce 70 char for json
>       (or at least QAPI json), that would save future
>       developer modifying QAPI definitions reformating time.
> ---
>  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 5573dcf8f08..81646126267 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -290,8 +290,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)
>  #
> @@ -305,10 +305,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.
>  #
> @@ -316,10 +317,12 @@
>  #
>  # Example:
>  #
> -# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": "aaaaaaa" } }
> +# -> { "execute" : "query-sev-attestation-report",
> +                   "arguments": { "mnonce": "aaaaaaa" } }

Shouldn't that start with a # ?
(Actually, last time I tried this command, I think I realised that we
should also put a real nonce value in there; it requirs a 16 byte value)

Dave

>  # <- { "return" : { "data": "aaaaaaaabbbddddd"} }
>  #
>  ##
> -{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
> +{ 'command': 'query-sev-attestation-report',
> +  'data': { 'mnonce': 'str' },
>    'returns': 'SevAttestationReport',
>    'if': 'defined(TARGET_I386)' }
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 02/23] qapi/misc-target: Wrap long 'SEV Attestation Report' long lines

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Wrap long lines before 70 characters for legibility.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

> ---
> Note: it would be nice if checkpatch enforce 70 char for json
>       (or at least QAPI json), that would save future
>       developer modifying QAPI definitions reformating time.

Yes.

Re: [PATCH v2 02/23] qapi/misc-target: Wrap long 'SEV Attestation Report' long lines

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
>> Wrap long lines before 70 characters for legibility.
>>
>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

With the typo spotted by Dave fixed, of course.

>
>> ---
>> Note: it would be nice if checkpatch enforce 70 char for json
>>       (or at least QAPI json), that would save future
>>       developer modifying QAPI definitions reformating time.
>
> Yes.

[PATCH v2 03/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.

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

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 81646126267..7db94206212 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -219,6 +219,46 @@
   'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
   'if': 'defined(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': 'defined(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': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
@@ -285,44 +325,3 @@
 ##
 { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
   'if': 'defined(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': 'defined(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': 'defined(TARGET_I386)' }
-- 
2.31.1

Re: [PATCH v2 03/23] qapi/misc-target: Group SEV QAPI definitions

[Dov Murik]

Phil,


On 16/06/2021 23:43, Philippe Mathieu-Daudé wrote:
> There is already a section with various SEV commands / types,
> so move the SEV guest attestation together.
> 

I have two questions (regarding several qapi patches in this series):

1. Should we extract all the SEV commands/types to a separate file?
Maybe sev.json, or confidential-guest-sev.json -- anticipating the other
platforms.

2. I see that some qapi types/commands are conditioned on
`defined(CONFIG_...)`. For example in qapi/tpm.json we have:

  { 'command': 'query-tpm-types', 'returns': ['TpmType'],
    'if': 'defined(CONFIG_TPM)' }

I wonder if the same applies to SEV.

-Dov


> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  qapi/misc-target.json | 81 +++++++++++++++++++++----------------------
>  1 file changed, 40 insertions(+), 41 deletions(-)
> 
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 81646126267..7db94206212 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -219,6 +219,46 @@
>    'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
>    'if': 'defined(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': 'defined(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': 'defined(TARGET_I386)' }
> +
>  ##
>  # @dump-skeys:
>  #
> @@ -285,44 +325,3 @@
>  ##
>  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
>    'if': 'defined(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': 'defined(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': 'defined(TARGET_I386)' }
> 

Re: [PATCH v2 03/23] qapi/misc-target: Group SEV QAPI definitions

[Philippe Mathieu-Daudé]

On 6/24/21 8:13 AM, Dov Murik wrote:
> Phil,
> 
> 
> On 16/06/2021 23:43, Philippe Mathieu-Daudé wrote:
>> There is already a section with various SEV commands / types,
>> so move the SEV guest attestation together.
>>
> 
> I have two questions (regarding several qapi patches in this series):
> 
> 1. Should we extract all the SEV commands/types to a separate file?
> Maybe sev.json, or confidential-guest-sev.json -- anticipating the other
> platforms.

Certainly. "confidential-guest" matches QEMU naming, and I agree
having the implementation as extension (here "-sev") will reduce
the noise on other maintainers each time this file is modified.

> 2. I see that some qapi types/commands are conditioned on
> `defined(CONFIG_...)`. For example in qapi/tpm.json we have:
> 
>   { 'command': 'query-tpm-types', 'returns': ['TpmType'],
>     'if': 'defined(CONFIG_TPM)' }
> 
> I wonder if the same applies to SEV.

This part I am not sure so I'll defer to Markus / Paolo / anyone
who knows. My understanding is QAPI is generated at some point
in the buildsys process, so only the configuration features
passed via the ninja invocation can be used (see after NINJA
in ./configure). I hope I'm wrong and buildsys/QAPI works as
you expect, but haven't tried :)

Regards,

Phil.

Re: [PATCH v2 03/23] qapi/misc-target: Group SEV QAPI definitions

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> There is already a section with various SEV commands / types,
> so move the SEV guest attestation together.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  qapi/misc-target.json | 81 +++++++++++++++++++++----------------------
>  1 file changed, 40 insertions(+), 41 deletions(-)
>
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 81646126267..7db94206212 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -219,6 +219,46 @@
>    'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
>    'if': 'defined(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': 'defined(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" } }

You're fixing the typo from PATCH 02 here.

> +# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
> +#
> +##
> +{ 'command': 'query-sev-attestation-report',
> +  'data': { 'mnonce': 'str' },
> +  'returns': 'SevAttestationReport',
> +  'if': 'defined(TARGET_I386)' }
> +
>  ##
>  # @dump-skeys:
>  #
> @@ -285,44 +325,3 @@
>  ##
>  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
>    'if': 'defined(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': 'defined(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': 'defined(TARGET_I386)' }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[PATCH v2 04/23] target/i386/monitor: Return QMP error when SEV is disabled in build

[Philippe Mathieu-Daudé]

If the management layer tries to inject a secret, it gets an empty
response in case the binary built without SEV:

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

Make it clearer by returning an error, 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: 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 119211f0b06..c83cca80dc2 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"
@@ -742,6 +743,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, QERR_UNSUPPORTED);
+        return;
+    }
     if (!has_gpa) {
         uint8_t *data;
         struct sev_secret_area *area;
-- 
2.31.1

[PATCH v2 05/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>
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 a9fe1662d39..694031e4aec 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 v2 06/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>
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 ae6d8404787..f4223f1febf 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 0227cb51778..d91c2ece784 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 v2 07/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>
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 f4223f1febf..afa19a0a161 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -25,7 +25,6 @@
 #define SEV_POLICY_SEV          0x20
 
 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 d91c2ece784..eb0c89bf2be 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 83df8c09f6a..0a36e81f66c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -64,7 +64,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;
@@ -362,12 +361,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)
 {
@@ -810,8 +803,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 v2 08/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>
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 eb0c89bf2be..4668365fd3e 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 v2 09/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>
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 0a36e81f66c..791804954e9 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -493,8 +493,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;
 
@@ -514,7 +514,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;
     }
 
@@ -525,7 +524,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
         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));
-            g_free(buf);
             return NULL;
         }
     }
@@ -540,7 +538,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
     if (ret) {
         error_setg_errno(errp, errno, "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);
@@ -548,9 +546,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 v2 10/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.

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 4668365fd3e..8eae5d2fa8d 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,45 +30,7 @@ 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;
-}
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 v2 11/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.

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

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index afa19a0a161..cb5702a2397 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -30,7 +30,5 @@ 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);
 
 #endif
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index c83cca80dc2..eb4539bf88b 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -762,9 +762,3 @@ 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);
-}
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index d556b4f091f..1f7573ad528 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_i386.h"
 
@@ -52,9 +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");
+    error_setg(errp, QERR_UNSUPPORTED);
     return NULL;
 }
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 791804954e9..809054a84f2 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -31,6 +31,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"
 
@@ -487,8 +489,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;
@@ -549,6 +551,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 v2 12/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.

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 eb4539bf88b..95b9ada189e 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -731,34 +731,3 @@ 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, QERR_UNSUPPORTED);
-        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);
-}
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 1f7573ad528..a66ae4f5efb 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, QERR_UNSUPPORTED);
 }
 
 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 809054a84f2..7cdf8d857c8 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -955,6 +955,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, QERR_UNSUPPORTED);
+        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 v2 13/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.

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

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index cb5702a2397..62ee2e1b16a 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -29,6 +29,5 @@ 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);
 
 #endif
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 95b9ada189e..6c6d0b5d707 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -726,8 +726,3 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
 
     return info;
 }
-
-SevCapability *qmp_query_sev_capabilities(Error **errp)
-{
-    return sev_get_capabilities(errp);
-}
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index a66ae4f5efb..e62ac5edd83 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -27,9 +27,9 @@ 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");
+    error_setg(errp, QERR_UNSUPPORTED);
     return NULL;
 }
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 7cdf8d857c8..23295d74c8b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -438,8 +438,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;
@@ -489,6 +488,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 v2 14/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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/sev_i386.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_i386.h b/target/i386/sev_i386.h
index 62ee2e1b16a..615cebea1a8 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -28,6 +28,5 @@ extern bool sev_es_enabled(void);
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
-extern char *sev_get_launch_measurement(void);
 
 #endif
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 6c6d0b5d707..d2289d1fb47 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -709,20 +709,3 @@ 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, "Measurement is not available");
-        return NULL;
-    }
-
-    info = g_malloc0(sizeof(*info));
-    info->data = data;
-
-    return info;
-}
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index e62ac5edd83..f5e7536f987 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, QERR_UNSUPPORTED);
     return NULL;
 }
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 23295d74c8b..4ddd24f6bdd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -718,8 +718,7 @@ free_measurement:
     g_free(measurement);
 }
 
-char *
-sev_get_launch_measurement(void)
+static char *sev_get_launch_measurement(void)
 {
     if (sev_guest &&
         sev_guest->state >= SEV_STATE_LAUNCH_SECRET) {
@@ -729,6 +728,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, "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 v2 15/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().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Note: what is left in sev_i386.h eventually belong to "sysemu/sev.h"
      meaning we could remove this local header.
---
 target/i386/sev_i386.h        |  2 --
 target/i386/monitor.c         | 37 ---------------------------------
 target/i386/sev-sysemu-stub.c | 10 ++++++++-
 target/i386/sev.c             | 39 +++++++++++++++++++++++++++++++++--
 4 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 615cebea1a8..1ddb8df9c99 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -15,7 +15,6 @@
 #define QEMU_SEV_I386_H
 
 #include "sysemu/sev.h"
-#include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG        0x1
 #define SEV_POLICY_NOKS         0x2
@@ -25,7 +24,6 @@
 #define SEV_POLICY_SEV          0x20
 
 extern bool sev_es_enabled(void);
-extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);
 extern uint32_t sev_get_reduced_phys_bits(void);
 
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index d2289d1fb47..af3501095e5 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -28,11 +28,8 @@
 #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"
-#include "sev_i386.h"
 #include "qapi/qapi-commands-misc-target.h"
 #include "qapi/qapi-commands-misc.h"
 #include "hw/i386/pc.h"
@@ -675,37 +672,3 @@ void hmp_info_io_apic(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "This command is obsolete and will be "
                    "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);
-}
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index f5e7536f987..7a35f0432b2 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_i386.h"
 
-SevInfo *sev_get_info(void)
+SevInfo *qmp_query_sev(Error **errp)
 {
+    error_setg(errp, QERR_UNSUPPORTED);
     return NULL;
 }
 
@@ -59,3 +62,8 @@ SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce, Error
     error_setg(errp, QERR_UNSUPPORTED);
     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 4ddd24f6bdd..faa3a4015c4 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -27,10 +27,12 @@
 #include "sev_i386.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
+#include "sysemu/sev.h"
 #include "trace.h"
 #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"
@@ -375,8 +377,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;
 
@@ -395,6 +396,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 v2 16/23] monitor: Restrict 'info sev' to x86 targets

[Philippe Mathieu-Daudé]

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 60fc92722ae..20adbea5154 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -49,5 +49,6 @@ 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);
 
 #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 7a35f0432b2..aba02f3c332 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 faa3a4015c4..339564ad2c8 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 v2 17/23] hw/i386/acpi-common: Remove unused includes

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/acpi-common.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 1f5947fcf96..77afebd9e1f 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -21,15 +21,10 @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/error.h"
-
-#include "exec/memory.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/aml-build.h"
-#include "hw/acpi/utils.h"
 #include "hw/i386/pc.h"
 #include "target/i386/cpu.h"
-
 #include "acpi-build.h"
 #include "acpi-common.h"
 
-- 
2.31.1

[PATCH v2 18/23] hw/i386: Rename acpi-build.c -> acpi-pc.c

[Philippe Mathieu-Daudé]

The acpi-build.c file contains routines common to the QEMU "PC"
machines (Q35+ICH & i440FX+PIIX), rename it as acpi-pc.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/{acpi-build.c => acpi-pc.c} | 0
 hw/i386/meson.build                 | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename hw/i386/{acpi-build.c => acpi-pc.c} (100%)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-pc.c
similarity index 100%
rename from hw/i386/acpi-build.c
rename to hw/i386/acpi-pc.c
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index e5d109f5c64..f95ed7d4efe 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -22,7 +22,7 @@
 i386_ss.add(when: 'CONFIG_PC', if_true: files(
   'pc.c',
   'pc_sysfw.c',
-  'acpi-build.c',
+  'acpi-pc.c',
   'port92.c'))
 
 subdir('kvm')
-- 
2.31.1

[PATCH v2 19/23] hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c

[Philippe Mathieu-Daudé]

pc_madt_cpu_entry() is specific to QEMU 'PC' machines,
move it to acpi-pc.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/acpi-common.c | 40 ----------------------------------------
 hw/i386/acpi-pc.c     | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 77afebd9e1f..5ae1853b6f2 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -23,49 +23,10 @@
 #include "qemu/osdep.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/aml-build.h"
-#include "hw/i386/pc.h"
 #include "target/i386/cpu.h"
 #include "acpi-build.h"
 #include "acpi-common.h"
 
-void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
-                       const CPUArchIdList *apic_ids, GArray *entry)
-{
-    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
-
-    /* ACPI spec says that LAPIC entry for non present
-     * CPU may be omitted from MADT or it must be marked
-     * as disabled. However omitting non present CPU from
-     * MADT breaks hotplug on linux. So possible CPUs
-     * should be put in MADT but kept disabled.
-     */
-    if (apic_id < 255) {
-        AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic);
-
-        apic->type = ACPI_APIC_PROCESSOR;
-        apic->length = sizeof(*apic);
-        apic->processor_id = uid;
-        apic->local_apic_id = apic_id;
-        if (apic_ids->cpus[uid].cpu != NULL) {
-            apic->flags = cpu_to_le32(1);
-        } else {
-            apic->flags = cpu_to_le32(0);
-        }
-    } else {
-        AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
-
-        apic->type = ACPI_APIC_LOCAL_X2APIC;
-        apic->length = sizeof(*apic);
-        apic->uid = cpu_to_le32(uid);
-        apic->x2apic_id = cpu_to_le32(apic_id);
-        if (apic_ids->cpus[uid].cpu != NULL) {
-            apic->flags = cpu_to_le32(1);
-        } else {
-            apic->flags = cpu_to_le32(0);
-        }
-    }
-}
-
 void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
                      X86MachineState *x86ms, AcpiDeviceIf *adev,
                      const char *oem_id, const char *oem_table_id)
@@ -155,4 +116,3 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
                  (void *)(table_data->data + madt_start), "APIC",
                  table_data->len - madt_start, 1, oem_id, oem_table_id);
 }
-
diff --git a/hw/i386/acpi-pc.c b/hw/i386/acpi-pc.c
index 796ffc6f5c4..a3cd60d81e6 100644
--- a/hw/i386/acpi-pc.c
+++ b/hw/i386/acpi-pc.c
@@ -2707,3 +2707,42 @@ void acpi_setup(void)
      */
     acpi_build_tables_cleanup(&tables, false);
 }
+
+void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
+                       const CPUArchIdList *apic_ids, GArray *entry)
+{
+    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
+
+    /*
+     * ACPI spec says that LAPIC entry for non present
+     * CPU may be omitted from MADT or it must be marked
+     * as disabled. However omitting non present CPU from
+     * MADT breaks hotplug on linux. So possible CPUs
+     * should be put in MADT but kept disabled.
+     */
+    if (apic_id < 255) {
+        AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic);
+
+        apic->type = ACPI_APIC_PROCESSOR;
+        apic->length = sizeof(*apic);
+        apic->processor_id = uid;
+        apic->local_apic_id = apic_id;
+        if (apic_ids->cpus[uid].cpu != NULL) {
+            apic->flags = cpu_to_le32(1);
+        } else {
+            apic->flags = cpu_to_le32(0);
+        }
+    } else {
+        AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
+
+        apic->type = ACPI_APIC_LOCAL_X2APIC;
+        apic->length = sizeof(*apic);
+        apic->uid = cpu_to_le32(uid);
+        apic->x2apic_id = cpu_to_le32(apic_id);
+        if (apic_ids->cpus[uid].cpu != NULL) {
+            apic->flags = cpu_to_le32(1);
+        } else {
+            apic->flags = cpu_to_le32(0);
+        }
+    }
+}
-- 
2.31.1

Re: [PATCH v2 19/23] hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c

[Igor Mammedov]

On Wed, 16 Jun 2021 22:43:24 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> pc_madt_cpu_entry() is specific to QEMU 'PC' machines,
> move it to acpi-pc.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/i386/acpi-common.c | 40 ----------------------------------------
>  hw/i386/acpi-pc.c     | 39 +++++++++++++++++++++++++++++++++++++++

it's used not only by PC machines but also microvm,
which didn't use acpi-build.c (aka acpi-pc.c)
it only links fine by virtue that PC machines
are object files are also included.

>  2 files changed, 39 insertions(+), 40 deletions(-)
> 
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 77afebd9e1f..5ae1853b6f2 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -23,49 +23,10 @@
>  #include "qemu/osdep.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/aml-build.h"
> -#include "hw/i386/pc.h"
>  #include "target/i386/cpu.h"
>  #include "acpi-build.h"
>  #include "acpi-common.h"
>  
> -void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> -                       const CPUArchIdList *apic_ids, GArray *entry)
> -{
> -    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
> -
> -    /* ACPI spec says that LAPIC entry for non present
> -     * CPU may be omitted from MADT or it must be marked
> -     * as disabled. However omitting non present CPU from
> -     * MADT breaks hotplug on linux. So possible CPUs
> -     * should be put in MADT but kept disabled.
> -     */
> -    if (apic_id < 255) {
> -        AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic);
> -
> -        apic->type = ACPI_APIC_PROCESSOR;
> -        apic->length = sizeof(*apic);
> -        apic->processor_id = uid;
> -        apic->local_apic_id = apic_id;
> -        if (apic_ids->cpus[uid].cpu != NULL) {
> -            apic->flags = cpu_to_le32(1);
> -        } else {
> -            apic->flags = cpu_to_le32(0);
> -        }
> -    } else {
> -        AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
> -
> -        apic->type = ACPI_APIC_LOCAL_X2APIC;
> -        apic->length = sizeof(*apic);
> -        apic->uid = cpu_to_le32(uid);
> -        apic->x2apic_id = cpu_to_le32(apic_id);
> -        if (apic_ids->cpus[uid].cpu != NULL) {
> -            apic->flags = cpu_to_le32(1);
> -        } else {
> -            apic->flags = cpu_to_le32(0);
> -        }
> -    }
> -}
> -
>  void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>                       X86MachineState *x86ms, AcpiDeviceIf *adev,
>                       const char *oem_id, const char *oem_table_id)
> @@ -155,4 +116,3 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
>                   (void *)(table_data->data + madt_start), "APIC",
>                   table_data->len - madt_start, 1, oem_id, oem_table_id);
>  }
> -
> diff --git a/hw/i386/acpi-pc.c b/hw/i386/acpi-pc.c
> index 796ffc6f5c4..a3cd60d81e6 100644
> --- a/hw/i386/acpi-pc.c
> +++ b/hw/i386/acpi-pc.c
> @@ -2707,3 +2707,42 @@ void acpi_setup(void)
>       */
>      acpi_build_tables_cleanup(&tables, false);
>  }
> +
> +void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> +                       const CPUArchIdList *apic_ids, GArray *entry)
> +{
> +    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
> +
> +    /*
> +     * ACPI spec says that LAPIC entry for non present
> +     * CPU may be omitted from MADT or it must be marked
> +     * as disabled. However omitting non present CPU from
> +     * MADT breaks hotplug on linux. So possible CPUs
> +     * should be put in MADT but kept disabled.
> +     */
> +    if (apic_id < 255) {
> +        AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic);
> +
> +        apic->type = ACPI_APIC_PROCESSOR;
> +        apic->length = sizeof(*apic);
> +        apic->processor_id = uid;
> +        apic->local_apic_id = apic_id;
> +        if (apic_ids->cpus[uid].cpu != NULL) {
> +            apic->flags = cpu_to_le32(1);
> +        } else {
> +            apic->flags = cpu_to_le32(0);
> +        }
> +    } else {
> +        AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
> +
> +        apic->type = ACPI_APIC_LOCAL_X2APIC;
> +        apic->length = sizeof(*apic);
> +        apic->uid = cpu_to_le32(uid);
> +        apic->x2apic_id = cpu_to_le32(apic_id);
> +        if (apic_ids->cpus[uid].cpu != NULL) {
> +            apic->flags = cpu_to_le32(1);
> +        } else {
> +            apic->flags = cpu_to_le32(0);
> +        }
> +    }
> +}

Re: [PATCH v2 19/23] hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c

[Philippe Mathieu-Daudé]

On 6/18/21 1:37 PM, Igor Mammedov wrote:
> On Wed, 16 Jun 2021 22:43:24 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> pc_madt_cpu_entry() is specific to QEMU 'PC' machines,
>> move it to acpi-pc.c.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/i386/acpi-common.c | 40 ----------------------------------------
>>  hw/i386/acpi-pc.c     | 39 +++++++++++++++++++++++++++++++++++++++
> 
> it's used not only by PC machines but also microvm,
> which didn't use acpi-build.c (aka acpi-pc.c)
> it only links fine by virtue that PC machines
> are object files are also included.

Is that something new? I can't see this in mainstream,
the microvm machine builds fine without this code.

Re: [PATCH v2 19/23] hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c

[Michael S. Tsirkin]

On Sat, Jun 19, 2021 at 10:45:17AM +0200, Philippe Mathieu-Daudé wrote:
> On 6/18/21 1:37 PM, Igor Mammedov wrote:
> > On Wed, 16 Jun 2021 22:43:24 +0200
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > 
> >> pc_madt_cpu_entry() is specific to QEMU 'PC' machines,
> >> move it to acpi-pc.c.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >>  hw/i386/acpi-common.c | 40 ----------------------------------------
> >>  hw/i386/acpi-pc.c     | 39 +++++++++++++++++++++++++++++++++++++++
> > 
> > it's used not only by PC machines but also microvm,
> > which didn't use acpi-build.c (aka acpi-pc.c)
> > it only links fine by virtue that PC machines
> > are object files are also included.
> 
> Is that something new? I can't see this in mainstream,
> the microvm machine builds fine without this code.

I think Igor means this:

hw/i386/generic_event_device_x86.c:    adevc->madt_cpu = pc_madt_cpu_entry;

and

hw/i386/microvm.c:        DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);

Admittedly given it's not limited to pc the function name is wrong ...

-- 
MST

Re: [PATCH v2 19/23] hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c

[Philippe Mathieu-Daudé]

On 6/19/21 11:32 PM, Michael S. Tsirkin wrote:
> On Sat, Jun 19, 2021 at 10:45:17AM +0200, Philippe Mathieu-Daudé wrote:
>> On 6/18/21 1:37 PM, Igor Mammedov wrote:
>>> On Wed, 16 Jun 2021 22:43:24 +0200
>>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>>> pc_madt_cpu_entry() is specific to QEMU 'PC' machines,
>>>> move it to acpi-pc.c.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  hw/i386/acpi-common.c | 40 ----------------------------------------
>>>>  hw/i386/acpi-pc.c     | 39 +++++++++++++++++++++++++++++++++++++++
>>>
>>> it's used not only by PC machines but also microvm,
>>> which didn't use acpi-build.c (aka acpi-pc.c)
>>> it only links fine by virtue that PC machines
>>> are object files are also included.
>>
>> Is that something new? I can't see this in mainstream,
>> the microvm machine builds fine without this code.
> 
> I think Igor means this:
> 
> hw/i386/generic_event_device_x86.c:    adevc->madt_cpu = pc_madt_cpu_entry;
> 
> and
> 
> hw/i386/microvm.c:        DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);

Oh I missed that, indeed I didn't runtime test.

Thanks Michael.

> Admittedly given it's not limited to pc the function name is wrong ...
> 

[PATCH v2 20/23] hw/acpi: Do not restrict ACPI core routines to x86 architecture

[Philippe Mathieu-Daudé]

ACPI core routines (in core.c) are not really x86-specific.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/acpi/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index dd69577212a..c23c67f4283 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -3,6 +3,7 @@
   'acpi_interface.c',
   'aml-build.c',
   'bios-linker-loader.c',
+  'core.c',
   'utils.c',
 ))
 acpi_ss.add(when: 'CONFIG_ACPI_CPU_HOTPLUG', if_true: files('cpu.c'))
@@ -14,7 +15,7 @@
 acpi_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: files('generic_event_device.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_HMAT', if_true: files('hmat.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'))
-acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('core.c', 'piix4.c', 'pcihp.c'), if_false: files('acpi-stub.c'))
+acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('piix4.c', 'pcihp.c'))
 acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
 acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
 acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
-- 
2.31.1

Re: [PATCH v2 20/23] hw/acpi: Do not restrict ACPI core routines to x86 architecture

[Michael S. Tsirkin]

On Wed, Jun 16, 2021 at 10:43:25PM +0200, Philippe Mathieu-Daudé wrote:
> ACPI core routines (in core.c) are not really x86-specific.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/acpi/meson.build | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index dd69577212a..c23c67f4283 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -3,6 +3,7 @@
>    'acpi_interface.c',
>    'aml-build.c',
>    'bios-linker-loader.c',
> +  'core.c',
>    'utils.c',
>  ))
>  acpi_ss.add(when: 'CONFIG_ACPI_CPU_HOTPLUG', if_true: files('cpu.c'))
> @@ -14,7 +15,7 @@
>  acpi_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: files('generic_event_device.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_HMAT', if_true: files('hmat.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'))
> -acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('core.c', 'piix4.c', 'pcihp.c'), if_false: files('acpi-stub.c'))
> +acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('piix4.c', 'pcihp.c'))


So is acpi-stub.c still useful then?

>  acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
>  acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
>  acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
> -- 
> 2.31.1

Re: [PATCH v2 20/23] hw/acpi: Do not restrict ACPI core routines to x86 architecture

[Philippe Mathieu-Daudé]

On 6/17/21 10:40 PM, Michael S. Tsirkin wrote:
> On Wed, Jun 16, 2021 at 10:43:25PM +0200, Philippe Mathieu-Daudé wrote:
>> ACPI core routines (in core.c) are not really x86-specific.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/acpi/meson.build | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
>> index dd69577212a..c23c67f4283 100644
>> --- a/hw/acpi/meson.build
>> +++ b/hw/acpi/meson.build
>> @@ -3,6 +3,7 @@
>>    'acpi_interface.c',
>>    'aml-build.c',
>>    'bios-linker-loader.c',
>> +  'core.c',
>>    'utils.c',
>>  ))
>>  acpi_ss.add(when: 'CONFIG_ACPI_CPU_HOTPLUG', if_true: files('cpu.c'))
>> @@ -14,7 +15,7 @@
>>  acpi_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: files('generic_event_device.c'))
>>  acpi_ss.add(when: 'CONFIG_ACPI_HMAT', if_true: files('hmat.c'))
>>  acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'))
>> -acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('core.c', 'piix4.c', 'pcihp.c'), if_false: files('acpi-stub.c'))
>> +acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('piix4.c', 'pcihp.c'))
> 
> 
> So is acpi-stub.c still useful then?

Certainly:

softmmu/vl.c:3256:                acpi_table_add(opts, &error_fatal);

Could be eventually merged with aml-build-stub.c...

[PATCH v2 21/23] hw/i386: Introduce X86_FW_OVMF Kconfig symbol

[Philippe Mathieu-Daudé]

Introduce the X86_FW_OVMF Kconfig symbol for OVMF-specific code.
Move the OVMF-specific code from pc_sysfw.c to pc_sysfw_ovmf.c,
adding a pair of stubs.
Update MAINTAINERS to reach OVMF maintainers when these new
files are modified.

This fixes when building the microvm machine standalone:

  /usr/bin/ld: libqemu-i386-softmmu.fa.p/target_i386_monitor.c.o: in
  function `qmp_sev_inject_launch_secret':
  target/i386/monitor.c:749: undefined reference to `pc_system_ovmf_table_find'

Fixes: f522cef9b35 ("sev: update sev-inject-launch-secret to make gpa optional")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Note:

1/ Obviously the MAINTAINERS change is pending Laszlo Ack.

2/ Various declarations from "hw/i386/pc.h" actually belong
   to a local header in hw/i386/, such pc_system_parse_ovmf_flash().
---
 include/hw/i386/pc.h          |   1 +
 hw/i386/pc_sysfw.c            | 107 ---------------------------
 hw/i386/pc_sysfw_ovmf-stubs.c |  26 +++++++
 hw/i386/pc_sysfw_ovmf.c       | 135 ++++++++++++++++++++++++++++++++++
 MAINTAINERS                   |   1 +
 hw/i386/Kconfig               |   4 +
 hw/i386/meson.build           |   2 +
 7 files changed, 169 insertions(+), 107 deletions(-)
 create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
 create mode 100644 hw/i386/pc_sysfw_ovmf.c

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1522a3359a9..6d5699f1ee8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -191,6 +191,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms);
 void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len);
+void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
 
 
 /* acpi-build.c */
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 6ce37a2b052..68d6b1f783e 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -124,113 +124,6 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
     }
 }
 
-#define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
-
-static uint8_t *ovmf_table;
-static int ovmf_table_len;
-
-static void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
-{
-    uint8_t *ptr;
-    QemuUUID guid;
-    int tot_len;
-
-    /* should only be called once */
-    if (ovmf_table) {
-        return;
-    }
-
-    if (flash_size < TARGET_PAGE_SIZE) {
-        return;
-    }
-
-    /*
-     * if this is OVMF there will be a table footer
-     * guid 48 bytes before the end of the flash file.  If it's
-     * not found, silently abort the flash parsing.
-     */
-    qemu_uuid_parse(OVMF_TABLE_FOOTER_GUID, &guid);
-    guid = qemu_uuid_bswap(guid); /* guids are LE */
-    ptr = flash_ptr + flash_size - 48;
-    if (!qemu_uuid_is_equal((QemuUUID *)ptr, &guid)) {
-        return;
-    }
-
-    /* if found, just before is two byte table length */
-    ptr -= sizeof(uint16_t);
-    tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t);
-
-    if (tot_len <= 0) {
-        return;
-    }
-
-    ovmf_table = g_malloc(tot_len);
-    ovmf_table_len = tot_len;
-
-    /*
-     * ptr is the foot of the table, so copy it all to the newly
-     * allocated ovmf_table and then set the ovmf_table pointer
-     * to the table foot
-     */
-    memcpy(ovmf_table, ptr - tot_len, tot_len);
-    ovmf_table += tot_len;
-}
-
-bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
-                               int *data_len)
-{
-    uint8_t *ptr = ovmf_table;
-    int tot_len = ovmf_table_len;
-    QemuUUID entry_guid;
-
-    if (qemu_uuid_parse(entry, &entry_guid) < 0) {
-        return false;
-    }
-
-    if (!ptr) {
-        return false;
-    }
-
-    entry_guid = qemu_uuid_bswap(entry_guid); /* guids are LE */
-    while (tot_len >= sizeof(QemuUUID) + sizeof(uint16_t)) {
-        int len;
-        QemuUUID *guid;
-
-        /*
-         * The data structure is
-         *   arbitrary length data
-         *   2 byte length of entire entry
-         *   16 byte guid
-         */
-        guid = (QemuUUID *)(ptr - sizeof(QemuUUID));
-        len = le16_to_cpu(*(uint16_t *)(ptr - sizeof(QemuUUID) -
-                                        sizeof(uint16_t)));
-
-        /*
-         * just in case the table is corrupt, wouldn't want to spin in
-         * the zero case
-         */
-        if (len < sizeof(QemuUUID) + sizeof(uint16_t)) {
-            return false;
-        } else if (len > tot_len) {
-            return false;
-        }
-
-        ptr -= len;
-        tot_len -= len;
-        if (qemu_uuid_is_equal(guid, &entry_guid)) {
-            if (data) {
-                *data = ptr;
-            }
-            if (data_len) {
-                *data_len = len - sizeof(QemuUUID) - sizeof(uint16_t);
-            }
-            return true;
-        }
-    }
-    return false;
-}
-
 /*
  * Map the pcms->flash[] from 4GiB downward, and realize.
  * Map them in descending order, i.e. pcms->flash[0] at the top,
diff --git a/hw/i386/pc_sysfw_ovmf-stubs.c b/hw/i386/pc_sysfw_ovmf-stubs.c
new file mode 100644
index 00000000000..aabe78b2710
--- /dev/null
+++ b/hw/i386/pc_sysfw_ovmf-stubs.c
@@ -0,0 +1,26 @@
+/*
+ * QEMU PC System Firmware (OVMF stubs)
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * Author:
+ *   Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * 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 "hw/i386/pc.h"
+
+bool pc_system_ovmf_table_find(const char *entry, uint8_t **data, int *data_len)
+{
+    g_assert_not_reached();
+}
+
+void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
+{
+    g_assert_not_reached();
+}
diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
new file mode 100644
index 00000000000..85b3044c59a
--- /dev/null
+++ b/hw/i386/pc_sysfw_ovmf.c
@@ -0,0 +1,135 @@
+/*
+ * QEMU PC System Firmware (OVMF specific)
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2011-2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+#include "cpu.h"
+
+#define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
+
+static uint8_t *ovmf_table;
+static int ovmf_table_len;
+
+void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
+{
+    uint8_t *ptr;
+    QemuUUID guid;
+    int tot_len;
+
+    /* should only be called once */
+    if (ovmf_table) {
+        return;
+    }
+
+    if (flash_size < TARGET_PAGE_SIZE) {
+        return;
+    }
+
+    /*
+     * if this is OVMF there will be a table footer
+     * guid 48 bytes before the end of the flash file.  If it's
+     * not found, silently abort the flash parsing.
+     */
+    qemu_uuid_parse(OVMF_TABLE_FOOTER_GUID, &guid);
+    guid = qemu_uuid_bswap(guid); /* guids are LE */
+    ptr = flash_ptr + flash_size - 48;
+    if (!qemu_uuid_is_equal((QemuUUID *)ptr, &guid)) {
+        return;
+    }
+
+    /* if found, just before is two byte table length */
+    ptr -= sizeof(uint16_t);
+    tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t);
+
+    if (tot_len <= 0) {
+        return;
+    }
+
+    ovmf_table = g_malloc(tot_len);
+    ovmf_table_len = tot_len;
+
+    /*
+     * ptr is the foot of the table, so copy it all to the newly
+     * allocated ovmf_table and then set the ovmf_table pointer
+     * to the table foot
+     */
+    memcpy(ovmf_table, ptr - tot_len, tot_len);
+    ovmf_table += tot_len;
+}
+
+bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
+                               int *data_len)
+{
+    uint8_t *ptr = ovmf_table;
+    int tot_len = ovmf_table_len;
+    QemuUUID entry_guid;
+
+    if (qemu_uuid_parse(entry, &entry_guid) < 0) {
+        return false;
+    }
+
+    if (!ptr) {
+        return false;
+    }
+
+    entry_guid = qemu_uuid_bswap(entry_guid); /* guids are LE */
+    while (tot_len >= sizeof(QemuUUID) + sizeof(uint16_t)) {
+        int len;
+        QemuUUID *guid;
+
+        /*
+         * The data structure is
+         *   arbitrary length data
+         *   2 byte length of entire entry
+         *   16 byte guid
+         */
+        guid = (QemuUUID *)(ptr - sizeof(QemuUUID));
+        len = le16_to_cpu(*(uint16_t *)(ptr - sizeof(QemuUUID) -
+                                        sizeof(uint16_t)));
+
+        /*
+         * just in case the table is corrupt, wouldn't want to spin in
+         * the zero case
+         */
+        if (len < sizeof(QemuUUID) + sizeof(uint16_t)) {
+            return false;
+        } else if (len > tot_len) {
+            return false;
+        }
+
+        ptr -= len;
+        tot_len -= len;
+        if (qemu_uuid_is_equal(guid, &entry_guid)) {
+            if (data) {
+                *data = ptr;
+            }
+            if (data_len) {
+                *data_len = len - sizeof(QemuUUID) - sizeof(uint16_t);
+            }
+            return true;
+        }
+    }
+    return false;
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index fbfaaa54721..0856a10e83a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2900,6 +2900,7 @@ EDK2 Firmware
 M: Laszlo Ersek <lersek@redhat.com>
 M: Philippe Mathieu-Daudé <philmd@redhat.com>
 S: Supported
+F: hw/i386/*ovmf*
 F: pc-bios/descriptors/??-edk2-*.json
 F: pc-bios/edk2-*
 F: roms/Makefile.edk2
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 66838fa397b..722f70eaeb8 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -1,5 +1,9 @@
+config X86_FW_OVMF
+    bool
+
 config SEV
     bool
+    select X86_FW_OVMF
     depends on KVM
 
 config PC
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index f95ed7d4efe..ca81875aa3d 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -24,6 +24,8 @@
   'pc_sysfw.c',
   'acpi-pc.c',
   'port92.c'))
+i386_ss.add(when: 'CONFIG_X86_FW_OVMF', if_true: files('pc_sysfw_ovmf.c'),
+                                        if_false: files('pc_sysfw_ovmf-stubs.c'))
 
 subdir('kvm')
 subdir('xen')
-- 
2.31.1

Re: [PATCH v2 21/23] hw/i386: Introduce X86_FW_OVMF Kconfig symbol

[Philippe Mathieu-Daudé]

On 6/16/21 10:43 PM, Philippe Mathieu-Daudé wrote:
> Introduce the X86_FW_OVMF Kconfig symbol for OVMF-specific code.
> Move the OVMF-specific code from pc_sysfw.c to pc_sysfw_ovmf.c,
> adding a pair of stubs.
> Update MAINTAINERS to reach OVMF maintainers when these new
> files are modified.
> 
> This fixes when building the microvm machine standalone:
> 
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/target_i386_monitor.c.o: in
>   function `qmp_sev_inject_launch_secret':
>   target/i386/monitor.c:749: undefined reference to `pc_system_ovmf_table_find'
> 
> Fixes: f522cef9b35 ("sev: update sev-inject-launch-secret to make gpa optional")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks, queued to fw-edk2 tree.

[PATCH v2 22/23] hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)

[Philippe Mathieu-Daudé]

The 'microvm' machine misses various dependencies:

  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_setup_microvm':
  hw/i386/acpi-microvm.c:247: undefined reference to `acpi_build_tables_init'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_build_microvm':
  hw/i386/acpi-microvm.c:192: undefined reference to `bios_linker_loader_alloc'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `build_dsdt_microvm':
  hw/i386/acpi-microvm.c:121: undefined reference to `init_aml_allocator'
  /usr/bin/ld: hw/i386/acpi-microvm.c:124: undefined reference to `acpi_data_push'
  /usr/bin/ld: hw/i386/acpi-microvm.c:126: undefined reference to `aml_scope'
  /usr/bin/ld: hw/i386/acpi-microvm.c:129: undefined reference to `build_ged_aml'
  /usr/bin/ld: hw/i386/acpi-microvm.c:131: undefined reference to `acpi_dsdt_add_power_button'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_dsdt_add_virtio':
  hw/i386/acpi-microvm.c:77: undefined reference to `aml_string'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `build_dsdt_microvm':
  hw/i386/acpi-microvm.c:138: undefined reference to `aml_scope'
  /usr/bin/ld: hw/i386/acpi-microvm.c:149: undefined reference to `build_header'
  /usr/bin/ld: hw/i386/acpi-microvm.c:152: undefined reference to `free_aml_allocator'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_build_microvm':
  hw/i386/acpi-microvm.c:202: undefined reference to `acpi_add_table'
  /usr/bin/ld: hw/i386/acpi-microvm.c:203: undefined reference to `build_fadt'
  /usr/bin/ld: hw/i386/acpi-microvm.c:206: undefined reference to `acpi_add_table'
  /usr/bin/ld: hw/i386/acpi-microvm.c:207: undefined reference to `acpi_build_madt'
  /usr/bin/ld: hw/i386/acpi-microvm.c:212: undefined reference to `build_xsdt'
  /usr/bin/ld: hw/i386/acpi-microvm.c:224: undefined reference to `build_rsdp'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_setup_microvm':
  hw/i386/acpi-microvm.c:251: undefined reference to `acpi_add_rom_blob'
  /usr/bin/ld: hw/i386/acpi-microvm.c:253: undefined reference to `acpi_add_rom_blob'
  /usr/bin/ld: hw/i386/acpi-microvm.c:255: undefined reference to `acpi_add_rom_blob'
  /usr/bin/ld: hw/i386/acpi-microvm.c:258: undefined reference to `acpi_build_tables_cleanup'
  /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_dsdt_add_pci':
  hw/i386/acpi-microvm.c:105: undefined reference to `acpi_dsdt_add_gpex'
  collect2: error: ld returned 1 exit status
  ninja: build stopped: subcommand failed.

Update the ACPI_HW_REDUCED symbol to select ACPI_MEMORY_HOTPLUG /
ACPI_NVDIMM / ACPI.

Fixes: 8045df14bcc ("microvm/acpi: add minimal acpi support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/acpi/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index 1932f66af8d..cfc4ede8d91 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -42,3 +42,7 @@ config ACPI_VMGENID
     depends on PC
 
 config ACPI_HW_REDUCED
+    bool
+    select ACPI
+    select ACPI_MEMORY_HOTPLUG
+    select ACPI_NVDIMM
-- 
2.31.1

Re: [PATCH v2 22/23] hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)

[Gerd Hoffmann]

On Wed, Jun 16, 2021 at 10:43:27PM +0200, Philippe Mathieu-Daudé wrote:
> The 'microvm' machine misses various dependencies:
> 
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_setup_microvm':
>   hw/i386/acpi-microvm.c:247: undefined reference to `acpi_build_tables_init'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_build_microvm':
>   hw/i386/acpi-microvm.c:192: undefined reference to `bios_linker_loader_alloc'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `build_dsdt_microvm':
>   hw/i386/acpi-microvm.c:121: undefined reference to `init_aml_allocator'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:124: undefined reference to `acpi_data_push'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:126: undefined reference to `aml_scope'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:129: undefined reference to `build_ged_aml'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:131: undefined reference to `acpi_dsdt_add_power_button'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_dsdt_add_virtio':
>   hw/i386/acpi-microvm.c:77: undefined reference to `aml_string'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `build_dsdt_microvm':
>   hw/i386/acpi-microvm.c:138: undefined reference to `aml_scope'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:149: undefined reference to `build_header'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:152: undefined reference to `free_aml_allocator'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_build_microvm':
>   hw/i386/acpi-microvm.c:202: undefined reference to `acpi_add_table'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:203: undefined reference to `build_fadt'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:206: undefined reference to `acpi_add_table'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:207: undefined reference to `acpi_build_madt'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:212: undefined reference to `build_xsdt'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:224: undefined reference to `build_rsdp'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_setup_microvm':
>   hw/i386/acpi-microvm.c:251: undefined reference to `acpi_add_rom_blob'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:253: undefined reference to `acpi_add_rom_blob'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:255: undefined reference to `acpi_add_rom_blob'
>   /usr/bin/ld: hw/i386/acpi-microvm.c:258: undefined reference to `acpi_build_tables_cleanup'
>   /usr/bin/ld: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o: in function `acpi_dsdt_add_pci':
>   hw/i386/acpi-microvm.c:105: undefined reference to `acpi_dsdt_add_gpex'
>   collect2: error: ld returned 1 exit status
>   ninja: build stopped: subcommand failed.
> 
> Update the ACPI_HW_REDUCED symbol to select ACPI_MEMORY_HOTPLUG /
> ACPI_NVDIMM / ACPI.
> 
> Fixes: 8045df14bcc ("microvm/acpi: add minimal acpi support")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

[PATCH v2 23/23] hw/i386/Kconfig: Add missing Kconfig dependency (runtime error)

[Philippe Mathieu-Daudé]

When building the 'microvm' machine stand-alone we get:

  $ qemu-system-x86_64 -M microvm
  **
  ERROR:qom/object.c:714:object_new_with_type: assertion failed: (type != NULL)
  Bail out! ERROR:qom/object.c:714:object_new_with_type: assertion failed: (type != NULL)
  Aborted (core dumped)

Looking at the backtrace:

  (gdb) bt
  #3  0x00007ff2330492ff in g_assertion_message_expr () at /lib64/libglib-2.0.so.0
  #4  0x000055a878c18341 in object_new_with_type (type=<optimized out>) at qom/object.c:714
  #5  0x000055a878c18399 in object_new (typename=typename@entry=0x55a878dec36a "isa-pit") at qom/object.c:747
  #6  0x000055a878cc8146 in qdev_new (name=name@entry=0x55a878dec36a "isa-pit") at hw/core/qdev.c:153
  #7  0x000055a878a8b439 in isa_new (name=name@entry=0x55a878dec36a "isa-pit") at hw/isa/isa-bus.c:160
  #8  0x000055a878adb782 in i8254_pit_init (base=64, isa_irq=0, alt_irq=0x0, bus=0x55a87ab38760) at include/hw/timer/i8254.h:54
  #9  microvm_devices_init (mms=0x55a87ac36800) at hw/i386/microvm.c:263
  #10 microvm_machine_state_init (machine=<optimized out>) at hw/i386/microvm.c:471
  #11 0x000055a878a944ab in machine_run_board_init (machine=machine@entry=0x55a87ac36800) at hw/core/machine.c:1239

The "isa-pit" type (TYPE_I8254) is missing. Add it.

Fixes: 0ebf007ddac ("hw/i386: Introduce the microvm machine type")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 722f70eaeb8..e723962be56 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -111,6 +111,7 @@ config MICROVM
     select ACPI_HW_REDUCED
     select PCI_EXPRESS_GENERIC_BRIDGE
     select USB_XHCI_SYSBUS
+    select I8254
 
 config X86_IOMMU
     bool
-- 
2.31.1

Re: [PATCH v2 23/23] hw/i386/Kconfig: Add missing Kconfig dependency (runtime error)

[Gerd Hoffmann]

On Wed, Jun 16, 2021 at 10:43:28PM +0200, Philippe Mathieu-Daudé wrote:
> When building the 'microvm' machine stand-alone we get:
> 
>   $ qemu-system-x86_64 -M microvm
>   **
>   ERROR:qom/object.c:714:object_new_with_type: assertion failed: (type != NULL)
>   Bail out! ERROR:qom/object.c:714:object_new_with_type: assertion failed: (type != NULL)
>   Aborted (core dumped)
> 
> Looking at the backtrace:
> 
>   (gdb) bt
>   #3  0x00007ff2330492ff in g_assertion_message_expr () at /lib64/libglib-2.0.so.0
>   #4  0x000055a878c18341 in object_new_with_type (type=<optimized out>) at qom/object.c:714
>   #5  0x000055a878c18399 in object_new (typename=typename@entry=0x55a878dec36a "isa-pit") at qom/object.c:747
>   #6  0x000055a878cc8146 in qdev_new (name=name@entry=0x55a878dec36a "isa-pit") at hw/core/qdev.c:153
>   #7  0x000055a878a8b439 in isa_new (name=name@entry=0x55a878dec36a "isa-pit") at hw/isa/isa-bus.c:160
>   #8  0x000055a878adb782 in i8254_pit_init (base=64, isa_irq=0, alt_irq=0x0, bus=0x55a87ab38760) at include/hw/timer/i8254.h:54
>   #9  microvm_devices_init (mms=0x55a87ac36800) at hw/i386/microvm.c:263
>   #10 microvm_machine_state_init (machine=<optimized out>) at hw/i386/microvm.c:471
>   #11 0x000055a878a944ab in machine_run_board_init (machine=machine@entry=0x55a87ac36800) at hw/core/machine.c:1239
> 
> The "isa-pit" type (TYPE_I8254) is missing. Add it.
> 
> Fixes: 0ebf007ddac ("hw/i386: Introduce the microvm machine type")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [PATCH v2 00/23] target/i386/sev: Housekeeping (OVMF + SEV-disabled binaries)

[Michael S. Tsirkin]

On Wed, Jun 16, 2021 at 10:43:05PM +0200, Philippe Mathieu-Daudé wrote:
> 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.
> 
> Some SEV patches I was following have been queued on Eduardo's
> 'x86-next' tree, so I used his tree as base, and included David and
> Connor patches to reduce merge conflicts.
> 
> Patches 1-16 are SEV related,
> patches 17-23 are OVMF related.
> 
> Special comment for Laszlo: Please don't review this version,
> wait for the respin (hoping the SEV cleanup patch get accepted,
> the respin will be focused on OVMF).
> 
> Based-on: https://gitlab.com/ehabkost/qemu/-/commits/x86-next/
> Supersedes: <20210610064556.1421620-1-philmd@redhat.com>


acpi things

Acked-by: Michael S. Tsirkin <mst@redhat.com>

do you want to split acpi things out so I can merge?

> Connor Kuehl (1):
>   MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV
> 
> Dr. David Alan Gilbert (1):
>   target/i386/sev: sev_get_attestation_report use g_autofree
> 
> Philippe Mathieu-Daudé (21):
>   qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
>   qapi/misc-target: Group SEV QAPI definitions
>   target/i386/monitor: Return QMP error when SEV is disabled in build
>   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: Restrict SEV to system emulation
>   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: Restrict 'info sev' to x86 targets
>   hw/i386/acpi-common: Remove unused includes
>   hw/i386: Rename acpi-build.c -> acpi-pc.c
>   hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c
>   hw/acpi: Do not restrict ACPI core routines to x86 architecture
>   hw/i386: Introduce X86_FW_OVMF Kconfig symbol
>   hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)
>   hw/i386/Kconfig: Add missing Kconfig dependency (runtime error)
> 
>  qapi/misc-target.json               |  78 ++++++++--------
>  include/hw/i386/pc.h                |   1 +
>  include/monitor/hmp-target.h        |   1 +
>  include/monitor/hmp.h               |   1 -
>  target/i386/sev_i386.h              |  11 ---
>  hw/i386/acpi-common.c               |  45 ----------
>  hw/i386/{acpi-build.c => acpi-pc.c} |  39 ++++++++
>  hw/i386/pc_sysfw.c                  | 107 ----------------------
>  hw/i386/pc_sysfw_ovmf-stubs.c       |  26 ++++++
>  hw/i386/pc_sysfw_ovmf.c             | 135 ++++++++++++++++++++++++++++
>  target/i386/cpu.c                   |   1 +
>  target/i386/monitor.c               |  91 -------------------
>  target/i386/sev-stub.c              |  49 +---------
>  target/i386/sev-sysemu-stub.c       |  69 ++++++++++++++
>  target/i386/sev.c                   | 130 +++++++++++++++++++++------
>  MAINTAINERS                         |   8 ++
>  hw/acpi/Kconfig                     |   4 +
>  hw/acpi/meson.build                 |   3 +-
>  hw/i386/Kconfig                     |   5 ++
>  hw/i386/meson.build                 |   4 +-
>  target/i386/meson.build             |   4 +-
>  21 files changed, 443 insertions(+), 369 deletions(-)
>  rename hw/i386/{acpi-build.c => acpi-pc.c} (98%)
>  create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
>  create mode 100644 hw/i386/pc_sysfw_ovmf.c
>  create mode 100644 target/i386/sev-sysemu-stub.c
> 
> -- 
> 2.31.1
> 

Re: [PATCH v2 00/23] target/i386/sev: Housekeeping (OVMF + SEV-disabled binaries)

[Igor Mammedov]

On Thu, 17 Jun 2021 16:42:24 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Jun 16, 2021 at 10:43:05PM +0200, Philippe Mathieu-Daudé wrote:
> > 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.
> > 
> > Some SEV patches I was following have been queued on Eduardo's
> > 'x86-next' tree, so I used his tree as base, and included David and
> > Connor patches to reduce merge conflicts.
> > 
> > Patches 1-16 are SEV related,
> > patches 17-23 are OVMF related.
> > 
> > Special comment for Laszlo: Please don't review this version,
> > wait for the respin (hoping the SEV cleanup patch get accepted,
> > the respin will be focused on OVMF).
> > 
> > Based-on: https://gitlab.com/ehabkost/qemu/-/commits/x86-next/
> > Supersedes: <20210610064556.1421620-1-philmd@redhat.com>  
> 
> 
> acpi things
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> do you want to split acpi things out so I can merge?
I agree they should have been split out.
(I see in cover letter SEV don't look into series anymore, so
this APCI patches which are totally irrelevant to SEV weren't
even unnoticed).

Perhaps I should take these into my ACPI refactoring series
(with your acks), which would need to be rebased on top of these.
Though I'd drop 19/23 which doesn't look correct to me.

> 
> > Connor Kuehl (1):
> >   MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV
> > 
> > Dr. David Alan Gilbert (1):
> >   target/i386/sev: sev_get_attestation_report use g_autofree
> > 
> > Philippe Mathieu-Daudé (21):
> >   qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
> >   qapi/misc-target: Group SEV QAPI definitions
> >   target/i386/monitor: Return QMP error when SEV is disabled in build
> >   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: Restrict SEV to system emulation
> >   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: Restrict 'info sev' to x86 targets
> >   hw/i386/acpi-common: Remove unused includes
> >   hw/i386: Rename acpi-build.c -> acpi-pc.c
> >   hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c
> >   hw/acpi: Do not restrict ACPI core routines to x86 architecture
> >   hw/i386: Introduce X86_FW_OVMF Kconfig symbol
> >   hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)
> >   hw/i386/Kconfig: Add missing Kconfig dependency (runtime error)
> > 
> >  qapi/misc-target.json               |  78 ++++++++--------
> >  include/hw/i386/pc.h                |   1 +
> >  include/monitor/hmp-target.h        |   1 +
> >  include/monitor/hmp.h               |   1 -
> >  target/i386/sev_i386.h              |  11 ---
> >  hw/i386/acpi-common.c               |  45 ----------
> >  hw/i386/{acpi-build.c => acpi-pc.c} |  39 ++++++++
> >  hw/i386/pc_sysfw.c                  | 107 ----------------------
> >  hw/i386/pc_sysfw_ovmf-stubs.c       |  26 ++++++
> >  hw/i386/pc_sysfw_ovmf.c             | 135 ++++++++++++++++++++++++++++
> >  target/i386/cpu.c                   |   1 +
> >  target/i386/monitor.c               |  91 -------------------
> >  target/i386/sev-stub.c              |  49 +---------
> >  target/i386/sev-sysemu-stub.c       |  69 ++++++++++++++
> >  target/i386/sev.c                   | 130 +++++++++++++++++++++------
> >  MAINTAINERS                         |   8 ++
> >  hw/acpi/Kconfig                     |   4 +
> >  hw/acpi/meson.build                 |   3 +-
> >  hw/i386/Kconfig                     |   5 ++
> >  hw/i386/meson.build                 |   4 +-
> >  target/i386/meson.build             |   4 +-
> >  21 files changed, 443 insertions(+), 369 deletions(-)
> >  rename hw/i386/{acpi-build.c => acpi-pc.c} (98%)
> >  create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
> >  create mode 100644 hw/i386/pc_sysfw_ovmf.c
> >  create mode 100644 target/i386/sev-sysemu-stub.c
> > 
> > -- 
> > 2.31.1
> >   
> 

Re: [PATCH v2 00/23] target/i386/sev: Housekeeping (OVMF + SEV-disabled binaries)

[Michael S. Tsirkin]

On Fri, Jun 18, 2021 at 01:46:23PM +0200, Igor Mammedov wrote:
> On Thu, 17 Jun 2021 16:42:24 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Jun 16, 2021 at 10:43:05PM +0200, Philippe Mathieu-Daudé wrote:
> > > 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.
> > > 
> > > Some SEV patches I was following have been queued on Eduardo's
> > > 'x86-next' tree, so I used his tree as base, and included David and
> > > Connor patches to reduce merge conflicts.
> > > 
> > > Patches 1-16 are SEV related,
> > > patches 17-23 are OVMF related.
> > > 
> > > Special comment for Laszlo: Please don't review this version,
> > > wait for the respin (hoping the SEV cleanup patch get accepted,
> > > the respin will be focused on OVMF).
> > > 
> > > Based-on: https://gitlab.com/ehabkost/qemu/-/commits/x86-next/
> > > Supersedes: <20210610064556.1421620-1-philmd@redhat.com>  
> > 
> > 
> > acpi things
> > 
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > do you want to split acpi things out so I can merge?
> I agree they should have been split out.
> (I see in cover letter SEV don't look into series anymore, so
> this APCI patches which are totally irrelevant to SEV weren't
> even unnoticed).
> 
> Perhaps I should take these into my ACPI refactoring series
> (with your acks), which would need to be rebased on top of these.

OK.

> Though I'd drop 19/23 which doesn't look correct to me.

Yes let's drop that one.

> > 
> > > Connor Kuehl (1):
> > >   MAINTAINERS: Add Connor Kuehl as reviewer for AMD SEV
> > > 
> > > Dr. David Alan Gilbert (1):
> > >   target/i386/sev: sev_get_attestation_report use g_autofree
> > > 
> > > Philippe Mathieu-Daudé (21):
> > >   qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
> > >   qapi/misc-target: Group SEV QAPI definitions
> > >   target/i386/monitor: Return QMP error when SEV is disabled in build
> > >   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: Restrict SEV to system emulation
> > >   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: Restrict 'info sev' to x86 targets
> > >   hw/i386/acpi-common: Remove unused includes
> > >   hw/i386: Rename acpi-build.c -> acpi-pc.c
> > >   hw/i386: Move pc_madt_cpu_entry() to acpi-pc.c
> > >   hw/acpi: Do not restrict ACPI core routines to x86 architecture
> > >   hw/i386: Introduce X86_FW_OVMF Kconfig symbol
> > >   hw/acpi/Kconfig: Add missing Kconfig dependencies (build error)
> > >   hw/i386/Kconfig: Add missing Kconfig dependency (runtime error)
> > > 
> > >  qapi/misc-target.json               |  78 ++++++++--------
> > >  include/hw/i386/pc.h                |   1 +
> > >  include/monitor/hmp-target.h        |   1 +
> > >  include/monitor/hmp.h               |   1 -
> > >  target/i386/sev_i386.h              |  11 ---
> > >  hw/i386/acpi-common.c               |  45 ----------
> > >  hw/i386/{acpi-build.c => acpi-pc.c} |  39 ++++++++
> > >  hw/i386/pc_sysfw.c                  | 107 ----------------------
> > >  hw/i386/pc_sysfw_ovmf-stubs.c       |  26 ++++++
> > >  hw/i386/pc_sysfw_ovmf.c             | 135 ++++++++++++++++++++++++++++
> > >  target/i386/cpu.c                   |   1 +
> > >  target/i386/monitor.c               |  91 -------------------
> > >  target/i386/sev-stub.c              |  49 +---------
> > >  target/i386/sev-sysemu-stub.c       |  69 ++++++++++++++
> > >  target/i386/sev.c                   | 130 +++++++++++++++++++++------
> > >  MAINTAINERS                         |   8 ++
> > >  hw/acpi/Kconfig                     |   4 +
> > >  hw/acpi/meson.build                 |   3 +-
> > >  hw/i386/Kconfig                     |   5 ++
> > >  hw/i386/meson.build                 |   4 +-
> > >  target/i386/meson.build             |   4 +-
> > >  21 files changed, 443 insertions(+), 369 deletions(-)
> > >  rename hw/i386/{acpi-build.c => acpi-pc.c} (98%)
> > >  create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
> > >  create mode 100644 hw/i386/pc_sysfw_ovmf.c
> > >  create mode 100644 target/i386/sev-sysemu-stub.c
> > > 
> > > -- 
> > > 2.31.1
> > >   
> > 

Re: [PATCH v2 00/23] target/i386/sev: Housekeeping (OVMF + SEV-disabled binaries)

[Laszlo Ersek]

On 06/16/21 22:43, Philippe Mathieu-Daudé wrote:
> Special comment for Laszlo: Please don't review this version,
> wait for the respin (hoping the SEV cleanup patch get accepted,
> the respin will be focused on OVMF).

Appreciate the note.

Laszlo