Re: [Qemu-devel] [PATCH v9 03/14] hw/arm/smmu-common: VMSAv8-64 page table walk

[Peter Maydell]

On 8 March 2018 at 18:56, Auger Eric <address@hidden> wrote:
> Hi Peter,
> On 07/03/18 17:35, Peter Maydell wrote:
>> On 7 March 2018 at 16:23, Auger Eric <address@hidden> wrote:
>>> Hi Peter,
>>>
>>> On 06/03/18 20:43, Peter Maydell wrote:
>>>> On 17 February 2018 at 18:46, Eric Auger <address@hidden> wrote:
>>>>> +#define is_permission_fault(ap, perm) \
>>>>> +    (((perm) & IOMMU_WO) && ((ap) & 0x2))
>>>>
>>>> Don't we also need to check AP bit 1 in some cases?
>>>> (when the StreamWorld is S or NS EL1 and either (a) the incoming
>>>> transaction has its attrs.user = 1 and STE.PRIVCFG is 0b0x, or
>>>> (b) STE.PRIVCFG is 0b10).
>>> I think I don't need to as I don't support this feature at the moment:
>>> spec says:
>>> "When SMMU_IDR1.ATTR_PERMS_OVR=0, this field is RES0 and the incoming
>>> PRIV attribute is used."
>>> But to be honest I was not aware this existed ;()
>>
>> I think you still need to check the incoming transaction
>> for user vs priv, even if you don't support STE.PRIVCFG.
>
> On the CPU side, you have MemTxAttrs as input from get_phys_addr_lpae().
>
> On IOMMU side, the current input callback for translation is
>
> static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr
> addr, IOMMUAccessFlags flag)
>
> where IOMMUAccessFlags just is R/W access flag.
>
> So I am not sure I have acess to those user/priv attributes.

Hmm, yes. This looks like a deficiency in our IOMMU framework.
For the moment put a TODO note that we treat all transactions
as privileged because QEMU's IOMMU code doesn't pass transaction
attributes around correctly.

(This will also be an issue for secure/nonsecure eventually.)

thanks
-- PMM