| Message ID | 20240216184153.2714504-5-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Avoid serializing LPI get() / put() | expand |
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
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.
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 --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;
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(-)