RE: [PATCH] Hexagon: add PC alignment check and exception

[<ltaylorsimpson@gmail.com>]

PS  You should also update the pkt_raises_exception function in translate.c
to return true for packets that contain these instructions.  This will
ensure that none of the machine state is changed before the check is
complete.

Taylor


> -----Original Message-----
> From: ltaylorsimpson@gmail.com <ltaylorsimpson@gmail.com>
> Sent: Monday, April 29, 2024 9:41 AM
> To: 'Matheus Tavares Bernardino' <quic_mathbern@quicinc.com>; qemu-
> devel@nongnu.org
> Cc: bcain@quicinc.com; sidneym@quicinc.com; ale@rev.ng; anjo@rev.ng
> Subject: RE: [PATCH] Hexagon: add PC alignment check and exception
> 
> 
> 
> > -----Original Message-----
> > From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> > Sent: Friday, April 26, 2024 1:16 PM
> > To: qemu-devel@nongnu.org
> > Cc: bcain@quicinc.com; sidneym@quicinc.com; ale@rev.ng; anjo@rev.ng;
> > ltaylorsimpson@gmail.com
> > Subject: [PATCH] Hexagon: add PC alignment check and exception
> >
> > The Hexagon Programmer's Reference Manual says that the exception
> 0x1e
> > should be raised upon an unaligned program counter. Let's implement
> > that and also add tests for both the most common case as well as
> > packets with multiple change-of-flow instructions.
> >
> > Signed-off-by: Matheus Tavares Bernardino
> <quic_mathbern@quicinc.com>
> > ---
> 
> 
> > --- a/target/hexagon/genptr.c
> > +++ b/target/hexagon/genptr.c
> > @@ -473,6 +473,7 @@ static void gen_write_new_pc_addr(DisasContext
> 
> You haven't added the check to gen_write_new_pc_pcrel.  It's not needed
> there because the encoding guarantees the target is always aligned -
right?
> However, there is a call to gen_write_new_pc_addr inside that function.
In
> this case, we'll add a check that isn't necessary.  Consider adding a
parameter
> to indicate if the check can be avoided.
> 
> 
> > a/tests/tcg/hexagon/Makefile.target
> > b/tests/tcg/hexagon/Makefile.target
> > index f839b2c0d5..02d7fff34c 100644
> > --- a/tests/tcg/hexagon/Makefile.target
> > +++ b/tests/tcg/hexagon/Makefile.target
> > @@ -51,6 +51,19 @@ HEX_TESTS += scatter_gather  HEX_TESTS +=
> hvx_misc
> > HEX_TESTS += hvx_histogram  HEX_TESTS += invalid-slots
> > +HEX_TESTS += unaligned_pc
> > +HEX_TESTS += unaligned_pc_multi_cof
> > +
> > +run-unaligned_pc: unaligned_pc
> > +run-unaligned_pc_multi_cof: unaligned_pc_multi_cof run-unaligned_pc
> > +run-unaligned_pc_multi_cof:
> > +	$(call run-test, $<, $(QEMU) $< 2> $<.stderr,"$< on
> > $(TARGET_NAME)"); \
> > +	if [ $$? -ne 1 ] ; then \
> > +		return 1; \
> > +	fi
> > +	$(call quiet-command, \
> > +		grep -q "exception 0x1e" $<.stderr, \
> > +		"GREP", "exception 0x1e");
> 
> We should also test endloop instructions.
> 
> Thanks,
> Taylor
>