[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH 21/24] ARM: vITS: handle INVALL command
On Fri, 4 Nov 2016, Andre Przywara wrote: > Hi, > > On 24/10/16 16:32, Vijay Kilari wrote: > > On Wed, Sep 28, 2016 at 11:54 PM, Andre Przywara <andre.przywara@xxxxxxx> > > wrote: > >> The INVALL command instructs an ITS to invalidate the configuration > >> data for all LPIs associated with a given redistributor (read: VCPU). > >> To avoid iterating (and mapping!) all guest tables, we instead go through > >> the host LPI table to find any LPIs targetting this VCPU. We then update > >> the configuration bits for the connected virtual LPIs. > >> > >> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx> > >> --- > >> xen/arch/arm/gic-its.c | 58 > >> +++++++++++++++++++++++++++++++++++++++++++ > >> xen/arch/arm/vgic-its.c | 30 ++++++++++++++++++++++ > >> xen/include/asm-arm/gic-its.h | 2 ++ > >> 3 files changed, 90 insertions(+) > >> > >> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > >> index 6f4329f..5129d6e 100644 > >> --- a/xen/arch/arm/gic-its.c > >> +++ b/xen/arch/arm/gic-its.c > >> @@ -228,6 +228,18 @@ static int its_send_cmd_inv(struct host_its *its, > >> return its_send_command(its, cmd); > >> } > >> > >> +static int its_send_cmd_invall(struct host_its *its, int cpu) > >> +{ > >> + uint64_t cmd[4]; > >> + > >> + cmd[0] = GITS_CMD_INVALL; > >> + cmd[1] = 0x00; > >> + cmd[2] = cpu & GENMASK(15, 0); > >> + cmd[3] = 0x00; > >> + > >> + return its_send_command(its, cmd); > >> +} > >> + > >> int gicv3_its_map_device(struct host_its *hw_its, struct domain *d, > >> int devid, int bits, bool valid) > >> { > >> @@ -668,6 +680,52 @@ uint32_t gicv3_lpi_lookup_lpi(struct domain *d, > >> uint32_t host_lpi, int *vcpu_id) > >> return hlpi.virt_lpi; > >> } > >> > >> +/* Iterate over all host LPIs, and updating the "enabled" state for a > >> given > >> + * guest redistributor (VCPU) given the respective state in the provided > >> + * proptable. This proptable is indexed by the stored virtual LPI number. > >> + * This is to implement a guest INVALL command. > >> + */ > >> +void gicv3_lpi_update_configurations(struct vcpu *v, uint8_t *proptable) > >> +{ > >> + int chunk, i; > >> + struct host_its *its; > >> + > >> + for (chunk = 0; chunk < MAX_HOST_LPIS / HOST_LPIS_PER_PAGE; chunk++) > >> + { > >> + if ( !lpi_data.host_lpis[chunk] ) > >> + continue; > >> + > >> + for (i = 0; i < HOST_LPIS_PER_PAGE; i++) > >> + { > >> + union host_lpi *hlpip = &lpi_data.host_lpis[chunk][i], hlpi; > >> + uint32_t hlpi_nr; > >> + > >> + hlpi.data = hlpip->data; > >> + if ( !hlpi.virt_lpi ) > >> + continue; > >> + > >> + if ( hlpi.dom_id != v->domain->domain_id ) > >> + continue; > >> + > >> + if ( hlpi.vcpu_id != v->vcpu_id ) > >> + continue; > >> + > >> + hlpi_nr = chunk * HOST_LPIS_PER_PAGE + i; > >> + > >> + if ( proptable[hlpi.virt_lpi] & LPI_PROP_ENABLED ) > >> + lpi_data.lpi_property[hlpi_nr - 8192] |= LPI_PROP_ENABLED; > >> + else > >> + lpi_data.lpi_property[hlpi_nr - 8192] &= > >> ~LPI_PROP_ENABLED; > >> + } > >> + } > > AFAIK, the initial design is to use tasklet to update property > > table as it consumes > > lot of time to update the table. > > This is a possible, but premature optimization. > Linux (at the moment, at least) only calls INVALL _once_, just after > initialising the collections. And at this point no LPI is mapped, so the > whole routine does basically nothing - and that quite fast. > We can later have any kind of fancy algorithm if there is a need for. I understand, but as-is it's so expensive that could be a DOS vector. Also other OSes could issue INVALL much more often than Linux. Considering that we might support device assigment with ITS soon, I think it might be best to parse per-domain virtual tables rather than the full list of physical LPIs, which theoretically could be much larger. Or alternatively we need to think about adding another field to lpi_data, to link together all lpis assigned to the same domain, but that would cost even more memory. Or we could rate-limit the INVALL calls to one every few seconds or something. Or all of the above :-) We need to protect Xen from too frequent and too expensive requests like this. > >> + > >> + /* Tell all ITSes that they should update the property table for CPU > >> 0, > >> + * which is where we map all LPIs to. > >> + */ > >> + list_for_each_entry(its, &host_its_list, entry) > >> + its_send_cmd_invall(its, 0); > >> +} > >> + > >> void gicv3_lpi_set_enable(struct host_its *its, > >> uint32_t deviceid, uint32_t eventid, > >> uint32_t host_lpi, bool enabled) > >> diff --git a/xen/arch/arm/vgic-its.c b/xen/arch/arm/vgic-its.c > >> index 74da8fc..1e429b7 100644 > >> --- a/xen/arch/arm/vgic-its.c > >> +++ b/xen/arch/arm/vgic-its.c > >> @@ -294,6 +294,33 @@ out_unlock: > >> return ret; > >> } > >> > >> +/* INVALL updates the per-LPI configuration status for every LPI mapped to > >> + * this redistributor. For the guest side we don't need to update > >> anything, > >> + * as we always refer to the actual table for the enabled bit and the > >> + * priority. > >> + * Enabling or disabling a virtual LPI however needs to be propagated to > >> + * the respective host LPI. Instead of iterating over all mapped LPIs in > >> our > >> + * emulated GIC (which is expensive due to the required on-demand > >> mapping), > >> + * we iterate over all mapped _host_ LPIs and filter for those which are > >> + * forwarded to this virtual redistributor. > >> + */ > >> +static int its_handle_invall(struct virt_its *its, uint64_t *cmdptr) > >> +{ > >> + uint32_t collid = its_cmd_get_collection(cmdptr); > >> + struct vcpu *vcpu; > >> + > >> + spin_lock(&its->its_lock); > >> + vcpu = get_vcpu_from_collection(its, collid); > >> + spin_unlock(&its->its_lock); > >> + > >> + if ( !vcpu ) > >> + return -1; > >> + > >> + gicv3_lpi_update_configurations(vcpu, its->d->arch.vgic.proptable); > >> + > >> + return 0; > >> +} > >> + > >> static int its_handle_mapc(struct virt_its *its, uint64_t *cmdptr) > >> { > >> uint32_t collid = its_cmd_get_collection(cmdptr); > >> @@ -515,6 +542,9 @@ static int vgic_its_handle_cmds(struct domain *d, > >> struct virt_its *its, > >> case GITS_CMD_INV: > >> its_handle_inv(its, cmdptr); > >> break; > >> + case GITS_CMD_INVALL: > >> + its_handle_invall(its, cmdptr); > >> + break; > >> case GITS_CMD_MAPC: > >> its_handle_mapc(its, cmdptr); > >> break; > >> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h > >> index 2cdb3e1..ba6b2d5 100644 > >> --- a/xen/include/asm-arm/gic-its.h > >> +++ b/xen/include/asm-arm/gic-its.h > >> @@ -146,6 +146,8 @@ int gicv3_lpi_drop_host_lpi(struct host_its *its, > >> uint32_t devid, uint32_t eventid, > >> uint32_t host_lpi); > >> > >> +void gicv3_lpi_update_configurations(struct vcpu *v, uint8_t *proptable); > >> + > >> static inline int gicv3_lpi_get_priority(struct domain *d, uint32_t lpi) > >> { > >> return d->arch.vgic.proptable[lpi - 8192] & 0xfc; > >> -- > >> 2.9.0 > >> > >> > >> _______________________________________________ > >> Xen-devel mailing list > >> Xen-devel@xxxxxxxxxxxxx > >> https://lists.xen.org/xen-devel > > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |