On 7/4/24 20:36, Jean-Philippe Brucker wrote:
> On Mon, Jul 01, 2024 at 11:02:38AM +0000, Mostafa Saleh wrote:
>> Previously, to check if faults are enabled, it was sufficient to check
>> the current stage of translation and check the corresponding
>> record_faults flag.
>>
>> However, with nesting, it is possible for stage-1 (nested) translation
>> to trigger a stage-2 fault, so we check SMMUPTWEventInfo as it would
>> have the correct stage set from the page table walk.
>>
>> Signed-off-by: Mostafa Saleh <smost...@google.com>
>> ---
>> hw/arm/smmuv3.c | 15 ++++++++-------
>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>> index 36eb6f514a..6c18dc0acf 100644
>> --- a/hw/arm/smmuv3.c
>> +++ b/hw/arm/smmuv3.c
>> @@ -34,9 +34,10 @@
>> #include "smmuv3-internal.h"
>> #include "smmu-internal.h"
>>
>> -#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
>> - (cfg)->record_faults : \
>> - (cfg)->s2cfg.record_faults)
>> +#define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1
>> && \
>> + (cfg)->record_faults) || \
>> + ((ptw_info).stage == SMMU_STAGE_2
>> && \
>> + (cfg)->s2cfg.record_faults))
> I guess this could be simplified as "(info.stage == STAGE_1) ? s1cfg : s2cfg"
> Anyway:
>
> Reviewed-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
Reviewed-by: Eric Auger <eric.au...@redhat.com>
Eric
>
>>
>> /**
>> * smmuv3_trigger_irq - pulse @irq if enabled and update
>> @@ -919,7 +920,7 @@ static SMMUTranslationStatus
>> smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
>> event->u.f_walk_eabt.addr2 = ptw_info.addr;
>> break;
>> case SMMU_PTW_ERR_TRANSLATION:
>> - if (PTW_RECORD_FAULT(cfg)) {
>> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
>> event->type = SMMU_EVT_F_TRANSLATION;
>> event->u.f_translation.addr = addr;
>> event->u.f_translation.addr2 = ptw_info.addr;
>> @@ -928,7 +929,7 @@ static SMMUTranslationStatus
>> smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
>> }
>> break;
>> case SMMU_PTW_ERR_ADDR_SIZE:
>> - if (PTW_RECORD_FAULT(cfg)) {
>> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
>> event->type = SMMU_EVT_F_ADDR_SIZE;
>> event->u.f_addr_size.addr = addr;
>> event->u.f_addr_size.addr2 = ptw_info.addr;
>> @@ -937,7 +938,7 @@ static SMMUTranslationStatus
>> smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
>> }
>> break;
>> case SMMU_PTW_ERR_ACCESS:
>> - if (PTW_RECORD_FAULT(cfg)) {
>> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
>> event->type = SMMU_EVT_F_ACCESS;
>> event->u.f_access.addr = addr;
>> event->u.f_access.addr2 = ptw_info.addr;
>> @@ -946,7 +947,7 @@ static SMMUTranslationStatus
>> smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
>> }
>> break;
>> case SMMU_PTW_ERR_PERMISSION:
>> - if (PTW_RECORD_FAULT(cfg)) {
>> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
>> event->type = SMMU_EVT_F_PERMISSION;
>> event->u.f_permission.addr = addr;
>> event->u.f_permission.addr2 = ptw_info.addr;
>> --
>> 2.45.2.803.g4e1b14247a-goog
>>
