Re: [PATCH for-4.14 2/8] x86/hvm: don't force vCPU 0 for IRQ 0 when using fixed destination mode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.06.2020 16:49, Roger Pau Monné wrote:
> On Thu, Jun 18, 2020 at 04:29:59PM +0200, Jan Beulich wrote:
>> On 18.06.2020 16:18, Roger Pau Monné wrote:
>>> On Thu, Jun 18, 2020 at 04:08:28PM +0200, Jan Beulich wrote:
>>>> On 18.06.2020 15:48, Roger Pau Monné wrote:
>>>>> On Thu, Jun 18, 2020 at 03:43:00PM +0200, Jan Beulich wrote:
>>>>>> On 12.06.2020 17:56, Roger Pau Monne wrote:
>>>>>>> When the IO APIC pin mapped to the ISA IRQ 0 has been configured to
>>>>>>> use fixed delivery mode do not forcefully route interrupts to vCPU 0,
>>>>>>> as the OS might have setup those interrupts to be injected to a
>>>>>>> different vCPU, and injecting to vCPU 0 can cause the OS to miss such
>>>>>>> interrupts or errors to happen due to unexpected vectors being
>>>>>>> injected on vCPU 0.
>>>>>>>
>>>>>>> In order to fix remove such handling altogether for fixed destination
>>>>>>> mode pins and just inject them according to the data setup in the
>>>>>>> IO-APIC entry.
>>>>>>>
>>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>>>>
>>>>>> Technically
>>>>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>>
>>>>>> I wonder though why this was done in the first place - it very much
>>>>>> feels like a workaround for certain guest behavior, and hence
>>>>>> getting rid of it may mean a certain risk of regressions. Not a
>>>>>> very good point in time to make risky changes ...
>>>>>
>>>>> We can defer to after the release I guess, but I will still ask for
>>>>> the changes to be backported.
>>>>
>>>> That's fine, albeit if we decide to delay it until 4.15 was branched,
>>>> then I think we want to also wait longer than usual until it would hit
>>>> the stable trees. Unfortunately c8e79412c001's description is of no
>>>> help to understand what or why "time jumps" may result from delivering
>>>> the interrupt as requested.
>>>
>>> Yes, I've also looked at the original commit and have no idea what it
>>> was actually trying to fix, and why delivering to vCPU 0 fixed it.
>>> FWIW, I tried delivering to a different vCPU and it seems to work
>>> fine.
>>
>> Right, I too was thinking that delivering to a "stable" CPU might be
>> all that's needed. In patch 3 this may then call for latching that
>> CPU, and preferring it over what vlapic_lowest_prio() produces.
> 
> Yes, I also considered that route for the lowest priority mode (which
> is dealt with in the next patch), but for fixed mode we need to
> delivered to the listed vCPUs, there's no trick we can play here
> IMO.

The set may still be empty, in which case the lowest-prio consideration
(of falling back to CPU0) may still apply here as well. But of course
there's nothing to latch here, as fixed mode means multi-cast if more
than one destination matches.

Jan