KVM Archive on lore.kernel.org
 help / color / Atom feed
* [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction
@ 2019-09-09 21:19 Bill Wendling
  2019-09-10 16:44 ` Jim Mattson
  2019-09-11 19:08 ` Sean Christopherson
  0 siblings, 2 replies; 6+ messages in thread
From: Bill Wendling @ 2019-09-09 21:19 UTC (permalink / raw)
  To: kvm

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;
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction
  2019-09-09 21:19 [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction Bill Wendling
@ 2019-09-10 16:44 ` Jim Mattson
  2019-09-11 19:08 ` Sean Christopherson
  1 sibling, 0 replies; 6+ messages in thread
From: Jim Mattson @ 2019-09-10 16:44 UTC (permalink / raw)
  To: Bill Wendling; +Cc: kvm list

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction
  2019-09-09 21:19 [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction Bill Wendling
  2019-09-10 16:44 ` Jim Mattson
@ 2019-09-11 19:08 ` Sean Christopherson
  2019-09-11 22:35   ` Bill Wendling
  1 sibling, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2019-09-11 19:08 UTC (permalink / raw)
  To: Bill Wendling; +Cc: kvm

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;
>  }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction
  2019-09-11 19:08 ` Sean Christopherson
@ 2019-09-11 22:35   ` Bill Wendling
  2019-09-12 20:59     ` [PATCH] " Bill Wendling
  0 siblings, 1 reply; 6+ messages in thread
From: Bill Wendling @ 2019-09-11 22:35 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm list

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;
> >  }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] x86: remove memory constraint from "mov" instruction
  2019-09-11 22:35   ` Bill Wendling
@ 2019-09-12 20:59     ` " Bill Wendling
  2019-09-19 21:44       ` Sean Christopherson
  0 siblings, 1 reply; 6+ messages in thread
From: Bill Wendling @ 2019-09-12 20:59 UTC (permalink / raw)
  To: kvm; +Cc: sean.j.christopherson, 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: remove memory constraint from "mov" instruction
  2019-09-12 20:59     ` [PATCH] " Bill Wendling
@ 2019-09-19 21:44       ` Sean Christopherson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-09-19 21:44 UTC (permalink / raw)
  To: Bill Wendling; +Cc: kvm, Paolo Bonzini, Radim Krčmář, Marc Orr

+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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-09 21:19 [kvm-unit-tests PATCH] x86: remove memory constraint from "mov" instruction Bill Wendling
2019-09-10 16:44 ` Jim Mattson
2019-09-11 19:08 ` Sean Christopherson
2019-09-11 22:35   ` Bill Wendling
2019-09-12 20:59     ` [PATCH] " Bill Wendling
2019-09-19 21:44       ` Sean Christopherson

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox