* [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional
@ 2022-10-07 15:21 Richard Henderson
2022-10-07 16:20 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2022-10-07 15:21 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
While the stage2 call to get_phys_addr_lpae should never set
attrs.secure when given a non-secure input, it's just as easy
to make the final update to attrs.secure be unconditional and
false in the case of non-secure input.
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Hi Peter,
This is the promised patch 1.5 for v3 FEAT_HAFDBS. It generates minor
conflicts down the line, which I have already fixed up locally. I think
the first one you would encounter is beyond the proposed 20 that you
indicated that you intend to take into target-arm.next right now.
r~
---
target/arm/ptw.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index b8c494ad9f..7d763a5847 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2365,17 +2365,16 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
result->cacheattrs = combine_cacheattrs(env, cacheattrs1,
result->cacheattrs);
- /* Check if IPA translates to secure or non-secure PA space. */
- if (is_secure) {
- if (ipa_secure) {
- result->attrs.secure =
- !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
- } else {
- result->attrs.secure =
- !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
- || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
- }
- }
+ /*
+ * Check if IPA translates to secure or non-secure PA space.
+ * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
+ */
+ result->attrs.secure =
+ (is_secure
+ && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))
+ && (ipa_secure
+ || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
+
return 0;
} else {
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional
2022-10-07 15:21 [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional Richard Henderson
@ 2022-10-07 16:20 ` Peter Maydell
2022-10-07 16:53 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2022-10-07 16:20 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, 7 Oct 2022 at 16:22, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> While the stage2 call to get_phys_addr_lpae should never set
> attrs.secure when given a non-secure input, it's just as easy
> to make the final update to attrs.secure be unconditional and
> false in the case of non-secure input.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Hi Peter,
>
> This is the promised patch 1.5 for v3 FEAT_HAFDBS. It generates minor
> conflicts down the line, which I have already fixed up locally. I think
> the first one you would encounter is beyond the proposed 20 that you
> indicated that you intend to take into target-arm.next right now.
>
>
> r~
>
> ---
> target/arm/ptw.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index b8c494ad9f..7d763a5847 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2365,17 +2365,16 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
> result->cacheattrs = combine_cacheattrs(env, cacheattrs1,
> result->cacheattrs);
>
> - /* Check if IPA translates to secure or non-secure PA space. */
> - if (is_secure) {
> - if (ipa_secure) {
> - result->attrs.secure =
> - !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
> - } else {
> - result->attrs.secure =
> - !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
> - || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
> - }
> - }
If:
is_secure == true
ipa_secure == false
(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW) is non-zero
(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW) is zero
then the old code sets attrs.secure to true...
> + /*
> + * Check if IPA translates to secure or non-secure PA space.
> + * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
> + */
> + result->attrs.secure =
> + (is_secure
> + && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))
> + && (ipa_secure
> + || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
...but the new code will set it to false, I think ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional
2022-10-07 16:20 ` Peter Maydell
@ 2022-10-07 16:53 ` Richard Henderson
2022-10-10 11:59 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2022-10-07 16:53 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel
On 10/7/22 09:20, Peter Maydell wrote:
>> - /* Check if IPA translates to secure or non-secure PA space. */
>> - if (is_secure) {
>> - if (ipa_secure) {
>> - result->attrs.secure =
>> - !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
>> - } else {
>> - result->attrs.secure =
>> - !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
>> - || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
>> - }
>> - }
>
> If:
> is_secure == true
> ipa_secure == false
> (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW) is non-zero
> (env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW) is zero
> then the old code sets attrs.secure to true...
No, I think the misalignment of the two lines wrt the !() may have been confusing:
if (true) {
if (false) {
} else {
secure = !((0) || (non-zero))
= !(1)
= 0
}
}
r~
>
>> + /*
>> + * Check if IPA translates to secure or non-secure PA space.
>> + * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
>> + */
>> + result->attrs.secure =
>> + (is_secure
>> + && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))
>> + && (ipa_secure
>> + || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
>
> ...but the new code will set it to false, I think ?
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional
2022-10-07 16:53 ` Richard Henderson
@ 2022-10-10 11:59 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2022-10-10 11:59 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, 7 Oct 2022 at 17:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/7/22 09:20, Peter Maydell wrote:
> >> - /* Check if IPA translates to secure or non-secure PA space. */
> >> - if (is_secure) {
> >> - if (ipa_secure) {
> >> - result->attrs.secure =
> >> - !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
> >> - } else {
> >> - result->attrs.secure =
> >> - !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
> >> - || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
> >> - }
> >> - }
> >
> > If:
> > is_secure == true
> > ipa_secure == false
> > (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW) is non-zero
> > (env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW) is zero
> > then the old code sets attrs.secure to true...
>
> No, I think the misalignment of the two lines wrt the !() may have been confusing:
Ah, yes -- I was indeed confused by the misalignment.
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-10 12:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-07 15:21 [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional Richard Henderson
2022-10-07 16:20 ` Peter Maydell
2022-10-07 16:53 ` Richard Henderson
2022-10-10 11:59 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.