On Wed, Jan 24, 2024 at 11:38:07AM +0000, Mark Rutland wrote: > On Tue, Jan 23, 2024 at 08:38:55PM +0000, Catalin Marinas wrote: > > > On Wed, Jan 17, 2024 at 08:36:18AM -0400, Jason Gunthorpe wrote: > > > > On Wed, Jan 17, 2024 at 12:30:00PM +0000, Mark Rutland wrote: > > > > > On Tue, Jan 16, 2024 at 02:51:21PM -0400, Jason Gunthorpe wrote: > > > > > > I'm just revising this and I'm wondering if you know why ARM64 has this: > > > > > > > > > > > > #define __raw_writeq __raw_writeq > > > > > > static __always_inline void __raw_writeq(u64 val, volatile void __iomem *addr) > > > > > > { > > > > > > asm volatile("str %x0, [%1]" : : "rZ" (val), "r" (addr)); > > > > > > } > > > > > > > > > > > > Instead of > > > > > > > > > > > > #define __raw_writeq __raw_writeq > > > > > > static __always_inline void __raw_writeq(u64 val, volatile void __iomem *addr) > > > > > > { > > > > > > asm volatile("str %x0, %1" : : "rZ" (val), "m" (*(volatile u64 *)addr)); > > > > > > } > > > > > > > > > > > > ?? Like x86 has. > > > > > > > > > > I believe this is for the same reason as doing so in all of our other IO > > > > > accessors. > > > > > > > > > > We've deliberately ensured that our IO accessors use a single base register > > > > > with no offset as this is the only form that HW can represent in ESR_ELx.ISS.SRT > > > > > when reporting a stage-2 abort, which a hypervisor may use for > > > > > emulating IO. > > > > > > > > Wow, harming bare metal performace to accommodate imperfect emulation > > > > sounds like a horrible reason :( > > > > > > Having working functionality everywhere is a very good reason. :) > > > > > > > So what happens with this patch where IO is done with STP? Are you > > > > going to tell me I can't do it because of this? > > > > > > I'm not personally going to make that judgement, but it's certainly something > > > for Catalin and Will to consider (and I've added Marc in case he has any > > > opinion). > > > > Good point, I missed this part. We definitely can't use STP in the I/O > > accessors, we'd have a big surprise when running the same code in a > > guest with emulated I/O. > > Just to be clear, that means we should drop this patch ("arm64/io: add > memcpy_toio_64") for now, right? In its current form yes, but that doesn't mean that memcpy_toio_64() cannot be reworked differently. > It would be helpful if we could explciitly say so in direct reply to that: > > https://lore.kernel.org/linux-arm-kernel/c3ae87aea7660c3d266905c19d10d8de0f9fb779.1700766072.git.leon@xxxxxxxxxx/ > > ... to avoid any confusion there. I will. > > If eight STRs without other operations interleaved give us the > > write-combining on most CPUs (with Normal NC), we should go with this > > instead of STP. > > Agreed; I've sent out a patch to allow the offset addressing at: > > https://lore.kernel.org/linux-arm-kernel/20240124111259.874975-1-mark.rutland@xxxxxxx/ > > ... and it should be possible to build atop that to use eight STRs. That's great, thanks. -- Catalin