Alexey Kardashevskiy <a...@ozlabs.ru> writes: > On 05/19/2015 06:14 PM, Nikunj A Dadhania wrote: >> Alexey Kardashevskiy <a...@ozlabs.ru> writes: >> >>> On 05/19/2015 04:56 PM, Nikunj A Dadhania wrote: >>>> Alexey Kardashevskiy <a...@ozlabs.ru> writes: >>>> >>>>> On 05/19/2015 02:51 PM, Nikunj A Dadhania wrote: >>>>>> Alexey Kardashevskiy <a...@ozlabs.ru> writes: >>>>>> >>>>>>> On 05/07/2015 05:21 PM, Nikunj A Dadhania wrote: >>>>>>>> Each hardware instance has a platform unique location code. The OF >>>>>>>> device tree that describes a part of a hardware entity must include >>>>>>>> the “ibm,loc-code” property with a value that represents the location >>>>>>>> code for that hardware entity. >>>>>>>> >>>>>>>> Populate ibm,loc-code. >>>>>>>> >>>>>>>> 1) PCI passthru devices need to identify with its own ibm,loc-code >>>>>>>> available on the host. In failure cases use: >>>>>>>> vfio_<name>:<phb-index>:<slot>.<fn> >>>>>>>> >>>>>>>> 2) Emulated devices encode as following: >>>>>>>> qemu_<name>:<phb-index>:<slot>.<fn> >>>>>>>> >>>>>>>> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> >>>>>>>> --- >>>>>>>> hw/ppc/spapr_pci.c | 98 >>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++------- >>>>>>>> 1 file changed, 86 insertions(+), 12 deletions(-) >>>>>>>> >>>>>>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>>>>>>> index 12f1b9c..d901007 100644 >>>>>>>> --- a/hw/ppc/spapr_pci.c >>>>>>>> +++ b/hw/ppc/spapr_pci.c >>>>>>>> @@ -769,6 +769,81 @@ static uint32_t >>>>>>>> spapr_phb_get_pci_drc_index(sPAPRPHBState *phb, >>>>>>>> return drck->get_index(drc); >>>>>>>> } >>>>>>>> >>>>>>>> +static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char >>>>>>>> **value) >>>>>>> >>>>>>> Does it have to be a separate function? >>>>>> >>>>>> For better readability, i would prefer it this way. >>>>> >>>>> This is why I asked - I was having problems understanding the difference >>>>> between these two having 6 words names ;) Do not insist though. >>>>> >>>> >>>> This is what I have now, simplified: >>>> >>>> +static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice >>>> *pdev) >>>> +{ >>>> + char *path = NULL, *buf = NULL; >>>> + char *host = NULL; >>>> + >>>> + /* Get the PCI VFIO host id */ >>>> + host = object_property_get_str(OBJECT(pdev), "host", NULL); >>>> + if (!host) { >>>> + goto err_out; >>>> + } >>>> + >>>> + /* Construct the path of the file that will give us the DT location */ >>>> + path = g_strdup_printf("/sys/bus/pci/devices/%s/devspec", host); >>>> + g_free(host); >>>> + if (path && !g_file_get_contents(path, &buf, NULL, NULL)) { >>>> + goto err_out; >>>> + } >>>> + 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); >>>> + g_free(buf); >>>> + if (path && !g_file_get_contents(path, &buf, NULL, NULL)) { >>>> + goto err_out; >>>> + } >>>> + return buf; >>>> + >>>> +err_out: >>>> + g_free(path); >>>> + return NULL; >>>> +} >>>> + >>>> +static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev) >>>> +{ >>>> + if (object_dynamic_cast(OBJECT(pdev), "vfio-pci")) { >>>> + char *buf = spapr_phb_vfio_get_loc_code(sphb, pdev); >>>> + >>>> + /* >>>> + * In case of failures reading the loc-code, make it up >>>> + * indicating a vfio device >>>> + */ >>>> + if (!buf) { >>>> + buf = g_strdup_printf("vfio_%s:%02d:%02d.%1d", pdev->name, >>>> + sphb->index, PCI_SLOT(pdev->devfn), >>>> + PCI_FUNC(pdev->devfn)); >>>> + } >>>> + return buf; >>>> + } else { >>>> + return g_strdup_printf("qemu_%s:%02d:%02d.%1d", pdev->name, >>>> + sphb->index, PCI_SLOT(pdev->devfn), >>>> + PCI_FUNC(pdev->devfn)); >>>> + } >>>> +} >>> >>> >>> I'd do this but I do not insist :) >> >> That is fine as well. >> >>> >>> static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev) >>> { >>> char *buf; >>> char *devtype = "qemu"; >>> >>> if (object_dynamic_cast(OBJECT(pdev), "vfio-pci")) { >>> buf = spapr_phb_vfio_get_loc_code(sphb, pdev); >>> if (buf) { >>> return buf; >>> } >>> devtype = "vfio"; >> >> That has to be a snprintf. > > No it does not. g_strdup_printf() below is enough
CC ppc64-softmmu/hw/ppc/spapr_pci.o hw/ppc/spapr_pci.c: In function ‘spapr_phb_get_loc_code’: hw/ppc/spapr_pci.c:807:21: error: initialization discards ‘const’ qualifier from pointer target type [-Werror] char *devtype = "qemu"; ^ hw/ppc/spapr_pci.c:814:17: error: assignment discards ‘const’ qualifier from pointer target type [-Werror] devtype = "vfio"; ^ cc1: all warnings being treated as errors make[1]: *** [hw/ppc/spapr_pci.o] Error 1 make: *** [subdir-ppc64-softmmu] Error 2 > >> >>> } >>> >>> /* >>> * In case of failures reading the loc-code, make it up >>> * indicating a vfio device >>> */ >>> buf = g_strdup_printf("%s_%s:%02d:%02d.%1d", >>> devtype, pdev->name, >>> sphb->index, PCI_SLOT(pdev->devfn), >>> PCI_FUNC(pdev->devfn)); >>> return buf; >>> } >> >> Regards >> Nikunj >> > > > -- > Alexey