Issue with PC updates.

2024-01-16 Thread Sid Manning
Hi Taylor,

I ran into an issue when a packet, not executed out of ram 
(get_page_addr_code_hostp returns -1, see translate-all.c) contains a fault.
This packet is an example:
{
p0 = cmp.eq(r6,#0x6)
if (p0.new) jump:t pass
memw(##0xf200) = r6
}

The above packet should always jump to "pass" since r6 is set to #0x6, but if 
the store faults, the jump is discarded.  This happens because 
do_raise_exception's call to cpu_loop_exit_restore is not able to find a TB to 
restore the PC to.  When an instruction is not associated with a physical RAM 
page translate-all will create a "one-shot" TB so when cpu_restore_state looks 
for the TB by calling tcg_tb_loopup none is found.  That keeps the PC from 
being restored.

The change attached restores some of the code from commit 
613653e500c0d482784f09aaa71f1297565b6815 / Hexagon (target/hexagon) Remove 
next_PC from runtime state.

There are two attachments, the qemu update also includes an update to 
translate-all.c that forces this problem to occur.  The second is the testcase 
which is built using vanilla llvm toolchain configured for hexagon.

Thanks,


pc-testcase.tar.gz
Description: pc-testcase.tar.gz


0001-Incorrect-PC-update-for-many-miss-packets.patch
Description: 0001-Incorrect-PC-update-for-many-miss-packets.patch


RE: [PATCH 1/1] accel/tcg: Allow the second page of an instruction to be MMIO

2023-02-07 Thread Sid Manning


> -Original Message-
> From: Jørgen Hansen 
> Sent: Tuesday, February 7, 2023 9:03 AM
> To: Richard Henderson ; qemu-
> de...@nongnu.org
> Cc: Sid Manning ; Mark Burton
> ; Brian Cain ; Matheus
> Bernardino ; Ajay Joshi
> 
> Subject: Re: [PATCH 1/1] accel/tcg: Allow the second page of an instruction to
> be MMIO
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 2/6/23 20:38, Richard Henderson wrote:
> > If an instruction straddles a page boundary, and the first page was
> > ram, but the second page was MMIO, we would abort.  Handle this as if
> > both pages are MMIO, by setting the ram_addr_t for the first page to
> > -1.
> >
> > Reported-by: Sid Manning 
> > Reported-by: Jørgen Hansen 
> > Signed-off-by: Richard Henderson 
> > ---
> >   accel/tcg/translator.c | 12 ++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index
> > ef5193c67e..1cf404ced0 100644
> > --- a/accel/tcg/translator.c
> > +++ b/accel/tcg/translator.c
> > @@ -176,8 +176,16 @@ static void *translator_access(CPUArchState *env,
> DisasContextBase *db,
> >   if (host == NULL) {
> >   tb_page_addr_t phys_page =
> >   get_page_addr_code_hostp(env, base, >host_addr[1]);
> > -/* We cannot handle MMIO as second page. */
> > -assert(phys_page != -1);
> > +
> > +/*
> > + * If the second page is MMIO, treat as if the first page
> > + * was MMIO as well, so that we do not cache the TB.
> > + */
> > +if (unlikely(phys_page == -1)) {
> > +tb_set_page_addr0(tb, -1);
> > +return NULL;
> > +}
> > +
> >   tb_set_page_addr1(tb, phys_page);
> >   #ifdef CONFIG_USER_ONLY
> >   page_protect(end);
> > --
> > 2.34.1
> >
> 
> Thanks a lot for the quick turnaround. I've verified that the patch resolves
> the issue we experienced.

I also verified this patch fixed my issue.  Thanks!


RE: accel/tcg/translator.c question about translator_access

2023-02-06 Thread Sid Manning


> -Original Message-
> From: Richard Henderson 
> Sent: Tuesday, January 31, 2023 11:46 PM
> To: Sid Manning ; qemu-devel@nongnu.org
> Cc: Mark Burton ; Brian Cain
> ; Matheus Bernardino
> 
> Subject: Re: accel/tcg/translator.c question about translator_access
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 1/31/23 17:06, Sid Manning wrote:
> > There is an assert in translator_access that I hit while running on a
> > version of QEMU integrated into a Virtual Platform.
> >
> > Since this function can return null anyway I tried the following experiment:
> ...
> > -assert(phys_page != -1);
> > +if(phys_page == -1) {
> > +return NULL;
> > +}
> ...
> > which avoided the issue and the test ran to completion.  What is this assert
> trying to catch?
> 
> 
> One half of the instruction in ram and the other half of the instruction in
> mmio.
> 
> If the entire instruction is in mmio, then we correctly translate, but do not
> cache the result (since the io can produce different results on every access).
> But if we have started caching the result, because we start in ram, then we
> will incorrectly cache the mmio access.
> 
> This really should never happen.  How did it occur?

This might be a synchronization problem with System-C, a packet is straddling a 
page boundary.  Software running on the ARM is dispatching code to run on the 
DSP.  I have only seen this when the cores are interacting in this way.

PS: Sorry for the delayed response, I was unexpectedly out of the office last 
week due to an ice storm and power outage.

> 
> 
> r~


accel/tcg/translator.c question about translator_access

2023-01-31 Thread Sid Manning
There is an assert in translator_access that I hit while running on a version 
of QEMU integrated into a Virtual Platform.

Since this function can return null anyway I tried the following experiment:

--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -172,7 +172,9 @@ static void *translator_access(CPUArchState *env, 
DisasContextBase *db,
 tb_page_addr_t phys_page =
 get_page_addr_code_hostp(env, base, >host_addr[1]);
 /* We cannot handle MMIO as second page. */
-assert(phys_page != -1);
+if(phys_page == -1) {
+return NULL;
+}
 tb_set_page_addr1(tb, phys_page);
#ifdef CONFIG_USER_ONLY
 page_protect(end);

which avoided the issue and the test ran to completion.  What is this assert 
trying to catch?


RE: ARM: ptw.c:S1_ptw_translate

2023-01-26 Thread Sid Manning


> -Original Message-
> From: Richard Henderson 
> Sent: Thursday, January 26, 2023 3:48 PM
> To: Sid Manning ; qemu-devel@nongnu.org
> Cc: phi...@linaro.org; Mark Burton ; Alex
> Bennée 
> Subject: Re: ARM: ptw.c:S1_ptw_translate
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> Please try the following.  It's essentially the same bug I had for mte.
> I've just realized that the testing I did under Linux with virtualization=on 
> was
> insufficient -- this path won't be exercised without KVM under TCG.
> 
> 
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c index
> 57f3615a66..2b125fff44 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -266,7 +266,7 @@ static bool S1_ptw_translate(CPUARMState *env,
> S1Translate *ptw,
>   if (unlikely(flags & TLB_INVALID_MASK)) {
>   goto fail;
>   }
> -ptw->out_phys = full->phys_addr;
> +ptw->out_phys = full->phys_addr | (addr & ~TARGET_PAGE_MASK);
>   ptw->out_rw = full->prot & PAGE_WRITE;
>   pte_attrs = full->pte_attrs;
>   pte_secure = full->attrs.secure;
> 
This change works!  Thank you for taking the time to look into it.

> 
> 
> r~


RE: ARM: ptw.c:S1_ptw_translate

2023-01-25 Thread Sid Manning


> -Original Message-
> From: qemu-devel-bounces+sidneym=quicinc@nongnu.org  devel-bounces+sidneym=quicinc@nongnu.org> On Behalf Of Sid
> Manning
> Sent: Thursday, January 5, 2023 7:08 PM
> To: 'Richard Henderson' ; qemu-
> de...@nongnu.org
> Cc: phi...@linaro.org; Mark Burton 
> Subject: RE: ARM: ptw.c:S1_ptw_translate
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> > -Original Message-
> > From: Richard Henderson 
> > Sent: Wednesday, January 4, 2023 11:42 PM
> > To: Sid Manning ; qemu-devel@nongnu.org
> > Cc: phi...@linaro.org; Mark Burton 
> > Subject: Re: ARM: ptw.c:S1_ptw_translate
> >
> > WARNING: This email originated from outside of Qualcomm. Please be
> > wary of any links or attachments, and do not enable macros.
> >
> > On 1/4/23 08:55, Sid Manning wrote:
> > > ptw.c:S1_ptw_translate
> > >
> > > After migrating to v7.2.0, an issue was found where we were not
> > > getting the correct virtual address from a load insn.  Reading the
> > > address used in the load insn from the debugger resulted in the
> > > execution of the insn getting the correct value but simply stepping
> > > over the
> > insn did not.
> > >
> > > This is the instruction:
> > >
> > > ldr   x0, [x1, #24]
> > >
> > > The debug path varies based on the regime and if regime is NOT stage
> > >two out_phys is set to addr if the regime is stage 2 then out_phys is
> > >set to s2.f.phys_addr.  In the non-debug path out_phys is always set
> > >to full- phys_addr.
> > >
> > > I got around this by only using full->phys_addr if regime_is_stage2
> > > was
> > true:
> > >
> > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> > >
> > > index 3745ac9723..87bc6754a6 100644
> > >
> > > --- a/target/arm/ptw.c
> > >
> > > +++ b/target/arm/ptw.c
> > >
> > > @@ -266,7 +266,12 @@ static bool S1_ptw_translate(CPUARMState
> *env,
> > > S1Translate *ptw,
> > >
> > >   if (unlikely(flags & TLB_INVALID_MASK)) {
> > >
> > >   goto fail;
> > >
> > >   }
> > >
> > > -ptw->out_phys = full->phys_addr;
> > >
> > > +
> > >
> > > +if (regime_is_stage2(s2_mmu_idx)) {
> > >
> > > +ptw->out_phys = full->phys_addr;
> > >
> > > +} else {
> > >
> > > +ptw->out_phys = addr;
> > >
> > > +}
> > >
> > >   ptw->out_rw = full->prot & PAGE_WRITE;
> > >
> > >   pte_attrs = full->pte_attrs;
> > >
> > >   pte_secure = full->attrs.secure;
> > >
> > > This change got me the answer I wanted but I’m not familiar enough
> > > with the code to know if this is correct or not.
> >
> > This is incorrect.  If you are getting the wrong value here, then
> > something has gone wrong elsewhere, as the s2_mmu_idx result was
> logged.
> >
> > Do you have a test case you can share?
> 
> This happens while booting QNX so I can't share it.  I don't have the source
> code either just the object code.  A number of cores are being started and
> the address happens to be what will eventually become the stack.
> 
> I'll see what I can come up with to better characterize is problem.

I have not been able to isolate the cause of this issue.  I have pulled in 
recent updates to ptw.c/cputlb.c/tlb_helper.c to see if one of the recent 
changes would help but they have not.

I'm running the same QNX images between a version of QEMU based on 7.1 and 
another based on 7.2.  QEMU has been patched to allow it to integrate into a 
System-C Virtual Platform but this problem seems to be contained in QEMU.

I defined DEBUG_TLB cputlb.c and set a breakpoint in tlb_add_large_page.  

On 7.1 I see consistent PA to VA mappings:

Thread 10 "vp" hit Breakpoint 1, tlb_add_large_page (env=0xeb3360, mmu_idx=0xa, 
vaddr=0xff809975f000, size=0x1000) at 
../../../../../../src/qemu/accel/tcg/cputlb.c:1079
tlb_set_page_with_attrs: vaddr=ff809975f000 paddr=0x000f35f3a000 prot=3 
idx=10
Thread 13 "vp" hit Breakpoint 1, tlb_add_large_page (env=0xee26e0, mmu_idx=0xa, 
vaddr=0xff809975f000, size=0x1000) at 
../../../../../../src/qemu/accel/tcg/cputlb.c:1079
tlb_set_page_with_attrs: vaddr=ff809975f000 paddr=0x000f35f3a000 prot=3 

RE: ARM: ptw.c:S1_ptw_translate

2023-01-05 Thread Sid Manning
> -Original Message-
> From: Richard Henderson 
> Sent: Wednesday, January 4, 2023 11:42 PM
> To: Sid Manning ; qemu-devel@nongnu.org
> Cc: phi...@linaro.org; Mark Burton 
> Subject: Re: ARM: ptw.c:S1_ptw_translate
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 1/4/23 08:55, Sid Manning wrote:
> > ptw.c:S1_ptw_translate
> >
> > After migrating to v7.2.0, an issue was found where we were not
> > getting the correct virtual address from a load insn.  Reading the
> > address used in the load insn from the debugger resulted in the
> > execution of the insn getting the correct value but simply stepping over the
> insn did not.
> >
> > This is the instruction:
> >
> > ldr   x0, [x1, #24]
> >
> > The debug path varies based on the regime and if regime is NOT stage
> > two out_phys is set to addr if the regime is stage 2 then out_phys is
> > set to s2.f.phys_addr.  In the non-debug path out_phys is always set to 
> > full-
> >phys_addr.
> >
> > I got around this by only using full->phys_addr if regime_is_stage2 was
> true:
> >
> > diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> >
> > index 3745ac9723..87bc6754a6 100644
> >
> > --- a/target/arm/ptw.c
> >
> > +++ b/target/arm/ptw.c
> >
> > @@ -266,7 +266,12 @@ static bool S1_ptw_translate(CPUARMState *env,
> > S1Translate *ptw,
> >
> >   if (unlikely(flags & TLB_INVALID_MASK)) {
> >
> >   goto fail;
> >
> >   }
> >
> > -ptw->out_phys = full->phys_addr;
> >
> > +
> >
> > +if (regime_is_stage2(s2_mmu_idx)) {
> >
> > +ptw->out_phys = full->phys_addr;
> >
> > +} else {
> >
> > +ptw->out_phys = addr;
> >
> > +}
> >
> >   ptw->out_rw = full->prot & PAGE_WRITE;
> >
> >   pte_attrs = full->pte_attrs;
> >
> >   pte_secure = full->attrs.secure;
> >
> > This change got me the answer I wanted but I’m not familiar enough
> > with the code to know if this is correct or not.
> 
> This is incorrect.  If you are getting the wrong value here, then something 
> has
> gone wrong elsewhere, as the s2_mmu_idx result was logged.
> 
> Do you have a test case you can share?

This happens while booting QNX so I can't share it.  I don't have the source 
code either just the object code.  A number of cores are being started and the 
address happens to be what will eventually become the stack.

I'll see what I can come up with to better characterize is problem.

Thanks,
> 
> 
> r~


ARM: ptw.c:S1_ptw_translate

2023-01-04 Thread Sid Manning
ptw.c:S1_ptw_translate

After migrating to v7.2.0, an issue was found where we were not getting the 
correct virtual address from a load insn.  Reading the address used in the load 
insn from the debugger resulted in the execution of the insn getting the 
correct value but simply stepping over the insn did not.

This is the instruction:
ldr   x0, [x1, #24]

The debug path varies based on the regime and if regime is NOT stage two 
out_phys is set to addr if the regime is stage 2 then out_phys is set to 
s2.f.phys_addr.  In the non-debug path out_phys is always set to 
full->phys_addr.

I got around this by only using full->phys_addr if regime_is_stage2 was true:

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 3745ac9723..87bc6754a6 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -266,7 +266,12 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate 
*ptw,
 if (unlikely(flags & TLB_INVALID_MASK)) {
 goto fail;
 }
-ptw->out_phys = full->phys_addr;
+
+if (regime_is_stage2(s2_mmu_idx)) {
+ptw->out_phys = full->phys_addr;
+} else {
+ptw->out_phys = addr;
+}
 ptw->out_rw = full->prot & PAGE_WRITE;
 pte_attrs = full->pte_attrs;
 pte_secure = full->attrs.secure;

This change got me the answer I wanted but I'm not familiar enough with the 
code to know if this is correct or not.



RE: [gdbstub] redirecting qemu console output to a debugger

2021-10-21 Thread Sid Manning
> -Original Message-
> From: Alex Bennée 
> Sent: Thursday, October 21, 2021 9:52 AM
> To: Philippe Mathieu-Daudé 
> Cc: Sid Manning ; Marc-André Lureau
> ; Paolo Bonzini ;
> qemu-devel@nongnu.org
> Subject: Re: [gdbstub] redirecting qemu console output to a debugger
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> Philippe Mathieu-Daudé  writes:
> 
> > Hi Sid,
> >
> > Cc'ing maintainers:
> >
> > $ ./scripts/get_maintainer.pl -f  chardev/char.c "Marc-André Lureau"
> >  (maintainer:chardev) Paolo Bonzini
> >  (reviewer:Character device...)
> >
> > $ ./scripts/get_maintainer.pl -f  gdbstub.c "Alex Bennée"
> >  (maintainer:GDB stub) "Philippe
> > Mathieu-Daudé"  (reviewer:GDB stub)
> >
> > On 10/21/21 14:37, Sid Manning wrote:
> >> Currently when I attach a debugger (lldb) to my qemu session all of the
> output goes to the shell running qemu not to the debugger.  Fixing this
> meant that I needed to point the semi-hosting output to the gdb chardev.  I
> started qemu like this:
> >>
> >> -s -S -semihosting-config target=auto,chardev=ch0 -chardev gdb,id=ch0
> 
> Mixing up semihosting and gdb is not going to end well. We do already re-
> direct semihosting output to the debugger when it's attached. To specify a
> socket for gdb to connect to you need:
> 
>   -chardev socket,path=/tmp/gdb-socket,server=on,wait=off,id=gdb0 -gdb
> chardev:gdb0

Thanks, this is pretty close to what I think needs to happen.  I'm using lldb 
and had a hard time finding the correct connection command.  For the record it 
is: 
(lldb) process connect unix-connect:///tmp/gdb-socket

A remaining issue is activating the gdb character device so the proper protocol 
packet is sent, the 'O' packet.
My command looks like this:

./qemu-system-hexagon -S -display none -monitor none -no-reboot \
-semihosting-config target=gdb,chardev=gdb0 \
-chardev 
socket,path=/tmp/gdb-socket,port=::1234,mux=on,server=on,wait=off,id=gdb0 \
-gdb chardev:gdb0 \
-kernel a.out

I needed to add mux=on to share gdb0.  This does indeed send the output back to 
the debugger, but instead of RSP 'O' packets lldb sees just plane text and 
drops the connection.  I need the cc->chr_write to point to gdb_monitor_write 
as it is it points to mux_chr_write.


> 
> The -chardev specifies the details of the socket and the -gdb tells gdb where
> it should make the gdbserver port visible. The only semihosting-config
> variable you may want to tweak is the target.
> 
> 
> >
> > I'm not sure why "chardev-gdb" is internal, maybe because it uses
> > static state as singleton, so can't be TYPE_USER_CREATABLE?
> >
> >   static GDBState gdbserver_state;
> 
> One good reason - we don't support multiple connections.
> 
> >
> > But TYPE_DBUS_VMSTATE is TYPE_USER_CREATABLE and have:
> >
> > static void
> > dbus_vmstate_complete(UserCreatable *uc, Error **errp) {
> > DBusVMState *self = DBUS_VMSTATE(uc);
> > g_autoptr(GError) err = NULL;
> >
> > if (!object_resolve_path_type("", TYPE_DBUS_VMSTATE, NULL)) {
> > error_setg(errp, "There is already an instance of %s",
> >TYPE_DBUS_VMSTATE);
> > return;
> > }
> > ...
> >
> > So it should be possible to have TYPE_CHARDEV_GDB implement
> > TYPE_USER_CREATABLE and check for singleton the same way, then
> remove
> > the ChardevClass::internal field IMO...
> >
> > But let see what the maintainers prefer :)
> >
> > Regards,
> >
> > Phil.
> 
> 
> --
> Alex Bennée


[gdbstub] redirecting qemu console output to a debugger

2021-10-21 Thread Sid Manning
Currently when I attach a debugger (lldb) to my qemu session all of the output 
goes to the shell running qemu not to the debugger.  Fixing this meant that I 
needed to point the semi-hosting output to the gdb chardev.  I started qemu 
like this:

-s -S -semihosting-config target=auto,chardev=ch0 -chardev gdb,id=ch0

But this failed with the error:
-chardev gdb,id=ch0: 'gdb' is not a valid char driver name

In order to fix this I needed to change the stub's chardev from internal to 
external:

@@ -3446,7 +3446,7 @@ static void char_gdb_class_init(ObjectClass *oc, void 
*data)
 {
 ChardevClass *cc = CHARDEV_CLASS(oc);
 
-cc->internal = true;
+cc->internal = false;
 cc->open = gdb_monitor_open;
 cc->chr_write = gdb_monitor_write;
 }

Afterward console output was routed to the debugger.  This is the only chardev 
device I found that is marked as internal so I suspect this is the wrong thing 
to do.  What is the proper way to redirect output from qemu to the debugger?

Thanks,



RE: [PATCH 01/20] Hexagon HVX (target/hexagon) README

2021-07-19 Thread Sid Manning


> -Original Message-
> From: Brian Cain 
> Sent: Monday, July 19, 2021 8:40 AM
> To: Rob Landley ; Taylor Simpson
> ; qemu-devel@nongnu.org; Sid Manning
> 
> Cc: a...@rev.ng; peter.mayd...@linaro.org; richard.hender...@linaro.org;
> phi...@redhat.com
> Subject: RE: [EXT] Re: [PATCH 01/20] Hexagon HVX (target/hexagon)
> README
> 
> 
> 
> > -Original Message-
> > From: Rob Landley 
> ...
> > On 7/12/21 8:42 AM, Brian Cain wrote:
> ...
> > > and there's also a binary hexagon-linux cross toolchain that we
> > > shared for use by kernel developers.  The hexagon linux toolchain is
> > > built on Ubuntu 16.04.
> >
> > Where's that one?
> 
> https://codelinaro.jfrog.io/artifactory/codelinaro-qemu/2021-05-
> 12/clang+llvm-12.0.0-cross-hexagon-unknown-linux-musl.tar.xz -
>   - Built on Ubuntu 16.04, similar dynamic dependencies as
> releases.llvm.org binaries
>   - Manifest:
>   - llvm+clang 12.0.0 tag
>   - Linux 5.6.18
>   - github.com/qemu/qemu
> 15106f7dc3290ff3254611f265849a314a93eb0e
>   - github.com/quic/musl
> aff74b395fbf59cd7e93b3691905aa1af6c0778c
> 
> 
> > > But when building your toolchain, omitting LLVM_ENABLE_LLD should
> > > work
> > just fine.
> >
> > It did, thanks.
> >
> > Now I'm trying to figure out what all the extra CFLAGS are for.
> 
> +Sid for some perspective on the rationale of these flags.  Some of these
> flags may be workarounds for toolchain issues.
> 
> > The clang_rt build has CMAKE_ASM_FLAGS="-G0 -mlong-calls -fno-pic
> > --target=hexagon-unknown-linux-musl" which
> > https://clang.llvm.org/docs/ClangCommandLineReference.html defines as:
> >
> > -G
> >   Put objects of at most  bytes into small data section (MIPS /
> > Hexagon)
> >
> > -mlong-calls
> >   Generate branches with extended addressability, usually via indirect
> jumps.
> >
> > I don't understand why your libcc build needs no-pic? (Are we only
> > building a static libgcc.a instead of a dynamic one? I'm fine with
> > that if so, but this needs to be specified in the MAKE_ASM_FLAGS why?)
> >
> > Why is it saying --target=hexagon-random-nonsense-musl to a toolchain
> > we built with exactly one target type? How does it NOT default to
> hexagon?
> > (Is this related to the build writing a hexagon-potato-banana-musl.cfg
> > file in the bin directory, meaning the config file is in the $PATH?
> > Does clang only look for it in the same directory the clang executable
> > lives in?)
> >
> > And while we're at it, the CONTENTS of hexagon-gratuitous-gnu-format.cfg
> is:
> >
> > cat < hexagon-unknown-linux-musl.cfg
> > -G0 --sysroot=${HEX_SYSROOT}
> > EOF
> >
> > Which is ALREADY saying -G0? (Why does it want to do that globally?
> > Some sort of bug workaround?) So why do we specify it again here?
> >
> > Next up build_musl_headers does CROSS_CFLAGS="-G0 -O0 -mv65
> > -fno-builtin -fno-rounding-math --target=hexagon-unknown-linux-musl"
> which:

I agree G0 is overplayed here.  G0 is the implied default on Linux.  On 
occasion we use a different configuration of clang where small data (G8) is the 
default so G0 is specified.


> >
> > -O0
> >   disable most of the optimizer

This should be changed.  This was added so I could factor out clang's floating 
point optimizations.  These optimizations caused musl-libc testsuite to fail 
some floating point accuracy tests.  I know at least some of those issues have 
now been resolved.

> >
> > -mv65
> >   -mtune for a specific hexagon generation.
> >   (Why? Does qemu only support that but not newer?)
Passing -mvXX it is now recommended practice.  A few years ago the default arch 
selected changed from the oldest support arch to the newest arch.  QEMU 
supports later architectures.

> >
> > -fno-builtin
> >   musl's ./configure already probes for this and will add it if
> >   the compiler supports it.
I did not know this, we can get rid of -fno-builtin if the driver is meeting 
musl's expectations.


> >
> > -fno-rounding-math
> >   the docs MENTION this, but do not explain it.

This was workaround and is no longer needed.  IIRC clang was asserting while 
building musl.

> >
> > And again with the -G0.
> >
> > These flags probably aren't needed _here_ because this is just the
> > headers install (which is basically a cp -a isn't it?). This looks
> > like it's copied verbatim from the musl library build. But that
> > library build happens in a bit, so relevant-i

RE: linux-user - time64 question

2020-05-27 Thread Sid Manning
> -Original Message-
> From: Laurent Vivier 
> Sent: Wednesday, May 27, 2020 11:23 AM
> To: Sid Manning ; qemu-devel@nongnu.org
> Subject: [EXT] Re: linux-user - time64 question
>
> Le 05/05/2020 à 23:38, Sid Manning a écrit :
> > I’m looking at a testcase failure when my target uses 64bit time in
> > msg.h (struct msqid_ds).  I’ve been able to get around this but
> > changing target_msqid_ds like so:
> >
> >
> > @@ -3900,18 +3901,9 @@ static inline abi_long do_semop(int semid,
> > abi_long ptr, unsigned nsops)
> >  struct target_msqid_ds
> >  {
> >  struct target_ipc_perm msg_perm;
> > -abi_ulong msg_stime;
> > -#if TARGET_ABI_BITS == 32
> > -abi_ulong __unused1;
> > -#endif
> > -abi_ulong msg_rtime;
> > -#if TARGET_ABI_BITS == 32
> > -abi_ulong __unused2;
> > -#endif
> > -abi_ulong msg_ctime;
> > -#if TARGET_ABI_BITS == 32
> > -abi_ulong __unused3;
> > -#endif
> > +abi_ullong msg_stime;
> > +abi_ullong msg_rtime;
> > +abi_ullong msg_ctime;
> >  abi_ulong __msg_cbytes;
> >  abi_ulong msg_qnum;
> >  abi_ulong msg_qbytes;
> >
> > It seems like either should have worked but I get garbage back in some
> > of the elements below msg_time fields without the change.
> >
> > If time_t is 64bits then it seems like stime/rtime/ctime should be
> > abi_ullong.
> >
> > My target is Hexagon and the TARGET_ABI_BITS is 32.
>
> The structure has been changed into the kernel for the y2038 and the change
> has not been reflected into qemu (and it needs).

Thanks for the info.  It was also pointed out to me that my C-library was 
missing  a #define, #define IPC_STAT 0x102
This caused the library not to recognize 64-bit time in some instances.


>
> See kernel commit:
>
> c2ab975c30f0 ("y2038: ipc: Report long times to user space")
>
> Thanks,
> Laurent


linux-user - time64 question

2020-05-05 Thread Sid Manning
I’m looking at a testcase failure when my target uses 64bit time in msg.h 
(struct msqid_ds).  I’ve been able to get around this but changing 
target_msqid_ds like so:

@@ -3900,18 +3901,9 @@ static inline abi_long do_semop(int semid, abi_long ptr,
unsigned nsops)
 struct target_msqid_ds
 {
 struct target_ipc_perm msg_perm;
-abi_ulong msg_stime;
-#if TARGET_ABI_BITS == 32
-abi_ulong __unused1;
-#endif
-abi_ulong msg_rtime;
-#if TARGET_ABI_BITS == 32
-abi_ulong __unused2;
-#endif
-abi_ulong msg_ctime;
-#if TARGET_ABI_BITS == 32
-abi_ulong __unused3;
-#endif
+abi_ullong msg_stime;
+abi_ullong msg_rtime;
+abi_ullong msg_ctime;
 abi_ulong __msg_cbytes;
 abi_ulong msg_qnum;
 abi_ulong msg_qbytes;

It seems like either should have worked but I get garbage back in some of the 
elements below msg_time fields without the change.

If time_t is 64bits then it seems like stime/rtime/ctime should be abi_ullong.

My target is Hexagon and the TARGET_ABI_BITS is 32.

Thanks,