Hi Volodymyr,
NIT: s/EL/IL/ in commit title
One remark below.
On 12/02/2025 23:03, Stefano Stabellini wrote:
>
>
> On Wed, 12 Feb 2025, Volodymyr Babchuk wrote:
>> ARM Architecture Reference Manual states that EL field of ESR_EL1
>> register should be 1 when EC is 0b000000 aka HSR_EC_UNKNOWN.
>>
>> Section D24.2.40, page D24-7337 of ARM DDI 0487L:
>>
>> IL, bit [25]
>> Instruction Length for synchronous exceptions. Possible values of this bit
>> are:
>>
>> [...]
>>
>> 0b1 - 32-bit instruction trapped.
>> This value is also used when the exception is one of the following:
>> [...]
>> - An exception reported using EC value 0b000000.
>>
>> To align code with the specification, set .len field to 1 in
>> inject_undef64_exception() and remove unneeded second parameter.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
>
> Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
>
>
>> ---
>> xen/arch/arm/arm64/vsysreg.c | 8 ++++----
>> xen/arch/arm/include/asm/arm64/traps.h | 2 +-
>> xen/arch/arm/include/asm/traps.h | 2 +-
>> xen/arch/arm/p2m.c | 2 +-
>> xen/arch/arm/traps.c | 24 ++++++++++++------------
>> xen/arch/arm/vcpreg.c | 24 ++++++++++++------------
>> xen/arch/arm/vsmc.c | 6 ++----
>> 7 files changed, 33 insertions(+), 35 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
>> index c73b2c95ce..29622a8593 100644
>> --- a/xen/arch/arm/arm64/vsysreg.c
>> +++ b/xen/arch/arm/arm64/vsysreg.c
>> @@ -95,7 +95,7 @@ void do_sysreg(struct cpu_user_regs *regs,
>> */
>> case HSR_SYSREG_ACTLR_EL1:
>> if ( regs_mode_is_user(regs) )
>> - return inject_undef_exception(regs, hsr);
>> + return inject_undef_exception(regs);
>> if ( hsr.sysreg.read )
>> set_user_reg(regs, regidx, v->arch.actlr);
>> break;
>> @@ -267,7 +267,7 @@ void do_sysreg(struct cpu_user_regs *regs,
>> case HSR_SYSREG_CNTP_TVAL_EL0:
>> case HSR_SYSREG_CNTP_CVAL_EL0:
>> if ( !vtimer_emulate(regs, hsr) )
>> - return inject_undef_exception(regs, hsr);
>> + return inject_undef_exception(regs);
>> break;
>>
>> /*
>> @@ -280,7 +280,7 @@ void do_sysreg(struct cpu_user_regs *regs,
>> case HSR_SYSREG_ICC_SGI0R_EL1:
>>
>> if ( !vgic_emulate(regs, hsr) )
>> - return inject_undef64_exception(regs, hsr.len);
>> + return inject_undef64_exception(regs);
>> break;
>>
>> /*
>> @@ -440,7 +440,7 @@ void do_sysreg(struct cpu_user_regs *regs,
>> gdprintk(XENLOG_ERR,
>> "unhandled 64-bit sysreg access %#"PRIregister"\n",
>> hsr.bits & HSR_SYSREG_REGS_MASK);
>> - inject_undef_exception(regs, hsr);
>> + inject_undef_exception(regs);
>> }
>>
>> /*
>> diff --git a/xen/arch/arm/include/asm/arm64/traps.h
>> b/xen/arch/arm/include/asm/arm64/traps.h
>> index a347cb13d6..3be2fa69ee 100644
>> --- a/xen/arch/arm/include/asm/arm64/traps.h
>> +++ b/xen/arch/arm/include/asm/arm64/traps.h
>> @@ -1,7 +1,7 @@
>> #ifndef __ASM_ARM64_TRAPS__
>> #define __ASM_ARM64_TRAPS__
>>
>> -void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
>> +void inject_undef64_exception(struct cpu_user_regs *regs);
>>
>> void do_sysreg(struct cpu_user_regs *regs,
>> const union hsr hsr);
>> diff --git a/xen/arch/arm/include/asm/traps.h
>> b/xen/arch/arm/include/asm/traps.h
>> index 9a60dbf70e..3b40afe262 100644
>> --- a/xen/arch/arm/include/asm/traps.h
>> +++ b/xen/arch/arm/include/asm/traps.h
>> @@ -44,7 +44,7 @@ int check_conditional_instr(struct cpu_user_regs *regs,
>> const union hsr hsr);
>>
>> void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
>>
>> -void inject_undef_exception(struct cpu_user_regs *regs, const union hsr
>> hsr);
>> +void inject_undef_exception(struct cpu_user_regs *regs);
>>
>> /* read as zero and write ignore */
>> void handle_raz_wi(struct cpu_user_regs *regs, int regidx, bool read,
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 65b70955e3..6196cad0d4 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -438,7 +438,7 @@ void p2m_set_way_flush(struct vcpu *v, struct
>> cpu_user_regs *regs,
>> {
>> gprintk(XENLOG_ERR,
>> "The cache should be flushed by VA rather than by
>> set/way.\n");
>> - inject_undef_exception(regs, hsr);
>> + inject_undef_exception(regs);
There's nothing using hsr anymore, so you should drop hsr parameter from
p2m_set_way_flush()
BTW. Are you going to also look at simplifying e.g. inject_iabt_exception() for
which IL is also 1?
~Michal
