Re: [PATCH v2 01/17] arm: Placeholder for handling Group0/1 traps

[Julien Grall <julien.grall@arm.com>]

Hi,

On 28/03/18 04:48, Manish Jaggi wrote:
> 
> 
> On 03/28/2018 06:18 AM, Julien Grall wrote:
>>
>> Hi,
>>
>> Sorry for the formatting.
>>
>> On Wed, 28 Mar 2018, 04:18 Stefano Stabellini, <sstabellini@kernel.org 
>> <mailto:sstabellini@kernel.org>> wrote:
>>
>>     On Tue, 27 Mar 2018, Marc Zyngier wrote:
>>     > On 27/03/18 11:10, Manish Jaggi wrote:
>>     > >
>>     > >
>>     > > On 03/27/2018 03:31 PM, Marc Zyngier wrote:
>>     > >> On 27/03/18 10:07, Manish Jaggi wrote:
>>     > >>> The errata will require to emulate the GIC virtual CPU
>>     interface in Xen.
>>     > >>> Because the hypervisor will update its internal state of the
>>     vGIC, we want
>>     > >>> to avoid messing up with it. So the errata is handled
>>     separately from the
>>     > >>> rest of the hypervisor.
>>     > >>>
>>     > >>> New file vgic-v3-sr.c is added which will hold trap and
>>     emulate code
>>     > >>> for group0 / group1 registers. Workaround for cavium Errata
>>     30115
>>     > >>> needs this emulation code.
>>     > >>>
>>     > >>> vgic_v3_handle_cpuif_access would be called from
>>     do_trap_guest_sync
>>     > >>> in subsequent patches based on errata macros.
>>     > >>>
>>     > >>> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com
>>     <mailto:manish.jaggi@cavium.com>>
>>     > >>> ---
>>     > >>>   xen/arch/arm/arm64/vgic-v3-sr.c   | 60
>>     +++++++++++++++++++++++++++++++++++++++
>>     > >>>   xen/include/asm-arm/arm64/traps.h |  2 ++
>>     > >>>   2 files changed, 62 insertions(+)
>>     > >>>   create mode 100644 xen/arch/arm/arm64/vgic-v3-sr.c
>>     > >>>
>>     > >>> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c
>>     b/xen/arch/arm/arm64/vgic-v3-sr.c
>>     > >>> new file mode 100644
>>     > >>> index 0000000000..39ab1ed6ca
>>     > >>> --- /dev/null
>>     > >>> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
>>     > >>> @@ -0,0 +1,60 @@
>>     > >>> +/*
>>     > >>> + * xen/arch/arm/arm64/vgic-v3-sr.c
>>     > >>> + *
>>     > >>> + * Code to emulate group0/group1 traps for handling
>>     > >>> + * cavium erratum 30115
>>     > >>> + *
>>     > >>> + * Manish Jaggi <manish.jaggi@cavium.com
>>     <mailto:manish.jaggi@cavium.com>>
>>     > >>> + * Copyright (c) 2018 Cavium.
>>     > >> IANAL, but I don't think this copyright notice is correct.
>>     > >>
>>     > >> I wrote about 90% of this series, and the copyright for that
>>     code is
>>     > >> owned by ARM, and licensed under the GPLv2. You have the right to
>>     > >> duplicate that code and do almost whatever you want with
>>     (within the
>>     > >> limits of the GPLv2), but you still don't own the copyright.
>>     > >>
>>     > >> I suggest you get in touch with your legal department for
>>     clarification
>>     > >> on the matter.
>>     > > I will remove the copyright line, and add this
>>     > > Original Author: Marc Zyngier <>
>>     > > Ported to Xen by: Manish Jaggi <>
>>     >
>>     > You're missing the point. I don't give a damn about the
>>     authorship (I'm
>>     > not exactly proud to have written that code).
>>
>>     :-D
>>
>>     > The problem at hand is the
>>     > ARM copyright, which should be preserved (as no-one in Cavium
>>     wrote a
>>     > single line of the original code).
>>
>>     I have been asking myself similar questions for a while now when we
>>     import code from Linux. The copyright/author line at the top of
>>     the file
>>     is somewhat arbitrary as every person that touched the code has
>>     copyright over her modifications. This is why at some point I
>>     thought we
>>     had to retain the full list of Signed-off-by lines that ever
>>     touched the
>>     code in question, but then, reading the DCO terms one more time:
>>
>>      (b) The contribution is based upon previous work that, to the best
>>          of my knowledge, is covered under an appropriate open source
>>          license and I have the right under that license to submit that
>>          work with modifications, whether created in whole or in part
>>          by me, under the same open source license (unless I am
>>          permitted to submit under a different license), as indicated
>>          in the file; or
>>
>>     I checked in the Linux kernel when code was imported from the Xen
>>     hypervisor to KVM a long time ago, and only a singled Signed-off-by of
>>     the person importing the code was used.
>>
>>
>> It does not mean it was a good practice ;).
>>
>>
>>     This is how I came to the conclusion that actually we don't need to do
>>     anything special, although it would be nice as a courtesy to
>>     retain the
>>     copyright/author lines at the top of the file. It would also be
>>     nice to
>>     have the original Linux commit id in the commit message to help
>>     tracking
>>     the history.
>>
>>
>> We had a similar discussion on the spectre patches a couple of months 
>> ago. The original author is useful to know if we need to relicense 
>> some files.
>>
>> Having just the commit id from Linux would create more overhead.
>>
>> Lars suggested to create a file listing where all the work come from. 
>> For commit, I think it would be ok to just add the Signed-off in the 
>> commit and retain copyright in the code.
>>
>> Of course, it is ok add the Cavium one if code is changed. But no 
>> copyright should be dropped.
>>
> How about
> 
> +/*
> + * xen/arch/arm/arm64/vgic-v3-sr.c
> + *
> + * Code to emulate group0/group1 traps for handling
> + * cavium erratum 30115
> + *
> + * This file merges code from Linux virt/kvm/arm/hyp/vgic-v3-sr.c
> + * Copyright (C) 2012-2015 - ARM Ltd
> + * Original Author: Marc Zyngier <marc.zyngier@arm.com>
> + * Code Ported to Xen by: Manish Jaggi <manish.jaggi@cavium.com>
> ...

That would be ok for me.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel