RE: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path

2020-09-18 Thread Salil Mehta
> From: Andrew Jones [mailto:drjo...@redhat.com]
> Sent: Friday, September 18, 2020 7:07 AM
> 
> On Fri, Sep 18, 2020 at 12:25:19AM +, Salil Mehta wrote:
> >
> > > From: Qemu-arm
> [mailto:qemu-arm-bounces+salil.mehta=huawei@nongnu.org]
> > > On Behalf Of Andrew Jones
> > > Sent: Thursday, September 17, 2020 9:13 AM
> > > To: fangying 
> > > Cc: peter.mayd...@linaro.org; Zhanghailiang
> ;
> > > Alexander Graf ; qemu-devel@nongnu.org; Chenzhendong (alex)
> > > ; shannon.zha...@gmail.com; qemu-...@nongnu.org;
> > > alistair.fran...@wdc.com; imamm...@redhat.com
> > > Subject: Re: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path
> > >
> > > On Thu, Sep 17, 2020 at 11:20:25AM +0800, Ying Fang wrote:
> > > > From: Andrew Jones 
> > > >
> > > > qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
> > > > also recursively adds any missing parent nodes.
> > > >
> > > > Cc: Peter Crosthwaite 
> > > > Cc: Alexander Graf 
> > > > Signed-off-by: Andrew Jones 
> > > > ---
> > > >  device_tree.c| 24 
> > > >  include/sysemu/device_tree.h |  1 +
> > > >  2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/device_tree.c b/device_tree.c
> > > > index b335dae707..1854be3a02 100644
> > > > --- a/device_tree.c
> > > > +++ b/device_tree.c
> > > > @@ -524,6 +524,30 @@ int qemu_fdt_add_subnode(void *fdt, const char 
> > > > *name)
> > > >  return retval;
> > > >  }
> > > >
> > > > +int qemu_fdt_add_path(void *fdt, const char *path)
> > > > +{
> > > > +char *parent;
> > > > +int offset;
> > > > +
> > > > +offset = fdt_path_offset(fdt, path);
> > > > +if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
> > > > +error_report("%s Couldn't find node %s: %s", __func__, path,
> > > > + fdt_strerror(offset));
> > > > +exit(1);
> > > > +}
> > > > +
> > > > +if (offset != -FDT_ERR_NOTFOUND) {
> > > > +return offset;
> > > > +}
> > > > +
> > > > +parent = g_strdup(path);
> > > > +strrchr(parent, '/')[0] = '\0';
> > > > +qemu_fdt_add_path(fdt, parent);
> > > > +g_free(parent);
> > > > +
> > > > +return qemu_fdt_add_subnode(fdt, path);
> > > > +}
> > >
> > > Igor didn't like the recursion when I posted this before so I changed
> > > it when doing the refresh[*] that I gave to Salil Mehta. Salil also
> > > works for Huawei, are you guys not working together?
> > >
> > > [*] https://github.com/rhdrjones/qemu/commits/virt-cpu-topology-refresh
> > >
> >
> >
> > I was not aware of this work going on. I am based at Cambridge and Fangying
> > in Hangzhou and work for different teams.
> >
> > Yes, I have it and have integrated it with the Virtual CPU hotplug branch
> > and I am testing them.
> >
> > I have also rebased virtual cpu hotplug patches and integrated the PMU
> > related changes recently been discussed in other mail-chain. Plus, I am
> > also dealing with the MPIDR/affinity part from the Kernel which has been
> > discussed earlier with the Marc Zyngier.
> >
> > It looks some of the changes are common here like ability to set MPIDR
> > of the vcpus at the time of their creation inside KVM. And the PPTT
> > table changes which were present in your refresh branch as well. Changes
> > related to the possible and present vcpus might need a sync as well.
> >
> > @Andrew, should I take out the part which is common and affects the
> > virtual cpu hotplug and push them separately. This way I can have part
> > of the changes related to the vcpu hotplug done which will also cover
> > this patch-set requirements as well?
> 
> Whatever works best for you and Ying Fang. It looks like this series
> only focuses on topology. It's not considering present and possible
> cpus, but it probably should. It also adds the cache hierarchy stuff,
> but I'm not sure it's approaching that in the right way. I think it
> may make sense to put this series on hold and take another look at
> your hotplug series when it's reposted before deciding what to do.


Ok fine. Let me collaborate with him internally first. Either of us
will have to rebase our patches on others code.


thanks


> 
> Thanks,
> drew
> 
> >
> > @Fangying, will this work for you?
> >
> >
> > Salil.
> >
> >




Re: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path

2020-09-18 Thread Andrew Jones
On Fri, Sep 18, 2020 at 12:25:19AM +, Salil Mehta wrote:
> 
> > From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei@nongnu.org]
> > On Behalf Of Andrew Jones
> > Sent: Thursday, September 17, 2020 9:13 AM
> > To: fangying 
> > Cc: peter.mayd...@linaro.org; Zhanghailiang 
> > ;
> > Alexander Graf ; qemu-devel@nongnu.org; Chenzhendong (alex)
> > ; shannon.zha...@gmail.com; qemu-...@nongnu.org;
> > alistair.fran...@wdc.com; imamm...@redhat.com
> > Subject: Re: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path
> > 
> > On Thu, Sep 17, 2020 at 11:20:25AM +0800, Ying Fang wrote:
> > > From: Andrew Jones 
> > >
> > > qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
> > > also recursively adds any missing parent nodes.
> > >
> > > Cc: Peter Crosthwaite 
> > > Cc: Alexander Graf 
> > > Signed-off-by: Andrew Jones 
> > > ---
> > >  device_tree.c| 24 
> > >  include/sysemu/device_tree.h |  1 +
> > >  2 files changed, 25 insertions(+)
> > >
> > > diff --git a/device_tree.c b/device_tree.c
> > > index b335dae707..1854be3a02 100644
> > > --- a/device_tree.c
> > > +++ b/device_tree.c
> > > @@ -524,6 +524,30 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
> > >  return retval;
> > >  }
> > >
> > > +int qemu_fdt_add_path(void *fdt, const char *path)
> > > +{
> > > +char *parent;
> > > +int offset;
> > > +
> > > +offset = fdt_path_offset(fdt, path);
> > > +if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
> > > +error_report("%s Couldn't find node %s: %s", __func__, path,
> > > + fdt_strerror(offset));
> > > +exit(1);
> > > +}
> > > +
> > > +if (offset != -FDT_ERR_NOTFOUND) {
> > > +return offset;
> > > +}
> > > +
> > > +parent = g_strdup(path);
> > > +strrchr(parent, '/')[0] = '\0';
> > > +qemu_fdt_add_path(fdt, parent);
> > > +g_free(parent);
> > > +
> > > +return qemu_fdt_add_subnode(fdt, path);
> > > +}
> > 
> > Igor didn't like the recursion when I posted this before so I changed
> > it when doing the refresh[*] that I gave to Salil Mehta. Salil also
> > works for Huawei, are you guys not working together?
> > 
> > [*] https://github.com/rhdrjones/qemu/commits/virt-cpu-topology-refresh
> > 
> 
> 
> I was not aware of this work going on. I am based at Cambridge and Fangying
> in Hangzhou and work for different teams.
> 
> Yes, I have it and have integrated it with the Virtual CPU hotplug branch
> and I am testing them.
> 
> I have also rebased virtual cpu hotplug patches and integrated the PMU
> related changes recently been discussed in other mail-chain. Plus, I am
> also dealing with the MPIDR/affinity part from the Kernel which has been
> discussed earlier with the Marc Zyngier. 
> 
> It looks some of the changes are common here like ability to set MPIDR
> of the vcpus at the time of their creation inside KVM. And the PPTT
> table changes which were present in your refresh branch as well. Changes
> related to the possible and present vcpus might need a sync as well.
> 
> @Andrew, should I take out the part which is common and affects the
> virtual cpu hotplug and push them separately. This way I can have part
> of the changes related to the vcpu hotplug done which will also cover
> this patch-set requirements as well? 

Whatever works best for you and Ying Fang. It looks like this series
only focuses on topology. It's not considering present and possible
cpus, but it probably should. It also adds the cache hierarchy stuff,
but I'm not sure it's approaching that in the right way. I think it
may make sense to put this series on hold and take another look at
your hotplug series when it's reposted before deciding what to do.

Thanks,
drew

> 
> @Fangying, will this work for you?
> 
> 
> Salil. 
> 
> 




RE: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path

2020-09-17 Thread Salil Mehta


