Re: aarch64 gcc kernel compilation
On Tue, Jul 17, 2018 at 02:23:36PM +0900, Ryo Shimizu wrote:
> union fpelem {
> uint64_t u64[2];
> - __uint128_t u128[1] __aligned(16);
> + /* __uint128_t u128[1] __aligned(16); */
> };
I like the alignement parts of the change, but I seriously do not
understand why the __uint128_t is a bad thing.
I'd be fine if that line is #if'd out for compilers not supporting
it, if they exist.
No support in printf and friends for this type is a strange justification
for removing a very natural representation for kernel purposes here
if it does not actually hurt (and we never printf it this way
at all, and if needed just a hexdump of the fpelem would be good enough).
I am not objecting this patch in general, but so far no coherent
proposal has been made why this should happen.
Martin
Re: aarch64 gcc kernel compilation
Can we drop it? The __uint128_t type is not used anywhere else in
aarch64 subdirs.
fortunately, we don't use member of fpreg, use just only as container.
Is this patch enough?
cvs -q diff -aup pcb.h reg.h
Index: pcb.h
===
RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/aarch64/include/pcb.h,v
retrieving revision 1.1
diff -a -u -p -r1.1 pcb.h
--- pcb.h 10 Aug 2014 05:47:38 - 1.1
+++ pcb.h 17 Jul 2018 04:59:37 -
@@ -45,6 +45,10 @@ struct md_coredump {
struct fpreg fpreg;
};
+/* fpreg should be aligned 16 */
+CTASSERT((offsetof(struct pcb, pcb_fpregs) & 15) == 0);
+CTASSERT((offsetof(struct md_coredump, fpreg) & 15) == 0);
+
#elif defined(__arm__)
#include
Index: reg.h
===
RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/aarch64/include/reg.h,v
retrieving revision 1.2
diff -a -u -p -r1.2 reg.h
--- reg.h 1 Apr 2018 04:35:03 - 1.2
+++ reg.h 17 Jul 2018 04:58:07 -
@@ -44,14 +44,14 @@ struct reg {
union fpelem {
uint64_t u64[2];
- __uint128_t u128[1] __aligned(16);
+ /* __uint128_t u128[1] __aligned(16); */
};
struct fpreg {
union fpelem fp_reg[32];
uint32_t fpcr;
uint32_t fpsr;
-};
+} __aligned(16);
#elif defined(__arm__)
--
ryo shimizu
sdmmc reader not working on 8.0_RC2/amd64
I am trying to use the SD/MMC reader in my PC (Intel NUC), however I always get the following error: sdmmc0: sdmmc_mem_enable failed with error 60 sdmmc0: couldn't enable card: 60 60 is probably ETIMEDOUT, no? Any ideas what could be done? For completeness, here is where the sdmmc attaches to: sdhc0 at pci2 dev 0 function 0: vendor 1217 product 8621 (rev. 0x01) sdhc0: interrupting at ioapic0 pin 17 sdhc0: SDHC 4.0, rev 6, SDMA, 5 kHz, HS SDR50 DDR50 SDR104 HS200 1.8V 3.3V, re-tuning mode 1, 512 byte blocks sdmmc0 at sdhc0 slot 0 -- Benny
Re: aarch64 gcc kernel compilation
On 16.07.2018 11:07, Kamil Rytarowski wrote: > On 16.07.2018 10:50, Kamil Rytarowski wrote: >> On 16.07.2018 00:00, Kamil Rytarowski wrote: >>> On 15.07.2018 20:08, Christos Zoulas wrote: Hi, Gcc is now working on aarch64 but the kernel does not compile because of some idiomatic clang code that is not supported by gcc (at least gcc-6) To define constants, it uses: static const uintmax_t FOO = __BIT(9), BAR = FOO; While this is nice, specially for the debugger, it produces an error in gcc. While fixing these is easy, gcc also complains about using the constants as switch labels. Thus it is better to just nukem all and rewrite them as: #define FOO __BIT(9) #define BAR FOO Should I go ahead and do it, or there is a smarter solution? christos >>> >>> I used to have problems to build rumpkernel aarch64 on Linux with GCC >>> (some years ago) due to usage __uint128_t in reg.h. >>> >>> Can we drop it? The __uint128_t type is not used anywhere else in >>> aarch64 subdirs. >>> >>> It's used in assembly in FPREG_Q0-FPREQ_Q31 in cpuswitch.S. The same >>> optimization can be done without the usage of __uint128_t, probably just >>> need for proper alignment of fp_reg (15). >> >> 16* >> >>> >>> There is also some mysterious fallout that General Purpose Registers in >>> core files are shipped with 128bit containers. It's not compatible with >>> LLDB and requires needless generic work for no purpose. >>> >>> I can try to prepare a patch blindly and share with aarch64 owners. >>> >> >> Looking deeper, there are various reports regarding aarch64 128-bit >> broken support. >> >> "Be careful of GCC's __uint128_t. It caused us problems on a number of >> platforms, like ARM64, ARMEL and S/390. We had to give up using it >> because it was so buggy. For example, GCC calculated the result of u = >> 93 - 0 - 0 - 0 (using the 128-bit types) as 18446744073709551615 on ARM64" >> >> https://stackoverflow.com/questions/11656241/how-to-print-uint128-t-number-using-gcc >> >> There are no utility features for such numbers such as PRIu128, no >> support in printf(3), snprintf(3) etc. >> >> I will prepare a patch for removal of this from public machine headers. >> > > I was asked to provide some links to gcc bugzilla: > > https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__int128_t > https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__uint128_t > > My reason is unportable construct of reg.h, no utility functions for > 128bits and alien style core files with 128bit containers for registers. > This has been rejected by Martin. signature.asc Description: OpenPGP digital signature
Re: hid.h fallout in third party code
On Sat, Jul 14, 2018 at 03:39:20PM +, co...@sdf.org wrote: > hi folks, > > in > https://github.com/NetBSD/src/commit/a9e749a2e2d0044b947401ce80790a5788fad76e#diff-9353912fc541114002b043446f11751e > bouyer had moved many definitions out of usbhid.h. > > This is a user-visible header and appears in third party packages, which > now need even more ifdefs, and those need to be versioned too, which is > extra ugly. I already fixed some packages, it was not that ugly. > > example of third party code using it: > https://sourceforge.net/p/vice-emu/code/HEAD/tree/tags/v3.2/vice/src/arch/unix/joy_usb.c#l72 > > How about this following diff, to retain the same visiblity for > the definitions? > > I needed HUP_GENERIC_DESKTOP. > > Index: dev/usb/usbhid.h > === > RCS file: /cvsroot/src/sys/dev/usb/usbhid.h,v > retrieving revision 1.17 > diff -u -r1.17 usbhid.h > --- dev/usb/usbhid.h 10 Dec 2017 17:03:07 - 1.17 > +++ dev/usb/usbhid.h 14 Jul 2018 15:35:39 - > @@ -35,6 +35,8 @@ > #ifndef _DEV_USB_USBHID_H_ > #define _DEV_USB_USBHID_H_ > > +#include > + > #define UR_GET_HID_DESCRIPTOR0x06 > #define UDESC_HID 0x21 > #define UDESC_REPORT0x22 Fine with me, but then you'd need to remove the extra include in other source files -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: aarch64 gcc kernel compilation
>Gcc is now working on aarch64 but the kernel does not compile because of
>some idiomatic clang code that is not supported by gcc (at least gcc-6)
>
>To define constants, it uses:
>
>static const uintmax_t
>FOO = __BIT(9),
>BAR = FOO;
>
>While this is nice, specially for the debugger, it produces an error
>in gcc. While fixing these is easy, gcc also complains about using the
>constants as switch labels. Thus it is better to just nukem all and
>rewrite them as:
>
>#define FOO __BIT(9)
>#define BAR FOO
>
>Should I go ahead and do it, or there is a smarter solution?
Yes, please!
I tested with below script in my environment (clang), there seems to be no
problem.
perl -i.old a64armreg_conv.pl aarch64/include/armreg.h
# a64armreg_conv.pl
while (<>) {
if (m#^static\s+(const\s*)?uintmax_t\s*(//.*)?$#) {
} elsif (m#^\s*(\w+)\s*=\s*(.*?)[,;]\s*//(.*)$#) {
print "#define $1 $2 //$3\n";
} elsif (m#^\s*(\w+)\s*=\s*(.*?)[,;]\s*$#) {
print "#define $1 $2\n";
} else {
print;
}
}
--
ryo shimizu
Re: aarch64 gcc kernel compilation
On 16.07.2018 10:50, Kamil Rytarowski wrote: > On 16.07.2018 00:00, Kamil Rytarowski wrote: >> On 15.07.2018 20:08, Christos Zoulas wrote: >>> Hi, >>> >>> Gcc is now working on aarch64 but the kernel does not compile because of >>> some idiomatic clang code that is not supported by gcc (at least gcc-6) >>> >>> To define constants, it uses: >>> >>> static const uintmax_t >>> FOO = __BIT(9), >>> BAR = FOO; >>> >>> While this is nice, specially for the debugger, it produces an error >>> in gcc. While fixing these is easy, gcc also complains about using the >>> constants as switch labels. Thus it is better to just nukem all and >>> rewrite them as: >>> >>> #define FOO __BIT(9) >>> #define BAR FOO >>> >>> Should I go ahead and do it, or there is a smarter solution? >>> >>> christos >>> >> >> I used to have problems to build rumpkernel aarch64 on Linux with GCC >> (some years ago) due to usage __uint128_t in reg.h. >> >> Can we drop it? The __uint128_t type is not used anywhere else in >> aarch64 subdirs. >> >> It's used in assembly in FPREG_Q0-FPREQ_Q31 in cpuswitch.S. The same >> optimization can be done without the usage of __uint128_t, probably just >> need for proper alignment of fp_reg (15). > > 16* > >> >> There is also some mysterious fallout that General Purpose Registers in >> core files are shipped with 128bit containers. It's not compatible with >> LLDB and requires needless generic work for no purpose. >> >> I can try to prepare a patch blindly and share with aarch64 owners. >> > > Looking deeper, there are various reports regarding aarch64 128-bit > broken support. > > "Be careful of GCC's __uint128_t. It caused us problems on a number of > platforms, like ARM64, ARMEL and S/390. We had to give up using it > because it was so buggy. For example, GCC calculated the result of u = > 93 - 0 - 0 - 0 (using the 128-bit types) as 18446744073709551615 on ARM64" > > https://stackoverflow.com/questions/11656241/how-to-print-uint128-t-number-using-gcc > > There are no utility features for such numbers such as PRIu128, no > support in printf(3), snprintf(3) etc. > > I will prepare a patch for removal of this from public machine headers. > I was asked to provide some links to gcc bugzilla: https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__int128_t https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=__uint128_t My reason is unportable construct of reg.h, no utility functions for 128bits and alien style core files with 128bit containers for registers. signature.asc Description: OpenPGP digital signature
