Re: aarch64 efi boot failures with qemu 6.0+

[Guenter Roeck]

On 7/27/21 3:36 AM, Igor Mammedov wrote:
> On Tue, 27 Jul 2021 05:01:23 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Jul 26, 2021 at 10:12:38PM -0700, Guenter Roeck wrote:
> > > On 7/26/21 9:45 PM, Michael S. Tsirkin wrote:
> > > > On Mon, Jul 26, 2021 at 06:00:57PM +0200, Ard Biesheuvel wrote:
> > > > > (cc Bjorn)
> > > > >
> > > > > On Mon, 26 Jul 2021 at 11:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > > > > > On 7/26/21 12:56 AM, Guenter Roeck wrote:
> > > > > > > On 7/25/21 3:14 PM, Michael S. Tsirkin wrote:
> > > > > > > > On Sat, Jul 24, 2021 at 11:52:34AM -0700, Guenter Roeck wrote:
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > starting with qemu v6.0, some of my aarch64 efi boot tests no longer
> > > > > > > > > work. Analysis shows that PCI devices with IO ports do not instantiate
> > > > > > > > > in qemu v6.0 (or v6.1-rc0) when booting through efi. The problem affects
> > > > > > > > > (at least) ne2k_pci, tulip, dc390, and am53c974. The problem only
> > > > > > > > > affects
> > > > > > > > > aarch64, not x86/x86_64.
> > > > > > > > >
> > > > > > > > > I bisected the problem to commit 0cf8882fd0 ("acpi/gpex: Inform os to
> > > > > > > > > keep firmware resource map"). Since this commit, PCI device BAR
> > > > > > > > > allocation has changed. Taking tulip as example, the kernel reports
> > > > > > > > > the following PCI bar assignments when running qemu v5.2.
> > > > > > > > >
> > > > > > > > > [    3.921801] pci 0000:00:01.0: [1011:0019] type 00 class 0x020000
> > > > > > > > > [    3.922207] pci 0000:00:01.0: reg 0x10: [io  0x0000-0x007f]
> > > > > > > > > [    3.922505] pci 0000:00:01.0: reg 0x14: [mem 0x10000000-0x1000007f]
> > > > >
> > > > > IIUC, these lines are read back from the BARs
> > > > >    
> > > > > > > > > [    3.927111] pci 0000:00:01.0: BAR 0: assigned [io  0x1000-0x107f]
> > > > > > > > > [    3.927455] pci 0000:00:01.0: BAR 1: assigned [mem
> > > > > > > > > 0x10000000-0x1000007f]
> > > > > > > > >    
> > > > >
> > > > > ... and this is the assignment created by the kernel.
> > > > >    
> > > > > > > > > With qemu v6.0, the assignment is reported as follows.
> > > > > > > > >
> > > > > > > > > [    3.922887] pci 0000:00:01.0: [1011:0019] type 00 class 0x020000
> > > > > > > > > [    3.923278] pci 0000:00:01.0: reg 0x10: [io  0x0000-0x007f]
> > > > > > > > > [    3.923451] pci 0000:00:01.0: reg 0x14: [mem 0x10000000-0x1000007f]
> > > > > > > > >    
> > > > >
> > > > > The problem here is that Linux, for legacy reasons, does not support
> > > > > I/O ports <= 0x1000 on PCI, so the I/O assignment created by EFI is
> > > > > rejected.
> > > > >
> > > > > This might make sense on x86, where legacy I/O ports may exist, but on
> > > > > other architectures, this makes no sense.
> > > >
> > > >
> > > > Fixing Linux makes sense but OTOH EFI probably shouldn't create mappings
> > > > that trip up existing guests, right?
> > > >    
> > >
> > > I think it is difficult to draw a line. Sure, maybe EFI should not create
> > > such mappings, but then maybe qemu should not suddenly start to enforce
> > > those mappings for existing guests either.
> >
> > I would say both. But about QEMU actually I think you have a point here.
> > Re-reading the spec:
> >
> > 0: No (The operating system shall not ignore the PCI configuration that 
> > firmware has done
> > at boot time. However, the operating system is free to configure the devices in 
> > this hierarchy
> > that have not been configured by the firmware. There may be a reduced level of 
> > hot plug
> > capability support in this hierarchy due to resource constraints. This 
> > situation is the same as
> > the legacy situation where this _DSM is not provided.)
> > 1: Yes (The operating system may ignore the PCI configuration that the firmware 
> > has done
> > at boot time, and reconfigure/rebalance the resources in the hierarchy.)
> >
> >
> > I think I misread the spec previously, and understood it to mean that
> > 1 means must ignore. In fact 1 gives the most flexibility.
> > So why are we suddenly telling the guest it must not override
> > firmware?
> >
> > The commit log says
> >      The diffences could result in resource assignment failure.
> >
> > which is kind of vague ...
> >
> > Jiahui Cen, Igor, what do you think about it?
> > I'm inclined to revert 0cf8882fd06ba0aeb1e90fa6f23fce85504d7e14
> > at least for now.
> Looking at patch history, it seems consensus was that it's better to
> enforce firmware allocations.
>
> Also letting OS do as it pleases might break PCI devices that
> don't tolerate reallocation. ex: firmware initializes PCI device
> IO/BARs and then fetches ACPI tables, which get patched with
> assigned resources.

On the other side, _not_ letting the OS do as it pleases _will_ break
PCI devices with don't meet OS requirements.

That makes me curious: There has been a lot of "may", "might", and
"could" associated with commit 0cf8882fd06b. Does anyone happen to
have a specific example of a problem that was actually fixed with
this patch ?

Thanks,
Guenter

> to me returning 0 seems to be correct choice.
> In addition resource hinting also works via firmware allocations,
> if we revert the commit it might change those configs.
>
> me wonders if there is a way make enforcement per device.
>
> > > For my own testing, I simply reverted commit 0cf8882fd0 in my copy of
> > > qemu. That solves my immediate problem, giving us time to find a solution
> > > that is acceptable for everyone. After all, it doesn't look like anyone
> > > else has noticed the problem, so there is no real urgency.
> > >
> > > Thanks,
> > > Guenter
> >
> > Well it's not like we have an army of testers, I think we should
> > treat each problem report seriously ...