[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2
From: |
Eric Auger |
Subject: |
Re: [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2 |
Date: |
Tue, 16 May 2023 19:19:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 |
Hi Mostafa,
On 5/15/23 17:37, Mostafa Saleh wrote:
> Hi Eric,
>
> Thanks a lot for taking the time to review the patches.
>
> On Mon, May 15, 2023 at 03:03:28PM +0200, Eric Auger wrote:
>>>
>>> +/* If stage-1 fault enabled and ptw event targets it. */
>>> +#define PTW_FAULT_S1(cfg, event) ((cfg)->record_faults && \
>>> + !(event).u.f_walk_eabt.s2)
>>> +/* If stage-2 fault enabled and ptw event targets it. */
>>> +#define PTW_FAULT_S2(cfg, event) ((cfg)->s2cfg.record_faults &&
>>> \
>>> + (event).u.f_walk_eabt.s2)
>>> +
>>> +#define PTW_FAULT_ALLOWED(cfg, event) (PTW_FAULT_S1(cfg, event) || \
>>> + PTW_FAULT_S2(cfg, event))
>> The name of the macro does not really reflect what it tests. I would
>> suggest PTW_RECORD_FAULT and PTW_RECORD_S1|S2_FAULT
>> I would also suggest (cfg->stage == 1) ? PTW_RECORD_S1_FAULT(cfg,
>> event) : PTW_RECORD_S2_FAULT(cfg, event)
>>
>> PTW_FAULT_S1() and PTW_FAULT_S2() are not used anywhere else.
>>
>> I would simplify PTW_RECORD_FAULT(cfg) (cfg->stage == 1) ?
>> (cfg)->record_faults: (cfg)->s2cfg.record_faults
> Yes, this is much simpler.
> I am wondering as stage-2 only SMMUs can have stage-1 faults as described in
> (IHI 0070.E.a) "3.4 Address sizes".
> If the input address of a transaction exceeds the size of the IAS.
> I guess this means that the fault record in this case is still controlled by
> S2R
> although it is stage-1 fault as there is no CD or stage-1 config.
Yes this sounds sensible.
Eric
>
> Thanks,
> Mostafa
>