Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Jan Beulich]

>>> On 21.09.16 at 15:34,  wrote:
> On 09/21/2016 06:39 AM, Jan Beulich wrote:
> On 20.09.16 at 02:19,  wrote:
>>> --- a/tools/firmware/hvmloader/acpi/dsdt.asl
>>> +++ /dev/null
>> Please try to represent this as a move, not as a delete+create.
> 
> This was done by 'git mv' and patches were generated with 'git
> format-patch -M5 ...' so I am not sure how I can convince git to show it
> as a rename. Maybe increase the argument to -M to something higher?

Or perhaps with the other adjustment carried out, the delta will
become small enough.

>>> +Scope ( \_SB.PCI0 )
>>> +{
>>> +Name ( BUFA, ResourceTemplate() { IRQ(Level, ActiveLow, Shared) { 
>>> 5, 10, 11 } } )
>>> +Name ( BUFB, Buffer() { 0x23, 0x00, 0x00, 0x18, 0x79, 0 } )
>>> +CreateWordField ( BUFB, 0x01, IRQV )
>>> +Device ( LNKA ) {
>>> +Name ( _HID,  EISAID("PNP0C0F") )
>>> +Name ( _UID, 1 )
>>> +Method ( _STA, 0 ) {
>>> +If ( And(PIRA, 0x80) ) {
>>> +Return ( 0x09 )
>>> +}
>>> +Else {
>>> +Return ( 0x0B )
>>> +}
>>> +}
>>> +Method ( _PRS ) {
>>> +Return ( BUFA )
>>> +}
>>> +Method ( _DIS ) {
>>> +Or ( PIRA, 0x80, PIRA )
>>> +}
>>> +Method ( _CRS ) {
>>> +And ( PIRA, 0x0f, Local0 )
>>> +ShiftLeft ( 0x1, Local0, IRQV )
>>> +Return ( BUFB )
>>> +}
>>> +Method ( _SRS, 1 ) {
>>> +CreateWordField ( ARG0, 0x01, IRQ1 )
>>> +FindSetRightBit ( IRQ1, Local0 )
>>> +Decrement ( Local0 )
>>> +Store ( Local0, PIRA )
>>> +}
>>> +}
>>> +Device ( LNKB ) {
>>> [...]
>>> +Name(PRTP, Package()
>>> +{
>>> +Package(){0x0001, 0, \_SB.PCI0.LNKB, 0},
>>> +Package(){0x0001, 1, \_SB.PCI0.LNKC, 0},
>>> +Package(){0x0001, 2, \_SB.PCI0.LNKD, 0},
>>> +Package(){0x0001, 3, \_SB.PCI0.LNKA, 0},
>>> +Package(){0x0002, 0, \_SB.PCI0.LNKC, 0},
>>> [...]
>>> +Package(){0x001f, 3, \_SB.PCI0.LNKC, 0},
>>> +})
>>> +
>>> +Name(PRTA, Package()
>>> +{
>>> +Package(){0x0001, 0, 0, 20},
>>> +Package(){0x0001, 1, 0, 21},
>>> +Package(){0x0001, 2, 0, 22},
>>> +Package(){0x0001, 3, 0, 23},
>>> +Package(){0x0002, 0, 0, 24},
>>> [...]
>>> +Package(){0x001f, 3, 0, 18},
>>> +})
>>> +}
>>> +}
>> I realize this is the easiest route, but how the various hard coded
>> numbers got generated would be completely lost, making it
>> extremely hard to make any changes here down the road. I
>> wonder whether the Makefile / output redirection approach couldn't
>> be easily extended to create all that data via a shell script fragment,
>> if retaining the C source (moved to a separate file) is indeed not an
>> option.
>>
>> Nor would I think that would qualify here as someone having
>> produced the replacement code without having looked at the
>> original, as was suggested as a criteria before.
> 
> I can do that (but then I think it would also make sense to have it
> generate _S5 and _PIC methods as well, even though they are "static").

Sounds reasonable.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Boris Ostrovsky]

