From: Marc Zyngier <maz@kernel.org>
To: Zenghui Yu <yuzenghui@huawei.com>,
kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Jason Cooper <jason@lakedaemon.net>,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 10/35] irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation
Date: Wed, 25 Sep 2019 15:41:44 +0100 [thread overview]
Message-ID: <6f4ccdfd-4b63-04cb-e7c0-f069e620127f@kernel.org> (raw)
In-Reply-To: <155660c2-7f30-e188-ca8d-c37fabea6d97@huawei.com>
On 25/09/2019 14:04, Zenghui Yu wrote:
> Hi Marc,
>
> Some questions about this patch, mostly to confirm that I would
> understand things here correctly.
>
> On 2019/9/24 2:25, Marc Zyngier wrote:
>> GICv4.1 defines a new VPE table that is potentially shared between
>> both the ITSs and the redistributors, following complicated affinity
>> rules.
>>
>> To make things more confusing, the programming of this table at
>> the redistributor level is reusing the GICv4.0 GICR_VPROPBASER register
>> for something completely different.
>>
>> The code flow is somewhat complexified by the need to respect the
>> affinities required by the HW, meaning that tables can either be
>> inherited from a previously discovered ITS or redistributor.
>>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>
> [...]
>
>> @@ -1962,6 +1965,65 @@ static bool its_parse_indirect_baser(struct its_node *its,
>> return indirect;
>> }
>>
>> +static u32 compute_common_aff(u64 val)
>> +{
>> + u32 aff, clpiaff;
>> +
>> + aff = FIELD_GET(GICR_TYPER_AFFINITY, val);
>> + clpiaff = FIELD_GET(GICR_TYPER_COMMON_LPI_AFF, val);
>> +
>> + return aff & ~(GENMASK(31, 0) >> (clpiaff * 8));
>> +}
>> +
>> +static u32 compute_its_aff(struct its_node *its)
>> +{
>> + u64 val;
>> + u32 svpet;
>> +
>> + /*
>> + * Reencode the ITS SVPET and MPIDR as a GICR_TYPER, and compute
>> + * the resulting affinity. We then use that to see if this match
>> + * our own affinity.
>> + */
>> + svpet = FIELD_GET(GITS_TYPER_SVPET, its->typer);
>> + val = FIELD_PREP(GICR_TYPER_COMMON_LPI_AFF, svpet);
>> + val |= FIELD_PREP(GICR_TYPER_AFFINITY, its->mpidr);
>> + return compute_common_aff(val);
>> +}
>> +
>> +static struct its_node *find_sibbling_its(struct its_node *cur_its)
>> +{
>> + struct its_node *its;
>> + u32 aff;
>> +
>> + if (!FIELD_GET(GITS_TYPER_SVPET, cur_its->typer))
>> + return NULL;
>> +
>> + aff = compute_its_aff(cur_its);
>> +
>> + list_for_each_entry(its, &its_nodes, entry) {
>> + u64 baser;
>> +
>> + if (!is_v4_1(its) || its == cur_its)
>> + continue;
>> +
>> + if (!FIELD_GET(GITS_TYPER_SVPET, its->typer))
>> + continue;
>> +
>> + if (aff != compute_its_aff(its))
>> + continue;
>> +
>> + /* GICv4.1 guarantees that the vPE table is GITS_BASER2 */
>> + baser = its->tables[2].val;
>> + if (!(baser & GITS_BASER_VALID))
>> + continue;
>> +
>> + return its;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static void its_free_tables(struct its_node *its)
>> {
>> int i;
>> @@ -2004,6 +2066,15 @@ static int its_alloc_tables(struct its_node *its)
>> break;
>>
>> case GITS_BASER_TYPE_VCPU:
>> + if (is_v4_1(its)) {
>> + struct its_node *sibbling;
>> +
>> + if ((sibbling = find_sibbling_its(its))) {
>> + its->tables[2] = sibbling->tables[2];
>
> This means thst the vPE table is shared between ITSs which are under the
> same affinity group?
That's indeed what this is trying to do...
> Don't we need to set GITS_BASER (by its_setup_baser()) here to tell this
> ITS where the vPE table lacates?
Absolutely. This is a bug. I didn't spot it as my model only has a
single ITS.
>
>> + continue;
>> + }
>> + }
>> +
>> indirect = its_parse_indirect_baser(its, baser,
>> psz, &order,
>> ITS_MAX_VPEID_BITS);
>> @@ -2025,6 +2096,212 @@ static int its_alloc_tables(struct its_node *its)
>> return 0;
>> }
>>
>> +static u64 inherit_vpe_l1_table_from_its(void)
>> +{
>> + struct its_node *its;
>> + u64 val;
>> + u32 aff;
>> +
>> + val = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
>> + aff = compute_common_aff(val);
>> +
>> + list_for_each_entry(its, &its_nodes, entry) {
>> + u64 baser;
>> +
>> + if (!is_v4_1(its))
>> + continue;
>> +
>> + if (!FIELD_GET(GITS_TYPER_SVPET, its->typer))
>> + continue;
>> +
>> + if (aff != compute_its_aff(its))
>> + continue;
>> +
>> + /* GICv4.1 guarantees that the vPE table is GITS_BASER2 */
>> + baser = its->tables[2].val;
>> + if (!(baser & GITS_BASER_VALID))
>> + continue;
>> +
>> + /* We have a winner! */
>> + val = GICR_VPROPBASER_4_1_VALID;
>> + if (baser & GITS_BASER_INDIRECT)
>> + val |= GICR_VPROPBASER_4_1_INDIRECT;
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE,
>> + FIELD_GET(GITS_BASER_PAGE_SIZE_MASK, baser));
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_ADDR,
>> + GITS_BASER_ADDR_48_to_52(baser) >> 12);
>
> I remember the spec says,
> GITS_BASER2 "points to" the ITS *vPE table*, which provides mappings
> from the vPEID to target Redistributor and associated vpt address.
> In GICv4.1 GICR_VPROPBASER "points to" the *vPE Configuration table*,
> which stores the locations of each vPE's LPI configuration and pending
> table.
>
> And the code here says, if we can find an ITS (which is under the same
> CommonLPIAff group with this Redistributor) has already been probed and
> allocated an vPE table, then use this vPE table as this Redist's vPE
> Configuration table.
> So I infer that in GICv4.1, *vPE table* and *vPE Configuration table*
> are actually the same concept? And this table is shared between ITSs
> and Redists which are under the same affinity group?
> Please fix me if I have any misunderstanding.
Indeed. The whole idea is that ITSs and RDs can share a vPE table if
they are in the same affinity group (such as a socket, for example).
This is what is missing from v4.0 where the ITS knows about vPEs, but
doesn't know about residency. With that in place, the HW is able to do a
lot more for us.
>
>> + val |= FIELD_PREP(GICR_VPROPBASER_SHAREABILITY_MASK,
>> + FIELD_GET(GITS_BASER_SHAREABILITY_MASK, baser));
>> + val |= FIELD_PREP(GICR_VPROPBASER_INNER_CACHEABILITY_MASK,
>> + FIELD_GET(GITS_BASER_INNER_CACHEABILITY_MASK, baser));
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_SIZE, GITS_BASER_NR_PAGES(baser) - 1);
>> +
>> + return val;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static u64 inherit_vpe_l1_table_from_rd(cpumask_t **mask)
>> +{
>> + u32 aff;
>> + u64 val;
>> + int cpu;
>> +
>> + val = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
>> + aff = compute_common_aff(val);
>> +
>> + for_each_possible_cpu(cpu) {
>> + void __iomem *base = gic_data_rdist_cpu(cpu)->rd_base;
>> + u32 tmp;
>> +
>> + if (!base || cpu == smp_processor_id())
>> + continue;
>> +
>> + val = gic_read_typer(base + GICR_TYPER);
>> + tmp = compute_common_aff(val);
>> + if (tmp != aff)
>> + continue;
>> +
>> + /*
>> + * At this point, we have a victim. This particular CPU
>> + * has already booted, and has an affinity that matches
>> + * ours wrt CommonLPIAff. Let's use its own VPROPBASER.
>> + * Make sure we don't write the Z bit in that case.
>> + */
>> + val = gits_read_vpropbaser(base + SZ_128K + GICR_VPROPBASER);
>> + val &= ~GICR_VPROPBASER_4_1_Z;
>> +
>> + *mask = gic_data_rdist_cpu(cpu)->vpe_table_mask;
>> +
>> + return val;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int allocate_vpe_l1_table(void)
>> +{
>> + void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
>> + u64 val, esz, gpsz, npg, pa;
>> + unsigned int psz = SZ_64K;
>> + unsigned int np, epp;
>> + struct page *page;
>> +
>> + if (!gic_rdists->has_rvpeid)
>> + return 0;
>> +
>> + /*
>> + * if VPENDBASER.Valid is set, disable any previously programmed
>> + * VPE by setting PendingLast while clearing Valid. This has the
>> + * effect of making sure no doorbell will be generated and we can
>> + * then safely clear VPROPBASER.Valid.
>> + */
>> + if (gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER) & GICR_VPENDBASER_Valid)
>> + gits_write_vpendbaser(GICR_VPENDBASER_PendingLast,
>> + vlpi_base + GICR_VPENDBASER);
>> +
>> + /*
>> + * If we can inherit the configuration from another RD, let's do
>> + * so. Otherwise, we have to go through the allocation process. We
>> + * assume that all RDs have the exact same requirements, as
>> + * nothing will work otherwise.
>> + */
>> + val = inherit_vpe_l1_table_from_rd(&gic_data_rdist()->vpe_table_mask);
>> + if (val & GICR_VPROPBASER_4_1_VALID)
>> + goto out;
>> +
>> + gic_data_rdist()->vpe_table_mask = kzalloc(sizeof(cpumask_t), GFP_KERNEL);
>
> I think we should check the allocation failure here.
Sure.
>
>> +
>> + val = inherit_vpe_l1_table_from_its();
>> + if (val & GICR_VPROPBASER_4_1_VALID)
>> + goto out;
>> +
>> + /* First probe the page size */
>> + val = FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE, GIC_PAGE_SIZE_64K);
>> + gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
>> + val = gits_read_vpropbaser(vlpi_base + GICR_VPROPBASER);
>> + gpsz = FIELD_GET(GICR_VPROPBASER_4_1_PAGE_SIZE, val);
>> +
>> + switch (gpsz) {
>> + default:
>> + gpsz = GIC_PAGE_SIZE_4K;
>> + /* fall through */
>> + case GIC_PAGE_SIZE_4K:
>> + psz = SZ_4K;
>> + break;
>> + case GIC_PAGE_SIZE_16K:
>> + psz = SZ_16K;
>> + break;
>> + case GIC_PAGE_SIZE_64K:
>> + psz = SZ_64K;
>> + break;
>> + }
>> +
>> + /*
>> + * Start populating the register from scratch, including RO fields
>> + * (which we want to print in debug cases...)
>> + */
>> + val = 0;
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE, gpsz);
>> + esz = FIELD_GET(GICR_VPROPBASER_4_1_ENTRY_SIZE, val);
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_ENTRY_SIZE, esz);
>
> 'esz' seems always be 0 here?
Yup, that's broken. Not sure what I had in mind here. esz (element size)
should be extracted before resetting val, and ORed back in the register
copy for debug purposes.
>
>> +
>> + /* How many entries per GIC page? */
>> + esz++;
>> + epp = psz / (esz * SZ_8);
>> +
>> + /*
>> + * If we need more than just a single L1 page, flag the table
>> + * as indirect and compute the number of required L1 pages.
>> + */
>> + if (epp < ITS_MAX_VPEID) {
>> + int nl2;
>> +
>> + gic_rdists->flags |= RDIST_FLAGS_VPE_INDIRECT;
>> + val |= GICR_VPROPBASER_4_1_INDIRECT;
>> +
>> + /* Number of L2 pages required to cover the VPEID space */
>> + nl2 = DIV_ROUND_UP(ITS_MAX_VPEID, epp);
>> +
>> + /* Number of L1 pages to point to the L2 pages */
>> + npg = DIV_ROUND_UP(nl2 * SZ_8, psz);
>> + } else {
>> + npg = 1;
>> + }
>> +
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_SIZE, npg);
>> +
>> + /* Right, that's the number of CPU pages we need for L1 */
>> + np = DIV_ROUND_UP(npg * psz, PAGE_SIZE);
>> +
>> + pr_debug("np = %d, npg = %lld, psz = %d, epp = %d, esz = %lld\n",
>> + np, npg, psz, epp, esz);
>> + page = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(np * PAGE_SIZE));
>> + if (!page)
>> + return -ENOMEM;
>> +
>> + gic_data_rdist()->vpe_l1_page = page;
>> + pa = virt_to_phys(page_address(page));
>> + WARN_ON(!IS_ALIGNED(pa, psz));
>> +
>> + val |= FIELD_PREP(GICR_VPROPBASER_4_1_ADDR, pa >> 12);
>> + val |= GICR_VPROPBASER_RaWb;
>> + val |= GICR_VPROPBASER_InnerShareable;
>> + val |= GICR_VPROPBASER_4_1_Z;
>> + val |= GICR_VPROPBASER_4_1_VALID;
>> +
>> +out:
>> + gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
>> + cpumask_set_cpu(smp_processor_id(), gic_data_rdist()->vpe_table_mask);
>> +
>> + pr_debug("CPU%d: VPROPBASER = %llx %*pbl\n",
>> + smp_processor_id(), val,
>> + cpumask_pr_args(gic_data_rdist()->vpe_table_mask));
>> +
>> + return 0;
>> +}
>> +
>> static int its_alloc_collections(struct its_node *its)
>> {
>> int i;
>> @@ -2224,7 +2501,7 @@ static void its_cpu_init_lpis(void)
>> val |= GICR_CTLR_ENABLE_LPIS;
>> writel_relaxed(val, rbase + GICR_CTLR);
>>
>> - if (gic_rdists->has_vlpis) {
>> + if (gic_rdists->has_vlpis && !gic_rdists->has_rvpeid) {
>> void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
>>
>> /*
>> @@ -2248,6 +2525,16 @@ static void its_cpu_init_lpis(void)
>> WARN_ON(val & GICR_VPENDBASER_Dirty);
>> }
>>
>> + if (allocate_vpe_l1_table()) {
>> + /*
>> + * If the allocation has failed, we're in massive trouble.
>> + * Disable direct injection, and pray that no VM was
>> + * already running...
>> + */
>> + gic_rdists->has_rvpeid = false;
>> + gic_rdists->has_vlpis = false;
>> + }
>> +
>> /* Make sure the GIC has seen the above */
>> dsb(sy);
>> out:
Thanks for having had a look!
M.
--
Jazz is not dead, it just smells funny...
next prev parent reply other threads:[~2019-09-25 14:41 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-23 18:25 [PATCH 00/35] irqchip/gic-v4: GICv4.1 architecture support Marc Zyngier
2019-09-23 18:25 ` [PATCH 01/35] KVM: arm64: vgic-v4: Move the GICv4 residency flow to be driven by vcpu_load/put Marc Zyngier
2019-09-23 18:25 ` [PATCH 02/35] irqchip/gic-v3-its: Factor out wait_for_syncr primitive Marc Zyngier
2019-09-24 8:58 ` Andrew Murray
2019-09-23 18:25 ` [PATCH 03/35] irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface Marc Zyngier
2019-09-26 14:57 ` Zenghui Yu
2019-09-26 15:34 ` Marc Zyngier
2019-09-26 16:17 ` Marc Zyngier
2019-09-27 6:59 ` Zenghui Yu
2019-09-23 18:25 ` [PATCH 04/35] irqchip/gic-v3: Detect GICv4.1 supporting RVPEID Marc Zyngier
2019-09-24 10:24 ` Andrew Murray
2019-09-24 10:49 ` Marc Zyngier
2019-09-24 11:00 ` Andrew Murray
2019-09-24 11:18 ` Marc Zyngier
2019-09-23 18:25 ` [PATCH 05/35] irqchip/gic-v3: Add GICv4.1 VPEID size discovery Marc Zyngier
2019-09-24 10:49 ` Andrew Murray
2019-09-24 11:00 ` Marc Zyngier
2019-09-23 18:25 ` [PATCH 06/35] irqchip/gic-v3-its: Make is_v4 use a TYPER copy Marc Zyngier
2019-09-23 18:25 ` [PATCH 07/35] irqchip/gic-v3-its: Kill its->ite_size and use TYPER copy instead Marc Zyngier
2019-09-23 18:25 ` [PATCH 08/35] irqchip/gic-v3-its: Kill its->device_ids " Marc Zyngier
2019-09-23 18:25 ` [PATCH 09/35] irqchip/gic-v3-its: Add get_vlpi_map() helper Marc Zyngier
2019-09-23 18:25 ` [PATCH 10/35] irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation Marc Zyngier
2019-09-25 13:04 ` Zenghui Yu
2019-09-25 14:41 ` Marc Zyngier [this message]
2019-09-25 17:14 ` Marc Zyngier
2019-09-26 15:19 ` Zenghui Yu
2019-09-26 15:57 ` Marc Zyngier
2019-09-26 16:27 ` Marc Zyngier
2019-09-27 2:01 ` Zenghui Yu
2019-09-27 1:59 ` Zenghui Yu
2019-09-23 18:25 ` [PATCH 11/35] irqchip/gic-v4.1: Implement the v4.1 flavour of VMAPP Marc Zyngier
2019-09-23 18:25 ` [PATCH 12/35] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set Marc Zyngier
2019-09-23 18:25 ` [PATCH 13/35] irqchip/gic-v4.1: Implement the v4.1 flavour of VMOVP Marc Zyngier
2019-09-23 18:25 ` [PATCH 14/35] irqchip/gic-v4.1: Plumb skeletal VPE irqchip Marc Zyngier
2019-09-23 18:25 ` [PATCH 15/35] irqchip/gic-v4.1: Add mask/unmask doorbell callbacks Marc Zyngier
2019-09-23 18:25 ` [PATCH 16/35] irqchip/gic-v4.1: Add VPE residency callback Marc Zyngier
2019-09-23 18:25 ` [PATCH 17/35] irqchip/gic-v4.1: Add VPE eviction callback Marc Zyngier
2019-09-23 18:25 ` [PATCH 18/35] irqchip/gic-v4.1: Add VPE INVALL callback Marc Zyngier
2019-09-23 18:25 ` [PATCH 19/35] irqchip/gic-v4.1: Suppress per-VLPI doorbell Marc Zyngier
2019-09-23 18:25 ` [PATCH 20/35] irqchip/gic-v4.1: Allow direct invalidation of VLPIs Marc Zyngier
2019-09-28 2:02 ` Zenghui Yu
2019-09-30 9:20 ` Marc Zyngier
2019-09-30 9:40 ` Zenghui Yu
2019-09-23 18:25 ` [PATCH 21/35] irqchip/gic-v4.1: Advertise support v4.1 to KVM Marc Zyngier
2019-09-23 18:25 ` [PATCH 22/35] irqchip/gic-v4.1: Map the ITS SGIR register page Marc Zyngier
2019-09-23 18:25 ` [PATCH 23/35] irqchip/gic-v4.1: Plumb skeletal VSGI irqchip Marc Zyngier
2019-09-23 18:25 ` [PATCH 24/35] irqchip/gic-v4.1: Add initial SGI configuration Marc Zyngier
2019-09-28 2:20 ` Zenghui Yu
2019-09-28 3:07 ` Zenghui Yu
2019-09-23 18:25 ` [PATCH 25/35] irqchip/gic-v4.1: Plumb mask/unmask SGI callbacks Marc Zyngier
2019-09-23 18:25 ` [PATCH 26/35] irqchip/gic-v4.1: Plumb get/set_irqchip_state " Marc Zyngier
2019-09-23 18:25 ` [PATCH 27/35] irqchip/gic-v4.1: Plumb set_vcpu_affinity " Marc Zyngier
2019-09-23 18:25 ` [PATCH 28/35] irqchip/gic-v4.1: Move doorbell management to the GICv4 abstraction layer Marc Zyngier
2019-09-23 18:26 ` [PATCH 29/35] irqchip/gic-v4.1: Add VSGI allocation/teardown Marc Zyngier
2019-09-23 18:26 ` [PATCH 30/35] irqchip/gic-v4.1: Add VSGI property setup Marc Zyngier
2019-09-23 18:26 ` [PATCH 31/35] irqchip/gic-v4.1: Eagerly vmap vPEs Marc Zyngier
2019-09-28 3:11 ` Zenghui Yu
2019-09-30 9:23 ` Marc Zyngier
2019-09-23 18:26 ` [PATCH 32/35] KVM: arm64: GICv4.1: Let doorbells be auto-enabled Marc Zyngier
2019-09-23 18:26 ` [PATCH 33/35] KVM: arm64: GICv4.1: Add direct injection capability to SGI registers Marc Zyngier
2019-09-23 18:26 ` [PATCH 34/35] KVM: arm64: GICv4.1: Configure SGIs as HW interrupts Marc Zyngier
2019-09-23 18:26 ` [PATCH 35/35] KVM: arm64: GICv4.1: Expose HW-based SGIs in debugfs Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6f4ccdfd-4b63-04cb-e7c0-f069e620127f@kernel.org \
--to=maz@kernel.org \
--cc=jason@lakedaemon.net \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=tglx@linutronix.de \
--cc=yuzenghui@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).