[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH 21/24] ARM: vITS: handle INVALL command



Hi,

On 10/11/16 00:21, Stefano Stabellini wrote:
On Fri, 4 Nov 2016, Andre Przywara wrote:
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 :-)

It is not necessary for an ITS implementation to wait until an INVALL/INV command is issued to take into account the change of the LPI configuration tables (aka property table in this thread).

So how about trapping the property table? We would still have to go through the property table the first time (i.e when writing into the GICR_PROPBASER), but INVALL would be a nop.

The idea would be unmapping the region when GICR_PROPBASER is written. So any read/write access would be trapped. For a write access, Xen will update the LPIs internal data structures and write the value in the guest page unmapped. If we don't want to have an overhead for the read access, we could just write-protect the page in stage-2 page table. So only write access would be trapped.

Going further, for the ITS, Xen is using the guest memory to store the ITS information. This means Xen has to validate the information at every access. So how about restricting the access in stage-2 page table? That would remove the overhead of validating data.

Any thoughts?

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.