[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: |
Mostafa Saleh |
Subject: |
Re: [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2 |
Date: |
Mon, 15 May 2023 15:37:55 +0000 |
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.
Thanks,
Mostafa