On Thu, Mar 26, 2015 at 12:12:12PM +0530, 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 hcall to populate ibm,loc-cde.
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>:<slot>.<fn>
Signed-off-by: Nikunj A Dadhania <address@hidden>
---
hw/ppc/spapr_hcall.c | 10 +++++++++
hw/ppc/spapr_pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++
hw/vfio/pci.c | 18 ++++++++++++++++
include/hw/ppc/spapr.h | 8 ++++++-
include/hw/vfio/vfio-common.h | 1 +
5 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4f76f1c..a577395 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -928,6 +928,15 @@ static target_ulong
h_client_architecture_support(PowerPCCPU *cpu_,
return H_SUCCESS;
}
+static target_ulong h_get_loc_code(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+ if (!spapr_h_get_loc_code(spapr, args[0], args[1], args[2], args[3])) {
+ return H_PARAMETER;
+ }
+ return H_SUCCESS;
+}
There's no point to this wrapper. The hypercalls are defined by PAPR,
so making an "spapr" version of the hypercall function is redundant.
+
static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX -
KVMPPC_HCALL_BASE + 1];
@@ -1010,6 +1019,7 @@ static void hypercall_register_types(void)
/* ibm,client-architecture-support support */
spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
+ spapr_register_hypercall(KVMPPC_H_GET_LOC_CODE, h_get_loc_code);
}
type_init(hypercall_register_types)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 05f4fac..65cdb91 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -35,6 +35,7 @@
#include "qemu/error-report.h"
#include "hw/pci/pci_bus.h"
+#include "hw/vfio/vfio-common.h"
/* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
#define RTAS_QUERY_FN 0
@@ -248,6 +249,54 @@ static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr,
bool msix,
}
}
+bool spapr_h_get_loc_code(sPAPREnvironment *spapr, target_ulong config_addr,
target_ulong buid,
+ target_ulong loc_code, target_ulong size)
bool as a success/failure indication isn't a normal interface. Just
get rid of the wrapper and return H_ERROR codes directly.
+{
+ sPAPRPHBState *sphb = NULL;
+ PCIDevice *pdev = NULL;
+ char *buf, path[PATH_MAX];
+ struct stat st;
+
+ sphb = find_phb(spapr, buid);
+ if (sphb) {
+ pdev = find_dev(spapr, buid, config_addr);
+ }
+
+ if (!sphb || !pdev) {
+ error_report("spapr_h_get_loc_code: Device not found");
+ return false;
+ }
+
+ /* For a VFIO device, get the location in the device tree */
+ if (pdev->is_vfio && vfio_get_devspec(pdev, &buf)) {
+ snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
+ g_free(buf);
+ if (stat(path, &st) < 0) {
+ goto fail;
This isn't really an acceptable use of goto. And the label is badly
named, because it doesn't fail, just fall back to an alternate method.
+ }
+
+ /* A valid file, now read the loc-code */
+ if (g_file_get_contents(path, &buf, NULL, NULL)) {
+ cpu_physical_memory_write(loc_code, buf, strlen(buf));
+ g_free(buf);
+ goto out;
This could just be a return.
+ }
+ }
+
+fail:
+ /*
+ * For non-vfio devices and failure cases, make up the location
+ * code out of the name, slot and function.
+ *
+ * qemu_<name>:<slot>.<fn>
+ */
+ snprintf(path, sizeof(path), "qemu_%s:%02d.%1d", pdev->name,
+ PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+ cpu_physical_memory_write(loc_code, path, size);
+ out:
+ return true;
+}
+
static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
uint32_t token, uint32_t nargs,
target_ulong args, uint32_t nret,
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 95d666e..dd97258 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3319,6 +3319,24 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice
*vdev)
vdev->req_enabled = false;
}
+bool vfio_get_devspec(PCIDevice *pdev, char **value)
+{
+ VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+ char path[PATH_MAX];
+ struct stat st;
+
+ snprintf(path, sizeof(path),
+ "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/devspec",
+ vdev->host.domain, vdev->host.bus, vdev->host.slot,
+ vdev->host.function);
+
+ if (stat(path, &st) < 0) {
+ return false;
+ }
+
+ return g_file_get_contents(path, value, NULL, NULL);
+}
+
static int vfio_initfn(PCIDevice *pdev)
{
VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index af71e8b..d3fad67 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -310,7 +310,10 @@ typedef struct sPAPREnvironment {
#define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
/* Client Architecture support */
#define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2)
-#define KVMPPC_HCALL_MAX KVMPPC_H_CAS
+#define KVMPPC_H_RTAS_UPDATE (KVMPPC_HCALL_BASE + 0x3)
+#define KVMPPC_H_REPORT_MC_ERR (KVMPPC_HCALL_BASE + 0x4)
+#define KVMPPC_H_GET_LOC_CODE (KVMPPC_HCALL_BASE + 0x5)
Come to that, I don't even understand why you're defining a new hcall.
Why not just put the loc-code in the initial device tree with the
other information.