Re: [xen-unstable-smoke test] 171511: regressions - FAIL

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2022 10:17, Julien Grall wrote:
> Hi Demi,
> 
> On 06/07/2022 09:05, Demi Marie Obenour wrote:
>> On Wed, Jul 06, 2022 at 08:53:49AM +0100, Julien Grall wrote:
>>> Hi Jan,
>>>
>>> On 06/07/2022 07:44, Jan Beulich wrote:
>>>> On 06.07.2022 05:39, osstest service owner wrote:
>>>>> flight 171511 xen-unstable-smoke real [real]
>>>>> flight 171517 xen-unstable-smoke real-retest [real]
>>>>> http://logs.test-lab.xenproject.org/osstest/logs/171511/
>>>>> http://logs.test-lab.xenproject.org/osstest/logs/171517/
>>>>>
>>>>> Regressions :-(
>>>>>
>>>>> Tests which did not succeed and are blocking,
>>>>> including tests which could not be run:
>>>>>    test-arm64-arm64-xl-xsm       8 xen-boot                 fail REGR. 
>>>>> vs. 171486
>>>>
>>>> Looking at what's under test, I guess ...
>>>>
>>>>> commit 8d410ac2c178e1dd1001cadddbe9ca75a9738c95
>>>>> Author: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
>>>>> Date:   Tue Jul 5 13:10:46 2022 +0200
>>>>>
>>>>>       EFI: preserve the System Resource Table for dom0
>>>>>       The EFI System Resource Table (ESRT) is necessary for fwupd to 
>>>>> identify
>>>>>       firmware updates to install.  According to the UEFI specification 
>>>>> §23.4,
>>>>>       the ESRT shall be stored in memory of type EfiBootServicesData.  
>>>>> However,
>>>>>       memory of type EfiBootServicesData is considered general-purpose 
>>>>> memory
>>>>>       by Xen, so the ESRT needs to be moved somewhere where Xen will not
>>>>>       overwrite it.  Copy the ESRT to memory of type 
>>>>> EfiRuntimeServicesData,
>>>>>       which Xen will not reuse.  dom0 can use the ESRT if (and only if) 
>>>>> it is
>>>>>       in memory of type EfiRuntimeServicesData.
>>>>>       Earlier versions of this patch reserved the memory in which the 
>>>>> ESRT was
>>>>>       located.  This created awkward alignment problems, and required 
>>>>> either
>>>>>       splitting the E820 table or wasting memory.  It also would have 
>>>>> required
>>>>>       a new platform op for dom0 to use to indicate if the ESRT is 
>>>>> reserved.
>>>>>       By copying the ESRT into EfiRuntimeServicesData memory, the E820 
>>>>> table
>>>>>       does not need to be modified, and dom0 can just check the type of 
>>>>> the
>>>>>       memory region containing the ESRT.  The copy is only done if the 
>>>>> ESRT is
>>>>>       not already in EfiRuntimeServicesData memory, avoiding memory leaks 
>>>>> on
>>>>>       repeated kexec.
>>>>>       See 
>>>>> https://lore.kernel.org/xen-devel/20200818184018.GN1679@mail-itl/T/
>>>>>       for details.
>>>>>       Signed-off-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
>>>>>       Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>
>>>> ... this is the most likely candidate, considering in the log all we
>>>> see is:
>>>>
>>>> Xen 4.17-unstable (c/s Mon Jun 27 15:15:39 2022 +0200 
>>>> git:61ff273322-dirty) EFI loader
>>>> Jul  5 23:09:15.692859 Using configuration file 'xen.cfg'
>>>> Jul  5 23:09:15.704878 vmlinuz: 0x00000083fb1ac000-0x00000083fc880a00
>>>> Jul  5 23:09:15.704931 initrd.gz: 0x00000083f94b7000-0x00000083fb1ab6e8
>>>> Jul  5 23:09:15.836836 xenpolicy: 0x00000083f94b4000-0x00000083f94b6a5f
>>>> Jul  5 23:09:15.980866 Using bootargs from Xen configuration file.
>>>>
>>>> But I guess we'll want to wait for the bi-sector to give us a more
>>>> solid indication ...
>>>
>>> I have tested a Xen with and without this patch this morning and can EFI. I
>>> haven't looked into details yet why.
>>>
>>> Can we consider to revert it?
>>
>> I'm fine with reverting it for now, but I would like to know what the
>> bug was.  Does a Xen with this patch boot okay on x86?
> 
> I haven't tried and I don't know whether we have UEFI system on x86.
> 
>>  If so, could it
>> be temporarily turned off on ARM until the problem can be tracked down?
> 
> I am not in favor of this approach. There are no reason for this to be 
> x86-only aside there is a bug in the code.

I agree - either we keep the patch in anticipation of a soon-ish fix,
or we revert it.

Jan