On Fri, 28 Sep 2018 14:10:26 +0530 Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote: > On 9/28/2018 11:10 AM, Gerd Hoffmann wrote: > >>> + case MBOCHS_EDID_REGION_INDEX: > >>> + ext->base.argsz = sizeof(*ext); > >>> + ext->base.offset = MBOCHS_EDID_OFFSET; > >>> + ext->base.size = MBOCHS_EDID_SIZE; > >>> + ext->base.flags = (VFIO_REGION_INFO_FLAG_READ | > >>> + VFIO_REGION_INFO_FLAG_WRITE | > >>> + VFIO_REGION_INFO_FLAG_CAPS); > >> > >> Any reason to not to use _MMAP flag? > > > > There is no page backing this. Also it is not performance-critical, > > edid updates should be rare, so the extra code for mmap support doesn't > > look like it is worth it. > > > > Also for the virtual registers (especially link_state) it is probably > > useful to have the write callback of the mdev driver called to get > > notified about the change. > > > >> How would QEMU side code read this region? will it be always trapped? > > > > qemu uses read & write syscalls (well, pread & pwrite actually). > > > >> If vendor driver sets _MMAP flag, will QEMU side handle that case as well? > > > > The current test branch doesn't, it expects read+write to work. > > https://git.kraxel.org/cgit/qemu/log/?h=sirius/edid-vfio > > > > Ok. > Can you add a comment in vfio.h that this region is non-mmappable? No, region access mechanisms are self describing through the region info ioctl, it's left to the implementation to decide what to support. The region definition should not impose such a restriction. Thanks, Alex
