Re: [PATCH v2] perf: arm_cspmu: Separate Arm and vendor module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux