On Fri, Aug 26, 2022 at 06:54:16PM +0000, Kalra, Ashish wrote:

[AMD Official Use Only - General]

Hello Jarkko,

quoted

quoted

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.

quoted

Also, are those really *errors*? That implies that there is something wrong.

quoted

Since you can have a legit configuration, IMHO they should be either warn or info. What do you think?

quoted

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