Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 3/9] KVM: arm/arm64: Don't cache the timer IRQ level
From: Christoffer Dall @ 2017-12-13 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

The timer was modeled after a strict idea of modelling an interrupt line
level in software, meaning that only transitions in the level needed to
be reported to the VGIC.  This works well for the timer, because the
arch timer code is in complete control of the device and can track the
transitions of the line.

However, as we are about to support using the HW bit in the VGIC not
just for the timer, but also for VFIO which cannot track transitions of
the interrupt line, we have to decide on an interface for level
triggered mapped interrupts to the GIC, which both the timer and VFIO
can use.

VFIO only sees an asserting transition of the physical interrupt line,
and tells the VGIC when that happens.  That means that part of the
interrupt flow is offloaded to the hardware.

To use the same interface for VFIO devices and the timer, we therefore
have to change the timer (we cannot change VFIO because it doesn't know
the details of the device it is assigning to a VM).

Luckily, changing the timer is simple, we just need to stop 'caching'
the line level, but instead let the VGIC know the state of the timer
every time there is a potential change in the line level, and when the
line level should be asserted from the timer ISR.  The VGIC can ignore
extra notifications using its validate mechanism.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/arch_timer.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 4151250ce8da..dd5aca05c500 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -99,11 +99,9 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
 	}
 	vtimer = vcpu_vtimer(vcpu);
 
-	if (!vtimer->irq.level) {
-		vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
-		if (kvm_timer_irq_can_fire(vtimer))
-			kvm_timer_update_irq(vcpu, true, vtimer);
-	}
+	vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
+	if (kvm_timer_irq_can_fire(vtimer))
+		kvm_timer_update_irq(vcpu, true, vtimer);
 
 	if (unlikely(!irqchip_in_kernel(vcpu->kvm)))
 		kvm_vtimer_update_mask_user(vcpu);
@@ -324,12 +322,20 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
 	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
 	struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
+	bool level;
 
 	if (unlikely(!timer->enabled))
 		return;
 
-	if (kvm_timer_should_fire(vtimer) != vtimer->irq.level)
-		kvm_timer_update_irq(vcpu, !vtimer->irq.level, vtimer);
+	/*
+	 * The vtimer virtual interrupt is a 'mapped' interrupt, meaning part
+	 * of its lifecycle is offloaded to the hardware, and we therefore may
+	 * not have lowered the irq.level value before having to signal a new
+	 * interrupt, but have to signal an interrupt every time the level is
+	 * asserted.
+	 */
+	level = kvm_timer_should_fire(vtimer);
+	kvm_timer_update_irq(vcpu, level, vtimer);
 
 	if (kvm_timer_should_fire(ptimer) != ptimer->irq.level)
 		kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
-- 
2.14.2

^ permalink raw reply related

* [PATCH v8 4/9] KVM: arm/arm64: vgic: Support level-triggered mapped interrupts
From: Christoffer Dall @ 2017-12-13 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

Level-triggered mapped IRQs are special because we only observe rising
edges as input to the VGIC, and we don't set the EOI flag and therefore
are not told when the level goes down, so that we can re-queue a new
interrupt when the level goes up.

One way to solve this problem is to side-step the logic of the VGIC and
special case the validation in the injection path, but it has the
unfortunate drawback of having to peak into the physical GIC state
whenever we want to know if the interrupt is pending on the virtual
distributor.

Instead, we can maintain the current semantics of a level triggered
interrupt by sort of treating it as an edge-triggered interrupt,
following from the fact that we only observe an asserting edge.  This
requires us to be a bit careful when populating the LRs and when folding
the state back in though:

 * We lower the line level when populating the LR, so that when
   subsequently observing an asserting edge, the VGIC will do the right
   thing.

 * If the guest never acked the interrupt while running (for example if
   it had masked interrupts at the CPU level while running), we have
   to preserve the pending state of the LR and move it back to the
   line_level field of the struct irq when folding LR state.

   If the guest never acked the interrupt while running, but changed the
   device state and lowered the line (again with interrupts masked) then
   we need to observe this change in the line_level.

   Both of the above situations are solved by sampling the physical line
   and set the line level when folding the LR back.

 * Finally, if the guest never acked the interrupt while running and
   sampling the line reveals that the device state has changed and the
   line has been lowered, we must clear the physical active state, since
   we will otherwise never be told when the interrupt becomes asserted
   again.

This has the added benefit of making the timer optimization patches
(https://lists.cs.columbia.edu/pipermail/kvmarm/2017-July/026343.html) a
bit simpler, because the timer code doesn't have to clear the active
state on the sync anymore.  It also potentially improves the performance
of the timer implementation because the GIC knows the state or the LR
and only needs to clear the
active state when the pending bit in the LR is still set, where the
timer has to always clear it when returning from running the guest with
an injected timer interrupt.

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/vgic/vgic-v2.c | 29 +++++++++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic-v3.c | 29 +++++++++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic.c    | 23 +++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic.h    |  7 +++++++
 4 files changed, 88 insertions(+)

diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 80897102da26..c32d7b93ffd1 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -105,6 +105,26 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 				irq->pending_latch = false;
 		}
 
+		/*
+		 * Level-triggered mapped IRQs are special because we only
+		 * observe rising edges as input to the VGIC.
+		 *
+		 * If the guest never acked the interrupt we have to sample
+		 * the physical line and set the line level, because the
+		 * device state could have changed or we simply need to
+		 * process the still pending interrupt later.
+		 *
+		 * If this causes us to lower the level, we have to also clear
+		 * the physical active state, since we will otherwise never be
+		 * told when the interrupt becomes asserted again.
+		 */
+		if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT)) {
+			irq->line_level = vgic_get_phys_line_level(irq);
+
+			if (!irq->line_level)
+				vgic_irq_set_phys_active(irq, false);
+		}
+
 		spin_unlock_irqrestore(&irq->irq_lock, flags);
 		vgic_put_irq(vcpu->kvm, irq);
 	}
@@ -162,6 +182,15 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 			val |= GICH_LR_EOI;
 	}
 
+	/*
+	 * Level-triggered mapped IRQs are special because we only observe
+	 * rising edges as input to the VGIC.  We therefore lower the line
+	 * level here, so that we can take new virtual IRQs.  See
+	 * vgic_v2_fold_lr_state for more info.
+	 */
+	if (vgic_irq_is_mapped_level(irq) && (val & GICH_LR_PENDING_BIT))
+		irq->line_level = false;
+
 	/* The GICv2 LR only holds five bits of priority. */
 	val |= (irq->priority >> 3) << GICH_LR_PRIORITY_SHIFT;
 
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 2f05f732d3fd..a14423a0d383 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -96,6 +96,26 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 				irq->pending_latch = false;
 		}
 
+		/*
+		 * Level-triggered mapped IRQs are special because we only
+		 * observe rising edges as input to the VGIC.
+		 *
+		 * If the guest never acked the interrupt we have to sample
+		 * the physical line and set the line level, because the
+		 * device state could have changed or we simply need to
+		 * process the still pending interrupt later.
+		 *
+		 * If this causes us to lower the level, we have to also clear
+		 * the physical active state, since we will otherwise never be
+		 * told when the interrupt becomes asserted again.
+		 */
+		if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT)) {
+			irq->line_level = vgic_get_phys_line_level(irq);
+
+			if (!irq->line_level)
+				vgic_irq_set_phys_active(irq, false);
+		}
+
 		spin_unlock_irqrestore(&irq->irq_lock, flags);
 		vgic_put_irq(vcpu->kvm, irq);
 	}
@@ -145,6 +165,15 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr)
 			val |= ICH_LR_EOI;
 	}
 
+	/*
+	 * Level-triggered mapped IRQs are special because we only observe
+	 * rising edges as input to the VGIC.  We therefore lower the line
+	 * level here, so that we can take new virtual IRQs.  See
+	 * vgic_v3_fold_lr_state for more info.
+	 */
+	if (vgic_irq_is_mapped_level(irq) && (val & ICH_LR_PENDING_BIT))
+		irq->line_level = false;
+
 	/*
 	 * We currently only support Group1 interrupts, which is a
 	 * known defect. This needs to be addressed at some point.
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index b168a328a9e0..607cbbc27a1c 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,6 +144,29 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
 	kfree(irq);
 }
 
+/* Get the input level of a mapped IRQ directly from the physical GIC */
+bool vgic_get_phys_line_level(struct vgic_irq *irq)
+{
+	bool line_level;
+
+	BUG_ON(!irq->hw);
+
+	WARN_ON(irq_get_irqchip_state(irq->host_irq,
+				      IRQCHIP_STATE_PENDING,
+				      &line_level));
+	return line_level;
+}
+
+/* Set/Clear the physical active state */
+void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)
+{
+
+	BUG_ON(!irq->hw);
+	WARN_ON(irq_set_irqchip_state(irq->host_irq,
+				      IRQCHIP_STATE_ACTIVE,
+				      active));
+}
+
 /**
  * kvm_vgic_target_oracle - compute the target vcpu for an irq
  *
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index efbcf8f96f9c..d0787983a357 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -104,6 +104,11 @@ static inline bool irq_is_pending(struct vgic_irq *irq)
 		return irq->pending_latch || irq->line_level;
 }
 
+static inline bool vgic_irq_is_mapped_level(struct vgic_irq *irq)
+{
+	return irq->config == VGIC_CONFIG_LEVEL && irq->hw;
+}
+
 /*
  * This struct provides an intermediate representation of the fields contained
  * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
@@ -140,6 +145,8 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev,
 struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 			      u32 intid);
 void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq);
+bool vgic_get_phys_line_level(struct vgic_irq *irq);
+void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);
 bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
 			   unsigned long flags);
 void vgic_kick_vcpus(struct kvm *kvm);
-- 
2.14.2

^ permalink raw reply related

* [PATCH v8 5/9] KVM: arm/arm64: Support a vgic interrupt line level sample function
From: Christoffer Dall @ 2017-12-13 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

The GIC sometimes need to sample the physical line of a mapped
interrupt.  As we know this to be notoriously slow, provide a callback
function for devices (such as the timer) which can do this much faster
than talking to the distributor, for example by comparing a few
in-memory values.  Fall back to the good old method of poking the
physical GIC if no callback is provided.

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 include/kvm/arm_vgic.h    | 13 ++++++++++++-
 virt/kvm/arm/arch_timer.c |  3 ++-
 virt/kvm/arm/vgic/vgic.c  | 13 +++++++++----
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 8c896540a72c..cdbd142ca7f2 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -130,6 +130,17 @@ struct vgic_irq {
 	u8 priority;
 	enum vgic_irq_config config;	/* Level or edge */
 
+	/*
+	 * Callback function pointer to in-kernel devices that can tell us the
+	 * state of the input level of mapped level-triggered IRQ faster than
+	 * peaking into the physical GIC.
+	 *
+	 * Always called in non-preemptible section and the functions can use
+	 * kvm_arm_get_running_vcpu() to get the vcpu pointer for private
+	 * IRQs.
+	 */
+	bool (*get_input_level)(int vintid);
+
 	void *owner;			/* Opaque pointer to reserve an interrupt
 					   for in-kernel devices. */
 };
@@ -331,7 +342,7 @@ void kvm_vgic_init_cpu_hardware(void);
 int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
 			bool level, void *owner);
 int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
-			  u32 vintid);
+			  u32 vintid, bool (*get_input_level)(int vindid));
 int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid);
 bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int vintid);
 
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index dd5aca05c500..e78ba5e20f74 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -840,7 +840,8 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
 		return -EINVAL;
 	}
 
-	ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq);
+	ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,
+				    NULL);
 	if (ret)
 		return ret;
 
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 607cbbc27a1c..80c5c609385a 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,13 +144,15 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
 	kfree(irq);
 }
 
-/* Get the input level of a mapped IRQ directly from the physical GIC */
 bool vgic_get_phys_line_level(struct vgic_irq *irq)
 {
 	bool line_level;
 
 	BUG_ON(!irq->hw);
 
+	if (irq->get_input_level)
+		return irq->get_input_level(irq->intid);
+
 	WARN_ON(irq_get_irqchip_state(irq->host_irq,
 				      IRQCHIP_STATE_PENDING,
 				      &line_level));
@@ -436,7 +438,8 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,
 
 /* @irq->irq_lock must be held */
 static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
-			    unsigned int host_irq)
+			    unsigned int host_irq,
+			    bool (*get_input_level)(int vindid))
 {
 	struct irq_desc *desc;
 	struct irq_data *data;
@@ -456,6 +459,7 @@ static int kvm_vgic_map_irq(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
 	irq->hw = true;
 	irq->host_irq = host_irq;
 	irq->hwintid = data->hwirq;
+	irq->get_input_level = get_input_level;
 	return 0;
 }
 
@@ -464,10 +468,11 @@ static inline void kvm_vgic_unmap_irq(struct vgic_irq *irq)
 {
 	irq->hw = false;
 	irq->hwintid = 0;
+	irq->get_input_level = NULL;
 }
 
 int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
-			  u32 vintid)
+			  u32 vintid, bool (*get_input_level)(int vindid))
 {
 	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, vintid);
 	unsigned long flags;
@@ -476,7 +481,7 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, unsigned int host_irq,
 	BUG_ON(!irq);
 
 	spin_lock_irqsave(&irq->irq_lock, flags);
-	ret = kvm_vgic_map_irq(vcpu, irq, host_irq);
+	ret = kvm_vgic_map_irq(vcpu, irq, host_irq, get_input_level);
 	spin_unlock_irqrestore(&irq->irq_lock, flags);
 	vgic_put_irq(vcpu->kvm, irq);
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH v8 6/9] KVM: arm/arm64: Support VGIC dist pend/active changes for mapped IRQs
From: Christoffer Dall @ 2017-12-13 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

For mapped IRQs (with the HW bit set in the LR) we have to follow some
rules of the architecture.  One of these rules is that VM must not be
allowed to deactivate a virtual interrupt with the HW bit set unless the
physical interrupt is also active.

This works fine when injecting mapped interrupts, because we leave it up
to the injector to either set EOImode==1 or manually set the active
state of the physical interrupt.

However, the guest can set virtual interrupt to be pending or active by
writing to the virtual distributor, which could lead to deactivating a
virtual interrupt with the HW bit set without the physical interrupt
being active.

We could set the physical interrupt to active whenever we are about to
enter the VM with a HW interrupt either pending or active, but that
would be really slow, especially on GICv2.  So we take the long way
around and do the hard work when needed, which is expected to be
extremely rare.

When the VM sets the pending state for a HW interrupt on the virtual
distributor we set the active state on the physical distributor, because
the virtual interrupt can become active and then the guest can
deactivate it.

When the VM clears the pending state we also clear it on the physical
side, because the injector might otherwise raise the interrupt.  We also
clear the physical active state when the virtual interrupt is not
active, since otherwise a SPEND/CPEND sequence from the guest would
prevent signaling of future interrupts.

Changing the state of mapped interrupts from userspace is not supported,
and it's expected that userspace unmaps devices from VFIO before
attempting to set the interrupt state, because the interrupt state is
driven by hardware.

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/vgic/vgic-mmio.c | 71 +++++++++++++++++++++++++++++++++++++++----
 virt/kvm/arm/vgic/vgic.c      |  7 +++++
 virt/kvm/arm/vgic/vgic.h      |  1 +
 3 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index fdad95f62fa3..83d82bd7dc4e 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -16,6 +16,7 @@
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
 #include <kvm/iodev.h>
+#include <kvm/arm_arch_timer.h>
 #include <kvm/arm_vgic.h>
 
 #include "vgic.h"
@@ -143,10 +144,22 @@ static struct kvm_vcpu *vgic_get_mmio_requester_vcpu(void)
 	return vcpu;
 }
 
+/* Must be called with irq->irq_lock held */
+static void vgic_hw_irq_spending(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
+				 bool is_uaccess)
+{
+	if (is_uaccess)
+		return;
+
+	irq->pending_latch = true;
+	vgic_irq_set_phys_active(irq, true);
+}
+
 void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
 			      gpa_t addr, unsigned int len,
 			      unsigned long val)
 {
+	bool is_uaccess = !vgic_get_mmio_requester_vcpu();
 	u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
 	int i;
 	unsigned long flags;
@@ -155,17 +168,45 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		spin_lock_irqsave(&irq->irq_lock, flags);
-		irq->pending_latch = true;
-
+		if (irq->hw)
+			vgic_hw_irq_spending(vcpu, irq, is_uaccess);
+		else
+			irq->pending_latch = true;
 		vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
 		vgic_put_irq(vcpu->kvm, irq);
 	}
 }
 
+/* Must be called with irq->irq_lock held */
+static void vgic_hw_irq_cpending(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
+				 bool is_uaccess)
+{
+	if (is_uaccess)
+		return;
+
+	irq->pending_latch = false;
+
+	/*
+	 * We don't want the guest to effectively mask the physical
+	 * interrupt by doing a write to SPENDR followed by a write to
+	 * CPENDR for HW interrupts, so we clear the active state on
+	 * the physical side if the virtual interrupt is not active.
+	 * This may lead to taking an additional interrupt on the
+	 * host, but that should not be a problem as the worst that
+	 * can happen is an additional vgic injection.  We also clear
+	 * the pending state to maintain proper semantics for edge HW
+	 * interrupts.
+	 */
+	vgic_irq_set_phys_pending(irq, false);
+	if (!irq->active)
+		vgic_irq_set_phys_active(irq, false);
+}
+
 void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
 			      gpa_t addr, unsigned int len,
 			      unsigned long val)
 {
+	bool is_uaccess = !vgic_get_mmio_requester_vcpu();
 	u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
 	int i;
 	unsigned long flags;
@@ -175,7 +216,10 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
 
 		spin_lock_irqsave(&irq->irq_lock, flags);
 
-		irq->pending_latch = false;
+		if (irq->hw)
+			vgic_hw_irq_cpending(vcpu, irq, is_uaccess);
+		else
+			irq->pending_latch = false;
 
 		spin_unlock_irqrestore(&irq->irq_lock, flags);
 		vgic_put_irq(vcpu->kvm, irq);
@@ -202,8 +246,19 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,
 	return value;
 }
 
+/* Must be called with irq->irq_lock held */
+static void vgic_hw_irq_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
+				      bool active, bool is_uaccess)
+{
+	if (is_uaccess)
+		return;
+
+	irq->active = active;
+	vgic_irq_set_phys_active(irq, active);
+}
+
 static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
-				    bool new_active_state)
+				    bool active)
 {
 	unsigned long flags;
 	struct kvm_vcpu *requester_vcpu = vgic_get_mmio_requester_vcpu();
@@ -231,8 +286,12 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
 	       irq->vcpu->cpu != -1) /* VCPU thread is running */
 		cond_resched_lock(&irq->irq_lock);
 
-	irq->active = new_active_state;
-	if (new_active_state)
+	if (irq->hw)
+		vgic_hw_irq_change_active(vcpu, irq, active, !requester_vcpu);
+	else
+		irq->active = active;
+
+	if (irq->active)
 		vgic_queue_irq_unlock(vcpu->kvm, irq, flags);
 	else
 		spin_unlock_irqrestore(&irq->irq_lock, flags);
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 80c5c609385a..870cdacd9e81 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -144,6 +144,13 @@ void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq)
 	kfree(irq);
 }
 
+void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending)
+{
+	WARN_ON(irq_set_irqchip_state(irq->host_irq,
+				      IRQCHIP_STATE_PENDING,
+				      pending));
+}
+
 bool vgic_get_phys_line_level(struct vgic_irq *irq)
 {
 	bool line_level;
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index d0787983a357..12c37b89f7a3 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -146,6 +146,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 			      u32 intid);
 void vgic_put_irq(struct kvm *kvm, struct vgic_irq *irq);
 bool vgic_get_phys_line_level(struct vgic_irq *irq);
+void vgic_irq_set_phys_pending(struct vgic_irq *irq, bool pending);
 void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active);
 bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
 			   unsigned long flags);
-- 
2.14.2

^ permalink raw reply related

* [PATCH v8 7/9] KVM: arm/arm64: Provide a get_input_level for the arch timer
From: Christoffer Dall @ 2017-12-13 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

The VGIC can now support the life-cycle of mapped level-triggered
interrupts, and we no longer have to read back the timer state on every
exit from the VM if we had an asserted timer interrupt signal, because
the VGIC already knows if we hit the unlikely case where the guest
disables the timer without ACKing the virtual timer interrupt.

This means we rework a bit of the code to factor out the functionality
to snapshot the timer state from vtimer_save_state(), and we can reuse
this functionality in the sync path when we have an irqchip in
userspace, and also to support our implementation of the
get_input_level() function for the timer.

This change also means that we can no longer rely on the timer's view of
the interrupt line to set the active state, because we no longer
maintain this state for mapped interrupts when exiting from the guest.
Instead, we only set the active state if the virtual interrupt is
active, and otherwise we simply let the timer fire again and raise the
virtual interrupt from the ISR.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 include/kvm/arm_arch_timer.h |  2 ++
 virt/kvm/arm/arch_timer.c    | 82 +++++++++++++++++++-------------------------
 2 files changed, 38 insertions(+), 46 deletions(-)

diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index 01ee473517e2..f57f795d704c 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -90,6 +90,8 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
 
 void kvm_timer_init_vhe(void);
 
+bool kvm_arch_timer_get_input_level(int vintid);
+
 #define vcpu_vtimer(v)	(&(v)->arch.timer_cpu.vtimer)
 #define vcpu_ptimer(v)	(&(v)->arch.timer_cpu.ptimer)
 
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index e78ba5e20f74..f8d09665ddce 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -343,6 +343,12 @@ static void kvm_timer_update_state(struct kvm_vcpu *vcpu)
 	phys_timer_emulate(vcpu);
 }
 
+static void __timer_snapshot_state(struct arch_timer_context *timer)
+{
+	timer->cnt_ctl = read_sysreg_el0(cntv_ctl);
+	timer->cnt_cval = read_sysreg_el0(cntv_cval);
+}
+
 static void vtimer_save_state(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
@@ -354,10 +360,8 @@ static void vtimer_save_state(struct kvm_vcpu *vcpu)
 	if (!vtimer->loaded)
 		goto out;
 
-	if (timer->enabled) {
-		vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
-		vtimer->cnt_cval = read_sysreg_el0(cntv_cval);
-	}
+	if (timer->enabled)
+		__timer_snapshot_state(vtimer);
 
 	/* Disable the virtual timer */
 	write_sysreg_el0(0, cntv_ctl);
@@ -454,8 +458,7 @@ static void kvm_timer_vcpu_load_vgic(struct kvm_vcpu *vcpu)
 	bool phys_active;
 	int ret;
 
-	phys_active = vtimer->irq.level ||
-		      kvm_vgic_map_is_active(vcpu, vtimer->irq.irq);
+	phys_active = kvm_vgic_map_is_active(vcpu, vtimer->irq.irq);
 
 	ret = irq_set_irqchip_state(host_vtimer_irq,
 				    IRQCHIP_STATE_ACTIVE,
@@ -541,54 +544,25 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
 	set_cntvoff(0);
 }
 
-static void unmask_vtimer_irq(struct kvm_vcpu *vcpu)
+/*
+ * With a userspace irqchip we have to check if the guest de-asserted the
+ * timer and if so, unmask the timer irq signal on the host interrupt
+ * controller to ensure that we see future timer signals.
+ */
+static void unmask_vtimer_irq_user(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
 
 	if (unlikely(!irqchip_in_kernel(vcpu->kvm))) {
-		kvm_vtimer_update_mask_user(vcpu);
-		return;
-	}
-
-	/*
-	 * If the guest disabled the timer without acking the interrupt, then
-	 * we must make sure the physical and virtual active states are in
-	 * sync by deactivating the physical interrupt, because otherwise we
-	 * wouldn't see the next timer interrupt in the host.
-	 */
-	if (!kvm_vgic_map_is_active(vcpu, vtimer->irq.irq)) {
-		int ret;
-		ret = irq_set_irqchip_state(host_vtimer_irq,
-					    IRQCHIP_STATE_ACTIVE,
-					    false);
-		WARN_ON(ret);
+		__timer_snapshot_state(vtimer);
+		if (!kvm_timer_should_fire(vtimer))
+			kvm_vtimer_update_mask_user(vcpu);
 	}
 }
 
-/**
- * kvm_timer_sync_hwstate - sync timer state from cpu
- * @vcpu: The vcpu pointer
- *
- * Check if any of the timers have expired while we were running in the guest,
- * and inject an interrupt if that was the case.
- */
 void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
 {
-	struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
-
-	/*
-	 * If we entered the guest with the vtimer output asserted we have to
-	 * check if the guest has modified the timer so that we should lower
-	 * the line at this point.
-	 */
-	if (vtimer->irq.level) {
-		vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);
-		vtimer->cnt_cval = read_sysreg_el0(cntv_cval);
-		if (!kvm_timer_should_fire(vtimer)) {
-			kvm_timer_update_irq(vcpu, false, vtimer);
-			unmask_vtimer_irq(vcpu);
-		}
-	}
+	unmask_vtimer_irq_user(vcpu);
 }
 
 int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)
@@ -819,6 +793,22 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+bool kvm_arch_timer_get_input_level(int vintid)
+{
+	struct kvm_vcpu *vcpu = kvm_arm_get_running_vcpu();
+	struct arch_timer_context *timer;
+
+	if (vintid == vcpu_vtimer(vcpu)->irq.irq)
+		timer = vcpu_vtimer(vcpu);
+	else
+		BUG(); /* We only map the vtimer so far */
+
+	if (timer->loaded)
+		__timer_snapshot_state(timer);
+
+	return kvm_timer_should_fire(timer);
+}
+
 int kvm_timer_enable(struct kvm_vcpu *vcpu)
 {
 	struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
@@ -841,7 +831,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
 	}
 
 	ret = kvm_vgic_map_phys_irq(vcpu, host_vtimer_irq, vtimer->irq.irq,
-				    NULL);
+				    kvm_arch_timer_get_input_level);
 	if (ret)
 		return ret;
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH v8 8/9] KVM: arm/arm64: Avoid work when userspace iqchips are not used
From: Christoffer Dall @ 2017-12-13 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

We currently check if the VM has a userspace irqchip on every exit from
the VCPU, and if so, we do some work to ensure correct timer behavior.
This is unfortunate, as we could avoid doing any work entirely, if we
didn't have to support irqchip in userspace.

Realizing the userspace irqchip on ARM is mostly a developer or hobby
feature, and is unlikely to be used in servers or other scenarios where
performance is a priority, we can use a refcounted static key to only
check the irqchip configuration when we have at least one VM that uses
an irqchip in userspace.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/arch_timer.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index f8d09665ddce..73d262c4712b 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -51,6 +51,8 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
 				 struct arch_timer_context *timer_ctx);
 static bool kvm_timer_should_fire(struct arch_timer_context *timer_ctx);
 
+static DEFINE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
+
 u64 kvm_phys_timer_read(void)
 {
 	return timecounter->cc->read(timecounter->cc);
@@ -562,7 +564,8 @@ static void unmask_vtimer_irq_user(struct kvm_vcpu *vcpu)
 
 void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
 {
-	unmask_vtimer_irq_user(vcpu);
+	if (static_branch_unlikely(&userspace_irqchip_in_use))
+		unmask_vtimer_irq_user(vcpu);
 }
 
 int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)
@@ -767,6 +770,8 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
 	soft_timer_cancel(&timer->bg_timer, &timer->expired);
 	soft_timer_cancel(&timer->phys_timer, NULL);
 	kvm_vgic_unmap_phys_irq(vcpu, vtimer->irq.irq);
+	if (timer->enabled && !irqchip_in_kernel(vcpu->kvm))
+		static_branch_dec(&userspace_irqchip_in_use);
 }
 
 static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)
@@ -819,8 +824,10 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
 		return 0;
 
 	/* Without a VGIC we do not map virtual IRQs to physical IRQs */
-	if (!irqchip_in_kernel(vcpu->kvm))
+	if (!irqchip_in_kernel(vcpu->kvm)) {
+		static_branch_inc(&userspace_irqchip_in_use);
 		goto no_vgic;
+	}
 
 	if (!vgic_initialized(vcpu->kvm))
 		return -ENODEV;
-- 
2.14.2

^ permalink raw reply related

* [PATCH v8 9/9] KVM: arm/arm64: Update timer and forwarded irq documentation
From: Christoffer Dall @ 2017-12-13 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213104602.16383-1-christoffer.dall@linaro.org>

Now when we've reworked how mapped level-triggered interrupts are
processed for the timer interrupts, we update the documentation
correspondingly.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt | 50 ++++++++++------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
index 38bca2835278..f68c7d95a341 100644
--- a/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
+++ b/Documentation/virtual/kvm/arm/vgic-mapped-irqs.txt
@@ -7,9 +7,10 @@ allowing software to inject virtual interrupts to a VM, which the guest
 OS sees as regular interrupts.  The code is famously known as the VGIC.
 
 Some of these virtual interrupts, however, correspond to physical
-interrupts from real physical devices.  One example could be the
-architected timer, which itself supports virtualization, and therefore
-lets a guest OS program the hardware device directly to raise an
+interrupts from real physical devices.  One example could be the ARM
+Generic Timers (also known as the "architected timers"), which are
+directly assigned to a VM while it's running, and therefore
+makes it possible for guest OSes to program the timers directly to raise an
 interrupt at some point in time.  When such an interrupt is raised, the
 host OS initially handles the interrupt and must somehow signal this
 event as a virtual interrupt to the guest.  Another example could be a
@@ -37,7 +38,7 @@ inactive.
 
 The LRs include an extra bit, called the HW bit.  When this bit is set,
 KVM must also program an additional field in the LR, the physical IRQ
-number, to link the virtual with the physical IRQ.
+number, to link the virtual and physical IRQs together.
 
 When the HW bit is set, KVM must EITHER set the Pending OR the Active
 bit, never both at the same time.
@@ -59,21 +60,21 @@ The state of forwarded physical interrupts is managed in the following way:
   - LR.Pending will stay set as long as the guest has not acked the interrupt.
   - LR.Pending transitions to LR.Active on the guest read of the IAR, as
     expected.
-  - On guest EOI, the *physical distributor* active bit gets cleared,
+  - On guest deactivate, the *physical distributor* active bit gets cleared,
     but the LR.Active is left untouched (set).
   - KVM clears the LR on VM exits when the physical distributor
     active state has been cleared.
 
 (*): The host handling is slightly more complicated.  For some forwarded
-interrupts (shared), KVM directly sets the active state on the physical
-distributor before entering the guest, because the interrupt is never actually
-handled on the host (see details on the timer as an example below).  For other
-forwarded interrupts (non-shared) the host does not deactivate the interrupt
-when the host ISR completes, but leaves the interrupt active until the guest
-deactivates it.  Leaving the interrupt active is allowed, because Linux
-configures the physical GIC with EOIMode=1, which causes EOI operations to
-perform a priority drop allowing the GIC to receive other interrupts of the
-default priority.
+interrupts (shared), in some cases, KVM directly sets the active state
+on the physical distributor before entering the guest, because the
+interrupt is never actually handled on the host (see details on the
+timer as an example below).  In other cases, the host does not
+deactivate the interrupt when the host ISR completes, but leaves the
+interrupt active until the guest deactivates it.  Leaving the interrupt
+active is allowed, because Linux configures the physical GIC with
+EOIMode=1, which causes EOI operations to perform a priority drop
+allowing the GIC to receive other interrupts of the default priority.
 
 
 Forwarded Edge and Level Triggered PPIs and SPIs
@@ -170,18 +171,13 @@ instead:
 
 1.  KVM runs the VCPU
 2.  The guest programs the time to fire in T+100
-4.  At T+100 the timer fires and a physical IRQ causes the VM to exit
+3.  At T+100 the timer fires and a physical IRQ causes the VM to exit
     (note that this initially only traps to EL2 and does not run the host ISR
     until KVM has returned to the host).
-5.  With interrupts still disabled on the CPU coming back from the guest, KVM
-    stores the virtual timer state to memory and disables the virtual hw timer.
-6.  KVM looks at the timer state (in memory) and injects a forwarded physical
-    interrupt because it concludes the timer has expired.
-7.  KVM marks the timer interrupt as active on the physical distributor
-7.  KVM enables the timer, enables interrupts, and runs the VCPU
-
-Notice that again the forwarded physical interrupt is injected to the
-guest without having actually been handled on the host.  In this case it
-is because the physical interrupt is never actually seen by the host because the
-timer is disabled upon guest return, and the virtual forwarded interrupt is
-injected on the KVM guest entry path.
+4.  When KVM returns to EL1 and enables interrupts, the timer interrupt
+    fires again, and the kvm arch timer ISR runs and injects a virtual
+    interrupt to the guest.
+5.  Because the timer interrupt has the vcpu affinity set, as the ISR
+    completes, the physical interrupt stays active on the physical
+    distributor.
+6.  KVM enables the timer, enables interrupts, and runs the VCPU
-- 
2.14.2

^ permalink raw reply related

* [PATCH] arch/arm64: elfcorehdr should be the first allocation
From: AKASHI Takahiro @ 2017-12-13 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211140714.GD2141@arm.com>

On Mon, Dec 11, 2017 at 02:07:14PM +0000, Will Deacon wrote:
> On Mon, Dec 11, 2017 at 11:03:32AM +0530, Prabhakar Kushwaha wrote:
> > From: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> > 
> > elfcorehdr_addr is assigned by kexec-utils and device tree of
> > dump kernel is fixed in chosen node with parameter "linux,elfcorehdr".
> > So, memory should be first reserved for elfcorehdr,
> > otherwise overlaps may happen with other memory allocations
> > which were done before the allocation of elcorehdr in the crash kernel
> 
> What happens in that case? Do you have a crash log we can include in
> the commit message?

In private discussions with Poonam, he said:
|   The overlap here I observed was for the reserved-mem areas in the dtb.
|   And they were specific to NXP device.

Since I have not got any details since then, I'm not sure
whether your patch is the way to go.
(I suspect that we might better fix the issue on kexec-tools side.)

Thanks,
-Takahiro AKASHI

> > Signed-off-by: Guanhua <guanhua.gao@nxp.com>
> > Signed-off-by: Poonam Aggrwal <poonam.aggrwal@nxp.com>
> > Signed-off-by: Abhimanyu Saini <abhimanyu.saini@nxp.com>
> > ---
> 
> Really? How on Earth did you get three people co-developing this patch?
> 
> >  arch/arm64/mm/init.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 5960bef0170d..551048cfcfff 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)
> >  	 * Register the kernel text, kernel data, initrd, and initial
> >  	 * pagetables with memblock.
> >  	 */
> > +
> > +	/* make this the first reservation so that there are no chances of
> > +	 * overlap */
> > +	reserve_elfcorehdr();
> >  	memblock_reserve(__pa_symbol(_text), _end - _text);
> >  #ifdef CONFIG_BLK_DEV_INITRD
> >  	if (initrd_start) {
> > @@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)
> >  
> >  	reserve_crashkernel();
> >  
> > -	reserve_elfcorehdr();
> 
> Why isn't this also a problem for reserve_crashkernel() or any other
> static reservations?
> 
> Will

^ permalink raw reply

* [PATCH] IPI performance benchmark
From: Yury Norov @ 2017-12-13 10:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211153332.GG15649@char.us.oracle.com>

On Mon, Dec 11, 2017 at 10:33:32AM -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Dec 11, 2017 at 05:16:00PM +0300, Yury Norov wrote:
> > This benchmark sends many IPIs in different modes and measures
> > time for IPI delivery (first column), and total time, ie including
> > time to acknowledge the receive by sender (second column).
> > 
> > The scenarios are:
> > Dry-run:	do everything except actually sending IPI. Useful
> > 		to estimate system overhead.
> > Self-IPI:	Send IPI to self CPU.
> > Normal IPI:	Send IPI to some other CPU.
> > Broadcast IPI:	Send broadcast IPI to all online CPUs.
> > 
> > For virtualized guests, sending and reveiving IPIs causes guest exit.
> > I used this test to measure performance impact on KVM subsystem of
> > Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
> > 
> > https://www.spinics.net/lists/kvm/msg156755.html
> > 
> > Test machine is ThunderX2, 112 online CPUs. Below the results normalized
> > to host dry-run time. Smaller - better.
> 
> Would it make sense to also add spinlock contention tests? Meaning make
> this framework a bit more generic so you could do IPI and you could
> also do spinlock contention?
> 
> Like:
> http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/drivers/spinlock_hog/spinlock_hog.c;h=040a154808452576b1aa5720a6282981319a5360;hb=HEAD

There's kernel/locking/locktorture.c for spinlock testing. Maybe it
worth to add new testcase there? If you find my 'framework' more
suitable for you, I'm also OK with it. Is my understanding correct
that you want something like broadcast IPI case, but with different
payload?

Yury

