From: Nathan Fontenot <nfont@linux.vnet.ibm.com>

This add entries to the root OF node to advertise our PHBs as being
DR-capable in accordance with PAPR specification.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
Changes since RFC:
- rebased against ppc-for-2.10
- use error_report
---
 hw/ppc/spapr.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5950c009ab7e..632040f35ecc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1151,6 +1151,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         exit(1);
     }
 
+    if (smc->dr_phb_enabled) {
+        int offset = fdt_path_offset(fdt, "/");
+        ret = spapr_drc_populate_dt(fdt, offset, NULL,
+                                    SPAPR_DR_CONNECTOR_TYPE_PHB);
+        if (ret < 0) {
+            error_report("Couldn't set up PHB DR device tree properties");
+            exit(1);
+        }
+    }
+
     return fdt;
 }
 

Quoting Greg Kurz (2017-07-25 13:02:03)
> From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> 
> This add entries to the root OF node to advertise our PHBs as being
> DR-capable in accordance with PAPR specification.
> 
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> Changes since RFC:
> - rebased against ppc-for-2.10
> - use error_report
> ---
>  hw/ppc/spapr.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5950c009ab7e..632040f35ecc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1151,6 +1151,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>          exit(1);
>      }
> 
> +    if (smc->dr_phb_enabled) {
> +        int offset = fdt_path_offset(fdt, "/");
> +        ret = spapr_drc_populate_dt(fdt, offset, NULL,
> +                                    SPAPR_DR_CONNECTOR_TYPE_PHB);

This might actually clobber LMB/MEM entries added earlier since they both
try to create the /ibm,drc* entries in / from scratch.
  
If that's the case, we can arrange the code something like:
  
  uint32_t root_drc_types = 0;
  
  if (smc->dr_lmb_enabled) {
      root_drc_types |= SPAPR_DR_CONNECTOR_TYPE_LMB;
  }
  
  if (smc->dr_phb_enabled) {
      root_drc_types |= SPAPR_DR_CONNECTOR_TYPE_PHB;
  }
  
  ...
  
  if (root_drc_types) {
    int offset = fdt_path_offset(fdt, "/");
    ret = spapr_drc_populate_dt(fdt, offset, NULL, root_drc_types);
  }


> +        if (ret < 0) {
> +            error_report("Couldn't set up PHB DR device tree properties");
> +            exit(1);
> +        }
> +    }
> +
>      return fdt;
>  }
> 
> 

On Tue, 25 Jul 2017 15:51:21 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> Quoting Greg Kurz (2017-07-25 13:02:03)
> > From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > 
> > This add entries to the root OF node to advertise our PHBs as being
> > DR-capable in accordance with PAPR specification.
> > 
> > Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > Changes since RFC:
> > - rebased against ppc-for-2.10
> > - use error_report
> > ---
> >  hw/ppc/spapr.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5950c009ab7e..632040f35ecc 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1151,6 +1151,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >          exit(1);
> >      }
> > 
> > +    if (smc->dr_phb_enabled) {
> > +        int offset = fdt_path_offset(fdt, "/");
> > +        ret = spapr_drc_populate_dt(fdt, offset, NULL,
> > +                                    SPAPR_DR_CONNECTOR_TYPE_PHB);  
> 
> This might actually clobber LMB/MEM entries added earlier since they both
> try to create the /ibm,drc* entries in / from scratch.
>   

Oh, looking at the description of fdt_setprop() in <libfdt.h>, I guess you're
probably right.

> If that's the case, we can arrange the code something like:
>   
>   uint32_t root_drc_types = 0;
>   
>   if (smc->dr_lmb_enabled) {
>       root_drc_types |= SPAPR_DR_CONNECTOR_TYPE_LMB;
>   }
>   
>   if (smc->dr_phb_enabled) {
>       root_drc_types |= SPAPR_DR_CONNECTOR_TYPE_PHB;
>   }
>   
>   ...
>   
>   if (root_drc_types) {
>     int offset = fdt_path_offset(fdt, "/");
>     ret = spapr_drc_populate_dt(fdt, offset, NULL, root_drc_types);
>   }
> 

Good idea. I'll do just that.

Thanks!

> 
> > +        if (ret < 0) {
> > +            error_report("Couldn't set up PHB DR device tree properties");
> > +            exit(1);
> > +        }
> > +    }
> > +
> >      return fdt;
> >  }
> > 
> >   
> 

On Tue, Jul 25, 2017 at 08:02:03PM +0200, Greg Kurz wrote:
> From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> 
> This add entries to the root OF node to advertise our PHBs as being
> DR-capable in accordance with PAPR specification.
> 
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> Changes since RFC:
> - rebased against ppc-for-2.10
> - use error_report
> ---
>  hw/ppc/spapr.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5950c009ab7e..632040f35ecc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1151,6 +1151,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>          exit(1);
>      }
>  
> +    if (smc->dr_phb_enabled) {
> +        int offset = fdt_path_offset(fdt, "/");

Offset of the root node is guaranteed to be 0, so you don't need fdt_path_offset().

> +        ret = spapr_drc_populate_dt(fdt, offset, NULL,
> +                                    SPAPR_DR_CONNECTOR_TYPE_PHB);
> +        if (ret < 0) {
> +            error_report("Couldn't set up PHB DR device tree properties");
> +            exit(1);
> +        }
> +    }
> +
>      return fdt;
>  }
>  
> 

-- 
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

On Wed, 26 Jul 2017 15:47:18 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Jul 25, 2017 at 08:02:03PM +0200, Greg Kurz wrote:
> > From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > 
> > This add entries to the root OF node to advertise our PHBs as being
> > DR-capable in accordance with PAPR specification.
> > 
> > Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > Changes since RFC:
> > - rebased against ppc-for-2.10
> > - use error_report
> > ---
> >  hw/ppc/spapr.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5950c009ab7e..632040f35ecc 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1151,6 +1151,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >          exit(1);
> >      }
> >  
> > +    if (smc->dr_phb_enabled) {
> > +        int offset = fdt_path_offset(fdt, "/");  
> 
> Offset of the root node is guaranteed to be 0, so you don't need fdt_path_offset().
> 

Oh, I didn't know that.

BTW, there are two other places that also need to be fixed then:

$ git grep -n 'fdt_path_offset(fdt, "/")'
hw/ppc/pnv.c:95:        cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"),
hw/ppc/spapr.c:355:            cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"),

I'll send a patch.

> > +        ret = spapr_drc_populate_dt(fdt, offset, NULL,
> > +                                    SPAPR_DR_CONNECTOR_TYPE_PHB);
> > +        if (ret < 0) {
> > +            error_report("Couldn't set up PHB DR device tree properties");
> > +            exit(1);
> > +        }
> > +    }
> > +
> >      return fdt;
> >  }
> >  
> >   
>