* [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code
@ 2015-03-30 11:02 Nikunj A Dadhania
2015-03-31 2:00 ` Alexey Kardashevskiy
0 siblings, 1 reply; 6+ messages in thread
From: Nikunj A Dadhania @ 2015-03-30 11:02 UTC (permalink / raw)
To: qemu-devel; +Cc: aik, qemu-ppc, agraf, nikunj, david
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.
Introduce an rtas call to populate ibm,loc-code.
1) PCI passthru devices need to identify with its own ibm,loc-code
available on the host.
2) Emulated devices encode as following:
qemu_<name>:<phb-index>:<slot>.<fn>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
Changelog
v2:
* Using rtas call for getting ibm,loc-code
* Added sPAPRPHBState::get_loc_code
* Refactored the return type of get_loc_code
* Drop stat(), and rely on g_file_get_contents
return type for file existence
v1:
* Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
to recognise vfio devices
* Removed wrapper for hcall
* Added sPAPRPHBClass::get_loc_code
hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++
include/hw/pci-host/spapr.h | 1 +
include/hw/ppc/spapr.h | 3 +-
4 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 05f4fac..fe6dfd5 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -580,6 +580,50 @@ param_error_exit:
rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
}
+static void rtas_ibm_get_loc_code(PowerPCCPU *cpu,
+ sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args, uint32_t nret,
+ target_ulong rets)
+{
+ sPAPRPHBState *sphb = NULL;
+ sPAPRPHBClass *spc = NULL;
+ char *buf = NULL;
+ PCIDevice *pdev;
+ uint64_t buid;
+ uint32_t config_addr, loc_code, size;
+
+ if ((nargs != 5) || (nret != 1)) {
+ goto param_error_exit;
+ }
+
+ config_addr = rtas_ld(args, 0);
+ buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+ loc_code = rtas_ld(args, 3);
+ size = rtas_ld(args, 4);
+
+ sphb = find_phb(spapr, buid);
+ pdev = find_dev(spapr, buid, config_addr);
+
+ if (!sphb || !pdev) {
+ goto param_error_exit;
+ }
+
+ spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+ if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
+ uint32_t loc_len = strlen(buf);
+
+ loc_len = (loc_len > size) ? size : loc_len;
+ cpu_physical_memory_write(loc_code, buf, loc_len);
+ g_free(buf);
+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+ return;
+ }
+
+param_error_exit:
+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
uint32_t token, uint32_t nargs,
@@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
spapr_tce_get_iommu(tcet));
}
+static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
+ char **loc_code)
+{
+ char *path = g_malloc(PATH_MAX);
+
+ if (!path) {
+ return false;
+ }
+
+ /*
+ * For non-vfio devices and failures make up the location code out
+ * of the name, slot and function.
+ *
+ * qemu_<name>:<phb-index>:<slot>.<fn>
+ */
+ snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
+ sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+ *loc_code = path;
+ return true;
+}
+
static int spapr_phb_children_reset(Object *child, void *opaque)
{
DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
@@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
dc->cannot_instantiate_with_device_add_yet = false;
spc->finish_realize = spapr_phb_finish_realize;
+ spc->get_loc_code = spapr_phb_get_loc_code;
}
static const TypeInfo spapr_phb_info = {
@@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void)
spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL,
"ibm,slot-error-detail",
rtas_ibm_slot_error_detail);
+ spapr_rtas_register(RTAS_IBM_GET_LOC_CODE,
+ "ibm,get-loc-code",
+ rtas_ibm_get_loc_code);
+
}
static void spapr_pci_register_types(void)
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 99a1be5..6217cc5 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -171,6 +171,45 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
return RTAS_OUT_SUCCESS;
}
+static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char **value)
+{
+ char *host;
+ char path[PATH_MAX];
+
+ host = object_property_get_str(OBJECT(pdev), "host", NULL);
+ if (!host) {
+ return false;
+ }
+
+ snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
+ g_free(host);
+
+ return g_file_get_contents(path, value, NULL, NULL);
+}
+
+static bool spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
+ char **loc_code)
+{
+ sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
+ char path[PATH_MAX], *buf = NULL;
+
+ /* Non VFIO devices */
+ if (!svphb) {
+ return false;
+ }
+
+ /* We have a vfio host bridge lets get the path. */
+ if (!spapr_phb_vfio_get_devspec_value(pdev, &buf)) {
+ return false;
+ }
+
+ snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
+ g_free(buf);
+
+ /* Read the loc-code and return */
+ return g_file_get_contents(path, loc_code, NULL, NULL);
+}
+
static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
{
sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
@@ -199,6 +238,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
spc->eeh_reset = spapr_phb_vfio_eeh_reset;
spc->eeh_configure = spapr_phb_vfio_eeh_configure;
+ spc->get_loc_code = spapr_phb_vfio_get_loc_code;
}
static const TypeInfo spapr_phb_vfio_info = {
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 895d273..bdf4677 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -53,6 +53,7 @@ struct sPAPRPHBClass {
int (*eeh_get_state)(sPAPRPHBState *sphb, int *state);
int (*eeh_reset)(sPAPRPHBState *sphb, int option);
int (*eeh_configure)(sPAPRPHBState *sphb);
+ bool (*get_loc_code)(sPAPRPHBState *sphb, PCIDevice *pdev, char **loc_code);
};
typedef struct spapr_pci_msi {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index af71e8b..fab1364 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -422,8 +422,9 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
#define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
#define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24)
#define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25)
+#define RTAS_IBM_GET_LOC_CODE (RTAS_TOKEN_BASE + 0x26)
-#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x27)
/* RTAS ibm,get-system-parameter token values */
#define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code
2015-03-30 11:02 [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code Nikunj A Dadhania
@ 2015-03-31 2:00 ` Alexey Kardashevskiy
2015-03-31 2:29 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 2:00 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-devel; +Cc: qemu-ppc, agraf, david
On 03/30/2015 10:02 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.
>
> Introduce an rtas call to populate ibm,loc-code.
> 1) PCI passthru devices need to identify with its own ibm,loc-code
> available on the host.
> 2) Emulated devices encode as following:
> qemu_<name>:<phb-index>:<slot>.<fn>
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>
>
> Changelog
> v2:
> * Using rtas call for getting ibm,loc-code
> * Added sPAPRPHBState::get_loc_code
> * Refactored the return type of get_loc_code
> * Drop stat(), and rely on g_file_get_contents
> return type for file existence
>
> v1:
> * Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
> to recognise vfio devices
> * Removed wrapper for hcall
> * Added sPAPRPHBClass::get_loc_code
>
> hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
> hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++
> include/hw/pci-host/spapr.h | 1 +
> include/hw/ppc/spapr.h | 3 +-
> 4 files changed, 113 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 05f4fac..fe6dfd5 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -580,6 +580,50 @@ param_error_exit:
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> }
>
> +static void rtas_ibm_get_loc_code(PowerPCCPU *cpu,
> + sPAPREnvironment *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args, uint32_t nret,
> + target_ulong rets)
> +{
> + sPAPRPHBState *sphb = NULL;
> + sPAPRPHBClass *spc = NULL;
> + char *buf = NULL;
> + PCIDevice *pdev;
> + uint64_t buid;
> + uint32_t config_addr, loc_code, size;
> +
> + if ((nargs != 5) || (nret != 1)) {
> + goto param_error_exit;
> + }
> +
> + config_addr = rtas_ld(args, 0);
> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> + loc_code = rtas_ld(args, 3);
> + size = rtas_ld(args, 4);
> +
> + sphb = find_phb(spapr, buid);
> + pdev = find_dev(spapr, buid, config_addr);
> +
> + if (!sphb || !pdev) {
> + goto param_error_exit;
> + }
> +
> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> + if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
> + uint32_t loc_len = strlen(buf);
> +
> + loc_len = (loc_len > size) ? size : loc_len;
> + cpu_physical_memory_write(loc_code, buf, loc_len);
> + g_free(buf);
> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> + return;
> + }
> +
> +param_error_exit:
> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +}
> +
> static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
> sPAPREnvironment *spapr,
> uint32_t token, uint32_t nargs,
> @@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> spapr_tce_get_iommu(tcet));
> }
>
> +static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
> + char **loc_code)
> +{
> + char *path = g_malloc(PATH_MAX);
> +
> + if (!path) {
> + return false;
> + }
> +
> + /*
> + * For non-vfio devices and failures make up the location code out
> + * of the name, slot and function.
> + *
> + * qemu_<name>:<phb-index>:<slot>.<fn>
> + */
> + snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
> + sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> + *loc_code = path;
> + return true;
> +}
> +
> static int spapr_phb_children_reset(Object *child, void *opaque)
> {
> DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
> @@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> dc->cannot_instantiate_with_device_add_yet = false;
> spc->finish_realize = spapr_phb_finish_realize;
> + spc->get_loc_code = spapr_phb_get_loc_code;
> }
>
> static const TypeInfo spapr_phb_info = {
> @@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void)
> spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL,
> "ibm,slot-error-detail",
> rtas_ibm_slot_error_detail);
> + spapr_rtas_register(RTAS_IBM_GET_LOC_CODE,
> + "ibm,get-loc-code",
s/ibm,get-loc-code/qemu,get-loc-code/ as it is not from sPAPR.
btw we could make it even simpler and just put a property under PHB which
would contain a map slot:function<->loc-code, the map would be per PHB and
SLOF would just parse it and not call RTAS or do a hypercall. When we get
PCI scan in QEMU, we will just remove this chunk from the device tree (and
put loc-code from it to device nodes), and we won't have polluted RTAS
token namespace. The current thing looks too complicated for such a simple
function. Dunno...
> + rtas_ibm_get_loc_code);
> +
Extra empty line is not needed here.
How is PCI-scan-in-QEMU assessment going on? :)
> }
>
> static void spapr_pci_register_types(void)
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 99a1be5..6217cc5 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -171,6 +171,45 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
> return RTAS_OUT_SUCCESS;
> }
>
> +static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char **value)
> +{
> + char *host;
> + char path[PATH_MAX];
> +
> + host = object_property_get_str(OBJECT(pdev), "host", NULL);
> + if (!host) {
> + return false;
> + }
> +
> + snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
> + g_free(host);
> +
> + return g_file_get_contents(path, value, NULL, NULL);
> +}
> +
> +static bool spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
> + char **loc_code)
> +{
> + sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> + char path[PATH_MAX], *buf = NULL;
> +
> + /* Non VFIO devices */
> + if (!svphb) {
> + return false;
> + }
> +
> + /* We have a vfio host bridge lets get the path. */
> + if (!spapr_phb_vfio_get_devspec_value(pdev, &buf)) {
> + return false;
> + }
> +
> + snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
> + g_free(buf);
> +
> + /* Read the loc-code and return */
> + return g_file_get_contents(path, loc_code, NULL, NULL);
> +}
> +
> static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
> {
> sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> @@ -199,6 +238,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
> spc->eeh_reset = spapr_phb_vfio_eeh_reset;
> spc->eeh_configure = spapr_phb_vfio_eeh_configure;
> + spc->get_loc_code = spapr_phb_vfio_get_loc_code;
> }
>
> static const TypeInfo spapr_phb_vfio_info = {
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 895d273..bdf4677 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -53,6 +53,7 @@ struct sPAPRPHBClass {
> int (*eeh_get_state)(sPAPRPHBState *sphb, int *state);
> int (*eeh_reset)(sPAPRPHBState *sphb, int option);
> int (*eeh_configure)(sPAPRPHBState *sphb);
> + bool (*get_loc_code)(sPAPRPHBState *sphb, PCIDevice *pdev, char **loc_code);
> };
>
> typedef struct spapr_pci_msi {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index af71e8b..fab1364 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -422,8 +422,9 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
> #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
> #define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24)
> #define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25)
> +#define RTAS_IBM_GET_LOC_CODE (RTAS_TOKEN_BASE + 0x26)
>
> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26)
> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x27)
>
> /* RTAS ibm,get-system-parameter token values */
> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
>
--
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code
2015-03-31 2:00 ` Alexey Kardashevskiy
@ 2015-03-31 2:29 ` David Gibson
2015-03-31 5:15 ` Nikunj A Dadhania
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2015-03-31 2:29 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, Nikunj A Dadhania, agraf
[-- Attachment #1: Type: text/plain, Size: 6256 bytes --]
On Tue, Mar 31, 2015 at 01:00:57PM +1100, Alexey Kardashevskiy wrote:
> On 03/30/2015 10:02 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.
> >
> >Introduce an rtas call to populate ibm,loc-code.
> >1) PCI passthru devices need to identify with its own ibm,loc-code
> > available on the host.
> >2) Emulated devices encode as following:
> > qemu_<name>:<phb-index>:<slot>.<fn>
> >
> >Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >---
> >
> >
> >Changelog
> >v2:
> >* Using rtas call for getting ibm,loc-code
> >* Added sPAPRPHBState::get_loc_code
> >* Refactored the return type of get_loc_code
> >* Drop stat(), and rely on g_file_get_contents
> > return type for file existence
> >
> >v1:
> >* Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
> > to recognise vfio devices
> >* Removed wrapper for hcall
> >* Added sPAPRPHBClass::get_loc_code
> >
> > hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
> > hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++
> > include/hw/pci-host/spapr.h | 1 +
> > include/hw/ppc/spapr.h | 3 +-
> > 4 files changed, 113 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> >index 05f4fac..fe6dfd5 100644
> >--- a/hw/ppc/spapr_pci.c
> >+++ b/hw/ppc/spapr_pci.c
> >@@ -580,6 +580,50 @@ param_error_exit:
> > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> > }
> >
> >+static void rtas_ibm_get_loc_code(PowerPCCPU *cpu,
> >+ sPAPREnvironment *spapr,
> >+ uint32_t token, uint32_t nargs,
> >+ target_ulong args, uint32_t nret,
> >+ target_ulong rets)
> >+{
> >+ sPAPRPHBState *sphb = NULL;
> >+ sPAPRPHBClass *spc = NULL;
> >+ char *buf = NULL;
> >+ PCIDevice *pdev;
> >+ uint64_t buid;
> >+ uint32_t config_addr, loc_code, size;
> >+
> >+ if ((nargs != 5) || (nret != 1)) {
> >+ goto param_error_exit;
> >+ }
> >+
> >+ config_addr = rtas_ld(args, 0);
> >+ buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
> >+ loc_code = rtas_ld(args, 3);
> >+ size = rtas_ld(args, 4);
> >+
> >+ sphb = find_phb(spapr, buid);
> >+ pdev = find_dev(spapr, buid, config_addr);
> >+
> >+ if (!sphb || !pdev) {
> >+ goto param_error_exit;
> >+ }
> >+
> >+ spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> >+ if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
> >+ uint32_t loc_len = strlen(buf);
> >+
> >+ loc_len = (loc_len > size) ? size : loc_len;
> >+ cpu_physical_memory_write(loc_code, buf, loc_len);
> >+ g_free(buf);
> >+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >+ return;
> >+ }
> >+
> >+param_error_exit:
> >+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> >+}
> >+
> > static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
> > sPAPREnvironment *spapr,
> > uint32_t token, uint32_t nargs,
> >@@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> > spapr_tce_get_iommu(tcet));
> > }
> >
> >+static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
> >+ char **loc_code)
> >+{
> >+ char *path = g_malloc(PATH_MAX);
> >+
> >+ if (!path) {
> >+ return false;
> >+ }
> >+
> >+ /*
> >+ * For non-vfio devices and failures make up the location code out
> >+ * of the name, slot and function.
> >+ *
> >+ * qemu_<name>:<phb-index>:<slot>.<fn>
> >+ */
> >+ snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
> >+ sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> >+ *loc_code = path;
> >+ return true;
> >+}
> >+
> > static int spapr_phb_children_reset(Object *child, void *opaque)
> > {
> > DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
> >@@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
> > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> > dc->cannot_instantiate_with_device_add_yet = false;
> > spc->finish_realize = spapr_phb_finish_realize;
> >+ spc->get_loc_code = spapr_phb_get_loc_code;
> > }
> >
> > static const TypeInfo spapr_phb_info = {
> >@@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void)
> > spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL,
> > "ibm,slot-error-detail",
> > rtas_ibm_slot_error_detail);
> >+ spapr_rtas_register(RTAS_IBM_GET_LOC_CODE,
> >+ "ibm,get-loc-code",
>
>
> s/ibm,get-loc-code/qemu,get-loc-code/ as it is not from sPAPR.
Agreed.
> btw we could make it even simpler and just put a property under PHB which
> would contain a map slot:function<->loc-code, the map would be per PHB and
> SLOF would just parse it and not call RTAS or do a hypercall. When we get
> PCI scan in QEMU, we will just remove this chunk from the device tree (and
> put loc-code from it to device nodes), and we won't have polluted RTAS token
> namespace. The current thing looks too complicated for such a simple
> function. Dunno...
I think that's a good idea. Introducing a callback to get device
information seems pretty odd, when the device tree exists to
communicate static device information to the guest.
If we're not able to go straight to qemu doing the PCI scan, I think a
PHB property listing this information is a better option than an RTAS
callback or hcall.
--
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
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code
2015-03-31 2:29 ` David Gibson
@ 2015-03-31 5:15 ` Nikunj A Dadhania
2015-03-31 7:16 ` Alexey Kardashevskiy
0 siblings, 1 reply; 6+ messages in thread
From: Nikunj A Dadhania @ 2015-03-31 5:15 UTC (permalink / raw)
To: David Gibson, Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, agraf
David Gibson <david@gibson.dropbear.id.au> writes:
> On Tue, Mar 31, 2015 at 01:00:57PM +1100, Alexey Kardashevskiy wrote:
>> On 03/30/2015 10:02 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.
>> >
>> >Introduce an rtas call to populate ibm,loc-code.
>> >1) PCI passthru devices need to identify with its own ibm,loc-code
>> > available on the host.
>> >2) Emulated devices encode as following:
>> > qemu_<name>:<phb-index>:<slot>.<fn>
>> >
>> >Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> >---
>> >
>> >
>> >Changelog
>> >v2:
>> >* Using rtas call for getting ibm,loc-code
>> >* Added sPAPRPHBState::get_loc_code
>> >* Refactored the return type of get_loc_code
>> >* Drop stat(), and rely on g_file_get_contents
>> > return type for file existence
>> >
>> >v1:
>> >* Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
>> > to recognise vfio devices
>> >* Removed wrapper for hcall
>> >* Added sPAPRPHBClass::get_loc_code
>> >
>> > hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>> > hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++
>> > include/hw/pci-host/spapr.h | 1 +
>> > include/hw/ppc/spapr.h | 3 +-
>> > 4 files changed, 113 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> >index 05f4fac..fe6dfd5 100644
>> >--- a/hw/ppc/spapr_pci.c
>> >+++ b/hw/ppc/spapr_pci.c
>> >@@ -580,6 +580,50 @@ param_error_exit:
>> > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> > }
>> >
>> >+static void rtas_ibm_get_loc_code(PowerPCCPU *cpu,
>> >+ sPAPREnvironment *spapr,
>> >+ uint32_t token, uint32_t nargs,
>> >+ target_ulong args, uint32_t nret,
>> >+ target_ulong rets)
>> >+{
>> >+ sPAPRPHBState *sphb = NULL;
>> >+ sPAPRPHBClass *spc = NULL;
>> >+ char *buf = NULL;
>> >+ PCIDevice *pdev;
>> >+ uint64_t buid;
>> >+ uint32_t config_addr, loc_code, size;
>> >+
>> >+ if ((nargs != 5) || (nret != 1)) {
>> >+ goto param_error_exit;
>> >+ }
>> >+
>> >+ config_addr = rtas_ld(args, 0);
>> >+ buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
>> >+ loc_code = rtas_ld(args, 3);
>> >+ size = rtas_ld(args, 4);
>> >+
>> >+ sphb = find_phb(spapr, buid);
>> >+ pdev = find_dev(spapr, buid, config_addr);
>> >+
>> >+ if (!sphb || !pdev) {
>> >+ goto param_error_exit;
>> >+ }
>> >+
>> >+ spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> >+ if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
>> >+ uint32_t loc_len = strlen(buf);
>> >+
>> >+ loc_len = (loc_len > size) ? size : loc_len;
>> >+ cpu_physical_memory_write(loc_code, buf, loc_len);
>> >+ g_free(buf);
>> >+ rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> >+ return;
>> >+ }
>> >+
>> >+param_error_exit:
>> >+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> >+}
>> >+
>> > static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>> > sPAPREnvironment *spapr,
>> > uint32_t token, uint32_t nargs,
>> >@@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
>> > spapr_tce_get_iommu(tcet));
>> > }
>> >
>> >+static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
>> >+ char **loc_code)
>> >+{
>> >+ char *path = g_malloc(PATH_MAX);
>> >+
>> >+ if (!path) {
>> >+ return false;
>> >+ }
>> >+
>> >+ /*
>> >+ * For non-vfio devices and failures make up the location code out
>> >+ * of the name, slot and function.
>> >+ *
>> >+ * qemu_<name>:<phb-index>:<slot>.<fn>
>> >+ */
>> >+ snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
>> >+ sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>> >+ *loc_code = path;
>> >+ return true;
>> >+}
>> >+
>> > static int spapr_phb_children_reset(Object *child, void *opaque)
>> > {
>> > DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
>> >@@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
>> > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> > dc->cannot_instantiate_with_device_add_yet = false;
>> > spc->finish_realize = spapr_phb_finish_realize;
>> >+ spc->get_loc_code = spapr_phb_get_loc_code;
>> > }
>> >
>> > static const TypeInfo spapr_phb_info = {
>> >@@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void)
>> > spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL,
>> > "ibm,slot-error-detail",
>> > rtas_ibm_slot_error_detail);
>> >+ spapr_rtas_register(RTAS_IBM_GET_LOC_CODE,
>> >+ "ibm,get-loc-code",
>>
>>
>> s/ibm,get-loc-code/qemu,get-loc-code/ as it is not from sPAPR.
>
> Agreed.
>
>> btw we could make it even simpler and just put a property under PHB which
>> would contain a map slot:function<->loc-code, the map would be per PHB and
>> SLOF would just parse it and not call RTAS or do a hypercall. When we get
>> PCI scan in QEMU, we will just remove this chunk from the device tree (and
>> put loc-code from it to device nodes), and we won't have polluted RTAS token
>> namespace. The current thing looks too complicated for such a simple
>> function. Dunno...
>
> I think that's a good idea. Introducing a callback to get device
> information seems pretty odd, when the device tree exists to
> communicate static device information to the guest.
>
> If we're not able to go straight to qemu doing the PCI scan, I think a
> PHB property listing this information is a better option than an RTAS
> callback or hcall.
IMHO, RTAS isnt complicated, it might be odd, as we havent done such
things earlier. We are just changing the place of communicating the
information. And then pushing the complexity to SLOF to figure out in
the PHB what is its loc-name.
Here with RTAS, we are requesting per device and getting the
information.
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code
2015-03-31 5:15 ` Nikunj A Dadhania
@ 2015-03-31 7:16 ` Alexey Kardashevskiy
2015-03-31 8:15 ` Nikunj A Dadhania
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kardashevskiy @ 2015-03-31 7:16 UTC (permalink / raw)
To: Nikunj A Dadhania, David Gibson; +Cc: qemu-ppc, qemu-devel, agraf
On 03/31/2015 04:15 PM, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
>
>> On Tue, Mar 31, 2015 at 01:00:57PM +1100, Alexey Kardashevskiy wrote:
>>> On 03/30/2015 10:02 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.
>>>>
>>>> Introduce an rtas call to populate ibm,loc-code.
>>>> 1) PCI passthru devices need to identify with its own ibm,loc-code
>>>> available on the host.
>>>> 2) Emulated devices encode as following:
>>>> qemu_<name>:<phb-index>:<slot>.<fn>
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>>
>>>> Changelog
>>>> v2:
>>>> * Using rtas call for getting ibm,loc-code
>>>> * Added sPAPRPHBState::get_loc_code
>>>> * Refactored the return type of get_loc_code
>>>> * Drop stat(), and rely on g_file_get_contents
>>>> return type for file existence
>>>>
>>>> v1:
>>>> * Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
>>>> to recognise vfio devices
>>>> * Removed wrapper for hcall
>>>> * Added sPAPRPHBClass::get_loc_code
>>>>
>>>> hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>> hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++
>>>> include/hw/pci-host/spapr.h | 1 +
>>>> include/hw/ppc/spapr.h | 3 +-
>>>> 4 files changed, 113 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>>> index 05f4fac..fe6dfd5 100644
>>>> --- a/hw/ppc/spapr_pci.c
>>>> +++ b/hw/ppc/spapr_pci.c
>>>> @@ -580,6 +580,50 @@ param_error_exit:
>>>> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>>> }
>>>>
>>>> +static void rtas_ibm_get_loc_code(PowerPCCPU *cpu,
>>>> + sPAPREnvironment *spapr,
>>>> + uint32_t token, uint32_t nargs,
>>>> + target_ulong args, uint32_t nret,
>>>> + target_ulong rets)
>>>> +{
>>>> + sPAPRPHBState *sphb = NULL;
>>>> + sPAPRPHBClass *spc = NULL;
>>>> + char *buf = NULL;
>>>> + PCIDevice *pdev;
>>>> + uint64_t buid;
>>>> + uint32_t config_addr, loc_code, size;
>>>> +
>>>> + if ((nargs != 5) || (nret != 1)) {
>>>> + goto param_error_exit;
>>>> + }
>>>> +
>>>> + config_addr = rtas_ld(args, 0);
>>>> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
>>>> + loc_code = rtas_ld(args, 3);
>>>> + size = rtas_ld(args, 4);
>>>> +
>>>> + sphb = find_phb(spapr, buid);
>>>> + pdev = find_dev(spapr, buid, config_addr);
>>>> +
>>>> + if (!sphb || !pdev) {
>>>> + goto param_error_exit;
>>>> + }
>>>> +
>>>> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>>>> + if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
>>>> + uint32_t loc_len = strlen(buf);
>>>> +
>>>> + loc_len = (loc_len > size) ? size : loc_len;
>>>> + cpu_physical_memory_write(loc_code, buf, loc_len);
>>>> + g_free(buf);
>>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>>> + return;
>>>> + }
>>>> +
>>>> +param_error_exit:
>>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>>> +}
>>>> +
>>>> static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>>>> sPAPREnvironment *spapr,
>>>> uint32_t token, uint32_t nargs,
>>>> @@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
>>>> spapr_tce_get_iommu(tcet));
>>>> }
>>>>
>>>> +static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
>>>> + char **loc_code)
>>>> +{
>>>> + char *path = g_malloc(PATH_MAX);
>>>> +
>>>> + if (!path) {
>>>> + return false;
>>>> + }
>>>> +
>>>> + /*
>>>> + * For non-vfio devices and failures make up the location code out
>>>> + * of the name, slot and function.
>>>> + *
>>>> + * qemu_<name>:<phb-index>:<slot>.<fn>
>>>> + */
>>>> + snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
>>>> + sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>>>> + *loc_code = path;
>>>> + return true;
>>>> +}
>>>> +
>>>> static int spapr_phb_children_reset(Object *child, void *opaque)
>>>> {
>>>> DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
>>>> @@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
>>>> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>>>> dc->cannot_instantiate_with_device_add_yet = false;
>>>> spc->finish_realize = spapr_phb_finish_realize;
>>>> + spc->get_loc_code = spapr_phb_get_loc_code;
>>>> }
>>>>
>>>> static const TypeInfo spapr_phb_info = {
>>>> @@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void)
>>>> spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL,
>>>> "ibm,slot-error-detail",
>>>> rtas_ibm_slot_error_detail);
>>>> + spapr_rtas_register(RTAS_IBM_GET_LOC_CODE,
>>>> + "ibm,get-loc-code",
>>>
>>>
>>> s/ibm,get-loc-code/qemu,get-loc-code/ as it is not from sPAPR.
>>
>> Agreed.
>>
>>> btw we could make it even simpler and just put a property under PHB which
>>> would contain a map slot:function<->loc-code, the map would be per PHB and
>>> SLOF would just parse it and not call RTAS or do a hypercall. When we get
>>> PCI scan in QEMU, we will just remove this chunk from the device tree (and
>>> put loc-code from it to device nodes), and we won't have polluted RTAS token
>>> namespace. The current thing looks too complicated for such a simple
>>> function. Dunno...
>>
>> I think that's a good idea. Introducing a callback to get device
>> information seems pretty odd, when the device tree exists to
>> communicate static device information to the guest.
>>
>> If we're not able to go straight to qemu doing the PCI scan, I think a
>> PHB property listing this information is a better option than an RTAS
>> callback or hcall.
>
> IMHO, RTAS isnt complicated, it might be odd, as we havent done such
> things earlier. We are just changing the place of communicating the
> information. And then pushing the complexity to SLOF to figure out in
> the PHB what is its loc-name.
What kind of "complexity to SLOF" do you mean? You can choose how to store
loc-code in the device tree, can be a separate subnode under PHB with
properties where names are slot:fn and content is loc-code.
> Here with RTAS, we are requesting per device and getting the
> information.
... and adding a RTAS token which we will remove later and have a gap in
the token numbers.
--
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code
2015-03-31 7:16 ` Alexey Kardashevskiy
@ 2015-03-31 8:15 ` Nikunj A Dadhania
0 siblings, 0 replies; 6+ messages in thread
From: Nikunj A Dadhania @ 2015-03-31 8:15 UTC (permalink / raw)
To: Alexey Kardashevskiy, David Gibson; +Cc: qemu-ppc, qemu-devel, agraf
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 03/31/2015 04:15 PM, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>>
>>> On Tue, Mar 31, 2015 at 01:00:57PM +1100, Alexey Kardashevskiy wrote:
>>>> On 03/30/2015 10:02 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.
>>>>>
>>>>> Introduce an rtas call to populate ibm,loc-code.
>>>>> 1) PCI passthru devices need to identify with its own ibm,loc-code
>>>>> available on the host.
>>>>> 2) Emulated devices encode as following:
>>>>> qemu_<name>:<phb-index>:<slot>.<fn>
>>>>>
>>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>>> ---
>>>>>
>>>>>
>>>>> Changelog
>>>>> v2:
>>>>> * Using rtas call for getting ibm,loc-code
>>>>> * Added sPAPRPHBState::get_loc_code
>>>>> * Refactored the return type of get_loc_code
>>>>> * Drop stat(), and rely on g_file_get_contents
>>>>> return type for file existence
>>>>>
>>>>> v1:
>>>>> * Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
>>>>> to recognise vfio devices
>>>>> * Removed wrapper for hcall
>>>>> * Added sPAPRPHBClass::get_loc_code
>>>>>
>>>>> hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
>>>>> hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++
>>>>> include/hw/pci-host/spapr.h | 1 +
>>>>> include/hw/ppc/spapr.h | 3 +-
>>>>> 4 files changed, 113 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>>>> index 05f4fac..fe6dfd5 100644
>>>>> --- a/hw/ppc/spapr_pci.c
>>>>> +++ b/hw/ppc/spapr_pci.c
>>>>> @@ -580,6 +580,50 @@ param_error_exit:
>>>>> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>>>> }
>>>>>
>>>>> +static void rtas_ibm_get_loc_code(PowerPCCPU *cpu,
>>>>> + sPAPREnvironment *spapr,
>>>>> + uint32_t token, uint32_t nargs,
>>>>> + target_ulong args, uint32_t nret,
>>>>> + target_ulong rets)
>>>>> +{
>>>>> + sPAPRPHBState *sphb = NULL;
>>>>> + sPAPRPHBClass *spc = NULL;
>>>>> + char *buf = NULL;
>>>>> + PCIDevice *pdev;
>>>>> + uint64_t buid;
>>>>> + uint32_t config_addr, loc_code, size;
>>>>> +
>>>>> + if ((nargs != 5) || (nret != 1)) {
>>>>> + goto param_error_exit;
>>>>> + }
>>>>> +
>>>>> + config_addr = rtas_ld(args, 0);
>>>>> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
>>>>> + loc_code = rtas_ld(args, 3);
>>>>> + size = rtas_ld(args, 4);
>>>>> +
>>>>> + sphb = find_phb(spapr, buid);
>>>>> + pdev = find_dev(spapr, buid, config_addr);
>>>>> +
>>>>> + if (!sphb || !pdev) {
>>>>> + goto param_error_exit;
>>>>> + }
>>>>> +
>>>>> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>>>>> + if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
>>>>> + uint32_t loc_len = strlen(buf);
>>>>> +
>>>>> + loc_len = (loc_len > size) ? size : loc_len;
>>>>> + cpu_physical_memory_write(loc_code, buf, loc_len);
>>>>> + g_free(buf);
>>>>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> +param_error_exit:
>>>>> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>>>> +}
>>>>> +
>>>>> static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
>>>>> sPAPREnvironment *spapr,
>>>>> uint32_t token, uint32_t nargs,
>>>>> @@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
>>>>> spapr_tce_get_iommu(tcet));
>>>>> }
>>>>>
>>>>> +static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
>>>>> + char **loc_code)
>>>>> +{
>>>>> + char *path = g_malloc(PATH_MAX);
>>>>> +
>>>>> + if (!path) {
>>>>> + return false;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * For non-vfio devices and failures make up the location code out
>>>>> + * of the name, slot and function.
>>>>> + *
>>>>> + * qemu_<name>:<phb-index>:<slot>.<fn>
>>>>> + */
>>>>> + snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name,
>>>>> + sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>>>>> + *loc_code = path;
>>>>> + return true;
>>>>> +}
>>>>> +
>>>>> static int spapr_phb_children_reset(Object *child, void *opaque)
>>>>> {
>>>>> DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
>>>>> @@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
>>>>> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>>>>> dc->cannot_instantiate_with_device_add_yet = false;
>>>>> spc->finish_realize = spapr_phb_finish_realize;
>>>>> + spc->get_loc_code = spapr_phb_get_loc_code;
>>>>> }
>>>>>
>>>>> static const TypeInfo spapr_phb_info = {
>>>>> @@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void)
>>>>> spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL,
>>>>> "ibm,slot-error-detail",
>>>>> rtas_ibm_slot_error_detail);
>>>>> + spapr_rtas_register(RTAS_IBM_GET_LOC_CODE,
>>>>> + "ibm,get-loc-code",
>>>>
>>>>
>>>> s/ibm,get-loc-code/qemu,get-loc-code/ as it is not from sPAPR.
>>>
>>> Agreed.
>>>
>>>> btw we could make it even simpler and just put a property under PHB which
>>>> would contain a map slot:function<->loc-code, the map would be per PHB and
>>>> SLOF would just parse it and not call RTAS or do a hypercall. When we get
>>>> PCI scan in QEMU, we will just remove this chunk from the device tree (and
>>>> put loc-code from it to device nodes), and we won't have polluted RTAS token
>>>> namespace. The current thing looks too complicated for such a simple
>>>> function. Dunno...
>>>
>>> I think that's a good idea. Introducing a callback to get device
>>> information seems pretty odd, when the device tree exists to
>>> communicate static device information to the guest.
>>>
>>> If we're not able to go straight to qemu doing the PCI scan, I think a
>>> PHB property listing this information is a better option than an RTAS
>>> callback or hcall.
>>
>> IMHO, RTAS isnt complicated, it might be odd, as we havent done such
>> things earlier. We are just changing the place of communicating the
>> information. And then pushing the complexity to SLOF to figure out in
>> the PHB what is its loc-name.
>
> What kind of "complexity to SLOF" do you mean?
FORTH :-)
> You can choose how to store loc-code in the device tree, can be a
> separate subnode under PHB with properties where names are slot:fn and
> content is loc-code.
We would have single property entry for all the device in particular
order in phb node, whatever we call it.
Now SLOF probes the devices one by one and for the devices it finds it
needs to go back to the phb node and parse the node, find its entry and
set loc.
While here its pretty simple:
If I find a pci device, call an RTAS, get the loc-code, put it in the
DT.
>> Here with RTAS, we are requesting per device and getting the
>> information.
>
> ... and adding a RTAS token which we will remove later and have a gap in
> the token numbers.
Yes.
Wasnt the suggestion to use RTAS? As we have more control and we
did not want to use HCALL
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-31 8:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-30 11:02 [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code Nikunj A Dadhania
2015-03-31 2:00 ` Alexey Kardashevskiy
2015-03-31 2:29 ` David Gibson
2015-03-31 5:15 ` Nikunj A Dadhania
2015-03-31 7:16 ` Alexey Kardashevskiy
2015-03-31 8:15 ` Nikunj A Dadhania
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).