Hello Peter,
Thanks for your quick response.
On 04.10.24 12:40, Peter Maydell wrote:
> On Fri, 4 Oct 2024 at 10:47, Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote:
>> I am investigating a data abort affecting the barebox bootloader built for aarch64
>> that only manifests with qemu-system-aarch64 -enable-kvm.
>>
>> The issue happens when using the post-indexed form of LDR on a MMIO address:
>>
>> ldr x0, =0x9000fe0 // MMIO address
>> ldr w1, [x0], #4 // data abort, but only with -enable-kvm
>
> Don't do this -- KVM doesn't support it. For access to MMIO,
> stick to instructions which will set the ISV bit in ESR_EL1.
>
> That is:
>
> * AArch64 loads and stores of a single general-purpose register
> (including the register specified with 0b11111, including those
> with Acquire/Release semantics, but excluding Load Exclusive
> or Store Exclusive and excluding those with writeback).
> * AArch32 instructions where the instruction:
> - Is an LDR, LDA, LDRT, LDRSH, LDRSHT, LDRH, LDAH, LDRHT,
> LDRSB, LDRSBT, LDRB, LDAB, LDRBT, STR, STL, STRT, STRH,
> STLH, STRHT, STRB, STLB, or STRBT instruction.
> - Is not performing register writeback.
> - Is not using R15 as a source or destination register.
>
> Your instruction is doing writeback. Do the address update
> as a separate instruction.
This was enlightening, thanks. I will prepare patches to implement
readl/writel in barebox in ARM assembly, like Linux does
instead of the current fallback to the generic C version
that just does volatile accesses.
> Strictly speaking this is a missing feature in KVM (in an
> ideal world it would let you do MMIO with any instruction
> that you could use on real hardware).
I assume that's because KVM doesn't want to handle interruptions
in the middle of such "composite" instructions?
> In practice it is not
> a major issue because you don't typically want to do odd
> things when you're doing MMIO, you just want to load or
> store a single data item. If you're running into this then
> your guest software is usually doing something a bit strange.
The AMBA Peripheral ID consists of 4 bytes with some padding
in-between. The barebox code to read it out looks is this:
static inline u32 amba_device_get_pid(void *base, u32 size)
{
int i;
u32 pid;
for (pid = 0, i = 0; i < 4; i++)
pid |= (readl(base + size - 0x20 + 4 * i) & 255) <<
(i * 8);
return pid;
}
static inline u32 __raw_readl(const volatile void __iomem *addr)
{
return *(const volatile u32 __force *)addr;
}
I wouldn't necessarily characterize this as strange, we just erroneously
assumed that with strongly ordered memory for MMIO regions and volatile
accesses we had our bases covered and indeed we did until the bases
shifted to include hardware-assisted virtualization. :-)
Thanks,
Ahmad
>
> thanks
> -- PMM
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
