Re: [XEN PATCH v3 2/3] xen/drivers/passthrough/arm/smmu-v3.c: fix violations of MISRA C:2012 Rule 3.1

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 29 Jun 2023, Luca Fancellu wrote:
> > On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> wrote:
> > 
> > In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few 
> > occurrences
> 
> here you use a different character to enclose the file path (` vs ‘) may I 
> suggest to
> use only (‘)?
> 
> > of nested '//' character sequences inside C-style comment blocks, which 
> > violate
> > Rule 3.1.
> > 
> > The patch aims to resolve those by replacing the nested comments with
> > equivalent constructs that do not violate the rule.
> > 
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> 
> You are missing the “---“ here, meaning that the lines below are part of the
> commit message and I’m sure you don’t want that.
> 
> Also here, may I suggest to use this commit title instead?
> “xen/arm: smmuv3: Fix violations of MISRA C:2012 Rule 3.1”
> 
> Apart from that, changes looks good to me:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> 
> Will be the maintainer/committer to decide if addressing these comment,
> if accepted, on commit or if you need to send another version, in which
> case you can retain my r-by provided that no other modifications are done.
 
I think it can be done on commit. With the changes suggested by Luca:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>



> > Changes:
> > - Resending the patch with the right maintainers in CC.
> > Changes in V2:
> > - Split the patch into a series and reworked the fix.
> > - Apply the fix to the arm32 `flushtlb.h' file, for consistency
> > Changes in V3:
> > - Revised the comment to make it clear the function the parallel control
> > flows in the comment belong to.
> > ---
> > xen/drivers/passthrough/arm/smmu-v3.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> > b/xen/drivers/passthrough/arm/smmu-v3.c
> > index 720aa69ff2..cdbb505134 100644
> > --- a/xen/drivers/passthrough/arm/smmu-v3.c
> > +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> > @@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct 
> > arm_smmu_domain *smmu_domain,
> > * before we read 'nr_ats_masters' in case of a concurrent call to
> > * arm_smmu_enable_ats():
> > *
> > - * // unmap() // arm_smmu_enable_ats()
> > - * TLBI+SYNC atomic_inc(&nr_ats_masters);
> > - * smp_mb(); [...]
> > - * atomic_read(&nr_ats_masters); pci_enable_ats() // writel()
> > + * --- unmap() ---                 --- arm_smmu_enable_ats() ---
> > + * TLBI+SYNC                       atomic_inc(&nr_ats_masters);
> > + * smp_mb();                       [...]
> > + * atomic_read(&nr_ats_masters);   pci_enable_ats() (see writel())
> > *
> > * Ensures that we always see the incremented 'nr_ats_masters' count if
> > * ATS was enabled at the PCI device before completion of the TLBI.
> > -- 
> > 2.34.1
> > 
> > 
> 
>