[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Xen/ARM - Query about a data abort seen while reading GICD registers


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>
  • From: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>
  • Date: Fri, 19 Nov 2021 16:55:53 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=kernel.org smtp.mailfrom=xilinx.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=xilinx.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=lPpJDle/FwP5aRVgESenpKYXejXjigrkBQrmbDvAqSI=; b=mCuNd7e7jbqJmAMC4kn9q60t63F7bq0kp5Aga6Dqv0GYUVb2ZOMepd/46RNxh8WGfWFthRxAjnMvaV73CdSY4YGrgYTA46gXbJJD8jqySeDvWbItyiTYU0Wm9Te0ukgk+Tj7J09wzk8IVym2wafM67gkjmPK8c/JmO2Bygvj/NtMgjXtogkPrFkYd6J0qBML3fQo3SzL2ABCwlBnf30D78n/T4tFJEYP/1TGwqeXms0cHA2kF+PpaL5FiomVQRVTWXlkTRQKXAs+ClozK4I68CAARZQSNY2zkE0ss0Uc9B/uYEqMYWymjJcMNk6zvhed0HQ+WqSC0XfMsjXq0yocBg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=P7YyjZghvNMT6KgPcNStKmTGQd5fALLkpzI9zFJNyTUWYK83svohI3Vu3GrEF771aXtkOVD0R+N0pcLtKh7uD8yFts9ZqfUgzx3V/1LVbTquPK0rf5AB3EahRq2e8ZggG9fRGgP3RI+0X+FUZCdrmRL5vUsRqFbPnAleOnvcTYjd7dadYjGdHjtIpdQkoL7EQyd64nGjQqDe6WQEXsJRGcq5b6I3/lIb8jGi/M7Uy4gBFrEtl951QY5C5E50e3pJ3aQwLcLxlGdWDDFX6GKQpze/+0Ew0OjHO6NRZauKlLNMvSMg//j1O87uyU+/+Xx2fwo37Ngxt22i32f9Pl8p+A==
  • Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 19 Nov 2021 16:56:10 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Stefano/Julien/Bertrand,

Thanks a lot for your inputs.

On 18/11/2021 01:11, Stefano Stabellini wrote:
On Wed, 17 Nov 2021, Julien Grall wrote:
I will combine my answers for this thread in one single e-mail.

On 17/11/2021 16:35, Bertrand Marquis wrote:
On 17 Nov 2021, at 16:21, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>
wrote:

Hi Bertrand,

Many thanks for your response.

On 17/11/2021 15:48, Bertrand Marquis wrote:
Hi Ayan,
On 16 Nov 2021, at 16:24, Ayan Kumar Halder
<ayan.kumar.halder@xxxxxxxxxx> wrote:

Hi Bertrand,

Thanks for looking into it.

On 16/11/2021 15:36, Bertrand Marquis wrote:
Hi Ayan,
On 16 Nov 2021, at 15:27, Ayan Kumar Halder
<ayan.kumar.halder@xxxxxxxxxx> wrote:

Hi Xen/Arm experts,

I am facing a very strange issue while running a baremetal
application as a DomU guest on arm64 platform.

The baremetal app tries to read the GICD register with post
indexing as follows :-
ldr x1, =0x3001000
ldr w2, [x1], #4 <<<------ PC = 0x40000ca8
Increment on on load is not supported by the emulation layer.

That is surprising. The reason being if I try to read the GICC
register (0x3002000) with post indexing then it works fine.
When the ISV bit is not set, Xen would have to decode the instruction to
actually emulate the access and properly modify the registers values as
long as providing the “emulated” access value.

This is very interesting. Is this being done for any of the other
instructions in Xen ?

No Xen is not trying to decode any instructions.

We actually decode some instructions (see arch/arm/decode.c). This was
necessary because early revision of Cortex-A15 would not properly fill
syndrome for Thumb instructions.

decode_instruction() could be extended to handle the specific instruction if
needed.

If you look at Linux source code, this is the kind of stuff that the kernel
is delegating to user application (qemu) to do as it can be quite complex.

There is not such a decoder in Xen right now which means those kind of
accesses are not supported for emulated mmio accesses.

I am actually trying to understand where I will need to make the changes
if I have to add support for the decoder. The reason being this issue is
being faced by one of our customer application.
Besides changing the instruction to prevent post increment, is there any
other mitigation ?

Not that I know of.

Decoding the instruction is the only solution if you don't want to update the
baremetal app.


The reason being I don't see a way to instruct the compiler to not
generate the post indexing instructions.
In general, it is not safe to let the compiler decide for you how to access
the MMIO registers as it can do all sort of optimization behind your back.
That's why...


You can define io access functions instead of letting the compiler generate
the read/write functions.
Look at arch/arm64/include/asm-arm/io.h in linux for example.

... Linux provides io{read, write} helpers.

@Ayan, is the code written in assembly or C? If the latter, how is it written?

I chatted with Ayan this morning and he will try to contact the original
author and get a proper answer, but I am pretty sure that it is written
in C. What makes it worse is that I believe the issue only started to
appear recently due to updating compiler (hence new compiler
optimizations.) Which means that this issue might become more common in
the future in other environments too :-(

I believe this is the code (source [1]):

     XScuGic_WriteReg(BaseAddress, XSCUGIC_DIST_EN_OFFSET, 0UL);

which expands to:

     #define XScuGic_WriteReg(BaseAddress, RegOffset, Data) \
        (Xil_Out32(((BaseAddress) + (RegOffset)), ((u32)(Data))))

Which is:

     static INLINE void Xil_Out32(UINTPTR Addr, u32 Value)
     {
        /* write 32 bit value to specified address */
     #ifndef ENABLE_SAFETY
        volatile u32 *LocalAddr = (volatile u32 *)Addr;
        *LocalAddr = Value;
     #else
        XStl_RegUpdate(Addr, Value);
     #endif
     }

[1] 
https://github.com/Xilinx/embeddedsw/tree/master/XilinxProcessorIPLib/drivers/scugic/examples/xscugic_low_level_example.c


So it seems to be a pretty standard volatile write in a static inline
function.


That said, the ldr writeback instructions should be safe to use. It happens
that we never had OS using them before.

Yeah I agree... My two cents is that if we can find a way to decode the
instruction without a huge amount of code then it would be worth doing
it.

I have submitted a patch to decode the ldr/str post-indexing instructions.
"[RFC PATCH] Added the logic to decode 32 bit ldr/str post-indexing instructions"

Please have a look and let me know your thoughts.
- Ayan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.