Re: [Xen-devel] [PATCH v4 14/27] ARM: vGICv3: introduce basic ITS emulation bits

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 03/04/17 21:28, Andre Przywara 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).
The ITS is a complex piece so I think it would be good to describe more 
in the commit message how this will work. Also a documentation in the 
tree would be very good to help understanding the code.
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
>  xen/arch/arm/Makefile             |   1 +
>  xen/arch/arm/vgic-v3-its.c        | 547 ++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic-v3.c            |   9 -
>  xen/include/asm-arm/gic_v3_defs.h |  19 ++
>  xen/include/asm-arm/gic_v3_its.h  |   2 +
>  5 files changed, 569 insertions(+), 9 deletions(-)
>  create mode 100644 xen/arch/arm/vgic-v3-its.c
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 6be85ab..49e1fb2 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -47,6 +47,7 @@ obj-y += traps.o
>  obj-y += vgic.o
>  obj-y += vgic-v2.o
>  obj-$(CONFIG_HAS_GICV3) += vgic-v3.o
> +obj-$(CONFIG_HAS_ITS) += vgic-v3-its.o
>  obj-y += vm_event.o
>  obj-y += vtimer.o
>  obj-y += vpsci.o
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> new file mode 100644
> index 0000000..fd3b9a1
> --- /dev/null
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -0,0 +1,547 @@
> +/*
> + * xen/arch/arm/vgic-v3-its.c
> + *
> + * ARM Interrupt Translation Service (ITS) emulation
> + *
> + * Andre Przywara <andre.przywara@xxxxxxx>
> + * Copyright (c) 2016,2017 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; under version 2 of the License.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#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_v3_its.h>
> +#include <asm/vgic.h>
> +#include <asm/vgic-emul.h>
> +
> +/* Data structure to describe a virtual ITS */
> +#define VIRT_ITS_ENABLED        0
> +#define VIRT_ITS_COLL_VALID     1
> +#define VIRT_ITS_DEV_VALID      2
> +#define VIRT_ITS_CMDBUF_VALID   3
> +struct virt_its {
> +    struct domain *d;
> +    spinlock_t vcmd_lock;       /* Protects the virtual command buffer. */
> +    uint64_t cbaser;
> +    uint64_t cwriter;
> +    uint64_t creadr;
> +    spinlock_t its_lock;        /* Protects the collection and device tables. 
> */
> +    uint64_t baser_dev, baser_coll;
Well, you renamed the baser{0,1} to a more accurate name. But now, you 
are missing document explaining which one is BASER0 and BASER1.
> +    unsigned int max_collections;
> +    unsigned int max_devices;
> +    unsigned int devid_bits;
> +    unsigned int intid_bits;
> +    unsigned long flags;
> +};
> +
> +/*
> + * An Interrupt Translation Table Entry: this is indexed by a
> + * DeviceID/EventID pair and is located in guest memory.
> + */
> +struct vits_itte
> +{
> +    uint32_t vlpi;
> +    uint16_t collection;
> +    uint16_t pad;
> +};
> +
> +static bool its_is_enabled(struct virt_its *its)
> +{
> +    return test_bit(VIRT_ITS_ENABLED, &its->flags);
> +}
> +
> +/**************************************
> + * Functions that handle ITS commands *
> + **************************************/
> +
> +static uint64_t its_cmd_mask_field(uint64_t *its_cmd, unsigned int word,
> +                                   unsigned int shift, unsigned 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)
s/uint32_t/uint64_t/

