Re: [RFC PATCH 10/16] hw/arm/smmuv3: Make TLB lookup work for stage-2

From: Eric Auger <eric.auger@redhat.com>
To: Mostafa Saleh <smostafa@google.com>
Cc: qemu-devel@nongnu.org, jean-philippe@linaro.org,
	peter.maydell@linaro.org, qemu-arm@nongnu.org
Subject: Re: [RFC PATCH 10/16] hw/arm/smmuv3: Make TLB lookup work for stage-2
Date: Thu, 16 Feb 2023 17:52:44 +0100	[thread overview]
Message-ID: <e349bd12-23c9-87bd-f9ba-654c7885489c@redhat.com> (raw)
In-Reply-To: <Y+40glPkaM0aK5C/@google.com>

On 2/16/23 14:49, Mostafa Saleh wrote:
> Hi Eric,
>
> On Thu, Feb 16, 2023 at 12:32:21PM +0100, Eric Auger wrote:
>>>      SMMUTransTableInfo *tt;
>>>      SMMUTransCfg *cfg = NULL;
>>> +    uint8_t granule_sz, tsz;
>>>      IOMMUTLBEntry entry = {
>>>          .target_as = &address_space_memory,
>>>          .iova = addr,
>>> @@ -764,21 +767,40 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>>          goto epilogue;
>>>      }
>>>  
>>> -    tt = select_tt(cfg, addr);
>> maybe we shall adapt select_tt for S2 instead of using temp? I know
>> there is a single range on S2 but well, use tt[0]?
> We can adapt select_tt for S2, but we would need to have an instance
> of SMMUTransTableInfo in SMMUS2Cfg instead of having vttb,tsz..
> inlined, as the function returns a pointer to the SMMUTransTableInfo.
> I don’t think we can reuse tt[0], as this would change when we support
> nesting, so I think we just isolate s1 and s2 from the beginning.

OK fair enough, let's wait for others' feedbakcs then.

Eric
>
>
>>> -    if (!tt) {
>>> -        if (cfg->record_faults) {
>>> -            event.type = SMMU_EVT_F_TRANSLATION;
>>> -            event.u.f_translation.addr = addr;
>>> -            event.u.f_translation.rnw = flag & 0x1;
>>> +    if (STAGE1_SUPPORTED(s->features)) {
>> maybe check the enable state instead.
> Yes, if stage-1 is not enabled, tt might not be valid, I will update it.
>
> Thanks,
> Mostafa
>