[edk2] Maintainers.txt: update OvmfPkg maintainership

Message ID 20170816171731.19559-1-leif.lindholm@linaro.org
State New
Headers show

Commit Message

Leif Lindholm Aug. 16, 2017, 5:17 p.m.
Add Ard Biesheuvel as a maintainer on OvmfPkg.
Change Jordan Justen from maintainer to reviewer.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

---
 Maintainers.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Kinney, Michael D Aug. 16, 2017, 5:23 p.m. | #1
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>


> -----Original Message-----

> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On

> Behalf Of Leif Lindholm

> Sent: Wednesday, August 16, 2017 10:18 AM

> To: edk2-devel@lists.01.org

> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen,

> Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek

> <lersek@redhat.com>; Andrew Fish <afish@apple.com>; Ard

> Biesheuvel <ard.biesheuvel@linaro.org>

> Subject: [edk2] [PATCH] Maintainers.txt: update OvmfPkg

> maintainership

> 

> Add Ard Biesheuvel as a maintainer on OvmfPkg.

> Change Jordan Justen from maintainer to reviewer.

> 

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Jordan Justen <jordan.l.justen@intel.com>

> Cc: Laszlo Ersek <lersek@redhat.com>

> Cc: Andrew Fish <afish@apple.com>

> Cc: Michael D Kinney <michael.d.kinney@intel.com>

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>  Maintainers.txt | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/Maintainers.txt b/Maintainers.txt

> index fd5c976056..82e864b0fd 100644

> --- a/Maintainers.txt

> +++ b/Maintainers.txt

> @@ -192,8 +192,9 @@ M: Ruiyu Ni <ruiyu.ni@intel.com>

> 

>  OvmfPkg

>  W: http://www.tianocore.org/ovmf/

> -M: Jordan Justen <jordan.l.justen@intel.com>

> +R: Jordan Justen <jordan.l.justen@intel.com>

>  M: Laszlo Ersek <lersek@redhat.com>

> +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>  S: Maintained

> 

>  PcAtChipsetPkg

> --

> 2.11.0

> 

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 16, 2017, 6:02 p.m. | #2
On 08/16/17 19:17, Leif Lindholm wrote:
> Add Ard Biesheuvel as a maintainer on OvmfPkg.

> Change Jordan Justen from maintainer to reviewer.

> 

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Jordan Justen <jordan.l.justen@intel.com>

> Cc: Laszlo Ersek <lersek@redhat.com>

> Cc: Andrew Fish <afish@apple.com>

> Cc: Michael D Kinney <michael.d.kinney@intel.com>

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>  Maintainers.txt | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/Maintainers.txt b/Maintainers.txt

> index fd5c976056..82e864b0fd 100644

> --- a/Maintainers.txt

> +++ b/Maintainers.txt

> @@ -192,8 +192,9 @@ M: Ruiyu Ni <ruiyu.ni@intel.com>

>  

>  OvmfPkg

>  W: http://www.tianocore.org/ovmf/

> -M: Jordan Justen <jordan.l.justen@intel.com>

> +R: Jordan Justen <jordan.l.justen@intel.com>

>  M: Laszlo Ersek <lersek@redhat.com>

> +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>  S: Maintained

>  

>  PcAtChipsetPkg

> 


For the record:

I disagree with the two-maintainer limit set forth in commit
d75b8ac278bc. (I recall that Leif found it strange too.) I would
strongly prefer *all three of us* to co-maintain OvmfPkg, with push
access -- "M" role -- tied specifically to OvmfPkg for *all* parties.

... I guess the two-maintainer limit is a debate for another day.

Acked-by: Laszlo Ersek <lersek@redhat.com>


Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Jordan Justen Aug. 16, 2017, 6:09 p.m. | #3
On 2017-08-16 10:17:31, Leif Lindholm wrote:
> Add Ard Biesheuvel as a maintainer on OvmfPkg.

> Change Jordan Justen from maintainer to reviewer.


I would have preferred to add Ard as a reviewer for now.

-Jordan

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Jordan Justen <jordan.l.justen@intel.com>

> Cc: Laszlo Ersek <lersek@redhat.com>

> Cc: Andrew Fish <afish@apple.com>

