[kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction

[kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction

[Bill Wendling]

The "mov" instruction to get the error code shouldn't move into a memory
location. Don't allow the compiler to make this decision. Instead
specify that only a register is appropriate here.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 lib/x86/desc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 5f37cef..451f504 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -263,7 +263,7 @@ unsigned exception_error_code(void)
 {
     unsigned short error_code;

-    asm("mov %%gs:6, %0" : "=rm"(error_code));
+    asm("mov %%gs:6, %0" : "=r"(error_code));
     return error_code;
 }

Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction

[Jim Mattson]

On Mon, Sep 9, 2019 at 2:19 PM Bill Wendling <morbo@google.com> wrote:
>
> The "mov" instruction to get the error code shouldn't move into a memory
> location. Don't allow the compiler to make this decision. Instead
> specify that only a register is appropriate here.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction

[Sean Christopherson]

On Mon, Sep 09, 2019 at 02:19:20PM -0700, Bill Wendling wrote:
> The "mov" instruction to get the error code shouldn't move into a memory
> location. Don't allow the compiler to make this decision. Instead
> specify that only a register is appropriate here.

I'd prefer the changelog to say something like:

  Remove a bogus memory contraint as x86 does not have a generic
  memory-to-memory "mov" instruction.

Saying "shouldn't move into a memory location" makes it sound like there's
an unwanted side effect when the compiler selects memory, though I suppose
you could argue that a build error is an unwanted side effect :-).

Out of curiosity, do any compilers actually generate errors because of
this, or is it simply dead code?

> 
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>  lib/x86/desc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 5f37cef..451f504 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -263,7 +263,7 @@ unsigned exception_error_code(void)
>  {
>      unsigned short error_code;
> 
> -    asm("mov %%gs:6, %0" : "=rm"(error_code));
> +    asm("mov %%gs:6, %0" : "=r"(error_code));
>      return error_code;
>  }

Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction

[Bill Wendling]

On Wed, Sep 11, 2019 at 2:08 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Mon, Sep 09, 2019 at 02:19:20PM -0700, Bill Wendling wrote:
> > The "mov" instruction to get the error code shouldn't move into a memory
> > location. Don't allow the compiler to make this decision. Instead
> > specify that only a register is appropriate here.
>
> I'd prefer the changelog to say something like:
>
>   Remove a bogus memory contraint as x86 does not have a generic
>   memory-to-memory "mov" instruction.
>
> Saying "shouldn't move into a memory location" makes it sound like there's
> an unwanted side effect when the compiler selects memory, though I suppose
> you could argue that a build error is an unwanted side effect :-).
>
No prob. :-) I'm not familiar with sending these patches via email. Do
I need to regenerate the patch and send via "send-email"? (Similar
question for the "Reviewed-By" comments.)

> Out of curiosity, do any compilers actually generate errors because of
> this, or is it simply dead code?
>
Yeah, clang uses a memory location, probably due to inlining causing
register pressure (that's a guess, I didn't explore it too deeply).

-bw

> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> >  lib/x86/desc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> > index 5f37cef..451f504 100644
> > --- a/lib/x86/desc.c
> > +++ b/lib/x86/desc.c
> > @@ -263,7 +263,7 @@ unsigned exception_error_code(void)
> >  {
> >      unsigned short error_code;
> >
> > -    asm("mov %%gs:6, %0" : "=rm"(error_code));
> > +    asm("mov %%gs:6, %0" : "=r"(error_code));
> >      return error_code;
> >  }

[PATCH] x86: remove memory constraint from "mov" instruction

[Bill Wendling]

Remove a bogus memory constraint as x86 does not have a generic
memory-to-memory "mov" instruction.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 lib/x86/desc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 5f37cef..451f504 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -263,7 +263,7 @@ unsigned exception_error_code(void)
 {
     unsigned short error_code;
 
-    asm("mov %%gs:6, %0" : "=rm"(error_code));
+    asm("mov %%gs:6, %0" : "=r"(error_code));
     return error_code;
 }
 
-- 
2.23.0.237.gc6a4ce50a0-goog

Re: [PATCH] x86: remove memory constraint from "mov" instruction

[Sean Christopherson]

+cc Paolo, Radim and Marc (to avoid saying the same thing twice)

I recommend having Paolo and Radim in the To: field when sending patches
for KVM or kvm-unit-tests, simply cc'ing the KVM list may not be enough to
ensure Paolo/Radim sees the patch.

https://lkml.kernel.org/r/0d59375c-9313-d31a-4af9-d68115e05d55@redhat.com

On Thu, Sep 12, 2019 at 01:59:44PM -0700, Bill Wendling wrote:
> Remove a bogus memory constraint as x86 does not have a generic
> memory-to-memory "mov" instruction.
> 
> Signed-off-by: Bill Wendling <morbo@google.com>

For the actual patch:

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> ---
>  lib/x86/desc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 5f37cef..451f504 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -263,7 +263,7 @@ unsigned exception_error_code(void)
>  {
>      unsigned short error_code;
>  
> -    asm("mov %%gs:6, %0" : "=rm"(error_code));
> +    asm("mov %%gs:6, %0" : "=r"(error_code));
>      return error_code;
>  }
>  
> -- 
> 2.23.0.237.gc6a4ce50a0-goog
> 

Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction

[Paolo Bonzini]

On 09/09/19 23:19, Bill Wendling wrote:
> The "mov" instruction to get the error code shouldn't move into a memory
> location. Don't allow the compiler to make this decision. Instead
> specify that only a register is appropriate here.
> 
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>  lib/x86/desc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 5f37cef..451f504 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -263,7 +263,7 @@ unsigned exception_error_code(void)
>  {
>      unsigned short error_code;
> 
> -    asm("mov %%gs:6, %0" : "=rm"(error_code));
> +    asm("mov %%gs:6, %0" : "=r"(error_code));
>      return error_code;
>  }
> 

Queued, thanks.  (With fixed commit message).

Paolo