qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Niklas Schnelle <schnelle@linux.ibm.com>
To: Alex Williamson <alex.williamson@redhat.com>,
	Farhan Ali <alifm@linux.ibm.com>
Cc: "Stefan Hajnoczi" <stefanha@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	qemu-s390x@nongnu.org, fam@euphon.net, philmd@linaro.org,
	kwolf@redhat.com, hreitz@redhat.com, thuth@redhat.com,
	mjrosato@linux.ibm.com, "Cédric Le Goater" <clg@redhat.com>,
	venture@google.com, crauer@google.com, pefoley@google.com,
	david@redhat.com
Subject: Re: [PATCH v3 0/3] Enable QEMU NVMe userspace driver on s390x
Date: Tue, 15 Apr 2025 09:28:21 +0200	[thread overview]
Message-ID: <bc3196307590a8454dd7edc1bb26fbb4002c3df0.camel@linux.ibm.com> (raw)
In-Reply-To: <20250411162819.1526fb12.alex.williamson@redhat.com>

On Fri, 2025-04-11 at 16:28 -0600, Alex Williamson wrote:
> > > 
--- snip ---
> > > Cc: folks related to that commit.
> > > 
> > > The original issue that brought us ram_device was a very obscure
> > > alignment of a memory region versus a device quirk only seen with
> > > assignment of specific RTL NICs.
> > > 
> > > The description for commit 4a2e242bbb30 ("memory: Don't use memcpy for
> > > ram_device regions") also addresses unaligned accesses, we don't expect
> > > drivers to use them and we don't want them to work differently in a VM
> > > than they might on bare metal.  We can debate whether that's valid or
> > > not, but that was the intent.
> > > 
> > > Have we re-introduced the chance that we're using optimized
> > > instructions only meant to target RAM here or is __builtin_memcpy
> > > implicitly safe for MMIO?  Thanks,
> > > 
> > > Alex  
> > 
> > 
> > Hi Stefan, Alex
> > 
> > 
> > Polite ping. Following up to understand how we should proceed with this 
> > series. Please let me know if there are any concerns that i haven't 
> > addressed?
> 
> I disassembled the current implementation using ldn_he_p/stn_he_p on
> x86_64 and it doesn't appear to introduce any of the mmx/sse optimized
> optimized code that we were trying to get away from in introducing the
> ram_device MemoryRegion and getting away from memcpy.  I wish I had
> some assurance that __builtin_memcpy won't invoke such operations, but
> it seems unlikely that it would for the discrete, fundamental size
> operations we're asking of it.  Therefore, maybe it is advisable to use
> the ld*_he_p/st*_he_p helpers rather than open code the memory derefs.
> 
> It's unfortunate that s390x needs to specifically restrict this access
> to PCI memory, but maybe that means that PCI specific version of these
> helpers are only created for s390x and elsewhere #define'd to the
> generic ld/st helpers, which maybe means the main interface should be a
> host_pci_{ld,st}n_he_p (maybe "le" given the implementation) type
> function.  I don't know if we'd then create a ram_pci_device variant
> memory region ops for use in vfio-pci, but it should probably be coded
> with that in mind.  Thanks,
> 
> Alex
> 

Hi Alex,

Just as a clarification because it was unclear earlier in the thread.
While it is true that the PCI instructions are restricted to PCI, there
is also no other kind of MMIO on s390x. This might not really help here
though because the PCI instructions can not be used to access
main/normal memory. So it's really a distinction between normal memory
and MMIO where MMIO would, if they were available, also include things
like GPU VRAM.

Thanks,
Niklas


  parent reply	other threads:[~2025-04-15  7:29 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-01 17:22 [PATCH v3 0/3] Enable QEMU NVMe userspace driver on s390x Farhan Ali
2025-04-01 17:22 ` [PATCH v3 1/3] util: Add functions for s390x mmio read/write Farhan Ali
2025-04-01 17:22 ` [PATCH v3 2/3] include: Add a header to define host PCI MMIO functions Farhan Ali
2025-04-02 14:09   ` Stefan Hajnoczi
2025-04-01 17:22 ` [PATCH v3 3/3] block/nvme: Use host PCI MMIO API Farhan Ali
2025-04-02 15:51 ` [PATCH v3 0/3] Enable QEMU NVMe userspace driver on s390x Stefan Hajnoczi
2025-04-03  7:47   ` Niklas Schnelle
2025-04-03 15:44     ` Stefan Hajnoczi
2025-04-03 16:27       ` Alex Williamson
2025-04-03 17:33         ` Farhan Ali
2025-04-03 18:05           ` Alex Williamson
2025-04-03 20:33             ` Farhan Ali
2025-04-03 21:24               ` Alex Williamson
2025-04-10 16:07                 ` Farhan Ali
2025-04-11 22:28                   ` Alex Williamson
2025-04-11 23:28                     ` Farhan Ali
2025-04-15  7:28                     ` Niklas Schnelle [this message]
2025-04-04  7:05               ` Cédric Le Goater

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=bc3196307590a8454dd7edc1bb26fbb4002c3df0.camel@linux.ibm.com \
    --to=schnelle@linux.ibm.com \
    --cc=alex.williamson@redhat.com \
    --cc=alifm@linux.ibm.com \
    --cc=clg@redhat.com \
    --cc=crauer@google.com \
    --cc=david@redhat.com \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mjrosato@linux.ibm.com \
    --cc=pefoley@google.com \
    --cc=philmd@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    --cc=venture@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).