> Cc: Michael D Kinney <michael.d.kinney@intel.com>

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---

>  Maintainers.txt | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

> 

> diff --git a/Maintainers.txt b/Maintainers.txt

> index fd5c976056..82e864b0fd 100644

> --- a/Maintainers.txt

> +++ b/Maintainers.txt

> @@ -192,8 +192,9 @@ M: Ruiyu Ni <ruiyu.ni@intel.com>

>  

>  OvmfPkg

>  W: http://www.tianocore.org/ovmf/

> -M: Jordan Justen <jordan.l.justen@intel.com>

> +R: Jordan Justen <jordan.l.justen@intel.com>

>  M: Laszlo Ersek <lersek@redhat.com>

> +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>  S: Maintained

>  

>  PcAtChipsetPkg

> -- 

> 2.11.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 16, 2017, 6:26 p.m. | #4
On 08/16/17 20:09, Jordan Justen wrote:
> On 2017-08-16 10:17:31, Leif Lindholm wrote:

>> Add Ard Biesheuvel as a maintainer on OvmfPkg.

>> Change Jordan Justen from maintainer to reviewer.

> 

> I would have preferred to add Ard as a reviewer for now.

Both approaches work for me.

(They are both inferior to the one with three "M" roles, IMO, which is
my preference.)

Laszlo

> -Jordan

> 

>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> Cc: Jordan Justen <jordan.l.justen@intel.com>

>> Cc: Laszlo Ersek <lersek@redhat.com>

>> Cc: Andrew Fish <afish@apple.com>

>> Cc: Michael D Kinney <michael.d.kinney@intel.com>

>>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

>> ---

>>  Maintainers.txt | 3 ++-

>>  1 file changed, 2 insertions(+), 1 deletion(-)

>>

>> diff --git a/Maintainers.txt b/Maintainers.txt

>> index fd5c976056..82e864b0fd 100644

>> --- a/Maintainers.txt

>> +++ b/Maintainers.txt

>> @@ -192,8 +192,9 @@ M: Ruiyu Ni <ruiyu.ni@intel.com>

>>  

>>  OvmfPkg

>>  W: http://www.tianocore.org/ovmf/

>> -M: Jordan Justen <jordan.l.justen@intel.com>

>> +R: Jordan Justen <jordan.l.justen@intel.com>

>>  M: Laszlo Ersek <lersek@redhat.com>

>> +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>  S: Maintained

>>  

>>  PcAtChipsetPkg

>> -- 

>> 2.11.0

>>


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Aug. 16, 2017, 7:17 p.m. | #5
On 16 August 2017 at 19:02, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/16/17 19:17, Leif Lindholm wrote:

>> Add Ard Biesheuvel as a maintainer on OvmfPkg.

>> Change Jordan Justen from maintainer to reviewer.

>>

>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> Cc: Jordan Justen <jordan.l.justen@intel.com>

>> Cc: Laszlo Ersek <lersek@redhat.com>

>> Cc: Andrew Fish <afish@apple.com>

>> Cc: Michael D Kinney <michael.d.kinney@intel.com>

>>

>> Contributed-under: TianoCore Contribution Agreement 1.1

>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

>> ---

>>  Maintainers.txt | 3 ++-

>>  1 file changed, 2 insertions(+), 1 deletion(-)

>>

>> diff --git a/Maintainers.txt b/Maintainers.txt

>> index fd5c976056..82e864b0fd 100644

>> --- a/Maintainers.txt

>> +++ b/Maintainers.txt

>> @@ -192,8 +192,9 @@ M: Ruiyu Ni <ruiyu.ni@intel.com>

>>

>>  OvmfPkg

>>  W: http://www.tianocore.org/ovmf/

>> -M: Jordan Justen <jordan.l.justen@intel.com>

>> +R: Jordan Justen <jordan.l.justen@intel.com>

>>  M: Laszlo Ersek <lersek@redhat.com>

>> +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>  S: Maintained

>>

>>  PcAtChipsetPkg

>>

>

> For the record:

>

> I disagree with the two-maintainer limit set forth in commit

> d75b8ac278bc. (I recall that Leif found it strange too.) I would

> strongly prefer *all three of us* to co-maintain OvmfPkg, with push

> access -- "M" role -- tied specifically to OvmfPkg for *all* parties.