> From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei@nongnu.org]
> On Behalf Of Andrew Jones
> Sent: Thursday, September 17, 2020 9:13 AM
> To: fangying 
> Cc: peter.mayd...@linaro.org; Zhanghailiang ;
> Alexander Graf ; qemu-devel@nongnu.org; Chenzhendong (alex)
> ; shannon.zha...@gmail.com; qemu-...@nongnu.org;
> alistair.fran...@wdc.com; imamm...@redhat.com
> Subject: Re: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path
> 
> On Thu, Sep 17, 2020 at 11:20:25AM +0800, Ying Fang wrote:
> > From: Andrew Jones 
> >
> > qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
> > also recursively adds any missing parent nodes.
> >
> > Cc: Peter Crosthwaite 
> > Cc: Alexander Graf 
> > Signed-off-by: Andrew Jones 
> > ---
> >  device_tree.c| 24 
> >  include/sysemu/device_tree.h |  1 +
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/device_tree.c b/device_tree.c
> > index b335dae707..1854be3a02 100644
> > --- a/device_tree.c
> > +++ b/device_tree.c
> > @@ -524,6 +524,30 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
> >  return retval;
> >  }
> >
> > +int qemu_fdt_add_path(void *fdt, const char *path)
> > +{
> > +char *parent;
> > +int offset;
> > +
> > +offset = fdt_path_offset(fdt, path);
> > +if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
> > +error_report("%s Couldn't find node %s: %s", __func__, path,
> > + fdt_strerror(offset));
> > +exit(1);
> > +}
> > +
> > +if (offset != -FDT_ERR_NOTFOUND) {
> > +return offset;
> > +}
> > +
> > +parent = g_strdup(path);
> > +strrchr(parent, '/')[0] = '\0';
> > +qemu_fdt_add_path(fdt, parent);
> > +g_free(parent);
> > +
> > +return qemu_fdt_add_subnode(fdt, path);
> > +}
> 
> Igor didn't like the recursion when I posted this before so I changed
> it when doing the refresh[*] that I gave to Salil Mehta. Salil also
> works for Huawei, are you guys not working together?
> 
> [*] https://github.com/rhdrjones/qemu/commits/virt-cpu-topology-refresh
> 


I was not aware of this work going on. I am based at Cambridge and Fangying
in Hangzhou and work for different teams.

Yes, I have it and have integrated it with the Virtual CPU hotplug branch
and I am testing them.

I have also rebased virtual cpu hotplug patches and integrated the PMU
related changes recently been discussed in other mail-chain. Plus, I am
also dealing with the MPIDR/affinity part from the Kernel which has been
discussed earlier with the Marc Zyngier. 

It looks some of the changes are common here like ability to set MPIDR
of the vcpus at the time of their creation inside KVM. And the PPTT
table changes which were present in your refresh branch as well. Changes
related to the possible and present vcpus might need a sync as well.

@Andrew, should I take out the part which is common and affects the
virtual cpu hotplug and push them separately. This way I can have part
of the changes related to the vcpu hotplug done which will also cover
this patch-set requirements as well? 

@Fangying, will this work for you?


Salil. 




Re: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path

2020-09-17 Thread Ying Fang




On 9/17/2020 4:12 PM, Andrew Jones wrote:

On Thu, Sep 17, 2020 at 11:20:25AM +0800, Ying Fang wrote:

From: Andrew Jones 

qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
also recursively adds any missing parent nodes.

Cc: Peter Crosthwaite 
Cc: Alexander Graf 
Signed-off-by: Andrew Jones 
---
  device_tree.c| 24 
  include/sysemu/device_tree.h |  1 +
  2 files changed, 25 insertions(+)

diff --git a/device_tree.c b/device_tree.c
index b335dae707..1854be3a02 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -524,6 +524,30 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
  return retval;
  }
  
+int qemu_fdt_add_path(void *fdt, const char *path)

+{
+char *parent;
+int offset;
+
+offset = fdt_path_offset(fdt, path);
+if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
+error_report("%s Couldn't find node %s: %s", __func__, path,
+ fdt_strerror(offset));
+exit(1);
+}
+
+if (offset != -FDT_ERR_NOTFOUND) {
+return offset;
+}
+
+parent = g_strdup(path);
+strrchr(parent, '/')[0] = '\0';
+qemu_fdt_add_path(fdt, parent);
+g_free(parent);
+
+return qemu_fdt_add_subnode(fdt, path);
+}


Igor didn't like the recursion when I posted this before so I changed
it when doing the refresh[*] that I gave to Salil Mehta. Salil also
works for Huawei, are you guys not working together?

[*] https://github.com/rhdrjones/qemu/commits/virt-cpu-topology-refresh


Thanks for the sync. I'll look into it. I did not know about the refresh
and the effort Salil Mehta has made on this. We are not in the same dept 
and work for different projects.


Thanks Ying.


Thanks,
drew


