Message-ID: <ZPb8MPNa4KWitO+U@lpieralisi>
Date:   Tue, 5 Sep 2023 12:00:16 +0200
From:   Lorenzo Pieralisi <lpieralisi@...nel.org>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Fang Xiang <fangxiang3@...omi.com>, tglx@...utronix.de,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] irqchip/gic-v3-its: Mark GIC as non-shareable by OF
 property

On Mon, Sep 04, 2023 at 08:24:51AM +0100, Marc Zyngier wrote:
> On Mon, 04 Sep 2023 03:04:31 +0100,
> Fang Xiang <fangxiang3@...omi.com> wrote:
> > 
> > GIC600/GIC700 integration in some platforms does not support the
> > sharability feature. Commit a8707f553884 ("irqchip/gic-v3: Add
> > Rockchip 3588001 erratum workaround") added flags to indicate the
> > integration design error, but the quirk is just applied with
> > RK3588/RK3588S and GIC600.
> > 
> > The property are named "force-non-shareable" to be vendor agnostic,
> > since apparently similar integration design errors exist in other
> > platforms and they can reuse the same property in their GIC ITS
> > device-node.
> 
> No. If this is a problem, this must be dealt with in the following
> manner:
> 
> - a standard property, which is documented in the ITS binding
> - an equivalent ACPI property, equally documented
> - separate instances of the property for the core GIC and the ITS
>   (because the RDs are *not* defined at the ITS level)

I am about to post a patchset to start discussion on how to "fix"
these designs (in DT - ACPI will follow the approach we agree on),
let's take it from there.

Thanks,
Lorenzo

> > Signed-off-by: Fang Xiang <fangxiang3@...omi.com>
> > ---
> >  drivers/irqchip/irq-gic-v3-its.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> > index e0c2b10d154d..2e92eac1e5f3 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -5055,6 +5055,16 @@ static int __init its_compute_its_list_map(struct resource *res,
> >         return its_number;
> >  }
> > 
> > +static void its_of_update_shareability(struct its_node *its)
> > +{
> > +       if (!fwnode_property_present(its->fwnode_handle,
> > +                               "force-non-shareable"))
> > +               return;
> > +
> > +       its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
> > +       gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
> 
> There is no reason why we should conflate the two, other than your
> implementation being monolithic.
> 
> > +}
> > +
> >  static int __init its_probe_one(struct resource *res,
> >                                 struct fwnode_handle *handle, int numa_node)
> >  {
> > @@ -5130,6 +5140,7 @@ static int __init its_probe_one(struct resource *res,
> >         its->msi_domain_flags = IRQ_DOMAIN_FLAG_ISOLATED_MSI;
> > 
> >         its_enable_quirks(its);
> > +       its_of_update_shareability(its);
> 
> As mentioned above, we need a mechanism that works across firmware implementations.
> 
> > 
> >         err = its_alloc_tables(its);
> >         if (err)
> > --
> > 2.34.1
> > 
> > #/******±¾Óʼþ¼°Æ丽¼þº¬ÓÐСÃ×¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚ·¢Ë͸øÉÏÃæµØÖ·ÖÐÁгöµÄ¸öÈË»òȺ×é¡£½ûÖ¹ÈκÎÆäËûÈËÒÔÈκÎÐÎʽʹÓ㨰üÀ¨µ«²»ÏÞÓÚÈ«²¿»ò²¿·ÖµØй¶¡¢¸´ÖÆ¡¢»òÉ¢·¢£©±¾ÓʼþÖеÄÐÅÏ¢¡£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇëÄúÁ¢¼´µç»°»òÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ£¡ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
> \
> 
> Please fix your email setup.
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

Powered by blists - more mailing lists