On 7 March 2018 at 16:23, Auger Eric <eric.au...@redhat.com> wrote:
> Hi Peter,
>
> On 06/03/18 20:43, Peter Maydell wrote:
>> On 17 February 2018 at 18:46, Eric Auger <eric.au...@redhat.com> wrote:
>>> +int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
>>> + IOMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
>>> +{
>>> + if (!cfg->aa64) {
>>> + error_setg(&error_fatal,
>>> + "SMMUv3 model does not support VMSAv8-32 page walk
>>> yet");
>>
>> This sort of guest-provokable error should not be fatal -- log
>> it with LOG_UNIMP and carry on as best you can (in this case
>> the spec should say what happens for SMMUv3 implementations which
>> don't support AArch32 tables).
> This code should never been entered as the check is done when decoding
> the CD in the smmuv3. However since it is a base class I wanted to
> enphase the ptw was only supporting AArch32.
Ah, right. That should be an assert() with a brief comment
about why the condition will never trigger.
>>> +#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.
>>> +
>>> +#define PTE_AP_TO_PERM(ap) \
>>> + (IOMMU_ACCESS_FLAG(true, !((ap) & 0x2)))
>>
>> Similarly here.
> ?
Can't just ignore AP bit 1 (or 0, if you're counting it that way).
thanks
-- PMM
