Re: [RFC PATCH] KVM: arm/arm64: vgic: change condition for level interrupt resampling

[Marc Zyngier <marc.zyngier@arm.com>]

On 12/03/18 02:33, Yang, Shunyong wrote:
> Hi, Marc,
> 
> On Sun, 2018-03-11 at 12:17 +0000, Marc Zyngier wrote:
>> On Sun, 11 Mar 2018 01:55:08 +0000
>> Christoffer Dall <cdall@kernel.org> wrote:
>>
>>>
>>> On Sat, Mar 10, 2018 at 12:20 PM, Marc Zyngier <marc.zyngier@arm.co
>>> m> wrote:
>>>>
>>>> On Fri, 09 Mar 2018 21:36:12 +0000,
>>>> Christoffer Dall wrote:  
>>>>>
>>>>>
>>>>> On Thu, Mar 08, 2018 at 05:28:44PM +0000, Marc Zyngier wrote:  
>>>>>>
>>>>>> I'd be more confident if we did forbid P+A for such
>>>>>> interrupts
>>>>>> altogether, as they really feel like another kind of HW
>>>>>> interrupt.  
>>>>> How about a slightly bigger hammer:  Can we avoid doing P+A for
>>>>> level
>>>>> interrupts completely?  I don't think that really makes much
>>>>> sense, and
>>>>> I think we simply everything if we just come back out and
>>>>> resample the
>>>>> line.  For an edge, something like a network card, there's a
>>>>> potential
>>>>> performance win to appending a new pending state, but I doubt
>>>>> that this
>>>>> is the case for level interrupts.  
>>>> I started implementing the same thing yesterday. Somehow, it
>>>> feels
>>>> slightly better to have the same flow for all level interrupts,
>>>> including the timer, and we only use the MI on EOI as a way to
>>>> trigger
>>>> the next state of injection. Still testing, but looking good so
>>>> far.
>>>>
>>>> I'm still puzzled that we have this level-but-not-quite behaviour
>>>> for
>>>> VFIO interrupts. At some point, it is going to bite us badly.
>>>>  
>>> Where is the departure from level-triggered behavior with VFIO?  As
>>> far as I can tell, the GIC flow of the interrupts will be just a
>>> level
>>> interrupt, 
>> The GIC is fine, I believe. What is not exactly fine is the
>> signalling
>> from the device, which will never be dropped until the EOI has been
>> detected.
>>
>>>
>>> but we just need to make sure the resamplefd mechanism is
>>> supported for both types of interrupts.  Whether or not that's a
>>> decent mechanism seems orthogonal to me, but that's a discussion
>>> for
>>> another day I think.
>> Given that VFIO is built around this mechanism, I don't think we have
>> a
>> choice but to support it. Anyway, I came up with the following patch,
>> which I tested on Seattle with mtty. It also survived my usual
>> hammering of cyclictest, hackbench  and bulk VM installs.
>>
>> Shunyong, could you please give it a go?
>>
>> Thanks,
>>
>> 	M.
>>
> 
> I have tested the patch. It works on QDF2400 platform
> and kvm_notify_acked_irq() is called when state is idle.

Thanks a lot for testing.

> 
> BTW, I have following questions when I was debugging the issue.
> Coud you please give me some help?
> 1)what does "mi" mean in gic code? such as lr_signals_eoi_mi();

MI stands for Maintenance Interrupts. Life is too short to write that
all the time ;-)

> 2)In some __hyp_text code where printk() will cause "HYP panic:", such
> as in __kvm_vcpu_run(). How can I output debug information?

You can't. None of the kernel is mapped at EL2 on pre-VHE hardware.
You'll need to find indirect ways of outputting information (store data
in memory, and output it once you're back to EL1).

Thanks again,

	M.
-- 
Jazz is not dead. It just smells funny...