Fail to pass suspend/resume test

The issue is caused by Implement-platform-Probe-driver-call.patch in the package.
This issue is gone after removing Implement-platform-Probe-driver-call.patch.
Have to force Xorg running as root after removing this patch.
/etc/X11/Xwrapper.config: needs_root_rights = yes

debdiff for jammy

debdiff for focal

[Regression potential]
1. Fail to login to Ubuntu desktop environment
The issue can be fixed by the config below
/etc/X11/Xwrapper.config: needs_root_rights = yes
2. Canonical QA has ran graphic related test cases without any regression issue.

The attachment "jammy.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

Sponsored for both jammy and focal.

Note to SRU members: it should be fine to not require this for hirsute and impish. The target series for these packages is focal and we only ever released the changes to the current development series, so that the next LTS had the changes - there are no use-cases right now for any series other than focal. Upgrade story should not be an actual issue.

@Robie
I uploaded new debdiff to update the referenced bug to public one.

Oh, my fault for not noticing this was a private bug, duh. Sorry about that. Let me re-sponsor.

> Have to force Xorg running as root after removing this patch.
> /etc/X11/Xwrapper.config: needs_root_rights = yes

Doesn't this regress behaviour? What are the consequences of this for users?

@Robie

In this MP, Implement-platform-Probe-driver-call.patch is dropped because it has a side effect which system cannot pass suspend/resume test.

Implement-platform-Probe-driver-call.patch was added for LP: #1921153.
This patch is the solution of LP: #1921153. It forces to use root as the user running Xorg.
When this patch is dropped, another solution (needs_root_rights = yes) is required for LP: #1921153.

xf86-video-armsoc-endlessm (1.4.1-0ubuntu8) jammy; urgency=medium

  * Bump version to change the referenced bug to public one

xf86-video-armsoc-endlessm (1.4.1-0ubuntu7) jammy; urgency=medium

  * Drop platformProbe implementation patch (LP: #1948764). This forces
    to use root as the user running Xorg.

 -- Ethan Hsieh <email address hidden> Wed, 03 Nov 2021 14:14:51 +0800

Do users typically use this package running Xorg as root already, or as non-root? After this proposed fix for resume/suspend, what proportion of users using this package are going to immediately be unable to run X before they discover and implement this workaround? How confident can we be of your answer to the previous question?

If we can be confident that users will not be affected in practice (for example all users are specialists who need this fix, and won't be affected by the other breakage), then I suppose this is OK. On the other hand, if there exist users that will have X broken until they've made a configuration change, then I have serious doubts that this is acceptable. We must not break existing users in an SRU.

You need to make the case please. If users will certainly not be affected then please explain how it is that you know this to be true. Without a full analysis and explanation, I don't think this SRU can be accepted as presented so far.

@Robie

The version of xf86-video-armsoc-endlessm in the GM image[1][2] delivered to Xilinx is 1.4.1-0ubuntu6~20.04ppa1.
This package has dropped Implement-platform-Probe-driver-call.patch.
1.4.1-0ubuntu6~20.04ppa1 is same as 1.4.1-0ubuntu8~20.04.
It didn't be upstreamed because of the tight schedule.
No user will use xf86-video-armsoc-endlessm (1.4.1-0ubuntu6~20.04).
And, the GM image already includes the following file.
/etc/X11/Xwrapper.config: needs_root_rights = yes
So, there will be no regression issue when xf86-video-armsoc-endlessm is upgraded from 1.4.1-0ubuntu6~20.04ppa1 to 1.4.1-0ubuntu8~20.04.

---
[1] https://ubuntu.com/download/xilinx
[2] https://people.canonical.com/~platform/images/xilinx/iot-zcu10x-classic-desktop-2004-x07-20210728-85.img.xz?_ga=2.119680587.272158238.1637765368-1058479259.1626138874

> No user will use xf86-video-armsoc-endlessm (1.4.1-0ubuntu6~20.04).

In *your* use case I'm sure this is true. But this package ships in Ubuntu for everyone, not just for you or your customer. Please provide a credible explanation of *why* you think there are no other users, rather than just asserting this to be true.

@Robie

The image[1] released by Canonical for Xilinx boards is the only one image which pre-loads this package. The users I mentioned are Xilinx board users. Sorry, I didn't use words precisely.

xf86-video-armsoc-endlessm was created for Xilinx boards (zcu102/104/106 and Kria) with mali GPU. Several patches in this package and the mali driver in linux-xilinx-zynqmp are from Xilinx. Some extra gdm3 and xorg config files are also required for enabling mali GPU. It's a customized package for Xilinx boards with mali GPU.

You're right. The package is public. Everyone can download it from Ubuntu-archive. But, if users want to use this package on other boards with mali GPU, they need to take many efforts to integrate and verify it on their boards. Meanwhile, there is already another package called xf86-video-armsoc[2] for other boards with mali GPU. And, this package (1.4.1-0ubuntu6~20.04) was released to focal on 06/17 2021. It seems to me that it has very low risk to backport 1.4.1-0ubuntu8 to focal.

---
[1] https://ubuntu.com/download/xilinx
[2] https://launchpad.net/ubuntu/+source/xf86-video-armsoc

It seems to me that there's no reason why a user of this package couldn't have either:
a) changed the configuration in /etc/X11/Xwrapper.config to not require root - my understanding is that with the current package that would work, or
b) Built a custom image using this package that did not contain the /etc/X11/Xwrapper.conf

If either of those are plausible, then when they pull updates and upgrade to this new package, their systems will break.

Is there any ballpark estimate for how hard it would be to fix the suspend issue *without* regressing the run-as-not-root case?

@Chris

Xilinx classic focal images[1][2] is the only one image released by Canonical. It includes xf86-video-armsoc-endlessm (1.4.1-0ubuntu6~20.04ppa1) and a meta package. (1.4.1-0ubuntu6~20.04ppa1) didn't be upstreamed because of the tight schedule. This package has dropped Implement-platform-Probe-driver-call.patch. The meta package includes the following config files:
/etc/X11/Xwrapper.config
/etc/X11/xorg.conf.d/xilinx-zynqmp.conf
/etc/udev/rules.d/50-mali-conf.rules
etc

The Xilinx image released by Canonical is for Xilinx zcu102/104/106 and this package is customized for this Xilinx image and these boards. The GPU on zcu102/104/106 is Mali-400 MP2. The config files are not part of this package because we may need different configurations on other Xilinx platforms with different Mali GPU.

The Xilinx image released by Canonical already includes Xwrapper.conf.
So, the Xilinx image's users won't see this issue when this package get updated.

---
[1] https://ubuntu.com/download/xilinx
[2] https://people.canonical.com/~platform/images/xilinx/iot-zcu10x-classic-desktop-2004-x07-20210728-85.img.xz?_ga=2.119680587.272158238.1637765368-1058479259.1626138874

@Chris @Robie

Not sure whether I didn't explain well. Let me take u-boot as an example.

There are several u-boot packages. These packages are soc-dependent.
When we modify common codes, we need to take other platforms into account and make sure there is no regression issue on other platforms.
u-boot[1]:
u-boot-amlogic A boot loader for amlogic systems
u-boot-exynos A boot loader for exynos systems
u-boot-imx A boot loader for imx systems
...
u-boot-tegra A boot loader for NVIDIA Tegra systems

u-boot-xlnx[2]:
u-boot-zynqmp u-boot bootloader for Xilinx Zynq UltraScale+ MPSoC systems

But, u-boot-xlnx is based on a different code base provided by Xilinx.
This package is for Xilinx desktop image (boards: zcu102/104/106 and Kria)

It's a similar situation to the armsoc driver.

xf86-video-armsoc[3]
xserver-xorg-video-armsoc-exynos X.Org X server -- ARM SoC display driver for Exynos DRM
xserver-xorg-video-armsoc-pl111 X.Org X server -- ARM SoC display driver for pl111 DRM

xf86-video-armsoc-endlessm[4]
xserver-xorg-video-armsoc-endlessm (this one is for Xilinx boards zcu102/104/106 and Kria)

xf86-video-armsoc-endlessm is created for the Xilinx desktop images.
It’s based on a different code base with Xilinx patches.
The mali kernel driver in the Xilinx images is provided by Xilinx, not generic one.
This armsoc xorg driver has better compatibility with this driver.
It seems to me that other platforms should not use this armsoc driver.

The Xilinx desktop images have included the patched Xwrapper.config.
There will be no regression issue when xf86-video-armsoc-endlessm gets updated to 1.4.1-0ubuntu8~20.04.

---
[1] https://launchpad.net/ubuntu/+source/u-boot
[2] https://launchpad.net/ubuntu/+source/u-boot-xlnx
[3] https://launchpad.net/ubuntu/+source/xf86-video-armsoc
[4] https://launchpad.net/ubuntu/+source/xf86-video-armsoc-endlessm

I think we're talking past each other again.

I believe you when you say that this update will not cause a regression for users of the Xilinx desktop image who have not edited the configuration in /etc/X11/Xwrapper.config

The issue we, on the SRU team, are concerned about are the two caveats there:
1) Users of the *Xilinx desktop image*, and
2) Who haven't edited the configuration.

The xf86-video-armsoc-endlessm package is in the Ubuntu archive, not just the Xilinx images, and the guarantees we try to provide for packages in the archive is that updates will not break anything that currently works.

My understanding is that, currently, a user of a Xilinx board could have built their own images from packages in the Ubuntu archive and not include the metapackage containing the /etc/X11/Xwrapper.config configuration, and this would work.

Also, a user of the Xilinx image who chose to edit /etc/X11/Xwrapper.config and remove “needs_root_rights = yes” would currently have a working system.

My understanding is that this update would result in both of the above cases failing to work. *This* is what we are concerned about.

It's possible that we need a dedicated policy for volatile hardware-enablement packages like this, both so that we can process them more quickly for you and so that users are made aware that these specific packages do not have the usual stability guarantees associated with the Ubuntu archive. If that seems like a good idea, the next step on that would probably be to start a discussion on ubuntu-devel@.

An upload of xf86-video-armsoc-endlessm to focal-proposed has been rejected from the upload queue for the following reason: "Proposed fix regresses non-root use as discussed in the bug.".