> And then also this could be used for an CPU intensive
> test to see how well virtualization schedulers work:
> 
> http://xenbits.xen.org/gitweb/?p=xentesttools/bootstrap.git;a=blob;f=root_image/drivers/ipi_hog/ipi_hog.c;h=6dcc4f3382c29c42fec077791cc53bc52e6d8868;hb=HEAD
> 
> (asking as it never occurred to me upstream those test-cases
> but if folks are OK with Yury little module, then other things
> could be done as well with it).
> 
> > 
> > Host, v4.14:
> > Dry-run:	  0	    1
> > Self-IPI:         9	   18
> > Normal IPI:      81	  110
> > Broadcast IPI:    0	 2106
> > 
> > Guest, v4.14:
> > Dry-run:          0	    1
> > Self-IPI:        10	   18
> > Normal IPI:     305	  525
> > Broadcast IPI:    0    	 9729
> > 
> > Guest, v4.14 + VHE:
> > Dry-run:          0	    1
> > Self-IPI:         9	   18
> > Normal IPI:     176	  343
> > Broadcast IPI:    0	 9885
> > 
> > CC: Andrew Morton <akpm@linux-foundation.org>
> > CC: Ashish Kalra <Ashish.Kalra@cavium.com>
> > CC: Christoffer Dall <christoffer.dall@linaro.org>
> > CC: Geert Uytterhoeven <geert@linux-m68k.org>
> > CC: Linu Cherian <Linu.Cherian@cavium.com>
> > CC: Sunil Goutham <Sunil.Goutham@cavium.com>
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> > ---
> >  arch/Kconfig           |  10 ++++
> >  kernel/Makefile        |   1 +
> >  kernel/ipi_benchmark.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 145 insertions(+)
> >  create mode 100644 kernel/ipi_benchmark.c
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 057370a0ac4e..80d6ef439199 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -82,6 +82,16 @@ config JUMP_LABEL
> >  	 ( On 32-bit x86, the necessary options added to the compiler
> >  	   flags may increase the size of the kernel slightly. )
> >  
> > +config IPI_BENCHMARK
> > +	tristate "Test IPI performance on SMP systems"
> > +	depends on SMP
> > +	help
> > +	  Test IPI performance on SMP systems. If system has only one online
> > +	  CPU, sending IPI to other CPU is obviously not possible, and ENOENT
> > +	  is returned for corresponding test.
> > +
> > +	  If unsure, say N.
> > +
> >  config STATIC_KEYS_SELFTEST
> >  	bool "Static key selftest"
> >  	depends on JUMP_LABEL
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index 172d151d429c..04e550e1990c 100644
> > --- a/kernel/Makefile
> > +++ b/kernel/Makefile
> > @@ -101,6 +101,7 @@ obj-$(CONFIG_TRACEPOINTS) += trace/
> >  obj-$(CONFIG_IRQ_WORK) += irq_work.o
> >  obj-$(CONFIG_CPU_PM) += cpu_pm.o
> >  obj-$(CONFIG_BPF) += bpf/
> > +obj-$(CONFIG_IPI_BENCHMARK) += ipi_benchmark.o
> >  
> >  obj-$(CONFIG_PERF_EVENTS) += events/
> >  
> > diff --git a/kernel/ipi_benchmark.c b/kernel/ipi_benchmark.c
> > new file mode 100644
> > index 000000000000..35f1f7598c36
> > --- /dev/null
> > +++ b/kernel/ipi_benchmark.c
> > @@ -0,0 +1,134 @@
> > +/*
> > + * Performance test for IPI on SMP machines.
> > + *
> > + * Copyright (c) 2017 Cavium Networks.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of version 2 of the GNU General Public
> > + * License as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/ktime.h>
> > +
> > +#define NTIMES 100000
> > +
> > +#define POKE_ANY	0
> > +#define DRY_RUN		1
> > +#define POKE_SELF	2
> > +#define POKE_ALL	3
> > +
> > +static void __init handle_ipi(void *t)
> > +{
> > +	ktime_t *time = (ktime_t *) t;
> > +
> > +	if (time)
> > +		*time = ktime_get() - *time;
> > +}
> > +
> > +static ktime_t __init send_ipi(int flags)
> > +{
> > +	ktime_t time;
> > +	unsigned int cpu = get_cpu();
> > +
> > +	switch (flags) {
> > +	case POKE_ALL:
> > +		/* If broadcasting, don't force all CPUs to update time. */
> > +		smp_call_function_many(cpu_online_mask, handle_ipi, NULL, 1);
> > +		/* Fall thru */
> > +	case DRY_RUN:
> > +		/* Do everything except actually sending IPI. */
> > +		time = 0;
> > +		break;
> > +	case POKE_ANY:
> > +		cpu = cpumask_any_but(cpu_online_mask, cpu);
> > +		if (cpu >= nr_cpu_ids) {
> > +			time = -ENOENT;
> > +			break;
> > +		}
> > +		/* Fall thru */
> > +	case POKE_SELF:
> > +		time = ktime_get();
> > +		smp_call_function_single(cpu, handle_ipi, &time, 1);
> > +		break;
> > +	default:
> > +		time = -EINVAL;
> > +	}
> > +
> > +	put_cpu();
> > +	return time;
> > +}
> > +
> > +static int __init __bench_ipi(unsigned long i, ktime_t *time, int flags)
> > +{
> > +	ktime_t t;
> > +
> > +	*time = 0;
> > +	while (i--) {
> > +		t = send_ipi(flags);
> > +		if ((int) t < 0)
> > +			return (int) t;
> > +
> > +		*time += t;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init bench_ipi(unsigned long times, int flags,
> > +				ktime_t *ipi, ktime_t *total)
> > +{
> > +	int ret;
> > +
> > +	*total = ktime_get();
> > +	ret = __bench_ipi(times, ipi, flags);
> > +	if (unlikely(ret))
> > +		return ret;
> > +
> > +	*total = ktime_get() - *total;
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init init_bench_ipi(void)
> > +{
> > +	ktime_t ipi, total;
> > +	int ret;
> > +
> > +	ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
> > +	if (ret)
> > +		pr_err("Dry-run FAILED: %d\n", ret);
> > +	else
> > +		pr_err("Dry-run:       %18llu, %18llu ns\n", ipi, total);
> > +
> > +	ret = bench_ipi(NTIMES, POKE_SELF, &ipi, &total);
> > +	if (ret)
> > +		pr_err("Self-IPI FAILED: %d\n", ret);
> > +	else
> > +		pr_err("Self-IPI:      %18llu, %18llu ns\n", ipi, total);
> > +
> > +	ret = bench_ipi(NTIMES, POKE_ANY, &ipi, &total);
> > +	if (ret)
> > +		pr_err("Normal IPI FAILED: %d\n", ret);
> > +	else
> > +		pr_err("Normal IPI:    %18llu, %18llu ns\n", ipi, total);
> > +
> > +	ret = bench_ipi(NTIMES, POKE_ALL, &ipi, &total);
> > +	if (ret)
> > +		pr_err("Broadcast IPI FAILED: %d\n", ret);
> > +	else
> > +		pr_err("Broadcast IPI: %18llu, %18llu ns\n", ipi, total);
> > +
> > +	/* Return error to avoid annoying rmmod. */
> > +	return -EINVAL;
> > +}
> > +module_init(init_bench_ipi);
> > +
> > +MODULE_LICENSE("GPL");
> > -- 
> > 2.11.0
> > 

^ permalink raw reply

* arm64 crashkernel fails to boot on acpi-only machines due to ACPI regions being no longer mapped as NOMAP
From: Ard Biesheuvel @ 2017-12-13 10:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213102624.GC28046@linaro.org>

On 13 December 2017 at 10:26, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
> Bhupesh, Ard,
>
> On Wed, Dec 13, 2017 at 03:21:59AM +0530, Bhupesh Sharma wrote:
>> Hi Ard, Akashi
>>
> (snip)
>
>> Looking deeper into the issue, since the arm64 kexec-tools uses the
>> 'linux,usable-memory-range' dt property to allow crash dump kernel to
>> identify its own usable memory and exclude, at its boot time, any
>> other memory areas that are part of the panicked kernel's memory.
>> (see https://www.kernel.org/doc/Documentation/devicetree/bindings/chosen.txt
>> , for details)
>
> Right.
>
>> 1). Now when 'kexec -p' is executed, this node is patched up only
>> with the crashkernel memory range:
>>
>>                 /* add linux,usable-memory-range */
>>                 nodeoffset = fdt_path_offset(new_buf, "/chosen");
>>                 result = fdt_setprop_range(new_buf, nodeoffset,
>>                                 PROP_USABLE_MEM_RANGE, &crash_reserved_mem,
>>                                 address_cells, size_cells);
>>
>> (see https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/arm64/kexec-arm64.c#n465
>> , for details)
>>
>> 2). This excludes the ACPI reclaim regions irrespective of whether
>> they are marked as System RAM or as RESERVED. As,
>> 'linux,usable-memory-range' dt node is patched up only with
>> 'crash_reserved_mem' and not 'system_memory_ranges'
>>
>> 3). As a result when the crashkernel boots up it doesn't find this
>> ACPI memory and crashes while trying to access the same:
>>
>> # kexec -p /boot/vmlinuz-`uname -r` --initrd=/boot/initramfs-`uname
>> -r`.img --reuse-cmdline -d
>>
>> [snip..]
>>
>> Reserved memory range
>> 000000000e800000-000000002e7fffff (0)
>>
>> Coredump memory ranges
>> 0000000000000000-000000000e7fffff (0)
>> 000000002e800000-000000003961ffff (0)
>> 0000000039d40000-000000003ed2ffff (0)
>> 000000003ed60000-000000003fbfffff (0)
>> 0000001040000000-0000001ffbffffff (0)
>> 0000002000000000-0000002ffbffffff (0)
>> 0000009000000000-0000009ffbffffff (0)
>> 000000a000000000-000000affbffffff (0)
>>
>> 4). So if we revert Ard's patch or just comment the fixing up of the
>> memory cap'ing passed to the crash kernel inside
>> 'arch/arm64/mm/init.c' (see below):
>>
>> static void __init fdt_enforce_memory_region(void)
>> {
>>         struct memblock_region reg = {
>>                 .size = 0,
>>         };
>>
>>         of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
>>
>>         if (reg.size)
>>                 //memblock_cap_memory_range(reg.base, reg.size); /*
>> comment this out */
>> }
>
> Please just don't do that. It can cause a fatal damage on
> memory contents of the *crashed* kernel.
>
>> 5). Both the above temporary solutions fix the problem.
>>
>> 6). However exposing all System RAM regions to the crashkernel is not
>> advisable and may cause the crashkernel or some crashkernel drivers to
>> fail.
>>
>> 6a). I am trying an approach now, where the ACPI reclaim regions are
>> added to '/proc/iomem' separately as ACPI reclaim regions by the
>> kernel code and on the other hand the user-space 'kexec-tools' will
>> pick up the ACPI reclaim regions from '/proc/iomem' and add it to the
>> dt node 'linux,usable-memory-range'
>
> I still don't understand why we need to carry over the information
> about "ACPI Reclaim memory" to crash dump kernel. In my understandings,
> such regions are free to be reused by the kernel after some point of
> initialization. Why does crash dump kernel need to know about them?
>

Not really. According to the UEFI spec, they can be reclaimed after
the OS has initialized, i.e., when it has consumed the ACPI tables and
no longer needs them. Of course, in order to be able to boot a kexec
kernel, those regions needs to be preserved, which is why they are
memblock_reserve()'d now.

So it seems that kexec does not honour the memblock_reserve() table
when booting the next kernel.

> (In other words, can or should we skip some part of ACPI-related init code
> on crash dump kernel?)
>

I don't think so. And the change to the handling of ACPI reclaim
regions only revealed the bug, not created it (given that other
memblock_reserve regions may be affected as well)


>> 6b). The kernel code currently looks like the following:
>>
>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> index 30ad2f085d1f..867bdec7c692 100644
>> --- a/arch/arm64/kernel/setup.c
>> +++ b/arch/arm64/kernel/setup.c
>> @@ -206,6 +206,7 @@ static void __init request_standard_resources(void)
>>  {
>>      struct memblock_region *region;
>>      struct resource *res;
>> +    phys_addr_t addr_start, addr_end;
>>
>>      kernel_code.start   = __pa_symbol(_text);
>>      kernel_code.end     = __pa_symbol(__init_begin - 1);
>> @@ -218,9 +219,17 @@ static void __init request_standard_resources(void)
>>              res->name  = "reserved";
>>              res->flags = IORESOURCE_MEM;
>>          } else {
>> -            res->name  = "System RAM";
>> -            res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>> +            addr_start =
>> __pfn_to_phys(memblock_region_reserved_base_pfn(region));
>> +            addr_end =
>> __pfn_to_phys(memblock_region_reserved_end_pfn(region)) - 1;
>> +            if ((efi_mem_type(addr_start) == EFI_ACPI_RECLAIM_MEMORY)
>> || (efi_mem_type(addr_end) == EFI_ACPI_RECLAIM_MEMORY)) {
>> +                res->name  = "ACPI reclaim region";
>> +                res->flags = IORESOURCE_MEM;
>> +            } else {
>> +                res->name  = "System RAM";
>> +                res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
>> +            }
>>          }
>> +
>>          res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
>>          res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;
>>
>> @@ -292,6 +301,7 @@ void __init setup_arch(char **cmdline_p)
>>
>>      request_standard_resources();
>>
>> +    efi_memmap_unmap();
>>      early_ioremap_reset();
>>
>>      if (acpi_disabled)
>> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> index 80d1a885def5..a7c522eac640 100644
>> --- a/drivers/firmware/efi/arm-init.c
>> +++ b/drivers/firmware/efi/arm-init.c
>> @@ -259,7 +259,6 @@ void __init efi_init(void)
>>
>>      reserve_regions();
>>      efi_esrt_init();
>> -    efi_memmap_unmap();
>>
>>      memblock_reserve(params.mmap & PAGE_MASK,
>>               PAGE_ALIGN(params.mmap_size +
>>
>>
>> After this change the ACPI reclaim regions are properly recognized in
>> '/proc/iomem':
>>
>> # cat /proc/iomem | grep -i ACPI
>> 396c0000-3975ffff : ACPI reclaim region
>> 39770000-397affff : ACPI reclaim region
>> 398a0000-398bffff : ACPI reclaim region
>>
>> 6c). I am currently changing the 'kexec-tools' and will finish the
>> testing over the next few days.
>>
>> I just wanted to know your opinion on this issue, so that I will be
>> able to propose a fix on the above lines.
>>
>> Also Cc'ing kexec mailing list for more inputs on changes proposed to
>> kexec-tools.
>>
>> Thanks,
>> Bhupesh

^ permalink raw reply

* [PATCH 2/4] arm: dts: sun8i: a83t: Add registers needed for MCPM
From: Maxime Ripard @ 2017-12-13 10:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211075001.6100-3-mylene.josserand@free-electrons.com>

Hi,

On Mon, Dec 11, 2017 at 08:49:59AM +0100, Myl?ne Josserand wrote:
> Add 3 registers needed for MCPM (ie SMP): prcm, cpucfg and r_cpucfg.
> prcm and cpucfg are identical with sun9i-a80. The only difference
> is the r_cpucfg that does not exist on sun9i.
> 
> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index a384b766f3dc..eeb2e7d0d6dc 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -323,6 +323,16 @@
>  			#reset-cells = <1>;
>  		};


> +		cpucfg at 01700000 {

Please drop the leading zero here, it generates a warning in dtc.

> +			compatible = "allwinner,sun9i-a80-cpucfg";

There's some significant differences between the A83t and the A80 IPs,
you should use a different compatible.

> +			reg = <0x01700000 0x100>;

the size is 1k (0x400)

> +		};
> +
> +		r_cpucfg at 1f01c00 {
> +			compatible = "allwinner,sun8i-a83t-r-cpucfg";
> +			reg = <0x1f01c00 0x100>;

You should order the nodes by physical address

> +		};
> +
>  		pio: pinctrl at 1c20800 {
>  			compatible = "allwinner,sun8i-a83t-pinctrl";
>  			interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
> @@ -493,6 +503,11 @@
>  			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		prcm at 1f01400 {
> +			compatible = "allwinner,sun9i-a80-prcm";

That block is significantly different on the A83t. Please use a
different compatible.

> +			reg = <0x1f01400 0x200>;
> +		};
> +

The size is 1k, again.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/dbb4fb64/attachment.sig>

^ permalink raw reply

* [PATCH 3/4] arm: dts: sun8i: a83t: Add CCI-400 node
From: Maxime Ripard @ 2017-12-13 10:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211075001.6100-4-mylene.josserand@free-electrons.com>

Hi,

On Mon, Dec 11, 2017 at 08:50:00AM +0100, Myl?ne Josserand wrote:
> Add CCI-400 node and control-port on CPUs needed by MCPM (ie SMP).
> 
> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 41 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index eeb2e7d0d6dc..3e2aad537972 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -62,48 +62,56 @@
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <0>;
> +			cci-control-port = <&cci_control0>;
>  		};
>  
>  		cpu at 1 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <1>;
> +			cci-control-port = <&cci_control0>;
>  		};
>  
>  		cpu at 2 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <2>;
> +			cci-control-port = <&cci_control0>;
>  		};
>  
>  		cpu at 3 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <3>;
> +			cci-control-port = <&cci_control0>;
>  		};
>  
>  		cpu at 100 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <0x100>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu at 101 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <0x101>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu at 102 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <0x102>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  
>  		cpu at 103 {
>  			compatible = "arm,cortex-a7";
>  			device_type = "cpu";
>  			reg = <0x103>;
> +			cci-control-port = <&cci_control1>;
>  		};
>  	};
>  
> @@ -314,6 +322,39 @@
>  			status = "disabled";
>  		};
>  
> +		cci: cci at 1790000 {

You're not using that label, and you should order the node by physical
address.

> +			compatible = "arm,cci-400";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			reg = <0x01790000 0x1000>;

The size is 0x10000.

> +			ranges = <0x0 0x01790000 0x10000>;
> +
> +			cci_control0: slave-if at 4000 {
> +				compatible = "arm,cci-400-ctrl-if";
> +				interface-type = "ace";
> +				reg = <0x4000 0x1000>;
> +			};
> +
> +			cci_control1: slave-if at 5000 {
> +				compatible = "arm,cci-400-ctrl-if";
> +				interface-type = "ace";
> +				reg = <0x5000 0x1000>;
> +			};
> +
> +			pmu at 9000 {
> +				compatible = "arm,cci-400-pmu,r1";
> +				reg = <0x9000 0x5000>;
> +				interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +		};

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/ebbba3d1/attachment.sig>

^ permalink raw reply

* [PATCH 4/4] arm: dts: sun8i: a83t: Set timer node to use phy timer
From: Maxime Ripard @ 2017-12-13 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171211075001.6100-5-mylene.josserand@free-electrons.com>

Hi,

On Mon, Dec 11, 2017 at 08:50:01AM +0100, Myl?ne Josserand wrote:
> By default, virtual timers are used. These timers need an offset
> that must be set by firmware, for example. In case of SMP support,
> after a reset, this offset is in "unknown" state and produced
> a hang of the kernel.
> 
> Use "arm,cpu-registers-not-fw-configured" property allows to use
> physical timers instead of virtual ones.
> 
> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>

Your commit log could be a little better, something like:

"
The ARM architected timers use an offset between their physical and
virtual counters. That offset should be configured by the bootloader
in CNTVOFF.

However, the A83t bootloader fails to do so, and we end up with an
undefined offset (which in our case is random), meaning that each CPU
will have a different time, which isn't working very well.

Fix that by setting the arm,cpu-registers-not-fw-configured that will
make Linux use the physical timers instead of the virtual ones. One
possible side effect would be that the virtualization features would
be disabled. However, due to the way the GIC has been integrated in
the system, it is already unusable so we're effectively not losing any
feature.
"

The commit title should be improved too.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171213/d0d1f918/attachment.sig>

^ permalink raw reply

* [PATCH v4 04/12] clk: qcom: Add HFPLL driver
From: Sricharan R @ 2017-12-13 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171212203515.gpnu7cnxilkdz4hc@rob-hp-laptop>

Hi Rob,
  Thanks for the review.

On 12/13/2017 2:05 AM, Rob Herring wrote:
> On Fri, Dec 08, 2017 at 03:12:22PM +0530, Sricharan R wrote:
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> On some devices (MSM8974 for example), the HFPLLs are
>> instantiated within the Krait processor subsystem as separate
>> register regions. Add a driver for these PLLs so that we can
>> provide HFPLL clocks for use by the system.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  .../devicetree/bindings/clock/qcom,hfpll.txt       |  40 ++++++++
>>  drivers/clk/qcom/Kconfig                           |   8 ++
>>  drivers/clk/qcom/Makefile                          |   1 +
>>  drivers/clk/qcom/hfpll.c                           | 106 +++++++++++++++++++++
>>  4 files changed, 155 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/qcom,hfpll.txt
>>  create mode 100644 drivers/clk/qcom/hfpll.c
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,hfpll.txt b/Documentation/devicetree/bindings/clock/qcom,hfpll.txt
>> new file mode 100644
>> index 0000000..fee92bb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,hfpll.txt
>> @@ -0,0 +1,40 @@
>> +High-Frequency PLL (HFPLL)
>> +
>> +PROPERTIES
>> +
>> +- compatible:
>> +	Usage: required
>> +	Value type: <string>
>> +	Definition: must be "qcom,hfpll"
> 
> Fine for a fallback, but please add SoC specific compatibles.
> 

 sure, will all add them.

>> +
>> +- reg:
>> +	Usage: required
>> +	Value type: <prop-encoded-array>
>> +	Definition: address and size of HPLL registers. An optional second
>> +		    element specifies the address and size of the alias
>> +		    register region.
>> +
>> +- clock-output-names:
>> +	Usage: required
>> +	Value type: <string>
>> +	Definition: Name of the PLL. Typically hfpllX where X is a CPU number
>> +		    starting at 0. Otherwise hfpll_Y where Y is more specific
>> +		    such as "l2".
>> +
>> +Example:
>> +
>> +1) An HFPLL for the L2 cache.
>> +
>> +	clock-controller at f9016000 {
>> +		compatible = "qcom,hfpll";
>> +		reg = <0xf9016000 0x30>;
>> +		clock-output-names = "hfpll_l2";
>> +	};
>> +
>> +2) An HFPLL for CPU0. This HFPLL has the alias register region.
>> +
>> +	clock-controller at f908a000 {
>> +		compatible = "qcom,hfpll";
>> +		reg = <0xf908a000 0x30>, <0xf900a000 0x30>;
>> +		clock-output-names = "hfpll0";
>> +	};
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index 20b5d6f..6c811bd 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -205,3 +205,11 @@ config SPMI_PMIC_CLKDIV
>>  	  Technologies, Inc. SPMI PMIC. It configures the frequency of
>>  	  clkdiv outputs of the PMIC. These clocks are typically wired
>>  	  through alternate functions on GPIO pins.
>> +
>> +config QCOM_HFPLL
>> +	tristate "High-Frequency PLL (HFPLL) Clock Controller"
>> +	depends on COMMON_CLK_QCOM
>> +	help
>> +	  Support for the high-frequency PLLs present on Qualcomm devices.
>> +	  Say Y if you want to support CPU frequency scaling on devices
>> +	  such as MSM8974, APQ8084, etc.
>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index 4795e21..4a4bf38 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -36,3 +36,4 @@ obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
>>  obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
>>  obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
>>  obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
>> +obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
>> diff --git a/drivers/clk/qcom/hfpll.c b/drivers/clk/qcom/hfpll.c
>> new file mode 100644
>> index 0000000..7405bb6
>> --- /dev/null
>> +++ b/drivers/clk/qcom/hfpll.c
>> @@ -0,0 +1,106 @@
>> +/*
>> + * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
> 
> It's 2017.
> 

 ok.

>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
> 
> Use SPDX tags.
> 

 ok.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

^ permalink raw reply

* [PATCH v4 05/12] clk: qcom: Add MSM8960/APQ8064's HFPLLs
From: Sricharan R @ 2017-12-13 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171212203631.kyruwhtpmigkwfat@rob-hp-laptop>

Hi Rob,

On 12/13/2017 2:06 AM, Rob Herring wrote:
> On Fri, Dec 08, 2017 at 03:12:23PM +0530, Sricharan R wrote:
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> Describe the HFPLLs present on MSM8960 and APQ8064 devices.
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  drivers/clk/qcom/gcc-msm8960.c               | 172 +++++++++++++++++++++++++++
>>  include/dt-bindings/clock/qcom,gcc-msm8960.h |   2 +
> 
> For the binding,
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 

 Thanks.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

^ permalink raw reply

* [PATCH v4 09/12] clk: qcom: Add Krait clock controller driver
From: Sricharan R @ 2017-12-13 11:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171212205104.vnnjr3w56w4mrfh6@rob-hp-laptop>

Hi Rob,

On 12/13/2017 2:21 AM, Rob Herring wrote:
> On Fri, Dec 08, 2017 at 03:12:27PM +0530, Sricharan R wrote:
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> The Krait CPU clocks are made up of a primary mux and secondary
>> mux for each CPU and the L2, controlled via cp15 accessors. For
>> Kraits within KPSSv1 each secondary mux accepts a different aux
>> source, but on KPSSv2 each secondary mux accepts the same aux
>> source.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  .../devicetree/bindings/clock/qcom,krait-cc.txt    |  22 ++
> 
> Please make bindings a separate patch.
> 

 ok.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

^ permalink raw reply

* [bpf-next V1-RFC PATCH 09/14] thunderx: setup xdp_rxq_info
From: Jesper Dangaard Brouer @ 2017-12-13 11:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <151316391502.14967.13292358380181773729.stgit@firesoul>

This driver uses a bool scheme for "enable"/"disable" when setting up
different resources.  Thus, the hook points for xdp_rxq_info is done
in the same function call nicvf_rcv_queue_config().  This is activated
through enable/disble via nicvf_config_data_transfer(), which is tied
into nicvf_stop()/nicvf_open().

Extending driver packet handler call-path nicvf_rcv_pkt_handler() with
a pointer to the given struct rcv_queue, in-order to access the
xdp_rxq_info data area (in nicvf_xdp_rx()).

Cc: linux-arm-kernel at lists.infradead.org
Cc: Sunil Goutham <sgoutham@cavium.com>
Cc: Robert Richter <rric@kernel.org>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   |   11 +++++++----
 drivers/net/ethernet/cavium/thunder/nicvf_queues.c |    7 +++++++
 drivers/net/ethernet/cavium/thunder/nicvf_queues.h |    2 ++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 52b3a6044f85..21618d0d694f 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -521,7 +521,7 @@ static void nicvf_unmap_page(struct nicvf *nic, struct page *page, u64 dma_addr)
 
 static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
 				struct cqe_rx_t *cqe_rx, struct snd_queue *sq,
-				struct sk_buff **skb)
+				struct rcv_queue *rq, struct sk_buff **skb)
 {
 	struct xdp_buff xdp;
 	struct page *page;
@@ -545,6 +545,7 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
 	xdp.data = (void *)cpu_addr;
 	xdp_set_data_meta_invalid(&xdp);
 	xdp.data_end = xdp.data + len;
+	xdp.rxq = &rq->xdp_rxq;
 	orig_data = xdp.data;
 
 	rcu_read_lock();
@@ -698,7 +699,8 @@ static inline void nicvf_set_rxhash(struct net_device *netdev,
 
 static void nicvf_rcv_pkt_handler(struct net_device *netdev,
 				  struct napi_struct *napi,
-				  struct cqe_rx_t *cqe_rx, struct snd_queue *sq)
+				  struct cqe_rx_t *cqe_rx,
+				  struct snd_queue *sq, struct rcv_queue *rq)
 {
 	struct sk_buff *skb = NULL;
 	struct nicvf *nic = netdev_priv(netdev);
@@ -724,7 +726,7 @@ static void nicvf_rcv_pkt_handler(struct net_device *netdev,
 	/* For XDP, ignore pkts spanning multiple pages */
 	if (nic->xdp_prog && (cqe_rx->rb_cnt == 1)) {
 		/* Packet consumed by XDP */
-		if (nicvf_xdp_rx(snic, nic->xdp_prog, cqe_rx, sq, &skb))
+		if (nicvf_xdp_rx(snic, nic->xdp_prog, cqe_rx, sq, rq, &skb))
 			return;
 	} else {
 		skb = nicvf_get_rcv_skb(snic, cqe_rx,
@@ -781,6 +783,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
 	struct cqe_rx_t *cq_desc;
 	struct netdev_queue *txq;
 	struct snd_queue *sq = &qs->sq[cq_idx];
+	struct rcv_queue *rq = &qs->rq[cq_idx];
 	unsigned int tx_pkts = 0, tx_bytes = 0, txq_idx;
 
 	spin_lock_bh(&cq->lock);
@@ -811,7 +814,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
 
 		switch (cq_desc->cqe_type) {
 		case CQE_TYPE_RX:
-			nicvf_rcv_pkt_handler(netdev, napi, cq_desc, sq);
+			nicvf_rcv_pkt_handler(netdev, napi, cq_desc, sq, rq);
 			work_done++;
 		break;
 		case CQE_TYPE_SEND:
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 095c18aeb8d5..2f2a736ea102 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -760,6 +760,7 @@ static void nicvf_rcv_queue_config(struct nicvf *nic, struct queue_set *qs,
 
 	if (!rq->enable) {
 		nicvf_reclaim_rcv_queue(nic, qs, qidx);
+		xdp_rxq_info_unreg(&rq->xdp_rxq);
 		return;
 	}
 
@@ -772,6 +773,12 @@ static void nicvf_rcv_queue_config(struct nicvf *nic, struct queue_set *qs,
 	/* all writes of RBDR data to be loaded into L2 Cache as well*/
 	rq->caching = 1;
 
+	/* XDP RX-queue info */
+	xdp_rxq_info_init(&rq->xdp_rxq);
+	rq->xdp_rxq.dev = nic->netdev;
+	rq->xdp_rxq.queue_index = qidx;
+	xdp_rxq_info_reg(&rq->xdp_rxq);
+
 	/* Send a mailbox msg to PF to config RQ */
 	mbx.rq.msg = NIC_MBOX_MSG_RQ_CFG;
 	mbx.rq.qs_num = qs->vnic_id;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
index 178ab6e8e3c5..7d1e4e2aaad0 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
@@ -12,6 +12,7 @@
 #include <linux/netdevice.h>
 #include <linux/iommu.h>
 #include <linux/bpf.h>
+#include <net/xdp.h>
 #include "q_struct.h"
 
 #define MAX_QUEUE_SET			128
@@ -255,6 +256,7 @@ struct rcv_queue {
 	u8		start_qs_rbdr_idx; /* RBDR idx in the above QS */
 	u8		caching;
 	struct		rx_tx_queue_stats stats;
+	struct xdp_rxq_info xdp_rxq;
 } ____cacheline_aligned_in_smp;
 
 struct cmp_queue {

^ permalink raw reply related

* [PATCH] IPI performance benchmark
From: Yury Norov @ 2017-12-13 11:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ef696fb0-612c-aee7-319e-eb12f8692ac3@de.ibm.com>

On Mon, Dec 11, 2017 at 05:30:25PM +0100, Christian Borntraeger wrote:
> 
> 
> On 12/11/2017 03:55 PM, Yury Norov wrote:
> > On Mon, Dec 11, 2017 at 03:35:02PM +0100, Christian Borntraeger wrote:
> >>
> >>
> >> On 12/11/2017 03:16 PM, Yury Norov wrote:
> >>> This benchmark sends many IPIs in different modes and measures
> >>> time for IPI delivery (first column), and total time, ie including
> >>> time to acknowledge the receive by sender (second column).
> >>>
> >>> The scenarios are:
> >>> Dry-run:	do everything except actually sending IPI. Useful
> >>> 		to estimate system overhead.
> >>> Self-IPI:	Send IPI to self CPU.
> >>> Normal IPI:	Send IPI to some other CPU.
> >>> Broadcast IPI:	Send broadcast IPI to all online CPUs.
> >>>
> >>> For virtualized guests, sending and reveiving IPIs causes guest exit.
> >>> I used this test to measure performance impact on KVM subsystem of
> >>> Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
> >>>
> >>> https://www.spinics.net/lists/kvm/msg156755.html
> >>>
> >>> Test machine is ThunderX2, 112 online CPUs. Below the results normalized
> >>> to host dry-run time. Smaller - better.
> >>>
> >>> Host, v4.14:
> >>> Dry-run:	  0	    1
> >>> Self-IPI:         9	   18
> >>> Normal IPI:      81	  110
> >>> Broadcast IPI:    0	 2106
> >>>
> >>> Guest, v4.14:
> >>> Dry-run:          0	    1
> >>> Self-IPI:        10	   18
> >>> Normal IPI:     305	  525
> >>> Broadcast IPI:    0    	 9729
> >>>
> >>> Guest, v4.14 + VHE:
> >>> Dry-run:          0	    1
> >>> Self-IPI:         9	   18
> >>> Normal IPI:     176	  343
> >>> Broadcast IPI:    0	 9885
> [...]
> >>> +static int __init init_bench_ipi(void)
> >>> +{
> >>> +	ktime_t ipi, total;
> >>> +	int ret;
> >>> +
> >>> +	ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
> >>> +	if (ret)
> >>> +		pr_err("Dry-run FAILED: %d\n", ret);
> >>> +	else
> >>> +		pr_err("Dry-run:       %18llu, %18llu ns\n", ipi, total);
> >>
> >> you do not use NTIMES here to calculate the average value. Is that intended?
> > 
> > I think, it's more visually to represent all results in number of dry-run
> > times, like I did in patch description. So on kernel side I expose raw data
> > and calculate final values after finishing tests.
> 
> I think it is highly confusing that the output from the patch description does not
> match the output from the real module. So can you make that match at least?

I think so. That's why I noticed that results are normalized to host dry-run
time, even more, they are small and better for human perception.

I was recommended not to public raw data, you'd understand. If this is
the blocker, I can post results from QEMU-hosted kernel.

> > If you think that average values are preferable, I can do that in v2.
> 
> The raw numbers a propably fine, but then you might want to print the number of 
> loop iterations in the output.

It's easy to do. But this number is the same for all tests, and what
really interesting is relative numbers, so I decided not to trash output.
If you insist on printing iterations number, just let me know and I'll add it.

> If we want to do something fancy, we could do a combination of a smaller inner
> loop doing the test, then an outer loops redoing the inner loop and then you 
> can do some min/max/average  calculation. Not s

^ permalink raw reply

* [PATCH] IPI performance benchmark
From: Christian Borntraeger @ 2017-12-13 11:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213112355.s3ubggurwx4v3r53@yury-thinkpad>



On 12/13/2017 12:23 PM, Yury Norov wrote:
> On Mon, Dec 11, 2017 at 05:30:25PM +0100, Christian Borntraeger wrote:
>>
>>
>> On 12/11/2017 03:55 PM, Yury Norov wrote:
>>> On Mon, Dec 11, 2017 at 03:35:02PM +0100, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 12/11/2017 03:16 PM, Yury Norov wrote:
>>>>> This benchmark sends many IPIs in different modes and measures
>>>>> time for IPI delivery (first column), and total time, ie including
>>>>> time to acknowledge the receive by sender (second column).
>>>>>
>>>>> The scenarios are:
>>>>> Dry-run:	do everything except actually sending IPI. Useful
>>>>> 		to estimate system overhead.
>>>>> Self-IPI:	Send IPI to self CPU.
>>>>> Normal IPI:	Send IPI to some other CPU.
>>>>> Broadcast IPI:	Send broadcast IPI to all online CPUs.
>>>>>
>>>>> For virtualized guests, sending and reveiving IPIs causes guest exit.
>>>>> I used this test to measure performance impact on KVM subsystem of
>>>>> Christoffer Dall's series "Optimize KVM/ARM for VHE systems".
>>>>>
>>>>> https://www.spinics.net/lists/kvm/msg156755.html
>>>>>
>>>>> Test machine is ThunderX2, 112 online CPUs. Below the results normalized
>>>>> to host dry-run time. Smaller - better.
>>>>>
>>>>> Host, v4.14:
>>>>> Dry-run:	  0	    1
>>>>> Self-IPI:         9	   18
>>>>> Normal IPI:      81	  110
>>>>> Broadcast IPI:    0	 2106
>>>>>
>>>>> Guest, v4.14:
>>>>> Dry-run:          0	    1
>>>>> Self-IPI:        10	   18
>>>>> Normal IPI:     305	  525
>>>>> Broadcast IPI:    0    	 9729
>>>>>
>>>>> Guest, v4.14 + VHE:
>>>>> Dry-run:          0	    1
>>>>> Self-IPI:         9	   18
>>>>> Normal IPI:     176	  343
>>>>> Broadcast IPI:    0	 9885
>> [...]
>>>>> +static int __init init_bench_ipi(void)
>>>>> +{
>>>>> +	ktime_t ipi, total;
>>>>> +	int ret;
>>>>> +
>>>>> +	ret = bench_ipi(NTIMES, DRY_RUN, &ipi, &total);
>>>>> +	if (ret)
>>>>> +		pr_err("Dry-run FAILED: %d\n", ret);
>>>>> +	else
>>>>> +		pr_err("Dry-run:       %18llu, %18llu ns\n", ipi, total);
>>>>
>>>> you do not use NTIMES here to calculate the average value. Is that intended?
>>>
>>> I think, it's more visually to represent all results in number of dry-run
>>> times, like I did in patch description. So on kernel side I expose raw data
>>> and calculate final values after finishing tests.
>>
>> I think it is highly confusing that the output from the patch description does not
>> match the output from the real module. So can you make that match at least?
> 
> I think so. That's why I noticed that results are normalized to host dry-run
> time, even more, they are small and better for human perception.
> 
> I was recommended not to public raw data, you'd understand. If this is
> the blocker, I can post results from QEMU-hosted kernel.

you could just post some example data from any random x86 laptop. I think it
would just be good to have the patch description output match the real output.

^ permalink raw reply

* [PATCH v4 08/12] clk: qcom: Add KPSS ACC/GCC driver
From: Sricharan R @ 2017-12-13 11:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171212203813.f3fkjw4uvxm43yok@rob-hp-laptop>

Hi Rob,

On 12/13/2017 2:08 AM, Rob Herring wrote:
> On Fri, Dec 08, 2017 at 03:12:26PM +0530, Sricharan R wrote:
>> From: Stephen Boyd <sboyd@codeaurora.org>
>>
>> The ACC and GCC regions present in KPSSv1 contain registers to
>> control clocks and power to each Krait CPU and L2. For CPUfreq
>> purposes probe these devices and expose a mux clock that chooses
>> between PXO and PLL8.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  .../devicetree/bindings/arm/msm/qcom,kpss-acc.txt  |  7 ++
>>  .../devicetree/bindings/arm/msm/qcom,kpss-gcc.txt  | 28 +++++++
> 
> Please make bindings a separate patch.

 ok.

> 
>>  drivers/clk/qcom/Kconfig                           |  8 ++
>>  drivers/clk/qcom/Makefile                          |  1 +
>>  drivers/clk/qcom/kpss-xcc.c                        | 96 ++++++++++++++++++++++
>>  5 files changed, 140 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>>  create mode 100644 drivers/clk/qcom/kpss-xcc.c
>>
>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>> index 1333db9..382a574 100644
>> --- a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-acc.txt
>> @@ -21,10 +21,17 @@ PROPERTIES
>>  		    the register region. An optional second element specifies
>>  		    the base address and size of the alias register region.
>>  
>> +- clock-output-names:
>> +	Usage: optional
>> +	Value type: <string>
>> +	Definition: Name of the output clock. Typically acpuX_aux where X is a
>> +		    CPU number starting at 0.
>> +
>>  Example:
>>  
>>  	clock-controller at 2088000 {
>>  		compatible = "qcom,kpss-acc-v2";
>>  		reg = <0x02088000 0x1000>,
>>  		      <0x02008000 0x1000>;
>> +		clock-output-names = "acpu0_aux";
>>  	};
>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>> new file mode 100644
>> index 0000000..d1e12f1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,kpss-gcc.txt
>> @@ -0,0 +1,28 @@
>> +Krait Processor Sub-system (KPSS) Global Clock Controller (GCC)
>> +
>> +PROPERTIES
>> +
>> +- compatible:
>> +	Usage: required
>> +	Value type: <string>
>> +	Definition: should be one of:
>> +			"qcom,kpss-gcc"
> 
> Only one implementation?

 hmm, missed "qcom,kpss-acc-v1", will add that too.
 
Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

^ permalink raw reply

* [PATCH] clk: imx51: uart4, uart5 gates only exist on imx50, imx53
From: Philipp Zabel @ 2017-12-13 11:57 UTC (permalink / raw)
  To: linux-arm-kernel

i.MX51 only has 3 UARTs and no CCGR7 register. In place of the CCGR7
register on i.MX50/i.MX53 that contains the ipg and per clock gates
for UARTs 4 and 5, on i.MX51 there is the CMEOR register.

Without this patch, the code disabling the UART clocks would also clear
the mod_en_ov_vpu bit in the CMEOR register, among others, which causes
register accesses to the VPU to lock up the system.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/clk/imx/clk-imx51-imx53.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-imx51-imx53.c b/drivers/clk/imx/clk-imx51-imx53.c
index 7bcaf270db117..c864992e6983f 100644
--- a/drivers/clk/imx/clk-imx51-imx53.c
+++ b/drivers/clk/imx/clk-imx51-imx53.c
@@ -257,10 +257,6 @@ static void __init mx5_clocks_common_init(void __iomem *ccm_base)
 	clk[IMX5_CLK_VPU_SEL]		= imx_clk_mux("vpu_sel", MXC_CCM_CBCMR, 14, 2, vpu_sel, ARRAY_SIZE(vpu_sel));
 	clk[IMX5_CLK_VPU_GATE]		= imx_clk_gate2("vpu_gate", "vpu_sel", MXC_CCM_CCGR5, 6);
 	clk[IMX5_CLK_VPU_REFERENCE_GATE] = imx_clk_gate2("vpu_reference_gate", "osc", MXC_CCM_CCGR5, 8);
-	clk[IMX5_CLK_UART4_IPG_GATE]	= imx_clk_gate2("uart4_ipg_gate", "ipg", MXC_CCM_CCGR7, 8);
-	clk[IMX5_CLK_UART4_PER_GATE]	= imx_clk_gate2("uart4_per_gate", "uart_root", MXC_CCM_CCGR7, 10);
-	clk[IMX5_CLK_UART5_IPG_GATE]	= imx_clk_gate2("uart5_ipg_gate", "ipg", MXC_CCM_CCGR7, 12);
-	clk[IMX5_CLK_UART5_PER_GATE]	= imx_clk_gate2("uart5_per_gate", "uart_root", MXC_CCM_CCGR7, 14);
 	clk[IMX5_CLK_GPC_DVFS]		= imx_clk_gate2("gpc_dvfs", "dummy", MXC_CCM_CCGR5, 24);
 
 	clk[IMX5_CLK_SSI_APM]		= imx_clk_mux("ssi_apm", MXC_CCM_CSCMR1, 8, 2, ssi_apm_sels, ARRAY_SIZE(ssi_apm_sels));
@@ -361,6 +357,10 @@ static void __init mx50_clocks_init(struct device_node *np)
 	clk[IMX5_CLK_USB_PHY1_GATE]	= imx_clk_gate2("usb_phy1_gate", "usb_phy_sel", MXC_CCM_CCGR4, 10);
 	clk[IMX5_CLK_USB_PHY2_GATE]	= imx_clk_gate2("usb_phy2_gate", "usb_phy_sel", MXC_CCM_CCGR4, 12);
 	clk[IMX5_CLK_I2C3_GATE]		= imx_clk_gate2("i2c3_gate", "per_root", MXC_CCM_CCGR1, 22);
+	clk[IMX5_CLK_UART4_IPG_GATE]	= imx_clk_gate2("uart4_ipg_gate", "ipg", MXC_CCM_CCGR7, 8);
+	clk[IMX5_CLK_UART4_PER_GATE]	= imx_clk_gate2("uart4_per_gate", "uart_root", MXC_CCM_CCGR7, 10);
+	clk[IMX5_CLK_UART5_IPG_GATE]	= imx_clk_gate2("uart5_ipg_gate", "ipg", MXC_CCM_CCGR7, 12);
+	clk[IMX5_CLK_UART5_PER_GATE]	= imx_clk_gate2("uart5_per_gate", "uart_root", MXC_CCM_CCGR7, 14);
 
 	clk[IMX5_CLK_CKO1_SEL]		= imx_clk_mux("cko1_sel", MXC_CCM_CCOSR, 0, 4,
 						mx53_cko1_sel, ARRAY_SIZE(mx53_cko1_sel));
@@ -562,6 +562,10 @@ static void __init mx53_clocks_init(struct device_node *np)
 	clk[IMX5_CLK_IEEE1588_PRED]	= imx_clk_divider("ieee1588_pred", "ieee1588_sel", MXC_CCM_CSCDR2, 6, 3);
 	clk[IMX5_CLK_IEEE1588_PODF]	= imx_clk_divider("ieee1588_podf", "ieee1588_pred", MXC_CCM_CSCDR2, 0, 6);
 	clk[IMX5_CLK_IEEE1588_GATE]	= imx_clk_gate2("ieee1588_serial_gate", "ieee1588_podf", MXC_CCM_CCGR7, 6);
+	clk[IMX5_CLK_UART4_IPG_GATE]	= imx_clk_gate2("uart4_ipg_gate", "ipg", MXC_CCM_CCGR7, 8);
+	clk[IMX5_CLK_UART4_PER_GATE]	= imx_clk_gate2("uart4_per_gate", "uart_root", MXC_CCM_CCGR7, 10);
+	clk[IMX5_CLK_UART5_IPG_GATE]	= imx_clk_gate2("uart5_ipg_gate", "ipg", MXC_CCM_CCGR7, 12);
+	clk[IMX5_CLK_UART5_PER_GATE]	= imx_clk_gate2("uart5_per_gate", "uart_root", MXC_CCM_CCGR7, 14);
 
 	clk[IMX5_CLK_CKO1_SEL]		= imx_clk_mux("cko1_sel", MXC_CCM_CCOSR, 0, 4,
 						mx53_cko1_sel, ARRAY_SIZE(mx53_cko1_sel));
-- 
2.11.0

^ permalink raw reply related

* [PATCH v11 0/3] iommu/smmu-v3: Workaround for hisilicon 161010801 erratum(reserve HW MSI)
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
  To: linux-arm-kernel

On certain HiSilicon platforms (hip06/hip07) the GIC ITS and PCIe RC
deviates from the standard implementation and this breaks PCIe MSI
functionality when SMMU is enabled.

The HiSilicon erratum 161010801 describes this limitation of certain
HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID
by extending the MSI payload data to 64 bits to include the deviceID.
Hence, the PCIe controller on this platforms has to differentiate the MSI
payload against other DMA payload and has to modify the MSI payload.
This basically makes it difficult for this platforms to have a SMMU
translation for MSI.

This patch implements an ACPI based quirk to reserve the hw msi regions
in the smmu-v3 driver which means these address regions will not be
translated and will be excluded from iova allocations.

To implement this quirk, the following changes are incorporated:
1. Added a generic helper function to IORT code to retrieve and reserve
   the associated ITS base address from a device IORT node. The function
   has a check for smmu model to determine whether the platform requires
   the HW MSI reservation or not.
2. Added smmu node entries and explicitly disabled them in hip06/hip07
    dts files so that users are warned about the non-DT support for this
    erratum.

Changelog:

v10 --> v11
-Addressed comments from Lorenzo(patch#1)
-Added Robin's Reviewed-by to patch #2

v9 --> v10
Addressed comments:
-Moved smmu model check to iort helper function to selectively apply
 the msi reservation which will make the fn call generic from iommu-dma.
-Removed PCI blacklisting patch, instead added smmu nodes(disabled)
 with comments to hip06/hip07 dts file.

v8 --> v9
-Thanks to Marc, fixed IORT helper function to reserve the ITS
 translater region only.
-Removed the DT support for MSI reservation and blacklisted
 HiSilicon PCIe controllers on DT based systems when SMMUv3 is
 enabled.

v7 --> v8
Addressed comments from Rob and Lorenzo:
 -Modified to use DT compatible string for errata.
 -Changed logic to retrieve the msi-parent for DT case.

v6 --> v7
Addressed request from Will to add DT support for the erratum:
 - added bt binding
 - add of_iommu_msi_get_resv_regions()
New arm64 silicon errata entry
Rename iort_iommu_{its->msi}_get_resv_regions

v5 --> v6
Addressed comments from Robin and Lorenzo:
-No change to patch#1 .
-Reverted v5 patch#2 as this might break the platforms where this quirk
  is not applicable. Provided a generic function in iommu code and added
  back the quirk implementation in SMMU v3 driver(patch#3)

v4 --> v5
Addressed comments from Robin and Lorenzo:
-Added a comment to make it clear that, for now, only straightforward
  HW topologies are handled while reserving ITS regions(patch #1).

v3 --> v4
Rebased on 4.13-rc1.
Addressed comments from Robin, Will and Lorenzo:
-As suggested by Robin, moved the ITS msi reservation into
  iommu_dma_get_resv_regions().
-Added its_count != resv region failure case(patch #1).

v2 --> v3
Addressed comments from Lorenzo and Robin:
-Removed dev_is_pci() check in smmuV3 driver.
-Don't treat device not having an ITS mapping as an error in
  iort helper function.

v1 --> v2
-patch 2/2: Invoke iort helper fn based on fwnode type(acpi).

RFCv2 -->PATCH
-Incorporated Lorenzo's review comments.

RFC v1 --> RFC v2
Based on Robin's review comments,
-Removed  the generic erratum framework.
-Using IORT/MADT tables to retrieve the ITS base addr instead
 of vendor specific CSRT table.

Shameer Kolothum (3):
  ACPI/IORT: Add msi address regions reservation helper
  iommu/dma: Add HW MSI(GICv3 ITS) address regions reservation
  arm64:dts:hisilicon Disable hisilicon smmu node on hip06/hip07

 arch/arm64/boot/dts/hisilicon/hip06.dtsi |  55 +++++++++++++++
 arch/arm64/boot/dts/hisilicon/hip07.dtsi |  24 +++++++
 drivers/acpi/arm64/iort.c                | 112 ++++++++++++++++++++++++++++++-
 drivers/iommu/dma-iommu.c                |   8 ++-
 drivers/irqchip/irq-gic-v3-its.c         |   3 +-
 include/linux/acpi_iort.h                |   7 +-
 6 files changed, 203 insertions(+), 6 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH v11 1/3] ACPI/IORT: Add msi address regions reservation helper
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213115830.61872-1-shameerali.kolothum.thodi@huawei.com>

On some platforms msi parent address regions have to be excluded from
normal IOVA allocation in that they are detected and decoded in a HW
specific way by system components and so they cannot be considered normal
IOVA address space.

Add a helper function that retrieves ITS address regions - the msi
parent - through IORT device <-> ITS mappings and reserves it so that
these regions will not be translated by IOMMU and will be excluded from
IOVA allocations. The function checks for the smmu model number and
only applies the msi reservation if the platform requires it.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/acpi/arm64/iort.c        | 112 +++++++++++++++++++++++++++++++++++++--
 drivers/irqchip/irq-gic-v3-its.c |   3 +-
 include/linux/acpi_iort.h        |   7 ++-
 3 files changed, 117 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 95255ec..3e0ce65 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -39,6 +39,7 @@
 struct iort_its_msi_chip {
 	struct list_head	list;
 	struct fwnode_handle	*fw_node;
+	phys_addr_t		base_addr;
 	u32			translation_id;
 };
 
@@ -161,14 +162,16 @@ typedef acpi_status (*iort_find_node_callback)
 static DEFINE_SPINLOCK(iort_msi_chip_lock);
 
 /**
- * iort_register_domain_token() - register domain token and related ITS ID
- * to the list from where we can get it back later on.
+ * iort_register_domain_token() - register domain token along with related
+ * ITS ID and base address to the list from where we can get it back later on.
  * @trans_id: ITS ID.
+ * @base: ITS base address.
  * @fw_node: Domain token.
  *
  * Returns: 0 on success, -ENOMEM if no memory when allocating list element
  */
-int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
+int iort_register_domain_token(int trans_id, phys_addr_t base,
+			       struct fwnode_handle *fw_node)
 {
 	struct iort_its_msi_chip *its_msi_chip;
 
@@ -178,6 +181,7 @@ int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
 
 	its_msi_chip->fw_node = fw_node;
 	its_msi_chip->translation_id = trans_id;
+	its_msi_chip->base_addr = base;
 
 	spin_lock(&iort_msi_chip_lock);
 	list_add(&its_msi_chip->list, &iort_msi_chip_list);
@@ -581,6 +585,24 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
 	return -ENODEV;
 }
 
+static int __maybe_unused iort_find_its_base(u32 its_id, phys_addr_t *base)
+{
+	struct iort_its_msi_chip *its_msi_chip;
+	int ret = -ENODEV;
+
+	spin_lock(&iort_msi_chip_lock);
+	list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
+		if (its_msi_chip->translation_id == its_id) {
+			*base = its_msi_chip->base_addr;
+			ret = 0;
+			break;
+		}
+	}
+	spin_unlock(&iort_msi_chip_lock);
+
+	return ret;
+}
+
 /**
  * iort_dev_find_its_id() - Find the ITS identifier for a device
  * @dev: The device.
@@ -740,6 +762,25 @@ static int __maybe_unused __get_pci_rid(struct pci_dev *pdev, u16 alias,
 	return 0;
 }
 
+static __maybe_unused struct acpi_iort_node *iort_get_msi_resv_iommu(
+						struct device *dev)
+{
+	struct acpi_iort_node *iommu;
+	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+
+	iommu = iort_get_iort_node(fwspec->iommu_fwnode);
+
+	if (iommu && (iommu->type == ACPI_IORT_NODE_SMMU_V3)) {
+		struct acpi_iort_smmu_v3 *smmu;
+
+		smmu = (struct acpi_iort_smmu_v3 *)iommu->node_data;
+		if (smmu->model == ACPI_IORT_SMMU_V3_HISILICON_HI161X)
+			return iommu;
+	}
+
+	return NULL;
+}
+
 static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
 			       struct fwnode_handle *fwnode,
 			       const struct iommu_ops *ops)
@@ -782,6 +823,69 @@ static inline int iort_add_device_replay(const struct iommu_ops *ops,
 
 	return err;
 }
+
+/**
+ * iort_iommu_msi_get_resv_regions - Reserved region driver helper
+ * @dev: Device from iommu_get_resv_regions()
+ * @head: Reserved region list from iommu_get_resv_regions()
+ *
+ * Returns: Number of msi reserved regions on success (0 if platform
+ *          doesn't require the reservation or no associated msi regions),
+ *          appropriate error value otherwise. The ITS interrupt translation
+ *          spaces (ITS_base + SZ_64K, SZ_64K) associated with the device
+ *          are the msi reserved regions.
+ */
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
+{
+	struct acpi_iort_its_group *its;
+	struct acpi_iort_node *iommu_node, *its_node = NULL;
+	int i, resv = 0;
+
+	iommu_node = iort_get_msi_resv_iommu(dev);
+	if (!iommu_node)
+		return 0;
+
+	/*
+	 * Current logic to reserve ITS regions relies on HW topologies
+	 * where a given PCI or named component maps its IDs to only one
+	 * ITS group; if a PCI or named component can map its IDs to
+	 * different ITS groups through IORT mappings this function has
+	 * to be reworked to ensure we reserve regions for all ITS groups
+	 * a given PCI or named component may map IDs to.
+	 */
+
+	for (i = 0; i < dev->iommu_fwspec->num_ids; i++) {
+		its_node = iort_node_map_id(iommu_node,
+					dev->iommu_fwspec->ids[i],
+					NULL, IORT_MSI_TYPE);
+		if (its_node)
+			break;
+	}
+
+	if (!its_node)
+		return 0;
+
+	/* Move to ITS specific data */
+	its = (struct acpi_iort_its_group *)its_node->node_data;
+
+	for (i = 0; i < its->its_count; i++) {
+		phys_addr_t base;
+
+		if (!iort_find_its_base(its->identifiers[i], &base)) {
+			int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+			struct iommu_resv_region *region;
+
+			region = iommu_alloc_resv_region(base + SZ_64K, SZ_64K,
+							 prot, IOMMU_RESV_MSI);
+			if (region) {
+				list_add_tail(&region->list, head);
+				resv++;
+			}
+		}
+	}
+
+	return (resv == its->its_count) ? resv : -ENODEV;
+}
 #else
 static inline const struct iommu_ops *iort_fwspec_iommu_ops(
 				struct iommu_fwspec *fwspec)
@@ -789,6 +893,8 @@ static inline const struct iommu_ops *iort_fwspec_iommu_ops(
 static inline int iort_add_device_replay(const struct iommu_ops *ops,
 					 struct device *dev)
 { return 0; }
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
+{ return 0; }
 #endif
 
 static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4039e64..d4cff12 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3450,7 +3450,8 @@ static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
 		return -ENOMEM;
 	}
 
-	err = iort_register_domain_token(its_entry->translation_id, dom_handle);
+	err = iort_register_domain_token(its_entry->translation_id, res.start,
+					 dom_handle);
 	if (err) {
 		pr_err("ITS@%pa: Unable to register GICv3 ITS domain token (ITS ID %d) to IORT\n",
 		       &res.start, its_entry->translation_id);
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 2f7a292..38cd77b 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -26,7 +26,8 @@
 #define IORT_IRQ_MASK(irq)		(irq & 0xffffffffULL)
 #define IORT_IRQ_TRIGGER_MASK(irq)	((irq >> 32) & 0xffffffffULL)
 
-int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
+int iort_register_domain_token(int trans_id, phys_addr_t base,
+			       struct fwnode_handle *fw_node);
 void iort_deregister_domain_token(int trans_id);
 struct fwnode_handle *iort_find_domain_token(int trans_id);
 #ifdef CONFIG_ACPI_IORT
@@ -38,6 +39,7 @@
 /* IOMMU interface */
 void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
 const struct iommu_ops *iort_iommu_configure(struct device *dev);
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
 #else
 static inline void acpi_iort_init(void) { }
 static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
@@ -52,6 +54,9 @@ static inline void iort_dma_setup(struct device *dev, u64 *dma_addr,
 static inline const struct iommu_ops *iort_iommu_configure(
 				      struct device *dev)
 { return NULL; }
+static inline
+int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
+{ return 0; }
 #endif
 
 #endif /* __ACPI_IORT_H__ */
-- 
1.9.1

^ permalink raw reply related

* [PATCH v11 2/3] iommu/dma: Add HW MSI(GICv3 ITS) address regions reservation
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213115830.61872-1-shameerali.kolothum.thodi@huawei.com>

Modified iommu_dma_get_resv_regions() to include GICv3 ITS
region on ACPI based ARM platfiorms which may require HW MSI
reservations.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/dma-iommu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 25914d3..f05f3cf 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -19,6 +19,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi_iort.h>
 #include <linux/device.h>
 #include <linux/dma-iommu.h>
 #include <linux/gfp.h>
@@ -167,13 +168,18 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
  *
  * IOMMU drivers can use this to implement their .get_resv_regions callback
  * for general non-IOMMU-specific reservations. Currently, this covers host
- * bridge windows for PCI devices.
+ * bridge windows for PCI devices and GICv3 ITS region reservation on ACPI
+ * based ARM platforms that may require HW MSI reservation.
  */
 void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 {
 	struct pci_host_bridge *bridge;
 	struct resource_entry *window;
 
+	if (!is_of_node(dev->iommu_fwspec->iommu_fwnode) &&
+		iort_iommu_msi_get_resv_regions(dev, list) < 0)
+		return;
+
 	if (!dev_is_pci(dev))
 		return;
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH v11 3/3] arm64:dts:hisilicon Disable hisilicon smmu node on hip06/hip07
From: Shameer Kolothum @ 2017-12-13 11:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20171213115830.61872-1-shameerali.kolothum.thodi@huawei.com>

The HiSilicon erratum 161010801 describes the limitation of
HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings
for MSI transactions.

PCIe controller on these platforms has to differentiate the
MSI payload against other DMA payload and has to modify the
MSI  payload. This makes it difficult for these platforms to
have SMMU translation for MSI. In order to workaround this,
ARM SMMUv3 driver requires a quirk to treat the MSI regions
separately. Such a quirk is currently missing for DT based
systems and therefore we need to explicitly disable the
hip06/hip07 smmu entries in dts.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
---
 arch/arm64/boot/dts/hisilicon/hip06.dtsi | 55 ++++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/hisilicon/hip07.dtsi | 24 ++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
index a049b64..d0d5933 100644
--- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
@@ -291,6 +291,13 @@
 			#interrupt-cells = <2>;
 			num-pins = <128>;
 		};
+
+		mbigen_pcie0: intc_pcie0 {
+			msi-parent = <&its_dsa 0x40085>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			num-pins = <10>;
+		};
 	};
 
 	mbigen_dsa at c0080000 {
@@ -312,6 +319,30 @@
 		};
 	};
 
+	/** HiSilicon erratum 161010801: This describes the limitation
+	 *  of HiSilicon platforms hip06/hip07 to support the SMMUv3
+	 *  mappings for PCIe MSI transactions.
+	 *  PCIe controller on these platforms has to differentiate the
+	 *  MSI payload against other DMA payload and has to modify the
+	 *  MSI payload. This makes it difficult for these platforms to
+	 *  have a SMMU translation for MSI. In order to workaround this,
+	 *  ARM SMMUv3 driver requires a quirk to treat the MSI regions
+	 *  separately. Such a quirk is currently missing for DT based
+	 *  systems. Hence please make sure that the smmu pcie node on
+	 *  hip06 is disabled as this will break the PCIe functionality
+	 *  when iommu-map entry is used along with the PCIe node.
+	 *  Refer:https://www.spinics.net/lists/arm-kernel/msg602812.html
+	 */
+	smmu0: smmu_pcie {
+		compatible = "arm,smmu-v3";
+		reg = <0x0 0xa0040000 0x0 0x20000>;
+		#iommu-cells = <1>;
+		dma-coherent;
+		smmu-cb-memtype = <0x0 0x1>;
+		hisilicon,broken-prefetch-cmd;
+		status = "disabled";
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <2>;
@@ -676,6 +707,30 @@
 				     <637 1>,<638 1>,<639 1>;
 			status = "disabled";
 		};
+
+		pcie0: pcie at a0090000 {
+			compatible = "hisilicon,hip06-pcie-ecam";
+			reg = <0 0xb0000000 0 0x2000000>,
+			      <0 0xa0090000 0 0x10000>;
+			bus-range = <0  31>;
+			msi-map = <0x0000 &its_dsa 0x0000 0x2000>;
+			msi-map-mask = <0xffff>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			dma-coherent;
+			ranges = <0x02000000 0 0xb2000000 0x0 0xb2000000 0
+				 0x5ff0000 0x01000000 0 0 0 0xb7ff0000
+				 0 0x10000>;
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0xf800 0 0 7>;
+			interrupt-map = <0x0 0 0 1 &mbigen_pcie0 650 4
+					0x0 0 0 2 &mbigen_pcie0 650 4
+					0x0 0 0 3 &mbigen_pcie0 650 4
+					0x0 0 0 4 &mbigen_pcie0 650 4>;
+			status = "disabled";
+		};
+
 	};
 
 };
diff --git a/arch/arm64/boot/dts/hisilicon/hip07.dtsi b/arch/arm64/boot/dts/hisilicon/hip07.dtsi
index 2c01a21..58fe013 100644
--- a/arch/arm64/boot/dts/hisilicon/hip07.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hip07.dtsi
@@ -1083,6 +1083,30 @@
 		};
 	};
 
+	/** HiSilicon erratum 161010801: This describes the limitation
+	 *  of HiSilicon platforms hip06/hip07 to support the SMMUv3
+	 *  mappings for PCIe MSI transactions.
+	 *  PCIe controller on these platforms has to differentiate the
+	 *  MSI payload against other DMA payload and has to modify the
+	 *  MSI payload. This makes it difficult for these platforms to
+	 *  have a SMMU translation for MSI. In order to workaround this,
+	 *  ARM SMMUv3 driver requires a quirk to treat the MSI regions
+	 *  separately. Such a quirk is currently missing for DT based
+	 *  systems. Hence please make sure that the smmu pcie node on
+	 *  hip06 is disabled as this will break the PCIe functionality
+	 *  when iommu-map entry is used along with the PCIe node.
+	 *  Refer:https://www.spinics.net/lists/arm-kernel/msg602812.html
+	 */
+	smmu0: smmu_pcie {
+		compatible = "arm,smmu-v3";
+		reg = <0x0 0xa0040000 0x0 0x20000>;
+		#iommu-cells = <1>;
+		dma-coherent;
+		smmu-cb-memtype = <0x0 0x1>;
+		hisilicon,broken-prefetch-cmd;
+		status = "disabled";
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <2>;
-- 
1.9.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox