On Fri, 28 Sep 2018 01:27:16 +0530 Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote: > On 9/21/2018 2:00 PM, Gerd Hoffmann wrote: > > Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx> > > @@ -964,6 +1050,20 @@ static int mbochs_get_region_info(struct mdev_device *mdev, > > region_info->flags = (VFIO_REGION_INFO_FLAG_READ | > > VFIO_REGION_INFO_FLAG_WRITE); > > break; > > + 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? > How would QEMU side code read this region? will it be always trapped? > If vendor driver sets _MMAP flag, will QEMU side handle that case as well? > I think since its blob, edid could be read by QEMU using one memcpy > rather than adding multiple memcpy of 4 or 8 bytes. "Trapping" would only come into play if the region were exposed to the VM, which there's no intention to do here afaik. Also, just because it doesn't support mmap doesn't mean that QEMU necessarily needs to break down accesses into smaller words, QEMU could do: pwrite(fd, buf, edid_max_size, region_offset + edid_offset) ie. write the entire edid area with one operation. I don't think there's anything in the specification that prevents mmap now, edid_offset could be at a page alignment, edid_max_size could be PAGE_SIZE, and a sparse mmap capability could indicate that only the EDID area is mmap'able, but is it worth the code to support that? Thanks, Alex