+
  void qemu_fdt_dumpdtb(void *fdt, int size)
  {
  const char *dumpdtb = qemu_opt_get(qemu_get_machine_opts(), "dumpdtb");
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 982c89345f..15fb98af98 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -104,6 +104,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
  uint32_t qemu_fdt_alloc_phandle(void *fdt);
  int qemu_fdt_nop_node(void *fdt, const char *node_path);
  int qemu_fdt_add_subnode(void *fdt, const char *name);
+int qemu_fdt_add_path(void *fdt, const char *path);
  
  #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \

  do {  
\
--
2.23.0




.





Re: [RFC PATCH 04/12] device_tree: add qemu_fdt_add_path

2020-09-17 Thread Andrew Jones
On Thu, Sep 17, 2020 at 11:20:25AM +0800, Ying Fang wrote:
> From: Andrew Jones 
> 
> qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
> also recursively adds any missing parent nodes.
> 
> Cc: Peter Crosthwaite 
> Cc: Alexander Graf 
> Signed-off-by: Andrew Jones 
> ---
>  device_tree.c| 24 
>  include/sysemu/device_tree.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/device_tree.c b/device_tree.c
> index b335dae707..1854be3a02 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -524,6 +524,30 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
>  return retval;
>  }
>  
> +int qemu_fdt_add_path(void *fdt, const char *path)
> +{
> +char *parent;
> +int offset;
> +
> +offset = fdt_path_offset(fdt, path);
> +if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
> +error_report("%s Couldn't find node %s: %s", __func__, path,
> + fdt_strerror(offset));
> +exit(1);
> +}
> +
> +if (offset != -FDT_ERR_NOTFOUND) {
> +return offset;
> +}
> +
> +parent = g_strdup(path);
> +strrchr(parent, '/')[0] = '\0';
> +qemu_fdt_add_path(fdt, parent);
> +g_free(parent);
> +
> +return qemu_fdt_add_subnode(fdt, path);
> +}

Igor didn't like the recursion when I posted this before so I changed
it when doing the refresh[*] that I gave to Salil Mehta. Salil also
works for Huawei, are you guys not working together?

[*] https://github.com/rhdrjones/qemu/commits/virt-cpu-topology-refresh

Thanks,
drew

> +
>  void qemu_fdt_dumpdtb(void *fdt, int size)
>  {
>  const char *dumpdtb = qemu_opt_get(qemu_get_machine_opts(), "dumpdtb");
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index 982c89345f..15fb98af98 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -104,6 +104,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char 
> *path);
>  uint32_t qemu_fdt_alloc_phandle(void *fdt);
>  int qemu_fdt_nop_node(void *fdt, const char *node_path);
>  int qemu_fdt_add_subnode(void *fdt, const char *name);
> +int qemu_fdt_add_path(void *fdt, const char *path);
>  
>  #define qemu_fdt_setprop_cells(fdt, node_path, property, ...)
>  \
>  do { 
>  \
> -- 
> 2.23.0
> 
> 




[RFC PATCH 04/12] device_tree: add qemu_fdt_add_path

2020-09-16 Thread Ying Fang
From: Andrew Jones 

qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
also recursively adds any missing parent nodes.

Cc: Peter Crosthwaite 
Cc: Alexander Graf 
Signed-off-by: Andrew Jones 
---
 device_tree.c| 24 
 include/sysemu/device_tree.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/device_tree.c b/device_tree.c
index b335dae707..1854be3a02 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -524,6 +524,30 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
 return retval;
 }
 
+int qemu_fdt_add_path(void *fdt, const char *path)
+{
+char *parent;
+int offset;
+
+offset = fdt_path_offset(fdt, path);
+if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
+error_report("%s Couldn't find node %s: %s", __func__, path,
+ fdt_strerror(offset));
+exit(1);
+}
+
+if (offset != -FDT_ERR_NOTFOUND) {
+return offset;
+}
+
+parent = g_strdup(path);
+strrchr(parent, '/')[0] = '\0';
+qemu_fdt_add_path(fdt, parent);
+g_free(parent);
+
+return qemu_fdt_add_subnode(fdt, path);
+}
+
 void qemu_fdt_dumpdtb(void *fdt, int size)
 {
 const char *dumpdtb = qemu_opt_get(qemu_get_machine_opts(), "dumpdtb");
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 982c89345f..15fb98af98 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -104,6 +104,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
 uint32_t qemu_fdt_alloc_phandle(void *fdt);
 int qemu_fdt_nop_node(void *fdt, const char *node_path);
 int qemu_fdt_add_subnode(void *fdt, const char *name);
+int qemu_fdt_add_path(void *fdt, const char *path);
 
 #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
 do {  \
-- 
2.23.0