On Mon, Dec 12, 2022 at 03:46:58PM -0800, Elliot Berman wrote:
>
>
> On 11/21/2022 7:23 AM, Greg Kroah-Hartman wrote:
> > On Mon, Nov 21, 2022 at 05:59:59AM -0800, Elliot Berman wrote:
> > > The resource manager is a special virtual machine which is always
> > > running on a Gunyah system. It provides APIs for creating and destroying
> > > VMs, secure memory management, sharing/lending of memory between VMs,
> > > and setup of inter-VM communication. Calls to the resource manager are
> > > made via message queues.
> > >
> > > This patch implements the basic probing and RPC mechanism to make those
> > > API calls. Request/response calls can be made with gh_rm_call.
> > > Drivers can also register to notifications pushed by RM via
> > > gh_rm_register_notifier
> > >
> > > Specific API calls that resource manager supports will be implemented in
> > > subsequent patches.
> > >
> > > Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
> > > ---
> > > MAINTAINERS | 2 +-
> > > drivers/virt/gunyah/Kconfig | 7 +
> > > drivers/virt/gunyah/Makefile | 2 +
> > > drivers/virt/gunyah/gunyah_rm_rpc.c | 570 +++++++++++++++++++++++++++
> > > drivers/virt/gunyah/gunyah_rsc_mgr.c | 50 +++
> > > drivers/virt/gunyah/rsc_mgr.h | 37 ++
> > > include/linux/gunyah_rsc_mgr.h | 18 +
> > > 7 files changed, 685 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/virt/gunyah/gunyah_rm_rpc.c
> > > create mode 100644 drivers/virt/gunyah/gunyah_rsc_mgr.c
> > > create mode 100644 drivers/virt/gunyah/rsc_mgr.h
> > > create mode 100644 include/linux/gunyah_rsc_mgr.h
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 502798197b80..b65f7ff444e5 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -8948,7 +8948,7 @@ F: Documentation/virt/gunyah/
> > > F: arch/arm64/gunyah/
> > > F: drivers/mailbox/gunyah-msgq.c
> > > F: drivers/virt/gunyah/
> > > -F: include/linux/gunyah.h
> > > +F: include/linux/gunyah*.h
> > > HABANALABS PCI DRIVER
> > > M: Oded Gabbay <ogabbay@xxxxxxxxxx>
> > > diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig
> > > index 127156a678a6..0bb497372d4e 100644
> > > --- a/drivers/virt/gunyah/Kconfig
> > > +++ b/drivers/virt/gunyah/Kconfig
> > > @@ -10,3 +10,10 @@ config GUNYAH
> > > Say Y/M here to enable the drivers needed to interact in a Gunyah
> > > virtual environment.
> > > +
> > > +if GUNYAH
> > > +config GUNYAH_RESOURCE_MANAGER
> > > + tristate
> > > + depends on MAILBOX
> > > + select GUNYAH_MESSAGE_QUEUES
> > > +endif
> > > diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile
> > > index 2ac4ee64b89d..b62ac4045621 100644
> > > --- a/drivers/virt/gunyah/Makefile
> > > +++ b/drivers/virt/gunyah/Makefile
> > > @@ -1 +1,3 @@
> > > obj-$(CONFIG_GUNYAH) += gunyah.o
> > > +
> > > +obj-$(CONFIG_GUNYAH_RESOURCE_MANAGER) += gunyah_rsc_mgr.o gunyah_rm_rpc.o
> > > diff --git a/drivers/virt/gunyah/gunyah_rm_rpc.c b/drivers/virt/gunyah/gunyah_rm_rpc.c
> > > new file mode 100644
> > > index 000000000000..45b1a8691982
> > > --- /dev/null
> > > +++ b/drivers/virt/gunyah/gunyah_rm_rpc.c
> > > @@ -0,0 +1,570 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
> > > + */
> > > +
> > > +#include <linux/of.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/sched.h>
> > > +#include <linux/gunyah.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_irq.h>
> > > +#include <linux/kthread.h>
> > > +#include <linux/notifier.h>
> > > +#include <linux/workqueue.h>
> > > +#include <linux/completion.h>
> > > +#include <linux/gunyah_rsc_mgr.h>
> > > +#include <linux/platform_device.h>
> >
> > This should not have anything to do with a platform device, please see
> > below.
> >
> > > +#include <linux/gunyah_rsc_mgr.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +#include "rsc_mgr.h"
> > > +
> > > +static int gh_rm_drv_probe(struct platform_device *pdev)
> >
> > Why is this tied to a platformm device?
> >
> > I don't understand the relationship here, sorry.
> > >> +{
> > > + struct gh_rm_rpc *rsc_mgr;
> > > +
> > > + rsc_mgr = gh_rm_rpc_init(pdev);
> >
> > Shouldn't this be creating a new one that is just a child passed in?
> > Shouldn't this call just take a 'struct device *'?
> >
>
> I'm following the suggestion from Arnd to have small core module that calls
> into initialization routines for the other parts of the driver, rather than
> creating bus for a few (2) devices.
>
> https://lore.kernel.org/all/a3754259-9989-495e-a6bd-5501daff06a2@xxxxxxxxxxxxxxxx/
2 devices is still a bus. Please don't mess with the driver model in
ways it is not ment to be messed with if at all possible.
And again, don't abuse platform devices for dynamically discovered
devices like this.
thanks,
greg k-h
