diff mbox series

[2/5] hw/ppc/pnv_xscom: PBA bar mask values are incorrect with homer/occ sizes

Message ID 20191119175056.32518-3-bala24@linux.ibm.com
State New
Headers show
Series ppc/pnv: fix Homer/Occ mappings on multichip systems | expand

Commit Message

Balamuruhan S Nov. 19, 2019, 5:50 p.m. UTC 
homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
and from xscom access should return correct mask values instead of actual
sizes.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
 hw/ppc/pnv_xscom.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

David Gibson Nov. 19, 2019, 9:56 p.m. UTC | #1 
On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> and from xscom access should return correct mask values instead of actual
> sizes.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
>  hw/ppc/pnv_xscom.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index f01d788a65..cdd5fa356e 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -46,6 +46,10 @@
>  #define P9_PBA_BARMASK0                 0x5012b04
>  #define P9_PBA_BARMASK2                 0x5012b06
>  
> +/* Mask to calculate Homer/Occ size */
> +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> +#define OCC_SIZE_MASK                   0x0000000000700000ull

Uuuhhhhh... AFAICT these defines have identical values to
PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
patch is actually changing.


>  static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
>  {
>      /*
> @@ -90,9 +94,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>          return PNV_HOMER_BASE(chip);
>  
>      case P9_PBA_BARMASK0: /* P9 homer region size */
> -        return PNV9_HOMER_SIZE;
>      case P8_PBA_BARMASK0: /* P8 homer region size */
> -        return PNV_HOMER_SIZE;
> +        return HOMER_SIZE_MASK;
>  
>      case P9_PBA_BAR2: /* P9 occ common area */
>          return PNV9_OCC_COMMON_AREA(chip);
> @@ -100,9 +103,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>          return PNV_OCC_COMMON_AREA(chip);
>  
>      case P9_PBA_BARMASK2: /* P9 occ common area size */
> -        return PNV9_OCC_COMMON_AREA_SIZE;
>      case P8_PBA_BARMASK2: /* P8 occ common area size */
> -        return PNV_OCC_COMMON_AREA_SIZE;
> +        return OCC_SIZE_MASK;
>  
>      case 0x1010c00:     /* PIBAM FIR */
>      case 0x1010c03:     /* PIBAM FIR MASK */
David Gibson Nov. 19, 2019, 10 p.m. UTC | #2 
On Wed, Nov 20, 2019 at 08:56:18AM +1100, David Gibson wrote:
> On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > and from xscom access should return correct mask values instead of actual
> > sizes.
> > 
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> >  hw/ppc/pnv_xscom.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > index f01d788a65..cdd5fa356e 100644
> > --- a/hw/ppc/pnv_xscom.c
> > +++ b/hw/ppc/pnv_xscom.c
> > @@ -46,6 +46,10 @@
> >  #define P9_PBA_BARMASK0                 0x5012b04
> >  #define P9_PBA_BARMASK2                 0x5012b06
> >  
> > +/* Mask to calculate Homer/Occ size */
> > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> 
> Uuuhhhhh... AFAICT these defines have identical values to
> PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
> patch is actually changing.

Oh, sorry, missed that the values were changed in 1/5.  Would have
been easier to follow if the two patches were folded together, but
never mind.  Applied.

> 
> 
> >  static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
> >  {
> >      /*
> > @@ -90,9 +94,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> >          return PNV_HOMER_BASE(chip);
> >  
> >      case P9_PBA_BARMASK0: /* P9 homer region size */
> > -        return PNV9_HOMER_SIZE;
> >      case P8_PBA_BARMASK0: /* P8 homer region size */
> > -        return PNV_HOMER_SIZE;
> > +        return HOMER_SIZE_MASK;
> >  
> >      case P9_PBA_BAR2: /* P9 occ common area */
> >          return PNV9_OCC_COMMON_AREA(chip);
> > @@ -100,9 +103,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> >          return PNV_OCC_COMMON_AREA(chip);
> >  
> >      case P9_PBA_BARMASK2: /* P9 occ common area size */
> > -        return PNV9_OCC_COMMON_AREA_SIZE;
> >      case P8_PBA_BARMASK2: /* P8 occ common area size */
> > -        return PNV_OCC_COMMON_AREA_SIZE;
> > +        return OCC_SIZE_MASK;
> >  
> >      case 0x1010c00:     /* PIBAM FIR */
> >      case 0x1010c03:     /* PIBAM FIR MASK */
>
David Gibson Nov. 19, 2019, 10:02 p.m. UTC | #3 
On Wed, Nov 20, 2019 at 09:00:32AM +1100, David Gibson wrote:
> On Wed, Nov 20, 2019 at 08:56:18AM +1100, David Gibson wrote:
> > On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> > > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > > and from xscom access should return correct mask values instead of actual
> > > sizes.
> > > 
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > > ---
> > >  hw/ppc/pnv_xscom.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > > index f01d788a65..cdd5fa356e 100644
> > > --- a/hw/ppc/pnv_xscom.c
> > > +++ b/hw/ppc/pnv_xscom.c
> > > @@ -46,6 +46,10 @@
> > >  #define P9_PBA_BARMASK0                 0x5012b04
> > >  #define P9_PBA_BARMASK2                 0x5012b06
> > >  
> > > +/* Mask to calculate Homer/Occ size */
> > > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> > 
> > Uuuhhhhh... AFAICT these defines have identical values to
> > PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
> > patch is actually changing.
> 
> Oh, sorry, missed that the values were changed in 1/5.  Would have
> been easier to follow if the two patches were folded together, but
> never mind.  Applied.

Ugh.. or not.  The first patch doesn't apply for me, looks like it
needs a rebase (or you have something else in your tree I don't know
about).
Balamuruhan S Nov. 20, 2019, 3:01 a.m. UTC | #4 
On Wed, Nov 20, 2019 at 09:02:26AM +1100, David Gibson wrote:
> On Wed, Nov 20, 2019 at 09:00:32AM +1100, David Gibson wrote:
> > On Wed, Nov 20, 2019 at 08:56:18AM +1100, David Gibson wrote:
> > > On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> > > > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > > > and from xscom access should return correct mask values instead of actual
> > > > sizes.
> > > > 
> > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > > > ---
> > > >  hw/ppc/pnv_xscom.c | 10 ++++++----
> > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > > > index f01d788a65..cdd5fa356e 100644
> > > > --- a/hw/ppc/pnv_xscom.c
> > > > +++ b/hw/ppc/pnv_xscom.c
> > > > @@ -46,6 +46,10 @@
> > > >  #define P9_PBA_BARMASK0                 0x5012b04
> > > >  #define P9_PBA_BARMASK2                 0x5012b06
> > > >  
> > > > +/* Mask to calculate Homer/Occ size */
> > > > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > > > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> > > 
> > > Uuuhhhhh... AFAICT these defines have identical values to
> > > PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
> > > patch is actually changing.
> > 
> > Oh, sorry, missed that the values were changed in 1/5.  Would have
> > been easier to follow if the two patches were folded together, but
> > never mind.  Applied.
> 
> Ugh.. or not.  The first patch doesn't apply for me, looks like it
> needs a rebase (or you have something else in your tree I don't know
> about).

I checked out from upstream master (commit: 8937f17249) and worked on
it.

-- Bala
> 
> -- 
> David Gibson			| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> 				| _way_ _around_!
> http://www.ozlabs.org/~dgibson
Balamuruhan S Nov. 20, 2019, 3:16 a.m. UTC | #5 
On Wed, Nov 20, 2019 at 08:31:03AM +0530, Balamuruhan S wrote:
> On Wed, Nov 20, 2019 at 09:02:26AM +1100, David Gibson wrote:
> > On Wed, Nov 20, 2019 at 09:00:32AM +1100, David Gibson wrote:
> > > On Wed, Nov 20, 2019 at 08:56:18AM +1100, David Gibson wrote:
> > > > On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> > > > > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > > > > and from xscom access should return correct mask values instead of actual
> > > > > sizes.
> > > > > 
> > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > > > > ---
> > > > >  hw/ppc/pnv_xscom.c | 10 ++++++----
> > > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > > > > index f01d788a65..cdd5fa356e 100644
> > > > > --- a/hw/ppc/pnv_xscom.c
> > > > > +++ b/hw/ppc/pnv_xscom.c
> > > > > @@ -46,6 +46,10 @@
> > > > >  #define P9_PBA_BARMASK0                 0x5012b04
> > > > >  #define P9_PBA_BARMASK2                 0x5012b06
> > > > >  
> > > > > +/* Mask to calculate Homer/Occ size */
> > > > > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > > > > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> > > > 
> > > > Uuuhhhhh... AFAICT these defines have identical values to
> > > > PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
> > > > patch is actually changing.
> > > 
> > > Oh, sorry, missed that the values were changed in 1/5.  Would have
> > > been easier to follow if the two patches were folded together, but
> > > never mind.  Applied.
> > 
> > Ugh.. or not.  The first patch doesn't apply for me, looks like it
> > needs a rebase (or you have something else in your tree I don't know
> > about).
> 
> I checked out from upstream master (commit: 8937f17249 ) and worked on
                                                 ^
						 |
ignore this its my first patch commit id (1/5)----

> it.

upstream commit id: f086f22d6c (VFIO fixes 2019-11-18), please let me
know if you would like me to respin the patches after rebase with v4.2.0
rc2 release.

-- Bala

> 
> -- Bala
> > 
> > -- 
> > David Gibson			| I'll have my music baroque, and my code
> > david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> > 				| _way_ _around_!
> > http://www.ozlabs.org/~dgibson
> 
> 
>
Cédric Le Goater Nov. 20, 2019, 7:18 a.m. UTC | #6 
On 19/11/2019 18:50, Balamuruhan S wrote:
> homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> and from xscom access should return correct mask values instead of actual
> sizes.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
>  hw/ppc/pnv_xscom.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index f01d788a65..cdd5fa356e 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -46,6 +46,10 @@
>  #define P9_PBA_BARMASK0                 0x5012b04
>  #define P9_PBA_BARMASK2                 0x5012b06
>  
> +/* Mask to calculate Homer/Occ size */
> +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> +#define OCC_SIZE_MASK                   0x0000000000700000ull
> +

Can't we deduce these values from the size ? 

>  static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
>  {
>      /*
> @@ -90,9 +94,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>          return PNV_HOMER_BASE(chip);
>  
>      case P9_PBA_BARMASK0: /* P9 homer region size */
> -        return PNV9_HOMER_SIZE;
>      case P8_PBA_BARMASK0: /* P8 homer region size */
> -        return PNV_HOMER_SIZE;
> +        return HOMER_SIZE_MASK;

I would prefer to move all the HOMER accesses in a XSCOM region
under the PnvHomer model than expanding the default handlers. 
You will need a different set of handlers for P8 and P9 and a 
different mapping address also. 

Could you do that please ? 
  
>      case P9_PBA_BAR2: /* P9 occ common area */
>          return PNV9_OCC_COMMON_AREA(chip);
> @@ -100,9 +103,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>          return PNV_OCC_COMMON_AREA(chip);
>  
>      case P9_PBA_BARMASK2: /* P9 occ common area size */
> -        return PNV9_OCC_COMMON_AREA_SIZE;
>      case P8_PBA_BARMASK2: /* P8 occ common area size */

Shouldn't that be PBA_*3 under P8 ? 

C. 

> -        return PNV_OCC_COMMON_AREA_SIZE;
> +        return OCC_SIZE_MASK;
>  
>      case 0x1010c00:     /* PIBAM FIR */
>      case 0x1010c03:     /* PIBAM FIR MASK */
>
Greg Kurz Nov. 20, 2019, 7:59 a.m. UTC | #7 
On Wed, 20 Nov 2019 08:46:51 +0530
Balamuruhan S <bala24@linux.ibm.com> wrote:

> On Wed, Nov 20, 2019 at 08:31:03AM +0530, Balamuruhan S wrote:
> > On Wed, Nov 20, 2019 at 09:02:26AM +1100, David Gibson wrote:
> > > On Wed, Nov 20, 2019 at 09:00:32AM +1100, David Gibson wrote:
> > > > On Wed, Nov 20, 2019 at 08:56:18AM +1100, David Gibson wrote:
> > > > > On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> > > > > > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > > > > > and from xscom access should return correct mask values instead of actual
> > > > > > sizes.
> > > > > > 
> > > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > > > > > ---
> > > > > >  hw/ppc/pnv_xscom.c | 10 ++++++----
> > > > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > > > > > index f01d788a65..cdd5fa356e 100644
> > > > > > --- a/hw/ppc/pnv_xscom.c
> > > > > > +++ b/hw/ppc/pnv_xscom.c
> > > > > > @@ -46,6 +46,10 @@
> > > > > >  #define P9_PBA_BARMASK0                 0x5012b04
> > > > > >  #define P9_PBA_BARMASK2                 0x5012b06
> > > > > >  
> > > > > > +/* Mask to calculate Homer/Occ size */
> > > > > > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > > > > > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> > > > > 
> > > > > Uuuhhhhh... AFAICT these defines have identical values to
> > > > > PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
> > > > > patch is actually changing.
> > > > 
> > > > Oh, sorry, missed that the values were changed in 1/5.  Would have
> > > > been easier to follow if the two patches were folded together, but
> > > > never mind.  Applied.
> > > 
> > > Ugh.. or not.  The first patch doesn't apply for me, looks like it
> > > needs a rebase (or you have something else in your tree I don't know
> > > about).
> > 
> > I checked out from upstream master (commit: 8937f17249 ) and worked on
>                                                  ^
> 						 |
> ignore this its my first patch commit id (1/5)----
> 
> > it.
> 
> upstream commit id: f086f22d6c (VFIO fixes 2019-11-18), please let me
> know if you would like me to respin the patches after rebase with v4.2.0
> rc2 release.
> 

Or maybe base your patches on David's ppc-for-5.0 branch available at
https://github.com/dgibson/qemu .

> -- Bala
> 
> > 
> > -- Bala
> > > 
> > > -- 
> > > David Gibson			| I'll have my music baroque, and my code
> > > david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> > > 				| _way_ _around_!
> > > http://www.ozlabs.org/~dgibson
> > 
> > 
> > 
>
Balamuruhan S Nov. 21, 2019, 8:34 a.m. UTC | #8 
On Wed, Nov 20, 2019 at 08:59:40AM +0100, Greg Kurz wrote:
> On Wed, 20 Nov 2019 08:46:51 +0530
> Balamuruhan S <bala24@linux.ibm.com> wrote:
> 
> > On Wed, Nov 20, 2019 at 08:31:03AM +0530, Balamuruhan S wrote:
> > > On Wed, Nov 20, 2019 at 09:02:26AM +1100, David Gibson wrote:
> > > > On Wed, Nov 20, 2019 at 09:00:32AM +1100, David Gibson wrote:
> > > > > On Wed, Nov 20, 2019 at 08:56:18AM +1100, David Gibson wrote:
> > > > > > On Tue, Nov 19, 2019 at 11:20:53PM +0530, Balamuruhan S wrote:
> > > > > > > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > > > > > > and from xscom access should return correct mask values instead of actual
> > > > > > > sizes.
> > > > > > > 
> > > > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > > > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > > > > > > ---
> > > > > > >  hw/ppc/pnv_xscom.c | 10 ++++++----
> > > > > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > > > > > > index f01d788a65..cdd5fa356e 100644
> > > > > > > --- a/hw/ppc/pnv_xscom.c
> > > > > > > +++ b/hw/ppc/pnv_xscom.c
> > > > > > > @@ -46,6 +46,10 @@
> > > > > > >  #define P9_PBA_BARMASK0                 0x5012b04
> > > > > > >  #define P9_PBA_BARMASK2                 0x5012b06
> > > > > > >  
> > > > > > > +/* Mask to calculate Homer/Occ size */
> > > > > > > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > > > > > > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> > > > > > 
> > > > > > Uuuhhhhh... AFAICT these defines have identical values to
> > > > > > PNV_HOMER_SIZE and PNV_OCC_COMMON_AREA_SIZE, so I don't see what this
> > > > > > patch is actually changing.
> > > > > 
> > > > > Oh, sorry, missed that the values were changed in 1/5.  Would have
> > > > > been easier to follow if the two patches were folded together, but
> > > > > never mind.  Applied.
> > > > 
> > > > Ugh.. or not.  The first patch doesn't apply for me, looks like it
> > > > needs a rebase (or you have something else in your tree I don't know
> > > > about).
> > > 
> > > I checked out from upstream master (commit: 8937f17249 ) and worked on
> >                                                  ^
> > 						 |
> > ignore this its my first patch commit id (1/5)----
> > 
> > > it.
> > 
> > upstream commit id: f086f22d6c (VFIO fixes 2019-11-18), please let me
> > know if you would like me to respin the patches after rebase with v4.2.0
> > rc2 release.
> > 
> 
> Or maybe base your patches on David's ppc-for-5.0 branch available at
> https://github.com/dgibson/qemu .

Thanks Greg, going forward I will base my patches on David's recent branch for
the release.

> 
> > -- Bala
> > 
> > > 
> > > -- Bala
> > > > 
> > > > -- 
> > > > David Gibson			| I'll have my music baroque, and my code
> > > > david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> > > > 				| _way_ _around_!
> > > > http://www.ozlabs.org/~dgibson
> > > 
> > > 
> > > 
> > 
>
Balamuruhan S Nov. 21, 2019, 8:37 a.m. UTC | #9 
On Wed, Nov 20, 2019 at 08:18:38AM +0100, Cédric Le Goater wrote:
> On 19/11/2019 18:50, Balamuruhan S wrote:
> > homer/occ sizes are calculated in skiboot with `(mask | 0xfffff) + 1`,
> > and from xscom access should return correct mask values instead of actual
> > sizes.
> > 
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> >  hw/ppc/pnv_xscom.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> > index f01d788a65..cdd5fa356e 100644
> > --- a/hw/ppc/pnv_xscom.c
> > +++ b/hw/ppc/pnv_xscom.c
> > @@ -46,6 +46,10 @@
> >  #define P9_PBA_BARMASK0                 0x5012b04
> >  #define P9_PBA_BARMASK2                 0x5012b06
> >  
> > +/* Mask to calculate Homer/Occ size */
> > +#define HOMER_SIZE_MASK                 0x0000000000300000ull
> > +#define OCC_SIZE_MASK                   0x0000000000700000ull
> > +
> 
> Can't we deduce these values from the size ? 

yes, that's a better way.

> 
> >  static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
> >  {
> >      /*
> > @@ -90,9 +94,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> >          return PNV_HOMER_BASE(chip);
> >  
> >      case P9_PBA_BARMASK0: /* P9 homer region size */
> > -        return PNV9_HOMER_SIZE;
> >      case P8_PBA_BARMASK0: /* P8 homer region size */
> > -        return PNV_HOMER_SIZE;
> > +        return HOMER_SIZE_MASK;
> 
> I would prefer to move all the HOMER accesses in a XSCOM region
> under the PnvHomer model than expanding the default handlers. 
> You will need a different set of handlers for P8 and P9 and a 
> different mapping address also. 
> 
> Could you do that please ? 

Sure Cedric, I can work on it.

>   
> >      case P9_PBA_BAR2: /* P9 occ common area */
> >          return PNV9_OCC_COMMON_AREA(chip);
> > @@ -100,9 +103,8 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> >          return PNV_OCC_COMMON_AREA(chip);
> >  
> >      case P9_PBA_BARMASK2: /* P9 occ common area size */
> > -        return PNV9_OCC_COMMON_AREA_SIZE;
> >      case P8_PBA_BARMASK2: /* P8 occ common area size */
> 
> Shouldn't that be PBA_*3 under P8 ? 

:( It's a miss from me. Thanks!

> 
> C. 
> 
> > -        return PNV_OCC_COMMON_AREA_SIZE;
> > +        return OCC_SIZE_MASK;
> >  
> >      case 0x1010c00:     /* PIBAM FIR */
> >      case 0x1010c03:     /* PIBAM FIR MASK */
> > 
>
diff mbox series

Patch

diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index f01d788a65..cdd5fa356e 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -46,6 +46,10 @@ 
 #define P9_PBA_BARMASK0                 0x5012b04
 #define P9_PBA_BARMASK2                 0x5012b06
 
+/* Mask to calculate Homer/Occ size */
+#define HOMER_SIZE_MASK                 0x0000000000300000ull
+#define OCC_SIZE_MASK                   0x0000000000700000ull
+
 static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
 {
     /*
@@ -90,9 +94,8 @@  static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
         return PNV_HOMER_BASE(chip);
 
     case P9_PBA_BARMASK0: /* P9 homer region size */
-        return PNV9_HOMER_SIZE;
     case P8_PBA_BARMASK0: /* P8 homer region size */
-        return PNV_HOMER_SIZE;
+        return HOMER_SIZE_MASK;
 
     case P9_PBA_BAR2: /* P9 occ common area */
         return PNV9_OCC_COMMON_AREA(chip);
@@ -100,9 +103,8 @@  static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
         return PNV_OCC_COMMON_AREA(chip);
 
     case P9_PBA_BARMASK2: /* P9 occ common area size */
-        return PNV9_OCC_COMMON_AREA_SIZE;
     case P8_PBA_BARMASK2: /* P8 occ common area size */
-        return PNV_OCC_COMMON_AREA_SIZE;
+        return OCC_SIZE_MASK;
 
     case 0x1010c00:     /* PIBAM FIR */
     case 0x1010c03:     /* PIBAM FIR MASK */