>


+1

> ... I guess the two-maintainer limit is a debate for another day.

>

> Acked-by: Laszlo Ersek <lersek@redhat.com>

>


Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Aug. 16, 2017, 7:23 p.m. | #6
On Wed, Aug 16, 2017 at 11:09:32AM -0700, Jordan Justen wrote:
> On 2017-08-16 10:17:31, Leif Lindholm wrote:

> > Add Ard Biesheuvel as a maintainer on OvmfPkg.

> > Change Jordan Justen from maintainer to reviewer.

> 

> I would have preferred to add Ard as a reviewer for now.


Apologies, I thought this what what had been discussed.

Ard has no interest in being an R on OvmfPkg - the value proposition
for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg
simplifies the task of maintaining feature parity between the two.
(It is no secret that I would love to see them as a single package,
making it easier to clean up the way EDK2-for-qemu gets packaged by
Linux distributions.)

I would certainly be OK with 3 M entries for OvmfPkg.

/
    Leif

> -Jordan

> 

> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > Cc: Jordan Justen <jordan.l.justen@intel.com>

> > Cc: Laszlo Ersek <lersek@redhat.com>

> > Cc: Andrew Fish <afish@apple.com>

> > Cc: Michael D Kinney <michael.d.kinney@intel.com>

> > 

> > Contributed-under: TianoCore Contribution Agreement 1.1

> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>

> > ---

> >  Maintainers.txt | 3 ++-

> >  1 file changed, 2 insertions(+), 1 deletion(-)

> > 

> > diff --git a/Maintainers.txt b/Maintainers.txt

> > index fd5c976056..82e864b0fd 100644

> > --- a/Maintainers.txt

> > +++ b/Maintainers.txt

> > @@ -192,8 +192,9 @@ M: Ruiyu Ni <ruiyu.ni@intel.com>

> >  

> >  OvmfPkg

> >  W: http://www.tianocore.org/ovmf/

> > -M: Jordan Justen <jordan.l.justen@intel.com>

> > +R: Jordan Justen <jordan.l.justen@intel.com>

> >  M: Laszlo Ersek <lersek@redhat.com>

> > +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >  S: Maintained

> >  

> >  PcAtChipsetPkg

> > -- 

> > 2.11.0

> > 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Jordan Justen Aug. 16, 2017, 10:37 p.m. | #7
On 2017-08-16 12:23:49, Leif Lindholm wrote:
> On Wed, Aug 16, 2017 at 11:09:32AM -0700, Jordan Justen wrote:

> > On 2017-08-16 10:17:31, Leif Lindholm wrote:

> > > Add Ard Biesheuvel as a maintainer on OvmfPkg.

> > > Change Jordan Justen from maintainer to reviewer.

> > 

> > I would have preferred to add Ard as a reviewer for now.

> 

> Apologies, I thought this what what had been discussed.

> 

> Ard has no interest in being an R on OvmfPkg


I worded the commit message in d75b8ac278 to indicate the package
maintainers 'own' how they use the package reviewers role. For my
part, I expected Laszlo and I would effectively treat Ard as a package
maintainer since we trust his work and he has push access already.

> - the value proposition

> for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg

> simplifies the task of maintaining feature parity between the two.

> (It is no secret that I would love to see them as a single package,

> making it easier to clean up the way EDK2-for-qemu gets packaged by

> Linux distributions.)


I would also prefer to have OVMF support ARM and eventually RISC-V as
well. I don't think Laszlo feels as confident about this though.

-Jordan
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 16, 2017, 11:47 p.m. | #8
On 08/17/17 00:37, Jordan Justen wrote:
> On 2017-08-16 12:23:49, Leif Lindholm wrote:


[snip]

>> - the value proposition

>> for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg

>> simplifies the task of maintaining feature parity between the two.

>> (It is no secret that I would love to see them as a single package,

>> making it easier to clean up the way EDK2-for-qemu gets packaged by

>> Linux distributions.)

> 

> I would also prefer to have OVMF support ARM and eventually RISC-V as

> well. I don't think Laszlo feels as confident about this though.


I have two concerns:

(1) Reorganizing OvmfPkg for this would take an immense amount of time
(with possible regressions).

(2) Sharing more code between modules that aren't inherently
architecture-independent (and virtualization platform-independent) is risky.

By "sharing more code", I mean extracting further library classes and
then unifying originally separate drivers. I also mean extracting common
files from separate library instances, and then unifying the lib
instances in a common directory, with multiple INF files, or with
arch-dependent sections in the one resultant INF file. Another method is
to control the same set of drivers / library instances differently, via
dynamic PCDs.

While all this is great for code de-duplication, the chance of
regressions skyrockets if the code de-dup is not matched by a similar
overlap in maintenance (that is, review and testing).

Personally I use QEMU/KVM (and occasionally QEMU/TCG) on x86 and
aarch64. I don't use 32-bit ARM (even guests, on aarch64 hosts), or any
kind of Xen. I've never seen RISC-V hardware (and probably won't --
nested TCG with QEMU doesn't count).

The best counter-indication for this kind of increased sharing is the
*numerous* Xen-related regressions that have slipped through in the
past, simply because none of the OvmfPkg maintainers use Xen. (And the
Xen project seems to be unwilling or unable to delegate an official
reviewer or co-maintainer for the Xen-related code in OvmfPkg, despite
my repeated requests.) This has happened under ArmVirtPkg too (I recall
ACPI vs. DT related changes -- surprisingly, even *that* selection is
specific to the virtualization platform.)

The bottleneck in open source development is not writing code, it is
reviewing and regression-testing code. (This is painfully obvious in
Linux kernel and QEMU development, but the same can be experienced on
edk2-devel as well.) Therefore OvmfPkg's structure should match the
distribution of OvmfPkg's active stake-holders over architectures and
virtualization platforms.

IMO the current code sharing between OvmfPkg and ArmVirtPkg, while
certainly not 100% polished, is workable -- meaning that it mostly
corresponds to the stakes that ArmVirtPkg and OvmfPkg maintainers and
long-term contributors hold in the shared modules.

In fact, these stakes would be much better reflected if Ard *too* were a
Maintainer for OvmfPkg.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Aug. 17, 2017, 10:12 a.m. | #9
On Thu, Aug 17, 2017 at 01:47:59AM +0200, Laszlo Ersek wrote:
> On 08/17/17 00:37, Jordan Justen wrote:

> > On 2017-08-16 12:23:49, Leif Lindholm wrote:

> 

> [snip]

> 

> >> - the value proposition

> >> for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg

> >> simplifies the task of maintaining feature parity between the two.

> >> (It is no secret that I would love to see them as a single package,

> >> making it easier to clean up the way EDK2-for-qemu gets packaged by

> >> Linux distributions.)

> > 

> > I would also prefer to have OVMF support ARM and eventually RISC-V as

> > well. I don't think Laszlo feels as confident about this though.

> 

> I have two concerns:

> 

> (1) Reorganizing OvmfPkg for this would take an immense amount of time

> (with possible regressions).


Well, I'm the one who keeps pushing for it, so it'd be impolite of me
to suggest that someone else should have to deal with it.

> (2) Sharing more code between modules that aren't inherently

> architecture-independent (and virtualization platform-independent) is risky.


Let me start by clarifying what I would like to see:
- ArmVirtPkg and OvmfPkg merged into one package.
- Merging a lot of common items into shared .dsc.inc and .fdf.inc
  across all platforms.
- ARM/AARCH64 platforms added to build.sh/create-release.py.
- End.

> By "sharing more code", I mean extracting further library classes and

> then unifying originally separate drivers. I also mean extracting common

> files from separate library instances, and then unifying the lib

> instances in a common directory, with multiple INF files, or with

> arch-dependent sections in the one resultant INF file. Another method is

> to control the same set of drivers / library instances differently, via

> dynamic PCDs.

> 

> While all this is great for code de-duplication, the chance of

> regressions skyrockets if the code de-dup is not matched by a similar

> overlap in maintenance (that is, review and testing).


All of the above, I consider a lot less important to deal with short
(or even medium) term. Where and when things make sense to break out,
I'm sure you will do so without prompting. What I want to do is remove
the current structural barrier we have.

> Personally I use QEMU/KVM (and occasionally QEMU/TCG) on x86 and

> aarch64. I don't use 32-bit ARM (even guests, on aarch64 hosts), or any

