Re: [Xen-devel] [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
On Sat, 2015-04-25 at 22:16 +0500, Julien Grall wrote: Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: Move last paramter of a handle_ro_raz call to next patch where it belongs. --- xen/arch/arm/traps.c | 52 -- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 8b1846a..b54aef6 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs, advance_pc(regs, hsr); } +/* Write only + write ignore */ [..] +/* Read only + read as zero */ I'm not sure if we finished the discussion on those comment on v1 before you sent the v2. I think we hadn't. The + is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ). I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0. For v3 I've made the change I proposed in 1429266891.25195.260.ca...@citrix.com Specifically Write only as write ignore and Read only as read as zero (essentially s/+/as/) Is that clear enough do you think? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
Hi Ian, On 08/05/15 11:16, Ian Campbell wrote: On Sat, 2015-04-25 at 22:16 +0500, Julien Grall wrote: Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: Move last paramter of a handle_ro_raz call to next patch where it belongs. --- xen/arch/arm/traps.c | 52 -- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 8b1846a..b54aef6 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs, advance_pc(regs, hsr); } +/* Write only + write ignore */ [..] +/* Read only + read as zero */ I'm not sure if we finished the discussion on those comment on v1 before you sent the v2. I think we hadn't. The + is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ). I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0. For v3 I've made the change I proposed in 1429266891.25195.260.ca...@citrix.com Specifically Write only as write ignore and Read only as read as zero (essentially s/+/as/) Is that clear enough do you think? Yes. Thanks. With this changes: Reviewed-by: Julien Grall julien.gr...@citrix.com Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: Move last paramter of a handle_ro_raz call to next patch where it belongs. --- xen/arch/arm/traps.c | 52 -- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 8b1846a..b54aef6 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs, advance_pc(regs, hsr); } +/* Write only + write ignore */ [..] +/* Read only + read as zero */ I'm not sure if we finished the discussion on those comment on v1 before you sent the v2. The + is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ). I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: Move last paramter of a handle_ro_raz call to next patch where it belongs. --- xen/arch/arm/traps.c | 52 -- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 8b1846a..b54aef6 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs, advance_pc(regs, hsr); } +/* Write only + write ignore */ +static void handle_wo_wi(struct cpu_user_regs *regs, + register_t *reg, + bool_t read, + const union hsr hsr) +{ +if ( read ) +return inject_undef_exception(regs, hsr); +/* else: ignore */ + +advance_pc(regs, hsr); +} + +/* Read only + read as zero */ +static void handle_ro_raz(struct cpu_user_regs *regs, + register_t *reg, + bool_t read, + const union hsr hsr) +{ +if ( !read ) +return inject_undef_exception(regs, hsr); +/* else: raz */ + +*reg = 0; + +advance_pc(regs, hsr); +} + static void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr) { @@ -1737,11 +1765,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis * is set to 0, which we emulated below. */ -if ( !cp32.read ) -return inject_undef_exception(regs, hsr); - -*r = 0; -break; +return handle_ro_raz(regs, r, cp32.read, hsr); case HSR_CPREG32(DBGDSCREXT): if ( usr_mode(regs) ) @@ -1768,11 +1792,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) case HSR_CPREG32(DBGOSLAR): if ( usr_mode(regs) ) return inject_undef_exception(regs, hsr); -/* WO */ -if ( cp32.read ) -return inject_undef_exception(regs, hsr); -/* else: ignore */ -break; +return handle_wo_wi(regs, r, cp32.read, hsr); default: gdprintk(XENLOG_ERR, %s p14, %d, r%d, cr%d, cr%d, %d @ 0x%PRIregister\n, @@ -1857,11 +1877,7 @@ static void do_sysreg(struct cpu_user_regs *regs, * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that * register as RAZ/WI above. So RO at both EL0 and EL1. */ -if ( !hsr.sysreg.read ) -return inject_undef_exception(regs, hsr); - -*x = 0; -break; +return handle_ro_raz(regs, x, hsr.sysreg.read, hsr); /* - Perf monitors */ case HSR_SYSREG_PMUSERENR_EL0: @@ -1891,10 +1907,8 @@ static void do_sysreg(struct cpu_user_regs *regs, /* Write only, Write ignore registers: */ case HSR_SYSREG_OSLAR_EL1: -if ( hsr.sysreg.read ) -return inject_undef_exception(regs, hsr); -/* else: write ignored */ -break; +return handle_wo_wi(regs, x, hsr.sysreg.read, hsr); + case HSR_SYSREG_CNTP_CTL_EL0: case HSR_SYSREG_CNTP_TVAL_EL0: case HSR_SYSREG_CNTP_CVAL_EL0: -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel