Re: [PATCH v6 00/11] x86: fix cpu hotplug with secure boot

[Laszlo Ersek <lersek@redhat.com>]

On 09/23/20 11:46, Igor Mammedov wrote:
> v6:
>   - [9/10] Add comment explaining why while_ctx2 restarts from the last processed CPU.
>   - rebase on top of current master, due to non trivial conflict
>     caused by microvm series, which moved/renamed pc_cpu_pre_plug()

So, I went back to my local branch where I had applied your v5, *plus*
the comment fixup ("[PATCH v5 9/10] fixup! x68: acpi: trigger SMI before
sending hotplug Notify event to OSPM") on top. I rebased that branch to
its *same* base commit, only squashing the comment fixup into patch#9.

Then I applied your v6 series on top of current master, using a
different (new) local branch.

Then I ran git-range-diff on these two local branches.

In patches 6, 7, 8, and 9, you've picked up my feedback tags from the v5
review session; that's good, there was nothing else to do.

There is a trivial difference in patch 2 -- trivial to review, that is;
I'm not saying that it's so trivial that git-rebase should have coped
with it automatically on your end. Here's the git-range-diff output:

>  2:  e606a75432a8 !  2:  94702d2e3125 x86: cpuhp: prevent guest crash on CPU hotplug when broadcast SMI is in use
>     @@ -12,7 +12,7 @@
>          Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Tested-by: Laszlo Ersek <lersek@redhat.com>
>     -    Message-Id: <20200907112348.530921-3-imammedo@redhat.com>
>     +    Message-Id: <20200923094650.1301166-3-imammedo@redhat.com>
>
>      diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
>      --- a/hw/acpi/ich9.c
>     @@ -40,17 +40,17 @@
>
>       void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>
>     -diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>     ---- a/hw/i386/pc.c
>     -+++ b/hw/i386/pc.c
>     +diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>     +--- a/hw/i386/x86.c
>     ++++ b/hw/i386/x86.c
>      @@
>               return;
>           }
>
>     -+    if (pcms->acpi_dev) {
>     ++    if (x86ms->acpi_dev) {
>      +        Error *local_err = NULL;
>      +
>     -+        hotplug_handler_pre_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
>     ++        hotplug_handler_pre_plug(HOTPLUG_HANDLER(x86ms->acpi_dev), dev,
>      +                                 &local_err);
>      +        if (local_err) {
>      +            error_propagate(errp, local_err);

Meaning that, in v6, you had to refer to "x86ms", rather than to "pcms",
and that the code had to be introduced in a different file / function.

The need for that originates from 0cca1a918b85 ("x86: move cpu hotplug
from pc to x86", 2020-09-17).

It looks innocent enough, but I should still retest patch#2. I'll report
back under that patch in this series.

Thanks
Laszlo