* [PATCH] x86/asm: Force native_apic_mem_read to use mov @ 2022-08-11 18:00 Adam Dunlap 2022-08-11 19:27 ` Sean Christopherson 2022-08-11 19:53 ` [PATCH] " H. Peter Anvin 0 siblings, 2 replies; 21+ messages in thread From: Adam Dunlap @ 2022-08-11 18:00 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Adam Dunlap, Ben Dooks, linux-kernel, llvm Cc: Jacob Xu, Alper Gun, Marc Orr Previously, when compiled with clang, native_apic_mem_read gets inlined into __xapic_wait_icr_idle and optimized to a testl instruction. When run in a VM with SEV-ES enabled, it attempts to emulate this instruction, but the emulator does not support it. Instead, use inline assembly to force native_apic_mem_read to use the mov instruction which is supported by the emulator. Signed-off-by: Adam Dunlap <acdunlap@google.com> Reviewed-by: Marc Orr <marcorr@google.com> Reviewed-by: Jacob Xu <jacobhxu@google.com> --- arch/x86/include/asm/apic.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index 3415321c8240..281db79e76a9 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) static inline u32 native_apic_mem_read(u32 reg) { - return *((volatile u32 *)(APIC_BASE + reg)); + volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); + u32 out; + + /* + * Functionally, what we want to do is simply return *addr. However, + * this accesses an MMIO which may need to be emulated in some cases. + * The emulator doesn't necessarily support all instructions, so we + * force the read from addr to use a mov instruction. + */ + asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); + + return out; } extern void native_apic_wait_icr_idle(void); -- 2.37.1.559.g78731f0fdb-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/asm: Force native_apic_mem_read to use mov 2022-08-11 18:00 [PATCH] x86/asm: Force native_apic_mem_read to use mov Adam Dunlap @ 2022-08-11 19:27 ` Sean Christopherson 2022-08-11 19:57 ` H. Peter Anvin 2022-08-11 19:53 ` [PATCH] " H. Peter Anvin 1 sibling, 1 reply; 21+ messages in thread From: Sean Christopherson @ 2022-08-11 19:27 UTC (permalink / raw) To: Adam Dunlap Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, linux-kernel, llvm, Jacob Xu, Alper Gun, Marc Orr On Thu, Aug 11, 2022, Adam Dunlap wrote: > Previously, when compiled with clang, native_apic_mem_read gets inlined > into __xapic_wait_icr_idle and optimized to a testl instruction. When > run in a VM with SEV-ES enabled, it attempts to emulate this > instruction, but the emulator does not support it. Instead, use inline > assembly to force native_apic_mem_read to use the mov instruction which > is supported by the emulator. > > Signed-off-by: Adam Dunlap <acdunlap@google.com> > Reviewed-by: Marc Orr <marcorr@google.com> > Reviewed-by: Jacob Xu <jacobhxu@google.com> > --- > arch/x86/include/asm/apic.h | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h > index 3415321c8240..281db79e76a9 100644 > --- a/arch/x86/include/asm/apic.h > +++ b/arch/x86/include/asm/apic.h > @@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) > > static inline u32 native_apic_mem_read(u32 reg) > { > - return *((volatile u32 *)(APIC_BASE + reg)); > + volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); > + u32 out; > + > + /* > + * Functionally, what we want to do is simply return *addr. However, > + * this accesses an MMIO which may need to be emulated in some cases. > + * The emulator doesn't necessarily support all instructions, so we > + * force the read from addr to use a mov instruction. > + */ > + asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); > + > + return out; Can't this just be: return readl((void __iomem *)(APIC_BASE + reg)); ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/asm: Force native_apic_mem_read to use mov 2022-08-11 19:27 ` Sean Christopherson @ 2022-08-11 19:57 ` H. Peter Anvin 2022-08-11 20:03 ` Sean Christopherson 0 siblings, 1 reply; 21+ messages in thread From: H. Peter Anvin @ 2022-08-11 19:57 UTC (permalink / raw) To: Sean Christopherson, Adam Dunlap Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, linux-kernel, llvm, Jacob Xu, Alper Gun, Marc Orr On August 11, 2022 12:27:10 PM PDT, Sean Christopherson <seanjc@google.com> wrote: >On Thu, Aug 11, 2022, Adam Dunlap wrote: >> Previously, when compiled with clang, native_apic_mem_read gets inlined >> into __xapic_wait_icr_idle and optimized to a testl instruction. When >> run in a VM with SEV-ES enabled, it attempts to emulate this >> instruction, but the emulator does not support it. Instead, use inline >> assembly to force native_apic_mem_read to use the mov instruction which >> is supported by the emulator. >> >> Signed-off-by: Adam Dunlap <acdunlap@google.com> >> Reviewed-by: Marc Orr <marcorr@google.com> >> Reviewed-by: Jacob Xu <jacobhxu@google.com> >> --- >> arch/x86/include/asm/apic.h | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h >> index 3415321c8240..281db79e76a9 100644 >> --- a/arch/x86/include/asm/apic.h >> +++ b/arch/x86/include/asm/apic.h >> @@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) >> >> static inline u32 native_apic_mem_read(u32 reg) >> { >> - return *((volatile u32 *)(APIC_BASE + reg)); >> + volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); >> + u32 out; >> + >> + /* >> + * Functionally, what we want to do is simply return *addr. However, >> + * this accesses an MMIO which may need to be emulated in some cases. >> + * The emulator doesn't necessarily support all instructions, so we >> + * force the read from addr to use a mov instruction. >> + */ >> + asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); >> + >> + return out; > >Can't this just be: > > return readl((void __iomem *)(APIC_BASE + reg)); The very point of the patch is to force a specific instruction sequence. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/asm: Force native_apic_mem_read to use mov 2022-08-11 19:57 ` H. Peter Anvin @ 2022-08-11 20:03 ` Sean Christopherson 2022-08-12 4:40 ` H. Peter Anvin 0 siblings, 1 reply; 21+ messages in thread From: Sean Christopherson @ 2022-08-11 20:03 UTC (permalink / raw) To: H. Peter Anvin Cc: Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, linux-kernel, llvm, Jacob Xu, Alper Gun, Marc Orr On Thu, Aug 11, 2022, H. Peter Anvin wrote: > On August 11, 2022 12:27:10 PM PDT, Sean Christopherson <seanjc@google.com> wrote: > >On Thu, Aug 11, 2022, Adam Dunlap wrote: > >> Previously, when compiled with clang, native_apic_mem_read gets inlined > >> into __xapic_wait_icr_idle and optimized to a testl instruction. When > >> run in a VM with SEV-ES enabled, it attempts to emulate this > >> instruction, but the emulator does not support it. Instead, use inline > >> assembly to force native_apic_mem_read to use the mov instruction which > >> is supported by the emulator. > >> > >> Signed-off-by: Adam Dunlap <acdunlap@google.com> > >> Reviewed-by: Marc Orr <marcorr@google.com> > >> Reviewed-by: Jacob Xu <jacobhxu@google.com> > >> --- > >> arch/x86/include/asm/apic.h | 13 ++++++++++++- > >> 1 file changed, 12 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h > >> index 3415321c8240..281db79e76a9 100644 > >> --- a/arch/x86/include/asm/apic.h > >> +++ b/arch/x86/include/asm/apic.h > >> @@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) > >> > >> static inline u32 native_apic_mem_read(u32 reg) > >> { > >> - return *((volatile u32 *)(APIC_BASE + reg)); > >> + volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); > >> + u32 out; > >> + > >> + /* > >> + * Functionally, what we want to do is simply return *addr. However, > >> + * this accesses an MMIO which may need to be emulated in some cases. > >> + * The emulator doesn't necessarily support all instructions, so we > >> + * force the read from addr to use a mov instruction. > >> + */ > >> + asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); > >> + > >> + return out; > > > >Can't this just be: > > > > return readl((void __iomem *)(APIC_BASE + reg)); > > The very point of the patch is to force a specific instruction sequence. Yes, and that specific emulator-friendly instruction also has to be forced for all of the core x86 read/write MMIO helpers. And it's also possible for MMIO read/write to be enlightened to skip the MOV and go straight to #VMGEXIT, i.e. the xAPIC code shouldn't assume MOV is the best/only option (ignoring the handling of the P54C erratum in the write path). ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/asm: Force native_apic_mem_read to use mov 2022-08-11 20:03 ` Sean Christopherson @ 2022-08-12 4:40 ` H. Peter Anvin 2022-08-12 18:32 ` Adam Dunlap 0 siblings, 1 reply; 21+ messages in thread From: H. Peter Anvin @ 2022-08-12 4:40 UTC (permalink / raw) To: Sean Christopherson Cc: Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, linux-kernel, llvm, Jacob Xu, Alper Gun, Marc Orr On August 11, 2022 1:03:11 PM PDT, Sean Christopherson <seanjc@google.com> wrote: >On Thu, Aug 11, 2022, H. Peter Anvin wrote: >> On August 11, 2022 12:27:10 PM PDT, Sean Christopherson <seanjc@google.com> wrote: >> >On Thu, Aug 11, 2022, Adam Dunlap wrote: >> >> Previously, when compiled with clang, native_apic_mem_read gets inlined >> >> into __xapic_wait_icr_idle and optimized to a testl instruction. When >> >> run in a VM with SEV-ES enabled, it attempts to emulate this >> >> instruction, but the emulator does not support it. Instead, use inline >> >> assembly to force native_apic_mem_read to use the mov instruction which >> >> is supported by the emulator. >> >> >> >> Signed-off-by: Adam Dunlap <acdunlap@google.com> >> >> Reviewed-by: Marc Orr <marcorr@google.com> >> >> Reviewed-by: Jacob Xu <jacobhxu@google.com> >> >> --- >> >> arch/x86/include/asm/apic.h | 13 ++++++++++++- >> >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h >> >> index 3415321c8240..281db79e76a9 100644 >> >> --- a/arch/x86/include/asm/apic.h >> >> +++ b/arch/x86/include/asm/apic.h >> >> @@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) >> >> >> >> static inline u32 native_apic_mem_read(u32 reg) >> >> { >> >> - return *((volatile u32 *)(APIC_BASE + reg)); >> >> + volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); >> >> + u32 out; >> >> + >> >> + /* >> >> + * Functionally, what we want to do is simply return *addr. However, >> >> + * this accesses an MMIO which may need to be emulated in some cases. >> >> + * The emulator doesn't necessarily support all instructions, so we >> >> + * force the read from addr to use a mov instruction. >> >> + */ >> >> + asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); >> >> + >> >> + return out; >> > >> >Can't this just be: >> > >> > return readl((void __iomem *)(APIC_BASE + reg)); >> >> The very point of the patch is to force a specific instruction sequence. > >Yes, and that specific emulator-friendly instruction also has to be forced for all >of the core x86 read/write MMIO helpers. And it's also possible for MMIO read/write >to be enlightened to skip the MOV and go straight to #VMGEXIT, i.e. the xAPIC code >shouldn't assume MOV is the best/only option (ignoring the handling of the P54C >erratum in the write path). That's not reasonable... but xAPIC is "special" enough. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/asm: Force native_apic_mem_read to use mov 2022-08-12 4:40 ` H. Peter Anvin @ 2022-08-12 18:32 ` Adam Dunlap 2022-08-12 18:35 ` [PATCH v2] " Adam Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Adam Dunlap @ 2022-08-12 18:32 UTC (permalink / raw) To: H. Peter Anvin Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, linux-kernel, llvm, Jacob Xu, Alper Gun, Marc Orr On Thu, Aug 11, 2022 at 9:40 PM H. Peter Anvin <hpa@zytor.com> wrote: > > On August 11, 2022 1:03:11 PM PDT, Sean Christopherson <seanjc@google.com> wrote: > >On Thu, Aug 11, 2022, H. Peter Anvin wrote: > >> On August 11, 2022 12:27:10 PM PDT, Sean Christopherson <seanjc@google.com> wrote: > >> >On Thu, Aug 11, 2022, Adam Dunlap wrote: > >> >> Previously, when compiled with clang, native_apic_mem_read gets inlined > >> >> into __xapic_wait_icr_idle and optimized to a testl instruction. When > >> >> run in a VM with SEV-ES enabled, it attempts to emulate this > >> >> instruction, but the emulator does not support it. Instead, use inline > >> >> assembly to force native_apic_mem_read to use the mov instruction which > >> >> is supported by the emulator. > >> >> > >> >> Signed-off-by: Adam Dunlap <acdunlap@google.com> > >> >> Reviewed-by: Marc Orr <marcorr@google.com> > >> >> Reviewed-by: Jacob Xu <jacobhxu@google.com> > >> >> --- > >> >> arch/x86/include/asm/apic.h | 13 ++++++++++++- > >> >> 1 file changed, 12 insertions(+), 1 deletion(-) > >> >> > >> >> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h > >> >> index 3415321c8240..281db79e76a9 100644 > >> >> --- a/arch/x86/include/asm/apic.h > >> >> +++ b/arch/x86/include/asm/apic.h > >> >> @@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) > >> >> > >> >> static inline u32 native_apic_mem_read(u32 reg) > >> >> { > >> >> - return *((volatile u32 *)(APIC_BASE + reg)); > >> >> + volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); > >> >> + u32 out; > >> >> + > >> >> + /* > >> >> + * Functionally, what we want to do is simply return *addr. However, > >> >> + * this accesses an MMIO which may need to be emulated in some cases. > >> >> + * The emulator doesn't necessarily support all instructions, so we > >> >> + * force the read from addr to use a mov instruction. > >> >> + */ > >> >> + asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); > >> >> + > >> >> + return out; > >> > > >> >Can't this just be: > >> > > >> > return readl((void __iomem *)(APIC_BASE + reg)); > >> > >> The very point of the patch is to force a specific instruction sequence. > > > >Yes, and that specific emulator-friendly instruction also has to be forced for all > >of the core x86 read/write MMIO helpers. And it's also possible for MMIO read/write > >to be enlightened to skip the MOV and go straight to #VMGEXIT, i.e. the xAPIC code > >shouldn't assume MOV is the best/only option (ignoring the handling of the P54C > >erratum in the write path). > > That's not reasonable... but xAPIC is "special" enough. Thanks for your responses. I think for now it makes sense to use the readl function because I haven't seen it require the ax register so can't verify the result. I will send out a modified patch using readl shortly. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2] x86/asm: Force native_apic_mem_read to use mov 2022-08-12 18:32 ` Adam Dunlap @ 2022-08-12 18:35 ` Adam Dunlap 2022-09-08 17:04 ` [PATCH v2 RESEND] " Adam Dunlap 0 siblings, 1 reply; 21+ messages in thread From: Adam Dunlap @ 2022-08-12 18:35 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Adam Dunlap, Ben Dooks, linux-kernel, llvm Cc: Jacob Xu, Alper Gun, Marc Orr Previously, when compiled with clang, native_apic_mem_read gets inlined into __xapic_wait_icr_idle and optimized to a testl instruction. When run in a VM with SEV-ES enabled, it attempts to emulate this instruction, but the emulator does not support it. Instead, use inline assembly to force native_apic_mem_read to use the mov instruction which is supported by the emulator. Signed-off-by: Adam Dunlap <acdunlap@google.com> --- V1 -> V2: Replaced asm with readl function which does the same thing arch/x86/include/asm/apic.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index 3415321c8240..b4c9034aa073 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -12,6 +12,7 @@ #include <asm/mpspec.h> #include <asm/msr.h> #include <asm/hardirq.h> +#include <asm/io.h> #define ARCH_APICTIMER_STOPS_ON_C3 1 @@ -109,7 +110,7 @@ static inline void native_apic_mem_write(u32 reg, u32 v) static inline u32 native_apic_mem_read(u32 reg) { - return *((volatile u32 *)(APIC_BASE + reg)); + return readl((void __iomem *)(APIC_BASE + reg)); } extern void native_apic_wait_icr_idle(void); -- 2.37.1.559.g78731f0fdb-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-08-12 18:35 ` [PATCH v2] " Adam Dunlap @ 2022-09-08 17:04 ` Adam Dunlap 2022-09-14 11:13 ` Peter Gonda 0 siblings, 1 reply; 21+ messages in thread From: Adam Dunlap @ 2022-09-08 17:04 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Adam Dunlap, Ben Dooks, linux-kernel, llvm, Peter Gonda Cc: Jacob Xu, Alper Gun, Marc Orr Previously, when compiled with clang, native_apic_mem_read gets inlined into __xapic_wait_icr_idle and optimized to a testl instruction. When run in a VM with SEV-ES enabled, it attempts to emulate this instruction, but the emulator does not support it. Instead, use inline assembly to force native_apic_mem_read to use the mov instruction which is supported by the emulator. Signed-off-by: Adam Dunlap <acdunlap@google.com> --- V1 -> V2: Replaced asm with readl function which does the same thing arch/x86/include/asm/apic.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index 3415321c8240..b4c9034aa073 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -12,6 +12,7 @@ #include <asm/mpspec.h> #include <asm/msr.h> #include <asm/hardirq.h> +#include <asm/io.h> #define ARCH_APICTIMER_STOPS_ON_C3 1 @@ -109,7 +110,7 @@ static inline void native_apic_mem_write(u32 reg, u32 v) static inline u32 native_apic_mem_read(u32 reg) { - return *((volatile u32 *)(APIC_BASE + reg)); + return readl((void __iomem *)(APIC_BASE + reg)); } extern void native_apic_wait_icr_idle(void); -- 2.37.1.559.g78731f0fdb-goog ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-08 17:04 ` [PATCH v2 RESEND] " Adam Dunlap @ 2022-09-14 11:13 ` Peter Gonda 2022-09-14 11:59 ` Marc Orr 2022-09-14 12:03 ` Dave Hansen 0 siblings, 2 replies; 21+ messages in thread From: Peter Gonda @ 2022-09-14 11:13 UTC (permalink / raw) To: Adam Dunlap Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun, Marc Orr On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: > > Previously, when compiled with clang, native_apic_mem_read gets inlined > into __xapic_wait_icr_idle and optimized to a testl instruction. When > run in a VM with SEV-ES enabled, it attempts to emulate this > instruction, but the emulator does not support it. Instead, use inline > assembly to force native_apic_mem_read to use the mov instruction which > is supported by the emulator. This seems to be an issue with the SEV-ES in guest #VC handler's "emulator" right? If that's the case I think this should be fixed in the #VC handler instead of fixing the code that is failing to be emulated. What if there are other places where a testl is used and our tests have not caught them. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-14 11:13 ` Peter Gonda @ 2022-09-14 11:59 ` Marc Orr 2022-09-14 11:59 ` Marc Orr 2022-09-14 12:03 ` Dave Hansen 1 sibling, 1 reply; 21+ messages in thread From: Marc Orr @ 2022-09-14 11:59 UTC (permalink / raw) To: Peter Gonda Cc: Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun On Wed, Sep 14, 2022 at 12:13 PM Peter Gonda <pgonda@google.com> wrote: > > On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: > > > > Previously, when compiled with clang, native_apic_mem_read gets inlined > > into __xapic_wait_icr_idle and optimized to a testl instruction. When > > run in a VM with SEV-ES enabled, it attempts to emulate this > > instruction, but the emulator does not support it. Instead, use inline > > assembly to force native_apic_mem_read to use the mov instruction which > > is supported by the emulator. > > This seems to be an issue with the SEV-ES in guest #VC handler's > "emulator" right? > > If that's the case I think this should be fixed in the #VC handler > instead of fixing the code that is failing to be emulated. What if > there are other places where a testl is used and our tests have not > caught them. That was my initial reaction too. But we spoke w/ Tom offline (before sending this) and my understanding was that we really should be using MOV for MMIO. I've cc'd Tom so he can speak to this directly though. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-14 11:59 ` Marc Orr @ 2022-09-14 11:59 ` Marc Orr 2022-09-15 7:51 ` Tom Lendacky 0 siblings, 1 reply; 21+ messages in thread From: Marc Orr @ 2022-09-14 11:59 UTC (permalink / raw) To: Peter Gonda, Lendacky, Thomas Cc: Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun On Wed, Sep 14, 2022 at 12:59 PM Marc Orr <marcorr@google.com> wrote: > > On Wed, Sep 14, 2022 at 12:13 PM Peter Gonda <pgonda@google.com> wrote: > > > > On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: > > > > > > Previously, when compiled with clang, native_apic_mem_read gets inlined > > > into __xapic_wait_icr_idle and optimized to a testl instruction. When > > > run in a VM with SEV-ES enabled, it attempts to emulate this > > > instruction, but the emulator does not support it. Instead, use inline > > > assembly to force native_apic_mem_read to use the mov instruction which > > > is supported by the emulator. > > > > This seems to be an issue with the SEV-ES in guest #VC handler's > > "emulator" right? > > > > If that's the case I think this should be fixed in the #VC handler > > instead of fixing the code that is failing to be emulated. What if > > there are other places where a testl is used and our tests have not > > caught them. > > That was my initial reaction too. But we spoke w/ Tom offline (before > sending this) and my understanding was that we really should be using > MOV for MMIO. I've cc'd Tom so he can speak to this directly though. Actually cc'ing Tom :-). ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-14 11:59 ` Marc Orr @ 2022-09-15 7:51 ` Tom Lendacky 0 siblings, 0 replies; 21+ messages in thread From: Tom Lendacky @ 2022-09-15 7:51 UTC (permalink / raw) To: Marc Orr, Peter Gonda Cc: Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun On 9/14/22 06:59, Marc Orr wrote: > On Wed, Sep 14, 2022 at 12:59 PM Marc Orr <marcorr@google.com> wrote: >> >> On Wed, Sep 14, 2022 at 12:13 PM Peter Gonda <pgonda@google.com> wrote: >>> >>> On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: >>>> >>>> Previously, when compiled with clang, native_apic_mem_read gets inlined >>>> into __xapic_wait_icr_idle and optimized to a testl instruction. When >>>> run in a VM with SEV-ES enabled, it attempts to emulate this >>>> instruction, but the emulator does not support it. Instead, use inline >>>> assembly to force native_apic_mem_read to use the mov instruction which >>>> is supported by the emulator. >>> >>> This seems to be an issue with the SEV-ES in guest #VC handler's >>> "emulator" right? >>> >>> If that's the case I think this should be fixed in the #VC handler >>> instead of fixing the code that is failing to be emulated. What if >>> there are other places where a testl is used and our tests have not >>> caught them. >> >> That was my initial reaction too. But we spoke w/ Tom offline (before >> sending this) and my understanding was that we really should be using >> MOV for MMIO. I've cc'd Tom so he can speak to this directly though. I did finally find the section in our PPR that references using the MOV instruction, but it was for MMIO configuration space, not general MMIO operations. So, yes, the #VC handler could be extended to handle a TEST instruction to fix this. My worry would be if the compiler decided to use a different instruction in the future. I see that the native_apic_mem_write() is using assembler to perform its operation, it just seemed right that the native_apic_mem_read() could do the same. Thanks, Tom > > Actually cc'ing Tom :-). ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-14 11:13 ` Peter Gonda 2022-09-14 11:59 ` Marc Orr @ 2022-09-14 12:03 ` Dave Hansen 2022-09-14 16:22 ` Sean Christopherson 1 sibling, 1 reply; 21+ messages in thread From: Dave Hansen @ 2022-09-14 12:03 UTC (permalink / raw) To: Peter Gonda, Adam Dunlap Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun, Marc Orr On 9/14/22 04:13, Peter Gonda wrote: > On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: >> Previously, when compiled with clang, native_apic_mem_read gets inlined >> into __xapic_wait_icr_idle and optimized to a testl instruction. When >> run in a VM with SEV-ES enabled, it attempts to emulate this >> instruction, but the emulator does not support it. Instead, use inline >> assembly to force native_apic_mem_read to use the mov instruction which >> is supported by the emulator. > This seems to be an issue with the SEV-ES in guest #VC handler's > "emulator" right? No. It's not just an SEV-ES thing. It's a problem for TDX and _probably_ a problem for normal virtualization where it's a host-side issue. Kirill wrote a lot of great background information in here: > https://lore.kernel.org/all/164946765464.4207.3715751176055921036.tip-bot2@tip-bot2/ So, the question is not "should we extend the MMIO instruction decoders to handle one more instruction?". It is "should we extend the MMIO decoders to handle *ALL* memory read instructions?" That's an even more emphatic "NO". readl() seems to be the right thing to do. Also, Dear TDX, SEV and virt folks: please look for more of these. They're going to bite you sooner or later. You should have caught this one before now. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-14 12:03 ` Dave Hansen @ 2022-09-14 16:22 ` Sean Christopherson 2022-09-15 8:09 ` Peter Gonda 0 siblings, 1 reply; 21+ messages in thread From: Sean Christopherson @ 2022-09-14 16:22 UTC (permalink / raw) To: Dave Hansen Cc: Peter Gonda, Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun, Marc Orr On Wed, Sep 14, 2022, Dave Hansen wrote: > On 9/14/22 04:13, Peter Gonda wrote: > > On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: > >> Previously, when compiled with clang, native_apic_mem_read gets inlined > >> into __xapic_wait_icr_idle and optimized to a testl instruction. When > >> run in a VM with SEV-ES enabled, it attempts to emulate this > >> instruction, but the emulator does not support it. Instead, use inline > >> assembly to force native_apic_mem_read to use the mov instruction which > >> is supported by the emulator. > > This seems to be an issue with the SEV-ES in guest #VC handler's > > "emulator" right? > > No. > > It's not just an SEV-ES thing. It's a problem for TDX and _probably_ a > problem for normal virtualization where it's a host-side issue. Kirill > wrote a lot of great background information in here: > > > https://lore.kernel.org/all/164946765464.4207.3715751176055921036.tip-bot2@tip-bot2/ > > So, the question is not "should we extend the MMIO instruction decoders > to handle one more instruction?". It is "should we extend the MMIO > decoders to handle *ALL* memory read instructions?" > > That's an even more emphatic "NO". +1, keep the guest-side decoding as simple as possible. > readl() seems to be the right thing to do. Also, Dear TDX, SEV and virt > folks: please look for more of these. They're going to bite you sooner > or later. You should have caught this one before now. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-09-14 16:22 ` Sean Christopherson @ 2022-09-15 8:09 ` Peter Gonda [not found] ` <CAMBK9=YB=8EQymDUda300qPFAL1=7dzC61c0pshrWEC5ibrUfQ@mail.gmail.com> 0 siblings, 1 reply; 21+ messages in thread From: Peter Gonda @ 2022-09-15 8:09 UTC (permalink / raw) To: Sean Christopherson Cc: Dave Hansen, Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun, Marc Orr On Wed, Sep 14, 2022 at 5:22 PM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Sep 14, 2022, Dave Hansen wrote: > > On 9/14/22 04:13, Peter Gonda wrote: > > > On Thu, Sep 8, 2022 at 6:05 PM Adam Dunlap <acdunlap@google.com> wrote: > > >> Previously, when compiled with clang, native_apic_mem_read gets inlined > > >> into __xapic_wait_icr_idle and optimized to a testl instruction. When > > >> run in a VM with SEV-ES enabled, it attempts to emulate this > > >> instruction, but the emulator does not support it. Instead, use inline > > >> assembly to force native_apic_mem_read to use the mov instruction which > > >> is supported by the emulator. > > > This seems to be an issue with the SEV-ES in guest #VC handler's > > > "emulator" right? > > > > No. > > > > It's not just an SEV-ES thing. It's a problem for TDX and _probably_ a > > problem for normal virtualization where it's a host-side issue. Kirill > > wrote a lot of great background information in here: > > > > > https://lore.kernel.org/all/164946765464.4207.3715751176055921036.tip-bot2@tip-bot2/ > > > > So, the question is not "should we extend the MMIO instruction decoders > > to handle one more instruction?". It is "should we extend the MMIO > > decoders to handle *ALL* memory read instructions?" > > > > That's an even more emphatic "NO". > > +1, keep the guest-side decoding as simple as possible. > > > readl() seems to be the right thing to do. Also, Dear TDX, SEV and virt > > folks: please look for more of these. They're going to bite you sooner > > or later. You should have caught this one before now. Thanks for the context here Dave. Fixing the offending MMIO instruction here makes sense. ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <CAMBK9=YB=8EQymDUda300qPFAL1=7dzC61c0pshrWEC5ibrUfQ@mail.gmail.com>]
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov [not found] ` <CAMBK9=YB=8EQymDUda300qPFAL1=7dzC61c0pshrWEC5ibrUfQ@mail.gmail.com> @ 2022-10-03 23:07 ` Adam Dunlap [not found] ` <B7175642-351D-44A0-B7AD-E69C6B64FC18@zytor.com> 1 sibling, 0 replies; 21+ messages in thread From: Adam Dunlap @ 2022-10-03 23:07 UTC (permalink / raw) To: Peter Gonda Cc: Sean Christopherson, Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, H. Peter Anvin, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun, Marc Orr, Lendacky, Thomas [resent with plain text] Thanks for all the responses. Is the consensus that we should use the readl function here or instead use inline assembly directly as in the patch I originally sent out: asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); ? The readl function has this exact same code, I'm just not sure which version fits better stylistically. On Mon, Oct 3, 2022 at 4:01 PM Adam Dunlap <acdunlap@google.com> wrote: > > Thanks for all the responses. Is the consensus that we should use the > readl function here or instead use inline assembly directly as in the patch > I originally sent out: > > asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); > > ? The readl function has this exact same code, I'm just not sure > which version fits better stylistically. ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <B7175642-351D-44A0-B7AD-E69C6B64FC18@zytor.com>]
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov [not found] ` <B7175642-351D-44A0-B7AD-E69C6B64FC18@zytor.com> @ 2022-10-05 13:28 ` Tom Lendacky 2022-11-17 21:23 ` Marc Orr 0 siblings, 1 reply; 21+ messages in thread From: Tom Lendacky @ 2022-10-05 13:28 UTC (permalink / raw) To: H. Peter Anvin, Adam Dunlap, Peter Gonda Cc: Sean Christopherson, Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun, Marc Orr On 10/3/22 18:11, H. Peter Anvin wrote: > On October 3, 2022 4:01:01 PM PDT, Adam Dunlap <acdunlap@google.com> wrote: >> Thanks for all the responses. Is the consensus that we should use the >> readl function here or instead use inline assembly directly as in the patch >> I originally sent out: >> >> asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); >> >> ? The readl function has this exact same code, I'm just not sure >> which version fits better stylistically. > > Is mov with an arbitrary addressing mode still acceptable for whatever is causing this problem? The acceptable forms of MOV are covered by insn_decode_mmio() in arch/x86/lib/insn-eval.c. Thanks, Tom ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-10-05 13:28 ` Tom Lendacky @ 2022-11-17 21:23 ` Marc Orr 2023-11-17 18:14 ` Sidharth Telang 0 siblings, 1 reply; 21+ messages in thread From: Marc Orr @ 2022-11-17 21:23 UTC (permalink / raw) To: Tom Lendacky Cc: H. Peter Anvin, Adam Dunlap, Peter Gonda, Sean Christopherson, Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, the arch/x86 maintainers, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, LKML, llvm, Jacob Xu, Alper Gun On Wed, Oct 5, 2022 at 6:29 AM Tom Lendacky <thomas.lendacky@amd.com> wrote: > > On 10/3/22 18:11, H. Peter Anvin wrote: > > On October 3, 2022 4:01:01 PM PDT, Adam Dunlap <acdunlap@google.com> wrote: > >> Thanks for all the responses. Is the consensus that we should use the > >> readl function here or instead use inline assembly directly as in the patch > >> I originally sent out: > >> > >> asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); > >> > >> ? The readl function has this exact same code, I'm just not sure > >> which version fits better stylistically. > > > > Is mov with an arbitrary addressing mode still acceptable for whatever is causing this problem? > > The acceptable forms of MOV are covered by insn_decode_mmio() in > arch/x86/lib/insn-eval.c. Is this blocked on an item? There seems to be consensus that this patch fixes a bug and is taking the right high-level approach (i.e., change the guest code to avoid triggering a sequence that isn't supported under CVM exception-based emulation). Without something like this, we weren't able to build the kernel w/ CLANG when it is configured to run under SEV-ES. We sent out two versions of the patch. One that does the mov directly [1] and a second that calls readl [2]. Is one of these two patches acceptable? Or do we need to follow up on something? [1] https://lore.kernel.org/lkml/0D6A1E49-F21B-42AA-BBBF-13BFC308BB1E@zytor.com/T/ [2] https://lore.kernel.org/all/20220812183501.3555820-1-acdunlap@google.com/ Thanks, Marc ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2022-11-17 21:23 ` Marc Orr @ 2023-11-17 18:14 ` Sidharth Telang 2023-11-17 19:23 ` Dave Hansen 0 siblings, 1 reply; 21+ messages in thread From: Sidharth Telang @ 2023-11-17 18:14 UTC (permalink / raw) To: marcorr Cc: acdunlap, ak, alpergun, ben-linux, bp, dave.hansen, dave.hansen, hpa, jacobhxu, kirill.shutemov, linux-kernel, llvm, mingo, nathan, ndesaulniers, pgonda, sathyanarayanan.kuppuswamy, seanjc, tglx, thomas.lendacky, trix, x86 > Is this blocked on an item? There seems to be consensus that this > patch fixes a bug and is taking the right high-level approach (i.e., > change the guest code to avoid triggering a sequence that isn't > supported under CVM exception-based emulation). Without something like > this, we weren't able to build the kernel w/ CLANG when it is > configured to run under SEV-ES. > We sent out two versions of the patch. One that does the mov directly > [1] and a second that calls readl [2]. Is one of these two patches > acceptable? Or do we need to follow up on something? > > [1] https://lore.kernel.org/lkml/0D6A1E49-F21B-42AA-BBBF-13BFC308BB1E@zytor.com/T/ > [2] https://lore.kernel.org/all/20220812183501.3555820-1-acdunlap@google.com/ Signal-boosting this thread: is this blocked on any item? We are still running into this issue (SEV-ES guest unexpectedly requests termination minutes after booting) and applying this patch seems to fix it. Thanks, Sid ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 RESEND] x86/asm: Force native_apic_mem_read to use mov 2023-11-17 18:14 ` Sidharth Telang @ 2023-11-17 19:23 ` Dave Hansen 0 siblings, 0 replies; 21+ messages in thread From: Dave Hansen @ 2023-11-17 19:23 UTC (permalink / raw) To: Sidharth Telang, marcorr Cc: acdunlap, ak, alpergun, ben-linux, bp, dave.hansen, hpa, jacobhxu, kirill.shutemov, linux-kernel, llvm, mingo, nathan, ndesaulniers, pgonda, sathyanarayanan.kuppuswamy, seanjc, tglx, thomas.lendacky, trix, x86 On 11/17/23 10:14, Sidharth Telang wrote: >> Is this blocked on an item? There seems to be consensus that this >> patch fixes a bug and is taking the right high-level approach (i.e., >> change the guest code to avoid triggering a sequence that isn't >> supported under CVM exception-based emulation). Without something like >> this, we weren't able to build the kernel w/ CLANG when it is >> configured to run under SEV-ES. > >> We sent out two versions of the patch. One that does the mov directly >> [1] and a second that calls readl [2]. Is one of these two patches >> acceptable? Or do we need to follow up on something? >> >> [1] https://lore.kernel.org/lkml/0D6A1E49-F21B-42AA-BBBF-13BFC308BB1E@zytor.com/T/ >> [2] https://lore.kernel.org/all/20220812183501.3555820-1-acdunlap@google.com/ > > Signal-boosting this thread: is this blocked on any item? Yes, it's blocked on you sending a well-described patch. You sent out two patches which both received a lot of discussion and induced a lot of confusion. Can you please take all the knowledge from this thread and send a third patch that has a proper changelog incorporating all that knowledge? Which approach should that patch have? Whatever one is as close to what native_apic_mem_write() does as possible. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/asm: Force native_apic_mem_read to use mov 2022-08-11 18:00 [PATCH] x86/asm: Force native_apic_mem_read to use mov Adam Dunlap 2022-08-11 19:27 ` Sean Christopherson @ 2022-08-11 19:53 ` H. Peter Anvin 1 sibling, 0 replies; 21+ messages in thread From: H. Peter Anvin @ 2022-08-11 19:53 UTC (permalink / raw) To: Adam Dunlap, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86, Nathan Chancellor, Nick Desaulniers, Tom Rix, Kirill A. Shutemov, Sean Christopherson, Kuppuswamy Sathyanarayanan, Andi Kleen, Ben Dooks, linux-kernel, llvm Cc: Jacob Xu, Alper Gun, Marc Orr On August 11, 2022 11:00:10 AM PDT, Adam Dunlap <acdunlap@google.com> wrote: >Previously, when compiled with clang, native_apic_mem_read gets inlined >into __xapic_wait_icr_idle and optimized to a testl instruction. When >run in a VM with SEV-ES enabled, it attempts to emulate this >instruction, but the emulator does not support it. Instead, use inline >assembly to force native_apic_mem_read to use the mov instruction which >is supported by the emulator. > >Signed-off-by: Adam Dunlap <acdunlap@google.com> >Reviewed-by: Marc Orr <marcorr@google.com> >Reviewed-by: Jacob Xu <jacobhxu@google.com> >--- > arch/x86/include/asm/apic.h | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > >diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h >index 3415321c8240..281db79e76a9 100644 >--- a/arch/x86/include/asm/apic.h >+++ b/arch/x86/include/asm/apic.h >@@ -109,7 +109,18 @@ static inline void native_apic_mem_write(u32 reg, u32 v) > > static inline u32 native_apic_mem_read(u32 reg) > { >- return *((volatile u32 *)(APIC_BASE + reg)); >+ volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg); >+ u32 out; >+ >+ /* >+ * Functionally, what we want to do is simply return *addr. However, >+ * this accesses an MMIO which may need to be emulated in some cases. >+ * The emulator doesn't necessarily support all instructions, so we >+ * force the read from addr to use a mov instruction. >+ */ >+ asm_inline("movl %1, %0" : "=r"(out) : "m"(*addr)); >+ >+ return out; > } > > extern void native_apic_wait_icr_idle(void); As I recall, there are some variants which only support using the ax register, so if you are going to do this might as well go all the way and force a specific instruction with specific registers, like mov (%rdx),%rax (by analogy with the I/O registers.) ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-11-17 19:23 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-11 18:00 [PATCH] x86/asm: Force native_apic_mem_read to use mov Adam Dunlap 2022-08-11 19:27 ` Sean Christopherson 2022-08-11 19:57 ` H. Peter Anvin 2022-08-11 20:03 ` Sean Christopherson 2022-08-12 4:40 ` H. Peter Anvin 2022-08-12 18:32 ` Adam Dunlap 2022-08-12 18:35 ` [PATCH v2] " Adam Dunlap 2022-09-08 17:04 ` [PATCH v2 RESEND] " Adam Dunlap 2022-09-14 11:13 ` Peter Gonda 2022-09-14 11:59 ` Marc Orr 2022-09-14 11:59 ` Marc Orr 2022-09-15 7:51 ` Tom Lendacky 2022-09-14 12:03 ` Dave Hansen 2022-09-14 16:22 ` Sean Christopherson 2022-09-15 8:09 ` Peter Gonda [not found] ` <CAMBK9=YB=8EQymDUda300qPFAL1=7dzC61c0pshrWEC5ibrUfQ@mail.gmail.com> 2022-10-03 23:07 ` Adam Dunlap [not found] ` <B7175642-351D-44A0-B7AD-E69C6B64FC18@zytor.com> 2022-10-05 13:28 ` Tom Lendacky 2022-11-17 21:23 ` Marc Orr 2023-11-17 18:14 ` Sidharth Telang 2023-11-17 19:23 ` Dave Hansen 2022-08-11 19:53 ` [PATCH] " H. Peter Anvin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.