Mailing List Archive: tools/libxc/xc_cpuid

tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm

Dec 2, 2011, 12:54 AM

Post #1 of 7 (199 views)

Keir,

do you recall what it was that 20976:f8692cc67d67 was supposed to fix?
The code is clearly wrong now on x86-64 - inline asm that uses push/pop
(or other stack pointer manipulation instructions) and memory accesses
that may reference the stack (even if not through %rsp) can't be
expected to work without -mno-red-zone.

The question is whether to use that command line option, or whether to
correct the inline assembly (besides the purpose of your change, I also
wonder why this isn't coded the obvious way, with rBX and rDX explicitly
named in the constraints - on 32-bit this may be to reduce register
pressure, but on 64-bit it's entirely unclear).

Thanks, Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm [ In reply to ]

keir.xen at gmail

Dec 1, 2011, 10:02 PM

Post #2 of 7 (193 views)

Permalink

On 02/12/2011 08:54, "Jan Beulich" <JBeulich@suse.com> wrote:

> Keir,
>
> do you recall what it was that 20976:f8692cc67d67 was supposed to fix?
> The code is clearly wrong now on x86-64 - inline asm that uses push/pop
> (or other stack pointer manipulation instructions) and memory accesses
> that may reference the stack (even if not through %rsp) can't be
> expected to work without -mno-red-zone.

I totally missed the red-zone constraint. :-(

> The question is whether to use that command line option, or whether to
> correct the inline assembly (besides the purpose of your change, I also
> wonder why this isn't coded the obvious way, with rBX and rDX explicitly
> named in the constraints - on 32-bit this may be to reduce register
> pressure, but on 64-bit it's entirely unclear).

I think reg constraint failures had only been reported on 32-bit. So how
about the attached patch?

-- Keir

> Thanks, Jan
>

Re: tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm [ In reply to ]

keir.xen at gmail

Dec 2, 2011, 12:40 AM

Post #3 of 7 (192 views)

Permalink

On 02/12/2011 15:26, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Fri, Dec 02, Keir Fraser wrote:
>
>> I think reg constraint failures had only been reported on 32-bit. So how
>> about the attached patch?
>
> + : "0" (input[0]), "1" (count), "S" (_regs)
>
> _regs is undeclared in 32bit, I think it should be regs.

Oops! Fixed!

Thanks,
Keir

> Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm [ In reply to ]

keir.xen at gmail

Dec 2, 2011, 2:08 AM

Post #4 of 7 (193 views)

Permalink

On 02/12/2011 16:47, "Olaf Hering" <olaf@aepfle.de> wrote:

> On Fri, Dec 02, Keir Fraser wrote:
>
>> I think reg constraint failures had only been reported on 32-bit. So how
>> about the attached patch?
>
> So finally I was able to test this change, and it fixes the reported segfault.
> Thanks!

I will sweep this through into 4.0/4.1 as part of a general backport sweep
next week.

-- Keir

> Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm [ In reply to ]

JBeulich at suse

Dec 2, 2011, 6:20 AM

Post #5 of 7 (195 views)

Permalink

>>> On 02.12.11 at 07:02, Keir Fraser <keir.xen@gmail.com> wrote:
> On 02/12/2011 08:54, "Jan Beulich" <JBeulich@suse.com> wrote:
>> The question is whether to use that command line option, or whether to
>> correct the inline assembly (besides the purpose of your change, I also
>> wonder why this isn't coded the obvious way, with rBX and rDX explicitly
>> named in the constraints - on 32-bit this may be to reduce register
>> pressure, but on 64-bit it's entirely unclear).
>
> I think reg constraint failures had only been reported on 32-bit. So how
> about the attached patch?

Looks good.

Acked-by: Jan Beulich <jbeulich@novell.com>

While we only got problems with this on 4.1.2, I would suggest to also
put this back into 4.0-testing.

Thanks, Jan

E-mail confidentiality notice. This message is intended for the addressees only. It may be private, confidential and may be covered by legal professional privilege or other confidentiality requirements. If you are not one of the intended recipients, please notify the sender immediately on +44 0 20-8215-3000 and delete the message from all locations in your computer network. Do not copy this email or use it for any purpose or disclose its contents to any person:to do so maybe unlawful.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm [ In reply to ]

olaf at aepfle

Dec 2, 2011, 7:26 AM

Post #6 of 7 (197 views)

Permalink

On Fri, Dec 02, Keir Fraser wrote:

> I think reg constraint failures had only been reported on 32-bit. So how
> about the attached patch?

+ : "0" (input[0]), "1" (count), "S" (_regs)

_regs is undeclared in 32bit, I think it should be regs.

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: tools/libxc/xc_cpuid_x86.c:cpuid()'s inline asm [ In reply to ]

olaf at aepfle

Dec 2, 2011, 8:47 AM

Post #7 of 7 (210 views)

Permalink

On Fri, Dec 02, Keir Fraser wrote:

> I think reg constraint failures had only been reported on 32-bit. So how
> about the attached patch?

So finally I was able to test this change, and it fixes the reported segfault.
Thanks!

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Mailing List Archive

Mailing List Archive

Attached Files: