Re: [Xen-devel] [PATCH] tools/tests/x86_emulate: #define unlikely in x86 emulator test harness

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH] tools/tests/x86_emulate: #define unlikely in 
x86 emulator test harness"):
> On 22.12.16 at 16:10,  wrote:
> > I did not find this important build fix for a regression in 4.8.0
> > because:
> 
> I wonder why you consider this important - the harness doesn't
> get built by default, and is of little use for other than smoke
> testing a limited set of changes to the insn emulator.

I think this must be being enabled by something in the Debian package
build.

> > Backporting this is made more awkward by the decision to make the fix
> > a portmanteau.  Do the x86 maintainers intend to provide a
> > ready-to-use backport or shall I try to prepare one ?
> 
> I've pushed one a minute ago to 4.8-staging.

Ah, great, thanks.

> > For now, is there any reason why I should not use my change
> >+#define unlikely(x) (x)
> > in an upload to Debian ?
> 
> I don't see anything speaking against that.

OK, thanks.  I think now there's a fix in 4.8-staging I will cherry
pick that instead.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools/tests/x86_emulate: #define unlikely in x86 emulator test harness

[Jan Beulich]

>>> On 22.12.16 at 16:10,  wrote:
> I did not find this important build fix for a regression in 4.8.0
> because:

I wonder why you consider this important - the harness doesn't
get built by default, and is of little use for other than smoke
testing a limited set of changes to the insn emulator.

> Backporting this is made more awkward by the decision to make the fix
> a portmanteau.  Do the x86 maintainers intend to provide a
> ready-to-use backport or shall I try to prepare one ?

I've pushed one a minute ago to 4.8-staging.

> For now, is there any reason why I should not use my change
>+#define unlikely(x) (x)
> in an upload to Debian ?

I don't see anything speaking against that.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools/tests/x86_emulate: #define unlikely in x86 emulator test harness

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] tools/tests/x86_emulate: #define unlikely in 
x86 emulator test harness"):
> On 22/12/16 14:58, Ian Jackson wrote:
> > "x86emul: in_longmode() should not ignore ->read_msr() errors" aka
> > c/s 122dd9575c7a introduced a use of unlikely() in
> > xen/arch/x86/x86_emulate/x86_emulate.c.
> >
> > I think this is probably intentional and fine.  However, there is no
> > definition of unlikely in the x86 emulator test harness, under tools.
> >
> > The result is this error:
> >x86_emulate/x86_emulate.c: In function 'in_longmode':
> >x86_emulate/x86_emulate.c:1300:10: error: implicit declaration of 
> > function 'unlikely' [-Werror=implicit-function-declaration]
> >  unlikely(ops->read_msr(MSR_EFER, , ctxt) != X86EMUL_OKAY) 
> > )
> >  ^~~~
> >
> > Fix this by providing a boring definition of unlikely().
> >
> > Signed-off-by: Ian Jackson 
> 
> This was fixed by 3e84c8da7d2c5442a12789dae7163dca6c0e154f

I did not find this important build fix for a regression in 4.8.0
because:

 * this commit contains a mixture of the build fix and other changes
 * `git log -G unlikely' produced a lot of output: too much to read
   the whole message for each commit through looking for this fix
 * the commit message did not contain a copy of the error message
 * once I had dug into the code myself and found 122dd9575c7a it
   didn't occur to me to `git log --grep 122dd9'.

> Part of that should backported to 4.8, where it is still broken.

Backporting this is made more awkward by the decision to make the fix
a portmanteau.  Do the x86 maintainers intend to provide a
ready-to-use backport or shall I try to prepare one ?

For now, is there any reason why I should not use my change
   +#define unlikely(x) (x)
in an upload to Debian ?

Thanks,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools/tests/x86_emulate: #define unlikely in x86 emulator test harness

[Andrew Cooper]

On 22/12/16 14:58, Ian Jackson wrote:

"x86emul: in_longmode() should not ignore ->read_msr() errors" aka
c/s 122dd9575c7a introduced a use of unlikely() in
xen/arch/x86/x86_emulate/x86_emulate.c.

I think this is probably intentional and fine.  However, there is no
definition of unlikely in the x86 emulator test harness, under tools.

The result is this error:
   x86_emulate/x86_emulate.c: In function 'in_longmode':
   x86_emulate/x86_emulate.c:1300:10: error: implicit declaration of function 
'unlikely' [-Werror=implicit-function-declaration]
 unlikely(ops->read_msr(MSR_EFER, , ctxt) != X86EMUL_OKAY) )
 ^~~~

Fix this by providing a boring definition of unlikely().

Signed-off-by: Ian Jackson 


This was fixed by 3e84c8da7d2c5442a12789dae7163dca6c0e154f

Part of that should backported to 4.8, where it is still broken.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] tools/tests/x86_emulate: #define unlikely in x86 emulator test harness

[Ian Jackson]

"x86emul: in_longmode() should not ignore ->read_msr() errors" aka
c/s 122dd9575c7a introduced a use of unlikely() in
xen/arch/x86/x86_emulate/x86_emulate.c.

I think this is probably intentional and fine.  However, there is no
definition of unlikely in the x86 emulator test harness, under tools.

The result is this error:
  x86_emulate/x86_emulate.c: In function 'in_longmode':
  x86_emulate/x86_emulate.c:1300:10: error: implicit declaration of function 
'unlikely' [-Werror=implicit-function-declaration]
unlikely(ops->read_msr(MSR_EFER, , ctxt) != X86EMUL_OKAY) )
^~~~

Fix this by providing a boring definition of unlikely().

Signed-off-by: Ian Jackson 
CC: Andrew Cooper 
CC: Jan Beulich 
---
 tools/tests/x86_emulator/x86_emulate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/tests/x86_emulator/x86_emulate.c 
b/tools/tests/x86_emulator/x86_emulate.c
index c46b7fc..e8f26fe 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -49,5 +49,6 @@ typedef bool bool_t;
 
 #define __init
 #define __maybe_unused __attribute__((__unused__))
+#define unlikely(x) (x)
 
 #include "x86_emulate/x86_emulate.c"
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel