2023-03-28 12:51:15

by Peng Fan

[permalink] [raw]
Subject: gic700 shareability question

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.


2023-03-31 10:25:29

by Marc Zyngier

[permalink] [raw]
Subject: Re: gic700 shareability question

+ Lorenzo

On Tue, 28 Mar 2023 13:48:19 +0100,
Peng Fan <[email protected]> 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.

2023-04-03 01:49:50

by Peng Fan

[permalink] [raw]
Subject: RE: gic700 shareability question

Hi Marc,

> Subject: Re: gic700 shareability question
>
> + Lorenzo
>
> On Tue, 28 Mar 2023 13:48:19 +0100,
> Peng Fan <[email protected]> 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.

2023-04-03 08:12:33

by Marc Zyngier

[permalink] [raw]
Subject: Re: gic700 shareability question

On Mon, 03 Apr 2023 02:36:31 +0100,
Peng Fan <[email protected]> wrote:
>
> Hi Marc,
>
> > Subject: Re: gic700 shareability question
> >
> > + Lorenzo
> >
> > On Tue, 28 Mar 2023 13:48:19 +0100,
> > Peng Fan <[email protected]> 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.

2023-04-03 08:35:23

by Lorenzo Pieralisi

[permalink] [raw]
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 <[email protected]> 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

2023-04-03 09:16:08

by Peng Fan

[permalink] [raw]
Subject: RE: gic700 shareability question

> 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 <[email protected]> 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