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

[Hector Martin]

On 2024/07/28 21:19, Yangyu Chen wrote:
> 
> 
>> 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.

Sure, that's fine. I would say go ahead and move the definitions to
JSON, then just submit that.

> 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/



- Hector