Re: [PATCH v4 16/19] KVM: Ensure validity of memslot with respect to kvm_get_dirty_log()

2019-12-24 Thread Peter Xu
On Tue, Dec 17, 2019 at 12:40:38PM -0800, Sean Christopherson wrote:
> +int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log,
> +   int *is_dirty, struct kvm_memory_slot **memslot)
>  {
>   struct kvm_memslots *slots;
> - struct kvm_memory_slot *memslot;
>   int i, as_id, id;
>   unsigned long n;
>   unsigned long any = 0;
>  
> + *memslot = NULL;
> + *is_dirty = 0;
> +
>   as_id = log->slot >> 16;
>   id = (u16)log->slot;
>   if (as_id >= KVM_ADDRESS_SPACE_NUM || id >= KVM_USER_MEM_SLOTS)
>   return -EINVAL;
>  
>   slots = __kvm_memslots(kvm, as_id);
> - memslot = id_to_memslot(slots, id);
> - if (!memslot->dirty_bitmap)
> + *memslot = id_to_memslot(slots, id);
> + if (!(*memslot)->dirty_bitmap)
>   return -ENOENT;
>  
> - n = kvm_dirty_bitmap_bytes(memslot);
> + kvm_arch_sync_dirty_log(kvm, *memslot);

Should this line belong to previous patch?

> +
> + n = kvm_dirty_bitmap_bytes(*memslot);
>  
>   for (i = 0; !any && i < n/sizeof(long); ++i)
> - any = memslot->dirty_bitmap[i];
> + any = (*memslot)->dirty_bitmap[i];
>  
> - if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
> + if (copy_to_user(log->dirty_bitmap, (*memslot)->dirty_bitmap, n))
>   return -EFAULT;
>  
>   if (any)
> -- 
> 2.24.1

-- 
Peter Xu

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 08/18] arm64: KVM: add support to save/restore SPE profiling buffer controls

2019-12-24 Thread Marc Zyngier
On Tue, 24 Dec 2019 15:17:39 +,
Andrew Murray  wrote:
> 
> On Tue, Dec 24, 2019 at 10:49:30AM +, Andrew Murray wrote:
> > On Sat, Dec 21, 2019 at 01:57:55PM +, Marc Zyngier wrote:
> > > On Fri, 20 Dec 2019 14:30:15 +
> > > Andrew Murray  wrote:
> > > 
> > > > From: Sudeep Holla 
> > > > 
> > > > Currently since we don't support profiling using SPE in the guests,
> > > > we just save the PMSCR_EL1, flush the profiling buffers and disable
> > > > sampling. However in order to support simultaneous sampling both in
> > > 
> > > Is the sampling actually simultaneous? I don't believe so (the whole
> > > series would be much simpler if it was).
> > 
> > No the SPE is used by either the guest or host at any one time. I guess
> > the term simultaneous was used to refer to illusion given to both guest
> > and host that they are able to use it whenever they like. I'll update
> > the commit message to drop the magic.
> >  
> > 
> > > 
> > > > the host and guests, we need to save and reatore the complete SPE
> > > 
> > > s/reatore/restore/
> > 
> > Noted.
> > 
> > 
> > > 
> > > > profiling buffer controls' context.
> > > > 
> > > > Let's add the support for the same and keep it disabled for now.
> > > > We can enable it conditionally only if guests are allowed to use
> > > > SPE.
> > > > 
> > > > Signed-off-by: Sudeep Holla 
> > > > [ Clear PMBSR bit when saving state to prevent spurious interrupts ]
> > > > Signed-off-by: Andrew Murray 
> > > > ---
> > > >  arch/arm64/kvm/hyp/debug-sr.c | 51 +--
> > > >  1 file changed, 43 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/kvm/hyp/debug-sr.c 
> > > > b/arch/arm64/kvm/hyp/debug-sr.c
> > > > index 8a70a493345e..12429b212a3a 100644
> > > > --- a/arch/arm64/kvm/hyp/debug-sr.c
> > > > +++ b/arch/arm64/kvm/hyp/debug-sr.c
> > > > @@ -85,7 +85,8 @@
> > > > default:write_debug(ptr[0], reg, 0);
> > > > \
> > > > }
> > > >  
> > > > -static void __hyp_text __debug_save_spe_nvhe(struct kvm_cpu_context 
> > > > *ctxt)
> > > > +static void __hyp_text
> > > > +__debug_save_spe_nvhe(struct kvm_cpu_context *ctxt, bool full_ctxt)
> > > 
> > > nit: don't split lines like this if you can avoid it. You can put the
> > > full_ctxt parameter on a separate line instead.
> > 
> > Yes understood.
> > 
> > 
> > > 
> > > >  {
> > > > u64 reg;
> > > >  
> > > > @@ -102,22 +103,46 @@ static void __hyp_text 
> > > > __debug_save_spe_nvhe(struct kvm_cpu_context *ctxt)
> > > > if (reg & BIT(SYS_PMBIDR_EL1_P_SHIFT))
> > > > return;
> > > >  
> > > > -   /* No; is the host actually using the thing? */
> > > > -   reg = read_sysreg_s(SYS_PMBLIMITR_EL1);
> > > > -   if (!(reg & BIT(SYS_PMBLIMITR_EL1_E_SHIFT)))
> > > > +   /* Save the control register and disable data generation */
> > > > +   ctxt->sys_regs[PMSCR_EL1] = read_sysreg_el1(SYS_PMSCR);
> > > > +
> > > > +   if (!ctxt->sys_regs[PMSCR_EL1])
> > > 
> > > Shouldn't you check the enable bits instead of relying on the whole
> > > thing being zero?
> > 
> > Yes that would make more sense (E1SPE and E0SPE).
> > 
> > I feel that this check makes an assumption about the guest/host SPE
> > driver... What happens if the SPE driver writes to some SPE registers
> > but doesn't enable PMSCR? If the guest is also using SPE then those
> > writes will be lost, when the host returns and the SPE driver enables
> > SPE it won't work.
> >
> > With a quick look at the SPE driver I'm not sure this will happen, but
> > even so it makes me nervous relying on these assumptions. I wonder if
> > this risk is present in other devices?

As a rule of thumb, you should always save whatever you're about to
overwrite if the registers are not under exclusive control of KVM. No
exception.

So if the guest is willing to use SPE *and* that it isn't enabled on
the host, these registers have to be saved on vcpu_load() and restored
on vcpu_put().

If SPE is enabled on the host, then trapping has to be enabled, and no
tracing occurs in the guest for this time slice.

> In fact, this may be a good reason to trap the SPE registers - this would
> allow you to conditionally save/restore based on a dirty bit. It would
> also allow you to re-evaluate the SPE interrupt (for example when the guest
> clears the status register) and thus potentially reduce any black hole.

I don't see what trapping buys you in the expected case (where the
guest is tracing and the host isn't). To clear PMBSR_EL1.S, you first
need to know that an interrupt has fired. So this brings you exactly
nothing in this particular case, and just adds overhead for everything
else. The whole point of the architecture is that in the non-contended
case, we can give SPE to the guest and mostly forget about it.

I strongly suggest that you start with the simplest possible, non
broken implementation. It doesn't matter if the black holes last for
seconds for now. Onc

Re: [PATCH v2 08/18] arm64: KVM: add support to save/restore SPE profiling buffer controls

2019-12-24 Thread Andrew Murray
On Tue, Dec 24, 2019 at 10:49:30AM +, Andrew Murray wrote:
> On Sat, Dec 21, 2019 at 01:57:55PM +, Marc Zyngier wrote:
> > On Fri, 20 Dec 2019 14:30:15 +
> > Andrew Murray  wrote:
> > 
> > > From: Sudeep Holla 
> > > 
> > > Currently since we don't support profiling using SPE in the guests,
> > > we just save the PMSCR_EL1, flush the profiling buffers and disable
> > > sampling. However in order to support simultaneous sampling both in
> > 
> > Is the sampling actually simultaneous? I don't believe so (the whole
> > series would be much simpler if it was).
> 
> No the SPE is used by either the guest or host at any one time. I guess
> the term simultaneous was used to refer to illusion given to both guest
> and host that they are able to use it whenever they like. I'll update
> the commit message to drop the magic.
>  
> 
> > 
> > > the host and guests, we need to save and reatore the complete SPE
> > 
> > s/reatore/restore/
> 
> Noted.
> 
> 
> > 
> > > profiling buffer controls' context.
> > > 
> > > Let's add the support for the same and keep it disabled for now.
> > > We can enable it conditionally only if guests are allowed to use
> > > SPE.
> > > 
> > > Signed-off-by: Sudeep Holla 
> > > [ Clear PMBSR bit when saving state to prevent spurious interrupts ]
> > > Signed-off-by: Andrew Murray 
> > > ---
> > >  arch/arm64/kvm/hyp/debug-sr.c | 51 +--
> > >  1 file changed, 43 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
> > > index 8a70a493345e..12429b212a3a 100644
> > > --- a/arch/arm64/kvm/hyp/debug-sr.c
> > > +++ b/arch/arm64/kvm/hyp/debug-sr.c
> > > @@ -85,7 +85,8 @@
> > >   default:write_debug(ptr[0], reg, 0);\
> > >   }
> > >  
> > > -static void __hyp_text __debug_save_spe_nvhe(struct kvm_cpu_context 
> > > *ctxt)
> > > +static void __hyp_text
> > > +__debug_save_spe_nvhe(struct kvm_cpu_context *ctxt, bool full_ctxt)
> > 
> > nit: don't split lines like this if you can avoid it. You can put the
> > full_ctxt parameter on a separate line instead.
> 
> Yes understood.
> 
> 
> > 
> > >  {
> > >   u64 reg;
> > >  
> > > @@ -102,22 +103,46 @@ static void __hyp_text __debug_save_spe_nvhe(struct 
> > > kvm_cpu_context *ctxt)
> > >   if (reg & BIT(SYS_PMBIDR_EL1_P_SHIFT))
> > >   return;
> > >  
> > > - /* No; is the host actually using the thing? */
> > > - reg = read_sysreg_s(SYS_PMBLIMITR_EL1);
> > > - if (!(reg & BIT(SYS_PMBLIMITR_EL1_E_SHIFT)))
> > > + /* Save the control register and disable data generation */
> > > + ctxt->sys_regs[PMSCR_EL1] = read_sysreg_el1(SYS_PMSCR);
> > > +
> > > + if (!ctxt->sys_regs[PMSCR_EL1])
> > 
> > Shouldn't you check the enable bits instead of relying on the whole
> > thing being zero?
> 
> Yes that would make more sense (E1SPE and E0SPE).
> 
> I feel that this check makes an assumption about the guest/host SPE
> driver... What happens if the SPE driver writes to some SPE registers
> but doesn't enable PMSCR? If the guest is also using SPE then those
> writes will be lost, when the host returns and the SPE driver enables
> SPE it won't work.
> 
> With a quick look at the SPE driver I'm not sure this will happen, but
> even so it makes me nervous relying on these assumptions. I wonder if
> this risk is present in other devices?

In fact, this may be a good reason to trap the SPE registers - this would
allow you to conditionally save/restore based on a dirty bit. It would
also allow you to re-evaluate the SPE interrupt (for example when the guest
clears the status register) and thus potentially reduce any black hole.

Thanks,

Andrew Murray

> 
> 
> > 
> > >   return;
> > >  
> > >   /* Yes; save the control register and disable data generation */
> > > - ctxt->sys_regs[PMSCR_EL1] = read_sysreg_el1(SYS_PMSCR);
> > 
> > You've already saved the control register...
> 
> I'll remove that.
> 
> 
> > 
> > >   write_sysreg_el1(0, SYS_PMSCR);
> > >   isb();
> > >  
> > >   /* Now drain all buffered data to memory */
> > >   psb_csync();
> > >   dsb(nsh);
> > > +
> > > + if (!full_ctxt)
> > > + return;
> > > +
> > > + ctxt->sys_regs[PMBLIMITR_EL1] = read_sysreg_s(SYS_PMBLIMITR_EL1);
> > > + write_sysreg_s(0, SYS_PMBLIMITR_EL1);
> > > +
> > > + /*
> > > +  * As PMBSR is conditionally restored when returning to the host we
> > > +  * must ensure the service bit is unset here to prevent a spurious
> > > +  * host SPE interrupt from being raised.
> > > +  */
> > > + ctxt->sys_regs[PMBSR_EL1] = read_sysreg_s(SYS_PMBSR_EL1);
> > > + write_sysreg_s(0, SYS_PMBSR_EL1);
> > > +
> > > + isb();
> > > +
> > > + ctxt->sys_regs[PMSICR_EL1] = read_sysreg_s(SYS_PMSICR_EL1);
> > > + ctxt->sys_regs[PMSIRR_EL1] = read_sysreg_s(SYS_PMSIRR_EL1);
> > > + ctxt->sys_regs[PMSFCR_EL1] = read_sysreg_s(SYS_PMSFCR_EL1);
> > > + ctxt->sys_regs[PMSEVFR_EL1] = read_sysreg_s(SYS_PMSEVFR_EL1);
> > > + ctxt->sys_regs[PMSLATFR_EL1] = read_sysreg_s(SYS

Re: [PATCH v2 14/18] KVM: arm64: spe: Provide guest virtual interrupts for SPE

2019-12-24 Thread Marc Zyngier

On 2019-12-24 13:36, Andrew Murray wrote:

On Tue, Dec 24, 2019 at 01:22:46PM +, Marc Zyngier wrote:

On 2019-12-24 13:08, Andrew Murray wrote:


[...]

> This does feel like the pragmatic approach - a larger black hole 
in

> exchange
> for performance. I imagine the blackhole would be naturally 
reduced on

> machines with high workloads.

Why? I don't see the relation between how busy the vcpu is and the 
size
of the blackhole. It is strictly a function of the frequency of 
exits.


Indeed, my assumption being that the busier a system is the more
interrupts, thus leading to more exits and so an increased frequency 
of

SPE interrupt evaluation and thus smaller black hole.


On a GICv4-enabled system, this isn't true anymore. My bet is that
people won't use SPE to optimize IO-oriented workloads, but more CPU
intensive workloads (that don't necessarily exit at all).

But never mind. Let's start with this approach, as it is simple and 
easy

to verify. If the black hole aspect becomes problematic, we know how
to reduce it (at the expense of entry/exit performance).

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 14/18] KVM: arm64: spe: Provide guest virtual interrupts for SPE

2019-12-24 Thread Andrew Murray
On Tue, Dec 24, 2019 at 01:22:46PM +, Marc Zyngier wrote:
> On 2019-12-24 13:08, Andrew Murray wrote:
> > On Tue, Dec 24, 2019 at 12:42:02PM +, Marc Zyngier wrote:
> > > On 2019-12-24 11:50, Andrew Murray wrote:
> > > > On Sun, Dec 22, 2019 at 12:07:50PM +, Marc Zyngier wrote:
> > > > > On Fri, 20 Dec 2019 14:30:21 +,
> > > > > Andrew Murray  wrote:
> > > > > >
> > > > > > Upon the exit of a guest, let's determine if the SPE device
> > > has
> > > > > generated
> > > > > > an interrupt - if so we'll inject a virtual interrupt to the
> > > > > guest.
> > > > > >
> > > > > > Upon the entry and exit of a guest we'll also update the state
> > > of
> > > > > the
> > > > > > physical IRQ such that it is active when a guest interrupt is
> > > > > pending
> > > > > > and the guest is running.
> > > > > >
> > > > > > Finally we map the physical IRQ to the virtual IRQ such that
> > > the
> > > > > guest
> > > > > > can deactivate the interrupt when it handles the interrupt.
> > > > > >
> > > > > > Signed-off-by: Andrew Murray 
> > > > > > ---
> > > > > >  include/kvm/arm_spe.h |  6 
> > > > > >  virt/kvm/arm/arm.c|  5 ++-
> > > > > >  virt/kvm/arm/spe.c| 71
> > > > > +++
> > > > > >  3 files changed, 81 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > > > > > index 9c65130d726d..91b2214f543a 100644
> > > > > > --- a/include/kvm/arm_spe.h
> > > > > > +++ b/include/kvm/arm_spe.h
> > > > > > @@ -37,6 +37,9 @@ static inline bool
> > > kvm_arm_support_spe_v1(void)
> > > > > >   
> > > > > > ID_AA64DFR0_PMSVER_SHIFT);
> > > > > >  }
> > > > > >
> > > > > > +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu);
> > > > > > +inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu);
> > > > > > +
> > > > > >  int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > > > > > struct kvm_device_attr *attr);
> > > > > >  int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> > > > > > @@ -49,6 +52,9 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu
> > > > > *vcpu);
> > > > > >  #define kvm_arm_support_spe_v1()   (false)
> > > > > >  #define kvm_arm_spe_irq_initialized(v) (false)
> > > > > >
> > > > > > +static inline void kvm_spe_flush_hwstate(struct kvm_vcpu
> > > *vcpu)
> > > > > {}
> > > > > > +static inline void kvm_spe_sync_hwstate(struct kvm_vcpu
> > > *vcpu) {}
> > > > > > +
> > > > > >  static inline int kvm_arm_spe_v1_set_attr(struct kvm_vcpu
> > > *vcpu,
> > > > > >   struct kvm_device_attr *attr)
> > > > > >  {
> > > > > > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > > > > > index 340d2388ee2c..a66085c8e785 100644
> > > > > > --- a/virt/kvm/arm/arm.c
> > > > > > +++ b/virt/kvm/arm/arm.c
> > > > > > @@ -741,6 +741,7 @@ int kvm_arch_vcpu_ioctl_run(struct
> > > kvm_vcpu
> > > > > *vcpu, struct kvm_run *run)
> > > > > > preempt_disable();
> > > > > >
> > > > > > kvm_pmu_flush_hwstate(vcpu);
> > > > > > +   kvm_spe_flush_hwstate(vcpu);
> > > > > >
> > > > > > local_irq_disable();
> > > > > >
> > > > > > @@ -782,6 +783,7 @@ int kvm_arch_vcpu_ioctl_run(struct
> > > kvm_vcpu
> > > > > *vcpu, struct kvm_run *run)
> > > > > > kvm_request_pending(vcpu)) {
> > > > > > vcpu->mode = OUTSIDE_GUEST_MODE;
> > > > > > isb(); /* Ensure work in x_flush_hwstate is 
> > > > > > committed */
> > > > > > +   kvm_spe_sync_hwstate(vcpu);
> > > > > > kvm_pmu_sync_hwstate(vcpu);
> > > > > > if 
> > > > > > (static_branch_unlikely(&userspace_irqchip_in_use))
> > > > > > kvm_timer_sync_hwstate(vcpu);
> > > > > > @@ -816,11 +818,12 @@ int kvm_arch_vcpu_ioctl_run(struct
> > > kvm_vcpu
> > > > > *vcpu, struct kvm_run *run)
> > > > > > kvm_arm_clear_debug(vcpu);
> > > > > >
> > > > > > /*
> > > > > > -* We must sync the PMU state before the vgic state so
> > > > > > +* We must sync the PMU and SPE state before the vgic 
> > > > > > state
> > > so
> > > > > >  * that the vgic can properly sample the updated state 
> > > > > > of
> > > the
> > > > > >  * interrupt line.
> > > > > >  */
> > > > > > kvm_pmu_sync_hwstate(vcpu);
> > > > > > +   kvm_spe_sync_hwstate(vcpu);
> > > > >
> > > > > The *HUGE* difference is that the PMU is purely a virtual
> > > interrupt,
> > > > > while you're trying to deal with a HW interrupt here.
> > > > >
> > > > > >
> > > > > > /*
> > > > > >  * Sync the vgic state before syncing the timer state
> > > because
> > > > > > diff --git a/virt/kvm/arm/spe.c b/virt/kvm/arm/spe.c
> > > > > > index 83ac2cce2cc3..097ed39014e4 100644
> > > > > > --- a/virt/kvm/arm/spe.c
> > > > > > +++ 

Re: [PATCH v2 14/18] KVM: arm64: spe: Provide guest virtual interrupts for SPE

2019-12-24 Thread Marc Zyngier

On 2019-12-24 13:08, Andrew Murray wrote:

On Tue, Dec 24, 2019 at 12:42:02PM +, Marc Zyngier wrote:

On 2019-12-24 11:50, Andrew Murray wrote:
> On Sun, Dec 22, 2019 at 12:07:50PM +, Marc Zyngier wrote:
> > On Fri, 20 Dec 2019 14:30:21 +,
> > Andrew Murray  wrote:
> > >
> > > Upon the exit of a guest, let's determine if the SPE device 
has

> > generated
> > > an interrupt - if so we'll inject a virtual interrupt to the
> > guest.
> > >
> > > Upon the entry and exit of a guest we'll also update the state 
of

> > the
> > > physical IRQ such that it is active when a guest interrupt is
> > pending
> > > and the guest is running.
> > >
> > > Finally we map the physical IRQ to the virtual IRQ such that 
the

> > guest
> > > can deactivate the interrupt when it handles the interrupt.
> > >
> > > Signed-off-by: Andrew Murray 
> > > ---
> > >  include/kvm/arm_spe.h |  6 
> > >  virt/kvm/arm/arm.c|  5 ++-
> > >  virt/kvm/arm/spe.c| 71
> > +++
> > >  3 files changed, 81 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > > index 9c65130d726d..91b2214f543a 100644
> > > --- a/include/kvm/arm_spe.h
> > > +++ b/include/kvm/arm_spe.h
> > > @@ -37,6 +37,9 @@ static inline bool 
kvm_arm_support_spe_v1(void)

> > >
ID_AA64DFR0_PMSVER_SHIFT);
> > >  }
> > >
> > > +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu);
> > > +inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu);
> > > +
> > >  int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > >  struct kvm_device_attr *attr);
> > >  int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> > > @@ -49,6 +52,9 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu
> > *vcpu);
> > >  #define kvm_arm_support_spe_v1()(false)
> > >  #define kvm_arm_spe_irq_initialized(v)  (false)
> > >
> > > +static inline void kvm_spe_flush_hwstate(struct kvm_vcpu 
*vcpu)

> > {}
> > > +static inline void kvm_spe_sync_hwstate(struct kvm_vcpu 
*vcpu) {}

> > > +
> > >  static inline int kvm_arm_spe_v1_set_attr(struct kvm_vcpu 
*vcpu,

> > >struct kvm_device_attr *attr)
> > >  {
> > > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > > index 340d2388ee2c..a66085c8e785 100644
> > > --- a/virt/kvm/arm/arm.c
> > > +++ b/virt/kvm/arm/arm.c
> > > @@ -741,6 +741,7 @@ int kvm_arch_vcpu_ioctl_run(struct 
kvm_vcpu

> > *vcpu, struct kvm_run *run)
> > >  preempt_disable();
> > >
> > >  kvm_pmu_flush_hwstate(vcpu);
> > > +kvm_spe_flush_hwstate(vcpu);
> > >
> > >  local_irq_disable();
> > >
> > > @@ -782,6 +783,7 @@ int kvm_arch_vcpu_ioctl_run(struct 
kvm_vcpu

> > *vcpu, struct kvm_run *run)
> > >  kvm_request_pending(vcpu)) {
> > >  vcpu->mode = OUTSIDE_GUEST_MODE;
> > >  isb(); /* Ensure work in x_flush_hwstate is 
committed */
> > > +kvm_spe_sync_hwstate(vcpu);
> > >  kvm_pmu_sync_hwstate(vcpu);
> > >  if 
(static_branch_unlikely(&userspace_irqchip_in_use))
> > >  kvm_timer_sync_hwstate(vcpu);
> > > @@ -816,11 +818,12 @@ int kvm_arch_vcpu_ioctl_run(struct 
kvm_vcpu

> > *vcpu, struct kvm_run *run)
> > >  kvm_arm_clear_debug(vcpu);
> > >
> > >  /*
> > > - * We must sync the PMU state before the vgic state so
> > > +		 * We must sync the PMU and SPE state before the vgic state 
so
> > >  		 * that the vgic can properly sample the updated state of 
the

> > >   * interrupt line.
> > >   */
> > >  kvm_pmu_sync_hwstate(vcpu);
> > > +kvm_spe_sync_hwstate(vcpu);
> >
> > The *HUGE* difference is that the PMU is purely a virtual 
interrupt,

> > while you're trying to deal with a HW interrupt here.
> >
> > >
> > >  /*
> > >  		 * Sync the vgic state before syncing the timer state 
because

> > > diff --git a/virt/kvm/arm/spe.c b/virt/kvm/arm/spe.c
> > > index 83ac2cce2cc3..097ed39014e4 100644
> > > --- a/virt/kvm/arm/spe.c
> > > +++ b/virt/kvm/arm/spe.c
> > > @@ -35,6 +35,68 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu
> > *vcpu)
> > >  return 0;
> > >  }
> > >
> > > +static inline void set_spe_irq_phys_active(struct
> > arm_spe_kvm_info *info,
> > > +   bool active)
> > > +{
> > > +int r;
> > > +r = irq_set_irqchip_state(info->physical_irq,
> > IRQCHIP_STATE_ACTIVE,
> > > +  active);
> > > +WARN_ON(r);
> > > +}
> > > +
> > > +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu)
> > > +{
> > > +struct kvm_spe *spe = &vcpu->arch.spe;
> > > +bool phys_active = false;
> > > +struct arm_spe_kvm

Re: [PATCH v2 14/18] KVM: arm64: spe: Provide guest virtual interrupts for SPE

2019-12-24 Thread Andrew Murray
On Tue, Dec 24, 2019 at 12:42:02PM +, Marc Zyngier wrote:
> On 2019-12-24 11:50, Andrew Murray wrote:
> > On Sun, Dec 22, 2019 at 12:07:50PM +, Marc Zyngier wrote:
> > > On Fri, 20 Dec 2019 14:30:21 +,
> > > Andrew Murray  wrote:
> > > >
> > > > Upon the exit of a guest, let's determine if the SPE device has
> > > generated
> > > > an interrupt - if so we'll inject a virtual interrupt to the
> > > guest.
> > > >
> > > > Upon the entry and exit of a guest we'll also update the state of
> > > the
> > > > physical IRQ such that it is active when a guest interrupt is
> > > pending
> > > > and the guest is running.
> > > >
> > > > Finally we map the physical IRQ to the virtual IRQ such that the
> > > guest
> > > > can deactivate the interrupt when it handles the interrupt.
> > > >
> > > > Signed-off-by: Andrew Murray 
> > > > ---
> > > >  include/kvm/arm_spe.h |  6 
> > > >  virt/kvm/arm/arm.c|  5 ++-
> > > >  virt/kvm/arm/spe.c| 71
> > > +++
> > > >  3 files changed, 81 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > > > index 9c65130d726d..91b2214f543a 100644
> > > > --- a/include/kvm/arm_spe.h
> > > > +++ b/include/kvm/arm_spe.h
> > > > @@ -37,6 +37,9 @@ static inline bool kvm_arm_support_spe_v1(void)
> > > >   
> > > > ID_AA64DFR0_PMSVER_SHIFT);
> > > >  }
> > > >
> > > > +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu);
> > > > +inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu);
> > > > +
> > > >  int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > > > struct kvm_device_attr *attr);
> > > >  int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> > > > @@ -49,6 +52,9 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu
> > > *vcpu);
> > > >  #define kvm_arm_support_spe_v1()   (false)
> > > >  #define kvm_arm_spe_irq_initialized(v) (false)
> > > >
> > > > +static inline void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu)
> > > {}
> > > > +static inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu) {}
> > > > +
> > > >  static inline int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > > >   struct kvm_device_attr *attr)
> > > >  {
> > > > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > > > index 340d2388ee2c..a66085c8e785 100644
> > > > --- a/virt/kvm/arm/arm.c
> > > > +++ b/virt/kvm/arm/arm.c
> > > > @@ -741,6 +741,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
> > > *vcpu, struct kvm_run *run)
> > > > preempt_disable();
> > > >
> > > > kvm_pmu_flush_hwstate(vcpu);
> > > > +   kvm_spe_flush_hwstate(vcpu);
> > > >
> > > > local_irq_disable();
> > > >
> > > > @@ -782,6 +783,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
> > > *vcpu, struct kvm_run *run)
> > > > kvm_request_pending(vcpu)) {
> > > > vcpu->mode = OUTSIDE_GUEST_MODE;
> > > > isb(); /* Ensure work in x_flush_hwstate is 
> > > > committed */
> > > > +   kvm_spe_sync_hwstate(vcpu);
> > > > kvm_pmu_sync_hwstate(vcpu);
> > > > if 
> > > > (static_branch_unlikely(&userspace_irqchip_in_use))
> > > > kvm_timer_sync_hwstate(vcpu);
> > > > @@ -816,11 +818,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu
> > > *vcpu, struct kvm_run *run)
> > > > kvm_arm_clear_debug(vcpu);
> > > >
> > > > /*
> > > > -* We must sync the PMU state before the vgic state so
> > > > +* We must sync the PMU and SPE state before the vgic 
> > > > state so
> > > >  * that the vgic can properly sample the updated state 
> > > > of the
> > > >  * interrupt line.
> > > >  */
> > > > kvm_pmu_sync_hwstate(vcpu);
> > > > +   kvm_spe_sync_hwstate(vcpu);
> > > 
> > > The *HUGE* difference is that the PMU is purely a virtual interrupt,
> > > while you're trying to deal with a HW interrupt here.
> > > 
> > > >
> > > > /*
> > > >  * Sync the vgic state before syncing the timer state 
> > > > because
> > > > diff --git a/virt/kvm/arm/spe.c b/virt/kvm/arm/spe.c
> > > > index 83ac2cce2cc3..097ed39014e4 100644
> > > > --- a/virt/kvm/arm/spe.c
> > > > +++ b/virt/kvm/arm/spe.c
> > > > @@ -35,6 +35,68 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu
> > > *vcpu)
> > > > return 0;
> > > >  }
> > > >
> > > > +static inline void set_spe_irq_phys_active(struct
> > > arm_spe_kvm_info *info,
> > > > +  bool active)
> > > > +{
> > > > +   int r;
> > > > +   r = irq_set_irqchip_state(info->physical_irq,
> > > IRQCHIP_STATE_ACTIVE,
> > > > + active);
> > > > +   WARN_ON

Re: [PATCH v2 00/18] arm64: KVM: add SPE profiling support

2019-12-24 Thread Andrew Murray
On Sun, Dec 22, 2019 at 12:22:10PM +, Marc Zyngier wrote:
> On Sat, 21 Dec 2019 10:48:16 +,
> Marc Zyngier  wrote:
> > 
> > [fixing email addresses]
> > 
> > Hi Andrew,
> > 
> > On 2019-12-20 14:30, Andrew Murray wrote:
> > > This series implements support for allowing KVM guests to use the Arm
> > > Statistical Profiling Extension (SPE).
> > 
> > Thanks for this. In future, please Cc me and Will on email addresses
> > we can actually read.
> > 
> > > It has been tested on a model to ensure that both host and guest can
> > > simultaneously use SPE with valid data. E.g.
> > > 
> > > $ perf record -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
> > > dd if=/dev/zero of=/dev/null count=1000
> > > $ perf report --dump-raw-trace > spe_buf.txt
> > > 
> > > As we save and restore the SPE context, the guest can access the SPE
> > > registers directly, thus in this version of the series we remove the
> > > trapping and emulation.
> > > 
> > > In the previous series of this support, when KVM SPE isn't
> > > supported (e.g. via CONFIG_KVM_ARM_SPE) we were able to return a
> > > value of 0 to all reads of the SPE registers - as we can no longer
> > > do this there isn't a mechanism to prevent the guest from using
> > > SPE - thus I'm keen for feedback on the best way of resolving
> > > this.
> > 
> > Surely there is a way to conditionally trap SPE registers, right? You
> > should still be able to do this if SPE is not configured for a given
> > guest (as we do for other feature such as PtrAuth).
> > 
> > > It appears necessary to pin the entire guest memory in order to
> > > provide guest SPE access - otherwise it is possible for the guest
> > > to receive Stage-2 faults.
> > 
> > Really? How can the guest receive a stage-2 fault? This doesn't fit
> > what I understand of the ARMv8 exception model. Or do you mean a SPE
> > interrupt describing a S2 fault?

Yes the latter.


> > 
> > And this is not just pinning the memory either. You have to ensure that
> > all S2 page tables are created ahead of SPE being able to DMA to guest
> > memory. This may have some impacts on the THP code...
> > 
> > I'll have a look at the actual series ASAP (but that's not very soon).
> 
> I found some time to go through the series, and there is clearly a lot
> of work left to do:
> 
> - There so nothing here to handle memory pinning whatsoever. If it
>   works, it is only thanks to some side effect.
> 
> - The missing trapping is deeply worrying. Given that this is an
>   optional feature, you cannot just let the guest do whatever it wants
>   in an uncontrolled manner.

Yes I'll add this.


> 
> - The interrupt handling is busted. You mix concepts picked from both
>   the PMU and the timer code, while the SPE device doesn't behave like
>   any of these two (it is neither a fully emulated device, nor a
>   device that is exclusively owned by a guest at any given time).
> 
> I expect some level of discussion on the list including at least Will
> and myself before you respin this.

Thanks for the quick feedback.

Andrew Murray

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


Re: [PATCH v2 00/18] arm64: KVM: add SPE profiling support

2019-12-24 Thread Andrew Murray
On Fri, Dec 20, 2019 at 05:55:25PM +, Mark Rutland wrote:
> Hi Andrew,
> 
> On Fri, Dec 20, 2019 at 02:30:07PM +, Andrew Murray wrote:
> > This series implements support for allowing KVM guests to use the Arm
> > Statistical Profiling Extension (SPE).
> > 
> > It has been tested on a model to ensure that both host and guest can
> > simultaneously use SPE with valid data. E.g.
> > 
> > $ perf record -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
> > dd if=/dev/zero of=/dev/null count=1000
> > $ perf report --dump-raw-trace > spe_buf.txt
> 
> What happens if I run perf record on the VMM, or on the CPU(s) that the
> VMM is running on? i.e.
> 
> $ perf record -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
> lkvm ${OPTIONS_FOR_GUEST_USING_SPE}
> 

By default perf excludes the guest, so this works as expected, just recording
activity of the process when it is outside the guest. (perf report appears
to give valid output).

Patch 15 currently prevents using perf to record inside the guest.


> ... or:
> 
> $ perf record -a -c 0 -e arm_spe/ts_enable=1,pa_enable=1,pct_enable=1/ \
> sleep 1000 &
> $ taskset -c 0 lkvm ${OPTIONS_FOR_GUEST_USING_SPE} &
> 
> > As we save and restore the SPE context, the guest can access the SPE
> > registers directly, thus in this version of the series we remove the
> > trapping and emulation.
> > 
> > In the previous series of this support, when KVM SPE isn't supported
> > (e.g. via CONFIG_KVM_ARM_SPE) we were able to return a value of 0 to
> > all reads of the SPE registers - as we can no longer do this there isn't
> > a mechanism to prevent the guest from using SPE - thus I'm keen for
> > feedback on the best way of resolving this.
> 
> When not providing SPE to the guest, surely we should be trapping the
> registers and injecting an UNDEF?

Yes we should, I'll update the series.


> 
> What happens today, without these patches?
> 

Prior to this series MDCR_EL2_TPMS is set and E2PB is unset resulting in all
SPE registers being trapped (with NULL handlers).


> > It appears necessary to pin the entire guest memory in order to provide
> > guest SPE access - otherwise it is possible for the guest to receive
> > Stage-2 faults.
> 
> AFAICT these patches do not implement this. I assume that's what you're
> trying to point out here, but I just want to make sure that's explicit.

That's right.


> 
> Maybe this is a reason to trap+emulate if there's something more
> sensible that hyp can do if it sees a Stage-2 fault.

Yes it's not really clear to me at the moment what to do about this.

Thanks,

Andrew Murray

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


Re: [PATCH v2 14/18] KVM: arm64: spe: Provide guest virtual interrupts for SPE

2019-12-24 Thread Marc Zyngier

On 2019-12-24 11:50, Andrew Murray wrote:

On Sun, Dec 22, 2019 at 12:07:50PM +, Marc Zyngier wrote:

On Fri, 20 Dec 2019 14:30:21 +,
Andrew Murray  wrote:
>
> Upon the exit of a guest, let's determine if the SPE device has 
generated
> an interrupt - if so we'll inject a virtual interrupt to the 
guest.

>
> Upon the entry and exit of a guest we'll also update the state of 
the
> physical IRQ such that it is active when a guest interrupt is 
pending

> and the guest is running.
>
> Finally we map the physical IRQ to the virtual IRQ such that the 
guest

> can deactivate the interrupt when it handles the interrupt.
>
> Signed-off-by: Andrew Murray 
> ---
>  include/kvm/arm_spe.h |  6 
>  virt/kvm/arm/arm.c|  5 ++-
>  virt/kvm/arm/spe.c| 71 
+++

>  3 files changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> index 9c65130d726d..91b2214f543a 100644
> --- a/include/kvm/arm_spe.h
> +++ b/include/kvm/arm_spe.h
> @@ -37,6 +37,9 @@ static inline bool kvm_arm_support_spe_v1(void)
>  ID_AA64DFR0_PMSVER_SHIFT);
>  }
>
> +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu);
> +inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu);
> +
>  int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
>struct kvm_device_attr *attr);
>  int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> @@ -49,6 +52,9 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu 
*vcpu);

>  #define kvm_arm_support_spe_v1()  (false)
>  #define kvm_arm_spe_irq_initialized(v)(false)
>
> +static inline void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu) 
{}

> +static inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu) {}
> +
>  static inline int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
>  struct kvm_device_attr *attr)
>  {
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 340d2388ee2c..a66085c8e785 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -741,6 +741,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu 
*vcpu, struct kvm_run *run)

>preempt_disable();
>
>kvm_pmu_flush_hwstate(vcpu);
> +  kvm_spe_flush_hwstate(vcpu);
>
>local_irq_disable();
>
> @@ -782,6 +783,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu 
*vcpu, struct kvm_run *run)

>kvm_request_pending(vcpu)) {
>vcpu->mode = OUTSIDE_GUEST_MODE;
>isb(); /* Ensure work in x_flush_hwstate is committed */
> +  kvm_spe_sync_hwstate(vcpu);
>kvm_pmu_sync_hwstate(vcpu);
>if (static_branch_unlikely(&userspace_irqchip_in_use))
>kvm_timer_sync_hwstate(vcpu);
> @@ -816,11 +818,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu 
*vcpu, struct kvm_run *run)

