spapr_pci: fix the path while fetching loc-code from host DT

[Qemu-devel] [PATCH v1] spapr_pci: fix the path while fetching loc-code from host DT

Posted by Seeteena Thoufeek 8 years, 3 months ago

The function spapr_phb_vfio_get_loc_code uses wrong path for
fetching loc-code from host DT

this is the call that needs to be fixed:
/* Construct and read from host device tree the loc-code */
path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);

Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5a3122a..ae86322 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -762,7 +762,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
     g_free(path);
 
     /* Construct and read from host device tree the loc-code */
-    path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
+    path = g_strdup_printf("/proc/device-tree/%s/ibm,loc-code", buf);
     g_free(buf);
     if (!g_file_get_contents(path, &buf, NULL, NULL)) {
         goto err_out;
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH v1] spapr_pci: fix the path while fetching loc-code from host DT

Posted by Alexey Kardashevskiy 8 years, 3 months ago

On 08/11/17 00:18, Seeteena Thoufeek wrote:
> The function spapr_phb_vfio_get_loc_code uses wrong path for
> fetching loc-code from host DT
> 
> this is the call that needs to be fixed:
> /* Construct and read from host device tree the loc-code */
> path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);


I do not see why would extra '/' hurt but where does it really fail?

My machines have leading '/' in devspec, do not yours?

aik@garrison2:~$ cat /sys/bus/pci/devices/0009:03:00.0/devspec && echo -e '\n'
/pciex@3fffe40500000/pci@0/pci@0/pci@1/usb-xhci@0




> 
> Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 5a3122a..ae86322 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -762,7 +762,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
>      g_free(path);
>  
>      /* Construct and read from host device tree the loc-code */
> -    path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
> +    path = g_strdup_printf("/proc/device-tree/%s/ibm,loc-code", buf);
>      g_free(buf);
>      if (!g_file_get_contents(path, &buf, NULL, NULL)) {
>          goto err_out;
> 


-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCH v1] spapr_pci: fix the path while fetching loc-code from host DT

Posted by David Gibson 8 years, 3 months ago

On Wed, Nov 08, 2017 at 01:30:04PM +1100, Alexey Kardashevskiy wrote:
> On 08/11/17 00:18, Seeteena Thoufeek wrote:
> > The function spapr_phb_vfio_get_loc_code uses wrong path for
> > fetching loc-code from host DT
> > 
> > this is the call that needs to be fixed:
> > /* Construct and read from host device tree the loc-code */
> > path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
> 
> 
> I do not see why would extra '/' hurt but where does it really fail?
> 
> My machines have leading '/' in devspec, do not yours?
> 
> aik@garrison2:~$ cat /sys/bus/pci/devices/0009:03:00.0/devspec && echo -e '\n'
> /pciex@3fffe40500000/pci@0/pci@0/pci@1/usb-xhci@0

Right, what Alexey said.  I see no reason to change this.

> 
> 
> 
> 
> > 
> > Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr_pci.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 5a3122a..ae86322 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -762,7 +762,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
> >      g_free(path);
> >  
> >      /* Construct and read from host device tree the loc-code */
> > -    path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
> > +    path = g_strdup_printf("/proc/device-tree/%s/ibm,loc-code", buf);
> >      g_free(buf);
> >      if (!g_file_get_contents(path, &buf, NULL, NULL)) {
> >          goto err_out;
> > 
> 
> 

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH v1] spapr_pci: fix the path while fetching loc-code from host DT

Posted by seeteena 8 years, 3 months ago

Apologize for the confusion. I thought devspec is not having leading '/'.

I checked devspec in my system and it works as expected.

On 11/08/2017 08:00 AM, Alexey Kardashevskiy wrote:
> On 08/11/17 00:18, Seeteena Thoufeek wrote:
>> The function spapr_phb_vfio_get_loc_code uses wrong path for
>> fetching loc-code from host DT
>>
>> this is the call that needs to be fixed:
>> /* Construct and read from host device tree the loc-code */
>> path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
>
> I do not see why would extra '/' hurt but where does it really fail?
>
> My machines have leading '/' in devspec, do not yours?
>
> aik@garrison2:~$ cat /sys/bus/pci/devices/0009:03:00.0/devspec && echo -e '\n'
> /pciex@3fffe40500000/pci@0/pci@0/pci@1/usb-xhci@0
>
>
>
>
>> Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
>> ---
>>   hw/ppc/spapr_pci.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 5a3122a..ae86322 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -762,7 +762,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
>>       g_free(path);
>>   
>>       /* Construct and read from host device tree the loc-code */
>> -    path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
>> +    path = g_strdup_printf("/proc/device-tree/%s/ibm,loc-code", buf);
>>       g_free(buf);
>>       if (!g_file_get_contents(path, &buf, NULL, NULL)) {
>>           goto err_out;
>>
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH v1] spapr_pci: fix the path while fetching loc-code from host DT

Posted by Greg Kurz 8 years, 3 months ago

On Tue,  7 Nov 2017 18:48:58 +0530
Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com> wrote:

> The function spapr_phb_vfio_get_loc_code uses wrong path for
> fetching loc-code from host DT
> 

Why is it wrong ? Are you experiencing actual issues ?

> this is the call that needs to be fixed:
> /* Construct and read from host device tree the loc-code */
> path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
> 

This statement is useless, especially with such a trivial patch.

Have a look at the following page:

https://robots.thoughtbot.com/5-useful-tips-for-a-better-commit-message

Cheers,

--
Greg

> Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 5a3122a..ae86322 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -762,7 +762,7 @@ static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb,  PCIDevice *pdev)
>      g_free(path);
>  
>      /* Construct and read from host device tree the loc-code */
> -    path = g_strdup_printf("/proc/device-tree%s/ibm,loc-code", buf);
> +    path = g_strdup_printf("/proc/device-tree/%s/ibm,loc-code", buf);
>      g_free(buf);
>      if (!g_file_get_contents(path, &buf, NULL, NULL)) {
>          goto err_out;