diff mbox series

[v3,04/10] KVM: arm64: vgic-its: Walk the LPI xarray in vgic_copy_lpi_list()

Message ID 20240216184153.2714504-5-oliver.upton@linux.dev
State New
Headers
Series KVM: arm64: Avoid serializing LPI get() / put() |

Commit Message

Oliver Upton Feb. 16, 2024, 6:41 p.m. UTC 
  Start iterating the LPI xarray in anticipation of removing the LPI
linked-list.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-its.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Zenghui Yu Feb. 18, 2024, 8:46 a.m. UTC | #1 
On 2024/2/17 2:41, Oliver Upton wrote:
> Start iterating the LPI xarray in anticipation of removing the LPI
> linked-list.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/vgic/vgic-its.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index fb2d3c356984..9ce2edfadd11 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -335,6 +335,7 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
>  int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  {
>  	struct vgic_dist *dist = &kvm->arch.vgic;
> +	XA_STATE(xas, &dist->lpi_xa, GIC_LPI_OFFSET);
>  	struct vgic_irq *irq;
>  	unsigned long flags;
>  	u32 *intids;
> @@ -353,7 +354,9 @@ int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  		return -ENOMEM;
>  
>  	raw_spin_lock_irqsave(&dist->lpi_list_lock, flags);
> -	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
> +	rcu_read_lock();
> +
> +	xas_for_each(&xas, irq, INTERRUPT_ID_BITS_ITS) {

We should use '1 << INTERRUPT_ID_BITS_ITS - 1' to represent the maximum
LPI interrupt ID.

>  		if (i == irq_count)
>  			break;
>  		/* We don't need to "get" the IRQ, as we hold the list lock. */
> @@ -361,6 +364,8 @@ int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
>  			continue;
>  		intids[i++] = irq->intid;
>  	}
> +
> +	rcu_read_unlock();
>  	raw_spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
>  
>  	*intid_ptr = intids;

Thanks,
Zenghui
Marc Zyngier Feb. 18, 2024, 10:28 a.m. UTC | #2 
On Sun, 18 Feb 2024 08:46:53 +0000,
Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> On 2024/2/17 2:41, Oliver Upton wrote:
> > Start iterating the LPI xarray in anticipation of removing the LPI
> > linked-list.
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  arch/arm64/kvm/vgic/vgic-its.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> > index fb2d3c356984..9ce2edfadd11 100644
> > --- a/arch/arm64/kvm/vgic/vgic-its.c
> > +++ b/arch/arm64/kvm/vgic/vgic-its.c
> > @@ -335,6 +335,7 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
> >  int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
> >  {
> >  	struct vgic_dist *dist = &kvm->arch.vgic;
> > +	XA_STATE(xas, &dist->lpi_xa, GIC_LPI_OFFSET);
> >  	struct vgic_irq *irq;
> >  	unsigned long flags;
> >  	u32 *intids;
> > @@ -353,7 +354,9 @@ int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
> >  		return -ENOMEM;
> >   	raw_spin_lock_irqsave(&dist->lpi_list_lock, flags);
> > -	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
> > +	rcu_read_lock();
> > +
> > +	xas_for_each(&xas, irq, INTERRUPT_ID_BITS_ITS) {
> 
> We should use '1 << INTERRUPT_ID_BITS_ITS - 1' to represent the maximum
> LPI interrupt ID.

Huh, well caught! I'm not even sure how it works, as that's way
smaller than the start of the walk (8192). Probably doesn't.

An alternative would be to use max_lpis_propbaser(), but I'm not sure
we always have a valid PROPBASER value set when we start using this
function. Worth investigating though.

Thanks,

	M.
Oliver Upton Feb. 18, 2024, 6:05 p.m. UTC | #3 
On Sun, Feb 18, 2024 at 10:28:19AM +0000, Marc Zyngier wrote:
> On Sun, 18 Feb 2024 08:46:53 +0000,
> Zenghui Yu <yuzenghui@huawei.com> wrote:
> > 
> > On 2024/2/17 2:41, Oliver Upton wrote:
> > > Start iterating the LPI xarray in anticipation of removing the LPI
> > > linked-list.
> > > 
> > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > > ---
> > >  arch/arm64/kvm/vgic/vgic-its.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> > > index fb2d3c356984..9ce2edfadd11 100644
> > > --- a/arch/arm64/kvm/vgic/vgic-its.c
> > > +++ b/arch/arm64/kvm/vgic/vgic-its.c
> > > @@ -335,6 +335,7 @@ static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
> > >  int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
> > >  {
> > >  	struct vgic_dist *dist = &kvm->arch.vgic;
> > > +	XA_STATE(xas, &dist->lpi_xa, GIC_LPI_OFFSET);
> > >  	struct vgic_irq *irq;
> > >  	unsigned long flags;
> > >  	u32 *intids;
> > > @@ -353,7 +354,9 @@ int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
> > >  		return -ENOMEM;
> > >   	raw_spin_lock_irqsave(&dist->lpi_list_lock, flags);
> > > -	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
> > > +	rcu_read_lock();
> > > +
> > > +	xas_for_each(&xas, irq, INTERRUPT_ID_BITS_ITS) {
> > 
> > We should use '1 << INTERRUPT_ID_BITS_ITS - 1' to represent the maximum
> > LPI interrupt ID.

/facepalm

Thanks Zenghui!

> Huh, well caught! I'm not even sure how it works, as that's way
> smaller than the start of the walk (8192). Probably doesn't.
> 
> An alternative would be to use max_lpis_propbaser(), but I'm not sure
> we always have a valid PROPBASER value set when we start using this
> function. Worth investigating though.

Given the plans to eventually replace this with xarray marks, I'd vote
for doing the lazy thing and deciding this at compile time.

I can squash this in when I apply the series if the rest of it isn't
offensive, otherwise respin with the change.
diff mbox series

Patch

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index fb2d3c356984..9ce2edfadd11 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -335,6 +335,7 @@  static int update_lpi_config(struct kvm *kvm, struct vgic_irq *irq,
 int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
+	XA_STATE(xas, &dist->lpi_xa, GIC_LPI_OFFSET);
 	struct vgic_irq *irq;
 	unsigned long flags;
 	u32 *intids;
@@ -353,7 +354,9 @@  int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
 		return -ENOMEM;
 
 	raw_spin_lock_irqsave(&dist->lpi_list_lock, flags);
-	list_for_each_entry(irq, &dist->lpi_list_head, lpi_list) {
+	rcu_read_lock();
+
+	xas_for_each(&xas, irq, INTERRUPT_ID_BITS_ITS) {
 		if (i == irq_count)
 			break;
 		/* We don't need to "get" the IRQ, as we hold the list lock. */
@@ -361,6 +364,8 @@  int vgic_copy_lpi_list(struct kvm *kvm, struct kvm_vcpu *vcpu, u32 **intid_ptr)
 			continue;
 		intids[i++] = irq->intid;
 	}
+
+	rcu_read_unlock();
 	raw_spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
 
 	*intid_ptr = intids;