>kvm_arm_clear_debug(vcpu);
>
>/*
> -   * We must sync the PMU state before the vgic state so
> +   * We must sync the PMU and SPE state before the vgic state so
> * that the vgic can properly sample the updated state of the
> * interrupt line.
> */
>kvm_pmu_sync_hwstate(vcpu);
> +  kvm_spe_sync_hwstate(vcpu);

The *HUGE* difference is that the PMU is purely a virtual interrupt,
while you're trying to deal with a HW interrupt here.

>
>/*
> * Sync the vgic state before syncing the timer state because
> diff --git a/virt/kvm/arm/spe.c b/virt/kvm/arm/spe.c
> index 83ac2cce2cc3..097ed39014e4 100644
> --- a/virt/kvm/arm/spe.c
> +++ b/virt/kvm/arm/spe.c
> @@ -35,6 +35,68 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu 
*vcpu)

>return 0;
>  }
>
> +static inline void set_spe_irq_phys_active(struct 
arm_spe_kvm_info *info,

> + bool active)
> +{
> +  int r;
> +	r = irq_set_irqchip_state(info->physical_irq, 
IRQCHIP_STATE_ACTIVE,

> +active);
> +  WARN_ON(r);
> +}
> +
> +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu)
> +{
> +  struct kvm_spe *spe = &vcpu->arch.spe;
> +  bool phys_active = false;
> +  struct arm_spe_kvm_info *info = arm_spe_get_kvm_info();
> +
> +  if (!kvm_arm_spe_v1_ready(vcpu))
> +  return;
> +
> +  if (irqchip_in_kernel(vcpu->kvm))
> +  phys_active = kvm_vgic_map_is_active(vcpu, spe->irq_num);
> +
> +  phys_active |= spe->irq_level;
> +
> +  set_spe_irq_phys_active(info, phys_active);

So you're happy to mess with the HW interrupt state even when you
don't have a HW irqchip? If you are going to copy paste the timer 
code

here, you'd need to support it all the way (no, don't).

> +}
> +
> +void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu)
> +{
> +  struct kvm_spe *spe = &vcpu->arch.spe;
> +  u64 pmbsr;
> +  int r;
> +  bool service;
> +  struct kvm_cpu_context *ctxt = &vcpu->arch.ctxt;
> +  struct arm_spe_kvm_info *i

Re: [PATCH v2 13/18] perf: arm_spe: Add KVM structure for obtaining IRQ info

2019-12-24 Thread Andrew Murray
On Sun, Dec 22, 2019 at 11:24:13AM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:20 +,
> Andrew Murray  wrote:
> > 
> > KVM requires knowledge of the physical SPE IRQ number such that it can
> > associate it with any virtual IRQ for guests that require SPE emulation.
> 
> This is at best extremely odd. The only reason for KVM to obtain this
> IRQ number is if it has exclusive access to the device.  This
> obviously isn't the case, as this device is shared between host and
> guest.

This was an attempt to set the interrupt as active such that host SPE driver
doesn't get spurious interrupts due to guest SPE activity. Though let's save
the discussion to patch 14.


> 
> > Let's create a structure to hold this information and an accessor that
> > KVM can use to retrieve this information.
> > 
> > We expect that each SPE device will have the same physical PPI number
> > and thus will warn when this is not the case.
> > 
> > Signed-off-by: Andrew Murray 
> > ---
> >  drivers/perf/arm_spe_pmu.c | 23 +++
> >  include/kvm/arm_spe.h  |  6 ++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> > index 4e4984a55cd1..2d24af4cfcab 100644
> > --- a/drivers/perf/arm_spe_pmu.c
> > +++ b/drivers/perf/arm_spe_pmu.c
> > @@ -34,6 +34,9 @@
> >  #include 
> >  #include 
> >  
> > +#include 
> > +#include 
> > +
> >  #include 
> >  #include 
> >  #include 
> > @@ -1127,6 +1130,24 @@ static void arm_spe_pmu_dev_teardown(struct 
> > arm_spe_pmu *spe_pmu)
> > free_percpu_irq(spe_pmu->irq, spe_pmu->handle);
> >  }
> >  
> > +#ifdef CONFIG_KVM_ARM_SPE
> > +static struct arm_spe_kvm_info arm_spe_kvm_info;
> > +
> > +struct arm_spe_kvm_info *arm_spe_get_kvm_info(void)
> > +{
> > +   return &arm_spe_kvm_info;
> > +}
> 
> How does this work when SPE is built as a module?
> 
> > +
> > +static void arm_spe_populate_kvm_info(struct arm_spe_pmu *spe_pmu)
> > +{
> > +   WARN_ON_ONCE(arm_spe_kvm_info.physical_irq != 0 &&
> > +arm_spe_kvm_info.physical_irq != spe_pmu->irq);
> > +   arm_spe_kvm_info.physical_irq = spe_pmu->irq;
> 
> What does 'physical' means here? It's an IRQ in the Linux sense, so
> it's already some random number that bears no relation to anything
> 'physical'.

It's some random number relating to the SPE device as opposed to the virtual
SPE device.

Thanks,

Andrew Murray

> 
> > +}
> > +#else
> > +static void arm_spe_populate_kvm_info(struct arm_spe_pmu *spe_pmu) {}
> > +#endif
> > +
> >  /* Driver and device probing */
> >  static int arm_spe_pmu_irq_probe(struct arm_spe_pmu *spe_pmu)
> >  {
> > @@ -1149,6 +1170,8 @@ static int arm_spe_pmu_irq_probe(struct arm_spe_pmu 
> > *spe_pmu)
> > }
> >  
> > spe_pmu->irq = irq;
> > +   arm_spe_populate_kvm_info(spe_pmu);
> > +
> > return 0;
> >  }
> >  
> > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > index d1f3c564dfd0..9c65130d726d 100644
> > --- a/include/kvm/arm_spe.h
> > +++ b/include/kvm/arm_spe.h
> > @@ -17,6 +17,12 @@ struct kvm_spe {
> > bool irq_level;
> >  };
> >  
> > +struct arm_spe_kvm_info {
> > +   int physical_irq;
> > +};
> > +
> > +struct arm_spe_kvm_info *arm_spe_get_kvm_info(void);
> > +
> >  #ifdef CONFIG_KVM_ARM_SPE
> >  #define kvm_arm_spe_v1_ready(v)((v)->arch.spe.ready)
> >  #define kvm_arm_spe_irq_initialized(v) \
> 
>   M.
> 
> -- 
> Jazz is not dead, it just smells funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 12/18] KVM: arm64: add a new vcpu device control group for SPEv1

2019-12-24 Thread Andrew Murray
On Sun, Dec 22, 2019 at 11:03:04AM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:19 +,
> Andrew Murray  wrote:
> > 
> > From: Sudeep Holla 
> > 
> > To configure the virtual SPEv1 overflow interrupt number, we use the
> > vcpu kvm_device ioctl, encapsulating the KVM_ARM_VCPU_SPE_V1_IRQ
> > attribute within the KVM_ARM_VCPU_SPE_V1_CTRL group.
> > 
> > After configuring the SPEv1, call the vcpu ioctl with attribute
> > KVM_ARM_VCPU_SPE_V1_INIT to initialize the SPEv1.
> > 
> > Signed-off-by: Sudeep Holla 
> > Signed-off-by: Andrew Murray 
> > ---
> >  Documentation/virt/kvm/devices/vcpu.txt |  28 
> >  arch/arm64/include/asm/kvm_host.h   |   2 +-
> >  arch/arm64/include/uapi/asm/kvm.h   |   4 +
> >  arch/arm64/kvm/Makefile |   1 +
> >  arch/arm64/kvm/guest.c  |   6 +
> >  arch/arm64/kvm/reset.c  |   3 +
> >  include/kvm/arm_spe.h   |  45 +++
> >  include/uapi/linux/kvm.h|   1 +
> >  virt/kvm/arm/arm.c  |   1 +
> >  virt/kvm/arm/spe.c  | 163 
> >  10 files changed, 253 insertions(+), 1 deletion(-)
> >  create mode 100644 virt/kvm/arm/spe.c
> > 
> > diff --git a/Documentation/virt/kvm/devices/vcpu.txt 
> > b/Documentation/virt/kvm/devices/vcpu.txt
> > index 6f3bd64a05b0..cefad056d677 100644
> > --- a/Documentation/virt/kvm/devices/vcpu.txt
> > +++ b/Documentation/virt/kvm/devices/vcpu.txt
> > @@ -74,3 +74,31 @@ Specifies the base address of the stolen time structure 
> > for this VCPU. The
> >  base address must be 64 byte aligned and exist within a valid guest memory
> >  region. See Documentation/virt/kvm/arm/pvtime.txt for more information
> >  including the layout of the stolen time structure.
> > +
> > +4. GROUP: KVM_ARM_VCPU_SPE_V1_CTRL
> > +Architectures: ARM64
> > +
> > +4.1. ATTRIBUTE: KVM_ARM_VCPU_SPE_V1_IRQ
> > +Parameters: in kvm_device_attr.addr the address for SPE buffer overflow 
> > interrupt
> > +   is a pointer to an int
> > +Returns: -EBUSY: The SPE overflow interrupt is already set
> > + -ENXIO: The overflow interrupt not set when attempting to get it
> > + -ENODEV: SPEv1 not supported
> > + -EINVAL: Invalid SPE overflow interrupt number supplied or
> > +  trying to set the IRQ number without using an in-kernel
> > +  irqchip.
> > +
> > +A value describing the SPEv1 (Statistical Profiling Extension v1) overflow
> > +interrupt number for this vcpu. This interrupt should be PPI and the 
> > interrupt
> > +type and number must be same for each vcpu.
> > +
> > +4.2 ATTRIBUTE: KVM_ARM_VCPU_SPE_V1_INIT
> > +Parameters: no additional parameter in kvm_device_attr.addr
> > +Returns: -ENODEV: SPEv1 not supported or GIC not initialized
> > + -ENXIO: SPEv1 not properly configured or in-kernel irqchip not
> > + configured as required prior to calling this attribute
> > + -EBUSY: SPEv1 already initialized
> > +
> > +Request the initialization of the SPEv1.  If using the SPEv1 with an 
> > in-kernel
> > +virtual GIC implementation, this must be done after initializing the 
> > in-kernel
> > +irqchip.
> > diff --git a/arch/arm64/include/asm/kvm_host.h 
> > b/arch/arm64/include/asm/kvm_host.h
> > index 333c6491bec7..d00f450dc4cd 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -39,7 +39,7 @@
> >  
> >  #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
> >  
> > -#define KVM_VCPU_MAX_FEATURES 7
> > +#define KVM_VCPU_MAX_FEATURES 8
> >  
> >  #define KVM_REQ_SLEEP \
> > KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h 
> > b/arch/arm64/include/uapi/asm/kvm.h
> > index 820e5751ada7..905a73f30079 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -106,6 +106,7 @@ struct kvm_regs {
> >  #define KVM_ARM_VCPU_SVE   4 /* enable SVE for this CPU */
> >  #define KVM_ARM_VCPU_PTRAUTH_ADDRESS   5 /* VCPU uses address 
> > authentication */
> >  #define KVM_ARM_VCPU_PTRAUTH_GENERIC   6 /* VCPU uses generic 
> > authentication */
> > +#define KVM_ARM_VCPU_SPE_V17 /* Support guest SPEv1 */
> >  
> >  struct kvm_vcpu_init {
> > __u32 target;
> > @@ -326,6 +327,9 @@ struct kvm_vcpu_events {
> >  #define   KVM_ARM_VCPU_TIMER_IRQ_PTIMER1
> >  #define KVM_ARM_VCPU_PVTIME_CTRL   2
> >  #define   KVM_ARM_VCPU_PVTIME_IPA  0
> > +#define KVM_ARM_VCPU_SPE_V1_CTRL   3
> > +#define   KVM_ARM_VCPU_SPE_V1_IRQ  0
> > +#define   KVM_ARM_VCPU_SPE_V1_INIT 1
> >  
> >  /* KVM_IRQ_LINE irq field index values */
> >  #define KVM_ARM_IRQ_VCPU2_SHIFT28
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 5ffbdc39e780..526f3bf09321 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -37,3 +37,4 @@ kvm-$(CONFIG_KVM

Re: [PATCH v2 09/18] arm64: KVM: enable conditional save/restore full SPE profiling buffer controls

2019-12-24 Thread Andrew Murray
On Fri, Dec 20, 2019 at 06:06:58PM +, Mark Rutland wrote:
> On Fri, Dec 20, 2019 at 02:30:16PM +, Andrew Murray wrote:
> > From: Sudeep Holla 
> > 
> > Now that we can save/restore the full SPE controls, we can enable it
> > if SPE is setup and ready to use in KVM. It's supported in KVM only if
> > all the CPUs in the system supports SPE.
> > 
> > However to support heterogenous systems, we need to move the check if
> > host supports SPE and do a partial save/restore.
> 
> I don't think that it makes sense to support this for heterogeneous
> systems, given their SPE capabilities and IMP DEF details will differ.
> 
> Is there some way we can limit this to homogeneous systems?

No problem, I'll see how to limit this.

Thanks,

Andrew Murray

> 
> Thanks,
> Mark.
> 
> > 
> > Signed-off-by: Sudeep Holla 
> > Signed-off-by: Andrew Murray 
> > ---
> >  arch/arm64/kvm/hyp/debug-sr.c | 33 -
> >  include/kvm/arm_spe.h |  6 ++
> >  2 files changed, 22 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
> > index 12429b212a3a..d8d857067e6d 100644
> > --- a/arch/arm64/kvm/hyp/debug-sr.c
> > +++ b/arch/arm64/kvm/hyp/debug-sr.c
> > @@ -86,18 +86,13 @@
> > }
> >  
> >  static void __hyp_text
> > -__debug_save_spe_nvhe(struct kvm_cpu_context *ctxt, bool full_ctxt)
> > +__debug_save_spe_context(struct kvm_cpu_context *ctxt, bool full_ctxt)
> >  {
> > u64 reg;
> >  
> > /* Clear pmscr in case of early return */
> > ctxt->sys_regs[PMSCR_EL1] = 0;
> >  
> > -   /* SPE present on this CPU? */
> > -   if (!cpuid_feature_extract_unsigned_field(read_sysreg(id_aa64dfr0_el1),
> > - ID_AA64DFR0_PMSVER_SHIFT))
> > -   return;
> > -
> > /* Yes; is it owned by higher EL? */
> > reg = read_sysreg_s(SYS_PMBIDR_EL1);
> > if (reg & BIT(SYS_PMBIDR_EL1_P_SHIFT))
> > @@ -142,7 +137,7 @@ __debug_save_spe_nvhe(struct kvm_cpu_context *ctxt, 
> > bool full_ctxt)
> >  }
> >  
> >  static void __hyp_text
> > -__debug_restore_spe_nvhe(struct kvm_cpu_context *ctxt, bool full_ctxt)
> > +__debug_restore_spe_context(struct kvm_cpu_context *ctxt, bool full_ctxt)
> >  {
> > if (!ctxt->sys_regs[PMSCR_EL1])
> > return;
> > @@ -210,11 +205,14 @@ void __hyp_text __debug_restore_guest_context(struct 
> > kvm_vcpu *vcpu)
> > struct kvm_guest_debug_arch *host_dbg;
> > struct kvm_guest_debug_arch *guest_dbg;
> >  
> > +   host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> > +   guest_ctxt = &vcpu->arch.ctxt;
> > +
> > +   __debug_restore_spe_context(guest_ctxt, kvm_arm_spe_v1_ready(vcpu));
> > +
> > if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
> > return;
> >  
> > -   host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> > -   guest_ctxt = &vcpu->arch.ctxt;
> > host_dbg = &vcpu->arch.host_debug_state.regs;
> > guest_dbg = kern_hyp_va(vcpu->arch.debug_ptr);
> >  
> > @@ -232,8 +230,7 @@ void __hyp_text __debug_restore_host_context(struct 
> > kvm_vcpu *vcpu)
> > host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> > guest_ctxt = &vcpu->arch.ctxt;
> >  
> > -   if (!has_vhe())
> > -   __debug_restore_spe_nvhe(host_ctxt, false);
> > +   __debug_restore_spe_context(host_ctxt, kvm_arm_spe_v1_ready(vcpu));
> >  
> > if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
> > return;
> > @@ -249,19 +246,21 @@ void __hyp_text __debug_restore_host_context(struct 
> > kvm_vcpu *vcpu)
> >  
> >  void __hyp_text __debug_save_host_context(struct kvm_vcpu *vcpu)
> >  {
> > -   /*
> > -* Non-VHE: Disable and flush SPE data generation
> > -* VHE: The vcpu can run, but it can't hide.
> > -*/
> > struct kvm_cpu_context *host_ctxt;
> >  
> > host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> > -   if (!has_vhe())
> > -   __debug_save_spe_nvhe(host_ctxt, false);
> > +   if (cpuid_feature_extract_unsigned_field(read_sysreg(id_aa64dfr0_el1),
> > +ID_AA64DFR0_PMSVER_SHIFT))
> > +   __debug_save_spe_context(host_ctxt, kvm_arm_spe_v1_ready(vcpu));
> >  }
> >  
> >  void __hyp_text __debug_save_guest_context(struct kvm_vcpu *vcpu)
> >  {
> > +   bool kvm_spe_ready = kvm_arm_spe_v1_ready(vcpu);
> > +
> > +   /* SPE present on this vCPU? */
> > +   if (kvm_spe_ready)
> > +   __debug_save_spe_context(&vcpu->arch.ctxt, kvm_spe_ready);
> >  }
> >  
> >  u32 __hyp_text __kvm_get_mdcr_el2(void)
> > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > index 48d118fdb174..30c40b1bc385 100644
> > --- a/include/kvm/arm_spe.h
> > +++ b/include/kvm/arm_spe.h
> > @@ -16,4 +16,10 @@ struct kvm_spe {
> > bool irq_level;
> >  };
> >  
> > +#ifdef CONFIG_KVM_ARM_SPE
> > +#define kvm_arm_spe_v1_ready(v)((v)->arch.spe.ready)
> > +#else
> > +#define kvm_arm_spe_v1_ready(v)(false)
> > +#endif /* CONFIG_K

Re: [PATCH v2 03/18] arm64: KVM: define SPE data structure for each vcpu

2019-12-24 Thread Andrew Murray
On Sat, Dec 21, 2019 at 01:19:36PM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:10 +
> Andrew Murray  wrote:
> 
> > From: Sudeep Holla 
> > 
> > In order to support virtual SPE for guest, so define some basic structs.
> > This features depends on host having hardware with SPE support.
> > 
> > Since we can support this only on ARM64, add a separate config symbol
> > for the same.
> > 
> > Signed-off-by: Sudeep Holla 
> > [ Add irq_level, rename irq to irq_num for kvm_spe ]
> > Signed-off-by: Andrew Murray 
> > ---
> >  arch/arm64/include/asm/kvm_host.h |  2 ++
> >  arch/arm64/kvm/Kconfig|  7 +++
> >  include/kvm/arm_spe.h | 19 +++
> >  3 files changed, 28 insertions(+)
> >  create mode 100644 include/kvm/arm_spe.h
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h 
> > b/arch/arm64/include/asm/kvm_host.h
> > index c61260cf63c5..f5dcff912645 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -35,6 +35,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
> >  
> > @@ -302,6 +303,7 @@ struct kvm_vcpu_arch {
> > struct vgic_cpu vgic_cpu;
> > struct arch_timer_cpu timer_cpu;
> > struct kvm_pmu pmu;
> > +   struct kvm_spe spe;
> >  
> > /*
> >  * Anything that is not used directly from assembly code goes
> > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> > index a475c68cbfec..af5be2c57dcb 100644
> > --- a/arch/arm64/kvm/Kconfig
> > +++ b/arch/arm64/kvm/Kconfig
> > @@ -35,6 +35,7 @@ config KVM
> > select HAVE_KVM_EVENTFD
> > select HAVE_KVM_IRQFD
> > select KVM_ARM_PMU if HW_PERF_EVENTS
> > +   select KVM_ARM_SPE if (HW_PERF_EVENTS && ARM_SPE_PMU)
> > select HAVE_KVM_MSI
> > select HAVE_KVM_IRQCHIP
> > select HAVE_KVM_IRQ_ROUTING
> > @@ -61,6 +62,12 @@ config KVM_ARM_PMU
> >   Adds support for a virtual Performance Monitoring Unit (PMU) in
> >   virtual machines.
> >  
> > +config KVM_ARM_SPE
> > +   bool
> > +   ---help---
> > + Adds support for a virtual Statistical Profiling Extension(SPE) in
> > + virtual machines.
> > +
> >  config KVM_INDIRECT_VECTORS
> > def_bool KVM && (HARDEN_BRANCH_PREDICTOR || HARDEN_EL2_VECTORS)
> >  
> > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > new file mode 100644
> > index ..48d118fdb174
> > --- /dev/null
> > +++ b/include/kvm/arm_spe.h
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 ARM Ltd.
> > + */
> > +
> > +#ifndef __ASM_ARM_KVM_SPE_H
> > +#define __ASM_ARM_KVM_SPE_H
> > +
> > +#include 
> > +#include 
> 
> I don't believe these are required at this stage.
> 
> > +
> > +struct kvm_spe {
> > +   int irq_num;
> 
> 'irq' was the right name *if* this represents a Linux irq. If this
> instead represents a guest PPI, then it should be named 'intid'.
> 
> In either case, please document what this represents.
> 
> > +   bool ready; /* indicates that SPE KVM instance is ready for use */
> > +   bool created; /* SPE KVM instance is created, may not be ready yet */
> > +   bool irq_level;
> 
> What does this represent? The state of the interrupt on the host? The
> guest? Something else? Also, please consider grouping related fields
> together.

It should be the state of the interrupt on the guest.

> 
> > +};
> 
> If you've added a config option that controls the selection of the SPE
> feature, why doesn't this result in an empty structure when it isn't
> selected?

OK, all noted.

Andrew Murray

> 
> > +
> > +#endif /* __ASM_ARM_KVM_SPE_H */
> 
> Thanks,
> 
>   M.
> -- 
> Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 14/18] KVM: arm64: spe: Provide guest virtual interrupts for SPE

2019-12-24 Thread Andrew Murray
On Sun, Dec 22, 2019 at 12:07:50PM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:21 +,
> Andrew Murray  wrote:
> > 
> > Upon the exit of a guest, let's determine if the SPE device has generated
> > an interrupt - if so we'll inject a virtual interrupt to the guest.
> > 
> > Upon the entry and exit of a guest we'll also update the state of the
> > physical IRQ such that it is active when a guest interrupt is pending
> > and the guest is running.
> > 
> > Finally we map the physical IRQ to the virtual IRQ such that the guest
> > can deactivate the interrupt when it handles the interrupt.
> > 
> > Signed-off-by: Andrew Murray 
> > ---
> >  include/kvm/arm_spe.h |  6 
> >  virt/kvm/arm/arm.c|  5 ++-
> >  virt/kvm/arm/spe.c| 71 +++
> >  3 files changed, 81 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/kvm/arm_spe.h b/include/kvm/arm_spe.h
> > index 9c65130d726d..91b2214f543a 100644
> > --- a/include/kvm/arm_spe.h
> > +++ b/include/kvm/arm_spe.h
> > @@ -37,6 +37,9 @@ static inline bool kvm_arm_support_spe_v1(void)
> >   ID_AA64DFR0_PMSVER_SHIFT);
> >  }
> >  
> > +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu);
> > +inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu);
> > +
> >  int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> > struct kvm_device_attr *attr);
> >  int kvm_arm_spe_v1_get_attr(struct kvm_vcpu *vcpu,
> > @@ -49,6 +52,9 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu *vcpu);
> >  #define kvm_arm_support_spe_v1()   (false)
> >  #define kvm_arm_spe_irq_initialized(v) (false)
> >  
> > +static inline void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu) {}
> > +static inline void kvm_spe_sync_hwstate(struct kvm_vcpu *vcpu) {}
> > +
> >  static inline int kvm_arm_spe_v1_set_attr(struct kvm_vcpu *vcpu,
> >   struct kvm_device_attr *attr)
> >  {
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 340d2388ee2c..a66085c8e785 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -741,6 +741,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
> > struct kvm_run *run)
> > preempt_disable();
> >  
> > kvm_pmu_flush_hwstate(vcpu);
> > +   kvm_spe_flush_hwstate(vcpu);
> >  
> > local_irq_disable();
> >  
> > @@ -782,6 +783,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
> > struct kvm_run *run)
> > kvm_request_pending(vcpu)) {
> > vcpu->mode = OUTSIDE_GUEST_MODE;
> > isb(); /* Ensure work in x_flush_hwstate is committed */
> > +   kvm_spe_sync_hwstate(vcpu);
> > kvm_pmu_sync_hwstate(vcpu);
> > if (static_branch_unlikely(&userspace_irqchip_in_use))
> > kvm_timer_sync_hwstate(vcpu);
> > @@ -816,11 +818,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
> > struct kvm_run *run)
> > kvm_arm_clear_debug(vcpu);
> >  
> > /*
> > -* We must sync the PMU state before the vgic state so
> > +* We must sync the PMU and SPE state before the vgic state so
> >  * that the vgic can properly sample the updated state of the
> >  * interrupt line.
> >  */
> > kvm_pmu_sync_hwstate(vcpu);
> > +   kvm_spe_sync_hwstate(vcpu);
> 
> The *HUGE* difference is that the PMU is purely a virtual interrupt,
> while you're trying to deal with a HW interrupt here.
> 
> >  
> > /*
> >  * Sync the vgic state before syncing the timer state because
> > diff --git a/virt/kvm/arm/spe.c b/virt/kvm/arm/spe.c
> > index 83ac2cce2cc3..097ed39014e4 100644
> > --- a/virt/kvm/arm/spe.c
> > +++ b/virt/kvm/arm/spe.c
> > @@ -35,6 +35,68 @@ int kvm_arm_spe_v1_enable(struct kvm_vcpu *vcpu)
> > return 0;
> >  }
> >  
> > +static inline void set_spe_irq_phys_active(struct arm_spe_kvm_info *info,
> > +  bool active)
> > +{
> > +   int r;
> > +   r = irq_set_irqchip_state(info->physical_irq, IRQCHIP_STATE_ACTIVE,
> > + active);
> > +   WARN_ON(r);
> > +}
> > +
> > +void kvm_spe_flush_hwstate(struct kvm_vcpu *vcpu)
> > +{
> > +   struct kvm_spe *spe = &vcpu->arch.spe;
> > +   bool phys_active = false;
> > +   struct arm_spe_kvm_info *info = arm_spe_get_kvm_info();
> > +
> > +   if (!kvm_arm_spe_v1_ready(vcpu))
> > +   return;
> > +
> > +   if (irqchip_in_kernel(vcpu->kvm))
> > +   phys_active = kvm_vgic_map_is_active(vcpu, spe->irq_num);
> > +
> > +   phys_active |= spe->irq_level;
> > +
> > +   set_spe_irq_phys_active(info, phys_active);
> 
> So you're happy to mess with the HW interrupt state even when you
> don't have a HW irqchip? If you are going to copy paste the timer code
> here, you'd need to support it all the way (no, don't).
>

[PATCH v3 14/32] irqchip/gic-v4.1: Suppress per-VLPI doorbell

2019-12-24 Thread Marc Zyngier
Since GICv4.1 gives us a per-VPE doorbell, avoid programming anything
else on VMOVI/VMAPI/VMAPTI and on any other action that would have
otherwise resulted in a per-VLPI doorbell to be programmed.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 403f5753e1ed..f00c8ddd3798 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -719,7 +719,7 @@ static struct its_vpe *its_build_vmapti_cmd(struct its_node 
*its,
 {
u32 db;
 
-   if (desc->its_vmapti_cmd.db_enabled)
+   if (!is_v4_1(its) && desc->its_vmapti_cmd.db_enabled)
db = desc->its_vmapti_cmd.vpe->vpe_db_lpi;
else
db = 1023;
@@ -742,7 +742,7 @@ static struct its_vpe *its_build_vmovi_cmd(struct its_node 
*its,
 {
u32 db;
 
-   if (desc->its_vmovi_cmd.db_enabled)
+   if (!is_v4_1(its) && desc->its_vmovi_cmd.db_enabled)
db = desc->its_vmovi_cmd.vpe->vpe_db_lpi;
else
db = 1023;
@@ -1353,6 +1353,13 @@ static void its_vlpi_set_doorbell(struct irq_data *d, 
bool enable)
u32 event = its_get_event_id(d);
struct its_vlpi_map *map;
 
+   /*
+* GICv4.1 does away with the per-LPI nonsense, nothing to do
+* here.
+*/
+   if (is_v4_1(its_dev->its))
+   return;
+
map = dev_event_to_vlpi_map(its_dev, event);
 
if (map->db_enabled == enable)
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 17/32] irqchip/gic-v4.1: Map the ITS SGIR register page

2019-12-24 Thread Marc Zyngier
One of the new features of GICv4.1 is to allow virtual SGIs to be
directly signaled to a VPE. For that, the ITS has grown a new
64kB page containing only a single register that is used to
signal a SGI to a given VPE.

Add a second mapping covering this new 64kB range, and take this
opportunity to limit the original mapping to 64kB, which is enough
to cover the span of the ITS registers.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 48d7ee443544..2f94fae67dfd 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -97,6 +97,7 @@ struct its_node {
struct mutexdev_alloc_lock;
struct list_headentry;
void __iomem*base;
+   void __iomem*sgir_base;
phys_addr_t phys_base;
struct its_cmd_block*cmd_base;
struct its_cmd_block*cmd_write;
@@ -4325,7 +4326,7 @@ static int __init its_probe_one(struct resource *res,
struct page *page;
int err;
 
-   its_base = ioremap(res->start, resource_size(res));
+   its_base = ioremap(res->start, SZ_64K);
if (!its_base) {
pr_warn("ITS@%pa: Unable to map ITS registers\n", &res->start);
return -ENOMEM;
@@ -4376,6 +4377,13 @@ static int __init its_probe_one(struct resource *res,
 
if (is_v4_1(its)) {
u32 svpet = FIELD_GET(GITS_TYPER_SVPET, typer);
+
+   its->sgir_base = ioremap(res->start + SZ_128K, SZ_64K);
+   if (!its->sgir_base) {
+   err = -ENOMEM;
+   goto out_free_its;
+   }
+
its->mpidr = readl_relaxed(its_base + GITS_MPIDR);
 
pr_info("ITS@%pa: Using GICv4.1 mode %08x %08x\n",
@@ -4389,7 +4397,7 @@ static int __init its_probe_one(struct resource *res,
get_order(ITS_CMD_QUEUE_SZ));
if (!page) {
err = -ENOMEM;
-   goto out_free_its;
+   goto out_unmap_sgir;
}
its->cmd_base = (void *)page_address(page);
its->cmd_write = its->cmd_base;
@@ -4456,6 +4464,9 @@ static int __init its_probe_one(struct resource *res,
its_free_tables(its);
 out_free_cmd:
free_pages((unsigned long)its->cmd_base, get_order(ITS_CMD_QUEUE_SZ));
+out_unmap_sgir:
+   if (its->sgir_base)
+   iounmap(its->sgir_base);
 out_free_its:
kfree(its);
 out_unmap:
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 25/32] irqchip/gic-v4.1: Add VSGI property setup

2019-12-24 Thread Marc Zyngier
Add the SGI configuration entry point for KVM to use.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v4.c   | 13 +
 include/linux/irqchip/arm-gic-v4.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 99b33f60ac63..f3f06c5c7e54 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -320,6 +320,19 @@ int its_prop_update_vlpi(int irq, u8 config, bool inv)
return irq_set_vcpu_affinity(irq, &info);
 }
 
+int its_prop_update_vsgi(int irq, u8 priority, bool group)
+{
+   struct its_cmd_info info = {
+   .cmd_type = PROP_UPDATE_SGI,
+   {
+   .priority   = priority,
+   .group  = group,
+   },
+   };
+
+   return irq_set_vcpu_affinity(irq, &info);
+}
+
 int its_init_v4(struct irq_domain *domain,
const struct irq_domain_ops *vpe_ops,
const struct irq_domain_ops *sgi_ops)
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index 9fbd0418f569..46c167a6349f 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -129,6 +129,7 @@ int its_map_vlpi(int irq, struct its_vlpi_map *map);
 int its_get_vlpi(int irq, struct its_vlpi_map *map);
 int its_unmap_vlpi(int irq);
 int its_prop_update_vlpi(int irq, u8 config, bool inv);
+int its_prop_update_vsgi(int irq, u8 priority, bool group);
 
 struct irq_domain_ops;
 int its_init_v4(struct irq_domain *domain,
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 32/32] KVM: arm64: GICv4.1: Expose HW-based SGIs in debugfs

2019-12-24 Thread Marc Zyngier
The vgic-state debugfs file could do with showing the pending state
of the HW-backed SGIs. Plug it into the low-level code.

Signed-off-by: Marc Zyngier 
---
 virt/kvm/arm/vgic/vgic-debug.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
index cc12fe9b2df3..b13a9e3f99dd 100644
--- a/virt/kvm/arm/vgic/vgic-debug.c
+++ b/virt/kvm/arm/vgic/vgic-debug.c
@@ -178,6 +178,8 @@ static void print_irq_state(struct seq_file *s, struct 
vgic_irq *irq,
struct kvm_vcpu *vcpu)
 {
char *type;
+   bool pending;
+
if (irq->intid < VGIC_NR_SGIS)
type = "SGI";
else if (irq->intid < VGIC_NR_PRIVATE_IRQS)
@@ -190,6 +192,16 @@ static void print_irq_state(struct seq_file *s, struct 
vgic_irq *irq,
if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS)
print_header(s, irq, vcpu);
 
+   pending = irq->pending_latch;
+   if (irq->hw && vgic_irq_is_sgi(irq->intid)) {
+   int err;
+
+   err = irq_get_irqchip_state(irq->host_irq,
+   IRQCHIP_STATE_PENDING,
+   &pending);
+   WARN_ON_ONCE(err);
+   }
+
seq_printf(s, "   %s %4d "
  "%2d "
  "%d%d%d%d%d%d%d "
@@ -201,7 +213,7 @@ static void print_irq_state(struct seq_file *s, struct 
vgic_irq *irq,
  "\n",
type, irq->intid,
(irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1,
-   irq->pending_latch,
+   pending,
irq->line_level,
irq->active,
irq->enabled,
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 22/32] irqchip/gic-v4.1: Plumb set_vcpu_affinity SGI callbacks

2019-12-24 Thread Marc Zyngier
As for VLPIs, there is a number of configuration bits that cannot
be directly communicated through the normal irqchip API, and we
have to use our good old friend set_vcpu_affinity.

This is used to configure group and priority for a given vSGI.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 18 ++
 include/linux/irqchip/arm-gic-v4.h |  5 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 47d63a5b43ff..5126bdcfe079 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3886,6 +3886,23 @@ static int its_sgi_get_irqchip_state(struct irq_data *d,
return 0;
 }
 
+static int its_sgi_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_cmd_info *info = vcpu_info;
+
+   switch (info->cmd_type) {
+   case PROP_UPDATE_SGI:
+   vpe->sgi_config[d->hwirq].priority = info->priority;
+   vpe->sgi_config[d->hwirq].group = info->group;
+   its_configure_sgi(d, false);
+   return 0;
+
+   default:
+   return -EINVAL;
+   }
+}
+
 static struct irq_chip its_sgi_irq_chip = {
.name   = "GICv4.1-sgi",
.irq_mask   = its_sgi_mask_irq,
@@ -3893,6 +3910,7 @@ static struct irq_chip its_sgi_irq_chip = {
.irq_set_affinity   = its_sgi_set_affinity,
.irq_set_irqchip_state  = its_sgi_set_irqchip_state,
.irq_get_irqchip_state  = its_sgi_get_irqchip_state,
+   .irq_set_vcpu_affinity  = its_sgi_set_vcpu_affinity,
 };
 
 static int its_sgi_irq_domain_alloc(struct irq_domain *domain,
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index 30b4855bf766..a1a9d40266f5 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -98,6 +98,7 @@ enum its_vcpu_info_cmd_type {
SCHEDULE_VPE,
DESCHEDULE_VPE,
INVALL_VPE,
+   PROP_UPDATE_SGI,
 };
 
 struct its_cmd_info {
@@ -110,6 +111,10 @@ struct its_cmd_info {
boolg0en;
boolg1en;
};
+   struct {
+   u8  priority;
+   boolgroup;
+   };
};
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 28/32] KVM: arm64: GICv4.1: Add direct injection capability to SGI registers

2019-12-24 Thread Marc Zyngier
Most of the GICv3 emulation code that deals with SGIs now has to be
aware of the v4.1 capabilities in order to benefit from it.

Add such support, keyed on the interrupt having the hw flag set and
being a SGI.

Signed-off-by: Marc Zyngier 
---
 virt/kvm/arm/vgic/vgic-mmio-v3.c | 15 +-
 virt/kvm/arm/vgic/vgic-mmio.c| 88 ++--
 2 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 7dfd15dbb308..d73f4ffd7d36 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -939,8 +940,18 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg, 
bool allow_group1)
 * generate interrupts of either group.
 */
if (!irq->group || allow_group1) {
-   irq->pending_latch = true;
-   vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
+   if (!irq->hw) {
+   irq->pending_latch = true;
+   vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
+   } else {
+   /* HW SGI? Ask the GIC to inject it */
+   int err;
+   err = irq_set_irqchip_state(irq->host_irq,
+   
IRQCHIP_STATE_PENDING,
+   true);
+   WARN_RATELIMIT(err, "IRQ %d", irq->host_irq);
+   raw_spin_unlock_irqrestore(&irq->irq_lock, 
flags);
+   }
} else {
raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
}
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index 0d090482720d..6ebf747a7806 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -5,6 +5,8 @@
 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -59,6 +61,11 @@ unsigned long vgic_mmio_read_group(struct kvm_vcpu *vcpu,
return value;
 }
 
+static void vgic_update_vsgi(struct vgic_irq *irq)
+{
+   WARN_ON(its_prop_update_vsgi(irq->host_irq, irq->priority, irq->group));
+}
+
 void vgic_mmio_write_group(struct kvm_vcpu *vcpu, gpa_t addr,
   unsigned int len, unsigned long val)
 {
@@ -71,7 +78,12 @@ void vgic_mmio_write_group(struct kvm_vcpu *vcpu, gpa_t addr,
 
raw_spin_lock_irqsave(&irq->irq_lock, flags);
irq->group = !!(val & BIT(i));
-   vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
+   if (irq->hw && vgic_irq_is_sgi(irq->intid)) {
+   vgic_update_vsgi(irq);
+   raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
+   } else {
+   vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
+   }
 
vgic_put_irq(vcpu->kvm, irq);
}
@@ -113,7 +125,21 @@ void vgic_mmio_write_senable(struct kvm_vcpu *vcpu,
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
raw_spin_lock_irqsave(&irq->irq_lock, flags);
-   if (vgic_irq_is_mapped_level(irq)) {
+   if (irq->hw && vgic_irq_is_sgi(irq->intid)) {
+   if (!irq->enabled) {
+   struct irq_data *data;
+
+   irq->enabled = true;
+   data = &irq_to_desc(irq->host_irq)->irq_data;
+   while (irqd_irq_disabled(data))
+   enable_irq(irq->host_irq);
+   }
+
+   raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
+   vgic_put_irq(vcpu->kvm, irq);
+
+   continue;
+   } else if (vgic_irq_is_mapped_level(irq)) {
bool was_high = irq->line_level;
 
/*
@@ -148,6 +174,8 @@ void vgic_mmio_write_cenable(struct kvm_vcpu *vcpu,
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
raw_spin_lock_irqsave(&irq->irq_lock, flags);
+   if (irq->hw && vgic_irq_is_sgi(irq->intid) && irq->enabled)
+   disable_irq_nosync(irq->host_irq);
 
irq->enabled = false;
 
@@ -167,10 +195,22 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu 
*vcpu,
for (i = 0; i < len * 8; i++) {
struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
unsigned long flags;
+   bool val;
 
raw_spin_lock_irqsave(&irq->irq_lock, flags);
-   if (irq_is_pending(irq))
-   value |= 

[PATCH v3 31/32] KVM: arm64: GICv4.1: Reload VLPI configuration on distributor enable/disable

2019-12-24 Thread Marc Zyngier
Each time a Group-enable bit gets flipped, the state of these bits
needs to be forwarded to the hardware. This is a pretty heavy
handed operation, requiring all vcpus to reload their GICv4
configuration. It is thus implemented as a new request type.

Of course, we only support Group-1 for now...

Signed-off-by: Marc Zyngier 
---
 arch/arm64/include/asm/kvm_host.h | 1 +
 virt/kvm/arm/arm.c| 8 
 virt/kvm/arm/vgic/vgic-mmio-v3.c  | 5 -
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index c61260cf63c5..4aeff0cc4ecf 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -45,6 +45,7 @@
 #define KVM_REQ_IRQ_PENDINGKVM_ARCH_REQ(1)
 #define KVM_REQ_VCPU_RESET KVM_ARCH_REQ(2)
 #define KVM_REQ_RECORD_STEAL   KVM_ARCH_REQ(3)
+#define KVM_REQ_RELOAD_GICv4   KVM_ARCH_REQ(4)
 
 DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
 
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 12e0280291ce..a398dd449880 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -682,6 +682,14 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
 
if (kvm_check_request(KVM_REQ_RECORD_STEAL, vcpu))
kvm_update_stolen_time(vcpu);
+
+   if (kvm_check_request(KVM_REQ_RELOAD_GICv4, vcpu)) {
+   /* The distributor enable bits were changed */
+   preempt_disable();
+   vgic_v4_put(vcpu, false);
+   vgic_v4_load(vcpu);
+   preempt_enable();
+   }
}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 31ff8c7e09b0..4e72fc08a973 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -132,7 +132,10 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
if (is_hwsgi != dist->nassgireq)
vgic_v4_configure_vsgis(vcpu->kvm);
 
-   if (!was_enabled && dist->enabled)
+   if (kvm_vgic_global_state.has_gicv4_1 &&
+   was_enabled != dist->enabled)
+   kvm_make_all_cpus_request(vcpu->kvm, 
KVM_REQ_RELOAD_GICv4);
+   else if (!was_enabled && dist->enabled)
vgic_kick_vcpus(vcpu->kvm);
 
mutex_unlock(&vcpu->kvm->lock);
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 29/32] KVM: arm64: GICv4.1: Allow SGIs to switch between HW and SW interrupts

2019-12-24 Thread Marc Zyngier
In order to let a guest buy in the new, active-less SGIs, we
need to be able to switch between the two modes.

Handle this by stopping all guest activity, transfer the state
from one mode to the other, and resume the guest.

Signed-off-by: Marc Zyngier 
---
 include/kvm/arm_vgic.h  |  3 ++
 virt/kvm/arm/vgic/vgic-v3.c |  2 +
 virt/kvm/arm/vgic/vgic-v4.c | 96 +
 virt/kvm/arm/vgic/vgic.h|  1 +
 4 files changed, 102 insertions(+)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 63457908c9c4..69f4164d6477 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -231,6 +231,9 @@ struct vgic_dist {
/* distributor enabled */
boolenabled;
 
+   /* Wants SGIs without active state */
+   boolnassgireq;
+
struct vgic_irq *spis;
 
struct vgic_io_device   dist_iodev;
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index c2fdea201747..c79a251c4974 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -540,6 +540,8 @@ int vgic_v3_map_resources(struct kvm *kvm)
goto out;
}
 
+   if (kvm_vgic_global_state.has_gicv4_1)
+   vgic_v4_configure_vsgis(kvm);
dist->ready = true;
 
 out:
diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
index c2fcde104ea2..063785fd2dc7 100644
--- a/virt/kvm/arm/vgic/vgic-v4.c
+++ b/virt/kvm/arm/vgic/vgic-v4.c
@@ -97,6 +97,102 @@ static irqreturn_t vgic_v4_doorbell_handler(int irq, void 
*info)
return IRQ_HANDLED;
 }
 
+static void vgic_v4_sync_sgi_config(struct its_vpe *vpe, struct vgic_irq *irq)
+{
+   vpe->sgi_config[irq->intid].enabled = irq->enabled;
+   vpe->sgi_config[irq->intid].group   = irq->group;
+   vpe->sgi_config[irq->intid].priority= irq->priority;
+}
+
+static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu)
+{
+   struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
+   int i;
+
+   /*
+* With GICv4.1, every virtual SGI can be directly injected. So
+* let's pretend that they are HW interrupts, tied to a host
+* IRQ. The SGI code will do its magic.
+*/
+   for (i = 0; i < VGIC_NR_SGIS; i++) {
+   struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, i);
+   struct irq_desc *desc;
+   int ret;
+
+   if (irq->hw) {
+   vgic_put_irq(vcpu->kvm, irq);
+   continue;
+   }
+
+   irq->hw = true;
+   irq->host_irq = irq_find_mapping(vpe->sgi_domain, i);
+   vgic_v4_sync_sgi_config(vpe, irq);
+   /*
+* SGIs are initialised as disabled. Enable them if
+* required by the rest of the VGIC init code.
+*/
+   desc = irq_to_desc(irq->host_irq);
+   ret = irq_domain_activate_irq(irq_desc_get_irq_data(desc),
+ false);
+   if (!WARN_ON(ret)) {
+   /* Transfer pending state */
+   ret = irq_set_irqchip_state(irq->host_irq,
+   IRQCHIP_STATE_PENDING,
+   irq->pending_latch);
+   WARN_ON(ret);
+   irq->pending_latch = false;
+   }
+
+   vgic_put_irq(vcpu->kvm, irq);
+   }
+}
+
+static void vgic_v4_disable_vsgis(struct kvm_vcpu *vcpu)
+{
+   int i;
+
+   for (i = 0; i < VGIC_NR_SGIS; i++) {
+   struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, i);
+   struct irq_desc *desc;
+   int ret;
+
+   if (!irq->hw) {
+   vgic_put_irq(vcpu->kvm, irq);
+   continue;
+   }
+
+   irq->hw = false;
+   ret = irq_get_irqchip_state(irq->host_irq,
+   IRQCHIP_STATE_PENDING,
+   &irq->pending_latch);
+   WARN_ON(ret);
+
+   desc = irq_to_desc(irq->host_irq);
+   irq_domain_deactivate_irq(irq_desc_get_irq_data(desc));
+
+   vgic_put_irq(vcpu->kvm, irq);
+   }
+}
+
+/* Must be called with the kvm lock held */
+void vgic_v4_configure_vsgis(struct kvm *kvm)
+{
+   struct vgic_dist *dist = &kvm->arch.vgic;
+   struct kvm_vcpu *vcpu;
+   int i;
+
+   kvm_arm_halt_guest(kvm);
+
+   kvm_for_each_vcpu(i, vcpu, kvm) {
+   if (dist->nassgireq)
+   vgic_v4_enable_vsgis(vcpu);
+   else
+   vgic_v4_disable_vsgis(vcpu);
+   }
+
+   kvm_arm_resume_guest(kvm);
+}
+
 /**
  * vgic_v4_init - Initialize the GICv4 data structures
  * @kvm:   Pointer to the VM be

[PATCH v3 16/32] irqchip/gic-v4.1: Advertise support v4.1 to KVM

2019-12-24 Thread Marc Zyngier
Tell KVM that we support v4.1. Nothing uses this information so far.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 9 -
 drivers/irqchip/irq-gic-v3.c   | 2 ++
 include/linux/irqchip/arm-gic-common.h | 2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 360ca2c1533f..48d7ee443544 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4739,6 +4739,7 @@ int __init its_init(struct fwnode_handle *handle, struct 
rdists *rdists,
struct device_node *of_node;
struct its_node *its;
bool has_v4 = false;
+   bool has_v4_1 = false;
int err;
 
gic_rdists = rdists;
@@ -4759,8 +4760,14 @@ int __init its_init(struct fwnode_handle *handle, struct 
rdists *rdists,
if (err)
return err;
 
-   list_for_each_entry(its, &its_nodes, entry)
+   list_for_each_entry(its, &its_nodes, entry) {
has_v4 |= is_v4(its);
+   has_v4_1 |= is_v4_1(its);
+   }
+
+   /* Don't bother with inconsistent systems */
+   if (WARN_ON(!has_v4_1 && rdists->has_rvpeid))
+   rdists->has_rvpeid = false;
 
if (has_v4 & rdists->has_vlpis) {
if (its_init_vpe_domain() ||
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 624f351c0362..fcbd220c92b3 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1784,6 +1784,7 @@ static void __init gic_of_setup_kvm_info(struct 
device_node *node)
gic_v3_kvm_info.vcpu = r;
 
gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
+   gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;
gic_set_kvm_info(&gic_v3_kvm_info);
 }
 
@@ -2094,6 +2095,7 @@ static void __init gic_acpi_setup_kvm_info(void)
}
 
gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
+   gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;
gic_set_kvm_info(&gic_v3_kvm_info);
 }
 
diff --git a/include/linux/irqchip/arm-gic-common.h 
b/include/linux/irqchip/arm-gic-common.h
index b9850f5f1906..fa8c0455c352 100644
--- a/include/linux/irqchip/arm-gic-common.h
+++ b/include/linux/irqchip/arm-gic-common.h
@@ -32,6 +32,8 @@ struct gic_kvm_info {
struct resource vctrl;
/* vlpi support */
boolhas_v4;
+   /* rvpeid support */
+   boolhas_v4_1;
 };
 
 const struct gic_kvm_info *gic_get_kvm_info(void);
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 20/32] irqchip/gic-v4.1: Plumb mask/unmask SGI callbacks

2019-12-24 Thread Marc Zyngier
Implement mask/unmask for virtual SGIs by calling into the
configuration helper.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 9ca2ad6000d8..c13c889553c5 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3809,6 +3809,22 @@ static void its_configure_sgi(struct irq_data *d, bool 
clear)
its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
 }
 
+static void its_sgi_mask_irq(struct irq_data *d)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+   vpe->sgi_config[d->hwirq].enabled = false;
+   its_configure_sgi(d, false);
+}
+
+static void its_sgi_unmask_irq(struct irq_data *d)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+   vpe->sgi_config[d->hwirq].enabled = true;
+   its_configure_sgi(d, false);
+}
+
 static int its_sgi_set_affinity(struct irq_data *d,
const struct cpumask *mask_val,
bool force)
@@ -3818,6 +3834,8 @@ static int its_sgi_set_affinity(struct irq_data *d,
 
 static struct irq_chip its_sgi_irq_chip = {
.name   = "GICv4.1-sgi",
+   .irq_mask   = its_sgi_mask_irq,
+   .irq_unmask = its_sgi_unmask_irq,
.irq_set_affinity   = its_sgi_set_affinity,
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 10/32] irqchip/gic-v4.1: Add mask/unmask doorbell callbacks

2019-12-24 Thread Marc Zyngier
masking/unmasking doorbells on GICv4.1 relies on a new INVDB command,
which broadcasts the invalidation to all RDs.

Implement the new command as well as the masking callbacks, and plug
the whole thing into the v4.1 VPE irqchip.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 73 ++
 include/linux/irqchip/arm-gic-v3.h |  3 +-
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 157f51398850..0da6baa48153 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -334,6 +334,10 @@ struct its_cmd_desc {
u16 seq_num;
u16 its_list;
} its_vmovp_cmd;
+
+   struct {
+   struct its_vpe *vpe;
+   } its_invdb_cmd;
};
 };
 
@@ -832,6 +836,21 @@ static struct its_vpe *its_build_vclear_cmd(struct 
its_node *its,
return valid_vpe(its, map->vpe);
 }
 
+static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
+  struct its_cmd_block *cmd,
+  struct its_cmd_desc *desc)
+{
+   if (WARN_ON(!is_v4_1(its)))
+   return NULL;
+
+   its_encode_cmd(cmd, GITS_CMD_INVDB);
+   its_encode_vpeid(cmd, desc->its_invdb_cmd.vpe->vpe_id);
+
+   its_fixup_cmd(cmd);
+
+   return valid_vpe(its, desc->its_invdb_cmd.vpe);
+}
+
 static u64 its_cmd_ptr_to_offset(struct its_node *its,
 struct its_cmd_block *ptr)
 {
@@ -1240,6 +1259,14 @@ static void its_send_vclear(struct its_device *dev, u32 
event_id)
its_send_single_vcommand(dev->its, its_build_vclear_cmd, &desc);
 }
 
+static void its_send_invdb(struct its_node *its, struct its_vpe *vpe)
+{
+   struct its_cmd_desc desc;
+
+   desc.its_invdb_cmd.vpe = vpe;
+   its_send_single_vcommand(its, its_build_invdb_cmd, &desc);
+}
+
 /*
  * irqchip functions - assumes MSI, mostly.
  */
@@ -3554,6 +3581,50 @@ static struct irq_chip its_vpe_irq_chip = {
.irq_set_vcpu_affinity  = its_vpe_set_vcpu_affinity,
 };
 
