On 2023-04-28 23:23, Besar Wicaksono wrote:
Hi Robin and Suzuki,
-----Original Message-----
From: Robin Murphy <robin.murphy@xxxxxxx>
Sent: Thursday, April 27, 2023 7:21 AM
To: Besar Wicaksono <bwicaksono@xxxxxxxxxx>; suzuki.poulose@xxxxxxx;
catalin.marinas@xxxxxxx; will@xxxxxxxxxx; mark.rutland@xxxxxxx
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
tegra@xxxxxxxxxxxxxxx; Thierry Reding <treding@xxxxxxxxxx>; Jonathan
Hunter <jonathanh@xxxxxxxxxx>; Vikram Sethi <vsethi@xxxxxxxxxx>; Richard
Wiley <rwiley@xxxxxxxxxx>; Eric Funsten <efunsten@xxxxxxxxxx>
Subject: Re: [PATCH v2] perf: arm_cspmu: Separate Arm and vendor module
External email: Use caution opening links or attachments
On 2023-04-18 07:20, Besar Wicaksono wrote:
Arm Coresight PMU driver consists of main standard code and vendor
backend code. Both are currently built as a single module.
This patch adds vendor registration API to separate the two to
keep things modular. Vendor module shall register to the main
module on loading and trigger device reprobe.
I think it might be considerably cleaner and safer if the main driver
retained at least some knowledge of the PMIIDR matches and used those to
explicity request the relevant module. Otherwise, not only is there an
awful lot of fiddly complexity here, but there's also quite a burden on
the user to know which modules they have to load to get full
functionality on any given system.
Do you mean like keep the existing match table as a whitelist, and associate
each entry with the backend module name to load it from the main driver ?
It would essentially be a table that matches a PMIIDR filter to a module
name. Having looked for existing examples of this kind of usage model,
in terms of the overall shape it might look closest to a very
stripped-down version of the crypto manager.
FYI I've just started working on adding devicetree support, and I do
need the generic architectural functionality to keep working in the
absence of any imp-def backend.
W.r.t the reprobe discussion with Suzuki, this would mean the expected
behavior is to attach the device to standard imp as fallback/default.
Suzuki, my preference is not supporting delayed reprobe on event->destroy
due to the potential access to stale data. We should just fail the backend
registration if one of the device is in use.
We shouldn't need to worry about that at all. If requesting an expected
implementation module fails and those PMUs end up falling back to
baseline functionality, there's no need to actively deny the backend if
the module is manually loaded later, it merely won't be used. As far as
reprobing goes, it seems reasonable for the user to remove/reload the
main module after fixing the backend module availability if it matters
to them. Or maybe we could just EPROBE_DEFER instead of falling back to
baseline if we know the module is enabled and *should* be available; I
don't have a particular preference either way. The main thing is just to
have the backend modules work in a simple, intuitive, and mostly
automatic manner, without all the complexity of effectively
reimplementing a whole custom driver model in between perf and the real
driver model.
I think the only really notable thing about this approach is that it
would probably need to make sure the PMU probe runs asynchronously from
the main module_init.
Thanks,
Robin.
