Re: [PATCH v1 2/4] KVM: arm/arm64: vgic: Improve comment on kvm_vgic_inject_irq

2019-02-26 Thread Leo Yan
On Fri, Feb 22, 2019 at 03:40:50PM +, Marc Zyngier wrote:

[...]

> > > The interrupt affinity is either defined by the distributor
> > > configuration (SPIs) or the ITS configuration (LPIs).
> > 
> > Given to the up example, I am struggling to understand how you can set
> > the interrupt affinity for virtio device.
> > 
> > Do you set the physical interrupt affinity to CPU0/1 in host OS and
> > forward it to guest OS?  Or set interrupt affinity in guest OS (I
> > tried on Juno board to set irq affinity in guest OS from
> > '/proc/irq/xxx/smp_affinity' but failed)?   Or this is accomplished by
> > user space tool (lkvm or qemu)?
> 
> virtio interrupts are purely virtual, and the host plays no part in
> their routing (nor does userspace). As for their affinity, that
> depends on the virtio driver. Some virtio devices allow their affinity
> to be changed, some don't. Here, this is a virtio-net device, which is
> perfectly happy to see its queue interrupts moved to a different vcpu.
> 
> I tend to run irqbalance in my guests so that it actually exercises
> the affinity setting in the background.
> 
> > Sorry if I am asking a stupid question :)
> 
> It's not stupid. You're simply confusing multiple independent layers.

Thanks a lot for explaination, Marc.

Thanks,
Leo Yan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v1 2/4] KVM: arm/arm64: vgic: Improve comment on kvm_vgic_inject_irq

2019-02-22 Thread Leo Yan
Hi Marc,

