[PATCH] xen/arm: Fix arm32 build failure when early printk is enabled

Michal Orzel posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20240228103555.172101-1-michal.orzel@amd.com
xen/arch/arm/include/asm/early_printk.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Michal Orzel 2 months ago
Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
failure on arm32, when early printk is enabled:
arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'

Fixes: 0441c3acc7e9 ("xen/arm: fixmap: Rename the fixmap slots to follow the x86 convention")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/include/asm/early_printk.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/early_printk.h b/xen/arch/arm/include/asm/early_printk.h
index f444e89a8638..46a5e562dd83 100644
--- a/xen/arch/arm/include/asm/early_printk.h
+++ b/xen/arch/arm/include/asm/early_printk.h
@@ -20,7 +20,7 @@
     (FIXMAP_ADDR(FIX_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
 
 #define TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS \
-    (TEMPORARY_FIXMAP_ADDR(FIXMAP_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
+    (TEMPORARY_FIXMAP_ADDR(FIX_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
 
 #endif /* !CONFIG_EARLY_PRINTK */
 
-- 
2.25.1
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Julien Grall 2 months ago
Hi Michal,

On 28/02/2024 10:35, Michal Orzel wrote:
> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
> failure on arm32, when early printk is enabled:
> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'

Good catch! Somewhat related I wonder whether we should add earlyprintk 
testing in gitlab?

> 
> Fixes: 0441c3acc7e9 ("xen/arm: fixmap: Rename the fixmap slots to follow the x86 convention")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Michal Orzel 2 months ago
Hi Julien,

On 28/02/2024 12:42, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 28/02/2024 10:35, Michal Orzel wrote:
>> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
>> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
>> failure on arm32, when early printk is enabled:
>> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'
> 
> Good catch! Somewhat related I wonder whether we should add earlyprintk
> testing in gitlab?
I thought about adding this and I think we should at least have build jobs (hypervisor only, no toolstack)
selecting early printk. When it comes to testing if early printk works, I'm not sure. It'd be nice
but FWIR we have limited bandwidth.

@Stefano, what's your opinion?

~Michal
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Stefano Stabellini 1 month, 4 weeks ago
On Wed, 28 Feb 2024, Michal Orzel wrote:
> Hi Julien,
> 
> On 28/02/2024 12:42, Julien Grall wrote:
> > 
> > 
> > Hi Michal,
> > 
> > On 28/02/2024 10:35, Michal Orzel wrote:
> >> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
> >> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
> >> failure on arm32, when early printk is enabled:
> >> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'
> > 
> > Good catch! Somewhat related I wonder whether we should add earlyprintk
> > testing in gitlab?
> I thought about adding this and I think we should at least have build jobs (hypervisor only, no toolstack)
> selecting early printk. When it comes to testing if early printk works, I'm not sure. It'd be nice
> but FWIR we have limited bandwidth.
> 
> @Stefano, what's your opinion?

I think it would be a good and quick test to have. To save testing
bandwidth I think we should reduce the amount of debug/non-debug
variations of the same tests that we have.
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Michal Orzel 1 month, 4 weeks ago

On 28/02/2024 23:27, Stefano Stabellini wrote:
> 
> 
> On Wed, 28 Feb 2024, Michal Orzel wrote:
>> Hi Julien,
>>
>> On 28/02/2024 12:42, Julien Grall wrote:
>>>
>>>
>>> Hi Michal,
>>>
>>> On 28/02/2024 10:35, Michal Orzel wrote:
>>>> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
>>>> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
>>>> failure on arm32, when early printk is enabled:
>>>> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'
>>>
>>> Good catch! Somewhat related I wonder whether we should add earlyprintk
>>> testing in gitlab?
>> I thought about adding this and I think we should at least have build jobs (hypervisor only, no toolstack)
>> selecting early printk. When it comes to testing if early printk works, I'm not sure. It'd be nice
>> but FWIR we have limited bandwidth.
>>
>> @Stefano, what's your opinion?
> 
> I think it would be a good and quick test to have. To save testing
> bandwidth I think we should reduce the amount of debug/non-debug
> variations of the same tests that we have.
Yes, I suggested that some time ago. We could keep both versions for generic tests,
but remove the non-debug version (unless you prefer to do the opposite) for:
Arm64:
- staticmem
- staticheap
- shared-mem
- boot-cpupools
Arm32:
- gzip
- without-dom0

This would save us some bandwidth that we could use for testing other features (e.g. early printk).

~Michal
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Julien Grall 1 month, 4 weeks ago
Hi,

On 29/02/2024 10:07, Michal Orzel wrote:
> 
> 
> On 28/02/2024 23:27, Stefano Stabellini wrote:
>>
>>
>> On Wed, 28 Feb 2024, Michal Orzel wrote:
>>> Hi Julien,
>>>
>>> On 28/02/2024 12:42, Julien Grall wrote:
>>>>
>>>>
>>>> Hi Michal,
>>>>
>>>> On 28/02/2024 10:35, Michal Orzel wrote:
>>>>> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
>>>>> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
>>>>> failure on arm32, when early printk is enabled:
>>>>> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'
>>>>
>>>> Good catch! Somewhat related I wonder whether we should add earlyprintk
>>>> testing in gitlab?
>>> I thought about adding this and I think we should at least have build jobs (hypervisor only, no toolstack)
>>> selecting early printk. When it comes to testing if early printk works, I'm not sure. It'd be nice
>>> but FWIR we have limited bandwidth.
>>>
>>> @Stefano, what's your opinion?
>>
>> I think it would be a good and quick test to have. To save testing
>> bandwidth I think we should reduce the amount of debug/non-debug
>> variations of the same tests that we have.
> Yes, I suggested that some time ago. We could keep both versions for generic tests,
> but remove the non-debug version (unless you prefer to do the opposite) for:

I think it makes sense during development window to use the debug 
version. However, I think we want some non-debug testing during the 
hardening phase.

Can gitlab read CONFIG_DEBUG from Config.mk?

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Michal Orzel 1 month, 4 weeks ago

On 29/02/2024 11:10, Julien Grall wrote:
> 
> 
> Hi,
> 
> On 29/02/2024 10:07, Michal Orzel wrote:
>>
>>
>> On 28/02/2024 23:27, Stefano Stabellini wrote:
>>>
>>>
>>> On Wed, 28 Feb 2024, Michal Orzel wrote:
>>>> Hi Julien,
>>>>
>>>> On 28/02/2024 12:42, Julien Grall wrote:
>>>>>
>>>>>
>>>>> Hi Michal,
>>>>>
>>>>> On 28/02/2024 10:35, Michal Orzel wrote:
>>>>>> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
>>>>>> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
>>>>>> failure on arm32, when early printk is enabled:
>>>>>> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'
>>>>>
>>>>> Good catch! Somewhat related I wonder whether we should add earlyprintk
>>>>> testing in gitlab?
>>>> I thought about adding this and I think we should at least have build jobs (hypervisor only, no toolstack)
>>>> selecting early printk. When it comes to testing if early printk works, I'm not sure. It'd be nice
>>>> but FWIR we have limited bandwidth.
>>>>
>>>> @Stefano, what's your opinion?
>>>
>>> I think it would be a good and quick test to have. To save testing
>>> bandwidth I think we should reduce the amount of debug/non-debug
>>> variations of the same tests that we have.
>> Yes, I suggested that some time ago. We could keep both versions for generic tests,
>> but remove the non-debug version (unless you prefer to do the opposite) for:
> 
> I think it makes sense during development window to use the debug
> version. However, I think we want some non-debug testing during the
> hardening phase.
> 
> Can gitlab read CONFIG_DEBUG from Config.mk?
At the moment, we have 2 types of jobs - non debug and debug (with -debug suffix).
They set "debug" variable accordingly, which is used later on to modify .config:
echo "CONFIG_DEBUG=${debug}" >> xen/.config

Without this line, Xen would be built according to default value of CONFIG_DEBUG.
That said, I don't think we want to get back to this behavior.

If we want to save some bandwidth, we should make a decision whether to keep debug or non-debug versions.
x86 has both versions for build jobs and mostly debug test jobs.

~Michal
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Stefano Stabellini 1 month, 4 weeks ago
On Thu, 29 Feb 2024, Michal Orzel wrote:
> On 29/02/2024 11:10, Julien Grall wrote:
> > 
> > 
> > Hi,
> > 
> > On 29/02/2024 10:07, Michal Orzel wrote:
> >>
> >>
> >> On 28/02/2024 23:27, Stefano Stabellini wrote:
> >>>
> >>>
> >>> On Wed, 28 Feb 2024, Michal Orzel wrote:
> >>>> Hi Julien,
> >>>>
> >>>> On 28/02/2024 12:42, Julien Grall wrote:
> >>>>>
> >>>>>
> >>>>> Hi Michal,
> >>>>>
> >>>>> On 28/02/2024 10:35, Michal Orzel wrote:
> >>>>>> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
> >>>>>> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
> >>>>>> failure on arm32, when early printk is enabled:
> >>>>>> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and *ABS* sections) for `*'
> >>>>>
> >>>>> Good catch! Somewhat related I wonder whether we should add earlyprintk
> >>>>> testing in gitlab?
> >>>> I thought about adding this and I think we should at least have build jobs (hypervisor only, no toolstack)
> >>>> selecting early printk. When it comes to testing if early printk works, I'm not sure. It'd be nice
> >>>> but FWIR we have limited bandwidth.
> >>>>
> >>>> @Stefano, what's your opinion?
> >>>
> >>> I think it would be a good and quick test to have. To save testing
> >>> bandwidth I think we should reduce the amount of debug/non-debug
> >>> variations of the same tests that we have.
> >> Yes, I suggested that some time ago. We could keep both versions for generic tests,
> >> but remove the non-debug version (unless you prefer to do the opposite) for:
> > 
> > I think it makes sense during development window to use the debug
> > version. However, I think we want some non-debug testing during the
> > hardening phase.
> > 
> > Can gitlab read CONFIG_DEBUG from Config.mk?
> At the moment, we have 2 types of jobs - non debug and debug (with -debug suffix).
> They set "debug" variable accordingly, which is used later on to modify .config:
> echo "CONFIG_DEBUG=${debug}" >> xen/.config
> 
> Without this line, Xen would be built according to default value of CONFIG_DEBUG.
> That said, I don't think we want to get back to this behavior.
> 
> If we want to save some bandwidth, we should make a decision whether to keep debug or non-debug versions.
> x86 has both versions for build jobs and mostly debug test jobs.

It is good to have some debug and non-debug jobs, but we probably don't
need both versions of every job.
Re: [PATCH] xen/arm: Fix arm32 build failure when early printk is enabled
Posted by Julien Grall 2 months ago

On 28/02/2024 11:42, Julien Grall wrote:
> Hi Michal,
> 
> On 28/02/2024 10:35, Michal Orzel wrote:
>> Commit 0441c3acc7e9 forgot to rename FIXMAP_CONSOLE to FIX_CONSOLE in
>> TEMPORARY_EARLY_UART_VIRTUAL_ADDRESS macro. This results in a build
>> failure on arm32, when early printk is enabled:
>> arch/arm/arm32/mmu/head.S:311: Error: invalid operands (*UND* and 
>> *ABS* sections) for `*'
> 
> Good catch! Somewhat related I wonder whether we should add earlyprintk 
> testing in gitlab?
> 
>>
>> Fixes: 0441c3acc7e9 ("xen/arm: fixmap: Rename the fixmap slots to 
>> follow the x86 convention")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

And committed.

Cheers,

-- 
Julien Grall