On Thu, Jul 20, 2023 at 05:39:56PM +0100, Peter Maydell wrote: > 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?
I swapped the two prototypes in order to make the new comment for get_phys_addr_with_secure_nogpc() more clear. Tried to convey that get_phys_addr() is the normal function and get_phys_addr_with_secure_nogpc() is special. So git is working as expected and this is a style change, I can switch them back if you prefer. Thanks, Jean > > > /** > > - * 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