> kind of Xen. I've never seen RISC-V hardware (and probably won't --

> nested TCG with QEMU doesn't count).


Let's be honest here. Yes, there is a risk of 32-bit ARM and RISC-V
ending up poorly tested. That is also much less of an issue than ARM64
and X64. If that changes, absolutely, validation for those needs to be
seriously improved.

But frankly, at this stage, _not_ setting up some CI jobs running
LuvOS on all QEMU/TCG targets on a nightly basis is just down to
laziness. (That finger is pointing very strongly towards myself.)
QEMU/KVM would require a little bit more effort, but not tons.

> The best counter-indication for this kind of increased sharing is the

> *numerous* Xen-related regressions that have slipped through in the

> past, simply because none of the OvmfPkg maintainers use Xen. (And the

> Xen project seems to be unwilling or unable to delegate an official

> reviewer or co-maintainer for the Xen-related code in OvmfPkg, despite

> my repeated requests.) This has happened under ArmVirtPkg too (I recall

> ACPI vs. DT related changes -- surprisingly, even *that* selection is

> specific to the virtualization platform.)


Sure, but the Xen situation is completely different, since (as you
say) there is no co-maintainer looking after that.

This also means it would be completely valid to break out the Xen
support into a separate package with S: Orphan.

> The bottleneck in open source development is not writing code, it is

> reviewing and regression-testing code. (This is painfully obvious in

> Linux kernel and QEMU development, but the same can be experienced on

> edk2-devel as well.) Therefore OvmfPkg's structure should match the

> distribution of OvmfPkg's active stake-holders over architectures and

> virtualization platforms.

> 

> IMO the current code sharing between OvmfPkg and ArmVirtPkg, while

> certainly not 100% polished, is workable -- meaning that it mostly

> corresponds to the stakes that ArmVirtPkg and OvmfPkg maintainers and

> long-term contributors hold in the shared modules.


Sure. And I am not suggesting that the _code_ sharing needs to change
at this point. I just feel there is more alignment between
ArmVirtPkg/Qemu and OvmfPkg/Qemu than there is between OvmfPkg/Qemu
and OvmfPkg/Xen (or indeed ArmVirtPkg/Qemu and ArmVirtPkg/Xen).

> In fact, these stakes would be much better reflected if Ard *too* were a

> Maintainer for OvmfPkg.


I'm glad we agree :)

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 17, 2017, 11:57 a.m. | #10
On 08/17/17 12:12, Leif Lindholm wrote:
> On Thu, Aug 17, 2017 at 01:47:59AM +0200, Laszlo Ersek wrote:

>> On 08/17/17 00:37, Jordan Justen wrote:

>>> On 2017-08-16 12:23:49, Leif Lindholm wrote:

>>

>> [snip]

>>

>>>> - the value proposition

>>>> for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg

>>>> simplifies the task of maintaining feature parity between the two.

>>>> (It is no secret that I would love to see them as a single package,

>>>> making it easier to clean up the way EDK2-for-qemu gets packaged by

>>>> Linux distributions.)

>>>

>>> I would also prefer to have OVMF support ARM and eventually RISC-V as

>>> well. I don't think Laszlo feels as confident about this though.

>>

>> I have two concerns:

>>

>> (1) Reorganizing OvmfPkg for this would take an immense amount of time

>> (with possible regressions).

> 

> Well, I'm the one who keeps pushing for it, so it'd be impolite of me

> to suggest that someone else should have to deal with it.

> 

>> (2) Sharing more code between modules that aren't inherently

>> architecture-independent (and virtualization platform-independent) is risky.

> 

> Let me start by clarifying what I would like to see:

> - ArmVirtPkg and OvmfPkg merged into one package.

> - Merging a lot of common items into shared .dsc.inc and .fdf.inc

>   across all platforms.

> - ARM/AARCH64 platforms added to build.sh/create-release.py.

> - End.


OK, this sounds a lot more palatable to me than unifying actual code.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Konrad Rzeszutek Wilk Aug. 23, 2017, 1:30 a.m. | #11
On Thu, Aug 17, 2017 at 01:47:59AM +0200, Laszlo Ersek wrote:
> On 08/17/17 00:37, Jordan Justen wrote:

> > On 2017-08-16 12:23:49, Leif Lindholm wrote:

> 

> [snip]

> 

> >> - the value proposition

> >> for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg

> >> simplifies the task of maintaining feature parity between the two.

> >> (It is no secret that I would love to see them as a single package,

> >> making it easier to clean up the way EDK2-for-qemu gets packaged by

> >> Linux distributions.)

> > 

> > I would also prefer to have OVMF support ARM and eventually RISC-V as

> > well. I don't think Laszlo feels as confident about this though.

> 

> I have two concerns:

> 

> (1) Reorganizing OvmfPkg for this would take an immense amount of time

> (with possible regressions).

> 

> (2) Sharing more code between modules that aren't inherently

> architecture-independent (and virtualization platform-independent) is risky.

> 

> By "sharing more code", I mean extracting further library classes and

> then unifying originally separate drivers. I also mean extracting common

> files from separate library instances, and then unifying the lib

> instances in a common directory, with multiple INF files, or with

> arch-dependent sections in the one resultant INF file. Another method is

> to control the same set of drivers / library instances differently, via

> dynamic PCDs.

> 

> While all this is great for code de-duplication, the chance of

> regressions skyrockets if the code de-dup is not matched by a similar

> overlap in maintenance (that is, review and testing).

> 

> Personally I use QEMU/KVM (and occasionally QEMU/TCG) on x86 and

> aarch64. I don't use 32-bit ARM (even guests, on aarch64 hosts), or any

> kind of Xen. I've never seen RISC-V hardware (and probably won't --

> nested TCG with QEMU doesn't count).

> 

> The best counter-indication for this kind of increased sharing is the

> *numerous* Xen-related regressions that have slipped through in the

> past, simply because none of the OvmfPkg maintainers use Xen. (And the

> Xen project seems to be unwilling or unable to delegate an official

> reviewer or co-maintainer for the Xen-related code in OvmfPkg, despite

> my repeated requests.) This has happened under ArmVirtPkg too (I recall


Who did you email/speak to? I hadn't seen any emails sent by
you to xen-devel mailing list, but perhaps I missed them?

It should be fairly simple to expand the 0-day OSSTest to build
TianoCore and launch guests with it as a nice regression test.

> ACPI vs. DT related changes -- surprisingly, even *that* selection is

> specific to the virtualization platform.)

> 

> The bottleneck in open source development is not writing code, it is

> reviewing and regression-testing code. (This is painfully obvious in

> Linux kernel and QEMU development, but the same can be experienced on

> edk2-devel as well.) Therefore OvmfPkg's structure should match the

> distribution of OvmfPkg's active stake-holders over architectures and

> virtualization platforms.

> 

> IMO the current code sharing between OvmfPkg and ArmVirtPkg, while

> certainly not 100% polished, is workable -- meaning that it mostly

> corresponds to the stakes that ArmVirtPkg and OvmfPkg maintainers and

> long-term contributors hold in the shared modules.

> 

> In fact, these stakes would be much better reflected if Ard *too* were a

> Maintainer for OvmfPkg.

> 

> Thanks

> Laszlo

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 23, 2017, 9:04 a.m. | #12
Hello Konrad,

On 08/23/17 03:30, Konrad Rzeszutek Wilk wrote:
> On Thu, Aug 17, 2017 at 01:47:59AM +0200, Laszlo Ersek wrote:

>> On 08/17/17 00:37, Jordan Justen wrote:

>>> On 2017-08-16 12:23:49, Leif Lindholm wrote:

>>

>> [snip]

>>

>>>> - the value proposition

>>>> for Linaro is that having maintainer parity ArmVirtPkg/OvmfPkg

>>>> simplifies the task of maintaining feature parity between the two.

>>>> (It is no secret that I would love to see them as a single package,

>>>> making it easier to clean up the way EDK2-for-qemu gets packaged by

>>>> Linux distributions.)

>>>

>>> I would also prefer to have OVMF support ARM and eventually RISC-V as

>>> well. I don't think Laszlo feels as confident about this though.

>>

>> I have two concerns:

>>

>> (1) Reorganizing OvmfPkg for this would take an immense amount of time

>> (with possible regressions).

>>

>> (2) Sharing more code between modules that aren't inherently

>> architecture-independent (and virtualization platform-independent) is risky.

>>

>> By "sharing more code", I mean extracting further library classes and

>> then unifying originally separate drivers. I also mean extracting common

>> files from separate library instances, and then unifying the lib

>> instances in a common directory, with multiple INF files, or with

>> arch-dependent sections in the one resultant INF file. Another method is

>> to control the same set of drivers / library instances differently, via

>> dynamic PCDs.

>>

>> While all this is great for code de-duplication, the chance of

>> regressions skyrockets if the code de-dup is not matched by a similar

>> overlap in maintenance (that is, review and testing).

>>

>> Personally I use QEMU/KVM (and occasionally QEMU/TCG) on x86 and

>> aarch64. I don't use 32-bit ARM (even guests, on aarch64 hosts), or any

>> kind of Xen. I've never seen RISC-V hardware (and probably won't --

>> nested TCG with QEMU doesn't count).

>>

>> The best counter-indication for this kind of increased sharing is the

>> *numerous* Xen-related regressions that have slipped through in the

>> past, simply because none of the OvmfPkg maintainers use Xen. (And the

>> Xen project seems to be unwilling or unable to delegate an official

>> reviewer or co-maintainer for the Xen-related code in OvmfPkg, despite

>> my repeated requests.) This has happened under ArmVirtPkg too (I recall

> 

> Who did you email/speak to? I hadn't seen any emails sent by

> you to xen-devel mailing list, but perhaps I missed them?


These emails are not easy to find (even in my own mailbox) because my
calls for help / suggestions for co-maintenance have been scattered over
time, loosely tied to OVMF regressions on Xen, or new Xen features in OVMF.

Keyword searches didn't help much, but I managed to find this email, for
example:

http://mid.mail-archive.com/f5e03398-33ca-c90d-743f-691d927657d3@redhat.com

Anthony, Gary, and xen-devel were addressed (among others). On 09/08/16
12:24, I wrote:

> Now, if you create a new platform (DSC + FDF) for Xen, that sort of

> forces someone from the Xen community to assume co-maintainership for

> the Xen bits. (Hopefully those bits would be easily identifiable by

> pathname.) I'd welcome that *very much*.


I remember more (for example I distinctly remember inviting Gary), but I
can't locate that message now.

On 08/23/17 03:30, Konrad Rzeszutek Wilk wrote:
> It should be fairly simple to expand the 0-day OSSTest to build

> TianoCore and launch guests with it as a nice regression test.


The point is to catch regressions before they are merged. This requires
someone who uses Xen every day to review and/or test patches posted to
edk2-devel that affect Xen code in OVMF.

(If the OSSTest tool can identify and pick such patches from edk2-devel
automatically, that would work too, of course.)

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Wei Liu Aug. 23, 2017, 9:17 a.m. | #13
On Wed, Aug 23, 2017 at 11:04:06AM +0200, Laszlo Ersek wrote:
> On 08/23/17 03:30, Konrad Rzeszutek Wilk wrote:

> > It should be fairly simple to expand the 0-day OSSTest to build

> > TianoCore and launch guests with it as a nice regression test.

> 

> The point is to catch regressions before they are merged. This requires

> someone who uses Xen every day to review and/or test patches posted to

> edk2-devel that affect Xen code in OVMF.

> 

> (If the OSSTest tool can identify and pick such patches from edk2-devel

> automatically, that would work too, of course.)

> 


We have been testing OVMF in osstest since two or three years ago,
albeit the test cases are limited to booting and installing a guest.

The regression is going to be caught after patches are merged though
because we're pulling from the official OVMF tree.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/Maintainers.txt b/Maintainers.txt
index fd5c976056..82e864b0fd 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -192,8 +192,9 @@  M: Ruiyu Ni <ruiyu.ni@intel.com>
 
 OvmfPkg
 W: http://www.tianocore.org/ovmf/
-M: Jordan Justen <jordan.l.justen@intel.com>
+R: Jordan Justen <jordan.l.justen@intel.com>
 M: Laszlo Ersek <lersek@redhat.com>
+M: Ard Biesheuvel <ard.biesheuvel@linaro.org>
 S: Maintained
 
 PcAtChipsetPkg