Re: [PATCH v2 001/281] target/i386: Add support for save/load of exception error code

["Alex Bennée" <alex.bennee@linaro.org>]

Igor Mammedov <imammedo@redhat.com> writes:

(trim the CC list somewhat)

> On Thu,  4 Sep 2025 09:06:35 +0100
> Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> From: Xin Wang <wangxinxin.wang@huawei.com>
>> 
>> For now, qemu save/load CPU exception info(such as exception_nr and
>> has_error_code), while the exception error_code is ignored. This will
>> cause the dest hypervisor reinject a vCPU exception with error_code(0),
>> potentially causing a guest kernel panic.
>> 
>> For instance, if src VM stopped with an user-mode write #PF (error_code 6),
>> the dest hypervisor will reinject an #PF with error_code(0) when vCPU resume,
>> then guest kernel panic as:
>>   BUG: unable to handle page fault for address: 00007f80319cb010
>>   #PF: supervisor read access in user mode
>>   #PF: error_code(0x0000) - not-present page
>>   RIP: 0033:0x40115d
>> 
>> To fix it, support save/load exception error_code.
>
> this potentially will break migration between new/old QEMU versions
> due to presence new subsection. But then according to commit message
> the guest might panic (on dst) when resumed anyways.
>
> So patch changes how guest will fail
> (panic: old => old, old => new
>  vs migration error: new => old ).
>
> Peter,
> do we care and do we need a compat knob to make existing
> machine type behave old way?

Igor,

So this patch is already in master, it was only posted in this series
because my master is never upto date with origin/master.

Apologies again for the noise.

>
>> 
>> Signed-off-by: Xin Wang <wangxinxin.wang@huawei.com>
>> Link: https://lore.kernel.org/r/20250819145834.3998-1-wangxinxin.wang@huawei.com
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  target/i386/machine.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>> 
>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> index dd2dac1d443..45b7cea80aa 100644
>> --- a/target/i386/machine.c
>> +++ b/target/i386/machine.c
>> @@ -462,6 +462,24 @@ static const VMStateDescription vmstate_exception_info = {
>>      }
>>  };
>>  
>> +static bool cpu_errcode_needed(void *opaque)
>> +{
>> +    X86CPU *cpu = opaque;
>> +
>> +    return cpu->env.has_error_code != 0;
>> +}
>> +
>> +static const VMStateDescription vmstate_error_code = {
>> +    .name = "cpu/error_code",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = cpu_errcode_needed,
>> +    .fields = (const VMStateField[]) {
>> +        VMSTATE_INT32(env.error_code, X86CPU),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>  /* Poll control MSR enabled by default */
>>  static bool poll_control_msr_needed(void *opaque)
>>  {
>> @@ -1746,6 +1764,7 @@ const VMStateDescription vmstate_x86_cpu = {
>>      },
>>      .subsections = (const VMStateDescription * const []) {
>>          &vmstate_exception_info,
>> +        &vmstate_error_code,
>>          &vmstate_async_pf_msr,
>>          &vmstate_async_pf_int_msr,
>>          &vmstate_pv_eoi_msr,

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro