Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Christoffer Dall]

On Tue, Sep 15, 2015 at 04:16:07PM +0100, Andre Przywara wrote:
> Hi Christoffer,
> 
> On 14/09/15 12:42, Christoffer Dall wrote:
> 
>  Where is this done? I see that the physical dist state is altered on the
>  actual IRQ forwarding, but not on later exits/entries? Do you mean
>  kvm_vgic_flush_hwstate() with "flush"?
> >>>
> >>> this is a bug and should be fixed in the 'fixes' patches I sent last
> >>> week.  We should set active state on every entry to the guest for IRQs
> >>> with the HW bit set in either pending or active state.
> >>
> >> OK, sorry, I missed that one patch, I was looking at what should become
> >> -rc1 soon (because that's what I want to rebase my ITS emulation patches
> >> on). That patch wasn't in queue at the time I started looking at it.
> >>
> >> So I updated to the latest queue containing those two fixes and also
> >> applied your v2 series. Indeed this series addresses some of the things
> >> I was wondering about the last time, but the main thing still persists:
> >> - Every time the physical dist state is active we have the virtual state
> >> still at pending or active.
> > 
> > For the arch timer, yes.
> > 
> > For a passthrough device, there should be a situation where the physical
> > dist state is active but we didn't see the virtual state updated at the
> > vgic yet (after physical IRQ fires and before the VFIO ISR calls
> > kvm_set_irq).
> 
> But then we wouldn't get into vgic_sync_hwirq(), because we wouldn't
> inject a mapped IRQ before kvm_set_irq() is called, would we?

Ah, you meant, if we are in vgic_sync_hwirq() and the dist state is
active, then we have the virtual state still at pending or active?

That's a slightly different question from what you posed above.

I haven't thought extremely carefully about it, but could you not have
(1) guest deactivates (2) physical interrupt is handled on different CPU
on host for passthrough device (3) VFIO ISR leaves the IRQ active (3)
guest exits and you now hit vgic_sync_hwirq() and the virtual interrupt
is now inactive but the physical interrupt is active?

> 
> >> - If the physical dist state is non-active, the virtual state is
> >> inactive (LR.state==8: HW bit) as well. The associated ELRSR bit is 1
> >> (LR empty).
> >> (I was tracing every HW mapped LR in vgic_sync_hwirq() for this)
> >>
> >> So that contradicts:
> >>
> >> +  - On guest EOI, the *physical distributor* active bit gets cleared,
> >> +but the LR.Active is left untouched (set).
> >>
> >> This is the main point I was actually wondering about: I cannot confirm
> >> this statement. In my tests the LR state and the physical dist state
> >> always correspond, as excepted by reading the spec.
> >>
> >> I reckon that these observations are mostly independent from the actual
> >> KVM code, as I try to observe hardware state (physical distributor and
> >> LRs) before KVM tinkers with them.
> > 
> > ok, I got this paragraph from Marc, so we really need to ask him?  Which
> > hardware are you seeing this behavior on?  Perhaps implementations vary
> > on this point?
> 
> I checked this on Midway and Juno. Both have a GIC-400, but I don't have
> access to any other GIC implementations.
> I added the two BUG_ONs shown below to prove that assumption.
> 
> Eric, I've been told you observed the behaviour with the GIC not syncing
> LR and phys state for a mapped HWIRQ which was not the timer.
> Can you reproduce this? Does it complain with the patch below?
> 
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 5942ce9..7fac16e 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1459,9 +1459,12 @@ static bool vgic_sync_hwirq(struct kvm_vcpu
>IRQCHIP_STATE_ACTIVE,
>false);
>   WARN_ON(ret);
> + BUG_ON(!(vlr.state & 3));
>   return false;
>   }
> 
> + BUG_ON(vlr.state & 3);
> +
>   return process_queued_irq(vcpu, lr, vlr);
>  }
> 
> > 
> > I have no objections removing this point from the doc though, I'm just
> > relaying information on this one.
> 
> I see, I talked with Marc and I am about to gather more data with the
> above patch to prove that this never happens.
> 
> >>
> >> ...
> >>
> >>>
>  Is this an observation, an implementation bug or is this mentioned in
>  the spec? Needing to spoon-feed the VGIC by doing it's job sounds a bit
>  awkward to me.
> >>>
> >>> What do you mean?  How are we spoon-feeding the VGIC?
> >>
> >> By looking at the physical dist state and all LRs and clearing the LR we
> >> do what the GIC is actually supposed to do for us - and what it actually
> >> does according to my observations.
> >>
> >> The point is that patch 1 in my ITS emulation series is reworking the LR
> >> handling and this patch was based on assumptions that seem to be no
> >> longer true (i.e. we don't care about inactive LRs except for our LR
> >> mapping code). So I want to be 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Andre Przywara]

Hi Christoffer,

On 14/09/15 12:42, Christoffer Dall wrote:

 Where is this done? I see that the physical dist state is altered on the
 actual IRQ forwarding, but not on later exits/entries? Do you mean
 kvm_vgic_flush_hwstate() with "flush"?
>>>
>>> this is a bug and should be fixed in the 'fixes' patches I sent last
>>> week.  We should set active state on every entry to the guest for IRQs
>>> with the HW bit set in either pending or active state.
>>
>> OK, sorry, I missed that one patch, I was looking at what should become
>> -rc1 soon (because that's what I want to rebase my ITS emulation patches
>> on). That patch wasn't in queue at the time I started looking at it.
>>
>> So I updated to the latest queue containing those two fixes and also
>> applied your v2 series. Indeed this series addresses some of the things
>> I was wondering about the last time, but the main thing still persists:
>> - Every time the physical dist state is active we have the virtual state
>> still at pending or active.
> 
> For the arch timer, yes.
> 
> For a passthrough device, there should be a situation where the physical
> dist state is active but we didn't see the virtual state updated at the
> vgic yet (after physical IRQ fires and before the VFIO ISR calls
> kvm_set_irq).

But then we wouldn't get into vgic_sync_hwirq(), because we wouldn't
inject a mapped IRQ before kvm_set_irq() is called, would we?

>> - If the physical dist state is non-active, the virtual state is
>> inactive (LR.state==8: HW bit) as well. The associated ELRSR bit is 1
>> (LR empty).
>> (I was tracing every HW mapped LR in vgic_sync_hwirq() for this)
>>
>> So that contradicts:
>>
>> +  - On guest EOI, the *physical distributor* active bit gets cleared,
>> +but the LR.Active is left untouched (set).
>>
>> This is the main point I was actually wondering about: I cannot confirm
>> this statement. In my tests the LR state and the physical dist state
>> always correspond, as excepted by reading the spec.
>>
>> I reckon that these observations are mostly independent from the actual
>> KVM code, as I try to observe hardware state (physical distributor and
>> LRs) before KVM tinkers with them.
> 
> ok, I got this paragraph from Marc, so we really need to ask him?  Which
> hardware are you seeing this behavior on?  Perhaps implementations vary
> on this point?

I checked this on Midway and Juno. Both have a GIC-400, but I don't have
access to any other GIC implementations.
I added the two BUG_ONs shown below to prove that assumption.

Eric, I've been told you observed the behaviour with the GIC not syncing
LR and phys state for a mapped HWIRQ which was not the timer.
Can you reproduce this? Does it complain with the patch below?

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 5942ce9..7fac16e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1459,9 +1459,12 @@ static bool vgic_sync_hwirq(struct kvm_vcpu
 IRQCHIP_STATE_ACTIVE,
 false);
WARN_ON(ret);
+   BUG_ON(!(vlr.state & 3));
return false;
}

+   BUG_ON(vlr.state & 3);
+
return process_queued_irq(vcpu, lr, vlr);
 }

> 
> I have no objections removing this point from the doc though, I'm just
> relaying information on this one.

I see, I talked with Marc and I am about to gather more data with the
above patch to prove that this never happens.

>>
>> ...
>>
>>>
 Is this an observation, an implementation bug or is this mentioned in
 the spec? Needing to spoon-feed the VGIC by doing it's job sounds a bit
 awkward to me.
>>>
>>> What do you mean?  How are we spoon-feeding the VGIC?
>>
>> By looking at the physical dist state and all LRs and clearing the LR we
>> do what the GIC is actually supposed to do for us - and what it actually
>> does according to my observations.
>>
>> The point is that patch 1 in my ITS emulation series is reworking the LR
>> handling and this patch was based on assumptions that seem to be no
>> longer true (i.e. we don't care about inactive LRs except for our LR
>> mapping code). So I want to be sure that I fully get what is going on
>> here and I struggle at this at the moment due to the above statement.
>>
>> What are the plans regarding your "v2: Rework architected timer..."
>> series? Will this be queued for 4.4? I want to do the
>> rebasing^Wrewriting of my series only once if possible ;-)
>>
> I think we should settle on this series ASAP and base your ITS stuff on
> top of it.  What do you think?

Yeah, that's what I was thinking too. So I will be working against
4.3-rc1 with your timer-rework-v2 branch plus the other fixes from the
kvm-arm queue merged.

Cheers,
Andre.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Christoffer Dall]

On Mon, Sep 07, 2015 at 06:45:42PM +0200, Eric Auger wrote:
> Hi Christoffer,
> On 09/04/2015 09:40 PM, Christoffer Dall wrote:
> > Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> > way we deal with them is not apparently easy to understand by reading
> > various specs.
> > 
> > Therefore, add a proper documentation file explaining the flow and
> > rationale of the behavior of the vgic.
> > 
> > Some of this text was contributed by Marc Zyngier and edited by me.
> > Omissions and errors are all mine.
> > 
> > Signed-off-by: Christoffer Dall 
> > ---
> >  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> > +
> >  1 file changed, 181 insertions(+)
> >  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > 
> > diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> > b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > new file mode 100644
> > index 000..24b6f28
> > --- /dev/null
> > +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > @@ -0,0 +1,181 @@
> > +KVM/ARM VGIC Forwarded Physical Interrupts
> > +==
> > +
> > +The KVM/ARM code implements software support for the ARM Generic
> > +Interrupt Controller's (GIC's) hardware support for virtualization by
> > +allowing software to inject virtual interrupts to a VM, which the guest
> > +OS sees as regular interrupts.  The code is famously known as the VGIC.
> > +
> > +Some of these virtual interrupts, however, correspond to physical
> > +interrupts from real physical devices.  One example could be the
> > +architected timer, which itself supports virtualization, and therefore
> > +lets a guest OS program the hardware device directly to raise an
> > +interrupt at some point in time.  When such an interrupt is raised, the
> > +host OS initially handles the interrupt and must somehow signal this
> > +event as a virtual interrupt to the guest.  Another example could be a
> > +passthrough device, where the physical interrupts are initially handled
> > +by the host, but the device driver for the device lives in the guest OS
> > +and KVM must therefore somehow inject a virtual interrupt on behalf of
> > +the physical one to the guest OS.
> > +
> > +These virtual interrupts corresponding to a physical interrupt on the
> > +host are called forwarded physical interrupts, but are also sometimes
> > +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
> > +
> > +Forwarded physical interrupts are handled slightly differently compared
> > +to virtual interrupts generated purely by a software emulated device.
> > +
> > +
> > +The HW bit
> > +--
> > +Virtual interrupts are signalled to the guest by programming the List
> > +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> > +with the virtual IRQ number and the state of the interrupt (Pending,
> > +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> > +interrupt, the LR state moves from Pending to Active, and finally to
> > +inactive.
> > +
> > +The LRs include an extra bit, called the HW bit.  When this bit is set,
> > +KVM must also program an additional field in the LR, the physical IRQ
> > +number, to link the virtual with the physical IRQ.
> > +
> > +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> > +bit, never both at the same time.
> > +
> > +Setting the HW bit causes the hardware to deactivate the physical
> > +interrupt on the physical distributor when the guest deactivates the
> > +corresponding virtual interrupt.
> > +
> > +
> > +Forwarded Physical Interrupts Life Cycle
> > +
> > +
> > +The state of forwarded physical interrupts is managed in the following way:
> > +
> > +  - The physical interrupt is acked by the host, and becomes active on
> > +the physical distributor (*).
> > +  - KVM sets the LR.Pending bit, because this is the only way the GICV
> > +interface is going to present it to the guest.
> > +  - LR.Pending will stay set as long as the guest has not acked the 
> > interrupt.
> > +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
> > +expected.
> > +  - On guest EOI, the *physical distributor* active bit gets cleared,
> > +but the LR.Active is left untouched (set).
> > +  - KVM clears the LR when on VM exits when the physical distributor
> s/when//?
> > +active state has been cleared.
> > +
> > +(*): The host handling is slightly more complicated.  For some devices
> > +(shared), KVM directly sets the active state on the physical distributor
> > +before entering the guest, and for some devices (non-shared) the host
> > +configures the GIC such that it does not deactivate the interrupt on
> > +host EOIs, but only performs a priority drop allowing the GIC to receive
> > +other interrupts and leaves the interrupt in the active state on the
> > +physical 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Christoffer Dall]

Hi Andre,

On Fri, Sep 11, 2015 at 12:21:22PM +0100, Andre Przywara wrote:
> Hi Christoffer,
> 
> (actually you are not supposed to reply during your holidays!)

yeah, I know, but I couldn't help myself here.

> 
> On 09/09/15 09:49, Christoffer Dall wrote:
> > On Tue, Sep 8, 2015 at 6:57 PM, Andre Przywara  
> > wrote:
> >> Hi Eric,
> >>
> >> thanks for you answer.
> >>
> >> On 08/09/15 09:43, Eric Auger wrote:
> >>> Hi Andre,
> >>> On 09/07/2015 01:25 PM, Andre Przywara wrote:
>  Hi,
> 
>  firstly: this text is really great, thanks for coming up with that.
>  See below for some information I got from tracing the host which I
>  cannot make sense of
> 
> 
>  On 04/09/15 20:40, Christoffer Dall wrote:
> > Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> > way we deal with them is not apparently easy to understand by reading
> > various specs.
> >
> > Therefore, add a proper documentation file explaining the flow and
> > rationale of the behavior of the vgic.
> >
> > Some of this text was contributed by Marc Zyngier and edited by me.
> > Omissions and errors are all mine.
> >
> > Signed-off-by: Christoffer Dall 
> > ---
> >  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> > +
> >  1 file changed, 181 insertions(+)
> >  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> >
> > diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> > b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > new file mode 100644
> > index 000..24b6f28
> > --- /dev/null
> > +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > @@ -0,0 +1,181 @@
> > +KVM/ARM VGIC Forwarded Physical Interrupts
> > +==
> > +
> > +The KVM/ARM code implements software support for the ARM Generic
> > +Interrupt Controller's (GIC's) hardware support for virtualization by
> > +allowing software to inject virtual interrupts to a VM, which the guest
> > +OS sees as regular interrupts.  The code is famously known as the VGIC.
> > +
> > +Some of these virtual interrupts, however, correspond to physical
> > +interrupts from real physical devices.  One example could be the
> > +architected timer, which itself supports virtualization, and therefore
> > +lets a guest OS program the hardware device directly to raise an
> > +interrupt at some point in time.  When such an interrupt is raised, the
> > +host OS initially handles the interrupt and must somehow signal this
> > +event as a virtual interrupt to the guest.  Another example could be a
> > +passthrough device, where the physical interrupts are initially handled
> > +by the host, but the device driver for the device lives in the guest OS
> > +and KVM must therefore somehow inject a virtual interrupt on behalf of
> > +the physical one to the guest OS.
> > +
> > +These virtual interrupts corresponding to a physical interrupt on the
> > +host are called forwarded physical interrupts, but are also sometimes
> > +referred to as 'virtualized physical interrupts' and 'mapped 
> > interrupts'.
> > +
> > +Forwarded physical interrupts are handled slightly differently compared
> > +to virtual interrupts generated purely by a software emulated device.
> > +
> > +
> > +The HW bit
> > +--
> > +Virtual interrupts are signalled to the guest by programming the List
> > +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> > +with the virtual IRQ number and the state of the interrupt (Pending,
> > +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> > +interrupt, the LR state moves from Pending to Active, and finally to
> > +inactive.
> > +
> > +The LRs include an extra bit, called the HW bit.  When this bit is set,
> > +KVM must also program an additional field in the LR, the physical IRQ
> > +number, to link the virtual with the physical IRQ.
> > +
> > +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> > +bit, never both at the same time.
> > +
> > +Setting the HW bit causes the hardware to deactivate the physical
> > +interrupt on the physical distributor when the guest deactivates the
> > +corresponding virtual interrupt.
> > +
> > +
> > +Forwarded Physical Interrupts Life Cycle
> > +
> > +
> > +The state of forwarded physical interrupts is managed in the following 
> > way:
> > +
> > +  - The physical interrupt is acked by the host, and becomes active on
> > +the physical distributor (*).
> > +  - KVM sets the LR.Pending bit, because this is the only way the 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Andre Przywara]

Hi Christoffer,

(actually you are not supposed to reply during your holidays!)

On 09/09/15 09:49, Christoffer Dall wrote:
> On Tue, Sep 8, 2015 at 6:57 PM, Andre Przywara  wrote:
>> Hi Eric,
>>
>> thanks for you answer.
>>
>> On 08/09/15 09:43, Eric Auger wrote:
>>> Hi Andre,
>>> On 09/07/2015 01:25 PM, Andre Przywara wrote:
 Hi,

 firstly: this text is really great, thanks for coming up with that.
 See below for some information I got from tracing the host which I
 cannot make sense of


 On 04/09/15 20:40, Christoffer Dall wrote:
> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> way we deal with them is not apparently easy to understand by reading
> various specs.
>
> Therefore, add a proper documentation file explaining the flow and
> rationale of the behavior of the vgic.
>
> Some of this text was contributed by Marc Zyngier and edited by me.
> Omissions and errors are all mine.
>
> Signed-off-by: Christoffer Dall 
> ---
>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> +
>  1 file changed, 181 insertions(+)
>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>
> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> new file mode 100644
> index 000..24b6f28
> --- /dev/null
> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> @@ -0,0 +1,181 @@
> +KVM/ARM VGIC Forwarded Physical Interrupts
> +==
> +
> +The KVM/ARM code implements software support for the ARM Generic
> +Interrupt Controller's (GIC's) hardware support for virtualization by
> +allowing software to inject virtual interrupts to a VM, which the guest
> +OS sees as regular interrupts.  The code is famously known as the VGIC.
> +
> +Some of these virtual interrupts, however, correspond to physical
> +interrupts from real physical devices.  One example could be the
> +architected timer, which itself supports virtualization, and therefore
> +lets a guest OS program the hardware device directly to raise an
> +interrupt at some point in time.  When such an interrupt is raised, the
> +host OS initially handles the interrupt and must somehow signal this
> +event as a virtual interrupt to the guest.  Another example could be a
> +passthrough device, where the physical interrupts are initially handled
> +by the host, but the device driver for the device lives in the guest OS
> +and KVM must therefore somehow inject a virtual interrupt on behalf of
> +the physical one to the guest OS.
> +
> +These virtual interrupts corresponding to a physical interrupt on the
> +host are called forwarded physical interrupts, but are also sometimes
> +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
> +
> +Forwarded physical interrupts are handled slightly differently compared
> +to virtual interrupts generated purely by a software emulated device.
> +
> +
> +The HW bit
> +--
> +Virtual interrupts are signalled to the guest by programming the List
> +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> +with the virtual IRQ number and the state of the interrupt (Pending,
> +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> +interrupt, the LR state moves from Pending to Active, and finally to
> +inactive.
> +
> +The LRs include an extra bit, called the HW bit.  When this bit is set,
> +KVM must also program an additional field in the LR, the physical IRQ
> +number, to link the virtual with the physical IRQ.
> +
> +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> +bit, never both at the same time.
> +
> +Setting the HW bit causes the hardware to deactivate the physical
> +interrupt on the physical distributor when the guest deactivates the
> +corresponding virtual interrupt.
> +
> +
> +Forwarded Physical Interrupts Life Cycle
> +
> +
> +The state of forwarded physical interrupts is managed in the following 
> way:
> +
> +  - The physical interrupt is acked by the host, and becomes active on
> +the physical distributor (*).
> +  - KVM sets the LR.Pending bit, because this is the only way the GICV
> +interface is going to present it to the guest.
> +  - LR.Pending will stay set as long as the guest has not acked the 
> interrupt.
> +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
> +expected.
> +  - On guest EOI, the *physical distributor* active bit gets cleared,
> +   

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Eric Auger]

Salut Andre,
On 09/09/2015 10:49 AM, Christoffer Dall wrote:
> On Tue, Sep 8, 2015 at 6:57 PM, Andre Przywara  wrote:
>> Hi Eric,
>>
>> thanks for you answer.
>>
>> On 08/09/15 09:43, Eric Auger wrote:
>>> Hi Andre,
>>> On 09/07/2015 01:25 PM, Andre Przywara wrote:
 Hi,

 firstly: this text is really great, thanks for coming up with that.
 See below for some information I got from tracing the host which I
 cannot make sense of


 On 04/09/15 20:40, Christoffer Dall wrote:
> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> way we deal with them is not apparently easy to understand by reading
> various specs.
>
> Therefore, add a proper documentation file explaining the flow and
> rationale of the behavior of the vgic.
>
> Some of this text was contributed by Marc Zyngier and edited by me.
> Omissions and errors are all mine.
>
> Signed-off-by: Christoffer Dall 
> ---
>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> +
>  1 file changed, 181 insertions(+)
>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>
> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> new file mode 100644
> index 000..24b6f28
> --- /dev/null
> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> @@ -0,0 +1,181 @@
> +KVM/ARM VGIC Forwarded Physical Interrupts
> +==
> +
> +The KVM/ARM code implements software support for the ARM Generic
> +Interrupt Controller's (GIC's) hardware support for virtualization by
> +allowing software to inject virtual interrupts to a VM, which the guest
> +OS sees as regular interrupts.  The code is famously known as the VGIC.
> +
> +Some of these virtual interrupts, however, correspond to physical
> +interrupts from real physical devices.  One example could be the
> +architected timer, which itself supports virtualization, and therefore
> +lets a guest OS program the hardware device directly to raise an
> +interrupt at some point in time.  When such an interrupt is raised, the
> +host OS initially handles the interrupt and must somehow signal this
> +event as a virtual interrupt to the guest.  Another example could be a
> +passthrough device, where the physical interrupts are initially handled
> +by the host, but the device driver for the device lives in the guest OS
> +and KVM must therefore somehow inject a virtual interrupt on behalf of
> +the physical one to the guest OS.
> +
> +These virtual interrupts corresponding to a physical interrupt on the
> +host are called forwarded physical interrupts, but are also sometimes
> +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
> +
> +Forwarded physical interrupts are handled slightly differently compared
> +to virtual interrupts generated purely by a software emulated device.
> +
> +
> +The HW bit
> +--
> +Virtual interrupts are signalled to the guest by programming the List
> +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> +with the virtual IRQ number and the state of the interrupt (Pending,
> +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> +interrupt, the LR state moves from Pending to Active, and finally to
> +inactive.
> +
> +The LRs include an extra bit, called the HW bit.  When this bit is set,
> +KVM must also program an additional field in the LR, the physical IRQ
> +number, to link the virtual with the physical IRQ.
> +
> +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> +bit, never both at the same time.
> +
> +Setting the HW bit causes the hardware to deactivate the physical
> +interrupt on the physical distributor when the guest deactivates the
> +corresponding virtual interrupt.
> +
> +
> +Forwarded Physical Interrupts Life Cycle
> +
> +
> +The state of forwarded physical interrupts is managed in the following 
> way:
> +
> +  - The physical interrupt is acked by the host, and becomes active on
> +the physical distributor (*).
> +  - KVM sets the LR.Pending bit, because this is the only way the GICV
> +interface is going to present it to the guest.
> +  - LR.Pending will stay set as long as the guest has not acked the 
> interrupt.
> +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
> +expected.
> +  - On guest EOI, the *physical distributor* active bit gets cleared,
> +but the LR.Active is left untouched (set).

 I tried 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Christoffer Dall]

On Tue, Sep 8, 2015 at 6:57 PM, Andre Przywara  wrote:
> Hi Eric,
>
> thanks for you answer.
>
> On 08/09/15 09:43, Eric Auger wrote:
>> Hi Andre,
>> On 09/07/2015 01:25 PM, Andre Przywara wrote:
>>> Hi,
>>>
>>> firstly: this text is really great, thanks for coming up with that.
>>> See below for some information I got from tracing the host which I
>>> cannot make sense of
>>>
>>>
>>> On 04/09/15 20:40, Christoffer Dall wrote:
 Forwarded physical interrupts on arm/arm64 is a tricky concept and the
 way we deal with them is not apparently easy to understand by reading
 various specs.

 Therefore, add a proper documentation file explaining the flow and
 rationale of the behavior of the vgic.

 Some of this text was contributed by Marc Zyngier and edited by me.
 Omissions and errors are all mine.

 Signed-off-by: Christoffer Dall 
 ---
  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
 +
  1 file changed, 181 insertions(+)
  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt

 diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
 b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
 new file mode 100644
 index 000..24b6f28
 --- /dev/null
 +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
 @@ -0,0 +1,181 @@
 +KVM/ARM VGIC Forwarded Physical Interrupts
 +==
 +
 +The KVM/ARM code implements software support for the ARM Generic
 +Interrupt Controller's (GIC's) hardware support for virtualization by
 +allowing software to inject virtual interrupts to a VM, which the guest
 +OS sees as regular interrupts.  The code is famously known as the VGIC.
 +
 +Some of these virtual interrupts, however, correspond to physical
 +interrupts from real physical devices.  One example could be the
 +architected timer, which itself supports virtualization, and therefore
 +lets a guest OS program the hardware device directly to raise an
 +interrupt at some point in time.  When such an interrupt is raised, the
 +host OS initially handles the interrupt and must somehow signal this
 +event as a virtual interrupt to the guest.  Another example could be a
 +passthrough device, where the physical interrupts are initially handled
 +by the host, but the device driver for the device lives in the guest OS
 +and KVM must therefore somehow inject a virtual interrupt on behalf of
 +the physical one to the guest OS.
 +
 +These virtual interrupts corresponding to a physical interrupt on the
 +host are called forwarded physical interrupts, but are also sometimes
 +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
 +
 +Forwarded physical interrupts are handled slightly differently compared
 +to virtual interrupts generated purely by a software emulated device.
 +
 +
 +The HW bit
 +--
 +Virtual interrupts are signalled to the guest by programming the List
 +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
 +with the virtual IRQ number and the state of the interrupt (Pending,
 +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
 +interrupt, the LR state moves from Pending to Active, and finally to
 +inactive.
 +
 +The LRs include an extra bit, called the HW bit.  When this bit is set,
 +KVM must also program an additional field in the LR, the physical IRQ
 +number, to link the virtual with the physical IRQ.
 +
 +When the HW bit is set, KVM must EITHER set the Pending OR the Active
 +bit, never both at the same time.
 +
 +Setting the HW bit causes the hardware to deactivate the physical
 +interrupt on the physical distributor when the guest deactivates the
 +corresponding virtual interrupt.
 +
 +
 +Forwarded Physical Interrupts Life Cycle
 +
 +
 +The state of forwarded physical interrupts is managed in the following 
 way:
 +
 +  - The physical interrupt is acked by the host, and becomes active on
 +the physical distributor (*).
 +  - KVM sets the LR.Pending bit, because this is the only way the GICV
 +interface is going to present it to the guest.
 +  - LR.Pending will stay set as long as the guest has not acked the 
 interrupt.
 +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
 +expected.
 +  - On guest EOI, the *physical distributor* active bit gets cleared,
 +but the LR.Active is left untouched (set).
>>>
>>> I tried hard in the last week, but couldn't confirm this. Tracing shows
>>> the following pattern over and over (case 1):
>>> (This is the kvm/kvm.git:queue branch from last 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Eric Auger]

Hi Marc,
On 09/07/2015 07:50 PM, Marc Zyngier wrote:
> On 07/09/15 17:45, Eric Auger wrote:
>> Hi Christoffer,
>> On 09/04/2015 09:40 PM, Christoffer Dall wrote:
>>> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
>>> way we deal with them is not apparently easy to understand by reading
>>> various specs.
>>>
>>> Therefore, add a proper documentation file explaining the flow and
>>> rationale of the behavior of the vgic.
>>>
>>> Some of this text was contributed by Marc Zyngier and edited by me.
>>> Omissions and errors are all mine.
>>>
>>> Signed-off-by: Christoffer Dall 
>>> ---
>>>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
>>> +
>>>  1 file changed, 181 insertions(+)
>>>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>>
>>> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
>>> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>> new file mode 100644
>>> index 000..24b6f28
>>> --- /dev/null
>>> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>> @@ -0,0 +1,181 @@
>>> +KVM/ARM VGIC Forwarded Physical Interrupts
>>> +==
> 
> [...]
> 
>>> +1.  KVM runs the VCPU
>>> +2.  The guest programs the time to fire in T+100
>>> +4.  At T+100 the timer fires and a physical IRQ causes the VM to exit
>>> +5.  With interrupts disabled on the CPU, KVM looks at the timer state
>>> +and injects a forwarded physical interrupt because it concludes the
>>> +timer has expired.
>> I don't get how we can trap without the virtual timer PPI handler being
>> entered on host side. Please can you elaborate on this?
> 
> On VM exit, we disable the virtual timer (see the code in
> hyp.S::save_timer_state where we clear the enable bit). We still perform
> the exit, but the cause for exit is now gone, and the handler will never
> fire.
OK thanks for the clarification

Eric
> 
> Thanks,
> 
>   M.
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Eric Auger]

Hi Andre,
On 09/07/2015 01:25 PM, Andre Przywara wrote:
> Hi,
> 
> firstly: this text is really great, thanks for coming up with that.
> See below for some information I got from tracing the host which I
> cannot make sense of
> 
> 
> On 04/09/15 20:40, Christoffer Dall wrote:
>> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
>> way we deal with them is not apparently easy to understand by reading
>> various specs.
>>
>> Therefore, add a proper documentation file explaining the flow and
>> rationale of the behavior of the vgic.
>>
>> Some of this text was contributed by Marc Zyngier and edited by me.
>> Omissions and errors are all mine.
>>
>> Signed-off-by: Christoffer Dall 
>> ---
>>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
>> +
>>  1 file changed, 181 insertions(+)
>>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>
>> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
>> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>> new file mode 100644
>> index 000..24b6f28
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>> @@ -0,0 +1,181 @@
>> +KVM/ARM VGIC Forwarded Physical Interrupts
>> +==
>> +
>> +The KVM/ARM code implements software support for the ARM Generic
>> +Interrupt Controller's (GIC's) hardware support for virtualization by
>> +allowing software to inject virtual interrupts to a VM, which the guest
>> +OS sees as regular interrupts.  The code is famously known as the VGIC.
>> +
>> +Some of these virtual interrupts, however, correspond to physical
>> +interrupts from real physical devices.  One example could be the
>> +architected timer, which itself supports virtualization, and therefore
>> +lets a guest OS program the hardware device directly to raise an
>> +interrupt at some point in time.  When such an interrupt is raised, the
>> +host OS initially handles the interrupt and must somehow signal this
>> +event as a virtual interrupt to the guest.  Another example could be a
>> +passthrough device, where the physical interrupts are initially handled
>> +by the host, but the device driver for the device lives in the guest OS
>> +and KVM must therefore somehow inject a virtual interrupt on behalf of
>> +the physical one to the guest OS.
>> +
>> +These virtual interrupts corresponding to a physical interrupt on the
>> +host are called forwarded physical interrupts, but are also sometimes
>> +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
>> +
>> +Forwarded physical interrupts are handled slightly differently compared
>> +to virtual interrupts generated purely by a software emulated device.
>> +
>> +
>> +The HW bit
>> +--
>> +Virtual interrupts are signalled to the guest by programming the List
>> +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
>> +with the virtual IRQ number and the state of the interrupt (Pending,
>> +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
>> +interrupt, the LR state moves from Pending to Active, and finally to
>> +inactive.
>> +
>> +The LRs include an extra bit, called the HW bit.  When this bit is set,
>> +KVM must also program an additional field in the LR, the physical IRQ
>> +number, to link the virtual with the physical IRQ.
>> +
>> +When the HW bit is set, KVM must EITHER set the Pending OR the Active
>> +bit, never both at the same time.
>> +
>> +Setting the HW bit causes the hardware to deactivate the physical
>> +interrupt on the physical distributor when the guest deactivates the
>> +corresponding virtual interrupt.
>> +
>> +
>> +Forwarded Physical Interrupts Life Cycle
>> +
>> +
>> +The state of forwarded physical interrupts is managed in the following way:
>> +
>> +  - The physical interrupt is acked by the host, and becomes active on
>> +the physical distributor (*).
>> +  - KVM sets the LR.Pending bit, because this is the only way the GICV
>> +interface is going to present it to the guest.
>> +  - LR.Pending will stay set as long as the guest has not acked the 
>> interrupt.
>> +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
>> +expected.
>> +  - On guest EOI, the *physical distributor* active bit gets cleared,
>> +but the LR.Active is left untouched (set).
> 
> I tried hard in the last week, but couldn't confirm this. Tracing shows
> the following pattern over and over (case 1):
> (This is the kvm/kvm.git:queue branch from last week, so including the
> mapped timer IRQ code. Tests were done on Juno and Midway)
> 
> ...
> 229.340171: kvm_exit: TRAP: HSR_EC: 0x0001 (WFx), PC: 0xffc98a64
> 229.340324: kvm_exit: IRQ: HSR_EC: 0x0001 (WFx), PC: 0xffc0001c63a0
> 229.340428: kvm_exit: TRAP: HSR_EC: 0x0024 (DABT_LOW), PC:
> 0xffc0004089d8
> 229.340430: kvm_vgic_sync_hwstate: LR0 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Christoffer Dall]

On Mon, Sep 07, 2015 at 12:25:27PM +0100, Andre Przywara wrote:
> Hi,
> 
> firstly: this text is really great, thanks for coming up with that.
> See below for some information I got from tracing the host which I
> cannot make sense of
> 
> 
> On 04/09/15 20:40, Christoffer Dall wrote:
> > Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> > way we deal with them is not apparently easy to understand by reading
> > various specs.
> > 
> > Therefore, add a proper documentation file explaining the flow and
> > rationale of the behavior of the vgic.
> > 
> > Some of this text was contributed by Marc Zyngier and edited by me.
> > Omissions and errors are all mine.
> > 
> > Signed-off-by: Christoffer Dall 
> > ---
> >  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> > +
> >  1 file changed, 181 insertions(+)
> >  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > 
> > diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> > b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > new file mode 100644
> > index 000..24b6f28
> > --- /dev/null
> > +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> > @@ -0,0 +1,181 @@
> > +KVM/ARM VGIC Forwarded Physical Interrupts
> > +==
> > +
> > +The KVM/ARM code implements software support for the ARM Generic
> > +Interrupt Controller's (GIC's) hardware support for virtualization by
> > +allowing software to inject virtual interrupts to a VM, which the guest
> > +OS sees as regular interrupts.  The code is famously known as the VGIC.
> > +
> > +Some of these virtual interrupts, however, correspond to physical
> > +interrupts from real physical devices.  One example could be the
> > +architected timer, which itself supports virtualization, and therefore
> > +lets a guest OS program the hardware device directly to raise an
> > +interrupt at some point in time.  When such an interrupt is raised, the
> > +host OS initially handles the interrupt and must somehow signal this
> > +event as a virtual interrupt to the guest.  Another example could be a
> > +passthrough device, where the physical interrupts are initially handled
> > +by the host, but the device driver for the device lives in the guest OS
> > +and KVM must therefore somehow inject a virtual interrupt on behalf of
> > +the physical one to the guest OS.
> > +
> > +These virtual interrupts corresponding to a physical interrupt on the
> > +host are called forwarded physical interrupts, but are also sometimes
> > +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
> > +
> > +Forwarded physical interrupts are handled slightly differently compared
> > +to virtual interrupts generated purely by a software emulated device.
> > +
> > +
> > +The HW bit
> > +--
> > +Virtual interrupts are signalled to the guest by programming the List
> > +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> > +with the virtual IRQ number and the state of the interrupt (Pending,
> > +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> > +interrupt, the LR state moves from Pending to Active, and finally to
> > +inactive.
> > +
> > +The LRs include an extra bit, called the HW bit.  When this bit is set,
> > +KVM must also program an additional field in the LR, the physical IRQ
> > +number, to link the virtual with the physical IRQ.
> > +
> > +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> > +bit, never both at the same time.
> > +
> > +Setting the HW bit causes the hardware to deactivate the physical
> > +interrupt on the physical distributor when the guest deactivates the
> > +corresponding virtual interrupt.
> > +
> > +
> > +Forwarded Physical Interrupts Life Cycle
> > +
> > +
> > +The state of forwarded physical interrupts is managed in the following way:
> > +
> > +  - The physical interrupt is acked by the host, and becomes active on
> > +the physical distributor (*).
> > +  - KVM sets the LR.Pending bit, because this is the only way the GICV
> > +interface is going to present it to the guest.
> > +  - LR.Pending will stay set as long as the guest has not acked the 
> > interrupt.
> > +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
> > +expected.
> > +  - On guest EOI, the *physical distributor* active bit gets cleared,
> > +but the LR.Active is left untouched (set).
> 
> I tried hard in the last week, but couldn't confirm this. Tracing shows
> the following pattern over and over (case 1):
> (This is the kvm/kvm.git:queue branch from last week, so including the
> mapped timer IRQ code. Tests were done on Juno and Midway)
> 
> ...
> 229.340171: kvm_exit: TRAP: HSR_EC: 0x0001 (WFx), PC: 0xffc98a64
> 229.340324: kvm_exit: IRQ: HSR_EC: 0x0001 (WFx), PC: 0xffc0001c63a0
> 229.340428: kvm_exit: 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Andre Przywara]

Hi Eric,

thanks for you answer.

On 08/09/15 09:43, Eric Auger wrote:
> Hi Andre,
> On 09/07/2015 01:25 PM, Andre Przywara wrote:
>> Hi,
>>
>> firstly: this text is really great, thanks for coming up with that.
>> See below for some information I got from tracing the host which I
>> cannot make sense of
>>
>>
>> On 04/09/15 20:40, Christoffer Dall wrote:
>>> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
>>> way we deal with them is not apparently easy to understand by reading
>>> various specs.
>>>
>>> Therefore, add a proper documentation file explaining the flow and
>>> rationale of the behavior of the vgic.
>>>
>>> Some of this text was contributed by Marc Zyngier and edited by me.
>>> Omissions and errors are all mine.
>>>
>>> Signed-off-by: Christoffer Dall 
>>> ---
>>>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
>>> +
>>>  1 file changed, 181 insertions(+)
>>>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>>
>>> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
>>> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>> new file mode 100644
>>> index 000..24b6f28
>>> --- /dev/null
>>> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>> @@ -0,0 +1,181 @@
>>> +KVM/ARM VGIC Forwarded Physical Interrupts
>>> +==
>>> +
>>> +The KVM/ARM code implements software support for the ARM Generic
>>> +Interrupt Controller's (GIC's) hardware support for virtualization by
>>> +allowing software to inject virtual interrupts to a VM, which the guest
>>> +OS sees as regular interrupts.  The code is famously known as the VGIC.
>>> +
>>> +Some of these virtual interrupts, however, correspond to physical
>>> +interrupts from real physical devices.  One example could be the
>>> +architected timer, which itself supports virtualization, and therefore
>>> +lets a guest OS program the hardware device directly to raise an
>>> +interrupt at some point in time.  When such an interrupt is raised, the
>>> +host OS initially handles the interrupt and must somehow signal this
>>> +event as a virtual interrupt to the guest.  Another example could be a
>>> +passthrough device, where the physical interrupts are initially handled
>>> +by the host, but the device driver for the device lives in the guest OS
>>> +and KVM must therefore somehow inject a virtual interrupt on behalf of
>>> +the physical one to the guest OS.
>>> +
>>> +These virtual interrupts corresponding to a physical interrupt on the
>>> +host are called forwarded physical interrupts, but are also sometimes
>>> +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
>>> +
>>> +Forwarded physical interrupts are handled slightly differently compared
>>> +to virtual interrupts generated purely by a software emulated device.
>>> +
>>> +
>>> +The HW bit
>>> +--
>>> +Virtual interrupts are signalled to the guest by programming the List
>>> +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
>>> +with the virtual IRQ number and the state of the interrupt (Pending,
>>> +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
>>> +interrupt, the LR state moves from Pending to Active, and finally to
>>> +inactive.
>>> +
>>> +The LRs include an extra bit, called the HW bit.  When this bit is set,
>>> +KVM must also program an additional field in the LR, the physical IRQ
>>> +number, to link the virtual with the physical IRQ.
>>> +
>>> +When the HW bit is set, KVM must EITHER set the Pending OR the Active
>>> +bit, never both at the same time.
>>> +
>>> +Setting the HW bit causes the hardware to deactivate the physical
>>> +interrupt on the physical distributor when the guest deactivates the
>>> +corresponding virtual interrupt.
>>> +
>>> +
>>> +Forwarded Physical Interrupts Life Cycle
>>> +
>>> +
>>> +The state of forwarded physical interrupts is managed in the following way:
>>> +
>>> +  - The physical interrupt is acked by the host, and becomes active on
>>> +the physical distributor (*).
>>> +  - KVM sets the LR.Pending bit, because this is the only way the GICV
>>> +interface is going to present it to the guest.
>>> +  - LR.Pending will stay set as long as the guest has not acked the 
>>> interrupt.
>>> +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
>>> +expected.
>>> +  - On guest EOI, the *physical distributor* active bit gets cleared,
>>> +but the LR.Active is left untouched (set).
>>
>> I tried hard in the last week, but couldn't confirm this. Tracing shows
>> the following pattern over and over (case 1):
>> (This is the kvm/kvm.git:queue branch from last week, so including the
>> mapped timer IRQ code. Tests were done on Juno and Midway)
>>
>> ...
>> 229.340171: kvm_exit: TRAP: HSR_EC: 0x0001 (WFx), PC: 0xffc98a64
>> 229.340324: kvm_exit: 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Andre Przywara]

Hi,

firstly: this text is really great, thanks for coming up with that.
See below for some information I got from tracing the host which I
cannot make sense of


On 04/09/15 20:40, Christoffer Dall wrote:
> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> way we deal with them is not apparently easy to understand by reading
> various specs.
> 
> Therefore, add a proper documentation file explaining the flow and
> rationale of the behavior of the vgic.
> 
> Some of this text was contributed by Marc Zyngier and edited by me.
> Omissions and errors are all mine.
> 
> Signed-off-by: Christoffer Dall 
> ---
>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> +
>  1 file changed, 181 insertions(+)
>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> 
> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> new file mode 100644
> index 000..24b6f28
> --- /dev/null
> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> @@ -0,0 +1,181 @@
> +KVM/ARM VGIC Forwarded Physical Interrupts
> +==
> +
> +The KVM/ARM code implements software support for the ARM Generic
> +Interrupt Controller's (GIC's) hardware support for virtualization by
> +allowing software to inject virtual interrupts to a VM, which the guest
> +OS sees as regular interrupts.  The code is famously known as the VGIC.
> +
> +Some of these virtual interrupts, however, correspond to physical
> +interrupts from real physical devices.  One example could be the
> +architected timer, which itself supports virtualization, and therefore
> +lets a guest OS program the hardware device directly to raise an
> +interrupt at some point in time.  When such an interrupt is raised, the
> +host OS initially handles the interrupt and must somehow signal this
> +event as a virtual interrupt to the guest.  Another example could be a
> +passthrough device, where the physical interrupts are initially handled
> +by the host, but the device driver for the device lives in the guest OS
> +and KVM must therefore somehow inject a virtual interrupt on behalf of
> +the physical one to the guest OS.
> +
> +These virtual interrupts corresponding to a physical interrupt on the
> +host are called forwarded physical interrupts, but are also sometimes
> +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
> +
> +Forwarded physical interrupts are handled slightly differently compared
> +to virtual interrupts generated purely by a software emulated device.
> +
> +
> +The HW bit
> +--
> +Virtual interrupts are signalled to the guest by programming the List
> +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> +with the virtual IRQ number and the state of the interrupt (Pending,
> +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> +interrupt, the LR state moves from Pending to Active, and finally to
> +inactive.
> +
> +The LRs include an extra bit, called the HW bit.  When this bit is set,
> +KVM must also program an additional field in the LR, the physical IRQ
> +number, to link the virtual with the physical IRQ.
> +
> +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> +bit, never both at the same time.
> +
> +Setting the HW bit causes the hardware to deactivate the physical
> +interrupt on the physical distributor when the guest deactivates the
> +corresponding virtual interrupt.
> +
> +
> +Forwarded Physical Interrupts Life Cycle
> +
> +
> +The state of forwarded physical interrupts is managed in the following way:
> +
> +  - The physical interrupt is acked by the host, and becomes active on
> +the physical distributor (*).
> +  - KVM sets the LR.Pending bit, because this is the only way the GICV
> +interface is going to present it to the guest.
> +  - LR.Pending will stay set as long as the guest has not acked the 
> interrupt.
> +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
> +expected.
> +  - On guest EOI, the *physical distributor* active bit gets cleared,
> +but the LR.Active is left untouched (set).

I tried hard in the last week, but couldn't confirm this. Tracing shows
the following pattern over and over (case 1):
(This is the kvm/kvm.git:queue branch from last week, so including the
mapped timer IRQ code. Tests were done on Juno and Midway)

...
229.340171: kvm_exit: TRAP: HSR_EC: 0x0001 (WFx), PC: 0xffc98a64
229.340324: kvm_exit: IRQ: HSR_EC: 0x0001 (WFx), PC: 0xffc0001c63a0
229.340428: kvm_exit: TRAP: HSR_EC: 0x0024 (DABT_LOW), PC:
0xffc0004089d8
229.340430: kvm_vgic_sync_hwstate: LR0 vIRQ: 27, HWIRQ: 27, LR.state: 8,
ELRSR: 1, dist active: 0, log. active: 1


My hunch is that the following happens (please correct me if needed!):
First there is an unrelated trap 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Eric Auger]

Hi Christoffer,
On 09/04/2015 09:40 PM, Christoffer Dall wrote:
> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
> way we deal with them is not apparently easy to understand by reading
> various specs.
> 
> Therefore, add a proper documentation file explaining the flow and
> rationale of the behavior of the vgic.
> 
> Some of this text was contributed by Marc Zyngier and edited by me.
> Omissions and errors are all mine.
> 
> Signed-off-by: Christoffer Dall 
> ---
>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
> +
>  1 file changed, 181 insertions(+)
>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> 
> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> new file mode 100644
> index 000..24b6f28
> --- /dev/null
> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
> @@ -0,0 +1,181 @@
> +KVM/ARM VGIC Forwarded Physical Interrupts
> +==
> +
> +The KVM/ARM code implements software support for the ARM Generic
> +Interrupt Controller's (GIC's) hardware support for virtualization by
> +allowing software to inject virtual interrupts to a VM, which the guest
> +OS sees as regular interrupts.  The code is famously known as the VGIC.
> +
> +Some of these virtual interrupts, however, correspond to physical
> +interrupts from real physical devices.  One example could be the
> +architected timer, which itself supports virtualization, and therefore
> +lets a guest OS program the hardware device directly to raise an
> +interrupt at some point in time.  When such an interrupt is raised, the
> +host OS initially handles the interrupt and must somehow signal this
> +event as a virtual interrupt to the guest.  Another example could be a
> +passthrough device, where the physical interrupts are initially handled
> +by the host, but the device driver for the device lives in the guest OS
> +and KVM must therefore somehow inject a virtual interrupt on behalf of
> +the physical one to the guest OS.
> +
> +These virtual interrupts corresponding to a physical interrupt on the
> +host are called forwarded physical interrupts, but are also sometimes
> +referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
> +
> +Forwarded physical interrupts are handled slightly differently compared
> +to virtual interrupts generated purely by a software emulated device.
> +
> +
> +The HW bit
> +--
> +Virtual interrupts are signalled to the guest by programming the List
> +Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
> +with the virtual IRQ number and the state of the interrupt (Pending,
> +Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
> +interrupt, the LR state moves from Pending to Active, and finally to
> +inactive.
> +
> +The LRs include an extra bit, called the HW bit.  When this bit is set,
> +KVM must also program an additional field in the LR, the physical IRQ
> +number, to link the virtual with the physical IRQ.
> +
> +When the HW bit is set, KVM must EITHER set the Pending OR the Active
> +bit, never both at the same time.
> +
> +Setting the HW bit causes the hardware to deactivate the physical
> +interrupt on the physical distributor when the guest deactivates the
> +corresponding virtual interrupt.
> +
> +
> +Forwarded Physical Interrupts Life Cycle
> +
> +
> +The state of forwarded physical interrupts is managed in the following way:
> +
> +  - The physical interrupt is acked by the host, and becomes active on
> +the physical distributor (*).
> +  - KVM sets the LR.Pending bit, because this is the only way the GICV
> +interface is going to present it to the guest.
> +  - LR.Pending will stay set as long as the guest has not acked the 
> interrupt.
> +  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
> +expected.
> +  - On guest EOI, the *physical distributor* active bit gets cleared,
> +but the LR.Active is left untouched (set).
> +  - KVM clears the LR when on VM exits when the physical distributor
s/when//?
> +active state has been cleared.
> +
> +(*): The host handling is slightly more complicated.  For some devices
> +(shared), KVM directly sets the active state on the physical distributor
> +before entering the guest, and for some devices (non-shared) the host
> +configures the GIC such that it does not deactivate the interrupt on
> +host EOIs, but only performs a priority drop allowing the GIC to receive
> +other interrupts and leaves the interrupt in the active state on the
> +physical distributor.
EOIMode == 1 is set globally and impacts all forwarded SPI/PPIs, shared
or not shared I think. reading the above lines I have the impression
this is a per-device programming.

My understanding is for the timer it is needed to manually set the
physical 

Re: [PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Marc Zyngier]

On 07/09/15 17:45, Eric Auger wrote:
> Hi Christoffer,
> On 09/04/2015 09:40 PM, Christoffer Dall wrote:
>> Forwarded physical interrupts on arm/arm64 is a tricky concept and the
>> way we deal with them is not apparently easy to understand by reading
>> various specs.
>>
>> Therefore, add a proper documentation file explaining the flow and
>> rationale of the behavior of the vgic.
>>
>> Some of this text was contributed by Marc Zyngier and edited by me.
>> Omissions and errors are all mine.
>>
>> Signed-off-by: Christoffer Dall 
>> ---
>>  Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 
>> +
>>  1 file changed, 181 insertions(+)
>>  create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>>
>> diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
>> b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>> new file mode 100644
>> index 000..24b6f28
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
>> @@ -0,0 +1,181 @@
>> +KVM/ARM VGIC Forwarded Physical Interrupts
>> +==

[...]

>> +1.  KVM runs the VCPU
>> +2.  The guest programs the time to fire in T+100
>> +4.  At T+100 the timer fires and a physical IRQ causes the VM to exit
>> +5.  With interrupts disabled on the CPU, KVM looks at the timer state
>> +and injects a forwarded physical interrupt because it concludes the
>> +timer has expired.
> I don't get how we can trap without the virtual timer PPI handler being
> entered on host side. Please can you elaborate on this?

On VM exit, we disable the virtual timer (see the code in
hyp.S::save_timer_state where we clear the enable bit). We still perform
the exit, but the cause for exit is now gone, and the handler will never
fire.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/8] arm/arm64: KVM: Add forwarded physical interrupts documentation

[Christoffer Dall]

Forwarded physical interrupts on arm/arm64 is a tricky concept and the
way we deal with them is not apparently easy to understand by reading
various specs.

Therefore, add a proper documentation file explaining the flow and
rationale of the behavior of the vgic.

Some of this text was contributed by Marc Zyngier and edited by me.
Omissions and errors are all mine.

Signed-off-by: Christoffer Dall 
---
 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 181 +
 1 file changed, 181 insertions(+)
 create mode 100644 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt

diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt 
b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
new file mode 100644
index 000..24b6f28
--- /dev/null
+++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
@@ -0,0 +1,181 @@
+KVM/ARM VGIC Forwarded Physical Interrupts
+==
+
+The KVM/ARM code implements software support for the ARM Generic
+Interrupt Controller's (GIC's) hardware support for virtualization by
+allowing software to inject virtual interrupts to a VM, which the guest
+OS sees as regular interrupts.  The code is famously known as the VGIC.
+
+Some of these virtual interrupts, however, correspond to physical
+interrupts from real physical devices.  One example could be the
+architected timer, which itself supports virtualization, and therefore
+lets a guest OS program the hardware device directly to raise an
+interrupt at some point in time.  When such an interrupt is raised, the
+host OS initially handles the interrupt and must somehow signal this
+event as a virtual interrupt to the guest.  Another example could be a
+passthrough device, where the physical interrupts are initially handled
+by the host, but the device driver for the device lives in the guest OS
+and KVM must therefore somehow inject a virtual interrupt on behalf of
+the physical one to the guest OS.
+
+These virtual interrupts corresponding to a physical interrupt on the
+host are called forwarded physical interrupts, but are also sometimes
+referred to as 'virtualized physical interrupts' and 'mapped interrupts'.
+
+Forwarded physical interrupts are handled slightly differently compared
+to virtual interrupts generated purely by a software emulated device.
+
+
+The HW bit
+--
+Virtual interrupts are signalled to the guest by programming the List
+Registers (LRs) on the GIC before running a VCPU.  The LR is programmed
+with the virtual IRQ number and the state of the interrupt (Pending,
+Active, or Pending+Active).  When the guest ACKs and EOIs a virtual
+interrupt, the LR state moves from Pending to Active, and finally to
+inactive.
+
+The LRs include an extra bit, called the HW bit.  When this bit is set,
+KVM must also program an additional field in the LR, the physical IRQ
+number, to link the virtual with the physical IRQ.
+
+When the HW bit is set, KVM must EITHER set the Pending OR the Active
+bit, never both at the same time.
+
+Setting the HW bit causes the hardware to deactivate the physical
+interrupt on the physical distributor when the guest deactivates the
+corresponding virtual interrupt.
+
+
+Forwarded Physical Interrupts Life Cycle
+
+
+The state of forwarded physical interrupts is managed in the following way:
+
+  - The physical interrupt is acked by the host, and becomes active on
+the physical distributor (*).
+  - KVM sets the LR.Pending bit, because this is the only way the GICV
+interface is going to present it to the guest.
+  - LR.Pending will stay set as long as the guest has not acked the interrupt.
+  - LR.Pending transitions to LR.Active on the guest read of the IAR, as
+expected.
+  - On guest EOI, the *physical distributor* active bit gets cleared,
+but the LR.Active is left untouched (set).
+  - KVM clears the LR when on VM exits when the physical distributor
+active state has been cleared.
+
+(*): The host handling is slightly more complicated.  For some devices
+(shared), KVM directly sets the active state on the physical distributor
+before entering the guest, and for some devices (non-shared) the host
+configures the GIC such that it does not deactivate the interrupt on
+host EOIs, but only performs a priority drop allowing the GIC to receive
+other interrupts and leaves the interrupt in the active state on the
+physical distributor.
+
+
+Forwarded Edge and Level Triggered PPIs and SPIs
+
+Forwarded physical interrupts injected should always be active on the
+physical distributor when injected to a guest.
+
+Level-triggered interrupts will keep the interrupt line to the GIC
+asserted, typically until the guest programs the device to deassert the
+line.  This means that the interrupt will remain pending on the physical
+distributor until the guest has reprogrammed the device.  Since we
+always run the VM