+static struct its_node *find_4_1_its(void)
+{
+   static struct its_node *its = NULL;
+
+   if (!its) {
+   list_for_each_entry(its, &its_nodes, entry) {
+   if (is_v4_1(its))
+   return its;
+   }
+
+   /* Oops? */
+   its = NULL;
+   }
+
+   return its;
+}
+
+static void its_vpe_4_1_send_inv(struct irq_data *d)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_node *its;
+
+   /*
+* GICv4.1 wants doorbells to be invalidated using the
+* INVDB command in order to be broadcast to all RDs. Send
+* it to the first valid ITS, and let the HW do its magic.
+*/
+   its = find_4_1_its();
+   if (its)
+   its_send_invdb(its, vpe);
+}
+
+static void its_vpe_4_1_mask_irq(struct irq_data *d)
+{
+   lpi_write_config(d->parent_data, LPI_PROP_ENABLED, 0);
+   its_vpe_4_1_send_inv(d);
+}
+
+static void its_vpe_4_1_unmask_irq(struct irq_data *d)
+{
+   lpi_write_config(d->parent_data, 0, LPI_PROP_ENABLED);
+   its_vpe_4_1_send_inv(d);
+}
+
 static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
 {
struct its_cmd_info *info = vcpu_info;
@@ -3575,6 +3646,8 @@ static int its_vpe_4_1_set_vcpu_affinity(struct irq_data 
*d, void *vcpu_info)
 
 static struct irq_chip its_vpe_4_1_irq_chip = {
.name   = "GICv4.1-vpe",
+   .irq_mask   = its_vpe_4_1_mask_irq,
+   .irq_unmask = its_vpe_4_1_unmask_irq,
.irq_eoi= irq_chip_eoi_parent,
.irq_set_affinity   = its_vpe_set_affinity,
.irq_set_vcpu_affinity  = its_vpe_4_1_set_vcpu_affinity,
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index df3b7cb50956..33519b7c96cf 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -486,8 +486,9 @@
 #define GITS_CMD_VMAPTIGITS_CMD_GICv4(GITS_CMD_MAPTI)
 #define GITS_CMD_VMOVI GITS_CMD_GICv4(GITS_CMD_MOVI)
 #define GITS_CMD_VSYNC GITS_CMD_GICv4(GITS_CMD_SYNC)
-/* VMOVP is the odd one, as it doesn't have a physical counterpart */
+/* VMOVP and INVDB are the odd ones, as they dont have a physical counterpart 
*/
 #define GITS_CMD_VMOVP GITS_CMD_GICv4(2)
+#define GITS_CMD_INVDB GITS_CMD_GICv4(0xe)
 
 /*
  * ITS error numbers
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 21/32] irqchip/gic-v4.1: Plumb get/set_irqchip_state SGI callbacks

2019-12-24 Thread Marc Zyngier
To implement the get/set_irqchip_state callbacks (limited to the
PENDING state), we have to use a particular set of hacks:

- Reading the pending state is done by using a pair of new redistributor
  registers (GICR_VSGIR, GICR_VSGIPENDR), which allow the 16 interrupts
  state to be retrieved.
- Setting the pending state is done by generating it as we'd otherwise do
  for a guest (writing to GITS_SGIR)
- Clearing the pending state is done by emiting a VSGI command with the
  "clear" bit set.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 56 ++
 include/linux/irqchip/arm-gic-v3.h | 14 
 2 files changed, 70 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c13c889553c5..47d63a5b43ff 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3832,11 +3832,67 @@ static int its_sgi_set_affinity(struct irq_data *d,
return -EINVAL;
 }
 
+static int its_sgi_set_irqchip_state(struct irq_data *d,
+enum irqchip_irq_state which,
+bool state)
+{
+   if (which != IRQCHIP_STATE_PENDING)
+   return -EINVAL;
+
+   if (state) {
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_node *its = find_4_1_its();
+   u64 val;
+
+   val  = FIELD_PREP(GITS_SGIR_VPEID, vpe->vpe_id);
+   val |= FIELD_PREP(GITS_SGIR_VINTID, d->hwirq);
+   writeq_relaxed(val, its->sgir_base + GITS_SGIR - SZ_128K);
+   } else {
+   its_configure_sgi(d, true);
+   }
+
+   return 0;
+}
+
+static int its_sgi_get_irqchip_state(struct irq_data *d,
+enum irqchip_irq_state which, bool *val)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   void __iomem *base = gic_data_rdist_cpu(vpe->col_idx)->rd_base + 
SZ_128K;
+   u32 count = 100;/* 1s! */
+   u32 status;
+
+   if (which != IRQCHIP_STATE_PENDING)
+   return -EINVAL;
+
+   writel_relaxed(vpe->vpe_id, base + GICR_VSGIR);
+   do {
+   status = readl_relaxed(base + GICR_VSGIPENDR);
+   if (!(status & GICR_VSGIPENDR_BUSY))
+   goto out;
+
+   count--;
+   if (!count) {
+   pr_err_ratelimited("Unable to get SGI status\n");
+   goto out;
+   }
+   cpu_relax();
+   udelay(1);
+   } while(count);
+
+out:
+   *val = !!(status & (1 << d->hwirq));
+
+   return 0;
+}
+
 static struct irq_chip its_sgi_irq_chip = {
.name   = "GICv4.1-sgi",
.irq_mask   = its_sgi_mask_irq,
.irq_unmask = its_sgi_unmask_irq,
.irq_set_affinity   = its_sgi_set_affinity,
+   .irq_set_irqchip_state  = its_sgi_set_irqchip_state,
+   .irq_get_irqchip_state  = its_sgi_get_irqchip_state,
 };
 
 static int its_sgi_irq_domain_alloc(struct irq_domain *domain,
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 64818ed4c238..a0566bd5516e 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -345,6 +345,15 @@
 #define GICR_VPENDBASER_4_1_VGRP1EN(1ULL << 58)
 #define GICR_VPENDBASER_4_1_VPEID  GENMASK_ULL(15, 0)
 
+#define GICR_VSGIR 0x0080
+
+#define GICR_VSGIR_VPEID   GENMASK(15, 0)
+
+#define GICR_VSGIPENDR 0x0088
+
+#define GICR_VSGIPENDR_BUSY(1U << 31)
+#define GICR_VSGIPENDR_PENDING GENMASK(15, 0)
+
 /*
  * ITS registers, offsets from ITS_base
  */
@@ -368,6 +377,11 @@
 
 #define GITS_TRANSLATER0x10040
 
+#define GITS_SGIR  0x20020
+
+#define GITS_SGIR_VPEIDGENMASK_ULL(47, 32)
+#define GITS_SGIR_VINTID   GENMASK_ULL(7, 0)
+
 #define GITS_CTLR_ENABLE   (1U << 0)
 #define GITS_CTLR_ImDe (1U << 1)
 #defineGITS_CTLR_ITS_NUMBER_SHIFT  4
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 18/32] irqchip/gic-v4.1: Plumb skeletal VSGI irqchip

2019-12-24 Thread Marc Zyngier
Since GICv4.1 has the capability to inject 16 SGIs into each VPE,
and that I'm keen not to invent too many specific interfaces to
manupulate these interrupts, let's pretend that each of these SGIs
is an actual Linux interrupt.

For that matter, let's introduce a minimal irqchip and irqdomain
setup that will get fleshed up in the following patches.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 68 +-
 drivers/irqchip/irq-gic-v4.c   |  8 +++-
 include/linux/irqchip/arm-gic-v4.h |  9 +++-
 3 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2f94fae67dfd..b38823b9c27f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3740,6 +3740,67 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
.irq_set_vcpu_affinity  = its_vpe_4_1_set_vcpu_affinity,
 };
 
+static int its_sgi_set_affinity(struct irq_data *d,
+   const struct cpumask *mask_val,
+   bool force)
+{
+   return -EINVAL;
+}
+
+static struct irq_chip its_sgi_irq_chip = {
+   .name   = "GICv4.1-sgi",
+   .irq_set_affinity   = its_sgi_set_affinity,
+};
+
+static int its_sgi_irq_domain_alloc(struct irq_domain *domain,
+   unsigned int virq, unsigned int nr_irqs,
+   void *args)
+{
+   struct its_vpe *vpe = args;
+   int i;
+
+   /* Yes, we do want 16 SGIs */
+   WARN_ON(nr_irqs != 16);
+
+   for (i = 0; i < 16; i++) {
+   vpe->sgi_config[i].priority = 0;
+   vpe->sgi_config[i].enabled = false;
+   vpe->sgi_config[i].group = false;
+
+   irq_domain_set_hwirq_and_chip(domain, virq + i, i,
+ &its_sgi_irq_chip, vpe);
+   irq_set_status_flags(virq + i, IRQ_DISABLE_UNLAZY);
+   }
+
+   return 0;
+}
+
+static void its_sgi_irq_domain_free(struct irq_domain *domain,
+   unsigned int virq,
+   unsigned int nr_irqs)
+{
+   /* Nothing to do */
+}
+
+static int its_sgi_irq_domain_activate(struct irq_domain *domain,
+  struct irq_data *d, bool reserve)
+{
+   return 0;
+}
+
+static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
+ struct irq_data *d)
+{
+   /* Nothing to do */
+}
+
+static struct irq_domain_ops its_sgi_domain_ops = {
+   .alloc  = its_sgi_irq_domain_alloc,
+   .free   = its_sgi_irq_domain_free,
+   .activate   = its_sgi_irq_domain_activate,
+   .deactivate = its_sgi_irq_domain_deactivate,
+};
+
 static int its_vpe_id_alloc(void)
 {
return ida_simple_get(&its_vpeid_ida, 0, ITS_MAX_VPEID, GFP_KERNEL);
@@ -4781,8 +4842,13 @@ int __init its_init(struct fwnode_handle *handle, struct 
rdists *rdists,
rdists->has_rvpeid = false;
 
if (has_v4 & rdists->has_vlpis) {
+   struct irq_domain_ops *sgi_ops = NULL;
+
+   if (has_v4_1)
+   sgi_ops = &its_sgi_domain_ops;
+
if (its_init_vpe_domain() ||
-   its_init_v4(parent_domain, &its_vpe_domain_ops)) {
+   its_init_v4(parent_domain, &its_vpe_domain_ops, sgi_ops)) {
rdists->has_vlpis = false;
pr_err("ITS: Disabling GICv4 support\n");
}
diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 45969927cc81..c01910d53f9e 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -85,6 +85,7 @@
 
 static struct irq_domain *gic_domain;
 static const struct irq_domain_ops *vpe_domain_ops;
+static const struct irq_domain_ops *sgi_domain_ops;
 
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {
@@ -216,12 +217,15 @@ int its_prop_update_vlpi(int irq, u8 config, bool inv)
return irq_set_vcpu_affinity(irq, &info);
 }
 
-int its_init_v4(struct irq_domain *domain, const struct irq_domain_ops *ops)
+int its_init_v4(struct irq_domain *domain,
+   const struct irq_domain_ops *vpe_ops,
+   const struct irq_domain_ops *sgi_ops)
 {
if (domain) {
pr_info("ITS: Enabling GICv4 support\n");
gic_domain = domain;
-   vpe_domain_ops = ops;
+   vpe_domain_ops = vpe_ops;
+   sgi_domain_ops = sgi_ops;
return 0;
}
 
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index d9c34968467a..30b4855bf766 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -49,6 +49,11 @@ struct its_vpe {
};
/* GICv4.1 implementations */
str

[PATCH v3 19/32] irqchip/gic-v4.1: Add initial SGI configuration

2019-12-24 Thread Marc Zyngier
The GICv4.1 ITS has yet another new command (VSGI) which allows
a VPE-targeted SGI to be configured (or have its pending state
cleared). Add support for this command and plumb it into the
activate irqdomain callback so that it is ready to be used.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 76 +-
 include/linux/irqchip/arm-gic-v3.h |  3 +-
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index b38823b9c27f..9ca2ad6000d8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -354,6 +354,15 @@ struct its_cmd_desc {
struct {
struct its_vpe *vpe;
} its_invdb_cmd;
+
+   struct {
+   struct its_vpe *vpe;
+   u8 sgi;
+   u8 priority;
+   bool enable;
+   bool group;
+   bool clear;
+   } its_vsgi_cmd;
};
 };
 
@@ -502,6 +511,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool 
db)
its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
 }
 
+static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
+{
+   its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
+{
+   its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+   its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+   its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+   its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+
 static inline void its_fixup_cmd(struct its_cmd_block *cmd)
 {
/* Let's fixup BE commands */
@@ -867,6 +901,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node 
*its,
return valid_vpe(its, desc->its_invdb_cmd.vpe);
 }
 
+static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+ struct its_cmd_block *cmd,
+ struct its_cmd_desc *desc)
+{
+   if (WARN_ON(!is_v4_1(its)))
+   return NULL;
+
+   its_encode_cmd(cmd, GITS_CMD_VSGI);
+   its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+   its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+   its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+   its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+   its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+   its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+   its_fixup_cmd(cmd);
+
+   return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
 static u64 its_cmd_ptr_to_offset(struct its_node *its,
 struct its_cmd_block *ptr)
 {
@@ -3740,6 +3794,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
.irq_set_vcpu_affinity  = its_vpe_4_1_set_vcpu_affinity,
 };
 
+static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_cmd_desc desc;
+
+   desc.its_vsgi_cmd.vpe = vpe;
+   desc.its_vsgi_cmd.sgi = d->hwirq;
+   desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
+   desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
+   desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
+   desc.its_vsgi_cmd.clear = clear;
+
+   its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);
+}
+
 static int its_sgi_set_affinity(struct irq_data *d,
const struct cpumask *mask_val,
bool force)
@@ -3785,13 +3854,18 @@ static void its_sgi_irq_domain_free(struct irq_domain 
*domain,
 static int its_sgi_irq_domain_activate(struct irq_domain *domain,
   struct irq_data *d, bool reserve)
 {
+   /* Write out the initial SGI configuration */
+   its_configure_sgi(d, false);
return 0;
 }
 
 static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
  struct irq_data *d)
 {
-   /* Nothing to do */
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+   vpe->sgi_config[d->hwirq].enabled = false;
+   its_configure_sgi(d, true);
 }
 
 static struct irq_domain_ops its_sgi_domain_ops = {
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index a30ea5294784..64818ed4c238 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -502,8 +502,9 @@
 #define GITS_CMD_VMAPTIGITS_CMD_GICv4(GITS_CMD_MAPTI)
 #define GITS_CMD_VMOVI   

[PATCH v3 15/32] irqchip/gic-v4.1: Allow direct invalidation of VLPIs

2019-12-24 Thread Marc Zyngier
Just like for INVALL, GICv4.1 has grown a VPE-aware INVLPI register.
Let's plumb it in and make use of the DirectLPI code in that case.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 53 --
 include/linux/irqchip/arm-gic-v3.h |  1 +
 2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index f00c8ddd3798..360ca2c1533f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -228,11 +228,26 @@ static struct its_vlpi_map *dev_event_to_vlpi_map(struct 
its_device *its_dev,
return &its_dev->event_map.vlpi_maps[event];
 }
 
-static struct its_collection *irq_to_col(struct irq_data *d)
+static struct its_vlpi_map *get_vlpi_map(struct irq_data *d)
 {
struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+   u32 event = its_get_event_id(d);
+
+   if (!irqd_is_forwarded_to_vcpu(d))
+   return NULL;
 
-   return dev_event_to_col(its_dev, its_get_event_id(d));
+   return dev_event_to_vlpi_map(its_dev, event);
+}
+
+static int irq_to_cpuid(struct irq_data *d)
+{
+   struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+   struct its_vlpi_map *map = get_vlpi_map(d);
+
+   if (map)
+   return map->vpe->col_idx;
+
+   return its_dev->event_map.col_map[its_get_event_id(d)];
 }
 
 static struct its_collection *valid_col(struct its_collection *col)
@@ -1270,17 +1285,6 @@ static void its_send_invdb(struct its_node *its, struct 
its_vpe *vpe)
 /*
  * irqchip functions - assumes MSI, mostly.
  */
-static struct its_vlpi_map *get_vlpi_map(struct irq_data *d)
-{
-   struct its_device *its_dev = irq_data_get_irq_chip_data(d);
-   u32 event = its_get_event_id(d);
-
-   if (!irqd_is_forwarded_to_vcpu(d))
-   return NULL;
-
-   return dev_event_to_vlpi_map(its_dev, event);
-}
-
 static void lpi_write_config(struct irq_data *d, u8 clr, u8 set)
 {
struct its_vlpi_map *map = get_vlpi_map(d);
@@ -1323,13 +1327,25 @@ static void wait_for_syncr(void __iomem *rdbase)
 
 static void direct_lpi_inv(struct irq_data *d)
 {
-   struct its_collection *col;
+   struct its_vlpi_map *map = get_vlpi_map(d);
void __iomem *rdbase;
+   u64 val;
+
+   if (map) {
+   struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+
+   WARN_ON(!is_v4_1(its_dev->its));
+
+   val  = GICR_INVLPIR_V;
+   val |= FIELD_PREP(GICR_INVLPIR_VPEID, map->vpe->vpe_id);
+   val |= FIELD_PREP(GICR_INVLPIR_INTID, map->vintid);
+   } else {
+   val = d->hwirq;
+   }
 
/* Target the redistributor this LPI is currently routed to */
-   col = irq_to_col(d);
-   rdbase = per_cpu_ptr(gic_rdists->rdist, col->col_id)->rd_base;
-   gic_write_lpir(d->hwirq, rdbase + GICR_INVLPIR);
+   rdbase = per_cpu_ptr(gic_rdists->rdist, irq_to_cpuid(d))->rd_base;
+   gic_write_lpir(val, rdbase + GICR_INVLPIR);
 
wait_for_syncr(rdbase);
 }
@@ -1339,7 +1355,8 @@ static void lpi_update_config(struct irq_data *d, u8 clr, 
u8 set)
struct its_device *its_dev = irq_data_get_irq_chip_data(d);
 
lpi_write_config(d, clr, set);
-   if (gic_rdists->has_direct_lpi && !irqd_is_forwarded_to_vcpu(d))
+   if (gic_rdists->has_direct_lpi &&
+   (is_v4_1(its_dev->its) || !irqd_is_forwarded_to_vcpu(d)))
direct_lpi_inv(d);
else if (!irqd_is_forwarded_to_vcpu(d))
its_send_inv(its_dev, its_get_event_id(d));
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 37b14fb82773..a30ea5294784 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -249,6 +249,7 @@
 #define GICR_TYPER_COMMON_LPI_AFF  GENMASK_ULL(25, 24)
 #define GICR_TYPER_AFFINITYGENMASK_ULL(63, 32)
 
+#define GICR_INVLPIR_INTID GENMASK_ULL(31, 0)
 #define GICR_INVLPIR_VPEID GENMASK_ULL(47, 32)
 #define GICR_INVLPIR_V GENMASK_ULL(63, 63)
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 30/32] KVM: arm64: GICv4.1: Plumb SGI implementation selection in the distributor

2019-12-24 Thread Marc Zyngier
The GICv4.1 architecture gives the hypervisor the option to let
the guest choose whether it wants the good old SGIs with an
active state, or the new, HW-based ones that do not have one.

For this, plumb the configuration of SGIs into the GICv3 MMIO
handling, present the GICD_TYPER2.nASSGIcap to the guest,
and handle the GICD_CTLR.nASSGIreq setting.

Signed-off-by: Marc Zyngier 
---
 virt/kvm/arm/vgic/vgic-mmio-v3.c | 48 ++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index d73f4ffd7d36..31ff8c7e09b0 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -3,6 +3,7 @@
  * VGICv3 MMIO handling functions
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -70,6 +71,8 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu 
*vcpu,
if (vgic->enabled)
value |= GICD_CTLR_ENABLE_SS_G1;
value |= GICD_CTLR_ARE_NS | GICD_CTLR_DS;
+   if (kvm_vgic_global_state.has_gicv4_1 && vgic->nassgireq)
+   value |= GICD_CTLR_nASSGIreq;
break;
case GICD_TYPER:
value = vgic->nr_spis + VGIC_NR_PRIVATE_IRQS;
@@ -81,6 +84,10 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu 
*vcpu,
value |= (INTERRUPT_ID_BITS_SPIS - 1) << 19;
}
break;
+   case GICD_TYPER2:
+   if (kvm_vgic_global_state.has_gicv4_1)
+   value = GICD_TYPER2_nASSGIcap;
+   break;
case GICD_IIDR:
value = (PRODUCT_ID_KVM << GICD_IIDR_PRODUCT_ID_SHIFT) |
(vgic->implementation_rev << GICD_IIDR_REVISION_SHIFT) |
@@ -98,17 +105,43 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
unsigned long val)
 {
struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-   bool was_enabled = dist->enabled;
 
switch (addr & 0x0c) {
-   case GICD_CTLR:
+   case GICD_CTLR: {
+   bool was_enabled, is_hwsgi;
+
+   mutex_lock(&vcpu->kvm->lock);
+
+   was_enabled = dist->enabled;
+   is_hwsgi = dist->nassgireq;
+
dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
 
+   /* Not a GICv4.1? No HW SGIs */
+   if (!kvm_vgic_global_state.has_gicv4_1)
+   val &= ~GICD_CTLR_nASSGIreq;
+
+   /* Dist stays enabled? nASSGIreq is RO */
+   if (was_enabled && dist->enabled) {
+   val &= ~GICD_CTLR_nASSGIreq;
+   val |= FIELD_PREP(GICD_CTLR_nASSGIreq, is_hwsgi);
+   }
+
+   /* Switching HW SGIs? */
+   dist->nassgireq = val & GICD_CTLR_nASSGIreq;
+   if (is_hwsgi != dist->nassgireq)
+   vgic_v4_configure_vsgis(vcpu->kvm);
+
if (!was_enabled && dist->enabled)
vgic_kick_vcpus(vcpu->kvm);
+
+   mutex_unlock(&vcpu->kvm->lock);
break;
+   }
case GICD_TYPER:
+   case GICD_TYPER2:
case GICD_IIDR:
+   /* This is at best for documentation purposes... */
return;
}
 }
@@ -117,10 +150,21 @@ static int vgic_mmio_uaccess_write_v3_misc(struct 
kvm_vcpu *vcpu,
   gpa_t addr, unsigned int len,
   unsigned long val)
 {
+   struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+
switch (addr & 0x0c) {
case GICD_IIDR:
if (val != vgic_mmio_read_v3_misc(vcpu, addr, len))
return -EINVAL;
+   return 0;
+   case GICD_CTLR:
+   /* Not a GICv4.1? No HW SGIs */
+   if (!kvm_vgic_global_state.has_gicv4_1)
+   val &= ~GICD_CTLR_nASSGIreq;
+
+   dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
+   dist->nassgireq = val & GICD_CTLR_nASSGIreq;
+   return 0;
}
 
vgic_mmio_write_v3_misc(vcpu, addr, len, val);
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 13/32] irqchip/gic-v4.1: Add VPE INVALL callback

2019-12-24 Thread Marc Zyngier
GICv4.1 redistributors have a VPE-aware INVALL register. Progress!
We can now emulate a guest-requested INVALL without emiting a
VINVALL command.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 14 ++
 include/linux/irqchip/arm-gic-v3.h |  6 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index a5567d099a3e..403f5753e1ed 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3670,6 +3670,19 @@ static void its_vpe_4_1_deschedule(struct its_vpe *vpe,
}
 }
 
+static void its_vpe_4_1_invall(struct its_vpe *vpe)
+{
+   void __iomem *rdbase;
+   u64 val;
+
+   val  = GICR_INVALLR_V;
+   val |= FIELD_PREP(GICR_INVALLR_VPEID, vpe->vpe_id);
+
+   /* Target the redistributor this vPE is currently known on */
+   rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
+   gic_write_lpir(val, rdbase + GICR_INVALLR);
+}
+
 static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
 {
struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
@@ -3685,6 +3698,7 @@ static int its_vpe_4_1_set_vcpu_affinity(struct irq_data 
*d, void *vcpu_info)
return 0;
 
case INVALL_VPE:
+   its_vpe_4_1_invall(vpe);
return 0;
 
default:
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 1c91dede7663..37b14fb82773 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -249,6 +249,12 @@
 #define GICR_TYPER_COMMON_LPI_AFF  GENMASK_ULL(25, 24)
 #define GICR_TYPER_AFFINITYGENMASK_ULL(63, 32)
 
+#define GICR_INVLPIR_VPEID GENMASK_ULL(47, 32)
+#define GICR_INVLPIR_V GENMASK_ULL(63, 63)
+
+#define GICR_INVALLR_VPEID GICR_INVLPIR_VPEID
+#define GICR_INVALLR_V GICR_INVLPIR_V
+
 #define GIC_V3_REDIST_SIZE 0x2
 
 #define LPI_PROP_GROUP1(1 << 1)
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 12/32] irqchip/gic-v4.1: Add VPE eviction callback

2019-12-24 Thread Marc Zyngier
When descheduling a VPE, special care must be taken to tell the GIC
about whether we want to receive a doorbell or not. This is a
major improvement on GICv4.0, where the doorbell had to be separately
enabled/disabled.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 53 +---
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 49850297fe56..a5567d099a3e 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2631,7 +2631,7 @@ static int __init allocate_lpi_tables(void)
return 0;
 }
 
-static u64 its_clear_vpend_valid(void __iomem *vlpi_base)
+static u64 its_clear_vpend_valid(void __iomem *vlpi_base, u64 clr, u64 set)
 {
u32 count = 100;/* 1s! */
bool clean;
@@ -2639,6 +2639,8 @@ static u64 its_clear_vpend_valid(void __iomem *vlpi_base)
 
val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
val &= ~GICR_VPENDBASER_Valid;
+   val &= ~clr;
+   val |= set;
gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
 
do {
@@ -2651,6 +2653,11 @@ static u64 its_clear_vpend_valid(void __iomem *vlpi_base)
}
} while (!clean && count);
 
+   if (unlikely(val & GICR_VPENDBASER_Dirty)) {
+   pr_err_ratelimited("ITS virtual pending table not cleaning\n");
+   val |= GICR_VPENDBASER_PendingLast;
+   }
+
return val;
 }
 
@@ -2759,7 +2766,7 @@ static void its_cpu_init_lpis(void)
 * ancient programming gets left in and has possibility of
 * corrupting memory.
 */
-   val = its_clear_vpend_valid(vlpi_base);
+   val = its_clear_vpend_valid(vlpi_base, 0, 0);
WARN_ON(val & GICR_VPENDBASER_Dirty);
}
 
@@ -3439,16 +3446,10 @@ static void its_vpe_deschedule(struct its_vpe *vpe)
void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
u64 val;
 
-   val = its_clear_vpend_valid(vlpi_base);
+   val = its_clear_vpend_valid(vlpi_base, 0, 0);
 
-   if (unlikely(val & GICR_VPENDBASER_Dirty)) {
-   pr_err_ratelimited("ITS virtual pending table not cleaning\n");
-   vpe->idai = false;
-   vpe->pending_last = true;
-   } else {
-   vpe->idai = !!(val & GICR_VPENDBASER_IDAI);
-   vpe->pending_last = !!(val & GICR_VPENDBASER_PendingLast);
-   }
+   vpe->idai = !!(val & GICR_VPENDBASER_IDAI);
+   vpe->pending_last = !!(val & GICR_VPENDBASER_PendingLast);
 }
 
 static void its_vpe_invall(struct its_vpe *vpe)
@@ -3640,6 +3641,35 @@ static void its_vpe_4_1_schedule(struct its_vpe *vpe,
gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
 }
 
+static void its_vpe_4_1_deschedule(struct its_vpe *vpe,
+  struct its_cmd_info *info)
+{
+   void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
+   u64 val;
+
+   if (info->req_db) {
+   /*
+* vPE is going to block: make the vPE non-resident with
+* PendingLast clear and DB set. The GIC guarantees that if
+* we read-back PendingLast clear, then a doorbell will be
+* delivered when an interrupt comes.
+*/
+   val = its_clear_vpend_valid(vlpi_base,
+   GICR_VPENDBASER_PendingLast,
+   GICR_VPENDBASER_4_1_DB);
+   vpe->pending_last = !!(val & GICR_VPENDBASER_PendingLast);
+   } else {
+   /*
+* We're not blocking, so just make the vPE non-resident
+* with PendingLast set, indicating that we'll be back.
+*/
+   val = its_clear_vpend_valid(vlpi_base,
+   0,
+   GICR_VPENDBASER_PendingLast);
+   vpe->pending_last = true;
+   }
+}
+
 static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
 {
struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
@@ -3651,6 +3681,7 @@ static int its_vpe_4_1_set_vcpu_affinity(struct irq_data 
*d, void *vcpu_info)
return 0;
 
case DESCHEDULE_VPE:
+   its_vpe_4_1_deschedule(vpe, info);
return 0;
 
case INVALL_VPE:
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 11/32] irqchip/gic-v4.1: Add VPE residency callback

2019-12-24 Thread Marc Zyngier
Making a VPE resident on GICv4.1 is pretty simple, as it is just a
single write to the local redistributor. We just need extra information
about which groups to enable, which the KVM code will have to provide.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 17 +
 include/linux/irqchip/arm-gic-v3.h |  9 +
 include/linux/irqchip/arm-gic-v4.h |  5 +
 3 files changed, 31 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 0da6baa48153..49850297fe56 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3625,12 +3625,29 @@ static void its_vpe_4_1_unmask_irq(struct irq_data *d)
its_vpe_4_1_send_inv(d);
 }
 
