Re: [PATCH V2 1/1] hw/intc/arm_gicv3_kvm: preserve pending interrupts during cpr

[Fabiano Rosas <farosas@suse.de>]

Steven Sistare <steven.sistare@oracle.com> writes:

> On 8/4/2025 10:47 AM, Peter Maydell wrote:
>> On Mon, 4 Aug 2025 at 15:23, Steven Sistare <steven.sistare@oracle.com> wrote:
>>>
>>> Fabiano, could you sanity check this patch? Thanks!
>>>
>>> Peter, more below:
>>>
>>> On 7/21/2025 6:24 AM, Peter Maydell wrote:
>>>>> @@ -883,13 +895,17 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>>>>>                                   GICD_CTLR)) {
>>>>>            error_setg(&s->migration_blocker, "This operating system kernel does "
>>>>>                                              "not support vGICv3 migration");
>>>>> -        if (migrate_add_blocker(&s->migration_blocker, errp) < 0) {
>>>>> +        if (migrate_add_blocker_modes(&s->migration_blocker, MIG_MODE_NORMAL,
>>>>> +                                      MIG_MODE_CPR_TRANSFER, errp) < 0) {
>>>>
>>>> Why did you change this? It's the general "if no support, can't
>>>> migrate at all" check, which seems unrelated to cpr-transfer.
>>>
>>> "If no support", then cpr-transfer should also be blocked.
>> 
>> But migrate_add_blocker() is a wrapper for
>> migrate_add_blocker_modes(..., MIG_MODE_ALL). So doesn't
>> this change go from "block migration for normal, and cpr-transfer,
>> and everything else" to "block migration for normal and
>> cpr-transfer but let the rest through"?
>> 
>> That doesn't seem like the right thing; if it *is* the right
>> thing then it should be a separate patch with a commit message
>> that explains why we do it; and we would probably want to
>> audit all the other uses of plain migrate_add_blocker() and/or
>> change that function's name or API...
>
> Yup, my bad. I will revert this hunk.
>

With that,

Reviewed-by: Fabiano Rosas <farosas@suse.de>