Re: [PATCH Part2 v6 02/49] iommu/amd: Introduce function to check SEV-SNP support

[Jarkko Sakkinen]

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