| Message ID | 20170403202829.7278-25-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 03/04/17 21:28, Andre Przywara wrote: > The INVALL command instructs an ITS to invalidate the configuration > data for all LPIs associated with a given redistributor (read: VCPU). > This is nasty to emulate exactly with our architecture, so we just scan > the pending table and inject _every_ LPI found there that got enabled. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/vgic-v3-its.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c > index 920c437..35a0730 100644 > --- a/xen/arch/arm/vgic-v3-its.c > +++ b/xen/arch/arm/vgic-v3-its.c > @@ -425,6 +425,49 @@ static int its_handle_inv(struct virt_its *its, uint64_t *cmdptr) > return 0; > } > > +/* > + * INVALL updates the per-LPI configuration status for every LPI mapped to > + * a particular redistributor. > + * We iterate over all mapped LPIs in our radix tree and update those. > + */ > +static int its_handle_invall(struct virt_its *its, uint64_t *cmdptr) > +{ > + uint32_t collid = its_cmd_get_collection(cmdptr); > + struct vcpu *vcpu; > + struct pending_irq *pirqs[16]; > + uint32_t vlpi = 0; > + int nr_lpis, i; Both nr_lpis and i should be unsigned int. > + > + /* We may want to revisit this implementation for DomUs. */ Please give a bit more details on what needs to be done. > + ASSERT(is_hardware_domain(its->d)); > + > + spin_lock(&its->its_lock); > + vcpu = get_vcpu_from_collection(its, collid); > + spin_unlock(&its->its_lock); > + > + read_lock(&its->d->arch.vgic.pend_lpi_tree_lock); > + > + do { do { > + nr_lpis = radix_tree_gang_lookup(&its->d->arch.vgic.pend_lpi_tree, > + (void **)pirqs, vlpi, > + ARRAY_SIZE(pirqs)); The 2 lines above are using hard tab. Please replace by soft tabs. > + > + for ( i = 0; i < nr_lpis; i++ ) > + { > + vlpi = pirqs[i]->irq; > + update_lpi_enabled_status(its, vcpu, vlpi); Don't you need to only invalidate on the current collection? > + } > + > + /* Protect from overflow when incrementing 0xffffffff */ > + if ( vlpi == ~0 || ++vlpi < its->d->arch.vgic.nr_lpis ) > + break; Can't we just move vlpi to uint64_t? > + } while ( nr_lpis == ARRAY_SIZE(pirqs)); Coding style while ( ... ); Also this code is not obvious to read. I don't understand why until "nr_lpis == ARRAY_SIZE(....)". Can you explain it? > + > + read_unlock(&its->d->arch.vgic.pend_lpi_tree_lock); > + > + return 0; > +} > + > static int its_handle_mapc(struct virt_its *its, uint64_t *cmdptr) > { > uint32_t collid = its_cmd_get_collection(cmdptr); > @@ -608,6 +651,9 @@ static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its, > case GITS_CMD_INV: > ret = its_handle_inv(its, cmdptr); > break; > + case GITS_CMD_INVALL: > + ret = its_handle_invall(its, cmdptr); > + break; > case GITS_CMD_MAPC: > ret = its_handle_mapc(its, cmdptr); > break; > Cheers,
diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c index 920c437..35a0730 100644 --- a/xen/arch/arm/vgic-v3-its.c +++ b/xen/arch/arm/vgic-v3-its.c @@ -425,6 +425,49 @@ static int its_handle_inv(struct virt_its *its, uint64_t *cmdptr) return 0; } +/* + * INVALL updates the per-LPI configuration status for every LPI mapped to + * a particular redistributor. + * We iterate over all mapped LPIs in our radix tree and update those. + */ +static int its_handle_invall(struct virt_its *its, uint64_t *cmdptr) +{ + uint32_t collid = its_cmd_get_collection(cmdptr); + struct vcpu *vcpu; + struct pending_irq *pirqs[16]; + uint32_t vlpi = 0; + int nr_lpis, i; + + /* We may want to revisit this implementation for DomUs. */ + ASSERT(is_hardware_domain(its->d)); + + spin_lock(&its->its_lock); + vcpu = get_vcpu_from_collection(its, collid); + spin_unlock(&its->its_lock); + + read_lock(&its->d->arch.vgic.pend_lpi_tree_lock); + + do { + nr_lpis = radix_tree_gang_lookup(&its->d->arch.vgic.pend_lpi_tree, + (void **)pirqs, vlpi, + ARRAY_SIZE(pirqs)); + + for ( i = 0; i < nr_lpis; i++ ) + { + vlpi = pirqs[i]->irq; + update_lpi_enabled_status(its, vcpu, vlpi); + } + + /* Protect from overflow when incrementing 0xffffffff */ + if ( vlpi == ~0 || ++vlpi < its->d->arch.vgic.nr_lpis ) + break; + } while ( nr_lpis == ARRAY_SIZE(pirqs)); + + read_unlock(&its->d->arch.vgic.pend_lpi_tree_lock); + + return 0; +} + static int its_handle_mapc(struct virt_its *its, uint64_t *cmdptr) { uint32_t collid = its_cmd_get_collection(cmdptr); @@ -608,6 +651,9 @@ static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its, case GITS_CMD_INV: ret = its_handle_inv(its, cmdptr); break; + case GITS_CMD_INVALL: + ret = its_handle_invall(its, cmdptr); + break; case GITS_CMD_MAPC: ret = its_handle_mapc(its, cmdptr); break;
The INVALL command instructs an ITS to invalidate the configuration data for all LPIs associated with a given redistributor (read: VCPU). This is nasty to emulate exactly with our architecture, so we just scan the pending table and inject _every_ LPI found there that got enabled. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/vgic-v3-its.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)