On Fri, Feb 22, 2019 at 08:37:56AM +, Marc Zyngier wrote:
> On Fri, 22 Feb 2019 16:23:24 +0800
> Leo Yan  wrote:
> 
> > The function kvm_vgic_inject_irq() is not only used by PPIs but also can
> > be used to inject interrupt for SPIs; this patch improves comment for
> > argument @cpuid to reflect support SPIs as well.
> > 
> > Signed-off-by: Leo Yan 
> > ---
> >  virt/kvm/arm/vgic/vgic.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > index 7cfdfbc910e0..79fe64c15051 100644
> > --- a/virt/kvm/arm/vgic/vgic.c
> > +++ b/virt/kvm/arm/vgic/vgic.c
> > @@ -394,7 +394,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct 
> > vgic_irq *irq,
> >  /**
> >   * kvm_vgic_inject_irq - Inject an IRQ from a device to the vgic
> >   * @kvm: The VM structure pointer
> > - * @cpuid:   The CPU for PPIs
> > + * @cpuid:   The CPU for PPIs and SPIs
> >   * @intid:   The INTID to inject a new state to.
> >   * @level:   Edge-triggered:  true:  to trigger the interrupt
> >   *   false: to ignore the call
> 
> What does the CPU mean for SPIs? By definition, the routing of an SPI
> is defined by the distributor configuration.

In the code, KVM injects PPIs by specifying CPU id, so that every PPI
is bound to specific target CPU.  But for SPIs, it always pass '0' for
cpuid, from my understanding this means VM will set interrupt affinity
to VCPU0 by default; in theory we also can set different cpuid for
SPIs so that the SPIs also can be handled by other secondary VCPUs;
this is why I think @cpuid also can be used by SPIs.

> And what about LPIs? SGIs?

TBH, I don't know LPIs and didn't use it before.

For SGIs, I read the code, it uses different path to handle SGI in KVM
so kvm_vgic_inject_irq() is not used for SGIs.

> I'm afraid you've misunderstood what cpuid is for.

Thanks for guidance.

Thanks,
Leo Yan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v1 2/4] KVM: arm/arm64: vgic: Improve comment on kvm_vgic_inject_irq

2019-02-22 Thread Leo Yan
On Fri, Feb 22, 2019 at 09:39:23AM +, Marc Zyngier wrote:

[...]

> > > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > > > index 7cfdfbc910e0..79fe64c15051 100644
> > > > --- a/virt/kvm/arm/vgic/vgic.c
> > > > +++ b/virt/kvm/arm/vgic/vgic.c
> > > > @@ -394,7 +394,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct 
> > > > vgic_irq *irq,
> > > >  /**
> > > >   * kvm_vgic_inject_irq - Inject an IRQ from a device to the vgic
> > > >   * @kvm: The VM structure pointer
> > > > - * @cpuid:   The CPU for PPIs
> > > > + * @cpuid:   The CPU for PPIs and SPIs
> > > >   * @intid:   The INTID to inject a new state to.
> > > >   * @level:   Edge-triggered:  true:  to trigger the interrupt
> > > >   *   false: to ignore the call  
> > > 
> > > What does the CPU mean for SPIs? By definition, the routing of an SPI
> > > is defined by the distributor configuration.  
> > 
> > In the code, KVM injects PPIs by specifying CPU id, so that every PPI
> > is bound to specific target CPU.  But for SPIs, it always pass '0' for
> > cpuid, from my understanding this means VM will set interrupt affinity
> > to VCPU0 by default; in theory we also can set different cpuid for
> > SPIs so that the SPIs also can be handled by other secondary VCPUs;
> > this is why I think @cpuid also can be used by SPIs.
> 
> SPIs are not hardcoded to vcpu0. This would be a gross violation of the
> architecture. To convince yourself of this, just run a guest:
> 
> root@unassigned-hostname:~# cat /proc/interrupts 
>CPU0   CPU1   
>   2:   7315   7353 GIC-0  27 Level arch_timer
>   4:158  0 GIC-0  33 Level uart-pl011
>  42:  0  0 GIC-0  23 Level arm-pmu
>  43:  0  0 pl061   3 Edge  ACPI:Event
>  44:  0  0   MSI 32768 Edge  virtio1-config
>  45:  10476  0   MSI 32769 Edge  virtio1-req.0
>  46:  0  0   MSI 16384 Edge  virtio0-config
>  47:  3 10   MSI 16385 Edge  virtio0-input.0
> [...]
> 
> On this last line, you can see an SPI being routed to both of these
> vcpus.
> 
> I urge you to read the code further, and understand that for any other
> interrupt class, the cpuid parameter is *ignored*. Yes, we pass zero in
> that case. We could also pass an approximation of PI with the same
> effect.

Very appreciate for the elaborated example; will read the code
furthermore.

> The interrupt affinity is either defined by the distributor
> configuration (SPIs) or the ITS configuration (LPIs).

Given to the up example, I am struggling to understand how you can set
the interrupt affinity for virtio device.

Do you set the physical interrupt affinity to CPU0/1 in host OS and
forward it to guest OS?  Or set interrupt affinity in guest OS (I
tried on Juno board to set irq affinity in guest OS from
'/proc/irq/xxx/smp_affinity' but failed)?   Or this is accomplished by
user space tool (lkvm or qemu)?

Sorry if I am asking a stupid question :)

Thanks,
Leo Yan
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v1 2/4] KVM: arm/arm64: vgic: Improve comment on kvm_vgic_inject_irq

2019-02-22 Thread Marc Zyngier
On Fri, 22 Feb 2019 12:49:06 +,
Leo Yan  wrote:
> 
> On Fri, Feb 22, 2019 at 09:39:23AM +, Marc Zyngier wrote:
> 
> [...]
> 
> > > > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > > > > index 7cfdfbc910e0..79fe64c15051 100644
> > > > > --- a/virt/kvm/arm/vgic/vgic.c
> > > > > +++ b/virt/kvm/arm/vgic/vgic.c
> > > > > @@ -394,7 +394,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, 
> > > > > struct vgic_irq *irq,
> > > > >  /**
> > > > >   * kvm_vgic_inject_irq - Inject an IRQ from a device to the vgic
> > > > >   * @kvm: The VM structure pointer
> > > > > - * @cpuid:   The CPU for PPIs
> > > > > + * @cpuid:   The CPU for PPIs and SPIs
> > > > >   * @intid:   The INTID to inject a new state to.
> > > > >   * @level:   Edge-triggered:  true:  to trigger the interrupt
> > > > >   * false: to ignore the call  
> > > > 
> > > > What does the CPU mean for SPIs? By definition, the routing of an SPI
> > > > is defined by the distributor configuration.  
> > > 
> > > In the code, KVM injects PPIs by specifying CPU id, so that every PPI
> > > is bound to specific target CPU.  But for SPIs, it always pass '0' for
> > > cpuid, from my understanding this means VM will set interrupt affinity
> > > to VCPU0 by default; in theory we also can set different cpuid for
> > > SPIs so that the SPIs also can be handled by other secondary VCPUs;
> > > this is why I think @cpuid also can be used by SPIs.
> > 
> > SPIs are not hardcoded to vcpu0. This would be a gross violation of the
> > architecture. To convince yourself of this, just run a guest:
> > 
> > root@unassigned-hostname:~# cat /proc/interrupts 
> >CPU0   CPU1   
> >   2:   7315   7353 GIC-0  27 Level arch_timer
> >   4:158  0 GIC-0  33 Level uart-pl011
> >  42:  0  0 GIC-0  23 Level arm-pmu
> >  43:  0  0 pl061   3 Edge  ACPI:Event
> >  44:  0  0   MSI 32768 Edge  virtio1-config
> >  45:  10476  0   MSI 32769 Edge  virtio1-req.0
> >  46:  0  0   MSI 16384 Edge  virtio0-config
> >  47:  3 10   MSI 16385 Edge  virtio0-input.0
> > [...]
> > 
> > On this last line, you can see an SPI being routed to both of these
> > vcpus.
> > 
> > I urge you to read the code further, and understand that for any other
> > interrupt class, the cpuid parameter is *ignored*. Yes, we pass zero in
> > that case. We could also pass an approximation of PI with the same
> > effect.
> 
> Very appreciate for the elaborated example; will read the code
> furthermore.
> 
> > The interrupt affinity is either defined by the distributor
> > configuration (SPIs) or the ITS configuration (LPIs).
> 
> Given to the up example, I am struggling to understand how you can set
> the interrupt affinity for virtio device.
> 
> Do you set the physical interrupt affinity to CPU0/1 in host OS and
> forward it to guest OS?  Or set interrupt affinity in guest OS (I
> tried on Juno board to set irq affinity in guest OS from
> '/proc/irq/xxx/smp_affinity' but failed)?   Or this is accomplished by
> user space tool (lkvm or qemu)?

virtio interrupts are purely virtual, and the host plays no part in
their routing (nor does userspace). As for their affinity, that
depends on the virtio driver. Some virtio devices allow their affinity
to be changed, some don't. Here, this is a virtio-net device, which is
perfectly happy to see its queue interrupts moved to a different vcpu.

I tend to run irqbalance in my guests so that it actually exercises
the affinity setting in the background.

> Sorry if I am asking a stupid question :)

It's not stupid. You're simply confusing multiple independent layers.

Thanks,

M.

-- 
Jazz is not dead, it just smell funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v1 2/4] KVM: arm/arm64: vgic: Improve comment on kvm_vgic_inject_irq

2019-02-22 Thread Marc Zyngier
On Fri, 22 Feb 2019 16:54:39 +0800
Leo Yan  wrote:

> Hi Marc,
> 
> On Fri, Feb 22, 2019 at 08:37:56AM +, Marc Zyngier wrote:
> > On Fri, 22 Feb 2019 16:23:24 +0800
> > Leo Yan  wrote:
> >   
> > > The function kvm_vgic_inject_irq() is not only used by PPIs but also can
> > > be used to inject interrupt for SPIs; this patch improves comment for
> > > argument @cpuid to reflect support SPIs as well.
> > > 
> > > Signed-off-by: Leo Yan 
> > > ---
> > >  virt/kvm/arm/vgic/vgic.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > > index 7cfdfbc910e0..79fe64c15051 100644
> > > --- a/virt/kvm/arm/vgic/vgic.c
> > > +++ b/virt/kvm/arm/vgic/vgic.c
> > > @@ -394,7 +394,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct 
> > > vgic_irq *irq,
> > >  /**
> > >   * kvm_vgic_inject_irq - Inject an IRQ from a device to the vgic
> > >   * @kvm: The VM structure pointer
> > > - * @cpuid:   The CPU for PPIs
> > > + * @cpuid:   The CPU for PPIs and SPIs
> > >   * @intid:   The INTID to inject a new state to.
> > >   * @level:   Edge-triggered:  true:  to trigger the interrupt
> > >   * false: to ignore the call  
> > 
> > What does the CPU mean for SPIs? By definition, the routing of an SPI
> > is defined by the distributor configuration.  
> 
> In the code, KVM injects PPIs by specifying CPU id, so that every PPI
> is bound to specific target CPU.  But for SPIs, it always pass '0' for
> cpuid, from my understanding this means VM will set interrupt affinity
> to VCPU0 by default; in theory we also can set different cpuid for
> SPIs so that the SPIs also can be handled by other secondary VCPUs;
> this is why I think @cpuid also can be used by SPIs.

SPIs are not hardcoded to vcpu0. This would be a gross violation of the
architecture. To convince yourself of this, just run a guest:

root@unassigned-hostname:~# cat /proc/interrupts 
   CPU0   CPU1   
  2:   7315   7353 GIC-0  27 Level arch_timer
  4:158  0 GIC-0  33 Level uart-pl011
 42:  0  0 GIC-0  23 Level arm-pmu
 43:  0  0 pl061   3 Edge  ACPI:Event
 44:  0  0   MSI 32768 Edge  virtio1-config
 45:  10476  0   MSI 32769 Edge  virtio1-req.0
 46:  0  0   MSI 16384 Edge  virtio0-config
 47:  3 10   MSI 16385 Edge  virtio0-input.0
[...]

On this last line, you can see an SPI being routed to both of these
vcpus.

I urge you to read the code further, and understand that for any other
interrupt class, the cpuid parameter is *ignored*. Yes, we pass zero in
that case. We could also pass an approximation of PI with the same
effect. The interrupt affinity is either defined by the distributor
configuration (SPIs) or the ITS configuration (LPIs).

M.
-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v1 2/4] KVM: arm/arm64: vgic: Improve comment on kvm_vgic_inject_irq

2019-02-22 Thread Marc Zyngier
On Fri, 22 Feb 2019 16:23:24 +0800
Leo Yan  wrote:

> The function kvm_vgic_inject_irq() is not only used by PPIs but also can
> be used to inject interrupt for SPIs; this patch improves comment for
> argument @cpuid to reflect support SPIs as well.
> 
> Signed-off-by: Leo Yan 
> ---
>  virt/kvm/arm/vgic/vgic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index 7cfdfbc910e0..79fe64c15051 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -394,7 +394,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct 
> vgic_irq *irq,
>  /**
>   * kvm_vgic_inject_irq - Inject an IRQ from a device to the vgic
>   * @kvm: The VM structure pointer
> - * @cpuid:   The CPU for PPIs
> + * @cpuid:   The CPU for PPIs and SPIs
>   * @intid:   The INTID to inject a new state to.
>   * @level:   Edge-triggered:  true:  to trigger the interrupt
>   * false: to ignore the call

What does the CPU mean for SPIs? By definition, the routing of an SPI
is defined by the distributor configuration. And what about LPIs? SGIs?

I'm afraid you've misunderstood what cpuid is for.

M.
-- 
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm