* Re: [PATCH 04/12] x86/sev: Do not hardcode GHCB protocol version
[not found] ` <20210721142015.1401-5-joro@8bytes.org>
@ 2021-07-21 21:17 ` Tom Lendacky
2021-07-31 7:18 ` Joerg Roedel
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lendacky @ 2021-07-21 21:17 UTC (permalink / raw)
To: Joerg Roedel, x86, Eric Biederman
Cc: kexec, Joerg Roedel, hpa, Andy Lutomirski, Dave Hansen,
Peter Zijlstra, Jiri Slaby, Dan Williams, Juergen Gross,
Kees Cook, David Rientjes, Cfir Cohen, Erdem Aktas,
Masami Hiramatsu, Mike Stunes, Sean Christopherson, Martin Radev,
Arvind Sankar, linux-coco, linux-kernel, kvm, virtualization
On 7/21/21 9:20 AM, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Introduce the sev_get_ghcb_proto_ver() which will return the negotiated
> GHCB protocol version and use it to set the version field in the GHCB.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> arch/x86/boot/compressed/sev.c | 5 +++++
> arch/x86/kernel/sev-shared.c | 5 ++++-
> arch/x86/kernel/sev.c | 5 +++++
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 1a2e49730f8b..101e08c67296 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -119,6 +119,11 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
> /* Include code for early handlers */
> #include "../../kernel/sev-shared.c"
>
> +static u64 sev_get_ghcb_proto_ver(void)
> +{
> + return GHCB_PROTOCOL_MAX;
> +}
> +
> static bool early_setup_sev_es(void)
> {
> if (!sev_es_negotiate_protocol(NULL))
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 73eeb5897d16..36eaac2773ed 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -28,6 +28,9 @@ struct sev_ghcb_protocol_info {
> unsigned int vm_proto;
> };
>
> +/* Returns the negotiated GHCB Protocol version */
> +static u64 sev_get_ghcb_proto_ver(void);
> +
> static bool __init sev_es_check_cpu_features(void)
> {
> if (!has_cpuflag(X86_FEATURE_RDRAND)) {
> @@ -122,7 +125,7 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
> enum es_result ret;
>
> /* Fill in protocol and format specifiers */
> - ghcb->protocol_version = GHCB_PROTOCOL_MAX;
> + ghcb->protocol_version = sev_get_ghcb_proto_ver();
So this probably needs better clarification in the spec, but the GHCB
version field is for the GHCB structure layout. So if you don't plan to
use the XSS field that was added for version 2 of the layout, then you
should report the GHCB structure version as 1.
Thanks,
Tom
> ghcb->ghcb_usage = GHCB_DEFAULT_USAGE;
>
> ghcb_set_sw_exit_code(ghcb, exit_code);
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 8084bfd7cce1..5d3422e8b25e 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -498,6 +498,11 @@ static enum es_result vc_slow_virt_to_phys(struct ghcb *ghcb, struct es_em_ctxt
> /* Negotiated GHCB protocol version */
> static struct sev_ghcb_protocol_info ghcb_protocol_info __ro_after_init;
>
> +static u64 sev_get_ghcb_proto_ver(void)
> +{
> + return ghcb_protocol_info.vm_proto;
> +}
> +
> static noinstr void __sev_put_ghcb(struct ghcb_state *state)
> {
> struct sev_es_runtime_data *data;
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 11/12] x86/sev: Handle CLFLUSH MMIO events
[not found] ` <20210721142015.1401-12-joro@8bytes.org>
@ 2021-07-30 22:42 ` Sean Christopherson
2021-07-31 7:17 ` Joerg Roedel
0 siblings, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2021-07-30 22:42 UTC (permalink / raw)
To: Joerg Roedel
Cc: x86, Eric Biederman, kexec, Joerg Roedel, hpa, Andy Lutomirski,
Dave Hansen, Peter Zijlstra, Jiri Slaby, Dan Williams,
Tom Lendacky, Juergen Gross, Kees Cook, David Rientjes,
Cfir Cohen, Erdem Aktas, Masami Hiramatsu, Mike Stunes,
Martin Radev, Arvind Sankar, linux-coco, linux-kernel, kvm,
virtualization
On Wed, Jul 21, 2021, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Handle CLFLUSH instruction to MMIO memory in the #VC handler. The
^
|- emulated
> instruction is ignored by the handler, as the Hypervisor is
> responsible for cache management of emulated MMIO memory.
>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> arch/x86/kernel/sev-shared.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index a7a0793c4f98..682fa202444f 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -632,6 +632,15 @@ static enum es_result vc_handle_mmio_twobyte_ops(struct ghcb *ghcb,
> long *reg_data;
>
> switch (insn->opcode.bytes[1]) {
> + /* CLFLUSH */
> + case 0xae:
> + /*
> + * Ignore CLFLUSHes - those go to emulated MMIO anyway and the
> + * hypervisor is responsible for cache management.
This wording can be misread as "the hypervisor is responsible for _all_ cache
management". Maybe just:
/*
* Ignore CLFLUSHes - the hyperivsor is responsible for cache
* management of emulated MMIO.
*/
Side topic, out of curisoity, what's mapping/accessing emulated MMIO as non-UC?
> + */
> + ret = ES_OK;
> + break;
> +
> /* MMIO Read w/ zero-extension */
> case 0xb6:
> bytes = 1;
> --
> 2.31.1
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 11/12] x86/sev: Handle CLFLUSH MMIO events
2021-07-30 22:42 ` [PATCH 11/12] x86/sev: Handle CLFLUSH MMIO events Sean Christopherson
@ 2021-07-31 7:17 ` Joerg Roedel
0 siblings, 0 replies; 4+ messages in thread
From: Joerg Roedel @ 2021-07-31 7:17 UTC (permalink / raw)
To: Sean Christopherson
Cc: Joerg Roedel, x86, Eric Biederman, kexec, hpa, Andy Lutomirski,
Dave Hansen, Peter Zijlstra, Jiri Slaby, Dan Williams,
Tom Lendacky, Juergen Gross, Kees Cook, David Rientjes,
Cfir Cohen, Erdem Aktas, Masami Hiramatsu, Mike Stunes,
Martin Radev, Arvind Sankar, linux-coco, linux-kernel, kvm,
virtualization
Hi Sean,
On Fri, Jul 30, 2021 at 10:42:30PM +0000, Sean Christopherson wrote:
> On Wed, Jul 21, 2021, Joerg Roedel wrote:
> This wording can be misread as "the hypervisor is responsible for _all_ cache
> management". Maybe just:
>
> /*
> * Ignore CLFLUSHes - the hyperivsor is responsible for cache
> * management of emulated MMIO.
> */
Right, will update the comment, thanks.
> Side topic, out of curisoity, what's mapping/accessing emulated MMIO as non-UC?
The CLFLUSHes happen when the kexec'ed kernel maps the VGA framebuffer
as unencrypted. Initially it is mapped encrypted and before re-mapping
the kernel flushes the range from the caches.
I have not investigated why this doesn't happen on the first boot,
though.
Regards,
Joerg
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 04/12] x86/sev: Do not hardcode GHCB protocol version
2021-07-21 21:17 ` [PATCH 04/12] x86/sev: Do not hardcode GHCB protocol version Tom Lendacky
@ 2021-07-31 7:18 ` Joerg Roedel
0 siblings, 0 replies; 4+ messages in thread
From: Joerg Roedel @ 2021-07-31 7:18 UTC (permalink / raw)
To: Tom Lendacky
Cc: Joerg Roedel, x86, Eric Biederman, kexec, hpa, Andy Lutomirski,
Dave Hansen, Peter Zijlstra, Jiri Slaby, Dan Williams,
Juergen Gross, Kees Cook, David Rientjes, Cfir Cohen, Erdem Aktas,
Masami Hiramatsu, Mike Stunes, Sean Christopherson, Martin Radev,
Arvind Sankar, linux-coco, linux-kernel, kvm, virtualization
Hi Tom,
On Wed, Jul 21, 2021 at 04:17:38PM -0500, Tom Lendacky wrote:
> On 7/21/21 9:20 AM, Joerg Roedel wrote:
> > /* Fill in protocol and format specifiers */
> > - ghcb->protocol_version = GHCB_PROTOCOL_MAX;
> > + ghcb->protocol_version = sev_get_ghcb_proto_ver();
>
> So this probably needs better clarification in the spec, but the GHCB
> version field is for the GHCB structure layout. So if you don't plan to
> use the XSS field that was added for version 2 of the layout, then you
> should report the GHCB structure version as 1.
Yeah, this makes sense, exept for the struct field-name ;) But anyway, I
keep the version field at 1 for now.
Regards,
Joerg
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-31 7:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210721142015.1401-1-joro@8bytes.org>
[not found] ` <20210721142015.1401-5-joro@8bytes.org>
2021-07-21 21:17 ` [PATCH 04/12] x86/sev: Do not hardcode GHCB protocol version Tom Lendacky
2021-07-31 7:18 ` Joerg Roedel
[not found] ` <20210721142015.1401-12-joro@8bytes.org>
2021-07-30 22:42 ` [PATCH 11/12] x86/sev: Handle CLFLUSH MMIO events Sean Christopherson
2021-07-31 7:17 ` Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox