Re: [PATCH v4 1/3] iommu/arm-smmu-v3: Introduce struct arm_smmu_event

[Pranjal Shrivastava <praan@google.com>]

On Wed, Nov 06, 2024 at 04:16:19PM -0800, Daniel Mentz wrote:
> On Fri, Oct 18, 2024 at 11:00 AM Pranjal Shrivastava <praan@google.com> wrote:
> >
> > Introduce `struct arm_smmu_event` to represent event records.
> > Parse out relevant fields from raw event records for ease and
> > use the new `struct arm_smmu_event` instead.
> >
> > Signed-off-by: Daniel Mentz <danielmentz@google.com>
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> 
> > +struct arm_smmu_event {
> > +       struct arm_smmu_device          *smmu;
> > +       u8                              id;
> > +       u8                              class;
> > +       u16                             stag;
> > +       u32                             sid;
> > +       u32                             ssid;
> > +       u64                             iova;
> > +       u64                             ipa;
> > +       u64                             raw[EVTQ_ENT_DWORDS];
> 
> Consider removing the member named raw from struct arm_smmu_event.
> Compare this with struct arm_smmu_cmdq_ent and
> arm_smmu_cmdq_build_cmd() which keep the encoded and decoded versions
> separate.

I had a similar implemntation in v3 [1] but it was decided [2]
to keep the "raw" event array within arm_smmu_event itself. Since
otherwise we'd have two variables, one pointing to the other when they
have the exact same scope and lifetime anyway.

Do we have a strong preference here?

> 
> > +       bool                            stall;
> > +       bool                            ssid_valid;
> > +       bool                            privileged;
> > +       bool                            instruction;
> > +       bool                            s2;
> > +       bool                            read;
> > +};

Thanks,
Praan

[1] https://lore.kernel.org/all/20240928005143.2378938-2-praan@google.com/
[2] https://lore.kernel.org/all/5eda3ba6-c35a-432b-be87-48bd8a0a3bf1@arm.com/