Hello, Back in changeset 19772:aaab04808ee7, you introduced headers.chk to check the header files for ansi conformance. To fix the current 32/64bit interaction errors with the kexec hypercalls, I need to use uint64_aligned_t as a datatype. For the normal compile, this is all fine, but as header.chk does not define __XEN__ or __XEN_TOOLS__, the declaration of uint64_aligned_t is never made, leading to the check failing. There are other hypercall interfaces which use these datatypes: domctl, sysctl and hvm_op, but these header files are explicitly filtered out from the prerequisites for header.chk. Given that uint64_aligned_t is a sensible datatype to be using with the hypercall interface, fixing the check seems to be the correct solution. In your oppinion, which is the best course of action? To define __XEN__ or __XEN_TOOLS__ as part of the check (this throws up other errors as part of the check process, suggesting that the header files are hiding ansi non-conformance in certain blocks), or dont predicate the definition of uint64_aligned_t on the presence of the above defines? In addition, why are certain header files excluded from being checked? Does this imply that then should be fixed up to be ansi conformant as well? -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com
>>> On 13.12.11 at 14:38, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > To fix the current 32/64bit interaction errors with the kexec > hypercalls, I need to use uint64_aligned_t as a datatype. For the > normal compile, this is all fine, but as header.chk does not define > __XEN__ or __XEN_TOOLS__, the declaration of uint64_aligned_t is never > made, leading to the check failing. > > There are other hypercall interfaces which use these datatypes: domctl, > sysctl and hvm_op, but these header files are explicitly filtered out > from the prerequisites for header.chk.This is really intentional for the domctl and sysctl cases - these are "internal" interfaces (between the tools and the hypervisor), and hence can be more relaxed in style. Pretty much the same goes for the hvm op definitions - we simply assume that users of this use a compiler capable of dealing with this (I''m not fully convinced this is The Right Thing To Do, but it''s the status quo at least).> Given that uint64_aligned_t is a > sensible datatype to be using with the hypercall interface, fixing the > check seems to be the correct solution.Sensible or not, you would have to find a way to define such a type in a compiler-independent (i.e. ANSI conforming) manner, and I''m afraid you won''t be able to.> In your oppinion, which is the best course of action? To define __XEN__ > or __XEN_TOOLS__ as part of the check (this throws up other errors as > part of the check process, suggesting that the header files are hiding > ansi non-conformance in certain blocks), or dont predicate the > definition of uint64_aligned_t on the presence of the above defines?__XEN__ and __XEN_TOOLS__ must specifically not be defined for these checks (and these symbols indeed get used to frame certain definitions that are considered "internal" as outlined above - obviously, by framing any definition this way we hide them from "foreign" consumers *and* the [pointless in this case] style checking). So bottom line is - no, you can''t use uint64_aligned_t as much as any other non-ANSI extension. Preventing anyone to try is what the check was introduced for. Jan
On 13/12/2011 14:17, "Jan Beulich" <JBeulich@suse.com> wrote:>> Given that uint64_aligned_t is a >> sensible datatype to be using with the hypercall interface, fixing the >> check seems to be the correct solution. > > Sensible or not, you would have to find a way to define such a type > in a compiler-independent (i.e. ANSI conforming) manner, and I''m > afraid you won''t be able to.Yes, as barbaric as it may seem you just have to make sure that uint64_t gets naturally 8-byte aligned. Luckily none of our interface structures are particularly large, and I believe you can get them checked for 32/64-bit invariance by adding them to xlat.lst. -- Keir
>>> On 13.12.11 at 15:21, Keir Fraser <keir@xen.org> wrote: > On 13/12/2011 14:17, "Jan Beulich" <JBeulich@suse.com> wrote: > >>> Given that uint64_aligned_t is a >>> sensible datatype to be using with the hypercall interface, fixing the >>> check seems to be the correct solution. >> >> Sensible or not, you would have to find a way to define such a type >> in a compiler-independent (i.e. ANSI conforming) manner, and I''m >> afraid you won''t be able to. > > Yes, as barbaric as it may seem you just have to make sure that uint64_t > gets naturally 8-byte aligned. Luckily none of our interface structures are > particularly large, and I believe you can get them checked for 32/64-bit > invariance by adding them to xlat.lst.... and invoking the resultant check macro somewhere in the sources. Jan
On 13/12/11 14:25, Jan Beulich wrote:>>>> On 13.12.11 at 15:21, Keir Fraser <keir@xen.org> wrote: >> On 13/12/2011 14:17, "Jan Beulich" <JBeulich@suse.com> wrote: >> >>>> Given that uint64_aligned_t is a >>>> sensible datatype to be using with the hypercall interface, fixing the >>>> check seems to be the correct solution. >>> Sensible or not, you would have to find a way to define such a type >>> in a compiler-independent (i.e. ANSI conforming) manner, and I''m >>> afraid you won''t be able to. >> Yes, as barbaric as it may seem you just have to make sure that uint64_t >> gets naturally 8-byte aligned. Luckily none of our interface structures are >> particularly large, and I believe you can get them checked for 32/64-bit >> invariance by adding them to xlat.lst. > ... and invoking the resultant check macro somewhere in the sources. > > JanRigh ok. Thanks for the explanation. Luckily, it appears that all my uint64_t''s do actually align on 8 bytes so I will just use that. -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com