|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 11/19] arm/its: Add ITS VM and VPE allocation/teardown
Hi Mykyta,
Thank you for the patch!
On Mon, Feb 2, 2026 at 6:14 PM Mykyta Poturai <Mykyta_Poturai@xxxxxxxx> wrote:
>
> Do necessary allocations for GICv4 VLPI injection.
> When creating a domain allocate its_vm and property tables.
> For each VCPU allocate a VPe with a unique vpe id and separate pending table.
>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> ---
> xen/arch/arm/gic-v3-its.c | 157 ++++++++++++----
> xen/arch/arm/gic-v3-lpi.c | 61 +++++-
> xen/arch/arm/gic-v3.c | 18 ++
> xen/arch/arm/gic-v4-its.c | 259 ++++++++++++++++++++++++++
> xen/arch/arm/include/asm/gic_v3_its.h | 17 ++
> xen/arch/arm/include/asm/gic_v4_its.h | 1 +
> xen/arch/arm/include/asm/vgic.h | 3 +
> xen/arch/arm/vgic.c | 25 ++-
> 8 files changed, 496 insertions(+), 45 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 2328595a85..fb1d2709be 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -31,6 +31,8 @@
> LIST_HEAD(host_its_list);
>
>
> +unsigned int nvpeid = 16;
> +
> /*
> * It is unlikely that a platform implements ITSes with different quirks,
> * so assume they all share the same.
> @@ -228,7 +230,7 @@ int gicv3_its_wait_commands(struct host_its *hw_its)
> return -ETIMEDOUT;
> }
>
> -static uint64_t encode_rdbase(struct host_its *hw_its, unsigned int cpu,
> +uint64_t encode_rdbase(struct host_its *hw_its, unsigned int cpu,
> uint64_t reg)
> {
> reg &= ~GENMASK(51, 16);
> @@ -443,6 +445,54 @@ struct its_baser *its_get_baser(struct host_its *hw_its,
> uint32_t type)
> return NULL;
> }
>
> +bool its_alloc_table_entry(struct its_baser *baser, uint32_t id)
> +{
> + uint64_t reg = baser->val;
> + bool indirect = reg & GITS_BASER_INDIRECT;
> + unsigned int idx;
> + __le64 *table;
> + unsigned int entry_size = GITS_BASER_ENTRY_SIZE(reg);
> +
> + /* Don't allow id that exceeds single, flat table limit */
> + if ( !indirect )
> + return (id < (baser->table_size / entry_size));
> +
> + /* Compute 1st level table index & check if that exceeds table limit */
> + idx = id / (baser->pagesz / entry_size);
> + if ( idx >= (baser->pagesz / GITS_LVL1_ENTRY_SIZE) )
> + return false;
> +
> + table = baser->base;
> +
> + /* Allocate memory for 2nd level table */
> + if (!table[idx])
> + {
> + unsigned int page_size = baser->pagesz;
> + void *buffer;
> +
> + buffer = alloc_xenheap_pages(get_order_from_bytes(page_size),
> + gicv3_its_get_memflags());
> + if ( !buffer )
> + return -ENOMEM;
returns -ENOMEM from a bool function, so OOM looks like
success.
---
does not zero the new L2 table before publishing it.
See, Arm IHI 0069H.b, 5.2.1 The ITS tables:
Behavior is UNPREDICTABLE if:
- Memory that is used for the level 2 tables does not contain zeros at
the time of the new allocation for use by the ITS.
---
The L2 tables allocated here are never freed. If this is
intended as “allocate once for ITS lifetime”, please add a short
comment stating that, to avoid the appearance of a leak.
> +
> + /* Flush Lvl2 table to PoC if hw doesn't support coherency */
> + if ( gicv3_its_get_cacheability() <= GIC_BASER_CACHE_nC )
> + clean_and_invalidate_dcache_va_range(buffer, page_size);
> +
> + table[idx] = cpu_to_le64(virt_to_maddr(buffer) | GITS_VALID_BIT);
Before writing virt_to_maddr(buffer) into the L1 entry, should
we validate that the physical address is encodable?
> +
> + /* Flush Lvl1 entry to PoC if hw doesn't support coherency */
> + if ( gicv3_its_get_cacheability() <= GIC_BASER_CACHE_nC )
> + clean_and_invalidate_dcache_va_range(table + idx,
> + GITS_LVL1_ENTRY_SIZE);
> +
> + /* Ensure updated table contents are visible to ITS hardware */
> + dsb(sy);
> + }
> +
> + return true;
> +}
> +
> static int its_map_baser(void __iomem *basereg, uint64_t regc,
> unsigned int nr_items, struct its_baser *baser)
> {
> @@ -737,13 +787,75 @@ static int gicv3_its_map_host_events(struct host_its
> *its,
> return ret;
> }
>
> - /* TODO: Consider using INVALL here. Didn't work on the model, though. */
> + return 0;
> +}
> +
> +static bool its_alloc_device_table(struct host_its *hw_its, uint32_t dev_id)
> +{
> + struct its_baser *baser;
> +
> + baser = its_get_baser(hw_its, GITS_BASER_TYPE_DEVICE);
> + if ( !baser )
> + return false;
> +
> + return its_alloc_table_entry(baser, dev_id);
> +}
> +
> +struct its_device *its_create_device(struct host_its *hw_its,
> + uint32_t host_devid, uint64_t nr_events)
> +{
> + void *itt_addr = NULL;
> + struct its_device *dev = NULL;
> + int ret;
> +
> + /* Sanitise the provided hardware values against the host ITS. */
> + if ( host_devid >= BIT(hw_its->devid_bits, UL) )
> + return NULL;
> +
> + dev = xzalloc(struct its_device);
> + if ( !dev )
> + return NULL;
> +
> + /* An Interrupt Translation Table needs to be 256-byte aligned. */
> + dev->itt_order = get_order_from_bytes(nr_events * hw_its->itte_size);
> + itt_addr = alloc_xenheap_pages(dev->itt_order, gicv3_its_get_memflags());
> + if ( !itt_addr )
> + goto fail_dev;
> +
> + clean_and_invalidate_dcache_va_range(itt_addr,
> + nr_events * hw_its->itte_size);
clean_and_invalidate_dcache_va_range() is unconditional.
Should this be guarded by
gicv3_its_get_cacheability() <= GIC_BASER_CACHE_nC
as in other paths?
It avoids a redundant flush on coherent systems.
> +
>
> - ret = its_send_cmd_sync(its, 0);
> + if ( !its_alloc_device_table(hw_its, host_devid) )
> + goto fail_itt;
> +
> + ret = its_send_cmd_mapd(hw_its, host_devid, max(fls(nr_events - 1), 1U),
> + virt_to_maddr(itt_addr), true);
> if ( ret )
> - return ret;
> + goto fail_itt;
>
> - return gicv3_its_wait_commands(its);
> + dev->itt_addr = itt_addr;
> + dev->hw_its = hw_its;
> + dev->host_devid = host_devid;
> + dev->eventids = nr_events;
> +
> + return dev;
> +
> +fail_itt:
> + free_xenheap_pages(itt_addr, dev->itt_order);
> +fail_dev:
> + xfree(dev);
> +
> + return NULL;
> +}
> +
> +static void its_free_device(struct its_device *dev)
> +{
> + xfree(dev->host_lpi_blocks);
does not release host LPI blocks via
gicv3_free_host_lpi_block().
> + xfree(dev->itt_addr);
uses xfree() for itt_addr, but it comes from
alloc_xenheap_pages().
> + if ( dev->pend_irqs )
nit: xfree checks if the provided pointer is equal to NULL
> + xfree(dev->pend_irqs);
> + xfree(dev);
> }
>
> /*
> @@ -758,12 +870,10 @@ int gicv3_its_map_guest_device(struct domain *d,
> paddr_t guest_doorbell, uint32_t guest_devid,
> uint64_t nr_events, bool valid)
> {
> - void *itt_addr = NULL;
> struct host_its *hw_its;
> struct its_device *dev = NULL;
> struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
> int i, ret = -ENOENT; /* "i" must be signed to check for >= 0
> below. */
> - unsigned int order;
>
> hw_its = gicv3_its_find_by_doorbell(host_doorbell);
> if ( !hw_its )
> @@ -823,23 +933,12 @@ int gicv3_its_map_guest_device(struct domain *d,
> if ( !valid )
> goto out_unlock;
>
> - ret = -ENOMEM;
> -
> - /* An Interrupt Translation Table needs to be 256-byte aligned. */
> - order = get_order_from_bytes(max(nr_events * hw_its->itte_size, 256UL));
> - itt_addr = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> - if ( !itt_addr )
> - goto out_unlock;
> -
> - memset(itt_addr, 0, PAGE_SIZE << order);
> -
> - clean_and_invalidate_dcache_va_range(itt_addr,
> - nr_events * hw_its->itte_size);
> -
> - dev = xzalloc(struct its_device);
> + dev = its_create_device(hw_its, host_devid, nr_events);
> if ( !dev )
> goto out_unlock;
>
> + ret = -ENOMEM;
> +
> /*
> * Allocate the pending_irqs for each virtual LPI. They will be put
> * into the domain's radix tree upon the guest's MAPTI command.
> @@ -860,14 +959,6 @@ int gicv3_its_map_guest_device(struct domain *d,
> if ( !dev->host_lpi_blocks )
> goto out_unlock;
>
> - ret = its_send_cmd_mapd(hw_its, host_devid, fls(nr_events - 1),
> - virt_to_maddr(itt_addr), true);
> - if ( ret )
> - goto out_unlock;
> -
> - dev->itt_addr = itt_addr;
> - dev->itt_order = order;
> - dev->hw_its = hw_its;
> dev->guest_doorbell = guest_doorbell;
> dev->guest_devid = guest_devid;
> dev->host_devid = host_devid;
> @@ -920,13 +1011,7 @@ out_unlock:
>
> out:
> if ( dev )
> - {
> - xfree(dev->pend_irqs);
> - xfree(dev->host_lpi_blocks);
> - }
> - if ( itt_addr )
> - free_xenheap_pages(itt_addr, order);
> - xfree(dev);
> + its_free_device(dev);
>
> return ret;
> }
> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> index c029d5d7a4..3c2649b695 100644
> --- a/xen/arch/arm/gic-v3-lpi.c
> +++ b/xen/arch/arm/gic-v3-lpi.c
> @@ -58,6 +58,7 @@ static DEFINE_PER_CPU(struct lpi_redist_data, lpi_redist);
>
> #define MAX_NR_HOST_LPIS (lpi_data.max_host_lpi_ids - LPI_OFFSET)
> #define HOST_LPIS_PER_PAGE (PAGE_SIZE / sizeof(union host_lpi))
> +uint32_t lpi_id_bits;
>
> static union host_lpi *gic_get_host_lpi(uint32_t plpi)
> {
> @@ -202,14 +203,11 @@ void gicv3_lpi_update_host_entry(uint32_t host_lpi, int
> domain_id,
> write_u64_atomic(&hlpip->data, hlpi.data);
> }
>
> -static int gicv3_lpi_allocate_pendtable(unsigned int cpu)
> +struct page_info *lpi_allocate_pendtable(void)
> {
> void *pendtable;
> unsigned int order;
>
> - if ( per_cpu(lpi_redist, cpu).pending_table )
> - return -EBUSY;
> -
> /*
> * The pending table holds one bit per LPI and even covers bits for
> * interrupt IDs below 8192, so we allocate the full range.
> @@ -219,20 +217,34 @@ static int gicv3_lpi_allocate_pendtable(unsigned int
> cpu)
> order = get_order_from_bytes(max(lpi_data.max_host_lpi_ids / 8,
> (unsigned long)SZ_64K));
> pendtable = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> if ( !pendtable )
> - return -ENOMEM;
> + return NULL;
>
> memset(pendtable, 0, PAGE_SIZE << order);
> /* Make sure the physical address can be encoded in the register. */
> if ( virt_to_maddr(pendtable) & ~GENMASK(51, 16) )
> {
> free_xenheap_pages(pendtable, order);
> - return -ERANGE;
> + return NULL;
> }
> clean_and_invalidate_dcache_va_range(pendtable,
> lpi_data.max_host_lpi_ids / 8);
>
> - per_cpu(lpi_redist, cpu).pending_table = pendtable;
> + return virt_to_page(pendtable);
> +}
> +
> +static int gicv3_lpi_allocate_pendtable(unsigned int cpu)
> +{
> + struct page_info *pendtable;
> +
> + if ( per_cpu(lpi_redist, cpu).pending_table )
> + return -EBUSY;
> +
> + pendtable = lpi_allocate_pendtable();
> + if ( !pendtable )
> + return -EINVAL;
>
> + per_cpu(lpi_redist, cpu).pending_table = page_to_virt(pendtable);
> +
> return 0;
> }
>
> @@ -274,6 +286,38 @@ static int gicv3_lpi_set_pendtable(void __iomem
> *rdist_base)
> return 0;
> }
>
> +void *lpi_allocate_proptable(void)
Since this helper is used to allocate the table programmed via
GICR_VPROPBASER (vLPI configuration table), it may be clearer to
name it lpi_allocate_vproptable() (and lpi_free_vproptable()) to avoid
confusion with the physical PROPBASER path.
> +{
> + void *table;
> + int order;
> +
> + /* The property table holds one byte per LPI. */
> + order = get_order_from_bytes(lpi_data.max_host_lpi_ids);
Do we really need to allocate one byte per *ID* here?
AFAIU for the (V)PROPBASER configuration table the entry for LPI N is
at (base + (N - 8192)), so the table needs only
(max_host_lpi_ids - 8192) bytes, not max_host_lpi_ids bytes.
Arm IHI 0069H.b, 5.1.1 "LPI Configuration tables" states:
For any LPI N, the location of the table entry is defined by
(base address + (N - 8192)).
Also see "GICv3 and GICv4 Software Overview" (DAI 0492B),
section "LPI Configuration table":
Size in bytes = 2^(GICR_PROPBASER.IDbits+1) – 8192
The "extra bytes" are relevant for the *pending* table semantics, not
for the configuration table. With the current allocation size, we also
init/flush only (max_host_lpi_ids - 8192) bytes, leaving a tail
uninitialized. Either allocate the adjusted size, or init/flush the
full allocated range.
---
nit: VPROPBASER is VM-wide (vLPI configuration is global to all vPEs in
a VM). It may be worth making the chosen IDbits/table size an explicit
per-VM value, i.e. based on programmed value from VPROPBASER.
> + table = alloc_xenheap_pages(order, gicv3_its_get_memflags());
> + if ( !table )
> + return NULL;
> +
> + /* Make sure the physical address can be encoded in the register. */
> + if ( (virt_to_maddr(table) & ~GENMASK(51, 12)) )
> + {
> + free_xenheap_pages(table, order);
> + return NULL;
> + }
> + memset(table, GIC_PRI_IRQ | LPI_PROP_RES1, MAX_NR_HOST_LPIS);
> + clean_and_invalidate_dcache_va_range(table, MAX_NR_HOST_LPIS);
> +
> + return table;
> +}
> +
> +void lpi_free_proptable(void *vproptable)
> +{
> + int order;
> +
> + /* The property table holds one byte per LPI. */
> + order = get_order_from_bytes(lpi_data.max_host_lpi_ids);
> + free_xenheap_pages(vproptable, order);
> +}
> +
> /*
> * Tell a redistributor about the (shared) property table, allocating one
> * if not already done.
> @@ -314,7 +358,8 @@ static int gicv3_lpi_set_proptable(void __iomem *
> rdist_base)
> }
>
> /* Encode the number of bits needed, minus one */
> - reg |= fls(lpi_data.max_host_lpi_ids - 1) - 1;
> + lpi_id_bits = fls(lpi_data.max_host_lpi_ids - 1);
> + reg |= lpi_id_bits - 1;
>
> reg |= virt_to_maddr(lpi_data.lpi_property);
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 14852d18c2..d4af332b0e 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -2083,6 +2083,22 @@ static bool gic_dist_supports_lpis(void)
> return (readl_relaxed(GICD + GICD_TYPER) & GICD_TYPE_LPIS);
> }
>
> +#ifdef CONFIG_GICV4
> +static void __init gicv4_init(void)
> +{
> + gicv3_info.hw_version = GIC_V4;
We should derive the HW version from GICD/GICR/GITS_PIDR2 in the
common GICv3/4 init path instead of hard‑coding GIC_V4 based on config.
That avoids config‑dependent values and keeps version detection
consistent.
> +
> +
> + gicv4_its_vpeid_allocator_init();
> +
> +}
> +#else
> +static void __init gicv4_init(void)
> +{
> + ASSERT_UNREACHABLE();
> +}
> +#endif
> +
> /* Set up the GIC */
> static int __init gicv3_init(void)
> {
> @@ -2157,6 +2173,8 @@ static int __init gicv3_init(void)
>
> gicv3_hyp_init();
>
> + if ( gic_is_gicv4() )
> + gicv4_init();
> out:
> spin_unlock(&gicv3.lock);
>
> diff --git a/xen/arch/arm/gic-v4-its.c b/xen/arch/arm/gic-v4-its.c
> index 358d0bffb9..fac3b44a94 100644
> --- a/xen/arch/arm/gic-v4-its.c
> +++ b/xen/arch/arm/gic-v4-its.c
> @@ -27,6 +27,83 @@
> #include <asm/vgic.h>
>
>
> +/*
> + * VPE ID is at most 16 bits.
> + * Using a bitmap here limits us to 65536 concurrent VPEs.
> + */
> +static unsigned long *vpeid_mask;
> +
> +static spinlock_t vpeid_alloc_lock = SPIN_LOCK_UNLOCKED;
> +
> +void __init gicv4_its_vpeid_allocator_init(void)
> +{
> + /* Allocate space for vpeid_mask based on MAX_VPEID */
> + vpeid_mask = xzalloc_array(unsigned long, BITS_TO_LONGS(MAX_VPEID));
> +
> + if ( !vpeid_mask )
> + panic("Could not allocate VPEID bitmap space\n");
> +}
> +
> +static int __init its_alloc_vpeid(struct its_vpe *vpe)
Several helpers in gic-v4-its.c are marked __init (e.g.
its_alloc_vpeid(), its_free_vpeid(), its_vpe_init(),
its_vpe_teardown()), but they are called from vcpu_vgic_init() via
vgic_v4_its_vpe_init(). This is not init-only code, and may cause
section mismatch warnings or runtime issues once init sections are
freed. Please drop __init for these functions.
---
vpe is unused
> +{
> + int id;
> +
> + spin_lock(&vpeid_alloc_lock);
> +
> + id = find_first_zero_bit(vpeid_mask, MAX_VPEID);
> +
> + if ( id == MAX_VPEID )
> + {
> + id = -EBUSY;
> + printk(XENLOG_ERR "VPEID pool exhausted\n");
> + goto out;
> + }
> +
> + set_bit(id, vpeid_mask);
> +
> +out:
> + spin_unlock(&vpeid_alloc_lock);
> +
> + return id;
> +}
> +
> +static void __init its_free_vpeid(uint32_t vpe_id)
> +{
> + spin_lock(&vpeid_alloc_lock);
> +
> + clear_bit(vpe_id, vpeid_mask);
> +
> + spin_unlock(&vpeid_alloc_lock);
> +}
> +
> +static bool __init its_alloc_vpe_entry(uint32_t vpe_id)
> +{
> + struct host_its *hw_its;
> +
> + /*
> + * Make sure the L2 tables are allocated on *all* v4 ITSs. We
> + * could try and only do it on ITSs corresponding to devices
> + * that have interrupts targeted at this VPE, but the
> + * complexity becomes crazy.
> + */
> + list_for_each_entry(hw_its, &host_its_list, entry)
> + {
> + struct its_baser *baser;
> +
> + if ( !hw_its->is_v4 )
> + continue;
> +
> + baser = its_get_baser(hw_its, GITS_BASER_TYPE_VCPU);
> + if ( !baser )
> + return false;
> +
> + if ( !its_alloc_table_entry(baser, vpe_id) )
> + return false;
> + }
> +
> + return true;
> +}
> +
> static int its_send_cmd_vsync(struct host_its *its, uint16_t vpeid)
> {
> uint64_t cmd[4];
> @@ -39,6 +116,188 @@ static int its_send_cmd_vsync(struct host_its *its,
> uint16_t vpeid)
> return its_send_command(its, cmd);
> }
>
> +static int its_send_cmd_vmapp(struct host_its *its, struct its_vpe *vpe,
> + bool valid)
> +{
> + uint64_t cmd[4];
> + uint16_t vpeid = vpe->vpe_id;
> + uint64_t vpt_addr;
> + int ret;
> +
> + cmd[0] = GITS_CMD_VMAPP;
> + cmd[1] = (uint64_t)vpeid << 32;
> + cmd[2] = valid ? GITS_VALID_BIT : 0;
> +
> + /* Unmap command */
> + if ( !valid )
> + goto out;
> +
> + /* Target redistributor */
> + cmd[2] |= encode_rdbase(its, vpe->col_idx, 0x0);
> + vpt_addr = virt_to_maddr(vpe->vpendtable);
> + cmd[3] = (vpt_addr & GENMASK(51, 16)) |
> + ((HOST_LPIS_NRBITS - 1) & GENMASK(4, 0));
> +
> + out:
> + ret = its_send_command(its, cmd);
> +
> + return ret;
> +}
> +
> +static int its_send_cmd_vinvall(struct host_its *its, struct its_vpe *vpe)
> +{
> + uint64_t cmd[4];
> + uint16_t vpeid = vpe->vpe_id;
> +
> + cmd[0] = GITS_CMD_VINVALL;
> + cmd[1] = (uint64_t)vpeid << 32;
> + cmd[2] = 0x00;
> + cmd[3] = 0x00;
> +
> + return its_send_command(its, cmd);
> +}
> +
> +static int its_map_vpe(struct host_its *its, struct its_vpe *vpe)
> +{
> + int ret;
> +
> + /*
> + * VMAPP command maps the vPE to the target RDbase, including an
> + * associated virtual LPI Pending table.
> + */
> + ret = its_send_cmd_vmapp(its, vpe, true);
> + if ( ret )
> + return ret;
> +
> + ret = its_send_cmd_vinvall(its, vpe);
> + if ( ret )
> + return ret;
> +
> + ret = its_send_cmd_vsync(its, vpe->vpe_id);
> + if ( ret )
> + return ret;
Error handling for ITS commands is minimal. If VMAPP succeeds and
VINVALL/VSYNC fails, we return without rollback or recovery.
Should we consider retrying (e.g. via CWRITER.Retry), or detect
error/stall via GITS_TYPER.SEIS / GITS_CREADR.Stalled and
unmap the VPE on failure?
> +
> + return 0;
> +}
> +static int __init its_vpe_init(struct its_vpe *vpe)
> +{
> + int vpe_id, rc = -ENOMEM;
> + struct page_info *vpendtable;
> + struct host_its *hw_its;
> +
> + /* Allocate vpe id */
> + vpe_id = its_alloc_vpeid(vpe);
> + if ( vpe_id < 0 )
> + return rc;
> +
> + /* Allocate VPT */
> + vpendtable = lpi_allocate_pendtable();
> +
> + if ( !vpendtable )
> + goto fail_vpt;
> +
> + if ( !its_alloc_vpe_entry(vpe_id) )
> + goto fail_entry;
> +
> + rwlock_init(&vpe->lock);
> + vpe->vpe_id = vpe_id;
> + vpe->vpendtable = page_to_virt(vpendtable);
> + /*
> + * We eagerly inform all the v4 ITS and map vPE to the first
> + * possible CPU
> + */
> + vpe->col_idx = cpumask_first(&cpu_online_map);
> + list_for_each_entry(hw_its, &host_its_list, entry)
> + {
> + if ( !hw_its->is_v4 )
> + continue;
> +
> + if ( its_map_vpe(hw_its, vpe) )
If its_map_vpe() fails for a later ITS, we should unmap the
VPE on the ITSes already handled. its_vpe_teardown() frees
SW allocations (vpe too) but does not undo prior VMAPPs,
leaving stale mappings behind.
> + goto fail_entry;
> + }
> +
> + return 0;
> +
> + fail_entry:
> + xfree(page_to_virt(vpendtable));
failure path frees vpendtable with xfree(),
but it was allocated with alloc_xenheap_pages().
---
The fail path frees vpendtable locally, and the caller then
calls its_vpe_teardown() on error. This risks double‑free,
Please have a single owner for cleanup.
> + fail_vpt:
> + its_free_vpeid(vpe_id);
> +
> + return rc;
> +}
> +
> +static void __init its_vpe_teardown(struct its_vpe *vpe)
> +{
> + unsigned int order;
> +
> + order = get_order_from_bytes(max(lpi_data.max_host_lpi_ids / 8,
> (unsigned long)SZ_64K));
> + its_free_vpeid(vpe->vpe_id);
> + free_xenheap_pages(vpe->vpendtable, order);
> + xfree(vpe);
> +}
> +
> +int vgic_v4_its_vm_init(struct domain *d)
> +{
> + unsigned int nr_vcpus = d->max_vcpus;
> + int ret = -ENOMEM;
> +
> + if ( !gicv3_its_host_has_its() )
> + return 0;
> +
> + d->arch.vgic.its_vm = xzalloc(struct its_vm);
> + if ( !d->arch.vgic.its_vm )
> + return ret;
> +
> + d->arch.vgic.its_vm->vpes = xzalloc_array(struct its_vpe *, nr_vcpus);
> + if ( !d->arch.vgic.its_vm->vpes )
> + goto fail_vpes;
> + d->arch.vgic.its_vm->nr_vpes = nr_vcpus;
> +
> + d->arch.vgic.its_vm->vproptable = lpi_allocate_proptable();
> + if ( !d->arch.vgic.its_vm->vproptable )
> + goto fail_vprop;
> +
> + return 0;
> +
> +fail_vprop:
> + xfree(d->arch.vgic.its_vm->vpes)
> + fail_vpes:
> + xfree(d->arch.vgic.its_vm);
use XFREE to cleanup the its_vm ptr too
> +
> + return ret;
> +}
> +
> +void vgic_v4_free_its_vm(struct domain *d)
> +{
> + struct its_vm *its_vm = d->arch.vgic.its_vm;
> + if ( its_vm->vpes )
nit: no need to check the ptr it is done by xfree
> + xfree(its_vm->vpes);
> + if ( its_vm->vproptable )
nit: looks like it is safe to pass NULL ptr to lpi_free_proptable
> + lpi_free_proptable(its_vm);
passes its_vm to lpi_free_proptable(), not
its_vm->vproptable.
> +}
> +
> +int vgic_v4_its_vpe_init(struct vcpu *vcpu)
> +{
> + int ret;
> + struct its_vm *its_vm = vcpu->domain->arch.vgic.its_vm;
> + unsigned int vcpuid = vcpu->vcpu_id;
> +
> + vcpu->arch.vgic.its_vpe = xzalloc(struct its_vpe);
> + if ( !vcpu->arch.vgic.its_vpe )
> + return -ENOMEM;
> +
> + its_vm->vpes[vcpuid] = vcpu->arch.vgic.its_vpe;
its_vm->vpes[vcpuid] is assigned before its_vpe_init(). If
its_vpe_init() fails, the array keeps a stale pointer. Either
move the assignment after success, or clear it on failure.
> + vcpu->arch.vgic.its_vpe->its_vm = its_vm;
> +
> + ret = its_vpe_init(vcpu->arch.vgic.its_vpe);
> + if ( ret )
> + {
> + its_vpe_teardown(vcpu->arch.vgic.its_vpe);
> + return ret;
> + }
> + return 0;
> +}
> +
> static int its_send_cmd_vmapti(struct host_its *its, struct its_device *dev,
> uint32_t eventid)
> {
> diff --git a/xen/arch/arm/include/asm/gic_v3_its.h
> b/xen/arch/arm/include/asm/gic_v3_its.h
> index bd2696f354..411beb81c8 100644
> --- a/xen/arch/arm/include/asm/gic_v3_its.h
> +++ b/xen/arch/arm/include/asm/gic_v3_its.h
> @@ -77,6 +77,7 @@
> #define GITS_BASER_ENTRY_SIZE_SHIFT 48
> #define GITS_BASER_ENTRY_SIZE(reg) \
> ((((reg) >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1)
> +#define GITS_LVL1_ENTRY_SIZE 8UL
> #define GITS_BASER_SHAREABILITY_SHIFT 10
> #define GITS_BASER_PAGE_SIZE_SHIFT 8
> #define GITS_BASER_SIZE_MASK 0xff
> @@ -117,9 +118,19 @@
> /* We allocate LPIs on the hosts in chunks of 32 to reduce handling
> overhead. */
> #define LPI_BLOCK 32U
>
> +extern unsigned int nvpeid;
> +/* The maximum number of VPEID bits supported by VLPI commands */
> +#define ITS_MAX_VPEID_BITS nvpeid
> +#define MAX_VPEID (1UL << ITS_MAX_VPEID_BITS)
> +
> #ifdef CONFIG_GICV4
> #include <asm/gic_v4_its.h>
> #endif
> +
> +extern uint32_t lpi_id_bits;
> +#define HOST_LPIS_NRBITS lpi_id_bits
> +#define MAX_HOST_LPIS BIT(lpi_id_bits, UL)
> +
> /*
> * Describes a device which is using the ITS and is used by a guest.
> * Since device IDs are per ITS (in contrast to vLPIs, which are per
> @@ -169,6 +180,7 @@ struct host_its {
> void *cmd_buf;
> unsigned int flags;
> struct its_baser tables[GITS_BASER_NR_REGS];
> + bool is_v4;
> };
>
> /* Map a collection for this host CPU to each host ITS. */
> @@ -273,8 +285,13 @@ struct pending_irq *gicv3_assign_guest_event(struct
> domain *d,
> void gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id,
> uint32_t virt_lpi);
> struct its_baser *its_get_baser(struct host_its *hw_its, uint32_t type);
> +bool its_alloc_table_entry(struct its_baser *baser, uint32_t id);
> +struct page_info *lpi_allocate_pendtable(void);
> +void *lpi_allocate_proptable(void);
> +void lpi_free_proptable(void *vproptable);
> void lpi_write_config(uint8_t *prop_table, uint32_t lpi, uint8_t clr,
> uint8_t set);
> +uint64_t encode_rdbase(struct host_its *hw_its, unsigned int cpu, uint64_t
> reg);
> int its_send_command(struct host_its *hw_its, const void *its_cmd);
>
> struct its_device *get_its_device(struct domain *d, paddr_t vdoorbell,
> diff --git a/xen/arch/arm/include/asm/gic_v4_its.h
> b/xen/arch/arm/include/asm/gic_v4_its.h
> index 722247ec60..fb0ef37bbe 100644
> --- a/xen/arch/arm/include/asm/gic_v4_its.h
> +++ b/xen/arch/arm/include/asm/gic_v4_its.h
> @@ -49,6 +49,7 @@ struct event_vlpi_map {
> unsigned int nr_vlpis;
> };
>
> +void gicv4_its_vpeid_allocator_init(void);
> #endif
>
> /*
> diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h
> index f12d736808..580310fec4 100644
> --- a/xen/arch/arm/include/asm/vgic.h
> +++ b/xen/arch/arm/include/asm/vgic.h
> @@ -414,6 +414,9 @@ bool gic_is_gicv4(void);
> #define gic_is_gicv4() (false)
> #endif
>
> +int vgic_v4_its_vm_init(struct domain *d);
> +void vgic_v4_free_its_vm(struct domain *d);
> +int vgic_v4_its_vpe_init(struct vcpu *vcpu);
> #endif /* !CONFIG_NEW_VGIC */
>
> /*** Common VGIC functions used by Xen arch code ****/
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 0da8c1a425..6baf870ad5 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -22,6 +22,7 @@
>
> #include <asm/mmio.h>
> #include <asm/gic.h>
> +#include <asm/gic_v3_its.h>
> #include <asm/vgic.h>
>
>
> @@ -329,6 +330,15 @@ int domain_vgic_init(struct domain *d, unsigned int
> nr_spis)
> for ( i = 0; i < NR_GIC_SGI; i++ )
> set_bit(i, d->arch.vgic.allocated_irqs);
>
> + if ( gic_is_gicv4() )
> + {
> + ret = vgic_v4_its_vm_init(d);
> + if ( ret )
> + {
> + printk(XENLOG_ERR "GICv4 its vm allocation failed\n");
> + return ret;
> + }
> + }
> return 0;
> }
>
> @@ -366,11 +376,14 @@ void domain_vgic_free(struct domain *d)
> #endif
> xfree(d->arch.vgic.pending_irqs);
> xfree(d->arch.vgic.allocated_irqs);
> +
> + if ( gic_is_gicv4() )
> + vgic_v4_free_its_vm(d);
vgic_v4_free_its_vm() on gic_is_gicv4() even when no
ITS, so its_vm can be NULL.
Best regards,
Mykola
> }
>
> int vcpu_vgic_init(struct vcpu *v)
> {
> - int i;
> + int i, ret;
>
> v->arch.vgic.private_irqs = xzalloc(struct vgic_irq_rank);
> if ( v->arch.vgic.private_irqs == NULL )
> @@ -389,6 +402,16 @@ int vcpu_vgic_init(struct vcpu *v)
> INIT_LIST_HEAD(&v->arch.vgic.lr_pending);
> spin_lock_init(&v->arch.vgic.lock);
>
> + if ( gic_is_gicv4() && gicv3_its_host_has_its())
> + {
> + ret = vgic_v4_its_vpe_init(v);
> + if ( ret )
> + {
> + printk(XENLOG_ERR "GICv4 its vpe allocation failed\n");
> + return ret;
> + }
> + }
> +
> return 0;
> }
>
> --
> 2.51.2
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |