[Qemu-devel] Bogus struct stat64 for qemu-microblaze (user emulation)?

[Qemu-devel] Bogus struct stat64 for qemu-microblaze (user emulation)?

[Rich Felker]

The qemu-microblaze definition of struct stat64 seems to mismatch the
kernel definition, which is using asm-generic/stat.h. See:

http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall_defs.h;h=c9e6323905486452f518102bf40ba73143c9d601;hb=HEAD#l1469
http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall.c;h=a50229d0d72fc68966515fcf2bc308b833a3c032;hb=HEAD#l4949

This seems to be causing a truncated-to-32-bit inode number to be
stored in the location where st_ino should reside, and a spurious copy
of the inode number to be written in a unused slot at the end of the
structure.

Is my analysis correct? Stefan Kristiansson and I found this while
working on the or1k port of musl libc, where it seems our structure
for the existing microblaze port is wrongly aligned with the qemu
definition rather than the definition the real kernel is using. Before
I try correcting this on our side, I want to make sure we're working
with the right version.

Rich

Re: [Qemu-devel] Bogus struct stat64 for qemu-microblaze (user emulation)?

[Peter Maydell]

On 16 July 2014 05:02, Rich Felker <dalias@libc.org> wrote:
> The qemu-microblaze definition of struct stat64 seems to mismatch the
> kernel definition, which is using asm-generic/stat.h. See:
>
> http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall_defs.h;h=c9e6323905486452f518102bf40ba73143c9d601;hb=HEAD#l1469
> http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall.c;h=a50229d0d72fc68966515fcf2bc308b833a3c032;hb=HEAD#l4949
>
> This seems to be causing a truncated-to-32-bit inode number to be
> stored in the location where st_ino should reside, and a spurious copy
> of the inode number to be written in a unused slot at the end of the
> structure.

Sounds quite plausible -- we've had issues with other archs
not having correct stat struct definitions in QEMU. I don't
suppose anybody's done much testing of the microblaze
linux-user code.

> Is my analysis correct? Stefan Kristiansson and I found this while
> working on the or1k port of musl libc, where it seems our structure
> for the existing microblaze port is wrongly aligned with the qemu
> definition rather than the definition the real kernel is using. Before
> I try correcting this on our side, I want to make sure we're working
> with the right version.

I would definitely trust the kernel definition, not QEMU's!

thanks
-- PMM

Re: [Qemu-devel] [musl] Re: Bogus struct stat64 for qemu-microblaze (user emulation)?

[Rich Felker]

On Wed, Jul 16, 2014 at 09:36:23AM +0100, Peter Maydell wrote:
> On 16 July 2014 05:02, Rich Felker <dalias@libc.org> wrote:
> > The qemu-microblaze definition of struct stat64 seems to mismatch the
> > kernel definition, which is using asm-generic/stat.h. See:
> >
> > http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall_defs.h;h=c9e6323905486452f518102bf40ba73143c9d601;hb=HEAD#l1469
> > http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall.c;h=a50229d0d72fc68966515fcf2bc308b833a3c032;hb=HEAD#l4949
> >
> > This seems to be causing a truncated-to-32-bit inode number to be
> > stored in the location where st_ino should reside, and a spurious copy
> > of the inode number to be written in a unused slot at the end of the
> > structure.
> 
> Sounds quite plausible -- we've had issues with other archs
> not having correct stat struct definitions in QEMU. I don't
> suppose anybody's done much testing of the microblaze
> linux-user code.

The bug seems to have been introduced here.

http://git.qemu.org/?p=qemu.git;a=commitdiff;h=a523eb06ec3fb2f4f4f4d362bb23704811d11379

I'm CC'ing the author/committer in case he has any input on why he did
this.

> > Is my analysis correct? Stefan Kristiansson and I found this while
> > working on the or1k port of musl libc, where it seems our structure
> > for the existing microblaze port is wrongly aligned with the qemu
> > definition rather than the definition the real kernel is using. Before
> > I try correcting this on our side, I want to make sure we're working
> > with the right version.
> 
> I would definitely trust the kernel definition, not QEMU's!

Yes.

Rich

Re: [Qemu-devel] [musl] Re: Bogus struct stat64 for qemu-microblaze (user emulation)?

[Edgar E. Iglesias]

On Wed, Jul 16, 2014 at 11:59:58AM -0400, Rich Felker wrote:
> On Wed, Jul 16, 2014 at 09:36:23AM +0100, Peter Maydell wrote:
> > On 16 July 2014 05:02, Rich Felker <dalias@libc.org> wrote:
> > > The qemu-microblaze definition of struct stat64 seems to mismatch the
> > > kernel definition, which is using asm-generic/stat.h. See:
> > >
> > > http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall_defs.h;h=c9e6323905486452f518102bf40ba73143c9d601;hb=HEAD#l1469
> > > http://git.qemu.org/?p=qemu.git;a=blob;f=linux-user/syscall.c;h=a50229d0d72fc68966515fcf2bc308b833a3c032;hb=HEAD#l4949
> > >
> > > This seems to be causing a truncated-to-32-bit inode number to be
> > > stored in the location where st_ino should reside, and a spurious copy
> > > of the inode number to be written in a unused slot at the end of the
> > > structure.
> > 
> > Sounds quite plausible -- we've had issues with other archs
> > not having correct stat struct definitions in QEMU. I don't
> > suppose anybody's done much testing of the microblaze
> > linux-user code.
> 
> The bug seems to have been introduced here.
> 
> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=a523eb06ec3fb2f4f4f4d362bb23704811d11379
> 
> I'm CC'ing the author/committer in case he has any input on why he did
> this.
> 
> > > Is my analysis correct? Stefan Kristiansson and I found this while
> > > working on the or1k port of musl libc, where it seems our structure
> > > for the existing microblaze port is wrongly aligned with the qemu
> > > definition rather than the definition the real kernel is using. Before
> > > I try correcting this on our side, I want to make sure we're working
> > > with the right version.
> > 
> > I would definitely trust the kernel definition, not QEMU's!
> 
> Yes.

Hi,

Yes, please trust the kernel.

We had a state of flux for a while where various ABIs circulated,
I don't remember the details.

If you create a patch and try both mb little and big-endian that
would be awesome.

Thanks,
Edgar


> 
> Rich
>