> +{
> +    paddr_t cmdbuf_addr = its->cbaser & GENMASK_ULL(51, 12);
> +    void *cmdbuf = NULL;
> +    uint64_t *cmdptr;
> +
> +    if ( writer >= ITS_CMD_BUFFER_SIZE(its->cbaser) )
> +        return -1;
You return an error value but the caller does not check it. Should not 
the caller do a different action when the return -1? If not, it should 
be documented.
> +
> +    spin_lock(&its->vcmd_lock);
I am still concerned about the locking here as you may interpret 32K of 
commands in one go. We spoke about mitigation and I was expected some 
TODOs in the code about that...
> +
> +    while ( its->creadr != writer )
> +    {
> +        int ret;
> +
> +        ret = 0;
> +
> +        /*
> +         * If this is the first command we handle or we cross a page boundary,
> +         * we need to (re)map the command buffer.
> +         */
> +        if ( !cmdbuf || (its->creadr & ~PAGE_MASK) == 0 )
> +        {
> +            if ( cmdbuf )
> +                unmap_one_guest_page(cmdbuf);
> +            cmdbuf = map_one_guest_page(d,
> +                                       (cmdbuf_addr + its->creadr) & 
> PAGE_MASK);
> +            if ( !cmdbuf )
> +                return -EFAULT;
> +        }
> +        cmdptr = cmdbuf + (its->creadr & ~PAGE_MASK);
> +
> +        switch ( its_cmd_get_command(cmdptr) )
> +        {
> +        case GITS_CMD_SYNC:
> +            /* We handle ITS commands synchronously, so we ignore SYNC. */
> +           break;
The indentation is wrong.

> +        default:
> +            gdprintk(XENLOG_WARNING, "ITS: unhandled ITS command %lu\n",
> +                     its_cmd_get_command(cmdptr));
> +            break;
> +        }
> +
> +        its->creadr += ITS_CMD_SIZE;
> +        if ( its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser) )
> +            its->creadr = 0;
> +
> +        if ( ret )
> +            gdprintk(XENLOG_WARNING,
> +                     "ITS: ITS command error %d while handling command %lu\n",
> +                     ret, its_cmd_get_command(cmdptr));
> +    }
> +    its->cwriter = writer;
I think its->cwriter should be updated before the loop. So another vCPU 
could read the correct CWRITER whilst this vCPU is executing the commands.
> +
> +    spin_unlock(&its->vcmd_lock);
> +
> +    if ( cmdbuf )
> +        unmap_one_guest_page(cmdbuf);
> +
> +    return 0;
> +}
> +
> +/*****************************
> + * ITS registers read access *
> + *****************************/
> +
> +static int vgic_v3_its_mmio_read(struct vcpu *v, mmio_info_t *info,
> +                                 register_t *r, void *priv)
> +{
> +    struct virt_its *its = priv;
> +    uint64_t reg;
> +
> +    switch ( info->gpa & 0xffff )
> +    {
> +    case VREG32(GITS_CTLR):
> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
> +        if ( its_is_enabled(its) )
> +            reg = GITS_CTLR_ENABLE | BIT(31);
Please define BIT(31). Also, technically the ITS is not quiescent when 
commands are executed (GITS_CTLR could be read from another vCPU).

> +        else
> +            reg = BIT(31);
Ditto.

> +        *r = vgic_reg32_extract(reg, 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 ( !vgic_reg64_check_access(info->dabt) ) goto bad_width;
> +
> +        reg = GITS_TYPER_PHYSICAL;
Can you please document the configuration of the vITS.

> +        reg |= (sizeof(struct vits_itte) - 1) << GITS_TYPER_ITT_SIZE_SHIFT;
> +        reg |= (its->intid_bits - 1) << GITS_TYPER_IDBITS_SHIFT;
> +        reg |= (its->devid_bits - 1) << GITS_TYPER_DEVIDS_SHIFT;
> +        *r = vgic_reg64_extract(reg, info);
> +        break;
> +    case VREG64(GITS_CBASER):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style if ( !vgic...

> +        *r = vgic_reg64_extract(its->cbaser, info);
I don't think you can assume vgic_reg64_extract will be atomic.

> +        break;
> +    case VREG64(GITS_CWRITER):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style

> +        *r = vgic_reg64_extract(its->cwriter, info);
Ditto.

> +        break;
> +    case VREG64(GITS_CREADR):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style

> +        *r = vgic_reg64_extract(its->creadr, info);
Ditto.

> +        break;
> +    case VREG64(GITS_BASER0):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style

> +        *r = vgic_reg64_extract(its->baser_dev, info);
Ditto.

> +        break;
> +    case VREG64(GITS_BASER1):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style

> +        *r = vgic_reg64_extract(its->baser_coll, info);
Ditto.

> +        break;
> +    case VRANGE64(GITS_BASER2, GITS_BASER7):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style

> +        *r = vgic_reg64_extract(0, info);
Please introduce a label read_as_zero_64 at the end and do the 
implementation of RAZ there. It will acts as a documentation too (see an 
example in vgic-v3.c).
Also, vgic_reg64_extract(0, info) will ... always return 0. So you can 
optimize it ;).
> +        break;
> +    case VREG32(GITS_PIDR2):
> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
> +        *r = vgic_reg32_extract(GICV3_GICD_PIDR2, info);
> +        break;
Please add all the registers even implementation defined and reserved 
one. Ignoring registers without any warning is usually a bad idea as it 
makes very difficult to debug it. You can look at vgic-v3.c for an example.
> +    }
> +
> +    return 1;
> +
> +bad_width:
Please print an error here (see vgic-v3.c).

> +    domain_crash_synchronous();
> +
> +    return 0;
> +}
> +
> +/******************************
> + * ITS registers write access *
> + ******************************/
> +
> +static int its_baser_table_size(uint64_t baser)
unsigned int for the return.

> +{
> +    int page_size = 0;
unsigned int.

> +
> +    switch ( (baser >> 8) & 3 )
Please define 8 and 3.

> +    {
> +    case 0: page_size = SZ_4K; break;
> +    case 1: page_size = SZ_16K; break;
> +    case 2:
> +    case 3: page_size = SZ_64K; break;
> +    }
It looks like to me that the switch could be turned into an array:

unsigned page_size[] = {SZ_4K, SZ_16K, SZ_64K, SZ_64K};

This woudl make the code simpler.

> +
> +    return page_size * ((baser & GENMASK_ULL(7, 0)) + 1);
> +}
> +
> +static int its_baser_nr_entries(uint64_t baser)
unsigned int for the return and the function would probably benefit to 
be inlined.
> +{
> +    int entry_size = ((baser & GENMASK_ULL(52, 48)) >> 48) + 1;
unsigned int for the type. Also please use a define for 48.

> +
> +    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 reg32, ctlr;
ctlr could be defined in the case...

> +
> +    switch ( info->gpa & 0xffff )
> +    {
> +    case VREG32(GITS_CTLR):
here. I tend to prefer to restrict the scope whenever it is possible.

> +        if ( info->dabt.size != DABT_WORD ) goto bad_width;
> +
> +        ctlr = its_is_enabled(its) ? GITS_CTLR_ENABLE : 0;
> +        reg32 = ctlr;
> +        vgic_reg32_update(&reg32, r, info);
> +
> +        if ( ctlr ^ reg32 )
> +            vgic_v3_its_change_its_status(its, reg32 & GITS_CTLR_ENABLE);
This does not look atomic, so you could end-up calling this twice.

> +        return 1;
> +
> +    case VREG32(GITS_IIDR):
> +        goto write_ignore_32;
> +    case VREG32(GITS_TYPER):
> +        goto write_ignore_32;
> +    case VREG64(GITS_CBASER):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style.

> +
> +        /* Changing base registers with the ITS enabled is UNPREDICTABLE. */
> +        if ( its_is_enabled(its) )
See my question about atomicity.

> +        {
> +            gdprintk(XENLOG_WARNING, "ITS: Domain %d tried to change CBASER with the 
> ITS enabled.\n", d->domain_id);
> +            return 1;
> +        }
> +
> +        reg = its->cbaser;
> +        vgic_reg64_update(&reg, r, info);
> +        sanitize_its_base_reg(&reg);
> +
> +        vgic_its_unmap_cmdbuf(its);
Looking at the implementation of vgic_its_unmap_cmdbuf, you have no 
safety to check whether the region was mapped before.
Also, from the spec, 8.19.2 in ARM IHI 0069C), GITS_CREADR (i.e 
its->creadr) should be reset to 0.
> +        its->cbaser = reg;
> +
> +       return 1;
> +
> +    case VREG64(GITS_CWRITER):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style.

> +        reg = its->cwriter & 0xfffe0;
> +        vgic_reg64_update(&reg, r, info);
> +        its->cwriter = reg & 0xfffe0;
> +
> +        if ( its_is_enabled(its) )
> +            vgic_its_handle_cmds(d, its, reg);
I was expecting you to check the return here.

> +
> +        return 1;
> +
> +    case VREG64(GITS_CREADR):
> +        goto write_ignore_64;
> +    case VREG64(GITS_BASER0):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style.

> +
> +        /* Changing base registers with the ITS enabled is UNPREDICTABLE. */
> +        if ( its_is_enabled(its) )
> +        {
> +            gdprintk(XENLOG_WARNING, "ITS: Domain %d tried to change BASER with the 
> ITS enabled.\n",
> +                     d->domain_id);
> +
> +            return 1;
> +        }
I was expecting some locking here because this could be called 
concurrently...
> +
> +        reg = its->baser_dev;
> +        vgic_reg64_update(&reg, r, info);
> +
> +        reg &= ~GITS_BASER_RO_MASK;
> +        reg |= (sizeof(uint64_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT;
> +        reg |= GITS_BASER_TYPE_DEVICE << GITS_BASER_TYPE_SHIFT;
> +        sanitize_its_base_reg(&reg);
> +
> +        /* Has the table address been changed or invalidated? */
> +        if ( !(reg & GITS_VALID_BIT) ||
> +             get_baser_phys_addr(reg) != get_baser_phys_addr(its->baser_dev) )
> +        {
> +            vgic_its_unmap_its_table(its->d, its->baser_dev);
> +            clear_bit(VIRT_ITS_DEV_VALID, &its->flags);
> +        }
> +
> +        if ( reg & GITS_VALID_BIT )
> +            its->max_devices = its_baser_nr_entries(reg);
> +        else
> +            its->max_devices = 0;
> +
> +        its->baser_dev = reg;
> +        return 1;
> +    case VREG64(GITS_BASER1):
> +        if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
> +
> +        /* Changing base registers with the ITS enabled is UNPREDICTABLE. */
> +        if ( its_is_enabled(its) )
> +        {
> +            gdprintk(XENLOG_INFO, "ITS: Domain %d tried to change BASER with the 
> ITS enabled.\n",
> +                     d->domain_id);
> +            return 1;
> +        }
> +
> +        reg = its->baser_coll;
> +        vgic_reg64_update(&reg, r, info);
> +        reg &= ~GITS_BASER_RO_MASK;
> +        reg |= (sizeof(uint16_t) - 1) << GITS_BASER_ENTRY_SIZE_SHIFT;
> +        reg |= GITS_BASER_TYPE_COLLECTION << GITS_BASER_TYPE_SHIFT;
> +        sanitize_its_base_reg(&reg);
> +
> +        if ( !(reg & GITS_VALID_BIT) ||
> +             get_baser_phys_addr(reg) != get_baser_phys_addr(its->baser_coll) )
> +        {
> +            vgic_its_unmap_its_table(its->d, its->baser_coll);
> +            clear_bit(VIRT_ITS_COLL_VALID, &its->flags);
> +        }
> +
> +        if ( reg & GITS_VALID_BIT )
> +            its->max_collections = its_baser_nr_entries(reg);
> +        else
> +            its->max_collections = 0;
> +        its->baser_coll = reg;
> +        return 1;
> +    case VRANGE64(GITS_BASER2, GITS_BASER7):
> +        goto write_ignore_64;
From the ITS register map, we would have to emulate more register (at 
least reserved, implementation defined and RAZ).

> +    default:
> +        gdprintk(XENLOG_G_WARNING, "ITS: unhandled ITS register 0x%lx\n",
> +                 info->gpa & 0xffff);
> +        return 0;
> +    }
> +
> +    return 1;
> +
> +write_ignore_64:
> +    if ( ! vgic_reg64_check_access(info->dabt) ) goto bad_width;
Coding style.

> +    return 1;
> +
> +write_ignore_32:
> +    if ( info->dabt.size != DABT_WORD ) goto bad_width;
> +    return 1;
> +
> +bad_width:
> +    printk(XENLOG_G_ERR "%pv vGICR: bad read width %d r%d offset %#08lx\n",
> +           v, info->dabt.size, info->dabt.reg, info->gpa & 0xffff);
> +
This is ITS emulation not GICR ;).

> +    domain_crash_synchronous();
> +
> +    return 0;
> +}
> +
> +static const struct mmio_handler_ops vgic_its_mmio_handler = {
> +    .read  = vgic_v3_its_mmio_read,
> +    .write = vgic_v3_its_mmio_write,
> +};
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 2c6b317..ebcfc16 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -158,15 +158,6 @@ static void vgic_store_irouter(struct domain *d, struct 
> vgic_irq_rank *rank,
>      write_atomic(&rank->vcpu[offset], new_vcpu->vcpu_id);
>  }
>
> -static inline bool vgic_reg64_check_access(struct hsr_dabt dabt)
> -{
> -    /*
> -     * 64 bits registers can be accessible using 32-bit and 64-bit unless
> -     * stated otherwise (See 8.1.3 ARM IHI 0069A).
> -     */
> -    return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD );
> -}
> -
A separate commit would have been nice.

[...]

> diff --git a/xen/include/asm-arm/gic_v3_defs.h 
> b/xen/include/asm-arm/gic_v3_defs.h
> index b01b6ed..8999937 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -155,6 +155,16 @@
>  #define LPI_PROP_RES1                (1 << 1)
>  #define LPI_PROP_ENABLED             (1 << 0)
>
> +/*
> + * PIDR2: Only bits[7:4] are not implementation defined. We are
> + * emulating a GICv3 ([7:4] = 0x3).
> + *
> + * We don't emulate a specific registers scheme so implement the others
> + * bits as RES0 as recommended by the spec (see 8.1.13 in ARM IHI 0069A).
> + */
> +#define GICV3_GICD_PIDR2  0x30
> +#define GICV3_GICR_PIDR2  GICV3_GICD_PIDR2
> +
Those values should not be defined in gic_v3_defs.h but a vgic headers. 
My rationale is, those value are implementation defined (e.g depends on 
the emulation).
>  #define GICH_VMCR_EOI                (1 << 9)
>  #define GICH_VMCR_VENG1              (1 << 1)
>
> @@ -198,6 +208,15 @@ struct rdist_region {
>      bool single_rdist;
>  };
>
> +/*
> + * 64 bits registers can be accessible using 32-bit and 64-bit unless
> + * stated otherwise (See 8.1.3 ARM IHI 0069A).
> + */
> +static inline bool vgic_reg64_check_access(struct hsr_dabt dabt)
> +{
> +    return ( dabt.size == DABT_DOUBLE_WORD || dabt.size == DABT_WORD );
> +}
This is the wrong place for this helper. I think this should go in 
vgic-emul.h.
> +
>  #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
>
>  /*
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index d8165c4..b2a9586 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -35,6 +35,7 @@
>  #define GITS_BASER5                     0x128
>  #define GITS_BASER6                     0x130
>  #define GITS_BASER7                     0x138
> +#define GITS_PIDR2                      GICR_PIDR2
>
>  /* Register bits */
>  #define GITS_VALID_BIT                  BIT_ULL(63)
> @@ -56,6 +57,7 @@
>  #define GITS_TYPER_ITT_SIZE_MASK        (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT)
>  #define GITS_TYPER_ITT_SIZE(r)          ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \
>                                                   GITS_TYPER_ITT_SIZE_SHIFT) + 
> 1)
> +#define GITS_TYPER_PHYSICAL             (1U << 0)
>
>  #define GITS_IIDR_VALUE                 0x34c
Regards,

--
Julien Grall

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