On Wed, 19 Jul 2023 at 16:56, Jean-Philippe Brucker
<jean-phili...@linaro.org> wrote:
>
> GPC checks are not performed on the output address for AT instructions,
> as stated by ARM DDI 0487J in D8.12.2:
>
> When populating PAR_EL1 with the result of an address translation
> instruction, granule protection checks are not performed on the final
> output address of a successful translation.
>
> Rename get_phys_addr_with_secure(), since it's only used to handle AT
> instructions.
>
> Signed-off-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
> ---
> This incidentally fixes a problem with AT S1E1 instructions which can
> output an IPA and should definitely not cause a GPC.
> ---
> target/arm/internals.h | 25 ++++++++++++++-----------
> target/arm/helper.c | 8 ++++++--
> target/arm/ptw.c | 11 ++++++-----
> 3 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 0f01bc32a8..fc90c364f7 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1190,12 +1190,11 @@ typedef struct GetPhysAddrResult {
> } GetPhysAddrResult;
>
> /**
> - * get_phys_addr_with_secure: get the physical address for a virtual address
> + * get_phys_addr: get the physical address for a virtual address
> * @env: CPUARMState
> * @address: virtual address to get physical address for
> * @access_type: 0 for read, 1 for write, 2 for execute
> * @mmu_idx: MMU index indicating required translation regime
> - * @is_secure: security state for the access
> * @result: set on translation success.
> * @fi: set to fault info if the translation fails
> *
> @@ -1212,26 +1211,30 @@ typedef struct GetPhysAddrResult {
> * * for PSMAv5 based systems we don't bother to return a full FSR format
> * value.
> */
> -bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
> - MMUAccessType access_type,
> - ARMMMUIdx mmu_idx, bool is_secure,
> - GetPhysAddrResult *result, ARMMMUFaultInfo
> *fi)
> +bool get_phys_addr(CPUARMState *env, target_ulong address,
> + MMUAccessType access_type, ARMMMUIdx mmu_idx,
> + GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
> __attribute__((nonnull));
What is going on in this bit of the patch ? We already
have a prototype for get_phys_addr() with a doc comment.
Is this git's diff-output producing something silly
for a change that is not logically touching get_phys_addr()'s
prototype and comment at all?
> /**
> - * get_phys_addr: get the physical address for a virtual address
> + * get_phys_addr_with_secure_nogpc: get the physical address for a virtual
> + * address
> * @env: CPUARMState
> * @address: virtual address to get physical address for
> * @access_type: 0 for read, 1 for write, 2 for execute
> * @mmu_idx: MMU index indicating required translation regime
> + * @is_secure: security state for the access
> * @result: set on translation success.
> * @fi: set to fault info if the translation fails
> *
> - * Similarly, but use the security regime of @mmu_idx.
> + * Similar to get_phys_addr, but use the given security regime and don't
> perform
> + * a Granule Protection Check on the resulting address.
> */
> -bool get_phys_addr(CPUARMState *env, target_ulong address,
> - MMUAccessType access_type, ARMMMUIdx mmu_idx,
> - GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
> +bool get_phys_addr_with_secure_nogpc(CPUARMState *env, target_ulong address,
> + MMUAccessType access_type,
> + ARMMMUIdx mmu_idx, bool is_secure,
> + GetPhysAddrResult *result,
> + ARMMMUFaultInfo *fi)
> __attribute__((nonnull));
thanks
-- PMM