On 09/21/2016 06:39 AM, Jan Beulich wrote:
 On 20.09.16 at 02:19,  wrote:
>> --- a/tools/firmware/hvmloader/acpi/dsdt.asl
>> +++ /dev/null
> Please try to represent this as a move, not as a delete+create.

This was done by 'git mv' and patches were generated with 'git
format-patch -M5 ...' so I am not sure how I can convince git to show it
as a rename. Maybe increase the argument to -M to something higher?


>
>> +Scope ( \_SB.PCI0 )
>> +{
>> +Name ( BUFA, ResourceTemplate() { IRQ(Level, ActiveLow, Shared) { 
>> 5, 10, 11 } } )
>> +Name ( BUFB, Buffer() { 0x23, 0x00, 0x00, 0x18, 0x79, 0 } )
>> +CreateWordField ( BUFB, 0x01, IRQV )
>> +Device ( LNKA ) {
>> +Name ( _HID,  EISAID("PNP0C0F") )
>> +Name ( _UID, 1 )
>> +Method ( _STA, 0 ) {
>> +If ( And(PIRA, 0x80) ) {
>> +Return ( 0x09 )
>> +}
>> +Else {
>> +Return ( 0x0B )
>> +}
>> +}
>> +Method ( _PRS ) {
>> +Return ( BUFA )
>> +}
>> +Method ( _DIS ) {
>> +Or ( PIRA, 0x80, PIRA )
>> +}
>> +Method ( _CRS ) {
>> +And ( PIRA, 0x0f, Local0 )
>> +ShiftLeft ( 0x1, Local0, IRQV )
>> +Return ( BUFB )
>> +}
>> +Method ( _SRS, 1 ) {
>> +CreateWordField ( ARG0, 0x01, IRQ1 )
>> +FindSetRightBit ( IRQ1, Local0 )
>> +Decrement ( Local0 )
>> +Store ( Local0, PIRA )
>> +}
>> +}
>> +Device ( LNKB ) {
>> [...]
>> +Name(PRTP, Package()
>> +{
>> +Package(){0x0001, 0, \_SB.PCI0.LNKB, 0},
>> +Package(){0x0001, 1, \_SB.PCI0.LNKC, 0},
>> +Package(){0x0001, 2, \_SB.PCI0.LNKD, 0},
>> +Package(){0x0001, 3, \_SB.PCI0.LNKA, 0},
>> +Package(){0x0002, 0, \_SB.PCI0.LNKC, 0},
>> [...]
>> +Package(){0x001f, 3, \_SB.PCI0.LNKC, 0},
>> +})
>> +
>> +Name(PRTA, Package()
>> +{
>> +Package(){0x0001, 0, 0, 20},
>> +Package(){0x0001, 1, 0, 21},
>> +Package(){0x0001, 2, 0, 22},
>> +Package(){0x0001, 3, 0, 23},
>> +Package(){0x0002, 0, 0, 24},
>> [...]
>> +Package(){0x001f, 3, 0, 18},
>> +})
>> +}
>> +}
> I realize this is the easiest route, but how the various hard coded
> numbers got generated would be completely lost, making it
> extremely hard to make any changes here down the road. I
> wonder whether the Makefile / output redirection approach couldn't
> be easily extended to create all that data via a shell script fragment,
> if retaining the C source (moved to a separate file) is indeed not an
> option.
>
> Nor would I think that would qualify here as someone having
> produced the replacement code without having looked at the
> original, as was suggested as a criteria before.

I can do that (but then I think it would also make sense to have it
generate _S5 and _PIC methods as well, even though they are "static").

I think bash script would be better than C since we mostly care about
loops that generate values and not language constructs such as stmt()
and push_blk().

-boris



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Jan Beulich]

>>> On 20.09.16 at 02:19,  wrote:
> --- a/tools/firmware/hvmloader/acpi/dsdt.asl
> +++ /dev/null

Please try to represent this as a move, not as a delete+create.

> +Scope ( \_SB.PCI0 )
> +{
> +Name ( BUFA, ResourceTemplate() { IRQ(Level, ActiveLow, Shared) { 5, 
> 10, 11 } } )
> +Name ( BUFB, Buffer() { 0x23, 0x00, 0x00, 0x18, 0x79, 0 } )
> +CreateWordField ( BUFB, 0x01, IRQV )
> +Device ( LNKA ) {
> +Name ( _HID,  EISAID("PNP0C0F") )
> +Name ( _UID, 1 )
> +Method ( _STA, 0 ) {
> +If ( And(PIRA, 0x80) ) {
> +Return ( 0x09 )
> +}
> +Else {
> +Return ( 0x0B )
> +}
> +}
> +Method ( _PRS ) {
> +Return ( BUFA )
> +}
> +Method ( _DIS ) {
> +Or ( PIRA, 0x80, PIRA )
> +}
> +Method ( _CRS ) {
> +And ( PIRA, 0x0f, Local0 )
> +ShiftLeft ( 0x1, Local0, IRQV )
> +Return ( BUFB )
> +}
> +Method ( _SRS, 1 ) {
> +CreateWordField ( ARG0, 0x01, IRQ1 )
> +FindSetRightBit ( IRQ1, Local0 )
> +Decrement ( Local0 )
> +Store ( Local0, PIRA )
> +}
> +}
> +Device ( LNKB ) {
>[...]
> +Name(PRTP, Package()
> +{
> +Package(){0x0001, 0, \_SB.PCI0.LNKB, 0},
> +Package(){0x0001, 1, \_SB.PCI0.LNKC, 0},
> +Package(){0x0001, 2, \_SB.PCI0.LNKD, 0},
> +Package(){0x0001, 3, \_SB.PCI0.LNKA, 0},
> +Package(){0x0002, 0, \_SB.PCI0.LNKC, 0},
>[...]
> +Package(){0x001f, 3, \_SB.PCI0.LNKC, 0},
> +})
> +
> +Name(PRTA, Package()
> +{
> +Package(){0x0001, 0, 0, 20},
> +Package(){0x0001, 1, 0, 21},
> +Package(){0x0001, 2, 0, 22},
> +Package(){0x0001, 3, 0, 23},
> +Package(){0x0002, 0, 0, 24},
>[...]
> +Package(){0x001f, 3, 0, 18},
> +})
> +}
> +}

I realize this is the easiest route, but how the various hard coded
numbers got generated would be completely lost, making it
extremely hard to make any changes here down the road. I
wonder whether the Makefile / output redirection approach couldn't
be easily extended to create all that data via a shell script fragment,
if retaining the C source (moved to a separate file) is indeed not an
option.

Nor would I think that would qualify here as someone having
produced the replacement code without having looked at the
original, as was suggested as a criteria before.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Boris Ostrovsky]

On 09/20/2016 10:19 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [PATCH v4 02/21] acpi: Prevent GPL-only code 
> from seeping into non-GPL binaries"):
>> But yes, I can split dsdt.asl as well. Should we keep _S5 definition as
>> GPL-only?
> I think once we're going down this route there is no benefit in trying
> to argue for individual bits of code-that-was-once-Lenovo's that there
> is no copyright interest.
>
> So I would split those lines out as well.  That will mean multiple
> includes.
>
> Does iasl have a suitable conditional include syntax or are you going
> to use `cat' or something in the Makefile ?


iasl has -I option but I don't see a conditional. (And I have very
vague understanding of ASL syntax in case there is something that can be
done in the language itself).

I ran a quick test with 'cat' and it seems to work OK. Besides, we are
already using 'cat' implicitly: we append mk_dsdt output to existing
*asl file.


-boris



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Ian Jackson]

Boris Ostrovsky writes ("Re: [PATCH v4 02/21] acpi: Prevent GPL-only code from 
seeping into non-GPL binaries"):
> But yes, I can split dsdt.asl as well. Should we keep _S5 definition as
> GPL-only?

I think once we're going down this route there is no benefit in trying
to argue for individual bits of code-that-was-once-Lenovo's that there
is no copyright interest.

So I would split those lines out as well.  That will mean multiple
includes.

Does iasl have a suitable conditional include syntax or are you going
to use `cat' or something in the Makefile ?

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Boris Ostrovsky]

On 09/20/2016 06:14 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("[PATCH v4 02/21] acpi: Prevent GPL-only code from 
> seeping into non-GPL binaries"):
>> Some code (specifically, introduced by commit 801d469ad ("[HVM] ACPI
>> support patch 3 of 4: ACPI _PRT table.")) has only been licensed under
>> GPLv2. We want to prevent this code from showing up in non-GPL
>> binaries which might become possible after we make ACPI builder code
>> available to users other than hvmloader.
>>
>> There are two pieces that we need to be careful about:
>> (1) A small piece of code in dsdt.asl that implements _PIC method
>> (2) A fragment of ASL generator in mk_dsdt.c that describes PCI
>> interrupt routing.
>>
>> The cleanest way to deal with this seems to be taking generatedi ASL
>> chunk from (2), adding it to dsdt.asl and keeping dsdt.asl GPL-only.
> This approach leaves us with the whole of dsdt.asl declared to be
> GPLv2.  If you are trying to relicence it as LGPLv2.1 this is not a
> good idea.
>
> Using this approach, if at some later point we get the missing ack
> from Lenovo, we would have to redo the licence review for the whole of
> dsdt.asl.  We would also have to ask Oracle's permission to relicense
> bits of the build system etc. !
>
> At the very least we should operate separate inbound/outbound
> licensing for this one file.
>
> But as I wrote on IRC, I think it would also be best to split out
> _just the troublesome portions_ into their own files, and include them
> at build time.  That way we have file-by-file source licensing.

I must have misunderstood you then, I thought we were talking only about
excising code from mk_dsdt.c.

But yes, I can split dsdt.asl as well. Should we keep _S5 definition as
GPL-only?

Lenovo patch was

+/* Poweroff support - ties in with qemu emulation */

+Name (\_S5, Package (0x04)
+{
+0x07,
+0x07,
+0x00,
+0x00
+})


and now it looks like

/* _S3 and _S4 are in separate SSDTs */
Name (\_S5, Package (0x04)
{
0x00,  /* PM1a_CNT.SLP_TYP */
0x00,  /* PM1b_CNT.SLP_TYP */
0x00,  /* reserved */
0x00   /* reserved */
})

My opinion is that using 0x7 would have been the original contribution
as the rest is is simply codifying what ACPI spec says. And since 0x7 is
no longer used we can re-license it together with the rest of dsdt.asl.


-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Lars Kurth]

On 20/09/2016 11:14, "Ian Jackson"  wrote:

>Boris Ostrovsky writes ("[PATCH v4 02/21] acpi: Prevent GPL-only code
>from seeping into non-GPL binaries"):
>> Some code (specifically, introduced by commit 801d469ad ("[HVM] ACPI
>> support patch 3 of 4: ACPI _PRT table.")) has only been licensed under
>> GPLv2. We want to prevent this code from showing up in non-GPL
>> binaries which might become possible after we make ACPI builder code
>> available to users other than hvmloader.
>> 
>> There are two pieces that we need to be careful about:
>> (1) A small piece of code in dsdt.asl that implements _PIC method
>> (2) A fragment of ASL generator in mk_dsdt.c that describes PCI
>> interrupt routing.
>> 
>> The cleanest way to deal with this seems to be taking generatedi ASL
>> chunk from (2), adding it to dsdt.asl and keeping dsdt.asl GPL-only.
>
>This approach leaves us with the whole of dsdt.asl declared to be
>GPLv2.  If you are trying to relicence it as LGPLv2.1 this is not a
>good idea.
>
>Using this approach, if at some later point we get the missing ack
>from Lenovo, we would have to redo the licence review for the whole of
>dsdt.asl.  We would also have to ask Oracle's permission to relicense
>bits of the build system etc. !
>
>At the very least we should operate separate inbound/outbound
>licensing for this one file.

Boris was waiting for the appropriate text from me, which I sent to
this thread just before your reply came in. I proposed two parts
./COPYING and ./gpl/COPYING

>But as I wrote on IRC, I think it would also be best to split out
>_just the troublesome portions_ into their own files, and include them
>at build time.  That way we have file-by-file source licensing.

As mentioned on IRC, if that can be done, that would preferable.
Aka, we would have a LGPLv2.1 dsdt.asl, which then would include
the troublesome portions from ./gpl/ optionally when the library
is built for gpl.

Regards
Lars

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Lars Kurth]

On 20/09/2016 01:19, "Boris Ostrovsky"  wrote:

>Some code (specifically, introduced by commit 801d469ad ("[HVM] ACPI
>support patch 3 of 4: ACPI _PRT table.")) has only been licensed under
>GPLv2. We want to prevent this code from showing up in non-GPL
>binaries which might become possible after we make ACPI builder code
>available to users other than hvmloader.
>
>There are two pieces that we need to be careful about:
>(1) A small piece of code in dsdt.asl that implements _PIC method
>(2) A fragment of ASL generator in mk_dsdt.c that describes PCI
>interrupt routing.
>
>The cleanest way to deal with this seems to be taking generatedi ASL
>chunk from (2), adding it to dsdt.asl and keeping dsdt.asl GPL-only.

...

>diff --git a/tools/firmware/hvmloader/acpi/gpl/COPYING
>b/tools/firmware/hvmloader/acpi/gpl/COPYING
>new file mode 100644
>index 000..8dfdf61
>--- /dev/null
>+++ b/tools/firmware/hvmloader/acpi/gpl/COPYING
>@@ -0,0 +1,5 @@
>+Unlike files in the directory above that are licensed under GNU Lesser
>+General Public License version 2.1, files here are licensed under GNU
>+General Public License version 2.
>+
>+A copy of this license can be obtained at 

Add the following:

INBOUND LICENSE

Contributions to this directory are made under the LGPLv2.1 *only* as
described
in the COPYING file of the parent directory. As LGPLv2.1 is compatible
with the
GPLv2, the resulting file is GPLv2 when seen as a whole.

The intention of this inbound license, is to avoid having to ask
subsequent 
contributors to this directory for permission to change the license from
GPLv2
to LGPLv2.1, once we get permission from the remaining copyright holders
of 
this directory to change the license to LGPLv2.1.

Note: The only outstanding permission required to re-license this
directory to
LGPLv2.1 is from Lenovo.


Regards
Lars


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 02/21] acpi: Prevent GPL-only code from seeping into non-GPL binaries

[Ian Jackson]

Boris Ostrovsky writes ("[PATCH v4 02/21] acpi: Prevent GPL-only code from 
seeping into non-GPL binaries"):
> Some code (specifically, introduced by commit 801d469ad ("[HVM] ACPI
> support patch 3 of 4: ACPI _PRT table.")) has only been licensed under
> GPLv2. We want to prevent this code from showing up in non-GPL
> binaries which might become possible after we make ACPI builder code
> available to users other than hvmloader.
> 
> There are two pieces that we need to be careful about:
> (1) A small piece of code in dsdt.asl that implements _PIC method
> (2) A fragment of ASL generator in mk_dsdt.c that describes PCI
> interrupt routing.
> 
> The cleanest way to deal with this seems to be taking generatedi ASL
> chunk from (2), adding it to dsdt.asl and keeping dsdt.asl GPL-only.

This approach leaves us with the whole of dsdt.asl declared to be
GPLv2.  If you are trying to relicence it as LGPLv2.1 this is not a
good idea.

Using this approach, if at some later point we get the missing ack
from Lenovo, we would have to redo the licence review for the whole of
dsdt.asl.  We would also have to ask Oracle's permission to relicense
bits of the build system etc. !

At the very least we should operate separate inbound/outbound
licensing for this one file.

But as I wrote on IRC, I think it would also be best to split out
_just the troublesome portions_ into their own files, and include them
at build time.  That way we have file-by-file source licensing.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel