gic700 shareability question

gic700 shareability question

[Peng Fan]

Hi Marc,

We have an SoC that use GIC-700, but not support shareability,
Currently I just hack the code as below. Do you think it is feasible
to add firmware bindings such that these can be used to define 
the correct shareability/cacheability instead of relying on the
programmability of the CBASER register?

Saying with "broken-shareability", we just clear all the shareability
settings.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 973ede0197e3..56c4eaf20029 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2359,6 +2359,9 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
        its_write_baser(its, baser, val);
        tmp = baser->val;

+       if (tmp & GITS_BASER_SHAREABILITY_MASK)
+                       tmp &= ~GITS_BASER_SHAREABILITY_MASK;
+
        if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
                /*
                 * Shareability didn't stick. Just use
@@ -3096,6 +3099,8 @@ static void its_cpu_init_lpis(void)
        gicr_write_propbaser(val, rbase + GICR_PROPBASER);
        tmp = gicr_read_propbaser(rbase + GICR_PROPBASER);

+       tmp &= ~GICR_PROPBASER_SHAREABILITY_MASK;
+
        if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) {
                if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) {
                        /*
@@ -3120,6 +3125,7 @@ static void its_cpu_init_lpis(void)
        gicr_write_pendbaser(val, rbase + GICR_PENDBASER);
        tmp = gicr_read_pendbaser(rbase + GICR_PENDBASER);

+       tmp &= ~GICR_PENDBASER_SHAREABILITY_MASK;
        if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) {
                /*
                 * The HW reports non-shareable, we must remove the
@@ -5094,6 +5100,7 @@ static int __init its_probe_one(struct resource *res,

        gits_write_cbaser(baser, its->base + GITS_CBASER);
        tmp = gits_read_cbaser(its->base + GITS_CBASER);
+       tmp &= ~GITS_CBASER_SHAREABILITY_MASK;

        if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
                if (!(tmp & GITS_CBASER_SHAREABILITY_MASK)) {

Thanks,
Peng.

Re: gic700 shareability question

[Marc Zyngier]

+ Lorenzo

On Tue, 28 Mar 2023 13:48:19 +0100,
Peng Fan <peng.fan@nxp.com> wrote:
> 
> Hi Marc,
> 
> We have an SoC that use GIC-700, but not support shareability,

Define this. The IP does support shareability, but your integration
doesn't?

> Currently I just hack the code as below. Do you think it is feasible
> to add firmware bindings such that these can be used to define 
> the correct shareability/cacheability instead of relying on the
> programmability of the CBASER register?
> 
> Saying with "broken-shareability", we just clear all the shareability
> settings.

This is the same thing as the Rockchip crap, so you are in good
company.

I've repeatedly stated that this needs to be handled:

- either by describing the full system topology and describe what is
  in the same inner-shareable domain as the CPUs, which needs to
  encompass both DT and ACPI (starting with DT seems reasonable),

- or as a SoC specific erratum, but not as a general "sh*t happened"
  property.

AFAIK, Lorenzo is looking into this.

	M.

-- 
Without deviation from the norm, progress is not possible.

RE: gic700 shareability question

[Peng Fan]

Hi Marc,

> Subject: Re: gic700 shareability question
> 
> + Lorenzo
> 
> On Tue, 28 Mar 2023 13:48:19 +0100,
> Peng Fan <peng.fan@nxp.com> wrote:
> >
> > Hi Marc,
> >
> > We have an SoC that use GIC-700, but not support shareability,
> 
> Define this. The IP does support shareability, but your integration doesn't?
> 
> > Currently I just hack the code as below. Do you think it is feasible
> > to add firmware bindings such that these can be used to define the
> > correct shareability/cacheability instead of relying on the
> > programmability of the CBASER register?
> >
> > Saying with "broken-shareability", we just clear all the shareability
> > settings.
> 
> This is the same thing as the Rockchip crap, so you are in good company.
> 
> I've repeatedly stated that this needs to be handled:
> 
> - either by describing the full system topology and describe what is
>   in the same inner-shareable domain as the CPUs, which needs to
>   encompass both DT and ACPI (starting with DT seems reasonable),
> 

We will give a look on this. But honestly not have a good idea on how.

> - or as a SoC specific erratum, but not as a general "sh*t happened"
>   property.

I will ask the hardware team to create an errata.
> 
> AFAIK, Lorenzo is looking into this.

Lorenzo, are you working on this? 

Thanks,
Peng.
> 
> 	M.
> 
> --
> Without deviation from the norm, progress is not possible.

Re: gic700 shareability question

[Marc Zyngier]

On Mon, 03 Apr 2023 02:36:31 +0100,
Peng Fan <peng.fan@nxp.com> wrote:
> 
> Hi Marc,
> 
> > Subject: Re: gic700 shareability question
> > 
> > + Lorenzo
> > 
> > On Tue, 28 Mar 2023 13:48:19 +0100,
> > Peng Fan <peng.fan@nxp.com> wrote:
> > >
> > > Hi Marc,
> > >
> > > We have an SoC that use GIC-700, but not support shareability,
> > 
> > Define this. The IP does support shareability, but your integration doesn't?
> > 
> > > Currently I just hack the code as below. Do you think it is feasible
> > > to add firmware bindings such that these can be used to define the
> > > correct shareability/cacheability instead of relying on the
> > > programmability of the CBASER register?
> > >
> > > Saying with "broken-shareability", we just clear all the shareability
> > > settings.
> > 
> > This is the same thing as the Rockchip crap, so you are in good company.
> > 
> > I've repeatedly stated that this needs to be handled:
> > 
> > - either by describing the full system topology and describe what is
> >   in the same inner-shareable domain as the CPUs, which needs to
> >   encompass both DT and ACPI (starting with DT seems reasonable),
> > 
> 
> We will give a look on this. But honestly not have a good idea on how.

For each node that can initiate memory transactions in the system, you
have a phandle to a node that describe the shareability. In your case,
you would have two nodes: one inner-shareable with at least the CPUs
and whatever IP block that is in the same IS domain, and another that
describe the outer-shareable domain.

Or another variation on the same theme.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: gic700 shareability question

[Lorenzo Pieralisi]

On Mon, Apr 03, 2023 at 01:36:31AM +0000, Peng Fan wrote:
> Hi Marc,
> 
> > Subject: Re: gic700 shareability question
> > 
> > + Lorenzo
> > 
> > On Tue, 28 Mar 2023 13:48:19 +0100,
> > Peng Fan <peng.fan@nxp.com> wrote:
> > >
> > > Hi Marc,
> > >
> > > We have an SoC that use GIC-700, but not support shareability,
> > 
> > Define this. The IP does support shareability, but your integration doesn't?
> > 
> > > Currently I just hack the code as below. Do you think it is feasible
> > > to add firmware bindings such that these can be used to define the
> > > correct shareability/cacheability instead of relying on the
> > > programmability of the CBASER register?
> > >
> > > Saying with "broken-shareability", we just clear all the shareability
> > > settings.
> > 
> > This is the same thing as the Rockchip crap, so you are in good company.
> > 
> > I've repeatedly stated that this needs to be handled:
> > 
> > - either by describing the full system topology and describe what is
> >   in the same inner-shareable domain as the CPUs, which needs to
> >   encompass both DT and ACPI (starting with DT seems reasonable),
> > 
> 
> We will give a look on this. But honestly not have a good idea on how.

It is a longer term fix for the issue, we are looking into this.

> > - or as a SoC specific erratum, but not as a general "sh*t happened"
> >   property.
> 
> I will ask the hardware team to create an errata.
> > 
> > AFAIK, Lorenzo is looking into this.
> 
> Lorenzo, are you working on this? 

Yes it is being worked on, that does not prevent though an errata
workaround to be applied, firmware bindings definitions can take
a while to sort out.

Lorenzo

RE: gic700 shareability question

[Peng Fan]

> Subject: Re: gic700 shareability question
> 
> On Mon, Apr 03, 2023 at 01:36:31AM +0000, Peng Fan wrote:
> > Hi Marc,
> >
> > > Subject: Re: gic700 shareability question
> > >
> > > + Lorenzo
> > >
> > > On Tue, 28 Mar 2023 13:48:19 +0100,
> > > Peng Fan <peng.fan@nxp.com> wrote:
> > > >
> > > > Hi Marc,
> > > >
> > > > We have an SoC that use GIC-700, but not support shareability,
> > >
> > > Define this. The IP does support shareability, but your integration
> doesn't?
> > >
> > > > Currently I just hack the code as below. Do you think it is
> > > > feasible to add firmware bindings such that these can be used to
> > > > define the correct shareability/cacheability instead of relying on
> > > > the programmability of the CBASER register?
> > > >
> > > > Saying with "broken-shareability", we just clear all the
> > > > shareability settings.
> > >
> > > This is the same thing as the Rockchip crap, so you are in good company.
> > >
> > > I've repeatedly stated that this needs to be handled:
> > >
> > > - either by describing the full system topology and describe what is
> > >   in the same inner-shareable domain as the CPUs, which needs to
> > >   encompass both DT and ACPI (starting with DT seems reasonable),
> > >
> >
> > We will give a look on this. But honestly not have a good idea on how.
> 
> It is a longer term fix for the issue, we are looking into this.
> 
> > > - or as a SoC specific erratum, but not as a general "sh*t happened"
> > >   property.
> >
> > I will ask the hardware team to create an errata.
> > >
> > > AFAIK, Lorenzo is looking into this.
> >
> > Lorenzo, are you working on this?
> 
> Yes it is being worked on, that does not prevent though an errata
> workaround to be applied, firmware bindings definitions can take a while to
> sort out.

Sure, we need go with errata. Thanks for working on this.

Thanks,
Peng.
> 
> Lorenzo