| Message ID | 1503556688-15412-4-git-send-email-sukadev@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Suka, Comments inline ... Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> writes: > diff --git a/Documentation/devicetree/bindings/powerpc/ibm,vas.txt b/Documentation/devicetree/bindings/powerpc/ibm,vas.txt > new file mode 100644 > index 0000000..0e3111d > --- /dev/null > +++ b/Documentation/devicetree/bindings/powerpc/ibm,vas.txt > @@ -0,0 +1,24 @@ > +* IBM Powerpc Virtual Accelerator Switchboard (VAS) > + > +VAS is a hardware mechanism that allows kernel subsystems and user processes > +to directly submit compression and other requests to Nest accelerators (NX) > +or other coprocessors functions. > + > +Required properties: > +- compatible : should be "ibm,vas" or "ibm,power9-vas" The driver doesn't look for the latter. > +- ibm,vas-id : A unique identifier for each instance of VAS in the system What is this? > +- reg : Should contain 4 pairs of 64-bit fields specifying the Hypervisor > + window context start and length, OS/User window context start and length, > + "Paste address" start and length, "Paste window id" start bit and number > + of bits) > +- name : "vas" I don't think the name is normally included in the binding, and in fact there's no reason why the name is important, so I'd be inclined to drop that. > diff --git a/MAINTAINERS b/MAINTAINERS > index 3c41902..abc235f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6425,6 +6425,14 @@ F: drivers/crypto/nx/nx.* > F: drivers/crypto/nx/nx_csbcpb.h > F: drivers/crypto/nx/nx_debugfs.h > > +IBM Power Virtual Accelerator Switchboard > +M: Sukadev Bhattiprolu > +L: linuxppc-dev@lists.ozlabs.org > +S: Supported > +F: arch/powerpc/platforms/powernv/vas* > +F: arch/powerpc/include/asm/vas.h > +F: arch/powerpc/include/uapi/asm/vas.h That's not in the right place, the file is sorted alphabetically. V comes after L. > diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig > index 6a6f4ef..f565454 100644 > --- a/arch/powerpc/platforms/powernv/Kconfig > +++ b/arch/powerpc/platforms/powernv/Kconfig > @@ -30,3 +30,17 @@ config OPAL_PRD > help > This enables the opal-prd driver, a facility to run processor > recovery diagnostics on OpenPower machines > + > +config PPC_VAS > + bool "IBM Virtual Accelerator Switchboard (VAS)" ^ bool, so never a module. > + depends on PPC_POWERNV && PPC_64K_PAGES > + default n It should be default y. I know the usual advice is to make things 'default n', but this has fairly tight depends already, so y is OK IMO. > diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c > new file mode 100644 > index 0000000..556156b > --- /dev/null > +++ b/arch/powerpc/platforms/powernv/vas.c > @@ -0,0 +1,183 @@ > +/* > + * Copyright 2016 IBM Corp. 2016-2017. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ #define pr_fmt(fmt) "vas: " fmt > +#include <linux/module.h> > +#include <linux/kernel.h> > +#include <linux/export.h> > +#include <linux/types.h> > +#include <linux/slab.h> > +#include <linux/platform_device.h> > +#include <linux/of_platform.h> > +#include <linux/of_address.h> > +#include <linux/of.h> > + > +#include "vas.h" > + > +static bool init_done; > +LIST_HEAD(vas_instances); Can be static. > + > +static int init_vas_instance(struct platform_device *pdev) > +{ > + int rc, vasid; > + struct vas_instance *vinst; > + struct device_node *dn = pdev->dev.of_node; > + struct resource *res; struct device_node *dn = pdev->dev.of_node; struct vas_instance *vinst; struct resource *res; int rc, vasid; Petty I know, but much prettier :) > + > + rc = of_property_read_u32(dn, "ibm,vas-id", &vasid); > + if (rc) { > + pr_err("VAS: No ibm,vas-id property for %s?\n", pdev->name); With the pr_fmt() above you don't need VAS: on the front of all these. > + return -ENODEV; > + } > + > + if (pdev->num_resources != 4) { > + pr_err("VAS: Unexpected DT configuration for [%s, %d]\n", > + pdev->name, vasid); > + return -ENODEV; > + } > + > + vinst = kcalloc(1, sizeof(*vinst), GFP_KERNEL); kzalloc() would be more normal given there's only 1. > + if (!vinst) > + return -ENOMEM; > + > + INIT_LIST_HEAD(&vinst->node); > + ida_init(&vinst->ida); > + mutex_init(&vinst->mutex); > + vinst->vas_id = vasid; > + vinst->pdev = pdev; > + > + res = &pdev->resource[0]; > + vinst->hvwc_bar_start = res->start; > + vinst->hvwc_bar_len = res->end - res->start + 1; > + > + res = &pdev->resource[1]; > + vinst->uwc_bar_start = res->start; > + vinst->uwc_bar_len = res->end - res->start + 1; You have vinst->pdev, why do you need to copy all those? I don't see the lens even used. > + res = &pdev->resource[2]; > + vinst->paste_base_addr = res->start; > + > + res = &pdev->resource[3]; > + vinst->paste_win_id_shift = 63 - res->end; ???? What if res->end is INT_MAX ? > + pr_devel("VAS: Initialized instance [%s, %d], paste_base 0x%llx, " > + "paste_win_id_shift 0x%llx\n", pdev->name, vasid, > + vinst->paste_base_addr, vinst->paste_win_id_shift); > + > + vinst->ready = true; I don't see ready used. It also shouldn't be necessary, it's not ready unless it's in the list, and if it's in the list then it's ready. If you're actually concerned about concurrent usage then you need a memory barrier here to order the stores to the vinst struct vs the addition to the list. And you need a matching barrier on the read side. > + list_add(&vinst->node, &vas_instances); > + > + dev_set_drvdata(&pdev->dev, vinst); > + > + return 0; > +} > + > +/* > + * Although this is read/used multiple times, it is written to only > + * during initialization. > + */ > +struct vas_instance *find_vas_instance(int vasid) > +{ > + struct list_head *ent; > + struct vas_instance *vinst; > + > + list_for_each(ent, &vas_instances) { > + vinst = list_entry(ent, struct vas_instance, node); > + if (vinst->vas_id == vasid) > + return vinst; > + } There's no locking here, or any reference counting of the instances. see > + pr_devel("VAS: Instance %d not found\n", vasid); > + return NULL; > +} > + > +bool vas_initialized(void) > +{ > + return init_done; > +} > + > +static int vas_probe(struct platform_device *pdev) > +{ > + if (!pdev || !pdev->dev.of_node) > + return -ENODEV; Neither of those can happen, or if they did it would be a BUG. > + return init_vas_instance(pdev); > +} > + > +static void free_inst(struct vas_instance *vinst) > +{ > + list_del(&vinst->node); > + kfree(vinst); And here you just delete and the free the instance, even though you have handed out pointers to the instance above in find_vas_instance(). So that needs work. Is there any hardware cleanup required before we delete the instance? Or do we just leave any windows there? Seems like to begin with you should probably just not support remove. > +static int vas_remove(struct platform_device *pdev) > +{ > + struct vas_instance *vinst; > + > + vinst = dev_get_drvdata(&pdev->dev); > + > + pr_devel("VAS: Removed instance [%s, %d]\n", pdev->name, > + vinst->vas_id); > + free_inst(vinst); > + > + return 0; > +} > +static const struct of_device_id powernv_vas_match[] = { > + { .compatible = "ibm,vas",}, > + {}, > +}; > + > +static struct platform_driver vas_driver = { > + .driver = { > + .name = "vas", > + .of_match_table = powernv_vas_match, > + }, > + .probe = vas_probe, > + .remove = vas_remove, > +}; > + > +module_platform_driver(vas_driver); Can't be a module. > + > +int vas_init(void) > +{ > + int found = 0; > + struct device_node *dn; > + > + for_each_compatible_node(dn, NULL, "ibm,vas") { > + of_platform_device_create(dn, NULL, NULL); > + found++; > + } > + > + if (!found) > + return -ENODEV; > + > + pr_devel("VAS: Found %d instances\n", found); > + init_done = true; What does init_done mean? The way it's currently written it just means there were some ibm,vas nodes in the device tree. But it doesn't say that we actually probed them correctly and created vas instances for them. So it doesn't really tell you much. Two of the callers of vas_initialized() immediately do a find_instance(), so they don't really need to call it at all, the lack of any instances will suffice. The other two callers are vas_copy_crb() and vas_paste_crb(). The only use of the former is in nx which doesn't check the return code. But both should never be called unless we allocated a window anyway. In short it seems unecessary. > + > + return 0; > +} > + > +void vas_exit(void) > +{ > + struct list_head *ent; > + struct vas_instance *vinst; > + > + list_for_each(ent, &vas_instances) { > + vinst = list_entry(ent, struct vas_instance, node); > + of_platform_depopulate(&vinst->pdev->dev); > + } > + > + init_done = false; > +} > + > +module_init(vas_init); > +module_exit(vas_exit); > +MODULE_DESCRIPTION("Bare metal IBM Virtual Accelerator Switchboard"); > +MODULE_AUTHOR("Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>"); > +MODULE_LICENSE("GPL"); Can't be a module. Just a device_initcall() should work for init, you shouldn't need vas_exit() or any of the other bits. cheers
Michael Ellerman [mpe@ellerman.id.au] wrote: > Hi Suka, > > Comments inline ... > > Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> writes: > > diff --git a/Documentation/devicetree/bindings/powerpc/ibm,vas.txt b/Documentation/devicetree/bindings/powerpc/ibm,vas.txt > > new file mode 100644 > > index 0000000..0e3111d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/powerpc/ibm,vas.txt > > @@ -0,0 +1,24 @@ > > +* IBM Powerpc Virtual Accelerator Switchboard (VAS) > > + > > +VAS is a hardware mechanism that allows kernel subsystems and user processes > > +to directly submit compression and other requests to Nest accelerators (NX) > > +or other coprocessors functions. > > + > > +Required properties: > > +- compatible : should be "ibm,vas" or "ibm,power9-vas" > > The driver doesn't look for the latter. Ok. I have removed it from this list of required properties > > > +- ibm,vas-id : A unique identifier for each instance of VAS in the system > > What is this? Like the ibm,chip-id, but in the future, there could be more than one instance of VAS per chip, so firmware assigns a unique id to each instance of VAS. > > > +- reg : Should contain 4 pairs of 64-bit fields specifying the Hypervisor > > + window context start and length, OS/User window context start and length, > > + "Paste address" start and length, "Paste window id" start bit and number > > + of bits) > > +- name : "vas" > > I don't think the name is normally included in the binding, and in fact > there's no reason why the name is important, so I'd be inclined to drop that. Ok. I dropped it. > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 3c41902..abc235f 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6425,6 +6425,14 @@ F: drivers/crypto/nx/nx.* > > F: drivers/crypto/nx/nx_csbcpb.h > > F: drivers/crypto/nx/nx_debugfs.h > > > > +IBM Power Virtual Accelerator Switchboard > > +M: Sukadev Bhattiprolu > > +L: linuxppc-dev@lists.ozlabs.org > > +S: Supported > > +F: arch/powerpc/platforms/powernv/vas* > > +F: arch/powerpc/include/asm/vas.h > > +F: arch/powerpc/include/uapi/asm/vas.h > > That's not in the right place, the file is sorted alphabetically. ah, fixed. > > V comes after L. > > > diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig > > index 6a6f4ef..f565454 100644 > > --- a/arch/powerpc/platforms/powernv/Kconfig > > +++ b/arch/powerpc/platforms/powernv/Kconfig > > @@ -30,3 +30,17 @@ config OPAL_PRD > > help > > This enables the opal-prd driver, a facility to run processor > > recovery diagnostics on OpenPower machines > > + > > +config PPC_VAS > > + bool "IBM Virtual Accelerator Switchboard (VAS)" > > ^ bool, so never a module. yes, it should be built in. > > > + depends on PPC_POWERNV && PPC_64K_PAGES > > + default n > > It should be default y. > > I know the usual advice is to make things 'default n', but this has > fairly tight depends already, so y is OK IMO. Ok. > > > diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c > > new file mode 100644 > > index 0000000..556156b > > --- /dev/null > > +++ b/arch/powerpc/platforms/powernv/vas.c > > @@ -0,0 +1,183 @@ > > +/* > > + * Copyright 2016 IBM Corp. > > 2016-2017. Ok. > > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * as published by the Free Software Foundation; either version > > + * 2 of the License, or (at your option) any later version. > > + */ > > #define pr_fmt(fmt) "vas: " fmt Ok > > > +#include <linux/module.h> > > +#include <linux/kernel.h> > > +#include <linux/export.h> > > +#include <linux/types.h> > > +#include <linux/slab.h> > > +#include <linux/platform_device.h> > > +#include <linux/of_platform.h> > > +#include <linux/of_address.h> > > +#include <linux/of.h> > > + > > +#include "vas.h" > > + > > +static bool init_done; > > +LIST_HEAD(vas_instances); > > Can be static. Yes > > > + > > +static int init_vas_instance(struct platform_device *pdev) > > +{ > > + int rc, vasid; > > + struct vas_instance *vinst; > > + struct device_node *dn = pdev->dev.of_node; > > + struct resource *res; > > struct device_node *dn = pdev->dev.of_node; > struct vas_instance *vinst; > struct resource *res; > int rc, vasid; > > Petty I know, but much prettier :) I usually go the opposite way (shortest first) so I have done that here also. For newer files I will invert the tree. > > > + > > + rc = of_property_read_u32(dn, "ibm,vas-id", &vasid); > > + if (rc) { > > + pr_err("VAS: No ibm,vas-id property for %s?\n", pdev->name); > > With the pr_fmt() above you don't need VAS: on the front of all these. Ok > > > + return -ENODEV; > > + } > > + > > + if (pdev->num_resources != 4) { > > + pr_err("VAS: Unexpected DT configuration for [%s, %d]\n", > > + pdev->name, vasid); > > + return -ENODEV; > > + } > > + > > + vinst = kcalloc(1, sizeof(*vinst), GFP_KERNEL); > > kzalloc() would be more normal given there's only 1. Yes. > > > + if (!vinst) > > + return -ENOMEM; > > + > > + INIT_LIST_HEAD(&vinst->node); > > + ida_init(&vinst->ida); > > + mutex_init(&vinst->mutex); > > + vinst->vas_id = vasid; > > + vinst->pdev = pdev; > > + > > + res = &pdev->resource[0]; > > + vinst->hvwc_bar_start = res->start; > > + vinst->hvwc_bar_len = res->end - res->start + 1; > > + > > + res = &pdev->resource[1]; > > + vinst->uwc_bar_start = res->start; > > + vinst->uwc_bar_len = res->end - res->start + 1; > > You have vinst->pdev, why do you need to copy all those? > > I don't see the lens even used. I have dropped the len fields. Kept the starts for now as it might be easier to understand what the field is. > > > + res = &pdev->resource[2]; > > + vinst->paste_base_addr = res->start; > > + > > + res = &pdev->resource[3]; > > + vinst->paste_win_id_shift = 63 - res->end; > > ???? > > What if res->end is INT_MAX ? Good question. I have added a check for res->end exceeding 62 but am not sure how else to error check this or, for that matter, the res->start fields that we get from skiboot. > > > + pr_devel("VAS: Initialized instance [%s, %d], paste_base 0x%llx, " > > + "paste_win_id_shift 0x%llx\n", pdev->name, vasid, > > + vinst->paste_base_addr, vinst->paste_win_id_shift); > > + > > + vinst->ready = true; > > I don't see ready used. Yes, we don't need it now. I have dropped it. > > It also shouldn't be necessary, it's not ready unless it's in the list, > and if it's in the list then it's ready. > > If you're actually concerned about concurrent usage then you need a > memory barrier here to order the stores to the vinst struct vs the > addition to the list. And you need a matching barrier on the read side. > > > + list_add(&vinst->node, &vas_instances); > > + > > + dev_set_drvdata(&pdev->dev, vinst); > > + > > + return 0; > > +} > > + > > +/* > > + * Although this is read/used multiple times, it is written to only > > + * during initialization. > > + */ > > +struct vas_instance *find_vas_instance(int vasid) > > +{ > > + struct list_head *ent; > > + struct vas_instance *vinst; > > + > > + list_for_each(ent, &vas_instances) { > > + vinst = list_entry(ent, struct vas_instance, node); > > + if (vinst->vas_id == vasid) > > + return vinst; > > + } > > There's no locking here, or any reference counting of the instances. see The vas_instances list is populated at startup and never really modified (besides, the vas_exit() code which never gets called and has now been dropped). I was trying to use vas_initialized() and avoid locking in find_vas_instance(). I have now dropped vas_initialized() and added a lock here - this is used in the window setup but not in copy/paste path, so the lock should not matter much. > > > + pr_devel("VAS: Instance %d not found\n", vasid); > > + return NULL; > > +} > > + > > +bool vas_initialized(void) > > +{ > > + return init_done; > > +} > > + > > +static int vas_probe(struct platform_device *pdev) > > +{ > > + if (!pdev || !pdev->dev.of_node) > > + return -ENODEV; > > Neither of those can happen, or if they did it would be a BUG. Ok. Changed to BUG_ON. > > > + return init_vas_instance(pdev); > > +} > > + > > +static void free_inst(struct vas_instance *vinst) > > +{ > > + list_del(&vinst->node); > > + kfree(vinst); > > And here you just delete and the free the instance, even though you have > handed out pointers to the instance above in find_vas_instance(). > > So that needs work. > > Is there any hardware cleanup required before we delete the instance? Or > do we just leave any windows there? > > Seems like to begin with you should probably just not support remove. Yes, I have dropped support for the remove() > > > +static int vas_remove(struct platform_device *pdev) > > +{ > > + struct vas_instance *vinst; > > + > > + vinst = dev_get_drvdata(&pdev->dev); > > + > > + pr_devel("VAS: Removed instance [%s, %d]\n", pdev->name, > > + vinst->vas_id); > > + free_inst(vinst); > > + > > + return 0; > > +} > > +static const struct of_device_id powernv_vas_match[] = { > > + { .compatible = "ibm,vas",}, > > + {}, > > +}; > > + > > +static struct platform_driver vas_driver = { > > + .driver = { > > + .name = "vas", > > + .of_match_table = powernv_vas_match, > > + }, > > + .probe = vas_probe, > > + .remove = vas_remove, > > +}; > > + > > +module_platform_driver(vas_driver); > > Can't be a module. Yes, its now built-in and not a module anymore. > > > + > > +int vas_init(void) > > +{ > > + int found = 0; > > + struct device_node *dn; > > + > > + for_each_compatible_node(dn, NULL, "ibm,vas") { > > + of_platform_device_create(dn, NULL, NULL); > > + found++; > > + } > > + > > + if (!found) > > + return -ENODEV; > > + > > + pr_devel("VAS: Found %d instances\n", found); > > + init_done = true; > > What does init_done mean? > > The way it's currently written it just means there were some ibm,vas > nodes in the device tree. But it doesn't say that we actually probed > them correctly and created vas instances for them. > > So it doesn't really tell you much. > > Two of the callers of vas_initialized() immediately do a > find_instance(), so they don't really need to call it at all, the lack > of any instances will suffice. > > The other two callers are vas_copy_crb() and vas_paste_crb(). The only > use of the former is in nx which doesn't check the return code. > > But both should never be called unless we allocated a window anyway. > > In short it seems unecessary. Yes, I have dropped vas_initialized(). > > > + > > + return 0; > > +} > > + > > +void vas_exit(void) > > +{ > > + struct list_head *ent; > > + struct vas_instance *vinst; > > + > > + list_for_each(ent, &vas_instances) { > > + vinst = list_entry(ent, struct vas_instance, node); > > + of_platform_depopulate(&vinst->pdev->dev); > > + } > > + > > + init_done = false; > > +} > > + > > +module_init(vas_init); > > +module_exit(vas_exit); > > +MODULE_DESCRIPTION("Bare metal IBM Virtual Accelerator Switchboard"); > > +MODULE_AUTHOR("Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>"); > > +MODULE_LICENSE("GPL"); > > Can't be a module. > > Just a device_initcall() should work for init, you shouldn't need > vas_exit() or any of the other bits. Yes, fixed. > > cheers
diff --git a/Documentation/devicetree/bindings/powerpc/ibm,vas.txt b/Documentation/devicetree/bindings/powerpc/ibm,vas.txt new file mode 100644 index 0000000..0e3111d --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/ibm,vas.txt @@ -0,0 +1,24 @@ +* IBM Powerpc Virtual Accelerator Switchboard (VAS) + +VAS is a hardware mechanism that allows kernel subsystems and user processes +to directly submit compression and other requests to Nest accelerators (NX) +or other coprocessors functions. + +Required properties: +- compatible : should be "ibm,vas" or "ibm,power9-vas" +- ibm,vas-id : A unique identifier for each instance of VAS in the system +- reg : Should contain 4 pairs of 64-bit fields specifying the Hypervisor + window context start and length, OS/User window context start and length, + "Paste address" start and length, "Paste window id" start bit and number + of bits) +- name : "vas" + +Example: + + vas@6019100000000 { + compatible = "ibm,vas", "ibm,power9-vas"; + reg = <0x6019100000000 0x2000000 0x6019000000000 0x100000000 0x8000000000000 0x100000000 0x20 0x10>; + name = "vas"; + ibm,vas-id = <0x1>; + }; + diff --git a/MAINTAINERS b/MAINTAINERS index 3c41902..abc235f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6425,6 +6425,14 @@ F: drivers/crypto/nx/nx.* F: drivers/crypto/nx/nx_csbcpb.h F: drivers/crypto/nx/nx_debugfs.h +IBM Power Virtual Accelerator Switchboard +M: Sukadev Bhattiprolu +L: linuxppc-dev@lists.ozlabs.org +S: Supported +F: arch/powerpc/platforms/powernv/vas* +F: arch/powerpc/include/asm/vas.h +F: arch/powerpc/include/uapi/asm/vas.h + IBM Power Linux RAID adapter M: Brian King <brking@us.ibm.com> S: Supported diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig index 6a6f4ef..f565454 100644 --- a/arch/powerpc/platforms/powernv/Kconfig +++ b/arch/powerpc/platforms/powernv/Kconfig @@ -30,3 +30,17 @@ config OPAL_PRD help This enables the opal-prd driver, a facility to run processor recovery diagnostics on OpenPower machines + +config PPC_VAS + bool "IBM Virtual Accelerator Switchboard (VAS)" + depends on PPC_POWERNV && PPC_64K_PAGES + default n + help + This enables support for IBM Virtual Accelerator Switchboard (VAS). + + VAS allows accelerators in co-processors like NX-GZIP and NX-842 + to be accessible to kernel subsystems and user processes. + + VAS adapters are found in POWER9 based systems. + + If unsure, say N. diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile index b5d98cb..e4db292 100644 --- a/arch/powerpc/platforms/powernv/Makefile +++ b/arch/powerpc/platforms/powernv/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_PPC_SCOM) += opal-xscom.o obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o obj-$(CONFIG_TRACEPOINTS) += opal-tracepoints.o obj-$(CONFIG_OPAL_PRD) += opal-prd.o +obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c new file mode 100644 index 0000000..6156fbe --- /dev/null +++ b/arch/powerpc/platforms/powernv/vas-window.c @@ -0,0 +1,19 @@ +/* + * Copyright 2016 IBM Corp. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include <linux/types.h> +#include <linux/mutex.h> + +#include "vas.h" + +/* stub for now */ +int vas_win_close(struct vas_window *window) +{ + return -1; +} diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c new file mode 100644 index 0000000..556156b --- /dev/null +++ b/arch/powerpc/platforms/powernv/vas.c @@ -0,0 +1,183 @@ +/* + * Copyright 2016 IBM Corp. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/export.h> +#include <linux/types.h> +#include <linux/slab.h> +#include <linux/platform_device.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> +#include <linux/of.h> + +#include "vas.h" + +static bool init_done; +LIST_HEAD(vas_instances); + +static int init_vas_instance(struct platform_device *pdev) +{ + int rc, vasid; + struct vas_instance *vinst; + struct device_node *dn = pdev->dev.of_node; + struct resource *res; + + rc = of_property_read_u32(dn, "ibm,vas-id", &vasid); + if (rc) { + pr_err("VAS: No ibm,vas-id property for %s?\n", pdev->name); + return -ENODEV; + } + + if (pdev->num_resources != 4) { + pr_err("VAS: Unexpected DT configuration for [%s, %d]\n", + pdev->name, vasid); + return -ENODEV; + } + + vinst = kcalloc(1, sizeof(*vinst), GFP_KERNEL); + if (!vinst) + return -ENOMEM; + + INIT_LIST_HEAD(&vinst->node); + ida_init(&vinst->ida); + mutex_init(&vinst->mutex); + vinst->vas_id = vasid; + vinst->pdev = pdev; + + res = &pdev->resource[0]; + vinst->hvwc_bar_start = res->start; + vinst->hvwc_bar_len = res->end - res->start + 1; + + res = &pdev->resource[1]; + vinst->uwc_bar_start = res->start; + vinst->uwc_bar_len = res->end - res->start + 1; + + res = &pdev->resource[2]; + vinst->paste_base_addr = res->start; + + res = &pdev->resource[3]; + vinst->paste_win_id_shift = 63 - res->end; + + pr_devel("VAS: Initialized instance [%s, %d], paste_base 0x%llx, " + "paste_win_id_shift 0x%llx\n", pdev->name, vasid, + vinst->paste_base_addr, vinst->paste_win_id_shift); + + vinst->ready = true; + list_add(&vinst->node, &vas_instances); + + dev_set_drvdata(&pdev->dev, vinst); + + return 0; +} + +/* + * Although this is read/used multiple times, it is written to only + * during initialization. + */ +struct vas_instance *find_vas_instance(int vasid) +{ + struct list_head *ent; + struct vas_instance *vinst; + + list_for_each(ent, &vas_instances) { + vinst = list_entry(ent, struct vas_instance, node); + if (vinst->vas_id == vasid) + return vinst; + } + + pr_devel("VAS: Instance %d not found\n", vasid); + return NULL; +} + +bool vas_initialized(void) +{ + return init_done; +} + +static int vas_probe(struct platform_device *pdev) +{ + if (!pdev || !pdev->dev.of_node) + return -ENODEV; + + return init_vas_instance(pdev); +} + +static void free_inst(struct vas_instance *vinst) +{ + list_del(&vinst->node); + + kfree(vinst); +} + +static int vas_remove(struct platform_device *pdev) +{ + struct vas_instance *vinst; + + vinst = dev_get_drvdata(&pdev->dev); + + pr_devel("VAS: Removed instance [%s, %d]\n", pdev->name, + vinst->vas_id); + free_inst(vinst); + + return 0; +} +static const struct of_device_id powernv_vas_match[] = { + { .compatible = "ibm,vas",}, + {}, +}; + +static struct platform_driver vas_driver = { + .driver = { + .name = "vas", + .of_match_table = powernv_vas_match, + }, + .probe = vas_probe, + .remove = vas_remove, +}; + +module_platform_driver(vas_driver); + +int vas_init(void) +{ + int found = 0; + struct device_node *dn; + + for_each_compatible_node(dn, NULL, "ibm,vas") { + of_platform_device_create(dn, NULL, NULL); + found++; + } + + if (!found) + return -ENODEV; + + pr_devel("VAS: Found %d instances\n", found); + init_done = true; + + return 0; +} + +void vas_exit(void) +{ + struct list_head *ent; + struct vas_instance *vinst; + + list_for_each(ent, &vas_instances) { + vinst = list_entry(ent, struct vas_instance, node); + of_platform_depopulate(&vinst->pdev->dev); + } + + init_done = false; +} + +module_init(vas_init); +module_exit(vas_exit); +MODULE_DESCRIPTION("Bare metal IBM Virtual Accelerator Switchboard"); +MODULE_AUTHOR("Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>"); +MODULE_LICENSE("GPL"); diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h index c66aaf0..150d7b1 100644 --- a/arch/powerpc/platforms/powernv/vas.h +++ b/arch/powerpc/platforms/powernv/vas.h @@ -382,4 +382,7 @@ struct vas_winctx { enum vas_notify_after_count notify_after_count; }; +extern bool vas_initialized(void); +extern struct vas_instance *find_vas_instance(int vasid); + #endif /* _VAS_H */
Implement vas_init() and vas_exit() functions for a new VAS module. This VAS module is essentially a library for other device drivers and kernel users of the NX coprocessors like NX-842 and NX-GZIP. In the future this will be extended to add support for user space to access the NX coprocessors. VAS is currently only supported with 64K page size. Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> --- Changelog[v5]: - [Ben Herrenschmidt]: Create and use platform device tree nodes, fix up the "reg" properties for the VAS DT node and use the platform device helpers to parse the reg properties; Use linked list of VAS instances (don't assume vasids are sequential); Use CONFIG_PPC_VAS instead of CONFIG_VAS. Changelog[v4]: - [Michael Neuling] Fix some accidental deletions; fix help text in Kconfig; change vas_initialized to a function; move from drivers/misc to arch/powerpc/kernel - Drop the vas_window_reset() interface. It is not needed as window will be initialized before each use. - Add a "depends on PPC_64K_PAGES" Changelog[v3]: - Zero vas_instances memory on allocation - [Haren Myneni] Fix description in Kconfig Changelog[v2]: - Get HVWC, UWC and window address parameters from device tree. --- .../devicetree/bindings/powerpc/ibm,vas.txt | 24 +++ MAINTAINERS | 8 + arch/powerpc/platforms/powernv/Kconfig | 14 ++ arch/powerpc/platforms/powernv/Makefile | 1 + arch/powerpc/platforms/powernv/vas-window.c | 19 +++ arch/powerpc/platforms/powernv/vas.c | 183 +++++++++++++++++++++ arch/powerpc/platforms/powernv/vas.h | 3 + 7 files changed, 252 insertions(+) create mode 100644 Documentation/devicetree/bindings/powerpc/ibm,vas.txt create mode 100644 arch/powerpc/platforms/powernv/vas-window.c create mode 100644 arch/powerpc/platforms/powernv/vas.c