lkml.org 
[lkml]   [2019]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 04/35] irqchip/gic-v3: Detect GICv4.1 supporting RVPEID
    On Tue, Sep 24, 2019 at 11:49:24AM +0100, Marc Zyngier wrote:
    > On 24/09/2019 11:24, Andrew Murray wrote:
    > > On Mon, Sep 23, 2019 at 07:25:35PM +0100, Marc Zyngier wrote:
    > >> GICv4.1 supports the RVPEID ("Residency per vPE ID"), which allows for
    > >> a much efficient way of making virtual CPUs resident (to allow direct
    > >> injection of interrupts).
    > >>
    > >> The functionnality needs to be discovered on each and every redistributor
    > >> in the system, and disabled if the settings are inconsistent.
    > >>
    > >> Signed-off-by: Marc Zyngier <maz@kernel.org>
    > >> ---
    > >> drivers/irqchip/irq-gic-v3.c | 21 ++++++++++++++++++---
    > >> include/linux/irqchip/arm-gic-v3.h | 2 ++
    > >> 2 files changed, 20 insertions(+), 3 deletions(-)
    > >>
    > >> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
    > >> index 422664ac5f53..0b545e2c3498 100644
    > >> --- a/drivers/irqchip/irq-gic-v3.c
    > >> +++ b/drivers/irqchip/irq-gic-v3.c
    > >> @@ -849,8 +849,21 @@ static int __gic_update_rdist_properties(struct redist_region *region,
    > >> void __iomem *ptr)
    > >> {
    > >> u64 typer = gic_read_typer(ptr + GICR_TYPER);
    > >> +
    > >> gic_data.rdists.has_vlpis &= !!(typer & GICR_TYPER_VLPIS);
    > >> - gic_data.rdists.has_direct_lpi &= !!(typer & GICR_TYPER_DirectLPIS);
    > >> +
    > >> + /* RVPEID implies some form of DirectLPI, no matter what the doc says... :-/ */
    > >
    > > I think the doc says, RVPEID is *always* 1 for GICv4.1 (and presumably beyond)
    > > and when RVPEID==1 then DirectLPI is *always* 0 - but that's OK because for
    > > GICv4.1 support for direct LPIs is mandatory.
    >
    > Well, v4.1 support for DirectLPI is pretty patchy. It has just enough
    > features to make it useful.
    >
    > >
    > >> + gic_data.rdists.has_rvpeid &= !!(typer & GICR_TYPER_RVPEID);
    > >> + gic_data.rdists.has_direct_lpi &= (!!(typer & GICR_TYPER_DirectLPIS) |
    > >> + gic_data.rdists.has_rvpeid);
    > >> +
    > >> + /* Detect non-sensical configurations */
    > >> + if (WARN_ON_ONCE(gic_data.rdists.has_rvpeid && !gic_data.rdists.has_vlpis)) {
    > >
    > > How feasible is the following suitation? All the redistributors in the system has
    > > vlpis=0, and only the first redistributor has rvpeid=1 (with the remaining ones
    > > rvpeid=0).If we evaluate this WARN_ON_ONCE on each call to
    > > __gic_update_rdist_properties we end up without direct LPI support, however if we
    > > evaluated this after iterating through all the redistributors then we'd end up
    > > with direct LPI support and a non-essential WARN.
    > >
    > > Should we do the WARN after iterating through all the redistributors once we
    > > know what the final values of these flags will be, perhaps in
    > > gic_update_rdist_properties?
    >
    > What does it gains us?

    It prevents an unnecessary WARN.

    If the first redistributor has rvpeid=1, vlpis=0, direct_lpi=1, and the others
    have rvpeid=0, vlpis=0, direct_lpi=0. At the end of iteration, without the
    WARN if statement, you end up wth rvpeid=0, vlpis=0, direct_lpi=0. I.e. it's
    done the right thing. In this use-case the WARN doesn't achieve anything other
    than give the user a pointless WARN. If the WARN was moved to after iteration
    then the WARN wouldn't fire.

    I have no idea how likely this use-case is.

    > The moment we've detected any inconsistency, any
    > use of DirectLPI or VLPIs is a big nono, because the redistributors care
    > not designed to communicate with each other, and we might as well do
    > that early. Frankly, the HW should have stayed in someone's lab. The
    > only reason I have that code in is to detect the FVP model being
    > misconfigured, which is pretty easy to do

    Thanks,

    Andrew Murray

    >
    > M.
    > --
    > Jazz is not dead, it just smells funny...

    \
     
     \ /
      Last update: 2019-09-24 13:01    [W:2.462 / U:0.316 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site