Re: [PATCH 3/7] target/hppa: fix access_id check

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/17/24 12:14, Sven Schnelle wrote:
>>          /* If bits [31:1] match, and bit 0 is set, suppress write.  */
>> -        int match = ent->access_id * 2 + 1;
>> -
>> -        if (match == env->cr[CR_PID1] || match == env->cr[CR_PID2] ||
>> -            match == env->cr[CR_PID3] || match == env->cr[CR_PID4]) {
>> -            prot &= PAGE_READ | PAGE_EXEC;
>> -            if (type == PAGE_WRITE) {
>> -                ret = EXCP_DMPI;
>> -                goto egress;
>> +        uint32_t pid;
>> +        if (match_prot_id(env, ent->access_id, &pid)) {
>> +            if ((pid & 1) && (prot & PROT_WRITE)) {
>> +                prot &= ~PROT_WRITE;
>>              }
>> +        } else {
>> +            prot = 0;
>>          }
>
> You're losing the data memory protection id trap.

Oops, indeed.

> Therefore I suggest
> [..]
> At this point there are now a couple of hppa_is_pa20() calls within
> hppa_get_physical_address, which could be unified to a single local
> bool.

Thanks, i'll take your version and update the patch.