All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: Oleksii <oleksii.kurochko@gmail.com>, xen-devel@lists.xenproject.org
Cc: Jan Beulich <jbeulich@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Gianluca Guida <gianluca@rivosinc.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
Date: Wed, 1 Mar 2023 13:58:40 +0000	[thread overview]
Message-ID: <297fb314-7752-fdf6-3003-f5bd1396c1e3@xen.org> (raw)
In-Reply-To: <3f108af54c7d824f59a7dc1daf7d4d0c23f617ec.camel@gmail.com>

On 01/03/2023 12:31, Oleksii wrote:
> Hi Julien,

Hi,

>>>>
>>>>> On 24/02/2023 11:31, Oleksii Kurochko wrote:
>>>>>> The following changes were made:
>>>>>> * make GENERIC_BUG_FRAME mandatory for ARM
>>>>>
>>>>> I have asked in patch #1 but will ask it again because I think
>>>>> this
>>>>> should be recorded in the commit message. Can you outline why
>>>>> it is
>>>>> not
>>>>> possible to completely switch to the generic version?
>>>>
>>>> I have just tried to remove it on arm64 and it seems to work.
>>>> This
>>>> was
>>>> only tested with GCC 10 though. But given the generic version is
>>>> not
>>>> not
>>>> using the %c modifier, then I wouldn't expect any issue.
>>>>
>>>> Cheers,
>>>>
>>>
>>> I tried to switch ARM to generic implementation.
>>>
>>> Here is the patch: [1]
>>
>> This is similar to the patch I wrote to test with generic
>> implementation
>> on Arm (see my other reply).
>>
>> [...]
>>
>>> (it will be merged with patch 3 if it is OK )
>>>
>>> And looks like we can switch ARM to generic implementation as all
>>> tests
>>> passed:
>>> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396
>>
>> Thanks for checking with the gitlab CI!
>>
>>>
>>> The only issue is with yocto-arm:
>>> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396/failures
>>> But I am not sure that it is because of my patch
>>
>> This looks unrelated to me. This is happening because there is a data
>> abort before PSCI (used to reboot the platform) is properly setup. I
>> think we should consider to only print once the error rather than
>> every
>> few iterations (not a request for you).
>>
>> That said, I am a bit puzzled why this issue is only happening in the
>> Yocto test (the Debian one pass). Do you know if the test is passing
>> in
>> the normal CI?
> I checked several pipelines on the normal CI and it is fine.
>>
>> If yes, what other modifications did you do?
> It looks like the issue happens after switch ARM to generic
> implementation of bug.h:
> -
> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792379063/failures
> [ failure ]
> -
> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792381665/failures
> [ passed ]
> 
> The difference between builds is only in the commit ' check if ARM can
> be fully switched to generic implementation '.
> For second one it is reverted so it looks like we can't switch ARM to
> generic implementation now as it is required addiotional investigation.

Thanks. Looking at the error again, it looks like the data abort is 
because we are accessing an unaligned address.

 From a brief look at arch/arm/xen.lds.S, I can at least see one case of 
misalignment (not clear why it would only happen now though). Can you try:

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 3f7ebd19f3ed..fb8155bd729f 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -67,6 +67,7 @@ SECTIONS
         *(.data.rel.ro)
         *(.data.rel.ro.*)

+      . = ALIGN(4);
         __proc_info_start = .;
         *(.proc.info)
         __proc_info_end = .;

> 
> There is no other significant changes ( only the changes mentioned in
> the current mail thread ).
> 
> Could we go ahead without switching ARM to generic implementation to
> not block other RISC-V patch series?

Given this is an alignment issue (Arm32 is more sensible to this than 
the other architecture, but this is still a potential problem for the 
other archs), I would really like to understand whether this is an issue 
in the common code or in the Arm linker script.

Cheers,

-- 
Julien Grall


  reply	other threads:[~2023-03-01 13:59 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-24 11:31 [PATCH v3 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
2023-02-24 11:31 ` [PATCH v3 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
2023-02-25 16:42   ` Julien Grall
2023-02-27  9:48     ` Jan Beulich
2023-02-28 17:21     ` Oleksii
2023-02-28 18:01       ` Julien Grall
2023-02-28 20:24         ` Oleksii
2023-02-27 14:23   ` Jan Beulich
2023-02-28 10:30     ` Oleksii
2023-02-28 10:42       ` Jan Beulich
2023-02-24 11:31 ` [PATCH v3 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
2023-02-25 16:47   ` Julien Grall
2023-02-28 12:38     ` Oleksii
2023-02-28 14:04       ` Julien Grall
2023-02-28 13:07     ` Oleksii
2023-02-28 13:30       ` Jan Beulich
2023-02-28 16:00         ` Oleksii
2023-02-27 14:29   ` Jan Beulich
2023-02-28 13:14     ` Oleksii
2023-02-24 11:31 ` [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h Oleksii Kurochko
2023-02-25 16:49   ` Julien Grall
2023-02-25 17:05     ` Julien Grall
2023-02-28 17:21       ` Oleksii
2023-02-28 17:57         ` Julien Grall
2023-03-01 12:31           ` Oleksii
2023-03-01 13:58             ` Julien Grall [this message]
2023-03-01 15:16               ` Oleksii
2023-03-01 15:21                 ` Julien Grall
2023-03-01 15:28                   ` Oleksii
2023-03-01 15:58                   ` Oleksii
2023-02-28 15:09     ` Oleksii
2023-02-28 17:48       ` Julien Grall
2023-03-01  8:58         ` Oleksii
2023-03-01  9:31           ` Julien Grall
2023-03-01 12:33             ` Oleksii
2023-02-24 11:31 ` [PATCH v3 4/4] xen/x86: switch x86 to use generic implemetation " Oleksii Kurochko
2023-02-27 14:46   ` Jan Beulich
2023-02-28 16:28     ` Oleksii
2023-02-28 16:36       ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=297fb314-7752-fdf6-3003-f5bd1396c1e3@xen.org \
    --to=julien@xen.org \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=gianluca@rivosinc.com \
    --cc=jbeulich@suse.com \
    --cc=oleksii.kurochko@gmail.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.