Re: [PATCH v12 2/3] x86/sev: Change snp_guest_issue_request's fw_err

From: Borislav Petkov
Date: Tue Jan 24 2023 - 08:54:41 EST


On Mon, Jan 23, 2023 at 01:22:07PM -0800, Dionna Amalie Glaze wrote:
> This isn't the primary problem that needs fixing, although it is part
> of it.

I'm replying to the 2/3 patch which is addressing this part.

> The problem is that the host can provide a throttling error and
> the guest will need to continue trying the exact same request or else
> end up locking themself out of the vmpck due to the IV reuse patch
> Peter sent.
>
> I think Sean's request to keep throttling a host problem in user space

+ Sean.

> is not the right one in this case. That would avoid scheduling the
> whole vCPU, but the guest code I'm proposing can do other useful work
> while waiting. There will be no other code that depends on that
> particular control flow.

I know there's another issue but my angle is this:

You have a patch with Fixes tags and they all are fixing some shortcomings. Now,
those are good candidates for stable.

In order to be backportable to stable, those fixes should be as minimal as
possible so that a stable backport does not turn into a nightmare. By the looks
of it, your patches could be simplified to the bare minimum fixes. Cleanups and
improvements can go ontop.

That is, provided we want them in stable and by the looks of it, we probably do.

Which then means, you could do the minimal fixes first and then the cleanups
ontop.

Then, please try to structure your commit messages in a format close to:

Problem is A.

It happens because of B.

Fix it by doing C.

(Potentially do D).

I'm having hard time parsing

https://lore.kernel.org/all/20230120214857.835931-4-dionnaglaze@xxxxxxxxxx/

Only in the second paragraph it is talking about what the problem is. And by the
looks of it, if the host throttles, then the guest should not return from that
request.

And adding module params for this are most of the time the wrong solution
because then people would have to *know* how to use them.

And before we add module params, the kernel should try to solve this without any
user interaction, if possible.

But I'm not 100% clear on what exactly we're fixing here so let's start first,
please, with a proper description of what the exact issue is and how it happens.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette