Re: [PATCH v1] drivers/perf: apple_m1: add known PMU events

From: Yangyu Chen
Date: Sun Jul 28 2024 - 08:42:25 EST




> On Jul 28, 2024, at 19:00, Hector Martin <marcan@xxxxxxxxx> wrote:
>
>
> On 2024/06/19 1:58, Marc Zyngier wrote:
>> On Tue, 18 Jun 2024 16:56:48 +0100,
>> Yangyu Chen <cyy@xxxxxxxxxxxx> wrote:
>>>
>>>
>>>
>>>> On Jun 18, 2024, at 22:03, Marc Zyngier <maz@xxxxxxxxxx> wrote:
>>>>
>>>> On Tue, 18 Jun 2024 14:49:48 +0100,
>>>> Yangyu Chen <cyy@xxxxxxxxxxxx> wrote:
>>>>>
>>>>> This patch adds known PMU events that can be found on /usr/share/kpep in
>>>>> macOS. The m1_pmu_events and m1_pmu_event_affinity are generated from
>>>>> the script [1], which consumes the plist file from Apple. And then added
>>>>> these events to m1_pmu_perf_map and m1_pmu_event_attrs with Apple's
>>>>> documentation [2].
>>>>>
>>>>> Link: https://github.com/cyyself/m1-pmu-gen [1]
>>>>> Link: https://developer.apple.com/download/apple-silicon-cpu-optimization-guide/ [2]
>>>>
>>>> This needs registration, and is thus impossible to freely visit.
>>>>
>>>>> Signed-off-by: Yangyu Chen <cyy@xxxxxxxxxxxx>
>>>>
>>>> What is the licence applicable to the original source file? Does it
>>>> explicitly allow redistribution in any form?
>>>>
>>>
>>> Oh. It's my fault. Sorry for the trouble caused.
>>
>> No trouble on my side. I'm just painfully aware that this is a legal
>> landmine, and that what is perfectly allowed in one country may be a
>> punishable offence in another. And since I'm not a lawyer, I want to
>> see crystal clear things in writing.
>>
>>>
>>>>
>>>> Other than the licensing concern, why should we bloat the kernel with
>>>> more of this stuff when everything is moving towards a bunch of JSON
>>>> files (tools/perf/pmu-events/arch/arm64).
>>>>
>>>
>>> Thanks for this hint. So, the thing to do might be to provide a
>>> generator that consumes Apple files and then generates a kernel
>>> patch for Linux perf tools to use rather than provide such details
>>> directly in the source code as you said from [1].
>>>
>>> Link: https://lore.kernel.org/lkml/87czn18zev.wl-maz@xxxxxxxxxx/ [1]
>>
>> Even better: teach the perf tool to directly consume the plist file,
>> but don't distribute the file or its content. People owning such a
>> machine can fish the file from the machine itself (or the installer
>> can extract it from the OS image as if it was firmware data).
>
> Maz,
>
> That would be a waste of time. Facts about hardware are not
> copyrightable. I see absolutely nothing objectionable in this patch. It
> doesn't matter where the information was sourced as long as it was
> legitimately available to the person (which it was, as long as they were
> running macOS on one of these machines).
>
> Let's look at the license for the ARMv8-A ARM:
>
>> Proprietary Notice
>> This document is protected by copyright and other related rights and the practice or implementation of the information contained
>> in this document may be protected by one or more patents or pending patent applications. No part of this document may be
>> reproduced in any form by any means without the express prior written permission of Arm. No license, express or implied, by
>> estoppel or otherwise to any intellectual property rights is granted by this document unless specifically stated.
>
> There is absolutely nothing in there granting a license to use the
> information in the document and things like register names in Linux or
> any other OS. And yet we can do that, because those things aren't
> copyrightable. It would defeat the entire point of the documentation if
> you could not use it, even though there is in fact no explicit copyright
> grant to allow you to use it. It is not needed.
>
> The same exact logic applies here. The macOS license does not grant us
> the right to reproduce portions of macOS, but that is completely
> irrelevant because the portion "reproduced" in the form of this patch is
> not, at all, copyrightable. If it were we would have much bigger issues
> and all kinds of code in Linux would be a copyvio. The fact that there
> was some automation involved in generating the patch contents is
> entirely irrelevant, as long as the output does not keep a copyright
> interest from the author of the input.
>
> I also have an actual lawyer's opinion that register names are not
> copyrightable, which further corroborates this interpretation.
>
> As far as I'm concerned this can be merged as is.

Even if there are no copyright concerns, as you said from [1], I
think I should remove the lines in m1_pmu_event_attrs and then patch
userspace Linux-perf tools with the definitions in the JSON file.

Since I haven't received any other advice on copyright concerns, I
am still waiting for other suggestions before submitting the patch
revision.

Thanks,
Yangyu Chen

[1] https://lore.kernel.org/lkml/dbf17fa6-1af6-467b-8b3d-dca8476dc785@xxxxxxxxx/

>
> Acked-by: Hector Martin <marcan@xxxxxxxxx>
>
> - Hector