[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code |
Date: |
Fri, 27 Mar 2015 14:28:00 +1100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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.
> +#define KVMPPC_HCALL_MAX KVMPPC_H_GET_LOC_CODE
>
> extern sPAPREnvironment *spapr;
>
> @@ -522,6 +525,9 @@ int spapr_tcet_dma_dt(void *fdt, int node_off, const char
> *propname,
> sPAPRTCETable *tcet);
> void spapr_pci_switch_vga(bool big_endian);
>
> +bool spapr_h_get_loc_code(sPAPREnvironment *spapr, target_ulong config_addr,
> target_ulong build,
> + target_ulong loc_code, target_ulong size);
> +
> #define TYPE_SPAPR_RTC "spapr-rtc"
>
> void spapr_rtc_read(DeviceState *dev, struct tm *tm, uint32_t *ns);
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0d1fb80..6dffa8c 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -140,6 +140,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as);
> void vfio_put_group(VFIOGroup *group);
> int vfio_get_device(VFIOGroup *group, const char *name,
> VFIODevice *vbasedev);
> +bool vfio_get_devspec(PCIDevice *pdev, char **value);
>
> extern const MemoryRegionOps vfio_region_ops;
> extern QLIST_HEAD(vfio_group_head, VFIOGroup) vfio_group_list;
--
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
pgpp1CTs6nWqr.pgp
Description: PGP signature
- [Qemu-ppc] [PATCH 1/2] vfio-pci: add flag to identify vfio pci device, Nikunj A Dadhania, 2015/03/26
- [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code, Nikunj A Dadhania, 2015/03/26
- Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code,
David Gibson <=
- Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code, Alexey Kardashevskiy, 2015/03/26
- Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code, Nikunj A Dadhania, 2015/03/27
- Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code, David Gibson, 2015/03/27
- Re: [Qemu-ppc] [PATCH 2/2] spapr: populate ibm,loc-code, Nikunj A Dadhania, 2015/03/27
Re: [Qemu-ppc] [PATCH 1/2] vfio-pci: add flag to identify vfio pci device, Alexey Kardashevskiy, 2015/03/26
Re: [Qemu-ppc] [PATCH 1/2] vfio-pci: add flag to identify vfio pci device, Alex Williamson, 2015/03/31