+static void its_vpe_4_1_schedule(struct its_vpe *vpe,
+struct its_cmd_info *info)
+{
+   void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
+   u64 val = 0;
+
+   /* Schedule the VPE */
+   val |= GICR_VPENDBASER_Valid;
+   val |= info->g0en ? GICR_VPENDBASER_4_1_VGRP0EN : 0;
+   val |= info->g1en ? GICR_VPENDBASER_4_1_VGRP1EN : 0;
+   val |= FIELD_PREP(GICR_VPENDBASER_4_1_VPEID, vpe->vpe_id);
+
+   gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+}
+
 static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
 {
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
struct its_cmd_info *info = vcpu_info;
 
switch (info->cmd_type) {
case SCHEDULE_VPE:
+   its_vpe_4_1_schedule(vpe, info);
return 0;
 
case DESCHEDULE_VPE:
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 33519b7c96cf..1c91dede7663 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -329,6 +329,15 @@
 #define GICR_VPENDBASER_IDAI   (1ULL << 62)
 #define GICR_VPENDBASER_Valid  (1ULL << 63)
 
+/*
+ * GICv4.1 VPENDBASER, used for VPE residency. On top of these fields,
+ * also use the above Valid, PendingLast and Dirty.
+ */
+#define GICR_VPENDBASER_4_1_DB (1ULL << 62)
+#define GICR_VPENDBASER_4_1_VGRP0EN(1ULL << 59)
+#define GICR_VPENDBASER_4_1_VGRP1EN(1ULL << 58)
+#define GICR_VPENDBASER_4_1_VPEID  GENMASK_ULL(15, 0)
+
 /*
  * ITS registers, offsets from ITS_base
  */
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index 498e523085a7..d9c34968467a 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -100,6 +100,11 @@ struct its_cmd_info {
union {
struct its_vlpi_map *map;
u8  config;
+   boolreq_db;
+   struct {
+   boolg0en;
+   boolg1en;
+   };
};
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 23/32] irqchip/gic-v4.1: Move doorbell management to the GICv4 abstraction layer

2019-12-24 Thread Marc Zyngier
In order to hide some of the differences between v4.0 and v4.1, move
the doorbell management out of the KVM code, and into the GICv4-specific
layer. This allows the calling code to ask for the doorbell when blocking,
and otherwise to leave the doorbell permanently disabled.

This matches the v4.1 code perfectly, and only results in a minor
refactoring of the v4.0 code.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v4.c   | 45 +++---
 include/kvm/arm_vgic.h |  1 +
 include/linux/irqchip/arm-gic-v4.h |  3 +-
 virt/kvm/arm/vgic/vgic-v3.c|  1 +
 virt/kvm/arm/vgic/vgic-v4.c| 34 ++
 5 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index c01910d53f9e..117ba6db023d 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -87,6 +87,11 @@ static struct irq_domain *gic_domain;
 static const struct irq_domain_ops *vpe_domain_ops;
 static const struct irq_domain_ops *sgi_domain_ops;
 
+static bool has_v4_1(void)
+{
+   return !!sgi_domain_ops;
+}
+
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {
int vpe_base_irq, i;
@@ -139,18 +144,50 @@ static int its_send_vpe_cmd(struct its_vpe *vpe, struct 
its_cmd_info *info)
return irq_set_vcpu_affinity(vpe->irq, info);
 }
 
-int its_schedule_vpe(struct its_vpe *vpe, bool on)
+int its_make_vpe_non_resident(struct its_vpe *vpe, bool db)
 {
-   struct its_cmd_info info;
+   struct irq_desc *desc = irq_to_desc(vpe->irq);
+   struct its_cmd_info info = { };
int ret;
 
WARN_ON(preemptible());
 
-   info.cmd_type = on ? SCHEDULE_VPE : DESCHEDULE_VPE;
+   info.cmd_type = DESCHEDULE_VPE;
+   if (has_v4_1()) {
+   /* GICv4.1 can directly deal with doorbells */
+   info.req_db = db;
+   } else {
+   /* Undo the nested disable_irq() calls... */
+   while (db && irqd_irq_disabled(&desc->irq_data))
+   enable_irq(vpe->irq);
+   }
+
+   ret = its_send_vpe_cmd(vpe, &info);
+   if (!ret)
+   vpe->resident = false;
+
+   return ret;
+}
+
+int its_make_vpe_resident(struct its_vpe *vpe, bool g0en, bool g1en)
+{
+   struct its_cmd_info info = { };
+   int ret;
+
+   WARN_ON(preemptible());
+
+   info.cmd_type = SCHEDULE_VPE;
+   if (has_v4_1()) {
+   info.g0en = g0en;
+   info.g1en = g1en;
+   } else {
+   /* Disabled the doorbell, as we're about to enter the guest */
+   disable_irq_nosync(vpe->irq);
+   }
 
ret = its_send_vpe_cmd(vpe, &info);
if (!ret)
-   vpe->resident = on;
+   vpe->resident = true;
 
return ret;
 }
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9d53f545a3d5..63457908c9c4 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -70,6 +70,7 @@ struct vgic_global {
 
/* Hardware has GICv4? */
boolhas_gicv4;
+   boolhas_gicv4_1;
 
/* GIC system register CPU interface */
struct static_key_false gicv3_cpuif;
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index a1a9d40266f5..cca4198fa1d5 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -120,7 +120,8 @@ struct its_cmd_info {
 
 int its_alloc_vcpu_irqs(struct its_vm *vm);
 void its_free_vcpu_irqs(struct its_vm *vm);
-int its_schedule_vpe(struct its_vpe *vpe, bool on);
+int its_make_vpe_resident(struct its_vpe *vpe, bool g0en, bool g1en);
+int its_make_vpe_non_resident(struct its_vpe *vpe, bool db);
 int its_invall_vpe(struct its_vpe *vpe);
 int its_map_vlpi(int irq, struct its_vlpi_map *map);
 int its_get_vlpi(int irq, struct its_vlpi_map *map);
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index f45635a6f0ec..c2fdea201747 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -595,6 +595,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
/* GICv4 support? */
if (info->has_v4) {
kvm_vgic_global_state.has_gicv4 = gicv4_enable;
+   kvm_vgic_global_state.has_gicv4_1 = info->has_v4_1;
kvm_info("GICv4 support %sabled\n",
 gicv4_enable ? "en" : "dis");
}
diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
index 46f875589c47..1eb0f8c76219 100644
--- a/virt/kvm/arm/vgic/vgic-v4.c
+++ b/virt/kvm/arm/vgic/vgic-v4.c
@@ -67,10 +67,10 @@
  * it. And if we've migrated our vcpu from one CPU to another, we must
  * tell the ITS (so that the messages reach the right redistributor).
  * This is done in two steps: first issue a irq_set_affinity() on the
- * irq corresponding to the vcpu, then call its_schedule_vpe(). You
- * must be in a non-preemp

[PATCH v3 24/32] irqchip/gic-v4.1: Add VSGI allocation/teardown

2019-12-24 Thread Marc Zyngier
Allocate per-VPE SGIs when initializing the GIC-specific part of the
VPE data structure.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v4.c   | 68 +-
 include/linux/irqchip/arm-gic-v4.h |  2 +
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 117ba6db023d..99b33f60ac63 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -92,6 +92,47 @@ static bool has_v4_1(void)
return !!sgi_domain_ops;
 }
 
+static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
+{
+   char *name;
+   int sgi_base;
+
+   if (!has_v4_1())
+   return 0;
+
+   name = kasprintf(GFP_KERNEL, "GICv4-sgi-%d", task_pid_nr(current));
+   if (!name)
+   goto err;
+
+   vpe->fwnode = irq_domain_alloc_named_id_fwnode(name, idx);
+   if (!vpe->fwnode)
+   goto err;
+
+   kfree(name);
+   name = NULL;
+
+   vpe->sgi_domain = irq_domain_create_linear(vpe->fwnode, 16,
+  sgi_domain_ops, vpe);
+   if (!vpe->sgi_domain)
+   goto err;
+
+   sgi_base = __irq_domain_alloc_irqs(vpe->sgi_domain, -1, 16,
+  NUMA_NO_NODE, vpe,
+  false, NULL);
+   if (sgi_base <= 0)
+   goto err;
+
+   return 0;
+
+err:
+   if (vpe->sgi_domain)
+   irq_domain_remove(vpe->sgi_domain);
+   if (vpe->fwnode)
+   irq_domain_free_fwnode(vpe->fwnode);
+   kfree(name);
+   return -ENOMEM;
+}
+
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {
int vpe_base_irq, i;
@@ -118,8 +159,13 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
if (vpe_base_irq <= 0)
goto err;
 
-   for (i = 0; i < vm->nr_vpes; i++)
+   for (i = 0; i < vm->nr_vpes; i++) {
+   int ret;
vm->vpes[i]->irq = vpe_base_irq + i;
+   ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
+   if (ret)
+   goto err;
+   }
 
return 0;
 
@@ -132,8 +178,28 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
return -ENOMEM;
 }
 
+static void its_free_sgi_irqs(struct its_vm *vm)
+{
+   int i;
+
+   if (!has_v4_1())
+   return;
+
+   for (i = 0; i < vm->nr_vpes; i++) {
+   unsigned int irq = irq_find_mapping(vm->vpes[i]->sgi_domain, 0);
+
+   if (WARN_ON(!irq))
+   continue;
+
+   irq_domain_free_irqs(irq, 16);
+   irq_domain_remove(vm->vpes[i]->sgi_domain);
+   irq_domain_free_fwnode(vm->vpes[i]->fwnode);
+   }
+}
+
 void its_free_vcpu_irqs(struct its_vm *vm)
 {
+   its_free_sgi_irqs(vm);
irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
irq_domain_remove(vm->domain);
irq_domain_free_fwnode(vm->fwnode);
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index cca4198fa1d5..9fbd0418f569 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -49,6 +49,8 @@ struct its_vpe {
};
/* GICv4.1 implementations */
struct {
+   struct fwnode_handle*fwnode;
+   struct irq_domain   *sgi_domain;
struct {
u8  priority;
boolenabled;
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 27/32] KVM: arm64: GICv4.1: Let doorbells be auto-enabled

2019-12-24 Thread Marc Zyngier
As GICv4.1 understands the life cycle of doorbells (instead of
just randomly firing them at the most inconvenient time), just
enable them at irq_request time, and be done with it.

Signed-off-by: Marc Zyngier 
---
 virt/kvm/arm/vgic/vgic-v4.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
index 1eb0f8c76219..c2fcde104ea2 100644
--- a/virt/kvm/arm/vgic/vgic-v4.c
+++ b/virt/kvm/arm/vgic/vgic-v4.c
@@ -141,6 +141,7 @@ int vgic_v4_init(struct kvm *kvm)
 
kvm_for_each_vcpu(i, vcpu, kvm) {
int irq = dist->its_vm.vpes[i]->irq;
+   unsigned long irq_flags = DB_IRQ_FLAGS;
 
/*
 * Don't automatically enable the doorbell, as we're
@@ -148,8 +149,14 @@ int vgic_v4_init(struct kvm *kvm)
 * blocked. Also disable the lazy disabling, as the
 * doorbell could kick us out of the guest too
 * early...
+*
+* On GICv4.1, the doorbell is managed in HW and must
+* be left enabled.
 */
-   irq_set_status_flags(irq, DB_IRQ_FLAGS);
+   if (kvm_vgic_global_state.has_gicv4_1)
+   irq_flags &= ~IRQ_NOAUTOEN;
+   irq_set_status_flags(irq, irq_flags);
+
ret = request_irq(irq, vgic_v4_doorbell_handler,
  0, "vcpu", vcpu);
if (ret) {
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 26/32] irqchip/gic-v4.1: Eagerly vmap vPEs

2019-12-24 Thread Marc Zyngier
Now that we have HW-accelerated SGIs being delivered to VPEs, it
becomes required to map the VPEs on all ITSs instead of relying
on the lazy approach that we would use when using the ITS-list
mechanism.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 39 +---
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5126bdcfe079..3234bb9fbdbe 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1554,12 +1554,31 @@ static int its_irq_set_irqchip_state(struct irq_data *d,
return 0;
 }
 
+/*
+ * Two favourable cases:
+ *
+ * (a) Either we have a GICv4.1, and all vPEs have to be mapped at all times
+ * for vSGI delivery
+ *
+ * (b) Or the ITSs do not use a list map, meaning that VMOVP is cheap enough
+ * and we're better off mapping all VPEs always
+ *
+ * If neither (a) nor (b) is true, then we map vPEs on demand.
+ *
+ */
+static bool gic_requires_eager_mapping(void)
+{
+   if (!its_list_map || gic_rdists->has_rvpeid)
+   return true;
+
+   return false;
+}
+
 static void its_map_vm(struct its_node *its, struct its_vm *vm)
 {
unsigned long flags;
 
-   /* Not using the ITS list? Everything is always mapped. */
-   if (!its_list_map)
+   if (gic_requires_eager_mapping())
return;
 
raw_spin_lock_irqsave(&vmovp_lock, flags);
@@ -1593,7 +1612,7 @@ static void its_unmap_vm(struct its_node *its, struct 
its_vm *vm)
unsigned long flags;
 
/* Not using the ITS list? Everything is always mapped. */
-   if (!its_list_map)
+   if (gic_requires_eager_mapping())
return;
 
raw_spin_lock_irqsave(&vmovp_lock, flags);
@@ -4109,8 +4128,12 @@ static int its_vpe_irq_domain_activate(struct irq_domain 
*domain,
struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
struct its_node *its;
 
-   /* If we use the list map, we issue VMAPP on demand... */
-   if (its_list_map)
+   /*
+* If we use the list map, we issue VMAPP on demand... Unless
+* we're on a GICv4.1 and we eagerly map the VPE on all ITSs
+* so that VSGIs can work.
+*/
+   if (!gic_requires_eager_mapping())
return 0;
 
/* Map the VPE to the first possible CPU */
@@ -4136,10 +4159,10 @@ static void its_vpe_irq_domain_deactivate(struct 
irq_domain *domain,
struct its_node *its;
 
/*
-* If we use the list map, we unmap the VPE once no VLPIs are
-* associated with the VM.
+* If we use the list map on GICv4.0, we unmap the VPE once no
+* VLPIs are associated with the VM.
 */
-   if (its_list_map)
+   if (!gic_requires_eager_mapping())
return;
 
list_for_each_entry(its, &its_nodes, entry) {
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 06/32] irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP

2019-12-24 Thread Marc Zyngier
The ITS VMAPP command gains some new fields with GICv4.1:
- a default doorbell, which allows a single doorbell to be used for
  all the VLPIs routed to a given VPE
- a pointer to the configuration table (instead of having it in a register
  that gets context switched)
- a flag indicating whether this is the first map or the last unmap for
  this particular VPE
- a flag indicating whether the pending table is known to be zeroed, or not

Plumb in the new fields in the VMAPP builder, and add the map/unmap
refcounting so that the ITS can do the right thing.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 60 +++---
 include/linux/irqchip/arm-gic-v4.h | 18 +++--
 2 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 15dea531c34f..e3c7d9ae0bdb 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -450,6 +450,27 @@ static void its_encode_vpt_size(struct its_cmd_block *cmd, 
u8 vpt_size)
its_mask_encode(&cmd->raw_cmd[3], vpt_size, 4, 0);
 }
 
+static void its_encode_vconf_addr(struct its_cmd_block *cmd, u64 vconf_pa)
+{
+   its_mask_encode(&cmd->raw_cmd[0], vconf_pa >> 16, 51, 16);
+}
+
+static void its_encode_alloc(struct its_cmd_block *cmd, bool alloc)
+{
+   its_mask_encode(&cmd->raw_cmd[0], alloc, 8, 8);
+}
+
+static void its_encode_ptz(struct its_cmd_block *cmd, bool ptz)
+{
+   its_mask_encode(&cmd->raw_cmd[0], ptz, 9, 9);
+}
+
+static void its_encode_vmapp_default_db(struct its_cmd_block *cmd,
+   u32 vpe_db_lpi)
+{
+   its_mask_encode(&cmd->raw_cmd[1], vpe_db_lpi, 31, 0);
+}
+
 static inline void its_fixup_cmd(struct its_cmd_block *cmd)
 {
/* Let's fixup BE commands */
@@ -633,19 +654,45 @@ static struct its_vpe *its_build_vmapp_cmd(struct 
its_node *its,
   struct its_cmd_block *cmd,
   struct its_cmd_desc *desc)
 {
-   unsigned long vpt_addr;
+   unsigned long vpt_addr, vconf_addr;
u64 target;
-
-   vpt_addr = 
virt_to_phys(page_address(desc->its_vmapp_cmd.vpe->vpt_page));
-   target = desc->its_vmapp_cmd.col->target_address + 
its->vlpi_redist_offset;
+   bool alloc;
 
its_encode_cmd(cmd, GITS_CMD_VMAPP);
its_encode_vpeid(cmd, desc->its_vmapp_cmd.vpe->vpe_id);
its_encode_valid(cmd, desc->its_vmapp_cmd.valid);
+
+   if (!desc->its_vmapp_cmd.valid) {
+   if (is_v4_1(its)) {
+   alloc = 
!atomic_dec_return(&desc->its_vmapp_cmd.vpe->vmapp_count);
+   its_encode_alloc(cmd, alloc);
+   }
+
+   goto out;
+   }
+
+   vpt_addr = 
virt_to_phys(page_address(desc->its_vmapp_cmd.vpe->vpt_page));
+   target = desc->its_vmapp_cmd.col->target_address + 
its->vlpi_redist_offset;
+
its_encode_target(cmd, target);
its_encode_vpt_addr(cmd, vpt_addr);
its_encode_vpt_size(cmd, LPI_NRBITS - 1);
 
+   if (!is_v4_1(its))
+   goto out;
+
+   vconf_addr = 
virt_to_phys(page_address(desc->its_vmapp_cmd.vpe->its_vm->vprop_page));
+
+   alloc = !atomic_fetch_inc(&desc->its_vmapp_cmd.vpe->vmapp_count);
+
+   its_encode_alloc(cmd, alloc);
+
+   /* We can only signal PTZ when alloc==1. Why do we have two bits? */
+   its_encode_ptz(cmd, alloc);
+   its_encode_vconf_addr(cmd, vconf_addr);
+   its_encode_vmapp_default_db(cmd, desc->its_vmapp_cmd.vpe->vpe_db_lpi);
+
+out:
its_fixup_cmd(cmd);
 
return valid_vpe(its, desc->its_vmapp_cmd.vpe);
@@ -3493,7 +3540,10 @@ static int its_vpe_init(struct its_vpe *vpe)
 
vpe->vpe_id = vpe_id;
vpe->vpt_page = vpt_page;
-   vpe->vpe_proxy_event = -1;
+   if (gic_rdists->has_rvpeid)
+   atomic_set(&vpe->vmapp_count, 0);
+   else
+   vpe->vpe_proxy_event = -1;
 
return 0;
 }
diff --git a/include/linux/irqchip/arm-gic-v4.h 
b/include/linux/irqchip/arm-gic-v4.h
index 5dbcfc65f21e..498e523085a7 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -39,8 +39,20 @@ struct its_vpe {
irq_hw_number_t vpe_db_lpi;
/* VPE resident */
boolresident;
-   /* VPE proxy mapping */
-   int vpe_proxy_event;
+   union {
+   /* GICv4.0 implementations */
+   struct {
+   /* VPE proxy mapping */
+   int vpe_proxy_event;
+   /* Implementation Defined Area Invalid */
+   boolidai;
+   };
+   /* GICv4.1 implementations */
+   struct {
+   atomic_t vmapp_count;
+   };
+   };
+
/*
 * This collecti

[PATCH v3 01/32] irqchip/gic-v3: Detect GICv4.1 supporting RVPEID

2019-12-24 Thread Marc Zyngier
GICv4.1 supports the RVPEID ("Residency per vPE ID"), which allows for
a much efficient way of making virtual CPUs resident (to allow direct
injection of interrupts).

The functionnality needs to be discovered on each and every redistributor
in the system, and disabled if the settings are inconsistent.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3.c   | 21 ++---
 include/linux/irqchip/arm-gic-v3.h |  2 ++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index d6218012097b..ffcb018395ed 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -858,8 +858,21 @@ static int __gic_update_rdist_properties(struct 
redist_region *region,
 void __iomem *ptr)
 {
u64 typer = gic_read_typer(ptr + GICR_TYPER);
+
gic_data.rdists.has_vlpis &= !!(typer & GICR_TYPER_VLPIS);
-   gic_data.rdists.has_direct_lpi &= !!(typer & GICR_TYPER_DirectLPIS);
+
+   /* RVPEID implies some form of DirectLPI, no matter what the doc 
says... :-/ */
+   gic_data.rdists.has_rvpeid &= !!(typer & GICR_TYPER_RVPEID);
+   gic_data.rdists.has_direct_lpi &= (!!(typer & GICR_TYPER_DirectLPIS) |
+  gic_data.rdists.has_rvpeid);
+
+   /* Detect non-sensical configurations */
+   if (WARN_ON_ONCE(gic_data.rdists.has_rvpeid && 
!gic_data.rdists.has_vlpis)) {
+   gic_data.rdists.has_direct_lpi = false;
+   gic_data.rdists.has_vlpis = false;
+   gic_data.rdists.has_rvpeid = false;
+   }
+
gic_data.ppi_nr = min(GICR_TYPER_NR_PPIS(typer), gic_data.ppi_nr);
 
return 1;
@@ -872,9 +885,10 @@ static void gic_update_rdist_properties(void)
if (WARN_ON(gic_data.ppi_nr == UINT_MAX))
gic_data.ppi_nr = 0;
pr_info("%d PPIs implemented\n", gic_data.ppi_nr);
-   pr_info("%sVLPI support, %sdirect LPI support\n",
+   pr_info("%sVLPI support, %sdirect LPI support, %sRVPEID support\n",
!gic_data.rdists.has_vlpis ? "no " : "",
-   !gic_data.rdists.has_direct_lpi ? "no " : "");
+   !gic_data.rdists.has_direct_lpi ? "no " : "",
+   !gic_data.rdists.has_rvpeid ? "no " : "");
 }
 
 /* Check whether it's single security state view */
@@ -1566,6 +1580,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
 &gic_data);
irq_domain_update_bus_token(gic_data.domain, DOMAIN_BUS_WIRED);
gic_data.rdists.rdist = alloc_percpu(typeof(*gic_data.rdists.rdist));
+   gic_data.rdists.has_rvpeid = true;
gic_data.rdists.has_vlpis = true;
gic_data.rdists.has_direct_lpi = true;
 
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index de991d6633a5..9a5f85d30701 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -234,6 +234,7 @@
 #define GICR_TYPER_VLPIS   (1U << 1)
 #define GICR_TYPER_DirectLPIS  (1U << 3)
 #define GICR_TYPER_LAST(1U << 4)
+#define GICR_TYPER_RVPEID  (1U << 7)
 
 #define GIC_V3_REDIST_SIZE 0x2
 
@@ -615,6 +616,7 @@ struct rdists {
u64 flags;
u32 gicd_typer;
boolhas_vlpis;
+   boolhas_rvpeid;
boolhas_direct_lpi;
 };
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 07/32] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set

2019-12-24 Thread Marc Zyngier
The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
  the doorbell, so we don't need to enable/disable it all
  the time

So let's escape early from the proxy related functions.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e3c7d9ae0bdb..86c69b5cc156 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3213,7 +3213,7 @@ static const struct irq_domain_ops its_domain_ops = {
 /*
  * This is insane.
  *
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
  * likely), the only way to perform an invalidate is to use a fake
  * device to issue an INV command, implying that the LPI has first
  * been mapped to some event on that device. Since this is not exactly
@@ -3221,9 +3221,20 @@ static const struct irq_domain_ops its_domain_ops = {
  * only issue an UNMAP if we're short on available slots.
  *
  * Broken by design(tm).
+ *
+ * GICv4.1, on the other hand, mandates that we're able to invalidate
+ * by writing to a MMIO register. It doesn't implement the whole of
+ * DirectLPI, but that's good enough. And most of the time, we don't
+ * even have to invalidate anything, as the redistributor can be told
+ * whether to generate a doorbell or not (we thus leave it enabled,
+ * always).
  */
 static void its_vpe_db_proxy_unmap_locked(struct its_vpe *vpe)
 {
+   /* GICv4.1 doesn't use a proxy, so nothing to do here */
+   if (gic_rdists->has_rvpeid)
+   return;
+
/* Already unmapped? */
if (vpe->vpe_proxy_event == -1)
return;
@@ -3246,6 +3257,10 @@ static void its_vpe_db_proxy_unmap_locked(struct its_vpe 
*vpe)
 
 static void its_vpe_db_proxy_unmap(struct its_vpe *vpe)
 {
+   /* GICv4.1 doesn't use a proxy, so nothing to do here */
+   if (gic_rdists->has_rvpeid)
+   return;
+
if (!gic_rdists->has_direct_lpi) {
unsigned long flags;
 
@@ -3257,6 +3272,10 @@ static void its_vpe_db_proxy_unmap(struct its_vpe *vpe)
 
 static void its_vpe_db_proxy_map_locked(struct its_vpe *vpe)
 {
+   /* GICv4.1 doesn't use a proxy, so nothing to do here */
+   if (gic_rdists->has_rvpeid)
+   return;
+
/* Already mapped? */
if (vpe->vpe_proxy_event != -1)
return;
@@ -3279,6 +3298,10 @@ static void its_vpe_db_proxy_move(struct its_vpe *vpe, 
int from, int to)
unsigned long flags;
struct its_collection *target_col;
 
+   /* GICv4.1 doesn't use a proxy, so nothing to do here */
+   if (gic_rdists->has_rvpeid)
+   return;
+
if (gic_rdists->has_direct_lpi) {
void __iomem *rdbase;
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 05/32] irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation

2019-12-24 Thread Marc Zyngier
GICv4.1 defines a new VPE table that is potentially shared between
both the ITSs and the redistributors, following complicated affinity
rules.

To make things more confusing, the programming of this table at
the redistributor level is reusing the GICv4.0 GICR_VPROPBASER register
for something completely different.

The code flow is somewhat complexified by the need to respect the
affinities required by the HW, meaning that tables can either be
inherited from a previously discovered ITS or redistributor.

Signed-off-by: Marc Zyngier 
---
 arch/arm/include/asm/arch_gicv3.h   |   2 +
 arch/arm64/include/asm/arch_gicv3.h |   1 +
 drivers/irqchip/irq-gic-v3-its.c| 312 +++-
 include/linux/irqchip/arm-gic-v3.h  |  33 ++-
 4 files changed, 341 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/arch_gicv3.h 
b/arch/arm/include/asm/arch_gicv3.h
index fa50bb04f580..b5752f0e8936 100644
--- a/arch/arm/include/asm/arch_gicv3.h
+++ b/arch/arm/include/asm/arch_gicv3.h
@@ -10,6 +10,7 @@
 #ifndef __ASSEMBLY__
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -327,6 +328,7 @@ static inline u64 __gic_readq_nonatomic(const volatile void 
__iomem *addr)
 /*
  * GITS_VPROPBASER - hi and lo bits may be accessed independently.
  */
+#define gits_read_vpropbaser(c)__gic_readq_nonatomic(c)
 #define gits_write_vpropbaser(v, c)__gic_writeq_nonatomic(v, c)
 
 /*
diff --git a/arch/arm64/include/asm/arch_gicv3.h 
b/arch/arm64/include/asm/arch_gicv3.h
index 89e4c8b79349..4750fc8030c3 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -141,6 +141,7 @@ static inline u32 gic_read_rpr(void)
 #define gicr_read_pendbaser(c) readq_relaxed(c)
 
 #define gits_write_vpropbaser(v, c)writeq_relaxed(v, c)
+#define gits_read_vpropbaser(c)readq_relaxed(c)
 
 #define gits_write_vpendbaser(v, c)writeq_relaxed(v, c)
 #define gits_read_vpendbaser(c)readq_relaxed(c)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d9d256471dc0..15dea531c34f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -45,6 +45,7 @@
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
+#define RDIST_FLAGS_VPE_INDIRECT   (1 << 2)
 
 static u32 lpi_id_bits;
 
@@ -106,6 +107,7 @@ struct its_node {
u64 typer;
u64 cbaser_save;
u32 ctlr_save;
+   u32 mpidr;
struct list_headits_device_list;
u64 flags;
unsigned long   list_nr;
@@ -116,6 +118,7 @@ struct its_node {
 };
 
 #define is_v4(its) (!!((its)->typer & GITS_TYPER_VLPIS))
+#define is_v4_1(its)   (!!((its)->typer & GITS_TYPER_VMAPP))
 #define device_ids(its)(FIELD_GET(GITS_TYPER_DEVBITS, 
(its)->typer) + 1)
 
 #define ITS_ITT_ALIGN  SZ_256
@@ -2099,6 +2102,65 @@ static bool its_parse_indirect_baser(struct its_node 
*its,
return indirect;
 }
 
+static u32 compute_common_aff(u64 val)
+{
+   u32 aff, clpiaff;
+
+   aff = FIELD_GET(GICR_TYPER_AFFINITY, val);
+   clpiaff = FIELD_GET(GICR_TYPER_COMMON_LPI_AFF, val);
+
+   return aff & ~(GENMASK(31, 0) >> (clpiaff * 8));
+}
+
+static u32 compute_its_aff(struct its_node *its)
+{
+   u64 val;
+   u32 svpet;
+
+   /*
+* Reencode the ITS SVPET and MPIDR as a GICR_TYPER, and compute
+* the resulting affinity. We then use that to see if this match
+* our own affinity.
+*/
+   svpet = FIELD_GET(GITS_TYPER_SVPET, its->typer);
+   val  = FIELD_PREP(GICR_TYPER_COMMON_LPI_AFF, svpet);
+   val |= FIELD_PREP(GICR_TYPER_AFFINITY, its->mpidr);
+   return compute_common_aff(val);
+}
+
+static struct its_node *find_sibling_its(struct its_node *cur_its)
+{
+   struct its_node *its;
+   u32 aff;
+
+   if (!FIELD_GET(GITS_TYPER_SVPET, cur_its->typer))
+   return NULL;
+
+   aff = compute_its_aff(cur_its);
+
+   list_for_each_entry(its, &its_nodes, entry) {
+   u64 baser;
+
+   if (!is_v4_1(its) || its == cur_its)
+   continue;
+
+   if (!FIELD_GET(GITS_TYPER_SVPET, its->typer))
+   continue;
+
+   if (aff != compute_its_aff(its))
+   continue;
+
+   /* GICv4.1 guarantees that the vPE table is GITS_BASER2 */
+   baser = its->tables[2].val;
+   if (!(baser & GITS_BASER_VALID))
+   continue;
+
+   return its;
+   }
+
+   return NULL;
+}
+
 static void its_free_tables(struct its_node *its)
 {
int i;
@@ -2141,6 +2203,17 @@ static int its_alloc_tables(struct its_node *its)
break;
 

[PATCH v3 08/32] irqchip/gic-v4.1: Implement the v4.1 flavour of VMOVP

2019-12-24 Thread Marc Zyngier
With GICv4.1, VMOVP is extended to allow a default doorbell to be
specified, as well as a validity bit for this doorbell. As an added
bonus, VMOVP isn't required anymore of moving a VPE between
redistributors that share the same affinity.

Let's add this support to the VMOVP builder, and make sure we don't
issue the command if we don't really need to.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 40 ++--
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 86c69b5cc156..fd9d3b6bb465 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -471,6 +471,17 @@ static void its_encode_vmapp_default_db(struct 
its_cmd_block *cmd,
its_mask_encode(&cmd->raw_cmd[1], vpe_db_lpi, 31, 0);
 }
 
+static void its_encode_vmovp_default_db(struct its_cmd_block *cmd,
+   u32 vpe_db_lpi)
+{
+   its_mask_encode(&cmd->raw_cmd[3], vpe_db_lpi, 31, 0);
+}
+
+static void its_encode_db(struct its_cmd_block *cmd, bool db)
+{
+   its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
+}
+
 static inline void its_fixup_cmd(struct its_cmd_block *cmd)
 {
/* Let's fixup BE commands */
@@ -757,6 +768,11 @@ static struct its_vpe *its_build_vmovp_cmd(struct its_node 
*its,
its_encode_vpeid(cmd, desc->its_vmovp_cmd.vpe->vpe_id);
its_encode_target(cmd, target);
 
+   if (is_v4_1(its)) {
+   its_encode_db(cmd, true);
+   its_encode_vmovp_default_db(cmd, 
desc->its_vmovp_cmd.vpe->vpe_db_lpi);
+   }
+
its_fixup_cmd(cmd);
 
return valid_vpe(its, desc->its_vmovp_cmd.vpe);
@@ -3328,7 +3344,7 @@ static int its_vpe_set_affinity(struct irq_data *d,
bool force)
 {
struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
-   int cpu = cpumask_first(mask_val);
+   int from, cpu = cpumask_first(mask_val);
 
/*
 * Changing affinity is mega expensive, so let's be as lazy as
@@ -3336,14 +3352,24 @@ static int its_vpe_set_affinity(struct irq_data *d,
 * into the proxy device, we need to move the doorbell
 * interrupt to its new location.
 */
-   if (vpe->col_idx != cpu) {
-   int from = vpe->col_idx;
+   if (vpe->col_idx == cpu)
+   goto out;
 
-   vpe->col_idx = cpu;
-   its_send_vmovp(vpe);
-   its_vpe_db_proxy_move(vpe, from, cpu);
-   }
+   from = vpe->col_idx;
+   vpe->col_idx = cpu;
+
+   /*
+* GICv4.1 allows us to skip VMOVP if moving to a cpu whose RD
+* is sharing its VPE table with the current one.
+*/
+   if (gic_data_rdist_cpu(cpu)->vpe_table_mask &&
+   cpumask_test_cpu(from, gic_data_rdist_cpu(cpu)->vpe_table_mask))
+   goto out;
 
+   its_send_vmovp(vpe);
+   its_vpe_db_proxy_move(vpe, from, cpu);
+
+out:
irq_data_update_effective_affinity(d, cpumask_of(cpu));
 
return IRQ_SET_MASK_OK_DONE;
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 00/32] irqchip/gic-v4: GICv4.1 architecture support

2019-12-24 Thread Marc Zyngier
[All I want for Christmas is... another monster GIC series!]

This rather long series expands the existing GICv4 support to deal with the
new GICv4.1 architecture, which comes with a set of major improvements
compared to v4.0:

- One architectural doorbell per vcpu, instead of one doorbell per VLPI

- Doorbell entirely managed by the HW, with an "at most once" delivery
  guarantee per non-residency phase and only when requested by the
  hypervisor

- A shared memory scheme between ITSs and redistributors, allowing for an
  optimised residency sequence (the use of VMOVP becomes less frequent)

- Support for direct virtual SGI delivery (the injection path still involves
  the hypervisor), at the cost of losing the active state on SGIs. It
  shouldn't be a big deal, but some guest operating systems might notice
  (Linux definitely won't care).

On the other hand, public documentation is not available yet, so that's a
bit annoying...

The series is roughly organised in 5 parts:

(1) Feature detection, errata workaround for TX1
(2) VPE table allocation, new flavours of VMAPP/VMOVP commands
(3) v4.1 doorbell management
(4) Virtual SGI support
(5) Plumbing of virtual SGIs in KVM

Ideally, I'd like to start merging some of this into 5.6.

Notes:

  - This series has uncovered a behaviour that looks like a HW bug on
the Cavium ThunderX (aka TX1) platform (see patch #3). I'd very
much welcome some clarification from the Marvell/Cavium folks on
Cc, as well as an official erratum number if this happens to be an
actual bug.

[v3 update]
People have ignored for two months now, and it is fairly obvious
that support for this machine is slowly bit-rotting. Maybe I'll
drop the patch and instead start the process of removing all TX1
support from the kernel (we'd certainly be better off without it).

* From v2 [2]:
  - Another bunch of fixes thanks to Zenghui Yu's very careful review
  - HW-accelerated SGIs are now optional thanks to new architected
discovery/selection bits exposed by KVM and used by the guest kernel
  - Rebased on v5.5-rc2

* From v1 [1]:
  - A bunch of minor reworks after Zenghui Yu's review
  - A workaround for what looks like a new and unexpected TX1 bug
  - A subtle reorder of the series so that some patches can go in early

[1] https://lore.kernel.org/lkml/20190923182606.32100-1-...@kernel.org/
[2] https://lore.kernel.org/lkml/20191027144234.8395-1-...@kernel.org/

Marc Zyngier (32):
  irqchip/gic-v3: Detect GICv4.1 supporting RVPEID
  irqchip/gic-v3: Add GICv4.1 VPEID size discovery
  irqchip/gic-v3: Workaround Cavium TX1 erratum when reading GICD_TYPER2
  irqchip/gic-v3: Use SGIs without active state if offered
  irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation
  irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP
  irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set
  irqchip/gic-v4.1: Implement the v4.1 flavour of VMOVP
  irqchip/gic-v4.1: Plumb skeletal VPE irqchip
  irqchip/gic-v4.1: Add mask/unmask doorbell callbacks
  irqchip/gic-v4.1: Add VPE residency callback
  irqchip/gic-v4.1: Add VPE eviction callback
  irqchip/gic-v4.1: Add VPE INVALL callback
  irqchip/gic-v4.1: Suppress per-VLPI doorbell
  irqchip/gic-v4.1: Allow direct invalidation of VLPIs
  irqchip/gic-v4.1: Advertise support v4.1 to KVM
  irqchip/gic-v4.1: Map the ITS SGIR register page
  irqchip/gic-v4.1: Plumb skeletal VSGI irqchip
  irqchip/gic-v4.1: Add initial SGI configuration
  irqchip/gic-v4.1: Plumb mask/unmask SGI callbacks
  irqchip/gic-v4.1: Plumb get/set_irqchip_state SGI callbacks
  irqchip/gic-v4.1: Plumb set_vcpu_affinity SGI callbacks
  irqchip/gic-v4.1: Move doorbell management to the GICv4 abstraction
layer
  irqchip/gic-v4.1: Add VSGI allocation/teardown
  irqchip/gic-v4.1: Add VSGI property setup
  irqchip/gic-v4.1: Eagerly vmap vPEs
  KVM: arm64: GICv4.1: Let doorbells be auto-enabled
  KVM: arm64: GICv4.1: Add direct injection capability to SGI registers
  KVM: arm64: GICv4.1: Allow SGIs to switch between HW and SW interrupts
  KVM: arm64: GICv4.1: Plumb SGI implementation selection in the
distributor
  KVM: arm64: GICv4.1: Reload VLPI configuration on distributor
enable/disable
  KVM: arm64: GICv4.1: Expose HW-based SGIs in debugfs

 arch/arm/include/asm/arch_gicv3.h  |   2 +
 arch/arm64/include/asm/arch_gicv3.h|   1 +
 arch/arm64/include/asm/kvm_host.h  |   1 +
 drivers/irqchip/irq-gic-v3-its.c   | 996 +++--
 drivers/irqchip/irq-gic-v3.c   |  57 +-
 drivers/irqchip/irq-gic-v4.c   | 134 +++-
 include/kvm/arm_vgic.h |   4 +
 include/linux/irqchip/arm-gic-common.h |   2 +
 include/linux/irqchip/arm-gic-v3.h |  76 +-
 include/linux/irqchip/arm-gic-v4.h |  43 +-
 virt/kvm/arm/arm.c |   8 +
 virt/kvm/arm/vgic/vgic-debug.c |  14 +-
 virt/kvm/arm/vgic/vgic-mmio-v3.c   |  68 +-
 virt/kvm/arm/vgic/vgic-mmio.c  |  88 ++-
 virt/kvm/ar

[PATCH v3 04/32] irqchip/gic-v3: Use SGIs without active state if offered

2019-12-24 Thread Marc Zyngier
If running under control of a hypervisor that implements GICv4.1
SGIs, allow the hypervisor to use them at the expense of loosing
the Active state (which we don't care about for SGIs).

This is trivially done by checking for GICD_TYPER2.nASSGIcap, and
setting GICD_CTLR.nASSGIreq when enabling Group-1 interrupts.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3.c   | 10 --
 include/linux/irqchip/arm-gic-v3.h |  2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 640d4db65b78..624f351c0362 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -724,6 +724,7 @@ static void __init gic_dist_init(void)
unsigned int i;
u64 affinity;
void __iomem *base = gic_data.dist_base;
+   u32 val;
 
/* Disable the distributor */
writel_relaxed(0, base + GICD_CTLR);
@@ -756,9 +757,14 @@ static void __init gic_dist_init(void)
/* Now do the common stuff, and wait for the distributor to drain */
gic_dist_config(base, GIC_LINE_NR, gic_dist_wait_for_rwp);
 
+   val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
+   if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
+   pr_info("Enabling SGIs without active state\n");
+   val |= GICD_CTLR_nASSGIreq;
+   }
+
/* Enable distributor with ARE, Group1 */
-   writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | 
GICD_CTLR_ENABLE_G1,
-  base + GICD_CTLR);
+   writel_relaxed(val, base + GICD_CTLR);
 
/*
 * Set all global interrupts to the boot CPU only. ARE must be
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 9dfe64189d99..72b69f4e6c7b 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -57,6 +57,7 @@
 #define GICD_SPENDSGIR 0x0F20
 
 #define GICD_CTLR_RWP  (1U << 31)
+#define GICD_CTLR_nASSGIreq(1U << 8)
 #define GICD_CTLR_DS   (1U << 6)
 #define GICD_CTLR_ARE_NS   (1U << 4)
 #define GICD_CTLR_ENABLE_G1A   (1U << 1)
@@ -90,6 +91,7 @@
 #define GICD_TYPER_ESPIS(typer)
\
(((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0)
 
+#define GICD_TYPER2_nASSGIcap  (1U << 8)
 #define GICD_TYPER2_VIL(1U << 7)
 #define GICD_TYPER2_VIDGENMASK(4, 0)
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 03/32] irqchip/gic-v3: Workaround Cavium TX1 erratum when reading GICD_TYPER2

2019-12-24 Thread Marc Zyngier
Despite the architecture spec being extremely clear about the fact
that reserved registers in the GIC distributor memory map are RES0
(and thus are not allowed to generate an exception), the Cavium
ThunderX (aka TX1) SoC explodes as such:

[0.00] GICv3: GIC: Using split EOI/Deactivate mode
[0.00] GICv3: 128 SPIs implemented
[0.00] GICv3: 0 Extended SPIs implemented
[0.00] Internal error: synchronous external abort: 96000210 [#1] SMP
[0.00] Modules linked in:
[0.00] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
5.4.0-rc4-00035-g3cf6a3d5725f #7956
[0.00] Hardware name: cavium,thunder-88xx (DT)
[0.00] pstate: 6085 (nZCv daIf -PAN -UAO)
[0.00] pc : __raw_readl+0x0/0x8
[0.00] lr : gic_init_bases+0x110/0x560
[0.00] sp : 800011243d90
[0.00] x29: 800011243d90 x28: 
[0.00] x27: 0018 x26: 0002
[0.00] x25: 8000116f x24: 000fbe6a2c80
[0.00] x23:  x22: 010fdc322b68
[0.00] x21: 800010a7a208 x20: 009b0404
[0.00] x19: 80001124dad0 x18: 0010
[0.00] x17: 4d8d492b x16: f67eb9af
[0.00] x15:  x14: 800011249908
[0.00] x13: 800091243ae7 x12: 800011243af4
[0.00] x11: 80001126e000 x10: 800011243a70
[0.00] x9 : ffd0 x8 : 80001069c828
[0.00] x7 : 0059 x6 : 8000113fb4d1
[0.00] x5 : 0001 x4 : 
[0.00] x3 :  x2 : 
[0.00] x1 :  x0 : 8000116f000c
[0.00] Call trace:
[0.00]  __raw_readl+0x0/0x8
[0.00]  gic_of_init+0x188/0x224
[0.00]  of_irq_init+0x200/0x3cc
[0.00]  irqchip_init+0x1c/0x40
[0.00]  init_IRQ+0x160/0x1d0
[0.00]  start_kernel+0x2ec/0x4b8
[0.00] Code: a8c47bfd d65f03c0 d538d080 d65f03c0 (b940)

when reading the GICv4.1 GICD_TYPER2 register, which is unexpected...

Work around it by adding a new quirk flagging all the A1 revisions
of the distributor, but it remains unknown whether this could affect
other revisions of this SoC (or even other SoCs from the same silicon
vendor).

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 286f98222878..640d4db65b78 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -34,6 +34,7 @@
 #define GICD_INT_NMI_PRI   (GICD_INT_DEF_PRI & ~0x80)
 
 #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996(1ULL << 0)
+#define FLAGS_WORKAROUND_GICD_TYPER2_TX1   (1ULL << 1)
 
 struct redist_region {
void __iomem*redist_base;
@@ -1464,6 +1465,15 @@ static bool gic_enable_quirk_msm8996(void *data)
return true;
 }
 
+static bool gic_enable_quirk_tx1(void *data)
+{
+   struct gic_chip_data *d = data;
+
+   d->flags |= FLAGS_WORKAROUND_GICD_TYPER2_TX1;
+
+   return true;
+}
+
 static bool gic_enable_quirk_hip06_07(void *data)
 {
struct gic_chip_data *d = data;
@@ -1502,6 +1512,12 @@ static const struct gic_quirk gic_quirks[] = {
.mask   = 0x,
.init   = gic_enable_quirk_hip06_07,
},
+   {
+   .desc   = "GICv3: Cavium TX1 GICD_TYPER2 erratum",
+   .iidr   = 0xa100034c,
+   .mask   = 0xfff00fff,
+   .init   = gic_enable_quirk_tx1,
+   },
{
}
 };
@@ -1577,7 +1593,12 @@ static int __init gic_init_bases(void __iomem *dist_base,
pr_info("%d SPIs implemented\n", GIC_LINE_NR - 32);
pr_info("%d Extended SPIs implemented\n", GIC_ESPI_NR);
 
-   gic_data.rdists.gicd_typer2 = readl_relaxed(gic_data.dist_base + 
GICD_TYPER2);
+   /*
+* ThunderX1 explodes on reading GICD_TYPER2, in total violation
+* of the spec (which says that reserved addresses are RES0).
+*/
+   if (!(gic_data.flags & FLAGS_WORKAROUND_GICD_TYPER2_TX1))
+   gic_data.rdists.gicd_typer2 = readl_relaxed(gic_data.dist_base 
+ GICD_TYPER2);
 
gic_data.domain = irq_domain_create_tree(handle, &gic_irq_domain_ops,
 &gic_data);
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 09/32] irqchip/gic-v4.1: Plumb skeletal VPE irqchip

2019-12-24 Thread Marc Zyngier
Just like for GICv4.0, each VPE has its own doorbell interrupt, and
thus an irqchip that manages them. Since the doorbell management is
quite different on GICv4.1, let's introduce an almost empty irqchip
the will get populated over the next new patches.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fd9d3b6bb465..157f51398850 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3554,6 +3554,32 @@ static struct irq_chip its_vpe_irq_chip = {
.irq_set_vcpu_affinity  = its_vpe_set_vcpu_affinity,
 };
 
+static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
+{
+   struct its_cmd_info *info = vcpu_info;
+
+   switch (info->cmd_type) {
+   case SCHEDULE_VPE:
+   return 0;
+
+   case DESCHEDULE_VPE:
+   return 0;
+
+   case INVALL_VPE:
+   return 0;
+
+   default:
+   return -EINVAL;
+   }
+}
+
+static struct irq_chip its_vpe_4_1_irq_chip = {
+   .name   = "GICv4.1-vpe",
+   .irq_eoi= irq_chip_eoi_parent,
+   .irq_set_affinity   = its_vpe_set_affinity,
+   .irq_set_vcpu_affinity  = its_vpe_4_1_set_vcpu_affinity,
+};
+
 static int its_vpe_id_alloc(void)
 {
return ida_simple_get(&its_vpeid_ida, 0, ITS_MAX_VPEID, GFP_KERNEL);
@@ -3634,6 +3660,7 @@ static void its_vpe_irq_domain_free(struct irq_domain 
*domain,
 static int its_vpe_irq_domain_alloc(struct irq_domain *domain, unsigned int 
virq,
unsigned int nr_irqs, void *args)
 {
+   struct irq_chip *irqchip = &its_vpe_irq_chip;
struct its_vm *vm = args;
unsigned long *bitmap;
struct page *vprop_page;
@@ -3661,6 +3688,9 @@ static int its_vpe_irq_domain_alloc(struct irq_domain 
*domain, unsigned int virq
vm->nr_db_lpis = nr_ids;
vm->vprop_page = vprop_page;
 
+   if (gic_rdists->has_rvpeid)
+   irqchip = &its_vpe_4_1_irq_chip;
+
for (i = 0; i < nr_irqs; i++) {
vm->vpes[i]->vpe_db_lpi = base + i;
err = its_vpe_init(vm->vpes[i]);
@@ -3671,7 +3701,7 @@ static int its_vpe_irq_domain_alloc(struct irq_domain 
*domain, unsigned int virq
if (err)
break;
irq_domain_set_hwirq_and_chip(domain, virq + i, i,
- &its_vpe_irq_chip, vm->vpes[i]);
+ irqchip, vm->vpes[i]);
set_bit(i, bitmap);
}
 
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v3 02/32] irqchip/gic-v3: Add GICv4.1 VPEID size discovery

2019-12-24 Thread Marc Zyngier
While GICv4.0 mandates 16 bit worth of VPEIDs, GICv4.1 allows smaller
implementations to be built. Add the required glue to dynamically
compute the limit.

Reviewed-by: Zenghui Yu 
Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3-its.c   | 11 ++-
 drivers/irqchip/irq-gic-v3.c   |  3 +++
 include/linux/irqchip/arm-gic-v3.h |  5 +
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e05673bcd52b..d9d256471dc0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -121,7 +121,16 @@ struct its_node {
 #define ITS_ITT_ALIGN  SZ_256
 
 /* The maximum number of VPEID bits supported by VLPI commands */
-#define ITS_MAX_VPEID_BITS (16)
+#define ITS_MAX_VPEID_BITS \
+   ({  \
+   int nvpeid = 16;\
+   if (gic_rdists->has_rvpeid &&   \
+   gic_rdists->gicd_typer2 & GICD_TYPER2_VIL)  \
+   nvpeid = 1 + (gic_rdists->gicd_typer2 & \
+ GICD_TYPER2_VID); \
+   \
+   nvpeid; \
+   })
 #define ITS_MAX_VPEID  (1 << (ITS_MAX_VPEID_BITS))
 
 /* Convert page order to size in bytes */
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index ffcb018395ed..286f98222878 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1576,6 +1576,9 @@ static int __init gic_init_bases(void __iomem *dist_base,
 
pr_info("%d SPIs implemented\n", GIC_LINE_NR - 32);
pr_info("%d Extended SPIs implemented\n", GIC_ESPI_NR);
+
+   gic_data.rdists.gicd_typer2 = readl_relaxed(gic_data.dist_base + 
GICD_TYPER2);
+
gic_data.domain = irq_domain_create_tree(handle, &gic_irq_domain_ops,
 &gic_data);
irq_domain_update_bus_token(gic_data.domain, DOMAIN_BUS_WIRED);
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 9a5f85d30701..9dfe64189d99 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -13,6 +13,7 @@
 #define GICD_CTLR  0x
 #define GICD_TYPER 0x0004
 #define GICD_IIDR  0x0008
+#define GICD_TYPER20x000C
 #define GICD_STATUSR   0x0010
 #define GICD_SETSPI_NSR0x0040
 #define GICD_CLRSPI_NSR0x0048
@@ -89,6 +90,9 @@
 #define GICD_TYPER_ESPIS(typer)
\
(((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0)
 
+#define GICD_TYPER2_VIL(1U << 7)
+#define GICD_TYPER2_VIDGENMASK(4, 0)
+
 #define GICD_IROUTER_SPI_MODE_ONE  (0U << 31)
 #define GICD_IROUTER_SPI_MODE_ANY  (1U << 31)
 
@@ -615,6 +619,7 @@ struct rdists {
void*prop_table_va;
u64 flags;
u32 gicd_typer;
+   u32 gicd_typer2;
boolhas_vlpis;
boolhas_rvpeid;
boolhas_direct_lpi;
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 10/18] arm64: KVM/debug: use EL1&0 stage 1 translation regime

2019-12-24 Thread Andrew Murray
On Sun, Dec 22, 2019 at 10:34:55AM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:17 +,
> Andrew Murray  wrote:
> > 
> > From: Sudeep Holla 
> > 
> > Now that we have all the save/restore mechanism in place, lets enable
> > the translation regime used by buffer from EL2 stage 1 to EL1 stage 1
> > on VHE systems.
> > 
> > Signed-off-by: Sudeep Holla 
> > [ Reword commit, don't trap to EL2 ]
> 
> Not trapping to EL2 for the case where we don't allow SPE in the
> guest is not acceptable.

Yes understood (because of this I had meant to send the series as RFC btw).


> 
> > Signed-off-by: Andrew Murray 
> > ---
> >  arch/arm64/kvm/hyp/switch.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 67b7c160f65b..6c153b79829b 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -100,6 +100,7 @@ static void activate_traps_vhe(struct kvm_vcpu *vcpu)
> >  
> > write_sysreg(val, cpacr_el1);
> >  
> > +   write_sysreg(vcpu->arch.mdcr_el2 | 3 << MDCR_EL2_E2PB_SHIFT, mdcr_el2);
> > write_sysreg(kvm_get_hyp_vector(), vbar_el1);
> >  }
> >  NOKPROBE_SYMBOL(activate_traps_vhe);
> > @@ -117,6 +118,7 @@ static void __hyp_text __activate_traps_nvhe(struct 
> > kvm_vcpu *vcpu)
> > __activate_traps_fpsimd32(vcpu);
> > }
> >  
> > +   write_sysreg(vcpu->arch.mdcr_el2 | 3 << MDCR_EL2_E2PB_SHIFT, mdcr_el2);
> 
> There is a _MASK macro that can replace this '3', and is in keeping
> with the rest of the code.

OK.


> 
> It still remains that it looks like the wrong place to do this, and
> vcpu_load seems much better. Why should you write to mdcr_el2 on each
> entry to the guest, since you know whether it has SPE enabled at the
> point where it gets scheduled?

Yes OK, I'll move what I can to vcpu_load.

Thanks,

Andrew Murray


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


Re: [PATCH v2 08/18] arm64: KVM: add support to save/restore SPE profiling buffer controls

2019-12-24 Thread Andrew Murray
On Sat, Dec 21, 2019 at 01:57:55PM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:15 +
> Andrew Murray  wrote:
> 
> > From: Sudeep Holla 
> > 
> > Currently since we don't support profiling using SPE in the guests,
> > we just save the PMSCR_EL1, flush the profiling buffers and disable
> > sampling. However in order to support simultaneous sampling both in
> 
> Is the sampling actually simultaneous? I don't believe so (the whole
> series would be much simpler if it was).

No the SPE is used by either the guest or host at any one time. I guess
the term simultaneous was used to refer to illusion given to both guest
and host that they are able to use it whenever they like. I'll update
the commit message to drop the magic.
 

> 
> > the host and guests, we need to save and reatore the complete SPE
> 
> s/reatore/restore/

Noted.


> 
> > profiling buffer controls' context.
> > 
> > Let's add the support for the same and keep it disabled for now.
> > We can enable it conditionally only if guests are allowed to use
> > SPE.
> > 
> > Signed-off-by: Sudeep Holla 
> > [ Clear PMBSR bit when saving state to prevent spurious interrupts ]
> > Signed-off-by: Andrew Murray 
> > ---
> >  arch/arm64/kvm/hyp/debug-sr.c | 51 +--
> >  1 file changed, 43 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
> > index 8a70a493345e..12429b212a3a 100644
> > --- a/arch/arm64/kvm/hyp/debug-sr.c
> > +++ b/arch/arm64/kvm/hyp/debug-sr.c
> > @@ -85,7 +85,8 @@
> > default:write_debug(ptr[0], reg, 0);\
> > }
> >  
> > -static void __hyp_text __debug_save_spe_nvhe(struct kvm_cpu_context *ctxt)
> > +static void __hyp_text
> > +__debug_save_spe_nvhe(struct kvm_cpu_context *ctxt, bool full_ctxt)
> 
> nit: don't split lines like this if you can avoid it. You can put the
> full_ctxt parameter on a separate line instead.

Yes understood.


> 
> >  {
> > u64 reg;
> >  
> > @@ -102,22 +103,46 @@ static void __hyp_text __debug_save_spe_nvhe(struct 
> > kvm_cpu_context *ctxt)
> > if (reg & BIT(SYS_PMBIDR_EL1_P_SHIFT))
> > return;
> >  
> > -   /* No; is the host actually using the thing? */
> > -   reg = read_sysreg_s(SYS_PMBLIMITR_EL1);
> > -   if (!(reg & BIT(SYS_PMBLIMITR_EL1_E_SHIFT)))
> > +   /* Save the control register and disable data generation */
> > +   ctxt->sys_regs[PMSCR_EL1] = read_sysreg_el1(SYS_PMSCR);
> > +
> > +   if (!ctxt->sys_regs[PMSCR_EL1])
> 
> Shouldn't you check the enable bits instead of relying on the whole
> thing being zero?

Yes that would make more sense (E1SPE and E0SPE).

I feel that this check makes an assumption about the guest/host SPE
driver... What happens if the SPE driver writes to some SPE registers
but doesn't enable PMSCR? If the guest is also using SPE then those
writes will be lost, when the host returns and the SPE driver enables
SPE it won't work.

With a quick look at the SPE driver I'm not sure this will happen, but
even so it makes me nervous relying on these assumptions. I wonder if
this risk is present in other devices?


> 
> > return;
> >  
> > /* Yes; save the control register and disable data generation */
> > -   ctxt->sys_regs[PMSCR_EL1] = read_sysreg_el1(SYS_PMSCR);
> 
> You've already saved the control register...

I'll remove that.


> 
> > write_sysreg_el1(0, SYS_PMSCR);
> > isb();
> >  
> > /* Now drain all buffered data to memory */
> > psb_csync();
> > dsb(nsh);
> > +
> > +   if (!full_ctxt)
> > +   return;
> > +
> > +   ctxt->sys_regs[PMBLIMITR_EL1] = read_sysreg_s(SYS_PMBLIMITR_EL1);
> > +   write_sysreg_s(0, SYS_PMBLIMITR_EL1);
> > +
> > +   /*
> > +* As PMBSR is conditionally restored when returning to the host we
> > +* must ensure the service bit is unset here to prevent a spurious
> > +* host SPE interrupt from being raised.
> > +*/
> > +   ctxt->sys_regs[PMBSR_EL1] = read_sysreg_s(SYS_PMBSR_EL1);
> > +   write_sysreg_s(0, SYS_PMBSR_EL1);
> > +
> > +   isb();
> > +
> > +   ctxt->sys_regs[PMSICR_EL1] = read_sysreg_s(SYS_PMSICR_EL1);
> > +   ctxt->sys_regs[PMSIRR_EL1] = read_sysreg_s(SYS_PMSIRR_EL1);
> > +   ctxt->sys_regs[PMSFCR_EL1] = read_sysreg_s(SYS_PMSFCR_EL1);
> > +   ctxt->sys_regs[PMSEVFR_EL1] = read_sysreg_s(SYS_PMSEVFR_EL1);
> > +   ctxt->sys_regs[PMSLATFR_EL1] = read_sysreg_s(SYS_PMSLATFR_EL1);
> > +   ctxt->sys_regs[PMBPTR_EL1] = read_sysreg_s(SYS_PMBPTR_EL1);
> >  }
> >  
> > -static void __hyp_text __debug_restore_spe_nvhe(struct kvm_cpu_context 
> > *ctxt)
> > +static void __hyp_text
> > +__debug_restore_spe_nvhe(struct kvm_cpu_context *ctxt, bool full_ctxt)
> >  {
> > if (!ctxt->sys_regs[PMSCR_EL1])
> > return;
> > @@ -126,6 +151,16 @@ static void __hyp_text __debug_restore_spe_nvhe(struct 
> > kvm_cpu_context *ctxt)
> > isb();
> >  
> > /* Re-enable data generation */
> > +   if (full_ctxt) {
> > +   write_sysre

Re: [PATCH v2 02/18] arm64: KVM: reset E2PB correctly in MDCR_EL2 when exiting the guest(VHE)

2019-12-24 Thread Andrew Murray
On Sat, Dec 21, 2019 at 01:12:14PM +, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:09 +
> Andrew Murray  wrote:
> 
> > From: Sudeep Holla 
> > 
> > On VHE systems, the reset value for MDCR_EL2.E2PB=b00 which defaults
> > to profiling buffer using the EL2 stage 1 translations. 
> 
> Does the reset value actually matter here? I don't see it being
> specific to VHE systems, and all we're trying to achieve is to restore
> the SPE configuration to a state where it can be used by the host.
> 
> > However if the
> > guest are allowed to use profiling buffers changing E2PB settings, we
> 
> How can the guest be allowed to change E2PB settings? Or do you mean
> here that allowing the guest to use SPE will mandate changes of the
> E2PB settings, and that we'd better restore the hypervisor state once
> we exit?
> 
> > need to ensure we resume back MDCR_EL2.E2PB=b00. Currently we just
> > do bitwise '&' with MDCR_EL2_E2PB_MASK which will retain the value.
> > 
> > So fix it by clearing all the bits in E2PB.
> > 
> > Signed-off-by: Sudeep Holla 
> > Signed-off-by: Andrew Murray 
> > ---
> >  arch/arm64/kvm/hyp/switch.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 72fbbd86eb5e..250f13910882 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -228,9 +228,7 @@ void deactivate_traps_vhe_put(void)
> >  {
> > u64 mdcr_el2 = read_sysreg(mdcr_el2);
> >  
> > -   mdcr_el2 &= MDCR_EL2_HPMN_MASK |
> > -   MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT |
> > -   MDCR_EL2_TPMS;
> > +   mdcr_el2 &= MDCR_EL2_HPMN_MASK | MDCR_EL2_TPMS;
> >  
> > write_sysreg(mdcr_el2, mdcr_el2);
> >  
> 
> I'm OK with this change, but I believe the commit message could use
> some tidying up.

No problem, I'll update the commit message.

Thanks,

Andrew Murray

> 
> Thanks,
> 
>   M.
> -- 
> Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 11/36] irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation

2019-12-24 Thread Marc Zyngier

Hi Zenghui,

On 2019-12-24 07:10, Zenghui Yu wrote:

Hi Marc,

[ +Wudi and Nianyao. As they spotted the following issue but
 I forgot to send it out. ]

On 2019/10/27 22:42, Marc Zyngier wrote:

GICv4.1 defines a new VPE table that is potentially shared between
both the ITSs and the redistributors, following complicated affinity
rules.
To make things more confusing, the programming of this table at
the redistributor level is reusing the GICv4.0 GICR_VPROPBASER 
register

for something completely different.
The code flow is somewhat complexified by the need to respect the
affinities required by the HW, meaning that tables can either be
inherited from a previously discovered ITS or redistributor.
Signed-off-by: Marc Zyngier 
---[...]
diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index 40912b3fb0e1..478d3678850c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c

[...]
@@ -2025,6 +2098,214 @@ static int its_alloc_tables(struct its_node 
*its)

return 0;
  }
  +static u64 inherit_vpe_l1_table_from_its(void)
+{
+   struct its_node *its;
+   u64 val;
+   u32 aff;
+
+   val = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
+   aff = compute_common_aff(val);
+
+   list_for_each_entry(its, &its_nodes, entry) {
+   u64 baser;
+
+   if (!is_v4_1(its))
+   continue;
+
+   if (!FIELD_GET(GITS_TYPER_SVPET, its->typer))
+   continue;
+
+   if (aff != compute_its_aff(its))
+   continue;
+
+   /* GICv4.1 guarantees that the vPE table is GITS_BASER2 */
+   baser = its->tables[2].val;
+   if (!(baser & GITS_BASER_VALID))
+   continue;
+
+   /* We have a winner! */
+   val  = GICR_VPROPBASER_4_1_VALID;
+   if (baser & GITS_BASER_INDIRECT)
+   val |= GICR_VPROPBASER_4_1_INDIRECT;
+   val |= FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE,
+ FIELD_GET(GITS_BASER_PAGE_SIZE_MASK, baser));
+   val |= FIELD_PREP(GICR_VPROPBASER_4_1_ADDR,
+ GITS_BASER_ADDR_48_to_52(baser) >> 12);


We've used GITS_BASER_ADDR_48_to_52() only in the KVM code where the
pagesize of ITS table is fixed to 64K.
It may not work when the pagesize is 4K or 16K?


You're absolutely right, we shouldn't mess with the 52bit macros when
PZ isn't set to 64k. I'm adding the following fix to this patch:

diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index e1f8d5f9a0e3..3234bb9fbdbe 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2433,7 +2433,7 @@ static u64 inherit_vpe_l1_table_from_its(void)
aff = compute_common_aff(val);

list_for_each_entry(its, &its_nodes, entry) {
-   u64 baser;
+   u64 baser, addr;

if (!is_v4_1(its))
continue;
@@ -2455,8 +2455,15 @@ static u64 inherit_vpe_l1_table_from_its(void)
val |= GICR_VPROPBASER_4_1_INDIRECT;
val |= FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE,
  FIELD_GET(GITS_BASER_PAGE_SIZE_MASK, baser));
-   val |= FIELD_PREP(GICR_VPROPBASER_4_1_ADDR,
- GITS_BASER_ADDR_48_to_52(baser) >> 12);
+   switch (FIELD_GET(GITS_BASER_PAGE_SIZE_MASK, baser)) {
+   case GIC_PAGE_SIZE_64K:
+   addr = GITS_BASER_ADDR_48_to_52(baser);
+   break;
+   default:
+   addr = baser & GENMASK_ULL(47, 12);
+   break;
+   }
+   val |= FIELD_PREP(GICR_VPROPBASER_4_1_ADDR, addr >> 12);
val |= FIELD_PREP(GICR_VPROPBASER_SHAREABILITY_MASK,
  FIELD_GET(GITS_BASER_SHAREABILITY_MASK, 
baser));
val |= FIELD_PREP(GICR_VPROPBASER_INNER_CACHEABILITY_MASK,


Thanks again,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm