|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 08/27] ARM: GICv3 ITS: introduce host LPI array
On Thu, 23 Mar 2017, Andre Przywara wrote:
> Hi,
>
> On 22/03/17 23:38, Stefano Stabellini wrote:
> > On Thu, 16 Mar 2017, Andre Przywara wrote:
> >> The number of LPIs on a host can be potentially huge (millions),
> >> although in practise will be mostly reasonable. So prematurely allocating
> >> an array of struct irq_desc's for each LPI is not an option.
> >> However Xen itself does not care about LPIs, as every LPI will be injected
> >> into a guest (Dom0 for now).
> >> Create a dense data structure (8 Bytes) for each LPI which holds just
> >> enough information to determine the virtual IRQ number and the VCPU into
> >> which the LPI needs to be injected.
> >> Also to not artificially limit the number of LPIs, we create a 2-level
> >> table for holding those structures.
> >> This patch introduces functions to initialize these tables and to
> >> create, lookup and destroy entries for a given LPI.
> >> By using the naturally atomic access guarantee the native uint64_t data
> >> type gives us, we allocate and access LPI information in a way that does
> >> not require a lock.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> >> ---
> >> xen/arch/arm/gic-v3-its.c | 90 ++++++++++++++++++-
> >> xen/arch/arm/gic-v3-lpi.c | 188
> >> +++++++++++++++++++++++++++++++++++++++
> >> xen/include/asm-arm/gic.h | 5 ++
> >> xen/include/asm-arm/gic_v3_its.h | 11 +++
> >> 4 files changed, 292 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> >> index 60b15b5..ed14d95 100644
> >> --- a/xen/arch/arm/gic-v3-its.c
> >> +++ b/xen/arch/arm/gic-v3-its.c
> >> @@ -148,6 +148,20 @@ static int its_send_cmd_sync(struct host_its *its,
> >> unsigned int cpu)
> >> return its_send_command(its, cmd);
> >> }
> >>
> >> +static int its_send_cmd_mapti(struct host_its *its,
> >> + uint32_t deviceid, uint32_t eventid,
> >> + uint32_t pintid, uint16_t icid)
> >> +{
> >> + uint64_t cmd[4];
> >> +
> >> + cmd[0] = GITS_CMD_MAPTI | ((uint64_t)deviceid << 32);
> >> + cmd[1] = eventid | ((uint64_t)pintid << 32);
> >> + cmd[2] = icid;
> >> + cmd[3] = 0x00;
> >> +
> >> + return its_send_command(its, cmd);
> >> +}
> >> +
> >> static int its_send_cmd_mapc(struct host_its *its, uint32_t collection_id,
> >> unsigned int cpu)
> >> {
> >> @@ -180,6 +194,19 @@ static int its_send_cmd_mapd(struct host_its *its,
> >> uint32_t deviceid,
> >> return its_send_command(its, cmd);
> >> }
> >>
> >> +static int its_send_cmd_inv(struct host_its *its,
> >> + uint32_t deviceid, uint32_t eventid)
> >> +{
> >> + uint64_t cmd[4];
> >> +
> >> + cmd[0] = GITS_CMD_INV | ((uint64_t)deviceid << 32);
> >> + cmd[1] = eventid;
> >> + cmd[2] = 0x00;
> >> + cmd[3] = 0x00;
> >> +
> >> + return its_send_command(its, cmd);
> >> +}
> >> +
> >> /* Set up the (1:1) collection mapping for the given host CPU. */
> >> int gicv3_its_setup_collection(unsigned int cpu)
> >> {
> >> @@ -462,7 +489,7 @@ int gicv3_its_init(void)
> >>
> >> static int remove_mapped_guest_device(struct its_devices *dev)
> >> {
> >> - int ret;
> >> + int ret, i;
> >>
> >> if ( dev->hw_its )
> >> {
> >> @@ -471,11 +498,19 @@ static int remove_mapped_guest_device(struct
> >> its_devices *dev)
> >> return ret;
> >> }
> >>
> >> + /*
> >> + * The only error the function below would return is -ENOENT, in which
> >> + * case there is nothing to free here. So we just ignore it.
> >> + */
> >> + for ( i = 0; i < DIV_ROUND_UP(dev->eventids, LPI_BLOCK); i++ )
> >> + gicv3_free_host_lpi_block(dev->hw_its, dev->host_lpis[i]);
> >> +
> >> ret = gicv3_its_wait_commands(dev->hw_its);
> >> if ( ret )
> >> return ret;
> >>
> >> xfree(dev->itt_addr);
> >> + xfree(dev->host_lpis);
> >> xfree(dev);
> >>
> >> return 0;
> >> @@ -513,6 +548,36 @@ static int compare_its_guest_devices(struct
> >> its_devices *dev,
> >> }
> >>
> >> /*
> >> + * On the host ITS @its, map @nr_events consecutive LPIs.
> >> + * The mapping connects a device @devid and event @eventid pair to LPI
> >> @lpi,
> >> + * increasing both @eventid and @lpi to cover the number of requested
> >> LPIs.
> >> + */
> >> +int gicv3_its_map_host_events(struct host_its *its,
> >> + uint32_t devid, uint32_t eventid, uint32_t
> >> lpi,
> >> + uint32_t nr_events)
> >> +{
> >> + uint32_t i;
> >> + int ret;
> >> +
> >> + for ( i = 0; i < nr_events; i++ )
> >> + {
> >> + /* For now we map every host LPI to host CPU 0 */
> >> + ret = its_send_cmd_mapti(its, devid, eventid + i, lpi + i, 0);
> >> + if ( ret )
> >> + return ret;
> >> + ret = its_send_cmd_inv(its, devid, eventid + i);
> >> + if ( ret )
> >> + return ret;
> >> + }
> >> +
> >> + ret = its_send_cmd_sync(its, 0);
> >> + if ( ret )
> >> + return ret;
> >> +
> >> + return gicv3_its_wait_commands(its);
> >> +}
> >> +
> >> +/*
> >> * Map a hardware device, identified by a certain host ITS and its device
> >> ID
> >> * to domain d, a guest ITS (identified by its doorbell address) and
> >> device ID.
> >> * Also provide the number of events (MSIs) needed for that device.
> >> @@ -528,7 +593,7 @@ int gicv3_its_map_guest_device(struct domain *d,
> >> struct host_its *hw_its;
> >> struct its_devices *dev = NULL, *temp;
> >> struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent =
> >> NULL;
> >> - int ret = -ENOENT;
> >> + int ret = -ENOENT, i;
> >>
> >> hw_its = gicv3_its_find_by_doorbell(host_doorbell);
> >> if ( !hw_its )
> >> @@ -574,6 +639,11 @@ int gicv3_its_map_guest_device(struct domain *d,
> >> if ( !dev )
> >> goto out_unlock;
> >>
> >> + dev->host_lpis = xzalloc_array(uint32_t,
> >> + DIV_ROUND_UP(nr_events, LPI_BLOCK));
> >> + if ( !dev->host_lpis )
> >> + goto out_unlock;
> >
> > The expectation is that we are going to allocate far more than 32 lpis
> > for one device, possibly thousands, right?
>
> Mmh, what makes you think so?
> My understanding is that you have one MSI per interrupt "reason" (TX,
> RX, error, for instance). And even if you have multiple queues, each
> connected to separate MSIs, that won't go into the thousands. MSI-X for
> instance is *limited* to 2048 MSIs.
>
> But in reality I dont' see nearly that many MSIs, mostly less than a
> dozen per device. And since 32 is the old PCI MSI limit, I thought this
> would be a good starting point. So most of the time we just need one block.
>
> I believe this could add up with virtual functions, but those are
> separate devices, right?
Yes, you are right about that, but even 2048 can make a difference when
taken 32 at a time: an array of 64 entries vs. 1 entry (if the
allocation is fully contiguous), and that for each device, in the case
of SR-IOV they can be hundreds.
On Thu, 23 Mar 2017, Julien Grall wrote:
> Not answering directly to the question. But I think this is an improvement and
> not necessary for a first version.
>
> I would like to see this series merged in Xen 4.9 as a tech preview, so I
> would suggest to gather this list of improvement (maybe on Jira?) and defer
> them for Xen 4.10. They would need to be addressed before making ITS stable.
> Stefano, does it sound good to you?
Yes, I am OK with not having this in 4.9, but we need to keep track
(Jira or otherwise) of these things.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |