On Fri, Aug 26, 2022 at 06:54:16PM +0000, Kalra, Ashish wrote:
> [AMD Official Use Only - General]
>
> Hello Jarkko,
>
> >>
> >> It really should be, in order to have any practical use:
> >>
> >> if (no_iommu) {
> >> pr_err("SEV-SNP: IOMMU is disabled.\n");
> >> return false;
> >> }
> >>
> >> if (iommu_default_passthrough()) {
> >> pr_err("SEV-SNP: IOMMU is configured in passthrough mode.\n");
> >> return false;
> >> }
> >>
> >> The comment is *completely* redundant, it absolutely does not serve
> >> any sane purpose. It just tells what the code already clearly stating.
> >>
> >> The combo error message on the other hand leaves you to the question
> >> "which one was it", and for that reason combining the checks leaves
> >> you to a louse debugging experience.
>
> >Also, are those really *errors*? That implies that there is something wrong.
>
> >Since you can have a legit configuration, IMHO they should be either warn or info. What do you think?
>
> >They are definitely not errors
>
> Yes, they can be warn or info, but as I mentioned above this patch is now part of IOMMU + SNP series,
> so these comments are now relevant for that.
Yeah, warn/info/error is less relevant than the
second point I was making.
It's a good idea to spit out two instead of one
to make best of spitting out anything in the first
place :-) That way you make no mistake interpreting
what does the log message connect to, which can
sometimes make a difference while debugging a
kernel issue.
BR, Jarkko
