On Tue, 29 Jan 2019 01:32:55 +0000,
Heyi Guo <guoh...@huawei.com> wrote:
>
> Hi Marc,
>
> Any comments?
None so far, I've queued this to let it soak, and will send it as a
fix if nothing else breaks.
Thanks,
M.
>
> Thanks,
>
> Heyi
>
>
> On 2019/1/24 21:37, Heyi Guo wrote:
> > 1. In current implementation, every VLPI will temporarily be mapped to
> > the first CPU in system (normally CPU0) and then moved to the real
> > scheduled CPU later.
> >
> > 2. So there is a time window and a VLPI may be sent to CPU0 instead of
> > the real scheduled vCPU, in a multi-CPU virtual machine.
> >
> > 3. However, CPU0 may have not been scheduled as a virtual CPU after
> > system boots up, so the value of its GICR_VPROPBASER is unknown at
> > that moment.
> >
> > 4. If the INTID of VLPI is larger than 2^(GICR_VPROPBASER.IDbits+1),
> > while IDbits is also in unknown state, GIC will behave as if the VLPI
> > is out of range and simply drop it, which results in interrupt missing
> > in Guest.
> >
> > As no code will clear GICR_VPROPBASER at runtime, we can safely
> > initialize the IDbits field at boot time for each CPU to get rid of
> > this issue.
> >
> > We also clear Valid bit of GICR_VPENDBASER in case any ancient
> > programming gets left in and causes memory corrupting. A new function
> > its_clear_vpend_valid() is added to reuse the code in
> > its_vpe_deschedule().
> >
> > Signed-off-by: Heyi Guo <guoh...@huawei.com>
> > Signed-off-by: Heyi Guo <heyi....@linaro.org>
> > ---
> >
> > Notes:
> > v2:
> > - Also wait GICR_VPENDBASER.Dirty bit to be 0 in initialization [Marc]
> > - Add a new function to reuse the code in its_vpe_deschedule() [Marc]
> >
> > drivers/irqchip/irq-gic-v3-its.c | 66
> > +++++++++++++++++++++++++++++-----------
> > 1 file changed, 49 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c
> > b/drivers/irqchip/irq-gic-v3-its.c
> > index 3365d44..531e945 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -2060,6 +2060,29 @@ static int __init allocate_lpi_tables(void)
> > return 0;
> > }
> > +static u64 its_clear_vpend_valid(void __iomem *vlpi_base)
> > +{
> > + u32 count = 1000000; /* 1s! */
> > + bool clean;
> > + u64 val;
> > +
> > + val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> > + val &= ~GICR_VPENDBASER_Valid;
> > + gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
> > +
> > + do {
> > + val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> > + clean = !(val & GICR_VPENDBASER_Dirty);
> > + if (!clean) {
> > + count--;
> > + cpu_relax();
> > + udelay(1);
> > + }
> > + } while (!clean && count);
> > +
> > + return val;
> > +}
> > +
> > static void its_cpu_init_lpis(void)
> > {
> > void __iomem *rbase = gic_data_rdist_rd_base();
> > @@ -2145,6 +2168,30 @@ static void its_cpu_init_lpis(void)
> > val |= GICR_CTLR_ENABLE_LPIS;
> > writel_relaxed(val, rbase + GICR_CTLR);
> > + if (gic_rdists->has_vlpis) {
> > + void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
> > +
> > + /*
> > + * It's possible for CPU to receive VLPIs before it is
> > + * sheduled as a vPE, especially for the first CPU, and the
> > + * VLPI with INTID larger than 2^(IDbits+1) will be considered
> > + * as out of range and dropped by GIC.
> > + * So we initialize IDbits to known value to avoid VLPI drop.
> > + */
> > + val = (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
> > + pr_debug("GICv4: CPU%d: Init IDbits to 0x%llx for
> > GICR_VPROPBASER\n",
> > + smp_processor_id(), val);
> > + gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
> > +
> > + /*
> > + * Also clear Valid bit of GICR_VPENDBASER, in case some
> > + * ancient programming gets left in and has possibility of
> > + * corrupting memory.
> > + */
> > + val = its_clear_vpend_valid(vlpi_base);
> > + WARN_ON(val & GICR_VPENDBASER_Dirty);
> > + }
> > +
> > /* Make sure the GIC has seen the above */
> > dsb(sy);
> > out:
> > @@ -2757,26 +2804,11 @@ static void its_vpe_schedule(struct its_vpe *vpe)
> > static void its_vpe_deschedule(struct its_vpe *vpe)
> > {
> > void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
> > - u32 count = 1000000; /* 1s! */
> > - bool clean;
> > u64 val;
> > - /* We're being scheduled out */
> > - val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> > - val &= ~GICR_VPENDBASER_Valid;
> > - gits_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
> > -
> > - do {
> > - val = gits_read_vpendbaser(vlpi_base + GICR_VPENDBASER);
> > - clean = !(val & GICR_VPENDBASER_Dirty);
> > - if (!clean) {
> > - count--;
> > - cpu_relax();
> > - udelay(1);
> > - }
> > - } while (!clean && count);
> > + val = its_clear_vpend_valid(vlpi_base);
> > - if (unlikely(!clean && !count)) {
> > + if (unlikely(val & GICR_VPENDBASER_Dirty)) {
> > pr_err_ratelimited("ITS virtual pending table not cleaning\n");
> > vpe->idai = false;
> > vpe->pending_last = true;
>
>
--
Jazz is not dead, it just smell funny.
