diff mbox

xen/arm64: Make sure we get all debug output

Message ID 1454604575-4229-1-git-send-email-dirk.behme@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dirk Behme Feb. 4, 2016, 4:49 p.m. UTC
From: Dirk Behme <dirk.behme@de.bosch.com>

Starting in the wrong ELx mode I get the following debug output:

...
- Current EL 00000004 -
- Xen must be entered in NS EL2 mode -
- Boot failed -

The output of "Please update the bootloader" is missing here.

Make sure this is output, too. With this, we get

...
- Current EL 00000004 -
- Xen must be entered in NS EL2 mode -
- Please update the bootloader -
- Boot failed -

as intended.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
 xen/arch/arm/arm64/head.S |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ian Campbell Feb. 4, 2016, 5 p.m. UTC | #1
On Thu, 2016-02-04 at 17:49 +0100, Dirk Behme wrote:
> From: Dirk Behme <dirk.behme@de.bosch.com>
> 
> Starting in the wrong ELx mode I get the following debug output:
> 
> ...
> - Current EL 00000004 -
> - Xen must be entered in NS EL2 mode -
> - Boot failed -
> 
> The output of "Please update the bootloader" is missing here.
> 
> Make sure this is output, too. With this, we get
> 
> ...
> - Current EL 00000004 -
> - Xen must be entered in NS EL2 mode -
> - Please update the bootloader -
> - Boot failed -
> 
> as intended.

Ah, this is because gas does not concatenate strings in the same way as C,
i.e. in C:
	"A" "B" "C"

becomes 'A', 'B', 'C', '\0'

while in gas it becomes 'A', '\0', 'B', '\0', 'C', '\0'

I'd like to modify the "...is missing here" above to go a bit further:

    ...is missing here, because string concatenation in gas, unlike in C,
    keeps the \0 between each individual string.

Would that be OK with you? (I can do it on commit if you are happy with it)

> 
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>  xen/arch/arm/arm64/head.S |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 9ed9a93..19fa2bb 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -309,8 +309,8 @@ common_start:
>          b.eq  el2 /* Yes */
>  
>          /* OK, we're boned. */
> -        PRINT("- Xen must be entered in NS EL2 mode -\r\n" \
> -              "- Please update the bootloader -\r\n")
> +        PRINT("- Xen must be entered in NS EL2 mode -\r\n")
> +        PRINT("- Please update the bootloader -\r\n")
>          b fail
>  
>  el2:    PRINT("- Xen starting at EL2 -\r\n")
Dirk Behme Feb. 4, 2016, 5:07 p.m. UTC | #2
On 04.02.2016 18:00, Ian Campbell wrote:
> On Thu, 2016-02-04 at 17:49 +0100, Dirk Behme wrote:
>> From: Dirk Behme <dirk.behme@de.bosch.com>
>>
>> Starting in the wrong ELx mode I get the following debug output:
>>
>> ...
>> - Current EL 00000004 -
>> - Xen must be entered in NS EL2 mode -
>> - Boot failed -
>>
>> The output of "Please update the bootloader" is missing here.
>>
>> Make sure this is output, too. With this, we get
>>
>> ...
>> - Current EL 00000004 -
>> - Xen must be entered in NS EL2 mode -
>> - Please update the bootloader -
>> - Boot failed -
>>
>> as intended.
>
> Ah, this is because gas does not concatenate strings in the same way as C,
> i.e. in C:
> 	"A" "B" "C"
>
> becomes 'A', 'B', 'C', '\0'
>
> while in gas it becomes 'A', '\0', 'B', '\0', 'C', '\0'
>
> I'd like to modify the "...is missing here" above to go a bit further:
>
>      ...is missing here, because string concatenation in gas, unlike in C,
>      keeps the \0 between each individual string.
>
> Would that be OK with you? (I can do it on commit if you are happy with it)


Sure, fine with me :)

Many thanks!

Dirk

>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>>   xen/arch/arm/arm64/head.S |    4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>> index 9ed9a93..19fa2bb 100644
>> --- a/xen/arch/arm/arm64/head.S
>> +++ b/xen/arch/arm/arm64/head.S
>> @@ -309,8 +309,8 @@ common_start:
>>           b.eq  el2 /* Yes */
>>
>>           /* OK, we're boned. */
>> -        PRINT("- Xen must be entered in NS EL2 mode -\r\n" \
>> -              "- Please update the bootloader -\r\n")
>> +        PRINT("- Xen must be entered in NS EL2 mode -\r\n")
>> +        PRINT("- Please update the bootloader -\r\n")
>>           b fail
>>
>>   el2:    PRINT("- Xen starting at EL2 -\r\n")
>
Dirk Behme Feb. 17, 2016, 4:49 p.m. UTC | #3
On 04.02.2016 18:07, Dirk Behme wrote:
> On 04.02.2016 18:00, Ian Campbell wrote:
>> On Thu, 2016-02-04 at 17:49 +0100, Dirk Behme wrote:
>>> From: Dirk Behme <dirk.behme@de.bosch.com>
>>>
>>> Starting in the wrong ELx mode I get the following debug output:
>>>
>>> ...
>>> - Current EL 00000004 -
>>> - Xen must be entered in NS EL2 mode -
>>> - Boot failed -
>>>
>>> The output of "Please update the bootloader" is missing here.
>>>
>>> Make sure this is output, too. With this, we get
>>>
>>> ...
>>> - Current EL 00000004 -
>>> - Xen must be entered in NS EL2 mode -
>>> - Please update the bootloader -
>>> - Boot failed -
>>>
>>> as intended.
>>
>> Ah, this is because gas does not concatenate strings in the same way
>> as C,
>> i.e. in C:
>>     "A" "B" "C"
>>
>> becomes 'A', 'B', 'C', '\0'
>>
>> while in gas it becomes 'A', '\0', 'B', '\0', 'C', '\0'
>>
>> I'd like to modify the "...is missing here" above to go a bit further:
>>
>>      ...is missing here, because string concatenation in gas, unlike
>> in C,
>>      keeps the \0 between each individual string.
>>
>> Would that be OK with you? (I can do it on commit if you are happy
>> with it)
>
>
> Sure, fine with me :)


