Re: [RFC PATCH 12/24] ARM: vGICv3: introduce basic ITS emulation bits

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 24/10/16 16:31, Vijay Kilari wrote:
> On Wed, Sep 28, 2016 at 11:54 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> Create a new file to hold the emulation code for the ITS widget.
>> For now we emulate the memory mapped ITS registers and provide a stub
>> to introduce the ITS command handling framework (but without actually
>> emulating any commands at this time).
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  xen/arch/arm/Makefile             |   1 +
>>  xen/arch/arm/vgic-its.c           | 378 ++++++++++++++++++++++++++++++++++++++
>>  xen/arch/arm/vgic-v3.c            |   9 -
>>  xen/include/asm-arm/gic_v3_defs.h |  19 ++
>>  4 files changed, 398 insertions(+), 9 deletions(-)
>>  create mode 100644 xen/arch/arm/vgic-its.c
>>
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index c2c4daa..cb0201f 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -44,6 +44,7 @@ obj-y += traps.o
>>  obj-y += vgic.o
>>  obj-y += vgic-v2.o
>>  obj-$(CONFIG_ARM_64) += vgic-v3.o
>> +obj-$(CONFIG_HAS_ITS) += vgic-its.o
>>  obj-y += vm_event.o
>>  obj-y += vtimer.o
>>  obj-y += vpsci.o
>> diff --git a/xen/arch/arm/vgic-its.c b/xen/arch/arm/vgic-its.c
>> new file mode 100644
>> index 0000000..875b992
>> --- /dev/null
>> +++ b/xen/arch/arm/vgic-its.c
>> @@ -0,0 +1,378 @@
>> +/*
>> + * xen/arch/arm/vgic-its.c
>> + *
>> + * ARM Interrupt Translation Service (ITS) emulation
>> + *
>> + * Andre Przywara <andre.przywara@arm.com>
>> + * Copyright (c) 2016 ARM Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <xen/bitops.h>
>> +#include <xen/config.h>
>> +#include <xen/domain_page.h>
>> +#include <xen/lib.h>
>> +#include <xen/init.h>
>> +#include <xen/softirq.h>
>> +#include <xen/irq.h>
>> +#include <xen/sched.h>
>> +#include <xen/sizes.h>
>> +#include <asm/current.h>
>> +#include <asm/mmio.h>
>> +#include <asm/gic_v3_defs.h>
>> +#include <asm/gic-its.h>
>> +#include <asm/vgic.h>
>> +#include <asm/vgic-emul.h>
>> +
>> +/* Data structure to describe a virtual ITS */
>> +struct virt_its {
>> +    struct domain *d;
>> +    struct host_its *hw_its;
>> +    spinlock_t vcmd_lock;       /* protects the virtual command buffer */
>> +    uint64_t cbaser;
>> +    uint64_t *cmdbuf;
>> +    int cwriter;
>> +    int creadr;
>> +    spinlock_t its_lock;        /* protects the collection and device tables */
>> +    uint64_t baser0, baser1;
>> +    uint16_t *coll_table;
>> +    int max_collections;
>> +    uint64_t *dev_table;
>> +    int max_devices;
>> +    bool enabled;
>> +};
>> +
>> +/* An Interrupt Translation Table Entry: this is indexed by a
>> + * DeviceID/EventID pair and is located in guest memory.
>> + */
>> +struct vits_itte
>> +{
>> +    uint64_t hlpi:24;
>> +    uint64_t vlpi:24;
>> +    uint64_t collection:16;
>> +};
>> +
>> +/**************************************
>> + * Functions that handle ITS commands *
>> + **************************************/
>> +
>> +static uint64_t its_cmd_mask_field(uint64_t *its_cmd,
>> +                                   int word, int shift, int size)
>> +{
>> +    return (le64_to_cpu(its_cmd[word]) >> shift) & (BIT(size) - 1);
>> +}
>> +
>> +#define its_cmd_get_command(cmd)        its_cmd_mask_field(cmd, 0,  0,  8)
>> +#define its_cmd_get_deviceid(cmd)       its_cmd_mask_field(cmd, 0, 32, 32)
>> +#define its_cmd_get_size(cmd)           its_cmd_mask_field(cmd, 1,  0,  5)
>> +#define its_cmd_get_id(cmd)             its_cmd_mask_field(cmd, 1,  0, 32)
>> +#define its_cmd_get_physical_id(cmd)    its_cmd_mask_field(cmd, 1, 32, 32)
>> +#define its_cmd_get_collection(cmd)     its_cmd_mask_field(cmd, 2,  0, 16)
>> +#define its_cmd_get_target_addr(cmd)    its_cmd_mask_field(cmd, 2, 16, 32)
>> +#define its_cmd_get_validbit(cmd)       its_cmd_mask_field(cmd, 2, 63,  1)
>> +
>> +#define ITS_CMD_BUFFER_SIZE(baser)      ((((baser) & 0xff) + 1) << 12)
>> +
>> +static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its,
>> +                                uint32_t writer)
>> +{
>> +    uint64_t *cmdptr;
>> +
>> +    if ( !its->cmdbuf )
>> +        return -1;
>> +
>> +    if ( writer >= ITS_CMD_BUFFER_SIZE(its->cbaser) )
>> +        return -1;
>> +
>> +    spin_lock(&its->vcmd_lock);
>> +
>> +    while ( its->creadr != writer )
>> +    {
>> +        cmdptr = its->cmdbuf + (its->creadr / sizeof(*its->cmdbuf));
>> +        switch (its_cmd_get_command(cmdptr))
>> +        {
>> +        case GITS_CMD_SYNC:
>> +            /* We handle ITS commands synchronously, so we ignore SYNC. */
>> +           break;
>> +        default:
>> +            gdprintk(XENLOG_G_WARNING, "ITS: unhandled ITS command %ld\n",
>> +                   its_cmd_get_command(cmdptr));
>> +            break;
>> +        }
>> +
>> +        its->creadr += ITS_CMD_SIZE;
>> +        if ( its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser) )
>> +            its->creadr = 0;
>> +    }
>> +    its->cwriter = writer;
>> +
>> +    spin_unlock(&its->vcmd_lock);
>> +
>> +    return 0;
>> +}
>> +
>> +/*****************************
>> + * ITS registers read access *
>> + *****************************/
>> +
>> +/* The physical address is encoded slightly differently depending on
>> + * the used page size: the highest four bits are stored in the lowest
>> + * four bits of the field for 64K pages.
>> + */
>> +static paddr_t get_baser_phys_addr(uint64_t reg)
>> +{
>> +    if ( reg & BIT(9) )
>> +        return (reg & GENMASK(47, 16)) | ((reg & GENMASK(15, 12)) << 36);
>> +    else
>> +        return reg & GENMASK(47, 12);
>> +}
>> +
>> +static int vgic_v3_its_mmio_read(struct vcpu *v, mmio_info_t *info,
>> +                                 register_t *r, void *priv)
>> +{
>> +    struct virt_its *its = priv;
>> +
>> +    switch ( info->gpa & 0xffff )
>> +    {
>> +    case VREG32(GITS_CTLR):
>> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg32_extract(its->enabled | BIT(31), info);
>> +       break;
>> +    case VREG32(GITS_IIDR):
>> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg32_extract(GITS_IIDR_VALUE, info);
>> +        break;
>> +    case VREG64(GITS_TYPER):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(0x1eff1, info);
>        GITS_TYPER.HCC is not set. Should be max vcpus of the domain

HCC is clear on purpose. We want the guest to provide memory for
everything that it allocates, to avoid it to hog Xen with allocations.

>        GITS_TYPER.ID_bits are also just set to 15.

Yeah, I guess it should match what the hardware ITS provides, to not
impose an artificial limit here.

>> +        break;
>> +    case VREG64(GITS_CBASER):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(its->cbaser, info);
>> +        break;
>> +    case VREG64(GITS_CWRITER):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(its->cwriter, info);
>> +        break;
>> +    case VREG64(GITS_CREADR):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(its->creadr, info);
>> +        break;
>> +    case VREG64(GITS_BASER0):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(its->baser0, info);
>> +        break;
>> +    case VREG64(GITS_BASER1):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(its->baser1, info);
>> +        break;
>> +    case VRANGE64(GITS_BASER2, GITS_BASER7):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg64_extract(0, info);
>> +        break;
>> +    case VREG32(GICD_PIDR2):
>> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
>> +        *r = vgic_reg32_extract(GICV3_GICD_PIDR2, info);
>> +        break;
>          missing default
>> +    }
>> +
>> +    return 1;
>> +
>> +bad_width:
>     print would be helpful

Yes.

>> +    domain_crash_synchronous();
>> +
>> +    return 0;
>> +}
>> +
>> +/******************************
>> + * ITS registers write access *
>> + ******************************/
>> +
>> +static int its_baser_table_size(uint64_t baser)
>> +{
>> +    int page_size = 0;
>> +
>> +    switch ( (baser >> 8) & 3 )
>> +    {
>> +    case 0: page_size = SZ_4K; break;
>> +    case 1: page_size = SZ_16K; break;
>> +    case 2:
>> +    case 3: page_size = SZ_64K; break;
>> +    }
>> +
>> +    return page_size * ((baser & GENMASK(7, 0)) + 1);
>> +}
>> +
>> +static int its_baser_nr_entries(uint64_t baser)
>> +{
>> +    int entry_size = ((baser & GENMASK(52, 48)) >> 48) + 1;
>> +
>> +    return its_baser_table_size(baser) / entry_size;
>> +}
>> +
>> +static int vgic_v3_its_mmio_write(struct vcpu *v, mmio_info_t *info,
>> +                                  register_t r, void *priv)
>> +{
>> +    struct domain *d = v->domain;
>> +    struct virt_its *its = priv;
>> +    uint64_t reg;
>> +    uint32_t ctlr;
>> +
>> +    switch ( info->gpa & 0xffff )
>> +    {
>> +    case VREG32(GITS_CTLR):
>> +        ctlr = its->enabled ? GITS_CTLR_ENABLE : 0;
>> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
>> +       vgic_reg32_update(&ctlr, r, info);
>> +       its->enabled = ctlr & GITS_CTLR_ENABLE;
>> +       /* TODO: trigger something ... */
>> +        return 1;
>> +    case VREG32(GITS_IIDR):
>> +        goto write_ignore_32;
>> +    case VREG32(GITS_TYPER):
>> +        goto write_ignore_32;
>> +    case VREG64(GITS_CBASER):
>> +        if ( info->dabt.size < DABT_WORD ) goto bad_width;
>> +
>> +        /* Changing base registers with the ITS enabled is UNPREDICTABLE. */
>> +        if ( its->enabled )
>> +            return 1;
>> +
>> +        reg = its->cbaser;
>> +        vgic_reg64_update(&reg, r, info);
>> +        /* TODO: sanitise! */
>> +        its->cbaser = reg;
>> +
>> +        if ( reg & BIT(63) )
>> +        {
>> +            its->cmdbuf = map_guest_pages(d, reg & GENMASK(51, 12), 1);
> 
>        Only one page of guest cmd queue is mapped. After cwriter
> moving beyond 1 page,
> panic is observed. Map all the guest pages

Right, good catch.

Cheers,
Andre.

>> +        }
>> +        else
>> +        {
>> +            unmap_guest_pages(its->cmdbuf, 1);
>    Same here.
>> +            its->cmdbuf = NULL;
>> +        }
>> +
>> +       return 1;

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