| Message ID | CAED3YRr-8ACi5FzsHy8AtijTMMS68aDW2sE1Qy5kmexkhGvETQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | make vfio and DAX cache work together | expand |
* Edge NFV (edgenfv@gmail.com) wrote: > Signed-off-by: Edge NFV <edgenfv@gmail.com> Hi, I take it that 'Edge NFV' isn't your real name; apologies if it is. It's unusual not to use a real name; I would be interested to know why you feel uncomfortable not doing. > --- > hw/vfio/common.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index ae5654fcdb..83e15bf7a3 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -668,6 +668,15 @@ static void vfio_listener_region_add(MemoryListener > *listener, > int128_get64(int128_sub(section->size, int128_one()))); > return; > } > + > + /* Do not add virtio fs cache section */ > + if (!strcmp(memory_region_name(section->mr), "virtio-fs-cache")) { So first, this is a patch that fixes something that isn't yet in qemu; the DAX mode of virtiofs. Secondly, hard coding the name like this is probably the wrong thing to do; we need a way for the cache to declare it wants to be omitted. Thirdly, shouldn't this actually be a change to vfio_listener_skip_section to add this test? Dave > + trace_vfio_listener_region_add_skip( > + section->offset_within_address_space, > + section->offset_within_address_space + > + int128_get64(int128_sub(section->size, int128_one()))); > + return; > + } > > if (unlikely((section->offset_within_address_space & > ~qemu_real_host_page_mask) != > -- > 2.25.1
On Mon, 26 Apr 2021 13:19:05 +0100 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Edge NFV (edgenfv@gmail.com) wrote: > > Signed-off-by: Edge NFV <edgenfv@gmail.com> > > Hi, > I take it that 'Edge NFV' isn't your real name; apologies if it is. > It's unusual not to use a real name; I would be interested to know > why you feel uncomfortable not doing. The documentation noted by Laurent even goes so far as to request a real name for the Sign-off. Intentionally masking your identity will only serve to raise suspicion and increase the chances that the patch is ignored. I think perhaps aside from one or two legacy contributors, all QEMU contributions are signed-off by real names these days. I also require a commit log describing the change. > > --- > > hw/vfio/common.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > > index ae5654fcdb..83e15bf7a3 100644 > > --- a/hw/vfio/common.c > > +++ b/hw/vfio/common.c > > @@ -668,6 +668,15 @@ static void vfio_listener_region_add(MemoryListener > > *listener, > > int128_get64(int128_sub(section->size, int128_one()))); > > return; > > } > > + > > + /* Do not add virtio fs cache section */ > > + if (!strcmp(memory_region_name(section->mr), "virtio-fs-cache")) { > > So first, this is a patch that fixes something that isn't yet in qemu; > the DAX mode of virtiofs. > Secondly, hard coding the name like this is probably the wrong thing to > do; we need a way for the cache to declare it wants to be omitted. > Thirdly, shouldn't this actually be a change to > vfio_listener_skip_section to add this test? Agree on all points, there needs to be justification on why this region cannot be a DMA target for the device, not simply wishing to skip it to workaround a boot failure. Thanks, Alex > > + trace_vfio_listener_region_add_skip( > > + section->offset_within_address_space, > > + section->offset_within_address_space + > > + int128_get64(int128_sub(section->size, int128_one()))); > > + return; > > + } > > > > if (unlikely((section->offset_within_address_space & > > ~qemu_real_host_page_mask) != > > -- > > 2.25.1
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index ae5654fcdb..83e15bf7a3 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -668,6 +668,15 @@ static void vfio_listener_region_add(MemoryListener *listener, int128_get64(int128_sub(section->size, int128_one()))); return; } + + /* Do not add virtio fs cache section */ + if (!strcmp(memory_region_name(section->mr), "virtio-fs-cache")) { + trace_vfio_listener_region_add_skip( + section->offset_within_address_space, + section->offset_within_address_space + + int128_get64(int128_sub(section->size, int128_one()))); + return; + } if (unlikely((section->offset_within_address_space &
Signed-off-by: Edge NFV <edgenfv@gmail.com> --- hw/vfio/common.c | 9 +++++++++ 1 file changed, 9 insertions(+) ~qemu_real_host_page_mask) !=