Could this be applied, then?

Best regards

Dirk


>>>
>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>> ---
>>>   xen/arch/arm/arm64/head.S |    4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>>> index 9ed9a93..19fa2bb 100644
>>> --- a/xen/arch/arm/arm64/head.S
>>> +++ b/xen/arch/arm/arm64/head.S
>>> @@ -309,8 +309,8 @@ common_start:
>>>           b.eq  el2 /* Yes */
>>>
>>>           /* OK, we're boned. */
>>> -        PRINT("- Xen must be entered in NS EL2 mode -\r\n" \
>>> -              "- Please update the bootloader -\r\n")
>>> +        PRINT("- Xen must be entered in NS EL2 mode -\r\n")
>>> +        PRINT("- Please update the bootloader -\r\n")
>>>           b fail
>>>
>>>   el2:    PRINT("- Xen starting at EL2 -\r\n")
>>
>
Ian Campbell Feb. 18, 2016, 10:15 a.m. UTC | #4
On Wed, 2016-02-17 at 17:49 +0100, Dirk Behme wrote:
> Could this be applied, then?

Yes, sorry, this fell through the cracks somehow. I've applied it to
#staging now (from where automated testing will push it into #master).

I also added the same change to arm32/head.S and noted that I'd done so in
the commit log, I figured it would be better to avoid another RTT and
another chance to slip through the cracks to ask in advance, I hope that's
ok.

I think this would be worth backporting to 4.6 and earlier (as far back as
it applies). Jan or Ian -- would one of you mind doing so when the time
comes please? (FYI I have no other pending backports)

Ian.
Jan Beulich Feb. 18, 2016, 10:21 a.m. UTC | #5
>>> On 18.02.16 at 11:15, <ian.campbell@citrix.com> wrote:
> On Wed, 2016-02-17 at 17:49 +0100, Dirk Behme wrote:
>> Could this be applied, then?
> 
> Yes, sorry, this fell through the cracks somehow. I've applied it to
> #staging now (from where automated testing will push it into #master).
> 
> I also added the same change to arm32/head.S and noted that I'd done so in
> the commit log, I figured it would be better to avoid another RTT and
> another chance to slip through the cracks to ask in advance, I hope that's
> ok.
> 
> I think this would be worth backporting to 4.6 and earlier (as far back as
> it applies). Jan or Ian -- would one of you mind doing so when the time
> comes please? (FYI I have no other pending backports)

Sure.

Jan
Ian Jackson Feb. 19, 2016, 5:13 p.m. UTC | #6
Jan Beulich writes ("Re: [PATCH] xen/arm64: Make sure we get all debug output"):
> On 18.02.16 at 11:15, <ian.campbell@citrix.com> wrote:
> > I think this would be worth backporting to 4.6 and earlier (as far back as
> > it applies). Jan or Ian -- would one of you mind doing so when the time
> > comes please? (FYI I have no other pending backports)
> 
> Sure.

I have added this to my list too (but won't mind if Jan gets to it
first).

Ian.
Jan Beulich Feb. 19, 2016, 5:16 p.m. UTC | #7
>>> On 19.02.16 at 18:13, <Ian.Jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [PATCH] xen/arm64: Make sure we get all debug 
> output"):
>> On 18.02.16 at 11:15, <ian.campbell@citrix.com> wrote:
>> > I think this would be worth backporting to 4.6 and earlier (as far back as
>> > it applies). Jan or Ian -- would one of you mind doing so when the time
>> > comes please? (FYI I have no other pending backports)
>> 
>> Sure.
> 
> I have added this to my list too (but won't mind if Jan gets to it
> first).

It's already done (I had pushed it together with the XSA-154 fixup).

Jan
diff mbox

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 9ed9a93..19fa2bb 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -309,8 +309,8 @@  common_start:
         b.eq  el2 /* Yes */
 
         /* OK, we're boned. */
-        PRINT("- Xen must be entered in NS EL2 mode -\r\n" \
-              "- Please update the bootloader -\r\n")
+        PRINT("- Xen must be entered in NS EL2 mode -\r\n")
+        PRINT("- Please update the bootloader -\r\n")
         b fail
 
 el2:    PRINT("- Xen starting at EL2 -\r\n")